[PATCH 0/4] Add Raspberry Pi's RP1 ADC

[PATCH 0/4] Add Raspberry Pi's RP1 ADC

[Stanimir Varbanov]

Hello,

The following patch-set is adding support for RP1 ADC driver,
and enable it for RPi5.

Comments are welcome!

regards,
~Stan

Stanimir Varbanov (4):
  dt-bindings: Add Raspberry Pi's RP1 ADC
  hwmon: adc: rp1: Add Raspberry Pi's RP1 ADC driver
  arm64: dts: rp1: Add Raspberry Pi's RP1 ADC DT node
  arm64: dts: broadcom: Enable RP1 ADC for RPi5

 .../bindings/hwmon/raspberrypi,rp1-adc.yaml   |  46 +++
 .../dts/broadcom/bcm2712-rpi-5-b-ovl-rp1.dts  |   8 +
 .../boot/dts/broadcom/bcm2712-rpi-5-b.dts     |   5 +
 arch/arm64/boot/dts/broadcom/rp1-common.dtsi  |   7 +
 drivers/hwmon/Kconfig                         |  10 +
 drivers/hwmon/Makefile                        |   1 +
 drivers/hwmon/rp1-adc.c                       | 301 ++++++++++++++++++
 7 files changed, 378 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/hwmon/raspberrypi,rp1-adc.yaml
 create mode 100644 drivers/hwmon/rp1-adc.c

-- 
2.47.0

[PATCH 1/4] dt-bindings: Add Raspberry Pi's RP1 ADC

[Stanimir Varbanov]

Document dt-bindings for Raspberry Pi's RP1 ADC.

Signed-off-by: Stanimir Varbanov <svarbanov@suse.de>
---
 .../bindings/hwmon/raspberrypi,rp1-adc.yaml   | 46 +++++++++++++++++++
 1 file changed, 46 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/hwmon/raspberrypi,rp1-adc.yaml

diff --git a/Documentation/devicetree/bindings/hwmon/raspberrypi,rp1-adc.yaml b/Documentation/devicetree/bindings/hwmon/raspberrypi,rp1-adc.yaml
new file mode 100644
index 000000000000..5266b253fd2b
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwmon/raspberrypi,rp1-adc.yaml
@@ -0,0 +1,46 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/hwmon/raspberrypi,rp1-adc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Rasberry Pi RP1 ADC device
+
+maintainers:
+  - Stanimir Varbanov <svarbanov@suse.de>
+
+description: |
+  The RP1 ADC is a five input successive-approximation ADC with 12-bit
+  resolution (ENOB 9.5-bit) at 500kSPS. It has four external inputs
+  and one internal temperature sensor.
+
+properties:
+  compatible:
+    const: raspberrypi,rp1-adc
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+  vref-supply:
+    description:
+      Reference voltage regulator 3.3V.
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - vref-supply
+
+additionalProperties: false
+
+examples:
+  - |
+    adc@400c8000 {
+      compatible = "raspberrypi,rp1-adc";
+      reg = <0x400c8000 0x4000>;
+      clocks = <&rp1_clocks 30>;
+      vref-supply = <&rp1_vdd_3v3>;
+    };
-- 
2.47.0

[PATCH 2/4] hwmon: adc: rp1: Add Raspberry Pi's RP1 ADC driver

[Stanimir Varbanov]

A five-input successive-approximation analogue-to-digital converter
with 12-bit (effective number of 9.5 bits) resolution at 500kSPS.
The ADC has four external inputs and one internal temperature sensor.

Signed-off-by: Phil Elwell <phil@raspberrypi.com>
Signed-off-by: Jonathan Bell <jonathan@raspberrypi.com>
Signed-off-by: Stanimir Varbanov <svarbanov@suse.de>
---
 drivers/hwmon/Kconfig   |  10 ++
 drivers/hwmon/Makefile  |   1 +
 drivers/hwmon/rp1-adc.c | 301 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 312 insertions(+)
 create mode 100644 drivers/hwmon/rp1-adc.c

diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 9d28fcf7cd2a..2cde8f83d1df 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -2629,6 +2629,16 @@ config SENSORS_INTEL_M10_BMC_HWMON
 	  sensors monitor various telemetry data of different components on the
 	  card, e.g. board temperature, FPGA core temperature/voltage/current.
 
+config SENSORS_RP1_ADC
+	tristate "RP1 ADC and temperature sensor driver"
+	depends on MISC_RP1
+	help
+	  The driver provides support for the hardware monitoring of voltage
+	  and temperature sensor of the Raspberry Pi RP1 peripheral chip.
+
+	  The RP1 ADC is used by Raspberry Pi 5 to monitor four analog inputs
+	  and one temperature sensor input.
+
 if ACPI
 
 comment "ACPI drivers"
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index cd8bc4752b4d..574e2c22636f 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -196,6 +196,7 @@ obj-$(CONFIG_SENSORS_PT5161L)	+= pt5161l.o
 obj-$(CONFIG_SENSORS_PWM_FAN)	+= pwm-fan.o
 obj-$(CONFIG_SENSORS_QNAP_MCU_HWMON)	+= qnap-mcu-hwmon.o
 obj-$(CONFIG_SENSORS_RASPBERRYPI_HWMON)	+= raspberrypi-hwmon.o
+obj-$(CONFIG_SENSORS_RP1_ADC)	+= rp1-adc.o
 obj-$(CONFIG_SENSORS_SBTSI)	+= sbtsi_temp.o
 obj-$(CONFIG_SENSORS_SBRMI)	+= sbrmi.o
 obj-$(CONFIG_SENSORS_SCH56XX_COMMON)+= sch56xx-common.o
