Support pli1209bc Digital Supervisor

PATCH Superseded Marcello Sylvester Bauer 3 participants 10 messages Feb 11, 2022 – Feb 14, 2022

Subsystems (3)

Device tree bindings Documentation Hardware monitoring

lore

Share & tools

lore

Marcello Sylvester Bauer

author

2022-02-11 17:17 UTC #1 lore raw

PATCH 1/4 dt-bindings: vendor-prefixes: add Vicor Corporation

In reply to Marcello Sylvester Bauer

Add vendor prefix for Vicor Corporation.

Signed-off-by Marcello Sylvester Bauer <[email protected]>

---
 Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
 1 file changed, 2 insertions(+)

Documentation/devicetree/bindings/vendor-prefixes.yaml +2 -0

index 294093d45a23..047a83a089ce 100644
@@ -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,.*":

Signature · 1 line

2.34.1

Marcello Sylvester Bauer

author

2022-02-11 17:17 UTC #2 lore raw

PATCH 2/4 dt-bindings:trivial-devices: Add pli1209bc

In reply to Marcello Sylvester Bauer

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

Signed-off-by Marcello Sylvester Bauer <[email protected]>

---
 Documentation/devicetree/bindings/trivial-devices.yaml | 2 ++
 1 file changed, 2 insertions(+)

Documentation/devicetree/bindings/trivial-devices.yaml +2 -0

index 091792ba993e..d03d90360aa0 100644
@@ -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

Signature · 1 line

2.34.1

Marcello Sylvester Bauer

author

2022-02-11 17:17 UTC #7 lore raw

PATCH 4/4 pmbus (pli1209bc): Add regulator support

In reply to Marcello Sylvester Bauer

Add regulator support for PLI1209BC Digital Supervisor.

Signed-off-by Marcello Sylvester Bauer <[email protected]>

---
 drivers/hwmon/pmbus/Kconfig     |  8 ++++++++
 drivers/hwmon/pmbus/pli1209bc.c | 33 +++++++++++++++++++++++++++++++++
 2 files changed, 41 insertions(+)

drivers/hwmon/pmbus/Kconfig +8 -0

index 831db423bea0..e64e0f2d93c9 100644
@@ -319,6 +319,14 @@ 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

drivers/hwmon/pmbus/pli1209bc.c +33 -0

