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

[Kory Maincent <kory.maincent@bootlin.com>]

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