diff --git a/drivers/hwmon/rp1-adc.c b/drivers/hwmon/rp1-adc.c
new file mode 100644
index 000000000000..2487d19c8ee9
--- /dev/null
+++ b/drivers/hwmon/rp1-adc.c
@@ -0,0 +1,301 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Driver for the RP1 ADC and temperature sensor
+ *
+ * Copyright (C) 2023 Raspberry Pi Ltd.
+ * Copyright (c) 2025, SUSE.
+ */
+
+#include <linux/clk.h>
+#include <linux/err.h>
+#include <linux/hwmon.h>
+#include <linux/io.h>
+#include <linux/iopoll.h>
+#include <linux/module.h>
+#include <linux/mod_devicetable.h>
+#include <linux/mutex.h>
+#include <linux/platform_device.h>
+#include <linux/pm.h>
+#include <linux/regulator/consumer.h>
+
+#define RP1_ADC_CS			0x00
+#define RP1_ADC_RESULT			0x04
+#define RP1_ADC_FCS			0x08
+#define RP1_ADC_FIFO			0x0c
+#define RP1_ADC_DIV			0x10
+#define RP1_ADC_INTR			0x14
+#define RP1_ADC_INTE			0x18
+#define RP1_ADC_INTF			0x1c
+#define RP1_ADC_INTS			0x20
+
+#define RP1_ADC_RWTYPE_SET		0x2000
+#define RP1_ADC_RWTYPE_CLR		0x3000
+
+#define RP1_ADC_CS_RROBIN_MASK		0x1f
+#define RP1_ADC_CS_RROBIN_SHIFT		16
+#define RP1_ADC_CS_AINSEL_MASK		0x7
+#define RP1_ADC_CS_AINSEL_SHIFT		12
+#define RP1_ADC_CS_ERR_STICKY		0x400
+#define RP1_ADC_CS_ERR			0x200
+#define RP1_ADC_CS_READY		0x100
+#define RP1_ADC_CS_START_MANY		0x8
+#define RP1_ADC_CS_START_ONCE		0x4
+#define RP1_ADC_CS_TS_EN		0x2
+#define RP1_ADC_CS_EN			0x1
+
+#define RP1_ADC_FCS_THRESH_MASK		0xf
+#define RP1_ADC_FCS_THRESH_SHIFT	24
+#define RP1_ADC_FCS_LEVEL_MASK		0xf
+#define RP1_ADC_FCS_LEVEL_SHIFT		16
+#define RP1_ADC_FCS_OVER		0x800
+#define RP1_ADC_FCS_UNDER		0x400
+#define RP1_ADC_FCS_FULL		0x200
+#define RP1_ADC_FCS_EMPTY		0x100
+#define RP1_ADC_FCS_DREQ_EN		0x8
+#define RP1_ADC_FCS_ERR			0x4
+#define RP1_ADC_FCS_SHIFR		0x2
+#define RP1_ADC_FCS_EN			0x1
+
+#define RP1_ADC_FIFO_ERR		0x8000
+#define RP1_ADC_FIFO_VAL_MASK		0xfff
+
+#define RP1_ADC_DIV_INT_MASK		0xffff
+#define RP1_ADC_DIV_INT_SHIFT		8
+#define RP1_ADC_DIV_FRAC_MASK		0xff
+#define RP1_ADC_DIV_FRAC_SHIFT		0
+
+#define RP1_ADC_TEMP_CHAN		4
+
+struct rp1_adc_data {
+	void __iomem		*base;
+	struct mutex		lock;
+	struct device		*hwmon;
+	int			vref_mv;
+	struct clk		*clk;
+	struct regulator	*reg;
+};
+
+static int rp1_adc_read(struct rp1_adc_data *rp1, int channel, long *val)
+{
+	u32 regval;
+	int ret;
+
+	writel(RP1_ADC_CS_AINSEL_MASK << RP1_ADC_CS_AINSEL_SHIFT,
+	       rp1->base + RP1_ADC_RWTYPE_CLR + RP1_ADC_CS);
+	writel(channel << RP1_ADC_CS_AINSEL_SHIFT,
+	       rp1->base + RP1_ADC_RWTYPE_SET + RP1_ADC_CS);
+	writel(RP1_ADC_CS_START_ONCE,
+	       rp1->base + RP1_ADC_RWTYPE_SET + RP1_ADC_CS);
+
+	ret = readl_poll_timeout(rp1->base + RP1_ADC_CS, regval,
+				 regval & RP1_ADC_CS_READY, 10, 1000);
+	if (ret)
+		return ret;
+
+	/* Asserted if the completed conversion had a convergence error */
+	if (readl(rp1->base + RP1_ADC_CS) & RP1_ADC_CS_ERR)
+		return -EIO;
+
+	*val = readl(rp1->base + RP1_ADC_RESULT);
+
+	return 0;
+}
+
+static int rp1_adc_read_temp(struct rp1_adc_data *rp1, long *val)
+{
+	int ret, mv;
+
+	writel(RP1_ADC_CS_TS_EN, rp1->base + RP1_ADC_RWTYPE_SET + RP1_ADC_CS);
+
+	ret = rp1_adc_read(rp1, RP1_ADC_TEMP_CHAN, val);
+	if (ret)
+		return ret;
+
+	mv = ((u64)rp1->vref_mv * *val) / 4095;
+
+	/* T = 27 - (ADC_voltage - 0.706)/0.001721 */
+	*val = 27000 - DIV_ROUND_CLOSEST((mv - 706) * (s64)1000000, 1721);
+
+	return 0;
+}
+
+static umode_t rp1_adc_hwmon_is_visible(const void *drvdata,
+					enum hwmon_sensor_types type,
+					u32 attr, int channel)
+{
+	return 0444;
+}
+
+static int rp1_adc_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
+			      u32 attr, int channel, long *val)
+{
+	struct rp1_adc_data *rp1 = dev_get_drvdata(dev);
+	int ret = -EOPNOTSUPP;
+
+	mutex_lock(&rp1->lock);
+
+	if (type == hwmon_temp && attr == hwmon_temp_input)
+		ret = rp1_adc_read_temp(rp1, val);
+	else if (type == hwmon_in && attr == hwmon_in_input)
+		ret = rp1_adc_read(rp1, channel, val);
+
+	mutex_unlock(&rp1->lock);
+
+	return ret;
+}
+
+static const char *const rp1_adc_hwmon_in_labels[] = {
+	"ain0", "ain1", "ain2", "ain3",
+};
+
+static const char *const rp1_adc_hwmon_temp_label = {
+	"RP1 die temp"
+};
+
+static int rp1_adc_hwmon_read_string(struct device *dev,
+				     enum hwmon_sensor_types type, u32 attr,
+				     int channel, const char **str)
+{
+	if (type == hwmon_temp && attr == hwmon_temp_label) {
+		*str = rp1_adc_hwmon_temp_label;
+	} else if (type == hwmon_in && attr == hwmon_in_label) {
+		if (channel < ARRAY_SIZE(rp1_adc_hwmon_in_labels))
+			*str = rp1_adc_hwmon_in_labels[channel];
+	} else {
+		return -EOPNOTSUPP;
+	}
+
+	return 0;
+}
+
+static const struct hwmon_ops rp1_adc_hwmon_ops = {
+	.is_visible	= rp1_adc_hwmon_is_visible,
+	.read		= rp1_adc_hwmon_read,
+	.read_string	= rp1_adc_hwmon_read_string,
+};
+
+static const struct hwmon_channel_info * const rp1_adc_hwmon_info[] = {
+	HWMON_CHANNEL_INFO(in, HWMON_I_INPUT | HWMON_I_LABEL,
+			       HWMON_I_INPUT | HWMON_I_LABEL,
+			       HWMON_I_INPUT | HWMON_I_LABEL,
+			       HWMON_I_INPUT | HWMON_I_LABEL),
+	HWMON_CHANNEL_INFO(temp, HWMON_T_INPUT | HWMON_T_LABEL),
+	NULL
+};
+
+static const struct hwmon_chip_info rp1_adc_chip_info = {
+	.ops	= &rp1_adc_hwmon_ops,
+	.info	= rp1_adc_hwmon_info,
+};
+
+static int rp1_adc_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct rp1_adc_data *rp1;
+	int ret;
+
+	rp1 = devm_kzalloc(dev, sizeof(*rp1), GFP_KERNEL);
+	if (!rp1)
+		return -ENOMEM;
+
+	rp1->base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(rp1->base))
+		return dev_err_probe(dev, PTR_ERR(rp1->base), "can't ioremap resource\n");
+
+	rp1->clk = devm_clk_get(dev, NULL);
+	if (IS_ERR(rp1->clk))
+		return dev_err_probe(dev, PTR_ERR(rp1->clk), "can't get clock\n");
+
+	rp1->reg = devm_regulator_get(dev, "vref");
+	if (IS_ERR(rp1->reg))
+		return dev_err_probe(dev, PTR_ERR(rp1->reg), "can't get regulator\n");
+
+	ret = devm_mutex_init(dev, &rp1->lock);
+	if (ret)
+		return ret;
+
+	ret = clk_set_rate(rp1->clk, 50000000);
+	if (ret)
+		return ret;
+
+	ret = clk_prepare_enable(rp1->clk);
+	if (ret)
+		return ret;
+
+	platform_set_drvdata(pdev, rp1);
+
+	ret = regulator_get_voltage(rp1->reg);
+	if (ret < 0)
+		return ret;
+
+	rp1->vref_mv = DIV_ROUND_CLOSEST(ret, 1000);
+
+	ret = regulator_enable(rp1->reg);
+	if (ret)
+		return ret;
+
+	rp1->hwmon = devm_hwmon_device_register_with_info(dev, "rp1_adc", rp1,
+							  &rp1_adc_chip_info, NULL);
+	if (IS_ERR(rp1->hwmon))
+		return PTR_ERR(rp1->hwmon);
+
+	/* Disable interrupts */
+	writel(0, rp1->base + RP1_ADC_INTE);
+
+	/* Enable the block, clearing any sticky error */
+	writel(RP1_ADC_CS_EN | RP1_ADC_CS_ERR_STICKY, rp1->base + RP1_ADC_CS);
+
+	return 0;
+}
+
+static void rp1_adc_remove(struct platform_device *pdev)
+{
+	struct rp1_adc_data *rp1 = platform_get_drvdata(pdev);
+
+	clk_disable_unprepare(rp1->clk);
+	regulator_disable(rp1->reg);
+}
+
+static int rp1_adc_suspend(struct device *dev)
+{
+	struct rp1_adc_data *rp1 = dev_get_drvdata(dev);
+
+	clk_disable_unprepare(rp1->clk);
+	return regulator_disable(rp1->reg);
+}
+
+static int rp1_adc_resume(struct device *dev)
+{
+	struct rp1_adc_data *rp1 = dev_get_drvdata(dev);
+	int ret;
+
+	ret = regulator_enable(rp1->reg);
+	if (ret)
+		return ret;
+
+	return clk_prepare_enable(rp1->clk);
+}
+
+static DEFINE_SIMPLE_DEV_PM_OPS(rp1_adc_pm_ops, rp1_adc_suspend, rp1_adc_resume);
+
+static const struct of_device_id rp1_adc_match_table[] = {
+	{ .compatible = "raspberrypi,rp1-adc", },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, rp1_adc_match_table);
+
+static struct platform_driver rp1_adc_driver = {
+	.probe = rp1_adc_probe,
+	.remove = rp1_adc_remove,
+	.driver = {
+		.name = "rp1-adc",
+		.of_match_table = rp1_adc_match_table,
+		.pm = pm_ptr(&rp1_adc_pm_ops),
+	},
+};
+module_platform_driver(rp1_adc_driver);
+
+MODULE_DESCRIPTION("RP1 ADC driver");
+MODULE_AUTHOR("Phil Elwell <phil@raspberrypi.com>");
+MODULE_AUTHOR("Stanimir Varbanov <svarbanov@suse.de>");
+MODULE_LICENSE("GPL");
-- 
2.47.0

