[PATCH 0/3] hwmon: (pmbus/tps53679) Add TPS536C7

[PATCH 0/3] hwmon: (pmbus/tps53679) Add TPS536C7

[Pradhan, Sanman]

From: Sanman Pradhan <psanman@juniper.net>

TPS536C7 is a TI dual-channel D-CAP+ step-down controller. Its two
channels are distributed across a configurable number of phases (N+M,
up to 12 total), and whether channel B (PMBus page 1) exists depends on
the phase configuration, so the page count is derived at probe time.

Patch 1 fixes a pre-existing TPS53676 bug in the phase-configuration
parser (it tested the firing-order bit instead of the PAGE bit), which
miscounts channel-B phases on dual-channel parts; it is also a
prerequisite for reusing that parser for TPS536C7. Patch 2 adds the
compatible string. Patch 3 adds the driver support via a shared helper,
exposing aggregate per-channel telemetry only (TPS536C7 can exceed
PMBUS_PHASES, so per-phase telemetry is not reported).

Tested on a TPS536C7 in a QFX5230 using a Linux 5.15 backport of this
driver; the part was configured for channel A only. CAPABILITY read back
0xd0; block reads of IC_DEVICE_ID and USER_DATA_03 failed with -EBADMSG
when PEC was enabled, while the same reads without PEC returned the
expected 6- and 24-byte payloads, and byte/word telemetry kept working
with PEC. The series itself is built and checked against hwmon-next. The
dual-channel (page 1) path is exercised by inspection only, as no such
part was available.

Note: Senthil Muniyappan, credited in patch 3, has left the company.

Sanman Pradhan (3):
  hwmon: (pmbus/tps53679) Fix TPS53676 phase page decoding
  dt-bindings: trivial-devices: Add TI TPS536C7
  hwmon: (pmbus/tps53679) Add support for TPS536C7

 .../devicetree/bindings/trivial-devices.yaml  |   2 +
 Documentation/hwmon/tps53679.rst              |  23 +++-
 drivers/hwmon/pmbus/Kconfig                   |   2 +-
 drivers/hwmon/pmbus/tps53679.c                | 128 ++++++++++++++++--
 4 files changed, 136 insertions(+), 19 deletions(-)


base-commit: ba08432bda66a7889d8f3d1581dabf10f59b25eb
-- 
2.34.1

[PATCH 1/3] hwmon: (pmbus/tps53679) Fix TPS53676 phase page decoding

[Pradhan, Sanman]

From: Sanman Pradhan <psanman@juniper.net>

tps53676_identify() reads the USER_DATA_03 phase configuration to count
the phases assigned to each channel and derive the number of PMBus pages.
In each 16-bit phase descriptor the channel (PAGE) is encoded in bit 4 and
the firing order in bits 3:0, but the code tested bit 3 (0x08), which is
part of the firing-order field.

TPS53676 supports up to seven phases, so firing-order bit 3 is never set.
As a result the existing test classifies every enabled phase as channel A.
On a dual-channel configuration the phases assigned to channel B are
therefore miscounted as channel A and page 1 is not exposed.

Test the PAGE field (bit 4) instead.

Fixes: cb3d37b59012 ("hwmon: (pmbus/tps53679) Add support for TI TPS53676")
Cc: stable@vger.kernel.org
Signed-off-by: Sanman Pradhan <psanman@juniper.net>
---
 drivers/hwmon/pmbus/tps53679.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hwmon/pmbus/tps53679.c b/drivers/hwmon/pmbus/tps53679.c
