Skip to contentSkip to conversation

Add support for MPS Multi-phase mp2888 controller

PATCH Superseded Vadim Pasternak 2 participants 7 messages Apr 26, 2021 – May 3, 2021

Subsystems (3)

v3 of 7 Superseded Latest is v7 →

lore
Share & tools
lore
#0 lore raw

v3 cover Add support for MPS Multi-phase mp2888 controller

Add driver and documentation for mp2888 device from Monolithic Power Systems, Inc. (MPS) vendor. This is a digital, multi-phase, pulse-width modulation controller.

Patch set includes:
Patch #1 - increases maximum number of phases. Patch #2 - provides mp2888 driver and documentation. Patch #3 - providesy binding documentation.

Vadim Pasternak (3):
hwmon: (pmbus) Increase maximum number of phases per page
hwmon: (pmbus) Add support for MPS Multi-phase mp2888 controller
dt-bindings: Add MP2888 voltage regulator device

 .../devicetree/bindings/trivial-devices.yaml       |   2 +
 Documentation/hwmon/mp2888.rst                     | 111 ++++++
 drivers/hwmon/pmbus/Kconfig                        |   9 +
 drivers/hwmon/pmbus/Makefile                       |   1 +
 drivers/hwmon/pmbus/mp2888.c                       | 387 +++++++++++++++++++++
 drivers/hwmon/pmbus/pmbus.h                        |   2 +-
 6 files changed, 511 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/hwmon/mp2888.rst
 create mode 100644 drivers/hwmon/pmbus/mp2888.c
Signature · 1 line
2.11.0
#1 lore raw

PATCH v3 1/3 hwmon: (pmbus) Increase maximum number of phases per page

Increase maximum number of phases from 8 to 10 to support multi-phase devices allowing up to 10 phases.

---
 drivers/hwmon/pmbus/pmbus.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/hwmon/pmbus/pmbus.h +1 -1
index 4c30ec89f5bf..fd43873011a4 100644
@@ -375,7 +375,7 @@ enum pmbus_sensor_classes {
 };
 
 #define PMBUS_PAGES	32	/* Per PMBus specification */
-#define PMBUS_PHASES	8	/* Maximum number of phases per page */
+#define PMBUS_PHASES	10	/* Maximum number of phases per page */
 
 /* Functionality bit mask */
 #define PMBUS_HAVE_VIN		BIT(0)
Signature · 1 line
2.11.0
#2 lore raw

PATCH v3 2/3 hwmon: (pmbus) Add support for MPS Multi-phase mp2888 controller

In reply to Vadim Pasternak

Add support for mp2888 device from Monolithic Power Systems, Inc. (MPS) vendor. This is a digital, multi-phase, pulse-width modulation controller.

This device supports:
- One power rail.
- Programmable Multi-Phase up to 10 Phases.
- PWM-VID Interface
- One pages 0 for telemetry.
- Programmable pins for PMBus Address.
- Built-In EEPROM to Store Custom Configurations.
- Can configured VOUT readout in direct or VID format and allows setting of different formats on rails 1 and 2. For VID the following protocols are available: VR13 mode with 5-mV DAC; VR13 mode with 10-mV DAC, IMVP9 mode with 5-mV DAC.

