[PATCH v3 0/3] Add SARADC support on Sophgo SoC

[PATCH v3 0/3] Add SARADC support on Sophgo SoC

[Thomas Bonnefille]

This patchset adds initial ADC support for Sophgo SoC. This driver can
work with or without interrupt and in "Active" and "No-Die" domains
depending on if a clock is provided.

Link: https://github.com/sophgo/sophgo-doc/releases/download/sg2002-trm-v1.0/sg2002_trm_en.pdf

Signed-off-by: Thomas Bonnefille <thomas.bonnefille@bootlin.com>
---
Changes in v3:
- Subdivide default cycle configuration into multiple elementary
  configurations
- Fix formatting in the driver
- Use devm_mutex_init
- Use devm_clk_get_enabled now because the clock is no more optional
- Remove handling of Saradc in No-Die Domain as RTC isn't implemented yet
- Use cv1800-saradc as default compatible instead of a wildcard
- Link to v2: https://lore.kernel.org/r/20240705-sg2002-adc-v2-0-83428c20a9b2@bootlin.com

Changes in v2:
- Drop modifications in MAINTAINERS file
- Rename the ADC from "sophgo-adc" to "sophgo-cv18xx-adc" to avoid
  conflict with ADCs available in future Sophgo SoCs.
- Reorder nodes in DT to match DTS coding style
- Switch from including <linux/of.h> to <linux/mod_devicetable.h>
- Use scoped_guard instead of mutex_lock/unlock
- Check IRQ Status in the handler
- Change IIO device name
- Use devm_clk_get_optional_enabled instead of a clock variable
- Init completion before the IRQ request
- Removed unnecessary iio_info structure in the private data of the
  driver
- Use SoC specific compatible in the bindings and device trees
- Link to v1: https://lore.kernel.org/r/20240702-sg2002-adc-v1-0-ac66e076a756@bootlin.com

---
Thomas Bonnefille (3):
      dt-bindings: iio: adc: sophgo,cv18xx-saradc.yaml: Add Sophgo SARADC binding documentation
      iio: adc: sophgo-saradc: Add driver for Sophgo SARADC
      riscv: dts: sophgo: Add SARADC description

 .../bindings/iio/adc/sophgo,cv18xx-saradc.yaml     |  48 +++++
 arch/riscv/boot/dts/sophgo/cv18xx.dtsi             |   8 +
 drivers/iio/adc/Kconfig                            |  10 +
 drivers/iio/adc/Makefile                           |   1 +
 drivers/iio/adc/sophgo-cv18xx-adc.c                | 206 +++++++++++++++++++++
 5 files changed, 273 insertions(+)
---
base-commit: 8400291e289ee6b2bf9779ff1c83a291501f017b
change-id: 20240527-sg2002-adc-924b862cd3f2

Best regards,
-- 
Thomas Bonnefille <thomas.bonnefille@bootlin.com>

[PATCH v3 1/3] dt-bindings: iio: adc: sophgo,cv18xx-saradc.yaml: Add Sophgo SARADC binding documentation

[Thomas Bonnefille]

The Sophgo SARADC is a Successive Approximation ADC that can be found in
the Sophgo SoC.

Signed-off-by: Thomas Bonnefille <thomas.bonnefille@bootlin.com>
---
 .../bindings/iio/adc/sophgo,cv18xx-saradc.yaml     | 48 ++++++++++++++++++++++
 1 file changed, 48 insertions(+)

diff --git a/Documentation/devicetree/bindings/iio/adc/sophgo,cv18xx-saradc.yaml b/Documentation/devicetree/bindings/iio/adc/sophgo,cv18xx-saradc.yaml
new file mode 100644
index 000000000000..79d8cb52279f
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/sophgo,cv18xx-saradc.yaml
@@ -0,0 +1,48 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/adc/sophgo,cv18xx-saradc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title:
+  Sophgo CV18XX SoC series 3 channels Successive Approximation Analog to
+  Digital Converters
+
+maintainers:
+  - Thomas Bonnefille <thomas.bonnefille@bootlin.com>
+
+description:
+  Datasheet at https://github.com/sophgo/sophgo-doc/releases
+
+properties:
+  compatible:
+    oneOf:
+      - items:
+          - const: sophgo,cv1800b-saradc
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+required:
+  - clocks
+  - compatible
+  - reg
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/sophgo,cv1800.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+    adc@30f0000 {
+        compatible = "sophgo,cv1800b-saradc";
+        clocks = <&clk CLK_SARADC>;
+        interrupts = <100 IRQ_TYPE_LEVEL_HIGH>;
+        reg = <0x030F0000 0x1000>;
+    };

-- 
2.45.2

[PATCH v3 2/3] iio: adc: sophgo-saradc: Add driver for Sophgo SARADC

[Thomas Bonnefille]

This adds a driver for the common Sophgo SARADC.

Signed-off-by: Thomas Bonnefille <thomas.bonnefille@bootlin.com>
---
 drivers/iio/adc/Kconfig             |  10 ++
 drivers/iio/adc/Makefile            |   1 +
 drivers/iio/adc/sophgo-cv18xx-adc.c | 206 ++++++++++++++++++++++++++++++++++++
 3 files changed, 217 insertions(+)

diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index f60fe85a30d5..10d6570233f5 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -1156,6 +1156,16 @@ config SC27XX_ADC
 	  This driver can also be built as a module. If so, the module
 	  will be called sc27xx_adc.
 
