Re: [PATCH] iio: adc: ad7476: Add support for ADS786X ADCs

[Ricardo Ribalda Delgado]

Hi Alexandru

On Fri, Jan 25, 2019 at 9:29 AM Alexandru Ardelean
<ardeleanalex@xxxxxxxxx> wrote:
>
> On Thu, Jan 24, 2019 at 11:33 PM Ricardo Ribalda Delgado
> <ricardo@xxxxxxxxxxx> wrote:
> >
> > Add support for ADS7866, ADS7867 and ADS7868 8/10/12 bit Single channel
> > ADC.
> >
> > Datasheet: http://www.ti.com/product/ADS7866
> > Datasheet: http://www.ti.com/product/ADS7867
> > Datasheet: http://www.ti.com/product/ADS7868
> >
> > Signed-off-by: Ricardo Ribalda Delgado <ricardo@xxxxxxxxxxx>
> > ---
> >  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).
>
> The driver is for Analog Devices parts.
> And these new parts are from TI.
>
> I'm not sure what the policy is for mixing manufacturers in drivers.
> But if that is allowed/fine, then maybe it would be a good idea to
> highlight that these chips are from another manufacturer.
> Or maybe rename the driver to a generic form to support both manufacturers ?
>
> In any case, Jonathan will have to tell us what to do here :)

Thanks for your review.

I have to send a v2, because I have inverted the name of the devices :S.

I will send it right away to avoid that anyone uses this version and
then wait for Jonathan feedback

>
> Thanks
> Alex
>
> >
> >           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..108c18d64f7d 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) + 4, 0, \
> > +               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(8),
> > +               .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(12),
> > +               .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
> >



-- 
Ricardo Ribalda
http://ribalda.com