* [PATCH v2 0/2] iio: adc: ti-adc128s052: Add support for adc102s021 and family
@ 2023-10-22 3:12 Sukrut Bellary
2023-10-22 3:12 ` [PATCH v2 1/2] dt-bindings: iio: adc: ti,adc128s052: Add adc08c and adc10c family Sukrut Bellary
2023-10-22 3:12 ` [PATCH v2 2/2] iio: adc: ti-adc128s052: Add lower resolution devices support Sukrut Bellary
0 siblings, 2 replies; 7+ messages in thread
From: Sukrut Bellary @ 2023-10-22 3:12 UTC (permalink / raw)
To: Jonathan Cameron, Lars-Peter Clausen, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Angelo Compagnucci,
Nishanth Menon
Cc: Sukrut Bellary, linux-iio, devicetree, linux-kernel
The patch series adds the support for adc102s021 and family.
The family of devices are easier to
support since they all (no matter the resolution) seem to respond in
12-bits with the LSBs set to 0 for the reduced resolution devices.
Series is based on next-20231020
Changes in v2:
Patch 1:
- No changes in dt-bindings
Patch 2:
- Arranged of_device_id and spi_device_id in numeric order.
- Used enum to index into adc128_config.
- Reorder adc128_config in alphabetical.
- Include channel resolution information.
- Shift is calculated per resolution and used in scaling and
raw data read.
- Link to v1: https://lore.kernel.org/all/20220701042919.18180-1-nm@ti.com/
Sukrut Bellary (2):
dt-bindings: iio: adc: ti,adc128s052: Add adc08c and adc10c family
iio: adc: ti-adc128s052: Add lower resolution devices support
.../bindings/iio/adc/ti,adc128s052.yaml | 6 +
drivers/iio/adc/ti-adc128s052.c | 131 +++++++++++++-----
2 files changed, 102 insertions(+), 35 deletions(-)
--
2.34.1
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH v2 1/2] dt-bindings: iio: adc: ti,adc128s052: Add adc08c and adc10c family 2023-10-22 3:12 [PATCH v2 0/2] iio: adc: ti-adc128s052: Add support for adc102s021 and family Sukrut Bellary @ 2023-10-22 3:12 ` Sukrut Bellary 2023-10-22 3:12 ` [PATCH v2 2/2] iio: adc: ti-adc128s052: Add lower resolution devices support Sukrut Bellary 1 sibling, 0 replies; 7+ messages in thread From: Sukrut Bellary @ 2023-10-22 3:12 UTC (permalink / raw) To: Jonathan Cameron, Lars-Peter Clausen, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Angelo Compagnucci, Nishanth Menon Cc: Sukrut Bellary, linux-iio, devicetree, linux-kernel, Krzysztof Kozlowski The adcxx4s communicates with a host processor via an SPI/Microwire Bus interface. The device family responds with 12-bit data, of which the LSB bits are 0 for the lower resolution devices. The unavailable bits are 0 in LSB. Shift is calculated per resolution and used in scaling and raw data read. I have been able to test adc102s051, hence adding just the missing ones in that family. Lets reuse the binding 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 Co-developed-by: Nishanth Menon <nm@ti.com> Signed-off-by: Nishanth Menon <nm@ti.com> Signed-off-by: Sukrut Bellary <sukrut.bellary@linux.com> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> --- Changes in v2: - No changes in dt-bindings - Link to v1: https://lore.kernel.org/all/20220701042919.18180-2-nm@ti.com/ --- .../devicetree/bindings/iio/adc/ti,adc128s052.yaml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/Documentation/devicetree/bindings/iio/adc/ti,adc128s052.yaml b/Documentation/devicetree/bindings/iio/adc/ti,adc128s052.yaml index 775eee972b12..392b4a3e867c 100644 --- a/Documentation/devicetree/bindings/iio/adc/ti,adc128s052.yaml +++ b/Documentation/devicetree/bindings/iio/adc/ti,adc128s052.yaml @@ -16,6 +16,12 @@ description: | properties: compatible: enum: + - ti,adc082s021 + - ti,adc082s051 + - ti,adc082s101 + - ti,adc102s021 + - ti,adc102s051 + - ti,adc102s101 - ti,adc122s021 - ti,adc122s051 - ti,adc122s101 -- 2.34.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v2 2/2] iio: adc: ti-adc128s052: Add lower resolution devices support 2023-10-22 3:12 [PATCH v2 0/2] iio: adc: ti-adc128s052: Add support for adc102s021 and family Sukrut Bellary 2023-10-22 3:12 ` [PATCH v2 1/2] dt-bindings: iio: adc: ti,adc128s052: Add adc08c and adc10c family Sukrut Bellary @ 2023-10-22 3:12 ` Sukrut Bellary 2023-10-22 16:00 ` Jonathan Cameron 1 sibling, 1 reply; 7+ messages in thread From: Sukrut Bellary @ 2023-10-22 3:12 UTC (permalink / raw) To: Jonathan Cameron, Lars-Peter Clausen, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Angelo Compagnucci, Nishanth Menon Cc: Sukrut Bellary, linux-iio, devicetree, linux-kernel The adcxx4s communicates with a host processor via an SPI/Microwire Bus interface. The device family responds with 12-bit data, of which the LSB bits are transmitted by the lower resolution devices as 0. The unavailable bits are 0 in LSB. Shift is calculated per resolution and used in scaling and raw data read. 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 Tested only with ti-adc102s051 on BegalePlay SBC. https://www.beagleboard.org/boards/beagleplay arm64: dts: ti: k3-am625-beagleplay: Add adc102s051 Add adc102s051 support. Tested on beaglePlay SBC https://www.beagleboard.org/boards/beagleplay Co-developed-by: Nishanth Menon <nm@ti.com> Signed-off-by: Nishanth Menon <nm@ti.com> Signed-off-by: Sukrut Bellary <sukrut.bellary@linux.com> --- Changes in v2: - Arranged of_device_id and spi_device_id in numeric order. - Used enum to index into adc128_config. - Reorder adc128_config in alphabetical. - Include channel resolution information. - Shift is calculated per resolution and used in scaling and raw data read. - Link to v1: https://lore.kernel.org/all/20220701042919.18180-3-nm@ti.com/ --- drivers/iio/adc/ti-adc128s052.c | 131 +++++++++++++++++++++++--------- 1 file changed, 96 insertions(+), 35 deletions(-) diff --git a/drivers/iio/adc/ti-adc128s052.c b/drivers/iio/adc/ti-adc128s052.c index a456ea78462f..61e3181b8daf 100644 --- a/drivers/iio/adc/ti-adc128s052.c +++ b/drivers/iio/adc/ti-adc128s052.c @@ -7,6 +7,22 @@ * 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 a 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 + * + * 8, 10, and 12 bits converters send 12-bit data with + * unavailable bits set to 0 in LSB. + * Shift is calculated per resolution and used in scaling and + * raw data read. */ #include <linux/err.h> @@ -53,7 +69,7 @@ static int adc128_adc_conversion(struct adc128 *adc, u8 channel) if (ret < 0) return ret; - return ((adc->buffer[0] << 8 | adc->buffer[1]) & 0xFFF); + return (adc->buffer[0] << 8 | adc->buffer[1]); } static int adc128_read_raw(struct iio_dev *indio_dev, @@ -70,7 +86,8 @@ static int adc128_read_raw(struct iio_dev *indio_dev, if (ret < 0) return ret; - *val = ret; + *val = (ret >> channel->scan_type.shift) & + GENMASK(channel->scan_type.realbits - 1, 0); return IIO_VAL_INT; case IIO_CHAN_INFO_SCALE: @@ -80,7 +97,7 @@ static int adc128_read_raw(struct iio_dev *indio_dev, return ret; *val = ret / 1000; - *val2 = 12; + *val2 = channel->scan_type.realbits; return IIO_VAL_FRACTIONAL_LOG2; default: @@ -89,24 +106,34 @@ static int adc128_read_raw(struct iio_dev *indio_dev, } -#define ADC128_VOLTAGE_CHANNEL(num) \ - { \ - .type = IIO_VOLTAGE, \ - .indexed = 1, \ - .channel = (num), \ - .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \ - .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) \ +#define _ADC128_VOLTAGE_CHANNEL(num, real_bits, store_bits) \ + { \ + .type = IIO_VOLTAGE, \ + .indexed = 1, \ + .channel = (num), \ + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \ + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \ + .scan_index = (num), \ + .scan_type = { \ + .sign = 'u', \ + .realbits = (real_bits), \ + .storagebits = (store_bits), \ + .shift = (12 - real_bits), \ + }, \ } -static const struct iio_chan_spec adc128s052_channels[] = { - ADC128_VOLTAGE_CHANNEL(0), - ADC128_VOLTAGE_CHANNEL(1), - ADC128_VOLTAGE_CHANNEL(2), - ADC128_VOLTAGE_CHANNEL(3), - ADC128_VOLTAGE_CHANNEL(4), - ADC128_VOLTAGE_CHANNEL(5), - ADC128_VOLTAGE_CHANNEL(6), - ADC128_VOLTAGE_CHANNEL(7), +#define ADC082_VOLTAGE_CHANNEL(num) _ADC128_VOLTAGE_CHANNEL(num, 8, 16) +#define ADC102_VOLTAGE_CHANNEL(num) _ADC128_VOLTAGE_CHANNEL(num, 10, 16) +#define ADC128_VOLTAGE_CHANNEL(num) _ADC128_VOLTAGE_CHANNEL(num, 12, 16) + +static const struct iio_chan_spec adc082s021_channels[] = { + ADC082_VOLTAGE_CHANNEL(0), + ADC082_VOLTAGE_CHANNEL(1), +}; + +static const struct iio_chan_spec adc102s021_channels[] = { + ADC102_VOLTAGE_CHANNEL(0), + ADC102_VOLTAGE_CHANNEL(1), }; static const struct iio_chan_spec adc122s021_channels[] = { @@ -121,10 +148,32 @@ static const struct iio_chan_spec adc124s021_channels[] = { ADC128_VOLTAGE_CHANNEL(3), }; +static const struct iio_chan_spec adc128s052_channels[] = { + ADC128_VOLTAGE_CHANNEL(0), + ADC128_VOLTAGE_CHANNEL(1), + ADC128_VOLTAGE_CHANNEL(2), + ADC128_VOLTAGE_CHANNEL(3), + ADC128_VOLTAGE_CHANNEL(4), + ADC128_VOLTAGE_CHANNEL(5), + ADC128_VOLTAGE_CHANNEL(6), + ADC128_VOLTAGE_CHANNEL(7), +}; + static const struct adc128_configuration adc128_config[] = { - { adc128s052_channels, ARRAY_SIZE(adc128s052_channels) }, + { adc082s021_channels, ARRAY_SIZE(adc082s021_channels) }, + { adc102s021_channels, ARRAY_SIZE(adc102s021_channels) }, { adc122s021_channels, ARRAY_SIZE(adc122s021_channels) }, { adc124s021_channels, ARRAY_SIZE(adc124s021_channels) }, + { adc128s052_channels, ARRAY_SIZE(adc128s052_channels) }, +}; + +/* Ensure match with adc128_config indices */ +enum adc128_configuration_index { + ADC128_CONFIG_INDEX_082S, + ADC128_CONFIG_INDEX_102S, + ADC128_CONFIG_INDEX_122S, + ADC128_CONFIG_INDEX_124S, + ADC128_CONFIG_INDEX_128S, }; static const struct iio_info adc128_info = { @@ -177,31 +226,43 @@ static int adc128_probe(struct spi_device *spi) } static const struct of_device_id adc128_of_match[] = { - { .compatible = "ti,adc128s052", .data = &adc128_config[0] }, - { .compatible = "ti,adc122s021", .data = &adc128_config[1] }, - { .compatible = "ti,adc122s051", .data = &adc128_config[1] }, - { .compatible = "ti,adc122s101", .data = &adc128_config[1] }, - { .compatible = "ti,adc124s021", .data = &adc128_config[2] }, - { .compatible = "ti,adc124s051", .data = &adc128_config[2] }, - { .compatible = "ti,adc124s101", .data = &adc128_config[2] }, + { .compatible = "ti,adc082s021", .data = &adc128_config[ADC128_CONFIG_INDEX_082S] }, + { .compatible = "ti,adc082s051", .data = &adc128_config[ADC128_CONFIG_INDEX_082S] }, + { .compatible = "ti,adc082s101", .data = &adc128_config[ADC128_CONFIG_INDEX_082S] }, + { .compatible = "ti,adc102s021", .data = &adc128_config[ADC128_CONFIG_INDEX_102S] }, + { .compatible = "ti,adc102s051", .data = &adc128_config[ADC128_CONFIG_INDEX_102S] }, + { .compatible = "ti,adc102s101", .data = &adc128_config[ADC128_CONFIG_INDEX_102S] }, + { .compatible = "ti,adc122s021", .data = &adc128_config[ADC128_CONFIG_INDEX_122S] }, + { .compatible = "ti,adc122s051", .data = &adc128_config[ADC128_CONFIG_INDEX_122S] }, + { .compatible = "ti,adc122s101", .data = &adc128_config[ADC128_CONFIG_INDEX_122S] }, + { .compatible = "ti,adc124s021", .data = &adc128_config[ADC128_CONFIG_INDEX_124S] }, + { .compatible = "ti,adc124s051", .data = &adc128_config[ADC128_CONFIG_INDEX_124S] }, + { .compatible = "ti,adc124s101", .data = &adc128_config[ADC128_CONFIG_INDEX_124S] }, + { .compatible = "ti,adc128s052", .data = &adc128_config[ADC128_CONFIG_INDEX_128S] }, { /* sentinel */ }, }; MODULE_DEVICE_TABLE(of, adc128_of_match); static const struct spi_device_id adc128_id[] = { - { "adc128s052", (kernel_ulong_t)&adc128_config[0] }, - { "adc122s021", (kernel_ulong_t)&adc128_config[1] }, - { "adc122s051", (kernel_ulong_t)&adc128_config[1] }, - { "adc122s101", (kernel_ulong_t)&adc128_config[1] }, - { "adc124s021", (kernel_ulong_t)&adc128_config[2] }, - { "adc124s051", (kernel_ulong_t)&adc128_config[2] }, - { "adc124s101", (kernel_ulong_t)&adc128_config[2] }, + { "adc082s021", (kernel_ulong_t)&adc128_config[ADC128_CONFIG_INDEX_082S] }, + { "adc082s051", (kernel_ulong_t)&adc128_config[ADC128_CONFIG_INDEX_082S] }, + { "adc082s101", (kernel_ulong_t)&adc128_config[ADC128_CONFIG_INDEX_082S] }, + { "adc102s021", (kernel_ulong_t)&adc128_config[ADC128_CONFIG_INDEX_102S] }, + { "adc102s051", (kernel_ulong_t)&adc128_config[ADC128_CONFIG_INDEX_102S] }, + { "adc102s101", (kernel_ulong_t)&adc128_config[ADC128_CONFIG_INDEX_102S] }, + { "adc122s021", (kernel_ulong_t)&adc128_config[ADC128_CONFIG_INDEX_122S] }, + { "adc122s051", (kernel_ulong_t)&adc128_config[ADC128_CONFIG_INDEX_122S] }, + { "adc122s101", (kernel_ulong_t)&adc128_config[ADC128_CONFIG_INDEX_122S] }, + { "adc124s021", (kernel_ulong_t)&adc128_config[ADC128_CONFIG_INDEX_124S] }, + { "adc124s051", (kernel_ulong_t)&adc128_config[ADC128_CONFIG_INDEX_124S] }, + { "adc124s101", (kernel_ulong_t)&adc128_config[ADC128_CONFIG_INDEX_124S] }, + { "adc128s052", (kernel_ulong_t)&adc128_config[ADC128_CONFIG_INDEX_128S] }, { } }; MODULE_DEVICE_TABLE(spi, adc128_id); static const struct acpi_device_id adc128_acpi_match[] = { - { "AANT1280", (kernel_ulong_t)&adc128_config[2] }, + { "AANT1280", (kernel_ulong_t)&adc128_config[ADC128_CONFIG_INDEX_124S] }, { } }; MODULE_DEVICE_TABLE(acpi, adc128_acpi_match); -- 2.34.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2 2/2] iio: adc: ti-adc128s052: Add lower resolution devices support 2023-10-22 3:12 ` [PATCH v2 2/2] iio: adc: ti-adc128s052: Add lower resolution devices support Sukrut Bellary @ 2023-10-22 16:00 ` Jonathan Cameron 2023-10-24 22:52 ` Sukrut Bellary 0 siblings, 1 reply; 7+ messages in thread From: Jonathan Cameron @ 2023-10-22 16:00 UTC (permalink / raw) To: Sukrut Bellary Cc: Lars-Peter Clausen, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Angelo Compagnucci, Nishanth Menon, linux-iio, devicetree, linux-kernel On Sat, 21 Oct 2023 20:12:03 -0700 Sukrut Bellary <sukrut.bellary@linux.com> wrote: > The adcxx4s communicates with a host processor via an SPI/Microwire Bus > interface. The device family responds with 12-bit data, of which the LSB > bits are transmitted by the lower resolution devices as 0. > The unavailable bits are 0 in LSB. > Shift is calculated per resolution and used in scaling and > raw data read. > > 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 > > Tested only with ti-adc102s051 on BegalePlay SBC. > https://www.beagleboard.org/boards/beagleplay > > arm64: dts: ti: k3-am625-beagleplay: Add adc102s051 > Add adc102s051 support. > Tested on beaglePlay SBC > https://www.beagleboard.org/boards/beagleplay > > Co-developed-by: Nishanth Menon <nm@ti.com> > Signed-off-by: Nishanth Menon <nm@ti.com> > Signed-off-by: Sukrut Bellary <sukrut.bellary@linux.com> > --- > Changes in v2: > - Arranged of_device_id and spi_device_id in numeric order. > - Used enum to index into adc128_config. > - Reorder adc128_config in alphabetical. > - Include channel resolution information. > - Shift is calculated per resolution and used in scaling and > raw data read. > - Link to v1: https://lore.kernel.org/all/20220701042919.18180-3-nm@ti.com/ > --- > drivers/iio/adc/ti-adc128s052.c | 131 +++++++++++++++++++++++--------- > 1 file changed, 96 insertions(+), 35 deletions(-) > > diff --git a/drivers/iio/adc/ti-adc128s052.c b/drivers/iio/adc/ti-adc128s052.c > index a456ea78462f..61e3181b8daf 100644 > --- a/drivers/iio/adc/ti-adc128s052.c > +++ b/drivers/iio/adc/ti-adc128s052.c > @@ -7,6 +7,22 @@ > * 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 a 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 > + * > + * 8, 10, and 12 bits converters send 12-bit data with > + * unavailable bits set to 0 in LSB. > + * Shift is calculated per resolution and used in scaling and > + * raw data read. > */ > > #include <linux/err.h> > @@ -53,7 +69,7 @@ static int adc128_adc_conversion(struct adc128 *adc, u8 channel) > if (ret < 0) > return ret; > > - return ((adc->buffer[0] << 8 | adc->buffer[1]) & 0xFFF); > + return (adc->buffer[0] << 8 | adc->buffer[1]); Firstly outer brackets don't add anything. Secondly, this is an endian conversion. return be16_to_cpu(adc->buffer); should do the job as we know it's aligned enough for a be16. > } > > static int adc128_read_raw(struct iio_dev *indio_dev, > @@ -70,7 +86,8 @@ static int adc128_read_raw(struct iio_dev *indio_dev, > if (ret < 0) > return ret; > > - *val = ret; > + *val = (ret >> channel->scan_type.shift) & > + GENMASK(channel->scan_type.realbits - 1, 0); > return IIO_VAL_INT; > > case IIO_CHAN_INFO_SCALE: > @@ -80,7 +97,7 @@ static int adc128_read_raw(struct iio_dev *indio_dev, > return ret; > > *val = ret / 1000; > - *val2 = 12; > + *val2 = channel->scan_type.realbits; > return IIO_VAL_FRACTIONAL_LOG2; > > default: > @@ -89,24 +106,34 @@ static int adc128_read_raw(struct iio_dev *indio_dev, > > } > > -#define ADC128_VOLTAGE_CHANNEL(num) \ > - { \ > - .type = IIO_VOLTAGE, \ > - .indexed = 1, \ > - .channel = (num), \ > - .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \ > - .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) \ > +#define _ADC128_VOLTAGE_CHANNEL(num, real_bits, store_bits) \ > + { \ > + .type = IIO_VOLTAGE, \ > + .indexed = 1, \ > + .channel = (num), \ > + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \ > + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \ > + .scan_index = (num), \ > + .scan_type = { \ > + .sign = 'u', \ > + .realbits = (real_bits), \ > + .storagebits = (store_bits), \ > + .shift = (12 - real_bits), \ > + }, \ > } > > -static const struct iio_chan_spec adc128s052_channels[] = { > - ADC128_VOLTAGE_CHANNEL(0), > - ADC128_VOLTAGE_CHANNEL(1), > - ADC128_VOLTAGE_CHANNEL(2), > - ADC128_VOLTAGE_CHANNEL(3), > - ADC128_VOLTAGE_CHANNEL(4), > - ADC128_VOLTAGE_CHANNEL(5), > - ADC128_VOLTAGE_CHANNEL(6), > - ADC128_VOLTAGE_CHANNEL(7), > +#define ADC082_VOLTAGE_CHANNEL(num) _ADC128_VOLTAGE_CHANNEL(num, 8, 16) > +#define ADC102_VOLTAGE_CHANNEL(num) _ADC128_VOLTAGE_CHANNEL(num, 10, 16) > +#define ADC128_VOLTAGE_CHANNEL(num) _ADC128_VOLTAGE_CHANNEL(num, 12, 16) > + > +static const struct iio_chan_spec adc082s021_channels[] = { > + ADC082_VOLTAGE_CHANNEL(0), > + ADC082_VOLTAGE_CHANNEL(1), > +}; > + > +static const struct iio_chan_spec adc102s021_channels[] = { > + ADC102_VOLTAGE_CHANNEL(0), > + ADC102_VOLTAGE_CHANNEL(1), > }; > > static const struct iio_chan_spec adc122s021_channels[] = { > @@ -121,10 +148,32 @@ static const struct iio_chan_spec adc124s021_channels[] = { > ADC128_VOLTAGE_CHANNEL(3), > }; > > +static const struct iio_chan_spec adc128s052_channels[] = { > + ADC128_VOLTAGE_CHANNEL(0), > + ADC128_VOLTAGE_CHANNEL(1), > + ADC128_VOLTAGE_CHANNEL(2), > + ADC128_VOLTAGE_CHANNEL(3), > + ADC128_VOLTAGE_CHANNEL(4), > + ADC128_VOLTAGE_CHANNEL(5), > + ADC128_VOLTAGE_CHANNEL(6), > + ADC128_VOLTAGE_CHANNEL(7), > +}; > + > static const struct adc128_configuration adc128_config[] = { > - { adc128s052_channels, ARRAY_SIZE(adc128s052_channels) }, > + { adc082s021_channels, ARRAY_SIZE(adc082s021_channels) }, > + { adc102s021_channels, ARRAY_SIZE(adc102s021_channels) }, > { adc122s021_channels, ARRAY_SIZE(adc122s021_channels) }, > { adc124s021_channels, ARRAY_SIZE(adc124s021_channels) }, > + { adc128s052_channels, ARRAY_SIZE(adc128s052_channels) }, > +}; > + > +/* Ensure match with adc128_config indices */ Make sure that is the case by using them when setting it up. static const struct adc128_configuration adc128_config[] { [ADC128_CONFIG_INDEX_082S] = { adc082s021_channels, ARRAY_SIZE(..) }, [ADC128_CONFIG_INDEX_102S] = .. } Or I think you can make it irrelevant by not using an array at all. static const struct ad128_configruation adc082s021_config = { }; etc then just use the address of the right one directly. In this driver, I suspect that is simpler than using the array. > +enum adc128_configuration_index { > + ADC128_CONFIG_INDEX_082S, > + ADC128_CONFIG_INDEX_102S, > + ADC128_CONFIG_INDEX_122S, > + ADC128_CONFIG_INDEX_124S, > + ADC128_CONFIG_INDEX_128S, > }; > > static const struct iio_info adc128_info = { > @@ -177,31 +226,43 @@ static int adc128_probe(struct spi_device *spi) > } > > static const struct of_device_id adc128_of_match[] = { > - { .compatible = "ti,adc128s052", .data = &adc128_config[0] }, > - { .compatible = "ti,adc122s021", .data = &adc128_config[1] }, > - { .compatible = "ti,adc122s051", .data = &adc128_config[1] }, > - { .compatible = "ti,adc122s101", .data = &adc128_config[1] }, > - { .compatible = "ti,adc124s021", .data = &adc128_config[2] }, > - { .compatible = "ti,adc124s051", .data = &adc128_config[2] }, > - { .compatible = "ti,adc124s101", .data = &adc128_config[2] }, > + { .compatible = "ti,adc082s021", .data = &adc128_config[ADC128_CONFIG_INDEX_082S] }, > + { .compatible = "ti,adc082s051", .data = &adc128_config[ADC128_CONFIG_INDEX_082S] }, > + { .compatible = "ti,adc082s101", .data = &adc128_config[ADC128_CONFIG_INDEX_082S] }, > + { .compatible = "ti,adc102s021", .data = &adc128_config[ADC128_CONFIG_INDEX_102S] }, > + { .compatible = "ti,adc102s051", .data = &adc128_config[ADC128_CONFIG_INDEX_102S] }, > + { .compatible = "ti,adc102s101", .data = &adc128_config[ADC128_CONFIG_INDEX_102S] }, > + { .compatible = "ti,adc122s021", .data = &adc128_config[ADC128_CONFIG_INDEX_122S] }, > + { .compatible = "ti,adc122s051", .data = &adc128_config[ADC128_CONFIG_INDEX_122S] }, > + { .compatible = "ti,adc122s101", .data = &adc128_config[ADC128_CONFIG_INDEX_122S] }, > + { .compatible = "ti,adc124s021", .data = &adc128_config[ADC128_CONFIG_INDEX_124S] }, > + { .compatible = "ti,adc124s051", .data = &adc128_config[ADC128_CONFIG_INDEX_124S] }, > + { .compatible = "ti,adc124s101", .data = &adc128_config[ADC128_CONFIG_INDEX_124S] }, > + { .compatible = "ti,adc128s052", .data = &adc128_config[ADC128_CONFIG_INDEX_128S] }, > { /* sentinel */ }, > }; > MODULE_DEVICE_TABLE(of, adc128_of_match); > > static const struct spi_device_id adc128_id[] = { > - { "adc128s052", (kernel_ulong_t)&adc128_config[0] }, > - { "adc122s021", (kernel_ulong_t)&adc128_config[1] }, > - { "adc122s051", (kernel_ulong_t)&adc128_config[1] }, > - { "adc122s101", (kernel_ulong_t)&adc128_config[1] }, > - { "adc124s021", (kernel_ulong_t)&adc128_config[2] }, > - { "adc124s051", (kernel_ulong_t)&adc128_config[2] }, > - { "adc124s101", (kernel_ulong_t)&adc128_config[2] }, > + { "adc082s021", (kernel_ulong_t)&adc128_config[ADC128_CONFIG_INDEX_082S] }, > + { "adc082s051", (kernel_ulong_t)&adc128_config[ADC128_CONFIG_INDEX_082S] }, > + { "adc082s101", (kernel_ulong_t)&adc128_config[ADC128_CONFIG_INDEX_082S] }, > + { "adc102s021", (kernel_ulong_t)&adc128_config[ADC128_CONFIG_INDEX_102S] }, > + { "adc102s051", (kernel_ulong_t)&adc128_config[ADC128_CONFIG_INDEX_102S] }, > + { "adc102s101", (kernel_ulong_t)&adc128_config[ADC128_CONFIG_INDEX_102S] }, > + { "adc122s021", (kernel_ulong_t)&adc128_config[ADC128_CONFIG_INDEX_122S] }, > + { "adc122s051", (kernel_ulong_t)&adc128_config[ADC128_CONFIG_INDEX_122S] }, > + { "adc122s101", (kernel_ulong_t)&adc128_config[ADC128_CONFIG_INDEX_122S] }, > + { "adc124s021", (kernel_ulong_t)&adc128_config[ADC128_CONFIG_INDEX_124S] }, > + { "adc124s051", (kernel_ulong_t)&adc128_config[ADC128_CONFIG_INDEX_124S] }, > + { "adc124s101", (kernel_ulong_t)&adc128_config[ADC128_CONFIG_INDEX_124S] }, > + { "adc128s052", (kernel_ulong_t)&adc128_config[ADC128_CONFIG_INDEX_128S] }, > { } > }; > MODULE_DEVICE_TABLE(spi, adc128_id); > > static const struct acpi_device_id adc128_acpi_match[] = { > - { "AANT1280", (kernel_ulong_t)&adc128_config[2] }, > + { "AANT1280", (kernel_ulong_t)&adc128_config[ADC128_CONFIG_INDEX_124S] }, > { } > }; > MODULE_DEVICE_TABLE(acpi, adc128_acpi_match); ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 2/2] iio: adc: ti-adc128s052: Add lower resolution devices support 2023-10-22 16:00 ` Jonathan Cameron @ 2023-10-24 22:52 ` Sukrut Bellary 2023-10-25 14:24 ` Jonathan Cameron 0 siblings, 1 reply; 7+ messages in thread From: Sukrut Bellary @ 2023-10-24 22:52 UTC (permalink / raw) To: Jonathan Cameron Cc: Lars-Peter Clausen, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Angelo Compagnucci, Nishanth Menon, linux-iio, devicetree, linux-kernel On Sun, Oct 22, 2023 at 05:00:48PM +0100, Jonathan Cameron wrote: > On Sat, 21 Oct 2023 20:12:03 -0700 > Sukrut Bellary <sukrut.bellary@linux.com> wrote: > > > The adcxx4s communicates with a host processor via an SPI/Microwire Bus > > interface. The device family responds with 12-bit data, of which the LSB > > bits are transmitted by the lower resolution devices as 0. > > The unavailable bits are 0 in LSB. > > Shift is calculated per resolution and used in scaling and > > raw data read. > > > > 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 > > > > Tested only with ti-adc102s051 on BegalePlay SBC. > > https://www.beagleboard.org/boards/beagleplay > > > > arm64: dts: ti: k3-am625-beagleplay: Add adc102s051 > > Add adc102s051 support. > > Tested on beaglePlay SBC > > https://www.beagleboard.org/boards/beagleplay > > > > Co-developed-by: Nishanth Menon <nm@ti.com> > > Signed-off-by: Nishanth Menon <nm@ti.com> > > Signed-off-by: Sukrut Bellary <sukrut.bellary@linux.com> > > --- > > Changes in v2: > > - Arranged of_device_id and spi_device_id in numeric order. > > - Used enum to index into adc128_config. > > - Reorder adc128_config in alphabetical. > > - Include channel resolution information. > > - Shift is calculated per resolution and used in scaling and > > raw data read. > > - Link to v1: https://lore.kernel.org/all/20220701042919.18180-3-nm@ti.com/ > > --- > > drivers/iio/adc/ti-adc128s052.c | 131 +++++++++++++++++++++++--------- > > 1 file changed, 96 insertions(+), 35 deletions(-) > > > > diff --git a/drivers/iio/adc/ti-adc128s052.c b/drivers/iio/adc/ti-adc128s052.c > > index a456ea78462f..61e3181b8daf 100644 > > --- a/drivers/iio/adc/ti-adc128s052.c > > +++ b/drivers/iio/adc/ti-adc128s052.c > > @@ -7,6 +7,22 @@ > > * 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 a 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 > > + * > > + * 8, 10, and 12 bits converters send 12-bit data with > > + * unavailable bits set to 0 in LSB. > > + * Shift is calculated per resolution and used in scaling and > > + * raw data read. > > */ > > > > #include <linux/err.h> > > @@ -53,7 +69,7 @@ static int adc128_adc_conversion(struct adc128 *adc, u8 channel) > > if (ret < 0) > > return ret; > > > > - return ((adc->buffer[0] << 8 | adc->buffer[1]) & 0xFFF); > > + return (adc->buffer[0] << 8 | adc->buffer[1]); > Firstly outer brackets don't add anything. > Secondly, this is an endian conversion. > > return be16_to_cpu(adc->buffer); > > should do the job as we know it's aligned enough for a be16. > > Thanks for the review. Yes, I will use be16_to_cpu(). > > } > > > > static int adc128_read_raw(struct iio_dev *indio_dev, > > @@ -70,7 +86,8 @@ static int adc128_read_raw(struct iio_dev *indio_dev, > > if (ret < 0) > > return ret; > > > > - *val = ret; > > + *val = (ret >> channel->scan_type.shift) & > > + GENMASK(channel->scan_type.realbits - 1, 0); > > return IIO_VAL_INT; > > > > case IIO_CHAN_INFO_SCALE: > > @@ -80,7 +97,7 @@ static int adc128_read_raw(struct iio_dev *indio_dev, > > return ret; > > > > *val = ret / 1000; > > - *val2 = 12; > > + *val2 = channel->scan_type.realbits; > > return IIO_VAL_FRACTIONAL_LOG2; > > > > default: > > @@ -89,24 +106,34 @@ static int adc128_read_raw(struct iio_dev *indio_dev, > > > > } > > > > -#define ADC128_VOLTAGE_CHANNEL(num) \ > > - { \ > > - .type = IIO_VOLTAGE, \ > > - .indexed = 1, \ > > - .channel = (num), \ > > - .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \ > > - .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) \ > > +#define _ADC128_VOLTAGE_CHANNEL(num, real_bits, store_bits) \ > > + { \ > > + .type = IIO_VOLTAGE, \ > > + .indexed = 1, \ > > + .channel = (num), \ > > + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \ > > + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \ > > + .scan_index = (num), \ > > + .scan_type = { \ > > + .sign = 'u', \ > > + .realbits = (real_bits), \ > > + .storagebits = (store_bits), \ > > + .shift = (12 - real_bits), \ > > + }, \ > > } > > > > -static const struct iio_chan_spec adc128s052_channels[] = { > > - ADC128_VOLTAGE_CHANNEL(0), > > - ADC128_VOLTAGE_CHANNEL(1), > > - ADC128_VOLTAGE_CHANNEL(2), > > - ADC128_VOLTAGE_CHANNEL(3), > > - ADC128_VOLTAGE_CHANNEL(4), > > - ADC128_VOLTAGE_CHANNEL(5), > > - ADC128_VOLTAGE_CHANNEL(6), > > - ADC128_VOLTAGE_CHANNEL(7), > > +#define ADC082_VOLTAGE_CHANNEL(num) _ADC128_VOLTAGE_CHANNEL(num, 8, 16) > > +#define ADC102_VOLTAGE_CHANNEL(num) _ADC128_VOLTAGE_CHANNEL(num, 10, 16) > > +#define ADC128_VOLTAGE_CHANNEL(num) _ADC128_VOLTAGE_CHANNEL(num, 12, 16) > > + > > +static const struct iio_chan_spec adc082s021_channels[] = { > > + ADC082_VOLTAGE_CHANNEL(0), > > + ADC082_VOLTAGE_CHANNEL(1), > > +}; > > + > > +static const struct iio_chan_spec adc102s021_channels[] = { > > + ADC102_VOLTAGE_CHANNEL(0), > > + ADC102_VOLTAGE_CHANNEL(1), > > }; > > > > static const struct iio_chan_spec adc122s021_channels[] = { > > @@ -121,10 +148,32 @@ static const struct iio_chan_spec adc124s021_channels[] = { > > ADC128_VOLTAGE_CHANNEL(3), > > }; > > > > +static const struct iio_chan_spec adc128s052_channels[] = { > > + ADC128_VOLTAGE_CHANNEL(0), > > + ADC128_VOLTAGE_CHANNEL(1), > > + ADC128_VOLTAGE_CHANNEL(2), > > + ADC128_VOLTAGE_CHANNEL(3), > > + ADC128_VOLTAGE_CHANNEL(4), > > + ADC128_VOLTAGE_CHANNEL(5), > > + ADC128_VOLTAGE_CHANNEL(6), > > + ADC128_VOLTAGE_CHANNEL(7), > > +}; > > + > > static const struct adc128_configuration adc128_config[] = { > > - { adc128s052_channels, ARRAY_SIZE(adc128s052_channels) }, > > + { adc082s021_channels, ARRAY_SIZE(adc082s021_channels) }, > > + { adc102s021_channels, ARRAY_SIZE(adc102s021_channels) }, > > { adc122s021_channels, ARRAY_SIZE(adc122s021_channels) }, > > { adc124s021_channels, ARRAY_SIZE(adc124s021_channels) }, > > + { adc128s052_channels, ARRAY_SIZE(adc128s052_channels) }, > > +}; > > + > > +/* Ensure match with adc128_config indices */ > > Make sure that is the case by using them when setting it up. > > static const struct adc128_configuration adc128_config[] { > [ADC128_CONFIG_INDEX_082S] = { > adc082s021_channels, ARRAY_SIZE(..) > }, > [ADC128_CONFIG_INDEX_102S] = .. > } > OK, I will use enum while setting it up +static const struct adc128_configuration adc128_config[] = { + [ADC128_CONFIG_INDEX_082S] = { + .channels = adc082s021_channels, + .num_channels = ARRAY_SIZE(adc082s021_channels) } Use enum in spi_device_id[] static const struct spi_device_id adc128_id[] = { + { "adc082s021", (kernel_ulong_t)ADC128_CONFIG_INDEX_082S }, + { "adc082s051", (kernel_ulong_t)ADC128_CONFIG_INDEX_082S }, } And use it in probe to extract the config for channels and num_channels using config = &adc128_config[spi_get_device_id(spi)->driver_data]; since we are not using of_device_id for extracting config, I can remove the .data entry or need to type case it. static const struct of_device_id adc128_of_match[] = { { .compatible = "ti,adc082s021", }, } > Or I think you can make it irrelevant by not using an array at all. > static const struct ad128_configruation adc082s021_config = { > }; > > etc then just use the address of the right one directly. > > In this driver, I suspect that is simpler than using the array. > > > +enum adc128_configuration_index { > > + ADC128_CONFIG_INDEX_082S, > > + ADC128_CONFIG_INDEX_102S, > > + ADC128_CONFIG_INDEX_122S, > > + ADC128_CONFIG_INDEX_124S, > > + ADC128_CONFIG_INDEX_128S, > > > }; > > > > static const struct iio_info adc128_info = { > > @@ -177,31 +226,43 @@ static int adc128_probe(struct spi_device *spi) > > } > > > > static const struct of_device_id adc128_of_match[] = { > > - { .compatible = "ti,adc128s052", .data = &adc128_config[0] }, > > - { .compatible = "ti,adc122s021", .data = &adc128_config[1] }, > > - { .compatible = "ti,adc122s051", .data = &adc128_config[1] }, > > - { .compatible = "ti,adc122s101", .data = &adc128_config[1] }, > > - { .compatible = "ti,adc124s021", .data = &adc128_config[2] }, > > - { .compatible = "ti,adc124s051", .data = &adc128_config[2] }, > > - { .compatible = "ti,adc124s101", .data = &adc128_config[2] }, > > + { .compatible = "ti,adc082s021", .data = &adc128_config[ADC128_CONFIG_INDEX_082S] }, > > + { .compatible = "ti,adc082s051", .data = &adc128_config[ADC128_CONFIG_INDEX_082S] }, > > + { .compatible = "ti,adc082s101", .data = &adc128_config[ADC128_CONFIG_INDEX_082S] }, > > + { .compatible = "ti,adc102s021", .data = &adc128_config[ADC128_CONFIG_INDEX_102S] }, > > + { .compatible = "ti,adc102s051", .data = &adc128_config[ADC128_CONFIG_INDEX_102S] }, > > + { .compatible = "ti,adc102s101", .data = &adc128_config[ADC128_CONFIG_INDEX_102S] }, > > + { .compatible = "ti,adc122s021", .data = &adc128_config[ADC128_CONFIG_INDEX_122S] }, > > + { .compatible = "ti,adc122s051", .data = &adc128_config[ADC128_CONFIG_INDEX_122S] }, > > + { .compatible = "ti,adc122s101", .data = &adc128_config[ADC128_CONFIG_INDEX_122S] }, > > + { .compatible = "ti,adc124s021", .data = &adc128_config[ADC128_CONFIG_INDEX_124S] }, > > + { .compatible = "ti,adc124s051", .data = &adc128_config[ADC128_CONFIG_INDEX_124S] }, > > + { .compatible = "ti,adc124s101", .data = &adc128_config[ADC128_CONFIG_INDEX_124S] }, > > + { .compatible = "ti,adc128s052", .data = &adc128_config[ADC128_CONFIG_INDEX_128S] }, > > { /* sentinel */ }, > > }; > > MODULE_DEVICE_TABLE(of, adc128_of_match); > > > > static const struct spi_device_id adc128_id[] = { > > - { "adc128s052", (kernel_ulong_t)&adc128_config[0] }, > > - { "adc122s021", (kernel_ulong_t)&adc128_config[1] }, > > - { "adc122s051", (kernel_ulong_t)&adc128_config[1] }, > > - { "adc122s101", (kernel_ulong_t)&adc128_config[1] }, > > - { "adc124s021", (kernel_ulong_t)&adc128_config[2] }, > > - { "adc124s051", (kernel_ulong_t)&adc128_config[2] }, > > - { "adc124s101", (kernel_ulong_t)&adc128_config[2] }, > > + { "adc082s021", (kernel_ulong_t)&adc128_config[ADC128_CONFIG_INDEX_082S] }, > > + { "adc082s051", (kernel_ulong_t)&adc128_config[ADC128_CONFIG_INDEX_082S] }, > > + { "adc082s101", (kernel_ulong_t)&adc128_config[ADC128_CONFIG_INDEX_082S] }, > > + { "adc102s021", (kernel_ulong_t)&adc128_config[ADC128_CONFIG_INDEX_102S] }, > > + { "adc102s051", (kernel_ulong_t)&adc128_config[ADC128_CONFIG_INDEX_102S] }, > > + { "adc102s101", (kernel_ulong_t)&adc128_config[ADC128_CONFIG_INDEX_102S] }, > > + { "adc122s021", (kernel_ulong_t)&adc128_config[ADC128_CONFIG_INDEX_122S] }, > > + { "adc122s051", (kernel_ulong_t)&adc128_config[ADC128_CONFIG_INDEX_122S] }, > > + { "adc122s101", (kernel_ulong_t)&adc128_config[ADC128_CONFIG_INDEX_122S] }, > > + { "adc124s021", (kernel_ulong_t)&adc128_config[ADC128_CONFIG_INDEX_124S] }, > > + { "adc124s051", (kernel_ulong_t)&adc128_config[ADC128_CONFIG_INDEX_124S] }, > > + { "adc124s101", (kernel_ulong_t)&adc128_config[ADC128_CONFIG_INDEX_124S] }, > > + { "adc128s052", (kernel_ulong_t)&adc128_config[ADC128_CONFIG_INDEX_128S] }, > > { } > > }; > > MODULE_DEVICE_TABLE(spi, adc128_id); > > > > static const struct acpi_device_id adc128_acpi_match[] = { > > - { "AANT1280", (kernel_ulong_t)&adc128_config[2] }, > > + { "AANT1280", (kernel_ulong_t)&adc128_config[ADC128_CONFIG_INDEX_124S] }, > > { } > > }; > > MODULE_DEVICE_TABLE(acpi, adc128_acpi_match); > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 2/2] iio: adc: ti-adc128s052: Add lower resolution devices support 2023-10-24 22:52 ` Sukrut Bellary @ 2023-10-25 14:24 ` Jonathan Cameron 2023-10-26 10:15 ` Sukrut Bellary 0 siblings, 1 reply; 7+ messages in thread From: Jonathan Cameron @ 2023-10-25 14:24 UTC (permalink / raw) To: Sukrut Bellary Cc: Jonathan Cameron, Lars-Peter Clausen, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Angelo Compagnucci, Nishanth Menon, linux-iio, devicetree, linux-kernel On Tue, 24 Oct 2023 15:52:24 -0700 Sukrut Bellary <sukrut.bellary@linux.com> wrote: > On Sun, Oct 22, 2023 at 05:00:48PM +0100, Jonathan Cameron wrote: > > On Sat, 21 Oct 2023 20:12:03 -0700 > > Sukrut Bellary <sukrut.bellary@linux.com> wrote: > > > > > The adcxx4s communicates with a host processor via an SPI/Microwire Bus > > > interface. The device family responds with 12-bit data, of which the LSB > > > bits are transmitted by the lower resolution devices as 0. > > > The unavailable bits are 0 in LSB. > > > Shift is calculated per resolution and used in scaling and > > > raw data read. > > > > > > 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 > > > > > > Tested only with ti-adc102s051 on BegalePlay SBC. > > > https://www.beagleboard.org/boards/beagleplay > > > > > > arm64: dts: ti: k3-am625-beagleplay: Add adc102s051 > > > Add adc102s051 support. > > > Tested on beaglePlay SBC > > > https://www.beagleboard.org/boards/beagleplay > > > > > > Co-developed-by: Nishanth Menon <nm@ti.com> > > > Signed-off-by: Nishanth Menon <nm@ti.com> > > > Signed-off-by: Sukrut Bellary <sukrut.bellary@linux.com> > > > --- > > > Changes in v2: > > > - Arranged of_device_id and spi_device_id in numeric order. > > > - Used enum to index into adc128_config. > > > - Reorder adc128_config in alphabetical. > > > - Include channel resolution information. > > > - Shift is calculated per resolution and used in scaling and > > > raw data read. > > > - Link to v1: https://lore.kernel.org/all/20220701042919.18180-3-nm@ti.com/ > > > --- > > > drivers/iio/adc/ti-adc128s052.c | 131 +++++++++++++++++++++++--------- > > > 1 file changed, 96 insertions(+), 35 deletions(-) > > > > > > diff --git a/drivers/iio/adc/ti-adc128s052.c b/drivers/iio/adc/ti-adc128s052.c > > > index a456ea78462f..61e3181b8daf 100644 > > > --- a/drivers/iio/adc/ti-adc128s052.c > > > +++ b/drivers/iio/adc/ti-adc128s052.c > > > @@ -7,6 +7,22 @@ > > > * 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 a 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 > > > + * > > > + * 8, 10, and 12 bits converters send 12-bit data with > > > + * unavailable bits set to 0 in LSB. > > > + * Shift is calculated per resolution and used in scaling and > > > + * raw data read. > > > */ > > > > > > #include <linux/err.h> > > > @@ -53,7 +69,7 @@ static int adc128_adc_conversion(struct adc128 *adc, u8 channel) > > > if (ret < 0) > > > return ret; > > > > > > - return ((adc->buffer[0] << 8 | adc->buffer[1]) & 0xFFF); > > > + return (adc->buffer[0] << 8 | adc->buffer[1]); > > Firstly outer brackets don't add anything. > > Secondly, this is an endian conversion. > > > > return be16_to_cpu(adc->buffer); > > > > should do the job as we know it's aligned enough for a be16. > > > > > > Thanks for the review. > Yes, I will use be16_to_cpu(). > > > > } > > > > > > static int adc128_read_raw(struct iio_dev *indio_dev, > > > @@ -70,7 +86,8 @@ static int adc128_read_raw(struct iio_dev *indio_dev, > > > if (ret < 0) > > > return ret; > > > > > > - *val = ret; > > > + *val = (ret >> channel->scan_type.shift) & > > > + GENMASK(channel->scan_type.realbits - 1, 0); > > > return IIO_VAL_INT; > > > > > > case IIO_CHAN_INFO_SCALE: > > > @@ -80,7 +97,7 @@ static int adc128_read_raw(struct iio_dev *indio_dev, > > > return ret; > > > > > > *val = ret / 1000; > > > - *val2 = 12; > > > + *val2 = channel->scan_type.realbits; > > > return IIO_VAL_FRACTIONAL_LOG2; > > > > > > default: > > > @@ -89,24 +106,34 @@ static int adc128_read_raw(struct iio_dev *indio_dev, > > > > > > } > > > > > > -#define ADC128_VOLTAGE_CHANNEL(num) \ > > > - { \ > > > - .type = IIO_VOLTAGE, \ > > > - .indexed = 1, \ > > > - .channel = (num), \ > > > - .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \ > > > - .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) \ > > > +#define _ADC128_VOLTAGE_CHANNEL(num, real_bits, store_bits) \ > > > + { \ > > > + .type = IIO_VOLTAGE, \ > > > + .indexed = 1, \ > > > + .channel = (num), \ > > > + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \ > > > + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \ > > > + .scan_index = (num), \ > > > + .scan_type = { \ > > > + .sign = 'u', \ > > > + .realbits = (real_bits), \ > > > + .storagebits = (store_bits), \ > > > + .shift = (12 - real_bits), \ > > > + }, \ > > > } > > > > > > -static const struct iio_chan_spec adc128s052_channels[] = { > > > - ADC128_VOLTAGE_CHANNEL(0), > > > - ADC128_VOLTAGE_CHANNEL(1), > > > - ADC128_VOLTAGE_CHANNEL(2), > > > - ADC128_VOLTAGE_CHANNEL(3), > > > - ADC128_VOLTAGE_CHANNEL(4), > > > - ADC128_VOLTAGE_CHANNEL(5), > > > - ADC128_VOLTAGE_CHANNEL(6), > > > - ADC128_VOLTAGE_CHANNEL(7), > > > +#define ADC082_VOLTAGE_CHANNEL(num) _ADC128_VOLTAGE_CHANNEL(num, 8, 16) > > > +#define ADC102_VOLTAGE_CHANNEL(num) _ADC128_VOLTAGE_CHANNEL(num, 10, 16) > > > +#define ADC128_VOLTAGE_CHANNEL(num) _ADC128_VOLTAGE_CHANNEL(num, 12, 16) > > > + > > > +static const struct iio_chan_spec adc082s021_channels[] = { > > > + ADC082_VOLTAGE_CHANNEL(0), > > > + ADC082_VOLTAGE_CHANNEL(1), > > > +}; > > > + > > > +static const struct iio_chan_spec adc102s021_channels[] = { > > > + ADC102_VOLTAGE_CHANNEL(0), > > > + ADC102_VOLTAGE_CHANNEL(1), > > > }; > > > > > > static const struct iio_chan_spec adc122s021_channels[] = { > > > @@ -121,10 +148,32 @@ static const struct iio_chan_spec adc124s021_channels[] = { > > > ADC128_VOLTAGE_CHANNEL(3), > > > }; > > > > > > +static const struct iio_chan_spec adc128s052_channels[] = { > > > + ADC128_VOLTAGE_CHANNEL(0), > > > + ADC128_VOLTAGE_CHANNEL(1), > > > + ADC128_VOLTAGE_CHANNEL(2), > > > + ADC128_VOLTAGE_CHANNEL(3), > > > + ADC128_VOLTAGE_CHANNEL(4), > > > + ADC128_VOLTAGE_CHANNEL(5), > > > + ADC128_VOLTAGE_CHANNEL(6), > > > + ADC128_VOLTAGE_CHANNEL(7), > > > +}; > > > + > > > static const struct adc128_configuration adc128_config[] = { > > > - { adc128s052_channels, ARRAY_SIZE(adc128s052_channels) }, > > > + { adc082s021_channels, ARRAY_SIZE(adc082s021_channels) }, > > > + { adc102s021_channels, ARRAY_SIZE(adc102s021_channels) }, > > > { adc122s021_channels, ARRAY_SIZE(adc122s021_channels) }, > > > { adc124s021_channels, ARRAY_SIZE(adc124s021_channels) }, > > > + { adc128s052_channels, ARRAY_SIZE(adc128s052_channels) }, > > > +}; > > > + > > > +/* Ensure match with adc128_config indices */ > > > > Make sure that is the case by using them when setting it up. > > > > static const struct adc128_configuration adc128_config[] { > > [ADC128_CONFIG_INDEX_082S] = { > > adc082s021_channels, ARRAY_SIZE(..) > > }, > > [ADC128_CONFIG_INDEX_102S] = .. > > } > > > > OK, I will use enum while setting it up > > +static const struct adc128_configuration adc128_config[] = { > + [ADC128_CONFIG_INDEX_082S] = { > + .channels = adc082s021_channels, > + .num_channels = ARRAY_SIZE(adc082s021_channels) > } > > Use enum in spi_device_id[] > > static const struct spi_device_id adc128_id[] = { > + { "adc082s021", (kernel_ulong_t)ADC128_CONFIG_INDEX_082S }, > + { "adc082s051", (kernel_ulong_t)ADC128_CONFIG_INDEX_082S }, > > } > > And use it in probe to extract the config for channels and num_channels using > > config = &adc128_config[spi_get_device_id(spi)->driver_data]; > > since we are not using of_device_id for extracting config, I can remove > the .data entry or need to type case it. Please don't. That is much more fragile than preferring to get the data from the firmware that we matched on. We have some IIO drivers doing this but only because we haven't yet updated them all yet. Jonathan > > static const struct of_device_id adc128_of_match[] = { > { .compatible = "ti,adc082s021", }, > } > > > Or I think you can make it irrelevant by not using an array at all. > > static const struct ad128_configruation adc082s021_config = { > > }; > > > > etc then just use the address of the right one directly. > > > > In this driver, I suspect that is simpler than using the array. > > > > > +enum adc128_configuration_index { > > > + ADC128_CONFIG_INDEX_082S, > > > + ADC128_CONFIG_INDEX_102S, > > > + ADC128_CONFIG_INDEX_122S, > > > + ADC128_CONFIG_INDEX_124S, > > > + ADC128_CONFIG_INDEX_128S, > > > > > }; > > > > > > static const struct iio_info adc128_info = { > > > @@ -177,31 +226,43 @@ static int adc128_probe(struct spi_device *spi) > > > } > > > > > > static const struct of_device_id adc128_of_match[] = { > > > - { .compatible = "ti,adc128s052", .data = &adc128_config[0] }, > > > - { .compatible = "ti,adc122s021", .data = &adc128_config[1] }, > > > - { .compatible = "ti,adc122s051", .data = &adc128_config[1] }, > > > - { .compatible = "ti,adc122s101", .data = &adc128_config[1] }, > > > - { .compatible = "ti,adc124s021", .data = &adc128_config[2] }, > > > - { .compatible = "ti,adc124s051", .data = &adc128_config[2] }, > > > - { .compatible = "ti,adc124s101", .data = &adc128_config[2] }, > > > + { .compatible = "ti,adc082s021", .data = &adc128_config[ADC128_CONFIG_INDEX_082S] }, > > > + { .compatible = "ti,adc082s051", .data = &adc128_config[ADC128_CONFIG_INDEX_082S] }, > > > + { .compatible = "ti,adc082s101", .data = &adc128_config[ADC128_CONFIG_INDEX_082S] }, > > > + { .compatible = "ti,adc102s021", .data = &adc128_config[ADC128_CONFIG_INDEX_102S] }, > > > + { .compatible = "ti,adc102s051", .data = &adc128_config[ADC128_CONFIG_INDEX_102S] }, > > > + { .compatible = "ti,adc102s101", .data = &adc128_config[ADC128_CONFIG_INDEX_102S] }, > > > + { .compatible = "ti,adc122s021", .data = &adc128_config[ADC128_CONFIG_INDEX_122S] }, > > > + { .compatible = "ti,adc122s051", .data = &adc128_config[ADC128_CONFIG_INDEX_122S] }, > > > + { .compatible = "ti,adc122s101", .data = &adc128_config[ADC128_CONFIG_INDEX_122S] }, > > > + { .compatible = "ti,adc124s021", .data = &adc128_config[ADC128_CONFIG_INDEX_124S] }, > > > + { .compatible = "ti,adc124s051", .data = &adc128_config[ADC128_CONFIG_INDEX_124S] }, > > > + { .compatible = "ti,adc124s101", .data = &adc128_config[ADC128_CONFIG_INDEX_124S] }, > > > + { .compatible = "ti,adc128s052", .data = &adc128_config[ADC128_CONFIG_INDEX_128S] }, > > > { /* sentinel */ }, > > > }; > > > MODULE_DEVICE_TABLE(of, adc128_of_match); > > > > > > static const struct spi_device_id adc128_id[] = { > > > - { "adc128s052", (kernel_ulong_t)&adc128_config[0] }, > > > - { "adc122s021", (kernel_ulong_t)&adc128_config[1] }, > > > - { "adc122s051", (kernel_ulong_t)&adc128_config[1] }, > > > - { "adc122s101", (kernel_ulong_t)&adc128_config[1] }, > > > - { "adc124s021", (kernel_ulong_t)&adc128_config[2] }, > > > - { "adc124s051", (kernel_ulong_t)&adc128_config[2] }, > > > - { "adc124s101", (kernel_ulong_t)&adc128_config[2] }, > > > + { "adc082s021", (kernel_ulong_t)&adc128_config[ADC128_CONFIG_INDEX_082S] }, > > > + { "adc082s051", (kernel_ulong_t)&adc128_config[ADC128_CONFIG_INDEX_082S] }, > > > + { "adc082s101", (kernel_ulong_t)&adc128_config[ADC128_CONFIG_INDEX_082S] }, > > > + { "adc102s021", (kernel_ulong_t)&adc128_config[ADC128_CONFIG_INDEX_102S] }, > > > + { "adc102s051", (kernel_ulong_t)&adc128_config[ADC128_CONFIG_INDEX_102S] }, > > > + { "adc102s101", (kernel_ulong_t)&adc128_config[ADC128_CONFIG_INDEX_102S] }, > > > + { "adc122s021", (kernel_ulong_t)&adc128_config[ADC128_CONFIG_INDEX_122S] }, > > > + { "adc122s051", (kernel_ulong_t)&adc128_config[ADC128_CONFIG_INDEX_122S] }, > > > + { "adc122s101", (kernel_ulong_t)&adc128_config[ADC128_CONFIG_INDEX_122S] }, > > > + { "adc124s021", (kernel_ulong_t)&adc128_config[ADC128_CONFIG_INDEX_124S] }, > > > + { "adc124s051", (kernel_ulong_t)&adc128_config[ADC128_CONFIG_INDEX_124S] }, > > > + { "adc124s101", (kernel_ulong_t)&adc128_config[ADC128_CONFIG_INDEX_124S] }, > > > + { "adc128s052", (kernel_ulong_t)&adc128_config[ADC128_CONFIG_INDEX_128S] }, > > > { } > > > }; > > > MODULE_DEVICE_TABLE(spi, adc128_id); > > > > > > static const struct acpi_device_id adc128_acpi_match[] = { > > > - { "AANT1280", (kernel_ulong_t)&adc128_config[2] }, > > > + { "AANT1280", (kernel_ulong_t)&adc128_config[ADC128_CONFIG_INDEX_124S] }, > > > { } > > > }; > > > MODULE_DEVICE_TABLE(acpi, adc128_acpi_match); > > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 2/2] iio: adc: ti-adc128s052: Add lower resolution devices support 2023-10-25 14:24 ` Jonathan Cameron @ 2023-10-26 10:15 ` Sukrut Bellary 0 siblings, 0 replies; 7+ messages in thread From: Sukrut Bellary @ 2023-10-26 10:15 UTC (permalink / raw) To: Jonathan Cameron Cc: Jonathan Cameron, Lars-Peter Clausen, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Angelo Compagnucci, Nishanth Menon, linux-iio, devicetree, linux-kernel On Wed, Oct 25, 2023 at 03:24:00PM +0100, Jonathan Cameron wrote: > On Tue, 24 Oct 2023 15:52:24 -0700 > Sukrut Bellary <sukrut.bellary@linux.com> wrote: > > > On Sun, Oct 22, 2023 at 05:00:48PM +0100, Jonathan Cameron wrote: > > > On Sat, 21 Oct 2023 20:12:03 -0700 > > > Sukrut Bellary <sukrut.bellary@linux.com> wrote: > > > > > > > The adcxx4s communicates with a host processor via an SPI/Microwire Bus > > > > interface. The device family responds with 12-bit data, of which the LSB > > > > bits are transmitted by the lower resolution devices as 0. > > > > The unavailable bits are 0 in LSB. > > > > Shift is calculated per resolution and used in scaling and > > > > raw data read. > > > > > > > > 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 > > > > > > > > Tested only with ti-adc102s051 on BegalePlay SBC. > > > > https://www.beagleboard.org/boards/beagleplay > > > > > > > > arm64: dts: ti: k3-am625-beagleplay: Add adc102s051 > > > > Add adc102s051 support. > > > > Tested on beaglePlay SBC > > > > https://www.beagleboard.org/boards/beagleplay > > > > > > > > Co-developed-by: Nishanth Menon <nm@ti.com> > > > > Signed-off-by: Nishanth Menon <nm@ti.com> > > > > Signed-off-by: Sukrut Bellary <sukrut.bellary@linux.com> > > > > --- > > > > Changes in v2: > > > > - Arranged of_device_id and spi_device_id in numeric order. > > > > - Used enum to index into adc128_config. > > > > - Reorder adc128_config in alphabetical. > > > > - Include channel resolution information. > > > > - Shift is calculated per resolution and used in scaling and > > > > raw data read. > > > > - Link to v1: https://lore.kernel.org/all/20220701042919.18180-3-nm@ti.com/ > > > > --- > > > > drivers/iio/adc/ti-adc128s052.c | 131 +++++++++++++++++++++++--------- > > > > 1 file changed, 96 insertions(+), 35 deletions(-) > > > > > > > > diff --git a/drivers/iio/adc/ti-adc128s052.c b/drivers/iio/adc/ti-adc128s052.c > > > > index a456ea78462f..61e3181b8daf 100644 > > > > --- a/drivers/iio/adc/ti-adc128s052.c > > > > +++ b/drivers/iio/adc/ti-adc128s052.c > > > > @@ -7,6 +7,22 @@ > > > > * 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 a 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 > > > > + * > > > > + * 8, 10, and 12 bits converters send 12-bit data with > > > > + * unavailable bits set to 0 in LSB. > > > > + * Shift is calculated per resolution and used in scaling and > > > > + * raw data read. > > > > */ > > > > > > > > #include <linux/err.h> > > > > @@ -53,7 +69,7 @@ static int adc128_adc_conversion(struct adc128 *adc, u8 channel) > > > > if (ret < 0) > > > > return ret; > > > > > > > > - return ((adc->buffer[0] << 8 | adc->buffer[1]) & 0xFFF); > > > > + return (adc->buffer[0] << 8 | adc->buffer[1]); > > > Firstly outer brackets don't add anything. > > > Secondly, this is an endian conversion. > > > > > > return be16_to_cpu(adc->buffer); > > > > > > should do the job as we know it's aligned enough for a be16. > > > > > > > > > > Thanks for the review. > > Yes, I will use be16_to_cpu(). > > > > > > } > > > > > > > > static int adc128_read_raw(struct iio_dev *indio_dev, > > > > @@ -70,7 +86,8 @@ static int adc128_read_raw(struct iio_dev *indio_dev, > > > > if (ret < 0) > > > > return ret; > > > > > > > > - *val = ret; > > > > + *val = (ret >> channel->scan_type.shift) & > > > > + GENMASK(channel->scan_type.realbits - 1, 0); > > > > return IIO_VAL_INT; > > > > > > > > case IIO_CHAN_INFO_SCALE: > > > > @@ -80,7 +97,7 @@ static int adc128_read_raw(struct iio_dev *indio_dev, > > > > return ret; > > > > > > > > *val = ret / 1000; > > > > - *val2 = 12; > > > > + *val2 = channel->scan_type.realbits; > > > > return IIO_VAL_FRACTIONAL_LOG2; > > > > > > > > default: > > > > @@ -89,24 +106,34 @@ static int adc128_read_raw(struct iio_dev *indio_dev, > > > > > > > > } > > > > > > > > -#define ADC128_VOLTAGE_CHANNEL(num) \ > > > > - { \ > > > > - .type = IIO_VOLTAGE, \ > > > > - .indexed = 1, \ > > > > - .channel = (num), \ > > > > - .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \ > > > > - .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) \ > > > > +#define _ADC128_VOLTAGE_CHANNEL(num, real_bits, store_bits) \ > > > > + { \ > > > > + .type = IIO_VOLTAGE, \ > > > > + .indexed = 1, \ > > > > + .channel = (num), \ > > > > + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \ > > > > + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \ > > > > + .scan_index = (num), \ > > > > + .scan_type = { \ > > > > + .sign = 'u', \ > > > > + .realbits = (real_bits), \ > > > > + .storagebits = (store_bits), \ > > > > + .shift = (12 - real_bits), \ > > > > + }, \ > > > > } > > > > > > > > -static const struct iio_chan_spec adc128s052_channels[] = { > > > > - ADC128_VOLTAGE_CHANNEL(0), > > > > - ADC128_VOLTAGE_CHANNEL(1), > > > > - ADC128_VOLTAGE_CHANNEL(2), > > > > - ADC128_VOLTAGE_CHANNEL(3), > > > > - ADC128_VOLTAGE_CHANNEL(4), > > > > - ADC128_VOLTAGE_CHANNEL(5), > > > > - ADC128_VOLTAGE_CHANNEL(6), > > > > - ADC128_VOLTAGE_CHANNEL(7), > > > > +#define ADC082_VOLTAGE_CHANNEL(num) _ADC128_VOLTAGE_CHANNEL(num, 8, 16) > > > > +#define ADC102_VOLTAGE_CHANNEL(num) _ADC128_VOLTAGE_CHANNEL(num, 10, 16) > > > > +#define ADC128_VOLTAGE_CHANNEL(num) _ADC128_VOLTAGE_CHANNEL(num, 12, 16) > > > > + > > > > +static const struct iio_chan_spec adc082s021_channels[] = { > > > > + ADC082_VOLTAGE_CHANNEL(0), > > > > + ADC082_VOLTAGE_CHANNEL(1), > > > > +}; > > > > + > > > > +static const struct iio_chan_spec adc102s021_channels[] = { > > > > + ADC102_VOLTAGE_CHANNEL(0), > > > > + ADC102_VOLTAGE_CHANNEL(1), > > > > }; > > > > > > > > static const struct iio_chan_spec adc122s021_channels[] = { > > > > @@ -121,10 +148,32 @@ static const struct iio_chan_spec adc124s021_channels[] = { > > > > ADC128_VOLTAGE_CHANNEL(3), > > > > }; > > > > > > > > +static const struct iio_chan_spec adc128s052_channels[] = { > > > > + ADC128_VOLTAGE_CHANNEL(0), > > > > + ADC128_VOLTAGE_CHANNEL(1), > > > > + ADC128_VOLTAGE_CHANNEL(2), > > > > + ADC128_VOLTAGE_CHANNEL(3), > > > > + ADC128_VOLTAGE_CHANNEL(4), > > > > + ADC128_VOLTAGE_CHANNEL(5), > > > > + ADC128_VOLTAGE_CHANNEL(6), > > > > + ADC128_VOLTAGE_CHANNEL(7), > > > > +}; > > > > + > > > > static const struct adc128_configuration adc128_config[] = { > > > > - { adc128s052_channels, ARRAY_SIZE(adc128s052_channels) }, > > > > + { adc082s021_channels, ARRAY_SIZE(adc082s021_channels) }, > > > > + { adc102s021_channels, ARRAY_SIZE(adc102s021_channels) }, > > > > { adc122s021_channels, ARRAY_SIZE(adc122s021_channels) }, > > > > { adc124s021_channels, ARRAY_SIZE(adc124s021_channels) }, > > > > + { adc128s052_channels, ARRAY_SIZE(adc128s052_channels) }, > > > > +}; > > > > + > > > > +/* Ensure match with adc128_config indices */ > > > > > > Make sure that is the case by using them when setting it up. > > > > > > static const struct adc128_configuration adc128_config[] { > > > [ADC128_CONFIG_INDEX_082S] = { > > > adc082s021_channels, ARRAY_SIZE(..) > > > }, > > > [ADC128_CONFIG_INDEX_102S] = .. > > > } > > > > > > > OK, I will use enum while setting it up > > > > +static const struct adc128_configuration adc128_config[] = { > > + [ADC128_CONFIG_INDEX_082S] = { > > + .channels = adc082s021_channels, > > + .num_channels = ARRAY_SIZE(adc082s021_channels) > > } > > > > Use enum in spi_device_id[] > > > > static const struct spi_device_id adc128_id[] = { > > + { "adc082s021", (kernel_ulong_t)ADC128_CONFIG_INDEX_082S }, > > + { "adc082s051", (kernel_ulong_t)ADC128_CONFIG_INDEX_082S }, > > > > } > > > > And use it in probe to extract the config for channels and num_channels using > > > > config = &adc128_config[spi_get_device_id(spi)->driver_data]; > > > > since we are not using of_device_id for extracting config, I can remove > > the .data entry or need to type case it. > > Please don't. That is much more fragile than preferring to get the data > from the firmware that we matched on. > > We have some IIO drivers doing this but only because we haven't yet updated > them all yet. > > Jonathan > OK, I will keep the of_device_id and spi_device_id as v2. static const struct of_device_id adc128_of_match[] = { + { .compatible = "ti,adc082s021", .data = &adc128_config[ADC128_CONFIG_INDEX_082S] }, + { .compatible = "ti,adc082s051", .data = &adc128_config[ADC128_CONFIG_INDEX_082S] }, } static const struct spi_device_id adc128_id[] = { + { "adc082s021", (kernel_ulong_t)&adc128_config[ADC128_CONFIG_INDEX_082S] }, + { "adc082s051", (kernel_ulong_t)&adc128_config[ADC128_CONFIG_INDEX_082S] }, } Currently it uses spi_get_device_match_data() to extract config. And in v3, I will use enum while setting up config. +static const struct adc128_configuration adc128_config[] = { + [ADC128_CONFIG_INDEX_082S] = { + .channels = adc082s021_channels, + .num_channels = ARRAY_SIZE(adc082s021_channels) } > > > > static const struct of_device_id adc128_of_match[] = { > > { .compatible = "ti,adc082s021", }, > > } > > > > > Or I think you can make it irrelevant by not using an array at all. > > > static const struct ad128_configruation adc082s021_config = { > > > }; > > > > > > etc then just use the address of the right one directly. > > > > > > In this driver, I suspect that is simpler than using the array. > > > > > > > +enum adc128_configuration_index { > > > > + ADC128_CONFIG_INDEX_082S, > > > > + ADC128_CONFIG_INDEX_102S, > > > > + ADC128_CONFIG_INDEX_122S, > > > > + ADC128_CONFIG_INDEX_124S, > > > > + ADC128_CONFIG_INDEX_128S, > > > > > > > }; > > > > > > > > static const struct iio_info adc128_info = { > > > > @@ -177,31 +226,43 @@ static int adc128_probe(struct spi_device *spi) > > > > } > > > > > > > > static const struct of_device_id adc128_of_match[] = { > > > > - { .compatible = "ti,adc128s052", .data = &adc128_config[0] }, > > > > - { .compatible = "ti,adc122s021", .data = &adc128_config[1] }, > > > > - { .compatible = "ti,adc122s051", .data = &adc128_config[1] }, > > > > - { .compatible = "ti,adc122s101", .data = &adc128_config[1] }, > > > > - { .compatible = "ti,adc124s021", .data = &adc128_config[2] }, > > > > - { .compatible = "ti,adc124s051", .data = &adc128_config[2] }, > > > > - { .compatible = "ti,adc124s101", .data = &adc128_config[2] }, > > > > + { .compatible = "ti,adc082s021", .data = &adc128_config[ADC128_CONFIG_INDEX_082S] }, > > > > + { .compatible = "ti,adc082s051", .data = &adc128_config[ADC128_CONFIG_INDEX_082S] }, > > > > + { .compatible = "ti,adc082s101", .data = &adc128_config[ADC128_CONFIG_INDEX_082S] }, > > > > + { .compatible = "ti,adc102s021", .data = &adc128_config[ADC128_CONFIG_INDEX_102S] }, > > > > + { .compatible = "ti,adc102s051", .data = &adc128_config[ADC128_CONFIG_INDEX_102S] }, > > > > + { .compatible = "ti,adc102s101", .data = &adc128_config[ADC128_CONFIG_INDEX_102S] }, > > > > + { .compatible = "ti,adc122s021", .data = &adc128_config[ADC128_CONFIG_INDEX_122S] }, > > > > + { .compatible = "ti,adc122s051", .data = &adc128_config[ADC128_CONFIG_INDEX_122S] }, > > > > + { .compatible = "ti,adc122s101", .data = &adc128_config[ADC128_CONFIG_INDEX_122S] }, > > > > + { .compatible = "ti,adc124s021", .data = &adc128_config[ADC128_CONFIG_INDEX_124S] }, > > > > + { .compatible = "ti,adc124s051", .data = &adc128_config[ADC128_CONFIG_INDEX_124S] }, > > > > + { .compatible = "ti,adc124s101", .data = &adc128_config[ADC128_CONFIG_INDEX_124S] }, > > > > + { .compatible = "ti,adc128s052", .data = &adc128_config[ADC128_CONFIG_INDEX_128S] }, > > > > { /* sentinel */ }, > > > > }; > > > > MODULE_DEVICE_TABLE(of, adc128_of_match); > > > > > > > > static const struct spi_device_id adc128_id[] = { > > > > - { "adc128s052", (kernel_ulong_t)&adc128_config[0] }, > > > > - { "adc122s021", (kernel_ulong_t)&adc128_config[1] }, > > > > - { "adc122s051", (kernel_ulong_t)&adc128_config[1] }, > > > > - { "adc122s101", (kernel_ulong_t)&adc128_config[1] }, > > > > - { "adc124s021", (kernel_ulong_t)&adc128_config[2] }, > > > > - { "adc124s051", (kernel_ulong_t)&adc128_config[2] }, > > > > - { "adc124s101", (kernel_ulong_t)&adc128_config[2] }, > > > > + { "adc082s021", (kernel_ulong_t)&adc128_config[ADC128_CONFIG_INDEX_082S] }, > > > > + { "adc082s051", (kernel_ulong_t)&adc128_config[ADC128_CONFIG_INDEX_082S] }, > > > > + { "adc082s101", (kernel_ulong_t)&adc128_config[ADC128_CONFIG_INDEX_082S] }, > > > > + { "adc102s021", (kernel_ulong_t)&adc128_config[ADC128_CONFIG_INDEX_102S] }, > > > > + { "adc102s051", (kernel_ulong_t)&adc128_config[ADC128_CONFIG_INDEX_102S] }, > > > > + { "adc102s101", (kernel_ulong_t)&adc128_config[ADC128_CONFIG_INDEX_102S] }, > > > > + { "adc122s021", (kernel_ulong_t)&adc128_config[ADC128_CONFIG_INDEX_122S] }, > > > > + { "adc122s051", (kernel_ulong_t)&adc128_config[ADC128_CONFIG_INDEX_122S] }, > > > > + { "adc122s101", (kernel_ulong_t)&adc128_config[ADC128_CONFIG_INDEX_122S] }, > > > > + { "adc124s021", (kernel_ulong_t)&adc128_config[ADC128_CONFIG_INDEX_124S] }, > > > > + { "adc124s051", (kernel_ulong_t)&adc128_config[ADC128_CONFIG_INDEX_124S] }, > > > > + { "adc124s101", (kernel_ulong_t)&adc128_config[ADC128_CONFIG_INDEX_124S] }, > > > > + { "adc128s052", (kernel_ulong_t)&adc128_config[ADC128_CONFIG_INDEX_128S] }, > > > > { } > > > > }; > > > > MODULE_DEVICE_TABLE(spi, adc128_id); > > > > > > > > static const struct acpi_device_id adc128_acpi_match[] = { > > > > - { "AANT1280", (kernel_ulong_t)&adc128_config[2] }, > > > > + { "AANT1280", (kernel_ulong_t)&adc128_config[ADC128_CONFIG_INDEX_124S] }, > > > > { } > > > > }; > > > > MODULE_DEVICE_TABLE(acpi, adc128_acpi_match); > > > > > > ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2023-10-26 10:15 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-10-22 3:12 [PATCH v2 0/2] iio: adc: ti-adc128s052: Add support for adc102s021 and family Sukrut Bellary 2023-10-22 3:12 ` [PATCH v2 1/2] dt-bindings: iio: adc: ti,adc128s052: Add adc08c and adc10c family Sukrut Bellary 2023-10-22 3:12 ` [PATCH v2 2/2] iio: adc: ti-adc128s052: Add lower resolution devices support Sukrut Bellary 2023-10-22 16:00 ` Jonathan Cameron 2023-10-24 22:52 ` Sukrut Bellary 2023-10-25 14:24 ` Jonathan Cameron 2023-10-26 10:15 ` Sukrut Bellary
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).