+config SOPHGO_CV18XX_ADC
+	tristate "Sophgo CV18XX series SARADC"
+	depends on ARCH_SOPHGO || COMPILE_TEST
+	help
+	  Say yes here to build support for the SARADC integrated inside
+	  the Sophgo CV18XX series SoCs.
+
+	  This driver can also be built as a module. If so, the module
+	  will be called sophgo_adc.
+
 config SPEAR_ADC
 	tristate "ST SPEAr ADC"
 	depends on PLAT_SPEAR || COMPILE_TEST
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index d370e066544e..24c241b12ef0 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -105,6 +105,7 @@ obj-$(CONFIG_ROCKCHIP_SARADC) += rockchip_saradc.o
 obj-$(CONFIG_RZG2L_ADC) += rzg2l_adc.o
 obj-$(CONFIG_SC27XX_ADC) += sc27xx_adc.o
 obj-$(CONFIG_SD_ADC_MODULATOR) += sd_adc_modulator.o
+obj-$(CONFIG_SOPHGO_CV18XX_ADC) += sophgo-cv18xx-adc.o
 obj-$(CONFIG_SPEAR_ADC) += spear_adc.o
 obj-$(CONFIG_STM32_ADC_CORE) += stm32-adc-core.o
 obj-$(CONFIG_STM32_ADC) += stm32-adc.o
diff --git a/drivers/iio/adc/sophgo-cv18xx-adc.c b/drivers/iio/adc/sophgo-cv18xx-adc.c
new file mode 100644
index 000000000000..27e1aac9560f
--- /dev/null
+++ b/drivers/iio/adc/sophgo-cv18xx-adc.c
@@ -0,0 +1,206 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ *  Sophgo CV18XX series SARADC Driver
+ *
+ *  Copyright (C) Bootlin 2024
+ *  Author: Thomas Bonnefille <thomas.bonnefille@bootlin.com>
+ */
+
+#include <linux/bitfield.h>
+#include <linux/clk.h>
+#include <linux/completion.h>
+#include <linux/dev_printk.h>
+#include <linux/interrupt.h>
+#include <linux/iio/iio.h>
+#include <linux/iopoll.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/platform_device.h>
+
+#define CV18XX_ADC_CTRL_REG			0x04
+#define		CV18XX_ADC_EN			BIT(0)
+#define		CV18XX_ADC_SEL(x)		BIT((x) + 4)
+#define CV18XX_ADC_STATUS_REG			0x08
+#define		CV18XX_ADC_BUSY			BIT(0)
+#define CV18XX_ADC_CYC_SET_REG			0x0C
+/* The default cycle configuration is set to maximize the accuracy */
+#define		CV18XX_ADC_DEF_STARTUP_CYCLE	0xF
+#define		CV18XX_ADC_DEF_SAMPLE_WINDOW	(0xF << 8)
+#define		CV18XX_ADC_DEF_CLOCK_DIVIDER	(0x1 << 12)
+#define		CV18XX_ADC_DEF_COMPARE_CYCLE	(0xF << 16)
+#define CV18XX_ADC_CH_RESULT_REG(x)		(0x10 + 4 * (x))
+#define		CV18XX_ADC_CH_RESULT		GENMASK(11, 0)
+#define		CV18XX_ADC_CH_VALID		BIT(15)
+#define CV18XX_ADC_INTR_EN_REG			0x20
+#define CV18XX_ADC_INTR_CLR_REG			0x24
+#define		CV18XX_ADC_INTR_CLR_BIT		BIT(0)
+#define CV18XX_ADC_INTR_STA_REG			0x28
+#define		CV18XX_ADC_INTR_STA_BIT		BIT(0)
+
+#define CV18XX_ADC_CHANNEL(index)					\
+	{								\
+		.type = IIO_VOLTAGE,					\
+		.indexed = 1,						\
+		.channel = index,					\
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),		\
+		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),	\
+		.scan_index = index,					\
+	}
+
+struct cv18xx_adc {
+	struct completion completion;
+	void __iomem *regs;
+	struct mutex lock; /* ADC Control and Result register */
+	int irq;
+};
+
+static const struct iio_chan_spec sophgo_channels[] = {
+	CV18XX_ADC_CHANNEL(1),
+	CV18XX_ADC_CHANNEL(2),
+	CV18XX_ADC_CHANNEL(3),
+};
+
+static void cv18xx_adc_start_measurement(struct cv18xx_adc *saradc,
+					    int channel)
+{
+	writel(0, saradc->regs + CV18XX_ADC_CTRL_REG);
+	writel(CV18XX_ADC_SEL(channel) | CV18XX_ADC_EN,
+	       saradc->regs + CV18XX_ADC_CTRL_REG);
+}
+
+static int cv18xx_adc_wait(struct cv18xx_adc *saradc)
+{
+	if (saradc->irq < 0) {
+		u32 reg;
+
+		return readl_poll_timeout(saradc->regs + CV18XX_ADC_STATUS_REG,
+					  reg, !(reg & CV18XX_ADC_BUSY),
+					  500, 1000000);
+	}
+	return wait_for_completion_timeout(&saradc->completion,
+					  msecs_to_jiffies(1000)) > 0
+					  ? 0 : -ETIMEDOUT;
+}
+
+static int cv18xx_adc_read_raw(struct iio_dev *indio_dev,
+				  struct iio_chan_spec const *chan,
+				  int *val, int *val2, long mask)
+{
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		{
+		struct cv18xx_adc *saradc = iio_priv(indio_dev);
+		u32 sample;
+		int ret;
+
+		scoped_guard(mutex, &saradc->lock) {
+			cv18xx_adc_start_measurement(saradc, chan->scan_index);
+			ret = cv18xx_adc_wait(saradc);
+			if (ret < 0)
+				return ret;
+
+			sample = readl(saradc->regs + CV18XX_ADC_CH_RESULT_REG(chan->scan_index));
+		}
+		if (!(sample & CV18XX_ADC_CH_VALID))
+			return -ENODATA;
+
+		*val = sample & CV18XX_ADC_CH_RESULT;
+		return IIO_VAL_INT;
+		}
+	case IIO_CHAN_INFO_SCALE:
+		*val = 3300;
+		*val2 = 12;
+		return IIO_VAL_FRACTIONAL_LOG2;
+	default:
+		return -EINVAL;
+	}
+}
+
+static irqreturn_t cv18xx_adc_interrupt_handler(int irq, void *private)
+{
+	struct cv18xx_adc *saradc = private;
+
+	if (!(FIELD_GET(CV18XX_ADC_INTR_STA_BIT,
+			readl(saradc->regs + CV18XX_ADC_INTR_STA_REG))))
+		return IRQ_NONE;
+
+	writel(CV18XX_ADC_INTR_CLR_BIT, saradc->regs + CV18XX_ADC_INTR_CLR_REG);
+	complete(&saradc->completion);
+	return IRQ_HANDLED;
+}
+
+static const struct iio_info cv18xx_adc_info = {
+	.read_raw = &cv18xx_adc_read_raw,
+};
+
+static int cv18xx_adc_probe(struct platform_device *pdev)
+{
+	struct cv18xx_adc *saradc;
+	struct iio_dev *indio_dev;
+	int ret;
+
+	indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*saradc));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	saradc = iio_priv(indio_dev);
+	indio_dev->name = "sophgo-cv18xx-adc";
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->info = &cv18xx_adc_info;
+	indio_dev->num_channels = ARRAY_SIZE(sophgo_channels);
+	indio_dev->channels = sophgo_channels;
+
+	ret = IS_ERR(devm_clk_get_enabled(&pdev->dev, NULL));
+	if (ret)
+		return ret;
+
+	saradc->regs = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(saradc->regs))
+		return PTR_ERR(saradc->regs);
+
+	saradc->irq = platform_get_irq_optional(pdev, 0);
+	if (saradc->irq >= 0) {
+		init_completion(&saradc->completion);
+		ret = devm_request_irq(&pdev->dev, saradc->irq,
+				       cv18xx_adc_interrupt_handler, 0,
+				       dev_name(&pdev->dev), saradc);
+		if (ret)
+			return ret;
+
+		writel(1, saradc->regs + CV18XX_ADC_INTR_EN_REG);
+	}
+
+	ret = devm_mutex_init(&pdev->dev, &saradc->lock);
+	if (ret)
+		return ret;
+
+	platform_set_drvdata(pdev, indio_dev);
+	writel(CV18XX_ADC_DEF_STARTUP_CYCLE |
+	       CV18XX_ADC_DEF_SAMPLE_WINDOW |
+	       CV18XX_ADC_DEF_CLOCK_DIVIDER |
+	       CV18XX_ADC_DEF_COMPARE_CYCLE
+	       , saradc->regs + CV18XX_ADC_CYC_SET_REG);
+	return devm_iio_device_register(&pdev->dev, indio_dev);
+
+}
+
+static const struct of_device_id cv18xx_adc_match[] = {
+	{ .compatible = "sophgo,cv1800b-saradc", },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, cv18xx_adc_match);
+
+static struct platform_driver cv18xx_adc_driver = {
+	.driver	= {
+		.name		= "sophgo-saradc",
+		.of_match_table	= cv18xx_adc_match,
+	},
+	.probe = cv18xx_adc_probe,
+};
+module_platform_driver(cv18xx_adc_driver);
+
+MODULE_AUTHOR("Thomas Bonnefille <thomas.bonnefille@bootlin.com>");
+MODULE_DESCRIPTION("Sophgo SARADC driver");
+MODULE_LICENSE("GPL");

