| Message ID | 20240919130444.2100447-2-aardelean@baylibre.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | iio: adc: ad7606: add support for AD7606C-{16,18} parts | expand |
On Thu, Sep 19, 2024 at 3:04 PM Alexandru Ardelean <aardelean@baylibre.com> wrote: > > There are some newer additions to the AD7606 family, which support 18 bit > precision. Up until now, all chips were 16 bit. > > This change adds a 'bits' parameter to the AD760X_CHANNEL macro and renames > 'ad7606_channels' -> 'ad7606_channels_16bit' for the current devices. > > The AD7606_SW_CHANNEL() macro is also introduced, as a short-hand for IIO > channels in SW mode. > > Signed-off-by: Alexandru Ardelean <aardelean@baylibre.com> > --- ... > diff --git a/drivers/iio/adc/ad7606.h b/drivers/iio/adc/ad7606.h > index 6649e84d25de..204a343067e5 100644 > --- a/drivers/iio/adc/ad7606.h > +++ b/drivers/iio/adc/ad7606.h > @@ -8,7 +8,7 @@ > #ifndef IIO_ADC_AD7606_H_ > #define IIO_ADC_AD7606_H_ > > -#define AD760X_CHANNEL(num, mask_sep, mask_type, mask_all) { \ > +#define AD760X_CHANNEL(num, mask_sep, mask_type, mask_all, bits) { \ > .type = IIO_VOLTAGE, \ > .indexed = 1, \ > .channel = num, \ > @@ -19,24 +19,26 @@ > .scan_index = num, \ > .scan_type = { \ > .sign = 's', \ > - .realbits = 16, \ > - .storagebits = 16, \ > + .realbits = (bits), \ > + .storagebits = (bits) > 16 ? 32 : 16, \ > .endianness = IIO_CPU, \ > }, \ > } > > #define AD7605_CHANNEL(num) \ > AD760X_CHANNEL(num, BIT(IIO_CHAN_INFO_RAW), \ > - BIT(IIO_CHAN_INFO_SCALE), 0) > + BIT(IIO_CHAN_INFO_SCALE), 0, 16) > > -#define AD7606_CHANNEL(num) \ > +#define AD7606_CHANNEL(num, bits) \ > AD760X_CHANNEL(num, BIT(IIO_CHAN_INFO_RAW), \ > BIT(IIO_CHAN_INFO_SCALE), \ > - BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO)) > + BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO), bits) > > -#define AD7616_CHANNEL(num) \ > +#define AD7606_SW_CHANNEL(num, bits) \ > AD760X_CHANNEL(num, BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),\ > - 0, BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO)) > + 0, BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO), bits) > + > +#define AD7616_CHANNEL(num) AD7606_SW_CHANNEL(num, 16) It looks like the AD7616_CHANNEL macro is no longer used, so can be dropped. Or alternately, don't change the lines below to use AD7606_SW_CHANNEL. With either of those changes: Reviewed-by: David Lechner <dlechner@baylibre.com> > > /** > * struct ad7606_chip_info - chip specific information > diff --git a/drivers/iio/adc/ad7606_spi.c b/drivers/iio/adc/ad7606_spi.c > index 62ec12195307..e00f58a6a0e9 100644 > --- a/drivers/iio/adc/ad7606_spi.c > +++ b/drivers/iio/adc/ad7606_spi.c > @@ -67,14 +67,14 @@ static const struct iio_chan_spec ad7616_sw_channels[] = { > > static const struct iio_chan_spec ad7606b_sw_channels[] = { > IIO_CHAN_SOFT_TIMESTAMP(8), > - AD7616_CHANNEL(0), > - AD7616_CHANNEL(1), > - AD7616_CHANNEL(2), > - AD7616_CHANNEL(3), > - AD7616_CHANNEL(4), > - AD7616_CHANNEL(5), > - AD7616_CHANNEL(6), > - AD7616_CHANNEL(7), > + AD7606_SW_CHANNEL(0, 16), > + AD7606_SW_CHANNEL(1, 16), > + AD7606_SW_CHANNEL(2, 16), > + AD7606_SW_CHANNEL(3, 16), > + AD7606_SW_CHANNEL(4, 16), > + AD7606_SW_CHANNEL(5, 16), > + AD7606_SW_CHANNEL(6, 16), > + AD7606_SW_CHANNEL(7, 16), > }; > > static const unsigned int ad7606B_oversampling_avail[9] = { > -- > 2.46.0 >
On Mon, Sep 23, 2024 at 4:51 PM David Lechner <dlechner@baylibre.com> wrote: > > On Thu, Sep 19, 2024 at 3:04 PM Alexandru Ardelean > <aardelean@baylibre.com> wrote: > > > > There are some newer additions to the AD7606 family, which support 18 bit > > precision. Up until now, all chips were 16 bit. > > > > This change adds a 'bits' parameter to the AD760X_CHANNEL macro and renames > > 'ad7606_channels' -> 'ad7606_channels_16bit' for the current devices. > > > > The AD7606_SW_CHANNEL() macro is also introduced, as a short-hand for IIO > > channels in SW mode. > > > > Signed-off-by: Alexandru Ardelean <aardelean@baylibre.com> > > --- > > ... > > > diff --git a/drivers/iio/adc/ad7606.h b/drivers/iio/adc/ad7606.h > > index 6649e84d25de..204a343067e5 100644 > > --- a/drivers/iio/adc/ad7606.h > > +++ b/drivers/iio/adc/ad7606.h > > @@ -8,7 +8,7 @@ > > #ifndef IIO_ADC_AD7606_H_ > > #define IIO_ADC_AD7606_H_ > > > > -#define AD760X_CHANNEL(num, mask_sep, mask_type, mask_all) { \ > > +#define AD760X_CHANNEL(num, mask_sep, mask_type, mask_all, bits) { \ > > .type = IIO_VOLTAGE, \ > > .indexed = 1, \ > > .channel = num, \ > > @@ -19,24 +19,26 @@ > > .scan_index = num, \ > > .scan_type = { \ > > .sign = 's', \ > > - .realbits = 16, \ > > - .storagebits = 16, \ > > + .realbits = (bits), \ > > + .storagebits = (bits) > 16 ? 32 : 16, \ > > .endianness = IIO_CPU, \ > > }, \ > > } > > > > #define AD7605_CHANNEL(num) \ > > AD760X_CHANNEL(num, BIT(IIO_CHAN_INFO_RAW), \ > > - BIT(IIO_CHAN_INFO_SCALE), 0) > > + BIT(IIO_CHAN_INFO_SCALE), 0, 16) > > > > -#define AD7606_CHANNEL(num) \ > > +#define AD7606_CHANNEL(num, bits) \ > > AD760X_CHANNEL(num, BIT(IIO_CHAN_INFO_RAW), \ > > BIT(IIO_CHAN_INFO_SCALE), \ > > - BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO)) > > + BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO), bits) > > > > -#define AD7616_CHANNEL(num) \ > > +#define AD7606_SW_CHANNEL(num, bits) \ > > AD760X_CHANNEL(num, BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),\ > > - 0, BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO)) > > + 0, BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO), bits) > > + > > +#define AD7616_CHANNEL(num) AD7606_SW_CHANNEL(num, 16) > > It looks like the AD7616_CHANNEL macro is no longer used, so can be > dropped. Or alternately, don't change the lines below to use > AD7606_SW_CHANNEL. Well, the AD7616_CHANNEL() macro is still being used for the actual AD7616 channels. For the AD7606B software channels, the AD7616_CHANNEL() macro was being re-used, which seemed like a bit of a lazy/convenient way to do it. The patch here, just cleans up that minor quirk, but just for AD7606B. The AD7616 driver part, still uses the AD7616_CHANNEL() macro. Though, interestingly, the AD7616_CHANNEL() macro is only used for SW channels. Maybe in a subsequent patch, the AD7616_CHANNEL() macro can be removed altogether. I thought about doing it in this series, but decided against it, to keep the series small. > > With either of those changes: > > Reviewed-by: David Lechner <dlechner@baylibre.com> > > > > > /** > > * struct ad7606_chip_info - chip specific information > > diff --git a/drivers/iio/adc/ad7606_spi.c b/drivers/iio/adc/ad7606_spi.c > > index 62ec12195307..e00f58a6a0e9 100644 > > --- a/drivers/iio/adc/ad7606_spi.c > > +++ b/drivers/iio/adc/ad7606_spi.c > > @@ -67,14 +67,14 @@ static const struct iio_chan_spec ad7616_sw_channels[] = { > > > > static const struct iio_chan_spec ad7606b_sw_channels[] = { > > IIO_CHAN_SOFT_TIMESTAMP(8), > > - AD7616_CHANNEL(0), > > - AD7616_CHANNEL(1), > > - AD7616_CHANNEL(2), > > - AD7616_CHANNEL(3), > > - AD7616_CHANNEL(4), > > - AD7616_CHANNEL(5), > > - AD7616_CHANNEL(6), > > - AD7616_CHANNEL(7), > > + AD7606_SW_CHANNEL(0, 16), > > + AD7606_SW_CHANNEL(1, 16), > > + AD7606_SW_CHANNEL(2, 16), > > + AD7606_SW_CHANNEL(3, 16), > > + AD7606_SW_CHANNEL(4, 16), > > + AD7606_SW_CHANNEL(5, 16), > > + AD7606_SW_CHANNEL(6, 16), > > + AD7606_SW_CHANNEL(7, 16), > > }; > > > > static const unsigned int ad7606B_oversampling_avail[9] = { > > -- > > 2.46.0 > >
On Tue, 24 Sep 2024 10:11:38 +0200 Alexandru Ardelean <aardelean@baylibre.com> wrote: > On Mon, Sep 23, 2024 at 4:51 PM David Lechner <dlechner@baylibre.com> wrote: > > > > On Thu, Sep 19, 2024 at 3:04 PM Alexandru Ardelean > > <aardelean@baylibre.com> wrote: > > > > > > There are some newer additions to the AD7606 family, which support 18 bit > > > precision. Up until now, all chips were 16 bit. > > > > > > This change adds a 'bits' parameter to the AD760X_CHANNEL macro and renames > > > 'ad7606_channels' -> 'ad7606_channels_16bit' for the current devices. > > > > > > The AD7606_SW_CHANNEL() macro is also introduced, as a short-hand for IIO > > > channels in SW mode. > > > > > > Signed-off-by: Alexandru Ardelean <aardelean@baylibre.com> > > > --- > > > > ... > > > > > diff --git a/drivers/iio/adc/ad7606.h b/drivers/iio/adc/ad7606.h > > > index 6649e84d25de..204a343067e5 100644 > > > --- a/drivers/iio/adc/ad7606.h > > > +++ b/drivers/iio/adc/ad7606.h > > > @@ -8,7 +8,7 @@ > > > #ifndef IIO_ADC_AD7606_H_ > > > #define IIO_ADC_AD7606_H_ > > > > > > -#define AD760X_CHANNEL(num, mask_sep, mask_type, mask_all) { \ > > > +#define AD760X_CHANNEL(num, mask_sep, mask_type, mask_all, bits) { \ > > > .type = IIO_VOLTAGE, \ > > > .indexed = 1, \ > > > .channel = num, \ > > > @@ -19,24 +19,26 @@ > > > .scan_index = num, \ > > > .scan_type = { \ > > > .sign = 's', \ > > > - .realbits = 16, \ > > > - .storagebits = 16, \ > > > + .realbits = (bits), \ > > > + .storagebits = (bits) > 16 ? 32 : 16, \ > > > .endianness = IIO_CPU, \ > > > }, \ > > > } > > > > > > #define AD7605_CHANNEL(num) \ > > > AD760X_CHANNEL(num, BIT(IIO_CHAN_INFO_RAW), \ > > > - BIT(IIO_CHAN_INFO_SCALE), 0) > > > + BIT(IIO_CHAN_INFO_SCALE), 0, 16) > > > > > > -#define AD7606_CHANNEL(num) \ > > > +#define AD7606_CHANNEL(num, bits) \ > > > AD760X_CHANNEL(num, BIT(IIO_CHAN_INFO_RAW), \ > > > BIT(IIO_CHAN_INFO_SCALE), \ > > > - BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO)) > > > + BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO), bits) > > > > > > -#define AD7616_CHANNEL(num) \ > > > +#define AD7606_SW_CHANNEL(num, bits) \ > > > AD760X_CHANNEL(num, BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),\ > > > - 0, BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO)) > > > + 0, BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO), bits) > > > + > > > +#define AD7616_CHANNEL(num) AD7606_SW_CHANNEL(num, 16) > > > > It looks like the AD7616_CHANNEL macro is no longer used, so can be > > dropped. Or alternately, don't change the lines below to use > > AD7606_SW_CHANNEL. > > Well, the AD7616_CHANNEL() macro is still being used for the actual > AD7616 channels. > For the AD7606B software channels, the AD7616_CHANNEL() macro was > being re-used, which seemed like a bit of a lazy/convenient way to do > it. > > The patch here, just cleans up that minor quirk, but just for AD7606B. > The AD7616 driver part, still uses the AD7616_CHANNEL() macro. > > Though, interestingly, the AD7616_CHANNEL() macro is only used for SW channels. > Maybe in a subsequent patch, the AD7616_CHANNEL() macro can be removed > altogether. > I thought about doing it in this series, but decided against it, to > keep the series small. It's been a while, and David hasn't said he disagrees with this argument so I've picked up David's tag whilst applying this. So David, if you'd rather I didn't then shout! Series applied to the togreg branch of iio.git and pushed out as testing for all the normal reasons + I'm waiting for rc1 to rebase on. Thanks Jonathan > > > > > With either of those changes: > > > > Reviewed-by: David Lechner <dlechner@baylibre.com> > > > > > > > > /** > > > * struct ad7606_chip_info - chip specific information > > > diff --git a/drivers/iio/adc/ad7606_spi.c b/drivers/iio/adc/ad7606_spi.c > > > index 62ec12195307..e00f58a6a0e9 100644 > > > --- a/drivers/iio/adc/ad7606_spi.c > > > +++ b/drivers/iio/adc/ad7606_spi.c > > > @@ -67,14 +67,14 @@ static const struct iio_chan_spec ad7616_sw_channels[] = { > > > > > > static const struct iio_chan_spec ad7606b_sw_channels[] = { > > > IIO_CHAN_SOFT_TIMESTAMP(8), > > > - AD7616_CHANNEL(0), > > > - AD7616_CHANNEL(1), > > > - AD7616_CHANNEL(2), > > > - AD7616_CHANNEL(3), > > > - AD7616_CHANNEL(4), > > > - AD7616_CHANNEL(5), > > > - AD7616_CHANNEL(6), > > > - AD7616_CHANNEL(7), > > > + AD7606_SW_CHANNEL(0, 16), > > > + AD7606_SW_CHANNEL(1, 16), > > > + AD7606_SW_CHANNEL(2, 16), > > > + AD7606_SW_CHANNEL(3, 16), > > > + AD7606_SW_CHANNEL(4, 16), > > > + AD7606_SW_CHANNEL(5, 16), > > > + AD7606_SW_CHANNEL(6, 16), > > > + AD7606_SW_CHANNEL(7, 16), > > > }; > > > > > > static const unsigned int ad7606B_oversampling_avail[9] = { > > > -- > > > 2.46.0 > > >
diff --git a/drivers/iio/adc/ad7606.c b/drivers/iio/adc/ad7606.c index 9b457472d49c..8ebfe8abc3f4 100644 --- a/drivers/iio/adc/ad7606.c +++ b/drivers/iio/adc/ad7606.c @@ -309,16 +309,16 @@ static const struct iio_chan_spec ad7605_channels[] = { AD7605_CHANNEL(3), }; -static const struct iio_chan_spec ad7606_channels[] = { +static const struct iio_chan_spec ad7606_channels_16bit[] = { IIO_CHAN_SOFT_TIMESTAMP(8), - AD7606_CHANNEL(0), - AD7606_CHANNEL(1), - AD7606_CHANNEL(2), - AD7606_CHANNEL(3), - AD7606_CHANNEL(4), - AD7606_CHANNEL(5), - AD7606_CHANNEL(6), - AD7606_CHANNEL(7), + AD7606_CHANNEL(0, 16), + AD7606_CHANNEL(1, 16), + AD7606_CHANNEL(2, 16), + AD7606_CHANNEL(3, 16), + AD7606_CHANNEL(4, 16), + AD7606_CHANNEL(5, 16), + AD7606_CHANNEL(6, 16), + AD7606_CHANNEL(7, 16), }; /* @@ -333,22 +333,22 @@ static const struct iio_chan_spec ad7606_channels[] = { */ static const struct iio_chan_spec ad7616_channels[] = { IIO_CHAN_SOFT_TIMESTAMP(16), - AD7606_CHANNEL(0), - AD7606_CHANNEL(1), - AD7606_CHANNEL(2), - AD7606_CHANNEL(3), - AD7606_CHANNEL(4), - AD7606_CHANNEL(5), - AD7606_CHANNEL(6), - AD7606_CHANNEL(7), - AD7606_CHANNEL(8), - AD7606_CHANNEL(9), - AD7606_CHANNEL(10), - AD7606_CHANNEL(11), - AD7606_CHANNEL(12), - AD7606_CHANNEL(13), - AD7606_CHANNEL(14), - AD7606_CHANNEL(15), + AD7606_CHANNEL(0, 16), + AD7606_CHANNEL(1, 16), + AD7606_CHANNEL(2, 16), + AD7606_CHANNEL(3, 16), + AD7606_CHANNEL(4, 16), + AD7606_CHANNEL(5, 16), + AD7606_CHANNEL(6, 16), + AD7606_CHANNEL(7, 16), + AD7606_CHANNEL(8, 16), + AD7606_CHANNEL(9, 16), + AD7606_CHANNEL(10, 16), + AD7606_CHANNEL(11, 16), + AD7606_CHANNEL(12, 16), + AD7606_CHANNEL(13, 16), + AD7606_CHANNEL(14, 16), + AD7606_CHANNEL(15, 16), }; static const struct ad7606_chip_info ad7606_chip_info_tbl[] = { @@ -358,25 +358,25 @@ static const struct ad7606_chip_info ad7606_chip_info_tbl[] = { .num_channels = 5, }, [ID_AD7606_8] = { - .channels = ad7606_channels, + .channels = ad7606_channels_16bit, .num_channels = 9, .oversampling_avail = ad7606_oversampling_avail, .oversampling_num = ARRAY_SIZE(ad7606_oversampling_avail), }, [ID_AD7606_6] = { - .channels = ad7606_channels, + .channels = ad7606_channels_16bit, .num_channels = 7, .oversampling_avail = ad7606_oversampling_avail, .oversampling_num = ARRAY_SIZE(ad7606_oversampling_avail), }, [ID_AD7606_4] = { - .channels = ad7606_channels, + .channels = ad7606_channels_16bit, .num_channels = 5, .oversampling_avail = ad7606_oversampling_avail, .oversampling_num = ARRAY_SIZE(ad7606_oversampling_avail), }, [ID_AD7606B] = { - .channels = ad7606_channels, + .channels = ad7606_channels_16bit, .num_channels = 9, .oversampling_avail = ad7606_oversampling_avail, .oversampling_num = ARRAY_SIZE(ad7606_oversampling_avail), diff --git a/drivers/iio/adc/ad7606.h b/drivers/iio/adc/ad7606.h index 6649e84d25de..204a343067e5 100644 --- a/drivers/iio/adc/ad7606.h +++ b/drivers/iio/adc/ad7606.h @@ -8,7 +8,7 @@ #ifndef IIO_ADC_AD7606_H_ #define IIO_ADC_AD7606_H_ -#define AD760X_CHANNEL(num, mask_sep, mask_type, mask_all) { \ +#define AD760X_CHANNEL(num, mask_sep, mask_type, mask_all, bits) { \ .type = IIO_VOLTAGE, \ .indexed = 1, \ .channel = num, \ @@ -19,24 +19,26 @@ .scan_index = num, \ .scan_type = { \ .sign = 's', \ - .realbits = 16, \ - .storagebits = 16, \ + .realbits = (bits), \ + .storagebits = (bits) > 16 ? 32 : 16, \ .endianness = IIO_CPU, \ }, \ } #define AD7605_CHANNEL(num) \ AD760X_CHANNEL(num, BIT(IIO_CHAN_INFO_RAW), \ - BIT(IIO_CHAN_INFO_SCALE), 0) + BIT(IIO_CHAN_INFO_SCALE), 0, 16) -#define AD7606_CHANNEL(num) \ +#define AD7606_CHANNEL(num, bits) \ AD760X_CHANNEL(num, BIT(IIO_CHAN_INFO_RAW), \ BIT(IIO_CHAN_INFO_SCALE), \ - BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO)) + BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO), bits) -#define AD7616_CHANNEL(num) \ +#define AD7606_SW_CHANNEL(num, bits) \ AD760X_CHANNEL(num, BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),\ - 0, BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO)) + 0, BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO), bits) + +#define AD7616_CHANNEL(num) AD7606_SW_CHANNEL(num, 16) /** * struct ad7606_chip_info - chip specific information diff --git a/drivers/iio/adc/ad7606_spi.c b/drivers/iio/adc/ad7606_spi.c index 62ec12195307..e00f58a6a0e9 100644 --- a/drivers/iio/adc/ad7606_spi.c +++ b/drivers/iio/adc/ad7606_spi.c @@ -67,14 +67,14 @@ static const struct iio_chan_spec ad7616_sw_channels[] = { static const struct iio_chan_spec ad7606b_sw_channels[] = { IIO_CHAN_SOFT_TIMESTAMP(8), - AD7616_CHANNEL(0), - AD7616_CHANNEL(1), - AD7616_CHANNEL(2), - AD7616_CHANNEL(3), - AD7616_CHANNEL(4), - AD7616_CHANNEL(5), - AD7616_CHANNEL(6), - AD7616_CHANNEL(7), + AD7606_SW_CHANNEL(0, 16), + AD7606_SW_CHANNEL(1, 16), + AD7606_SW_CHANNEL(2, 16), + AD7606_SW_CHANNEL(3, 16), + AD7606_SW_CHANNEL(4, 16), + AD7606_SW_CHANNEL(5, 16), + AD7606_SW_CHANNEL(6, 16), + AD7606_SW_CHANNEL(7, 16), }; static const unsigned int ad7606B_oversampling_avail[9] = {
There are some newer additions to the AD7606 family, which support 18 bit precision. Up until now, all chips were 16 bit. This change adds a 'bits' parameter to the AD760X_CHANNEL macro and renames 'ad7606_channels' -> 'ad7606_channels_16bit' for the current devices. The AD7606_SW_CHANNEL() macro is also introduced, as a short-hand for IIO channels in SW mode. Signed-off-by: Alexandru Ardelean <aardelean@baylibre.com> --- drivers/iio/adc/ad7606.c | 58 ++++++++++++++++++------------------ drivers/iio/adc/ad7606.h | 18 ++++++----- drivers/iio/adc/ad7606_spi.c | 16 +++++----- 3 files changed, 47 insertions(+), 45 deletions(-)