[PATCH v3 1/2] iio: adc: driver for ti adc081s/adc101s/adc121s

[PATCH v3 1/2] iio: adc: driver for ti adc081s/adc101s/adc121s

[Milan Stevanovic]

    Add Linux device driver for TI single-channel CMOS
    8/10/12-bit analog-to-digital converter with a
    high-speed serial interface.

Signed-off-by: Milan Stevanovic <milan.o.stevanovic@gmail.com>

---
Changes in v2:
     - Fix typo error
     - Keep Copyright comment
Changes in v3:
     - Split patch in two patches.
     - Second patch is license description
---
 drivers/iio/adc/ad7476.c | 23 ++++++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/adc/ad7476.c b/drivers/iio/adc/ad7476.c
index b7706bf..0ea0f90 100644
--- a/drivers/iio/adc/ad7476.c
+++ b/drivers/iio/adc/ad7476.c
@@ -1,5 +1,6 @@
 /*
- * AD7466/7/8 AD7476/5/7/8 (A) SPI ADC driver
+ * Analog Devices AD7466/7/8 AD7476/5/7/8 (A) SPI ADC driver
+ * TI ADC081S/ADC101S/ADC121S 8/10/12-bit SPI ADC driver
  *
  * Copyright 2010 Analog Devices Inc.
  *
@@ -56,6 +57,9 @@ enum ad7476_supported_device_ids {
 	ID_AD7468,
 	ID_AD7495,
 	ID_AD7940,
+	ID_ADC081S,
+	ID_ADC101S,
+	ID_ADC121S,
 };
 
 static irqreturn_t ad7476_trigger_handler(int irq, void  *p)
@@ -147,6 +151,8 @@ static int ad7476_read_raw(struct iio_dev *indio_dev,
 	},							\
 }
 
+#define ADC081S_CHAN(bits) _AD7476_CHAN((bits), 12 - (bits), \
+		BIT(IIO_CHAN_INFO_RAW))
 #define AD7476_CHAN(bits) _AD7476_CHAN((bits), 13 - (bits), \
 		BIT(IIO_CHAN_INFO_RAW))
 #define AD7940_CHAN(bits) _AD7476_CHAN((bits), 15 - (bits), \
@@ -192,6 +198,18 @@ static const struct ad7476_chip_info ad7476_chip_info_tbl[] = {
 		.channel[0] = AD7940_CHAN(14),
 		.channel[1] = IIO_CHAN_SOFT_TIMESTAMP(1),
 	},
+	[ID_ADC081S] = {
+		.channel[0] = ADC081S_CHAN(8),
+		.channel[1] = IIO_CHAN_SOFT_TIMESTAMP(1),
+	},
+	[ID_ADC101S] = {
+		.channel[0] = ADC081S_CHAN(10),
+		.channel[1] = IIO_CHAN_SOFT_TIMESTAMP(1),
+	},
+	[ID_ADC121S] = {
+		.channel[0] = ADC081S_CHAN(12),
+		.channel[1] = IIO_CHAN_SOFT_TIMESTAMP(1),
+	},
 };
 
 static const struct iio_info ad7476_info = {
@@ -294,6 +312,9 @@ static const struct spi_device_id ad7476_id[] = {
 	{"ad7910", ID_AD7467},
 	{"ad7920", ID_AD7466},
 	{"ad7940", ID_AD7940},
+	{"adc081s", ID_ADC081S},
+	{"adc101s", ID_ADC101S},
+	{"adc121s", ID_ADC121S},
 	{}
 };
 MODULE_DEVICE_TABLE(spi, ad7476_id);
-- 
2.7.4

[PATCH v3 2/2] iio: adc: change license description

[Milan Stevanovic]

   Using an SPDX tag. Remove a license notice to keep
   the whole purpose of using an SPDx id.

Signed-off-by: Milan Stevanovic <milan.o.stevanovic@gmail.com>
---
 drivers/iio/adc/ad7476.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/adc/ad7476.c b/drivers/iio/adc/ad7476.c
index 0ea0f90..4fe3cf1 100644
--- a/drivers/iio/adc/ad7476.c
+++ b/drivers/iio/adc/ad7476.c
@@ -1,10 +1,10 @@
+// SPDX-License-Identifier: GPL-2.0
 /*
  * Analog Devices AD7466/7/8 AD7476/5/7/8 (A) SPI ADC driver
  * TI ADC081S/ADC101S/ADC121S 8/10/12-bit SPI ADC driver
  *
  * Copyright 2010 Analog Devices Inc.
  *
- * Licensed under the GPL-2 or later.
  */
 
 #include <linux/device.h>