-- 
2.45.2

[PATCH v3 3/3] riscv: dts: sophgo: Add SARADC description

[Thomas Bonnefille]

Adds SARADC nodes for the common Successive Approximation Analog to
Digital Converter used in Sophgo CV18xx series SoC.
This patch adds two nodes for the two controllers the board, one in
the Active domain and the other in the No-Die domain.

Signed-off-by: Thomas Bonnefille <thomas.bonnefille@bootlin.com>
---
 arch/riscv/boot/dts/sophgo/cv18xx.dtsi | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/riscv/boot/dts/sophgo/cv18xx.dtsi b/arch/riscv/boot/dts/sophgo/cv18xx.dtsi
index 891932ae470f..e6c1a84d3e55 100644
--- a/arch/riscv/boot/dts/sophgo/cv18xx.dtsi
+++ b/arch/riscv/boot/dts/sophgo/cv18xx.dtsi
@@ -133,6 +133,14 @@ portd: gpio-controller@0 {
 			};
 		};
 
+		saradc: adc@30f0000 {
+			compatible = "sophgo,cv1800b-saradc";
+			clocks = <&clk CLK_SARADC>;
+			interrupts = <100 IRQ_TYPE_LEVEL_HIGH>;
+			reg = <0x030F0000 0x1000>;
+			status = "disabled";
+		};
+
 		i2c0: i2c@4000000 {
 			compatible = "snps,designware-i2c";
 			reg = <0x04000000 0x10000>;

-- 
2.45.2

Re: [PATCH v3 1/3] dt-bindings: iio: adc: sophgo,cv18xx-saradc.yaml: Add Sophgo SARADC binding documentation

[Inochi Amaoto]

On Wed, Jul 31, 2024 at 02:24:14PM GMT, Thomas Bonnefille wrote:
> The Sophgo SARADC is a Successive Approximation ADC that can be found in
> the Sophgo SoC.
> 
> Signed-off-by: Thomas Bonnefille <thomas.bonnefille@bootlin.com>
> ---
>  .../bindings/iio/adc/sophgo,cv18xx-saradc.yaml     | 48 ++++++++++++++++++++++
>  1 file changed, 48 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/sophgo,cv18xx-saradc.yaml b/Documentation/devicetree/bindings/iio/adc/sophgo,cv18xx-saradc.yaml
> new file mode 100644
> index 000000000000..79d8cb52279f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/sophgo,cv18xx-saradc.yaml
> @@ -0,0 +1,48 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/adc/sophgo,cv18xx-saradc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title:
> +  Sophgo CV18XX SoC series 3 channels Successive Approximation Analog to
> +  Digital Converters
> +
> +maintainers:
> +  - Thomas Bonnefille <thomas.bonnefille@bootlin.com>
> +
> +description:
> +  Datasheet at https://github.com/sophgo/sophgo-doc/releases
> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - items:
> +          - const: sophgo,cv1800b-saradc

There is no need to use "oneOF" and "items"

Suggestions: add a compatible like "cv1800-saradc" as fallback
and add use "sophgo,cv1800b-saradc" as specific compatible.
Use the "cv1800-saradc" in the cv18xx.dtsi and override the
compatible with specific one if necessary.

For example:
- items:
    - enum:
        - sophgo,cv1800b-saradc
    - const: sophgo,cv1800-saradc
- const: sophgo,cv1800b-saradc

Regards,
Inochi

Re: [PATCH v3 1/3] dt-bindings: iio: adc: sophgo,cv18xx-saradc.yaml: Add Sophgo SARADC binding documentation

[Krzysztof Kozlowski]

On 31/07/2024 14:24, Thomas Bonnefille wrote:
> The Sophgo SARADC is a Successive Approximation ADC that can be found in
> the Sophgo SoC.
> 
> Signed-off-by: Thomas Bonnefille <thomas.bonnefille@bootlin.com>

A nit, subject: drop second/last, redundant "binding documentation". The
"dt-bindings" prefix is already stating that these are bindings and this
is documentation. Cannot be anything else.
See also:
https://elixir.bootlin.com/linux/v6.7-rc8/source/Documentation/devicetree/bindings/submitting-patches.rst#L18


> ---
>  .../bindings/iio/adc/sophgo,cv18xx-saradc.yaml     | 48 ++++++++++++++++++++++
>  1 file changed, 48 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/sophgo,cv18xx-saradc.yaml b/Documentation/devicetree/bindings/iio/adc/sophgo,cv18xx-saradc.yaml
> new file mode 100644
> index 000000000000..79d8cb52279f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/sophgo,cv18xx-saradc.yaml
> @@ -0,0 +1,48 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/adc/sophgo,cv18xx-saradc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title:
> +  Sophgo CV18XX SoC series 3 channels Successive Approximation Analog to
> +  Digital Converters
> +
> +maintainers:
> +  - Thomas Bonnefille <thomas.bonnefille@bootlin.com>
> +
> +description:
> +  Datasheet at https://github.com/sophgo/sophgo-doc/releases
> +
> +properties:
> +  compatible:
> +    oneOf:

Drop

> +      - items:

Drop, use const directly.


> +          - const: sophgo,cv1800b-saradc
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 1
> +
> +required:
> +  - clocks
> +  - compatible
> +  - reg

Odd order. compatible is always first. Anyway, list here has the same
order as "properties:".

> +
> +unevaluatedProperties: false

I don't see any other $ref. Don't you miss adc.yaml? Or channels? Or
some more properties? This looks incomplete for ADC.

> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/sophgo,cv1800.h>
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    adc@30f0000 {
> +        compatible = "sophgo,cv1800b-saradc";
> +        clocks = <&clk CLK_SARADC>;
> +        interrupts = <100 IRQ_TYPE_LEVEL_HIGH>;
> +        reg = <0x030F0000 0x1000>;

Hex is *always* lower case, see DTS coding style.

Order the properties correctly, so again, please read DTS coding style.

> +    };
> 

Best regards,
Krzysztof

Re: [PATCH v3 3/3] riscv: dts: sophgo: Add SARADC description

[Krzysztof Kozlowski]

On 31/07/2024 14:24, Thomas Bonnefille wrote:
> Adds SARADC nodes for the common Successive Approximation Analog to
> Digital Converter used in Sophgo CV18xx series SoC.
> This patch adds two nodes for the two controllers the board, one in
> the Active domain and the other in the No-Die domain.
> 
> Signed-off-by: Thomas Bonnefille <thomas.bonnefille@bootlin.com>
> ---
>  arch/riscv/boot/dts/sophgo/cv18xx.dtsi | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/arch/riscv/boot/dts/sophgo/cv18xx.dtsi b/arch/riscv/boot/dts/sophgo/cv18xx.dtsi
> index 891932ae470f..e6c1a84d3e55 100644
> --- a/arch/riscv/boot/dts/sophgo/cv18xx.dtsi
> +++ b/arch/riscv/boot/dts/sophgo/cv18xx.dtsi
> @@ -133,6 +133,14 @@ portd: gpio-controller@0 {
>  			};
>  		};
>  
> +		saradc: adc@30f0000 {
> +			compatible = "sophgo,cv1800b-saradc";
> +			clocks = <&clk CLK_SARADC>;
> +			interrupts = <100 IRQ_TYPE_LEVEL_HIGH>;
> +			reg = <0x030F0000 0x1000>;

Please read carefully DTS coding style.

Best regards,
Krzysztof

Re: [PATCH v3 1/3] dt-bindings: iio: adc: sophgo,cv18xx-saradc.yaml: Add Sophgo SARADC binding documentation

[Thomas Bonnefille]

On 7/31/24 2:41 PM, Inochi Amaoto wrote:
> On Wed, Jul 31, 2024 at 02:24:14PM GMT, Thomas Bonnefille wrote:
>> The Sophgo SARADC is a Successive Approximation ADC that can be found in
>> the Sophgo SoC.
>>
>> Signed-off-by: Thomas Bonnefille <thomas.bonnefille@bootlin.com>
>> ---
>>   .../bindings/iio/adc/sophgo,cv18xx-saradc.yaml     | 48 ++++++++++++++++++++++
>>   1 file changed, 48 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/iio/adc/sophgo,cv18xx-saradc.yaml b/Documentation/devicetree/bindings/iio/adc/sophgo,cv18xx-saradc.yaml
>> new file mode 100644
>> index 000000000000..79d8cb52279f
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/iio/adc/sophgo,cv18xx-saradc.yaml
>> @@ -0,0 +1,48 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/iio/adc/sophgo,cv18xx-saradc.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title:
>> +  Sophgo CV18XX SoC series 3 channels Successive Approximation Analog to
>> +  Digital Converters
>> +
>> +maintainers:
>> +  - Thomas Bonnefille <thomas.bonnefille@bootlin.com>
>> +
>> +description:
>> +  Datasheet at https://github.com/sophgo/sophgo-doc/releases
>> +
>> +properties:
>> +  compatible:
>> +    oneOf:
>> +      - items:
>> +          - const: sophgo,cv1800b-saradc
> 
> There is no need to use "oneOF" and "items"
> 

Thank you very much, I'll do that.

> Suggestions: add a compatible like "cv1800-saradc" as fallback
> and add use "sophgo,cv1800b-saradc" as specific compatible.
> Use the "cv1800-saradc" in the cv18xx.dtsi and override the
> compatible with specific one if necessary.
> 

If I understand correctly, maintainers doesn't want the use of wildcards 
as generic compatibles [1]. They prefer to use the most basic SoC as the 
generic compatible.
Is the CV1800 a real SoC or is it just a kind of wildcard to say CV18* ?

> For example:
> - items:
>      - enum:
>          - sophgo,cv1800b-saradc
>      - const: sophgo,cv1800-saradc
> - const: sophgo,cv1800b-saradc
> 

To avoid the issue of falling back on a wildcard I planned to do this 
instead:
properties:
   compatible:
     const: sophgo,cv1800b-saradc



> Regards,
> Inochi

[1] : https://lore.kernel.org/all/20240708165719.000021b9@Huawei.com/

Thank you for your comments :)
Thomas

