Integrates Vcore, VDIMM, 3.3V, 5V, 12V voltage and system temperature
monitoring into the driver via the hwmon subsystem, enabling
standardized reporting via tools like lm-sensors.
Signed-off-by: Yen-Chi Huang <jesse.huang@portwell.com.tw>
---
[Re-sending to fix message threading, no content changes since v2.]
---
drivers/platform/x86/portwell-ec.c | 178 ++++++++++++++++++++++++++++-
1 file changed, 176 insertions(+), 2 deletions(-)
diff --git a/drivers/platform/x86/portwell-ec.c b/drivers/platform/x86/portwell-ec.c
index a68522aaa3fa..ac113aaf8bb0 100644
--- a/drivers/platform/x86/portwell-ec.c
+++ b/drivers/platform/x86/portwell-ec.c
@@ -25,6 +25,7 @@
#include <linux/bitfield.h>
#include <linux/dmi.h>
#include <linux/gpio/driver.h>
+#include <linux/hwmon.h>
#include <linux/init.h>
#include <linux/io.h>
#include <linux/ioport.h>
@@ -52,16 +53,64 @@
#define PORTWELL_EC_FW_VENDOR_LENGTH 3
#define PORTWELL_EC_FW_VENDOR_NAME "PWG"
+#define PORTWELL_EC_ADC_MAX 1023
+
static bool force;
module_param(force, bool, 0444);
MODULE_PARM_DESC(force, "Force loading EC driver without checking DMI boardname");
+struct pwec_hwmon_data {
+ const char *label;
+ u8 lsb_reg;
+ u32 scale;
+};
+
+struct pwec_data {
+ const struct pwec_hwmon_data *hwmon_in_data;
+ int hwmon_in_num;
+ const struct pwec_hwmon_data *hwmon_temp_data;
+ int hwmon_temp_num;
+ const struct hwmon_channel_info * const *hwmon_info;
+};
+
+static const struct pwec_hwmon_data pwec_nano_hwmon_in[] = {
+ { "Vcore", 0x20, 3000 },
+ { "VDIMM", 0x32, 3000 },
+ { "3.3V", 0x22, 6000 },
+ { "5V", 0x24, 9600 },
+ { "12V", 0x30, 19800 },
+};
+
+static const struct pwec_hwmon_data pwec_nano_hwmon_temp[] = {
+ { "System Temperature", 0x02, 0 },
+};
+
+static const struct hwmon_channel_info *pwec_nano_hwmon_info[] = {
+ HWMON_CHANNEL_INFO(temp, HWMON_T_INPUT | HWMON_T_LABEL),
+ 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_I_INPUT | HWMON_I_LABEL),
+ NULL
+};
+
+static const struct pwec_data pwec_board_data_nano = {
+ .hwmon_in_data = pwec_nano_hwmon_in,
+ .hwmon_in_num = ARRAY_SIZE(pwec_nano_hwmon_in),
+ .hwmon_temp_data = pwec_nano_hwmon_temp,
+ .hwmon_temp_num = ARRAY_SIZE(pwec_nano_hwmon_temp),
+ .hwmon_info = pwec_nano_hwmon_info
+};
+
static const struct dmi_system_id pwec_dmi_table[] = {
{
.ident = "NANO-6064 series",
.matches = {
DMI_MATCH(DMI_BOARD_NAME, "NANO-6064"),
},
+ .driver_data = (void *)&pwec_board_data_nano,
},
{ }
};
@@ -79,6 +128,19 @@ static u8 pwec_read(u8 address)
return inb(PORTWELL_EC_IOSPACE + address);
}
+static u16 pwec_read16_stable(u8 lsb_reg)
+{
+ u8 lsb, msb, old_msb;
+
+ do {
+ old_msb = pwec_read(lsb_reg+1);
+ lsb = pwec_read(lsb_reg);
+ msb = pwec_read(lsb_reg+1);
+ } while (msb != old_msb);
+
+ return (msb << 8) | lsb;
+}
+
/* GPIO functions */
static int pwec_gpio_get(struct gpio_chip *chip, unsigned int offset)
@@ -204,6 +266,110 @@ static struct watchdog_device ec_wdt_dev = {
.max_timeout = PORTWELL_WDT_EC_MAX_COUNT_SECOND,
};
+/* HWMON functions */
+
+static umode_t pwec_hwmon_is_visible(const void *data, enum hwmon_sensor_types type,
+ u32 attr, int channel)
+{
+ const struct pwec_data *d = data;
+
+ switch (type) {
+ case hwmon_temp:
+ if (channel < d->hwmon_temp_num)
+ return 0444;
+ break;
+ case hwmon_in:
+ if (channel < d->hwmon_in_num)
+ return 0444;
+ break;
+ default:
+ break;
+ }
+
+ return 0;
+}
+
+static int pwec_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
+ u32 attr, int channel, long *val)
+{
+ struct pwec_data *data = dev_get_drvdata(dev);
+ u16 tmp;
+
+ switch (type) {
+ case hwmon_temp:
+ if (channel < data->hwmon_temp_num) {
+ *val = pwec_read(data->hwmon_temp_data[channel].lsb_reg) * 1000;
+ return 0;
+ }
+ break;
+ case hwmon_in:
+ if (channel < data->hwmon_in_num) {
+ tmp = pwec_read16_stable(data->hwmon_in_data[channel].lsb_reg);
+ *val = (data->hwmon_in_data[channel].scale * tmp) / PORTWELL_EC_ADC_MAX;
+ return 0;
+ }
+ break;
+ default:
+ break;
+ }
+
+ return -EOPNOTSUPP;
+}
+
+static int pwec_hwmon_read_string(struct device *dev, enum hwmon_sensor_types type,
+ u32 attr, int channel, const char **str)
+{
+ struct pwec_data *data = dev_get_drvdata(dev);
+
+ switch (type) {
+ case hwmon_temp:
+ if (channel < data->hwmon_temp_num) {
+ *str = data->hwmon_temp_data[channel].label;
+ return 0;
+ }
+ break;
+ case hwmon_in:
+ if (channel < data->hwmon_in_num) {
+ *str = data->hwmon_in_data[channel].label;
+ return 0;
+ }
+ break;
+ default:
+ break;
+ }
+
+ return -EOPNOTSUPP;
+}
+
+static const struct hwmon_ops pwec_hwmon_ops = {
+ .is_visible = pwec_hwmon_is_visible,
+ .read = pwec_hwmon_read,
+ .read_string = pwec_hwmon_read_string,
+};
+
+static struct hwmon_chip_info pwec_chip_info = {
+ .ops = &pwec_hwmon_ops,
+};
+
+static int pwec_hwmon_init(struct device *dev)
+{
+ struct pwec_data *data = dev_get_platdata(dev);
+ void *hwmon;
+ int ret;
+
+ if (!IS_REACHABLE(CONFIG_HWMON))
+ return 0;
+
+ pwec_chip_info.info = data->hwmon_info;
+ hwmon = devm_hwmon_device_register_with_info(dev, "portwell_ec", data, &pwec_chip_info,
+ NULL);
+ ret = PTR_ERR_OR_ZERO(hwmon);
+ if (ret)
+ dev_err(dev, "Failed to register hwmon_dev: %d\n", ret);
+
+ return ret;
+}
+
static int pwec_firmware_vendor_check(void)
{
u8 buf[PORTWELL_EC_FW_VENDOR_LENGTH + 1];
@@ -242,6 +408,10 @@ static int pwec_probe(struct platform_device *pdev)
return ret;
}
+ ret = pwec_hwmon_init(&pdev->dev);
+ if (ret < 0)
+ return ret;
+
return 0;
}
@@ -274,11 +444,14 @@ static struct platform_device *pwec_dev;
static int __init pwec_init(void)
{
+ const struct dmi_system_id *match;
int ret;
- if (!dmi_check_system(pwec_dmi_table)) {
+ match = dmi_first_match(pwec_dmi_table);
+ if (!match) {
if (!force)
return -ENODEV;
+ match = &pwec_dmi_table[0];
pr_warn("force load portwell-ec without DMI check\n");
}
@@ -286,7 +459,8 @@ static int __init pwec_init(void)
if (ret)
return ret;
- pwec_dev = platform_device_register_simple("portwell-ec", -1, NULL, 0);
+ pwec_dev = platform_device_register_data(NULL, "portwell-ec", -1, match->driver_data,
+ sizeof(struct pwec_data));
if (IS_ERR(pwec_dev)) {
platform_driver_unregister(&pwec_driver);
return PTR_ERR(pwec_dev);
--
2.34.1On Tue, 15 Jul 2025, Yen-Chi Huang wrote:
> Integrates Vcore, VDIMM, 3.3V, 5V, 12V voltage and system temperature
> monitoring into the driver via the hwmon subsystem, enabling
> standardized reporting via tools like lm-sensors.
>
> Signed-off-by: Yen-Chi Huang <jesse.huang@portwell.com.tw>
> ---
>
> [Re-sending to fix message threading, no content changes since v2.]
> ---
> drivers/platform/x86/portwell-ec.c | 178 ++++++++++++++++++++++++++++-
> 1 file changed, 176 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/platform/x86/portwell-ec.c b/drivers/platform/x86/portwell-ec.c
> index a68522aaa3fa..ac113aaf8bb0 100644
> --- a/drivers/platform/x86/portwell-ec.c
> +++ b/drivers/platform/x86/portwell-ec.c
> @@ -25,6 +25,7 @@
> #include <linux/bitfield.h>
> #include <linux/dmi.h>
> #include <linux/gpio/driver.h>
> +#include <linux/hwmon.h>
> #include <linux/init.h>
> #include <linux/io.h>
> #include <linux/ioport.h>
> @@ -52,16 +53,64 @@
> #define PORTWELL_EC_FW_VENDOR_LENGTH 3
> #define PORTWELL_EC_FW_VENDOR_NAME "PWG"
>
> +#define PORTWELL_EC_ADC_MAX 1023
> +
> static bool force;
> module_param(force, bool, 0444);
> MODULE_PARM_DESC(force, "Force loading EC driver without checking DMI boardname");
>
> +struct pwec_hwmon_data {
> + const char *label;
> + u8 lsb_reg;
> + u32 scale;
> +};
> +
> +struct pwec_data {
> + const struct pwec_hwmon_data *hwmon_in_data;
> + int hwmon_in_num;
> + const struct pwec_hwmon_data *hwmon_temp_data;
> + int hwmon_temp_num;
> + const struct hwmon_channel_info * const *hwmon_info;
> +};
> +
> +static const struct pwec_hwmon_data pwec_nano_hwmon_in[] = {
> + { "Vcore", 0x20, 3000 },
> + { "VDIMM", 0x32, 3000 },
> + { "3.3V", 0x22, 6000 },
> + { "5V", 0x24, 9600 },
> + { "12V", 0x30, 19800 },
> +};
> +
> +static const struct pwec_hwmon_data pwec_nano_hwmon_temp[] = {
> + { "System Temperature", 0x02, 0 },
> +};
> +
> +static const struct hwmon_channel_info *pwec_nano_hwmon_info[] = {
> + HWMON_CHANNEL_INFO(temp, HWMON_T_INPUT | HWMON_T_LABEL),
> + 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_I_INPUT | HWMON_I_LABEL),
> + NULL
> +};
> +
> +static const struct pwec_data pwec_board_data_nano = {
> + .hwmon_in_data = pwec_nano_hwmon_in,
> + .hwmon_in_num = ARRAY_SIZE(pwec_nano_hwmon_in),
> + .hwmon_temp_data = pwec_nano_hwmon_temp,
> + .hwmon_temp_num = ARRAY_SIZE(pwec_nano_hwmon_temp),
> + .hwmon_info = pwec_nano_hwmon_info
Please add comma to any that is not a real terminator so that a future
changes won't need to add the comma if more fields get added.
> +};
> +
> static const struct dmi_system_id pwec_dmi_table[] = {
> {
> .ident = "NANO-6064 series",
> .matches = {
> DMI_MATCH(DMI_BOARD_NAME, "NANO-6064"),
> },
> + .driver_data = (void *)&pwec_board_data_nano,
Casting a pointer to void * is not required.
> },
> { }
> };
> @@ -79,6 +128,19 @@ static u8 pwec_read(u8 address)
> return inb(PORTWELL_EC_IOSPACE + address);
> }
>
> +static u16 pwec_read16_stable(u8 lsb_reg)
> +{
> + u8 lsb, msb, old_msb;
> +
> + do {
> + old_msb = pwec_read(lsb_reg+1);
Please add spaces around + as per the coding style guidance.
> + lsb = pwec_read(lsb_reg);
> + msb = pwec_read(lsb_reg+1);
Ditto.
> + } while (msb != old_msb);
> +
> + return (msb << 8) | lsb;
> +}
> +
> /* GPIO functions */
>
> static int pwec_gpio_get(struct gpio_chip *chip, unsigned int offset)
> @@ -204,6 +266,110 @@ static struct watchdog_device ec_wdt_dev = {
> .max_timeout = PORTWELL_WDT_EC_MAX_COUNT_SECOND,
> };
>
> +/* HWMON functions */
> +
> +static umode_t pwec_hwmon_is_visible(const void *data, enum hwmon_sensor_types type,
> + u32 attr, int channel)
> +{
> + const struct pwec_data *d = data;
> +
> + switch (type) {
> + case hwmon_temp:
> + if (channel < d->hwmon_temp_num)
> + return 0444;
> + break;
I'd suggest you change these to:
return channel < d->hwmon_temp_num ? 0444 : 0;
> + case hwmon_in:
> + if (channel < d->hwmon_in_num)
> + return 0444;
> + break;
> + default:
> + break;
...and this to direct return 0; to simplify the code flow.
> + }
> +
> + return 0;
> +}
> +
> +static int pwec_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
> + u32 attr, int channel, long *val)
> +{
> + struct pwec_data *data = dev_get_drvdata(dev);
> + u16 tmp;
> +
> + switch (type) {
> + case hwmon_temp:
> + if (channel < data->hwmon_temp_num) {
> + *val = pwec_read(data->hwmon_temp_data[channel].lsb_reg) * 1000;
There might have been some problem in preparing this series as literal
1000 is still there despite your cover letter suggesting it was changed?
Please check the other expected changes as well, on a glance they seemed
to be in place but it has been a while since I've looked on this patch.
> + return 0;
> + }
> + break;
> + case hwmon_in:
> + if (channel < data->hwmon_in_num) {
> + tmp = pwec_read16_stable(data->hwmon_in_data[channel].lsb_reg);
> + *val = (data->hwmon_in_data[channel].scale * tmp) / PORTWELL_EC_ADC_MAX;
> + return 0;
> + }
> + break;
> + default:
> + break;
> + }
> +
> + return -EOPNOTSUPP;
> +}
> +
> +static int pwec_hwmon_read_string(struct device *dev, enum hwmon_sensor_types type,
> + u32 attr, int channel, const char **str)
> +{
> + struct pwec_data *data = dev_get_drvdata(dev);
> +
> + switch (type) {
> + case hwmon_temp:
> + if (channel < data->hwmon_temp_num) {
> + *str = data->hwmon_temp_data[channel].label;
> + return 0;
> + }
> + break;
> + case hwmon_in:
> + if (channel < data->hwmon_in_num) {
> + *str = data->hwmon_in_data[channel].label;
> + return 0;
> + }
> + break;
> + default:
> + break;
> + }
> +
> + return -EOPNOTSUPP;
> +}
> +
> +static const struct hwmon_ops pwec_hwmon_ops = {
> + .is_visible = pwec_hwmon_is_visible,
> + .read = pwec_hwmon_read,
> + .read_string = pwec_hwmon_read_string,
> +};
> +
> +static struct hwmon_chip_info pwec_chip_info = {
> + .ops = &pwec_hwmon_ops,
> +};
> +
> +static int pwec_hwmon_init(struct device *dev)
> +{
> + struct pwec_data *data = dev_get_platdata(dev);
> + void *hwmon;
> + int ret;
> +
> + if (!IS_REACHABLE(CONFIG_HWMON))
> + return 0;
> +
> + pwec_chip_info.info = data->hwmon_info;
> + hwmon = devm_hwmon_device_register_with_info(dev, "portwell_ec", data, &pwec_chip_info,
> + NULL);
> + ret = PTR_ERR_OR_ZERO(hwmon);
> + if (ret)
> + dev_err(dev, "Failed to register hwmon_dev: %d\n", ret);
> +
> + return ret;
> +}
> +
> static int pwec_firmware_vendor_check(void)
> {
> u8 buf[PORTWELL_EC_FW_VENDOR_LENGTH + 1];
> @@ -242,6 +408,10 @@ static int pwec_probe(struct platform_device *pdev)
> return ret;
> }
>
> + ret = pwec_hwmon_init(&pdev->dev);
> + if (ret < 0)
> + return ret;
> +
> return 0;
> }
>
> @@ -274,11 +444,14 @@ static struct platform_device *pwec_dev;
>
> static int __init pwec_init(void)
> {
> + const struct dmi_system_id *match;
> int ret;
>
> - if (!dmi_check_system(pwec_dmi_table)) {
> + match = dmi_first_match(pwec_dmi_table);
> + if (!match) {
> if (!force)
> return -ENODEV;
> + match = &pwec_dmi_table[0];
> pr_warn("force load portwell-ec without DMI check\n");
> }
>
> @@ -286,7 +459,8 @@ static int __init pwec_init(void)
> if (ret)
> return ret;
>
> - pwec_dev = platform_device_register_simple("portwell-ec", -1, NULL, 0);
> + pwec_dev = platform_device_register_data(NULL, "portwell-ec", -1, match->driver_data,
> + sizeof(struct pwec_data));
> if (IS_ERR(pwec_dev)) {
> platform_driver_unregister(&pwec_driver);
> return PTR_ERR(pwec_dev);
>
--
i.
Hi Ilpo,
Thanks for the thorough review, and sorry for the basic issues.
On 7/21/2025 9:56 PM, Ilpo Jarvinen wrote:
> On Tue, 15 Jul 2025, Yen-Chi Huang wrote:
>
>> +static const struct pwec_data pwec_board_data_nano = {
>> + .hwmon_in_data = pwec_nano_hwmon_in,
>> + .hwmon_in_num = ARRAY_SIZE(pwec_nano_hwmon_in),
>> + .hwmon_temp_data = pwec_nano_hwmon_temp,
>> + .hwmon_temp_num = ARRAY_SIZE(pwec_nano_hwmon_temp),
>> + .hwmon_info = pwec_nano_hwmon_info
>
> Please add comma to any that is not a real terminator so that a future
> changes won't need to add the comma if more fields get added.
Will fix in patch v3.
>> static const struct dmi_system_id pwec_dmi_table[] = {
>> {
>> .ident = "NANO-6064 series",
>> .matches = {
>> DMI_MATCH(DMI_BOARD_NAME, "NANO-6064"),
>> },
>> + .driver_data = (void *)&pwec_board_data_nano,
>
> Casting a pointer to void * is not required.
`pwec_board_data_nano` is declared `const`, so dropping the cast produces:
warning: initialization discards 'const' qualifier from pointer target type
Hence the explicit `(void *)` cast is needed to avoid the warning while
keeping the data read-only.
>> + old_msb = pwec_read(lsb_reg+1);
>
> Please add spaces around + as per the coding style guidance.
>
>> + lsb = pwec_read(lsb_reg);
>> + msb = pwec_read(lsb_reg+1);
>
> Ditto.
Will fix in patch v3.
>> + switch (type) {
>> + case hwmon_temp:
>> + if (channel < d->hwmon_temp_num)
>> + return 0444;
>> + break;
>
> I'd suggest you change these to:
>
> return channel < d->hwmon_temp_num ? 0444 : 0;
>
>> + case hwmon_in:
>> + if (channel < d->hwmon_in_num)
>> + return 0444;
>> + break;
>> + default:
>> + break;
>
> ...and this to direct return 0; to simplify the code flow.
Will update as suggested in patch v3.
>> + switch (type) {
>> + case hwmon_temp:
>> + if (channel < data->hwmon_temp_num) {
>> + *val = pwec_read(data->hwmon_temp_data[channel].lsb_reg) * 1000;
>
> There might have been some problem in preparing this series as literal
> 1000 is still there despite your cover letter suggesting it was changed?
>
> Please check the other expected changes as well, on a glance they seemed
> to be in place but it has been a while since I've looked on this patch.
Will replace the literal with `MILLIDEGREE_PER_DEGREE` from
`<linux/units.h>` and double-check all other expected changes in v3.
Best regards,
Yen-Chi Huang
© 2016 - 2025 Red Hat, Inc.