Hi,
On 11/30/20 9:46 PM, Jonathan Cameron wrote:
> On Mon, 30 Nov 2020 16:32:21 +0200
> Andy Shevchenko <andy.shevchenko@xxxxxxxxx> wrote:
>
>> On Mon, Nov 30, 2020 at 4:20 PM Hans de Goede <hdegoede@xxxxxxxxxx> 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@xxxxxxxxx>
> 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.
FWIW I decided to go the full mile and since I cherry-picked the final versions
of the patches (including the regulator changes) into my local tree anyway
I decided to spin it up on a Thinkpad Yoga 11e 4th gen which has the dual accelerometer
setup.
I'm happy to report that the final version of the patches work as advertised
there.
Regards,
Hans
>>> Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>
>>> ---
>>> 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
>>>
>>
>>
>
