On Sat, 4 Jul 2020 16:45:05 +0100
Jonathan Cameron <jic23@xxxxxxxxxx> wrote:
> On Tue, 30 Jun 2020 10:12:51 +0300
> Andy Shevchenko <andy.shevchenko@xxxxxxxxx> wrote:
>
> > On Sun, Jun 28, 2020 at 3:39 PM Jonathan Cameron <jic23@xxxxxxxxxx> wrote:
> > >
> > > From: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx>
> > >
> > > Whilst this driver already supports explicit ACPI bindings we
> > > might as well also allow for PRP0001 based binding.
> > >
> > > I'm also keen to remove of_match_ptr from IIO drivers to avoid
> > > this (now) anti-pattern getting coppied into new drivers.
> > >
> >
> > Code LGTM, but see below.
> >
> > > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx>
> > > ---
> > > drivers/iio/adc/ti-adc081c.c | 6 ++----
> > > 1 file changed, 2 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/iio/adc/ti-adc081c.c b/drivers/iio/adc/ti-adc081c.c
> > > index 82e524b3db88..e00350e6503f 100644
> > > --- a/drivers/iio/adc/ti-adc081c.c
> > > +++ b/drivers/iio/adc/ti-adc081c.c
> > > @@ -18,7 +18,7 @@
> > > #include <linux/err.h>
> > > #include <linux/i2c.h>
> > > #include <linux/module.h>
> > > -#include <linux/of.h>
> > > +#include <linux/mod_devicetable.h>
> > > #include <linux/acpi.h>
> > >
> > > #include <linux/iio/iio.h>
> > > @@ -230,7 +230,6 @@ static const struct i2c_device_id adc081c_id[] = {
> > > };
> > > MODULE_DEVICE_TABLE(i2c, adc081c_id);
> > >
> > > -#ifdef CONFIG_OF
> > > static const struct of_device_id adc081c_of_match[] = {
> > > { .compatible = "ti,adc081c" },
> > > { .compatible = "ti,adc101c" },
> > > @@ -238,7 +237,6 @@ static const struct of_device_id adc081c_of_match[] = {
> > > { }
> > > };
> > > MODULE_DEVICE_TABLE(of, adc081c_of_match);
> > > -#endif
> > >
> > > #ifdef CONFIG_ACPI
> > > static const struct acpi_device_id adc081c_acpi_match[] = {
> >
> > These IDs seem to me artificial (and non-official). Perhaps in a
> > separate patch remove them?
> > Or do we have confirmation (in writing) from TI that these are okay?
> +CC Dan O'Donovan,
>
> It seems highly unlikely these are 'official'.
>
> Dan. You added them, can you give us some background (admittedly
> 4 years ago so you may not recall!)
>
> Unfortunately I was rather less aware of ACPI than I have become in
> the meantime, so let these in without questioning them.
>
> If we have these out there in in the wild, we can still add a note
> to make it clear that people should avoid using them in future,
> or copying the approach in other drivers.
>
As this is a separate issue (kind of) I've applied this patch and
we can address whether to remove the ACPI bindings separately.
Thanks,
Jonathan
> Jonathan
>
> >
> > > @@ -253,7 +251,7 @@ MODULE_DEVICE_TABLE(acpi, adc081c_acpi_match);
> > > static struct i2c_driver adc081c_driver = {
> > > .driver = {
> > > .name = "adc081c",
> > > - .of_match_table = of_match_ptr(adc081c_of_match),
> > > + .of_match_table = adc081c_of_match,
> > > .acpi_match_table = ACPI_PTR(adc081c_acpi_match),
> > > },
> > > .probe = adc081c_probe,
> > > --
> > > 2.27.0
> > >
> >
> >
>
