devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/4] Add support for the Renesas RZ/N1 ADC
@ 2025-10-29 14:46 Herve Codina (Schneider Electric)
  2025-10-29 14:46 ` [PATCH v2 1/4] dt-bindings: iio: adc: Add " Herve Codina (Schneider Electric)
                   ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: Herve Codina (Schneider Electric) @ 2025-10-29 14:46 UTC (permalink / raw)
  To: Wolfram Sang, Herve Codina, Jonathan Cameron, David Lechner,
	Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Geert Uytterhoeven, Magnus Damm, Liam Girdwood,
	Mark Brown
  Cc: linux-iio, linux-renesas-soc, devicetree, linux-kernel,
	Pascal Eberhard, Miquel Raynal, Thomas Petazzoni

Hi,

The Renesas RZ/N1 ADC controller is the ADC controller available in the
Renesas RZ/N1 SoCs family.

It can use up to two internal ADC cores (ADC1 and ADC2) those internal
cores are handled through ADC controller virtual channels.

Best regards,
Herve Codina

Changes v1 -> v2:

v1: https://lore.kernel.org/lkml/20251015142816.1274605-1-herve.codina@bootlin.com/

  Rebase on top of v6.18-rc3 to have commit db82b8dbf5f0 ("PM: runtime:
  Fix conditional guard definitions")

  Patch 1:
    - Remove unneeded 'dependencies' part.
    - Rename "adc-clk" clock to "adc".
    - Move 'additionalProperties: false' just before the example.
    - Use const instead of enum for the "renesas,r9a06g032-adc"
      compatible string.
    - Fix the ACD typo.

  Patch 2:
    - Fix the ACD typo.
    - Rename "adc-clk" clock to "adc".
    - Update included headers and sort them.
    - Align register definitions at the same column.
    - Inline the FIELD_GET() instead of having macros.
    - Introduce RZN1_ADC_NO_CHANNEL
    - Get Vref voltage value at probe().
    - Remove the bitmap in rzn1_adc_set_iio_dev_channels().
    - Use dev_err_probe() in rzn1_adc_set_iio_dev_channels().
    - Use auto-cleanup variant for PM runtime "resume and get".
    - Use scoped_guard() for mutex.
    - Use devm_mutex_init().
    - Use the fixed "rzn1-adc" string for indio_dev->name.
    - Use DEFINE_RUNTIME_DEV_PM_OPS().
    - Fix rzn1_adc_of_match table and remove of_match_ptr().
    - Add a comment related to decoupling between IIO chans and ADC1 or
      ADC2 core chans
    - Update and add several comments related to ADC core usage and the
      relationship with ADC core regulator presence.
    - Remove clocks and regulators handling from PM runtime
      suspend()/remove().
    - Simplify the driver removing the no more relevant struct
      rzn1_adc_core.

  Patch 3:
    - Rename "adc-clk" clock to "adc".
    - Add 'Reviewed-by: Wolfram Sang'

  Patch 4
    - Removed the linux-iio list

Herve Codina (Schneider Electric) (4):
  dt-bindings: iio: adc: Add the Renesas RZ/N1 ADC
  iio: adc: Add support for the Renesas RZ/N1 ADC
  ARM: dts: renesas: r9a06g032: Add the ADC device
  MAINTAINERS: Add the Renesas RZ/N1 ADC driver entry

 .../bindings/iio/adc/renesas,rzn1-adc.yaml    | 111 ++++
 MAINTAINERS                                   |   7 +
 arch/arm/boot/dts/renesas/r9a06g032.dtsi      |  10 +
 drivers/iio/adc/Kconfig                       |  10 +
 drivers/iio/adc/Makefile                      |   1 +
 drivers/iio/adc/rzn1-adc.c                    | 493 ++++++++++++++++++
 6 files changed, 632 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/renesas,rzn1-adc.yaml
 create mode 100644 drivers/iio/adc/rzn1-adc.c

-- 
2.51.0


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v2 1/4] dt-bindings: iio: adc: Add the Renesas RZ/N1 ADC
  2025-10-29 14:46 [PATCH v2 0/4] Add support for the Renesas RZ/N1 ADC Herve Codina (Schneider Electric)
@ 2025-10-29 14:46 ` Herve Codina (Schneider Electric)
  2025-10-30 18:02   ` Rob Herring (Arm)
  2025-10-29 14:46 ` [PATCH v2 2/4] iio: adc: Add support for " Herve Codina (Schneider Electric)
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 11+ messages in thread
From: Herve Codina (Schneider Electric) @ 2025-10-29 14:46 UTC (permalink / raw)
  To: Wolfram Sang, Herve Codina, Jonathan Cameron, David Lechner,
	Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Geert Uytterhoeven, Magnus Damm, Liam Girdwood,
	Mark Brown
  Cc: linux-iio, linux-renesas-soc, devicetree, linux-kernel,
	Pascal Eberhard, Miquel Raynal, Thomas Petazzoni

The Renesas RZ/N1 ADC controller is the ADC controller available in the
Renesas RZ/N1 SoCs family.

Signed-off-by: Herve Codina (Schneider Electric) <herve.codina@bootlin.com>
---
 .../bindings/iio/adc/renesas,rzn1-adc.yaml    | 111 ++++++++++++++++++
 1 file changed, 111 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/renesas,rzn1-adc.yaml