[PATCH 3/4] arm64: dts: rp1: Add Raspberry Pi's RP1 ADC DT node

[Stanimir Varbanov]

Add Raspberry Pi's RP1 ADC DT controller node.

Signed-off-by: Stanimir Varbanov <svarbanov@suse.de>
---
 arch/arm64/boot/dts/broadcom/rp1-common.dtsi | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/arm64/boot/dts/broadcom/rp1-common.dtsi b/arch/arm64/boot/dts/broadcom/rp1-common.dtsi
index 6bdc304c5f24..a0b06d7ce9f2 100644
--- a/arch/arm64/boot/dts/broadcom/rp1-common.dtsi
+++ b/arch/arm64/boot/dts/broadcom/rp1-common.dtsi
@@ -40,6 +40,13 @@ rp1_gpio: pinctrl@400d0000 {
 			     <2 IRQ_TYPE_LEVEL_HIGH>;
 	};
 
+	rp1_adc: adc@400c8000 {
+		compatible = "raspberrypi,rp1-adc";
+		reg = <0x00 0x400c8000 0x0 0x4000>;
+		clocks = <&rp1_clocks RP1_CLK_ADC>;
+		status = "disabled";
+	};
+
 	rp1_eth: ethernet@40100000 {
 		compatible = "raspberrypi,rp1-gem";
 		reg = <0x00 0x40100000  0x0 0x4000>;
-- 
2.47.0

[PATCH 4/4] arm64: dts: broadcom: Enable RP1 ADC for RPi5

[Stanimir Varbanov]

Enable RP1 ADC for Raspberry Pi 5.

Signed-off-by: Stanimir Varbanov <svarbanov@suse.de>
---
 arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b-ovl-rp1.dts | 8 ++++++++
 arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dts         | 5 +++++
 2 files changed, 13 insertions(+)

diff --git a/arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b-ovl-rp1.dts b/arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b-ovl-rp1.dts
index 6ea3c102e0d6..7943ec3fd6ea 100644
--- a/arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b-ovl-rp1.dts
+++ b/arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b-ovl-rp1.dts
@@ -51,6 +51,14 @@ sd_vcc_reg: sd-vcc-reg {
 		enable-active-high;
 		gpios = <&gio_aon 4 GPIO_ACTIVE_HIGH>;
 	};
+
+	rp1_vdd_3v3: rp1_vdd_3v3 {
+		compatible = "regulator-fixed";
+		regulator-name = "vdd-3v3";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+		regulator-always-on;
+	};
 };
 
 /* The Debug UART, on Rpi5 it's on JST-SH 1.0mm 3-pin connector
diff --git a/arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dts b/arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dts
index c70d1cb7f3b6..cb75792b659a 100644
--- a/arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dts
+++ b/arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dts
@@ -24,6 +24,11 @@ &pcie2 {
 	status = "okay";
 };
 
+&rp1_adc {
+	vref-supply = <&rp1_vdd_3v3>;
+	status = "okay";
+};
+
 &rp1_eth {
 	status = "okay";
 	phy-mode = "rgmii-id";
-- 
2.47.0

Re: [PATCH 2/4] hwmon: adc: rp1: Add Raspberry Pi's RP1 ADC driver

[Guenter Roeck]

On 9/24/25 17:04, Stanimir Varbanov wrote:
> A five-input successive-approximation analogue-to-digital converter
> with 12-bit (effective number of 9.5 bits) resolution at 500kSPS.
> The ADC has four external inputs and one internal temperature sensor.
> 
> Signed-off-by: Phil Elwell <phil@raspberrypi.com>
> Signed-off-by: Jonathan Bell <jonathan@raspberrypi.com>
> Signed-off-by: Stanimir Varbanov <svarbanov@suse.de>
> ---
>   drivers/hwmon/Kconfig   |  10 ++
>   drivers/hwmon/Makefile  |   1 +
>   drivers/hwmon/rp1-adc.c | 301 ++++++++++++++++++++++++++++++++++++++++
>   3 files changed, 312 insertions(+)
>   create mode 100644 drivers/hwmon/rp1-adc.c
> 
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 9d28fcf7cd2a..2cde8f83d1df 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -2629,6 +2629,16 @@ config SENSORS_INTEL_M10_BMC_HWMON
>   	  sensors monitor various telemetry data of different components on the
>   	  card, e.g. board temperature, FPGA core temperature/voltage/current.
>   
> +config SENSORS_RP1_ADC
> +	tristate "RP1 ADC and temperature sensor driver"
> +	depends on MISC_RP1
> +	help
> +	  The driver provides support for the hardware monitoring of voltage
> +	  and temperature sensor of the Raspberry Pi RP1 peripheral chip.
> +
> +	  The RP1 ADC is used by Raspberry Pi 5 to monitor four analog inputs
> +	  and one temperature sensor input.
> +
>   if ACPI
>   
>   comment "ACPI drivers"
> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> index cd8bc4752b4d..574e2c22636f 100644
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -196,6 +196,7 @@ obj-$(CONFIG_SENSORS_PT5161L)	+= pt5161l.o
>   obj-$(CONFIG_SENSORS_PWM_FAN)	+= pwm-fan.o
>   obj-$(CONFIG_SENSORS_QNAP_MCU_HWMON)	+= qnap-mcu-hwmon.o
>   obj-$(CONFIG_SENSORS_RASPBERRYPI_HWMON)	+= raspberrypi-hwmon.o
> +obj-$(CONFIG_SENSORS_RP1_ADC)	+= rp1-adc.o
>   obj-$(CONFIG_SENSORS_SBTSI)	+= sbtsi_temp.o
>   obj-$(CONFIG_SENSORS_SBRMI)	+= sbrmi.o
>   obj-$(CONFIG_SENSORS_SCH56XX_COMMON)+= sch56xx-common.o
> diff --git a/drivers/hwmon/rp1-adc.c b/drivers/hwmon/rp1-adc.c
> new file mode 100644
> index 000000000000..2487d19c8ee9
> --- /dev/null
> +++ b/drivers/hwmon/rp1-adc.c
> @@ -0,0 +1,301 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Driver for the RP1 ADC and temperature sensor
> + *
> + * Copyright (C) 2023 Raspberry Pi Ltd.
> + * Copyright (c) 2025, SUSE.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/err.h>
> +#include <linux/hwmon.h>
> +#include <linux/io.h>
> +#include <linux/iopoll.h>
> +#include <linux/module.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/mutex.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm.h>
> +#include <linux/regulator/consumer.h>
> +
> +#define RP1_ADC_CS			0x00
> +#define RP1_ADC_RESULT			0x04
> +#define RP1_ADC_FCS			0x08
> +#define RP1_ADC_FIFO			0x0c
> +#define RP1_ADC_DIV			0x10
> +#define RP1_ADC_INTR			0x14
> +#define RP1_ADC_INTE			0x18
> +#define RP1_ADC_INTF			0x1c
> +#define RP1_ADC_INTS			0x20
> +
> +#define RP1_ADC_RWTYPE_SET		0x2000
> +#define RP1_ADC_RWTYPE_CLR		0x3000
> +
> +#define RP1_ADC_CS_RROBIN_MASK		0x1f
> +#define RP1_ADC_CS_RROBIN_SHIFT		16
> +#define RP1_ADC_CS_AINSEL_MASK		0x7
> +#define RP1_ADC_CS_AINSEL_SHIFT		12
> +#define RP1_ADC_CS_ERR_STICKY		0x400
> +#define RP1_ADC_CS_ERR			0x200
> +#define RP1_ADC_CS_READY		0x100
> +#define RP1_ADC_CS_START_MANY		0x8
> +#define RP1_ADC_CS_START_ONCE		0x4
> +#define RP1_ADC_CS_TS_EN		0x2
> +#define RP1_ADC_CS_EN			0x1
> +
> +#define RP1_ADC_FCS_THRESH_MASK		0xf
> +#define RP1_ADC_FCS_THRESH_SHIFT	24
> +#define RP1_ADC_FCS_LEVEL_MASK		0xf
> +#define RP1_ADC_FCS_LEVEL_SHIFT		16
> +#define RP1_ADC_FCS_OVER		0x800
> +#define RP1_ADC_FCS_UNDER		0x400
> +#define RP1_ADC_FCS_FULL		0x200
> +#define RP1_ADC_FCS_EMPTY		0x100
> +#define RP1_ADC_FCS_DREQ_EN		0x8
> +#define RP1_ADC_FCS_ERR			0x4
> +#define RP1_ADC_FCS_SHIFR		0x2
> +#define RP1_ADC_FCS_EN			0x1
> +
> +#define RP1_ADC_FIFO_ERR		0x8000
> +#define RP1_ADC_FIFO_VAL_MASK		0xfff
> +
> +#define RP1_ADC_DIV_INT_MASK		0xffff
> +#define RP1_ADC_DIV_INT_SHIFT		8
> +#define RP1_ADC_DIV_FRAC_MASK		0xff
> +#define RP1_ADC_DIV_FRAC_SHIFT		0
> +
> +#define RP1_ADC_TEMP_CHAN		4
> +
> +struct rp1_adc_data {
> +	void __iomem		*base;
> +	struct mutex		lock;
> +	struct device		*hwmon;
> +	int			vref_mv;
> +	struct clk		*clk;
> +	struct regulator	*reg;
> +};
> +
> +static int rp1_adc_read(struct rp1_adc_data *rp1, int channel, long *val)
> +{
> +	u32 regval;
> +	int ret;
> +
> +	writel(RP1_ADC_CS_AINSEL_MASK << RP1_ADC_CS_AINSEL_SHIFT,
> +	       rp1->base + RP1_ADC_RWTYPE_CLR + RP1_ADC_CS);
> +	writel(channel << RP1_ADC_CS_AINSEL_SHIFT,
> +	       rp1->base + RP1_ADC_RWTYPE_SET + RP1_ADC_CS);
> +	writel(RP1_ADC_CS_START_ONCE,
> +	       rp1->base + RP1_ADC_RWTYPE_SET + RP1_ADC_CS);
> +
> +	ret = readl_poll_timeout(rp1->base + RP1_ADC_CS, regval,
> +				 regval & RP1_ADC_CS_READY, 10, 1000);
> +	if (ret)
> +		return ret;
> +
> +	/* Asserted if the completed conversion had a convergence error */
> +	if (readl(rp1->base + RP1_ADC_CS) & RP1_ADC_CS_ERR)
> +		return -EIO;
> +
> +	*val = readl(rp1->base + RP1_ADC_RESULT);
> +
> +	return 0;
> +}
> +
> +static int rp1_adc_read_temp(struct rp1_adc_data *rp1, long *val)
> +{
> +	int ret, mv;
> +
> +	writel(RP1_ADC_CS_TS_EN, rp1->base + RP1_ADC_RWTYPE_SET + RP1_ADC_CS);
> +
> +	ret = rp1_adc_read(rp1, RP1_ADC_TEMP_CHAN, val);
> +	if (ret)
> +		return ret;
> +
> +	mv = ((u64)rp1->vref_mv * *val) / 4095;

This ends up being a 64-bit divide operation. Does this work if build for arm
(32 bit) ?

> +
> +	/* T = 27 - (ADC_voltage - 0.706)/0.001721 */
> +	*val = 27000 - DIV_ROUND_CLOSEST((mv - 706) * (s64)1000000, 1721);
> +
> +	return 0;
> +}
> +
> +static umode_t rp1_adc_hwmon_is_visible(const void *drvdata,
> +					enum hwmon_sensor_types type,
> +					u32 attr, int channel)
> +{
> +	return 0444;
> +}
> +
> +static int rp1_adc_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
> +			      u32 attr, int channel, long *val)
> +{
> +	struct rp1_adc_data *rp1 = dev_get_drvdata(dev);
> +	int ret = -EOPNOTSUPP;
> +
> +	mutex_lock(&rp1->lock);
> +

The lock is no longer needed. See commit 3ad2a7b9b15d ("hwmon: Serialize accesses
in hwmon core") in linux-next.

> +	if (type == hwmon_temp && attr == hwmon_temp_input)
> +		ret = rp1_adc_read_temp(rp1, val);
> +	else if (type == hwmon_in && attr == hwmon_in_input)
> +		ret = rp1_adc_read(rp1, channel, val);
> +
> +	mutex_unlock(&rp1->lock);
> +
> +	return ret;
> +}
> +
> +static const char *const rp1_adc_hwmon_in_labels[] = {
> +	"ain0", "ain1", "ain2", "ain3",
> +};
> +
> +static const char *const rp1_adc_hwmon_temp_label = {
> +	"RP1 die temp"
> +};
> +
> +static int rp1_adc_hwmon_read_string(struct device *dev,
> +				     enum hwmon_sensor_types type, u32 attr,
> +				     int channel, const char **str)
> +{
> +	if (type == hwmon_temp && attr == hwmon_temp_label) {
> +		*str = rp1_adc_hwmon_temp_label;
> +	} else if (type == hwmon_in && attr == hwmon_in_label) {
> +		if (channel < ARRAY_SIZE(rp1_adc_hwmon_in_labels))
> +			*str = rp1_adc_hwmon_in_labels[channel];
> +	} else {
> +		return -EOPNOTSUPP;
> +	}
> +
> +	return 0;
> +}
> +
> +static const struct hwmon_ops rp1_adc_hwmon_ops = {
> +	.is_visible	= rp1_adc_hwmon_is_visible,
> +	.read		= rp1_adc_hwmon_read,
> +	.read_string	= rp1_adc_hwmon_read_string,
> +};
> +
> +static const struct hwmon_channel_info * const rp1_adc_hwmon_info[] = {
> +	HWMON_CHANNEL_INFO(in, HWMON_I_INPUT | HWMON_I_LABEL,
> +			       HWMON_I_INPUT | HWMON_I_LABEL,
> +			       HWMON_I_INPUT | HWMON_I_LABEL,
> +			       HWMON_I_INPUT | HWMON_I_LABEL),
> +	HWMON_CHANNEL_INFO(temp, HWMON_T_INPUT | HWMON_T_LABEL),
> +	NULL
> +};
> +
> +static const struct hwmon_chip_info rp1_adc_chip_info = {
> +	.ops	= &rp1_adc_hwmon_ops,
> +	.info	= rp1_adc_hwmon_info,
> +};
> +
> +static int rp1_adc_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct rp1_adc_data *rp1;
> +	int ret;
> +
> +	rp1 = devm_kzalloc(dev, sizeof(*rp1), GFP_KERNEL);
> +	if (!rp1)
> +		return -ENOMEM;
> +
> +	rp1->base = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(rp1->base))
> +		return dev_err_probe(dev, PTR_ERR(rp1->base), "can't ioremap resource\n");
> +
> +	rp1->clk = devm_clk_get(dev, NULL);
> +	if (IS_ERR(rp1->clk))
> +		return dev_err_probe(dev, PTR_ERR(rp1->clk), "can't get clock\n");
> +
> +	rp1->reg = devm_regulator_get(dev, "vref");
> +	if (IS_ERR(rp1->reg))
> +		return dev_err_probe(dev, PTR_ERR(rp1->reg), "can't get regulator\n");
> +
> +	ret = devm_mutex_init(dev, &rp1->lock);
> +	if (ret)
> +		return ret;
> +
> +	ret = clk_set_rate(rp1->clk, 50000000);
> +	if (ret)
> +		return ret;
> +
> +	ret = clk_prepare_enable(rp1->clk);
> +	if (ret)
> +		return ret;
> +

