diff mbox series

[v7,3/3] staging: iio: ad2s1210: Add device tree table.

Message ID ee142522b129dd6fd8d71b5567d9c6711e89efbe.1541000375.git.nishadkamdar@gmail.com (mailing list archive)
State New, archived
Headers show
Series staging: iio: ad2s1210: Switch to the gpio descriptor interface. | expand

Commit Message

Nishad Kamdar Oct. 31, 2018, 4 p.m. UTC
Add device tree table for matching vendor ID.

Signed-off-by: Nishad Kamdar <nishadkamdar@gmail.com>
---
 drivers/staging/iio/resolver/ad2s1210.c | 7 +++++++
 1 file changed, 7 insertions(+)

Comments

Himanshu Jha Nov. 1, 2018, 3:35 p.m. UTC | #1
On Wed, Oct 31, 2018 at 09:30:36PM +0530, Nishad Kamdar wrote:
> Add device tree table for matching vendor ID.
> 
> Signed-off-by: Nishad Kamdar <nishadkamdar@gmail.com>
> ---
>  drivers/staging/iio/resolver/ad2s1210.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/staging/iio/resolver/ad2s1210.c b/drivers/staging/iio/resolver/ad2s1210.c
> index d3e7d5aad2c8..7c50def91a2b 100644
> --- a/drivers/staging/iio/resolver/ad2s1210.c
> +++ b/drivers/staging/iio/resolver/ad2s1210.c
> @@ -701,6 +701,12 @@ static int ad2s1210_remove(struct spi_device *spi)
>  	return 0;
>  }
>  
> +static const struct of_device_id ad2s1210_of_match[] = {
> +	{ .compatible = "adi,ad2s1210", },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, ad2s1210_of_match);

I believe this needs to be documented at:

Documentation/devicetree/bindings/iio/resolver/ad2s1210.txt

Cc'ed to devictree list + Rob(DT Maintainer).

Just wondering why didn't it came up till now from the IIO reviewers ? v7!!
Jonathan Cameron Nov. 3, 2018, 12:39 p.m. UTC | #2
On Thu, 1 Nov 2018 21:05:09 +0530
Himanshu Jha <himanshujha199640@gmail.com> wrote:

> On Wed, Oct 31, 2018 at 09:30:36PM +0530, Nishad Kamdar wrote:
> > Add device tree table for matching vendor ID.
> > 
> > Signed-off-by: Nishad Kamdar <nishadkamdar@gmail.com>
> > ---
> >  drivers/staging/iio/resolver/ad2s1210.c | 7 +++++++
> >  1 file changed, 7 insertions(+)
> > 
> > diff --git a/drivers/staging/iio/resolver/ad2s1210.c b/drivers/staging/iio/resolver/ad2s1210.c
> > index d3e7d5aad2c8..7c50def91a2b 100644
> > --- a/drivers/staging/iio/resolver/ad2s1210.c
> > +++ b/drivers/staging/iio/resolver/ad2s1210.c
> > @@ -701,6 +701,12 @@ static int ad2s1210_remove(struct spi_device *spi)
> >  	return 0;
> >  }
> >  
> > +static const struct of_device_id ad2s1210_of_match[] = {
> > +	{ .compatible = "adi,ad2s1210", },
> > +	{ }
> > +};
> > +MODULE_DEVICE_TABLE(of, ad2s1210_of_match);  
> 
> I believe this needs to be documented at:
> 
> Documentation/devicetree/bindings/iio/resolver/ad2s1210.txt
> 
> Cc'ed to devictree list + Rob(DT Maintainer).
> 
> Just wondering why didn't it came up till now from the IIO reviewers ? v7!!

Because in staging drivers graduations we often hold off doing the
dt-bindings document until we have full visibility of where we are going.

A lot of them have dodgy DT bindings (and that might even be the reason
they are in staging).  What we don't want is to have a doc for a silly
binding in the 'official' list as we'll have to support it for ever.

It needs documenting before moving out staging, but not necessarily now.
Particularly as this device is complex and has a 'lot' of other stuff
that isn't currently supported and quite possibly never will be.
Some of that would have non obvious dt bindings if we did support it.
For example we 'might' route the encoder outputs round to the inputs
of a counter driver and end up with a complex entity representing
the facilities that both fo them provide.