Re: [PATCH v3 1/3] dt-bindings: iio: adc: sophgo,cv18xx-saradc.yaml: Add Sophgo SARADC binding documentation

[Inochi Amaoto]

On Wed, Jul 31, 2024 at 04:48:01PM GMT, Thomas Bonnefille wrote:
> 
> 
> On 7/31/24 2:41 PM, Inochi Amaoto wrote:
> > On Wed, Jul 31, 2024 at 02:24:14PM GMT, Thomas Bonnefille wrote:
> > > The Sophgo SARADC is a Successive Approximation ADC that can be found in
> > > the Sophgo SoC.
> > > 
> > > Signed-off-by: Thomas Bonnefille <thomas.bonnefille@bootlin.com>
> > > ---
> > >   .../bindings/iio/adc/sophgo,cv18xx-saradc.yaml     | 48 ++++++++++++++++++++++
> > >   1 file changed, 48 insertions(+)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/iio/adc/sophgo,cv18xx-saradc.yaml b/Documentation/devicetree/bindings/iio/adc/sophgo,cv18xx-saradc.yaml
> > > new file mode 100644
> > > index 000000000000..79d8cb52279f
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/iio/adc/sophgo,cv18xx-saradc.yaml
> > > @@ -0,0 +1,48 @@
> > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/iio/adc/sophgo,cv18xx-saradc.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title:
> > > +  Sophgo CV18XX SoC series 3 channels Successive Approximation Analog to
> > > +  Digital Converters
> > > +
> > > +maintainers:
> > > +  - Thomas Bonnefille <thomas.bonnefille@bootlin.com>
> > > +
> > > +description:
> > > +  Datasheet at https://github.com/sophgo/sophgo-doc/releases
> > > +
> > > +properties:
> > > +  compatible:
> > > +    oneOf:
> > > +      - items:
> > > +          - const: sophgo,cv1800b-saradc
> > 
> > There is no need to use "oneOF" and "items"
> > 
> 
> Thank you very much, I'll do that.
> 
> > Suggestions: add a compatible like "cv1800-saradc" as fallback
> > and add use "sophgo,cv1800b-saradc" as specific compatible.
> > Use the "cv1800-saradc" in the cv18xx.dtsi and override the
> > compatible with specific one if necessary.
> > 
> 
> If I understand correctly, maintainers doesn't want the use of wildcards as
> generic compatibles [1]. They prefer to use the most basic SoC as the
> generic compatible.
> Is the CV1800 a real SoC or is it just a kind of wildcard to say CV18* ?

