* [PATCH v2] iio: adc: ad7476: Add support for ADS786X ADCs
@ 2019-01-25 10:04 Ricardo Ribalda Delgado
2019-01-26 18:21 ` Jonathan Cameron
0 siblings, 1 reply; 6+ messages in thread
From: Ricardo Ribalda Delgado @ 2019-01-25 10:04 UTC (permalink / raw)
To: Alexandru Ardelean, Jonathan Cameron, LKML, alexandru.ardelean,
linux-iio
Cc: Ricardo Ribalda Delgado
Add support for ADS7866, ADS7867 and ADS7868 8/10/12 bit Single channel
ADC.
Datasheet: http://www.ti.com/lit/ds/symlink/ads7868.pdf
Signed-off-by: Ricardo Ribalda Delgado <ricardo@ribalda.com>
---
v2: I have missnamed the devices
drivers/iio/adc/Kconfig | 3 ++-
drivers/iio/adc/ad7476.c | 20 ++++++++++++++++++++
2 files changed, 22 insertions(+), 1 deletion(-)
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index f9354e5ee65c..d86900fc2634 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -64,7 +64,8 @@ config AD7476
help
Say yes here to build support for Analog Devices AD7273, AD7274, AD7276,
AD7277, AD7278, AD7475, AD7476, AD7477, AD7478, AD7466, AD7467, AD7468,
- AD7495, AD7910, AD7920, AD7920 SPI analog to digital converters (ADC).
+ AD7495, AD7910, AD7920, AD7920, ADS7866, ADS7867, ADS7868 SPI analog
+ to digital converters (ADC).
To compile this driver as a module, choose M here: the
module will be called ad7476.
diff --git a/drivers/iio/adc/ad7476.c b/drivers/iio/adc/ad7476.c
index 0549686b9ef8..76747488044b 100644
--- a/drivers/iio/adc/ad7476.c
+++ b/drivers/iio/adc/ad7476.c
@@ -59,6 +59,9 @@ enum ad7476_supported_device_ids {
ID_ADC081S,
ID_ADC101S,
ID_ADC121S,
+ ID_ADS7866,
+ ID_ADS7867,
+ ID_ADS7868,
};
static irqreturn_t ad7476_trigger_handler(int irq, void *p)
@@ -157,6 +160,8 @@ static int ad7476_read_raw(struct iio_dev *indio_dev,
#define AD7940_CHAN(bits) _AD7476_CHAN((bits), 15 - (bits), \
BIT(IIO_CHAN_INFO_RAW))
#define AD7091R_CHAN(bits) _AD7476_CHAN((bits), 16 - (bits), 0)
+#define ADS786X_CHAN(bits) _AD7476_CHAN((bits), 12 - (bits), \
+ BIT(IIO_CHAN_INFO_RAW))
static const struct ad7476_chip_info ad7476_chip_info_tbl[] = {
[ID_AD7091R] = {
@@ -209,6 +214,18 @@ static const struct ad7476_chip_info ad7476_chip_info_tbl[] = {
.channel[0] = ADC081S_CHAN(12),
.channel[1] = IIO_CHAN_SOFT_TIMESTAMP(1),
},
+ [ID_ADS7866] = {
+ .channel[0] = ADS786X_CHAN(12),
+ .channel[1] = IIO_CHAN_SOFT_TIMESTAMP(1),
+ },
+ [ID_ADS7867] = {
+ .channel[0] = ADS786X_CHAN(10),
+ .channel[1] = IIO_CHAN_SOFT_TIMESTAMP(1),
+ },
+ [ID_ADS7868] = {
+ .channel[0] = ADS786X_CHAN(8),
+ .channel[1] = IIO_CHAN_SOFT_TIMESTAMP(1),
+ },
};
static const struct iio_info ad7476_info = {
@@ -314,6 +331,9 @@ static const struct spi_device_id ad7476_id[] = {
{"adc081s", ID_ADC081S},
{"adc101s", ID_ADC101S},
{"adc121s", ID_ADC121S},
+ {"ads7866", ID_ADS7866},
+ {"ads7867", ID_ADS7867},
+ {"ads7868", ID_ADS7868},
{}
};
MODULE_DEVICE_TABLE(spi, ad7476_id);
--
2.20.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2] iio: adc: ad7476: Add support for ADS786X ADCs
2019-01-25 10:04 [PATCH v2] iio: adc: ad7476: Add support for ADS786X ADCs Ricardo Ribalda Delgado
@ 2019-01-26 18:21 ` Jonathan Cameron
2019-01-28 7:38 ` Alexandru Ardelean
0 siblings, 1 reply; 6+ messages in thread
From: Jonathan Cameron @ 2019-01-26 18:21 UTC (permalink / raw)
To: Ricardo Ribalda Delgado
Cc: Alexandru Ardelean, LKML, alexandru.ardelean, linux-iio
On Fri, 25 Jan 2019 11:04:51 +0100
Ricardo Ribalda Delgado <ricardo@ribalda.com> wrote:
> Add support for ADS7866, ADS7867 and ADS7868 8/10/12 bit Single channel
> ADC.
>
> Datasheet: http://www.ti.com/lit/ds/symlink/ads7868.pdf
>
> Signed-off-by: Ricardo Ribalda Delgado <ricardo@ribalda.com>
> ---
> v2: I have missnamed the devices
> drivers/iio/adc/Kconfig | 3 ++-
> drivers/iio/adc/ad7476.c | 20 ++++++++++++++++++++
> 2 files changed, 22 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index f9354e5ee65c..d86900fc2634 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -64,7 +64,8 @@ config AD7476
> help
> Say yes here to build support for Analog Devices AD7273, AD7274, AD7276,
> AD7277, AD7278, AD7475, AD7476, AD7477, AD7478, AD7466, AD7467, AD7468,
> - AD7495, AD7910, AD7920, AD7920 SPI analog to digital converters (ADC).
> + AD7495, AD7910, AD7920, AD7920, ADS7866, ADS7867, ADS7868 SPI analog
> + to digital converters (ADC).
As commented in the earlier thread (after you sent this!), please make sure
the help text makes it clear that these aren't all Analog devices parts.
Likely to cause confusion otherwise.
Good to see you didn't spin another driver but instead tracked down that
this one would work well!
Thanks,
Jonathan
>
> To compile this driver as a module, choose M here: the
> module will be called ad7476.
> diff --git a/drivers/iio/adc/ad7476.c b/drivers/iio/adc/ad7476.c
> index 0549686b9ef8..76747488044b 100644
> --- a/drivers/iio/adc/ad7476.c
> +++ b/drivers/iio/adc/ad7476.c
> @@ -59,6 +59,9 @@ enum ad7476_supported_device_ids {
> ID_ADC081S,
> ID_ADC101S,
> ID_ADC121S,
> + ID_ADS7866,
> + ID_ADS7867,
> + ID_ADS7868,
> };
>
> static irqreturn_t ad7476_trigger_handler(int irq, void *p)
> @@ -157,6 +160,8 @@ static int ad7476_read_raw(struct iio_dev *indio_dev,
> #define AD7940_CHAN(bits) _AD7476_CHAN((bits), 15 - (bits), \
> BIT(IIO_CHAN_INFO_RAW))
> #define AD7091R_CHAN(bits) _AD7476_CHAN((bits), 16 - (bits), 0)
> +#define ADS786X_CHAN(bits) _AD7476_CHAN((bits), 12 - (bits), \
> + BIT(IIO_CHAN_INFO_RAW))
>
> static const struct ad7476_chip_info ad7476_chip_info_tbl[] = {
> [ID_AD7091R] = {
> @@ -209,6 +214,18 @@ static const struct ad7476_chip_info ad7476_chip_info_tbl[] = {
> .channel[0] = ADC081S_CHAN(12),
> .channel[1] = IIO_CHAN_SOFT_TIMESTAMP(1),
> },
> + [ID_ADS7866] = {
> + .channel[0] = ADS786X_CHAN(12),
> + .channel[1] = IIO_CHAN_SOFT_TIMESTAMP(1),
> + },
> + [ID_ADS7867] = {
> + .channel[0] = ADS786X_CHAN(10),
> + .channel[1] = IIO_CHAN_SOFT_TIMESTAMP(1),
> + },
> + [ID_ADS7868] = {
> + .channel[0] = ADS786X_CHAN(8),
> + .channel[1] = IIO_CHAN_SOFT_TIMESTAMP(1),
> + },
> };
>
> static const struct iio_info ad7476_info = {
> @@ -314,6 +331,9 @@ static const struct spi_device_id ad7476_id[] = {
> {"adc081s", ID_ADC081S},
> {"adc101s", ID_ADC101S},
> {"adc121s", ID_ADC121S},
> + {"ads7866", ID_ADS7866},
> + {"ads7867", ID_ADS7867},
> + {"ads7868", ID_ADS7868},
> {}
> };
> MODULE_DEVICE_TABLE(spi, ad7476_id);
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] iio: adc: ad7476: Add support for ADS786X ADCs
2019-01-26 18:21 ` Jonathan Cameron
@ 2019-01-28 7:38 ` Alexandru Ardelean
2019-01-28 7:42 ` Ricardo Ribalda Delgado
0 siblings, 1 reply; 6+ messages in thread
From: Alexandru Ardelean @ 2019-01-28 7:38 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Ricardo Ribalda Delgado, LKML, alexandru.ardelean, linux-iio
On Sat, Jan 26, 2019 at 8:21 PM Jonathan Cameron <jic23@kernel.org> wrote:
>
> On Fri, 25 Jan 2019 11:04:51 +0100
> Ricardo Ribalda Delgado <ricardo@ribalda.com> wrote:
>
> > Add support for ADS7866, ADS7867 and ADS7868 8/10/12 bit Single channel
> > ADC.
> >
I don't want this reply be offensive or anything.
But since I've seen this precedence, I do have to ask if these driver
changes were tested on at least one of the TI chips/eval-boards.
Ideally, before adding new drivers it would be nice to make sure that
they were run on actual HW.
Good code-review can get us only so far.
We could ask someone who has any of these chips to test.
Or maybe some eval boards from TI could be obtained to test these changes.
Overall changes seem good [I haven't taken a close look], but HW
testing is king.
Thanks
Alex
> > Datasheet: http://www.ti.com/lit/ds/symlink/ads7868.pdf
> >
> > Signed-off-by: Ricardo Ribalda Delgado <ricardo@ribalda.com>
> > ---
> > v2: I have missnamed the devices
> > drivers/iio/adc/Kconfig | 3 ++-
> > drivers/iio/adc/ad7476.c | 20 ++++++++++++++++++++
> > 2 files changed, 22 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> > index f9354e5ee65c..d86900fc2634 100644
> > --- a/drivers/iio/adc/Kconfig
> > +++ b/drivers/iio/adc/Kconfig
> > @@ -64,7 +64,8 @@ config AD7476
> > help
> > Say yes here to build support for Analog Devices AD7273, AD7274, AD7276,
> > AD7277, AD7278, AD7475, AD7476, AD7477, AD7478, AD7466, AD7467, AD7468,
> > - AD7495, AD7910, AD7920, AD7920 SPI analog to digital converters (ADC).
> > + AD7495, AD7910, AD7920, AD7920, ADS7866, ADS7867, ADS7868 SPI analog
> > + to digital converters (ADC).
>
> As commented in the earlier thread (after you sent this!), please make sure
> the help text makes it clear that these aren't all Analog devices parts.
>
> Likely to cause confusion otherwise.
>
> Good to see you didn't spin another driver but instead tracked down that
> this one would work well!
>
> Thanks,
>
> Jonathan
>
> >
> > To compile this driver as a module, choose M here: the
> > module will be called ad7476.
> > diff --git a/drivers/iio/adc/ad7476.c b/drivers/iio/adc/ad7476.c
> > index 0549686b9ef8..76747488044b 100644
> > --- a/drivers/iio/adc/ad7476.c
> > +++ b/drivers/iio/adc/ad7476.c
> > @@ -59,6 +59,9 @@ enum ad7476_supported_device_ids {
> > ID_ADC081S,
> > ID_ADC101S,
> > ID_ADC121S,
> > + ID_ADS7866,
> > + ID_ADS7867,
> > + ID_ADS7868,
> > };
> >
> > static irqreturn_t ad7476_trigger_handler(int irq, void *p)
> > @@ -157,6 +160,8 @@ static int ad7476_read_raw(struct iio_dev *indio_dev,
> > #define AD7940_CHAN(bits) _AD7476_CHAN((bits), 15 - (bits), \
> > BIT(IIO_CHAN_INFO_RAW))
> > #define AD7091R_CHAN(bits) _AD7476_CHAN((bits), 16 - (bits), 0)
> > +#define ADS786X_CHAN(bits) _AD7476_CHAN((bits), 12 - (bits), \
> > + BIT(IIO_CHAN_INFO_RAW))
> >
> > static const struct ad7476_chip_info ad7476_chip_info_tbl[] = {
> > [ID_AD7091R] = {
> > @@ -209,6 +214,18 @@ static const struct ad7476_chip_info ad7476_chip_info_tbl[] = {
> > .channel[0] = ADC081S_CHAN(12),
> > .channel[1] = IIO_CHAN_SOFT_TIMESTAMP(1),
> > },
> > + [ID_ADS7866] = {
> > + .channel[0] = ADS786X_CHAN(12),
> > + .channel[1] = IIO_CHAN_SOFT_TIMESTAMP(1),
> > + },
> > + [ID_ADS7867] = {
> > + .channel[0] = ADS786X_CHAN(10),
> > + .channel[1] = IIO_CHAN_SOFT_TIMESTAMP(1),
> > + },
> > + [ID_ADS7868] = {
> > + .channel[0] = ADS786X_CHAN(8),
> > + .channel[1] = IIO_CHAN_SOFT_TIMESTAMP(1),
> > + },
> > };
> >
> > static const struct iio_info ad7476_info = {
> > @@ -314,6 +331,9 @@ static const struct spi_device_id ad7476_id[] = {
> > {"adc081s", ID_ADC081S},
> > {"adc101s", ID_ADC101S},
> > {"adc121s", ID_ADC121S},
> > + {"ads7866", ID_ADS7866},
> > + {"ads7867", ID_ADS7867},
> > + {"ads7868", ID_ADS7868},
> > {}
> > };
> > MODULE_DEVICE_TABLE(spi, ad7476_id);
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] iio: adc: ad7476: Add support for ADS786X ADCs
2019-01-28 7:38 ` Alexandru Ardelean
@ 2019-01-28 7:42 ` Ricardo Ribalda Delgado
2019-01-28 7:47 ` Alexandru Ardelean
0 siblings, 1 reply; 6+ messages in thread
From: Ricardo Ribalda Delgado @ 2019-01-28 7:42 UTC (permalink / raw)
To: Alexandru Ardelean; +Cc: Jonathan Cameron, LKML, alexandru.ardelean, linux-iio
HI Alexandru
On Mon, Jan 28, 2019 at 8:38 AM Alexandru Ardelean
<ardeleanalex@gmail.com> wrote:
>
> On Sat, Jan 26, 2019 at 8:21 PM Jonathan Cameron <jic23@kernel.org> wrote:
> >
> > On Fri, 25 Jan 2019 11:04:51 +0100
> > Ricardo Ribalda Delgado <ricardo@ribalda.com> wrote:
> >
> > > Add support for ADS7866, ADS7867 and ADS7868 8/10/12 bit Single channel
> > > ADC.
> > >
>
> I don't want this reply be offensive or anything.
> But since I've seen this precedence, I do have to ask if these driver
> changes were tested on at least one of the TI chips/eval-boards.
>
> Ideally, before adding new drivers it would be nice to make sure that
> they were run on actual HW.
> Good code-review can get us only so far.
>
> We could ask someone who has any of these chips to test.
> Or maybe some eval boards from TI could be obtained to test these changes.
>
> Overall changes seem good [I haven't taken a close look], but HW
> testing is king.
>
I have access to ADS7868 (the 8 bit version) which has been connected
in "loopback" to the ti-dac7612 (also new driver). I can run any test
that you need or even give you or someone access (privately) to the
board via vpn
or something like that.
Would that work for you?
Best regards!
> Thanks
> Alex
>
> > > Datasheet: http://www.ti.com/lit/ds/symlink/ads7868.pdf
> > >
> > > Signed-off-by: Ricardo Ribalda Delgado <ricardo@ribalda.com>
> > > ---
> > > v2: I have missnamed the devices
> > > drivers/iio/adc/Kconfig | 3 ++-
> > > drivers/iio/adc/ad7476.c | 20 ++++++++++++++++++++
> > > 2 files changed, 22 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> > > index f9354e5ee65c..d86900fc2634 100644
> > > --- a/drivers/iio/adc/Kconfig
> > > +++ b/drivers/iio/adc/Kconfig
> > > @@ -64,7 +64,8 @@ config AD7476
> > > help
> > > Say yes here to build support for Analog Devices AD7273, AD7274, AD7276,
> > > AD7277, AD7278, AD7475, AD7476, AD7477, AD7478, AD7466, AD7467, AD7468,
> > > - AD7495, AD7910, AD7920, AD7920 SPI analog to digital converters (ADC).
> > > + AD7495, AD7910, AD7920, AD7920, ADS7866, ADS7867, ADS7868 SPI analog
> > > + to digital converters (ADC).
> >
> > As commented in the earlier thread (after you sent this!), please make sure
> > the help text makes it clear that these aren't all Analog devices parts.
> >
> > Likely to cause confusion otherwise.
> >
> > Good to see you didn't spin another driver but instead tracked down that
> > this one would work well!
> >
> > Thanks,
> >
> > Jonathan
> >
> > >
> > > To compile this driver as a module, choose M here: the
> > > module will be called ad7476.
> > > diff --git a/drivers/iio/adc/ad7476.c b/drivers/iio/adc/ad7476.c
> > > index 0549686b9ef8..76747488044b 100644
> > > --- a/drivers/iio/adc/ad7476.c
> > > +++ b/drivers/iio/adc/ad7476.c
> > > @@ -59,6 +59,9 @@ enum ad7476_supported_device_ids {
> > > ID_ADC081S,
> > > ID_ADC101S,
> > > ID_ADC121S,
> > > + ID_ADS7866,
> > > + ID_ADS7867,
> > > + ID_ADS7868,
> > > };
> > >
> > > static irqreturn_t ad7476_trigger_handler(int irq, void *p)
> > > @@ -157,6 +160,8 @@ static int ad7476_read_raw(struct iio_dev *indio_dev,
> > > #define AD7940_CHAN(bits) _AD7476_CHAN((bits), 15 - (bits), \
> > > BIT(IIO_CHAN_INFO_RAW))
> > > #define AD7091R_CHAN(bits) _AD7476_CHAN((bits), 16 - (bits), 0)
> > > +#define ADS786X_CHAN(bits) _AD7476_CHAN((bits), 12 - (bits), \
> > > + BIT(IIO_CHAN_INFO_RAW))
> > >
> > > static const struct ad7476_chip_info ad7476_chip_info_tbl[] = {
> > > [ID_AD7091R] = {
> > > @@ -209,6 +214,18 @@ static const struct ad7476_chip_info ad7476_chip_info_tbl[] = {
> > > .channel[0] = ADC081S_CHAN(12),
> > > .channel[1] = IIO_CHAN_SOFT_TIMESTAMP(1),
> > > },
> > > + [ID_ADS7866] = {
> > > + .channel[0] = ADS786X_CHAN(12),
> > > + .channel[1] = IIO_CHAN_SOFT_TIMESTAMP(1),
> > > + },
> > > + [ID_ADS7867] = {
> > > + .channel[0] = ADS786X_CHAN(10),
> > > + .channel[1] = IIO_CHAN_SOFT_TIMESTAMP(1),
> > > + },
> > > + [ID_ADS7868] = {
> > > + .channel[0] = ADS786X_CHAN(8),
> > > + .channel[1] = IIO_CHAN_SOFT_TIMESTAMP(1),
> > > + },
> > > };
> > >
> > > static const struct iio_info ad7476_info = {
> > > @@ -314,6 +331,9 @@ static const struct spi_device_id ad7476_id[] = {
> > > {"adc081s", ID_ADC081S},
> > > {"adc101s", ID_ADC101S},
> > > {"adc121s", ID_ADC121S},
> > > + {"ads7866", ID_ADS7866},
> > > + {"ads7867", ID_ADS7867},
> > > + {"ads7868", ID_ADS7868},
> > > {}
> > > };
> > > MODULE_DEVICE_TABLE(spi, ad7476_id);
> >
--
Ricardo Ribalda
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] iio: adc: ad7476: Add support for ADS786X ADCs
2019-01-28 7:42 ` Ricardo Ribalda Delgado
@ 2019-01-28 7:47 ` Alexandru Ardelean
2019-01-28 7:51 ` Ricardo Ribalda Delgado
0 siblings, 1 reply; 6+ messages in thread
From: Alexandru Ardelean @ 2019-01-28 7:47 UTC (permalink / raw)
To: Ricardo Ribalda Delgado
Cc: Jonathan Cameron, LKML, alexandru.ardelean, linux-iio
On Mon, Jan 28, 2019 at 9:43 AM Ricardo Ribalda Delgado
<ricardo@ribalda.com> wrote:
>
> HI Alexandru
>
> On Mon, Jan 28, 2019 at 8:38 AM Alexandru Ardelean
> <ardeleanalex@gmail.com> wrote:
> >
> > On Sat, Jan 26, 2019 at 8:21 PM Jonathan Cameron <jic23@kernel.org> wrote:
> > >
> > > On Fri, 25 Jan 2019 11:04:51 +0100
> > > Ricardo Ribalda Delgado <ricardo@ribalda.com> wrote:
> > >
> > > > Add support for ADS7866, ADS7867 and ADS7868 8/10/12 bit Single channel
> > > > ADC.
> > > >
> >
> > I don't want this reply be offensive or anything.
> > But since I've seen this precedence, I do have to ask if these driver
> > changes were tested on at least one of the TI chips/eval-boards.
> >
> > Ideally, before adding new drivers it would be nice to make sure that
> > they were run on actual HW.
> > Good code-review can get us only so far.
> >
> > We could ask someone who has any of these chips to test.
> > Or maybe some eval boards from TI could be obtained to test these changes.
> >
> > Overall changes seem good [I haven't taken a close look], but HW
> > testing is king.
> >
>
> I have access to ADS7868 (the 8 bit version) which has been connected
> in "loopback" to the ti-dac7612 (also new driver). I can run any test
> that you need or even give you or someone access (privately) to the
> board via vpn
> or something like that.
>
> Would that work for you?
No, that's fine :)
Thank you for the answer.
And apologies [again] if this was a bit too forward/harsh from my side.
Thanks
Alex
>
> Best regards!
>
> > Thanks
> > Alex
> >
> > > > Datasheet: http://www.ti.com/lit/ds/symlink/ads7868.pdf
> > > >
> > > > Signed-off-by: Ricardo Ribalda Delgado <ricardo@ribalda.com>
> > > > ---
> > > > v2: I have missnamed the devices
> > > > drivers/iio/adc/Kconfig | 3 ++-
> > > > drivers/iio/adc/ad7476.c | 20 ++++++++++++++++++++
> > > > 2 files changed, 22 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> > > > index f9354e5ee65c..d86900fc2634 100644
> > > > --- a/drivers/iio/adc/Kconfig
> > > > +++ b/drivers/iio/adc/Kconfig
> > > > @@ -64,7 +64,8 @@ config AD7476
> > > > help
> > > > Say yes here to build support for Analog Devices AD7273, AD7274, AD7276,
> > > > AD7277, AD7278, AD7475, AD7476, AD7477, AD7478, AD7466, AD7467, AD7468,
> > > > - AD7495, AD7910, AD7920, AD7920 SPI analog to digital converters (ADC).
> > > > + AD7495, AD7910, AD7920, AD7920, ADS7866, ADS7867, ADS7868 SPI analog
> > > > + to digital converters (ADC).
> > >
> > > As commented in the earlier thread (after you sent this!), please make sure
> > > the help text makes it clear that these aren't all Analog devices parts.
> > >
> > > Likely to cause confusion otherwise.
> > >
> > > Good to see you didn't spin another driver but instead tracked down that
> > > this one would work well!
> > >
> > > Thanks,
> > >
> > > Jonathan
> > >
> > > >
> > > > To compile this driver as a module, choose M here: the
> > > > module will be called ad7476.
> > > > diff --git a/drivers/iio/adc/ad7476.c b/drivers/iio/adc/ad7476.c
> > > > index 0549686b9ef8..76747488044b 100644
> > > > --- a/drivers/iio/adc/ad7476.c
> > > > +++ b/drivers/iio/adc/ad7476.c
> > > > @@ -59,6 +59,9 @@ enum ad7476_supported_device_ids {
> > > > ID_ADC081S,
> > > > ID_ADC101S,
> > > > ID_ADC121S,
> > > > + ID_ADS7866,
> > > > + ID_ADS7867,
> > > > + ID_ADS7868,
> > > > };
> > > >
> > > > static irqreturn_t ad7476_trigger_handler(int irq, void *p)
> > > > @@ -157,6 +160,8 @@ static int ad7476_read_raw(struct iio_dev *indio_dev,
> > > > #define AD7940_CHAN(bits) _AD7476_CHAN((bits), 15 - (bits), \
> > > > BIT(IIO_CHAN_INFO_RAW))
> > > > #define AD7091R_CHAN(bits) _AD7476_CHAN((bits), 16 - (bits), 0)
> > > > +#define ADS786X_CHAN(bits) _AD7476_CHAN((bits), 12 - (bits), \
> > > > + BIT(IIO_CHAN_INFO_RAW))
> > > >
> > > > static const struct ad7476_chip_info ad7476_chip_info_tbl[] = {
> > > > [ID_AD7091R] = {
> > > > @@ -209,6 +214,18 @@ static const struct ad7476_chip_info ad7476_chip_info_tbl[] = {
> > > > .channel[0] = ADC081S_CHAN(12),
> > > > .channel[1] = IIO_CHAN_SOFT_TIMESTAMP(1),
> > > > },
> > > > + [ID_ADS7866] = {
> > > > + .channel[0] = ADS786X_CHAN(12),
> > > > + .channel[1] = IIO_CHAN_SOFT_TIMESTAMP(1),
> > > > + },
> > > > + [ID_ADS7867] = {
> > > > + .channel[0] = ADS786X_CHAN(10),
> > > > + .channel[1] = IIO_CHAN_SOFT_TIMESTAMP(1),
> > > > + },
> > > > + [ID_ADS7868] = {
> > > > + .channel[0] = ADS786X_CHAN(8),
> > > > + .channel[1] = IIO_CHAN_SOFT_TIMESTAMP(1),
> > > > + },
> > > > };
> > > >
> > > > static const struct iio_info ad7476_info = {
> > > > @@ -314,6 +331,9 @@ static const struct spi_device_id ad7476_id[] = {
> > > > {"adc081s", ID_ADC081S},
> > > > {"adc101s", ID_ADC101S},
> > > > {"adc121s", ID_ADC121S},
> > > > + {"ads7866", ID_ADS7866},
> > > > + {"ads7867", ID_ADS7867},
> > > > + {"ads7868", ID_ADS7868},
> > > > {}
> > > > };
> > > > MODULE_DEVICE_TABLE(spi, ad7476_id);
> > >
>
>
>
> --
> Ricardo Ribalda
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] iio: adc: ad7476: Add support for ADS786X ADCs
2019-01-28 7:47 ` Alexandru Ardelean
@ 2019-01-28 7:51 ` Ricardo Ribalda Delgado
0 siblings, 0 replies; 6+ messages in thread
From: Ricardo Ribalda Delgado @ 2019-01-28 7:51 UTC (permalink / raw)
To: Alexandru Ardelean; +Cc: Jonathan Cameron, LKML, alexandru.ardelean, linux-iio
HI Alexandru
On Mon, Jan 28, 2019 at 8:47 AM Alexandru Ardelean
<ardeleanalex@gmail.com> wrote:
>
> On Mon, Jan 28, 2019 at 9:43 AM Ricardo Ribalda Delgado
> <ricardo@ribalda.com> wrote:
> >
> > HI Alexandru
> >
> > On Mon, Jan 28, 2019 at 8:38 AM Alexandru Ardelean
> > <ardeleanalex@gmail.com> wrote:
> > >
> > > On Sat, Jan 26, 2019 at 8:21 PM Jonathan Cameron <jic23@kernel.org> wrote:
> > > >
> > > > On Fri, 25 Jan 2019 11:04:51 +0100
> > > > Ricardo Ribalda Delgado <ricardo@ribalda.com> wrote:
> > > >
> > > > > Add support for ADS7866, ADS7867 and ADS7868 8/10/12 bit Single channel
> > > > > ADC.
> > > > >
> > >
> > > I don't want this reply be offensive or anything.
> > > But since I've seen this precedence, I do have to ask if these driver
> > > changes were tested on at least one of the TI chips/eval-boards.
> > >
> > > Ideally, before adding new drivers it would be nice to make sure that
> > > they were run on actual HW.
> > > Good code-review can get us only so far.
> > >
> > > We could ask someone who has any of these chips to test.
> > > Or maybe some eval boards from TI could be obtained to test these changes.
> > >
> > > Overall changes seem good [I haven't taken a close look], but HW
> > > testing is king.
> > >
> >
> > I have access to ADS7868 (the 8 bit version) which has been connected
> > in "loopback" to the ti-dac7612 (also new driver). I can run any test
> > that you need or even give you or someone access (privately) to the
> > board via vpn
> > or something like that.
> >
> > Would that work for you?
>
> No, that's fine :)
> Thank you for the answer.
> And apologies [again] if this was a bit too forward/harsh from my side.
>
Absolutely no worries! it wasn't. When I use a driver from kernel.org
I expect it
to have the maximum quality and work on real hardware. This is how we get there.
Best regards!
> Thanks
> Alex
>
> >
> > Best regards!
> >
> > > Thanks
> > > Alex
> > >
> > > > > Datasheet: http://www.ti.com/lit/ds/symlink/ads7868.pdf
> > > > >
> > > > > Signed-off-by: Ricardo Ribalda Delgado <ricardo@ribalda.com>
> > > > > ---
> > > > > v2: I have missnamed the devices
> > > > > drivers/iio/adc/Kconfig | 3 ++-
> > > > > drivers/iio/adc/ad7476.c | 20 ++++++++++++++++++++
> > > > > 2 files changed, 22 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> > > > > index f9354e5ee65c..d86900fc2634 100644
> > > > > --- a/drivers/iio/adc/Kconfig
> > > > > +++ b/drivers/iio/adc/Kconfig
> > > > > @@ -64,7 +64,8 @@ config AD7476
> > > > > help
> > > > > Say yes here to build support for Analog Devices AD7273, AD7274, AD7276,
> > > > > AD7277, AD7278, AD7475, AD7476, AD7477, AD7478, AD7466, AD7467, AD7468,
> > > > > - AD7495, AD7910, AD7920, AD7920 SPI analog to digital converters (ADC).
> > > > > + AD7495, AD7910, AD7920, AD7920, ADS7866, ADS7867, ADS7868 SPI analog
> > > > > + to digital converters (ADC).
> > > >
> > > > As commented in the earlier thread (after you sent this!), please make sure
> > > > the help text makes it clear that these aren't all Analog devices parts.
> > > >
> > > > Likely to cause confusion otherwise.
> > > >
> > > > Good to see you didn't spin another driver but instead tracked down that
> > > > this one would work well!
> > > >
> > > > Thanks,
> > > >
> > > > Jonathan
> > > >
> > > > >
> > > > > To compile this driver as a module, choose M here: the
> > > > > module will be called ad7476.
> > > > > diff --git a/drivers/iio/adc/ad7476.c b/drivers/iio/adc/ad7476.c
> > > > > index 0549686b9ef8..76747488044b 100644
> > > > > --- a/drivers/iio/adc/ad7476.c
> > > > > +++ b/drivers/iio/adc/ad7476.c
> > > > > @@ -59,6 +59,9 @@ enum ad7476_supported_device_ids {
> > > > > ID_ADC081S,
> > > > > ID_ADC101S,
> > > > > ID_ADC121S,
> > > > > + ID_ADS7866,
> > > > > + ID_ADS7867,
> > > > > + ID_ADS7868,
> > > > > };
> > > > >
> > > > > static irqreturn_t ad7476_trigger_handler(int irq, void *p)
> > > > > @@ -157,6 +160,8 @@ static int ad7476_read_raw(struct iio_dev *indio_dev,
> > > > > #define AD7940_CHAN(bits) _AD7476_CHAN((bits), 15 - (bits), \
> > > > > BIT(IIO_CHAN_INFO_RAW))
> > > > > #define AD7091R_CHAN(bits) _AD7476_CHAN((bits), 16 - (bits), 0)
> > > > > +#define ADS786X_CHAN(bits) _AD7476_CHAN((bits), 12 - (bits), \
> > > > > + BIT(IIO_CHAN_INFO_RAW))
> > > > >
> > > > > static const struct ad7476_chip_info ad7476_chip_info_tbl[] = {
> > > > > [ID_AD7091R] = {
> > > > > @@ -209,6 +214,18 @@ static const struct ad7476_chip_info ad7476_chip_info_tbl[] = {
> > > > > .channel[0] = ADC081S_CHAN(12),
> > > > > .channel[1] = IIO_CHAN_SOFT_TIMESTAMP(1),
> > > > > },
> > > > > + [ID_ADS7866] = {
> > > > > + .channel[0] = ADS786X_CHAN(12),
> > > > > + .channel[1] = IIO_CHAN_SOFT_TIMESTAMP(1),
> > > > > + },
> > > > > + [ID_ADS7867] = {
> > > > > + .channel[0] = ADS786X_CHAN(10),
> > > > > + .channel[1] = IIO_CHAN_SOFT_TIMESTAMP(1),
> > > > > + },
> > > > > + [ID_ADS7868] = {
> > > > > + .channel[0] = ADS786X_CHAN(8),
> > > > > + .channel[1] = IIO_CHAN_SOFT_TIMESTAMP(1),
> > > > > + },
> > > > > };
> > > > >
> > > > > static const struct iio_info ad7476_info = {
> > > > > @@ -314,6 +331,9 @@ static const struct spi_device_id ad7476_id[] = {
> > > > > {"adc081s", ID_ADC081S},
> > > > > {"adc101s", ID_ADC101S},
> > > > > {"adc121s", ID_ADC121S},
> > > > > + {"ads7866", ID_ADS7866},
> > > > > + {"ads7867", ID_ADS7867},
> > > > > + {"ads7868", ID_ADS7868},
> > > > > {}
> > > > > };
> > > > > MODULE_DEVICE_TABLE(spi, ad7476_id);
> > > >
> >
> >
> >
> > --
> > Ricardo Ribalda
--
Ricardo Ribalda
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-01-28 7:51 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-25 10:04 [PATCH v2] iio: adc: ad7476: Add support for ADS786X ADCs Ricardo Ribalda Delgado
2019-01-26 18:21 ` Jonathan Cameron
2019-01-28 7:38 ` Alexandru Ardelean
2019-01-28 7:42 ` Ricardo Ribalda Delgado
2019-01-28 7:47 ` Alexandru Ardelean
2019-01-28 7:51 ` Ricardo Ribalda Delgado
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).