Add pmbus support for Sony APS-379 power supplies. There are a few PMBUS
commands that return data that is undocumented/invalid so these need to
be rejected with -ENXIO. The READ_VOUT command returns data in linear11
format instead of linear16 so we need to workaround this.

Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
---
 drivers/hwmon/pmbus/Kconfig   |   6 ++
 drivers/hwmon/pmbus/Makefile  |   1 +
 drivers/hwmon/pmbus/aps-379.c | 196 ++++++++++++++++++++++++++++++++++
 3 files changed, 203 insertions(+)
 create mode 100644 drivers/hwmon/pmbus/aps-379.c

diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig
index fc1273abe357..29076921e330 100644
--- a/drivers/hwmon/pmbus/Kconfig
+++ b/drivers/hwmon/pmbus/Kconfig
@@ -77,6 +77,12 @@ config SENSORS_ADP1050_REGULATOR
 	  µModule regulators that can provide microprocessor power from 54V
 	  power distribution architecture.
 
+config SENSORS_APS_379
+	tristate "Sony APS-379 Power Supplies"
+	help
+	  If you say yes here you get hardware monitoring support for Sony
+	  APS-379 Power Supplies.
+
 config SENSORS_BEL_PFE
 	tristate "Bel PFE Compatible Power Supplies"
 	help
diff --git a/drivers/hwmon/pmbus/Makefile b/drivers/hwmon/pmbus/Makefile
index d6c86924f887..94f36c7069ec 100644
--- a/drivers/hwmon/pmbus/Makefile
+++ b/drivers/hwmon/pmbus/Makefile
@@ -9,6 +9,7 @@ obj-$(CONFIG_SENSORS_ACBEL_FSG032) += acbel-fsg032.o
 obj-$(CONFIG_SENSORS_ADM1266)	+= adm1266.o
 obj-$(CONFIG_SENSORS_ADM1275)	+= adm1275.o
 obj-$(CONFIG_SENSORS_ADP1050)	+= adp1050.o
+obj-$(CONFIG_SENSORS_APS_379)	+= aps-379.o
 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
