[PATCH net-next v5] net: dsa: microchip: enable the SGMII port of the KSZ9897S

[PATCH net-next v5] net: dsa: microchip: enable the SGMII port of the KSZ9897S

[Tapio Reijonen]

The KSZ9897 has an SGMII/1000BASE-X capable port 7, but its chip_info
entry never set .sgmii_port. ksz_has_sgmii_port() is therefore false,
ksz9477_pcs_create() is never called and ksz9477_phylink_mac_select_pcs()
returns NULL for port 7, so a phy-mode = "sgmii" or "1000base-x" link on
that port cannot come up.

Only the KSZ9897S has that port; the KSZ9897R has a second RGMII port
instead. Both report chip ID 0x00989700, so the port cannot be enabled
from the chip ID alone, and enabling it unconditionally breaks the
KSZ9897R badly: ksz9477_setup() propagates the failure of
xpcs_create_pcs_mdiodev() on a part without an XPCS, so the switch as a
whole stops probing.

The variants do differ in the read-only bit 7 of the port 7 XMII control
0 register, which reads one on the KSZ9897S and zero on the KSZ9897R.
Compare the KSZ9897S data sheet DS00002394C section 5.2.4.1 with the
KSZ9897R data sheet DS00002330D section 5.2.3.1.

Read that bit in ksz_switch_register() and use it to select a KSZ9897S
chip_data entry, which is the KSZ9897 one plus .sgmii_port = 7, with
port 7 no longer marked as supporting MII, RMII and RGMII. On the
KSZ9897S that port is permanently an SGMII port and only its
differential pairs are brought out, so advertising the xMII modes on it
would let phylink accept a mode the driver never programs:
ksz_phylink_need_config() skips the XMII control registers for the SGMII
port. The KSZ9477, which also has SGMII on port 7, is described the same
way. See DS00002394C section 4.11.

KSZ9897S_CHIP_ID selects that chip_data entry and nothing else.
dev->chip_id keeps the KSZ9897 chip ID that both parts report, so no
other chip ID comparison has to know about the variant,
ksz_check_device_id() is unchanged, and a device tree naming the KSZ9897
keeps matching whichever part is fitted. No binding change is needed and
existing device trees keep working.

Tested on a KSZ9897S with a 1000BASE-LX fibre SFP on port 7: bit 7 of
the port 7 XMII control 0 register reads one, the switch is identified
as a KSZ9897S and the PCS is created. The port advertises
1000baseX/Full, clause 37 in-band autonegotiation completes against the
link partner, and the link comes up at 1Gbps/full duplex and passes
traffic.

Suggested-by: Nicolai Buchwitz <nb@tipi-net.de>
Suggested-by: Jerry Ray <Jerry.Ray@microchip.com>
Suggested-by: Tristram Ha <Tristram.Ha@microchip.com>
Signed-off-by: Tapio Reijonen <tapio.reijonen@vaisala.com>
---
Changes in v5:
- Keep the KSZ9897 chip ID in dev->chip_id and use KSZ9897S_CHIP_ID
  only to select the chip_data entry, as suggested by Tristram Ha.
  The variant is now resolved in ksz_switch_register() instead of
  ksz_switch_detect(), which drops the ksz_check_device_id() special
  case v4 needed, leaves both of those functions unchanged, and keeps
  KSZ9897S_CHIP_ID out of every other chip ID comparison in the
  driver.
- Link to v4: https://lore.kernel.org/r/20260903-ksz9897-sgmii-port-v4-1-538db2ffef2b@vaisala.com

Changes in v4:
- KSZ9897S: stop advertising MII, RMII and RGMII on port 7. On the S
  that port is permanently SGMII with only its differential pairs
  brought out (DS00002394C section 4.11), and
  ksz_phylink_need_config() skips the XMII control registers for the
  SGMII port, so phylink could accept an xMII mode that the driver
  never programs. The KSZ9477, which also has SGMII on port 7, is
  already described this way. v3 inherited the KSZ9897 values, which
  are correct only for the R, whose port 7 is a second RGMII port.
  Flagged by the netdev Sashiko review bot on v3 and then confirmed
  against the data sheet.
- The in-tree LAN9646 entry pairs .sgmii_port = 7 with the same xMII
  advertisement and looks wrong for the same reason; not touched here.
- Link to v3: https://lore.kernel.org/r/20260831-ksz9897-sgmii-port-v3-1-2376aa4edf11@vaisala.com

