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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <aPtZ1jE7D7JI2wic@pengutronix.de>
Date: Fri, 24 Oct 2025 12:49:58 +0200
From: Oleksij Rempel <o.rempel@...gutronix.de>
To: Thomas Wismer <thomas@...mer.xyz>
Cc: Kory Maincent <kory.maincent@...tlin.com>,
	Andrew Lunn <andrew+netdev@...n.ch>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Rob Herring <robh@...nel.org>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>,
	Thomas Wismer <thomas.wismer@....ch>, netdev@...r.kernel.org,
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH net-next v2 1/2] net: pse-pd: tps23881: Add support for
 TPS23881B

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:
> From: Thomas Wismer <thomas.wismer@....ch>
> 
> 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 <thomas.wismer@....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 */

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).

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


> +	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 |