Other drivers call devm_clk_get_enabled() and then set the clock rate.
Doing this here would avoid having to call clk_disable_unprepare()
in the remove function.

> +	platform_set_drvdata(pdev, rp1);
> +
> +	ret = regulator_get_voltage(rp1->reg);
> +	if (ret < 0)
> +		return ret;
> +
> +	rp1->vref_mv = DIV_ROUND_CLOSEST(ret, 1000);
> +
> +	ret = regulator_enable(rp1->reg);

devm_regulator_get_enable() ?

> +	if (ret)
> +		return ret;
> +
> +	rp1->hwmon = devm_hwmon_device_register_with_info(dev, "rp1_adc", rp1,
> +							  &rp1_adc_chip_info, NULL);
> +	if (IS_ERR(rp1->hwmon))
> +		return PTR_ERR(rp1->hwmon);
> +
> +	/* Disable interrupts */
> +	writel(0, rp1->base + RP1_ADC_INTE);
> +
> +	/* Enable the block, clearing any sticky error */
> +	writel(RP1_ADC_CS_EN | RP1_ADC_CS_ERR_STICKY, rp1->base + RP1_ADC_CS);
> +
> +	return 0;
> +}
> +
> +static void rp1_adc_remove(struct platform_device *pdev)
> +{
> +	struct rp1_adc_data *rp1 = platform_get_drvdata(pdev);
> +
> +	clk_disable_unprepare(rp1->clk);
> +	regulator_disable(rp1->reg);

As written, this disables the clock and regulator before the hwmon device is
removed. Either make sure to use devm functions for clock and regulator, or
you can not use devm_hwmon_device_register_with_info() but must remove the
hwmon device here.

> +}
> +
> +static int rp1_adc_suspend(struct device *dev)
> +{
> +	struct rp1_adc_data *rp1 = dev_get_drvdata(dev);
> +
> +	clk_disable_unprepare(rp1->clk);
> +	return regulator_disable(rp1->reg);
> +}
> +
> +static int rp1_adc_resume(struct device *dev)
> +{
> +	struct rp1_adc_data *rp1 = dev_get_drvdata(dev);
> +	int ret;
> +
> +	ret = regulator_enable(rp1->reg);
> +	if (ret)
> +		return ret;
> +
> +	return clk_prepare_enable(rp1->clk);
> +}
> +
> +static DEFINE_SIMPLE_DEV_PM_OPS(rp1_adc_pm_ops, rp1_adc_suspend, rp1_adc_resume);
> +
> +static const struct of_device_id rp1_adc_match_table[] = {
> +	{ .compatible = "raspberrypi,rp1-adc", },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, rp1_adc_match_table);
> +
> +static struct platform_driver rp1_adc_driver = {
> +	.probe = rp1_adc_probe,
> +	.remove = rp1_adc_remove,
> +	.driver = {
> +		.name = "rp1-adc",
> +		.of_match_table = rp1_adc_match_table,
> +		.pm = pm_ptr(&rp1_adc_pm_ops),
> +	},
> +};
> +module_platform_driver(rp1_adc_driver);
> +
> +MODULE_DESCRIPTION("RP1 ADC driver");
> +MODULE_AUTHOR("Phil Elwell <phil@raspberrypi.com>");
> +MODULE_AUTHOR("Stanimir Varbanov <svarbanov@suse.de>");
> +MODULE_LICENSE("GPL");