diff --git a/Documentation/devicetree/bindings/iio/adc/renesas,rzn1-adc.yaml b/Documentation/devicetree/bindings/iio/adc/renesas,rzn1-adc.yaml
new file mode 100644
index 000000000000..1a40352165fb
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/renesas,rzn1-adc.yaml
@@ -0,0 +1,111 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/adc/renesas,rzn1-adc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Renesas RZ/N1 Analog to Digital Converter (ADC)
+
+maintainers:
+  - Herve Codina <herve.codina@bootlin.com>
+
+description:
+  The Renesas RZ/N1 ADC controller available in the Renesas RZ/N1 SoCs family
+  can use up to two internal ADC cores (ADC1 and ADC2) those internal cores are
+  handled through ADC controller virtual channels.
+
+properties:
+  compatible:
+    items:
+      - const: renesas,r9a06g032-adc   # RZ/N1D
+      - const: renesas,rzn1-adc
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    items:
+      - description: APB internal bus clock
+      - description: ADC clock
+
+  clock-names:
+    items:
+      - const: pclk
+      - const: adc
+
+  power-domains:
+    maxItems: 1
+
+  adc1-avdd-supply:
+    description:
+      ADC1 analog power supply.
+
+  adc1-vref-supply:
+    description:
+      ADC1 reference voltage supply.
+
+  adc2-avdd-supply:
+    description:
+      ADC2 analog power supply.
+
+  adc2-vref-supply:
+    description:
+      ADC2 reference voltage supply.
+
+  '#io-channel-cells':
+    const: 1
+    description: |
+      Channels numbers available:
+        if ADC1 is used (i.e. adc1-{avdd,vref}-supply present):
+          - 0: ADC1 IN0
+          - 1: ADC1 IN1
+          - 2: ADC1 IN2
+          - 3: ADC1 IN3
+          - 4: ADC1 IN4
+          - 5: ADC1 IN6
+          - 6: ADC1 IN7
+          - 7: ADC1 IN8
+        if ADC2 is used (i.e. adc2-{avdd,vref}-supply present):
+          - 8: ADC2 IN0
+          - 9: ADC2 IN1
+          - 10: ADC2 IN2
+          - 11: ADC2 IN3
+          - 12: ADC2 IN4
+          - 13: ADC2 IN6
+          - 14: ADC2 IN7
+          - 15: ADC2 IN8
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - clock-names
+  - power-domains
+  - '#io-channel-cells'
+
+# At least one of avvd/vref supplies
+anyOf:
+  - required:
+      - adc1-vref-supply
+      - adc1-avdd-supply
+  - required:
+      - adc2-vref-supply
+      - adc2-avdd-supply
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/r9a06g032-sysctrl.h>
+
+    adc: adc@40065000 {
+      compatible = "renesas,r9a06g032-adc", "renesas,rzn1-adc";
+      reg = <0x40065000 0x200>;
+      clocks = <&sysctrl R9A06G032_HCLK_ADC>, <&sysctrl R9A06G032_CLK_ADC>;
+      clock-names = "pclk", "adc";
+      power-domains = <&sysctrl>;
+      adc1-avdd-supply = <&adc1_avdd>;
+      adc1-vref-supply = <&adc1_vref>;
+      #io-channel-cells = <1>;
+    };
+...
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 2/4] iio: adc: Add support for the Renesas RZ/N1 ADC
  2025-10-29 14:46 [PATCH v2 0/4] Add support for the Renesas RZ/N1 ADC Herve Codina (Schneider Electric)
  2025-10-29 14:46 ` [PATCH v2 1/4] dt-bindings: iio: adc: Add " Herve Codina (Schneider Electric)
@ 2025-10-29 14:46 ` Herve Codina (Schneider Electric)
  2025-10-30  9:00   ` Andy Shevchenko
  2025-11-03 11:34   ` Nuno Sá
  2025-10-29 14:46 ` [PATCH v2 3/4] ARM: dts: renesas: r9a06g032: Add the ADC device Herve Codina (Schneider Electric)
  2025-10-29 14:46 ` [PATCH v2 4/4] MAINTAINERS: Add the Renesas RZ/N1 ADC driver entry Herve Codina (Schneider Electric)
  3 siblings, 2 replies; 11+ messages in thread
From: Herve Codina (Schneider Electric) @ 2025-10-29 14:46 UTC (permalink / raw)
  To: Wolfram Sang, Herve Codina, Jonathan Cameron, David Lechner,
	Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Geert Uytterhoeven, Magnus Damm, Liam Girdwood,
	Mark Brown
  Cc: linux-iio, linux-renesas-soc, devicetree, linux-kernel,
	Pascal Eberhard, Miquel Raynal, Thomas Petazzoni

The Renesas RZ/N1 ADC controller is the ADC controller available in the
Renesas RZ/N1 SoCs family. It can use up to two internal ADC cores (ADC1
and ADC2) those internal cores are not directly accessed but are handled
through ADC controller virtual channels.

Signed-off-by: Herve Codina (Schneider Electric) <herve.codina@bootlin.com>
---
 drivers/iio/adc/Kconfig    |  10 +
 drivers/iio/adc/Makefile   |   1 +
 drivers/iio/adc/rzn1-adc.c | 493 +++++++++++++++++++++++++++++++++++++
 3 files changed, 504 insertions(+)
 create mode 100644 drivers/iio/adc/rzn1-adc.c

diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 58a14e6833f6..113f6a5c9745 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -1403,6 +1403,16 @@ config RZG2L_ADC
 	  To compile this driver as a module, choose M here: the
 	  module will be called rzg2l_adc.
 
+config RZN1_ADC
+	tristate "Renesas RZ/N1 ADC driver"
+	depends on ARCH_RZN1 || COMPILE_TEST
+	help
+	  Say yes here to build support for the ADC found in Renesas
+	  RZ/N1 family.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called rzn1-adc.
+
 config SC27XX_ADC
 	tristate "Spreadtrum SC27xx series PMICs ADC"
 	depends on MFD_SC27XX_PMIC || COMPILE_TEST
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index d008f78dc010..ba7a8a63d070 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -123,6 +123,7 @@ obj-$(CONFIG_ROHM_BD79112) += rohm-bd79112.o
 obj-$(CONFIG_ROHM_BD79124) += rohm-bd79124.o
 obj-$(CONFIG_ROCKCHIP_SARADC) += rockchip_saradc.o
 obj-$(CONFIG_RZG2L_ADC) += rzg2l_adc.o
+obj-$(CONFIG_RZN1_ADC) += rzn1-adc.o
 obj-$(CONFIG_SC27XX_ADC) += sc27xx_adc.o
 obj-$(CONFIG_SD_ADC_MODULATOR) += sd_adc_modulator.o
 obj-$(CONFIG_SOPHGO_CV1800B_ADC) += sophgo-cv1800b-adc.o
