| Message ID | 1515961960-35157-1-git-send-email-milan.o.stevanovic@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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); -- To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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); -- To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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?
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? > -- To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. -- To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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.
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 -- To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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);
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(-)