diff --git a/drivers/hwmon/pmbus/aps-379.c b/drivers/hwmon/pmbus/aps-379.c
new file mode 100644
index 000000000000..e4c4c2d12bc9
--- /dev/null
+++ b/drivers/hwmon/pmbus/aps-379.c
@@ -0,0 +1,196 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Hardware monitoring driver for Sony APS-379 Power Supplies
+ *
+ * Copyright 2026 Allied Telesis Labs
+ */
+
+#include <linux/i2c.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/pmbus.h>
+#include "pmbus.h"
+
+struct aps_379_data {
+	struct pmbus_driver_info info;
+	u8 vout_linear_exponent;
+};
+
+#define to_aps_379_data(x) container_of(x, struct aps_379_data, info)
+
+static const struct i2c_device_id aps_379_id[] = {
+	{ "aps-379", 0 },
+	{},
+};
+
+static int aps_379_read_byte_data(struct i2c_client *client, int page, int reg)
+{
+	const struct pmbus_driver_info *info = pmbus_get_driver_info(client);
+	struct aps_379_data *data = to_aps_379_data(info);
+	int ret;
+
+	if (page > 0)
+		return -ENXIO;
+
+	switch (reg) {
+	case PMBUS_VOUT_MODE:
+		/*
+		 * The VOUT format used by the chip is linear11,
+		 * not linear16. Report that VOUT is in linear mode
+		 * and return exponent value extracted while probing
+		 * the chip.
+		 */
+		ret = data->vout_linear_exponent;
+		break;
+	default:
+		ret = -ENODATA;
+		break;
+	}
+
+	return ret;
+}
+
+/*
+ * The APS-379 uses linear11 format instead of linear16. We've reported the exponent
+ * via the PMBUS_VOUT_MODE so we just return the mantissa here.
+ */
+static int aps_379_read_vout(struct i2c_client *client)
+{
+	int ret;
+	s32 mantissa;
+
+	ret = pmbus_read_word_data(client, 0, 0xff, PMBUS_READ_VOUT);
+	if (ret < 0)
+		return ret;
+
+	mantissa = ((s16)((ret & 0x7ff) << 5)) >> 5;
+	ret = mantissa;
+
+	return ret;
+}
+
+static int aps_379_read_word_data(struct i2c_client *client, int page, int phase, int reg)
+{
+	int ret;
+
+	if (page > 0)
+		return -ENXIO;
+
+	switch (reg) {
+	case PMBUS_VOUT_UV_WARN_LIMIT:
+	case PMBUS_VOUT_OV_WARN_LIMIT:
+	case PMBUS_VOUT_UV_FAULT_LIMIT:
+	case PMBUS_VOUT_OV_FAULT_LIMIT:
+	case PMBUS_PIN_OP_WARN_LIMIT:
+	case PMBUS_POUT_OP_WARN_LIMIT:
+	case PMBUS_MFR_IIN_MAX:
+	case PMBUS_MFR_PIN_MAX:
+	case PMBUS_MFR_VOUT_MIN:
+	case PMBUS_MFR_VOUT_MAX:
+	case PMBUS_MFR_IOUT_MAX:
+	case PMBUS_MFR_POUT_MAX:
+	case PMBUS_MFR_MAX_TEMP_1:
+		/* These commands return data but it is invalid/un-documented */
+		ret = -ENXIO;
+		break;
+	case PMBUS_READ_VOUT:
+		ret = aps_379_read_vout(client);
+		break;
+	default:
+		if (reg >= PMBUS_VIRT_BASE)
+			ret = -ENXIO;
+		else
+			ret = -ENODATA;
+		break;
+	}
+
+	return ret;
+
+}
+
+static struct pmbus_driver_info aps_379_info = {
+	.pages = 1,
+	.format[PSC_VOLTAGE_OUT] = linear,
+	.format[PSC_CURRENT_OUT] = linear,
+	.format[PSC_POWER] = linear,
+	.format[PSC_TEMPERATURE] = linear,
+	.format[PSC_FAN] = linear,
+	.func[0] = PMBUS_HAVE_VOUT |
+		PMBUS_HAVE_IOUT |
+		PMBUS_HAVE_PIN | PMBUS_HAVE_POUT |
+		PMBUS_HAVE_TEMP |
+		PMBUS_HAVE_FAN12,
+	.read_byte_data = aps_379_read_byte_data,
+	.read_word_data = aps_379_read_word_data,
+};
+
+static int aps_379_probe(struct i2c_client *client)
+{
+	struct device *dev = &client->dev;
+	const struct i2c_device_id *mid;
+	struct pmbus_driver_info *info;
+	struct aps_379_data *data;
+	u8 buf[I2C_SMBUS_BLOCK_MAX + 1];
+	int ret;
+
+	data = devm_kzalloc(&client->dev, sizeof(*data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	memcpy(&data->info, &aps_379_info, sizeof(*info));
+	info = &data->info;
+
+	if (!i2c_check_functionality(client->adapter,
+				     I2C_FUNC_SMBUS_READ_BYTE_DATA
+				     | I2C_FUNC_SMBUS_READ_WORD_DATA
+				     | I2C_FUNC_SMBUS_READ_BLOCK_DATA))
+		return -ENODEV;
+
+	ret = i2c_smbus_read_block_data(client, PMBUS_MFR_MODEL, buf);
+	if (ret < 0) {
+		dev_err(dev, "Failed to read Manufacturer Model\n");
+		return ret;
+	}
+
+	for (mid = aps_379_id; mid->name[0]; mid++) {
+		if (!strncasecmp(buf, mid->name, strlen(mid->name)))
+			break;
+	}
+	if (!mid->name[0]) {
+		buf[ret] = '\0';
+		dev_err(dev, "Unsupported Manufacturer Model '%s'\n", buf);
+		return -ENODEV;
+	}
+
+	ret = i2c_smbus_read_word_data(client, PMBUS_READ_VOUT);
+	if (ret < 0) {
+		dev_err(dev, "Can't get vout exponent.\n");
+		return ret;
+	}
+	data->vout_linear_exponent = (u8)((ret >> 11) & 0x1f);
+
+	return pmbus_do_probe(client, info);
+}
+
+static const struct of_device_id __maybe_unused aps_379_of_match[] = {
+	{ .compatible = "sony,aps-379" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, aps_379_of_match);
+
+static struct i2c_driver aps_379_driver = {
+	.driver = {
+		.name = "aps-379",
+		.of_match_table = of_match_ptr(aps_379_of_match),
+	},
+	.probe = aps_379_probe,
+	.id_table = aps_379_id,
+};
+
+module_i2c_driver(aps_379_driver);
+
+MODULE_AUTHOR("Chris Packham");
+MODULE_DESCRIPTION("PMBus driver for Sony APS-379");
+MODULE_LICENSE("GPL");
+MODULE_IMPORT_NS("PMBUS");
-- 
2.53.0

Hi Chris,

On 3/31/26 16:19, Chris Packham wrote:
> Add pmbus support for Sony APS-379 power supplies. There are a few PMBUS
> commands that return data that is undocumented/invalid so these need to
> be rejected with -ENXIO. The READ_VOUT command returns data in linear11
> format instead of linear16 so we need to workaround this.
> 
> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>

Feedback inline.

Thanks,
Guenter

> ---
>   drivers/hwmon/pmbus/Kconfig   |   6 ++
>   drivers/hwmon/pmbus/Makefile  |   1 +
>   drivers/hwmon/pmbus/aps-379.c | 196 ++++++++++++++++++++++++++++++++++
>   3 files changed, 203 insertions(+)
>   create mode 100644 drivers/hwmon/pmbus/aps-379.c
> 
> diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig
> index fc1273abe357..29076921e330 100644
> --- a/drivers/hwmon/pmbus/Kconfig
> +++ b/drivers/hwmon/pmbus/Kconfig
> @@ -77,6 +77,12 @@ config SENSORS_ADP1050_REGULATOR
>   	  µModule regulators that can provide microprocessor power from 54V
>   	  power distribution architecture.
>   
> +config SENSORS_APS_379
> +	tristate "Sony APS-379 Power Supplies"
> +	help
> +	  If you say yes here you get hardware monitoring support for Sony
> +	  APS-379 Power Supplies.
> +
>   config SENSORS_BEL_PFE
>   	tristate "Bel PFE Compatible Power Supplies"
>   	help
> diff --git a/drivers/hwmon/pmbus/Makefile b/drivers/hwmon/pmbus/Makefile
> index d6c86924f887..94f36c7069ec 100644
> --- a/drivers/hwmon/pmbus/Makefile
> +++ b/drivers/hwmon/pmbus/Makefile
> @@ -9,6 +9,7 @@ obj-$(CONFIG_SENSORS_ACBEL_FSG032) += acbel-fsg032.o
>   obj-$(CONFIG_SENSORS_ADM1266)	+= adm1266.o
>   obj-$(CONFIG_SENSORS_ADM1275)	+= adm1275.o
>   obj-$(CONFIG_SENSORS_ADP1050)	+= adp1050.o
> +obj-$(CONFIG_SENSORS_APS_379)	+= aps-379.o
>   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
> diff --git a/drivers/hwmon/pmbus/aps-379.c b/drivers/hwmon/pmbus/aps-379.c
> new file mode 100644
> index 000000000000..e4c4c2d12bc9
> --- /dev/null
> +++ b/drivers/hwmon/pmbus/aps-379.c

Driver documentation is missing.

This power supply does not seem to be documented anywhere, so this is actually quite
important.

Having said this, the behavior seems quite similar to BluTek BPA-RS600. Are those
power supplies from the same OEM ?

> @@ -0,0 +1,196 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Hardware monitoring driver for Sony APS-379 Power Supplies
> + *
> + * Copyright 2026 Allied Telesis Labs
> + */
> +
> +#include <linux/i2c.h>
> +#include <linux/init.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/pmbus.h>
> +#include "pmbus.h"
> +
> +struct aps_379_data {
> +	struct pmbus_driver_info info;
> +	u8 vout_linear_exponent;
> +};
> +
> +#define to_aps_379_data(x) container_of(x, struct aps_379_data, info)
> +
> +static const struct i2c_device_id aps_379_id[] = {
> +	{ "aps-379", 0 },
> +	{},
> +};
> +
> +static int aps_379_read_byte_data(struct i2c_client *client, int page, int reg)
> +{
> +	const struct pmbus_driver_info *info = pmbus_get_driver_info(client);
> +	struct aps_379_data *data = to_aps_379_data(info);
> +	int ret;
> +
> +	if (page > 0)
> +		return -ENXIO;

Unnecessary since there is only one page.

Yes, I know, other drivers do it, but it is really pointless.

> +
> +	switch (reg) {
> +	case PMBUS_VOUT_MODE:
> +		/*
> +		 * The VOUT format used by the chip is linear11,
> +		 * not linear16. Report that VOUT is in linear mode
> +		 * and return exponent value extracted while probing
> +		 * the chip.
> +		 */
> +		ret = data->vout_linear_exponent;
> +		break;
> +	default:
> +		ret = -ENODATA;
> +		break;
> +	}
> +
> +	return ret;
> +}
> +
> +/*
> + * The APS-379 uses linear11 format instead of linear16. We've reported the exponent
> + * via the PMBUS_VOUT_MODE so we just return the mantissa here.
> + */
> +static int aps_379_read_vout(struct i2c_client *client)
> +{
> +	int ret;
> +	s32 mantissa;
> +
> +	ret = pmbus_read_word_data(client, 0, 0xff, PMBUS_READ_VOUT);
> +	if (ret < 0)
> +		return ret;
> +
> +	mantissa = ((s16)((ret & 0x7ff) << 5)) >> 5;

sign_extend32() ?

Also, is the exponent known to be static ? If not it may be necessary
to adjust it. If yes, I'd suggest to add a comment above.

> +	ret = mantissa;

That assignment is really unnecessary.

> +
> +	return ret;
> +}
> +
> +static int aps_379_read_word_data(struct i2c_client *client, int page, int phase, int reg)
> +{
> +	int ret;
> +
> +	if (page > 0)
> +		return -ENXIO;

Unnecessary.

> +
> +	switch (reg) {
> +	case PMBUS_VOUT_UV_WARN_LIMIT:
> +	case PMBUS_VOUT_OV_WARN_LIMIT:
> +	case PMBUS_VOUT_UV_FAULT_LIMIT:
> +	case PMBUS_VOUT_OV_FAULT_LIMIT:
> +	case PMBUS_PIN_OP_WARN_LIMIT:
> +	case PMBUS_POUT_OP_WARN_LIMIT:
> +	case PMBUS_MFR_IIN_MAX:
> +	case PMBUS_MFR_PIN_MAX:
> +	case PMBUS_MFR_VOUT_MIN:
> +	case PMBUS_MFR_VOUT_MAX:
> +	case PMBUS_MFR_IOUT_MAX:
> +	case PMBUS_MFR_POUT_MAX:
> +	case PMBUS_MFR_MAX_TEMP_1:
> +		/* These commands return data but it is invalid/un-documented */
> +		ret = -ENXIO;
> +		break;

I'd suggest to return directly in this function. There is no real value
to assign the return value to ret just to return it.

> +	case PMBUS_READ_VOUT:
> +		ret = aps_379_read_vout(client);
> +		break;
> +	default:
> +		if (reg >= PMBUS_VIRT_BASE)
> +			ret = -ENXIO;
> +		else
> +			ret = -ENODATA;
> +		break;
> +	}
> +
> +	return ret;
> +
> +}
> +
> +static struct pmbus_driver_info aps_379_info = {
> +	.pages = 1,
> +	.format[PSC_VOLTAGE_OUT] = linear,
> +	.format[PSC_CURRENT_OUT] = linear,
> +	.format[PSC_POWER] = linear,
> +	.format[PSC_TEMPERATURE] = linear,
> +	.format[PSC_FAN] = linear,
> +	.func[0] = PMBUS_HAVE_VOUT |
> +		PMBUS_HAVE_IOUT |
> +		PMBUS_HAVE_PIN | PMBUS_HAVE_POUT |
> +		PMBUS_HAVE_TEMP |
> +		PMBUS_HAVE_FAN12,
> +	.read_byte_data = aps_379_read_byte_data,
> +	.read_word_data = aps_379_read_word_data,
> +};
> +
> +static int aps_379_probe(struct i2c_client *client)
> +{
> +	struct device *dev = &client->dev;
> +	const struct i2c_device_id *mid;
> +	struct pmbus_driver_info *info;
> +	struct aps_379_data *data;
> +	u8 buf[I2C_SMBUS_BLOCK_MAX + 1];
> +	int ret;
> +
> +	data = devm_kzalloc(&client->dev, sizeof(*data), GFP_KERNEL);
> +	if (!data)
> +		return -ENOMEM;
> +
> +	memcpy(&data->info, &aps_379_info, sizeof(*info));
> +	info = &data->info;
> +
> +	if (!i2c_check_functionality(client->adapter,
> +				     I2C_FUNC_SMBUS_READ_BYTE_DATA
> +				     | I2C_FUNC_SMBUS_READ_WORD_DATA
> +				     | I2C_FUNC_SMBUS_READ_BLOCK_DATA))
> +		return -ENODEV;
> +
> +	ret = i2c_smbus_read_block_data(client, PMBUS_MFR_MODEL, buf);
> +	if (ret < 0) {
> +		dev_err(dev, "Failed to read Manufacturer Model\n");
> +		return ret;
> +	}
> +
> +	for (mid = aps_379_id; mid->name[0]; mid++) {
> +		if (!strncasecmp(buf, mid->name, strlen(mid->name)))
> +			break;
> +	}

That seems to be excessive. There is only one power supply.
If more are added in the future, a looop can be added. Right
now a simple comparison should do.

Thanks,
Guenter

> +	if (!mid->name[0]) {
> +		buf[ret] = '\0';
> +		dev_err(dev, "Unsupported Manufacturer Model '%s'\n", buf);
> +		return -ENODEV;
> +	}
> +
> +	ret = i2c_smbus_read_word_data(client, PMBUS_READ_VOUT);
> +	if (ret < 0) {
> +		dev_err(dev, "Can't get vout exponent.\n");
> +		return ret;
> +	}
> +	data->vout_linear_exponent = (u8)((ret >> 11) & 0x1f);
> +
> +	return pmbus_do_probe(client, info);
> +}
> +
> +static const struct of_device_id __maybe_unused aps_379_of_match[] = {
> +	{ .compatible = "sony,aps-379" },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, aps_379_of_match);
> +
> +static struct i2c_driver aps_379_driver = {
> +	.driver = {
> +		.name = "aps-379",
> +		.of_match_table = of_match_ptr(aps_379_of_match),
> +	},
> +	.probe = aps_379_probe,
> +	.id_table = aps_379_id,
> +};
> +
> +module_i2c_driver(aps_379_driver);
> +
> +MODULE_AUTHOR("Chris Packham");
> +MODULE_DESCRIPTION("PMBus driver for Sony APS-379");
> +MODULE_LICENSE("GPL");
> +MODULE_IMPORT_NS("PMBUS");

On 01/04/2026 12:56, Guenter Roeck wrote:
> Hi Chris,
>
> On 3/31/26 16:19, Chris Packham wrote:
>> Add pmbus support for Sony APS-379 power supplies. There are a few PMBUS
>> commands that return data that is undocumented/invalid so these need to
>> be rejected with -ENXIO. The READ_VOUT command returns data in linear11
>> format instead of linear16 so we need to workaround this.
>>
>> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
>
> Feedback inline.
>
> Thanks,
> Guenter
>
>> ---
>>   drivers/hwmon/pmbus/Kconfig   |   6 ++
>>   drivers/hwmon/pmbus/Makefile  |   1 +
>>   drivers/hwmon/pmbus/aps-379.c | 196 ++++++++++++++++++++++++++++++++++
>>   3 files changed, 203 insertions(+)
>>   create mode 100644 drivers/hwmon/pmbus/aps-379.c
>>
>> diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig
>> index fc1273abe357..29076921e330 100644
>> --- a/drivers/hwmon/pmbus/Kconfig
>> +++ b/drivers/hwmon/pmbus/Kconfig
>> @@ -77,6 +77,12 @@ config SENSORS_ADP1050_REGULATOR
>>         µModule regulators that can provide microprocessor power from 
>> 54V
>>         power distribution architecture.
>>   +config SENSORS_APS_379
>> +    tristate "Sony APS-379 Power Supplies"
>> +    help
>> +      If you say yes here you get hardware monitoring support for Sony
>> +      APS-379 Power Supplies.
>> +
>>   config SENSORS_BEL_PFE
>>       tristate "Bel PFE Compatible Power Supplies"
>>       help
>> diff --git a/drivers/hwmon/pmbus/Makefile b/drivers/hwmon/pmbus/Makefile
>> index d6c86924f887..94f36c7069ec 100644
>> --- a/drivers/hwmon/pmbus/Makefile
>> +++ b/drivers/hwmon/pmbus/Makefile
>> @@ -9,6 +9,7 @@ obj-$(CONFIG_SENSORS_ACBEL_FSG032) += acbel-fsg032.o
>>   obj-$(CONFIG_SENSORS_ADM1266)    += adm1266.o
>>   obj-$(CONFIG_SENSORS_ADM1275)    += adm1275.o
>>   obj-$(CONFIG_SENSORS_ADP1050)    += adp1050.o
>> +obj-$(CONFIG_SENSORS_APS_379)    += aps-379.o
>>   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
>> diff --git a/drivers/hwmon/pmbus/aps-379.c 
>> b/drivers/hwmon/pmbus/aps-379.c
>> new file mode 100644
>> index 000000000000..e4c4c2d12bc9
>> --- /dev/null
>> +++ b/drivers/hwmon/pmbus/aps-379.c
>
> Driver documentation is missing.
>
> This power supply does not seem to be documented anywhere, so this is 
> actually quite
> important.

I'll see what I can add. I do have a list of the actual supported PMBus 
commands so that would be helpful to share.

>  Having said this, the behavior seems quite similar to BluTek 
> BPA-RS600. Are those
> power supplies from the same OEM ?

The BPA-RS600 is a smaller form factor. BluTek do make PSUs in the same 
from factor (there's a DC input one we use that I might get around to 
upstreaming one day), but they are genuinely different suppliers and the 
definitely don't interop electrically. Part of the reason for us using 
the APS-379 is manufactured by a Japanese company which lets us sell it 
into markets that have country of origin restrictions (that's as much 
politics as I care to discuss). They all have different 
quirks/deviations from the PMBus spec.

I did of course refer to the bpa-rs600 driver as I was familiar with it. 
I pulled the linear11 vout workaround from mp5990 and adapted it for my 
needs.

>
>> @@ -0,0 +1,196 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later
>> +/*
>> + * Hardware monitoring driver for Sony APS-379 Power Supplies
>> + *
>> + * Copyright 2026 Allied Telesis Labs
>> + */
>> +
>> +#include <linux/i2c.h>
>> +#include <linux/init.h>
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/pmbus.h>
>> +#include "pmbus.h"
>> +
>> +struct aps_379_data {
>> +    struct pmbus_driver_info info;
>> +    u8 vout_linear_exponent;
>> +};
>> +
>> +#define to_aps_379_data(x) container_of(x, struct aps_379_data, info)
>> +
>> +static const struct i2c_device_id aps_379_id[] = {
>> +    { "aps-379", 0 },
>> +    {},
>> +};
>> +
>> +static int aps_379_read_byte_data(struct i2c_client *client, int 
>> page, int reg)
>> +{
>> +    const struct pmbus_driver_info *info = 
>> pmbus_get_driver_info(client);
>> +    struct aps_379_data *data = to_aps_379_data(info);
>> +    int ret;
>> +
>> +    if (page > 0)
>> +        return -ENXIO;
>
> Unnecessary since there is only one page.
>
> Yes, I know, other drivers do it, but it is really pointless.
>
Ack.
>> +
>> +    switch (reg) {
>> +    case PMBUS_VOUT_MODE:
>> +        /*
>> +         * The VOUT format used by the chip is linear11,
>> +         * not linear16. Report that VOUT is in linear mode
>> +         * and return exponent value extracted while probing
>> +         * the chip.
>> +         */
>> +        ret = data->vout_linear_exponent;
>> +        break;
>> +    default:
>> +        ret = -ENODATA;
>> +        break;
>> +    }
>> +
>> +    return ret;
>> +}
>> +
>> +/*
>> + * The APS-379 uses linear11 format instead of linear16. We've 
>> reported the exponent
>> + * via the PMBUS_VOUT_MODE so we just return the mantissa here.
>> + */
>> +static int aps_379_read_vout(struct i2c_client *client)
>> +{
>> +    int ret;
>> +    s32 mantissa;
>> +
>> +    ret = pmbus_read_word_data(client, 0, 0xff, PMBUS_READ_VOUT);
>> +    if (ret < 0)
>> +        return ret;
>> +
>> +    mantissa = ((s16)((ret & 0x7ff) << 5)) >> 5;
>
> sign_extend32() ?
I'll look into it.
>
> Also, is the exponent known to be static ? If not it may be necessary
> to adjust it. If yes, I'd suggest to add a comment above.

I can't be 100% sure but the datasheet says the "scaling factor" for 
READ_VOUT is 2^-4 (before going on to explain how to do the the linear11 
decoding). I've only ever read -4 as the exponent so I think it's OK to 
assume a fixed exponent.

>
>> +    ret = mantissa;
>
> That assignment is really unnecessary.
Ack.
>
>> +
>> +    return ret;
>> +}
>> +
>> +static int aps_379_read_word_data(struct i2c_client *client, int 
>> page, int phase, int reg)
>> +{
>> +    int ret;
>> +
>> +    if (page > 0)
>> +        return -ENXIO;
>
> Unnecessary.
>
Ack.
>> +
>> +    switch (reg) {
>> +    case PMBUS_VOUT_UV_WARN_LIMIT:
>> +    case PMBUS_VOUT_OV_WARN_LIMIT:
>> +    case PMBUS_VOUT_UV_FAULT_LIMIT:
>> +    case PMBUS_VOUT_OV_FAULT_LIMIT:
>> +    case PMBUS_PIN_OP_WARN_LIMIT:
>> +    case PMBUS_POUT_OP_WARN_LIMIT:
>> +    case PMBUS_MFR_IIN_MAX:
>> +    case PMBUS_MFR_PIN_MAX:
>> +    case PMBUS_MFR_VOUT_MIN:
>> +    case PMBUS_MFR_VOUT_MAX:
>> +    case PMBUS_MFR_IOUT_MAX:
>> +    case PMBUS_MFR_POUT_MAX:
>> +    case PMBUS_MFR_MAX_TEMP_1:
>> +        /* These commands return data but it is 
>> invalid/un-documented */
>> +        ret = -ENXIO;
>> +        break;
>
> I'd suggest to return directly in this function. There is no real value
> to assign the return value to ret just to return it.
>
Ack.
>> +    case PMBUS_READ_VOUT:
>> +        ret = aps_379_read_vout(client);
>> +        break;
>> +    default:
>> +        if (reg >= PMBUS_VIRT_BASE)
>> +            ret = -ENXIO;
>> +        else
>> +            ret = -ENODATA;
>> +        break;
>> +    }
>> +
>> +    return ret;
>> +
>> +}
>> +
>> +static struct pmbus_driver_info aps_379_info = {
>> +    .pages = 1,
>> +    .format[PSC_VOLTAGE_OUT] = linear,
>> +    .format[PSC_CURRENT_OUT] = linear,
>> +    .format[PSC_POWER] = linear,
>> +    .format[PSC_TEMPERATURE] = linear,
>> +    .format[PSC_FAN] = linear,
>> +    .func[0] = PMBUS_HAVE_VOUT |
>> +        PMBUS_HAVE_IOUT |
>> +        PMBUS_HAVE_PIN | PMBUS_HAVE_POUT |
>> +        PMBUS_HAVE_TEMP |
>> +        PMBUS_HAVE_FAN12,
>> +    .read_byte_data = aps_379_read_byte_data,
>> +    .read_word_data = aps_379_read_word_data,
>> +};
>> +
>> +static int aps_379_probe(struct i2c_client *client)
>> +{
>> +    struct device *dev = &client->dev;
>> +    const struct i2c_device_id *mid;
>> +    struct pmbus_driver_info *info;
>> +    struct aps_379_data *data;
>> +    u8 buf[I2C_SMBUS_BLOCK_MAX + 1];
>> +    int ret;
>> +
>> +    data = devm_kzalloc(&client->dev, sizeof(*data), GFP_KERNEL);
>> +    if (!data)
>> +        return -ENOMEM;
>> +
>> +    memcpy(&data->info, &aps_379_info, sizeof(*info));
>> +    info = &data->info;
>> +
>> +    if (!i2c_check_functionality(client->adapter,
>> +                     I2C_FUNC_SMBUS_READ_BYTE_DATA
>> +                     | I2C_FUNC_SMBUS_READ_WORD_DATA
>> +                     | I2C_FUNC_SMBUS_READ_BLOCK_DATA))
>> +        return -ENODEV;
>> +
>> +    ret = i2c_smbus_read_block_data(client, PMBUS_MFR_MODEL, buf);
>> +    if (ret < 0) {
>> +        dev_err(dev, "Failed to read Manufacturer Model\n");
>> +        return ret;
>> +    }
>> +
>> +    for (mid = aps_379_id; mid->name[0]; mid++) {
>> +        if (!strncasecmp(buf, mid->name, strlen(mid->name)))
>> +            break;
>> +    }
>
> That seems to be excessive. There is only one power supply.
> If more are added in the future, a looop can be added. Right
> now a simple comparison should do.

Sure. One of the things I borrowed from the BPA driver.

>
> Thanks,
> Guenter
>
>> +    if (!mid->name[0]) {
>> +        buf[ret] = '\0';
>> +        dev_err(dev, "Unsupported Manufacturer Model '%s'\n", buf);
>> +        return -ENODEV;
>> +    }
>> +
>> +    ret = i2c_smbus_read_word_data(client, PMBUS_READ_VOUT);
>> +    if (ret < 0) {
>> +        dev_err(dev, "Can't get vout exponent.\n");
>> +        return ret;
>> +    }
>> +    data->vout_linear_exponent = (u8)((ret >> 11) & 0x1f);
>> +
>> +    return pmbus_do_probe(client, info);
>> +}
>> +
>> +static const struct of_device_id __maybe_unused aps_379_of_match[] = {
>> +    { .compatible = "sony,aps-379" },
>> +    {},
>> +};
>> +MODULE_DEVICE_TABLE(of, aps_379_of_match);
>> +
>> +static struct i2c_driver aps_379_driver = {
>> +    .driver = {
>> +        .name = "aps-379",
>> +        .of_match_table = of_match_ptr(aps_379_of_match),
>> +    },
>> +    .probe = aps_379_probe,
>> +    .id_table = aps_379_id,
>> +};
>> +
>> +module_i2c_driver(aps_379_driver);
>> +
>> +MODULE_AUTHOR("Chris Packham");
>> +MODULE_DESCRIPTION("PMBus driver for Sony APS-379");
>> +MODULE_LICENSE("GPL");
>> +MODULE_IMPORT_NS("PMBUS");
>

On 3/31/26 17:26, Chris Packham wrote:
> 
> On 01/04/2026 12:56, Guenter Roeck wrote:
>> Hi Chris,
>>
>> On 3/31/26 16:19, Chris Packham wrote:
>>> Add pmbus support for Sony APS-379 power supplies. There are a few PMBUS
>>> commands that return data that is undocumented/invalid so these need to
>>> be rejected with -ENXIO. The READ_VOUT command returns data in linear11
>>> format instead of linear16 so we need to workaround this.
>>>
>>> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
>>
>> Feedback inline.
>>
>> Thanks,
>> Guenter
>>
>>> ---
>>>    drivers/hwmon/pmbus/Kconfig   |   6 ++
>>>    drivers/hwmon/pmbus/Makefile  |   1 +
>>>    drivers/hwmon/pmbus/aps-379.c | 196 ++++++++++++++++++++++++++++++++++
>>>    3 files changed, 203 insertions(+)
>>>    create mode 100644 drivers/hwmon/pmbus/aps-379.c
>>>
>>> diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig
>>> index fc1273abe357..29076921e330 100644
>>> --- a/drivers/hwmon/pmbus/Kconfig
>>> +++ b/drivers/hwmon/pmbus/Kconfig
>>> @@ -77,6 +77,12 @@ config SENSORS_ADP1050_REGULATOR
>>>          µModule regulators that can provide microprocessor power from
>>> 54V
>>>          power distribution architecture.
>>>    +config SENSORS_APS_379
>>> +    tristate "Sony APS-379 Power Supplies"
>>> +    help
>>> +      If you say yes here you get hardware monitoring support for Sony
>>> +      APS-379 Power Supplies.
>>> +
>>>    config SENSORS_BEL_PFE
>>>        tristate "Bel PFE Compatible Power Supplies"
>>>        help
>>> diff --git a/drivers/hwmon/pmbus/Makefile b/drivers/hwmon/pmbus/Makefile
>>> index d6c86924f887..94f36c7069ec 100644
>>> --- a/drivers/hwmon/pmbus/Makefile
>>> +++ b/drivers/hwmon/pmbus/Makefile
>>> @@ -9,6 +9,7 @@ obj-$(CONFIG_SENSORS_ACBEL_FSG032) += acbel-fsg032.o
>>>    obj-$(CONFIG_SENSORS_ADM1266)    += adm1266.o
>>>    obj-$(CONFIG_SENSORS_ADM1275)    += adm1275.o
>>>    obj-$(CONFIG_SENSORS_ADP1050)    += adp1050.o
>>> +obj-$(CONFIG_SENSORS_APS_379)    += aps-379.o
>>>    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
>>> diff --git a/drivers/hwmon/pmbus/aps-379.c
>>> b/drivers/hwmon/pmbus/aps-379.c
>>> new file mode 100644
>>> index 000000000000..e4c4c2d12bc9
>>> --- /dev/null
>>> +++ b/drivers/hwmon/pmbus/aps-379.c
>>
>> Driver documentation is missing.
>>
>> This power supply does not seem to be documented anywhere, so this is
>> actually quite
>> important.
> 
> I'll see what I can add. I do have a list of the actual supported PMBus
> commands so that would be helpful to share.
> 
>>   Having said this, the behavior seems quite similar to BluTek
>> BPA-RS600. Are those
>> power supplies from the same OEM ?
> 
> The BPA-RS600 is a smaller form factor. BluTek do make PSUs in the same
> from factor (there's a DC input one we use that I might get around to
> upstreaming one day), but they are genuinely different suppliers and the
> definitely don't interop electrically. Part of the reason for us using
> the APS-379 is manufactured by a Japanese company which lets us sell it
> into markets that have country of origin restrictions (that's as much
> politics as I care to discuss). They all have different
> quirks/deviations from the PMBus spec.
> 
> I did of course refer to the bpa-rs600 driver as I was familiar with it.
> I pulled the linear11 vout workaround from mp5990 and adapted it for my
> needs.
> 

I asked the AI version of Google search if they use the same OEM. It suggested
that they probably use the same or a similar controller chip and use the
firmware provided from that manufacturer as base for their firmware
implementation. Guess that makes sense.

Guenter