Yeah, that is the wildcard too. 

> 
> > For example:
> > - items:
> >      - enum:
> >          - sophgo,cv1800b-saradc
> >      - const: sophgo,cv1800-saradc
> > - const: sophgo,cv1800b-saradc
> > 
> 
> To avoid the issue of falling back on a wildcard I planned to do this
> instead:
> properties:
>   compatible:
>     const: sophgo,cv1800b-saradc
> 

I have read the link, it seems the upstream may prefer this solution.
Let's use cv1800b as the basic fallback.

> 
> 
> > Regards,
> > Inochi
> 
> [1] : https://lore.kernel.org/all/20240708165719.000021b9@Huawei.com/
> 
> Thank you for your comments :)
> Thomas

Re: [PATCH v3 0/3] Add SARADC support on Sophgo SoC

[Chen Wang]

Please add the code name of the specific chip product to the 
patch/commit/email title for easy distinguish.

Sophgo has several product series, not just cv18xx, for example, sg2042 
etc, and there may be others in the future.

You can refer to this example [1]

[1]: 
https://lore.kernel.org/linux-riscv/IA1PR20MB4953C1876484E149AA390DD5BB1D2@IA1PR20MB4953.namprd20.prod.outlook.com/

Thanks,

Chen

On 2024/7/31 20:24, Thomas Bonnefille wrote:
> This patchset adds initial ADC support for Sophgo SoC. This driver can
> work with or without interrupt and in "Active" and "No-Die" domains
> depending on if a clock is provided.
>
> Link: https://github.com/sophgo/sophgo-doc/releases/download/sg2002-trm-v1.0/sg2002_trm_en.pdf
>
> Signed-off-by: Thomas Bonnefille <thomas.bonnefille@bootlin.com>
> ---
> Changes in v3:
> - Subdivide default cycle configuration into multiple elementary
>    configurations
> - Fix formatting in the driver
> - Use devm_mutex_init
> - Use devm_clk_get_enabled now because the clock is no more optional
> - Remove handling of Saradc in No-Die Domain as RTC isn't implemented yet
> - Use cv1800-saradc as default compatible instead of a wildcard
> - Link to v2: https://lore.kernel.org/r/20240705-sg2002-adc-v2-0-83428c20a9b2@bootlin.com
>
> Changes in v2:
> - Drop modifications in MAINTAINERS file
> - Rename the ADC from "sophgo-adc" to "sophgo-cv18xx-adc" to avoid
>    conflict with ADCs available in future Sophgo SoCs.
> - Reorder nodes in DT to match DTS coding style
> - Switch from including <linux/of.h> to <linux/mod_devicetable.h>
> - Use scoped_guard instead of mutex_lock/unlock
> - Check IRQ Status in the handler
> - Change IIO device name
> - Use devm_clk_get_optional_enabled instead of a clock variable
> - Init completion before the IRQ request
> - Removed unnecessary iio_info structure in the private data of the
>    driver
> - Use SoC specific compatible in the bindings and device trees
> - Link to v1: https://lore.kernel.org/r/20240702-sg2002-adc-v1-0-ac66e076a756@bootlin.com
>
> ---
> Thomas Bonnefille (3):
>        dt-bindings: iio: adc: sophgo,cv18xx-saradc.yaml: Add Sophgo SARADC binding documentation
>        iio: adc: sophgo-saradc: Add driver for Sophgo SARADC
>        riscv: dts: sophgo: Add SARADC description
>
>   .../bindings/iio/adc/sophgo,cv18xx-saradc.yaml     |  48 +++++
>   arch/riscv/boot/dts/sophgo/cv18xx.dtsi             |   8 +
>   drivers/iio/adc/Kconfig                            |  10 +
>   drivers/iio/adc/Makefile                           |   1 +
>   drivers/iio/adc/sophgo-cv18xx-adc.c                | 206 +++++++++++++++++++++
>   5 files changed, 273 insertions(+)
> ---
> base-commit: 8400291e289ee6b2bf9779ff1c83a291501f017b
> change-id: 20240527-sg2002-adc-924b862cd3f2
>
> Best regards,