diff --git a/drivers/iio/adc/rzn1-adc.c b/drivers/iio/adc/rzn1-adc.c
new file mode 100644
index 000000000000..52ec13adddef
--- /dev/null
+++ b/drivers/iio/adc/rzn1-adc.c
@@ -0,0 +1,493 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Renesas RZ/N1 ADC driver
+ *
+ * Copyright (C) 2025 Schneider-Electric
+ *
+ * Author: Herve Codina <herve.codina@bootlin.com>
+ *
+ * The RZ/N1 ADC controller can handle channels from its internal ADC1 and/or
+ * ADC2 cores. The driver use ADC1 and/or ADC2 cores depending on the presence
+ * of the related power supplies (AVDD and VREF) description in the device-tree.
+ */
+
+#include <linux/bitfield.h>
+#include <linux/bits.h>
+#include <linux/cleanup.h>
+#include <linux/clk.h>
+#include <linux/iio/iio.h>
+#include <linux/io.h>
+#include <linux/iopoll.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/regulator/consumer.h>
+
+#define RZN1_ADC_CONTROL_REG			0x2c
+#define RZN1_ADC_CONTROL_ADC_BUSY		BIT(6)
+
+#define RZN1_ADC_FORCE_REG			0x30
+#define RZN1_ADC_SET_FORCE_REG			0x34
+#define RZN1_ADC_CLEAR_FORCE_REG		0x38
+#define RZN1_ADC_FORCE_VC(_n)			BIT(_n)
+
+#define RZN1_ADC_CONFIG_REG			0x40
+#define RZN1_ADC_CONFIG_ADC_POWER_DOWN		BIT(3)
+
+#define RZN1_ADC_VC_REG(_n)			(0xc0 + 4 * (_n))
+#define RZN1_ADC_VC_ADC2_ENABLE			BIT(16)
+#define RZN1_ADC_VC_ADC1_ENABLE			BIT(15)
+#define RZN1_ADC_VC_ADC2_CHANNEL_SEL_MASK	GENMASK(5, 3)
+#define RZN1_ADC_VC_ADC1_CHANNEL_SEL_MASK	GENMASK(2, 0)
+
+#define RZN1_ADC_ADC1_DATA_REG(_n)		(0x100 + 4 * (_n))
+#define RZN1_ADC_ADC2_DATA_REG(_n)		(0x140 + 4 * (_n))
+#define RZN1_ADC_ADCX_DATA_DATA_MASK		GENMASK(11, 0)
+
+#define RZN1_ADC_NO_CHANNEL	-1
+
+#define RZN1_ADC_CHANNEL_SHARED_SCALE(_ch, _ds_name) {		\
+	.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),   \
+	.datasheet_name = (_ds_name),				\
+}
+
+#define RZN1_ADC_CHANNEL_SEPARATED_SCALE(_ch, _ds_name) {	\
+	.type = IIO_VOLTAGE,					\
+	.indexed = 1,						\
+	.channel = (_ch),					\
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |		\
+			      BIT(IIO_CHAN_INFO_SCALE),		\
+	.datasheet_name = (_ds_name),				\
+}
+
+/*
+ * 8 ADC1_IN signals existed numbered 0..4, 6..8
+ * ADCx_IN5 doesn't exist in RZ/N1 datasheet
+ */
+static struct iio_chan_spec rzn1_adc1_channels[] = {
+	RZN1_ADC_CHANNEL_SHARED_SCALE(0, "ADC1_IN0"),
+	RZN1_ADC_CHANNEL_SHARED_SCALE(1, "ADC1_IN1"),
+	RZN1_ADC_CHANNEL_SHARED_SCALE(2, "ADC1_IN2"),
+	RZN1_ADC_CHANNEL_SHARED_SCALE(3, "ADC1_IN3"),
+	RZN1_ADC_CHANNEL_SHARED_SCALE(4, "ADC1_IN4"),
+	RZN1_ADC_CHANNEL_SHARED_SCALE(5, "ADC1_IN6"),
+	RZN1_ADC_CHANNEL_SHARED_SCALE(6, "ADC1_IN7"),
+	RZN1_ADC_CHANNEL_SHARED_SCALE(7, "ADC1_IN8"),
+};
+
+static struct iio_chan_spec rzn1_adc2_channels[] = {
+	RZN1_ADC_CHANNEL_SHARED_SCALE(8,  "ADC2_IN0"),
+	RZN1_ADC_CHANNEL_SHARED_SCALE(9,  "ADC2_IN1"),
+	RZN1_ADC_CHANNEL_SHARED_SCALE(10, "ADC2_IN2"),
+	RZN1_ADC_CHANNEL_SHARED_SCALE(11, "ADC2_IN3"),
+	RZN1_ADC_CHANNEL_SHARED_SCALE(12, "ADC2_IN4"),
+	RZN1_ADC_CHANNEL_SHARED_SCALE(13, "ADC2_IN6"),
+	RZN1_ADC_CHANNEL_SHARED_SCALE(14, "ADC2_IN7"),
+	RZN1_ADC_CHANNEL_SHARED_SCALE(15, "ADC2_IN8"),
+};
+
+/*
+ * If both ADCs core are used, scale cannot be common. Indeed, scale is
+ * based on Vref connected on each ADC core.
+ */
+static struct iio_chan_spec rzn1_adc1_adc2_channels[] = {
+	RZN1_ADC_CHANNEL_SEPARATED_SCALE(0, "ADC1_IN0"),
+	RZN1_ADC_CHANNEL_SEPARATED_SCALE(1, "ADC1_IN1"),
+	RZN1_ADC_CHANNEL_SEPARATED_SCALE(2, "ADC1_IN2"),
+	RZN1_ADC_CHANNEL_SEPARATED_SCALE(3, "ADC1_IN3"),
+	RZN1_ADC_CHANNEL_SEPARATED_SCALE(4, "ADC1_IN4"),
+	RZN1_ADC_CHANNEL_SEPARATED_SCALE(5, "ADC1_IN6"),
+	RZN1_ADC_CHANNEL_SEPARATED_SCALE(6, "ADC1_IN7"),
+	RZN1_ADC_CHANNEL_SEPARATED_SCALE(7, "ADC1_IN8"),
+	RZN1_ADC_CHANNEL_SEPARATED_SCALE(8,  "ADC2_IN0"),
+	RZN1_ADC_CHANNEL_SEPARATED_SCALE(9,  "ADC2_IN1"),
+	RZN1_ADC_CHANNEL_SEPARATED_SCALE(10, "ADC2_IN2"),
+	RZN1_ADC_CHANNEL_SEPARATED_SCALE(11, "ADC2_IN3"),
+	RZN1_ADC_CHANNEL_SEPARATED_SCALE(12, "ADC2_IN4"),
+	RZN1_ADC_CHANNEL_SEPARATED_SCALE(13, "ADC2_IN6"),
+	RZN1_ADC_CHANNEL_SEPARATED_SCALE(14, "ADC2_IN7"),
+	RZN1_ADC_CHANNEL_SEPARATED_SCALE(15, "ADC2_IN8"),
+};
+
+struct rzn1_adc {
+	struct device *dev;
+	void __iomem *regs;
+	struct mutex lock; /* ADC lock */
+	int adc1_vref_mv; /* ADC1 Vref in mV. Negative if ADC1 is not used */
+	int adc2_vref_mv; /* ADC2 Vref in mV. Negative if ADC2 is not used */
+};
+
+static int rzn1_adc_power(struct rzn1_adc *rzn1_adc, bool power)
+{
+	u32 v;
+
+	writel(power ? 0 : RZN1_ADC_CONFIG_ADC_POWER_DOWN,
+	       rzn1_adc->regs + RZN1_ADC_CONFIG_REG);
+
+	/* Wait for the ADC_BUSY to clear */
+	return readl_poll_timeout_atomic(rzn1_adc->regs + RZN1_ADC_CONTROL_REG,
+					 v, !(v & RZN1_ADC_CONTROL_ADC_BUSY),
+					 0, 500);
+}
+
+static void rzn1_adc_vc_setup_conversion(struct rzn1_adc *rzn1_adc, u32 ch,
+					 int adc1_ch, int adc2_ch)
+{
+	u32 vc = 0;
+
+	if (adc1_ch != RZN1_ADC_NO_CHANNEL)
+		vc |= RZN1_ADC_VC_ADC1_ENABLE |
+		      FIELD_PREP(RZN1_ADC_VC_ADC1_CHANNEL_SEL_MASK, adc1_ch);
+
+	if (adc2_ch != RZN1_ADC_NO_CHANNEL)
+		vc |= RZN1_ADC_VC_ADC2_ENABLE |
+		      FIELD_PREP(RZN1_ADC_VC_ADC2_CHANNEL_SEL_MASK, adc2_ch);
+
+	writel(vc, rzn1_adc->regs + RZN1_ADC_VC_REG(ch));
+}
+
+static int rzn1_adc_vc_start_conversion(struct rzn1_adc *rzn1_adc, u32 ch)
+{
+	u32 val;
+
+	val = readl(rzn1_adc->regs + RZN1_ADC_FORCE_REG);
+	if (val & RZN1_ADC_FORCE_VC(ch))
+		return -EBUSY;
+
+	writel(RZN1_ADC_FORCE_VC(ch), rzn1_adc->regs + RZN1_ADC_SET_FORCE_REG);
+
+	return 0;
+}
+
+static void rzn1_adc_vc_stop_conversion(struct rzn1_adc *rzn1_adc, u32 ch)
+{
+	writel(RZN1_ADC_FORCE_VC(ch), rzn1_adc->regs + RZN1_ADC_CLEAR_FORCE_REG);
+}
+
+static int rzn1_adc_vc_wait_conversion(struct rzn1_adc *rzn1_adc, u32 ch,
+				       u32 *adc1_data, u32 *adc2_data)
+{
+	u32 data_reg;
+	int ret;
+	u32 v;
+
+	/*
+	 * When a VC is selected, it needs 20 ADC clocks to perform the
+	 * conversion.
+	 *
+	 * The worst case is when the 16 VCs need to perform a conversion and
+	 * our VC is the lowest in term of priority.
+	 *
+	 * In that case, the conversion is performed in 16 * 20 ADC clocks.
+	 *
+	 * The ADC clock can be set from 4MHz to 20MHz. This leads to a worst
+	 * case of  16 * 20 * 1/4Mhz = 80us.
+	 *
+	 * Round it up to 100us.
+	 */
+
+	/* Wait for the ADC_FORCE_VC(n) to clear */
+	ret = readl_poll_timeout_atomic(rzn1_adc->regs + RZN1_ADC_FORCE_REG,
+					v, !(v & RZN1_ADC_FORCE_VC(ch)),
+					0, 100);
+	if (ret)
+		return ret;
+
+	if (adc1_data) {
+		data_reg = readl(rzn1_adc->regs + RZN1_ADC_ADC1_DATA_REG(ch));
+		*adc1_data = FIELD_GET(RZN1_ADC_ADCX_DATA_DATA_MASK, data_reg);
+	}
+
+	if (adc2_data) {
+		data_reg = readl(rzn1_adc->regs + RZN1_ADC_ADC2_DATA_REG(ch));
+		*adc2_data = FIELD_GET(RZN1_ADC_ADCX_DATA_DATA_MASK, data_reg);
+	}
+
+	return 0;
+}
+
+static int rzn1_adc_read_raw_ch(struct rzn1_adc *rzn1_adc, unsigned int chan, int *val)
+{
+	u32 *adc1_data, *adc2_data;
+	int adc1_ch, adc2_ch;
+	u32 adc_data;
+	int ret;
+
+	/*
+	 * IIO chan are decoupled from chans used in rzn1_adc_vc_*() functions.
+	 * The RZ/N1 ADC VC controller can handle on a single VC chan one
+	 * channel from the ADC1 core and one channel from the ADC2 core.
+	 *
+	 * Even if IIO chans are mapped 1:1 to ADC core chans and so uses only
+	 * a chan from ADC1 or a chan from ADC2, future improvements can define
+	 * an IIO chan that uses one chan from ADC1 and one chan from ADC2.
+	 */
+
+	if (chan < 8) {
+		/* chan 0..7 used to get ADC1 ch 0..7 */
+		adc1_ch = chan;
+		adc1_data = &adc_data;
+		adc2_ch = RZN1_ADC_NO_CHANNEL;
+		adc2_data = NULL;
+	} else if (chan < 16) {
+		/* chan 8..15 used to get ADC2 ch 0..7 */
+		adc1_ch = RZN1_ADC_NO_CHANNEL;
+		adc1_data = NULL;
+		adc2_ch = chan - 8;
+		adc2_data = &adc_data;
+	} else {
+		return -EINVAL;
+	}
+
+	ACQUIRE(pm_runtime_active_auto_try_enabled, pm)(rzn1_adc->dev);
+	ret = ACQUIRE_ERR(pm_runtime_active_auto_try_enabled, &pm);
+	if (ret < 0)
+		return ret;
+
+	scoped_guard(mutex, &rzn1_adc->lock) {
+		rzn1_adc_vc_setup_conversion(rzn1_adc, chan, adc1_ch, adc2_ch);
+
+		ret = rzn1_adc_vc_start_conversion(rzn1_adc, chan);
+		if (ret)
+			return ret;
+
+		ret = rzn1_adc_vc_wait_conversion(rzn1_adc, chan, adc1_data, adc2_data);
+		if (ret) {
+			rzn1_adc_vc_stop_conversion(rzn1_adc, chan);
+			return ret;
+		}
+	}
+
+	*val = adc_data;
+	ret = IIO_VAL_INT;
+
+	return 0;
+}
+
+static int rzn1_adc_get_vref_mv(struct rzn1_adc *rzn1_adc, unsigned int chan)
+{
+	/*
+	 * chan 0..7 use ADC1 ch 0..7. Vref related to ADC1 core
+	 * chan 8..15 use ADC2 ch 0..7. Vref related to ADC2 core
+	 */
+	if (chan < 8)
+		return rzn1_adc->adc1_vref_mv;
+	else if (chan < 16)
+		return rzn1_adc->adc2_vref_mv;
+
+	return -EINVAL;
+}
+
+static int rzn1_adc_read_raw(struct iio_dev *indio_dev, struct iio_chan_spec const *chan,
+			     int *val, int *val2, long mask)
+{
+	struct rzn1_adc *rzn1_adc = iio_priv(indio_dev);
+	int ret;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		ret = rzn1_adc_read_raw_ch(rzn1_adc, chan->channel, val);
+		if (ret)
+			return ret;
+		return IIO_VAL_INT;
+
+	case IIO_CHAN_INFO_SCALE:
+		ret = rzn1_adc_get_vref_mv(rzn1_adc, chan->channel);
+		if (ret < 0)
+			return ret;
+		*val = ret;
+		*val2 = 12;
+		return IIO_VAL_FRACTIONAL_LOG2;
+
+	default:
+		break;
+	}
+
+	return -EINVAL;
+}
+
+static const struct iio_info rzn1_adc_info = {
+	.read_raw = &rzn1_adc_read_raw
+};
+
+static int rzn1_adc_set_iio_dev_channels(struct rzn1_adc *rzn1_adc,
+					 struct iio_dev *indio_dev)
+{
+	/*
+	 * When an ADC core is not used, its related vref_mv is set to a
+	 * negative error code. Use the correct IIO channels table based on
+	 * those vref_mv values.
+	 */
+	if (rzn1_adc->adc1_vref_mv >= 0) {
+		if (rzn1_adc->adc2_vref_mv >= 0) {
+			indio_dev->channels = rzn1_adc1_adc2_channels;
+			indio_dev->num_channels = ARRAY_SIZE(rzn1_adc1_adc2_channels);
+		} else {
+			indio_dev->channels = rzn1_adc1_channels;
+			indio_dev->num_channels = ARRAY_SIZE(rzn1_adc1_channels);
+		}
+		return 0;
+	}
+
+	if (rzn1_adc->adc2_vref_mv >= 0) {
+		indio_dev->channels = rzn1_adc2_channels;
+		indio_dev->num_channels = ARRAY_SIZE(rzn1_adc2_channels);
+		return 0;
+	}
+
+	return dev_err_probe(rzn1_adc->dev, -ENODEV,
+			     "Failed to set IIO channels, no ADC core used\n");
+}
+
+static int rzn1_adc_core_get_regulators(struct rzn1_adc *rzn1_adc,
+					int *adc_vref_mv,
+					const char *avdd_name, const char *vref_name)
+{
+	struct device *dev = rzn1_adc->dev;
+	int ret;
+
+	/*
+	 * For a given ADC core (ADC1 or ADC2), both regulators (AVDD and VREF)
+	 * must be available in order to have the ADC core used.
+	 *
+	 * We use the regulators presence to check the usage of the related
+	 * ADC core. If both regulators are available, the ADC core is used.
+	 * Otherwise, the ADC core is not used.
+	 *
+	 * The adc_vref_mv value is set to a negative error code (-ENODEV) when
+	 * the ADC core is not used. Otherwise it is set to the VRef mV value.
+	 */
+
+	*adc_vref_mv = -ENODEV;
+
+	ret = devm_regulator_get_enable_optional(dev, avdd_name);
+	if (ret < 0) {
+		if (ret != -ENODEV)
+			return dev_err_probe(dev, ret,
+					     "Failed to get '%s' regulator\n",
+					     avdd_name);
+		return 0;
+	}
+
+	ret = devm_regulator_get_enable_read_voltage(dev, vref_name);
+	if (ret < 0) {
+		if (ret != -ENODEV)
+			return dev_err_probe(dev, ret,
+					     "Failed to get '%s' regulator\n",
+					     vref_name);
+		return 0;
+	}
+
+	/*
+	 * Both regulators are available.
+	 * Set adc_vref_mv to the Vref value in mV. This, as the value set is
+	 * positive, also signals that the ADC is used.
+	 */
+	*adc_vref_mv = ret / 1000;
+
+	return 0;
+}
+
+static int rzn1_adc_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct iio_dev *indio_dev;
+	struct rzn1_adc *rzn1_adc;
+	struct clk *clk;
+	int ret;
+
+	indio_dev = devm_iio_device_alloc(dev, sizeof(*rzn1_adc));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	rzn1_adc = iio_priv(indio_dev);
+	rzn1_adc->dev = dev;
+
+	ret = devm_mutex_init(dev, &rzn1_adc->lock);
+	if (ret)
+		return ret;
+
+	rzn1_adc->regs = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(rzn1_adc->regs))
+		return PTR_ERR(rzn1_adc->regs);
+
+	clk = devm_clk_get_enabled(dev, "pclk");
+	if (IS_ERR(clk))
+		return dev_err_probe(dev, PTR_ERR(clk), "Failed to get pclk\n");
+
+	clk = devm_clk_get_enabled(dev, "adc");
+	if (IS_ERR(clk))
+		return dev_err_probe(dev, PTR_ERR(clk), "Failed to get adc clk\n");
+
+	ret = rzn1_adc_core_get_regulators(rzn1_adc, &rzn1_adc->adc1_vref_mv,
+					   "adc1-avdd", "adc1-vref");
+	if (ret)
+		return ret;
+
+	ret = rzn1_adc_core_get_regulators(rzn1_adc, &rzn1_adc->adc2_vref_mv,
+					   "adc2-avdd", "adc2-vref");
+	if (ret)
+		return ret;
+
+	platform_set_drvdata(pdev, indio_dev);
+
+	indio_dev->name = "rzn1-adc";
+	indio_dev->info = &rzn1_adc_info;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	ret = rzn1_adc_set_iio_dev_channels(rzn1_adc, indio_dev);
+	if (ret)
+		return ret;
+
+	pm_runtime_set_autosuspend_delay(dev, 500);
+	pm_runtime_use_autosuspend(dev);
+	ret = devm_pm_runtime_enable(dev);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to enable runtime PM\n");
+
+	return devm_iio_device_register(dev, indio_dev);
+}
+
+static int rzn1_adc_pm_runtime_suspend(struct device *dev)
+{
+	struct iio_dev *indio_dev = dev_get_drvdata(dev);
+	struct rzn1_adc *rzn1_adc = iio_priv(indio_dev);
+
+	return rzn1_adc_power(rzn1_adc, false);
+}
+
+static int rzn1_adc_pm_runtime_resume(struct device *dev)
+{
+	struct iio_dev *indio_dev = dev_get_drvdata(dev);
+	struct rzn1_adc *rzn1_adc = iio_priv(indio_dev);
+
+	return rzn1_adc_power(rzn1_adc, true);
+}
+
+static DEFINE_RUNTIME_DEV_PM_OPS(rzn1_adc_pm_ops,
+				 rzn1_adc_pm_runtime_suspend,
+				 rzn1_adc_pm_runtime_resume, NULL);
+
+static const struct of_device_id rzn1_adc_of_match[] = {
+	{ .compatible = "renesas,rzn1-adc" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, rzn1_adc_of_match);
+
+static struct platform_driver rzn1_adc_driver = {
+	.probe = rzn1_adc_probe,
+	.driver = {
+		.name = "rzn1-adc",
+		.of_match_table = rzn1_adc_of_match,
+		.pm = pm_ptr(&rzn1_adc_pm_ops),
+	},
+};
+module_platform_driver(rzn1_adc_driver);
+
+MODULE_AUTHOR("Herve Codina <herve.codina@bootlin.com>");
+MODULE_DESCRIPTION("Renesas RZ/N1 ADC Driver");
+MODULE_LICENSE("GPL");
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 3/4] ARM: dts: renesas: r9a06g032: Add the ADC device
  2025-10-29 14:46 [PATCH v2 0/4] Add support for the Renesas RZ/N1 ADC Herve Codina (Schneider Electric)
  2025-10-29 14:46 ` [PATCH v2 1/4] dt-bindings: iio: adc: Add " Herve Codina (Schneider Electric)
  2025-10-29 14:46 ` [PATCH v2 2/4] iio: adc: Add support for " Herve Codina (Schneider Electric)