index da345eb105fd..ec479b22aa10 100644
@@ -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,39 @@ static int pli1209bc_read_word_data(struct i2c_client *client, int page,
 		if (data < 0)
 			return data;
 		return div_s64(data + 5LL, 10L);
+#if IS_ENABLED(CONFIG_SENSORS_PLI1209BC_REGULATOR)
+	/*
+	 * PMBUS_READ_VOUT and PMBUS_READ_TEMPERATURE_1 returns 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);
+#endif
 	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,

@@ -64,6 +93,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)

Signature · 1 line

2.34.1

Marcello Sylvester Bauer

author

2022-02-11 17:17 UTC #0 lore raw

cover Support pli1209bc Digital Supervisor

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].

[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                   |  17 +++
 drivers/hwmon/pmbus/Makefile                  |   1 +
 drivers/hwmon/pmbus/pli1209bc.c               | 137 ++++++++++++++++++
 6 files changed, 232 insertions(+)
 create mode 100644 Documentation/hwmon/pli1209bc.rst
 create mode 100644 drivers/hwmon/pmbus/pli1209bc.c

Signature · 1 line

2.34.1

Marcello Sylvester Bauer

author

2022-02-11 17:18 UTC #3 lore raw

PATCH 3/4 pmbus: Add support for pli1209bc

PLI1209BC is a Digital Supervisor from Vicor Corporation.

Signed-off-by Marcello Sylvester Bauer <[email protected]>

---
 Documentation/hwmon/pli1209bc.rst |  73 +++++++++++++++++++++
 drivers/hwmon/pmbus/Kconfig       |   9 +++
 drivers/hwmon/pmbus/Makefile      |   1 +
 drivers/hwmon/pmbus/pli1209bc.c   | 104 ++++++++++++++++++++++++++++++
 4 files changed, 187 insertions(+)
 create mode 100644 Documentation/hwmon/pli1209bc.rst
 create mode 100644 drivers/hwmon/pmbus/pli1209bc.c

Documentation/hwmon/pli1209bc.rst new file +73 -0

new file mode 100644
index 000000000000..a3f686d03cf2
@@ -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 <[email protected]>
+
+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.
+======================= ========================================================

drivers/hwmon/pmbus/Kconfig +9 -0

index c96f7b7338bd..831db423bea0 100644
@@ -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

drivers/hwmon/pmbus/Makefile +1 -0

index e5935f70c9e0..7ce74e3b8552 100644
@@ -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

drivers/hwmon/pmbus/pli1209bc.c new file +104 -0

104 lines not on this page · raw message

Signature · 1 line

2.34.1

Guenter Roeck

maintainer

2022-02-11 19:40 UTC #4 lore raw

Re: [PATCH v1 3/4] pmbus: Add support for pli1209bc

Quoted 153 lines · Marcello Sylvester Bauer

PLI1209BC is a Digital Supervisor from Vicor Corporation.

Signed-off-by Marcello Sylvester Bauer <[email protected]>

---
  Documentation/hwmon/pli1209bc.rst |  73 +++++++++++++++++++++
  drivers/hwmon/pmbus/Kconfig       |   9 +++
  drivers/hwmon/pmbus/Makefile      |   1 +
  drivers/hwmon/pmbus/pli1209bc.c   | 104 ++++++++++++++++++++++++++++++
  4 files changed, 187 insertions(+)

create mode 100644 Documentation/hwmon/pli1209bc.rst create mode 100644 drivers/hwmon/pmbus/pli1209bc.c

Documentation/hwmon/pli1209bc.rst new file +73 -0

new file mode 100644
index 000000000000..a3f686d03cf2
@@ -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 <[email protected]>
+
+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.
+======================= ========================================================

drivers/hwmon/pmbus/Kconfig +9 -0

index c96f7b7338bd..831db423bea0 100644
@@ -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

drivers/hwmon/pmbus/Makefile +1 -0

index e5935f70c9e0..7ce74e3b8552 100644
@@ -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

drivers/hwmon/pmbus/pli1209bc.c new file +24 -0

new file mode 100644
index 000000000000..da345eb105fd
@@ -0,0 +1,104 @@
+// 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)

drivers/hwmon/pmbus/pli1209bc.c line 34

Quoted 10 lines

drivers/hwmon/pmbus/pli1209bc.c new file +10 -0

+{
+	int data;
+
+	switch (reg) {
+	/* PMBUS_READ_PIN uses a direct format with R=1 */
+	case PMBUS_READ_PIN:
+		data = pmbus_read_word_data(client, page, phase, reg);
+		if (data < 0)
+			return data;
+		return div_s64(data + 5LL, 10L);

This results in a loss of precision. I think it would be better to avoid that and instead model the output power as direct format with R=1, and multiply register values reported for PMBUS_READ_POUT with 10.

drivers/hwmon/pmbus/pli1209bc.c line 47

Quoted 13 lines

drivers/hwmon/pmbus/pli1209bc.c new file +13 -0

+	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] = linear,
+	.format[PSC_TEMPERATURE] = linear,

Those look like direct values to me. Linear would mean there is a mantissa and an exponent, but I don't see that in the datasheet. Please clarify; we don't want to have power values of 1024 and above show up as negative numbers.

drivers/hwmon/pmbus/pli1209bc.c line 60

Quoted 13 lines

drivers/hwmon/pmbus/pli1209bc.c new file +13 -0

+	.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,
+	.func[0] = 0, /* supervisor summing page without voltage readings */

As far as I can see from the manual, the actually reported values for other attributes match those of page 1, making the values on page 0 redundant. I think this should be mentioned in a comment to avoid confusion.

Quoted 20 lines

drivers/hwmon/pmbus/pli1209bc.c new file +20 -0

+	.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);

drivers/hwmon/pmbus/pli1209bc.c line 90

Quoted 10 lines

drivers/hwmon/pmbus/pli1209bc.c new file +10 -0

+
+#ifdef CONFIG_OF
+static const struct of_device_id pli1209bc_of_match[] = {
+	{ .compatible = "vicor,pli1209bc" },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, pli1209bc_of_match);
+#endif
+
+/* This is the driver that will be inserted */

Pretty useless comment.

Quoted 14 lines

drivers/hwmon/pmbus/pli1209bc.c new file +14 -0

+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 <[email protected]>");
+MODULE_DESCRIPTION("PMBus driver for Vicor PLI1209BC");
+MODULE_LICENSE("GPL");

