| Message ID | 20220203155920.18586-1-andriy.shevchenko@linux.intel.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [v1,1/3] iio: imu: inv_mpu6050: Drop wrong use of ACPI_PTR() | expand |
On Thu, 3 Feb 2022 17:59:18 +0200 Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > ACPI_PTR() is more harmful than helpful. For example, in this case > if CONFIG_ACPI=n, the ID table left unused which is not what we want. > > Instead of adding ifdeffery or attribute here and there, drop ACPI_PTR(). > > Fixes: 3b3870646642 ("iio: imu: inv_mpu6050: Mark acpi match table as maybe unused") > Fixes: fd64df16f40e ("iio: imu: inv_mpu6050: Add SPI support for MPU6000") > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Hi Andy, Whilst I fully support tidying this up, what is 'fixing' as such? Will get rid of an unused warning for the spi case but that sort of things doesn't always get fixes tags. They tend to result in backports and I wouldn't think it was worth backporting this unless I'm missing something... Jonathan > --- > drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c | 7 +++---- > drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c | 4 ++-- > 2 files changed, 5 insertions(+), 6 deletions(-) > > diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c > index fe03707ec2d3..ccb06d9af760 100644 > --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c > +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c > @@ -3,11 +3,11 @@ > * Copyright (C) 2012 Invensense, Inc. > */ > > -#include <linux/acpi.h> > #include <linux/delay.h> > #include <linux/err.h> > #include <linux/i2c.h> > #include <linux/iio/iio.h> > +#include <linux/mod_devicetable.h> > #include <linux/module.h> > #include <linux/of_device.h> > #include <linux/property.h> > @@ -249,11 +249,10 @@ static const struct of_device_id inv_of_match[] = { > }; > MODULE_DEVICE_TABLE(of, inv_of_match); > > -static const struct acpi_device_id __maybe_unused inv_acpi_match[] = { > +static const struct acpi_device_id inv_acpi_match[] = { > {"INVN6500", INV_MPU6500}, > { }, > }; > - > MODULE_DEVICE_TABLE(acpi, inv_acpi_match); > > static struct i2c_driver inv_mpu_driver = { > @@ -262,7 +261,7 @@ static struct i2c_driver inv_mpu_driver = { > .id_table = inv_mpu_id, > .driver = { > .of_match_table = inv_of_match, > - .acpi_match_table = ACPI_PTR(inv_acpi_match), > + .acpi_match_table = inv_acpi_match, > .name = "inv-mpu6050-i2c", > .pm = &inv_mpu_pmops, > }, > diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c > index 6800356b25fb..44b4f74b9256 100644 > --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c > +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c > @@ -2,8 +2,8 @@ > /* > * Copyright (C) 2015 Intel Corporation Inc. > */ > +#include <linux/mod_devicetable.h> > #include <linux/module.h> > -#include <linux/acpi.h> > #include <linux/of.h> > #include <linux/property.h> > #include <linux/spi/spi.h> > @@ -148,7 +148,7 @@ static struct spi_driver inv_mpu_driver = { > .id_table = inv_mpu_id, > .driver = { > .of_match_table = inv_of_match, > - .acpi_match_table = ACPI_PTR(inv_acpi_match), > + .acpi_match_table = inv_acpi_match, > .name = "inv-mpu6000-spi", > .pm = &inv_mpu_pmops, > },
On Sat, Feb 05, 2022 at 04:45:35PM +0000, Jonathan Cameron wrote: > On Thu, 3 Feb 2022 17:59:18 +0200 > Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > > > ACPI_PTR() is more harmful than helpful. For example, in this case > > if CONFIG_ACPI=n, the ID table left unused which is not what we want. > > > > Instead of adding ifdeffery or attribute here and there, drop ACPI_PTR(). > > > > Fixes: 3b3870646642 ("iio: imu: inv_mpu6050: Mark acpi match table as maybe unused") > > Fixes: fd64df16f40e ("iio: imu: inv_mpu6050: Add SPI support for MPU6000") > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > Hi Andy, > > Whilst I fully support tidying this up, what is 'fixing' as such? > Will get rid of an unused warning for the spi case but that sort > of things doesn't always get fixes tags. True, however I can find a handful examples when this kind of patches were backported. > They tend to result > in backports and I wouldn't think it was worth backporting this > unless I'm missing something... It's not critical, so can you drop the tags when applying, if you think that's okay?
On Sat, 5 Feb 2022 20:45:01 +0200 Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > On Sat, Feb 05, 2022 at 04:45:35PM +0000, Jonathan Cameron wrote: > > On Thu, 3 Feb 2022 17:59:18 +0200 > > Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > > > > > ACPI_PTR() is more harmful than helpful. For example, in this case > > > if CONFIG_ACPI=n, the ID table left unused which is not what we want. > > > > > > Instead of adding ifdeffery or attribute here and there, drop ACPI_PTR(). > > > > > > Fixes: 3b3870646642 ("iio: imu: inv_mpu6050: Mark acpi match table as maybe unused") > > > Fixes: fd64df16f40e ("iio: imu: inv_mpu6050: Add SPI support for MPU6000") > > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > > > Hi Andy, > > > > Whilst I fully support tidying this up, what is 'fixing' as such? > > Will get rid of an unused warning for the spi case but that sort > > of things doesn't always get fixes tags. > > True, however I can find a handful examples when this kind of patches were backported. > > > They tend to result > > in backports and I wouldn't think it was worth backporting this > > unless I'm missing something... > > It's not critical, so can you drop the tags when applying, if you think that's > okay? > Sure. I've dropped the Fixes tags and applied the series. Thanks, Jonathan
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c index fe03707ec2d3..ccb06d9af760 100644 --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c @@ -3,11 +3,11 @@ * Copyright (C) 2012 Invensense, Inc. */ -#include <linux/acpi.h> #include <linux/delay.h> #include <linux/err.h> #include <linux/i2c.h> #include <linux/iio/iio.h> +#include <linux/mod_devicetable.h> #include <linux/module.h> #include <linux/of_device.h> #include <linux/property.h> @@ -249,11 +249,10 @@ static const struct of_device_id inv_of_match[] = { }; MODULE_DEVICE_TABLE(of, inv_of_match); -static const struct acpi_device_id __maybe_unused inv_acpi_match[] = { +static const struct acpi_device_id inv_acpi_match[] = { {"INVN6500", INV_MPU6500}, { }, }; - MODULE_DEVICE_TABLE(acpi, inv_acpi_match); static struct i2c_driver inv_mpu_driver = { @@ -262,7 +261,7 @@ static struct i2c_driver inv_mpu_driver = { .id_table = inv_mpu_id, .driver = { .of_match_table = inv_of_match, - .acpi_match_table = ACPI_PTR(inv_acpi_match), + .acpi_match_table = inv_acpi_match, .name = "inv-mpu6050-i2c", .pm = &inv_mpu_pmops, }, diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c index 6800356b25fb..44b4f74b9256 100644 --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c @@ -2,8 +2,8 @@ /* * Copyright (C) 2015 Intel Corporation Inc. */ +#include <linux/mod_devicetable.h> #include <linux/module.h> -#include <linux/acpi.h> #include <linux/of.h> #include <linux/property.h> #include <linux/spi/spi.h> @@ -148,7 +148,7 @@ static struct spi_driver inv_mpu_driver = { .id_table = inv_mpu_id, .driver = { .of_match_table = inv_of_match, - .acpi_match_table = ACPI_PTR(inv_acpi_match), + .acpi_match_table = inv_acpi_match, .name = "inv-mpu6000-spi", .pm = &inv_mpu_pmops, },
ACPI_PTR() is more harmful than helpful. For example, in this case if CONFIG_ACPI=n, the ID table left unused which is not what we want. Instead of adding ifdeffery or attribute here and there, drop ACPI_PTR(). Fixes: 3b3870646642 ("iio: imu: inv_mpu6050: Mark acpi match table as maybe unused") Fixes: fd64df16f40e ("iio: imu: inv_mpu6050: Add SPI support for MPU6000") Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> --- drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c | 7 +++---- drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c | 4 ++-- 2 files changed, 5 insertions(+), 6 deletions(-)