index fa0fdf1e3e6c9..680339a6b90fe 100644
--- a/drivers/hwmon/pmbus/tps53679.c
+++ b/drivers/hwmon/pmbus/tps53679.c
@@ -188,7 +188,7 @@ static int tps53676_identify(struct i2c_client *client,
 		return -EIO;
 	for (i = 0; i < 2 * TPS53676_MAX_PHASES; i += 2) {
 		if (buf[i + 1] & 0x80) {
-			if (buf[i] & 0x08)
+			if (buf[i] & BIT(4))
 				phases_b++;
 			else
 				phases_a++;
-- 
2.34.1

[PATCH 2/3] dt-bindings: trivial-devices: Add TI TPS536C7

[Pradhan, Sanman]

From: Sanman Pradhan <psanman@juniper.net>

TPS536C7 is a PMBus-compliant dual-channel D-CAP+ multiphase step-down
controller. Add its compatible string.

Signed-off-by: Sanman Pradhan <psanman@juniper.net>
---
 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 94e63d8a58ad5..f4739e0677203 100644
--- a/Documentation/devicetree/bindings/trivial-devices.yaml
+++ b/Documentation/devicetree/bindings/trivial-devices.yaml
@@ -530,6 +530,8 @@ properties:
           - ti,tps53685
             # TI Dual channel DCAP+ multiphase controller TPS53688
           - ti,tps53688
+            # TI Dual channel DCAP+ multiphase controller TPS536C7
+          - ti,tps536c7
             # TI DC-DC converters on PMBus
           - ti,tps544b20
           - ti,tps544b25
-- 
2.34.1

[PATCH] hwmon: (pmbus/tps53679) Add support for TPS536C7

[Pradhan, Sanman]

From: Sanman Pradhan <psanman@juniper.net>

TPS536C7 is a dual-channel D-CAP+ step-down controller whose channels are
distributed across a configurable number of phases (N+M, up to 12 total).
Whether channel B (PMBus page 1) exists depends on the phase configuration
register, so the page count is derived at probe time.

Factor the TPS53676 phase-map parsing into a shared helper and reuse it
for TPS536C7. Only aggregate per-channel telemetry is exposed: leave
info->phases[] unset so the PMBus core never programs the PHASE selector
on every read (and because TPS536C7 can place up to 12 phases on channel
A, exceeding PMBUS_PHASES). Report VOUT in linear format like TPS53676.

Since the core never programs PHASE, set it to 0xff on each populated
page so READ_IOUT reports the aggregate channel current, and verify it
(PHASE can be write-protected). Fail identification if aggregate
selection cannot be established, since otherwise a single phase current
could be reported as the channel total.

A configuration with no channel-A phase is rejected with -EOPNOTSUPP:
the PMBus core models pages as the contiguous range 0..pages-1 and
cannot represent a channel-A-absent topology.

On the tested TPS536C7 (device ID 54 49 53 6c 70 00, CAPABILITY 0xd0),
block reads of IC_DEVICE_ID and USER_DATA_03 return -EBADMSG with PEC
enabled, while retrying those two reads without PEC returns the expected
6- and 24-byte payloads. Standard byte/word telemetry keeps using PEC,
so only those two identification reads fall back to no-PEC.

Co-developed-by: Senthil Muniyappan <smuniyappan@juniper.net>
Co-developed-by: Vaibhav Agarwal <avaibhav@juniper.net>
Signed-off-by: Sanman Pradhan <psanman@juniper.net>
---
 Documentation/hwmon/tps53679.rst |  23 ++++--
 drivers/hwmon/pmbus/Kconfig      |   2 +-
 drivers/hwmon/pmbus/tps53679.c   | 128 +++++++++++++++++++++++++++----
 3 files changed, 134 insertions(+), 19 deletions(-)

diff --git a/Documentation/hwmon/tps53679.rst b/Documentation/hwmon/tps53679.rst
index 2280e043c4de9..3d584197b3a03 100644
--- a/Documentation/hwmon/tps53679.rst
+++ b/Documentation/hwmon/tps53679.rst
@@ -75,6 +75,14 @@ Supported chips:
 
     Datasheet: Available under NDA
 
+  * Texas Instruments TPS536C7
+
+    Prefix: 'tps536c7'
+
+    Addresses scanned: -
+
+    Datasheet: https://www.ti.com/lit/gpn/TPS536C7
+
 
 Authors:
 	Vadim Pasternak <vadimp@mellanox.com>
@@ -85,7 +93,12 @@ Description
 -----------
 
 Chips in this series are multi-phase step-down converters with one or two
-output channels and up to 8 phases per channel.
+output channels and up to 12 phases in total, depending on the device.
+
+For TPS536C7 only aggregate per-channel telemetry is exposed; the
+individual per-phase output currents the hardware can report are not.
+Attributes for the second output channel are present only when channel B
+is configured.
 
 
 Usage Notes
@@ -124,7 +137,7 @@ in1_crit_alarm		Input voltage critical high alarm.
 in[N]_label		"vout[1-2]"
 
 			- TPS53647, TPS53667: N=2
-			- TPS53622, TPS53659, TPS53679, TPS53688: N=2,3
+			- TPS53622, TPS53659, TPS53679, TPS53688, TPS536C7: N=2,3
 
 in[N]_input		Measured output voltage.
 
@@ -151,7 +164,7 @@ in[N]_crit_alarm	Output voltage critical high alarm.
 temp[N]_input		Measured temperature.
 
 			- TPS53647, TPS53667: N=1
-			- TPS53622, TPS53659, TPS53679, TPS53681, TPS53688: N=1,2
+			- TPS53622, TPS53659, TPS53679, TPS53681, TPS53688, TPS536C7: N=1,2
 
 temp[N]_max		Maximum temperature.
 
@@ -168,7 +181,7 @@ power1_input		Measured input power.
 power[N]_label		"pout[1-2]".
 
 			- TPS53647, TPS53667: N=2
-			- TPS53622, TPS53659, TPS53676, TPS53679, TPS53681, TPS53688: N=2,3
+			- TPS53622, TPS53659, TPS53676, TPS53679, TPS53681, TPS53688, TPS536C7: N=2,3
 
 power[N]_input		Measured output power.
 
@@ -191,7 +204,7 @@ curr[N]_label		"iout[1-2]" or "iout1.[0-5]".
 			telemetry supported on TPS53676 and TPS53681 only.
 
 			- TPS53647, TPS53667: N=2
-			- TPS53622, TPS53659, TPS53679, TPS53688: N=2,3
+			- TPS53622, TPS53659, TPS53679, TPS53688, TPS536C7: N=2,3
 			- TPS53676: N=2-8
 			- TPS53681: N=2-9
 
diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig
index bcfdc4ce4c100..b6658ff242ef9 100644
--- a/drivers/hwmon/pmbus/Kconfig
+++ b/drivers/hwmon/pmbus/Kconfig
@@ -794,7 +794,7 @@ config SENSORS_TPS53679
 	help
 	  If you say yes here you get hardware monitoring support for TI
 	  TPS53622, TPS53647, TPS53659, TPS53667, TPS53676, TPS53679, TPS53681,
-	  TPS53685, and TPS53688.
+	  TPS53685, TPS53688, and TPS536C7.
 
 	  This driver can also be built as a module. If so, the module will
 	  be called tps53679.
diff --git a/drivers/hwmon/pmbus/tps53679.c b/drivers/hwmon/pmbus/tps53679.c
index 680339a6b90fe..98a68211a2bbf 100644
--- a/drivers/hwmon/pmbus/tps53679.c
+++ b/drivers/hwmon/pmbus/tps53679.c
@@ -17,13 +17,20 @@
 
 enum chips {
 	tps53622, tps53647, tps53659, tps53667, tps53676, tps53679, tps53681,
-	tps53685, tps53688
+	tps53685, tps53688, tps536c7
 };
 
 #define TPS53647_PAGE_NUM		1
 
-#define TPS53676_USER_DATA_03		0xb3
+#define TPS536XX_PHASE_CONFIG		0xb3
 #define TPS53676_MAX_PHASES		7
+#define TPS536C7_MAX_PHASES		12
+
+#define TPS536XX_PHASE_ENABLE		BIT(7)
+#define TPS536XX_PHASE_PAGE		BIT(4)
+
+#define TPS53676_DEVICE_ID		"TI\x53\x67\x60\x00"
+#define TPS536C7_DEVICE_ID		"TI\x53\x6c\x70\x00"
 
 #define TPS53679_PROT_VR12_5MV		0x01 /* VR12.0 mode, 5-mV DAC */
 #define TPS53679_PROT_VR12_5_10MV	0x02 /* VR12.5 mode, 10-mV DAC */
@@ -166,34 +173,72 @@ static int tps53681_identify(struct i2c_client *client,
 					    TPS53681_DEVICE_ID);
 }
 
-static int tps53676_identify(struct i2c_client *client,
-			     struct pmbus_driver_info *info)
+static int tps536xx_read_block(struct i2c_client *client, u8 reg, u8 *buf,
+			       bool retry_without_pec)
+{
+	int ret = i2c_smbus_read_block_data(client, reg, buf);
+
+	/*
+	 * Some TPS536C7 samples return an invalid PEC on the device-ID and
+	 * phase-configuration block reads. Retry once without PEC; ordinary
+	 * telemetry keeps using PEC.
+	 */
+	if (ret == -EBADMSG && retry_without_pec &&
+	    (client->flags & I2C_CLIENT_PEC)) {
+		client->flags &= ~I2C_CLIENT_PEC;
+		ret = i2c_smbus_read_block_data(client, reg, buf);
+		client->flags |= I2C_CLIENT_PEC;
+	}
+	return ret;
+}
+
+static int tps536xx_read_phases(struct i2c_client *client,
+				const char *device_id, int max_phases,
+				bool retry_without_pec, int *phases_a,
+				int *phases_b)
 {
 	u8 buf[I2C_SMBUS_BLOCK_MAX];
-	int phases_a = 0, phases_b = 0;
 	int i, ret;
 
-	ret = i2c_smbus_read_block_data(client, PMBUS_IC_DEVICE_ID, buf);
+	ret = tps536xx_read_block(client, PMBUS_IC_DEVICE_ID, buf,
+				  retry_without_pec);
 	if (ret < 0)
 		return ret;
-	if (ret != 6 || memcmp(buf, "TI\x53\x67\x60\x00", 6)) {
+	if (ret != 6 || memcmp(buf, device_id, 6)) {
 		dev_err(&client->dev, "Unexpected device ID: %*ph\n", ret, buf);
 		return -ENODEV;
 	}
 
-	ret = i2c_smbus_read_block_data(client, TPS53676_USER_DATA_03, buf);
+	ret = tps536xx_read_block(client, TPS536XX_PHASE_CONFIG, buf,
+				  retry_without_pec);
 	if (ret < 0)
 		return ret;
 	if (ret != 24)
 		return -EIO;
-	for (i = 0; i < 2 * TPS53676_MAX_PHASES; i += 2) {
-		if (buf[i + 1] & 0x80) {
-			if (buf[i] & BIT(4))
-				phases_b++;
+
+	*phases_a = 0;
+	*phases_b = 0;
+	for (i = 0; i < 2 * max_phases; i += 2) {
+		if (buf[i + 1] & TPS536XX_PHASE_ENABLE) {
+			if (buf[i] & TPS536XX_PHASE_PAGE)
+				(*phases_b)++;
 			else
-				phases_a++;
+				(*phases_a)++;
 		}
 	}
+	return 0;
+}
+
+static int tps53676_identify(struct i2c_client *client,
+			     struct pmbus_driver_info *info)
+{
+	int phases_a, phases_b, ret;
+
+	ret = tps536xx_read_phases(client, TPS53676_DEVICE_ID,
+				   TPS53676_MAX_PHASES, false,
+				   &phases_a, &phases_b);
+	if (ret)
+		return ret;
 
 	info->format[PSC_VOLTAGE_OUT] = linear;
 	info->pages = 1;
@@ -205,6 +250,58 @@ static int tps53676_identify(struct i2c_client *client,
 	return 0;
 }
 
+static int tps536c7_identify(struct i2c_client *client,
+			     struct pmbus_driver_info *info)
+{
+	int phases_a, phases_b, page, ret;
+
+	ret = tps536xx_read_phases(client, TPS536C7_DEVICE_ID,
+				   TPS536C7_MAX_PHASES, true,
+				   &phases_a, &phases_b);
+	if (ret)
+		return ret;
+	if (!phases_a) {
+		dev_err(&client->dev,
+			"TPS536C7 without channel A is not supported\n");
+		return -EOPNOTSUPP;
+	}
+
+	info->format[PSC_VOLTAGE_OUT] = linear;
+	/*
+	 * TPS536C7 can place up to 12 phases on channel A, which exceeds
+	 * PMBUS_PHASES. Report aggregate per-channel telemetry only and do
+	 * not populate info->phases[].
+	 */
+	info->pages = phases_b ? 2 : 1;
+
+	/*
+	 * With info->phases[] left unset the PMBus core never programs the
+	 * PHASE selector, so make sure each page reports the aggregate
+	 * current (PHASE = 0xff) rather than whatever a previous boot left.
+	 */
+	for (page = 0; page < info->pages; page++) {
+		ret = pmbus_read_byte_data(client, page, PMBUS_PHASE);
+		if (ret < 0)
+			return ret;
+		if (ret == 0xff)
+			continue;
+		ret = pmbus_write_byte_data(client, page, PMBUS_PHASE, 0xff);
+		if (ret < 0)
+			return ret;
+		/* PHASE may be write-protected; confirm it actually changed. */
+		ret = pmbus_read_byte_data(client, page, PMBUS_PHASE);
+		if (ret < 0)
+			return ret;
+		if (ret != 0xff) {
+			dev_err(&client->dev,
+				"failed to select aggregate PHASE on page %d\n",
+				page);
+			return -EIO;
+		}
+	}
+	return 0;
+}
+
 static int tps53681_read_word_data(struct i2c_client *client, int page,
 				   int phase, int reg)
 {
@@ -286,6 +383,9 @@ static int tps53679_probe(struct i2c_client *client)
 		info->pages = TPS53679_PAGE_NUM;
 		info->identify = tps53685_identify;
 		break;
+	case tps536c7:
+		info->identify = tps536c7_identify;
+		break;
 	default:
 		return -ENODEV;
 	}
@@ -304,6 +404,7 @@ static const struct i2c_device_id tps53679_id[] = {
 	{ .name = "tps53681", .driver_data = tps53681 },
 	{ .name = "tps53685", .driver_data = tps53685 },
 	{ .name = "tps53688", .driver_data = tps53688 },
+	{ .name = "tps536c7", .driver_data = tps536c7 },
 	{ }
 };
 
@@ -319,6 +420,7 @@ static const struct of_device_id __maybe_unused tps53679_of_match[] = {
 	{.compatible = "ti,tps53681", .data = (void *)tps53681},
 	{.compatible = "ti,tps53685", .data = (void *)tps53685},
 	{.compatible = "ti,tps53688", .data = (void *)tps53688},
+	{.compatible = "ti,tps536c7", .data = (void *)tps536c7},
 	{}
 };
 MODULE_DEVICE_TABLE(of, tps53679_of_match);
-- 
2.34.1