| Message ID | 1390521623-6491-2-git-send-email-courtney.cavin@sonymobile.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 01/23/2014 04:00 PM, Courtney Cavin wrote: > Cc: Christopher Heiny <cheiny@synaptics.com> > Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com> > Signed-off-by: Courtney Cavin <courtney.cavin@sonymobile.com> > --- > drivers/input/rmi4/rmi_bus.c | 4 ++-- > drivers/input/rmi4/rmi_bus.h | 2 +- > drivers/input/rmi4/rmi_driver.c | 17 ++++++++++++----- > drivers/input/rmi4/rmi_f11.c | 4 +++- > 4 files changed, 18 insertions(+), 9 deletions(-) > > diff --git a/drivers/input/rmi4/rmi_bus.c b/drivers/input/rmi4/rmi_bus.c > index 96a76e7..8a939f3 100644 > --- a/drivers/input/rmi4/rmi_bus.c > +++ b/drivers/input/rmi4/rmi_bus.c > @@ -37,7 +37,7 @@ static void rmi_release_device(struct device *dev) > kfree(rmi_dev); > } > > -struct device_type rmi_device_type = { > +static struct device_type rmi_device_type = { > .name = "rmi_sensor", > .release = rmi_release_device, > }; This struct is used by diagnostic modules to identify sensor devices, so it cannot be static. > @@ -145,7 +145,7 @@ static void rmi_release_function(struct device *dev) > kfree(fn); > } > > -struct device_type rmi_function_type = { > +static struct device_type rmi_function_type = { > .name = "rmi_function", > .release = rmi_release_function, > }; This struct is used by diagnostic modules to identify function devices, so it cannot be static. > diff --git a/drivers/input/rmi4/rmi_bus.h b/drivers/input/rmi4/rmi_bus.h > index decb479..2bad2ed 100644 > --- a/drivers/input/rmi4/rmi_bus.h > +++ b/drivers/input/rmi4/rmi_bus.h > @@ -103,7 +103,7 @@ int __must_check __rmi_register_function_handler(struct rmi_function_handler *, > #define rmi_register_function_handler(handler) \ > __rmi_register_function_handler(handler, THIS_MODULE, KBUILD_MODNAME) > > -void rmi_unregister_function_handler(struct rmi_function_handler *); > +void rmi_unregister_function_handler(struct rmi_function_handler *handler); > > /** > * struct rmi_driver - driver for an RMI4 sensor on the RMI bus. > diff --git a/drivers/input/rmi4/rmi_driver.c b/drivers/input/rmi4/rmi_driver.c > index 3483e5b..5c6379c 100644 > --- a/drivers/input/rmi4/rmi_driver.c > +++ b/drivers/input/rmi4/rmi_driver.c > @@ -32,6 +32,8 @@ > #include "rmi_bus.h" > #include "rmi_driver.h" > > +#define RMI4_MAX_N_IRQS 256 I see what you're trying to do here, but the IRQ counting patch submitted previously does this in a more efficient way, though at a slight cost in compute time during initialization. > + > #define HAS_NONSTANDARD_PDT_MASK 0x40 > #define RMI4_MAX_PAGE 0xff > #define RMI4_PAGE_SIZE 0x100 > @@ -260,7 +262,7 @@ static void process_one_interrupt(struct rmi_function *fn, > unsigned long *irq_status, struct rmi_driver_data *data) > { > struct rmi_function_handler *fh; > - DECLARE_BITMAP(irq_bits, data->num_of_irq_regs); > + DECLARE_BITMAP(irq_bits, RMI4_MAX_N_IRQS); > > if (!fn || !fn->dev.driver) > return; > @@ -325,7 +327,7 @@ static int process_interrupt_requests(struct rmi_device *rmi_dev) > static int rmi_driver_set_input_params(struct rmi_device *rmi_dev, > struct input_dev *input) > { > - // FIXME: set up parent > + /* FIXME: set up parent */ > input->name = SYNAPTICS_INPUT_DEVICE_NAME; > input->id.vendor = SYNAPTICS_VENDOR_ID; > input->id.bustype = BUS_RMI; > @@ -466,7 +468,7 @@ static int rmi_driver_reset_handler(struct rmi_device *rmi_dev) > /* > * Construct a function's IRQ mask. This should be called once and stored. > */ > -int rmi_driver_irq_get_mask(struct rmi_device *rmi_dev, > +static int rmi_driver_irq_get_mask(struct rmi_device *rmi_dev, > struct rmi_function *fn) { > int i; > struct rmi_driver_data *data = dev_get_drvdata(&rmi_dev->dev); > @@ -665,7 +667,7 @@ static int rmi_scan_pdt(struct rmi_device *rmi_dev) > pdt_entry.function_number, page); > done = false; > > - // XXX need to make sure we create F01 first... > + /* XXX need to make sure we create F01 first... */ > retval = create_function(rmi_dev, > &pdt_entry, &irq_count, page_start); > > @@ -674,6 +676,11 @@ static int rmi_scan_pdt(struct rmi_device *rmi_dev) > } > done = done || data->f01_bootloader_mode; > } > + if (irq_count > RMI4_MAX_N_IRQS) { > + dev_err(dev, "Too many IRQs specified\n"); > + retval = -EINVAL; > + goto error_exit; > + } > data->irq_count = irq_count; > data->num_of_irq_regs = (irq_count + 7) / 8; > dev_dbg(dev, "%s: Done with PDT scan.\n", __func__); > @@ -953,7 +960,7 @@ static int rmi_driver_probe(struct device *dev) > return retval; > } > > -struct rmi_driver rmi_physical_driver = { > +static struct rmi_driver rmi_physical_driver = { > .driver = { > .owner = THIS_MODULE, > .name = "rmi_physical", > diff --git a/drivers/input/rmi4/rmi_f11.c b/drivers/input/rmi4/rmi_f11.c > index c2be9de..4e0a296 100644 > --- a/drivers/input/rmi4/rmi_f11.c > +++ b/drivers/input/rmi4/rmi_f11.c > @@ -610,6 +610,8 @@ static void rmi_f11_abs_pos_report(struct f11_data *f11, > int temp; > u8 abs_base = n_finger * RMI_F11_ABS_BYTES; > > + orient = z = w_min = w_max = 0; > + > if (finger_state) { > x = (data->abs_pos[abs_base] << 4) | > (data->abs_pos[abs_base + 2] & 0x0F); > @@ -1413,7 +1415,7 @@ static int rmi_f11_config(struct rmi_function *fn) > return 0; > } > > -int rmi_f11_attention(struct rmi_function *fn, > +static int rmi_f11_attention(struct rmi_function *fn, > unsigned long *irq_bits) > { > struct rmi_device *rmi_dev = fn->rmi_dev; > -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Feb 05, 2014 at 12:08:12AM +0100, Christopher Heiny wrote: > On 01/23/2014 04:00 PM, Courtney Cavin wrote: > > Cc: Christopher Heiny <cheiny@synaptics.com> > > Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com> > > Signed-off-by: Courtney Cavin <courtney.cavin@sonymobile.com> > > --- > > drivers/input/rmi4/rmi_bus.c | 4 ++-- > > drivers/input/rmi4/rmi_bus.h | 2 +- > > drivers/input/rmi4/rmi_driver.c | 17 ++++++++++++----- > > drivers/input/rmi4/rmi_f11.c | 4 +++- > > 4 files changed, 18 insertions(+), 9 deletions(-) > > > > diff --git a/drivers/input/rmi4/rmi_bus.c b/drivers/input/rmi4/rmi_bus.c > > index 96a76e7..8a939f3 100644 > > --- a/drivers/input/rmi4/rmi_bus.c > > +++ b/drivers/input/rmi4/rmi_bus.c > > @@ -37,7 +37,7 @@ static void rmi_release_device(struct device *dev) > > kfree(rmi_dev); > > } > > > > -struct device_type rmi_device_type = { > > +static struct device_type rmi_device_type = { > > .name = "rmi_sensor", > > .release = rmi_release_device, > > }; > > This struct is used by diagnostic modules to identify sensor devices, so > it cannot be static. > Why is this not exposed in a header file then? It does not appear to be needed by anything in-tree, so I don't see a reason for keeping it in the global namespace. Especially if it isn't exposed anywhere. > > @@ -145,7 +145,7 @@ static void rmi_release_function(struct device *dev) > > kfree(fn); > > } > > > > -struct device_type rmi_function_type = { > > +static struct device_type rmi_function_type = { > > .name = "rmi_function", > > .release = rmi_release_function, > > }; > > This struct is used by diagnostic modules to identify function devices, > so it cannot be static. > Same. > > diff --git a/drivers/input/rmi4/rmi_bus.h b/drivers/input/rmi4/rmi_bus.h > > index decb479..2bad2ed 100644 > > --- a/drivers/input/rmi4/rmi_bus.h > > +++ b/drivers/input/rmi4/rmi_bus.h > > @@ -103,7 +103,7 @@ int __must_check __rmi_register_function_handler(struct rmi_function_handler *, > > #define rmi_register_function_handler(handler) \ > > __rmi_register_function_handler(handler, THIS_MODULE, KBUILD_MODNAME) > > > > -void rmi_unregister_function_handler(struct rmi_function_handler *); > > +void rmi_unregister_function_handler(struct rmi_function_handler *handler); > > > > /** > > * struct rmi_driver - driver for an RMI4 sensor on the RMI bus. > > diff --git a/drivers/input/rmi4/rmi_driver.c b/drivers/input/rmi4/rmi_driver.c > > index 3483e5b..5c6379c 100644 > > --- a/drivers/input/rmi4/rmi_driver.c > > +++ b/drivers/input/rmi4/rmi_driver.c > > @@ -32,6 +32,8 @@ > > #include "rmi_bus.h" > > #include "rmi_driver.h" > > > > +#define RMI4_MAX_N_IRQS 256 > > I see what you're trying to do here, but the IRQ counting patch > submitted previously does this in a more efficient way, though at a > slight cost in compute time during initialization. > What I'm trying to do here is get rid of dynamic stack allocation in process_one_interrupt(), which sparse seems to hate. Ideally though these bitmaps should all be statically sized. It would be very unusual to have enough functions & IRQs within each function to reach 256 IRQs. > > + > > #define HAS_NONSTANDARD_PDT_MASK 0x40 > > #define RMI4_MAX_PAGE 0xff > > #define RMI4_PAGE_SIZE 0x100 > > @@ -260,7 +262,7 @@ static void process_one_interrupt(struct rmi_function *fn, > > unsigned long *irq_status, struct rmi_driver_data *data) > > { > > struct rmi_function_handler *fh; > > - DECLARE_BITMAP(irq_bits, data->num_of_irq_regs); > > + DECLARE_BITMAP(irq_bits, RMI4_MAX_N_IRQS); > > > > if (!fn || !fn->dev.driver) > > return; > > @@ -325,7 +327,7 @@ static int process_interrupt_requests(struct rmi_device *rmi_dev) > > static int rmi_driver_set_input_params(struct rmi_device *rmi_dev, > > struct input_dev *input) > > { > > - // FIXME: set up parent > > + /* FIXME: set up parent */ > > input->name = SYNAPTICS_INPUT_DEVICE_NAME; > > input->id.vendor = SYNAPTICS_VENDOR_ID; > > input->id.bustype = BUS_RMI; > > @@ -466,7 +468,7 @@ static int rmi_driver_reset_handler(struct rmi_device *rmi_dev) > > /* > > * Construct a function's IRQ mask. This should be called once and stored. > > */ > > -int rmi_driver_irq_get_mask(struct rmi_device *rmi_dev, > > +static int rmi_driver_irq_get_mask(struct rmi_device *rmi_dev, > > struct rmi_function *fn) { > > int i; > > struct rmi_driver_data *data = dev_get_drvdata(&rmi_dev->dev); > > @@ -665,7 +667,7 @@ static int rmi_scan_pdt(struct rmi_device *rmi_dev) > > pdt_entry.function_number, page); > > done = false; > > > > - // XXX need to make sure we create F01 first... > > + /* XXX need to make sure we create F01 first... */ > > retval = create_function(rmi_dev, > > &pdt_entry, &irq_count, page_start); > > > > @@ -674,6 +676,11 @@ static int rmi_scan_pdt(struct rmi_device *rmi_dev) > > } > > done = done || data->f01_bootloader_mode; > > } > > + if (irq_count > RMI4_MAX_N_IRQS) { > > + dev_err(dev, "Too many IRQs specified\n"); > > + retval = -EINVAL; > > + goto error_exit; > > + } > > data->irq_count = irq_count; > > data->num_of_irq_regs = (irq_count + 7) / 8; > > dev_dbg(dev, "%s: Done with PDT scan.\n", __func__); > > @@ -953,7 +960,7 @@ static int rmi_driver_probe(struct device *dev) > > return retval; > > } > > > > -struct rmi_driver rmi_physical_driver = { > > +static struct rmi_driver rmi_physical_driver = { > > .driver = { > > .owner = THIS_MODULE, > > .name = "rmi_physical", > > diff --git a/drivers/input/rmi4/rmi_f11.c b/drivers/input/rmi4/rmi_f11.c > > index c2be9de..4e0a296 100644 > > --- a/drivers/input/rmi4/rmi_f11.c > > +++ b/drivers/input/rmi4/rmi_f11.c > > @@ -610,6 +610,8 @@ static void rmi_f11_abs_pos_report(struct f11_data *f11, > > int temp; > > u8 abs_base = n_finger * RMI_F11_ABS_BYTES; > > > > + orient = z = w_min = w_max = 0; > > + > > if (finger_state) { > > x = (data->abs_pos[abs_base] << 4) | > > (data->abs_pos[abs_base + 2] & 0x0F); > > @@ -1413,7 +1415,7 @@ static int rmi_f11_config(struct rmi_function *fn) > > return 0; > > } > > > > -int rmi_f11_attention(struct rmi_function *fn, > > +static int rmi_f11_attention(struct rmi_function *fn, > > unsigned long *irq_bits) > > { > > struct rmi_device *rmi_dev = fn->rmi_dev; > > > -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Feb 04, 2014 at 03:08:12PM -0800, Christopher Heiny wrote: > On 01/23/2014 04:00 PM, Courtney Cavin wrote: > >Cc: Christopher Heiny <cheiny@synaptics.com> > >Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com> > >Signed-off-by: Courtney Cavin <courtney.cavin@sonymobile.com> > >--- > > drivers/input/rmi4/rmi_bus.c | 4 ++-- > > drivers/input/rmi4/rmi_bus.h | 2 +- > > drivers/input/rmi4/rmi_driver.c | 17 ++++++++++++----- > > drivers/input/rmi4/rmi_f11.c | 4 +++- > > 4 files changed, 18 insertions(+), 9 deletions(-) > > > >diff --git a/drivers/input/rmi4/rmi_bus.c b/drivers/input/rmi4/rmi_bus.c > >index 96a76e7..8a939f3 100644 > >--- a/drivers/input/rmi4/rmi_bus.c > >+++ b/drivers/input/rmi4/rmi_bus.c > >@@ -37,7 +37,7 @@ static void rmi_release_device(struct device *dev) > > kfree(rmi_dev); > > } > > > >-struct device_type rmi_device_type = { > >+static struct device_type rmi_device_type = { > > .name = "rmi_sensor", > > .release = rmi_release_device, > > }; > > This struct is used by diagnostic modules to identify sensor > devices, so it cannot be static. Then we need to declare it somewhere or provide an accessor function. Thanks.
On 02/05/2014 05:09 PM, Dmitry Torokhov wrote: > On Tue, Feb 04, 2014 at 03:08:12PM -0800, Christopher Heiny wrote: >> >On 01/23/2014 04:00 PM, Courtney Cavin wrote: >>> > >Cc: Christopher Heiny<cheiny@synaptics.com> >>> > >Cc: Dmitry Torokhov<dmitry.torokhov@gmail.com> >>> > >Signed-off-by: Courtney Cavin<courtney.cavin@sonymobile.com> >>> > >--- >>> > > drivers/input/rmi4/rmi_bus.c | 4 ++-- >>> > > drivers/input/rmi4/rmi_bus.h | 2 +- >>> > > drivers/input/rmi4/rmi_driver.c | 17 ++++++++++++----- >>> > > drivers/input/rmi4/rmi_f11.c | 4 +++- >>> > > 4 files changed, 18 insertions(+), 9 deletions(-) >>> > > >>> > >diff --git a/drivers/input/rmi4/rmi_bus.c b/drivers/input/rmi4/rmi_bus.c >>> > >index 96a76e7..8a939f3 100644 >>> > >--- a/drivers/input/rmi4/rmi_bus.c >>> > >+++ b/drivers/input/rmi4/rmi_bus.c >>> > >@@ -37,7 +37,7 @@ static void rmi_release_device(struct device *dev) >>> > > kfree(rmi_dev); >>> > > } >>> > > >>> > >-struct device_type rmi_device_type = { >>> > >+static struct device_type rmi_device_type = { >>> > > .name = "rmi_sensor", >>> > > .release = rmi_release_device, >>> > > }; >> > >> >This struct is used by diagnostic modules to identify sensor >> >devices, so it cannot be static. > > Then we need to declare it somewhere or provide an accessor function. Currently it's in a header not included in the patches. We'll move it to rmi_bus.h. -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Feb 05, 2014 at 05:36:09PM -0800, Christopher Heiny wrote: > On 02/05/2014 05:09 PM, Dmitry Torokhov wrote: > >On Tue, Feb 04, 2014 at 03:08:12PM -0800, Christopher Heiny wrote: > >>>On 01/23/2014 04:00 PM, Courtney Cavin wrote: > >>>> >Cc: Christopher Heiny<cheiny@synaptics.com> > >>>> >Cc: Dmitry Torokhov<dmitry.torokhov@gmail.com> > >>>> >Signed-off-by: Courtney Cavin<courtney.cavin@sonymobile.com> > >>>> >--- > >>>> > drivers/input/rmi4/rmi_bus.c | 4 ++-- > >>>> > drivers/input/rmi4/rmi_bus.h | 2 +- > >>>> > drivers/input/rmi4/rmi_driver.c | 17 ++++++++++++----- > >>>> > drivers/input/rmi4/rmi_f11.c | 4 +++- > >>>> > 4 files changed, 18 insertions(+), 9 deletions(-) > >>>> > > >>>> >diff --git a/drivers/input/rmi4/rmi_bus.c b/drivers/input/rmi4/rmi_bus.c > >>>> >index 96a76e7..8a939f3 100644 > >>>> >--- a/drivers/input/rmi4/rmi_bus.c > >>>> >+++ b/drivers/input/rmi4/rmi_bus.c > >>>> >@@ -37,7 +37,7 @@ static void rmi_release_device(struct device *dev) > >>>> > kfree(rmi_dev); > >>>> > } > >>>> > > >>>> >-struct device_type rmi_device_type = { > >>>> >+static struct device_type rmi_device_type = { > >>>> > .name = "rmi_sensor", > >>>> > .release = rmi_release_device, > >>>> > }; > >>> > >>>This struct is used by diagnostic modules to identify sensor > >>>devices, so it cannot be static. > > > >Then we need to declare it somewhere or provide an accessor function. > > Currently it's in a header not included in the patches. We'll move > it to rmi_bus.h. Hmm, we do have rmi_is_physical_device() to identify whether it is a sensor or a function, so I believe we should mark all structures static to avoid anyone poking at them. Thanks.
On 02/12/2014 10:36 PM, Dmitry Torokhov wrote: > On Wed, Feb 05, 2014 at 05:36:09PM -0800, Christopher Heiny wrote: >> >On 02/05/2014 05:09 PM, Dmitry Torokhov wrote: >>> > >On Tue, Feb 04, 2014 at 03:08:12PM -0800, Christopher Heiny wrote: >>>>> > >>>On 01/23/2014 04:00 PM, Courtney Cavin wrote: >>>>>>> > >>>> >Cc: Christopher Heiny<cheiny@synaptics.com> >>>>>>> > >>>> >Cc: Dmitry Torokhov<dmitry.torokhov@gmail.com> >>>>>>> > >>>> >Signed-off-by: Courtney Cavin<courtney.cavin@sonymobile.com> >>>>>>> > >>>> >--- >>>>>>> > >>>> > drivers/input/rmi4/rmi_bus.c | 4 ++-- >>>>>>> > >>>> > drivers/input/rmi4/rmi_bus.h | 2 +- >>>>>>> > >>>> > drivers/input/rmi4/rmi_driver.c | 17 ++++++++++++----- >>>>>>> > >>>> > drivers/input/rmi4/rmi_f11.c | 4 +++- >>>>>>> > >>>> > 4 files changed, 18 insertions(+), 9 deletions(-) >>>>>>> > >>>> > >>>>>>> > >>>> >diff --git a/drivers/input/rmi4/rmi_bus.c b/drivers/input/rmi4/rmi_bus.c >>>>>>> > >>>> >index 96a76e7..8a939f3 100644 >>>>>>> > >>>> >--- a/drivers/input/rmi4/rmi_bus.c >>>>>>> > >>>> >+++ b/drivers/input/rmi4/rmi_bus.c >>>>>>> > >>>> >@@ -37,7 +37,7 @@ static void rmi_release_device(struct device *dev) >>>>>>> > >>>> > kfree(rmi_dev); >>>>>>> > >>>> > } >>>>>>> > >>>> > >>>>>>> > >>>> >-struct device_type rmi_device_type = { >>>>>>> > >>>> >+static struct device_type rmi_device_type = { >>>>>>> > >>>> > .name = "rmi_sensor", >>>>>>> > >>>> > .release = rmi_release_device, >>>>>>> > >>>> > }; >>>>> > >>> >>>>> > >>>This struct is used by diagnostic modules to identify sensor >>>>> > >>>devices, so it cannot be static. >>> > > >>> > >Then we need to declare it somewhere or provide an accessor function. >> > >> >Currently it's in a header not included in the patches. We'll move >> >it to rmi_bus.h. > Hmm, we do have rmi_is_physical_device() to identify whether it is a > sensor or a function, so I believe we should mark all structures static > to avoid anyone poking at them. I was poking around in the dependent code late last night and came to the same conclusion. I'll send a micropatch later today to get it out of the way.
On Thursday, February 13, 2014 10:56:25 AM Christopher Heiny wrote: > On 02/12/2014 10:36 PM, Dmitry Torokhov wrote: > > On Wed, Feb 05, 2014 at 05:36:09PM -0800, Christopher Heiny wrote: > >> >On 02/05/2014 05:09 PM, Dmitry Torokhov wrote: > >>> > >On Tue, Feb 04, 2014 at 03:08:12PM -0800, Christopher Heiny wrote: > >>>>> > >>>On 01/23/2014 04:00 PM, Courtney Cavin wrote: > >>>>>>> > >>>> >Cc: Christopher Heiny<cheiny@synaptics.com> > >>>>>>> > >>>> >Cc: Dmitry Torokhov<dmitry.torokhov@gmail.com> > >>>>>>> > >>>> >Signed-off-by: Courtney Cavin<courtney.cavin@sonymobile.com> > >>>>>>> > >>>> >--- > >>>>>>> > >>>> > > >>>>>>> > >>>> > drivers/input/rmi4/rmi_bus.c | 4 ++-- > >>>>>>> > >>>> > drivers/input/rmi4/rmi_bus.h | 2 +- > >>>>>>> > >>>> > drivers/input/rmi4/rmi_driver.c | 17 ++++++++++++----- > >>>>>>> > >>>> > drivers/input/rmi4/rmi_f11.c | 4 +++- > >>>>>>> > >>>> > 4 files changed, 18 insertions(+), 9 deletions(-) > >>>>>>> > >>>> > > >>>>>>> > >>>> >diff --git a/drivers/input/rmi4/rmi_bus.c > >>>>>>> > >>>> >b/drivers/input/rmi4/rmi_bus.c > >>>>>>> > >>>> >index 96a76e7..8a939f3 100644 > >>>>>>> > >>>> >--- a/drivers/input/rmi4/rmi_bus.c > >>>>>>> > >>>> >+++ b/drivers/input/rmi4/rmi_bus.c > >>>>>>> > >>>> >@@ -37,7 +37,7 @@ static void rmi_release_device(struct > >>>>>>> > >>>> >device *dev) > >>>>>>> > >>>> > > >>>>>>> > >>>> > kfree(rmi_dev); > >>>>>>> > >>>> > > >>>>>>> > >>>> > } > >>>>>>> > >>>> > > >>>>>>> > >>>> >-struct device_type rmi_device_type = { > >>>>>>> > >>>> >+static struct device_type rmi_device_type = { > >>>>>>> > >>>> > > >>>>>>> > >>>> > .name = "rmi_sensor", > >>>>>>> > >>>> > .release = rmi_release_device, > >>>>>>> > >>>> > > >>>>>>> > >>>> > }; > >>>>> > >>> > >>>>> > >>>This struct is used by diagnostic modules to identify sensor > >>>>> > >>>devices, so it cannot be static. > >>> > > > >>> > >Then we need to declare it somewhere or provide an accessor function. > >> > > >> >Currently it's in a header not included in the patches. We'll move > >> >it to rmi_bus.h. > > > > Hmm, we do have rmi_is_physical_device() to identify whether it is a > > sensor or a function, so I believe we should mark all structures static > > to avoid anyone poking at them. > > I was poking around in the dependent code late last night and came to > the same conclusion. I'll send a micropatch later today to get it out > of the way. No need, I untangled relevant bits from the one Courtney sent. Thanks.
diff --git a/drivers/input/rmi4/rmi_bus.c b/drivers/input/rmi4/rmi_bus.c index 96a76e7..8a939f3 100644 --- a/drivers/input/rmi4/rmi_bus.c +++ b/drivers/input/rmi4/rmi_bus.c @@ -37,7 +37,7 @@ static void rmi_release_device(struct device *dev) kfree(rmi_dev); } -struct device_type rmi_device_type = { +static struct device_type rmi_device_type = { .name = "rmi_sensor", .release = rmi_release_device, }; @@ -145,7 +145,7 @@ static void rmi_release_function(struct device *dev) kfree(fn); } -struct device_type rmi_function_type = { +static struct device_type rmi_function_type = { .name = "rmi_function", .release = rmi_release_function, }; diff --git a/drivers/input/rmi4/rmi_bus.h b/drivers/input/rmi4/rmi_bus.h index decb479..2bad2ed 100644 --- a/drivers/input/rmi4/rmi_bus.h +++ b/drivers/input/rmi4/rmi_bus.h @@ -103,7 +103,7 @@ int __must_check __rmi_register_function_handler(struct rmi_function_handler *, #define rmi_register_function_handler(handler) \ __rmi_register_function_handler(handler, THIS_MODULE, KBUILD_MODNAME) -void rmi_unregister_function_handler(struct rmi_function_handler *); +void rmi_unregister_function_handler(struct rmi_function_handler *handler); /** * struct rmi_driver - driver for an RMI4 sensor on the RMI bus. diff --git a/drivers/input/rmi4/rmi_driver.c b/drivers/input/rmi4/rmi_driver.c index 3483e5b..5c6379c 100644 --- a/drivers/input/rmi4/rmi_driver.c +++ b/drivers/input/rmi4/rmi_driver.c @@ -32,6 +32,8 @@ #include "rmi_bus.h" #include "rmi_driver.h" +#define RMI4_MAX_N_IRQS 256 + #define HAS_NONSTANDARD_PDT_MASK 0x40 #define RMI4_MAX_PAGE 0xff #define RMI4_PAGE_SIZE 0x100 @@ -260,7 +262,7 @@ static void process_one_interrupt(struct rmi_function *fn, unsigned long *irq_status, struct rmi_driver_data *data) { struct rmi_function_handler *fh; - DECLARE_BITMAP(irq_bits, data->num_of_irq_regs); + DECLARE_BITMAP(irq_bits, RMI4_MAX_N_IRQS); if (!fn || !fn->dev.driver) return; @@ -325,7 +327,7 @@ static int process_interrupt_requests(struct rmi_device *rmi_dev) static int rmi_driver_set_input_params(struct rmi_device *rmi_dev, struct input_dev *input) { - // FIXME: set up parent + /* FIXME: set up parent */ input->name = SYNAPTICS_INPUT_DEVICE_NAME; input->id.vendor = SYNAPTICS_VENDOR_ID; input->id.bustype = BUS_RMI; @@ -466,7 +468,7 @@ static int rmi_driver_reset_handler(struct rmi_device *rmi_dev) /* * Construct a function's IRQ mask. This should be called once and stored. */ -int rmi_driver_irq_get_mask(struct rmi_device *rmi_dev, +static int rmi_driver_irq_get_mask(struct rmi_device *rmi_dev, struct rmi_function *fn) { int i; struct rmi_driver_data *data = dev_get_drvdata(&rmi_dev->dev); @@ -665,7 +667,7 @@ static int rmi_scan_pdt(struct rmi_device *rmi_dev) pdt_entry.function_number, page); done = false; - // XXX need to make sure we create F01 first... + /* XXX need to make sure we create F01 first... */ retval = create_function(rmi_dev, &pdt_entry, &irq_count, page_start); @@ -674,6 +676,11 @@ static int rmi_scan_pdt(struct rmi_device *rmi_dev) } done = done || data->f01_bootloader_mode; } + if (irq_count > RMI4_MAX_N_IRQS) { + dev_err(dev, "Too many IRQs specified\n"); + retval = -EINVAL; + goto error_exit; + } data->irq_count = irq_count; data->num_of_irq_regs = (irq_count + 7) / 8; dev_dbg(dev, "%s: Done with PDT scan.\n", __func__); @@ -953,7 +960,7 @@ static int rmi_driver_probe(struct device *dev) return retval; } -struct rmi_driver rmi_physical_driver = { +static struct rmi_driver rmi_physical_driver = { .driver = { .owner = THIS_MODULE, .name = "rmi_physical", diff --git a/drivers/input/rmi4/rmi_f11.c b/drivers/input/rmi4/rmi_f11.c index c2be9de..4e0a296 100644 --- a/drivers/input/rmi4/rmi_f11.c +++ b/drivers/input/rmi4/rmi_f11.c @@ -610,6 +610,8 @@ static void rmi_f11_abs_pos_report(struct f11_data *f11, int temp; u8 abs_base = n_finger * RMI_F11_ABS_BYTES; + orient = z = w_min = w_max = 0; + if (finger_state) { x = (data->abs_pos[abs_base] << 4) | (data->abs_pos[abs_base + 2] & 0x0F); @@ -1413,7 +1415,7 @@ static int rmi_f11_config(struct rmi_function *fn) return 0; } -int rmi_f11_attention(struct rmi_function *fn, +static int rmi_f11_attention(struct rmi_function *fn, unsigned long *irq_bits) { struct rmi_device *rmi_dev = fn->rmi_dev;
Cc: Christopher Heiny <cheiny@synaptics.com> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com> Signed-off-by: Courtney Cavin <courtney.cavin@sonymobile.com> --- drivers/input/rmi4/rmi_bus.c | 4 ++-- drivers/input/rmi4/rmi_bus.h | 2 +- drivers/input/rmi4/rmi_driver.c | 17 ++++++++++++----- drivers/input/rmi4/rmi_f11.c | 4 +++- 4 files changed, 18 insertions(+), 9 deletions(-)