lkml.org 
[lkml]   [2016]   [Aug]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH 03/11] Input: synaptics-rmi4 - have only one struct platform data
    From
    Date
    Resending as plain text

    On 08/18/2016 02:24 AM, Benjamin Tissoires wrote:
    > If struct rmi_device_platform_data contains pointers to other struct,
    > it gets difficult to allocate a fixed size struct and copy it over between
    > drivers.
    >
    > Change the pointers into a struct and change the code in rmi4 accordingly.
    >
    > Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
    >
    > ---
    >
    > this patch will conflict with Andrew's patch to switch hid-rmi
    > to use rmi4_core...
    > ---
    > drivers/input/rmi4/rmi_f11.c | 4 ++--
    > drivers/input/rmi4/rmi_f12.c | 4 ++--
    > drivers/input/rmi4/rmi_f30.c | 7 +++----
    > include/linux/rmi.h | 4 ++--
    > 4 files changed, 9 insertions(+), 10 deletions(-)
    >
    > diff --git a/drivers/input/rmi4/rmi_f11.c b/drivers/input/rmi4/rmi_f11.c
    > index 20c7134..b14a7b6 100644
    > --- a/drivers/input/rmi4/rmi_f11.c
    > +++ b/drivers/input/rmi4/rmi_f11.c
    > @@ -1063,8 +1063,8 @@ static int rmi_f11_initialize(struct rmi_function *fn)
    > rc = rmi_2d_sensor_of_probe(&fn->dev, &f11->sensor_pdata);
    > if (rc)
    > return rc;
    > - } else if (pdata->sensor_pdata) {
    > - f11->sensor_pdata = *pdata->sensor_pdata;
    > + } else {
    > + f11->sensor_pdata = pdata->sensor_pdata;
    > }
    >
    > f11->rezero_wait_ms = f11->sensor_pdata.rezero_wait;
    > diff --git a/drivers/input/rmi4/rmi_f12.c b/drivers/input/rmi4/rmi_f12.c
    > index 332c02f..a631bed 100644
    > --- a/drivers/input/rmi4/rmi_f12.c
    > +++ b/drivers/input/rmi4/rmi_f12.c
    > @@ -274,8 +274,8 @@ static int rmi_f12_probe(struct rmi_function *fn)
    > ret = rmi_2d_sensor_of_probe(&fn->dev, &f12->sensor_pdata);
    > if (ret)
    > return ret;
    > - } else if (pdata->sensor_pdata) {
    > - f12->sensor_pdata = *pdata->sensor_pdata;
    > + } else {
    > + f12->sensor_pdata = pdata->sensor_pdata;
    > }
    >
    > ret = rmi_read_register_desc(rmi_dev, query_addr,
    > diff --git a/drivers/input/rmi4/rmi_f30.c b/drivers/input/rmi4/rmi_f30.c
    > index 760aff1..7990bb0 100644
    > --- a/drivers/input/rmi4/rmi_f30.c
    > +++ b/drivers/input/rmi4/rmi_f30.c
    > @@ -192,7 +192,7 @@ static int rmi_f30_config(struct rmi_function *fn)
    > rmi_get_platform_data(fn->rmi_dev);
    > int error;
    >
    > - if (pdata->f30_data && pdata->f30_data->disable) {
    > + if (pdata && pdata->f30_data.disable) {

    My one comment is that pdata struct is embedded in the transport device
    so rmi_get_platform_data() will not return NULL. Making the check of
    pdata unnecessary.


    > drv->clear_irq_bits(fn->rmi_dev, fn->irq_mask);
    > } else {
    > /* Write Control Register values back to device */
    > @@ -362,8 +362,7 @@ static inline int rmi_f30_initialize(struct rmi_function *fn)
    > * f30->has_mech_mouse_btns, but I am
    > * not sure, so use only the pdata info
    > */
    > - if (pdata->f30_data &&
    > - pdata->f30_data->buttonpad)
    > + if (pdata && pdata->f30_data.buttonpad)

    Same with this check of pdata.

    > break;
    > }
    > }
    > @@ -378,7 +377,7 @@ static int rmi_f30_probe(struct rmi_function *fn)
    > const struct rmi_device_platform_data *pdata =
    > rmi_get_platform_data(fn->rmi_dev);
    >
    > - if (pdata->f30_data && pdata->f30_data->disable)
    > + if (pdata && pdata->f30_data.disable)

    And this one.

    That's a fairly minor comment and I could see an argument for keeping
    the checks in the event that the implementation of
    rmi_get_platform_data() changes.

    So:
    Reviewed-by: Andrew Duggan <aduggan@synaptics.com>

    Andrew

    > return 0;
    >
    > rc = rmi_f30_initialize(fn);
    > diff --git a/include/linux/rmi.h b/include/linux/rmi.h
    > index e0aca14..4a071e8 100644
    > --- a/include/linux/rmi.h
    > +++ b/include/linux/rmi.h
    > @@ -211,9 +211,9 @@ struct rmi_device_platform_data {
    > struct rmi_device_platform_data_spi spi_data;
    >
    > /* function handler pdata */
    > - struct rmi_2d_sensor_platform_data *sensor_pdata;
    > + struct rmi_2d_sensor_platform_data sensor_pdata;
    > struct rmi_f01_power_management power_management;
    > - struct rmi_f30_data *f30_data;
    > + struct rmi_f30_data f30_data;
    > };
    >
    > /**
    >

    \
     
     \ /
      Last update: 2016-09-17 09:57    [W:4.094 / U:0.012 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site