HI Alexandru On Mon, Jan 28, 2019 at 8:47 AM Alexandru Ardelean <ardeleanalex@xxxxxxxxx> wrote: > > On Mon, Jan 28, 2019 at 9:43 AM Ricardo Ribalda Delgado > <ricardo@xxxxxxxxxxx> wrote: > > > > HI Alexandru > > > > On Mon, Jan 28, 2019 at 8:38 AM Alexandru Ardelean > > <ardeleanalex@xxxxxxxxx> wrote: > > > > > > On Sat, Jan 26, 2019 at 8:21 PM Jonathan Cameron <jic23@xxxxxxxxxx> wrote: > > > > > > > > On Fri, 25 Jan 2019 11:04:51 +0100 > > > > Ricardo Ribalda Delgado <ricardo@xxxxxxxxxxx> 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@xxxxxxxxxxx> > > > > > --- > > > > > 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
