linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/4] Support pli1209bc Digital Supervisor
@ 2022-02-14 12:40 Marcello Sylvester Bauer
  2022-02-14 12:40 ` [PATCH v2 1/4] dt-bindings: vendor-prefixes: add Vicor Corporation Marcello Sylvester Bauer
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Marcello Sylvester Bauer @ 2022-02-14 12:40 UTC (permalink / raw)
  To: Guenter Roeck, Jean Delvare
  Cc: linux-kernel, linux-hwmon, Patrick Rudolph, Marcello Sylvester Bauer

Hi,

This patch set adds support for PLI1209BC Digital Supervisor from Vicor
Corporation. It replaces the previous submitted driver "bcm6123" [1],
since there are multiple digital supervisors, which uses BCMs in different
configurations [2].

Changes in v2:
- Multiply PMBUS_READ_POUT with 10 (R=1)
  instead of dividing PMBUS_READ_PIN by 10.
- Set all pmbus formats to direct.
- Comment reason why page 0 is redundant.
- Import pmbus namespace.


[1]: https://www.spinics.net/lists/linux-hwmon/msg14097.html
[2]: https://www.spinics.net/lists/linux-hwmon/msg14123.html

Marcello Sylvester Bauer (4):
  dt-bindings: vendor-prefixes: add Vicor Corporation
  dt-bindings:trivial-devices: Add pli1209bc
  pmbus: Add support for pli1209bc
  pmbus (pli1209bc): Add regulator support

 .../devicetree/bindings/trivial-devices.yaml  |   2 +
 .../devicetree/bindings/vendor-prefixes.yaml  |   2 +
 Documentation/hwmon/pli1209bc.rst             |  73 +++++++++
 drivers/hwmon/pmbus/Kconfig                   |  16 ++
 drivers/hwmon/pmbus/Makefile                  |   1 +
 drivers/hwmon/pmbus/pli1209bc.c               | 145 ++++++++++++++++++
 6 files changed, 239 insertions(+)
 create mode 100644 Documentation/hwmon/pli1209bc.rst
 create mode 100644 drivers/hwmon/pmbus/pli1209bc.c

-- 
2.34.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v2 1/4] dt-bindings: vendor-prefixes: add Vicor Corporation
  2022-02-14 12:40 [PATCH v2 0/4] Support pli1209bc Digital Supervisor Marcello Sylvester Bauer
@ 2022-02-14 12:40 ` Marcello Sylvester Bauer
  2022-02-14 12:40 ` [PATCH v2 2/4] dt-bindings:trivial-devices: Add pli1209bc Marcello Sylvester Bauer
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 9+ messages in thread
From: Marcello Sylvester Bauer @ 2022-02-14 12:40 UTC (permalink / raw)
  To: Guenter Roeck, Jean Delvare, Rob Herring
  Cc: linux-kernel, linux-hwmon, Patrick Rudolph,
	Marcello Sylvester Bauer, Rob Herring, devicetree

Add vendor prefix for Vicor Corporation.

Signed-off-by: Marcello Sylvester Bauer <sylv@sylv.io>
---
 Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
index 294093d45a23..047a83a089ce 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -1298,6 +1298,8 @@ patternProperties:
     description: Vertexcom Technologies, Inc.
   "^via,.*":
     description: VIA Technologies, Inc.
+  "^vicor,.*":
+    description: Vicor Corporation
   "^videostrong,.*":
     description: Videostrong Technology Co., Ltd.
   "^virtio,.*":
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v2 2/4] dt-bindings:trivial-devices: Add pli1209bc
  2022-02-14 12:40 [PATCH v2 0/4] Support pli1209bc Digital Supervisor Marcello Sylvester Bauer
  2022-02-14 12:40 ` [PATCH v2 1/4] dt-bindings: vendor-prefixes: add Vicor Corporation Marcello Sylvester Bauer