Guenter Roeck

maintainer

2022-02-11 19:44 UTC #8 lore raw

Re: [PATCH v1 4/4] pmbus (pli1209bc): Add regulator support

In reply to Marcello Sylvester Bauer

Quoted 14 lines · Marcello Sylvester Bauer

Add regulator support for PLI1209BC Digital Supervisor.
Signed-off-by Marcello Sylvester Bauer <[email protected]>
---
  drivers/hwmon/pmbus/Kconfig     |  8 ++++++++
  drivers/hwmon/pmbus/pli1209bc.c | 33 +++++++++++++++++++++++++++++++++
  2 files changed, 41 insertions(+)
drivers/hwmon/pmbus/Kconfig +0 -0
index 831db423bea0..e64e0f2d93c9 100644
@@ -319,6 +319,14 @@ config SENSORS_PLI1209BC
  	  This driver can also be built as a module. If so, the module will

drivers/hwmon/pmbus/Kconfig config SENSORS_PLI1209BC · line 329

Quoted 10 lines

drivers/hwmon/pmbus/Kconfig +8 -0

  	  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.
+
+

Single empty line please

Quoted 16 lines

drivers/hwmon/pmbus/Kconfig +0 -0

  config SENSORS_PM6764TR
  	tristate "ST PM6764TR"
  	help

drivers/hwmon/pmbus/pli1209bc.c +1 -0

index da345eb105fd..ec479b22aa10 100644
@@ -8,6 +8,7 @@
  #include <linux/i2c.h>
  #include <linux/module.h>
  #include <linux/pmbus.h>
