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

[Hans de Goede <hdegoede@redhat.com>]

Hi,

On 11/30/20 9:46 PM, Jonathan Cameron wrote:
> 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.

Thanks, the fixed up version looks good.

And sorry for not catching this myself, some of the ACPI functions are stubbed
when CONFIG_ACPI is not set and I was relying on this here. But I missed acpi_device_hid()
not being stubbed.

I was aware I was using some unstubbed things in patch 3/3 so that already had the #if CONFIG_ACPI.

> Please sanity check I didn't mess it up.

Done, I'll also add the final version to my local tree so that its get tested
when I spin that up on one of the affected devices.

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

Thanks & Regards,

Hans



>>> 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
>>>  
>>
>>
>