Re: [PATCH v5 0/2] iio: adc: Add the Ingenic X1600 ADC

[H. Nikolaus Schaller]

> Am 05.11.2026 um 22:32 schrieb Olivier Louvignes <olivier@xxxxxxxxx>:
> 
> A driver for the SAR ADC of the Ingenic X1600: four auxiliary channels,
> 12 bits, one interrupt per channel. Tested on a HiBy R1, whose keys are on
> channel 0 behind adc-keys.
> 
> As in v3 and v4, this version is X1600-only. The X1830 and the X2000 have
> the same register layout by their programming manuals, and a chip_info
> with the channel count and the resolution can add them later, with a test
> on either.
> 
> Changes in v5:
> - Binding: the reference is the SADC_VREFP pin (X1600 datasheet: ball
>  P12, 3.3 V typical, at most the ADC's own supply), so an optional
>  vref-supply, and the description says so in Nikolaus's words. The
>  driver takes the supply's voltage, and 3.3 V without one. The ADC's
>  analog supply (SADC_AVDD) is left out, like the SoC's other supply
>  pins (Conor's question; Nikolaus answered it).
> - Binding example: reg covers the 4 KiB the programming manual's memory
>  map gives the SADC (Conor; Nikolaus on the map).
> - The driver's declarations in reverse xmas tree, the timeout macro in
>  jiffies with an if/else on the wait, a blank line, and clamp()'s
>  bounds on one line (Andy).
> - The clock stays unnamed: Krzysztof (on v2) and Conor (on v1) asked for
>  clock-names to go, as the block has one clock. A variant with more
>  would come with a compatible of its own and could name them there
>  (Andy asked for a name).
> - The name stays "adc" rather than the vendor's "sadc" (Nikolaus): the
>  JZ4740 datasheet calls its block the SADC too, and mainline binds it
>  as ingenic,jz4740-adc.

Agreed. I wasn't aware of that. So let's stay consistent with the inconsistency :)

> - Andy's Reviewed-by on patch 2, and Nikolaus's Tested-by (LX16, driver
>  only, given on v4) on both.
> - Rebased on togreg after -rc1.
> 
> Tested on the HiBy R1 with v5: without a vref-supply, as the R1 is
> described, the scale falls back to 3.3 V and the three buttons on
> channel 0 work through adc-keys; with a vref-supply pointed, for the
> test only, at the board's 3.3 V PMIC rail, the driver waits for the
> regulator and takes its voltage. v5 is also built with W=1 with and
> without CONFIG_REGULATOR.
> 
> Changes in v4:
> - The Kconfig prompt, the header comment and MODULE_DESCRIPTION() are
>  the same text (Andy).
> - The read computes the data register's bank and half next to each
>  other, and the two macros take them (Andy).
> - X1600_ADC_CLK_RATE_HZ (Andy).
> - Read and write helpers that add the base (Andy). Plain MMIO rather
>  than regmap: nothing is cached or shared, and the only
>  read-modify-writes are under the conversion mutex or in probe.
> - The device clock rate is clamped before dividing (Andy); the divider
>  comes out the same for every rate.
> - Binding: the description's last sentence and clock-names dropped; the
>  driver takes its one clock unnamed (Krzysztof on v2 and Conor on v1:
>  their reviews and v3 crossed).
> - Joshua's Reviewed-by on patch 2: the changes to it are the points
>  above, with no change in behaviour.
> 
> Changes in v3:
> - Cover letter (Andy, Joshua).
> - MAINTAINERS: no list line; the binding's path in patch 1, the driver's
>  in patch 2 (Joshua).
> - Includes for what the driver uses, copyright line, MODULE_AUTHOR(),
>  reverse xmas tree, open-coded return, no message after
>  devm_request_irq(), a blank line (Joshua; Andy on the return).
> - fsleep(2 * USEC_PER_MSEC) (Andy).
> - The ADC clock divider is now even, as the X1600 programming manual
>  has it; v2 wrote the divider minus one, as the vendor kernel does. The
>  divider fields are named as in the manual, and the two clocks only
>  repeated sampling uses stay at their reset values.
> - The probe comment on staying powered up no longer names the keys
>  (Jonathan).
> 
> Changes in v2 (Jonathan):
> - The comparison with the JZ47xx ADCs dropped from the file header.
> - No mod_devicetable.h include.
> - X1600_ADC_VREF_mV.
> - The status declaration split from its readl() in the interrupt
>  handler.
> - field_get() for the channel's result.
> - fsleep(2000) instead of usleep_range().
> 
> v4: https://lore.kernel.org/all/20261102201155.10948-1-olivier@xxxxxxxxx/
> v3: https://lore.kernel.org/all/20261026083631.48406-1-olivier@xxxxxxxxx/
> v2: https://lore.kernel.org/all/20261024041357.34346-1-olivier@xxxxxxxxx/
> v1: https://lore.kernel.org/all/20261023204209.48556-1-olivier@xxxxxxxxx/
> 
> Olivier Louvignes (2):
>  dt-bindings: iio: adc: Add the Ingenic X1600 ADC
>  iio: adc: Add a driver for the Ingenic X1600 ADC
> 
> .../bindings/iio/adc/ingenic,x1600-adc.yaml   |  59 ++++
> MAINTAINERS                                   |   6 +
> drivers/iio/adc/Kconfig                       |  10 +
> drivers/iio/adc/Makefile                      |   1 +
> drivers/iio/adc/ingenic-x1600-adc.c           | 269 ++++++++++++++++++
> 5 files changed, 345 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/iio/adc/ingenic,x1600-adc.yaml
> create mode 100644 drivers/iio/adc/ingenic-x1600-adc.c
> 
> 
> base-commit: 1390ca4953a7400ca932cb49635f02c727f885a6
> -- 
> 2.39.5
>