Re: [PATCH v3 2/3] iio: adc: sophgo-saradc: Add driver for Sophgo SARADC

[Chen Wang]

On 2024/7/31 20:24, Thomas Bonnefille wrote:
> This adds a driver for the common Sophgo SARADC.
>
> Signed-off-by: Thomas Bonnefille <thomas.bonnefille@bootlin.com>
> ---
>   drivers/iio/adc/Kconfig             |  10 ++
>   drivers/iio/adc/Makefile            |   1 +
>   drivers/iio/adc/sophgo-cv18xx-adc.c | 206 ++++++++++++++++++++++++++++++++++++
>   3 files changed, 217 insertions(+)
>
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index f60fe85a30d5..10d6570233f5 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -1156,6 +1156,16 @@ config SC27XX_ADC
>   	  This driver can also be built as a module. If so, the module
>   	  will be called sc27xx_adc.
>   
> +config SOPHGO_CV18XX_ADC
> +	tristate "Sophgo CV18XX series SARADC"
> +	depends on ARCH_SOPHGO || COMPILE_TEST
> +	help
> +	  Say yes here to build support for the SARADC integrated inside
> +	  the Sophgo CV18XX series SoCs.
> +
> +	  This driver can also be built as a module. If so, the module
> +	  will be called sophgo_adc.

"sophgo_adc"? or "sophgo-cv18xx-adc"

> +
>   config SPEAR_ADC
>   	tristate "ST SPEAr ADC"
>   	depends on PLAT_SPEAR || COMPILE_TEST
> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> index d370e066544e..24c241b12ef0 100644
> --- a/drivers/iio/adc/Makefile
> +++ b/drivers/iio/adc/Makefile
> @@ -105,6 +105,7 @@ obj-$(CONFIG_ROCKCHIP_SARADC) += rockchip_saradc.o
>   obj-$(CONFIG_RZG2L_ADC) += rzg2l_adc.o
>   obj-$(CONFIG_SC27XX_ADC) += sc27xx_adc.o
>   obj-$(CONFIG_SD_ADC_MODULATOR) += sd_adc_modulator.o
> +obj-$(CONFIG_SOPHGO_CV18XX_ADC) += sophgo-cv18xx-adc.o
>   obj-$(CONFIG_SPEAR_ADC) += spear_adc.o
>   obj-$(CONFIG_STM32_ADC_CORE) += stm32-adc-core.o
>   obj-$(CONFIG_STM32_ADC) += stm32-adc.o
> diff --git a/drivers/iio/adc/sophgo-cv18xx-adc.c b/drivers/iio/adc/sophgo-cv18xx-adc.c
> new file mode 100644
> index 000000000000..27e1aac9560f
> --- /dev/null
> +++ b/drivers/iio/adc/sophgo-cv18xx-adc.c
> @@ -0,0 +1,206 @@
> [......]
> +
> +static const struct of_device_id cv18xx_adc_match[] = {
> +	{ .compatible = "sophgo,cv1800b-saradc", },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, cv18xx_adc_match);
> +
> +static struct platform_driver cv18xx_adc_driver = {
> +	.driver	= {
> +		.name		= "sophgo-saradc",
Add product name to differ other sogho soc. You can refer to 
drivers/iio/adc/ti_am335x_adc.c.
> +		.of_match_table	= cv18xx_adc_match,
> +	},
> +	.probe = cv18xx_adc_probe,
> +};
> +module_platform_driver(cv18xx_adc_driver);
> +
> +MODULE_AUTHOR("Thomas Bonnefille <thomas.bonnefille@bootlin.com>");
> +MODULE_DESCRIPTION("Sophgo SARADC driver");
Add product name to differ, don't use general name.
> +MODULE_LICENSE("GPL");
>

