diff mbox series

[v2] iio: frequency: ad9832: devicetree probing support

Message ID 20250322163211.253009-1-simeddon@gmail.com (mailing list archive)
State Accepted
Headers show
Series [v2] iio: frequency: ad9832: devicetree probing support | expand

Commit Message

Siddharth Menon March 22, 2025, 4:28 p.m. UTC
Introduce struct for device match of_device_id to avoid relying on fallback
mechanisms, which could lead to false matches against other AD9832 variants
in the future.

Suggested-by: Marcelo Schmitt <marcelo.schmitt1@gmail.com>
Signed-off-by: Siddharth Menon <simeddon@gmail.com>
---
 v1->v2:
 - updated commit message to be more informative
 - minor changes to code formatting
 drivers/staging/iio/frequency/ad9832.c | 8 ++++++++
 1 file changed, 8 insertions(+)

Comments

Jonathan Cameron March 23, 2025, 12:02 p.m. UTC | #1
On Sat, 22 Mar 2025 21:58:11 +0530
Siddharth Menon <simeddon@gmail.com> wrote:

> Introduce struct for device match of_device_id to avoid relying on fallback
> mechanisms, which could lead to false matches against other AD9832 variants
> in the future.
> 
> Suggested-by: Marcelo Schmitt <marcelo.schmitt1@gmail.com>
> Signed-off-by: Siddharth Menon <simeddon@gmail.com>
> ---
Marcelo gave a Reviewed-by tag.  This time I've picked it up
manually but in general it is the responsibility of the submitter
to include those on new versions. Obviously sometimes things race though so
maybe that is what happened here!

Applied

Thanks,

Jonathan


>  v1->v2:
>  - updated commit message to be more informative
>  - minor changes to code formatting
>  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 140ee4f9c137..7d4f655f6df1 100644
> --- a/drivers/staging/iio/frequency/ad9832.c
> +++ b/drivers/staging/iio/frequency/ad9832.c
> @@ -431,6 +431,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},
> @@ -441,6 +448,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,
Siddharth Menon March 23, 2025, 9:49 p.m. UTC | #2
On Sun, 23 Mar 2025 at 17:32, Jonathan Cameron <jic23@kernel.org> wrote:
> Marcelo gave a Reviewed-by tag.  This time I've picked it up
> manually but in general it is the responsibility of the submitter
> to include those on new versions. Obviously sometimes things race though so
> maybe that is what happened here!

Sorry, I was unsure whether the tag still applied after modifying a patch or
if it needed to be reviewed again.

Thanks,
Siddharth Menon
Jonathan Cameron March 30, 2025, 4:40 p.m. UTC | #3
On Mon, 24 Mar 2025 03:19:31 +0530
Siddharth Menon <simeddon@gmail.com> wrote:

> On Sun, 23 Mar 2025 at 17:32, Jonathan Cameron <jic23@kernel.org> wrote:
> > Marcelo gave a Reviewed-by tag.  This time I've picked it up
> > manually but in general it is the responsibility of the submitter
> > to include those on new versions. Obviously sometimes things race though so
> > maybe that is what happened here!  
> 
> Sorry, I was unsure whether the tag still applied after modifying a patch or
> if it needed to be reviewed again.

It is up to you as the submitter to make that decision, but if you
do drop a tag due to changes, I'd expect to see a comment under the ---
(alongside the change log) saying why.

Jonathan

> 
> Thanks,
> Siddharth Menon
Rob Herring (Arm) April 16, 2025, 6:58 p.m. UTC | #4
On Sat, Mar 22, 2025 at 09:58:11PM +0530, Siddharth Menon wrote:
> Introduce struct for device match of_device_id to avoid relying on fallback
> mechanisms, which could lead to false matches against other AD9832 variants
> in the future.
> 
> Suggested-by: Marcelo Schmitt <marcelo.schmitt1@gmail.com>
> Signed-off-by: Siddharth Menon <simeddon@gmail.com>
> ---
>  v1->v2:
>  - updated commit message to be more informative
>  - minor changes to code formatting
>  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 140ee4f9c137..7d4f655f6df1 100644
> --- a/drivers/staging/iio/frequency/ad9832.c
> +++ b/drivers/staging/iio/frequency/ad9832.c
> @@ -431,6 +431,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" },

Is there some reason we can't add a binding for this?

Rob
Jonathan Cameron April 18, 2025, 2:26 p.m. UTC | #5
On Wed, 16 Apr 2025 13:58:46 -0500
Rob Herring <robh@kernel.org> wrote:

> On Sat, Mar 22, 2025 at 09:58:11PM +0530, Siddharth Menon wrote:
> > Introduce struct for device match of_device_id to avoid relying on fallback
> > mechanisms, which could lead to false matches against other AD9832 variants
> > in the future.
> > 
> > Suggested-by: Marcelo Schmitt <marcelo.schmitt1@gmail.com>
> > Signed-off-by: Siddharth Menon <simeddon@gmail.com>
> > ---
> >  v1->v2:
> >  - updated commit message to be more informative
> >  - minor changes to code formatting
> >  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 140ee4f9c137..7d4f655f6df1 100644
> > --- a/drivers/staging/iio/frequency/ad9832.c
> > +++ b/drivers/staging/iio/frequency/ad9832.c
> > @@ -431,6 +431,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" },  
> 
> Is there some reason we can't add a binding for this?
> 
It might take some thought on how to actually describe some aspects
of this device that aren't yet supported by the driver,
but in principle we certainly should.  For drivers that have been
in staging a long time I'm a little reluctant to do merge a binding
until we are fairly sure the driver is going to move out of staging though.

>
diff mbox series

Patch

diff --git a/drivers/staging/iio/frequency/ad9832.c b/drivers/staging/iio/frequency/ad9832.c
index 140ee4f9c137..7d4f655f6df1 100644
--- a/drivers/staging/iio/frequency/ad9832.c
+++ b/drivers/staging/iio/frequency/ad9832.c
@@ -431,6 +431,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},
@@ -441,6 +448,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,