| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <67eb366e-1207-4e9a-9659-3482c8a9ec9b@roeck-us.net>
Date: Mon, 12 Jan 2026 07:26:49 -0800
From: Guenter Roeck <linux@...ck-us.net>
To: Vasileios Amoiridis <vassilisamir@...il.com>, robh@...nel.org,
krzk+dt@...nel.org, conor+dt@...nel.org
Cc: devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-hwmon@...r.kernel.org, linux-doc@...r.kernel.org,
Vasileios Amoiridis <vasileios.amoiridis@...n.ch>
Subject: Re: [PATCH v1 2/2] hwmon: Add support for HiTRON HAC300S PSU
On 1/6/26 08:03, Vasileios Amoiridis wrote:
> From: Vasileios Amoiridis <vasileios.amoiridis@...n.ch>
>
> Add Support for HiTRON HAC300S PSU. This is a AC/DC hot-swappable
> CompactPCI Serial Dual output active current sharing switching power
> supply with a 312W rating.
>
> Signed-off-by: Vasileios Amoiridis <vasileios.amoiridis@...n.ch>
> ---
> Documentation/hwmon/hac300s.rst | 37 ++++++++
Needs to be added to index.rst.
> MAINTAINERS | 7 ++
> drivers/hwmon/pmbus/Kconfig | 9 ++
> drivers/hwmon/pmbus/Makefile | 1 +
> drivers/hwmon/pmbus/hac300s.c | 152 ++++++++++++++++++++++++++++++++
> 5 files changed, 206 insertions(+)
> create mode 100644 Documentation/hwmon/hac300s.rst
> create mode 100644 drivers/hwmon/pmbus/hac300s.c
>
> diff --git a/Documentation/hwmon/hac300s.rst b/Documentation/hwmon/hac300s.rst
> new file mode 100644
> index 000000000000..573269fc81f8
> --- /dev/null
> +++ b/Documentation/hwmon/hac300s.rst
> @@ -0,0 +1,37 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +Kernel driver hac300s
> +=====================
> +
> +Supported chips:
> +
> + * HiTRON HAC300S
> +
> + Prefix: 'hac300s'
> +
> + Datasheet: Publicly available at HiTRON website.
> +
> +Author:
> +
> + - Vasileios Amoiridis <vasileios.amoiridis@...n.ch>
> +
> +Description
> +-----------
> +
> +This driver implements support for the HiTRON HAC300S PSU. It is a Universal
s/implements support/supports/
> +AC input harmonic correction AC-DC hot-swappable CompactPCI Serial Dual output
> +(with 5V standby) 312 Watts active current sharing switching power supply.
> +
> +The device has an input of 90-264VAC and 2 nominal output voltaged at 12V and
> +5V which they can supplu up to 25A and 2.5A respectively.
> +
> +Sysfs entries
> +-------------
> +
> +======= ==========================================
> +curr1 Output current
> +in1 Output voltage
> +power1 Output power
> +temp1 Ambient temperature inside the module
> +temp2 Internal secondary component's temperature
> +======= ==========================================
> diff --git a/MAINTAINERS b/MAINTAINERS
> index a0dd762f5648..feb8ec4d9b17 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -11254,6 +11254,13 @@ F: kernel/time/timer_list.c
> F: kernel/time/timer_migration.*
> F: tools/testing/selftests/timers/
>
> +HITRON HAC300S PSU DRIVER
> +M: Vasileios Amoiridis <vasileios.amoiridis@...n.ch>
> +L: linux-hwmon@...r.kernel.org
> +S: Maintained
> +F: Documentation/hwmon/hac300s.rst
> +F: drivers/hwmon/pmbus/hac300s.c
> +
> DELAY, SLEEP, TIMEKEEPING, TIMERS [RUST]
> M: Andreas Hindborg <a.hindborg@...nel.org>
> R: Boqun Feng <boqun.feng@...il.com>
> diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig
> index f3fb94cebf1a..4c2cb51dbe3f 100644
> --- a/drivers/hwmon/pmbus/Kconfig
> +++ b/drivers/hwmon/pmbus/Kconfig
> @@ -124,6 +124,15 @@ config SENSORS_FSP_3Y
> This driver can also be built as a module. If so, the module will
> be called fsp-3y.
>
> +config SENSORS_HAC300S
> + tristate "Hitron HAC300S-D120E PSU"
> + help
> + If you say yes here you get hardware monitoring support for the
> + Hitron HAC300S-D120E Power Supply.
> +
> + This driver can also be built as a module. If so, the module will
> + be called hac300s.
> +
> config SENSORS_IBM_CFFPS
> tristate "IBM Common Form Factor Power Supply"
> depends on LEDS_CLASS
> diff --git a/drivers/hwmon/pmbus/Makefile b/drivers/hwmon/pmbus/Makefile
> index 349a89b6d92e..b92309019d35 100644
> --- a/drivers/hwmon/pmbus/Makefile
> +++ b/drivers/hwmon/pmbus/Makefile
> @@ -13,6 +13,7 @@ obj-$(CONFIG_SENSORS_BEL_PFE) += bel-pfe.o
> obj-$(CONFIG_SENSORS_BPA_RS600) += bpa-rs600.o
> obj-$(CONFIG_SENSORS_DELTA_AHE50DC_FAN) += delta-ahe50dc-fan.o
> obj-$(CONFIG_SENSORS_FSP_3Y) += fsp-3y.o
> +obj-$(CONFIG_SENSORS_HAC300S) += hac300s.o
> obj-$(CONFIG_SENSORS_IBM_CFFPS) += ibm-cffps.o
> obj-$(CONFIG_SENSORS_DPS920AB) += dps920ab.o
> obj-$(CONFIG_SENSORS_INA233) += ina233.o
> diff --git a/drivers/hwmon/pmbus/hac300s.c b/drivers/hwmon/pmbus/hac300s.c
> new file mode 100644
> index 000000000000..a1640449e5f5
> --- /dev/null
> +++ b/drivers/hwmon/pmbus/hac300s.c
> @@ -0,0 +1,152 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +// SPDX-FileCopyrightText: 2024 CERN (home.cern)
> +/*
> + * Hardware monitoring driver for Hi-Tron HAC300S PSU.
> + *
> + * NOTE: The HAC300S device does not support the PMBUS_VOUT_MODE register.
> + * On top of that, it returns the Voltage output values in Linear11 which is
> + * not adhering to the PMBus specifications. (PMBus Specification Part II,
> + * Section 7.1-7.3). For that reason the PMBUS_VOUT_MODE register is being faked
> + * and returns the exponent value of the READ_VOUT register. The exponent part
> + * of the VOUT_* registers is being cleared in order to return the mantissa to
> + * the pmbus core.
> + */
> +
> +#include <linux/bitfield.h>
> +#include <linux/bits.h>
> +#include <linux/err.h>
> +#include <linux/i2c.h>
> +#include <linux/init.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/pmbus.h>
> +
> +#include "pmbus.h"
> +
> +#define LINEAR11_EXPONENT_MASK GENMASK(15, 11)
> +#define LINEAR11_MANTISSA_MASK GENMASK(10, 0)
> +
> +#define to_hac300s_data(x) container_of(x, struct hac300s_data, info)
> +
> +struct hac300s_data {
> + struct pmbus_driver_info info;
> + bool vout_linear11;
> + s8 exponent;
> +};
> +
> +static int hac300s_read_byte_data(struct i2c_client *client, int page, int reg)
> +{
> + const struct pmbus_driver_info *info = pmbus_get_driver_info(client);
> + struct hac300s_data *data = to_hac300s_data(info);
> +
> + if (reg == PMBUS_VOUT_MODE && data->vout_linear11)
> + return data->exponent;
> +
> + return pmbus_read_byte_data(client, page, reg);
> +}
> +
> +static int hac300s_read_word_data(struct i2c_client *client, int page,
> + int phase, int reg)
> +{
> + const struct pmbus_driver_info *info = pmbus_get_driver_info(client);
> + struct hac300s_data *data = to_hac300s_data(info);
> + int rv;
> +
> + rv = pmbus_read_word_data(client, page, phase, reg);
> + if (rv < 0)
> + return rv;
> +
> + switch (reg) {
> + case PMBUS_VIRT_READ_IOUT_AVG:
> + case PMBUS_VIRT_READ_POUT_AVG:
> + case PMBUS_VIRT_READ_TEMP_AVG:
> + return -ENXIO;
> + case PMBUS_VOUT_OV_WARN_LIMIT:
> + case PMBUS_VOUT_UV_WARN_LIMIT:
> + case PMBUS_VOUT_OV_FAULT_LIMIT:
> + case PMBUS_VOUT_UV_FAULT_LIMIT:
> + case PMBUS_MFR_VOUT_MAX:
> + case PMBUS_MFR_VOUT_MIN:
> + case PMBUS_READ_VOUT:
> + if (data->vout_linear11)
> + return FIELD_GET(LINEAR11_MANTISSA_MASK, rv);
Is it guaranteed that the exponent is always the same ? Because if not the
conversion will have to be explicit.
> + fallthrough;
> + default:
> + return rv;
This is wrong. The register should only be read by affected commands, and
the function should return -ENODATA for the others.
> + }
> +}
> +
> +#define HAC300S_SW_FUNC (PMBUS_HAVE_TEMP | PMBUS_HAVE_TEMP2 | \
> + PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT | \
> + PMBUS_HAVE_IOUT | PMBUS_HAVE_STATUS_IOUT | \
> + PMBUS_HAVE_POUT | PMBUS_HAVE_STATUS_TEMP)
> +
Unnecessary define since it is only used once. Please fold into the declaration
below.
> +static struct pmbus_driver_info hac300s_info = {
> + .pages = 1,
> + .func[0] = HAC300S_SW_FUNC,
> + .read_byte_data = hac300s_read_byte_data,
> + .read_word_data = hac300s_read_word_data,
> + .format[PSC_VOLTAGE_OUT] = linear,
> +};
> +
> +static struct pmbus_platform_data hac300s_pdata = {
> + .flags = PMBUS_NO_CAPABILITY,
> +};
> +
> +static int hac300s_probe(struct i2c_client *client)
> +{
> + struct hac300s_data *data;
> + int rv;
> +
> + data = devm_kzalloc(&client->dev, sizeof(struct hac300s_data), GFP_KERNEL);
> + if (!data)
> + return -ENOMEM;
> +
> + if (!i2c_check_functionality(client->adapter,
> + I2C_FUNC_SMBUS_READ_BYTE_DATA |
> + I2C_FUNC_SMBUS_READ_WORD_DATA))
> + return -ENODEV;
> +
> + rv = i2c_smbus_read_byte_data(client, PMBUS_VOUT_MODE);
> + if (rv < 0) {
This needs explanation. Why try to read PMBUS_VOUT_MODE if the failure is
expected ? Are there variants of the PSU which return something useful here ?
The note above does not explain the reason for this conditional.
If this is supposed to check if this is really the expected device,
read and verify PMBUS_MFR_ID instead.
Thanks,
Guenter
> + data->vout_linear11 = true;
> + /* LINEAR11 format, use exponent from READ_VOUT register */
> + rv = i2c_smbus_read_word_data(client, PMBUS_READ_VOUT);
> + if (rv < 0)
> + return dev_err_probe(&client->dev, rv, "Failed to read vout_mode\n");
> +
> + data->exponent = FIELD_GET(LINEAR11_EXPONENT_MASK, rv);
> + }
> +
> + data->info = hac300s_info;
> + client->dev.platform_data = &hac300s_pdata;
> + return pmbus_do_probe(client, &data->info);
> +}
> +
> +static const struct of_device_id hac300s_of_match[] = {
> + { .compatible = "hitron,hac300s" },
> + {}
> +};
> +MODULE_DEVICE_TABLE(of, hac300s_of_match);
> +
> +static const struct i2c_device_id hac300s_id[] = {
> + {"hac300s", 0},
> + {}
> +};
> +MODULE_DEVICE_TABLE(i2c, hac300s_id);
> +
> +static struct i2c_driver hac300s_driver = {
> + .driver = {
> + .name = "hac300s",
> + .of_match_table = hac300s_of_match,
> + },
> + .probe = hac300s_probe,
> + .id_table = hac300s_id,
> +
> +};
> +module_i2c_driver(hac300s_driver);
> +
> +MODULE_AUTHOR("Vasileios Amoiridis");
> +MODULE_DESCRIPTION("PMBus driver for Hi-Tron HAC300S PSU");
> +MODULE_LICENSE("GPL");
> +MODULE_IMPORT_NS("PMBUS");
Powered by blists - more mailing lists