@ 2025-10-29 14:46 ` Herve Codina (Schneider Electric)
  2025-10-29 14:46 ` [PATCH v2 4/4] MAINTAINERS: Add the Renesas RZ/N1 ADC driver entry Herve Codina (Schneider Electric)
  3 siblings, 0 replies; 11+ messages in thread
From: Herve Codina (Schneider Electric) @ 2025-10-29 14:46 UTC (permalink / raw)
  To: Wolfram Sang, Herve Codina, Jonathan Cameron, David Lechner,
	Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Geert Uytterhoeven, Magnus Damm, Liam Girdwood,
	Mark Brown
  Cc: linux-iio, linux-renesas-soc, devicetree, linux-kernel,
	Pascal Eberhard, Miquel Raynal, Thomas Petazzoni

The ADC available in the r9a06g032 SoC can use up to two internal ADC
cores (ADC1 and ADC2) those internal cores are handled through ADC
controller virtual channels.

Describe this device.

Signed-off-by: Herve Codina (Schneider Electric) <herve.codina@bootlin.com>
Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 arch/arm/boot/dts/renesas/r9a06g032.dtsi | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/arm/boot/dts/renesas/r9a06g032.dtsi b/arch/arm/boot/dts/renesas/r9a06g032.dtsi
index 13a60656b044..2c1577923223 100644
--- a/arch/arm/boot/dts/renesas/r9a06g032.dtsi
+++ b/arch/arm/boot/dts/renesas/r9a06g032.dtsi
@@ -290,6 +290,16 @@ i2c2: i2c@40064000 {
 			status = "disabled";
 		};
 