@ 2022-02-14 12:40 ` Marcello Sylvester Bauer
  2022-02-14 12:40 ` [PATCH v2 3/4] pmbus: Add support for pli1209bc Marcello Sylvester Bauer
  2022-02-14 12:40 ` [PATCH v2 4/4] pmbus (pli1209bc): Add regulator support Marcello Sylvester Bauer
  3 siblings, 0 replies; 9+ messages in thread
From: Marcello Sylvester Bauer @ 2022-02-14 12:40 UTC (permalink / raw)
  To: Guenter Roeck, Jean Delvare, Rob Herring
  Cc: linux-kernel, linux-hwmon, Patrick Rudolph,
	Marcello Sylvester Bauer, Rob Herring, devicetree

Add trivial device entry for PLI1209BC Digital Supervisor from Vicor
Corporation.

Signed-off-by: Marcello Sylvester Bauer <sylv@sylv.io>
---
 Documentation/devicetree/bindings/trivial-devices.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/trivial-devices.yaml b/Documentation/devicetree/bindings/trivial-devices.yaml
index 091792ba993e..d03d90360aa0 100644
--- a/Documentation/devicetree/bindings/trivial-devices.yaml
+++ b/Documentation/devicetree/bindings/trivial-devices.yaml
@@ -354,6 +354,8 @@ properties:
           - ti,tps544c25
             # Winbond/Nuvoton H/W Monitor
           - winbond,w83793
+            # Vicor Corporation Digital Supervisor
+          - vicor,pli1209bc
             # i2c trusted platform module (TPM)
           - winbond,wpct301
 
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v2 3/4] pmbus: Add support for pli1209bc
  2022-02-14 12:40 [PATCH v2 0/4] Support pli1209bc Digital Supervisor Marcello Sylvester Bauer
  2022-02-14 12:40 ` [PATCH v2 1/4] dt-bindings: vendor-prefixes: add Vicor Corporation Marcello Sylvester Bauer
  2022-02-14 12:40 ` [PATCH v2 2/4] dt-bindings:trivial-devices: Add pli1209bc Marcello Sylvester Bauer
@ 2022-02-14 12:40 ` Marcello Sylvester Bauer
  2022-02-14 17:36   ` Guenter Roeck
  2022-02-14 12:40 ` [PATCH v2 4/4] pmbus (pli1209bc): Add regulator support Marcello Sylvester Bauer
  3 siblings, 1 reply; 9+ messages in thread
From: Marcello Sylvester Bauer @ 2022-02-14 12:40 UTC (permalink / raw)
  To: Guenter Roeck, Jean Delvare, Jonathan Corbet
  Cc: linux-kernel, linux-hwmon, Patrick Rudolph,
	Marcello Sylvester Bauer, linux-doc

PLI1209BC is a Digital Supervisor from Vicor Corporation.

Signed-off-by: Marcello Sylvester Bauer <sylv@sylv.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@sylv.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;
+	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@sylv.io>");
+MODULE_DESCRIPTION("PMBus driver for Vicor PLI1209BC");
+MODULE_LICENSE("GPL");
+MODULE_IMPORT_NS(PMBUS);
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v2 4/4] pmbus (pli1209bc): Add regulator support
  2022-02-14 12:40 [PATCH v2 0/4] Support pli1209bc Digital Supervisor Marcello Sylvester Bauer
                   ` (2 preceding siblings ...)
  2022-02-14 12:40 ` [PATCH v2 3/4] pmbus: Add support for pli1209bc Marcello Sylvester Bauer
@ 2022-02-14 12:40 ` Marcello Sylvester Bauer
  2022-02-14 17:40   ` Guenter Roeck
  3 siblings, 1 reply; 9+ messages in thread
From: Marcello Sylvester Bauer @ 2022-02-14 12:40 UTC (permalink / raw)
  To: Guenter Roeck, Jean Delvare
  Cc: linux-kernel, linux-hwmon, Patrick Rudolph, Marcello Sylvester Bauer

Add regulator support for PLI1209BC Digital Supervisor.

Signed-off-by: Marcello Sylvester Bauer <sylv@sylv.io>
---
 drivers/hwmon/pmbus/Kconfig     |  7 +++++++
 drivers/hwmon/pmbus/pli1209bc.c | 31 +++++++++++++++++++++++++++++++
 2 files changed, 38 insertions(+)

diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig
index 831db423bea0..8b8f0d8733b2 100644
--- a/drivers/hwmon/pmbus/Kconfig
+++ b/drivers/hwmon/pmbus/Kconfig
@@ -319,6 +319,13 @@ config SENSORS_PLI1209BC
 	  This driver can also be built as a module. If so, the module will
 	  be called pli1209bc.
 
+config SENSORS_PLI1209BC_REGULATOR
+	bool "Regulator support for PLI1209BC"
+	depends on SENSORS_PLI1209BC && REGULATOR
+	help
+	  If you say yes here you get regulator support for Vicor PLI1209BC
+	  Digital Supervisor.
+
 config SENSORS_PM6764TR
 	tristate "ST PM6764TR"
 	help
diff --git a/drivers/hwmon/pmbus/pli1209bc.c b/drivers/hwmon/pmbus/pli1209bc.c
index 8a9af2ccc46f..7212d73f6e04 100644
--- a/drivers/hwmon/pmbus/pli1209bc.c
+++ b/drivers/hwmon/pmbus/pli1209bc.c
@@ -8,6 +8,7 @@
 #include <linux/i2c.h>
 #include <linux/module.h>
 #include <linux/pmbus.h>
+#include <linux/regulator/driver.h>
 #include "pmbus.h"
 
 /*
@@ -32,11 +33,37 @@ static int pli1209bc_read_word_data(struct i2c_client *client, int page,
 		if (data < 0)
 			return data;
 		return data * 10;
+	/*
+	 * PMBUS_READ_VOUT and PMBUS_READ_TEMPERATURE_1 return invalid data
+	 * when the BCM is turned off. Since it is not possible to return
+	 * ENODATA error, return zero instead.
+	 */
+	case PMBUS_READ_VOUT:
+	case PMBUS_READ_TEMPERATURE_1:
+		data = pmbus_read_word_data(client, page, phase,
+					    PMBUS_STATUS_WORD);
+		if (data < 0)
+			return data;
+		if (data & PB_STATUS_POWER_GOOD_N)
+			return 0L;
+		return pmbus_read_word_data(client, page, phase, reg);
 	default:
 		return -ENODATA;
 	}
 }
 
+#if IS_ENABLED(CONFIG_SENSORS_PLI1209BC_REGULATOR)
+static const struct regulator_desc pli1209bc_reg_desc = {
+	.name = "vout2",
+	.id = 1,
+	.of_match = of_match_ptr("vout2"),
+	.regulators_node = of_match_ptr("regulators"),
+	.ops = &pmbus_regulator_ops,
+	.type = REGULATOR_VOLTAGE,
+	.owner = THIS_MODULE,
+};
+#endif
+
 static struct pmbus_driver_info pli1209bc_info = {
 	.pages = 2,
 	.format[PSC_VOLTAGE_IN] = direct,
@@ -74,6 +101,10 @@ static struct pmbus_driver_info pli1209bc_info = {
 	    | PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP
 	    | PMBUS_HAVE_STATUS_IOUT | PMBUS_HAVE_STATUS_INPUT,
 	.read_word_data = pli1209bc_read_word_data,
+#if IS_ENABLED(CONFIG_SENSORS_PLI1209BC_REGULATOR)
+	.num_regulators = 1,
+	.reg_desc = &pli1209bc_reg_desc,
+#endif
 };
 
 static int pli1209bc_probe(struct i2c_client *client)
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 3/4] pmbus: Add support for pli1209bc
  2022-02-14 12:40 ` [PATCH v2 3/4] pmbus: Add support for pli1209bc Marcello Sylvester Bauer
@ 2022-02-14 17:36   ` Guenter Roeck
  2022-02-14 18:34     ` sylv
  0 siblings, 1 reply; 9+ messages in thread
From: Guenter Roeck @ 2022-02-14 17:36 UTC (permalink / raw)
  To: Marcello Sylvester Bauer, Jean Delvare, Jonathan Corbet
  Cc: linux-kernel, linux-hwmon, Patrick Rudolph, linux-doc

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@sylv.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@sylv.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@sylv.io>");
> +MODULE_DESCRIPTION("PMBus driver for Vicor PLI1209BC");
> +MODULE_LICENSE("GPL");
> +MODULE_IMPORT_NS(PMBUS);


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 4/4] pmbus (pli1209bc): Add regulator support
  2022-02-14 12:40 ` [PATCH v2 4/4] pmbus (pli1209bc): Add regulator support Marcello Sylvester Bauer
