| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CA+U=DsprCm6GpPSROAN9dC5De4=ivhY9m_75Kn-h=jWpw-5JfA@mail.gmail.com>
Date: Mon, 28 Jan 2019 09:47:34 +0200
From: Alexandru Ardelean <ardeleanalex@...il.com>
To: Ricardo Ribalda Delgado <ricardo@...alda.com>
Cc: Jonathan Cameron <jic23@...nel.org>,
LKML <linux-kernel@...r.kernel.org>,
alexandru.ardelean@...log.com, linux-iio@...r.kernel.org
Subject: Re: [PATCH v2] iio: adc: ad7476: Add support for ADS786X ADCs
On Mon, Jan 28, 2019 at 9:43 AM Ricardo Ribalda Delgado
<ricardo@...alda.com> wrote:
>
> HI Alexandru
>
> On Mon, Jan 28, 2019 at 8:38 AM Alexandru Ardelean
> <ardeleanalex@...il.com> wrote:
> >
> > On Sat, Jan 26, 2019 at 8:21 PM Jonathan Cameron <jic23@...nel.org> wrote:
> > >
> > > On Fri, 25 Jan 2019 11:04:51 +0100
> > > Ricardo Ribalda Delgado <ricardo@...alda.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@...alda.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
Powered by blists - more mailing lists