+		adc: adc@40065000 {
+			compatible = "renesas,r9a06g032-adc", "renesas,rzn1-adc";
+			reg = <0x40065000 0x200>;
+			clocks = <&sysctrl R9A06G032_HCLK_ADC>, <&sysctrl R9A06G032_CLK_ADC>;
+			clock-names = "pclk", "adc";
+			power-domains = <&sysctrl>;
+			#io-channel-cells = <1>;
+			status = "disabled";
+		};
+
 		pinctrl: pinctrl@40067000 {
 			compatible = "renesas,r9a06g032-pinctrl", "renesas,rzn1-pinctrl";
 			reg = <0x40067000 0x1000>, <0x51000000 0x480>;
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 4/4] MAINTAINERS: Add the Renesas RZ/N1 ADC driver entry
  2025-10-29 14:46 [PATCH v2 0/4] Add support for the Renesas RZ/N1 ADC Herve Codina (Schneider Electric)
                   ` (2 preceding siblings ...)
  2025-10-29 14:46 ` [PATCH v2 3/4] ARM: dts: renesas: r9a06g032: Add the ADC device Herve Codina (Schneider Electric)
@ 2025-10-29 14:46 ` Herve Codina (Schneider Electric)
  3 siblings, 0 replies; 11+ messages in thread
From: Herve Codina (Schneider Electric) @ 2025-10-29 14:46 UTC (permalink / raw)
  To: Wolfram Sang, Herve Codina, Jonathan Cameron, David Lechner,
	Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Geert Uytterhoeven, Magnus Damm, Liam Girdwood,
	Mark Brown
  Cc: linux-iio, linux-renesas-soc, devicetree, linux-kernel,
	Pascal Eberhard, Miquel Raynal, Thomas Petazzoni

After contributing the driver, add myself as the maintainer for the
Renesas RZ/N1 ADC driver.

Signed-off-by: Herve Codina (Schneider Electric) <herve.codina@bootlin.com>
---
 MAINTAINERS | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 3da2c26a796b..a67babe1a5b4 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -21900,6 +21900,13 @@ F:	include/dt-bindings/net/pcs-rzn1-miic.h
 F:	include/linux/pcs-rzn1-miic.h
 F:	net/dsa/tag_rzn1_a5psw.c
 
+RENESAS RZ/N1 ADC DRIVER
+M:	Herve Codina <herve.codina@bootlin.com>
+L:	linux-renesas-soc@vger.kernel.org
+S:	Supported
+F:	Documentation/devicetree/bindings/iio/adc/renesas,rzn1-adc.yaml
+F:	drivers/iio/adc/rzn1-adc.c
+
 RENESAS RZ/N1 DWMAC GLUE LAYER
 M:	Romain Gantois <romain.gantois@bootlin.com>
 S:	Maintained
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 2/4] iio: adc: Add support for the Renesas RZ/N1 ADC
  2025-10-29 14:46 ` [PATCH v2 2/4] iio: adc: Add support for " Herve Codina (Schneider Electric)
