| 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: <aWjy405syjr134lR@vamoirid-laptop>
Date: Thu, 15 Jan 2026 15:00:03 +0100
From: Vasileios Amoiridis <vassilisamir@...il.com>
To: Guenter Roeck <linux@...ck-us.net>
Cc: robh@...nel.org, krzk+dt@...nel.org, conor+dt@...nel.org,
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 Mon, Jan 12, 2026 at 07:26:49AM -0800, Guenter Roeck wrote:
> 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.
>
ACK
> > 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/
>
ACK
> > +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.
>
Yes, after a lot of testing with different values, it remained the same.
> > + 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.
>
ACK. I hadn't understood that this was the usecase of this function.
> > + }
> > +}
> > +
> > +#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.
>
ACK.
> > +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
>
That was just some sanity check, it can indeed be removed.
Cheers,
Vasilis
> > + 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