[PATCH 1/2] soc: bcm2835: raspberrypi-firmware: Add voltage domain IDs

[PATCH 1/2] soc: bcm2835: raspberrypi-firmware: Add voltage domain IDs

[Shubham Chakraborty]

Add firmware voltage domain identifiers for the Raspberry Pi
mailbox property interface.

These IDs are used by firmware clients to query voltage rails
through the RPI_FIRMWARE_GET_VOLTAGE property.

Signed-off-by: Shubham Chakraborty <chakrabortyshubham66@gmail.com>
---
 include/soc/bcm2835/raspberrypi-firmware.h | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/include/soc/bcm2835/raspberrypi-firmware.h b/include/soc/bcm2835/raspberrypi-firmware.h
index e1f87fbfe554..fd2e051ce05b 100644
--- a/include/soc/bcm2835/raspberrypi-firmware.h
+++ b/include/soc/bcm2835/raspberrypi-firmware.h
@@ -156,6 +156,14 @@ enum rpi_firmware_clk_id {
 	RPI_FIRMWARE_NUM_CLK_ID,
 };
 
+enum rpi_firmware_volt_id {
+	RPI_FIRMWARE_VOLT_ID_RESERVED = 0,
+	RPI_FIRMWARE_VOLT_ID_CORE = 1,
+	RPI_FIRMWARE_VOLT_ID_SDRAM_C = 2,
+	RPI_FIRMWARE_VOLT_ID_SDRAM_I = 3,
+	RPI_FIRMWARE_VOLT_ID_SDRAM_P = 4,
+};
+
 /**
  * struct rpi_firmware_clk_rate_request - Firmware Request for a rate
  * @id:	ID of the clock being queried
-- 
2.54.0

[PATCH 2/2] hwmon: raspberrypi: Add voltage input support

[Shubham Chakraborty]

Extend the raspberrypi-hwmon driver to expose firmware-provided
voltage measurements through the hwmon subsystem.

The driver now exports the following voltage inputs:

  - in0_input (core)
  - in1_input (sdram_c)
  - in2_input (sdram_i)
  - in3_input (sdram_p)

Voltage values returned by firmware are converted from microvolts
to millivolts as expected by the hwmon subsystem.

The existing undervoltage sticky alarm handling is preserved and
associated with the first voltage channel.

Tested in -
- Raspberry Pi 3b+ (Linux raspberrypi 6.12.75+rpt-rpi-v8 #1 SMP PREEMPT
  Debian 1:6.12.75-1+rpt1 (2026-03-11) aarch64 GNU/Linux)

Signed-off-by: Shubham Chakraborty <chakrabortyshubham66@gmail.com>
---
 drivers/hwmon/raspberrypi-hwmon.c | 112 +++++++++++++++++++++++++++++-
 1 file changed, 109 insertions(+), 3 deletions(-)

diff --git a/drivers/hwmon/raspberrypi-hwmon.c b/drivers/hwmon/raspberrypi-hwmon.c
index a2938881ccd2..c73a970db025 100644
--- a/drivers/hwmon/raspberrypi-hwmon.c
+++ b/drivers/hwmon/raspberrypi-hwmon.c
@@ -5,6 +5,7 @@
  * Based on firmware/raspberrypi.c by Noralf Trønnes
  *
  * Copyright (C) 2018 Stefan Wahren <stefan.wahren@i2se.com>
+ * Copyright (C) 2026 Shubham Chakraborty <chakrabortyshubham66@gmail.com>
  */
 #include <linux/device.h>
 #include <linux/devm-helpers.h>
@@ -18,6 +19,11 @@
 
 #define UNDERVOLTAGE_STICKY_BIT	BIT(16)
 
+struct rpi_firmware_get_value {
+	__le32 id;
+	__le32 val;
+} __packed;
+
 struct rpi_hwmon_data {
 	struct device *hwmon_dev;
 	struct rpi_firmware *fw;
@@ -56,6 +62,23 @@ static void rpi_firmware_get_throttled(struct rpi_hwmon_data *data)
 	hwmon_notify_event(data->hwmon_dev, hwmon_in, hwmon_in_lcrit_alarm, 0);
 }
 