@ 2025-10-30  9:00   ` Andy Shevchenko
  2025-11-02 11:42     ` Jonathan Cameron
  2025-11-03  8:40     ` Herve Codina
  2025-11-03 11:34   ` Nuno Sá
  1 sibling, 2 replies; 11+ messages in thread
From: Andy Shevchenko @ 2025-10-30  9:00 UTC (permalink / raw)
  To: Herve Codina (Schneider Electric)
  Cc: Wolfram Sang, Jonathan Cameron, David Lechner, Nuno Sá,
	Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Geert Uytterhoeven, Magnus Damm, Liam Girdwood, Mark Brown,
	linux-iio, linux-renesas-soc, devicetree, linux-kernel,
	Pascal Eberhard, Miquel Raynal, Thomas Petazzoni

On Wed, Oct 29, 2025 at 03:46:42PM +0100, Herve Codina (Schneider Electric) wrote:
> The Renesas RZ/N1 ADC controller is the ADC controller available in the
> Renesas RZ/N1 SoCs family. It can use up to two internal ADC cores (ADC1
> and ADC2) those internal cores are not directly accessed but are handled
> through ADC controller virtual channels.

Looks much better, thanks! My comments below.

...

+ array_size,h

> +#include <linux/bitfield.h>
> +#include <linux/bits.h>
> +#include <linux/cleanup.h>
> +#include <linux/clk.h>

+ dev_printk.h
+ err.h

> +#include <linux/iio/iio.h>
> +#include <linux/io.h>
> +#include <linux/iopoll.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/module.h>

+ mutex.h

> +#include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/regulator/consumer.h>

+ types.h

...

> +#define RZN1_ADC_CONTROL_REG			0x2c

I would go with fixed-width values, e.g., 0x02c for the register definitions.

> +#define RZN1_ADC_CONTROL_ADC_BUSY		BIT(6)
> +
> +#define RZN1_ADC_FORCE_REG			0x30
> +#define RZN1_ADC_SET_FORCE_REG			0x34
> +#define RZN1_ADC_CLEAR_FORCE_REG		0x38

> +#define RZN1_ADC_CONFIG_REG			0x40

> +
> +#define RZN1_ADC_VC_REG(_n)			(0xc0 + 4 * (_n))

> +#define RZN1_ADC_ADC1_DATA_REG(_n)		(0x100 + 4 * (_n))
> +#define RZN1_ADC_ADC2_DATA_REG(_n)		(0x140 + 4 * (_n))

...

> +static int rzn1_adc_get_vref_mv(struct rzn1_adc *rzn1_adc, unsigned int chan)
> +{
> +	/*
> +	 * chan 0..7 use ADC1 ch 0..7. Vref related to ADC1 core
> +	 * chan 8..15 use ADC2 ch 0..7. Vref related to ADC2 core
> +	 */

Split it to two one line comments per each conditional.

> +	if (chan < 8)
> +		return rzn1_adc->adc1_vref_mv;
> +	else if (chan < 16)

Redundant 'else'.

> +		return rzn1_adc->adc2_vref_mv;
> +
> +	return -EINVAL;
> +}

...

> +static int rzn1_adc_read_raw(struct iio_dev *indio_dev, struct iio_chan_spec const *chan,
> +			     int *val, int *val2, long mask)
> +{
> +	struct rzn1_adc *rzn1_adc = iio_priv(indio_dev);
> +	int ret;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		ret = rzn1_adc_read_raw_ch(rzn1_adc, chan->channel, val);
> +		if (ret)
> +			return ret;
> +		return IIO_VAL_INT;
> +
> +	case IIO_CHAN_INFO_SCALE:
> +		ret = rzn1_adc_get_vref_mv(rzn1_adc, chan->channel);
> +		if (ret < 0)
> +			return ret;
> +		*val = ret;
> +		*val2 = 12;
> +		return IIO_VAL_FRACTIONAL_LOG2;

> +	default:
> +		break;
> +	}
> +
> +	return -EINVAL;

	default:
		return -EINVAL;

> +}

...

> +static const struct iio_info rzn1_adc_info = {
> +	.read_raw = &rzn1_adc_read_raw

Leave trailing comma, this is not a terminator entry.

> +};

...