-- 
2.7.4

Re: [PATCH v3 1/2] iio: adc: driver for ti adc081s/adc101s/adc121s

[Jonathan Cameron]

On Sun, 14 Jan 2018 21:32:39 +0100
Milan Stevanovic <milan.o.stevanovic@gmail.com> wrote:

>     Add Linux device driver for TI single-channel CMOS
>     8/10/12-bit analog-to-digital converter with a
>     high-speed serial interface.
> 
> Signed-off-by: Milan Stevanovic <milan.o.stevanovic@gmail.com>
>

Great, applied to the togreg branch of iio.git and pushed out as testing
for the autobuilders to play with it.

A useful follow up for this at some point would be to put in
a proper devicetree table and document the bindings but not real
rush on that.

Thanks,

Jonathan
 
> ---
> Changes in v2:
>      - Fix typo error
>      - Keep Copyright comment
> Changes in v3:
>      - Split patch in two patches.
>      - Second patch is license description
> ---
>  drivers/iio/adc/ad7476.c | 23 ++++++++++++++++++++++-
>  1 file changed, 22 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/adc/ad7476.c b/drivers/iio/adc/ad7476.c
> index b7706bf..0ea0f90 100644
> --- a/drivers/iio/adc/ad7476.c
> +++ b/drivers/iio/adc/ad7476.c
> @@ -1,5 +1,6 @@
>  /*
> - * AD7466/7/8 AD7476/5/7/8 (A) SPI ADC driver
> + * Analog Devices AD7466/7/8 AD7476/5/7/8 (A) SPI ADC driver
> + * TI ADC081S/ADC101S/ADC121S 8/10/12-bit SPI ADC driver
>   *
>   * Copyright 2010 Analog Devices Inc.
>   *
> @@ -56,6 +57,9 @@ enum ad7476_supported_device_ids {
>  	ID_AD7468,
>  	ID_AD7495,
>  	ID_AD7940,
> +	ID_ADC081S,
> +	ID_ADC101S,
> +	ID_ADC121S,
>  };
>  
>  static irqreturn_t ad7476_trigger_handler(int irq, void  *p)
> @@ -147,6 +151,8 @@ static int ad7476_read_raw(struct iio_dev *indio_dev,
>  	},							\
>  }
>  
> +#define ADC081S_CHAN(bits) _AD7476_CHAN((bits), 12 - (bits), \
> +		BIT(IIO_CHAN_INFO_RAW))
>  #define AD7476_CHAN(bits) _AD7476_CHAN((bits), 13 - (bits), \
>  		BIT(IIO_CHAN_INFO_RAW))
>  #define AD7940_CHAN(bits) _AD7476_CHAN((bits), 15 - (bits), \
> @@ -192,6 +198,18 @@ static const struct ad7476_chip_info ad7476_chip_info_tbl[] = {
>  		.channel[0] = AD7940_CHAN(14),
>  		.channel[1] = IIO_CHAN_SOFT_TIMESTAMP(1),
>  	},
> +	[ID_ADC081S] = {
> +		.channel[0] = ADC081S_CHAN(8),
> +		.channel[1] = IIO_CHAN_SOFT_TIMESTAMP(1),
> +	},
> +	[ID_ADC101S] = {
> +		.channel[0] = ADC081S_CHAN(10),
> +		.channel[1] = IIO_CHAN_SOFT_TIMESTAMP(1),
> +	},
> +	[ID_ADC121S] = {
> +		.channel[0] = ADC081S_CHAN(12),
> +		.channel[1] = IIO_CHAN_SOFT_TIMESTAMP(1),
> +	},
>  };
>  
>  static const struct iio_info ad7476_info = {
> @@ -294,6 +312,9 @@ static const struct spi_device_id ad7476_id[] = {
>  	{"ad7910", ID_AD7467},
>  	{"ad7920", ID_AD7466},
>  	{"ad7940", ID_AD7940},
> +	{"adc081s", ID_ADC081S},
> +	{"adc101s", ID_ADC101S},
> +	{"adc121s", ID_ADC121S},
>  	{}
>  };
>  MODULE_DEVICE_TABLE(spi, ad7476_id);

Re: [PATCH v3 2/2] iio: adc: change license description

[Jonathan Cameron]

On Sun, 14 Jan 2018 21:32:40 +0100
Milan Stevanovic <milan.o.stevanovic@gmail.com> wrote:

>    Using an SPDX tag. Remove a license notice to keep
>    the whole purpose of using an SPDx id.
> 
> Signed-off-by: Milan Stevanovic <milan.o.stevanovic@gmail.com>
> ---
>  drivers/iio/adc/ad7476.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/adc/ad7476.c b/drivers/iio/adc/ad7476.c
> index 0ea0f90..4fe3cf1 100644
> --- a/drivers/iio/adc/ad7476.c
> +++ b/drivers/iio/adc/ad7476.c
> @@ -1,10 +1,10 @@
> +// SPDX-License-Identifier: GPL-2.0
>  /*
>   * Analog Devices AD7466/7/8 AD7476/5/7/8 (A) SPI ADC driver
>   * TI ADC081S/ADC101S/ADC121S 8/10/12-bit SPI ADC driver
>   *
>   * Copyright 2010 Analog Devices Inc.
>   *
> - * Licensed under the GPL-2 or later.
But this leaves a pointless blank line so good to clean that up as well.

I've done it whilst applying.  Applied to the togreg branch of iio.git
and pushed out as testing for the autobuilders to play with it.

Thanks,

Jonathan

>   */
>  
>  #include <linux/device.h>

