Thread (5 messages) 5 messages, 2 authors, 2025-10-29

Re: [PATCH net-next v2 1/2] net: pse-pd: tps23881: Add support for TPS23881B

From: Oleksij Rempel <o.rempel@pengutronix.de>
Date: 2025-10-24 10:50:19
Also in: linux-devicetree, lkml

Hi Thomas,

Thank you for your work.

Looks good, here are some nits:

On Thu, Oct 23, 2025 at 12:05:18AM +0200, Thomas Wismer wrote:
quoted hunk ↗ jump to hunk
From: Thomas Wismer <redacted>

The TPS23881B uses different firmware than the TPS23881. Trying to load the
TPS23881 firmware on a TPS23881B device fails and must be omitted.

The TPS23881B ships with a more recent ROM firmware. Moreover, no updated
firmware has been released yet and so the firmware loading step must be
skipped. As of today, the TPS23881B is intended to use its ROM firmware.

Signed-off-by: Thomas Wismer <redacted>
---
 drivers/net/pse-pd/tps23881.c | 65 +++++++++++++++++++++++++++--------
 1 file changed, 51 insertions(+), 14 deletions(-)
diff --git a/drivers/net/pse-pd/tps23881.c b/drivers/net/pse-pd/tps23881.c
index b724b222ab44..f45c08759082 100644
--- a/drivers/net/pse-pd/tps23881.c
+++ b/drivers/net/pse-pd/tps23881.c
@@ -55,8 +55,6 @@
 #define TPS23881_REG_TPON	BIT(0)
 #define TPS23881_REG_FWREV	0x41
 #define TPS23881_REG_DEVID	0x43
-#define TPS23881_REG_DEVID_MASK	0xF0
-#define TPS23881_DEVICE_ID	0x02
 #define TPS23881_REG_CHAN1_CLASS	0x4c
 #define TPS23881_REG_SRAM_CTRL	0x60
 #define TPS23881_REG_SRAM_DATA	0x61
@@ -1012,8 +1010,28 @@ static const struct pse_controller_ops tps23881_ops = {
 	.pi_get_pw_req = tps23881_pi_get_pw_req,
 };
 
-static const char fw_parity_name[] = "ti/tps23881/tps23881-parity-14.bin";
-static const char fw_sram_name[] = "ti/tps23881/tps23881-sram-14.bin";
+struct tps23881_info {
+	u8 dev_id;	/* device ID and silicon revision */
+	const char *fw_parity_name;	/* parity code firmware file name */
+	const char *fw_sram_name;	/* SRAM code firmware file name */
+};
+
+enum tps23881_model {
+	TPS23881,
+	TPS23881B,
+};
+
+static const struct tps23881_info tps23881_info[] = {
+	[TPS23881] = {
+		.dev_id = 0x22,
+		.fw_parity_name = "ti/tps23881/tps23881-parity-14.bin",
+		.fw_sram_name = "ti/tps23881/tps23881-sram-14.bin",
+	},
+	[TPS23881B] = {
+		.dev_id = 0x24,
+		/* skip SRAM load, ROM firmware already IEEE802.3bt compliant */
TL;DR:

A more accurate comment would be:
/* skip SRAM load, ROM provides Clause 145 hardware-level support */

Longer version:

Please reference IEEE 802.3-2022 (Clause 145) instead of the "IEEE
802.3bt" amendment:
- The IEEE 802.3-2022 standard is free for everyone with the IEEE Xplore
  program.
- The "bt" amendment costs money.
- Using the free standard helps all community members review this driver.

The chip (hardware) alone cannot be "compliant." Full compliance for
Type 3 or Type 4 needs the whole system to work correctly:
- The Linux system must handle the DLL (LLDP) classification in software.
  The chip's ROM does not do this.
- The board must supply the correct voltage (e.g., 52V to 57V for Type
  4). This chip's minimum 44V is not compliant with Clause 145.
- The final power supply and board must be designed to handle the high
  power (like 90W).

[...]
quoted hunk ↗ jump to hunk
@@ -1422,6 +1442,10 @@ static int tps23881_i2c_probe(struct i2c_client *client)
[...]
 
-	dev_info(&client->dev, "Firmware revision 0x%x\n", ret);
+	if (ret == 0xFF)
+		dev_warn(&client->dev, "Device entered safe mode\n");
                return -ENODEV; /* Or another appropriate error */

The datasheet states this happens on an "un-recoverable firmware fault."
According to the datasheet, when in "Safe Mode," all ports are shut
down. The device is not in a functional state to act as a PSE
controller.

quoted hunk ↗ jump to hunk
+	else
+		dev_info(&client->dev, "Firmware revision 0x%x%s\n", ret,
+			 ret == 0x00 ? " (ROM firmware)" : "");
 
 	/* Set configuration B, 16 bit access on a single device address */
 	ret = i2c_smbus_read_byte_data(client, TPS23881_REG_GEN_MASK);
@@ -1504,7 +1534,14 @@ static const struct i2c_device_id tps23881_id[] = {
 MODULE_DEVICE_TABLE(i2c, tps23881_id);
I guess tps23881_id should be updated too.

Best Regards,
Oleksij
-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help