[PATCH net 0/2] net: pse-pd: si3474: two driver fixes

[PATCH net 0/2] net: pse-pd: si3474: two driver fixes

[Piotr Kubik]

Two fixes for the Si3474 PSE driver, found while preparing 2-pair and
semi-auto support for it.

Patch 1 stops si3474_pi_enable() from clearing detection and
classification of the other PI in the same quad. Without it, a PD on
that PI gets no power after a later plug-in.

Patch 2 rejects a channel number of 8 and a 4-pair PI whose channels
are not one channel pair of a quad.

Both were tested on an SDG-9000 board (MediaTek MT7988A, two Si3474)
with a Linux 6.18 kernel that carries the same patches, and build-tested
on the net tree.

Carlo Szelinsky's "net: pse-pd" series [1] also touches si3474.c, in
si3474_i2c_probe(). The two series change different hunks, and his
patch 9 applies on top of this series.

[1] https://lore.kernel.org/all/20261018082314.48950-1-github@szelinsky.de/

Piotr Kubik (2):
  net: pse-pd: si3474: keep detection enabled on the other PIs
  net: pse-pd: si3474: validate the channels of a PI

 drivers/net/pse-pd/si3474.c | 23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

-- 
2.55.0

[PATCH net 1/2] net: pse-pd: si3474: keep detection enabled on the other PIs

[Piotr Kubik]

DETECT_CLASS_ENABLE holds the detection and classification bits of
all four channels of a quad. si3474_pi_enable() wrote the register
with the bits of its own PI only, so it cleared them for the other PI
of the quad. In AUTO mode that PI keeps its admin state and a PD that
already has power keeps it, but the port runs no new detection: a PD
plugged in later, or one waiting for detection when the other PI is
enabled, gets no power.

Read the register and set the PI bits, as the PORT_MODE update above
already does.

Discovered by an AI code review agent. Confirmed on an SDG-9000 board
(MediaTek MT7988A, two Si3474 in AUTO pin mode) with Linux 6.18:
with both PIs of one quad disabled, enabling PI A and then PI B left
DETECT_CLASS_ENABLE with the bits of PI B only, and a PD on PI A never
got power. No kernel change was needed to reproduce it.

Tested on the same board with the same sequence in both orders:
DETECT_CLASS_ENABLE keeps the bits of both PIs, and the PD gets power.
No kernel selftest covers PSE hardware.

Fixes: a2317231df4b ("net: pse-pd: Add Si3474 PSE controller driver")
Signed-off-by: Piotr Kubik <piotr@kubik.pl>
---
 drivers/net/pse-pd/si3474.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/net/pse-pd/si3474.c b/drivers/net/pse-pd/si3474.c
index 1845b9c51cf7..99e96f6bdefc 100644
--- a/drivers/net/pse-pd/si3474.c
+++ b/drivers/net/pse-pd/si3474.c
@@ -312,8 +312,13 @@ static int si3474_pi_enable(struct pse_controller_dev *pcdev, int id)
 	/* DETECT_CLASS_ENABLE must be set when using AUTO mode,
 	 * otherwise PI does not power up - datasheet section 2.10.2
 	 */
-	val = CHAN_BIT(chan0) | CHAN_UPPER_BIT(chan0) |
-	      CHAN_BIT(chan1) | CHAN_UPPER_BIT(chan1);
+	ret = i2c_smbus_read_byte_data(client, DETECT_CLASS_ENABLE_REG);
+	if (ret < 0)
+		return ret;
+
+	val = (u8)ret;
+	val |= CHAN_BIT(chan0) | CHAN_UPPER_BIT(chan0) |
+	       CHAN_BIT(chan1) | CHAN_UPPER_BIT(chan1);
 
 	ret = i2c_smbus_write_byte_data(client, DETECT_CLASS_ENABLE_REG, val);
 	if (ret)
-- 
2.55.0

Re: [PATCH net 1/2] net: pse-pd: si3474: keep detection enabled on the other PIs

[Kory Maincent]

On 10/19/26 18:24, Piotr Kubik wrote:
> DETECT_CLASS_ENABLE holds the detection and classification bits of
> all four channels of a quad. si3474_pi_enable() wrote the register
> with the bits of its own PI only, so it cleared them for the other PI
> of the quad. In AUTO mode that PI keeps its admin state and a PD that
> already has power keeps it, but the port runs no new detection: a PD
> plugged in later, or one waiting for detection when the other PI is
> enabled, gets no power.
> 
> Read the register and set the PI bits, as the PORT_MODE update above
> already does.
> 
> Discovered by an AI code review agent. Confirmed on an SDG-9000 board
> (MediaTek MT7988A, two Si3474 in AUTO pin mode) with Linux 6.18:
> with both PIs of one quad disabled, enabling PI A and then PI B left
> DETECT_CLASS_ENABLE with the bits of PI B only, and a PD on PI A never
> got power. No kernel change was needed to reproduce it.
> 
> Tested on the same board with the same sequence in both orders:
> DETECT_CLASS_ENABLE keeps the bits of both PIs, and the PD gets power.
> No kernel selftest covers PSE hardware.