Changes in v3:
- Only enable the SGMII port on the KSZ9897S. v2 enabled it for every
  KSZ9897, which makes a KSZ9897R fail to probe at all, because
  ksz9477_setup() propagates the xpcs_create_pcs_mdiodev() failure on a
  part that has no XPCS. Reported by Nicolai Buchwitz.
- Tell the two variants apart at run time in ksz_switch_detect(), from
  the read-only bit 7 of the port 7 XMII control 0 register, and select
  a KSZ9897S chip_info entry, next to the existing KSZ8765 and KSZ9563
  variant detection. Register suggested by Nicolai Buchwitz and
  confirmed by Jerry Ray against both data sheets.
- Accept a detected KSZ9897S for a device tree naming the KSZ9897, so
  no binding change is needed and existing device trees keep working.
- Tested on a KSZ9897S with a 1000BASE-LX fibre SFP on port 7.
- Link to v2: https://lore.kernel.org/r/20260811-ksz9897-sgmii-port-v2-1-04ae287a0b27@vaisala.com

Changes in v2:
- Resend only, no functional change: v1 was dropped when the netdev
  patch queue overflowed.
- Rebased onto current net-next.
- Link to v1: https://lore.kernel.org/r/20260703-ksz9897-sgmii-port-v1-1-387e7632dd0b@vaisala.com
---
 drivers/net/dsa/microchip/ksz_common.c      | 61 ++++++++++++++++++++++++++++-
 drivers/net/dsa/microchip/ksz_common.h      |  5 +++
 include/linux/platform_data/microchip-ksz.h |  1 +
 3 files changed, 66 insertions(+), 1 deletion(-)

diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index 1a9d6f83a0232830ac4996ae5f53b8bde0b3747d..81798a9fa671dbb490cef89e0b18398242879068 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -1486,6 +1486,41 @@ const struct ksz_chip_data ksz_switch_chips[] = {
 		.gbit_capable	= {true, true, true, true, true, true, true},
 	},
 