@ 2022-02-14 17:40   ` Guenter Roeck
  2022-02-14 18:19     ` sylv
  0 siblings, 1 reply; 9+ messages in thread
From: Guenter Roeck @ 2022-02-14 17:40 UTC (permalink / raw)
  To: Marcello Sylvester Bauer, Jean Delvare
  Cc: linux-kernel, linux-hwmon, Patrick Rudolph

On 2/14/22 04:40, Marcello Sylvester Bauer wrote:
> Add regulator support for PLI1209BC Digital Supervisor.
> 
> Signed-off-by: Marcello Sylvester Bauer <sylv@sylv.io>
> ---
>   drivers/hwmon/pmbus/Kconfig     |  7 +++++++
>   drivers/hwmon/pmbus/pli1209bc.c | 31 +++++++++++++++++++++++++++++++
>   2 files changed, 38 insertions(+)
> 
> diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig
> index 831db423bea0..8b8f0d8733b2 100644
> --- a/drivers/hwmon/pmbus/Kconfig
> +++ b/drivers/hwmon/pmbus/Kconfig
> @@ -319,6 +319,13 @@ config SENSORS_PLI1209BC
>   	  This driver can also be built as a module. If so, the module will
>   	  be called pli1209bc.
>   
> +config SENSORS_PLI1209BC_REGULATOR
> +	bool "Regulator support for PLI1209BC"
> +	depends on SENSORS_PLI1209BC && REGULATOR
> +	help
> +	  If you say yes here you get regulator support for Vicor PLI1209BC
> +	  Digital Supervisor.
> +
>   config SENSORS_PM6764TR
>   	tristate "ST PM6764TR"
>   	help
> diff --git a/drivers/hwmon/pmbus/pli1209bc.c b/drivers/hwmon/pmbus/pli1209bc.c
> index 8a9af2ccc46f..7212d73f6e04 100644
> --- a/drivers/hwmon/pmbus/pli1209bc.c
> +++ b/drivers/hwmon/pmbus/pli1209bc.c
> @@ -8,6 +8,7 @@
>   #include <linux/i2c.h>
>   #include <linux/module.h>
>   #include <linux/pmbus.h>
> +#include <linux/regulator/driver.h>
>   #include "pmbus.h"
>   
>   /*
> @@ -32,11 +33,37 @@ static int pli1209bc_read_word_data(struct i2c_client *client, int page,
>   		if (data < 0)
>   			return data;
>   		return data * 10;
> +	/*
> +	 * PMBUS_READ_VOUT and PMBUS_READ_TEMPERATURE_1 return invalid data
> +	 * when the BCM is turned off. Since it is not possible to return
> +	 * ENODATA error, return zero instead.
> +	 */
> +	case PMBUS_READ_VOUT:
> +	case PMBUS_READ_TEMPERATURE_1:
> +		data = pmbus_read_word_data(client, page, phase,
> +					    PMBUS_STATUS_WORD);
> +		if (data < 0)
> +			return data;
> +		if (data & PB_STATUS_POWER_GOOD_N)
> +			return 0L;

Why 0L ? The return value of pli1209bc_read_word_data() is int.