Reviewed-by: Kory Maincent <kory.maincent@bootlin.com>

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

[PATCH net 2/2] net: pse-pd: si3474: validate the channels of a PI

[Piotr Kubik]

si3474_get_of_channels() accepted channel 8, one past the last channel.
The driver then accessed channel 4 instead: quad 1, channel index 0.

It also accepted a 4-pair PI whose two channels are not a channel pair
of one quad. The chip forms a 4-pair port only from channels 0+1 or 2+3
of a quad (2P/4P mode bit in POWER_ALLOCATION, PORT_REMAP default), and
the driver sends every register access of a PI to the quad of its first
channel, so such a PI enabled and measured the wrong channels.

Reject both at probe.

Discovered by an AI code review agent; no shipped device tree triggers
it. Validated and tested on an SDG-9000 board (MediaTek MT7988A, two
Si3474) with Linux 6.18 and a modified device tree: one chip with a
channel at reg 8, the other with a 4-pair PI on channels 1 and 2. The
driver now rejects both at probe, and the unmodified device tree still
probes all eight PIs. No kernel selftest covers PSE hardware.

Fixes: a2317231df4b ("net: pse-pd: Add Si3474 PSE controller driver")
Signed-off-by: Piotr Kubik <piotr@kubik.pl>
---
 drivers/net/pse-pd/si3474.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/net/pse-pd/si3474.c b/drivers/net/pse-pd/si3474.c
index 99e96f6bdefc..2f18714c95f1 100644
--- a/drivers/net/pse-pd/si3474.c
+++ b/drivers/net/pse-pd/si3474.c
@@ -250,9 +250,9 @@ static int si3474_get_of_channels(struct si3474_priv *priv)
 					"Failed to read channel reg property\n");
 				return ret;
 			}
-			if (chan_id > SI3474_MAX_CHANS) {
+			if (chan_id >= SI3474_MAX_CHANS) {
 				dev_err(&priv->client[0]->dev,
-					"Incorrect channel number: %d\n", chan_id);
+					"Incorrect channel number: %u\n", chan_id);
 				return -EINVAL;
 			}
 
@@ -267,6 +267,16 @@ static int si3474_get_of_channels(struct si3474_priv *priv)
 				pi->np);
 			return -EINVAL;
 		}
+
+		/* The 2P/4P mode bit pairs channels 0+1 and 2+3 of a quad */
+		if (pairset_found &&
+		    (priv->pi[pi_no].chan[0] ^ 1) != priv->pi[pi_no].chan[1]) {
+			dev_err(&priv->client[0]->dev,
+				"PI %pOF: channels %u and %u are not a channel pair\n",
+				pi->np, priv->pi[pi_no].chan[0],
+				priv->pi[pi_no].chan[1]);
+			return -EINVAL;
+		}
 	}
 
 	return 0;
-- 
2.55.0

Re: [PATCH net 2/2] net: pse-pd: si3474: validate the channels of a PI

[Kory Maincent]

On 10/19/26 18:24, Piotr Kubik wrote:
> si3474_get_of_channels() accepted channel 8, one past the last channel.
> The driver then accessed channel 4 instead: quad 1, channel index 0.
> 
> It also accepted a 4-pair PI whose two channels are not a channel pair
> of one quad. The chip forms a 4-pair port only from channels 0+1 or 2+3
> of a quad (2P/4P mode bit in POWER_ALLOCATION, PORT_REMAP default), and
> the driver sends every register access of a PI to the quad of its first
> channel, so such a PI enabled and measured the wrong channels.
> 
> Reject both at probe.
> 
> Discovered by an AI code review agent; no shipped device tree triggers
> it. Validated and tested on an SDG-9000 board (MediaTek MT7988A, two
> Si3474) with Linux 6.18 and a modified device tree: one chip with a
> channel at reg 8, the other with a 4-pair PI on channels 1 and 2. The
> driver now rejects both at probe, and the unmodified device tree still
> probes all eight PIs. No kernel selftest covers PSE hardware.

Reviewed-by: Kory Maincent <kory.maincent@bootlin.com>

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