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

[Thomas Wismer <thomas@wismer.xyz>]

Hi Oleksij,

Many thanks for the review.

On Fri, Oct 24, 2025 at 12:49:58 +0200, Oleksij Rempel wrote:
> > +		/* 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.
[...]

Good to know that the IEEE 802.3-2022 standard is available free of charge.
I'm happy to adopt your accurate wording.

> [...]
> > @@ -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.

The datasheets (TPS23881 from May 2023 / TPS23881B from May 2025) state that,
regarding entering "safe mode", "any channels that are currently powered will
remain powered, but the majority of the operation will be disabled". Because the
rest of the driver code assumes the device to be in a functional state, bailing
out here makes perfect sense.

> > static const struct i2c_device_id tps23881_id[] = {
> >  	{ "tps23881" },
> > 	{ }
> > };
> > MODULE_DEVICE_TABLE(i2c, tps23881_id);
> 
> I guess tps23881_id should be updated too.

Agreed. I will fix this.

Best regards,
Thomas