[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20220701174011.14d17f43@jic23-huawei>
Date: Fri, 1 Jul 2022 17:40:11 +0100
From: Jonathan Cameron <jic23@...nel.org>
To: Nishanth Menon <nm@...com>
Cc: Angelo Compagnucci <angelo.compagnucci@...il.com>,
Nuno Sá <nuno.sa@...log.com>,
Christophe JAILLET <christophe.jaillet@...adoo.fr>,
Alexandru Ardelean <ardeleanalex@...il.com>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
Rob Herring <robh+dt@...nel.org>,
Lars-Peter Clausen <lars@...afoo.de>,
Florian Eckert <fe@....tdt.de>, <linux-kernel@...r.kernel.org>,
<devicetree@...r.kernel.org>, <linux-iio@...r.kernel.org>
Subject: Re: [PATCH 2/2] iio: adc: ti-adc128s052: Add lower resolution
devices support
On Thu, 30 Jun 2022 23:29:19 -0500
Nishanth Menon <nm@...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@...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 },
Powered by blists - more mailing lists