| Message ID | 20250322081108.202654-1-simeddon@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | iio: frequency: ad9832: devicetree probing support | expand |
LGTM, one minor suggestion inline. With that applied, Reviewed-by: Marcelo Schmitt <marcelo.schmitt1@gmail.com> On 03/22, Siddharth Menon wrote: > Introduce struct for device match of_device_id > > Suggested-by: Marcelo Schmitt <marcelo.schmitt1@gmail.com> > Signed-off-by: Siddharth Menon <simeddon@gmail.com> > --- > drivers/staging/iio/frequency/ad9832.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/drivers/staging/iio/frequency/ad9832.c b/drivers/staging/iio/frequency/ad9832.c > index 9a27acd88926..506478ddede3 100644 > --- a/drivers/staging/iio/frequency/ad9832.c > +++ b/drivers/staging/iio/frequency/ad9832.c > @@ -439,6 +439,13 @@ static int ad9832_probe(struct spi_device *spi) > return devm_iio_device_register(&spi->dev, indio_dev); > } > > +static const struct of_device_id ad9832_of_match[] = { > + { .compatible = "adi,ad9832" }, > + { .compatible = "adi,ad9835" }, > + {}, I think Jonathan prefers to have a space separating the null terminator braces. { }, > +}; > +MODULE_DEVICE_TABLE(of, ad9832_of_match); > + > static const struct spi_device_id ad9832_id[] = { > {"ad9832", 0}, > {"ad9835", 0}, > @@ -449,6 +456,7 @@ MODULE_DEVICE_TABLE(spi, ad9832_id); > static struct spi_driver ad9832_driver = { > .driver = { > .name = "ad9832", > + .of_match_table = ad9832_of_match, > }, > .probe = ad9832_probe, > .id_table = ad9832_id, > -- > 2.48.1 >
On Sat, 22 Mar 2025 13:37:45 +0530 Siddharth Menon <simeddon@gmail.com> wrote: > Introduce struct for device match of_device_id Good to say why. Key is that it will probe from a dts entry without this due to the various matching fallback paths, but it will be matching with the less informative bit after the manufacturer. So we might get a false match in future against some other ad9832 part. > > Suggested-by: Marcelo Schmitt <marcelo.schmitt1@gmail.com> > Signed-off-by: Siddharth Menon <simeddon@gmail.com> > --- > drivers/staging/iio/frequency/ad9832.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/drivers/staging/iio/frequency/ad9832.c b/drivers/staging/iio/frequency/ad9832.c > index 9a27acd88926..506478ddede3 100644 > --- a/drivers/staging/iio/frequency/ad9832.c > +++ b/drivers/staging/iio/frequency/ad9832.c > @@ -439,6 +439,13 @@ static int ad9832_probe(struct spi_device *spi) > return devm_iio_device_register(&spi->dev, indio_dev); > } > > +static const struct of_device_id ad9832_of_match[] = { > + { .compatible = "adi,ad9832" }, > + { .compatible = "adi,ad9835" }, > + {}, No comma on a terminating entry like this. We don't want to make it easy to add stuff after it! Also, I'm trying to standardize the formatting of these tables across IIO. So I made the random choice of having a space between the brackets and am slowly changing drivers to that standard. { } > +}; > +MODULE_DEVICE_TABLE(of, ad9832_of_match); > + > static const struct spi_device_id ad9832_id[] = { > {"ad9832", 0}, > {"ad9835", 0}, > @@ -449,6 +456,7 @@ MODULE_DEVICE_TABLE(spi, ad9832_id); > static struct spi_driver ad9832_driver = { > .driver = { > .name = "ad9832", > + .of_match_table = ad9832_of_match, > }, > .probe = ad9832_probe, > .id_table = ad9832_id,
On Sat, 22 Mar 2025 10:40:17 -0300 Marcelo Schmitt <marcelo.schmitt1@gmail.com> wrote: > LGTM, one minor suggestion inline. > With that applied, > Reviewed-by: Marcelo Schmitt <marcelo.schmitt1@gmail.com> > > On 03/22, Siddharth Menon wrote: > > Introduce struct for device match of_device_id > > > > Suggested-by: Marcelo Schmitt <marcelo.schmitt1@gmail.com> > > Signed-off-by: Siddharth Menon <simeddon@gmail.com> > > --- > > drivers/staging/iio/frequency/ad9832.c | 8 ++++++++ > > 1 file changed, 8 insertions(+) > > > > diff --git a/drivers/staging/iio/frequency/ad9832.c b/drivers/staging/iio/frequency/ad9832.c > > index 9a27acd88926..506478ddede3 100644 > > --- a/drivers/staging/iio/frequency/ad9832.c > > +++ b/drivers/staging/iio/frequency/ad9832.c > > @@ -439,6 +439,13 @@ static int ad9832_probe(struct spi_device *spi) > > return devm_iio_device_register(&spi->dev, indio_dev); > > } > > > > +static const struct of_device_id ad9832_of_match[] = { > > + { .compatible = "adi,ad9832" }, > > + { .compatible = "adi,ad9835" }, > > + {}, > I think Jonathan prefers to have a space separating the null terminator braces. > { }, Indeed, but just for the record, no trailing comma on 'terminating entries'. Whilst that's not in the style guide that is pretty much default preference across the kernel (unlike the bracket things which is my random choice :) Jonathan > > > +}; > > +MODULE_DEVICE_TABLE(of, ad9832_of_match); > > + > > static const struct spi_device_id ad9832_id[] = { > > {"ad9832", 0}, > > {"ad9835", 0}, > > @@ -449,6 +456,7 @@ MODULE_DEVICE_TABLE(spi, ad9832_id); > > static struct spi_driver ad9832_driver = { > > .driver = { > > .name = "ad9832", > > + .of_match_table = ad9832_of_match, > > }, > > .probe = ad9832_probe, > > .id_table = ad9832_id, > > -- > > 2.48.1 > >
diff --git a/drivers/staging/iio/frequency/ad9832.c b/drivers/staging/iio/frequency/ad9832.c index 9a27acd88926..506478ddede3 100644 --- a/drivers/staging/iio/frequency/ad9832.c +++ b/drivers/staging/iio/frequency/ad9832.c @@ -439,6 +439,13 @@ static int ad9832_probe(struct spi_device *spi) return devm_iio_device_register(&spi->dev, indio_dev); } +static const struct of_device_id ad9832_of_match[] = { + { .compatible = "adi,ad9832" }, + { .compatible = "adi,ad9835" }, + {}, +}; +MODULE_DEVICE_TABLE(of, ad9832_of_match); + static const struct spi_device_id ad9832_id[] = { {"ad9832", 0}, {"ad9835", 0}, @@ -449,6 +456,7 @@ MODULE_DEVICE_TABLE(spi, ad9832_id); static struct spi_driver ad9832_driver = { .driver = { .name = "ad9832", + .of_match_table = ad9832_of_match, }, .probe = ad9832_probe, .id_table = ad9832_id,
Introduce struct for device match of_device_id Suggested-by: Marcelo Schmitt <marcelo.schmitt1@gmail.com> Signed-off-by: Siddharth Menon <simeddon@gmail.com> --- drivers/staging/iio/frequency/ad9832.c | 8 ++++++++ 1 file changed, 8 insertions(+)