[PATCH 2/2] hwmon: raspberrypi: Add voltage input support
Guenter Roeck
linux at roeck-us.net
Sat May 16 11:24:59 PDT 2026
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 at 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 at i2se.com>
>> + * Copyright (C) 2026 Shubham Chakraborty <chakrabortyshubham66 at 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 at gmx.net>");
>> +MODULE_AUTHOR("Shubham Chakraborty <chakrabortyshubham66 at gmail.com>");
>> MODULE_DESCRIPTION("Raspberry Pi voltage sensor driver");
>> MODULE_LICENSE("GPL v2");
>> MODULE_ALIAS("platform:raspberrypi-hwmon");
>
>
More information about the linux-rpi-kernel
mailing list