| 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: <e718a9cf-8888-d24d-5d29-d883e8a0025f@roeck-us.net>
Date: Mon, 14 Feb 2022 09:36:06 -0800
From: Guenter Roeck <linux@...ck-us.net>
To: Marcello Sylvester Bauer <sylv@...v.io>,
Jean Delvare <jdelvare@...e.com>,
Jonathan Corbet <corbet@....net>
Cc: linux-kernel@...r.kernel.org, linux-hwmon@...r.kernel.org,
Patrick Rudolph <patrick.rudolph@...ements.com>,
linux-doc@...r.kernel.org
Subject: Re: [PATCH v2 3/4] pmbus: Add support for pli1209bc
On 2/14/22 04:40, Marcello Sylvester Bauer wrote:
> PLI1209BC is a Digital Supervisor from Vicor Corporation.
>
> Signed-off-by: Marcello Sylvester Bauer <sylv@...v.io>
> ---
> Documentation/hwmon/pli1209bc.rst | 73 +++++++++++++++++++
> drivers/hwmon/pmbus/Kconfig | 9 +++
> drivers/hwmon/pmbus/Makefile | 1 +
> drivers/hwmon/pmbus/pli1209bc.c | 114 ++++++++++++++++++++++++++++++
> 4 files changed, 197 insertions(+)
> create mode 100644 Documentation/hwmon/pli1209bc.rst
> create mode 100644 drivers/hwmon/pmbus/pli1209bc.c
>
> diff --git a/Documentation/hwmon/pli1209bc.rst b/Documentation/hwmon/pli1209bc.rst
> new file mode 100644
> index 000000000000..a3f686d03cf2
> --- /dev/null
> +++ b/Documentation/hwmon/pli1209bc.rst
> @@ -0,0 +1,73 @@
> +Kernel driver pli1209bc
> +=======================
> +
> +Supported chips:
> +
> + * Digital Supervisor PLI1209BC
> +
> + Prefix: 'pli1209bc'
> +
> + Addresses scanned: 0x50 - 0x5F
> +
> + Datasheet: https://www.vicorpower.com/documents/datasheets/ds-PLI1209BCxyzz-VICOR.pdf
> +
> +Authors:
> + - Marcello Sylvester Bauer <sylv@...v.io>
> +
> +Description
> +-----------
> +
> +The Vicor PLI1209BC is an isolated digital power system supervisor thatprovides
> +a communication interface between a host processor and one Bus Converter Module
> +(BCM). The PLI communicates with a system controller via a PMBus compatible
> +interface over an isolated UART interface. Through the PLI, the host processor
> +can configure, set protection limits, and monitor the BCM.
> +
> +Sysfs entries
> +-------------
> +
> +======================= ========================================================
> +in1_label "vin2"
> +in1_input Input voltage.
> +in1_rated_min Minimum rated input voltage.
> +in1_rated_max Maximum rated input voltage.
> +in1_max Maximum input voltage.
> +in1_max_alarm Input voltage high alarm.
> +in1_crit Critical input voltage.
> +in1_crit_alarm Input voltage critical alarm.
> +
> +in2_label "vout2"
> +in2_input Output voltage.
> +in2_rated_min Minimum rated output voltage.
> +in2_rated_max Maximum rated output voltage.
> +in2_alarm Output voltage alarm
> +
> +curr1_label "iin2"
> +curr1_input Input current.
> +curr1_max Maximum input current.
> +curr1_max_alarm Maximum input current high alarm.
> +curr1_crit Critical input current.
> +curr1_crit_alarm Input current critical alarm.
> +
> +curr2_label "iout2"
> +curr2_input Output current.
> +curr2_crit Critical output current.
> +curr2_crit_alarm Output current critical alarm.
> +curr2_max Maximum output current.
> +curr2_max_alarm Output current high alarm.
> +
> +power1_label "pin2"
> +power1_input Input power.
> +power1_alarm Input power alarm.
> +
> +power2_label "pout2"
> +power2_input Output power.
> +power2_rated_max Maximum rated output power.
> +
> +temp1_input Die temperature.
> +temp1_alarm Die temperature alarm.
> +temp1_max Maximum die temperature.
> +temp1_max_alarm Die temperature high alarm.
> +temp1_crit Critical die temperature.
> +temp1_crit_alarm Die temperature critical alarm.
> +======================= ========================================================
> diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig
> index c96f7b7338bd..831db423bea0 100644
> --- a/drivers/hwmon/pmbus/Kconfig
> +++ b/drivers/hwmon/pmbus/Kconfig
> @@ -310,6 +310,15 @@ config SENSORS_PIM4328
> This driver can also be built as a module. If so, the module will
> be called pim4328.
>
> +config SENSORS_PLI1209BC
> + tristate "Vicor PLI1209BC"
> + help
> + If you say yes here you get hardware monitoring support for Vicor
> + PLI1209BC Digital Supervisor.
> +
> + This driver can also be built as a module. If so, the module will
> + be called pli1209bc.
> +
> config SENSORS_PM6764TR
> tristate "ST PM6764TR"
> help
> diff --git a/drivers/hwmon/pmbus/Makefile b/drivers/hwmon/pmbus/Makefile
> index e5935f70c9e0..7ce74e3b8552 100644
> --- a/drivers/hwmon/pmbus/Makefile
> +++ b/drivers/hwmon/pmbus/Makefile
> @@ -34,6 +34,7 @@ obj-$(CONFIG_SENSORS_MP2888) += mp2888.o
> obj-$(CONFIG_SENSORS_MP2975) += mp2975.o
> obj-$(CONFIG_SENSORS_MP5023) += mp5023.o
> obj-$(CONFIG_SENSORS_PM6764TR) += pm6764tr.o
> +obj-$(CONFIG_SENSORS_PLI1209BC) += pli1209bc.o
> obj-$(CONFIG_SENSORS_PXE1610) += pxe1610.o
> obj-$(CONFIG_SENSORS_Q54SJ108A2) += q54sj108a2.o
> obj-$(CONFIG_SENSORS_STPDDC60) += stpddc60.o
> diff --git a/drivers/hwmon/pmbus/pli1209bc.c b/drivers/hwmon/pmbus/pli1209bc.c
> new file mode 100644
> index 000000000000..8a9af2ccc46f
> --- /dev/null
> +++ b/drivers/hwmon/pmbus/pli1209bc.c
> @@ -0,0 +1,114 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Hardware monitoring driver for Vicor PLI1209BC Digital Supervisor
> + *
> + * Copyright (c) 2022 9elements GmbH
> + */
> +
> +#include <linux/i2c.h>
> +#include <linux/module.h>
> +#include <linux/pmbus.h>
> +#include "pmbus.h"
> +
> +/*
> + * The capability command is only supported at page 0. Probing the device while
> + * the page register is set to 1 will falsely enable PEC support. Disable
> + * capability probing accordingly, since the PLI1209BC does not have any
> + * additional capabilities.
> + */
> +static struct pmbus_platform_data pli1209bc_plat_data = {
> + .flags = PMBUS_NO_CAPABILITY,
> +};
> +
> +static int pli1209bc_read_word_data(struct i2c_client *client, int page,
> + int phase, int reg)
> +{
> + int data;
> +
> + switch (reg) {
> + /* PMBUS_READ_POUT uses a direct format with R=0 */
> + case PMBUS_READ_POUT:
> + data = pmbus_read_word_data(client, page, phase, reg);
> + if (data < 0)
> + return data;
> + return data * 10;
We have to be more careful here. While the datasheet doesn't explicitly
say if the reported value is signed or not, the standard says that
it is supposed to be signed, so let's assume that this is the case.
That means that 'data' is really 16-bit signed value. We have to make
sure that it doesn't over- or underflow when multiplying it,
and that the sign is retained. Something like the following should do.
data = sign_extend32(data, 15) * 10;
return clamp_val(data, -32768, 32767) & 0xffff;
Otherwise the patch looks good.
Thanks,
Guenter
> + default:
> + return -ENODATA;
> + }
> +}
> +
> +static struct pmbus_driver_info pli1209bc_info = {
> + .pages = 2,
> + .format[PSC_VOLTAGE_IN] = direct,
> + .format[PSC_VOLTAGE_OUT] = direct,
> + .format[PSC_CURRENT_IN] = direct,
> + .format[PSC_CURRENT_OUT] = direct,
> + .format[PSC_POWER] = direct,
> + .format[PSC_TEMPERATURE] = direct,
> + .m[PSC_VOLTAGE_IN] = 1,
> + .b[PSC_VOLTAGE_IN] = 0,
> + .R[PSC_VOLTAGE_IN] = 1,
> + .m[PSC_VOLTAGE_OUT] = 1,
> + .b[PSC_VOLTAGE_OUT] = 0,
> + .R[PSC_VOLTAGE_OUT] = 1,
> + .m[PSC_CURRENT_IN] = 1,
> + .b[PSC_CURRENT_IN] = 0,
> + .R[PSC_CURRENT_IN] = 3,
> + .m[PSC_CURRENT_OUT] = 1,
> + .b[PSC_CURRENT_OUT] = 0,
> + .R[PSC_CURRENT_OUT] = 2,
> + .m[PSC_POWER] = 1,
> + .b[PSC_POWER] = 0,
> + .R[PSC_POWER] = 1,
> + .m[PSC_TEMPERATURE] = 1,
> + .b[PSC_TEMPERATURE] = 0,
> + .R[PSC_TEMPERATURE] = 0,
> + /*
> + * Page 0 sums up all attributes except voltage readings.
> + * The pli1209 digital supervisor only contains a single BCM, making
> + * page 0 redundant.
> + */
> + .func[1] = PMBUS_HAVE_VIN | PMBUS_HAVE_VOUT
> + | PMBUS_HAVE_IIN | PMBUS_HAVE_IOUT
> + | PMBUS_HAVE_PIN | PMBUS_HAVE_POUT
> + | PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP
> + | PMBUS_HAVE_STATUS_IOUT | PMBUS_HAVE_STATUS_INPUT,
> + .read_word_data = pli1209bc_read_word_data,
> +};
> +
> +static int pli1209bc_probe(struct i2c_client *client)
> +{
> + client->dev.platform_data = &pli1209bc_plat_data;
> + return pmbus_do_probe(client, &pli1209bc_info);
> +}
> +
> +static const struct i2c_device_id pli1209bc_id[] = {
> + {"pli1209bc", 0},
> + {}
> +};
> +
> +MODULE_DEVICE_TABLE(i2c, pli1209bc_id);
> +
> +#ifdef CONFIG_OF
> +static const struct of_device_id pli1209bc_of_match[] = {
> + { .compatible = "vicor,pli1209bc" },
> + { },
> +};
> +MODULE_DEVICE_TABLE(of, pli1209bc_of_match);
> +#endif
> +
> +static struct i2c_driver pli1209bc_driver = {
> + .driver = {
> + .name = "pli1209bc",
> + .of_match_table = of_match_ptr(pli1209bc_of_match),
> + },
> + .probe_new = pli1209bc_probe,
> + .id_table = pli1209bc_id,
> +};
> +
> +module_i2c_driver(pli1209bc_driver);
> +
> +MODULE_AUTHOR("Marcello Sylvester Bauer <sylv@...v.io>");
> +MODULE_DESCRIPTION("PMBus driver for Vicor PLI1209BC");
> +MODULE_LICENSE("GPL");
> +MODULE_IMPORT_NS(PMBUS);
Powered by blists - more mailing lists