* [PATCH v4 0/2] iio: adc: Add the Ingenic X1600 ADC @ 2026-11-02 20:11 Olivier Louvignes 2026-11-02 20:11 ` [PATCH v4 1/2] dt-bindings: " Olivier Louvignes 2026-11-02 20:11 ` [PATCH v4 2/2] iio: adc: Add a driver for " Olivier Louvignes 0 siblings, 2 replies; 6+ messages in thread From: Olivier Louvignes @ 2026-11-02 20:11 UTC (permalink / raw) To: Jonathan Cameron Cc: David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Paul Cercueil, H . Nikolaus Schaller, Joshua Crofts, linux-iio, devicetree, linux-mips, linux-kernel, Olivier Louvignes 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, 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 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(). v3: https://lore.kernel.org/all/20261026083631.48406-1-olivier@mgcrea.io/ v2: https://lore.kernel.org/all/20261024041357.34346-1-olivier@mgcrea.io/ v1: https://lore.kernel.org/all/20261023204209.48556-1-olivier@mgcrea.io/ 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 | 53 ++++ MAINTAINERS | 6 + drivers/iio/adc/Kconfig | 10 + drivers/iio/adc/Makefile | 1 + drivers/iio/adc/ingenic-x1600-adc.c | 257 ++++++++++++++++++ 5 files changed, 327 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: a3b3580713f3ac5a32dc2874ee546828977a1d68 -- 2.54.0 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v4 1/2] dt-bindings: iio: adc: Add the Ingenic X1600 ADC 2026-11-02 20:11 [PATCH v4 0/2] iio: adc: Add the Ingenic X1600 ADC Olivier Louvignes @ 2026-11-02 20:11 ` Olivier Louvignes 2026-11-02 20:48 ` Conor Dooley 2026-11-02 20:11 ` [PATCH v4 2/2] iio: adc: Add a driver for " Olivier Louvignes 1 sibling, 1 reply; 6+ messages in thread From: Olivier Louvignes @ 2026-11-02 20:11 UTC (permalink / raw) To: Jonathan Cameron Cc: David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Paul Cercueil, H . Nikolaus Schaller, Joshua Crofts, linux-iio, devicetree, linux-mips, linux-kernel, Olivier Louvignes The X1600's SAR ADC has four auxiliary channels and none of the touchscreen or battery engines of the JZ47xx ADCs in ingenic,adc.yaml, so it gets a binding of its own. Signed-off-by: Olivier Louvignes <olivier@mgcrea.io> --- .../bindings/iio/adc/ingenic,x1600-adc.yaml | 53 +++++++++++++++++++ MAINTAINERS | 5 ++ 2 files changed, 58 insertions(+) create mode 100644 Documentation/devicetree/bindings/iio/adc/ingenic,x1600-adc.yaml diff --git a/Documentation/devicetree/bindings/iio/adc/ingenic,x1600-adc.yaml b/Documentation/devicetree/bindings/iio/adc/ingenic,x1600-adc.yaml new file mode 100644 index 000000000000..f6356ea97980 --- /dev/null +++ b/Documentation/devicetree/bindings/iio/adc/ingenic,x1600-adc.yaml @@ -0,0 +1,53 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/iio/adc/ingenic,x1600-adc.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Ingenic X1600 ADC + +maintainers: + - Olivier Louvignes <olivier@mgcrea.io> + +description: + The SAR ADC of the Ingenic X1600 SoC, with four auxiliary input channels + measured against a 3.3 V reference. + +properties: + compatible: + const: ingenic,x1600-adc + + reg: + maxItems: 1 + + clocks: + maxItems: 1 + + interrupts: + maxItems: 1 + + '#io-channel-cells': + const: 1 + +required: + - compatible + - reg + - clocks + - interrupts + - '#io-channel-cells' + +additionalProperties: false + +examples: + - | + adc@10070000 { + compatible = "ingenic,x1600-adc"; + reg = <0x10070000 0x30>; + + clocks = <&cgu 28>; + + interrupt-parent = <&intc>; + interrupts = <11>; + + #io-channel-cells = <1>; + }; diff --git a/MAINTAINERS b/MAINTAINERS index ecc22d1149d1..fd4bb5c6bf16 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -12925,6 +12925,11 @@ F: include/linux/mfd/ingenic-tcu.h F: sound/soc/codecs/jz47* F: sound/soc/jz4740/ +INGENIC X1600 ADC DRIVER +M: Olivier Louvignes <olivier@mgcrea.io> +S: Maintained +F: Documentation/devicetree/bindings/iio/adc/ingenic,x1600-adc.yaml + INJOINIC IP5xxx POWER BANK IC DRIVER M: Samuel Holland <samuel@sholland.org> S: Maintained -- 2.54.0 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v4 1/2] dt-bindings: iio: adc: Add the Ingenic X1600 ADC 2026-11-02 20:11 ` [PATCH v4 1/2] dt-bindings: " Olivier Louvignes @ 2026-11-02 20:48 ` Conor Dooley 0 siblings, 0 replies; 6+ messages in thread From: Conor Dooley @ 2026-11-02 20:48 UTC (permalink / raw) To: Olivier Louvignes Cc: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Paul Cercueil, H . Nikolaus Schaller, Joshua Crofts, linux-iio, devicetree, linux-mips, linux-kernel [-- Attachment #1: Type: text/plain, Size: 2889 bytes --] On Mon, Nov 02, 2026 at 09:11:54PM +0100, Olivier Louvignes wrote: > The X1600's SAR ADC has four auxiliary channels and none of the > touchscreen or battery engines of the JZ47xx ADCs in ingenic,adc.yaml, > so it gets a binding of its own. > > Signed-off-by: Olivier Louvignes <olivier@mgcrea.io> > --- > .../bindings/iio/adc/ingenic,x1600-adc.yaml | 53 +++++++++++++++++++ > MAINTAINERS | 5 ++ > 2 files changed, 58 insertions(+) > create mode 100644 Documentation/devicetree/bindings/iio/adc/ingenic,x1600-adc.yaml > > diff --git a/Documentation/devicetree/bindings/iio/adc/ingenic,x1600-adc.yaml b/Documentation/devicetree/bindings/iio/adc/ingenic,x1600-adc.yaml > new file mode 100644 > index 000000000000..f6356ea97980 > --- /dev/null > +++ b/Documentation/devicetree/bindings/iio/adc/ingenic,x1600-adc.yaml > @@ -0,0 +1,53 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/iio/adc/ingenic,x1600-adc.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Ingenic X1600 ADC > + > +maintainers: > + - Olivier Louvignes <olivier@mgcrea.io> > + > +description: > + The SAR ADC of the Ingenic X1600 SoC, with four auxiliary input channels > + measured against a 3.3 V reference. The wording here makes me wonder if a 3v3 supply is missing here? Or is this some internal supply on the soc? Otherwise, this looks fine really. > + > +properties: > + compatible: > + const: ingenic,x1600-adc > + > + reg: > + maxItems: 1 > + > + clocks: > + maxItems: 1 > + > + interrupts: > + maxItems: 1 > + > + '#io-channel-cells': > + const: 1 > + > +required: > + - compatible > + - reg > + - clocks > + - interrupts > + - '#io-channel-cells' > + > +additionalProperties: false > + > +examples: > + - | > + adc@10070000 { > + compatible = "ingenic,x1600-adc"; > + reg = <0x10070000 0x30>; That said, the 0x30 looks suspiciously small - but the region is aligned neatly so I suppose it's probably not problematic.. Cheers, Conor. > + > + clocks = <&cgu 28>; > + > + interrupt-parent = <&intc>; > + interrupts = <11>; > + > + #io-channel-cells = <1>; > + }; > diff --git a/MAINTAINERS b/MAINTAINERS > index ecc22d1149d1..fd4bb5c6bf16 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -12925,6 +12925,11 @@ F: include/linux/mfd/ingenic-tcu.h > F: sound/soc/codecs/jz47* > F: sound/soc/jz4740/ > > +INGENIC X1600 ADC DRIVER > +M: Olivier Louvignes <olivier@mgcrea.io> > +S: Maintained > +F: Documentation/devicetree/bindings/iio/adc/ingenic,x1600-adc.yaml > + > INJOINIC IP5xxx POWER BANK IC DRIVER > M: Samuel Holland <samuel@sholland.org> > S: Maintained > -- > 2.54.0 > [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v4 2/2] iio: adc: Add a driver for the Ingenic X1600 ADC 2026-11-02 20:11 [PATCH v4 0/2] iio: adc: Add the Ingenic X1600 ADC Olivier Louvignes 2026-11-02 20:11 ` [PATCH v4 1/2] dt-bindings: " Olivier Louvignes @ 2026-11-02 20:11 ` Olivier Louvignes 2026-11-03 10:47 ` Andy Shevchenko 1 sibling, 1 reply; 6+ messages in thread From: Olivier Louvignes @ 2026-11-02 20:11 UTC (permalink / raw) To: Jonathan Cameron Cc: David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Paul Cercueil, H . Nikolaus Schaller, Joshua Crofts, linux-iio, devicetree, linux-mips, linux-kernel, Olivier Louvignes The X1600's SAR ADC has four auxiliary channels, each with its own start bit, interrupt flag and 12-bit result, and none of the touchscreen or battery engines of the JZ47xx ADCs that ingenic-adc drives. Add a small driver that exposes the four channels as voltages against the 3.3 V reference. The registers are in chapter 19 of the X1600 programming manual. The ADC clock divider is even, as the manual has it (an odd value divides by one less); the vendor kernel writes the divider minus one, which gives a slightly faster clock than it asks for. Tested on a HiBy R1, whose play/pause and volume keys sit on channel 0 behind adc-keys: all three read and report correctly. Signed-off-by: Olivier Louvignes <olivier@mgcrea.io> Reviewed-by: Joshua Crofts <joshua.crofts1@gmail.com> --- MAINTAINERS | 1 + drivers/iio/adc/Kconfig | 10 ++ drivers/iio/adc/Makefile | 1 + drivers/iio/adc/ingenic-x1600-adc.c | 257 ++++++++++++++++++++++++++++ 4 files changed, 269 insertions(+) create mode 100644 drivers/iio/adc/ingenic-x1600-adc.c diff --git a/MAINTAINERS b/MAINTAINERS index fd4bb5c6bf16..d1472b569fc0 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -12929,6 +12929,7 @@ INGENIC X1600 ADC DRIVER M: Olivier Louvignes <olivier@mgcrea.io> S: Maintained F: Documentation/devicetree/bindings/iio/adc/ingenic,x1600-adc.yaml +F: drivers/iio/adc/ingenic-x1600-adc.c INJOINIC IP5xxx POWER BANK IC DRIVER M: Samuel Holland <samuel@sholland.org> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig index a802d8ee4dd9..7b5c3d3fcfdb 100644 --- a/drivers/iio/adc/Kconfig +++ b/drivers/iio/adc/Kconfig @@ -881,6 +881,16 @@ config INGENIC_ADC This driver can also be built as a module. If so, the module will be called ingenic_adc. +config INGENIC_X1600_ADC + tristate "Ingenic X1600 ADC driver" + depends on MIPS || COMPILE_TEST + help + Say yes here to build support for the SAR ADC of the Ingenic X1600 + SoC and its four auxiliary channels. + + This driver can also be built as a module. If so, the module will be + called ingenic-x1600-adc. + config INTEL_DC_TI_ADC tristate "Intel Bay Trail / Cherry Trail Dollar Cove TI ADC driver" depends on INTEL_SOC_PMIC_CHTDC_TI diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile index 9e82c21dbd37..f76e62b0cd04 100644 --- a/drivers/iio/adc/Makefile +++ b/drivers/iio/adc/Makefile @@ -77,6 +77,7 @@ obj-$(CONFIG_IMX8QXP_ADC) += imx8qxp-adc.o obj-$(CONFIG_IMX93_ADC) += imx93_adc.o obj-$(CONFIG_INA2XX_ADC) += ina2xx-adc.o obj-$(CONFIG_INGENIC_ADC) += ingenic-adc.o +obj-$(CONFIG_INGENIC_X1600_ADC) += ingenic-x1600-adc.o obj-$(CONFIG_INTEL_DC_TI_ADC) += intel_dc_ti_adc.o obj-$(CONFIG_INTEL_MRFLD_ADC) += intel_mrfld_adc.o obj-$(CONFIG_LP8788_ADC) += lp8788_adc.o diff --git a/drivers/iio/adc/ingenic-x1600-adc.c b/drivers/iio/adc/ingenic-x1600-adc.c new file mode 100644 index 000000000000..03a26e2bffc3 --- /dev/null +++ b/drivers/iio/adc/ingenic-x1600-adc.c @@ -0,0 +1,257 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Ingenic X1600 ADC driver + * + * Copyright (c) 2026 Olivier Louvignes <olivier@mgcrea.io> + * + * The X1600's SAR ADC has four auxiliary channels, each with its own start + * bit, interrupt and 12-bit result (X1600 programming manual, chapter 19). + */ + +#include <linux/array_size.h> +#include <linux/bitfield.h> +#include <linux/bits.h> +#include <linux/cleanup.h> +#include <linux/clk.h> +#include <linux/completion.h> +#include <linux/delay.h> +#include <linux/err.h> +#include <linux/iio/iio.h> +#include <linux/interrupt.h> +#include <linux/io.h> +#include <linux/jiffies.h> +#include <linux/math.h> +#include <linux/minmax.h> +#include <linux/module.h> +#include <linux/mutex.h> +#include <linux/platform_device.h> +#include <linux/time.h> +#include <linux/types.h> + +#define X1600_ADC_REG_ENABLE 0x00 +#define X1600_ADC_REG_CTRL 0x08 +#define X1600_ADC_REG_STATUS 0x0c +#define X1600_ADC_REG_DATA(bank) (0x10 + (bank) * 4) +#define X1600_ADC_REG_CLKDIV 0x20 +#define X1600_ADC_REG_STABLE 0x24 + +/* ENABLE: bits 3..0 start a channel's conversion; bit 15 powers down. */ +#define X1600_ADC_ENABLE_POWER_DOWN BIT(15) +/* CTRL: bits 3..0 mask the channels' interrupts. STATUS: write 1 to clear. */ +#define X1600_ADC_CHANNELS_MASK GENMASK(3, 0) +#define X1600_ADC_DATA_MASK(shift) (GENMASK(11, 0) << ((shift) * 16)) +/* CLKDIV: an even divider from the device clock; US and MS: for repeats. */ +#define X1600_ADC_CLKDIV_CLKDIV GENMASK(7, 0) +#define X1600_ADC_CLKDIV_US GENMASK(15, 8) +#define X1600_ADC_CLKDIV_MS GENMASK(31, 16) +#define X1600_ADC_STABLE_TIME GENMASK(15, 0) + +#define X1600_ADC_NUM_CHANNELS 4 +#define X1600_ADC_VREF_mV 3300 +#define X1600_ADC_BITS 12 +/* The ADC clock the vendor kernel runs at. */ +#define X1600_ADC_CLK_RATE_HZ 1166667 +#define X1600_ADC_TIMEOUT_MS 20 + +struct x1600_adc { + void __iomem *base; + struct clk *clk; + struct completion done; + /* Serialises conversions: there is one completion for all channels. */ + struct mutex lock; +}; + +static u32 x1600_adc_read(struct x1600_adc *adc, unsigned int offset) +{ + return readl(adc->base + offset); +} + +static void x1600_adc_write(struct x1600_adc *adc, unsigned int offset, u32 value) +{ + writel(value, adc->base + offset); +} + +static irqreturn_t x1600_adc_irq(int irq, void *data) +{ + struct x1600_adc *adc = data; + u32 status; + + status = x1600_adc_read(adc, X1600_ADC_REG_STATUS) & X1600_ADC_CHANNELS_MASK; + if (!status) + return IRQ_NONE; + + x1600_adc_write(adc, X1600_ADC_REG_STATUS, status); + complete(&adc->done); + return IRQ_HANDLED; +} + +static int x1600_adc_read_raw_channel(struct x1600_adc *adc, unsigned int ch, int *val) +{ + unsigned int bank, shift; + int ret = 0; + u32 enable, data; + + guard(mutex)(&adc->lock); + + reinit_completion(&adc->done); + x1600_adc_write(adc, X1600_ADC_REG_STATUS, BIT(ch)); + enable = x1600_adc_read(adc, X1600_ADC_REG_ENABLE); + x1600_adc_write(adc, X1600_ADC_REG_ENABLE, enable | BIT(ch)); + + if (!wait_for_completion_timeout(&adc->done, + msecs_to_jiffies(X1600_ADC_TIMEOUT_MS))) + ret = -ETIMEDOUT; + + enable = x1600_adc_read(adc, X1600_ADC_REG_ENABLE); + x1600_adc_write(adc, X1600_ADC_REG_ENABLE, enable & ~BIT(ch)); + if (ret) + return ret; + + /* Two results to a data register, the even channel's in the low half. */ + bank = ch / 2; + shift = ch % 2; + data = x1600_adc_read(adc, X1600_ADC_REG_DATA(bank)); + *val = field_get(X1600_ADC_DATA_MASK(shift), data); + return 0; +} + +static int x1600_adc_read_raw(struct iio_dev *iio_dev, + struct iio_chan_spec const *chan, + int *val, int *val2, long mask) +{ + struct x1600_adc *adc = iio_priv(iio_dev); + int ret; + + switch (mask) { + case IIO_CHAN_INFO_RAW: + ret = x1600_adc_read_raw_channel(adc, chan->channel, val); + if (ret) + return ret; + + return IIO_VAL_INT; + case IIO_CHAN_INFO_SCALE: + *val = X1600_ADC_VREF_mV; + *val2 = X1600_ADC_BITS; + return IIO_VAL_FRACTIONAL_LOG2; + default: + return -EINVAL; + } +} + +static const struct iio_info x1600_adc_info = { + .read_raw = x1600_adc_read_raw, +}; + +#define X1600_ADC_CHANNEL(_ch) { \ + .type = IIO_VOLTAGE, \ + .indexed = 1, \ + .channel = (_ch), \ + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \ + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \ +} + +static const struct iio_chan_spec x1600_adc_channels[] = { + X1600_ADC_CHANNEL(0), + X1600_ADC_CHANNEL(1), + X1600_ADC_CHANNEL(2), + X1600_ADC_CHANNEL(3), +}; + +static void x1600_adc_power_down(void *data) +{ + struct x1600_adc *adc = data; + + x1600_adc_write(adc, X1600_ADC_REG_CTRL, X1600_ADC_CHANNELS_MASK); + x1600_adc_write(adc, X1600_ADC_REG_ENABLE, X1600_ADC_ENABLE_POWER_DOWN); +} + +static int x1600_adc_probe(struct platform_device *pdev) +{ + struct device *dev = &pdev->dev; + struct iio_dev *iio_dev; + struct x1600_adc *adc; + unsigned long rate; + u32 div, reg; + int irq, ret; + + iio_dev = devm_iio_device_alloc(dev, sizeof(*adc)); + if (!iio_dev) + return -ENOMEM; + + adc = iio_priv(iio_dev); + init_completion(&adc->done); + ret = devm_mutex_init(dev, &adc->lock); + if (ret) + return ret; + + adc->base = devm_platform_ioremap_resource(pdev, 0); + if (IS_ERR(adc->base)) + return PTR_ERR(adc->base); + + adc->clk = devm_clk_get_enabled(dev, NULL); + if (IS_ERR(adc->clk)) + return dev_err_probe(dev, PTR_ERR(adc->clk), "Unable to get clock\n"); + + /* Powered down, interrupts masked and cleared, as the vendor does. */ + x1600_adc_write(adc, X1600_ADC_REG_ENABLE, X1600_ADC_ENABLE_POWER_DOWN); + x1600_adc_write(adc, X1600_ADC_REG_CTRL, X1600_ADC_CHANNELS_MASK); + x1600_adc_write(adc, X1600_ADC_REG_STATUS, X1600_ADC_CHANNELS_MASK); + + rate = clamp(clk_get_rate(adc->clk), 2UL * X1600_ADC_CLK_RATE_HZ, + 254UL * X1600_ADC_CLK_RATE_HZ); + div = round_up(DIV_ROUND_UP(rate, X1600_ADC_CLK_RATE_HZ), 2); + /* US and MS at their reset values: single conversions don't use them. */ + x1600_adc_write(adc, X1600_ADC_REG_CLKDIV, + FIELD_PREP(X1600_ADC_CLKDIV_CLKDIV, div) | + FIELD_PREP(X1600_ADC_CLKDIV_US, 16) | + FIELD_PREP(X1600_ADC_CLKDIV_MS, 4)); + reg = x1600_adc_read(adc, X1600_ADC_REG_STABLE) & ~X1600_ADC_STABLE_TIME; + x1600_adc_write(adc, X1600_ADC_REG_STABLE, + reg | FIELD_PREP(X1600_ADC_STABLE_TIME, 1)); + + irq = platform_get_irq(pdev, 0); + if (irq < 0) + return irq; + + ret = devm_request_irq(dev, irq, x1600_adc_irq, 0, dev_name(dev), adc); + if (ret) + return ret; + + /* + * Left powered up: a typical use, such as buttons read through adc-keys, + * polls every few tens of milliseconds, and powering up takes 2 ms. + */ + x1600_adc_write(adc, X1600_ADC_REG_ENABLE, 0); + fsleep(2 * USEC_PER_MSEC); + x1600_adc_write(adc, X1600_ADC_REG_CTRL, 0); + + ret = devm_add_action_or_reset(dev, x1600_adc_power_down, adc); + if (ret) + return ret; + + iio_dev->name = "x1600-adc"; + iio_dev->modes = INDIO_DIRECT_MODE; + iio_dev->channels = x1600_adc_channels; + iio_dev->num_channels = ARRAY_SIZE(x1600_adc_channels); + iio_dev->info = &x1600_adc_info; + + return devm_iio_device_register(dev, iio_dev); +} + +static const struct of_device_id x1600_adc_of_match[] = { + { .compatible = "ingenic,x1600-adc" }, + { } +}; +MODULE_DEVICE_TABLE(of, x1600_adc_of_match); + +static struct platform_driver x1600_adc_driver = { + .driver = { + .name = "ingenic-x1600-adc", + .of_match_table = x1600_adc_of_match, + }, + .probe = x1600_adc_probe, +}; +module_platform_driver(x1600_adc_driver); +MODULE_AUTHOR("Olivier Louvignes <olivier@mgcrea.io>"); +MODULE_DESCRIPTION("Ingenic X1600 ADC driver"); +MODULE_LICENSE("GPL"); -- 2.54.0 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v4 2/2] iio: adc: Add a driver for the Ingenic X1600 ADC 2026-11-02 20:11 ` [PATCH v4 2/2] iio: adc: Add a driver for " Olivier Louvignes @ 2026-11-03 10:47 ` Andy Shevchenko 2026-11-04 2:04 ` Jonathan Cameron 0 siblings, 1 reply; 6+ messages in thread From: Andy Shevchenko @ 2026-11-03 10:47 UTC (permalink / raw) To: Olivier Louvignes Cc: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Paul Cercueil, H . Nikolaus Schaller, Joshua Crofts, linux-iio, devicetree, linux-mips, linux-kernel On Mon, Nov 02, 2026 at 09:11:55PM +0100, Olivier Louvignes wrote: > The X1600's SAR ADC has four auxiliary channels, each with its own start > bit, interrupt flag and 12-bit result, and none of the touchscreen or > battery engines of the JZ47xx ADCs that ingenic-adc drives. Add a small > driver that exposes the four channels as voltages against the 3.3 V > reference. > > The registers are in chapter 19 of the X1600 programming manual. The > ADC clock divider is even, as the manual has it (an odd value divides > by one less); the vendor kernel writes the divider minus one, which > gives a slightly faster clock than it asks for. > > Tested on a HiBy R1, whose play/pause and volume keys sit on channel 0 > behind adc-keys: all three read and report correctly. A few nit-picks below, otherwise feel free to add Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com> ... > +static int x1600_adc_read_raw_channel(struct x1600_adc *adc, unsigned int ch, int *val) > +{ > + unsigned int bank, shift; > + int ret = 0; > + u32 enable, data; Keep reversed xmas tree order, also split assignment. unsigned int bank, shift; u32 enable, data; int ret; > + guard(mutex)(&adc->lock); > + > + reinit_completion(&adc->done); > + x1600_adc_write(adc, X1600_ADC_REG_STATUS, BIT(ch)); > + enable = x1600_adc_read(adc, X1600_ADC_REG_ENABLE); > + x1600_adc_write(adc, X1600_ADC_REG_ENABLE, enable | BIT(ch)); > + if (!wait_for_completion_timeout(&adc->done, > + msecs_to_jiffies(X1600_ADC_TIMEOUT_MS))) > + ret = -ETIMEDOUT; #define _TIMEOUT msecs_to_jiffies(...plain number...) if (wait_for_completion_timeout(&adc->done, X1600_ADC_TIMEOUT)) ret = 0; else ret = -ETIMEDOUT; > + enable = x1600_adc_read(adc, X1600_ADC_REG_ENABLE); > + x1600_adc_write(adc, X1600_ADC_REG_ENABLE, enable & ~BIT(ch)); > + if (ret) > + return ret; > + > + /* Two results to a data register, the even channel's in the low half. */ > + bank = ch / 2; > + shift = ch % 2; > + data = x1600_adc_read(adc, X1600_ADC_REG_DATA(bank)); > + *val = field_get(X1600_ADC_DATA_MASK(shift), data); + blank line. > + return 0; > +} ... > +static int x1600_adc_probe(struct platform_device *pdev) > +{ > + struct device *dev = &pdev->dev; > + struct iio_dev *iio_dev; > + struct x1600_adc *adc; > + unsigned long rate; > + u32 div, reg; > + int irq, ret; > + > + iio_dev = devm_iio_device_alloc(dev, sizeof(*adc)); > + if (!iio_dev) > + return -ENOMEM; > + > + adc = iio_priv(iio_dev); > + init_completion(&adc->done); > + ret = devm_mutex_init(dev, &adc->lock); > + if (ret) > + return ret; > + > + adc->base = devm_platform_ioremap_resource(pdev, 0); > + if (IS_ERR(adc->base)) > + return PTR_ERR(adc->base); > + > + adc->clk = devm_clk_get_enabled(dev, NULL); Can we have clocks always be named? Otherwise if any new dual-clock HW appear here, the code will be disrupted. > + if (IS_ERR(adc->clk)) > + return dev_err_probe(dev, PTR_ERR(adc->clk), "Unable to get clock\n"); > + > + /* Powered down, interrupts masked and cleared, as the vendor does. */ > + x1600_adc_write(adc, X1600_ADC_REG_ENABLE, X1600_ADC_ENABLE_POWER_DOWN); > + x1600_adc_write(adc, X1600_ADC_REG_CTRL, X1600_ADC_CHANNELS_MASK); > + x1600_adc_write(adc, X1600_ADC_REG_STATUS, X1600_ADC_CHANNELS_MASK); > + rate = clamp(clk_get_rate(adc->clk), 2UL * X1600_ADC_CLK_RATE_HZ, > + 254UL * X1600_ADC_CLK_RATE_HZ); I would put it as rate = clamp(clk_get_rate(adc->clk), 2UL * X1600_ADC_CLK_RATE_HZ, 254UL * X1600_ADC_CLK_RATE_HZ); for the sake of the logical split (and yes, 81 characters in this case is okay). > + div = round_up(DIV_ROUND_UP(rate, X1600_ADC_CLK_RATE_HZ), 2); > + /* US and MS at their reset values: single conversions don't use them. */ > + x1600_adc_write(adc, X1600_ADC_REG_CLKDIV, > + FIELD_PREP(X1600_ADC_CLKDIV_CLKDIV, div) | > + FIELD_PREP(X1600_ADC_CLKDIV_US, 16) | > + FIELD_PREP(X1600_ADC_CLKDIV_MS, 4)); > + reg = x1600_adc_read(adc, X1600_ADC_REG_STABLE) & ~X1600_ADC_STABLE_TIME; > + x1600_adc_write(adc, X1600_ADC_REG_STABLE, > + reg | FIELD_PREP(X1600_ADC_STABLE_TIME, 1)); > + > + irq = platform_get_irq(pdev, 0); > + if (irq < 0) > + return irq; > + > + ret = devm_request_irq(dev, irq, x1600_adc_irq, 0, dev_name(dev), adc); > + if (ret) > + return ret; > + > + /* > + * Left powered up: a typical use, such as buttons read through adc-keys, > + * polls every few tens of milliseconds, and powering up takes 2 ms. > + */ > + x1600_adc_write(adc, X1600_ADC_REG_ENABLE, 0); > + fsleep(2 * USEC_PER_MSEC); > + x1600_adc_write(adc, X1600_ADC_REG_CTRL, 0); > + > + ret = devm_add_action_or_reset(dev, x1600_adc_power_down, adc); > + if (ret) > + return ret; > + > + iio_dev->name = "x1600-adc"; > + iio_dev->modes = INDIO_DIRECT_MODE; > + iio_dev->channels = x1600_adc_channels; > + iio_dev->num_channels = ARRAY_SIZE(x1600_adc_channels); > + iio_dev->info = &x1600_adc_info; > + > + return devm_iio_device_register(dev, iio_dev); > +} -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v4 2/2] iio: adc: Add a driver for the Ingenic X1600 ADC 2026-11-03 10:47 ` Andy Shevchenko @ 2026-11-04 2:04 ` Jonathan Cameron 0 siblings, 0 replies; 6+ messages in thread From: Jonathan Cameron @ 2026-11-04 2:04 UTC (permalink / raw) To: Andy Shevchenko Cc: Olivier Louvignes, David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Paul Cercueil, H . Nikolaus Schaller, Joshua Crofts, linux-iio, devicetree, linux-mips, linux-kernel On Tue, 3 Nov 2026 12:47:14 +0200 Andy Shevchenko <andriy.shevchenko@intel.com> wrote: > On Mon, Nov 02, 2026 at 09:11:55PM +0100, Olivier Louvignes wrote: > > The X1600's SAR ADC has four auxiliary channels, each with its own start > > bit, interrupt flag and 12-bit result, and none of the touchscreen or > > battery engines of the JZ47xx ADCs that ingenic-adc drives. Add a small > > driver that exposes the four channels as voltages against the 3.3 V > > reference. > > > > The registers are in chapter 19 of the X1600 programming manual. The > > ADC clock divider is even, as the manual has it (an odd value divides > > by one less); the vendor kernel writes the divider minus one, which > > gives a slightly faster clock than it asks for. > > > > Tested on a HiBy R1, whose play/pause and volume keys sit on channel 0 > > behind adc-keys: all three read and report correctly. > > A few nit-picks below, otherwise feel free to add > Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com> FWIW I took another quick look and don't have anything to add. So given this is clearly approaching being applied, feel free to send a new version once you've addresses the questions from Conor. No need to wait the normal period under these circumstances. Jonathan ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-11-04 2:04 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2026-11-02 20:11 [PATCH v4 0/2] iio: adc: Add the Ingenic X1600 ADC Olivier Louvignes 2026-11-02 20:11 ` [PATCH v4 1/2] dt-bindings: " Olivier Louvignes 2026-11-02 20:48 ` Conor Dooley 2026-11-02 20:11 ` [PATCH v4 2/2] iio: adc: Add a driver for " Olivier Louvignes 2026-11-03 10:47 ` Andy Shevchenko 2026-11-04 2:04 ` Jonathan Cameron
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®