> +		return pmbus_read_word_data(client, page, phase, reg);
>   	default:
>   		return -ENODATA;
>   	}
>   }
>   
> +#if IS_ENABLED(CONFIG_SENSORS_PLI1209BC_REGULATOR)
> +static const struct regulator_desc pli1209bc_reg_desc = {
> +	.name = "vout2",
> +	.id = 1,
> +	.of_match = of_match_ptr("vout2"),
> +	.regulators_node = of_match_ptr("regulators"),
> +	.ops = &pmbus_regulator_ops,
> +	.type = REGULATOR_VOLTAGE,
> +	.owner = THIS_MODULE,
> +};
> +#endif
> +
>   static struct pmbus_driver_info pli1209bc_info = {
>   	.pages = 2,
>   	.format[PSC_VOLTAGE_IN] = direct,
> @@ -74,6 +101,10 @@ static struct pmbus_driver_info pli1209bc_info = {
>   	    | PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP
>   	    | PMBUS_HAVE_STATUS_IOUT | PMBUS_HAVE_STATUS_INPUT,
>   	.read_word_data = pli1209bc_read_word_data,
> +#if IS_ENABLED(CONFIG_SENSORS_PLI1209BC_REGULATOR)
> +	.num_regulators = 1,
> +	.reg_desc = &pli1209bc_reg_desc,
> +#endif
>   };
>   
>   static int pli1209bc_probe(struct i2c_client *client)


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 4/4] pmbus (pli1209bc): Add regulator support
  2022-02-14 17:40   ` Guenter Roeck
@ 2022-02-14 18:19     ` sylv
  0 siblings, 0 replies; 9+ messages in thread
From: sylv @ 2022-02-14 18:19 UTC (permalink / raw)
  To: Guenter Roeck, Jean Delvare; +Cc: linux-kernel, linux-hwmon, Patrick Rudolph

On Mon, 2022-02-14 at 09:40 -0800, Guenter Roeck wrote:
> On 2/14/22 04:40, Marcello Sylvester Bauer wrote:
> > Add regulator support for PLI1209BC Digital Supervisor.
> > 
> > Signed-off-by: Marcello Sylvester Bauer <sylv@sylv.io>
> > ---
> >   drivers/hwmon/pmbus/Kconfig     |  7 +++++++
> >   drivers/hwmon/pmbus/pli1209bc.c | 31
> > +++++++++++++++++++++++++++++++
> >   2 files changed, 38 insertions(+)
> > 
> > diff --git a/drivers/hwmon/pmbus/Kconfig
> > b/drivers/hwmon/pmbus/Kconfig
> > index 831db423bea0..8b8f0d8733b2 100644
> > --- a/drivers/hwmon/pmbus/Kconfig
> > +++ b/drivers/hwmon/pmbus/Kconfig
> > @@ -319,6 +319,13 @@ config SENSORS_PLI1209BC
> >           This driver can also be built as a module. If so, the
> > module will
> >           be called pli1209bc.
> >   
> > +config SENSORS_PLI1209BC_REGULATOR
> > +       bool "Regulator support for PLI1209BC"
> > +       depends on SENSORS_PLI1209BC && REGULATOR
> > +       help
> > +         If you say yes here you get regulator support for Vicor
> > PLI1209BC
> > +         Digital Supervisor.
> > +
> >   config SENSORS_PM6764TR
> >         tristate "ST PM6764TR"
> >         help
> > diff --git a/drivers/hwmon/pmbus/pli1209bc.c
> > b/drivers/hwmon/pmbus/pli1209bc.c
> > index 8a9af2ccc46f..7212d73f6e04 100644
> > --- a/drivers/hwmon/pmbus/pli1209bc.c
> > +++ b/drivers/hwmon/pmbus/pli1209bc.c
> > @@ -8,6 +8,7 @@
> >   #include <linux/i2c.h>
> >   #include <linux/module.h>
> >   #include <linux/pmbus.h>
> > +#include <linux/regulator/driver.h>
> >   #include "pmbus.h"
> >   
> >   /*
> > @@ -32,11 +33,37 @@ static int pli1209bc_read_word_data(struct
> > i2c_client *client, int page,
> >                 if (data < 0)
> >                         return data;
> >                 return data * 10;
> > +       /*
> > +        * PMBUS_READ_VOUT and PMBUS_READ_TEMPERATURE_1 return
> > invalid data
> > +        * when the BCM is turned off. Since it is not possible to
> > return
> > +        * ENODATA error, return zero instead.
> > +        */
> > +       case PMBUS_READ_VOUT:
> > +       case PMBUS_READ_TEMPERATURE_1:
> > +               data = pmbus_read_word_data(client, page, phase,
> > +                                           PMBUS_STATUS_WORD);
> > +               if (data < 0)
> > +                       return data;
> > +               if (data & PB_STATUS_POWER_GOOD_N)
> > +                       return 0L;
> 
> Why 0L ? The return value of pli1209bc_read_word_data() is int.