Re: [PATCH 2/4] hwmon: adc: rp1: Add Raspberry Pi's RP1 ADC driver

[Guenter Roeck]

On Thu, Sep 25, 2025 at 03:04:14AM +0300, Stanimir Varbanov wrote:
> A five-input successive-approximation analogue-to-digital converter
> with 12-bit (effective number of 9.5 bits) resolution at 500kSPS.
> The ADC has four external inputs and one internal temperature sensor.
> 
> Signed-off-by: Phil Elwell <phil@raspberrypi.com>
> Signed-off-by: Jonathan Bell <jonathan@raspberrypi.com>
> Signed-off-by: Stanimir Varbanov <svarbanov@suse.de>

I just realized that there is already a hwmon driver for
Rasperri Pi - drivers/hwmon/raspberrypi-hwmon.c.

Please add this code to that driver.

> ---
>  drivers/hwmon/Kconfig   |  10 ++
>  drivers/hwmon/Makefile  |   1 +
>  drivers/hwmon/rp1-adc.c | 301 ++++++++++++++++++++++++++++++++++++++++

Also, this needs documentation.

Thanks,
Guenter

Re: [PATCH 2/4] hwmon: adc: rp1: Add Raspberry Pi's RP1 ADC driver

[Stefan Wahren]

Hi Guenter,