> +	if (rzn1_adc->adc1_vref_mv >= 0) {

Can we call it _mV?

...

> +	if (rzn1_adc->adc2_vref_mv >= 0) {

Ditto.


...

> +	ret = devm_regulator_get_enable_optional(dev, avdd_name);
> +	if (ret < 0) {
> +		if (ret != -ENODEV)
> +			return dev_err_probe(dev, ret,
> +					     "Failed to get '%s' regulator\n",
> +					     avdd_name);
> +		return 0;
> +	}

	if (ret == -ENODEV)
		return dev_err_probe(); // takes less LoCs
	if (ret < 0) // do we need ' < 0' part?
		return 0;

> +	ret = devm_regulator_get_enable_read_voltage(dev, vref_name);
> +	if (ret < 0) {
> +		if (ret != -ENODEV)
> +			return dev_err_probe(dev, ret,
> +					     "Failed to get '%s' regulator\n",
> +					     vref_name);
> +		return 0;
> +	}

In the same way as above.

...

> +static DEFINE_RUNTIME_DEV_PM_OPS(rzn1_adc_pm_ops,
> +				 rzn1_adc_pm_runtime_suspend,
> +				 rzn1_adc_pm_runtime_resume, NULL);

Please, split it based on logic:

static DEFINE_RUNTIME_DEV_PM_OPS(rzn1_adc_pm_ops,
				 rzn1_adc_pm_runtime_suspend, rzn1_adc_pm_runtime_resume, NULL);

OR

static DEFINE_RUNTIME_DEV_PM_OPS(rzn1_adc_pm_ops,
				 rzn1_adc_pm_runtime_suspend,
				 rzn1_adc_pm_runtime_resume,
				 NULL);

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 1/4] dt-bindings: iio: adc: Add the Renesas RZ/N1 ADC
  2025-10-29 14:46 ` [PATCH v2 1/4] dt-bindings: iio: adc: Add " Herve Codina (Schneider Electric)
@ 2025-10-30 18:02   ` Rob Herring (Arm)
  0 siblings, 0 replies; 11+ messages in thread
From: Rob Herring (Arm) @ 2025-10-30 18:02 UTC (permalink / raw)
  To: Herve Codina (Schneider Electric)
  Cc: Jonathan Cameron, Mark Brown, devicetree, Andy Shevchenko,
	David Lechner, Pascal Eberhard, Miquel Raynal, linux-kernel,
	Wolfram Sang, Conor Dooley, Geert Uytterhoeven,
	Krzysztof Kozlowski, Thomas Petazzoni, linux-renesas-soc,
	Liam Girdwood, Nuno Sá, linux-iio, Magnus Damm


On Wed, 29 Oct 2025 15:46:41 +0100, Herve Codina (Schneider Electric) wrote:
> The Renesas RZ/N1 ADC controller is the ADC controller available in the
> Renesas RZ/N1 SoCs family.
> 
> Signed-off-by: Herve Codina (Schneider Electric) <herve.codina@bootlin.com>
> ---
>  .../bindings/iio/adc/renesas,rzn1-adc.yaml    | 111 ++++++++++++++++++
>  1 file changed, 111 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/adc/renesas,rzn1-adc.yaml
> 

Reviewed-by: Rob Herring (Arm) <robh@kernel.org>


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 2/4] iio: adc: Add support for the Renesas RZ/N1 ADC
  2025-10-30  9:00   ` Andy Shevchenko
@ 2025-11-02 11:42     ` Jonathan Cameron
  2025-11-03  8:40     ` Herve Codina
  1 sibling, 0 replies; 11+ messages in thread
From: Jonathan Cameron @ 2025-11-02 11:42 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Herve Codina (Schneider Electric), Wolfram Sang, David Lechner,
	Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Geert Uytterhoeven, Magnus Damm, Liam Girdwood,
	Mark Brown, linux-iio, linux-renesas-soc, devicetree,
	linux-kernel, Pascal Eberhard, Miquel Raynal, Thomas Petazzoni

On Thu, 30 Oct 2025 11:00:12 +0200
Andy Shevchenko <andriy.shevchenko@intel.com> wrote:

> On Wed, Oct 29, 2025 at 03:46:42PM +0100, Herve Codina (Schneider Electric) wrote:
> > The Renesas RZ/N1 ADC controller is the ADC controller available in the
> > Renesas RZ/N1 SoCs family. It can use up to two internal ADC cores (ADC1
> > and ADC2) those internal cores are not directly accessed but are handled
> > through ADC controller virtual channels.  
> 
> Looks much better, thanks! My comments below.
FWIW I took a look as well but found the only things I noticed were a small
subset of Andy's much more thorough review ;)

Jonathan

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 2/4] iio: adc: Add support for the Renesas RZ/N1 ADC
  2025-10-30  9:00   ` Andy Shevchenko
  2025-11-02 11:42     ` Jonathan Cameron
@ 2025-11-03  8:40     ` Herve Codina
  2025-11-03  9:39       ` Andy Shevchenko
  1 sibling, 1 reply; 11+ messages in thread
From: Herve Codina @ 2025-11-03  8:40 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Wolfram Sang, Jonathan Cameron, David Lechner, Nuno Sá,
	Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Geert Uytterhoeven, Magnus Damm, Liam Girdwood, Mark Brown,
	linux-iio, linux-renesas-soc, devicetree, linux-kernel,
	Pascal Eberhard, Miquel Raynal, Thomas Petazzoni

Hi Andy,

On Thu, 30 Oct 2025 11:00:12 +0200
Andy Shevchenko <andriy.shevchenko@intel.com> wrote:

...
> 
> > +	ret = devm_regulator_get_enable_optional(dev, avdd_name);
> > +	if (ret < 0) {
> > +		if (ret != -ENODEV)
> > +			return dev_err_probe(dev, ret,
> > +					     "Failed to get '%s' regulator\n",
> > +					     avdd_name);
> > +		return 0;
> > +	}  
> 
> 	if (ret == -ENODEV)
> 		return dev_err_probe(); // takes less LoCs
> 	if (ret < 0) // do we need ' < 0' part?
> 		return 0;
> 

Well, I need to abort on error returned by devm_regulator_get_enable_optional()
but I need also to filter out the ENODEV error.

ENODEV, returned by devm_regulator_get_enable_optional(), means that the
regulator is not present. This should not be seen as an error by the caller.
Indeed, the regulator is not present and so, the related ADC core will not
be used. This is not an error from the caller perspective.

The code you proposed is not correct regarding this point.

Instead of my original code, I can propose the following:
	if (ret < 0) {
		if (ret == -ENODEV)
			return 0;

		return dev_err_probe(dev, ret,
				     "Failed to get '%s' regulator\n",
				     avdd_name);
	}

What do you think about it?

For other comments you have sent, I agree with them and I will take them into
account in the next iteration.

Best regards,
Hervé


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 2/4] iio: adc: Add support for the Renesas RZ/N1 ADC
  2025-11-03  8:40     ` Herve Codina