+#include <linux/regulator/driver.h>
  #include "pmbus.h"
 
  /*

drivers/hwmon/pmbus/pli1209bc.c static int pli1209bc_read_word_data(struct i2c_client *client, int page, · line 36

Quoted 4 lines

drivers/hwmon/pmbus/pli1209bc.c +1 -0

@@ -32,11 +33,39 @@ static int pli1209bc_read_word_data(struct i2c_client *client, int page,
  		if (data < 0)
  			return data;
  		return div_s64(data + 5LL, 10L);
+#if IS_ENABLED(CONFIG_SENSORS_PLI1209BC_REGULATOR)

I don't think this needs to be conditional.

drivers/hwmon/pmbus/pli1209bc.c static int pli1209bc_read_word_data(struct i2c_client *client, int page, · line 38

Quoted 2 lines

drivers/hwmon/pmbus/pli1209bc.c +2 -0

+	/*
+	 * PMBUS_READ_VOUT and PMBUS_READ_TEMPERATURE_1 returns invalid data,

s/returns/return/ and no ','

Quoted 44 lines

drivers/hwmon/pmbus/pli1209bc.c +29 -0

+	 * 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);
+#endif
  	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,

@@ -64,6 +93,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)

kernel test robot

build bot

2022-02-12 23:13 UTC #6 lore raw

Re: [PATCH v1 3/4] pmbus: Add support for pli1209bc

In reply to Marcello Sylvester Bauer

Thank you for the patch! Yet something to improve:

Hi Marcello,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on groeck-staging/hwmon-next]
[also build test ERROR on robh/for-next v5.17-rc3 next-20220211] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Marcello-Sylvester-Bauer/Support-pli1209bc-Digital-Supervisor/20220212-011956
base:   https://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git hwmon-next
config: hexagon-randconfig-r024-20220213 (https://download.01.org/0day-ci/archive/20220213/202202130705.3YDPfyow-lkp(a)intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project c7eb84634519e6497be42f5fe323f9a04ed67127)

reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/0b0d15e123c13b2eb898502661b18d81ed87e325
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Marcello-Sylvester-Bauer/Support-pli1209bc-Digital-Supervisor/20220212-011956 git checkout 0b0d15e123c13b2eb898502661b18d81ed87e325
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash

If you fix the issue, kindly add following tag as appropriate

Reported-by kernel test robot <[email protected]>

All errors (new ones prefixed by >>, old ones prefixed by <<):

>> ERROR: modpost: module pli1209bc uses symbol pmbus_read_word_data from namespace PMBUS, but does not import it.
>> ERROR: modpost: module pli1209bc uses symbol pmbus_do_probe from namespace PMBUS, but does not import it.

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

sylv

author

2022-02-14 09:26 UTC #5 lore raw

Re: [PATCH v1 3/4] pmbus: Add support for pli1209bc

In reply to Guenter Roeck

Quoted 183 lines · Guenter Roeck

Quoted 182 lines · Marcello Sylvester Bauer
PLI1209BC is a Digital Supervisor from Vicor Corporation.
Signed-off-by Marcello Sylvester Bauer <[email protected]>
---
Documentation/hwmon/pli1209bc.rst | 73 +++++++++++++++++++++ drivers/hwmon/pmbus/Kconfig | 9 +++ drivers/hwmon/pmbus/Makefile | 1 + drivers/hwmon/pmbus/pli1209bc.c | 104
++++++++++++++++++++++++++++++
  4 files changed, 187 insertions(+)
create mode 100644 Documentation/hwmon/pli1209bc.rst create mode 100644 drivers/hwmon/pmbus/pli1209bc.c
Documentation/hwmon/pli1209bc.rst new file +12 -0
new file mode 100644
index 000000000000..a3f686d03cf2
@@ -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
Documentation/hwmon/pli1209bc.rst new file +8 -0
+
+Authors:
+    - Marcello Sylvester Bauer <[email protected]>
+
+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
Documentation/hwmon/pli1209bc.rst new file +6 -0
+can configure, set protection limits, and monitor the BCM.
+
+Sysfs entries
+-------------
+
+=======================
========================================================
Documentation/hwmon/pli1209bc.rst new file +44 -0
+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.
+=======================
========================================================
drivers/hwmon/pmbus/Kconfig +0 -0
index c96f7b7338bd..831db423bea0 100644
@@ -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
drivers/hwmon/pmbus/Makefile +0 -0
index e5935f70c9e0..7ce74e3b8552 100644
@@ -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
drivers/hwmon/pmbus/pli1209bc.c new file +3 -0
new file mode 100644
index 000000000000..da345eb105fd
@@ -0,0 +1,104 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Hardware monitoring driver for Vicor PLI1209BC Digital
Supervisor
+ *
+ * Copyright (c) 2022 9elements GmbH
+ */
drivers/hwmon/pmbus/pli1209bc.c new file +8 -0
+
+#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.
+ */
drivers/hwmon/pmbus/pli1209bc.c new file +5 -0
+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)
drivers/hwmon/pmbus/pli1209bc.c new file +7 -0
+{
+       int data;
+
+       switch (reg) {
+       /* PMBUS_READ_PIN uses a direct format with R=1 */
+       case PMBUS_READ_PIN:
+               data = pmbus_read_word_data(client, page, phase,
reg);
+ if (data < 0)

Quoted 9 lines

Quoted 2 lines
+ return data;
+ return div_s64(data + 5LL, 10L);
This results in a loss of precision. I think it would be better to avoid that
and instead model the output power as direct format with R=1, and multiply
register values reported for PMBUS_READ_POUT with 10.

Makes sense, ack.

Quoted 10 lines

Quoted 10 lines
+       default:
+               return -ENODATA;
+       }
drivers/hwmon/pmbus/pli1209bc.c new file +7 -0
+}
+
+static struct pmbus_driver_info pli1209bc_info = {
+       .pages = 2,
+       .format[PSC_VOLTAGE_IN] = direct,
+       .format[PSC_VOLTAGE_OUT] = direct,
+       .format[PSC_CURRENT_IN] = direct,

Quoted 8 lines

Quoted 3 lines
+       .format[PSC_CURRENT_OUT] = direct,
+       .format[PSC_POWER] = linear,
+       .format[PSC_TEMPERATURE] = linear,
Those look like direct values to me. Linear would mean there is a mantissa and an exponent, but I don't see that in the datasheet. Please clarify; we don't want to have power values of 1024 and above show up as negative numbers.

My bad. I misunderstood the meaning of linear and thought it means direct m=1,R=0,b=0. Thanks.

Quoted 16 lines

Quoted 15 lines
+       .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,
+       .func[0] = 0, /* supervisor summing page without voltage readings */

Quoted 6 lines

As far as I can see from the manual, the actually reported values for other
attributes match those of page 1, making the values on page 0 redundant.
I think this should be mentioned in a comment to avoid confusion.

Sure, sgtm.

Quoted 25 lines

+ .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,

drivers/hwmon/pmbus/pli1209bc.c new file +18 -0

+};
+
+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" },