Re: [PATCH v3 1/2] iio: adc: driver for ti adc081s/adc101s/adc121s

[Milan Stevanovic]

On 01/21/2018 01:39 PM, Jonathan Cameron wrote:
> On Sun, 14 Jan 2018 21:32:39 +0100
> Milan Stevanovic <milan.o.stevanovic@gmail.com> wrote:
>
>>      Add Linux device driver for TI single-channel CMOS
>>      8/10/12-bit analog-to-digital converter with a
>>      high-speed serial interface.
>>
>> Signed-off-by: Milan Stevanovic <milan.o.stevanovic@gmail.com>
>>
> Great, applied to the togreg branch of iio.git and pushed out as testing
> for the autobuilders to play with it.
>
> A useful follow up for this at some point would be to put in
> a proper devicetree table and document the bindings but not real
> rush on that.
>
> Thanks,
>
> Jonathan
>   
Ok.. Thanks a lot.. I will add devicetree later

Best regards
Milan
>> ---
>> Changes in v2:
>>       - Fix typo error
>>       - Keep Copyright comment
>> Changes in v3:
>>       - Split patch in two patches.
>>       - Second patch is license description
>> ---
>>   drivers/iio/adc/ad7476.c | 23 ++++++++++++++++++++++-
>>   1 file changed, 22 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/iio/adc/ad7476.c b/drivers/iio/adc/ad7476.c
>> index b7706bf..0ea0f90 100644
>> --- a/drivers/iio/adc/ad7476.c
>> +++ b/drivers/iio/adc/ad7476.c
>> @@ -1,5 +1,6 @@
>>   /*
>> - * AD7466/7/8 AD7476/5/7/8 (A) SPI ADC driver
>> + * Analog Devices AD7466/7/8 AD7476/5/7/8 (A) SPI ADC driver
>> + * TI ADC081S/ADC101S/ADC121S 8/10/12-bit SPI ADC driver
>>    *
>>    * Copyright 2010 Analog Devices Inc.
>>    *
>> @@ -56,6 +57,9 @@ enum ad7476_supported_device_ids {
>>   	ID_AD7468,
>>   	ID_AD7495,
>>   	ID_AD7940,
>> +	ID_ADC081S,
>> +	ID_ADC101S,
>> +	ID_ADC121S,
>>   };
>>   
>>   static irqreturn_t ad7476_trigger_handler(int irq, void  *p)
>> @@ -147,6 +151,8 @@ static int ad7476_read_raw(struct iio_dev *indio_dev,
>>   	},							\
>>   }
>>   
>> +#define ADC081S_CHAN(bits) _AD7476_CHAN((bits), 12 - (bits), \
>> +		BIT(IIO_CHAN_INFO_RAW))
>>   #define AD7476_CHAN(bits) _AD7476_CHAN((bits), 13 - (bits), \
>>   		BIT(IIO_CHAN_INFO_RAW))
>>   #define AD7940_CHAN(bits) _AD7476_CHAN((bits), 15 - (bits), \
>> @@ -192,6 +198,18 @@ static const struct ad7476_chip_info ad7476_chip_info_tbl[] = {
>>   		.channel[0] = AD7940_CHAN(14),
>>   		.channel[1] = IIO_CHAN_SOFT_TIMESTAMP(1),
>>   	},
>> +	[ID_ADC081S] = {
>> +		.channel[0] = ADC081S_CHAN(8),
>> +		.channel[1] = IIO_CHAN_SOFT_TIMESTAMP(1),
>> +	},
>> +	[ID_ADC101S] = {
>> +		.channel[0] = ADC081S_CHAN(10),
>> +		.channel[1] = IIO_CHAN_SOFT_TIMESTAMP(1),
>> +	},
>> +	[ID_ADC121S] = {
>> +		.channel[0] = ADC081S_CHAN(12),
>> +		.channel[1] = IIO_CHAN_SOFT_TIMESTAMP(1),
>> +	},
>>   };
>>   
>>   static const struct iio_info ad7476_info = {
>> @@ -294,6 +312,9 @@ static const struct spi_device_id ad7476_id[] = {
>>   	{"ad7910", ID_AD7467},
>>   	{"ad7920", ID_AD7466},
>>   	{"ad7940", ID_AD7940},
>> +	{"adc081s", ID_ADC081S},
>> +	{"adc101s", ID_ADC101S},
>> +	{"adc121s", ID_ADC121S},
>>   	{}
>>   };
>>   MODULE_DEVICE_TABLE(spi, ad7476_id);

Re: [PATCH v3 1/2] iio: adc: driver for ti adc081s/adc101s/adc121s

[Andy Shevchenko]

On Sun, Jan 14, 2018 at 10:32 PM, Milan Stevanovic
<milan.o.stevanovic@gmail.com> wrote:
>     Add Linux device driver for TI single-channel CMOS
>     8/10/12-bit analog-to-digital converter with a
>     high-speed serial interface.
>
> Signed-off-by: Milan Stevanovic <milan.o.stevanovic@gmail.com>

> + * Analog Devices AD7466/7/8 AD7476/5/7/8 (A) SPI ADC driver
> + * TI ADC081S/ADC101S/ADC121S 8/10/12-bit SPI ADC driver

Guys, I'm not sure I understood this mix.

We have like few TI drivers here:

drivers/iio/adc/ti-adc0832.c:352:module_spi_driver(adc0832_driver);
drivers/iio/adc/ti-adc084s021.c:269:module_spi_driver(adc084s021_driver);
drivers/iio/adc/ti-adc108s102.c:343:module_spi_driver(adc108s102_driver);
drivers/iio/adc/ti-adc12138.c:547:module_spi_driver(adc12138_driver);
drivers/iio/adc/ti-adc128s052.c:211:module_spi_driver(adc128_driver);
drivers/iio/adc/ti-adc161s626.c:276:module_spi_driver(ti_adc_driver);
drivers/iio/adc/ti-ads7950.c:519:module_spi_driver(ti_ads7950_driver);
drivers/iio/adc/ti-ads8688.c:481:module_spi_driver(ads8688_driver);
drivers/iio/adc/ti-tlc4541.c:266:module_spi_driver(tlc4541_driver);

What's wrong with them?

Is it here code duplication between two vendors?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 1/2] iio: adc: driver for ti adc081s/adc101s/adc121s

[Lars-Peter Clausen]

On 01/26/2018 07:19 PM, Andy Shevchenko wrote:
> On Sun, Jan 14, 2018 at 10:32 PM, Milan Stevanovic
> <milan.o.stevanovic@gmail.com> wrote:
>>     Add Linux device driver for TI single-channel CMOS
>>     8/10/12-bit analog-to-digital converter with a
>>     high-speed serial interface.
>>
>> Signed-off-by: Milan Stevanovic <milan.o.stevanovic@gmail.com>
> 
>> + * Analog Devices AD7466/7/8 AD7476/5/7/8 (A) SPI ADC driver
>> + * TI ADC081S/ADC101S/ADC121S 8/10/12-bit SPI ADC driver
> 
> Guys, I'm not sure I understood this mix.

You often have the case where two or even more vendors produce parts that
are (mostly) pin and register map compatible. This is typically to fulfill
the second source requirement that some customers have.

It is not uncommon to see drivers that support parts from different vendors.

> 
> We have like few TI drivers here:
> 
> drivers/iio/adc/ti-adc0832.c:352:module_spi_driver(adc0832_driver);
> drivers/iio/adc/ti-adc084s021.c:269:module_spi_driver(adc084s021_driver);
> drivers/iio/adc/ti-adc108s102.c:343:module_spi_driver(adc108s102_driver);
> drivers/iio/adc/ti-adc12138.c:547:module_spi_driver(adc12138_driver);
> drivers/iio/adc/ti-adc128s052.c:211:module_spi_driver(adc128_driver);
> drivers/iio/adc/ti-adc161s626.c:276:module_spi_driver(ti_adc_driver);
> drivers/iio/adc/ti-ads7950.c:519:module_spi_driver(ti_ads7950_driver);
> drivers/iio/adc/ti-ads8688.c:481:module_spi_driver(ads8688_driver);
> drivers/iio/adc/ti-tlc4541.c:266:module_spi_driver(tlc4541_driver);
> 
> What's wrong with them?

They are probably not register map compatible with those other drivers. (Or
nobody cared to check if they are register map compatible).

> 
> Is it here code duplication between two vendors?
> 

Re: [PATCH v3 1/2] iio: adc: driver for ti adc081s/adc101s/adc121s

[Lars-Peter Clausen]

On 01/26/2018 07:25 PM, Lars-Peter Clausen wrote:
> On 01/26/2018 07:19 PM, Andy Shevchenko wrote:
>> On Sun, Jan 14, 2018 at 10:32 PM, Milan Stevanovic
>> <milan.o.stevanovic@gmail.com> wrote:
>>>     Add Linux device driver for TI single-channel CMOS
>>>     8/10/12-bit analog-to-digital converter with a
>>>     high-speed serial interface.
>>>
>>> Signed-off-by: Milan Stevanovic <milan.o.stevanovic@gmail.com>
>>
>>> + * Analog Devices AD7466/7/8 AD7476/5/7/8 (A) SPI ADC driver
>>> + * TI ADC081S/ADC101S/ADC121S 8/10/12-bit SPI ADC driver
>>
>> Guys, I'm not sure I understood this mix.
> 
> You often have the case where two or even more vendors produce parts that
> are (mostly) pin and register map compatible. This is typically to fulfill
> the second source requirement that some customers have.
> 
> It is not uncommon to see drivers that support parts from different vendors.

And I should probably add that this driver is for devices which are single
channel ADCs with a SPI interface and no configuration registers. There are
only so many ways to implement this (And I ensure you hardware designers
will definitely create implementations of all of those limited ways), so
there will probably be overlap between vendors.

- Lars

Re: [PATCH v3 1/2] iio: adc: driver for ti adc081s/adc101s/adc121s

[Lars-Peter Clausen]

On 01/26/2018 07:19 PM, Andy Shevchenko wrote:
> On Sun, Jan 14, 2018 at 10:32 PM, Milan Stevanovic
> <milan.o.stevanovic@gmail.com> wrote:
>>     Add Linux device driver for TI single-channel CMOS
>>     8/10/12-bit analog-to-digital converter with a
>>     high-speed serial interface.
>>
>> Signed-off-by: Milan Stevanovic <milan.o.stevanovic@gmail.com>
> 
>> + * Analog Devices AD7466/7/8 AD7476/5/7/8 (A) SPI ADC driver
>> + * TI ADC081S/ADC101S/ADC121S 8/10/12-bit SPI ADC driver
> 
> Guys, I'm not sure I understood this mix.
> 
> We have like few TI drivers here:
> 
> drivers/iio/adc/ti-adc0832.c:352:module_spi_driver(adc0832_driver);
> drivers/iio/adc/ti-adc084s021.c:269:module_spi_driver(adc084s021_driver);
> drivers/iio/adc/ti-adc108s102.c:343:module_spi_driver(adc108s102_driver);
> drivers/iio/adc/ti-adc12138.c:547:module_spi_driver(adc12138_driver);
> drivers/iio/adc/ti-adc128s052.c:211:module_spi_driver(adc128_driver);
> drivers/iio/adc/ti-adc161s626.c:276:module_spi_driver(ti_adc_driver);
> drivers/iio/adc/ti-ads7950.c:519:module_spi_driver(ti_ads7950_driver);
> drivers/iio/adc/ti-ads8688.c:481:module_spi_driver(ads8688_driver);
> drivers/iio/adc/ti-tlc4541.c:266:module_spi_driver(tlc4541_driver);
> 
> What's wrong with them?
> 
> Is it here code duplication between two vendors?
> 

Looking at this list ti-adc161s626 could probably be combined with the
ad7476 driver. It fits the bill, single channel ADC with SPI interface and
no control registers.

Re: [PATCH v3 1/2] iio: adc: driver for ti adc081s/adc101s/adc121s

[Andy Shevchenko]

On Fri, Jan 26, 2018 at 8:25 PM, Lars-Peter Clausen <lars@metafoo.de> wrote:
> On 01/26/2018 07:19 PM, Andy Shevchenko wrote:
>> On Sun, Jan 14, 2018 at 10:32 PM, Milan Stevanovic
>> <milan.o.stevanovic@gmail.com> wrote:
>>>     Add Linux device driver for TI single-channel CMOS
>>>     8/10/12-bit analog-to-digital converter with a
>>>     high-speed serial interface.
>>>
>>> Signed-off-by: Milan Stevanovic <milan.o.stevanovic@gmail.com>
>>
>>> + * Analog Devices AD7466/7/8 AD7476/5/7/8 (A) SPI ADC driver
>>> + * TI ADC081S/ADC101S/ADC121S 8/10/12-bit SPI ADC driver
>>
>> Guys, I'm not sure I understood this mix.
>
> You often have the case where two or even more vendors produce parts that
> are (mostly) pin and register map compatible. This is typically to fulfill
> the second source requirement that some customers have.
>
> It is not uncommon to see drivers that support parts from different vendors.

Yep, though in this case we have, it seems, a counterpart (i2c
variant) in drivers/iio/adc/ti-adc081c.c

>> We have like few TI drivers here:
>>
>> drivers/iio/adc/ti-adc0832.c:352:module_spi_driver(adc0832_driver);
>> drivers/iio/adc/ti-adc084s021.c:269:module_spi_driver(adc084s021_driver);
>> drivers/iio/adc/ti-adc108s102.c:343:module_spi_driver(adc108s102_driver);
>> drivers/iio/adc/ti-adc12138.c:547:module_spi_driver(adc12138_driver);
>> drivers/iio/adc/ti-adc128s052.c:211:module_spi_driver(adc128_driver);
>> drivers/iio/adc/ti-adc161s626.c:276:module_spi_driver(ti_adc_driver);
>> drivers/iio/adc/ti-ads7950.c:519:module_spi_driver(ti_ads7950_driver);
>> drivers/iio/adc/ti-ads8688.c:481:module_spi_driver(ads8688_driver);
>> drivers/iio/adc/ti-tlc4541.c:266:module_spi_driver(tlc4541_driver);
>>
>> What's wrong with them?
>
> They are probably not register map compatible with those other drivers. (Or
> nobody cared to check if they are register map compatible).

I would believe in the latter than former.

>> Is it here code duplication between two vendors?

...and instead of doing such mix I would really rather have a separate
glue driver to the same code.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 1/2] iio: adc: driver for ti adc081s/adc101s/adc121s

[Milan Stevanovic]

On 01/26/2018 07:43 PM, Andy Shevchenko wrote:
> On Fri, Jan 26, 2018 at 8:25 PM, Lars-Peter Clausen <lars@metafoo.de> wrote:
>> On 01/26/2018 07:19 PM, Andy Shevchenko wrote:
>>> On Sun, Jan 14, 2018 at 10:32 PM, Milan Stevanovic
>>> <milan.o.stevanovic@gmail.com> wrote:
>>>>      Add Linux device driver for TI single-channel CMOS
>>>>      8/10/12-bit analog-to-digital converter with a
>>>>      high-speed serial interface.
>>>>
>>>> Signed-off-by: Milan Stevanovic <milan.o.stevanovic@gmail.com>
>>>> + * Analog Devices AD7466/7/8 AD7476/5/7/8 (A) SPI ADC driver
>>>> + * TI ADC081S/ADC101S/ADC121S 8/10/12-bit SPI ADC driver
>>> Guys, I'm not sure I understood this mix.
>> You often have the case where two or even more vendors produce parts that
>> are (mostly) pin and register map compatible. This is typically to fulfill
>> the second source requirement that some customers have.
>>
>> It is not uncommon to see drivers that support parts from different vendors.
> Yep, though in this case we have, it seems, a counterpart (i2c
> variant) in drivers/iio/adc/ti-adc081c.c
>
>>> We have like few TI drivers here:
>>>
>>> drivers/iio/adc/ti-adc0832.c:352:module_spi_driver(adc0832_driver);
>>> drivers/iio/adc/ti-adc084s021.c:269:module_spi_driver(adc084s021_driver);
>>> drivers/iio/adc/ti-adc108s102.c:343:module_spi_driver(adc108s102_driver);
>>> drivers/iio/adc/ti-adc12138.c:547:module_spi_driver(adc12138_driver);
>>> drivers/iio/adc/ti-adc128s052.c:211:module_spi_driver(adc128_driver);
>>> drivers/iio/adc/ti-adc161s626.c:276:module_spi_driver(ti_adc_driver);
>>> drivers/iio/adc/ti-ads7950.c:519:module_spi_driver(ti_ads7950_driver);
>>> drivers/iio/adc/ti-ads8688.c:481:module_spi_driver(ads8688_driver);
>>> drivers/iio/adc/ti-tlc4541.c:266:module_spi_driver(tlc4541_driver);
>>>
>>> What's wrong with them?
>> They are probably not register map compatible with those other drivers. (Or
>> nobody cared to check if they are register map compatible).
> I would believe in the latter than former.
>
>>> Is it here code duplication between two vendors?
> ...and instead of doing such mix I would really rather have a separate
> glue driver to the same code.
>
I spoke about this with Jonathan. Generally we can share a few lines
here and there but not enough to overcome the fact that the
drivers just became a lot less readable.
There are comments for that in patch/10132693