Am 25.09.25 um 04:42 schrieb Guenter Roeck:
> On Thu, Sep 25, 2025 at 03:04:14AM +0300, Stanimir Varbanov wrote:
>> A five-input successive-approximation analogue-to-digital converter
>> with 12-bit (effective number of 9.5 bits) resolution at 500kSPS.
>> The ADC has four external inputs and one internal temperature sensor.
>>
>> Signed-off-by: Phil Elwell <phil@raspberrypi.com>
>> Signed-off-by: Jonathan Bell <jonathan@raspberrypi.com>
>> Signed-off-by: Stanimir Varbanov <svarbanov@suse.de>
> I just realized that there is already a hwmon driver for
> Rasperri Pi - drivers/hwmon/raspberrypi-hwmon.c.
>
> Please add this code to that driver.
could you please explain the reason for this?

Yes, both drivers are for Raspberry Pi boards, but they don't share any 
code base. The raspberrypi-hwmon uses a mailbox interfaces to get the 
sensor data and works for the board generation 1 - 4. This driver works 
completely differently ( MMIO ), doesn't depend on the mailbox interface 
and applies only for board generation 5. Actually I don't see a benefit 
of merging them.

Best regards
>
>> ---
>>   drivers/hwmon/Kconfig   |  10 ++
>>   drivers/hwmon/Makefile  |   1 +
>>   drivers/hwmon/rp1-adc.c | 301 ++++++++++++++++++++++++++++++++++++++++
> Also, this needs documentation.
>
> Thanks,
> Guenter

Re: [PATCH 2/4] hwmon: adc: rp1: Add Raspberry Pi's RP1 ADC driver

[Guenter Roeck]

On Thu, Sep 25, 2025 at 07:26:10AM +0200, Stefan Wahren wrote:
> Hi Guenter,
> 
> Am 25.09.25 um 04:42 schrieb Guenter Roeck:
> > On Thu, Sep 25, 2025 at 03:04:14AM +0300, Stanimir Varbanov wrote:
> > > A five-input successive-approximation analogue-to-digital converter
> > > with 12-bit (effective number of 9.5 bits) resolution at 500kSPS.
> > > The ADC has four external inputs and one internal temperature sensor.
> > > 
> > > Signed-off-by: Phil Elwell <phil@raspberrypi.com>
> > > Signed-off-by: Jonathan Bell <jonathan@raspberrypi.com>
> > > Signed-off-by: Stanimir Varbanov <svarbanov@suse.de>
> > I just realized that there is already a hwmon driver for
> > Rasperri Pi - drivers/hwmon/raspberrypi-hwmon.c.
> > 
> > Please add this code to that driver.
> could you please explain the reason for this?
> 
> Yes, both drivers are for Raspberry Pi boards, but they don't share any code
> base. The raspberrypi-hwmon uses a mailbox interfaces to get the sensor data
> and works for the board generation 1 - 4. This driver works completely
> differently ( MMIO ), doesn't depend on the mailbox interface and applies
> only for board generation 5. Actually I don't see a benefit of merging them.
> 
Ok. Please make sure to add this explanation to the patch description.

Guenter

Re: [PATCH 1/4] dt-bindings: Add Raspberry Pi's RP1 ADC

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1986 bytes --]

On Thu, Sep 25, 2025 at 03:04:13AM +0300, Stanimir Varbanov wrote:
> Document dt-bindings for Raspberry Pi's RP1 ADC.
> 
> Signed-off-by: Stanimir Varbanov <svarbanov@suse.de>
> ---
>  .../bindings/hwmon/raspberrypi,rp1-adc.yaml   | 46 +++++++++++++++++++
>  1 file changed, 46 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/hwmon/raspberrypi,rp1-adc.yaml
> 
> diff --git a/Documentation/devicetree/bindings/hwmon/raspberrypi,rp1-adc.yaml b/Documentation/devicetree/bindings/hwmon/raspberrypi,rp1-adc.yaml
> new file mode 100644
> index 000000000000..5266b253fd2b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/hwmon/raspberrypi,rp1-adc.yaml
> @@ -0,0 +1,46 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/hwmon/raspberrypi,rp1-adc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Rasberry Pi RP1 ADC device
> +
> +maintainers:
> +  - Stanimir Varbanov <svarbanov@suse.de>
> +
> +description: |
> +  The RP1 ADC is a five input successive-approximation ADC with 12-bit
> +  resolution (ENOB 9.5-bit) at 500kSPS. It has four external inputs
> +  and one internal temperature sensor.
> +
> +properties:
> +  compatible:
> +    const: raspberrypi,rp1-adc
> +
> +  reg:
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 1
> +
> +  vref-supply:
> +    description:
> +      Reference voltage regulator 3.3V.

Looks like you're missing the io-channels-cells property that allows
this device to be a provider of adc channels to other devices.

> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - vref-supply
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    adc@400c8000 {
> +      compatible = "raspberrypi,rp1-adc";
> +      reg = <0x400c8000 0x4000>;
> +      clocks = <&rp1_clocks 30>;
> +      vref-supply = <&rp1_vdd_3v3>;
> +    };
> -- 
> 2.47.0
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 1/4] dt-bindings: Add Raspberry Pi's RP1 ADC

