[PATCH net-next v2 0/4] net: dsa: ks8995: Post-move fixes

[PATCH net-next v2 0/4] net: dsa: ks8995: Post-move fixes

[Linus Walleij]

This fixes some glearing issues in the Micrel KS8995 driver
pointed out by Vladimir.

This patch series implements some required functionality
and strips the driver down to just KS8995 deeming the other
"micrel" variants to be actually handled by the Microchip
KSZ driver.

If the KS8995 should actually *also* be managed by the Microchip
driver and this driver deleted remains to be seen. It is clearly
the origin chip for that hardware: it is very close to the
"KSZ8 family" but there are differences.

It definitely has a different custom tag format for proper DSA
tagging, but I have implemented that: I now have to figure out
whether to do that on top of this driver or the KSZ driver before
continuing.

In the meantime, this patch series makes the situation better.

Signed-off-by: Linus Walleij <linusw@kernel.org>
---
Changes in v2:
- Do port_bitmask in another way and fix a bug where BIT(port_bitmask)
  was used instead of just port_bitmask.
- Link to v1: https://lore.kernel.org/r/20260118-ks8995-fixups-v1-0-10a493f0339d@kernel.org

---
Linus Walleij (4):
      net: dsa: ks8995: Add shutdown callback
      net: dsa: ks8955: Delete KSZ8864 and KSZ8795 support
      net: dsa: ks8995: Add stub bridge join/leave
      net: dsa: ks8995: Implement port isolation

 drivers/net/dsa/ks8995.c | 317 +++++++++++++++++++++++++++--------------------
 1 file changed, 185 insertions(+), 132 deletions(-)
---
base-commit: 8f0b4cce4481fb22653697cced8d0d04027cb1e8
change-id: 20260118-ks8995-fixups-84f25ac3f407

Best regards,
-- 
Linus Walleij <linusw@kernel.org>

[PATCH net-next v2 1/4] net: dsa: ks8995: Add shutdown callback

[Linus Walleij]

The DSA framework requires that dsa_switch_shutdown() be
called when the driver is shut down.

Fixes: a7fe8b266f65 ("net: dsa: ks8995: Add basic switch set-up")
Reported-by: Vladimir Oltean <olteanv@gmail.com>
Signed-off-by: Linus Walleij <linusw@kernel.org>
---
 drivers/net/dsa/ks8995.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/net/dsa/ks8995.c b/drivers/net/dsa/ks8995.c
index 77d8b842693c..ff01097601ec 100644
--- a/drivers/net/dsa/ks8995.c
+++ b/drivers/net/dsa/ks8995.c
@@ -838,6 +838,13 @@ static void ks8995_remove(struct spi_device *spi)
 	gpiod_set_value_cansleep(ks->reset_gpio, 1);
 }
 
