[v1] net: pse-pd: Add TPS23881B support

[PATCH 2/3] net: pse-pd: tps23881: Add support for TPS23881B

Posted by Thomas Wismer 2 months, 2 weeks ago

From: Thomas Wismer <thomas.wismer@scs.ch>

The TPS23881B device requires different firmware, but has a more recent ROM
firmware. Since no updated firmware has been released yet, the firmware
loading step must be skipped. The device runs from its ROM firmware.

Signed-off-by: Thomas Wismer <thomas.wismer@scs.ch>
---
 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 */
+	},
+};
 
 struct tps23881_fw_conf {
 	u8 reg;
@@ -1085,16 +1103,17 @@ static int tps23881_flash_sram_fw_part(struct i2c_client *client,
 	return ret;
 }
 
-static int tps23881_flash_sram_fw(struct i2c_client *client)
+static int tps23881_flash_sram_fw(struct i2c_client *client,
+				  const struct tps23881_info *info)
 {
 	int ret;
 
-	ret = tps23881_flash_sram_fw_part(client, fw_parity_name,
+	ret = tps23881_flash_sram_fw_part(client, info->fw_parity_name,
 					  tps23881_fw_parity_conf);
 	if (ret)
 		return ret;
 
-	ret = tps23881_flash_sram_fw_part(client, fw_sram_name,
+	ret = tps23881_flash_sram_fw_part(client, info->fw_sram_name,
 					  tps23881_fw_sram_conf);
 	if (ret)
 		return ret;
@@ -1412,6 +1431,7 @@ static int tps23881_setup_irq(struct tps23881_priv *priv, int irq)
 static int tps23881_i2c_probe(struct i2c_client *client)
 {
 	struct device *dev = &client->dev;
+	const struct tps23881_info *info;
 	struct tps23881_priv *priv;
 	struct gpio_desc *reset;
 	int ret;
@@ -1422,6 +1442,10 @@ static int tps23881_i2c_probe(struct i2c_client *client)
 		return -ENXIO;
 	}
 
+	info = device_get_match_data(dev);
+	if (!info)
+		return -EINVAL;
+
 	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
 	if (!priv)
 		return -ENOMEM;
@@ -1440,7 +1464,7 @@ static int tps23881_i2c_probe(struct i2c_client *client)
 		 * to Load TPS2388x SRAM and Parity Code over I2C" (Rev E))
 		 * indicates we should delay that programming by at least 50ms. So
 		 * we'll wait the entire 50ms here to ensure we're safe to go to the
-		 * SRAM loading proceedure.
+		 * SRAM loading procedure.
 		 */
 		msleep(50);
 	}
@@ -1449,20 +1473,26 @@ static int tps23881_i2c_probe(struct i2c_client *client)
 	if (ret < 0)
 		return ret;
 
-	if (FIELD_GET(TPS23881_REG_DEVID_MASK, ret) != TPS23881_DEVICE_ID) {
+	if (ret != info->dev_id) {
 		dev_err(dev, "Wrong device ID\n");
 		return -ENXIO;
 	}
 
-	ret = tps23881_flash_sram_fw(client);
-	if (ret < 0)
-		return ret;
+	if (info->fw_sram_name) {
+		ret = tps23881_flash_sram_fw(client, info);
+		if (ret < 0)
+			return ret;
+	}
 
 	ret = i2c_smbus_read_byte_data(client, TPS23881_REG_FWREV);
 	if (ret < 0)
 		return ret;
 
-	dev_info(&client->dev, "Firmware revision 0x%x\n", ret);
+	if (ret == 0xFF)
+		dev_warn(&client->dev, "Device entered safe mode\n");
+	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);
 
 static const struct of_device_id tps23881_of_match[] = {
-	{ .compatible = "ti,tps23881", },
+	{
+		.compatible = "ti,tps23881",
+		.data = &tps23881_info[TPS23881]
+	},
+	{
+		.compatible = "ti,tps23881b",
+		.data = &tps23881_info[TPS23881B]
+	},
 	{ },
 };
 MODULE_DEVICE_TABLE(of, tps23881_of_match);
-- 
2.43.0

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

Posted by Kory Maincent 2 months, 1 week ago

On Sat,  4 Oct 2025 20:03:51 +0200
Thomas Wismer <thomas@wismer.xyz> wrote:

> From: Thomas Wismer <thomas.wismer@scs.ch>
> 
> The TPS23881B device requires different firmware, but has a more recent ROM
> firmware. Since no updated firmware has been released yet, the firmware
> loading step must be skipped. The device runs from its ROM firmware.
> 
> Signed-off-by: Thomas Wismer <thomas.wismer@scs.ch>
> ---
>  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 */
> +	},

You are breaking Kyle's patch:
https://patchwork.kernel.org/project/netdevbpf/patch/20240731154152.4020668-1-kyle.swenson@est.tech/

You should check only the device id and not the silicon id.

Regards,
-- 
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

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

Posted by Thomas Wismer 2 months, 1 week ago

Am Mon, 6 Oct 2025 15:05:05 +0200
schrieb Kory Maincent <kory.maincent@bootlin.com>:

> On Sat,  4 Oct 2025 20:03:51 +0200
> Thomas Wismer <thomas@wismer.xyz> wrote:
> 
> > From: Thomas Wismer <thomas.wismer@scs.ch>
> > 
> > The TPS23881B device requires different firmware, but has a more
> > recent ROM firmware. Since no updated firmware has been released
> > yet, the firmware loading step must be skipped. The device runs
> > from its ROM firmware.
> > 
> > Signed-off-by: Thomas Wismer <thomas.wismer@scs.ch>
> > ---
> >  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 */
> > +	},  
> 
> You are breaking Kyle's patch:
> https://patchwork.kernel.org/project/netdevbpf/patch/20240731154152.4020668-1-kyle.swenson@est.tech/
> 
> You should check only the device id and not the silicon id.

On the TPS23881, the register "DEVICE ID" reads as 0x22 (Device ID number
DID = 0010b, silicon revision number SR = 0010b). On the TPS23881B, 0x24
(DID = 0010b, SR = 0100b) is returned. Both devices report the same
device ID number DID and can only be distinguished by their silicon
revision number SR.

Unfortunately, Kyle's assumption that the driver should work fine with
any silicon revision proved to be wrong. The TPS23881 firmware is not
compatible with the TPS23881B and must not be attempted to be loaded. As
of today, the TPS23881B must be operated using the ROM firmware.

> Regards,

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

Posted by Kory Maincent 2 months, 1 week ago

On Mon, 6 Oct 2025 23:23:18 +0200
Thomas Wismer <thomas@wismer.xyz> wrote:

> Am Mon, 6 Oct 2025 15:05:05 +0200
> schrieb Kory Maincent <kory.maincent@bootlin.com>:
> 
> > On Sat,  4 Oct 2025 20:03:51 +0200
> > Thomas Wismer <thomas@wismer.xyz> wrote:
> >   
> > > From: Thomas Wismer <thomas.wismer@scs.ch>
> > > 
> > > The TPS23881B device requires different firmware, but has a more
> > > recent ROM firmware. Since no updated firmware has been released
> > > yet, the firmware loading step must be skipped. The device runs
> > > from its ROM firmware.
> > > 
> > > Signed-off-by: Thomas Wismer <thomas.wismer@scs.ch>
> > > ---
> > >  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 */
> > > +	},    
> > 
> > You are breaking Kyle's patch:
> > https://patchwork.kernel.org/project/netdevbpf/patch/20240731154152.4020668-1-kyle.swenson@est.tech/
> > 
> > You should check only the device id and not the silicon id.  
> 
> On the TPS23881, the register "DEVICE ID" reads as 0x22 (Device ID number
> DID = 0010b, silicon revision number SR = 0010b). On the TPS23881B, 0x24
> (DID = 0010b, SR = 0100b) is returned. Both devices report the same
> device ID number DID and can only be distinguished by their silicon
> revision number SR.
> 
> Unfortunately, Kyle's assumption that the driver should work fine with
> any silicon revision proved to be wrong. The TPS23881 firmware is not
> compatible with the TPS23881B and must not be attempted to be loaded. As
> of today, the TPS23881B must be operated using the ROM firmware.

Indeed you are right, I misread the datasheet, on my head I thought it was the
device ID which changes between the TPS23881 and the TPS23881B.

Regards,
-- 
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

[PATCH 1/3] net: pse-pd: tps23881: Fix current measurement scaling
[PATCH 2/3] net: pse-pd: tps23881: Add support for TPS23881B
[PATCH 3/3] dt-bindings: pse-pd: ti,tps23881: Add TPS23881B