[Guenter Roeck]

On Thu, Sep 25, 2025 at 08:40:54PM +0100, Conor Dooley wrote:
> On Thu, Sep 25, 2025 at 03:04:13AM +0300, Stanimir Varbanov wrote:
> > Document dt-bindings for Raspberry Pi's RP1 ADC.
> > 
> > Signed-off-by: Stanimir Varbanov <svarbanov@suse.de>
> > ---
> >  .../bindings/hwmon/raspberrypi,rp1-adc.yaml   | 46 +++++++++++++++++++
> >  1 file changed, 46 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/hwmon/raspberrypi,rp1-adc.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/hwmon/raspberrypi,rp1-adc.yaml b/Documentation/devicetree/bindings/hwmon/raspberrypi,rp1-adc.yaml
> > new file mode 100644
> > index 000000000000..5266b253fd2b
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/hwmon/raspberrypi,rp1-adc.yaml
> > @@ -0,0 +1,46 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/hwmon/raspberrypi,rp1-adc.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Rasberry Pi RP1 ADC device
> > +
> > +maintainers:
> > +  - Stanimir Varbanov <svarbanov@suse.de>
> > +
> > +description: |
> > +  The RP1 ADC is a five input successive-approximation ADC with 12-bit
> > +  resolution (ENOB 9.5-bit) at 500kSPS. It has four external inputs
> > +  and one internal temperature sensor.
> > +
> > +properties:
> > +  compatible:
> > +    const: raspberrypi,rp1-adc
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  clocks:
> > +    maxItems: 1
> > +
> > +  vref-supply:
> > +    description:
> > +      Reference voltage regulator 3.3V.
> 
> Looks like you're missing the io-channels-cells property that allows
> this device to be a provider of adc channels to other devices.
> 
Only makes sense if the driver is implemented as iio driver.
Which would be fine with me, assuming this is a generic ADC.
The iio -> hwmon bridge can then be used to instantiate a
hwmon device if needed.

Guenter

Re: [PATCH 2/4] hwmon: adc: rp1: Add Raspberry Pi's RP1 ADC driver

[Stanimir Varbanov]

Hi,

On 9/25/25 5:34 PM, Guenter Roeck wrote:
> On Thu, Sep 25, 2025 at 07:26:10AM +0200, Stefan Wahren wrote:
>> Hi Guenter,
>>
>> Am 25.09.25 um 04:42 schrieb Guenter Roeck:
>>> On Thu, Sep 25, 2025 at 03:04:14AM +0300, Stanimir Varbanov wrote:
>>>> A five-input successive-approximation analogue-to-digital converter
>>>> with 12-bit (effective number of 9.5 bits) resolution at 500kSPS.
>>>> The ADC has four external inputs and one internal temperature sensor.
>>>>
>>>> Signed-off-by: Phil Elwell <phil@raspberrypi.com>
>>>> Signed-off-by: Jonathan Bell <jonathan@raspberrypi.com>
>>>> Signed-off-by: Stanimir Varbanov <svarbanov@suse.de>
>>> I just realized that there is already a hwmon driver for
>>> Rasperri Pi - drivers/hwmon/raspberrypi-hwmon.c.
>>>
>>> Please add this code to that driver.
>> could you please explain the reason for this?
>>
>> Yes, both drivers are for Raspberry Pi boards, but they don't share any code
>> base. The raspberrypi-hwmon uses a mailbox interfaces to get the sensor data
>> and works for the board generation 1 - 4. This driver works completely
>> differently ( MMIO ), doesn't depend on the mailbox interface and applies
>> only for board generation 5. Actually I don't see a benefit of merging them.

Thank you Stefan for the explanation.

>>
> Ok. Please make sure to add this explanation to the patch description.

Sure, I will extend the description in next version.

~Stan

Re: [PATCH 1/4] dt-bindings: Add Raspberry Pi's RP1 ADC

[Stanimir Varbanov]

Hi,

On 9/25/25 11:37 PM, Guenter Roeck wrote:
> On Thu, Sep 25, 2025 at 08:40:54PM +0100, Conor Dooley wrote:
>> On Thu, Sep 25, 2025 at 03:04:13AM +0300, Stanimir Varbanov wrote:
>>> Document dt-bindings for Raspberry Pi's RP1 ADC.
>>>
>>> Signed-off-by: Stanimir Varbanov <svarbanov@suse.de>
>>> ---
>>>  .../bindings/hwmon/raspberrypi,rp1-adc.yaml   | 46 +++++++++++++++++++
>>>  1 file changed, 46 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/hwmon/raspberrypi,rp1-adc.yaml
>>>
>>> diff --git a/Documentation/devicetree/bindings/hwmon/raspberrypi,rp1-adc.yaml b/Documentation/devicetree/bindings/hwmon/raspberrypi,rp1-adc.yaml
>>> new file mode 100644
>>> index 000000000000..5266b253fd2b
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/hwmon/raspberrypi,rp1-adc.yaml
>>> @@ -0,0 +1,46 @@
>>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/hwmon/raspberrypi,rp1-adc.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: Rasberry Pi RP1 ADC device
>>> +
>>> +maintainers:
>>> +  - Stanimir Varbanov <svarbanov@suse.de>
>>> +
>>> +description: |
>>> +  The RP1 ADC is a five input successive-approximation ADC with 12-bit
>>> +  resolution (ENOB 9.5-bit) at 500kSPS. It has four external inputs
>>> +  and one internal temperature sensor.
>>> +
>>> +properties:
>>> +  compatible:
>>> +    const: raspberrypi,rp1-adc
>>> +
>>> +  reg:
>>> +    maxItems: 1
>>> +
>>> +  clocks:
>>> +    maxItems: 1
>>> +
>>> +  vref-supply:
>>> +    description:
>>> +      Reference voltage regulator 3.3V.
>>
>> Looks like you're missing the io-channels-cells property that allows
>> this device to be a provider of adc channels to other devices.
>>
> Only makes sense if the driver is implemented as iio driver.
> Which would be fine with me, assuming this is a generic ADC.
> The iio -> hwmon bridge can then be used to instantiate a
> hwmon device if needed.
> 

According to the RP1 peripheral document the information about ADC is
limited and I cannot be 100% sure that this is generic ADC, but it looks
like it is. On RPi5 board the ADC inputs are not configurable, but that
could change on another board.

I personally don't have objections to implement it as IIO driver.

Phil, are you fine with implementing the driver as IIO?

~Stan

Re: [PATCH 1/4] dt-bindings: Add Raspberry Pi's RP1 ADC

[Phil Elwell]

Hi Stanimir, Guenter,