+	[KSZ9897S] = {
+		.chip_id = KSZ9897S_CHIP_ID,
+		.dev_name = "KSZ9897S",
+		.num_vlans = 4096,
+		.num_alus = 4096,
+		.num_statics = 16,
+		.cpu_ports = 0x7F,	/* can be configured as cpu port */
+		.port_cnt = 7,		/* total physical port count */
+		.port_nirqs = 2,
+		.num_tx_queues = 4,
+		.num_ipms = 8,
+		.ops = &ksz9477_dev_ops,
+		.switch_ops = &ksz9477_switch_ops,
+		.phylink_mac_ops = &ksz9477_phylink_mac_ops,
+		.phy_errata_9477 = true,
+		.mib_names = ksz9477_mib_names,
+		.mib_cnt = ARRAY_SIZE(ksz9477_mib_names),
+		.reg_mib_cnt = MIB_COUNTER_NUM,
+		.regs = ksz9477_regs,
+		.masks = ksz9477_masks,
+		.shifts = ksz9477_shifts,
+		.xmii_ctrl0 = ksz9477_xmii_ctrl0,
+		.xmii_ctrl1 = ksz9477_xmii_ctrl1,
+		.supports_mii	= {false, false, false, false,
+				   false, true, false},
+		.supports_rmii	= {false, false, false, false,
+				   false, true, false},
+		.supports_rgmii = {false, false, false, false,
+				   false, true, false},
+		.internal_phy	= {true, true, true, true,
+				   true, false, false},
+		.gbit_capable	= {true, true, true, true, true, true, true},
+		.sgmii_port = 7,
+	},
+
 	[KSZ9893] = {
 		.chip_id = KSZ9893_CHIP_ID,
 		.dev_name = "KSZ9893",
@@ -3897,6 +3932,7 @@ int ksz_switch_register(struct ksz_device *dev)
 	struct device_node *ports;
 	phy_interface_t interface;
 	unsigned int port_num;
+	u32 lookup_chip_id;
 	int ret;
 	int i;
 
@@ -3933,7 +3969,30 @@ int ksz_switch_register(struct ksz_device *dev)
 	if (ret)
 		return ret;
 
-	info = ksz_lookup_info(dev->chip_id);
+	lookup_chip_id = dev->chip_id;
+
+	/* The KSZ9897S and the KSZ9897R report the same chip ID and differ
+	 * only in chip_data: only the S has the SGMII port 7, the R has a
+	 * second RGMII port instead. Bit 7 of the port 7 XMII control 0
+	 * register is read-only and tells them apart; it reads one on the S.
+	 * Compare the KSZ9897S data sheet DS00002394C section 5.2.4.1 with
+	 * the KSZ9897R data sheet DS00002330D section 5.2.3.1.
+	 *
+	 * Only the chip_data entry differs, so dev->chip_id keeps the shared
+	 * chip ID and nothing else has to know about the variant.
+	 */
+	if (dev->chip_id == KSZ9897_CHIP_ID) {
+		u8 val;
+
+		ret = ksz_read8(dev, KSZ9897_REG_PORT7_XMII_CTRL_0, &val);
+		if (ret)
+			return ret;
+
+		if (val & KSZ9897_PORT7_SGMII_SEL)
+			lookup_chip_id = KSZ9897S_CHIP_ID;
+	}
+
+	info = ksz_lookup_info(lookup_chip_id);
 	if (!info)
 		return -ENODEV;
 
diff --git a/drivers/net/dsa/microchip/ksz_common.h b/drivers/net/dsa/microchip/ksz_common.h
index cbe98494578c371ee22b80f9508c3b35b8d6291a..0cff33df06a4b8ca585d94f9fe437ad62bcd82dc 100644
--- a/drivers/net/dsa/microchip/ksz_common.h
+++ b/drivers/net/dsa/microchip/ksz_common.h
@@ -284,6 +284,7 @@ enum ksz_model {
 	KSZ9477,
 	KSZ9896,
 	KSZ9897,
+	KSZ9897S,
 	KSZ9893,
 	KSZ9563,
 	KSZ9567,
@@ -874,6 +875,10 @@ static inline bool ksz_is_sgmii_port(struct ksz_device *dev, int port)
 #define SKU_ID_KSZ8563			0x3c
 #define SKU_ID_KSZ9563			0x1c
 
+/* KSZ9897 specific register: port 7 XMII control 0 */
+#define KSZ9897_REG_PORT7_XMII_CTRL_0	0x7300
+#define KSZ9897_PORT7_SGMII_SEL		BIT(7)
+
 /* Driver set switch broadcast storm protection at 10% rate. */
 #define BROADCAST_STORM_PROT_RATE	10
 
diff --git a/include/linux/platform_data/microchip-ksz.h b/include/linux/platform_data/microchip-ksz.h
index 028781ad40593248bae1b501e40ce31c5e0c69ca..fe8df1774289c401445ef308c8d3656e1e7c25b6 100644
--- a/include/linux/platform_data/microchip-ksz.h
+++ b/include/linux/platform_data/microchip-ksz.h
@@ -34,6 +34,7 @@ enum ksz_chip_id {
 	KSZ9477_CHIP_ID = 0x00947700,
 	KSZ9896_CHIP_ID = 0x00989600,
 	KSZ9897_CHIP_ID = 0x00989700,
+	KSZ9897S_CHIP_ID = 0x00989701,
 	KSZ9893_CHIP_ID = 0x00989300,
 	KSZ9563_CHIP_ID = 0x00956300,
 	KSZ8567_CHIP_ID = 0x00856700,

---
base-commit: 1bb784eb6e38fd73143f021608e4ef3095d0c0d7
change-id: 20260703-ksz9897-sgmii-port-48ddb100e015

Best regards,
-- 
Tapio Reijonen <tapio.reijonen@vaisala.com>

Re: [PATCH net-next v5] net: dsa: microchip: enable the SGMII port of the KSZ9897S

[Linus Walleij]

On Fri, Sep 4, 2026 at 8:03 AM Tapio Reijonen
<tapio.reijonen@vaisala.com> wrote:

> The KSZ9897 has an SGMII/1000BASE-X capable port 7, but its chip_info
> entry never set .sgmii_port. ksz_has_sgmii_port() is therefore false,
> ksz9477_pcs_create() is never called and ksz9477_phylink_mac_select_pcs()
> returns NULL for port 7, so a phy-mode = "sgmii" or "1000base-x" link on
> that port cannot come up.
>
> Only the KSZ9897S has that port; the KSZ9897R has a second RGMII port
> instead. Both report chip ID 0x00989700, so the port cannot be enabled
> from the chip ID alone, and enabling it unconditionally breaks the
> KSZ9897R badly: ksz9477_setup() propagates the failure of
> xpcs_create_pcs_mdiodev() on a part without an XPCS, so the switch as a
> whole stops probing.
>
> The variants do differ in the read-only bit 7 of the port 7 XMII control
> 0 register, which reads one on the KSZ9897S and zero on the KSZ9897R.
> Compare the KSZ9897S data sheet DS00002394C section 5.2.4.1 with the
> KSZ9897R data sheet DS00002330D section 5.2.3.1.
>
> Read that bit in ksz_switch_register() and use it to select a KSZ9897S
> chip_data entry, which is the KSZ9897 one plus .sgmii_port = 7, with
> port 7 no longer marked as supporting MII, RMII and RGMII. On the
> KSZ9897S that port is permanently an SGMII port and only its
> differential pairs are brought out, so advertising the xMII modes on it
> would let phylink accept a mode the driver never programs:
> ksz_phylink_need_config() skips the XMII control registers for the SGMII
> port. The KSZ9477, which also has SGMII on port 7, is described the same
> way. See DS00002394C section 4.11.
>
> KSZ9897S_CHIP_ID selects that chip_data entry and nothing else.
> dev->chip_id keeps the KSZ9897 chip ID that both parts report, so no
> other chip ID comparison has to know about the variant,
> ksz_check_device_id() is unchanged, and a device tree naming the KSZ9897
> keeps matching whichever part is fitted. No binding change is needed and
> existing device trees keep working.
>
> Tested on a KSZ9897S with a 1000BASE-LX fibre SFP on port 7: bit 7 of
> the port 7 XMII control 0 register reads one, the switch is identified
> as a KSZ9897S and the PCS is created. The port advertises
> 1000baseX/Full, clause 37 in-band autonegotiation completes against the
> link partner, and the link comes up at 1Gbps/full duplex and passes
> traffic.
>
> Suggested-by: Nicolai Buchwitz <nb@tipi-net.de>
> Suggested-by: Jerry Ray <Jerry.Ray@microchip.com>
> Suggested-by: Tristram Ha <Tristram.Ha@microchip.com>
> Signed-off-by: Tapio Reijonen <tapio.reijonen@vaisala.com>

I think this approach looks good.
Reviewed-by: Linus Walleij <linusw@kernel.org>

Yours,
Linus Walleij

Re: [PATCH net-next v5] net: dsa: microchip: enable the SGMII port of the KSZ9897S

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider:

Critical: 0 · High: 0 · Medium: 1 · Low: 0

- [Medium] Public interface defect: `KSZ9897S_CHIP_ID` (0x00989701) is
  added to `enum ksz_chip_id` in include/linux/platform_data/microchip-…

--

net: dsa: microchip: enable the SGMII port of the KSZ9897S

This adds a KSZ9897S chip_data entry with .sgmii_port = 7 and selects it in
ksz_switch_register() by reading the read-only bit 7 of the port 7 XMII
control 0 register, while leaving dev->chip_id at the shared KSZ9897 value.
A new KSZ9897S_CHIP_ID is added to enum ksz_chip_id to tag that entry.

> diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
> index 1a9d6f83a0232..81798a9fa671d 100644
> --- a/drivers/net/dsa/microchip/ksz_common.c
> +++ b/drivers/net/dsa/microchip/ksz_common.c
> @@ -1486,6 +1486,41 @@ const struct ksz_chip_data ksz_switch_chips[] = {
>  		.gbit_capable	= {true, true, true, true, true, true, true},
>  	},
>  
> +	[KSZ9897S] = {
> +		.chip_id = KSZ9897S_CHIP_ID,
> +		.dev_name = "KSZ9897S",

[ ... ]

> +		.gbit_capable	= {true, true, true, true, true, true, true},
> +		.sgmii_port = 7,
> +	},
> +
>  	[KSZ9893] = {
>  		.chip_id = KSZ9893_CHIP_ID,
>  		.dev_name = "KSZ9893",

[ ... ]

> @@ -3933,7 +3969,30 @@ int ksz_switch_register(struct ksz_device *dev)
>  	if (ret)
>  		return ret;
>  
> -	info = ksz_lookup_info(dev->chip_id);
> +	lookup_chip_id = dev->chip_id;
> +

[ ... ]

> +	 * Only the chip_data entry differs, so dev->chip_id keeps the shared
> +	 * chip ID and nothing else has to know about the variant.
> +	 */
> +	if (dev->chip_id == KSZ9897_CHIP_ID) {
> +		u8 val;
> +
> +		ret = ksz_read8(dev, KSZ9897_REG_PORT7_XMII_CTRL_0, &val);
> +		if (ret)
> +			return ret;
> +
> +		if (val & KSZ9897_PORT7_SGMII_SEL)
> +			lookup_chip_id = KSZ9897S_CHIP_ID;
> +	}
> +
> +	info = ksz_lookup_info(lookup_chip_id);
>  	if (!info)
>  		return -ENODEV;
>  

> diff --git a/include/linux/platform_data/microchip-ksz.h b/include/linux/platform_data/microchip-ksz.h
> index 028781ad40593..fe8df1774289c 100644
> --- a/include/linux/platform_data/microchip-ksz.h
> +++ b/include/linux/platform_data/microchip-ksz.h
> @@ -34,6 +34,7 @@ enum ksz_chip_id {
>  	KSZ9477_CHIP_ID = 0x00947700,
>  	KSZ9896_CHIP_ID = 0x00989600,
>  	KSZ9897_CHIP_ID = 0x00989700,
> +	KSZ9897S_CHIP_ID = 0x00989701,
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

[Severity: Medium]
Is enum ksz_chip_id in include/linux/platform_data/microchip-ksz.h the right
place for this value, given that no code path can ever set dev->chip_id to
0x00989701?

ksz_switch_detect() masks off the revision byte and stores only the fixed
IDs, so a KSZ9897S always ends up with dev->chip_id == KSZ9897_CHIP_ID,
which the commit message describes as intentional:

    dev->chip_id keeps the KSZ9897 chip ID that both parts report

Because enum ksz_chip_id is the platform-data selection interface, a board
that sets ksz_platform_data.chip_id = KSZ9897S_CHIP_ID now reaches
ksz_check_device_id() with an expected ID that resolves through
ksz_lookup_info() but never matches the detected one:

drivers/net/dsa/microchip/ksz_common.c:ksz_check_device_id() {
	if (dev->pdata) {
		expected_chip_id = dev->pdata->chip_id;
		expected_chip_data = ksz_lookup_info(expected_chip_id);
		if (WARN_ON(!expected_chip_data))
			return -ENODEV;
	...
	if (expected_chip_id != dev->chip_id) {
		dev_err(dev->dev,
			"Device tree specifies chip %s but found %s, please fix it!\n",
			expected_chip_data->dev_name, dev->info->dev_name);
		return -ENODEV;
	}
}

Would such a board fail to probe with the confusing message "Device tree
specifies chip KSZ9897S but found KSZ9897, please fix it!"?

There is also the invariant question: every other entry in
ksz_switch_chips[] has a .chip_id equal to an ID the hardware can actually
report, and that value is consumed as a real chip ID elsewhere, for example
via dev->info->chip_id in lan937x_main.c. The new entry uses .chip_id purely
as a lookup key for ksz_lookup_info(lookup_chip_id) in
ksz_switch_register().

Would a private enum ksz_model index, or a bool in struct ksz_chip_data,
express the S/R variant without publishing an ID in the shared platform-data
header that no caller can use?

>  	KSZ9893_CHIP_ID = 0x00989300,
>  	KSZ9563_CHIP_ID = 0x00956300,
>  	KSZ8567_CHIP_ID = 0x00856700,

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260904-ksz9897-sgmii-port-v5-1-9bd4a0381aa8%40vaisala.com

Re: [PATCH net-next v5] net: dsa: microchip: enable the SGMII port of the KSZ9897S

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Fri, 04 Sep 2026 06:03:00 +0000 you wrote:
> The KSZ9897 has an SGMII/1000BASE-X capable port 7, but its chip_info
> entry never set .sgmii_port. ksz_has_sgmii_port() is therefore false,
> ksz9477_pcs_create() is never called and ksz9477_phylink_mac_select_pcs()
> returns NULL for port 7, so a phy-mode = "sgmii" or "1000base-x" link on
> that port cannot come up.
> 
> Only the KSZ9897S has that port; the KSZ9897R has a second RGMII port
> instead. Both report chip ID 0x00989700, so the port cannot be enabled
> from the chip ID alone, and enabling it unconditionally breaks the
> KSZ9897R badly: ksz9477_setup() propagates the failure of
> xpcs_create_pcs_mdiodev() on a part without an XPCS, so the switch as a
> whole stops probing.
> 
> [...]

Here is the summary with links:
  - [net-next,v5] net: dsa: microchip: enable the SGMII port of the KSZ9897S
    https://git.kernel.org/netdev/net-next/c/3037c2f6a9de

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html