Re: [PATCH 2/2] iio: adc: ti-adc128s052: Add lower resolution devices support

Jonathan Cameron
2022-07-01T16:30:46Z

On Thu, 30 Jun 2022 23:29:19 -0500
Nishanth Menon <nm@ti.com> wrote:

> The adcxx4s communicates with a host processor via an SPI/Microwire Bus
> interface. The device family responds with 12bit data, of which the LSB
> bits are transmitted by the lower resolution devices as 0. We don't need
> to mess with ADC108S102_BITS as a result for the lower resolution
> devices.

whilst not strictly necessary I would prefer we did as it may reduce
storage requirements, computation complexity etc for userspace (particularly
if they are 8 bits).

Would affect scale and shift so slightly more complex, but not a lot
so still a fairly simple change. Just add resolution to your config
structure and duplicate it where necessary to allow whatever resolutions
we support.

>
> I have been able to test adc102s051, hence adding just the missing
> ones in that family.
>
> Lets reuse the driver to support the family of devices with name
> ADC<bb><c>S<sss>, where
> * bb is the resolution in number of bits (8, 10, 12)
> * c is the number of channels (1, 2, 4, 8)
> * sss is the maximum conversion speed (021 for 200 kSPS, 051 for 500 kSPS
> and 101 for 1 MSPS)
>
> Complete datasheets are available at TI's website here:
> https://www.ti.com/lit/gpn/adc<bb><c>s<sss>.pdf
>
> Also see: drivers/hwmon/adcxx.c
>
> Signed-off-by: Nishanth Menon <nm@ti.com>
> ---
>
> This does add on additional sparse warnings around casting .data value
> to const from int for the of_match_table, and a bunch around the .cls
> field for acpi_device_id - maybe someone could suggest a smarter way to
> fix those.
>
> Applies after https://lore.kernel.org/linux-iio/20220630230107.13438-1-nm@ti.com/
>
> drivers/iio/adc/ti-adc128s052.c | 26 ++++++++++++++++++++++++++
> 1 file changed, 26 insertions(+)
>
> diff --git a/drivers/iio/adc/ti-adc128s052.c b/drivers/iio/adc/ti-adc128s052.c
> index 21a7764cbb93..0a3aab4df60e 100644
> --- a/drivers/iio/adc/ti-adc128s052.c
> +++ b/drivers/iio/adc/ti-adc128s052.c
> @@ -7,6 +7,20 @@
> * https://www.ti.com/lit/ds/symlink/adc128s052.pdf
> * https://www.ti.com/lit/ds/symlink/adc122s021.pdf
> * https://www.ti.com/lit/ds/symlink/adc124s021.pdf
> + *
> + * The adcxx4s communicates with a host processor via an SPI/Microwire Bus
> + * interface. This driver supports the whole family of devices with name
> + * ADC<bb><c>S<sss>, where
> + * bb is the resolution in number of bits (8, 10, 12)
> + * c is the number of channels (1, 2, 4, 8)
> + * sss is the maximum conversion speed (021 for 200 kSPS, 051 for 500 kSPS
> + * and 101 for 1 MSPS)
> + *
> + * Complete datasheets are available at TI's website here:
> + * https://www.ti.com/lit/gpn/adc<bb><c>s<sss>.pdf
> + *
> + * Handling of 8, 10 and 12 bits converters are the same, the
> + * unavailable bits are 0 in LSB :)
> */
>
> #include <linux/acpi.h>
> @@ -185,6 +199,12 @@ static const struct of_device_id adc128_of_match[] = {
> { .compatible = "ti,adc122s021", .data = 1},
> { .compatible = "ti,adc122s051", .data = 1},
> { .compatible = "ti,adc122s101", .data = 1},
> + { .compatible = "ti,adc102s021", .data = 1},
> + { .compatible = "ti,adc102s051", .data = 1},
> + { .compatible = "ti,adc102s101", .data = 1},

Numeric order preferred. Not sure why these are in the middle.

> + { .compatible = "ti,adc082s021", .data = 1},
> + { .compatible = "ti,adc082s051", .data = 1},
> + { .compatible = "ti,adc082s101", .data = 1},

Andy's comment on using pointers instead follows through to here.
To describe the channels more fully you'll need additional
_config[] structures.

> { .compatible = "ti,adc124s021", .data = 2},
> { .compatible = "ti,adc124s051", .data = 2},
> { .compatible = "ti,adc124s101", .data = 2},
> @@ -197,6 +217,12 @@ static const struct spi_device_id adc128_id[] = {
> { "adc122s021", 1 },
> { "adc122s051", 1 },
> { "adc122s101", 1 },
> + { "adc102s021", 1 },
> + { "adc102s051", 1 },
> + { "adc102s101", 1 },
> + { "adc082s021", 1 },
> + { "adc082s051", 1 },
> + { "adc082s101", 1 },
> { "adc124s021", 2 },
> { "adc124s051", 2 },
> { "adc124s101", 2 },