ack. 0L makes no sense here.

> 
> > +               return pmbus_read_word_data(client, page, phase,
> > reg);
> >         default:
> >                 return -ENODATA;
> >         }
> >   }
> >   
> > +#if IS_ENABLED(CONFIG_SENSORS_PLI1209BC_REGULATOR)
> > +static const struct regulator_desc pli1209bc_reg_desc = {
> > +       .name = "vout2",
> > +       .id = 1,
> > +       .of_match = of_match_ptr("vout2"),
> > +       .regulators_node = of_match_ptr("regulators"),
> > +       .ops = &pmbus_regulator_ops,
> > +       .type = REGULATOR_VOLTAGE,
> > +       .owner = THIS_MODULE,
> > +};
> > +#endif
> > +
> >   static struct pmbus_driver_info pli1209bc_info = {
> >         .pages = 2,
> >         .format[PSC_VOLTAGE_IN] = direct,
> > @@ -74,6 +101,10 @@ static struct pmbus_driver_info pli1209bc_info
> > = {
> >             | PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP
> >             | PMBUS_HAVE_STATUS_IOUT | PMBUS_HAVE_STATUS_INPUT,
> >         .read_word_data = pli1209bc_read_word_data,
> > +#if IS_ENABLED(CONFIG_SENSORS_PLI1209BC_REGULATOR)
> > +       .num_regulators = 1,
> > +       .reg_desc = &pli1209bc_reg_desc,
> > +#endif
> >   };
> >   
> >   static int pli1209bc_probe(struct i2c_client *client)
> 


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 3/4] pmbus: Add support for pli1209bc
  2022-02-14 17:36   ` Guenter Roeck
@ 2022-02-14 18:34     ` sylv
  0 siblings, 0 replies; 9+ messages in thread
From: sylv @ 2022-02-14 18:34 UTC (permalink / raw)
  To: Guenter Roeck, Jean Delvare, Jonathan Corbet
  Cc: linux-kernel, linux-hwmon, Patrick Rudolph, linux-doc

On Mon, 2022-02-14 at 09:36 -0800, Guenter Roeck wrote:
> 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@sylv.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@sylv.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;

Being careful makes sense indeed.
I guess we should not use definitions in limit.h, since int size is
arch specific. I'll take it like this.

Thanks,
Marcello

> 
> 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@sylv.io>");
> > +MODULE_DESCRIPTION("PMBus driver for Vicor PLI1209BC");
> > +MODULE_LICENSE("GPL");
> > +MODULE_IMPORT_NS(PMBUS);
> 


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2022-02-14 18:34 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-14 12:40 [PATCH v2 0/4] Support pli1209bc Digital Supervisor Marcello Sylvester Bauer
2022-02-14 12:40 ` [PATCH v2 1/4] dt-bindings: vendor-prefixes: add Vicor Corporation Marcello Sylvester Bauer
2022-02-14 12:40 ` [PATCH v2 2/4] dt-bindings:trivial-devices: Add pli1209bc Marcello Sylvester Bauer
2022-02-14 12:40 ` [PATCH v2 3/4] pmbus: Add support for pli1209bc Marcello Sylvester Bauer
2022-02-14 17:36   ` Guenter Roeck
2022-02-14 18:34     ` sylv
2022-02-14 12:40 ` [PATCH v2 4/4] pmbus (pli1209bc): Add regulator support Marcello Sylvester Bauer
2022-02-14 17:40   ` Guenter Roeck
2022-02-14 18:19     ` sylv

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).