Re: [PATCH v3 1/3] dt-bindings: iio: adc: sophgo,cv18xx-saradc.yaml: Add Sophgo SARADC binding documentation

[Jonathan Cameron]

On Wed, 31 Jul 2024 15:57:27 +0200
Krzysztof Kozlowski <krzk@kernel.org> wrote:

> On 31/07/2024 14:24, Thomas Bonnefille wrote:
> > The Sophgo SARADC is a Successive Approximation ADC that can be found in
> > the Sophgo SoC.
> > 
> > Signed-off-by: Thomas Bonnefille <thomas.bonnefille@bootlin.com>  
> 
> A nit, subject: drop second/last, redundant "binding documentation". The
> "dt-bindings" prefix is already stating that these are bindings and this
> is documentation. Cannot be anything else.
> See also:
> https://elixir.bootlin.com/linux/v6.7-rc8/source/Documentation/devicetree/bindings/submitting-patches.rst#L18
> 
> 
> > ---
> >  .../bindings/iio/adc/sophgo,cv18xx-saradc.yaml     | 48 ++++++++++++++++++++++
> >  1 file changed, 48 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/iio/adc/sophgo,cv18xx-saradc.yaml b/Documentation/devicetree/bindings/iio/adc/sophgo,cv18xx-saradc.yaml
> > new file mode 100644
> > index 000000000000..79d8cb52279f
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/iio/adc/sophgo,cv18xx-saradc.yaml
> > @@ -0,0 +1,48 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/iio/adc/sophgo,cv18xx-saradc.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title:
> > +  Sophgo CV18XX SoC series 3 channels Successive Approximation Analog to
> > +  Digital Converters
> > +
> > +maintainers:
> > +  - Thomas Bonnefille <thomas.bonnefille@bootlin.com>
> > +
> > +description:
> > +  Datasheet at https://github.com/sophgo/sophgo-doc/releases
> > +
> > +properties:
> > +  compatible:
> > +    oneOf:  
> 
> Drop
> 
> > +      - items:  
> 
> Drop, use const directly.
> 
> 
> > +          - const: sophgo,cv1800b-saradc
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  interrupts:
> > +    maxItems: 1
> > +
> > +  clocks:
> > +    maxItems: 1
> > +
> > +required:
> > +  - clocks
> > +  - compatible
> > +  - reg  
> 
> Odd order. compatible is always first. Anyway, list here has the same
> order as "properties:".
> 
> > +
> > +unevaluatedProperties: false  
> 
> I don't see any other $ref. Don't you miss adc.yaml? Or channels? Or
> some more properties? This looks incomplete for ADC.

It's acceptable on ADCs in particular (but more generally)
to just assume all channels are exposed.  They may all be wired
to internal power lines anyway, in which case what is there is
a chip feature.  This only works if their isn't any channel specific
configuration, then not providing the channels child nodes is fine.

However, that requires us to be fairly sure there won't ever be
a per channel thing that needs configuring from DT.
That's generally only the case in simple devices.

So might be better to put the channels nodes in there and deal with
dynamic channel configuration (so don't present any without
a channel node) from the start as it's more future proof.


> 
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/clock/sophgo,cv1800.h>
> > +    #include <dt-bindings/interrupt-controller/irq.h>
> > +    adc@30f0000 {
> > +        compatible = "sophgo,cv1800b-saradc";
> > +        clocks = <&clk CLK_SARADC>;
> > +        interrupts = <100 IRQ_TYPE_LEVEL_HIGH>;
> > +        reg = <0x030F0000 0x1000>;  
> 
> Hex is *always* lower case, see DTS coding style.
> 
> Order the properties correctly, so again, please read DTS coding style.
> 
> > +    };
> >   
> 
> Best regards,
> Krzysztof
> 

Re: [PATCH v3 2/3] iio: adc: sophgo-saradc: Add driver for Sophgo SARADC

[Jonathan Cameron]

On Wed, 31 Jul 2024 14:24:15 +0200
Thomas Bonnefille <thomas.bonnefille@bootlin.com> wrote:

> This adds a driver for the common Sophgo SARADC.
> 
> Signed-off-by: Thomas Bonnefille <thomas.bonnefille@bootlin.com>

One small bug (you will return 1 on an error), and a few minor other comments inline.

Thanks,

Jonathan

> diff --git a/drivers/iio/adc/sophgo-cv18xx-adc.c b/drivers/iio/adc/sophgo-cv18xx-adc.c
> new file mode 100644
> index 000000000000..27e1aac9560f
> --- /dev/null
> +++ b/drivers/iio/adc/sophgo-cv18xx-adc.c
> @@ -0,0 +1,206 @@

> +
> +#define CV18XX_ADC_CTRL_REG			0x04
> +#define		CV18XX_ADC_EN			BIT(0)
> +#define		CV18XX_ADC_SEL(x)		BIT((x) + 4)
> +#define CV18XX_ADC_STATUS_REG			0x08
> +#define		CV18XX_ADC_BUSY			BIT(0)
> +#define CV18XX_ADC_CYC_SET_REG			0x0C
> +/* The default cycle configuration is set to maximize the accuracy */
> +#define		CV18XX_ADC_DEF_STARTUP_CYCLE	0xF
> +#define		CV18XX_ADC_DEF_SAMPLE_WINDOW	(0xF << 8)

define masks for the various fields and use FIELD_PREP()
on those with the default values - but do that inline not up here
where we should only set the masks.

