Re: [PATCH v2 1/3] iio: accel: bmc150: Removed unused bmc150_accel_dat irq member

[Jonathan Cameron <jic23@kernel.org>]

On Mon, 30 Nov 2020 16:32:21 +0200
Andy Shevchenko <andy.shevchenko@gmail.com> wrote:

> On Mon, Nov 30, 2020 at 4:20 PM Hans de Goede <hdegoede@redhat.com> wrote:
> >
> > The bmc150_accel_dat struct irq member is only ever used inside
> > bmc150_accel_core_probe, drop it and just use the function argument
> > directly.  
> 
> FWIW, for all three
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
This crossed with a series adding regulator control to this driver, but I'm fairly
sure that won't cause any problems so I've dealt with the fuzz and applied it anyway.

However...

drivers/iio/accel/bmc150-accel-i2c.c: In function ‘bmc150_accel_probe’:
drivers/iio/accel/bmc150-accel-i2c.c:55:28: error: implicit declaration of function ‘acpi_device_hid’; did you mean ‘dmi_device_id’? [-Werror=implicit-function-declarati
on]                                                                                                                                                                      
   55 |  if (!id && adev && strcmp(acpi_device_hid(adev), "BOSC0200") == 0) {
      |                            ^~~~~~~~~~~~~~~            
      |                            dmi_device_id
drivers/iio/accel/bmc150-accel-i2c.c:55:28: warning: passing argument 1 of ‘strcmp’ makes pointer from integer without a cast [-Wint-conversion]
   55 |  if (!id && adev && strcmp(acpi_device_hid(adev), "BOSC0200") == 0) {
      |                            ^~~~~~~~~~~~~~~~~~~~~
      |                            |
      |                            int


I've added #ifdef CONFIG_ACPI around the relevant block and shuffled around assignment of
adev + added a __maybe_unused marking to it.  Perhaps I should have pulled that block
out into another function but it seemed more trouble than it was worth.

I'm slightly confused on how I ended up with a test .config that doesn't have CONFIG_ACPI
but that's another story and handy on this occasion as we didn't have to wait for 0-day
to notice this.

Please sanity check I didn't mess it up.

Series applied to the togreg branch of iio.git and pushed out as testing for
the autobuilders to see what else we missed.

Thanks

Jonathan

> 
> > Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> > ---
> >  drivers/iio/accel/bmc150-accel-core.c | 7 ++-----
> >  1 file changed, 2 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/iio/accel/bmc150-accel-core.c b/drivers/iio/accel/bmc150-accel-core.c
> > index 48435865fdaf..088716d55855 100644
> > --- a/drivers/iio/accel/bmc150-accel-core.c
> > +++ b/drivers/iio/accel/bmc150-accel-core.c
> > @@ -183,7 +183,6 @@ enum bmc150_accel_trigger_id {
> >
> >  struct bmc150_accel_data {
> >         struct regmap *regmap;
> > -       int irq;
> >         struct bmc150_accel_interrupt interrupts[BMC150_ACCEL_INTERRUPTS];
> >         struct bmc150_accel_trigger triggers[BMC150_ACCEL_TRIGGERS];
> >         struct mutex mutex;
> > @@ -1568,7 +1567,6 @@ int bmc150_accel_core_probe(struct device *dev, struct regmap *regmap, int irq,
> >
> >         data = iio_priv(indio_dev);
> >         dev_set_drvdata(dev, indio_dev);
> > -       data->irq = irq;
> >
> >         data->regmap = regmap;
> >
> > @@ -1599,9 +1597,8 @@ int bmc150_accel_core_probe(struct device *dev, struct regmap *regmap, int irq,
> >                 return ret;
> >         }
> >
> > -       if (data->irq > 0) {
> > -               ret = devm_request_threaded_irq(
> > -                                               dev, data->irq,
> > +       if (irq > 0) {
> > +               ret = devm_request_threaded_irq(dev, irq,
> >                                                 bmc150_accel_irq_handler,
> >                                                 bmc150_accel_irq_thread_handler,
> >                                                 IRQF_TRIGGER_RISING,
> > --
> > 2.28.0
> >  
> 
>