Agreed, the DT binding doc needs to come soon and before the move out
staging, but I am quite happy with it being in the next series.

A line in the description to that effect would have been useful of
course!

Jonathan

> 
>
Jonathan Cameron Nov. 3, 2018, 1:04 p.m. UTC | #3
On Sat, 3 Nov 2018 12:39:27 +0000
Jonathan Cameron <jic23@kernel.org> wrote:

> On Thu, 1 Nov 2018 21:05:09 +0530
> Himanshu Jha <himanshujha199640@gmail.com> wrote:
> 
> > On Wed, Oct 31, 2018 at 09:30:36PM +0530, Nishad Kamdar wrote:  
> > > Add device tree table for matching vendor ID.
> > > 
> > > Signed-off-by: Nishad Kamdar <nishadkamdar@gmail.com>
> > > ---
> > >  drivers/staging/iio/resolver/ad2s1210.c | 7 +++++++
> > >  1 file changed, 7 insertions(+)
> > > 
> > > diff --git a/drivers/staging/iio/resolver/ad2s1210.c b/drivers/staging/iio/resolver/ad2s1210.c
> > > index d3e7d5aad2c8..7c50def91a2b 100644
> > > --- a/drivers/staging/iio/resolver/ad2s1210.c
> > > +++ b/drivers/staging/iio/resolver/ad2s1210.c
> > > @@ -701,6 +701,12 @@ static int ad2s1210_remove(struct spi_device *spi)
> > >  	return 0;
> > >  }
> > >  
> > > +static const struct of_device_id ad2s1210_of_match[] = {
> > > +	{ .compatible = "adi,ad2s1210", },
> > > +	{ }
> > > +};
> > > +MODULE_DEVICE_TABLE(of, ad2s1210_of_match);    
> > 
> > I believe this needs to be documented at:
> > 
> > Documentation/devicetree/bindings/iio/resolver/ad2s1210.txt
> > 
> > Cc'ed to devictree list + Rob(DT Maintainer).
> > 
> > Just wondering why didn't it came up till now from the IIO reviewers ? v7!!  
> 
> Because in staging drivers graduations we often hold off doing the
> dt-bindings document until we have full visibility of where we are going.
> 
> A lot of them have dodgy DT bindings (and that might even be the reason
> they are in staging).  What we don't want is to have a doc for a silly
> binding in the 'official' list as we'll have to support it for ever.
> 
> It needs documenting before moving out staging, but not necessarily now.
> Particularly as this device is complex and has a 'lot' of other stuff
> that isn't currently supported and quite possibly never will be.
> Some of that would have non obvious dt bindings if we did support it.
> For example we 'might' route the encoder outputs round to the inputs
> of a counter driver and end up with a complex entity representing
> the facilities that both fo them provide.
> 
> Agreed, the DT binding doc needs to come soon and before the move out
> staging, but I am quite happy with it being in the next series.
> 
> A line in the description to that effect would have been useful of
> course!
> 
Applied, with a line on the intent to document once driver is cleaned
up added.

Thanks,

Jonathan
> Jonathan
> 
> > 
> >   
>
diff mbox series

Patch

diff --git a/drivers/staging/iio/resolver/ad2s1210.c b/drivers/staging/iio/resolver/ad2s1210.c
index d3e7d5aad2c8..7c50def91a2b 100644
--- a/drivers/staging/iio/resolver/ad2s1210.c
+++ b/drivers/staging/iio/resolver/ad2s1210.c
@@ -701,6 +701,12 @@  static int ad2s1210_remove(struct spi_device *spi)
 	return 0;
 }
 
+static const struct of_device_id ad2s1210_of_match[] = {
+	{ .compatible = "adi,ad2s1210", },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, ad2s1210_of_match);
+
 static const struct spi_device_id ad2s1210_id[] = {
 	{ "ad2s1210" },
 	{}
@@ -710,6 +716,7 @@  MODULE_DEVICE_TABLE(spi, ad2s1210_id);
 static struct spi_driver ad2s1210_driver = {
 	.driver = {
 		.name = DRV_NAME,
+		.of_match_table = of_match_ptr(ad2s1210_of_match),
 	},
 	.probe = ad2s1210_probe,
 	.remove = ad2s1210_remove,