> +#define		CV18XX_ADC_DEF_CLOCK_DIVIDER	(0x1 << 12)
> +#define		CV18XX_ADC_DEF_COMPARE_CYCLE	(0xF << 16)
> +#define CV18XX_ADC_CH_RESULT_REG(x)		(0x10 + 4 * (x))
> +#define		CV18XX_ADC_CH_RESULT		GENMASK(11, 0)
> +#define		CV18XX_ADC_CH_VALID		BIT(15)
> +#define CV18XX_ADC_INTR_EN_REG			0x20
> +#define CV18XX_ADC_INTR_CLR_REG			0x24
> +#define		CV18XX_ADC_INTR_CLR_BIT		BIT(0)
> +#define CV18XX_ADC_INTR_STA_REG			0x28
> +#define		CV18XX_ADC_INTR_STA_BIT		BIT(0)

> +static const struct iio_chan_spec sophgo_channels[] = {
> +	CV18XX_ADC_CHANNEL(1),

why index from 1?  We tend to use 0 as the base, though there is no ABI
requirement to do so.  Hence this is ok, just unusual.

> +	CV18XX_ADC_CHANNEL(2),
> +	CV18XX_ADC_CHANNEL(3),
> +};

> +static int cv18xx_adc_read_raw(struct iio_dev *indio_dev,
> +				  struct iio_chan_spec const *chan,
> +				  int *val, int *val2, long mask)
> +{
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		{

I'd move { to previous line.

That will avoid need to indent another tab for the scope.

> +		struct cv18xx_adc *saradc = iio_priv(indio_dev);
> +		u32 sample;
> +		int ret;

Can move ert into scoped_guard scope.

> +
> +		scoped_guard(mutex, &saradc->lock) {
> +			cv18xx_adc_start_measurement(saradc, chan->scan_index);
> +			ret = cv18xx_adc_wait(saradc);
> +			if (ret < 0)
> +				return ret;
> +
> +			sample = readl(saradc->regs + CV18XX_ADC_CH_RESULT_REG(chan->scan_index));
> +		}
> +		if (!(sample & CV18XX_ADC_CH_VALID))
> +			return -ENODATA;
> +
> +		*val = sample & CV18XX_ADC_CH_RESULT;
> +		return IIO_VAL_INT;
> +		}
> +	case IIO_CHAN_INFO_SCALE:
> +		*val = 3300;
> +		*val2 = 12;
> +		return IIO_VAL_FRACTIONAL_LOG2;
> +	default:
> +		return -EINVAL;
> +	}
> +}


> +static int cv18xx_adc_probe(struct platform_device *pdev)
> +{
> +	struct cv18xx_adc *saradc;
> +	struct iio_dev *indio_dev;
> +	int ret;
> +
> +	indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*saradc));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	saradc = iio_priv(indio_dev);
> +	indio_dev->name = "sophgo-cv18xx-adc";
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +	indio_dev->info = &cv18xx_adc_info;
> +	indio_dev->num_channels = ARRAY_SIZE(sophgo_channels);
> +	indio_dev->channels = sophgo_channels;
> +
> +	ret = IS_ERR(devm_clk_get_enabled(&pdev->dev, NULL));
> +	if (ret)

PTR_ERR() not IS_ERR() for the return as you are returning a bool currently
not an error code.  Similar to what you do for the regulator below.

Better to use a local struct clk * and check that rather than convert
directly to the error value.


> +		return ret;
> +
> +	saradc->regs = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(saradc->regs))
> +		return PTR_ERR(saradc->regs);
> +
> +	saradc->irq = platform_get_irq_optional(pdev, 0);
> +	if (saradc->irq >= 0) {
> +		init_completion(&saradc->completion);
> +		ret = devm_request_irq(&pdev->dev, saradc->irq,
> +				       cv18xx_adc_interrupt_handler, 0,
> +				       dev_name(&pdev->dev), saradc);
> +		if (ret)
> +			return ret;
> +
> +		writel(1, saradc->regs + CV18XX_ADC_INTR_EN_REG);
> +	}
> +
> +	ret = devm_mutex_init(&pdev->dev, &saradc->lock);
> +	if (ret)
> +		return ret;
> +
> +	platform_set_drvdata(pdev, indio_dev);
> +	writel(CV18XX_ADC_DEF_STARTUP_CYCLE |
> +	       CV18XX_ADC_DEF_SAMPLE_WINDOW |
> +	       CV18XX_ADC_DEF_CLOCK_DIVIDER |
> +	       CV18XX_ADC_DEF_COMPARE_CYCLE
> +	       , saradc->regs + CV18XX_ADC_CYC_SET_REG);

Blank line here. + that leading comma should be on end of the line above.

> +	return devm_iio_device_register(&pdev->dev, indio_dev);
> +
No blank line here.
> +}

Re: [PATCH v3 1/3] dt-bindings: iio: adc: sophgo,cv18xx-saradc.yaml: Add Sophgo SARADC binding documentation

[Krzysztof Kozlowski]

On 03/08/2024 12:39, Jonathan Cameron wrote:
>>
>>> +
>>> +unevaluatedProperties: false  
>>
>> I don't see any other $ref. Don't you miss adc.yaml? Or channels? Or
>> some more properties? This looks incomplete for ADC.
> 
> It's acceptable on ADCs in particular (but more generally)
> to just assume all channels are exposed.  They may all be wired
> to internal power lines anyway, in which case what is there is
> a chip feature.  This only works if their isn't any channel specific
> configuration, then not providing the channels child nodes is fine.
> 
> However, that requires us to be fairly sure there won't ever be
> a per channel thing that needs configuring from DT.
> That's generally only the case in simple devices.
> 
> So might be better to put the channels nodes in there and deal with
> dynamic channel configuration (so don't present any without
> a channel node) from the start as it's more future proof.

OK. Then anyway this should be additionalProperties: false (unless I
missed somewhere $ref?).

Best regards,
Krzysztof