+static int rpi_firmware_get_voltage(struct rpi_hwmon_data *data, u32 id,
+				    long *val)
+{
+	struct rpi_firmware_get_value packet;
+	int ret;
+
+	packet.id = cpu_to_le32(id);
+	packet.val = 0;
+	ret = rpi_firmware_property(data->fw, RPI_FIRMWARE_GET_VOLTAGE,
+				    &packet, sizeof(packet));
+	if (ret)
+		return ret;
+
+	*val = le32_to_cpu(packet.val) / 1000;
+	return 0;
+}
+
 static void get_values_poll(struct work_struct *work)
 {
 	struct rpi_hwmon_data *data;
@@ -77,19 +100,101 @@ static int rpi_read(struct device *dev, enum hwmon_sensor_types type,
 {
 	struct rpi_hwmon_data *data = dev_get_drvdata(dev);
 
-	*val = !!(data->last_throttled & UNDERVOLTAGE_STICKY_BIT);
+	if (type == hwmon_in) {
+		switch (attr) {
+		case hwmon_in_input:
+			switch (channel) {
+			case 0:
+				return rpi_firmware_get_voltage(data,
+						RPI_FIRMWARE_VOLT_ID_CORE,
+						val);
+			case 1:
+				return rpi_firmware_get_voltage(data,
+						RPI_FIRMWARE_VOLT_ID_SDRAM_C,
+						val);
+			case 2:
+				return rpi_firmware_get_voltage(data,
+						RPI_FIRMWARE_VOLT_ID_SDRAM_I,
+						val);
+			case 3:
+				return rpi_firmware_get_voltage(data,
+						RPI_FIRMWARE_VOLT_ID_SDRAM_P,
+						val);
+			default:
+				return -EOPNOTSUPP;
+			}
+		case hwmon_in_lcrit_alarm:
+			if (channel == 0) {
+				*val = !!(data->last_throttled & UNDERVOLTAGE_STICKY_BIT);
+				return 0;
+			}
+			return -EOPNOTSUPP;
+		default:
+			return -EOPNOTSUPP;
+		}
+	}
+
+	return -EOPNOTSUPP;
+}
+
+static int rpi_read_string(struct device *dev, enum hwmon_sensor_types type,
+			   u32 attr, int channel, const char **str)
+{
+	if (type == hwmon_in && attr == hwmon_in_label) {
+		switch (channel) {
+		case 0:
+			*str = "core";
+			return 0;
+		case 1:
+			*str = "sdram_c";
+			return 0;
+		case 2:
+			*str = "sdram_i";
+			return 0;
+		case 3:
+			*str = "sdram_p";
+			return 0;
+		default:
+			return -EOPNOTSUPP;
+		}
+	}
+
+	return -EOPNOTSUPP;
+}
+
+static umode_t rpi_is_visible(const void *_data, enum hwmon_sensor_types type,
+			      u32 attr, int channel)
+{
+	if (type == hwmon_in) {
+		switch (attr) {
+		case hwmon_in_input:
+		case hwmon_in_label:
+			return 0444;
+		case hwmon_in_lcrit_alarm:
+			if (channel == 0)
+				return 0444;
+			return 0;
+		default:
+			return 0;
+		}
+	}
+
 	return 0;
 }
 
 static const struct hwmon_channel_info * const rpi_info[] = {
 	HWMON_CHANNEL_INFO(in,
-			   HWMON_I_LCRIT_ALARM),
+			   HWMON_I_INPUT | HWMON_I_LABEL | HWMON_I_LCRIT_ALARM,
+			   HWMON_I_INPUT | HWMON_I_LABEL,
+			   HWMON_I_INPUT | HWMON_I_LABEL,
+			   HWMON_I_INPUT | HWMON_I_LABEL),
 	NULL
 };
 
 static const struct hwmon_ops rpi_hwmon_ops = {
-	.visible = 0444,
+	.is_visible = rpi_is_visible,
 	.read = rpi_read,
+	.read_string = rpi_read_string,
 };
 
 static const struct hwmon_chip_info rpi_chip_info = {
@@ -159,6 +264,7 @@ static struct platform_driver rpi_hwmon_driver = {
 module_platform_driver(rpi_hwmon_driver);
 
 MODULE_AUTHOR("Stefan Wahren <wahrenst@gmx.net>");
+MODULE_AUTHOR("Shubham Chakraborty <chakrabortyshubham66@gmail.com>");
 MODULE_DESCRIPTION("Raspberry Pi voltage sensor driver");
 MODULE_LICENSE("GPL v2");
 MODULE_ALIAS("platform:raspberrypi-hwmon");
-- 
2.54.0

Re: [PATCH 2/2] hwmon: raspberrypi: Add voltage input support

[Guenter Roeck]

On 5/16/26 09:44, Shubham Chakraborty wrote:
> Extend the raspberrypi-hwmon driver to expose firmware-provided
> voltage measurements through the hwmon subsystem.
> 
> The driver now exports the following voltage inputs:
> 
>    - in0_input (core)
>    - in1_input (sdram_c)
>    - in2_input (sdram_i)
>    - in3_input (sdram_p)
> 
> Voltage values returned by firmware are converted from microvolts
> to millivolts as expected by the hwmon subsystem.
> 
> The existing undervoltage sticky alarm handling is preserved and
> associated with the first voltage channel.
> 
> Tested in -
> - Raspberry Pi 3b+ (Linux raspberrypi 6.12.75+rpt-rpi-v8 #1 SMP PREEMPT
>    Debian 1:6.12.75-1+rpt1 (2026-03-11) aarch64 GNU/Linux)
> 
> Signed-off-by: Shubham Chakraborty <chakrabortyshubham66@gmail.com>

I wasn't copied on patch 1/2, so I have no idea what is in there
and how it is related to this patch. Either way it seems unlikely that
the new functionality is supported by all versions of Raspberry Pi supported
by this driver. Just returning an error when the user tries to read a sensor
that is not supported is unacceptable. This needs either evidence that the
sensors are supported by all board variants and firmware versions supported
by this driver, or the is_visible function needs to selectively enable the
supported sensors.

> ---
>   drivers/hwmon/raspberrypi-hwmon.c | 112 +++++++++++++++++++++++++++++-

Documentation/hwmon/raspberrypi-hwmon.rst needs to be updated as well.

>   1 file changed, 109 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/hwmon/raspberrypi-hwmon.c b/drivers/hwmon/raspberrypi-hwmon.c
> index a2938881ccd2..c73a970db025 100644
> --- a/drivers/hwmon/raspberrypi-hwmon.c
> +++ b/drivers/hwmon/raspberrypi-hwmon.c
> @@ -5,6 +5,7 @@
>    * Based on firmware/raspberrypi.c by Noralf Trønnes
>    *
>    * Copyright (C) 2018 Stefan Wahren <stefan.wahren@i2se.com>
> + * Copyright (C) 2026 Shubham Chakraborty <chakrabortyshubham66@gmail.com>
>    */
>   #include <linux/device.h>
>   #include <linux/devm-helpers.h>
> @@ -18,6 +19,11 @@
>   
>   #define UNDERVOLTAGE_STICKY_BIT	BIT(16)
>   
> +struct rpi_firmware_get_value {
> +	__le32 id;
> +	__le32 val;
> +} __packed;
> +
>   struct rpi_hwmon_data {
>   	struct device *hwmon_dev;
>   	struct rpi_firmware *fw;
> @@ -56,6 +62,23 @@ static void rpi_firmware_get_throttled(struct rpi_hwmon_data *data)
>   	hwmon_notify_event(data->hwmon_dev, hwmon_in, hwmon_in_lcrit_alarm, 0);
>   }
>   
> +static int rpi_firmware_get_voltage(struct rpi_hwmon_data *data, u32 id,
> +				    long *val)
> +{
> +	struct rpi_firmware_get_value packet;
> +	int ret;
> +
> +	packet.id = cpu_to_le32(id);
> +	packet.val = 0;
> +	ret = rpi_firmware_property(data->fw, RPI_FIRMWARE_GET_VOLTAGE,
> +				    &packet, sizeof(packet));
> +	if (ret)
> +		return ret;
> +
> +	*val = le32_to_cpu(packet.val) / 1000;
> +	return 0;
> +}
> +
>   static void get_values_poll(struct work_struct *work)
>   {
>   	struct rpi_hwmon_data *data;
> @@ -77,19 +100,101 @@ static int rpi_read(struct device *dev, enum hwmon_sensor_types type,
>   {
>   	struct rpi_hwmon_data *data = dev_get_drvdata(dev);
>   
> -	*val = !!(data->last_throttled & UNDERVOLTAGE_STICKY_BIT);
> +	if (type == hwmon_in) {
> +		switch (attr) {
> +		case hwmon_in_input:
> +			switch (channel) {
> +			case 0:
> +				return rpi_firmware_get_voltage(data,
> +						RPI_FIRMWARE_VOLT_ID_CORE,
> +						val);
> +			case 1:
> +				return rpi_firmware_get_voltage(data,
> +						RPI_FIRMWARE_VOLT_ID_SDRAM_C,
> +						val);
> +			case 2:
> +				return rpi_firmware_get_voltage(data,
> +						RPI_FIRMWARE_VOLT_ID_SDRAM_I,
> +						val);
> +			case 3:
> +				return rpi_firmware_get_voltage(data,
> +						RPI_FIRMWARE_VOLT_ID_SDRAM_P,
> +						val);
> +			default:
> +				return -EOPNOTSUPP;
> +			}
> +		case hwmon_in_lcrit_alarm:
> +			if (channel == 0) {
> +				*val = !!(data->last_throttled & UNDERVOLTAGE_STICKY_BIT);
> +				return 0;
> +			}
> +			return -EOPNOTSUPP;
> +		default:
> +			return -EOPNOTSUPP;
> +		}
> +	}
> +
> +	return -EOPNOTSUPP;
> +}
> +
> +static int rpi_read_string(struct device *dev, enum hwmon_sensor_types type,
> +			   u32 attr, int channel, const char **str)
> +{
> +	if (type == hwmon_in && attr == hwmon_in_label) {
> +		switch (channel) {
> +		case 0:
> +			*str = "core";
> +			return 0;
> +		case 1:
> +			*str = "sdram_c";
> +			return 0;
> +		case 2:
> +			*str = "sdram_i";
> +			return 0;
> +		case 3:
> +			*str = "sdram_p";
> +			return 0;
> +		default:
> +			return -EOPNOTSUPP;
> +		}

This can be implemented as string array.

> +	}
> +
> +	return -EOPNOTSUPP;
> +}
> +
> +static umode_t rpi_is_visible(const void *_data, enum hwmon_sensor_types type,
> +			      u32 attr, int channel)
> +{
> +	if (type == hwmon_in) {
> +		switch (attr) {
> +		case hwmon_in_input:
> +		case hwmon_in_label:
> +			return 0444;
> +		case hwmon_in_lcrit_alarm:
> +			if (channel == 0)
> +				return 0444;
> +			return 0;
> +		default:
> +			return 0;
> +		}
> +	}
> +
>   	return 0;
>   }
>   
>   static const struct hwmon_channel_info * const rpi_info[] = {
>   	HWMON_CHANNEL_INFO(in,
> -			   HWMON_I_LCRIT_ALARM),
> +			   HWMON_I_INPUT | HWMON_I_LABEL | HWMON_I_LCRIT_ALARM,
> +			   HWMON_I_INPUT | HWMON_I_LABEL,
> +			   HWMON_I_INPUT | HWMON_I_LABEL,
> +			   HWMON_I_INPUT | HWMON_I_LABEL),
>   	NULL
>   };
>   
>   static const struct hwmon_ops rpi_hwmon_ops = {
> -	.visible = 0444,
> +	.is_visible = rpi_is_visible,
>   	.read = rpi_read,
> +	.read_string = rpi_read_string,
>   };
>   
>   static const struct hwmon_chip_info rpi_chip_info = {
> @@ -159,6 +264,7 @@ static struct platform_driver rpi_hwmon_driver = {
>   module_platform_driver(rpi_hwmon_driver);
>   
>   MODULE_AUTHOR("Stefan Wahren <wahrenst@gmx.net>");
> +MODULE_AUTHOR("Shubham Chakraborty <chakrabortyshubham66@gmail.com>");
>   MODULE_DESCRIPTION("Raspberry Pi voltage sensor driver");
>   MODULE_LICENSE("GPL v2");
>   MODULE_ALIAS("platform:raspberrypi-hwmon");

Re: [PATCH 2/2] hwmon: raspberrypi: Add voltage input support

[Guenter Roeck]

On 5/16/26 10:13, Guenter Roeck wrote:
> On 5/16/26 09:44, Shubham Chakraborty wrote:
>> Extend the raspberrypi-hwmon driver to expose firmware-provided
>> voltage measurements through the hwmon subsystem.
>>
>> The driver now exports the following voltage inputs:
>>
>>    - in0_input (core)
>>    - in1_input (sdram_c)
>>    - in2_input (sdram_i)
>>    - in3_input (sdram_p)
>>
>> Voltage values returned by firmware are converted from microvolts
>> to millivolts as expected by the hwmon subsystem.
>>
>> The existing undervoltage sticky alarm handling is preserved and
>> associated with the first voltage channel.
>>
>> Tested in -
>> - Raspberry Pi 3b+ (Linux raspberrypi 6.12.75+rpt-rpi-v8 #1 SMP PREEMPT
>>    Debian 1:6.12.75-1+rpt1 (2026-03-11) aarch64 GNU/Linux)
>>
>> Signed-off-by: Shubham Chakraborty <chakrabortyshubham66@gmail.com>
> 
> I wasn't copied on patch 1/2, so I have no idea what is in there
> and how it is related to this patch. Either way it seems unlikely that
> the new functionality is supported by all versions of Raspberry Pi supported
> by this driver. Just returning an error when the user tries to read a sensor
> that is not supported is unacceptable. This needs either evidence that the
> sensors are supported by all board variants and firmware versions supported
> by this driver, or the is_visible function needs to selectively enable the
> supported sensors.
> 
>> ---
>>   drivers/hwmon/raspberrypi-hwmon.c | 112 +++++++++++++++++++++++++++++-
> 
> Documentation/hwmon/raspberrypi-hwmon.rst needs to be updated as well.
> 
>>   1 file changed, 109 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/hwmon/raspberrypi-hwmon.c b/drivers/hwmon/raspberrypi-hwmon.c
>> index a2938881ccd2..c73a970db025 100644
>> --- a/drivers/hwmon/raspberrypi-hwmon.c
>> +++ b/drivers/hwmon/raspberrypi-hwmon.c
>> @@ -5,6 +5,7 @@
>>    * Based on firmware/raspberrypi.c by Noralf Trønnes
>>    *
>>    * Copyright (C) 2018 Stefan Wahren <stefan.wahren@i2se.com>
>> + * Copyright (C) 2026 Shubham Chakraborty <chakrabortyshubham66@gmail.com>
>>    */
>>   #include <linux/device.h>
>>   #include <linux/devm-helpers.h>
>> @@ -18,6 +19,11 @@
>>   #define UNDERVOLTAGE_STICKY_BIT    BIT(16)
>> +struct rpi_firmware_get_value {
>> +    __le32 id;
>> +    __le32 val;
>> +} __packed;
>> +

More feedback:

Why define this here but RPI_FIRMWARE_VOLT_ID_CORE etc. in the
firmware API include file ? struct rpi_firmware_clk_rate_request
is quite similar and defined (and documented) in the include file.

Also, the name is quite generic, suggesting a common structure.
rpi_firmware_clk_rate_request, as used for the clock, is much more
specific. I would suggest to either use a single structure for all
requests or clarify that this is for voltages only (i.e., name the
structure and the returned value appropriately).

Last but not least, please send both patches to the same set of people
and mailing lists.

Thanks,
Guenter

>>   struct rpi_hwmon_data {
>>       struct device *hwmon_dev;
>>       struct rpi_firmware *fw;
>> @@ -56,6 +62,23 @@ static void rpi_firmware_get_throttled(struct rpi_hwmon_data *data)
>>       hwmon_notify_event(data->hwmon_dev, hwmon_in, hwmon_in_lcrit_alarm, 0);
>>   }
>> +static int rpi_firmware_get_voltage(struct rpi_hwmon_data *data, u32 id,
>> +                    long *val)
>> +{
>> +    struct rpi_firmware_get_value packet;
>> +    int ret;
>> +
>> +    packet.id = cpu_to_le32(id);
>> +    packet.val = 0;
>> +    ret = rpi_firmware_property(data->fw, RPI_FIRMWARE_GET_VOLTAGE,
>> +                    &packet, sizeof(packet));
>> +    if (ret)
>> +        return ret;
>> +
>> +    *val = le32_to_cpu(packet.val) / 1000;
>> +    return 0;
>> +}
>> +
>>   static void get_values_poll(struct work_struct *work)
>>   {
>>       struct rpi_hwmon_data *data;
>> @@ -77,19 +100,101 @@ static int rpi_read(struct device *dev, enum hwmon_sensor_types type,
>>   {
>>       struct rpi_hwmon_data *data = dev_get_drvdata(dev);
>> -    *val = !!(data->last_throttled & UNDERVOLTAGE_STICKY_BIT);
>> +    if (type == hwmon_in) {
>> +        switch (attr) {
>> +        case hwmon_in_input:
>> +            switch (channel) {
>> +            case 0:
>> +                return rpi_firmware_get_voltage(data,
>> +                        RPI_FIRMWARE_VOLT_ID_CORE,
>> +                        val);
>> +            case 1:
>> +                return rpi_firmware_get_voltage(data,
>> +                        RPI_FIRMWARE_VOLT_ID_SDRAM_C,
>> +                        val);
>> +            case 2:
>> +                return rpi_firmware_get_voltage(data,
>> +                        RPI_FIRMWARE_VOLT_ID_SDRAM_I,
>> +                        val);
>> +            case 3:
>> +                return rpi_firmware_get_voltage(data,
>> +                        RPI_FIRMWARE_VOLT_ID_SDRAM_P,
>> +                        val);
>> +            default:
>> +                return -EOPNOTSUPP;
>> +            }
>> +        case hwmon_in_lcrit_alarm:
>> +            if (channel == 0) {
>> +                *val = !!(data->last_throttled & UNDERVOLTAGE_STICKY_BIT);
>> +                return 0;
>> +            }
>> +            return -EOPNOTSUPP;
>> +        default:
>> +            return -EOPNOTSUPP;
>> +        }
>> +    }
>> +
>> +    return -EOPNOTSUPP;
>> +}
>> +
>> +static int rpi_read_string(struct device *dev, enum hwmon_sensor_types type,
>> +               u32 attr, int channel, const char **str)
>> +{
>> +    if (type == hwmon_in && attr == hwmon_in_label) {
>> +        switch (channel) {
>> +        case 0:
>> +            *str = "core";
>> +            return 0;
>> +        case 1:
>> +            *str = "sdram_c";
>> +            return 0;
>> +        case 2:
>> +            *str = "sdram_i";
>> +            return 0;
>> +        case 3:
>> +            *str = "sdram_p";
>> +            return 0;
>> +        default:
>> +            return -EOPNOTSUPP;
>> +        }
> 
> This can be implemented as string array.
> 
>> +    }
>> +
>> +    return -EOPNOTSUPP;
>> +}
>> +
>> +static umode_t rpi_is_visible(const void *_data, enum hwmon_sensor_types type,
>> +                  u32 attr, int channel)
>> +{
>> +    if (type == hwmon_in) {
>> +        switch (attr) {
>> +        case hwmon_in_input:
>> +        case hwmon_in_label:
>> +            return 0444;
>> +        case hwmon_in_lcrit_alarm:
>> +            if (channel == 0)
>> +                return 0444;
>> +            return 0;
>> +        default:
>> +            return 0;
>> +        }
>> +    }
>> +
>>       return 0;
>>   }
>>   static const struct hwmon_channel_info * const rpi_info[] = {
>>       HWMON_CHANNEL_INFO(in,
>> -               HWMON_I_LCRIT_ALARM),
>> +               HWMON_I_INPUT | HWMON_I_LABEL | HWMON_I_LCRIT_ALARM,
>> +               HWMON_I_INPUT | HWMON_I_LABEL,
>> +               HWMON_I_INPUT | HWMON_I_LABEL,
>> +               HWMON_I_INPUT | HWMON_I_LABEL),
>>       NULL
>>   };
>>   static const struct hwmon_ops rpi_hwmon_ops = {
>> -    .visible = 0444,
>> +    .is_visible = rpi_is_visible,
>>       .read = rpi_read,
>> +    .read_string = rpi_read_string,
>>   };
>>   static const struct hwmon_chip_info rpi_chip_info = {
>> @@ -159,6 +264,7 @@ static struct platform_driver rpi_hwmon_driver = {
>>   module_platform_driver(rpi_hwmon_driver);
>>   MODULE_AUTHOR("Stefan Wahren <wahrenst@gmx.net>");
>> +MODULE_AUTHOR("Shubham Chakraborty <chakrabortyshubham66@gmail.com>");
>>   MODULE_DESCRIPTION("Raspberry Pi voltage sensor driver");
>>   MODULE_LICENSE("GPL v2");
>>   MODULE_ALIAS("platform:raspberrypi-hwmon");
> 
> 

[PATCH v2 0/2] raspberrypi: firmware and hwmon voltage support

[Shubham Chakraborty]

Changes in v2:
- Patch 1/2: no changes
- Patch 2/2:
  - hide unsupported voltage sensors using .is_visible()
  - replace the label switch with a string array
  - update raspberrypi-hwmon documentation


Shubham Chakraborty (2):
  soc: bcm2835: raspberrypi-firmware: Add voltage domain IDs
  hwmon: raspberrypi: Add voltage input support

 Documentation/hwmon/raspberrypi-hwmon.rst  |  15 ++-
 drivers/hwmon/raspberrypi-hwmon.c          | 134 ++++++++++++++++++++-
 include/soc/bcm2835/raspberrypi-firmware.h |   8 ++
 3 files changed, 152 insertions(+), 5 deletions(-)

-- 
2.54.0

[PATCH v2 1/2] soc: bcm2835: raspberrypi-firmware: Add voltage domain IDs

[Shubham Chakraborty]

Add firmware voltage domain identifiers for the Raspberry Pi
mailbox property interface.

These IDs are used by firmware clients to query voltage rails
through the RPI_FIRMWARE_GET_VOLTAGE property.

Signed-off-by: Shubham Chakraborty <chakrabortyshubham66@gmail.com>
---
 include/soc/bcm2835/raspberrypi-firmware.h | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/include/soc/bcm2835/raspberrypi-firmware.h b/include/soc/bcm2835/raspberrypi-firmware.h
index e1f87fbfe554..fd2e051ce05b 100644
--- a/include/soc/bcm2835/raspberrypi-firmware.h
+++ b/include/soc/bcm2835/raspberrypi-firmware.h
@@ -156,6 +156,14 @@ enum rpi_firmware_clk_id {
 	RPI_FIRMWARE_NUM_CLK_ID,
 };
 
+enum rpi_firmware_volt_id {
+	RPI_FIRMWARE_VOLT_ID_RESERVED = 0,
+	RPI_FIRMWARE_VOLT_ID_CORE = 1,
+	RPI_FIRMWARE_VOLT_ID_SDRAM_C = 2,
+	RPI_FIRMWARE_VOLT_ID_SDRAM_I = 3,
+	RPI_FIRMWARE_VOLT_ID_SDRAM_P = 4,
+};
+
 /**
  * struct rpi_firmware_clk_rate_request - Firmware Request for a rate
  * @id:	ID of the clock being queried
-- 
2.54.0

[PATCH v2 2/2] hwmon: raspberrypi: Add voltage input support

[Shubham Chakraborty]

Extend the raspberrypi-hwmon driver to expose firmware-provided
voltage measurements through the hwmon subsystem.

The driver now exports the following voltage inputs:

  - in0_input (core)
  - in1_input (sdram_c)
  - in2_input (sdram_i)
  - in3_input (sdram_p)

Voltage values returned by firmware are converted from microvolts
to millivolts as expected by the hwmon subsystem.

Update the documentation related to it.

The existing undervoltage sticky alarm handling is preserved and
associated with the first voltage channel.

Tested in -
- Raspberry Pi 3b+ (Linux raspberrypi 6.12.75+rpt-rpi-v8 #1 SMP PREEMPT
  Debian 1:6.12.75-1+rpt1 (2026-03-11) aarch64 GNU/Linux)

Signed-off-by: Shubham Chakraborty <chakrabortyshubham66@gmail.com>
---
 Documentation/hwmon/raspberrypi-hwmon.rst |  15 ++-
 drivers/hwmon/raspberrypi-hwmon.c         | 134 +++++++++++++++++++++-
 2 files changed, 144 insertions(+), 5 deletions(-)

diff --git a/Documentation/hwmon/raspberrypi-hwmon.rst b/Documentation/hwmon/raspberrypi-hwmon.rst
index 8038ade36490..db315184b861 100644
--- a/Documentation/hwmon/raspberrypi-hwmon.rst
+++ b/Documentation/hwmon/raspberrypi-hwmon.rst
@@ -20,6 +20,17 @@ undervoltage conditions.
 Sysfs entries
 -------------
 
-======================= ==================
+======================= ======================================================
+in0_input		Core voltage in millivolts
+in1_input		SDRAM controller voltage in millivolts
+in2_input		SDRAM I/O voltage in millivolts
+in3_input		SDRAM PHY voltage in millivolts
+in0_label		"core"
+in1_label		"sdram_c"
+in2_label		"sdram_i"
+in3_label		"sdram_p"
 in0_lcrit_alarm		Undervoltage alarm
-======================= ==================
+======================= ======================================================
+
+The voltage inputs and labels are only exposed if the firmware reports support
+for the corresponding voltage ID.
diff --git a/drivers/hwmon/raspberrypi-hwmon.c b/drivers/hwmon/raspberrypi-hwmon.c
index a2938881ccd2..4f96f37116f3 100644
--- a/drivers/hwmon/raspberrypi-hwmon.c
+++ b/drivers/hwmon/raspberrypi-hwmon.c
@@ -5,6 +5,7 @@
  * Based on firmware/raspberrypi.c by Noralf Trønnes
  *
  * Copyright (C) 2018 Stefan Wahren <stefan.wahren@i2se.com>
+ * Copyright (C) 2026 Shubham Chakraborty <chakrabortyshubham66@gmail.com>
  */
 #include <linux/device.h>
 #include <linux/devm-helpers.h>
@@ -18,13 +19,26 @@
 
 #define UNDERVOLTAGE_STICKY_BIT	BIT(16)
 
+struct rpi_firmware_get_value {
+	__le32 id;
+	__le32 val;
+} __packed;
+
 struct rpi_hwmon_data {
 	struct device *hwmon_dev;
 	struct rpi_firmware *fw;
+	u32 valid_inputs;
 	u32 last_throttled;
 	struct delayed_work get_values_poll_work;
 };
 
+static const char * const rpi_hwmon_labels[] = {
+	"core",
+	"sdram_c",
+	"sdram_i",
+	"sdram_p",
+};
+
 static void rpi_firmware_get_throttled(struct rpi_hwmon_data *data)
 {
 	u32 new_uv, old_uv, value;
@@ -56,6 +70,23 @@ static void rpi_firmware_get_throttled(struct rpi_hwmon_data *data)
 	hwmon_notify_event(data->hwmon_dev, hwmon_in, hwmon_in_lcrit_alarm, 0);
 }
 
+static int rpi_firmware_get_voltage(struct rpi_hwmon_data *data, u32 id,
+				    long *val)
+{
+	struct rpi_firmware_get_value packet;
+	int ret;
+
+	packet.id = cpu_to_le32(id);
+	packet.val = 0;
+	ret = rpi_firmware_property(data->fw, RPI_FIRMWARE_GET_VOLTAGE,
+				    &packet, sizeof(packet));
+	if (ret)
+		return ret;
+
+	*val = le32_to_cpu(packet.val) / 1000;
+	return 0;
+}
+
 static void get_values_poll(struct work_struct *work)
 {
 	struct rpi_hwmon_data *data;
@@ -77,19 +108,94 @@ static int rpi_read(struct device *dev, enum hwmon_sensor_types type,
 {
 	struct rpi_hwmon_data *data = dev_get_drvdata(dev);
 
-	*val = !!(data->last_throttled & UNDERVOLTAGE_STICKY_BIT);
+	if (type == hwmon_in) {
+		switch (attr) {
+		case hwmon_in_input:
+			switch (channel) {
+			case 0:
+				return rpi_firmware_get_voltage(data,
+						RPI_FIRMWARE_VOLT_ID_CORE,
+						val);
+			case 1:
+				return rpi_firmware_get_voltage(data,
+						RPI_FIRMWARE_VOLT_ID_SDRAM_C,
+						val);
+			case 2:
+				return rpi_firmware_get_voltage(data,
+						RPI_FIRMWARE_VOLT_ID_SDRAM_I,
+						val);
+			case 3:
+				return rpi_firmware_get_voltage(data,
+						RPI_FIRMWARE_VOLT_ID_SDRAM_P,
+						val);
+			default:
+				return -EOPNOTSUPP;
+			}
+		case hwmon_in_lcrit_alarm:
+			if (channel == 0) {
+				*val = !!(data->last_throttled & UNDERVOLTAGE_STICKY_BIT);
+				return 0;
+			}
+			return -EOPNOTSUPP;
+		default:
+			return -EOPNOTSUPP;
+		}
+	}
+
+	return -EOPNOTSUPP;
+}
+
+static int rpi_read_string(struct device *dev, enum hwmon_sensor_types type,
+			   u32 attr, int channel, const char **str)
+{
+	if (type == hwmon_in && attr == hwmon_in_label) {
+		if (channel >= ARRAY_SIZE(rpi_hwmon_labels))
+			return -EOPNOTSUPP;
+
+		*str = rpi_hwmon_labels[channel];
+		return 0;
+	}
+
+	return -EOPNOTSUPP;
+}
+
+static umode_t rpi_is_visible(const void *_data, enum hwmon_sensor_types type,
+			      u32 attr, int channel)
+{
+	const struct rpi_hwmon_data *data = _data;
+
+	if (type == hwmon_in) {
+		switch (attr) {
+		case hwmon_in_input:
+		case hwmon_in_label:
+			if (!(data->valid_inputs & BIT(channel)))
+				return 0;
+			return 0444;
+		case hwmon_in_lcrit_alarm:
+			if (channel == 0)
+				return 0444;
+			return 0;
+		default:
+			return 0;
+		}
+	}
+
 	return 0;
 }
 
 static const struct hwmon_channel_info * const rpi_info[] = {
 	HWMON_CHANNEL_INFO(in,
-			   HWMON_I_LCRIT_ALARM),
+			   HWMON_I_INPUT | HWMON_I_LABEL | HWMON_I_LCRIT_ALARM,
+			   HWMON_I_INPUT | HWMON_I_LABEL,
+			   HWMON_I_INPUT | HWMON_I_LABEL,
+			   HWMON_I_INPUT | HWMON_I_LABEL),
 	NULL
 };
 
 static const struct hwmon_ops rpi_hwmon_ops = {
-	.visible = 0444,
+	.is_visible = rpi_is_visible,
 	.read = rpi_read,
+	.read_string = rpi_read_string,
 };
 
 static const struct hwmon_chip_info rpi_chip_info = {
@@ -101,6 +207,7 @@ static int rpi_hwmon_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct rpi_hwmon_data *data;
+	long voltage;
 	int ret;
 
 	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
@@ -110,6 +217,26 @@ static int rpi_hwmon_probe(struct platform_device *pdev)
 	/* Parent driver assure that firmware is correct */
 	data->fw = dev_get_drvdata(dev->parent);
 
+	ret = rpi_firmware_get_voltage(data, RPI_FIRMWARE_VOLT_ID_CORE,
+				       &voltage);
+	if (!ret)
+		data->valid_inputs |= BIT(0);
+
+	ret = rpi_firmware_get_voltage(data, RPI_FIRMWARE_VOLT_ID_SDRAM_C,
+				       &voltage);
+	if (!ret)
+		data->valid_inputs |= BIT(1);
+
+	ret = rpi_firmware_get_voltage(data, RPI_FIRMWARE_VOLT_ID_SDRAM_I,
+				       &voltage);
+	if (!ret)
+		data->valid_inputs |= BIT(2);
+
+	ret = rpi_firmware_get_voltage(data, RPI_FIRMWARE_VOLT_ID_SDRAM_P,
+				       &voltage);
+	if (!ret)
+		data->valid_inputs |= BIT(3);
+
 	data->hwmon_dev = devm_hwmon_device_register_with_info(dev, "rpi_volt",
 							       data,
 							       &rpi_chip_info,
@@ -159,6 +286,7 @@ static struct platform_driver rpi_hwmon_driver = {
 module_platform_driver(rpi_hwmon_driver);
 
 MODULE_AUTHOR("Stefan Wahren <wahrenst@gmx.net>");
+MODULE_AUTHOR("Shubham Chakraborty <chakrabortyshubham66@gmail.com>");
 MODULE_DESCRIPTION("Raspberry Pi voltage sensor driver");
 MODULE_LICENSE("GPL v2");
 MODULE_ALIAS("platform:raspberrypi-hwmon");
-- 
2.54.0

Re: [PATCH v2 1/2] soc: bcm2835: raspberrypi-firmware: Add voltage domain IDs

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The voltage domain IDs for SDRAM_I and SDRAM_P are swapped, violating the Raspberry Pi firmware mailbox property interface.
--

commit 55efe0976ecb0653da890b61d597ee771424aed8
Author: Shubham Chakraborty <chakrabortyshubham66@gmail.com>

soc: bcm2835: raspberrypi-firmware: Add voltage domain IDs

This commit adds firmware voltage domain identifiers for the Raspberry Pi
mailbox property interface. Firmware clients use these IDs to query voltage
rails via the RPI_FIRMWARE_GET_VOLTAGE property.

> diff --git a/include/soc/bcm2835/raspberrypi-firmware.h b/include/soc/bcm2835/raspberrypi-firmware.h
> index e1f87fbfe5542..fd2e051ce05b7 100644
> --- a/include/soc/bcm2835/raspberrypi-firmware.h
> +++ b/include/soc/bcm2835/raspberrypi-firmware.h

[ ... ]

> +enum rpi_firmware_volt_id {
> +	RPI_FIRMWARE_VOLT_ID_RESERVED = 0,
> +	RPI_FIRMWARE_VOLT_ID_CORE = 1,
> +	RPI_FIRMWARE_VOLT_ID_SDRAM_C = 2,
> +	RPI_FIRMWARE_VOLT_ID_SDRAM_I = 3,
> +	RPI_FIRMWARE_VOLT_ID_SDRAM_P = 4,

Are the IDs for SDRAM_I and SDRAM_P swapped here?

According to the official Raspberry Pi mailbox property interface, the
voltage domain IDs are defined as SDRAM_P (PHY) = 3 and SDRAM_I (I/O) = 4.

If these values are inverted, could it cause firmware clients that query or
modify these domains to access the wrong voltage rails? This might lead
to incorrect voltage reporting or hardware instability if used by scaling
drivers.

> +};

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260516191555.17978-1-chakrabortyshubham66@gmail.com?part=1

Re: [PATCH v2 2/2] hwmon: raspberrypi: Add voltage input support

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] Unconditional self-rescheduling of the background workqueue leads to a use-after-free on device removal.
- [Medium] The driver incorrectly detects and exposes unsupported voltage channels due to unverified firmware responses.
--

commit aa874218705dba1c84bc41a7c3b61abd93bbf0a2
Author: Shubham Chakraborty <chakrabortyshubham66@gmail.com>

hwmon: raspberrypi: Add voltage input support

This commit extends the raspberrypi-hwmon driver to expose firmware-provided
voltage measurements. It adds core and SDRAM voltage inputs, converting
the values from microvolts to millivolts for the hwmon subsystem.
Inputs are only exposed if the firmware reports support for the corresponding
voltage ID.

> diff --git a/drivers/hwmon/raspberrypi-hwmon.c b/drivers/hwmon/raspberrypi-hwmon.c
> index a2938881ccd2f..4f96f37116f32 100644
> --- a/drivers/hwmon/raspberrypi-hwmon.c
> +++ b/drivers/hwmon/raspberrypi-hwmon.c

[ ... ]

> @@ -56,6 +70,23 @@ static void rpi_firmware_get_throttled(struct rpi_hwmon_data *data)
>  	hwmon_notify_event(data->hwmon_dev, hwmon_in, hwmon_in_lcrit_alarm, 0);
>  }
>  
> +static int rpi_firmware_get_voltage(struct rpi_hwmon_data *data, u32 id,
> +				    long *val)
> +{
> +	struct rpi_firmware_get_value packet;
> +	int ret;
> +
> +	packet.id = cpu_to_le32(id);
> +	packet.val = 0;
> +	ret = rpi_firmware_property(data->fw, RPI_FIRMWARE_GET_VOLTAGE,
> +				    &packet, sizeof(packet));

If the firmware does not support the RPI_FIRMWARE_GET_VOLTAGE tag or the
specific voltage ID, could the mailbox transaction still return 0 without
modifying the tag buffer?

If so, packet.val would remain 0 and this function would return success.
During probe, this would cause the channel to be incorrectly flagged as
supported, exposing sysfs files that permanently read 0mV.

Would it be safer to initialize packet.val to a sentinel value like ~0
before querying the firmware to verify the voltage ID is actually supported?

> +	if (ret)
> +		return ret;
> +
> +	*val = le32_to_cpu(packet.val) / 1000;
> +	return 0;
> +}
> +
>  static void get_values_poll(struct work_struct *work)
>  {
>  	struct rpi_hwmon_data *data;

Does the unconditional rescheduling in get_values_poll() cause a
use-after-free regression during driver removal?

If the work function executes concurrently with driver unbind,
cancel_delayed_work_sync() will wait for it to finish. However,
get_values_poll() currently reschedules itself just before returning:

get_values_poll() {
        ...
        schedule_delayed_work(&data->get_values_poll_work, 2 * HZ);
}

After cancellation completes, devres frees the memory for data. When the
rescheduled work executes later, it accesses the freed data pointer.

Could a shutdown flag be added to safely prevent rescheduling during
teardown?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260516191555.17978-1-chakrabortyshubham66@gmail.com?part=2

Re: [PATCH v2 1/2] soc: bcm2835: raspberrypi-firmware: Add voltage domain IDs

[Guenter Roeck]

On 5/16/26 12:15, Shubham Chakraborty wrote:
> Add firmware voltage domain identifiers for the Raspberry Pi
> mailbox property interface.
> 
> These IDs are used by firmware clients to query voltage rails
> through the RPI_FIRMWARE_GET_VOLTAGE property.
> 
> Signed-off-by: Shubham Chakraborty <chakrabortyshubham66@gmail.com>
> ---
>   include/soc/bcm2835/raspberrypi-firmware.h | 8 ++++++++
>   1 file changed, 8 insertions(+)
> 
> diff --git a/include/soc/bcm2835/raspberrypi-firmware.h b/include/soc/bcm2835/raspberrypi-firmware.h
> index e1f87fbfe554..fd2e051ce05b 100644
> --- a/include/soc/bcm2835/raspberrypi-firmware.h
> +++ b/include/soc/bcm2835/raspberrypi-firmware.h
> @@ -156,6 +156,14 @@ enum rpi_firmware_clk_id {
>   	RPI_FIRMWARE_NUM_CLK_ID,
>   };
>   
> +enum rpi_firmware_volt_id {
> +	RPI_FIRMWARE_VOLT_ID_RESERVED = 0,

Is that needed ?

> +	RPI_FIRMWARE_VOLT_ID_CORE = 1,
> +	RPI_FIRMWARE_VOLT_ID_SDRAM_C = 2,
> +	RPI_FIRMWARE_VOLT_ID_SDRAM_I = 3,
> +	RPI_FIRMWARE_VOLT_ID_SDRAM_P = 4,

Regarding Sashiko's feedback: I don't know where it got the
information from, but a web search suggests that it has a point;
RPI_FIRMWARE_VOLT_ID_SDRAM_I and RPI_FIRMWARE_VOLT_ID_SDRAM_P appear
to be swapped. If that is not the case, please provide evidence.

Thanks,
Guenter

Re: [PATCH v2 2/2] hwmon: raspberrypi: Add voltage input support

[Guenter Roeck]

On 5/16/26 12:15, Shubham Chakraborty wrote:
> Extend the raspberrypi-hwmon driver to expose firmware-provided
> voltage measurements through the hwmon subsystem.
> 
> The driver now exports the following voltage inputs:
> 
>    - in0_input (core)
>    - in1_input (sdram_c)
>    - in2_input (sdram_i)
>    - in3_input (sdram_p)
> 
> Voltage values returned by firmware are converted from microvolts
> to millivolts as expected by the hwmon subsystem.
> 
> Update the documentation related to it.
> 
> The existing undervoltage sticky alarm handling is preserved and
> associated with the first voltage channel.
> 
> Tested in -
> - Raspberry Pi 3b+ (Linux raspberrypi 6.12.75+rpt-rpi-v8 #1 SMP PREEMPT
>    Debian 1:6.12.75-1+rpt1 (2026-03-11) aarch64 GNU/Linux)
> 
> Signed-off-by: Shubham Chakraborty <chakrabortyshubham66@gmail.com>
> ---
>   Documentation/hwmon/raspberrypi-hwmon.rst |  15 ++-
>   drivers/hwmon/raspberrypi-hwmon.c         | 134 +++++++++++++++++++++-
>   2 files changed, 144 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/hwmon/raspberrypi-hwmon.rst b/Documentation/hwmon/raspberrypi-hwmon.rst
> index 8038ade36490..db315184b861 100644
> --- a/Documentation/hwmon/raspberrypi-hwmon.rst
> +++ b/Documentation/hwmon/raspberrypi-hwmon.rst
> @@ -20,6 +20,17 @@ undervoltage conditions.
>   Sysfs entries
>   -------------
>   
> -======================= ==================
> +======================= ======================================================
> +in0_input		Core voltage in millivolts
> +in1_input		SDRAM controller voltage in millivolts
> +in2_input		SDRAM I/O voltage in millivolts
> +in3_input		SDRAM PHY voltage in millivolts
> +in0_label		"core"
> +in1_label		"sdram_c"
> +in2_label		"sdram_i"
> +in3_label		"sdram_p"
>   in0_lcrit_alarm		Undervoltage alarm
> -======================= ==================
> +======================= ======================================================
> +
> +The voltage inputs and labels are only exposed if the firmware reports support
> +for the corresponding voltage ID.
> diff --git a/drivers/hwmon/raspberrypi-hwmon.c b/drivers/hwmon/raspberrypi-hwmon.c
> index a2938881ccd2..4f96f37116f3 100644
> --- a/drivers/hwmon/raspberrypi-hwmon.c
> +++ b/drivers/hwmon/raspberrypi-hwmon.c
> @@ -5,6 +5,7 @@
>    * Based on firmware/raspberrypi.c by Noralf Trønnes
>    *
>    * Copyright (C) 2018 Stefan Wahren <stefan.wahren@i2se.com>
> + * Copyright (C) 2026 Shubham Chakraborty <chakrabortyshubham66@gmail.com>
>    */
>   #include <linux/device.h>
>   #include <linux/devm-helpers.h>
> @@ -18,13 +19,26 @@
>   
>   #define UNDERVOLTAGE_STICKY_BIT	BIT(16)
>   
> +struct rpi_firmware_get_value {
> +	__le32 id;
> +	__le32 val;
> +} __packed;

My earlier comment is still valid: This should be defined in
the include file, and it should be query-specific, just like
struct rpi_firmware_clk_rate_request.

> +
>   struct rpi_hwmon_data {
>   	struct device *hwmon_dev;
>   	struct rpi_firmware *fw;
> +	u32 valid_inputs;
>   	u32 last_throttled;
>   	struct delayed_work get_values_poll_work;
>   };
>   
> +static const char * const rpi_hwmon_labels[] = {
> +	"core",
> +	"sdram_c",
> +	"sdram_i",
> +	"sdram_p",
> +};
> +
>   static void rpi_firmware_get_throttled(struct rpi_hwmon_data *data)
>   {
>   	u32 new_uv, old_uv, value;
> @@ -56,6 +70,23 @@ static void rpi_firmware_get_throttled(struct rpi_hwmon_data *data)
>   	hwmon_notify_event(data->hwmon_dev, hwmon_in, hwmon_in_lcrit_alarm, 0);
>   }
>   
> +static int rpi_firmware_get_voltage(struct rpi_hwmon_data *data, u32 id,
> +				    long *val)
> +{
> +	struct rpi_firmware_get_value packet;
> +	int ret;
> +
> +	packet.id = cpu_to_le32(id);
> +	packet.val = 0;
> +	ret = rpi_firmware_property(data->fw, RPI_FIRMWARE_GET_VOLTAGE,
> +				    &packet, sizeof(packet));
> +	if (ret)
> +		return ret;
> +
> +	*val = le32_to_cpu(packet.val) / 1000;

I would suggest to use DIV_ROUND_CLOSEST().

> +	return 0;
> +}
> +
>   static void get_values_poll(struct work_struct *work)
>   {
>   	struct rpi_hwmon_data *data;
> @@ -77,19 +108,94 @@ static int rpi_read(struct device *dev, enum hwmon_sensor_types type,
>   {
>   	struct rpi_hwmon_data *data = dev_get_drvdata(dev);
>   
> -	*val = !!(data->last_throttled & UNDERVOLTAGE_STICKY_BIT);
> +	if (type == hwmon_in) {
> +		switch (attr) {
> +		case hwmon_in_input:
> +			switch (channel) {
> +			case 0:
> +				return rpi_firmware_get_voltage(data,
> +						RPI_FIRMWARE_VOLT_ID_CORE,
> +						val);
> +			case 1:
> +				return rpi_firmware_get_voltage(data,
> +						RPI_FIRMWARE_VOLT_ID_SDRAM_C,
> +						val);
> +			case 2:
> +				return rpi_firmware_get_voltage(data,
> +						RPI_FIRMWARE_VOLT_ID_SDRAM_I,
> +						val);
> +			case 3:
> +				return rpi_firmware_get_voltage(data,
> +						RPI_FIRMWARE_VOLT_ID_SDRAM_P,
> +						val);
> +			default:
> +				return -EOPNOTSUPP;

With

static const int voltage_regs[] = {
	RPI_FIRMWARE_VOLT_ID_CORE, RPI_FIRMWARE_VOLT_ID_SDRAM_C, RPI_FIRMWARE_VOLT_ID_SDRAM_I,
	RPI_FIRMWARE_VOLT_ID_SDRAM_P };

this can be simplified to
	return rpi_firmware_get_voltage(data, voltage_regs[channel];

> +			}
> +		case hwmon_in_lcrit_alarm:
> +			if (channel == 0) {
> +				*val = !!(data->last_throttled & UNDERVOLTAGE_STICKY_BIT);
> +				return 0;
> +			}

The channel check is not really necessary.

> +			return -EOPNOTSUPP;
> +		default:
> +			return -EOPNOTSUPP;
> +		}
> +	}
> +
> +	return -EOPNOTSUPP;
> +}
> +
> +static int rpi_read_string(struct device *dev, enum hwmon_sensor_types type,
> +			   u32 attr, int channel, const char **str)
> +{
> +	if (type == hwmon_in && attr == hwmon_in_label) {
> +		if (channel >= ARRAY_SIZE(rpi_hwmon_labels))
> +			return -EOPNOTSUPP;

Unnecessary check.

> +
> +		*str = rpi_hwmon_labels[channel];
> +		return 0;
> +	}
> +
> +	return -EOPNOTSUPP;
> +}
> +
> +static umode_t rpi_is_visible(const void *_data, enum hwmon_sensor_types type,
> +			      u32 attr, int channel)
> +{
> +	const struct rpi_hwmon_data *data = _data;
> +
> +	if (type == hwmon_in) {
> +		switch (attr) {
> +		case hwmon_in_input:
> +		case hwmon_in_label:
> +			if (!(data->valid_inputs & BIT(channel)))
> +				return 0;
> +			return 0444;
> +		case hwmon_in_lcrit_alarm:
> +			if (channel == 0)
> +				return 0444;
> +			return 0;
> +		default:
> +			return 0;
> +		}
> +	}
> +
>   	return 0;
>   }
>   
>   static const struct hwmon_channel_info * const rpi_info[] = {
>   	HWMON_CHANNEL_INFO(in,
> -			   HWMON_I_LCRIT_ALARM),
> +			   HWMON_I_INPUT | HWMON_I_LABEL | HWMON_I_LCRIT_ALARM,
> +			   HWMON_I_INPUT | HWMON_I_LABEL,
> +			   HWMON_I_INPUT | HWMON_I_LABEL,
> +			   HWMON_I_INPUT | HWMON_I_LABEL),
>   	NULL
>   };
>   
>   static const struct hwmon_ops rpi_hwmon_ops = {
> -	.visible = 0444,
> +	.is_visible = rpi_is_visible,
>   	.read = rpi_read,
> +	.read_string = rpi_read_string,
>   };
>   
>   static const struct hwmon_chip_info rpi_chip_info = {
> @@ -101,6 +207,7 @@ static int rpi_hwmon_probe(struct platform_device *pdev)
>   {
>   	struct device *dev = &pdev->dev;
>   	struct rpi_hwmon_data *data;
> +	long voltage;
>   	int ret;
>   
>   	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
> @@ -110,6 +217,26 @@ static int rpi_hwmon_probe(struct platform_device *pdev)
>   	/* Parent driver assure that firmware is correct */
>   	data->fw = dev_get_drvdata(dev->parent);
>   
> +	ret = rpi_firmware_get_voltage(data, RPI_FIRMWARE_VOLT_ID_CORE,
> +				       &voltage);
> +	if (!ret)
> +		data->valid_inputs |= BIT(0);
> +
> +	ret = rpi_firmware_get_voltage(data, RPI_FIRMWARE_VOLT_ID_SDRAM_C,
> +				       &voltage);
> +	if (!ret)
> +		data->valid_inputs |= BIT(1);
> +
> +	ret = rpi_firmware_get_voltage(data, RPI_FIRMWARE_VOLT_ID_SDRAM_I,
> +				       &voltage);
> +	if (!ret)
> +		data->valid_inputs |= BIT(2);
> +
> +	ret = rpi_firmware_get_voltage(data, RPI_FIRMWARE_VOLT_ID_SDRAM_P,
> +				       &voltage);
> +	if (!ret)
> +		data->valid_inputs |= BIT(3);
> +

This can be implemented in a loop, using the above voltage_regs array.

Thanks,
Guenter

>   	data->hwmon_dev = devm_hwmon_device_register_with_info(dev, "rpi_volt",
>   							       data,
>   							       &rpi_chip_info,
> @@ -159,6 +286,7 @@ static struct platform_driver rpi_hwmon_driver = {
>   module_platform_driver(rpi_hwmon_driver);
>   
>   MODULE_AUTHOR("Stefan Wahren <wahrenst@gmx.net>");
> +MODULE_AUTHOR("Shubham Chakraborty <chakrabortyshubham66@gmail.com>");
>   MODULE_DESCRIPTION("Raspberry Pi voltage sensor driver");
>   MODULE_LICENSE("GPL v2");
>   MODULE_ALIAS("platform:raspberrypi-hwmon");

Re: [PATCH v2 2/2] hwmon: raspberrypi: Add voltage input support

[Guenter Roeck]

On 5/16/26 12:45, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [Critical] Unconditional self-rescheduling of the background workqueue leads to a use-after-free on device removal.
> - [Medium] The driver incorrectly detects and exposes unsupported voltage channels due to unverified firmware responses.
> --
> 
> commit aa874218705dba1c84bc41a7c3b61abd93bbf0a2
> Author: Shubham Chakraborty <chakrabortyshubham66@gmail.com>
> 
> hwmon: raspberrypi: Add voltage input support
> 
> This commit extends the raspberrypi-hwmon driver to expose firmware-provided
> voltage measurements. It adds core and SDRAM voltage inputs, converting
> the values from microvolts to millivolts for the hwmon subsystem.
> Inputs are only exposed if the firmware reports support for the corresponding
> voltage ID.
> 
>> diff --git a/drivers/hwmon/raspberrypi-hwmon.c b/drivers/hwmon/raspberrypi-hwmon.c
>> index a2938881ccd2f..4f96f37116f32 100644
>> --- a/drivers/hwmon/raspberrypi-hwmon.c
>> +++ b/drivers/hwmon/raspberrypi-hwmon.c
> 
> [ ... ]
> 
>> @@ -56,6 +70,23 @@ static void rpi_firmware_get_throttled(struct rpi_hwmon_data *data)
>>   	hwmon_notify_event(data->hwmon_dev, hwmon_in, hwmon_in_lcrit_alarm, 0);
>>   }
>>   
>> +static int rpi_firmware_get_voltage(struct rpi_hwmon_data *data, u32 id,
>> +				    long *val)
>> +{
>> +	struct rpi_firmware_get_value packet;
>> +	int ret;
>> +
>> +	packet.id = cpu_to_le32(id);
>> +	packet.val = 0;
>> +	ret = rpi_firmware_property(data->fw, RPI_FIRMWARE_GET_VOLTAGE,
>> +				    &packet, sizeof(packet));
> 
> If the firmware does not support the RPI_FIRMWARE_GET_VOLTAGE tag or the
> specific voltage ID, could the mailbox transaction still return 0 without
> modifying the tag buffer?
> 
> If so, packet.val would remain 0 and this function would return success.
> During probe, this would cause the channel to be incorrectly flagged as
> supported, exposing sysfs files that permanently read 0mV.
> 
> Would it be safer to initialize packet.val to a sentinel value like ~0
> before querying the firmware to verify the voltage ID is actually supported?
> 

I guess we have to assume (hope) that the firmware interface returns
an error in this case.

>> +	if (ret)
>> +		return ret;
>> +
>> +	*val = le32_to_cpu(packet.val) / 1000;
>> +	return 0;
>> +}
>> +
>>   static void get_values_poll(struct work_struct *work)
>>   {
>>   	struct rpi_hwmon_data *data;
> 
> Does the unconditional rescheduling in get_values_poll() cause a
> use-after-free regression during driver removal?
> 
> If the work function executes concurrently with driver unbind,
> cancel_delayed_work_sync() will wait for it to finish. However,
> get_values_poll() currently reschedules itself just before returning:
> 
> get_values_poll() {
>          ...
>          schedule_delayed_work(&data->get_values_poll_work, 2 * HZ);
> }
> 
> After cancellation completes, devres frees the memory for data. When the
> rescheduled work executes later, it accesses the freed data pointer.
> 
> Could a shutdown flag be added to safely prevent rescheduling during
> teardown?
> 

That does indeed look like a problem. Please feel free to submit a separate
patch to fix it.

Thanks,
Guenter

[PATCH v3 0/3] raspberrypi-hwmon voltage support and teardown fix

[Shubham Chakraborty]

This series adds firmware-backed voltage inputs to raspberrypi-hwmon and
includes a separate fix for the delayed polling work teardown path.

Patch 1 adds the firmware voltage IDs and the shared voltage request
structure to the Raspberry Pi firmware API header.

Patch 2 extends raspberrypi-hwmon to expose the firmware-provided core
and SDRAM voltage inputs through hwmon and documents the new sysfs
entries.

Patch 3 addresses the delayed polling work teardown concern raised
during review.

Changes in v3:
- corrected the SDRAM_P and SDRAM_I voltage ID mapping
- moved the voltage request structure into the firmware API header
- made the voltage request structure voltage-specific
- split the delayed-work teardown change into a separate patch

Tested on:
- Raspberry Pi 3B+ running Linux 6.12.75+rpt-rpi-v8

Shubham Chakraborty (3):
  soc: bcm2835: raspberrypi-firmware: Add voltage domain IDs
  hwmon: raspberrypi: Add voltage input support
  hwmon: raspberrypi: Fix delayed-work teardown race

 Documentation/hwmon/raspberrypi-hwmon.rst  |  15 ++-
 drivers/hwmon/raspberrypi-hwmon.c          | 139 ++++++++++++++++++++-
 include/soc/bcm2835/raspberrypi-firmware.h |  25 ++++
 3 files changed, 171 insertions(+), 8 deletions(-)

-- 
2.54.0

[PATCH v3 1/3] soc: bcm2835: raspberrypi-firmware: Add voltage domain IDs

[Shubham Chakraborty]

Add Raspberry Pi firmware voltage domain identifiers for the mailbox
property interface.

Also add the voltage request structure used with
RPI_FIRMWARE_GET_VOLTAGE so firmware clients can share the common API
definition from the firmware header.

Signed-off-by: Shubham Chakraborty <chakrabortyshubham66@gmail.com>
---
 include/soc/bcm2835/raspberrypi-firmware.h | 25 ++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/include/soc/bcm2835/raspberrypi-firmware.h b/include/soc/bcm2835/raspberrypi-firmware.h
index e1f87fbfe554..975bef529854 100644
--- a/include/soc/bcm2835/raspberrypi-firmware.h
+++ b/include/soc/bcm2835/raspberrypi-firmware.h
@@ -156,6 +156,31 @@ enum rpi_firmware_clk_id {
 	RPI_FIRMWARE_NUM_CLK_ID,
 };
 
+enum rpi_firmware_volt_id {
+	RPI_FIRMWARE_VOLT_ID_CORE = 1,
+	RPI_FIRMWARE_VOLT_ID_SDRAM_C = 2,
+	RPI_FIRMWARE_VOLT_ID_SDRAM_P = 3,
+	RPI_FIRMWARE_VOLT_ID_SDRAM_I = 4,
+	RPI_FIRMWARE_NUM_VOLT_ID,
+};
+
+/**
+ * struct rpi_firmware_get_voltage_request - Firmware request for a voltage
+ * @id:		ID of the voltage being queried
+ * @value:	Voltage in microvolts. Set by the firmware.
+ *
+ * Used by @RPI_FIRMWARE_GET_VOLTAGE.
+ */
+struct rpi_firmware_get_voltage_request {
+	__le32 id;
+	__le32 value;
+} __packed;
+
+#define RPI_FIRMWARE_GET_VOLTAGE_REQUEST(_id)	\
+	{					\
+		.id = cpu_to_le32(_id),		\
+	}
+
 /**
  * struct rpi_firmware_clk_rate_request - Firmware Request for a rate
  * @id:	ID of the clock being queried
-- 
2.54.0

[PATCH v3 2/3] hwmon: raspberrypi: Add voltage input support

[Shubham Chakraborty]

Extend the raspberrypi-hwmon driver to expose firmware-provided
voltage measurements through the hwmon subsystem.

The driver now exports the following voltage inputs:

  - in0_input (core)
  - in1_input (sdram_c)
  - in2_input (sdram_i)
  - in3_input (sdram_p)

Voltage values returned by firmware are converted from microvolts
to millivolts as expected by the hwmon subsystem.

Update the documentation related to it.

The existing undervoltage sticky alarm handling is preserved and
associated with the first voltage channel.

Tested in -
- Raspberry Pi 3b+ (Linux raspberrypi 6.12.75+rpt-rpi-v8 #1 SMP PREEMPT
  Debian 1:6.12.75-1+rpt1 (2026-03-11) aarch64 GNU/Linux)

Signed-off-by: Shubham Chakraborty <chakrabortyshubham66@gmail.com>
---
 Documentation/hwmon/raspberrypi-hwmon.rst |  15 ++-
 drivers/hwmon/raspberrypi-hwmon.c         | 127 +++++++++++++++++++++-
 2 files changed, 137 insertions(+), 5 deletions(-)

diff --git a/Documentation/hwmon/raspberrypi-hwmon.rst b/Documentation/hwmon/raspberrypi-hwmon.rst
index 8038ade36490..db315184b861 100644
--- a/Documentation/hwmon/raspberrypi-hwmon.rst
+++ b/Documentation/hwmon/raspberrypi-hwmon.rst
@@ -20,6 +20,17 @@ undervoltage conditions.
 Sysfs entries
 -------------
 
-======================= ==================
+======================= ======================================================
+in0_input		Core voltage in millivolts
+in1_input		SDRAM controller voltage in millivolts
+in2_input		SDRAM I/O voltage in millivolts
+in3_input		SDRAM PHY voltage in millivolts
+in0_label		"core"
+in1_label		"sdram_c"
+in2_label		"sdram_i"
+in3_label		"sdram_p"
 in0_lcrit_alarm		Undervoltage alarm
-======================= ==================
+======================= ======================================================
+
+The voltage inputs and labels are only exposed if the firmware reports support
+for the corresponding voltage ID.
diff --git a/drivers/hwmon/raspberrypi-hwmon.c b/drivers/hwmon/raspberrypi-hwmon.c
index a2938881ccd2..8ce6dacc19b0 100644
--- a/drivers/hwmon/raspberrypi-hwmon.c
+++ b/drivers/hwmon/raspberrypi-hwmon.c
@@ -5,6 +5,7 @@
  * Based on firmware/raspberrypi.c by Noralf Trønnes
  *
  * Copyright (C) 2018 Stefan Wahren <stefan.wahren@i2se.com>
+ * Copyright (C) 2026 Shubham Chakraborty <chakrabortyshubham66@gmail.com>
  */
 #include <linux/device.h>
 #include <linux/devm-helpers.h>
@@ -21,10 +22,18 @@
 struct rpi_hwmon_data {
 	struct device *hwmon_dev;
 	struct rpi_firmware *fw;
+	u32 valid_inputs;
 	u32 last_throttled;
 	struct delayed_work get_values_poll_work;
 };
 
+static const char * const rpi_hwmon_labels[] = {
+	"core",
+	"sdram_c",
+	"sdram_i",
+	"sdram_p",
+};
+
 static void rpi_firmware_get_throttled(struct rpi_hwmon_data *data)
 {
 	u32 new_uv, old_uv, value;
@@ -56,6 +65,21 @@ static void rpi_firmware_get_throttled(struct rpi_hwmon_data *data)
 	hwmon_notify_event(data->hwmon_dev, hwmon_in, hwmon_in_lcrit_alarm, 0);
 }
 
+static int rpi_firmware_get_voltage(struct rpi_hwmon_data *data, u32 id,
+				    long *val)
+{
+	struct rpi_firmware_get_voltage_request packet =
+		RPI_FIRMWARE_GET_VOLTAGE_REQUEST(id);
+	int ret;
+	ret = rpi_firmware_property(data->fw, RPI_FIRMWARE_GET_VOLTAGE,
+				    &packet, sizeof(packet));
+	if (ret)
+		return ret;
+
+	*val = le32_to_cpu(packet.value) / 1000;
+	return 0;
+}
+
 static void get_values_poll(struct work_struct *work)
 {
 	struct rpi_hwmon_data *data;
@@ -77,19 +101,94 @@ static int rpi_read(struct device *dev, enum hwmon_sensor_types type,
 {
 	struct rpi_hwmon_data *data = dev_get_drvdata(dev);
 
-	*val = !!(data->last_throttled & UNDERVOLTAGE_STICKY_BIT);
+	if (type == hwmon_in) {
+		switch (attr) {
+		case hwmon_in_input:
+			switch (channel) {
+			case 0:
+				return rpi_firmware_get_voltage(data,
+						RPI_FIRMWARE_VOLT_ID_CORE,
+						val);
+			case 1:
+				return rpi_firmware_get_voltage(data,
+						RPI_FIRMWARE_VOLT_ID_SDRAM_C,
+						val);
+			case 2:
+				return rpi_firmware_get_voltage(data,
+						RPI_FIRMWARE_VOLT_ID_SDRAM_I,
+						val);
+			case 3:
+				return rpi_firmware_get_voltage(data,
+						RPI_FIRMWARE_VOLT_ID_SDRAM_P,
+						val);
+			default:
+				return -EOPNOTSUPP;
+			}
+		case hwmon_in_lcrit_alarm:
+			if (channel == 0) {
+				*val = !!(data->last_throttled & UNDERVOLTAGE_STICKY_BIT);
+				return 0;
+			}
+			return -EOPNOTSUPP;
+		default:
+			return -EOPNOTSUPP;
+		}
+	}
+
+	return -EOPNOTSUPP;
+}
+
+static int rpi_read_string(struct device *dev, enum hwmon_sensor_types type,
+			   u32 attr, int channel, const char **str)
+{
+	if (type == hwmon_in && attr == hwmon_in_label) {
+		if (channel >= ARRAY_SIZE(rpi_hwmon_labels))
+			return -EOPNOTSUPP;
+
+		*str = rpi_hwmon_labels[channel];
+		return 0;
+	}
+
+	return -EOPNOTSUPP;
+}
+
+static umode_t rpi_is_visible(const void *_data, enum hwmon_sensor_types type,
+			      u32 attr, int channel)
+{
+	const struct rpi_hwmon_data *data = _data;
+
+	if (type == hwmon_in) {
+		switch (attr) {
+		case hwmon_in_input:
+		case hwmon_in_label:
+			if (!(data->valid_inputs & BIT(channel)))
+				return 0;
+			return 0444;
+		case hwmon_in_lcrit_alarm:
+			if (channel == 0)
+				return 0444;
+			return 0;
+		default:
+			return 0;
+		}
+	}
+
 	return 0;
 }
 
 static const struct hwmon_channel_info * const rpi_info[] = {
 	HWMON_CHANNEL_INFO(in,
-			   HWMON_I_LCRIT_ALARM),
+			   HWMON_I_INPUT | HWMON_I_LABEL | HWMON_I_LCRIT_ALARM,
+			   HWMON_I_INPUT | HWMON_I_LABEL,
+			   HWMON_I_INPUT | HWMON_I_LABEL,
+			   HWMON_I_INPUT | HWMON_I_LABEL),
 	NULL
 };
 
 static const struct hwmon_ops rpi_hwmon_ops = {
-	.visible = 0444,
+	.is_visible = rpi_is_visible,
 	.read = rpi_read,
+	.read_string = rpi_read_string,
 };
 
 static const struct hwmon_chip_info rpi_chip_info = {
@@ -101,6 +200,7 @@ static int rpi_hwmon_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct rpi_hwmon_data *data;
+	long voltage;
 	int ret;
 
 	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
@@ -110,6 +210,26 @@ static int rpi_hwmon_probe(struct platform_device *pdev)
 	/* Parent driver assure that firmware is correct */
 	data->fw = dev_get_drvdata(dev->parent);
 
+	ret = rpi_firmware_get_voltage(data, RPI_FIRMWARE_VOLT_ID_CORE,
+				       &voltage);
+	if (!ret)
+		data->valid_inputs |= BIT(0);
+
+	ret = rpi_firmware_get_voltage(data, RPI_FIRMWARE_VOLT_ID_SDRAM_C,
+				       &voltage);
+	if (!ret)
+		data->valid_inputs |= BIT(1);
+
+	ret = rpi_firmware_get_voltage(data, RPI_FIRMWARE_VOLT_ID_SDRAM_I,
+				       &voltage);
+	if (!ret)
+		data->valid_inputs |= BIT(2);
+
+	ret = rpi_firmware_get_voltage(data, RPI_FIRMWARE_VOLT_ID_SDRAM_P,
+				       &voltage);
+	if (!ret)
+		data->valid_inputs |= BIT(3);
+
 	data->hwmon_dev = devm_hwmon_device_register_with_info(dev, "rpi_volt",
 							       data,
 							       &rpi_chip_info,
@@ -159,6 +279,7 @@ static struct platform_driver rpi_hwmon_driver = {
 module_platform_driver(rpi_hwmon_driver);
 
 MODULE_AUTHOR("Stefan Wahren <wahrenst@gmx.net>");
+MODULE_AUTHOR("Shubham Chakraborty <chakrabortyshubham66@gmail.com>");
 MODULE_DESCRIPTION("Raspberry Pi voltage sensor driver");
 MODULE_LICENSE("GPL v2");
 MODULE_ALIAS("platform:raspberrypi-hwmon");
-- 
2.54.0

[PATCH v3 3/3] hwmon: raspberrypi: Fix delayed-work teardown race

[Shubham Chakraborty]

The delayed polling work rearms itself from the work function, so use
explicit delayed-work setup and cleanup instead of
devm_delayed_work_autocancel().

Initialize the delayed work with INIT_DELAYED_WORK() and register a
devres cleanup action that calls disable_delayed_work_sync() during
teardown.

This addresses the concern raised during review about the polling work
being able to requeue itself while the driver is being removed.

Signed-off-by: Shubham Chakraborty <chakrabortyshubham66@gmail.com>
---
 drivers/hwmon/raspberrypi-hwmon.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/hwmon/raspberrypi-hwmon.c b/drivers/hwmon/raspberrypi-hwmon.c
index 8ce6dacc19b0..0bbc735f74a4 100644
--- a/drivers/hwmon/raspberrypi-hwmon.c
+++ b/drivers/hwmon/raspberrypi-hwmon.c
@@ -8,7 +8,6 @@
  * Copyright (C) 2026 Shubham Chakraborty <chakrabortyshubham66@gmail.com>
  */
 #include <linux/device.h>
-#include <linux/devm-helpers.h>
 #include <linux/err.h>
 #include <linux/hwmon.h>
 #include <linux/module.h>
@@ -96,6 +95,13 @@ static void get_values_poll(struct work_struct *work)
 	schedule_delayed_work(&data->get_values_poll_work, 2 * HZ);
 }
 
+static void rpi_hwmon_cancel_poll_work(void *res)
+{
+	struct rpi_hwmon_data *data = res;
+
+	disable_delayed_work_sync(&data->get_values_poll_work);
+}
+
 static int rpi_read(struct device *dev, enum hwmon_sensor_types type,
 		    u32 attr, int channel, long *val)
 {
@@ -237,8 +243,8 @@ static int rpi_hwmon_probe(struct platform_device *pdev)
 	if (IS_ERR(data->hwmon_dev))
 		return PTR_ERR(data->hwmon_dev);
 
-	ret = devm_delayed_work_autocancel(dev, &data->get_values_poll_work,
-					   get_values_poll);
+	INIT_DELAYED_WORK(&data->get_values_poll_work, get_values_poll);
+	ret = devm_add_action_or_reset(dev, rpi_hwmon_cancel_poll_work, data);
 	if (ret)
 		return ret;
 	platform_set_drvdata(pdev, data);
-- 
2.54.0

Re: [PATCH v3 1/3] soc: bcm2835: raspberrypi-firmware: Add voltage domain IDs

[Guenter Roeck]

On Sun, May 17, 2026 at 01:34:43PM +0530, Shubham Chakraborty wrote:
> Add Raspberry Pi firmware voltage domain identifiers for the mailbox
> property interface.
> 
> Also add the voltage request structure used with
> RPI_FIRMWARE_GET_VOLTAGE so firmware clients can share the common API
> definition from the firmware header.
> 
> Signed-off-by: Shubham Chakraborty <chakrabortyshubham66@gmail.com>

I'll need an Acked-by: from a maintainer to apply this patch,
or some other maintainer will have to pick it up.

Thanks,
Guenter

> ---
>  include/soc/bcm2835/raspberrypi-firmware.h | 25 ++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
> 
> diff --git a/include/soc/bcm2835/raspberrypi-firmware.h b/include/soc/bcm2835/raspberrypi-firmware.h
> index e1f87fbfe554..975bef529854 100644
> --- a/include/soc/bcm2835/raspberrypi-firmware.h
> +++ b/include/soc/bcm2835/raspberrypi-firmware.h
> @@ -156,6 +156,31 @@ enum rpi_firmware_clk_id {
>  	RPI_FIRMWARE_NUM_CLK_ID,
>  };
>  
> +enum rpi_firmware_volt_id {
> +	RPI_FIRMWARE_VOLT_ID_CORE = 1,
> +	RPI_FIRMWARE_VOLT_ID_SDRAM_C = 2,
> +	RPI_FIRMWARE_VOLT_ID_SDRAM_P = 3,
> +	RPI_FIRMWARE_VOLT_ID_SDRAM_I = 4,
> +	RPI_FIRMWARE_NUM_VOLT_ID,
> +};
> +
> +/**
> + * struct rpi_firmware_get_voltage_request - Firmware request for a voltage
> + * @id:		ID of the voltage being queried
> + * @value:	Voltage in microvolts. Set by the firmware.
> + *
> + * Used by @RPI_FIRMWARE_GET_VOLTAGE.
> + */
> +struct rpi_firmware_get_voltage_request {
> +	__le32 id;
> +	__le32 value;
> +} __packed;
> +
> +#define RPI_FIRMWARE_GET_VOLTAGE_REQUEST(_id)	\
> +	{					\
> +		.id = cpu_to_le32(_id),		\
> +	}
> +
>  /**
>   * struct rpi_firmware_clk_rate_request - Firmware Request for a rate
>   * @id:	ID of the clock being queried

Re: [PATCH v3 2/3] hwmon: raspberrypi: Add voltage input support

[Florian Fainelli]

On 5/17/26 01:04, Shubham Chakraborty wrote:
> Extend the raspberrypi-hwmon driver to expose firmware-provided
> voltage measurements through the hwmon subsystem.
> 
> The driver now exports the following voltage inputs:
> 
>    - in0_input (core)
>    - in1_input (sdram_c)
>    - in2_input (sdram_i)
>    - in3_input (sdram_p)
> 
> Voltage values returned by firmware are converted from microvolts
> to millivolts as expected by the hwmon subsystem.
> 
> Update the documentation related to it.
> 
> The existing undervoltage sticky alarm handling is preserved and
> associated with the first voltage channel.
> 
> Tested in -
> - Raspberry Pi 3b+ (Linux raspberrypi 6.12.75+rpt-rpi-v8 #1 SMP PREEMPT
>    Debian 1:6.12.75-1+rpt1 (2026-03-11) aarch64 GNU/Linux)
> 
> Signed-off-by: Shubham Chakraborty <chakrabortyshubham66@gmail.com>

Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
-- 
Florian

Re: [PATCH v3 1/3] soc: bcm2835: raspberrypi-firmware: Add voltage domain IDs

[Florian Fainelli]

On 5/17/26 01:04, Shubham Chakraborty wrote:
> Add Raspberry Pi firmware voltage domain identifiers for the mailbox
> property interface.
> 
> Also add the voltage request structure used with
> RPI_FIRMWARE_GET_VOLTAGE so firmware clients can share the common API
> definition from the firmware header.
> 
> Signed-off-by: Shubham Chakraborty <chakrabortyshubham66@gmail.com>

Acked-by: Florian Fainelli <florian.fainelli@broadcom.com>
-- 
Florian

Re: [PATCH v3 1/3] soc: bcm2835: raspberrypi-firmware: Add voltage domain IDs

[Guenter Roeck]

On Sun, May 17, 2026 at 01:34:43PM +0530, Shubham Chakraborty wrote:
> Add Raspberry Pi firmware voltage domain identifiers for the mailbox
> property interface.
> 
> Also add the voltage request structure used with
> RPI_FIRMWARE_GET_VOLTAGE so firmware clients can share the common API
> definition from the firmware header.
> 
> Signed-off-by: Shubham Chakraborty <chakrabortyshubham66@gmail.com>
> Acked-by: Florian Fainelli <florian.fainelli@broadcom.com>

Applied.

Thanks,
Guenter

Re: [PATCH v3 2/3] hwmon: raspberrypi: Add voltage input support

[Guenter Roeck]

On Sun, May 17, 2026 at 01:34:44PM +0530, Shubham Chakraborty wrote:
> Extend the raspberrypi-hwmon driver to expose firmware-provided
> voltage measurements through the hwmon subsystem.
> 
> The driver now exports the following voltage inputs:
> 
>   - in0_input (core)
>   - in1_input (sdram_c)
>   - in2_input (sdram_i)
>   - in3_input (sdram_p)
> 
> Voltage values returned by firmware are converted from microvolts
> to millivolts as expected by the hwmon subsystem.
> 
> Update the documentation related to it.
> 
> The existing undervoltage sticky alarm handling is preserved and
> associated with the first voltage channel.
> 
> Tested in -
> - Raspberry Pi 3b+ (Linux raspberrypi 6.12.75+rpt-rpi-v8 #1 SMP PREEMPT
>   Debian 1:6.12.75-1+rpt1 (2026-03-11) aarch64 GNU/Linux)
> 
> Signed-off-by: Shubham Chakraborty <chakrabortyshubham66@gmail.com>
> Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>

Applied, after fixing:

WARNING: Missing a blank line after declarations
#207: FILE: drivers/hwmon/raspberrypi-hwmon.c:74:
+	int ret;
+	ret = rpi_firmware_property(data->fw, RPI_FIRMWARE_GET_VOLTAGE,

Please run checkpatch on your patches.

Thanks,
Guenter

Re: [PATCH v3 3/3] hwmon: raspberrypi: Fix delayed-work teardown race

[Guenter Roeck]

On Sun, May 17, 2026 at 01:34:45PM +0530, Shubham Chakraborty wrote:
> The delayed polling work rearms itself from the work function, so use
> explicit delayed-work setup and cleanup instead of
> devm_delayed_work_autocancel().
> 
> Initialize the delayed work with INIT_DELAYED_WORK() and register a
> devres cleanup action that calls disable_delayed_work_sync() during
> teardown.
> 
> This addresses the concern raised during review about the polling work
> being able to requeue itself while the driver is being removed.
> 
> Signed-off-by: Shubham Chakraborty <chakrabortyshubham66@gmail.com>

Applied.

Thanks,
Guenter