On Tue, 30 Sept 2025 at 11:21, Stanimir Varbanov <svarbanov@suse.de> wrote:
>
> Hi,
>
> On 9/25/25 11:37 PM, Guenter Roeck wrote:
> > On Thu, Sep 25, 2025 at 08:40:54PM +0100, Conor Dooley wrote:
> >> On Thu, Sep 25, 2025 at 03:04:13AM +0300, Stanimir Varbanov wrote:
> >>> Document dt-bindings for Raspberry Pi's RP1 ADC.
> >>>
> >>> Signed-off-by: Stanimir Varbanov <svarbanov@suse.de>
> >>> ---
> >>>  .../bindings/hwmon/raspberrypi,rp1-adc.yaml   | 46 +++++++++++++++++++
> >>>  1 file changed, 46 insertions(+)
> >>>  create mode 100644 Documentation/devicetree/bindings/hwmon/raspberrypi,rp1-adc.yaml
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/hwmon/raspberrypi,rp1-adc.yaml b/Documentation/devicetree/bindings/hwmon/raspberrypi,rp1-adc.yaml
> >>> new file mode 100644
> >>> index 000000000000..5266b253fd2b
> >>> --- /dev/null
> >>> +++ b/Documentation/devicetree/bindings/hwmon/raspberrypi,rp1-adc.yaml
> >>> @@ -0,0 +1,46 @@
> >>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> >>> +%YAML 1.2
> >>> +---
> >>> +$id: http://devicetree.org/schemas/hwmon/raspberrypi,rp1-adc.yaml#
> >>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> >>> +
> >>> +title: Rasberry Pi RP1 ADC device
> >>> +
> >>> +maintainers:
> >>> +  - Stanimir Varbanov <svarbanov@suse.de>
> >>> +
> >>> +description: |
> >>> +  The RP1 ADC is a five input successive-approximation ADC with 12-bit
> >>> +  resolution (ENOB 9.5-bit) at 500kSPS. It has four external inputs
> >>> +  and one internal temperature sensor.
> >>> +
> >>> +properties:
> >>> +  compatible:
> >>> +    const: raspberrypi,rp1-adc
> >>> +
> >>> +  reg:
> >>> +    maxItems: 1
> >>> +
> >>> +  clocks:
> >>> +    maxItems: 1
> >>> +
> >>> +  vref-supply:
> >>> +    description:
> >>> +      Reference voltage regulator 3.3V.
> >>
> >> Looks like you're missing the io-channels-cells property that allows
> >> this device to be a provider of adc channels to other devices.
> >>
> > Only makes sense if the driver is implemented as iio driver.
> > Which would be fine with me, assuming this is a generic ADC.
> > The iio -> hwmon bridge can then be used to instantiate a
> > hwmon device if needed.
> >
>
> According to the RP1 peripheral document the information about ADC is
> limited and I cannot be 100% sure that this is generic ADC, but it looks
> like it is. On RPi5 board the ADC inputs are not configurable, but that
> could change on another board.
>
> I personally don't have objections to implement it as IIO driver.
>
> Phil, are you fine with implementing the driver as IIO?

The problem with adding unused functionality, apart from the effort
involved, is that testing it is harder. Will the IIO driver be
inherently better/simpler because some of the hwmon support gets
picked up by the generic IIO/HWMON bridge?

Ultimately we'll make whatever changes are considered necessary in
order to get this simple driver accepted, but it would be nice to feel
there was some real world benefit now for the work, not on Pi 6/7/etc.

Phil

Re: [PATCH 1/4] dt-bindings: Add Raspberry Pi's RP1 ADC

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 3561 bytes --]

On Tue, Sep 30, 2025 at 11:47:58AM +0100, Phil Elwell wrote:
> Hi Stanimir, Guenter,
> 
> On Tue, 30 Sept 2025 at 11:21, Stanimir Varbanov <svarbanov@suse.de> wrote:
> >
> > Hi,
> >
> > On 9/25/25 11:37 PM, Guenter Roeck wrote:
> > > On Thu, Sep 25, 2025 at 08:40:54PM +0100, Conor Dooley wrote:
> > >> On Thu, Sep 25, 2025 at 03:04:13AM +0300, Stanimir Varbanov wrote:
> > >>> Document dt-bindings for Raspberry Pi's RP1 ADC.
> > >>>
> > >>> Signed-off-by: Stanimir Varbanov <svarbanov@suse.de>
> > >>> ---
> > >>>  .../bindings/hwmon/raspberrypi,rp1-adc.yaml   | 46 +++++++++++++++++++
> > >>>  1 file changed, 46 insertions(+)
> > >>>  create mode 100644 Documentation/devicetree/bindings/hwmon/raspberrypi,rp1-adc.yaml
> > >>>
> > >>> diff --git a/Documentation/devicetree/bindings/hwmon/raspberrypi,rp1-adc.yaml b/Documentation/devicetree/bindings/hwmon/raspberrypi,rp1-adc.yaml
> > >>> new file mode 100644
> > >>> index 000000000000..5266b253fd2b
> > >>> --- /dev/null
> > >>> +++ b/Documentation/devicetree/bindings/hwmon/raspberrypi,rp1-adc.yaml
> > >>> @@ -0,0 +1,46 @@
> > >>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > >>> +%YAML 1.2
> > >>> +---
> > >>> +$id: http://devicetree.org/schemas/hwmon/raspberrypi,rp1-adc.yaml#
> > >>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > >>> +
> > >>> +title: Rasberry Pi RP1 ADC device
> > >>> +
> > >>> +maintainers:
> > >>> +  - Stanimir Varbanov <svarbanov@suse.de>
> > >>> +
> > >>> +description: |
> > >>> +  The RP1 ADC is a five input successive-approximation ADC with 12-bit
> > >>> +  resolution (ENOB 9.5-bit) at 500kSPS. It has four external inputs
> > >>> +  and one internal temperature sensor.
> > >>> +
> > >>> +properties:
> > >>> +  compatible:
> > >>> +    const: raspberrypi,rp1-adc
> > >>> +
> > >>> +  reg:
> > >>> +    maxItems: 1
> > >>> +
> > >>> +  clocks:
> > >>> +    maxItems: 1
> > >>> +
> > >>> +  vref-supply:
> > >>> +    description:
> > >>> +      Reference voltage regulator 3.3V.
> > >>
> > >> Looks like you're missing the io-channels-cells property that allows
> > >> this device to be a provider of adc channels to other devices.
> > >>
> > > Only makes sense if the driver is implemented as iio driver.
> > > Which would be fine with me, assuming this is a generic ADC.
> > > The iio -> hwmon bridge can then be used to instantiate a
> > > hwmon device if needed.
> > >
> >
> > According to the RP1 peripheral document the information about ADC is
> > limited and I cannot be 100% sure that this is generic ADC, but it looks
> > like it is. On RPi5 board the ADC inputs are not configurable, but that
> > could change on another board.
> >
> > I personally don't have objections to implement it as IIO driver.
> >
> > Phil, are you fine with implementing the driver as IIO?
> 
> The problem with adding unused functionality, apart from the effort
> involved, is that testing it is harder. Will the IIO driver be
> inherently better/simpler because some of the hwmon support gets
> picked up by the generic IIO/HWMON bridge?
> 
> Ultimately we'll make whatever changes are considered necessary in
> order to get this simple driver accepted, but it would be nice to feel
> there was some real world benefit now for the work, not on Pi 6/7/etc.

tbh, I don't care how the driver side of things is done, I would just
like for the binding documentation to be complete. If there's no usecase
at all where the device provides io channels, then omit the property.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]