+static void ks8995_shutdown(struct spi_device *spi)
+{
+	struct ks8995_switch *ks = spi_get_drvdata(spi);
+
+	dsa_switch_shutdown(ks->ds);
+}
+
 /* ------------------------------------------------------------------------ */
 static struct spi_driver ks8995_driver = {
 	.driver = {
@@ -846,6 +853,7 @@ static struct spi_driver ks8995_driver = {
 	},
 	.probe	  = ks8995_probe,
 	.remove	  = ks8995_remove,
+	.shutdown = ks8995_shutdown,
 	.id_table = ks8995_id,
 };
 

-- 
2.52.0

Re: [PATCH net-next v2 1/4] net: dsa: ks8995: Add shutdown callback

[Vladimir Oltean]

On Mon, Jan 19, 2026 at 03:30:05PM +0100, Linus Walleij wrote:
> The DSA framework requires that dsa_switch_shutdown() be
> called when the driver is shut down.
> 
> Fixes: a7fe8b266f65 ("net: dsa: ks8995: Add basic switch set-up")

$ git tag --contains a7fe8b266f65
v6.18

We are in the RC stage for v6.19, so this is 'net' material, not
'net-next', the patch has already made it into a released kernel.

Re: [PATCH net-next v2 1/4] net: dsa: ks8995: Add shutdown callback

[Jakub Kicinski]

On Mon, 19 Jan 2026 17:38:50 +0200 Vladimir Oltean wrote:
> On Mon, Jan 19, 2026 at 03:30:05PM +0100, Linus Walleij wrote:
> > The DSA framework requires that dsa_switch_shutdown() be
> > called when the driver is shut down.
> > 
> > Fixes: a7fe8b266f65 ("net: dsa: ks8995: Add basic switch set-up")  
> 
> $ git tag --contains a7fe8b266f65
> v6.18
> 
> We are in the RC stage for v6.19, so this is 'net' material, not
> 'net-next', the patch has already made it into a released kernel.

The AI code review points out that the DSA docs/you suggest mutual
exclusion with .remove. Do we also want this to be addressed?

Re: [PATCH net-next v2 1/4] net: dsa: ks8995: Add shutdown callback

[Vladimir Oltean]

On Mon, Jan 19, 2026 at 08:33:41AM -0800, Jakub Kicinski wrote:
> On Mon, 19 Jan 2026 17:38:50 +0200 Vladimir Oltean wrote:
> > On Mon, Jan 19, 2026 at 03:30:05PM +0100, Linus Walleij wrote:
> > > The DSA framework requires that dsa_switch_shutdown() be
> > > called when the driver is shut down.
> > > 
> > > Fixes: a7fe8b266f65 ("net: dsa: ks8995: Add basic switch set-up")  
> > 
> > $ git tag --contains a7fe8b266f65
> > v6.18
> > 
> > We are in the RC stage for v6.19, so this is 'net' material, not
> > 'net-next', the patch has already made it into a released kernel.
> 
> The AI code review points out that the DSA docs/you suggest mutual
> exclusion with .remove. Do we also want this to be addressed?

Oh, yes, I didn't get that far into actual code review.

Re: [PATCH net-next v2 1/4] net: dsa: ks8995: Add shutdown callback

[Linus Walleij]

On Mon, Jan 19, 2026 at 5:33 PM Jakub Kicinski <kuba@kernel.org> wrote:
> On Mon, 19 Jan 2026 17:38:50 +0200 Vladimir Oltean wrote:
> > On Mon, Jan 19, 2026 at 03:30:05PM +0100, Linus Walleij wrote:
> > > The DSA framework requires that dsa_switch_shutdown() be
> > > called when the driver is shut down.
> > >
> > > Fixes: a7fe8b266f65 ("net: dsa: ks8995: Add basic switch set-up")
> >
> > $ git tag --contains a7fe8b266f65
> > v6.18
> >
> > We are in the RC stage for v6.19, so this is 'net' material, not
> > 'net-next', the patch has already made it into a released kernel.
>
> The AI code review points out that the DSA docs/you suggest mutual
> exclusion with .remove. Do we also want this to be addressed?

That's pretty impressive. I need to learn how to
use this approach for my subsystems.

(Also fixing my patches.)

Yours,
Linus Walleij

[PATCH net-next v2 2/4] net: dsa: ks8955: Delete KSZ8864 and KSZ8795 support

[Linus Walleij]

After studying the datasheets for a bit, I can conclude that
the register maps for the two KSZ variants explicitly said to
be supported by this driver are fully supported by the newer
Micrel KSZ driver, including full VLAN support and a different
custom tag than what the KS8995 is using.

Delete this support, users should be using the KSZ driver
CONFIG_NET_DSA_MICROCHIP_KSZ_SPI and any new device trees should
use:
micrel,ksz8864 -> microchip,ksz8864
micrel,ksz8795 -> microchip,ksz8795

Apparently Microchip acquired Micrel at some point and this
created the confusion.

Fixes: a7fe8b266f65 ("net: dsa: ks8995: Add basic switch set-up")
Reported-by: Vladimir Oltean <olteanv@gmail.com>
Signed-off-by: Linus Walleij <linusw@kernel.org>
---
 drivers/net/dsa/ks8995.c | 160 +++++++++--------------------------------------
 1 file changed, 28 insertions(+), 132 deletions(-)

diff --git a/drivers/net/dsa/ks8995.c b/drivers/net/dsa/ks8995.c
index ff01097601ec..5ad62fa4e52c 100644
--- a/drivers/net/dsa/ks8995.c
+++ b/drivers/net/dsa/ks8995.c
@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0
 /*
- * SPI driver for Micrel/Kendin KS8995M and KSZ8864RMN ethernet switches
+ * SPI driver for Micrel/Kendin KS8995M ethernet switch.
  *
  * Copyright (C) 2008 Gabor Juhos <juhosg at openwrt.org>
  * Copyright (C) 2025 Linus Walleij <linus.walleij@linaro.org>
@@ -114,11 +114,7 @@
 #define KS8995_REG_IAD1		0x76    /* Indirect Access Data 1 */
 #define KS8995_REG_IAD0		0x77    /* Indirect Access Data 0 */
 
-#define KSZ8864_REG_ID1		0xfe	/* Chip ID in bit 7 */
-
 #define KS8995_REGS_SIZE	0x80
-#define KSZ8864_REGS_SIZE	0x100
-#define KSZ8795_REGS_SIZE	0x100
 
 #define ID1_CHIPID_M		0xf
 #define ID1_CHIPID_S		4
@@ -127,11 +123,8 @@
 #define ID1_START_SW		1	/* start the switch */
 
 #define FAMILY_KS8995		0x95
-#define FAMILY_KSZ8795		0x87
 #define CHIPID_M		0
 #define KS8995_CHIP_ID		0x00
-#define KSZ8864_CHIP_ID		0x01
-#define KSZ8795_CHIP_ID		0x09
 
 #define KS8995_CMD_WRITE	0x02U
 #define KS8995_CMD_READ		0x03U
@@ -140,49 +133,6 @@
 #define KS8995_NUM_PORTS	5 /* 5 ports including the CPU port */
 #define KS8995_RESET_DELAY	10 /* usec */
 
-enum ks8995_chip_variant {
-	ks8995,
-	ksz8864,
-	ksz8795,
-	max_variant
-};
-
-struct ks8995_chip_params {
-	char *name;
-	int family_id;
-	int chip_id;
-	int regs_size;
-	int addr_width;
-	int addr_shift;
-};
-
-static const struct ks8995_chip_params ks8995_chip[] = {
-	[ks8995] = {
-		.name = "KS8995MA",
-		.family_id = FAMILY_KS8995,
-		.chip_id = KS8995_CHIP_ID,
-		.regs_size = KS8995_REGS_SIZE,
-		.addr_width = 8,
-		.addr_shift = 0,
-	},
-	[ksz8864] = {
-		.name = "KSZ8864RMN",
-		.family_id = FAMILY_KS8995,
-		.chip_id = KSZ8864_CHIP_ID,
-		.regs_size = KSZ8864_REGS_SIZE,
-		.addr_width = 8,
-		.addr_shift = 0,
-	},
-	[ksz8795] = {
-		.name = "KSZ8795CLX",
-		.family_id = FAMILY_KSZ8795,
-		.chip_id = KSZ8795_CHIP_ID,
-		.regs_size = KSZ8795_REGS_SIZE,
-		.addr_width = 12,
-		.addr_shift = 1,
-	},
-};
-
 struct ks8995_switch {
 	struct spi_device	*spi;
 	struct device		*dev;
@@ -190,23 +140,18 @@ struct ks8995_switch {
 	struct mutex		lock;
 	struct gpio_desc	*reset_gpio;
 	struct bin_attribute	regs_attr;
-	const struct ks8995_chip_params	*chip;
 	int			revision_id;
 	unsigned int max_mtu[KS8995_NUM_PORTS];
 };
 
 static const struct spi_device_id ks8995_id[] = {
-	{"ks8995", ks8995},
-	{"ksz8864", ksz8864},
-	{"ksz8795", ksz8795},
+	{"ks8995", 0},
 	{ }
 };
 MODULE_DEVICE_TABLE(spi, ks8995_id);
 
 static const struct of_device_id ks8995_spi_of_match[] = {
 	{ .compatible = "micrel,ks8995" },
-	{ .compatible = "micrel,ksz8864" },
-	{ .compatible = "micrel,ksz8795" },
 	{ },
 };
 MODULE_DEVICE_TABLE(of, ks8995_spi_of_match);
@@ -237,10 +182,10 @@ static inline __be16 create_spi_cmd(struct ks8995_switch *ks, int cmd,
 {
 	u16 result = cmd;
 
-	/* make room for address (incl. address shift) */
-	result <<= ks->chip->addr_width + ks->chip->addr_shift;
+	/* make room for address */
+	result <<= 8;
 	/* add address */
-	result |= address << ks->chip->addr_shift;
+	result |= address;
 	/* SPI protocol needs big endian */
 	return cpu_to_be16(result);
 }
@@ -346,79 +291,37 @@ static int ks8995_reset(struct ks8995_switch *ks)
 static int ks8995_get_revision(struct ks8995_switch *ks)
 {
 	int err;
-	u8 id0, id1, ksz8864_id;
+	u8 id0, id1;
 
 	/* read family id */
 	err = ks8995_read_reg(ks, KS8995_REG_ID0, &id0);
-	if (err) {
-		err = -EIO;
-		goto err_out;
-	}
+	if (err)
+		return -EIO;
 
 	/* verify family id */
-	if (id0 != ks->chip->family_id) {
+	if (id0 != FAMILY_KS8995) {
 		dev_err(&ks->spi->dev, "chip family id mismatch: expected 0x%02x but 0x%02x read\n",
-			ks->chip->family_id, id0);
-		err = -ENODEV;
-		goto err_out;
+			FAMILY_KS8995, id0);
+		return -ENODEV;
 	}
 
-	switch (ks->chip->family_id) {
-	case FAMILY_KS8995:
-		/* try reading chip id at CHIP ID1 */
-		err = ks8995_read_reg(ks, KS8995_REG_ID1, &id1);
-		if (err) {
-			err = -EIO;
-			goto err_out;
-		}
-
-		/* verify chip id */
-		if ((get_chip_id(id1) == CHIPID_M) &&
-		    (get_chip_id(id1) == ks->chip->chip_id)) {
-			/* KS8995MA */
-			ks->revision_id = get_chip_rev(id1);
-		} else if (get_chip_id(id1) != CHIPID_M) {
-			/* KSZ8864RMN */
-			err = ks8995_read_reg(ks, KS8995_REG_ID1, &ksz8864_id);
-			if (err) {
-				err = -EIO;
-				goto err_out;
-			}
-
-			if ((ksz8864_id & 0x80) &&
-			    (ks->chip->chip_id == KSZ8864_CHIP_ID)) {
-				ks->revision_id = get_chip_rev(id1);
-			}
-
-		} else {
-			dev_err(&ks->spi->dev, "unsupported chip id for KS8995 family: 0x%02x\n",
-				id1);
-			err = -ENODEV;
-		}
-		break;
-	case FAMILY_KSZ8795:
-		/* try reading chip id at CHIP ID1 */
-		err = ks8995_read_reg(ks, KS8995_REG_ID1, &id1);
-		if (err) {
-			err = -EIO;
-			goto err_out;
-		}
+	/* try reading chip id at CHIP ID1 */
+	err = ks8995_read_reg(ks, KS8995_REG_ID1, &id1);
+	if (err)
+		return -EIO;
 
-		if (get_chip_id(id1) == ks->chip->chip_id) {
-			ks->revision_id = get_chip_rev(id1);
-		} else {
-			dev_err(&ks->spi->dev, "unsupported chip id for KSZ8795 family: 0x%02x\n",
-				id1);
-			err = -ENODEV;
-		}
-		break;
-	default:
-		dev_err(&ks->spi->dev, "unsupported family id: 0x%02x\n", id0);
-		err = -ENODEV;
-		break;
+	/* verify chip id */
+	if ((get_chip_id(id1) == CHIPID_M) &&
+	    (get_chip_id(id1) == KS8995_CHIP_ID)) {
+		/* KS8995MA */
+		ks->revision_id = get_chip_rev(id1);
+	} else {
+		dev_err(&ks->spi->dev, "unsupported chip id for KS8995 family: 0x%02x\n",
+			id1);
+		return -ENODEV;
 	}
-err_out:
-	return err;
+
+	return 0;
 }
 
 static int ks8995_check_config(struct ks8995_switch *ks)
@@ -747,12 +650,6 @@ static int ks8995_probe(struct spi_device *spi)
 {
 	struct ks8995_switch *ks;
 	int err;
-	int variant = spi_get_device_id(spi)->driver_data;
-
-	if (variant >= max_variant) {
-		dev_err(&spi->dev, "bad chip variant %d\n", variant);
-		return -ENODEV;
-	}
 
 	ks = devm_kzalloc(&spi->dev, sizeof(*ks), GFP_KERNEL);
 	if (!ks)
@@ -761,7 +658,6 @@ static int ks8995_probe(struct spi_device *spi)
 	mutex_init(&ks->lock);
 	ks->spi = spi;
 	ks->dev = &spi->dev;
-	ks->chip = &ks8995_chip[variant];
 
 	ks->reset_gpio = devm_gpiod_get_optional(&spi->dev, "reset",
 						 GPIOD_OUT_HIGH);
@@ -804,8 +700,8 @@ static int ks8995_probe(struct spi_device *spi)
 	if (err)
 		return err;
 
-	dev_info(&spi->dev, "%s device found, Chip ID:%x, Revision:%x\n",
-		 ks->chip->name, ks->chip->chip_id, ks->revision_id);
+	dev_info(&spi->dev, "KS8955MA device found, Chip ID:%x, Revision:%x\n",
+		 KS8995_CHIP_ID, ks->revision_id);
 
 	err = ks8995_check_config(ks);
 	if (err)

-- 
2.52.0

Re: [net-next,v2,2/4] net: dsa: ks8955: Delete KSZ8864 and KSZ8795 support

[Jakub Kicinski]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.

For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
---
net: dsa: ks8955: Delete KSZ8864 and KSZ8795 support

This commit removes support for KSZ8864 and KSZ8795 chip variants from
the ks8995 driver, since those variants are better supported by the
dedicated Microchip KSZ driver.

> diff --git a/drivers/net/dsa/ks8995.c b/drivers/net/dsa/ks8995.c
> index ff01097601ec..5ad62fa4e52c 100644
> --- a/drivers/net/dsa/ks8995.c
> +++ b/drivers/net/dsa/ks8995.c

[ ... ]

> @@ -804,8 +700,8 @@ static int ks8995_probe(struct spi_device *spi)
>  	if (err)
>  		return err;
>
> -	dev_info(&spi->dev, "%s device found, Chip ID:%x, Revision:%x\n",
> -		 ks->chip->name, ks->chip->chip_id, ks->revision_id);
> +	dev_info(&spi->dev, "KS8955MA device found, Chip ID:%x, Revision:%x\n",
                             ^^^^^^^

Should this be "KS8995MA" instead of "KS8955MA"? The chip name appears to
have a typo - the driver is for KS8995 (as in the file name, module
description, and all other references), but the dev_info prints "KS8955MA"
with transposed digits.

> +		 KS8995_CHIP_ID, ks->revision_id);
-- 
pw-bot: cr

Re: [PATCH net-next v2 2/4] net: dsa: ks8955: Delete KSZ8864 and KSZ8795 support

[Vladimir Oltean]

On Mon, Jan 19, 2026 at 03:30:06PM +0100, Linus Walleij wrote:
> After studying the datasheets for a bit, I can conclude that
> the register maps for the two KSZ variants explicitly said to
> be supported by this driver are fully supported by the newer
> Micrel KSZ driver, including full VLAN support and a different
> custom tag than what the KS8995 is using.
> 
> Delete this support, users should be using the KSZ driver
> CONFIG_NET_DSA_MICROCHIP_KSZ_SPI and any new device trees should
> use:
> micrel,ksz8864 -> microchip,ksz8864
> micrel,ksz8795 -> microchip,ksz8795

So the binding changes you've done to Documentation/devicetree/bindings/net/micrel-ks8995.txt
in commit a0f29a07b654 ("dt-bindings: dsa: Rewrite Micrel KS8995 in schema")
were apparently backwards-compatible. But this isn't - you're offering
no forward path for existing device trees.

IMO, even if nobody cares about these compatible strings (given that the
"PHY" driver dates from 2016 and the KSZ DSA driver has supported these
chips since 2019), IMO it's pretty hard to sell a loss of hardware support
in a patch set targeted to 'net'.

Does it make more sense to retarget the patches to 'net-next', drop
the Fixes: tags and to somehow mark the driver as "experimental", to set
the expectations about the fact that it's still under development and
many things aren't how they should be?

> 
> Apparently Microchip acquired Micrel at some point and this
> created the confusion.
> 
> Fixes: a7fe8b266f65 ("net: dsa: ks8995: Add basic switch set-up")
> Reported-by: Vladimir Oltean <olteanv@gmail.com>
> Signed-off-by: Linus Walleij <linusw@kernel.org>
> ---
>  drivers/net/dsa/ks8995.c | 160 +++++++++--------------------------------------
>  1 file changed, 28 insertions(+), 132 deletions(-)

Re: [PATCH net-next v2 2/4] net: dsa: ks8955: Delete KSZ8864 and KSZ8795 support

[Linus Walleij]

On Mon, Jan 19, 2026 at 10:58 PM Vladimir Oltean <olteanv@gmail.com> wrote:
> On Mon, Jan 19, 2026 at 03:30:06PM +0100, Linus Walleij wrote:

> > After studying the datasheets for a bit, I can conclude that
> > the register maps for the two KSZ variants explicitly said to
> > be supported by this driver are fully supported by the newer
> > Micrel KSZ driver, including full VLAN support and a different
> > custom tag than what the KS8995 is using.
> >
> > Delete this support, users should be using the KSZ driver
> > CONFIG_NET_DSA_MICROCHIP_KSZ_SPI and any new device trees should
> > use:
> > micrel,ksz8864 -> microchip,ksz8864
> > micrel,ksz8795 -> microchip,ksz8795
>
> So the binding changes you've done to Documentation/devicetree/bindings/net/micrel-ks8995.txt
> in commit a0f29a07b654 ("dt-bindings: dsa: Rewrite Micrel KS8995 in schema")
> were apparently backwards-compatible. But this isn't - you're offering
> no forward path for existing device trees.

There is theoretically no contract that Linux has to respond to
all compatible strings listed in a DT binding document, but yeah it
would be nice to not screw things up for any potential users.

> IMO, even if nobody cares about these compatible strings (given that the
> "PHY" driver dates from 2016 and the KSZ DSA driver has supported these
> chips since 2019), IMO it's pretty hard to sell a loss of hardware support
> in a patch set targeted to 'net'.
>
> Does it make more sense to retarget the patches to 'net-next', drop
> the Fixes: tags and to somehow mark the driver as "experimental", to set
> the expectations about the fact that it's still under development and
> many things aren't how they should be?

I was thinking actually adding the micrel,* strings to the ksz driver
in this patch, or as a separate patch and mark them for backward
compatibility. But maybe that also need to qualify as non-fixes?

I can surely drop the Fixes: tags, mark experimental and perhaps also
put the revised tag patches on top to be safe, if we think this is
the best approach.

I did grep around, there are no users for the bindings in the in-kernel
trees or in OpenWrt. But who knows what it out there.

Yours,
Linus Walleij

[PATCH net-next v2 3/4] net: dsa: ks8995: Add stub bridge join/leave

[Linus Walleij]

Implementing ks8995_port_pre_bridge_flags() and
ks8995_port_bridge_flags() without port_bridge_join()
is a no-op.

This adds stubs for bridge join/leave callbacks following
the pattern of drivers/net/dsa/microchip/ksz_common.c:
as we have STP callbacks and these will be called right
after bridge join/leave these will take care of the
job of setting up the learning which is all we support.

Fixes: a7fe8b266f65 ("net: dsa: ks8995: Add basic switch set-up")
Reported-by: Vladimir Oltean <olteanv@gmail.com>
Signed-off-by: Linus Walleij <linusw@kernel.org>
---
 drivers/net/dsa/ks8995.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/drivers/net/dsa/ks8995.c b/drivers/net/dsa/ks8995.c
index 5ad62fa4e52c..060bc8303a14 100644
--- a/drivers/net/dsa/ks8995.c
+++ b/drivers/net/dsa/ks8995.c
@@ -461,6 +461,26 @@ static void ks8995_port_disable(struct dsa_switch *ds, int port)
 	dev_dbg(ks->dev, "disable port %d\n", port);
 }
 
+static int ks8995_port_bridge_join(struct dsa_switch *ds, int port,
+				   struct dsa_bridge bridge,
+				   bool *tx_fwd_offload,
+				   struct netlink_ext_ack *extack)
+{
+	/* port_stp_state_set() will be called after to put the port in
+	 * appropriate state so there is no need to do anything.
+	 */
+
+	return 0;
+}
+
+static void ks8995_port_bridge_leave(struct dsa_switch *ds, int port,
+				     struct dsa_bridge bridge)
+{
+	/* port_stp_state_set() will be called after to put the port in
+	 * forwarding state so there is no need to do anything.
+	 */
+}
+
 static int ks8995_port_pre_bridge_flags(struct dsa_switch *ds, int port,
 					struct switchdev_brport_flags flags,
 					struct netlink_ext_ack *extack)
@@ -635,6 +655,8 @@ static int ks8995_get_max_mtu(struct dsa_switch *ds, int port)
 static const struct dsa_switch_ops ks8995_ds_ops = {
 	.get_tag_protocol = ks8995_get_tag_protocol,
 	.setup = ks8995_setup,
+	.port_bridge_join = ks8995_port_bridge_join,
+	.port_bridge_leave = ks8995_port_bridge_leave,
 	.port_pre_bridge_flags = ks8995_port_pre_bridge_flags,
 	.port_bridge_flags = ks8995_port_bridge_flags,
 	.port_enable = ks8995_port_enable,

-- 
2.52.0

Re: [PATCH net-next v2 3/4] net: dsa: ks8995: Add stub bridge join/leave

[Vladimir Oltean]

On Mon, Jan 19, 2026 at 03:30:07PM +0100, Linus Walleij wrote:
> Implementing ks8995_port_pre_bridge_flags() and
> ks8995_port_bridge_flags() without port_bridge_join()
> is a no-op.
> 
> This adds stubs for bridge join/leave callbacks following
> the pattern of drivers/net/dsa/microchip/ksz_common.c:
> as we have STP callbacks and these will be called right
> after bridge join/leave these will take care of the
> job of setting up the learning which is all we support.
> 
> Fixes: a7fe8b266f65 ("net: dsa: ks8995: Add basic switch set-up")
> Reported-by: Vladimir Oltean <olteanv@gmail.com>
> Signed-off-by: Linus Walleij <linusw@kernel.org>
> ---
>  drivers/net/dsa/ks8995.c | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
> 
> diff --git a/drivers/net/dsa/ks8995.c b/drivers/net/dsa/ks8995.c
> index 5ad62fa4e52c..060bc8303a14 100644
> --- a/drivers/net/dsa/ks8995.c
> +++ b/drivers/net/dsa/ks8995.c
> @@ -461,6 +461,26 @@ static void ks8995_port_disable(struct dsa_switch *ds, int port)
>  	dev_dbg(ks->dev, "disable port %d\n", port);
>  }
>  
> +static int ks8995_port_bridge_join(struct dsa_switch *ds, int port,
> +				   struct dsa_bridge bridge,
> +				   bool *tx_fwd_offload,
> +				   struct netlink_ext_ack *extack)
> +{
> +	/* port_stp_state_set() will be called after to put the port in
> +	 * appropriate state so there is no need to do anything.
> +	 */

Not directly related, but reviewing ks8995_port_stp_state_set() I
noticed another issue: the driver implementation of the
BR_STATE_LEARNING and BR_STATE_FORWARDING states should take into
consideration a previous call to ks8995_port_bridge_flags() which has
disabled BR_LEARNING for the port.

Look at ksz_port_stp_state_set() to compare how it first tests for
p->learning before touching PORT_LEARN_DISABLE.

This becomes a problem with this patch, because this patch brings
ks8995_port_bridge_flags() to life and makes user space able to turn off
address learning for the port. So it is relevant, and it would be good
to fix it before enabling the feature.

And yet one more potential issue: standalone user ports should have
address learning disabled. At driver probe time it is the driver's
responsibility to ensure that this is the case. After the port joins a
bridge and leaves it afterwards, the setting changes are driven by DSA.
I don't know what the default setting is in your case.

> +
> +	return 0;
> +}
> +
> +static void ks8995_port_bridge_leave(struct dsa_switch *ds, int port,
> +				     struct dsa_bridge bridge)
> +{
> +	/* port_stp_state_set() will be called after to put the port in
> +	 * forwarding state so there is no need to do anything.
> +	 */
> +}
> +
>  static int ks8995_port_pre_bridge_flags(struct dsa_switch *ds, int port,
>  					struct switchdev_brport_flags flags,
>  					struct netlink_ext_ack *extack)
> @@ -635,6 +655,8 @@ static int ks8995_get_max_mtu(struct dsa_switch *ds, int port)
>  static const struct dsa_switch_ops ks8995_ds_ops = {
>  	.get_tag_protocol = ks8995_get_tag_protocol,
>  	.setup = ks8995_setup,
> +	.port_bridge_join = ks8995_port_bridge_join,
> +	.port_bridge_leave = ks8995_port_bridge_leave,
>  	.port_pre_bridge_flags = ks8995_port_pre_bridge_flags,
>  	.port_bridge_flags = ks8995_port_bridge_flags,
>  	.port_enable = ks8995_port_enable,
> 
> -- 
> 2.52.0
> 

[PATCH net-next v2 4/4] net: dsa: ks8995: Implement port isolation

[Linus Walleij]

It is unsound to not have proper port isolation on a
switch which supports it.

Set each port as isolated by default in the setup callback
and de-isolate and isolate the ports in the bridge join/leave
callbacks.

Fixes: a7fe8b266f65 ("net: dsa: ks8995: Add basic switch set-up")
Reported-by: Vladimir Oltean <olteanv@gmail.com>
Signed-off-by: Linus Walleij <linusw@kernel.org>
---
 drivers/net/dsa/ks8995.c | 131 ++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 129 insertions(+), 2 deletions(-)

diff --git a/drivers/net/dsa/ks8995.c b/drivers/net/dsa/ks8995.c
index 060bc8303a14..574e14743a36 100644
--- a/drivers/net/dsa/ks8995.c
+++ b/drivers/net/dsa/ks8995.c
@@ -80,6 +80,11 @@
 #define KS8995_PC0_TAG_REM	BIT(1)	/* Enable tag removal on port */
 #define KS8995_PC0_PRIO_EN	BIT(0)	/* Enable priority handling */
 
+#define KS8995_PC1_SNIFF_PORT	BIT(7)	/* This port is a sniffer port */
+#define KS8995_PC1_RCV_SNIFF	BIT(6)	/* Packets received goes to sniffer port(s) */
+#define KS8995_PC1_XMIT_SNIFF	BIT(5)	/* Packets transmitted goes to sniffer port(s) */
+#define KS8995_PC1_PORT_VLAN	GENMASK(4, 0)	/* Port isolation mask */
+
 #define KS8995_PC2_TXEN		BIT(2)	/* Enable TX on port */
 #define KS8995_PC2_RXEN		BIT(1)	/* Enable RX on port */
 #define KS8995_PC2_LEARN_DIS	BIT(0)	/* Disable learning on port */
@@ -441,6 +446,44 @@ dsa_tag_protocol ks8995_get_tag_protocol(struct dsa_switch *ds,
 
 static int ks8995_setup(struct dsa_switch *ds)
 {
+	struct ks8995_switch *ks = ds->priv;
+	int ret;
+	u8 val;
+	int i;
+
+	/* Isolate all user ports so they can only send packets to itself and the CPU port */
+	for (i = 0; i < KS8995_CPU_PORT; i++) {
+		ret = ks8995_read_reg(ks, KS8995_REG_PC(i, KS8995_REG_PC1), &val);
+		if (ret) {
+			dev_err(ks->dev, "failed to read KS8995_REG_PC1 on port %d\n", i);
+			return ret;
+		}
+
+		val &= ~KS8995_PC1_PORT_VLAN;
+		val |= (BIT(i) | BIT(KS8995_CPU_PORT));
+
+		ret = ks8995_write_reg(ks, KS8995_REG_PC(i, KS8995_REG_PC1), val);
+		if (ret) {
+			dev_err(ks->dev, "failed to write KS8995_REG_PC1 on port %d\n", i);
+			return ret;
+		}
+	}
+
+	/* The CPU port should be able to talk to all ports */
+	ret = ks8995_read_reg(ks, KS8995_REG_PC(KS8995_CPU_PORT, KS8995_REG_PC1), &val);
+	if (ret) {
+		dev_err(ks->dev, "failed to read KS8995_REG_PC1 on CPU port\n");
+		return ret;
+	}
+
+	val |= KS8995_PC1_PORT_VLAN;
+
+	ret = ks8995_write_reg(ks, KS8995_REG_PC(KS8995_CPU_PORT, KS8995_REG_PC1), val);
+	if (ret) {
+		dev_err(ks->dev, "failed to write KS8995_REG_PC1 on CPU port\n");
+		return ret;
+	}
+
 	return 0;
 }
 
@@ -466,8 +509,44 @@ static int ks8995_port_bridge_join(struct dsa_switch *ds, int port,
 				   bool *tx_fwd_offload,
 				   struct netlink_ext_ack *extack)
 {
+	struct ks8995_switch *ks = ds->priv;
+	u8 port_bitmap = 0;
+	int ret;
+	u8 val;
+	int i;
+
+	/* De-isolate this port from any other port on the bridge */
+	port_bitmap |= BIT(port);
+	for (i = 0; i < KS8995_CPU_PORT; i++) {
+		if (i == port)
+			continue;
+		if (!dsa_port_offloads_bridge(dsa_to_port(ds, i), &bridge))
+			continue;
+		port_bitmap |= BIT(i);
+	}
+
+	/* Update all affected ports with the new bitmask */
+	for (i = 0; i < KS8995_CPU_PORT; i++) {
+		if (!dsa_port_offloads_bridge(dsa_to_port(ds, i), &bridge))
+			continue;
+
+		ret = ks8995_read_reg(ks, KS8995_REG_PC(i, KS8995_REG_PC1), &val);
+		if (ret) {
+			dev_err(ks->dev, "failed to read KS8995_REG_PC1 on port %d\n", i);
+			return ret;
+		}
+
+		val |= port_bitmap;
+
+		ret = ks8995_write_reg(ks, KS8995_REG_PC(i, KS8995_REG_PC1), val);
+		if (ret) {
+			dev_err(ks->dev, "failed to write KS8995_REG_PC1 on port %d\n", i);
+			return ret;
+		}
+	}
+
 	/* port_stp_state_set() will be called after to put the port in
-	 * appropriate state so there is no need to do anything.
+	 * appropriate state.
 	 */
 
 	return 0;
@@ -476,8 +555,56 @@ static int ks8995_port_bridge_join(struct dsa_switch *ds, int port,
 static void ks8995_port_bridge_leave(struct dsa_switch *ds, int port,
 				     struct dsa_bridge bridge)
 {
+	struct ks8995_switch *ks = ds->priv;
+	u8 port_bitmap = 0;
+	int ret;
+	u8 val;
+	int i;
+
+	/* Isolate this port from any other port on the bridge */
+	for (i = 0; i < KS8995_CPU_PORT; i++) {
+		/* Current port handled last */
+		if (i == port)
+			continue;
+		/* Not on this bridge */
+		if (!dsa_port_offloads_bridge(dsa_to_port(ds, i), &bridge))
+			continue;
+
+		ret = ks8995_read_reg(ks, KS8995_REG_PC(i, KS8995_REG_PC1), &val);
+		if (ret) {
+			dev_err(ks->dev, "failed to read KS8995_REG_PC1 on port %d\n", i);
+			return;
+		}
+
+		val &= ~BIT(port);
+
+		ret = ks8995_write_reg(ks, KS8995_REG_PC(i, KS8995_REG_PC1), val);
+		if (ret) {
+			dev_err(ks->dev, "failed to write KS8995_REG_PC1 on port %d\n", i);
+			return;
+		}
+
+		/* Accumulate this port for access by current */
+		port_bitmap |= BIT(i);
+	}
+
+	/* Isolate this port from all other ports formerly on the bridge */
+	ret = ks8995_read_reg(ks, KS8995_REG_PC(port, KS8995_REG_PC1), &val);
+	if (ret) {
+		dev_err(ks->dev, "failed to read KS8995_REG_PC1 on port %d\n", port);
+		return;
+	}
+
+	val &= ~port_bitmap;
+
+	ret = ks8995_write_reg(ks, KS8995_REG_PC(port, KS8995_REG_PC1), val);
+	if (ret) {
+		dev_err(ks->dev, "failed to write KS8995_REG_PC1 on port %d\n", port);
+		return;
+	}
+
 	/* port_stp_state_set() will be called after to put the port in
-	 * forwarding state so there is no need to do anything.
+	 * forwarding state.
 	 */
 }
 

-- 
2.52.0

Re: [PATCH net-next v2 4/4] net: dsa: ks8995: Implement port isolation

[Vladimir Oltean]

On Mon, Jan 19, 2026 at 03:30:08PM +0100, Linus Walleij wrote:
> It is unsound to not have proper port isolation on a
> switch which supports it.
> 
> Set each port as isolated by default in the setup callback
> and de-isolate and isolate the ports in the bridge join/leave
> callbacks.
> 
> Fixes: a7fe8b266f65 ("net: dsa: ks8995: Add basic switch set-up")
> Reported-by: Vladimir Oltean <olteanv@gmail.com>
> Signed-off-by: Linus Walleij <linusw@kernel.org>
> ---
>  drivers/net/dsa/ks8995.c | 131 ++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 129 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/dsa/ks8995.c b/drivers/net/dsa/ks8995.c
> index 060bc8303a14..574e14743a36 100644
> --- a/drivers/net/dsa/ks8995.c
> +++ b/drivers/net/dsa/ks8995.c
> @@ -80,6 +80,11 @@
>  #define KS8995_PC0_TAG_REM	BIT(1)	/* Enable tag removal on port */
>  #define KS8995_PC0_PRIO_EN	BIT(0)	/* Enable priority handling */
>  
> +#define KS8995_PC1_SNIFF_PORT	BIT(7)	/* This port is a sniffer port */
> +#define KS8995_PC1_RCV_SNIFF	BIT(6)	/* Packets received goes to sniffer port(s) */
> +#define KS8995_PC1_XMIT_SNIFF	BIT(5)	/* Packets transmitted goes to sniffer port(s) */
> +#define KS8995_PC1_PORT_VLAN	GENMASK(4, 0)	/* Port isolation mask */
> +
>  #define KS8995_PC2_TXEN		BIT(2)	/* Enable TX on port */
>  #define KS8995_PC2_RXEN		BIT(1)	/* Enable RX on port */
>  #define KS8995_PC2_LEARN_DIS	BIT(0)	/* Disable learning on port */
> @@ -441,6 +446,44 @@ dsa_tag_protocol ks8995_get_tag_protocol(struct dsa_switch *ds,
>  
>  static int ks8995_setup(struct dsa_switch *ds)
>  {
> +	struct ks8995_switch *ks = ds->priv;
> +	int ret;
> +	u8 val;
> +	int i;
> +
> +	/* Isolate all user ports so they can only send packets to itself and the CPU port */

I would refrain from using "isolation" related terminology for user port
separation, because of the naming collision with the BR_ISOLATED bridge
port flag (from "man bridge", isolated bridge ports "will be able to
communicate with non-isolated ports only").

> +	for (i = 0; i < KS8995_CPU_PORT; i++) {
> +		ret = ks8995_read_reg(ks, KS8995_REG_PC(i, KS8995_REG_PC1), &val);
> +		if (ret) {
> +			dev_err(ks->dev, "failed to read KS8995_REG_PC1 on port %d\n", i);
> +			return ret;
> +		}
> +
> +		val &= ~KS8995_PC1_PORT_VLAN;
> +		val |= (BIT(i) | BIT(KS8995_CPU_PORT));
> +
> +		ret = ks8995_write_reg(ks, KS8995_REG_PC(i, KS8995_REG_PC1), val);

Do you actually need to perform a register read at probe time, or could
you just call ks8995_write_reg() with known good values for the sniff
port bits too?

> +		if (ret) {
> +			dev_err(ks->dev, "failed to write KS8995_REG_PC1 on port %d\n", i);
> +			return ret;
> +		}
> +	}
> +
> +	/* The CPU port should be able to talk to all ports */
> +	ret = ks8995_read_reg(ks, KS8995_REG_PC(KS8995_CPU_PORT, KS8995_REG_PC1), &val);
> +	if (ret) {
> +		dev_err(ks->dev, "failed to read KS8995_REG_PC1 on CPU port\n");
> +		return ret;
> +	}
> +
> +	val |= KS8995_PC1_PORT_VLAN;

Writing this value enables hairpinning (reflection of forwarded traffic)
on the CPU port, because KS8995_PC1_PORT_VLAN (GENMASK(4, 0)) includes
KS8995_CPU_PORT (4).

> +
> +	ret = ks8995_write_reg(ks, KS8995_REG_PC(KS8995_CPU_PORT, KS8995_REG_PC1), val);
> +	if (ret) {
> +		dev_err(ks->dev, "failed to write KS8995_REG_PC1 on CPU port\n");
> +		return ret;
> +	}
> +
>  	return 0;
>  }
>  
> @@ -466,8 +509,44 @@ static int ks8995_port_bridge_join(struct dsa_switch *ds, int port,
>  				   bool *tx_fwd_offload,
>  				   struct netlink_ext_ack *extack)
>  {
> +	struct ks8995_switch *ks = ds->priv;
> +	u8 port_bitmap = 0;
> +	int ret;
> +	u8 val;
> +	int i;
> +
> +	/* De-isolate this port from any other port on the bridge */
> +	port_bitmap |= BIT(port);

A bit strange to unconditionally modify the initialization value of a
variable rather than just assign BIT(port) as its initializer.

> +	for (i = 0; i < KS8995_CPU_PORT; i++) {
> +		if (i == port)
> +			continue;
> +		if (!dsa_port_offloads_bridge(dsa_to_port(ds, i), &bridge))
> +			continue;

dsa_to_port() has an embedded loop over ports inside, so actually this
loop iterates ds->num_ports^2 times. It is recommended that you use the
dsa_switch_for_each_user_port() iterator and that gives you "dp"
directly (i becomes dp->index).

> +		port_bitmap |= BIT(i);
> +	}
> +
> +	/* Update all affected ports with the new bitmask */
> +	for (i = 0; i < KS8995_CPU_PORT; i++) {
> +		if (!dsa_port_offloads_bridge(dsa_to_port(ds, i), &bridge))
> +			continue;
> +
> +		ret = ks8995_read_reg(ks, KS8995_REG_PC(i, KS8995_REG_PC1), &val);
> +		if (ret) {
> +			dev_err(ks->dev, "failed to read KS8995_REG_PC1 on port %d\n", i);
> +			return ret;
> +		}
> +
> +		val |= port_bitmap;

Same hairpinning problem. When a new port joins a bridge, the existing
ports start enabling forwarding to themselves as well (the port_bitmap
written to port i contains BIT(i) set).

> +
> +		ret = ks8995_write_reg(ks, KS8995_REG_PC(i, KS8995_REG_PC1), val);
> +		if (ret) {
> +			dev_err(ks->dev, "failed to write KS8995_REG_PC1 on port %d\n", i);
> +			return ret;
> +		}
> +	}
> +
>  	/* port_stp_state_set() will be called after to put the port in
> -	 * appropriate state so there is no need to do anything.
> +	 * appropriate state.
>  	 */
>  
>  	return 0;
> @@ -476,8 +555,56 @@ static int ks8995_port_bridge_join(struct dsa_switch *ds, int port,
>  static void ks8995_port_bridge_leave(struct dsa_switch *ds, int port,
>  				     struct dsa_bridge bridge)
>  {
> +	struct ks8995_switch *ks = ds->priv;
> +	u8 port_bitmap = 0;
> +	int ret;
> +	u8 val;
> +	int i;
> +
> +	/* Isolate this port from any other port on the bridge */
> +	for (i = 0; i < KS8995_CPU_PORT; i++) {
> +		/* Current port handled last */
> +		if (i == port)
> +			continue;
> +		/* Not on this bridge */
> +		if (!dsa_port_offloads_bridge(dsa_to_port(ds, i), &bridge))
> +			continue;
> +
> +		ret = ks8995_read_reg(ks, KS8995_REG_PC(i, KS8995_REG_PC1), &val);
> +		if (ret) {
> +			dev_err(ks->dev, "failed to read KS8995_REG_PC1 on port %d\n", i);
> +			return;
> +		}
> +
> +		val &= ~BIT(port);
> +
> +		ret = ks8995_write_reg(ks, KS8995_REG_PC(i, KS8995_REG_PC1), val);
> +		if (ret) {
> +			dev_err(ks->dev, "failed to write KS8995_REG_PC1 on port %d\n", i);
> +			return;
> +		}
> +
> +		/* Accumulate this port for access by current */
> +		port_bitmap |= BIT(i);
> +	}
> +
> +	/* Isolate this port from all other ports formerly on the bridge */
> +	ret = ks8995_read_reg(ks, KS8995_REG_PC(port, KS8995_REG_PC1), &val);
> +	if (ret) {
> +		dev_err(ks->dev, "failed to read KS8995_REG_PC1 on port %d\n", port);
> +		return;
> +	}
> +
> +	val &= ~port_bitmap;
> +
> +	ret = ks8995_write_reg(ks, KS8995_REG_PC(port, KS8995_REG_PC1), val);
> +	if (ret) {
> +		dev_err(ks->dev, "failed to write KS8995_REG_PC1 on port %d\n", port);
> +		return;
> +	}
> +

The register layout seems identical with the one from ksz8_cfg_port_member(),
but that being said, I don't think I have a problem with the KS8995
driver continuing to be maintained separately (for now).

>  	/* port_stp_state_set() will be called after to put the port in
> -	 * forwarding state so there is no need to do anything.
> +	 * forwarding state.
>  	 */
>  }
>  
> 
> -- 
> 2.52.0
>