Quoted 8 lines

drivers/hwmon/pmbus/pli1209bc.c line 90
Quoted 6 lines
+ { },
drivers/hwmon/pmbus/pli1209bc.c new file +5 -0
+};
+MODULE_DEVICE_TABLE(of, pli1209bc_of_match);
+#endif
+
+/* This is the driver that will be inserted */
Pretty useless comment.

ack.

Thanks for your quick review Guenter.

Marcello

Quoted 17 lines

Quoted 16 lines
+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,
drivers/hwmon/pmbus/pli1209bc.c new file +7 -0
+};
+
+module_i2c_driver(pli1209bc_driver);
+
+MODULE_AUTHOR("Marcello Sylvester Bauer <[email protected]>");
+MODULE_DESCRIPTION("PMBus driver for Vicor PLI1209BC");
+MODULE_LICENSE("GPL");

sylv

author

2022-02-14 09:33 UTC #9 lore raw

Re: [PATCH v1 4/4] pmbus (pli1209bc): Add regulator support

In reply to Guenter Roeck

Quoted 23 lines · Guenter Roeck

Quoted 22 lines · Marcello Sylvester Bauer
Add regulator support for PLI1209BC Digital Supervisor.
Signed-off-by Marcello Sylvester Bauer <[email protected]>
---
drivers/hwmon/pmbus/Kconfig | 8 ++++++++
drivers/hwmon/pmbus/pli1209bc.c | 33
+++++++++++++++++++++++++++++++++
  2 files changed, 41 insertions(+)
drivers/hwmon/pmbus/Kconfig +0 -0
index 831db423bea0..e64e0f2d93c9 100644
@@ -319,6 +319,14 @@ 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

Quoted 8 lines

drivers/hwmon/pmbus/Kconfig config SENSORS_PLI1209BC · line 329
Quoted 6 lines
+       help
+         If you say yes here you get regulator support for Vicor PLI1209BC
+         Digital Supervisor.
drivers/hwmon/pmbus/Kconfig +2 -0
+
+
Single empty line please

ack.

Quoted 17 lines

config SENSORS_PM6764TR
tristate "ST PM6764TR"
help

drivers/hwmon/pmbus/pli1209bc.c +1 -0

index da345eb105fd..ec479b22aa10 100644
@@ -8,6 +8,7 @@
   #include <linux/i2c.h>
   #include <linux/module.h>
   #include <linux/pmbus.h>
+#include <linux/regulator/driver.h>
   #include "pmbus.h"

Quoted 8 lines

drivers/hwmon/pmbus/pli1209bc.c static int pli1209bc_read_word_data(struct · line 37

Quoted 6 lines

drivers/hwmon/pmbus/pli1209bc.c +1 -0

@@ -32,11 +33,39 @@ static int pli1209bc_read_word_data(struct
 i2c_client *client, int page,
                 if (data < 0)
                         return data;
                 return div_s64(data + 5LL, 10L);
+#if IS_ENABLED(CONFIG_SENSORS_PLI1209BC_REGULATOR)

I don't think this needs to be conditional.

Sure, this would cause a redundant status check, if regulator support is disabled. But that's not a big overhead I guess.

Quoted 6 lines

drivers/hwmon/pmbus/pli1209bc.c static int pli1209bc_read_word_data(struct · line 40
Quoted 4 lines
drivers/hwmon/pmbus/pli1209bc.c +2 -0
+       /*
+        * PMBUS_READ_VOUT and PMBUS_READ_TEMPERATURE_1 returns
 invalid data,
s/returns/return/ and no ','

ack.

Thanks,
Marcello

Quoted 49 lines

Quoted 48 lines
drivers/hwmon/pmbus/pli1209bc.c +1 -0
+        * 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);
+#endif
        default:
                return -ENODATA;
        }
}
drivers/hwmon/pmbus/pli1209bc.c +12 -0
+#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,
drivers/hwmon/pmbus/pli1209bc.c +0 -0
@@ -64,6 +93,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,
drivers/hwmon/pmbus/pli1209bc.c +4 -0
+#if IS_ENABLED(CONFIG_SENSORS_PLI1209BC_REGULATOR)
+       .num_regulators = 1,
+       .reg_desc = &pli1209bc_reg_desc,
+#endif
};

static int pli1209bc_probe(struct i2c_client *client)