@ 2025-11-03  9:39       ` Andy Shevchenko
  0 siblings, 0 replies; 11+ messages in thread
From: Andy Shevchenko @ 2025-11-03  9:39 UTC (permalink / raw)
  To: Herve Codina
  Cc: Wolfram Sang, Jonathan Cameron, David Lechner, Nuno Sá,
	Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Geert Uytterhoeven, Magnus Damm, Liam Girdwood, Mark Brown,
	linux-iio, linux-renesas-soc, devicetree, linux-kernel,
	Pascal Eberhard, Miquel Raynal, Thomas Petazzoni

On Mon, Nov 03, 2025 at 09:40:45AM +0100, Herve Codina wrote:
> On Thu, 30 Oct 2025 11:00:12 +0200
> Andy Shevchenko <andriy.shevchenko@intel.com> wrote:

...

> > > +	ret = devm_regulator_get_enable_optional(dev, avdd_name);
> > > +	if (ret < 0) {
> > > +		if (ret != -ENODEV)
> > > +			return dev_err_probe(dev, ret,
> > > +					     "Failed to get '%s' regulator\n",
> > > +					     avdd_name);
> > > +		return 0;
> > > +	}  
> > 
> > 	if (ret == -ENODEV)
> > 		return dev_err_probe(); // takes less LoCs
> > 	if (ret < 0) // do we need ' < 0' part?
> > 		return 0;
> 
> Well, I need to abort on error returned by devm_regulator_get_enable_optional()
> but I need also to filter out the ENODEV error.
> 
> ENODEV, returned by devm_regulator_get_enable_optional(), means that the
> regulator is not present. This should not be seen as an error by the caller.
> Indeed, the regulator is not present and so, the related ADC core will not
> be used. This is not an error from the caller perspective.
> 
> The code you proposed is not correct regarding this point.

Yeah, sorry for that, but I think you got the idea...

The (working) solution should be like this:

	if (ret == -ENODEV)
		return 0;
	if (ret < 0) // do we need ' < 0' part?
		return dev_err_probe(); // takes less LoCs

> Instead of my original code, I can propose the following:
> 	if (ret < 0) {
> 		if (ret == -ENODEV)
> 			return 0;
> 
> 		return dev_err_probe(dev, ret,
> 				     "Failed to get '%s' regulator\n",
> 				     avdd_name);
> 	}
> 
> What do you think about it?

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 2/4] iio: adc: Add support for the Renesas RZ/N1 ADC
  2025-10-29 14:46 ` [PATCH v2 2/4] iio: adc: Add support for " Herve Codina (Schneider Electric)
  2025-10-30  9:00   ` Andy Shevchenko
@ 2025-11-03 11:34   ` Nuno Sá
  1 sibling, 0 replies; 11+ messages in thread
From: Nuno Sá @ 2025-11-03 11:34 UTC (permalink / raw)
  To: Herve Codina (Schneider Electric), Wolfram Sang, Jonathan Cameron,
	David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Geert Uytterhoeven,
	Magnus Damm, Liam Girdwood, Mark Brown
  Cc: linux-iio, linux-renesas-soc, devicetree, linux-kernel,
	Pascal Eberhard, Miquel Raynal, Thomas Petazzoni

On Wed, 2025-10-29 at 15:46 +0100, Herve Codina (Schneider Electric) wrote:
> The Renesas RZ/N1 ADC controller is the ADC controller available in the
> Renesas RZ/N1 SoCs family. It can use up to two internal ADC cores (ADC1
> and ADC2) those internal cores are not directly accessed but are handled
> through ADC controller virtual channels.
> 
> Signed-off-by: Herve Codina (Schneider Electric) <herve.codina@bootlin.com>
> ---

Not much to add to Andy's review. Looks in good shape... Just one small remark
from me. With it and Andy's stuff addressed:

Reviewed-by: Nuno Sá <nuno.sa@analog.com>

>  drivers/iio/adc/Kconfig    |  10 +
>  drivers/iio/adc/Makefile   |   1 +
>  drivers/iio/adc/rzn1-adc.c | 493 +++++++++++++++++++++++++++++++++++++
>  3 files changed, 504 insertions(+)
>  create mode 100644 drivers/iio/adc/rzn1-adc.c
> 
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 58a14e6833f6..113f6a5c9745 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -1403,6 +1403,16 @@ config RZG2L_ADC
>  	  To compile this driver as a module, choose M here: the
>  	  module will be called rzg2l_adc.
>  
> +config RZN1_ADC
> +	tristate "Renesas RZ/N1 ADC driver"
> +	depends on ARCH_RZN1 || COMPILE_TEST
> +	help
> +	  Say yes here to build support for the ADC found in Renesas
> +	  RZ/N1 family.
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called rzn1-adc.
> +
>  config SC27XX_ADC
>  	tristate "Spreadtrum SC27xx series PMICs ADC"
>  	depends on MFD_SC27XX_PMIC || COMPILE_TEST
> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> index d008f78dc010..ba7a8a63d070 100644
> --- a/drivers/iio/adc/Makefile
> +++ b/drivers/iio/adc/Makefile
> @@ -123,6 +123,7 @@ obj-$(CONFIG_ROHM_BD79112) += rohm-bd79112.o
>  obj-$(CONFIG_ROHM_BD79124) += rohm-bd79124.o
>  obj-$(CONFIG_ROCKCHIP_SARADC) += rockchip_saradc.o
>  obj-$(CONFIG_RZG2L_ADC) += rzg2l_adc.o
> +obj-$(CONFIG_RZN1_ADC) += rzn1-adc.o
>  obj-$(CONFIG_SC27XX_ADC) += sc27xx_adc.o
>  obj-$(CONFIG_SD_ADC_MODULATOR) += sd_adc_modulator.o
>  obj-$(CONFIG_SOPHGO_CV1800B_ADC) += sophgo-cv1800b-adc.o
> diff --git a/drivers/iio/adc/rzn1-adc.c b/drivers/iio/adc/rzn1-adc.c
> new file mode 100644
> index 000000000000..52ec13adddef
> --- /dev/null
> +++ b/drivers/iio/adc/rzn1-adc.c
> @@ -0,0 +1,493 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Renesas RZ/N1 ADC driver
> + *
> + * Copyright (C) 2025 Schneider-Electric
> + *
> + * Author: Herve Codina <herve.codina@bootlin.com>
> + *
> + * The RZ/N1 ADC controller can handle channels from its internal ADC1 and/or
> + * ADC2 cores. The driver use ADC1 and/or ADC2 cores depending on the presence
> + * of the related power supplies (AVDD and VREF) description in the device-tree.
> + */

...

> 
> +
> +	platform_set_drvdata(pdev, indio_dev);
> +

If I'm not missing nothing, there's no real need to pass in indio_dev. So, why not
passing rzn1_adc directly and avoid the pointer arithmetic's in the pm callbacks?

- Nuno Sá


^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2025-11-03 11:34 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-29 14:46 [PATCH v2 0/4] Add support for the Renesas RZ/N1 ADC Herve Codina (Schneider Electric)
2025-10-29 14:46 ` [PATCH v2 1/4] dt-bindings: iio: adc: Add " Herve Codina (Schneider Electric)
2025-10-30 18:02   ` Rob Herring (Arm)
2025-10-29 14:46 ` [PATCH v2 2/4] iio: adc: Add support for " Herve Codina (Schneider Electric)
2025-10-30  9:00   ` Andy Shevchenko
2025-11-02 11:42     ` Jonathan Cameron
2025-11-03  8:40     ` Herve Codina
2025-11-03  9:39       ` Andy Shevchenko
2025-11-03 11:34   ` Nuno Sá
2025-10-29 14:46 ` [PATCH v2 3/4] ARM: dts: renesas: r9a06g032: Add the ADC device Herve Codina (Schneider Electric)
2025-10-29 14:46 ` [PATCH v2 4/4] MAINTAINERS: Add the Renesas RZ/N1 ADC driver entry Herve Codina (Schneider Electric)

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).