---
v2->v3
Comments pointed out by Guenter:
- Fix precision for PMBUS_READ_VIN (it requires adding scale for PMBUS_VIN_OV_FAULT_LIMIT and PMBUS_VIN_UV_WARN_LIMIT.
- Fix precision for PMBUS_READ_TEMPERATURE_1 (it requires adding scale for PMBUS_OT_WARN_LIMIT).
- Use DIV_ROUND_CLOSEST_ULL for scaling PMBUS_READ_POUT, PMBUS_READ_PIN readouts.
Notes and fixes from Vadim:
- READ_IOUT in linear11 does not give write calculation (tested with external load), while in direct format readouts are correct.
- Disable non-configured phases in mp2888_identify_multiphase().

v1->v2:
Comments pointed out by Guenter:
- Use standard access for getting PMBUS_OT_WARN_LIMIT, PMBUS_VIN_OV_FAULT_LIMIT, PMBUS_VIN_UV_WARN_LIMIT.
- Use linear11 conversion for PMBUS_READ_VIN, PMBUS_READ_POUT, PMBUS_READ_PIN, PMBUS_READ_TEMPERATURE_1 and adjust coefficients.
- Add reading phases current from the dedicated registers.
- Add comment for not implemented or implemented not according to the spec registers, for which "ENXIO" code is returned.
- Set PMBUS_HAVE_IOUT" statically.
Notes from Vadim:
- READ_IOUT uses direct format, so I did not adjust it like the below registers.

---
 Documentation/hwmon/mp2888.rst | 111 ++++++++++++
 drivers/hwmon/pmbus/Kconfig    |   9 +
 drivers/hwmon/pmbus/Makefile   |   1 +
 drivers/hwmon/pmbus/mp2888.c   | 387 +++++++++++++++++++++++++++++++++++++++++
 4 files changed, 508 insertions(+)
 create mode 100644 Documentation/hwmon/mp2888.rst
 create mode 100644 drivers/hwmon/pmbus/mp2888.c
Documentation/hwmon/mp2888.rst new file +111 -0
new file mode 100644
index 000000000000..7839a010642a
@@ -0,0 +1,111 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+Kernel driver mp2888
+====================
+
+Supported chips:
+
+  * MPS MP12254
+
+    Prefix: 'mp2888'
+
+Author:
+
+	Vadim Pasternak <[email protected]>
+
+Description
+-----------
+
+This driver implements support for Monolithic Power Systems, Inc. (MPS)
+vendor dual-loop, digital, multi-phase controller MP2888.
+
+This device: supports:
+
+- One power rail.
+- Programmable Multi-Phase up to 10 Phases.
+- PWM-VID Interface
+- One pages 0 for telemetry.
+- Programmable pins for PMBus Address.
+- Built-In EEPROM to Store Custom Configurations.
+
+Device complaint with:
+
+- PMBus rev 1.3 interface.
+
+Device supports direct format for reading output current, output voltage,
+input and output power and temperature.
+Device supports linear format for reading input voltage and input power.
+
+The driver provides the next attributes for the current:
+
+- for current out input and maximum alarm;
+- for phase current: input and label.
+
+The driver exports the following attributes via the 'sysfs' files, where:
+
+- 'n' is number of configured phases (from 1 to 10);
+- index 1 for "iout";
+- indexes 2 ... 1 + n for phases.
+
+**curr1_alarm**
+
+**curr[1-{1+n}]_input**
+
+**curr[1-{1+n}]_label**
+
+The driver provides the next attributes for the voltage:
+
+- for voltage in: input, low and high critical thresholds, low and high
+  critical alarms;
+- for voltage out: input and high alarm;
+
+The driver exports the following attributes via the 'sysfs' files, where
+
+**in1_crit**
+
+**in1_crit_alarm**
+
+**in1_input**
+
+**in1_label**
+
+**in1_min**
+
+**in1_min_alarm**
+
+**in2_alarm**
+
+**in2_input**
+
+**in2_label**
+
+The driver provides the next attributes for the power:
+
+- for power in alarm and input.
+- for power out: cap, cap alarm an input.
+
+The driver exports the following attributes via the 'sysfs' files, where
+- indexes 1 for "pin";
+- indexes 2 for "pout";
+
+**power1_alarm**
+
+**power1_input**
+
+**power1_label**
+
+**power2_cap**
+
+**power2_cap_alarm**
+
+**power2_input**
+
+**power2_label**
+
+The driver provides the next attributes for the temperature:
+
+**temp1_input**
+
+**temp1_max**
+
+**temp1_max_alarm**
drivers/hwmon/pmbus/Kconfig +9 -0
index 32d2fc850621..a57571928b31 100644
@@ -211,6 +211,15 @@ config SENSORS_MAX8688
 	  This driver can also be built as a module. If so, the module will
 	  be called max8688.
 
+config SENSORS_MP2888
+	tristate "MPS MP2888"
+	help
+	  If you say yes here you get hardware monitoring support for MPS
+	  MP2888 Digital, Multi-Phase, Pulse-Width Modulation Controller.
+
+	  This driver can also be built as a module. If so, the module will
+	  be called mp2888.
+
 config SENSORS_MP2975
 	tristate "MPS MP2975"
 	help
drivers/hwmon/pmbus/Makefile +1 -0
index 6a4ba0fdc1db..a6d7352621ca 100644
@@ -24,6 +24,7 @@ obj-$(CONFIG_SENSORS_MAX20751)	+= max20751.o
 obj-$(CONFIG_SENSORS_MAX31785)	+= max31785.o
 obj-$(CONFIG_SENSORS_MAX34440)	+= max34440.o
 obj-$(CONFIG_SENSORS_MAX8688)	+= max8688.o
+obj-$(CONFIG_SENSORS_MP2888)	+= mp2888.o
 obj-$(CONFIG_SENSORS_MP2975)	+= mp2975.o
 obj-$(CONFIG_SENSORS_PM6764TR)	+= pm6764tr.o
 obj-$(CONFIG_SENSORS_PXE1610)	+= pxe1610.o
drivers/hwmon/pmbus/mp2888.c new file +387 -0

387 lines not on this page · raw message

Signature · 1 line
2.11.0
maintainer
#3 lore raw

Re: [PATCH hwmon-next v3 2/3] hwmon: (pmbus) Add support for MPS Multi-phase mp2888 controller

Quoted 403 lines · Vadim Pasternak

Add support for mp2888 device from Monolithic Power Systems, Inc. (MPS) vendor. This is a digital, multi-phase, pulse-width modulation controller.

This device supports:
- One power rail.
- Programmable Multi-Phase up to 10 Phases.
- PWM-VID Interface
- One pages 0 for telemetry.
- Programmable pins for PMBus Address.
- Built-In EEPROM to Store Custom Configurations.
- Can configured VOUT readout in direct or VID format and allows setting of different formats on rails 1 and 2. For VID the following protocols are available: VR13 mode with 5-mV DAC; VR13 mode with 10-mV DAC, IMVP9 mode with 5-mV DAC.

---
v2->v3
Comments pointed out by Guenter:
- Fix precision for PMBUS_READ_VIN (it requires adding scale for PMBUS_VIN_OV_FAULT_LIMIT and PMBUS_VIN_UV_WARN_LIMIT.
- Fix precision for PMBUS_READ_TEMPERATURE_1 (it requires adding scale for PMBUS_OT_WARN_LIMIT).
- Use DIV_ROUND_CLOSEST_ULL for scaling PMBUS_READ_POUT, PMBUS_READ_PIN readouts.
Notes and fixes from Vadim:
- READ_IOUT in linear11 does not give write calculation (tested with external load), while in direct format readouts are correct.
- Disable non-configured phases in mp2888_identify_multiphase().

v1->v2:
Comments pointed out by Guenter:
- Use standard access for getting PMBUS_OT_WARN_LIMIT, PMBUS_VIN_OV_FAULT_LIMIT, PMBUS_VIN_UV_WARN_LIMIT.
- Use linear11 conversion for PMBUS_READ_VIN, PMBUS_READ_POUT, PMBUS_READ_PIN, PMBUS_READ_TEMPERATURE_1 and adjust coefficients.
- Add reading phases current from the dedicated registers.
- Add comment for not implemented or implemented not according to the spec registers, for which "ENXIO" code is returned.
- Set PMBUS_HAVE_IOUT" statically.
Notes from Vadim:
- READ_IOUT uses direct format, so I did not adjust it like the below registers.

---
 Documentation/hwmon/mp2888.rst | 111 ++++++++++++
 drivers/hwmon/pmbus/Kconfig    |   9 +
 drivers/hwmon/pmbus/Makefile   |   1 +
 drivers/hwmon/pmbus/mp2888.c   | 387 +++++++++++++++++++++++++++++++++++++++++
 4 files changed, 508 insertions(+)
 create mode 100644 Documentation/hwmon/mp2888.rst
 create mode 100644 drivers/hwmon/pmbus/mp2888.c
Documentation/hwmon/mp2888.rst new file +111 -0
new file mode 100644
index 000000000000..7839a010642a
@@ -0,0 +1,111 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+Kernel driver mp2888
+====================
+
+Supported chips:
+
+  * MPS MP12254
+
+    Prefix: 'mp2888'
+
+Author:
+
+	Vadim Pasternak <[email protected]>
+
+Description
+-----------
+
+This driver implements support for Monolithic Power Systems, Inc. (MPS)
+vendor dual-loop, digital, multi-phase controller MP2888.
+
+This device: supports:
+
+- One power rail.
+- Programmable Multi-Phase up to 10 Phases.
+- PWM-VID Interface
+- One pages 0 for telemetry.
+- Programmable pins for PMBus Address.
+- Built-In EEPROM to Store Custom Configurations.
+
+Device complaint with:
+
+- PMBus rev 1.3 interface.
+
+Device supports direct format for reading output current, output voltage,
+input and output power and temperature.
+Device supports linear format for reading input voltage and input power.
+
+The driver provides the next attributes for the current:
+
+- for current out input and maximum alarm;
+- for phase current: input and label.
+
+The driver exports the following attributes via the 'sysfs' files, where:
+
+- 'n' is number of configured phases (from 1 to 10);
+- index 1 for "iout";
+- indexes 2 ... 1 + n for phases.
+
+**curr1_alarm**
+
+**curr[1-{1+n}]_input**
+
+**curr[1-{1+n}]_label**
+
+The driver provides the next attributes for the voltage:
+
+- for voltage in: input, low and high critical thresholds, low and high
+  critical alarms;
+- for voltage out: input and high alarm;
+
+The driver exports the following attributes via the 'sysfs' files, where
+
+**in1_crit**
+
+**in1_crit_alarm**
+
+**in1_input**
+
+**in1_label**
+
+**in1_min**
+
+**in1_min_alarm**
+
+**in2_alarm**
+
+**in2_input**
+
+**in2_label**
+
+The driver provides the next attributes for the power:
+
+- for power in alarm and input.
+- for power out: cap, cap alarm an input.
+
+The driver exports the following attributes via the 'sysfs' files, where
+- indexes 1 for "pin";
+- indexes 2 for "pout";
+
+**power1_alarm**
+
+**power1_input**
+
+**power1_label**
+
+**power2_cap**
+
+**power2_cap_alarm**
+
+**power2_input**
+
+**power2_label**
+
+The driver provides the next attributes for the temperature:
+
+**temp1_input**
+
+**temp1_max**
+
+**temp1_max_alarm**
drivers/hwmon/pmbus/Kconfig +9 -0
index 32d2fc850621..a57571928b31 100644
@@ -211,6 +211,15 @@ config SENSORS_MAX8688
 	  This driver can also be built as a module. If so, the module will
 	  be called max8688.
 
+config SENSORS_MP2888
+	tristate "MPS MP2888"
+	help
+	  If you say yes here you get hardware monitoring support for MPS
+	  MP2888 Digital, Multi-Phase, Pulse-Width Modulation Controller.
+
+	  This driver can also be built as a module. If so, the module will
+	  be called mp2888.
+
 config SENSORS_MP2975
 	tristate "MPS MP2975"
 	help
drivers/hwmon/pmbus/Makefile +1 -0
index 6a4ba0fdc1db..a6d7352621ca 100644
@@ -24,6 +24,7 @@ obj-$(CONFIG_SENSORS_MAX20751)	+= max20751.o
 obj-$(CONFIG_SENSORS_MAX31785)	+= max31785.o
 obj-$(CONFIG_SENSORS_MAX34440)	+= max34440.o
 obj-$(CONFIG_SENSORS_MAX8688)	+= max8688.o
+obj-$(CONFIG_SENSORS_MP2888)	+= mp2888.o
 obj-$(CONFIG_SENSORS_MP2975)	+= mp2975.o
 obj-$(CONFIG_SENSORS_PM6764TR)	+= pm6764tr.o
 obj-$(CONFIG_SENSORS_PXE1610)	+= pxe1610.o
drivers/hwmon/pmbus/mp2888.c new file +195 -0
new file mode 100644
index 000000000000..4b90f0e8cbd7
@@ -0,0 +1,387 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Hardware monitoring driver for MPS Multi-phase Digital VR Controllers
+ *
+ * Copyright (C) 2020 Nvidia Technologies Ltd.
+ */
+
+#include <linux/err.h>
+#include <linux/i2c.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include "pmbus.h"
+
+/* Vendor specific registers. */
+#define MP2888_MFR_SYS_CONFIG	0x44
+#define MP2888_MFR_READ_CS1_2	0x73
+#define MP2888_MFR_READ_CS3_4	0x74
+#define MP2888_MFR_READ_CS5_6	0x75
+#define MP2888_MFR_READ_CS7_8	0x76
+#define MP2888_MFR_READ_CS9_10	0x77
+#define MP2888_MFR_VR_CONFIG1	0xe1
+
+#define MP2888_TOTAL_CURRENT_RESOLUTION	BIT(3)
+#define MP2888_PHASE_CURRENT_RESOLUTION	BIT(4)
+#define MP2888_DRMOS_KCS		GENMASK(2, 0)
+#define MP2888_VIN_LIMIT_UNIT		8
+#define MP2888_TEMP_UNIT		10
+#define MP2888_VIN_MANTISSA		32
+#define MP2888_MAX_PHASE		10
+
+struct mp2888_data {
+	struct pmbus_driver_info info;
+	int total_curr_resolution;
+	int phase_curr_resolution;
+	int curr_sense_gain;
+};
+
+#define to_mp2888_data(x)  container_of(x, struct mp2888_data, info)
+
+static int mp2888_read_byte_data(struct i2c_client *client, int page, int reg)
+{
+	switch (reg) {
+	case PMBUS_VOUT_MODE:
+		/* Enforce VOUT direct format. */
+		return PB_VOUT_MODE_DIRECT;
+	default:
+		return -ENODATA;
+	}
+}
+
+/* Convert linear sensor values to milli- or micro-units depending on sensor type. */
+static s64 mp2888_reg2data_linear11(int data, enum pmbus_sensor_classes class)
+{
+	s16 exponent;
+	s32 mantissa;
+	s64 val;
+
+	exponent = ((s16)data) >> 11;
+	mantissa = ((s16)((data & 0x7ff) << 5)) >> 5;
+	val = mantissa;
+
+	/* Scale result to micro-units for power sensors. */
+	if (class == PSC_POWER)
+		val = val * 1000LL;
+
+	if (exponent >= 0)
+		val <<= exponent;
+	else
+		val >>= -exponent;
+
+	return val;
+}
+
+static int
+mp2888_current_sense_gain_and_resolution_get(struct i2c_client *client, struct mp2888_data *data)
+{
+	int ret;
+
+	/*
+	 * Obtain DrMOS current sense gain of power stage from the register
+	 * , bits 0-2. The value is selected as below:
+	 * 00b - 5µA/A, 01b - 8.5µA/A, 10b - 9.7µA/A, 11b - 10µA/A. Other
+	 * values are reserved.
+	 */
+	ret = i2c_smbus_read_word_data(client, MP2888_MFR_SYS_CONFIG);
+	if (ret < 0)
+		return ret;
+
+	switch (ret & MP2888_DRMOS_KCS) {
+	case 0:
+		data->curr_sense_gain = 85;
+		break;
+	case 1:
+		data->curr_sense_gain = 97;
+		break;
+	case 2:
+		data->curr_sense_gain = 100;
+		break;
+	case 3:
+		data->curr_sense_gain = 50;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	/*
+	 * Obtain resolution selector for total and phase current report and protection.
+	 * 0: original resolution; 1: half resolution (in such case phase current value should
+	 * be doubled.
+	 */
+	data->total_curr_resolution = (ret & MP2888_TOTAL_CURRENT_RESOLUTION) >> 3;
+	data->phase_curr_resolution = (ret & MP2888_PHASE_CURRENT_RESOLUTION) >> 4;
+
+	return 0;
+}
+
+static int
+mp2888_read_phase(struct i2c_client *client, struct mp2888_data *data, int page, int phase, u8 reg)
+{
+	int ret;
+
+	ret = pmbus_read_word_data(client, page, phase, reg);
+	if (ret < 0)
+		return ret;
+
+	if (!((phase + 1) % 2))
+		ret >>= 8;
+	ret &= 0xff;
+
+	/*
+	 * Output value is calculated as: (READ_CSx / 80 – 1.23) / (Kcs * Rcs)
+	 * where:
+	 * - Kcs is the DrMOS current sense gain of power stage, which is obtained from the
+	 *   register MP2888_MFR_VR_CONFIG1, bits 13-12 with the following selection of DrMOS
+	 *   (data->curr_sense_gain):
+	 *   00b - 5µA/A, 01b - 8.5µA/A, 10b - 9.7µA/A, 11b - 10µA/A.
+	 * - Rcs is the internal phase current sense resistor. This parameter depends on hardware
+	 *   assembly. By default it is set to 1kΩ. In case of different assembly, user should
+	 *   scale this parameter by dividing it by Rcs.
+	 * If phase current resolution bit is set to 1, READ_CSx value should be doubled.
+	 * Note, that current phase sensing, providing by the device is not accurate. This is
+	 * because sampling of current occurrence of bit weight has a big deviation, especially for
+	 * light load.
+	 */
+	if (data->phase_curr_resolution)
+		ret *= 2;
+	return DIV_ROUND_CLOSEST(ret * 100 - 9800, data->curr_sense_gain);
+}
+
+static int
+mp2888_read_phases(struct i2c_client *client, struct mp2888_data *data, int page, int phase)
+{
+	int ret;
+
+	switch (phase) {
+	case 0 ... 1:
+		ret = mp2888_read_phase(client, data, page, phase, MP2888_MFR_READ_CS1_2);
+		break;
+	case 2 ... 3:
+		ret = mp2888_read_phase(client, data, page, phase, MP2888_MFR_READ_CS3_4);
+		break;
+	case 4 ... 5:
+		ret = mp2888_read_phase(client, data, page, phase, MP2888_MFR_READ_CS5_6);
+		break;
+	case 6 ... 7:
+		ret = mp2888_read_phase(client, data, page, phase, MP2888_MFR_READ_CS7_8);
+		break;
+	case 8 ... 9:
+		ret = mp2888_read_phase(client, data, page, phase, MP2888_MFR_READ_CS9_10);
+		break;
+	default:
+		return -ENODATA;
+	}
+	return ret;
+}
+
+static int mp2888_read_word_data(struct i2c_client *client, int page, int phase, int reg)
+{
+	const struct pmbus_driver_info *info = pmbus_get_driver_info(client);
+	struct mp2888_data *data = to_mp2888_data(info);
+	s64 val;
+	int ret;
+
+	switch (reg) {
+	case PMBUS_READ_VIN:
+		ret = pmbus_read_word_data(client, page, phase, reg);
+		if (ret <= 0)
+			return ret;
+
+		/*
+		 * READ_VIN register has unused bits 15:10 with fixed value 111011b. Clear these
+		 * bits and scale with coefficient 0.03125 by using mantissa 32.
+		 */
+		ret = (ret & GENMASK(9, 0));

drivers/hwmon/pmbus/mp2888.c line 205

Quoted 10 lines
drivers/hwmon/pmbus/mp2888.c new file +10 -0
+		break;
+	case PMBUS_VIN_OV_FAULT_LIMIT:
+	case PMBUS_VIN_UV_WARN_LIMIT:
+		ret = pmbus_read_word_data(client, page, phase, reg);
+		if (ret <= 0)
+			return ret;
+
+		val = mp2888_reg2data_linear11(ret, PSC_VOLTAGE_IN);
+		ret = val * MP2888_VIN_MANTISSA;
+		break;

Unless I am missing something, this is linear11 with, for some reason, bit 10 erroneously set for PMBUS_READ_VIN. That means that setting the format for PSC_VOLTAGE_IN and clearing bit 10 for PMBUS_READ_VIN should work without local conversion. Am I missing something ?

drivers/hwmon/pmbus/mp2888.c line 210

Quoted 5 lines
drivers/hwmon/pmbus/mp2888.c new file +5 -0
+	case PMBUS_OT_WARN_LIMIT:
+		ret = pmbus_read_word_data(client, page, phase, reg);
+		if (ret < 0)
+			return ret;
+		ret *= MP2888_TEMP_UNIT;

Explanation would be useful: chip reports limits in degrees C, but the actual temperature in 10th of degrees C. This is needed to match the two.

Quoted 5 lines
drivers/hwmon/pmbus/mp2888.c new file +5 -0
+		break;
+	case PMBUS_READ_IOUT:
+		if (phase != 0xff)
+			return mp2888_read_phases(client, data, page, phase);
+

drivers/hwmon/pmbus/mp2888.c line 225

Quoted 10 lines
drivers/hwmon/pmbus/mp2888.c new file +10 -0
+		ret = pmbus_read_word_data(client, page, phase, reg);
+		if (ret < 0)
+			return ret;
+		/*
+		 * READ_IOUT register has unused bits 15:12 with fixed value 1110b. Clear these
+		 * bits and scale with total current resolution. Data is provided in direct format.
+		 */
+		ret &= GENMASK(11, 0);
+		ret = data->total_curr_resolution ? DIV_ROUND_CLOSEST(ret, 2) :
+		      DIV_ROUND_CLOSEST(ret, 4);

I don't think this is a good idea: It unnecessarily reduces the resolution (which is bad enough already). The format for output current is already set to direct with a mantissa of 1. It might make sense to adjust the mantissa to always report the correct current. Specifically, if current resolution is set to 1, the value here should be multiplied, not divided.

drivers/hwmon/pmbus/mp2888.c line 232

Quoted 7 lines
drivers/hwmon/pmbus/mp2888.c new file +7 -0
+		break;
+	case PMBUS_READ_POUT:
+	case PMBUS_READ_PIN:
+		ret = pmbus_read_word_data(client, page, phase, reg);
+		if (ret < 0)
+			return ret;
+		val = mp2888_reg2data_linear11(ret, PSC_POWER);

This call seems pretty pointless: The exponent bits are always 0, and we know that the resolution is either 0.5W or 1W.

Conversion to micro-watt should be handled by the PMBus core.

drivers/hwmon/pmbus/mp2888.c line 233

Quoted 1 line
drivers/hwmon/pmbus/mp2888.c new file +1 -0
+		ret = data->total_curr_resolution ? val : DIV_ROUND_CLOSEST_ULL(val, 2);

Same as above: Any adjustment should be a multiplication, not a divide operation, to avoid resolution loss.

Quoted 6 lines
drivers/hwmon/pmbus/mp2888.c new file +6 -0
+		break;
+	/*
+	 * The below registers are not implemented by device or implemented not according to the
+	 * spec. Skip all of them to avoid exposing non-relevant inputs to sysfs.
+	 */
+	case PMBUS_OT_FAULT_LIMIT:

drivers/hwmon/pmbus/mp2888.c line 249

Quoted 10 lines
drivers/hwmon/pmbus/mp2888.c new file +10 -0
+	case PMBUS_UT_WARN_LIMIT:
+	case PMBUS_UT_FAULT_LIMIT:
+	case PMBUS_VIN_UV_FAULT_LIMIT:
+	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_VIN_OV_WARN_LIMIT:
+	case PMBUS_IOUT_OC_LV_FAULT_LIMIT:
+	case PMBUS_IOUT_OC_WARN_LIMIT:

Datasheet says this is a 10-bit value with a scale of 1A or 2A depending on the current resolution.

drivers/hwmon/pmbus/mp2888.c line 253

Quoted 4 lines
drivers/hwmon/pmbus/mp2888.c new file +4 -0
+	case PMBUS_IOUT_OC_FAULT_LIMIT:
+	case PMBUS_IOUT_UC_FAULT_LIMIT:
+	case PMBUS_POUT_OP_FAULT_LIMIT:
+	case PMBUS_POUT_OP_WARN_LIMIT:

Datasheet: 10-bit value with 1W/LSB or 2W/LSB depending on current resolution.

Unless there is reqason to believe that the datasheet is wrong, those should be supported. If the datasheet is wrong it should be explained.

Quoted 69 lines
drivers/hwmon/pmbus/mp2888.c new file +69 -0
+	case PMBUS_PIN_OP_WARN_LIMIT:
+	case PMBUS_MFR_VIN_MIN:
+	case PMBUS_MFR_VIN_MAX:
+	case PMBUS_MFR_VOUT_MAX:
+	case PMBUS_MFR_IIN_MAX:
+	case PMBUS_MFR_IOUT_MAX:
+	case PMBUS_MFR_PIN_MAX:
+	case PMBUS_MFR_POUT_MAX:
+	case PMBUS_MFR_MAX_TEMP_1:
+		return -ENXIO;
+	default:
+		return -ENODATA;
+	}
+
+	return ret;
+}
+
+static int
+mp2888_identify_multiphase(struct i2c_client *client, struct mp2888_data *data,
+			   struct pmbus_driver_info *info)
+{
+	int i, ret;
+
+	ret = i2c_smbus_write_byte_data(client, PMBUS_PAGE, 0);
+	if (ret < 0)
+		return ret;
+
+	/* Identify multiphase number - could be from 1 to 10. */
+	ret = i2c_smbus_read_word_data(client, MP2888_MFR_VR_CONFIG1);
+	if (ret <= 0)
+		return ret;
+
+	info->phases[0] = ret & GENMASK(3, 0);
+
+	/*
+	 * The device provides a total of 10 PWM pins, and can be configured to different phase
+	 * count applications for rail.
+	 */
+	if (info->phases[0] > MP2888_MAX_PHASE)
+		return -EINVAL;
+
+	/* Disable non-configured phases. */
+	for (i = info->phases[0]; i < MP2888_MAX_PHASE; i++)
+		info->pfunc[i] = 0;
+
+	return 0;
+}
+
+static struct pmbus_driver_info mp2888_info = {
+	.pages = 1,
+	.format[PSC_VOLTAGE_IN] = direct,
+	.format[PSC_VOLTAGE_OUT] = direct,
+	.format[PSC_TEMPERATURE] = direct,
+	.format[PSC_CURRENT_IN] = linear,
+	.format[PSC_CURRENT_OUT] = direct,
+	.format[PSC_POWER] = direct,
+	.m[PSC_VOLTAGE_IN] = MP2888_VIN_MANTISSA,
+	.m[PSC_TEMPERATURE] = 1,
+	.R[PSC_TEMPERATURE] = 1,
+	.m[PSC_VOLTAGE_OUT] = 1,
+	.R[PSC_VOLTAGE_OUT] = 3,
+	.m[PSC_CURRENT_OUT] = 1,
+	.m[PSC_POWER] = 1,
+	.func[0] = PMBUS_HAVE_VIN | PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT | PMBUS_HAVE_IOUT |
+		   PMBUS_HAVE_STATUS_IOUT | PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP |
+		   PMBUS_HAVE_POUT | PMBUS_HAVE_PIN | PMBUS_HAVE_STATUS_INPUT |
+		   PMBUS_PHASE_VIRTUAL,
+	.pfunc[0] = PMBUS_HAVE_IOUT,
+	.pfunc[1] = PMBUS_HAVE_IOUT,

drivers/hwmon/pmbus/mp2888.c line 332

Quoted 10 lines
drivers/hwmon/pmbus/mp2888.c new file +10 -0
+	.pfunc[2] = PMBUS_HAVE_IOUT,
+	.pfunc[3] = PMBUS_HAVE_IOUT,
+	.pfunc[4] = PMBUS_HAVE_IOUT,
+	.pfunc[5] = PMBUS_HAVE_IOUT,
+	.pfunc[6] = PMBUS_HAVE_IOUT,
+	.pfunc[7] = PMBUS_HAVE_IOUT,
+	.pfunc[8] = PMBUS_HAVE_IOUT,
+	.pfunc[9] = PMBUS_HAVE_IOUT,
+	.read_byte_data = mp2888_read_byte_data,
+	.read_word_data = mp2888_read_word_data,

I don't see a write function here. Did you try setting limits ? Given the adjustments needed for reading limits, I don't think this will work.

Thanks,
Guenter

Quoted 56 lines
drivers/hwmon/pmbus/mp2888.c new file +55 -0
+};
+
+static int mp2888_probe(struct i2c_client *client)
+{
+	struct pmbus_driver_info *info;
+	struct mp2888_data *data;
+	int ret;
+
+	data = devm_kzalloc(&client->dev, sizeof(struct mp2888_data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	memcpy(&data->info, &mp2888_info, sizeof(*info));
+	info = &data->info;
+
+	/* Identify multiphase configuration. */
+	ret = mp2888_identify_multiphase(client, data, info);
+	if (ret)
+		return ret;
+
+	/* Obtain current sense gain of power stage and current resolution. */
+	ret = mp2888_current_sense_gain_and_resolution_get(client, data);
+	if (ret)
+		return ret;
+
+	return pmbus_do_probe(client, info);
+}
+
+static const struct i2c_device_id mp2888_id[] = {
+	{"mp2888", 0},
+	{}
+};
+
+MODULE_DEVICE_TABLE(i2c, mp2888_id);
+
+static const struct of_device_id __maybe_unused mp2888_of_match[] = {
+	{.compatible = "mps,mp2888"},
+	{}
+};
+MODULE_DEVICE_TABLE(of, mp2888_of_match);
+
+static struct i2c_driver mp2888_driver = {
+	.driver = {
+		.name = "mp2888",
+		.of_match_table = of_match_ptr(mp2888_of_match),
+	},
+	.probe_new = mp2888_probe,
+	.id_table = mp2888_id,
+};
+
+module_i2c_driver(mp2888_driver);
+
+MODULE_AUTHOR("Vadim Pasternak <[email protected]>");
+MODULE_DESCRIPTION("PMBus driver for MPS MP2888 device");
+MODULE_LICENSE("GPL");
#4 lore raw

RE: [PATCH hwmon-next v3 2/3] hwmon: (pmbus) Add support for MPS Multi-phase mp2888 controller

Hi Guenter!

Quoted 448 lines

-----Original Message-----
From: Guenter Roeck <[email protected]> On Behalf Of Guenter Roeck
Sent: Sunday, May 2, 2021 6:47 PM
To: Vadim Pasternak <[email protected]>; [email protected]
Cc: [email protected]; [email protected]
Subject: Re: [PATCH hwmon-next v3 2/3] hwmon: (pmbus) Add support for MPS Multi-phase mp2888 controller

Quoted 31 lines · Vadim Pasternak

Add support for mp2888 device from Monolithic Power Systems, Inc. (MPS) vendor. This is a digital, multi-phase, pulse-width modulation controller.

This device supports:
- One power rail.
- Programmable Multi-Phase up to 10 Phases.
- PWM-VID Interface
- One pages 0 for telemetry.
- Programmable pins for PMBus Address.
- Built-In EEPROM to Store Custom Configurations.
- Can configured VOUT readout in direct or VID format and allows setting of different formats on rails 1 and 2. For VID the following protocols are available: VR13 mode with 5-mV DAC; VR13 mode with 10-mV DAC, IMVP9 mode with 5-mV DAC.

---
v2->v3
Comments pointed out by Guenter:
- Fix precision for PMBUS_READ_VIN (it requires adding scale for PMBUS_VIN_OV_FAULT_LIMIT and PMBUS_VIN_UV_WARN_LIMIT.
- Fix precision for PMBUS_READ_TEMPERATURE_1 (it requires adding scale for PMBUS_OT_WARN_LIMIT).
- Use DIV_ROUND_CLOSEST_ULL for scaling PMBUS_READ_POUT, PMBUS_READ_PIN readouts.
Notes and fixes from Vadim:
- READ_IOUT in linear11 does not give write calculation (tested with external load), while in direct format readouts are correct.
- Disable non-configured phases in mp2888_identify_multiphase().

Quoted 6 lines

v1->v2:
Comments pointed out by Guenter:
- Use standard access for getting PMBUS_OT_WARN_LIMIT, PMBUS_VIN_OV_FAULT_LIMIT, PMBUS_VIN_UV_WARN_LIMIT.
- Use linear11 conversion for PMBUS_READ_VIN, PMBUS_READ_POUT, PMBUS_READ_PIN, PMBUS_READ_TEMPERATURE_1 and adjust

coefficients.

Quoted 7 lines

- Add reading phases current from the dedicated registers.
- Add comment for not implemented or implemented not according to the spec registers, for which "ENXIO" code is returned.
- Set PMBUS_HAVE_IOUT" statically.
Notes from Vadim:
- READ_IOUT uses direct format, so I did not adjust it like the below registers.

Quoted 5 lines
---
 Documentation/hwmon/mp2888.rst | 111 ++++++++++++
 drivers/hwmon/pmbus/Kconfig    |   9 +
 drivers/hwmon/pmbus/Makefile   |   1 +
 drivers/hwmon/pmbus/mp2888.c   | 387

+++++++++++++++++++++++++++++++++++++++++

Quoted 2 lines
 4 files changed, 508 insertions(+)
 create mode 100644 Documentation/hwmon/mp2888.rst  create mode

100644

Review

Quoted 123 lines

drivers/hwmon/pmbus/mp2888.c

(quoted diff) +0 -0

b/Documentation/hwmon/mp2888.rst new file mode 100644 index 000000000000..7839a010642a

Documentation/hwmon/mp2888.rst new file +114 -0
@@ -0,0 +1,111 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+Kernel driver mp2888
+====================
+
+Supported chips:
+
+  * MPS MP12254
+
+    Prefix: 'mp2888'
+
+Author:
+
+	Vadim Pasternak <[email protected]>
+
+Description
+-----------
+
+This driver implements support for Monolithic Power Systems, Inc.
+(MPS) vendor dual-loop, digital, multi-phase controller MP2888.
+
+This device: supports:
+
+- One power rail.
+- Programmable Multi-Phase up to 10 Phases.
+- PWM-VID Interface
+- One pages 0 for telemetry.
+- Programmable pins for PMBus Address.
+- Built-In EEPROM to Store Custom Configurations.
+
+Device complaint with:
+
+- PMBus rev 1.3 interface.
+
+Device supports direct format for reading output current, output
+voltage, input and output power and temperature.
+Device supports linear format for reading input voltage and input power.
+
+The driver provides the next attributes for the current:
+
+- for current out input and maximum alarm;
+- for phase current: input and label.
+
+The driver exports the following attributes via the 'sysfs' files, where:
+
+- 'n' is number of configured phases (from 1 to 10);
+- index 1 for "iout";
+- indexes 2 ... 1 + n for phases.
+
+**curr1_alarm**
+
+**curr[1-{1+n}]_input**
+
+**curr[1-{1+n}]_label**
+
+The driver provides the next attributes for the voltage:
+
+- for voltage in: input, low and high critical thresholds, low and
+high
+  critical alarms;
+- for voltage out: input and high alarm;
+
+The driver exports the following attributes via the 'sysfs' files,
+where
+
+**in1_crit**
+
+**in1_crit_alarm**
+
+**in1_input**
+
+**in1_label**
+
+**in1_min**
+
+**in1_min_alarm**
+
+**in2_alarm**
+
+**in2_input**
+
+**in2_label**
+
+The driver provides the next attributes for the power:
+
+- for power in alarm and input.
+- for power out: cap, cap alarm an input.
+
+The driver exports the following attributes via the 'sysfs' files,
+where
+- indexes 1 for "pin";
+- indexes 2 for "pout";
+
+**power1_alarm**
+
+**power1_input**
+
+**power1_label**
+
+**power2_cap**
+
+**power2_cap_alarm**
+
+**power2_input**
+
+**power2_label**
+
+The driver provides the next attributes for the temperature:
+
+**temp1_input**
+
+**temp1_max**
+
+**temp1_max_alarm**
(quoted diff) +0 -0

b/drivers/hwmon/pmbus/Kconfig

Quoted 20 lines

index 32d2fc850621..a57571928b31 100644

drivers/hwmon/pmbus/Kconfig +9 -0
@@ -211,6 +211,15 @@ config SENSORS_MAX8688
 	  This driver can also be built as a module. If so, the module will
 	  be called max8688.
 
+config SENSORS_MP2888
+	tristate "MPS MP2888"
+	help
+	  If you say yes here you get hardware monitoring support for MPS
+	  MP2888 Digital, Multi-Phase, Pulse-Width Modulation Controller.
+
+	  This driver can also be built as a module. If so, the module will
+	  be called mp2888.
+
 config SENSORS_MP2975
 	tristate "MPS MP2975"
 	help
(quoted diff) +0 -0
Quoted 1 line

b/drivers/hwmon/pmbus/Makefile index 6a4ba0fdc1db..a6d7352621ca

100644

drivers/hwmon/pmbus/Makefile obj-$(CONFIG_SENSORS_MAX20751) +=

Quoted 3 lines
drivers/hwmon/pmbus/Makefile +0 -0
@@ -24,6 +24,7 @@ obj-$(CONFIG_SENSORS_MAX20751)	+=

max20751.o

Quoted 90 lines
drivers/hwmon/pmbus/Makefile +1 -0
 obj-$(CONFIG_SENSORS_MAX31785)	+= max31785.o
 obj-$(CONFIG_SENSORS_MAX34440)	+= max34440.o
 obj-$(CONFIG_SENSORS_MAX8688)	+= max8688.o
+obj-$(CONFIG_SENSORS_MP2888)	+= mp2888.o
 obj-$(CONFIG_SENSORS_MP2975)	+= mp2975.o
 obj-$(CONFIG_SENSORS_PM6764TR)	+= pm6764tr.o
 obj-$(CONFIG_SENSORS_PXE1610)	+= pxe1610.o
(quoted diff) +0 -0

b/drivers/hwmon/pmbus/mp2888.c new file mode 100644 index 000000000000..4b90f0e8cbd7

drivers/hwmon/pmbus/mp2888.c new file +77 -0
@@ -0,0 +1,387 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Hardware monitoring driver for MPS Multi-phase Digital VR
+Controllers
+ *
+ * Copyright (C) 2020 Nvidia Technologies Ltd.
+ */
+
+#include <linux/err.h>
+#include <linux/i2c.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include "pmbus.h"
+
+/* Vendor specific registers. */
+#define MP2888_MFR_SYS_CONFIG	0x44
+#define MP2888_MFR_READ_CS1_2	0x73
+#define MP2888_MFR_READ_CS3_4	0x74
+#define MP2888_MFR_READ_CS5_6	0x75
+#define MP2888_MFR_READ_CS7_8	0x76
+#define MP2888_MFR_READ_CS9_10	0x77
+#define MP2888_MFR_VR_CONFIG1	0xe1
+
+#define MP2888_TOTAL_CURRENT_RESOLUTION	BIT(3)
+#define MP2888_PHASE_CURRENT_RESOLUTION	BIT(4)
+#define MP2888_DRMOS_KCS		GENMASK(2, 0)
+#define MP2888_VIN_LIMIT_UNIT		8
+#define MP2888_TEMP_UNIT		10
+#define MP2888_VIN_MANTISSA		32
+#define MP2888_MAX_PHASE		10
+
+struct mp2888_data {
+	struct pmbus_driver_info info;
+	int total_curr_resolution;
+	int phase_curr_resolution;
+	int curr_sense_gain;
+};
+
+#define to_mp2888_data(x)  container_of(x, struct mp2888_data, info)
+
+static int mp2888_read_byte_data(struct i2c_client *client, int page,
+int reg) {
+	switch (reg) {
+	case PMBUS_VOUT_MODE:
+		/* Enforce VOUT direct format. */
+		return PB_VOUT_MODE_DIRECT;
+	default:
+		return -ENODATA;
+	}
+}
+
+/* Convert linear sensor values to milli- or micro-units depending on
+sensor type. */ static s64 mp2888_reg2data_linear11(int data, enum
+pmbus_sensor_classes class) {
+	s16 exponent;
+	s32 mantissa;
+	s64 val;
+
+	exponent = ((s16)data) >> 11;
+	mantissa = ((s16)((data & 0x7ff) << 5)) >> 5;
+	val = mantissa;
+
+	/* Scale result to micro-units for power sensors. */
+	if (class == PSC_POWER)
+		val = val * 1000LL;
+
+	if (exponent >= 0)
+		val <<= exponent;
+	else
+		val >>= -exponent;
+
+	return val;
+}
+
+static int
+mp2888_current_sense_gain_and_resolution_get(struct i2c_client

drivers/hwmon/pmbus/mp2888.c line 87

Quoted 10 lines
drivers/hwmon/pmbus/mp2888.c new file +10 -0
+*client, struct mp2888_data *data) {
+	int ret;
+
+	/*
+	 * Obtain DrMOS current sense gain of power stage from the register
+	 * , bits 0-2. The value is selected as below:
+	 * 00b - 5µA/A, 01b - 8.5µA/A, 10b - 9.7µA/A, 11b - 10µA/A. Other
+	 * values are reserved.
+	 */
+	ret = i2c_smbus_read_word_data(client,

MP2888_MFR_SYS_CONFIG);

Quoted 12 lines
drivers/hwmon/pmbus/mp2888.c new file +12 -0
+	if (ret < 0)
+		return ret;
+
+	switch (ret & MP2888_DRMOS_KCS) {
+	case 0:
+		data->curr_sense_gain = 85;
+		break;
+	case 1:
+		data->curr_sense_gain = 97;
+		break;
+	case 2:
+		data->curr_sense_gain = 100;

drivers/hwmon/pmbus/mp2888.c

Quoted 10 lines
drivers/hwmon/pmbus/mp2888.c new file +10 -0
+		break;
+	case 3:
+		data->curr_sense_gain = 50;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	/*
+	 * Obtain resolution selector for total and phase current report and

protection.

drivers/hwmon/pmbus/mp2888.c line 110

Quoted 1 line
drivers/hwmon/pmbus/mp2888.c new file +1 -0
+	 * 0: original resolution; 1: half resolution (in such case phase current

value should

drivers/hwmon/pmbus/mp2888.c

Quoted 3 lines
drivers/hwmon/pmbus/mp2888.c new file +3 -0
+	 * be doubled.
+	 */
+	data->total_curr_resolution = (ret &

MP2888_TOTAL_CURRENT_RESOLUTION) >> 3;

Quoted 13 lines
drivers/hwmon/pmbus/mp2888.c new file +13 -0
+	data->phase_curr_resolution = (ret &
+MP2888_PHASE_CURRENT_RESOLUTION) >> 4;
+
+	return 0;
+}
+
+static int
+mp2888_read_phase(struct i2c_client *client, struct mp2888_data
+*data, int page, int phase, u8 reg) {
+	int ret;
+
+	ret = pmbus_read_word_data(client, page, phase, reg);
+	if (ret < 0)

drivers/hwmon/pmbus/mp2888.c

Quoted 10 lines
drivers/hwmon/pmbus/mp2888.c new file +10 -0
+		return ret;
+
+	if (!((phase + 1) % 2))
+		ret >>= 8;
+	ret &= 0xff;
+
+	/*
+	 * Output value is calculated as: (READ_CSx / 80 – 1.23) / (Kcs * Rcs)
+	 * where:
+	 * - Kcs is the DrMOS current sense gain of power stage, which is

obtained from the

drivers/hwmon/pmbus/mp2888.c line 137

Quoted 1 line
drivers/hwmon/pmbus/mp2888.c new file +1 -0
+	 *   register MP2888_MFR_VR_CONFIG1, bits 13-12 with the following

selection of DrMOS

drivers/hwmon/pmbus/mp2888.c

Quoted 3 lines
drivers/hwmon/pmbus/mp2888.c new file +3 -0
+	 *   (data->curr_sense_gain):
+	 *   00b - 5µA/A, 01b - 8.5µA/A, 10b - 9.7µA/A, 11b - 10µA/A.
+	 * - Rcs is the internal phase current sense resistor. This parameter

depends on hardware

drivers/hwmon/pmbus/mp2888.c line 141

Quoted 1 line
drivers/hwmon/pmbus/mp2888.c new file +1 -0
+	 *   assembly. By default it is set to 1kΩ. In case of different assembly,

user should

drivers/hwmon/pmbus/mp2888.c line 143

Quoted 2 lines
drivers/hwmon/pmbus/mp2888.c new file +2 -0
+	 *   scale this parameter by dividing it by Rcs.
+	 * If phase current resolution bit is set to 1, READ_CSx value should

be doubled.

drivers/hwmon/pmbus/mp2888.c line 144

Quoted 1 line
drivers/hwmon/pmbus/mp2888.c new file +1 -0
+	 * Note, that current phase sensing, providing by the device is not

accurate. This is

drivers/hwmon/pmbus/mp2888.c line 145

Quoted 1 line
drivers/hwmon/pmbus/mp2888.c new file +1 -0
+	 * because sampling of current occurrence of bit weight has a big

deviation, especially for

Quoted 6 lines
drivers/hwmon/pmbus/mp2888.c new file +5 -0
+	 * light load.
+	 */
+	if (data->phase_curr_resolution)
+		ret *= 2;
+	return DIV_ROUND_CLOSEST(ret * 100 - 9800, data-

curr_sense_gain); }

drivers/hwmon/pmbus/mp2888.c

Quoted 9 lines
drivers/hwmon/pmbus/mp2888.c new file +9 -0
+
+static int
+mp2888_read_phases(struct i2c_client *client, struct mp2888_data
+*data, int page, int phase) {
+	int ret;
+
+	switch (phase) {
+	case 0 ... 1:
+		ret = mp2888_read_phase(client, data, page, phase,

MP2888_MFR_READ_CS1_2);

drivers/hwmon/pmbus/mp2888.c

Quoted 3 lines
drivers/hwmon/pmbus/mp2888.c new file +3 -0
+		break;
+	case 2 ... 3:
+		ret = mp2888_read_phase(client, data, page, phase,

MP2888_MFR_READ_CS3_4);

drivers/hwmon/pmbus/mp2888.c

Quoted 3 lines
drivers/hwmon/pmbus/mp2888.c new file +3 -0
+		break;
+	case 4 ... 5:
+		ret = mp2888_read_phase(client, data, page, phase,

MP2888_MFR_READ_CS5_6);

drivers/hwmon/pmbus/mp2888.c

Quoted 3 lines
drivers/hwmon/pmbus/mp2888.c new file +3 -0
+		break;
+	case 6 ... 7:
+		ret = mp2888_read_phase(client, data, page, phase,

MP2888_MFR_READ_CS7_8);

drivers/hwmon/pmbus/mp2888.c

Quoted 3 lines
drivers/hwmon/pmbus/mp2888.c new file +3 -0
+		break;
+	case 8 ... 9:
+		ret = mp2888_read_phase(client, data, page, phase,

MP2888_MFR_READ_CS9_10);

drivers/hwmon/pmbus/mp2888.c

Quoted 10 lines
drivers/hwmon/pmbus/mp2888.c new file +10 -0
+		break;
+	default:
+		return -ENODATA;
+	}
+	return ret;
+}
+
+static int mp2888_read_word_data(struct i2c_client *client, int page,
+int phase, int reg) {
+	const struct pmbus_driver_info *info =

pmbus_get_driver_info(client);

drivers/hwmon/pmbus/mp2888.c

Quoted 12 lines
drivers/hwmon/pmbus/mp2888.c new file +12 -0
+	struct mp2888_data *data = to_mp2888_data(info);
+	s64 val;
+	int ret;
+
+	switch (reg) {
+	case PMBUS_READ_VIN:
+		ret = pmbus_read_word_data(client, page, phase, reg);
+		if (ret <= 0)
+			return ret;
+
+		/*
+		 * READ_VIN register has unused bits 15:10 with fixed value

111011b. Clear these

Quoted 10 lines
drivers/hwmon/pmbus/mp2888.c new file +10 -0
+		 * bits and scale with coefficient 0.03125 by using mantissa 32.
+		 */
+		ret = (ret & GENMASK(9, 0));
+		break;
+	case PMBUS_VIN_OV_FAULT_LIMIT:
+	case PMBUS_VIN_UV_WARN_LIMIT:
+		ret = pmbus_read_word_data(client, page, phase, reg);
+		if (ret <= 0)
+			return ret;
+
Quoted 8 lines

drivers/hwmon/pmbus/mp2888.c line 205

Quoted 3 lines
drivers/hwmon/pmbus/mp2888.c new file +3 -0
+		val = mp2888_reg2data_linear11(ret, PSC_VOLTAGE_IN);
+		ret = val * MP2888_VIN_MANTISSA;
+		break;

Unless I am missing something, this is linear11 with, for some reason, bit 10 erroneously set for PMBUS_READ_VIN. That means that setting the format for PSC_VOLTAGE_IN and clearing bit 10 for PMBUS_READ_VIN should work without local conversion. Am I missing something ?

On board I have vin 12V.
From register I am getting 0xed80.
If I am masking bit 10, 0xe980, it gives me 48.12 V, masking bits 11,10 gives 24.00. For getting 12, I should mask bits 11, 10 and shift left bits 8 - 0. But I it doesn't look right things to do.

Quoted 10 lines

drivers/hwmon/pmbus/mp2888.c line 210

Quoted 6 lines
drivers/hwmon/pmbus/mp2888.c new file +5 -0
+	case PMBUS_OT_WARN_LIMIT:
+		ret = pmbus_read_word_data(client, page, phase, reg);
+		if (ret < 0)
+			return ret;
+		ret *= MP2888_TEMP_UNIT;

Explanation would be useful: chip reports limits in degrees C, but the actual temperature in 10th of degrees C. This is needed to match the two.

ACK.

Quoted 18 lines

drivers/hwmon/pmbus/mp2888.c

Quoted 4 lines
drivers/hwmon/pmbus/mp2888.c new file +4 -0
+		break;
+	case PMBUS_READ_IOUT:
+		if (phase != 0xff)
+			return mp2888_read_phases(client, data, page,

phase);

drivers/hwmon/pmbus/mp2888.c

Quoted 6 lines
drivers/hwmon/pmbus/mp2888.c new file +6 -0
+
+		ret = pmbus_read_word_data(client, page, phase, reg);
+		if (ret < 0)
+			return ret;
+		/*
+		 * READ_IOUT register has unused bits 15:12 with fixed value

1110b. Clear these

drivers/hwmon/pmbus/mp2888.c

Quoted 1 line
drivers/hwmon/pmbus/mp2888.c new file +1 -0
+		 * bits and scale with total current resolution. Data is provided

in direct format.

drivers/hwmon/pmbus/mp2888.c

Quoted 3 lines
drivers/hwmon/pmbus/mp2888.c new file +3 -0
+		 */
+		ret &= GENMASK(11, 0);
+		ret = data->total_curr_resolution ?

DIV_ROUND_CLOSEST(ret, 2) :

Quoted 8 lines

drivers/hwmon/pmbus/mp2888.c line 225

Quoted 1 line
drivers/hwmon/pmbus/mp2888.c new file +1 -0
+		      DIV_ROUND_CLOSEST(ret, 4);

I don't think this is a good idea: It unnecessarily reduces the resolution (which is bad enough already). The format for output current is already set to direct with a mantissa of 1. It might make sense to adjust the mantissa to always report the correct current. Specifically, if current resolution is set to 1, the value here should be multiplied, not divided.

Yes, I'll do it through mantissa.

Quoted 12 lines

drivers/hwmon/pmbus/mp2888.c line 232

Quoted 8 lines
drivers/hwmon/pmbus/mp2888.c new file +7 -0
+		break;
+	case PMBUS_READ_POUT:
+	case PMBUS_READ_PIN:
+		ret = pmbus_read_word_data(client, page, phase, reg);
+		if (ret < 0)
+			return ret;
+		val = mp2888_reg2data_linear11(ret, PSC_POWER);

This call seems pretty pointless: The exponent bits are always 0, and we know that the resolution is either 0.5W or 1W.

Quoted 7 lines

Conversion to micro-watt should be handled by the PMBus core.

drivers/hwmon/pmbus/mp2888.c

Quoted 2 lines
drivers/hwmon/pmbus/mp2888.c new file +2 -0
+		ret = data->total_curr_resolution ? val :
+DIV_ROUND_CLOSEST_ULL(val, 2);

Same as above: Any adjustment should be a multiplication, not a divide operation, to avoid resolution loss.

ACK.

Quoted 7 lines

drivers/hwmon/pmbus/mp2888.c

Quoted 4 lines
drivers/hwmon/pmbus/mp2888.c new file +3 -0
+		break;
+	/*
+	 * The below registers are not implemented by device or

implemented not according to the

drivers/hwmon/pmbus/mp2888.c

Quoted 1 line
drivers/hwmon/pmbus/mp2888.c new file +1 -0
+	 * spec. Skip all of them to avoid exposing non-relevant inputs to

sysfs.

Quoted 15 lines

drivers/hwmon/pmbus/mp2888.c line 249

Quoted 12 lines
drivers/hwmon/pmbus/mp2888.c new file +12 -0
+	 */
+	case PMBUS_OT_FAULT_LIMIT:
+	case PMBUS_UT_WARN_LIMIT:
+	case PMBUS_UT_FAULT_LIMIT:
+	case PMBUS_VIN_UV_FAULT_LIMIT:
+	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_VIN_OV_WARN_LIMIT:
+	case PMBUS_IOUT_OC_LV_FAULT_LIMIT:
+	case PMBUS_IOUT_OC_WARN_LIMIT:

Datasheet says this is a 10-bit value with a scale of 1A or 2A depending on the current resolution.

OK

Quoted 8 lines

drivers/hwmon/pmbus/mp2888.c line 253

Quoted 5 lines
drivers/hwmon/pmbus/mp2888.c new file +4 -0
+	case PMBUS_IOUT_OC_FAULT_LIMIT:
+	case PMBUS_IOUT_UC_FAULT_LIMIT:
+	case PMBUS_POUT_OP_FAULT_LIMIT:
+	case PMBUS_POUT_OP_WARN_LIMIT:

Datasheet: 10-bit value with 1W/LSB or 2W/LSB depending on current resolution.

OK

Along with it I should disable PMBUS_POUT_MAX. In datasheet it is for different purpose. I missed it.

Quoted 76 lines

Unless there is reqason to believe that the datasheet is wrong, those should be supported. If the datasheet is wrong it should be explained.

Quoted 9 lines
drivers/hwmon/pmbus/mp2888.c new file +9 -0
+	case PMBUS_PIN_OP_WARN_LIMIT:
+	case PMBUS_MFR_VIN_MIN:
+	case PMBUS_MFR_VIN_MAX:
+	case PMBUS_MFR_VOUT_MAX:
+	case PMBUS_MFR_IIN_MAX:
+	case PMBUS_MFR_IOUT_MAX:
+	case PMBUS_MFR_PIN_MAX:
+	case PMBUS_MFR_POUT_MAX:
+	case PMBUS_MFR_MAX_TEMP_1:

drivers/hwmon/pmbus/mp2888.c

Quoted 10 lines
drivers/hwmon/pmbus/mp2888.c new file +10 -0
+		return -ENXIO;
+	default:
+		return -ENODATA;
+	}
+
+	return ret;
+}
+
+static int
+mp2888_identify_multiphase(struct i2c_client *client, struct mp2888_data

*data,

drivers/hwmon/pmbus/mp2888.c

Quoted 9 lines
drivers/hwmon/pmbus/mp2888.c new file +9 -0
+			   struct pmbus_driver_info *info) {
+	int i, ret;
+
+	ret = i2c_smbus_write_byte_data(client, PMBUS_PAGE, 0);
+	if (ret < 0)
+		return ret;
+
+	/* Identify multiphase number - could be from 1 to 10. */
+	ret = i2c_smbus_read_word_data(client,

MP2888_MFR_VR_CONFIG1);

drivers/hwmon/pmbus/mp2888.c

Quoted 7 lines
drivers/hwmon/pmbus/mp2888.c new file +7 -0
+	if (ret <= 0)
+		return ret;
+
+	info->phases[0] = ret & GENMASK(3, 0);
+
+	/*
+	 * The device provides a total of 10 PWM pins, and can be configured

to different phase

Quoted 18 lines
drivers/hwmon/pmbus/mp2888.c new file +18 -0
+	 * count applications for rail.
+	 */
+	if (info->phases[0] > MP2888_MAX_PHASE)
+		return -EINVAL;
+
+	/* Disable non-configured phases. */
+	for (i = info->phases[0]; i < MP2888_MAX_PHASE; i++)
+		info->pfunc[i] = 0;
+
+	return 0;
+}
+
+static struct pmbus_driver_info mp2888_info = {
+	.pages = 1,
+	.format[PSC_VOLTAGE_IN] = direct,
+	.format[PSC_VOLTAGE_OUT] = direct,
+	.format[PSC_TEMPERATURE] = direct,
+	.format[PSC_CURRENT_IN] = linear,

drivers/hwmon/pmbus/mp2888.c

Quoted 10 lines
drivers/hwmon/pmbus/mp2888.c new file +10 -0
+	.format[PSC_CURRENT_OUT] = direct,
+	.format[PSC_POWER] = direct,
+	.m[PSC_VOLTAGE_IN] = MP2888_VIN_MANTISSA,
+	.m[PSC_TEMPERATURE] = 1,
+	.R[PSC_TEMPERATURE] = 1,
+	.m[PSC_VOLTAGE_OUT] = 1,
+	.R[PSC_VOLTAGE_OUT] = 3,
+	.m[PSC_CURRENT_OUT] = 1,
+	.m[PSC_POWER] = 1,
+	.func[0] = PMBUS_HAVE_VIN | PMBUS_HAVE_VOUT |

PMBUS_HAVE_STATUS_VOUT | PMBUS_HAVE_IOUT |

drivers/hwmon/pmbus/mp2888.c

Quoted 1 line
drivers/hwmon/pmbus/mp2888.c new file +1 -0
+		   PMBUS_HAVE_STATUS_IOUT | PMBUS_HAVE_TEMP |

PMBUS_HAVE_STATUS_TEMP |

drivers/hwmon/pmbus/mp2888.c

Quoted 1 line
drivers/hwmon/pmbus/mp2888.c new file +1 -0
+		   PMBUS_HAVE_POUT | PMBUS_HAVE_PIN |

PMBUS_HAVE_STATUS_INPUT |

Quoted 16 lines

drivers/hwmon/pmbus/mp2888.c line 332

Quoted 13 lines
drivers/hwmon/pmbus/mp2888.c new file +13 -0
+		   PMBUS_PHASE_VIRTUAL,
+	.pfunc[0] = PMBUS_HAVE_IOUT,
+	.pfunc[1] = PMBUS_HAVE_IOUT,
+	.pfunc[2] = PMBUS_HAVE_IOUT,
+	.pfunc[3] = PMBUS_HAVE_IOUT,
+	.pfunc[4] = PMBUS_HAVE_IOUT,
+	.pfunc[5] = PMBUS_HAVE_IOUT,
+	.pfunc[6] = PMBUS_HAVE_IOUT,
+	.pfunc[7] = PMBUS_HAVE_IOUT,
+	.pfunc[8] = PMBUS_HAVE_IOUT,
+	.pfunc[9] = PMBUS_HAVE_IOUT,
+	.read_byte_data = mp2888_read_byte_data,
+	.read_word_data = mp2888_read_word_data,

I don't see a write function here. Did you try setting limits ? Given the adjustments needed for reading limits, I don't think this will work.

No, I didn't try.

Quoted 60 lines

Thanks,
Guenter

drivers/hwmon/pmbus/mp2888.c

Quoted 8 lines
drivers/hwmon/pmbus/mp2888.c new file +8 -0
+};
+
+static int mp2888_probe(struct i2c_client *client) {
+	struct pmbus_driver_info *info;
+	struct mp2888_data *data;
+	int ret;
+
+	data = devm_kzalloc(&client->dev, sizeof(struct mp2888_data),

GFP_KERNEL);

drivers/hwmon/pmbus/mp2888.c

Quoted 12 lines
drivers/hwmon/pmbus/mp2888.c new file +12 -0
+	if (!data)
+		return -ENOMEM;
+
+	memcpy(&data->info, &mp2888_info, sizeof(*info));
+	info = &data->info;
+
+	/* Identify multiphase configuration. */
+	ret = mp2888_identify_multiphase(client, data, info);
+	if (ret)
+		return ret;
+
+	/* Obtain current sense gain of power stage and current resolution.

*/

Quoted 34 lines
drivers/hwmon/pmbus/mp2888.c new file +33 -0
+	ret = mp2888_current_sense_gain_and_resolution_get(client, data);
+	if (ret)
+		return ret;
+
+	return pmbus_do_probe(client, info); }
+
+static const struct i2c_device_id mp2888_id[] = {
+	{"mp2888", 0},
+	{}
+};
+
+MODULE_DEVICE_TABLE(i2c, mp2888_id);
+
+static const struct of_device_id __maybe_unused mp2888_of_match[] = {
+	{.compatible = "mps,mp2888"},
+	{}
+};
+MODULE_DEVICE_TABLE(of, mp2888_of_match);
+
+static struct i2c_driver mp2888_driver = {
+	.driver = {
+		.name = "mp2888",
+		.of_match_table = of_match_ptr(mp2888_of_match),
+	},
+	.probe_new = mp2888_probe,
+	.id_table = mp2888_id,
+};
+
+module_i2c_driver(mp2888_driver);
+
+MODULE_AUTHOR("Vadim Pasternak <[email protected]>");
+MODULE_DESCRIPTION("PMBus driver for MPS MP2888 device");
+MODULE_LICENSE("GPL");
maintainer
#5 lore raw

Re: [PATCH hwmon-next v3 2/3] hwmon: (pmbus) Add support for MPS Multi-phase mp2888 controller

Quoted 2 lines · Vadim Pasternak

Hi Guenter!

...

Quoted 15 lines
Quoted 6 lines
Quoted 6 lines
drivers/hwmon/pmbus/mp2888.c new file +6 -0
+	case PMBUS_VIN_OV_FAULT_LIMIT:
+	case PMBUS_VIN_UV_WARN_LIMIT:
+		ret = pmbus_read_word_data(client, page, phase, reg);
+		if (ret <= 0)
+			return ret;
+
Quoted 8 lines

drivers/hwmon/pmbus/mp2888.c line 205

Quoted 3 lines
drivers/hwmon/pmbus/mp2888.c new file +3 -0
+		val = mp2888_reg2data_linear11(ret, PSC_VOLTAGE_IN);
+		ret = val * MP2888_VIN_MANTISSA;
+		break;

Unless I am missing something, this is linear11 with, for some reason, bit 10 erroneously set for PMBUS_READ_VIN. That means that setting the format for PSC_VOLTAGE_IN and clearing bit 10 for PMBUS_READ_VIN should work without local conversion. Am I missing something ?

Quoted 6 lines

On board I have vin 12V.
From register I am getting 0xed80.
If I am masking bit 10, 0xe980, it gives me 48.12 V, masking bits 11,10 gives 24.00. For getting 12, I should mask bits 11, 10 and shift left bits 8 - 0. But I it doesn't look right things to do.

Turns out that is because the data format for READ_VIN is not linear11, it is something like linear10. The mantissa is 10 bit (unsigned), the exponent is 6 bit. So the exponent is -5, not -6 as one would assume.

With that in mind, I'd still prefer to fix up READ_VIN to linear11 instead of converting the limits to direct mode.

Thanks,
Guenter

#6 lore raw

PATCH v3 3/3 dt-bindings: Add MP2888 voltage regulator device

In reply to Vadim Pasternak

Monolithic Power Systems, Inc. (MPS) dual-loop, digital, multi-phase controller.

---
 Documentation/devicetree/bindings/trivial-devices.yaml | 2 ++
 1 file changed, 2 insertions(+)
Documentation/devicetree/bindings/trivial-devices.yaml +2 -0
index a327130d1faa..4f6d149bfb3f 100644
@@ -98,6 +98,8 @@ properties:
           - fsl,mpl3115
             # MPR121: Proximity Capacitive Touch Sensor Controller
           - fsl,mpr121
+            # Monolithic Power Systems Inc. multi-phase controller mp2888
+          - mps,mp2888
             # Monolithic Power Systems Inc. multi-phase controller mp2975
           - mps,mp2975
             # G751: Digital Temperature Sensor and Thermal Watchdog with Two-Wire Interface
Signature · 1 line
2.11.0