[PATCH v2 1/2] net: dsa: microchip: add KSZ9896 switch support

[PATCH v2 1/2] net: dsa: microchip: add KSZ9896 switch support

[Romain Naour]

From: Romain Naour <romain.naour@skf.com>

Add support for the KSZ9896 6-port Gigabit Ethernet Switch to the
ksz9477 driver.

Although the KSZ9896 is already listed in the device tree binding
documentation since a1c0ed24fe9b (dt-bindings: net: dsa: document
additional Microchip KSZ9477 family switches) the chip id
(0x00989600) is not recognized by ksz_switch_detect() and rejected
by the driver.

The KSZ9896 is similar to KSZ9897 but has only one configurable
MII/RMII/RGMII/GMII cpu port.

Signed-off-by: Romain Naour <romain.naour@skf.com>
---
It seems that the KSZ9896 support has been sent to the kernel netdev
mailing list a while ago but no further patch was sent after the
initial review:
https://www.spinics.net/lists/netdev/msg554771.html

The initial testing with the ksz9896 was done on a 5.10 kernel
but due to recent changes in dsa microchip driver it was required
to rework this initial version for 6.0-rc2 kernel.

v2: remove duplicated SoB line
---
 drivers/net/dsa/microchip/ksz_common.c | 30 ++++++++++++++++++++++++++
 drivers/net/dsa/microchip/ksz_common.h |  2 ++
 drivers/net/dsa/microchip/ksz_spi.c    |  6 ++++++
 3 files changed, 38 insertions(+)

diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index 6bd69a7e6809..759d98f4e317 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -547,6 +547,34 @@ const struct ksz_chip_data ksz_switch_chips[] = {
 				   true, false, false},
 	},
 
+	[KSZ9896] = {
+		.chip_id = KSZ9896_CHIP_ID,
+		.dev_name = "KSZ9896",
+		.num_vlans = 4096,
+		.num_alus = 4096,
+		.num_statics = 16,
+		.cpu_ports = 0x3F,	/* can be configured as cpu port */
+		.port_cnt = 6,		/* total physical port count */
+		.ops = &ksz9477_dev_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},
+		.supports_rmii	= {false, false, false, false,
+				   false, true},
+		.supports_rgmii = {false, false, false, false,
+				   false, true},
+		.internal_phy	= {true, true, true, true,
+				   true, false},
+	},
+
 	[KSZ9897] = {
 		.chip_id = KSZ9897_CHIP_ID,
 		.dev_name = "KSZ9897",
@@ -1370,6 +1398,7 @@ static enum dsa_tag_protocol ksz_get_tag_protocol(struct dsa_switch *ds,
 		proto = DSA_TAG_PROTO_KSZ9893;
 
 	if (dev->chip_id == KSZ9477_CHIP_ID ||
+	    dev->chip_id == KSZ9896_CHIP_ID ||
 	    dev->chip_id == KSZ9897_CHIP_ID ||
 	    dev->chip_id == KSZ9567_CHIP_ID)
 		proto = DSA_TAG_PROTO_KSZ9477;
@@ -1730,6 +1759,7 @@ static int ksz_switch_detect(struct ksz_device *dev)
 
 		switch (id32) {
 		case KSZ9477_CHIP_ID:
+		case KSZ9896_CHIP_ID:
 		case KSZ9897_CHIP_ID:
 		case KSZ9893_CHIP_ID:
 		case KSZ9567_CHIP_ID:
diff --git a/drivers/net/dsa/microchip/ksz_common.h b/drivers/net/dsa/microchip/ksz_common.h
index 0d9520dc6d2d..65980da61b54 100644
--- a/drivers/net/dsa/microchip/ksz_common.h
+++ b/drivers/net/dsa/microchip/ksz_common.h
@@ -129,6 +129,7 @@ enum ksz_model {
 	KSZ8765,
 	KSZ8830,
 	KSZ9477,
+	KSZ9896,
 	KSZ9897,
 	KSZ9893,
 	KSZ9567,
@@ -145,6 +146,7 @@ enum ksz_chip_id {
 	KSZ8765_CHIP_ID = 0x8765,
 	KSZ8830_CHIP_ID = 0x8830,
 	KSZ9477_CHIP_ID = 0x00947700,
+	KSZ9896_CHIP_ID = 0x00989600,
 	KSZ9897_CHIP_ID = 0x00989700,
 	KSZ9893_CHIP_ID = 0x00989300,
 	KSZ9567_CHIP_ID = 0x00956700,
diff --git a/drivers/net/dsa/microchip/ksz_spi.c b/drivers/net/dsa/microchip/ksz_spi.c
index 05bd089795f8..1b6ff5e3d575 100644
--- a/drivers/net/dsa/microchip/ksz_spi.c
+++ b/drivers/net/dsa/microchip/ksz_spi.c
@@ -146,6 +146,10 @@ static const struct of_device_id ksz_dt_ids[] = {
 		.compatible = "microchip,ksz9477",
 		.data = &ksz_switch_chips[KSZ9477]
 	},
+	{
+		.compatible = "microchip,ksz9896",
+		.data = &ksz_switch_chips[KSZ9896]
+	},
 	{
 		.compatible = "microchip,ksz9897",
 		.data = &ksz_switch_chips[KSZ9897]
@@ -197,6 +201,7 @@ static const struct spi_device_id ksz_spi_ids[] = {
 	{ "ksz8863" },
 	{ "ksz8873" },
 	{ "ksz9477" },
+	{ "ksz9896" },
 	{ "ksz9897" },
 	{ "ksz9893" },
 	{ "ksz9563" },
@@ -226,6 +231,7 @@ static struct spi_driver ksz_spi_driver = {
 module_spi_driver(ksz_spi_driver);
 
 MODULE_ALIAS("spi:ksz9477");
+MODULE_ALIAS("spi:ksz9896");
 MODULE_ALIAS("spi:ksz9897");
 MODULE_ALIAS("spi:ksz9893");
 MODULE_ALIAS("spi:ksz9563");
-- 
2.34.3

[PATCH v2 2/2] net: dsa: microchip: add KSZ9896 to KSZ9477 I2C driver

[Romain Naour]

From: Romain Naour <romain.naour@skf.com>

Add support for the KSZ9896 6-port Gigabit Ethernet Switch to the
ksz9477 driver. The KSZ9896 supports both SPI (already in) and I2C.

Signed-off-by: Romain Naour <romain.naour@skf.com>
---
The KSZ9896 support i2c interface, it seems safe to enable as is but
runtime testing is really needed (my KSZ9896 is wired with spi).

v2: remove duplicated SoB line
---
 drivers/net/dsa/microchip/ksz9477_i2c.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/dsa/microchip/ksz9477_i2c.c b/drivers/net/dsa/microchip/ksz9477_i2c.c
index 99966514d444..8fbc122e3384 100644
--- a/drivers/net/dsa/microchip/ksz9477_i2c.c
+++ b/drivers/net/dsa/microchip/ksz9477_i2c.c
@@ -91,6 +91,10 @@ static const struct of_device_id ksz9477_dt_ids[] = {
 		.compatible = "microchip,ksz9477",
 		.data = &ksz_switch_chips[KSZ9477]
 	},
+	{
+		.compatible = "microchip,ksz9896",
+		.data = &ksz_switch_chips[KSZ9896]
+	},
 	{
 		.compatible = "microchip,ksz9897",
 		.data = &ksz_switch_chips[KSZ9897]
-- 
2.34.3

Re: [PATCH v2 1/2] net: dsa: microchip: add KSZ9896 switch support

[Vladimir Oltean]

Hi Romain,

On Tue, Aug 30, 2022 at 09:58:59AM +0200, Romain Naour wrote:
> It seems that the KSZ9896 support has been sent to the kernel netdev
> mailing list a while ago but no further patch was sent after the
> initial review:
> https://www.spinics.net/lists/netdev/msg554771.html
> 
> The initial testing with the ksz9896 was done on a 5.10 kernel
> but due to recent changes in dsa microchip driver it was required
> to rework this initial version for 6.0-rc2 kernel.
> 
> v2: remove duplicated SoB line

To be clear, was the patch also tested on net-next, or only formatted there?

Re: [PATCH v2 1/2] net: dsa: microchip: add KSZ9896 switch support

[Romain Naour]

Hi Vladimir,

Le 31/08/2022 à 17:38, Vladimir Oltean a écrit :
> Hi Romain,
> 
> On Tue, Aug 30, 2022 at 09:58:59AM +0200, Romain Naour wrote:
>> It seems that the KSZ9896 support has been sent to the kernel netdev
>> mailing list a while ago but no further patch was sent after the
>> initial review:
>> https://www.spinics.net/lists/netdev/msg554771.html
>>
>> The initial testing with the ksz9896 was done on a 5.10 kernel
>> but due to recent changes in dsa microchip driver it was required
>> to rework this initial version for 6.0-rc2 kernel.
>>
>> v2: remove duplicated SoB line
> 
> To be clear, was the patch also tested on net-next, or only formatted there?

The patch was runtime tested on a 6.0-rc2 kernel and a second time on a 6.0-rc3
kernel but not on net-next.

Is it ok with rc releases or do I need to test on net-next too?

Best regards,
Romain

Re: [PATCH v2 1/2] net: dsa: microchip: add KSZ9896 switch support

[Vladimir Oltean]

On Wed, Aug 31, 2022 at 05:43:27PM +0200, Romain Naour wrote:
> The patch was runtime tested on a 6.0-rc2 kernel and a second time on a 6.0-rc3
> kernel but not on net-next.
> 
> Is it ok with rc releases or do I need to test on net-next too?

The kernel development process is that you normally test a patch on the
git tree on which it is to be eventually applied.

The net-next.git tree is periodically (weekly) merged with the 6.0
release candidates where bug fixes land, but it contains newly developed
material intended for the 6.1 release candidates (hence the "next" name).

If you keep formatting development patches against the plain 6.0 release
candidates, you may eventually run into a conflict with some other new
development, and you may never even know.

Re: [PATCH v2 1/2] net: dsa: microchip: add KSZ9896 switch support

[Romain Naour]

Hi Vladimir,

Le 31/08/2022 à 17:51, Vladimir Oltean a écrit :
> On Wed, Aug 31, 2022 at 05:43:27PM +0200, Romain Naour wrote:
>> The patch was runtime tested on a 6.0-rc2 kernel and a second time on a 6.0-rc3
>> kernel but not on net-next.
>>
>> Is it ok with rc releases or do I need to test on net-next too?
> 
> The kernel development process is that you normally test a patch on the
> git tree on which it is to be eventually applied.
> 
> The net-next.git tree is periodically (weekly) merged with the 6.0
> release candidates where bug fixes land, but it contains newly developed
> material intended for the 6.1 release candidates (hence the "next" name).

The gap between the kernel 6.0 and the kernel vendor (5.10) used initially is
huge. Initially the 6.0 kernel didn't boot at all on the custom board I'm using
with the KSZ9896. The 6.0-rc2 kernel seemed bleeding-edge enough for upstream.

> 
> If you keep formatting development patches against the plain 6.0 release
> candidates, you may eventually run into a conflict with some other new
> development, and you may never even know.

Actually there was no conflict until the merge of the series "net: dsa:
microchip: add error handling and register access validation"

At least I need to add the .gbit_capable entry in ksz_switch_chips[].

I'm not sure about the new register validation for KSZ9896.

Best regards,
Romain

Re: [PATCH v2 1/2] net: dsa: microchip: add KSZ9896 switch support

[Vladimir Oltean]

On Thu, Sep 01, 2022 at 12:25:03AM +0200, Romain Naour wrote:
> The gap between the kernel 6.0 and the kernel vendor (5.10) used initially is
> huge. Initially the 6.0 kernel didn't boot at all on the custom board I'm using
> with the KSZ9896. The 6.0-rc2 kernel seemed bleeding-edge enough for upstream.

If you don't plan to remain forever on a BSP based on 5.10, this is why
it is preferable to have the changes required for your platform to work
submitted upstream. It should make uprevs easier, and when there are
breakages, git bisect is at your disposal.

> > If you keep formatting development patches against the plain 6.0 release
> > candidates, you may eventually run into a conflict with some other new
> > development, and you may never even know.
> 
> Actually there was no conflict until the merge of the series "net: dsa:
> microchip: add error handling and register access validation"
> 
> At least I need to add the .gbit_capable entry in ksz_switch_chips[].
> 
> I'm not sure about the new register validation for KSZ9896.

So there's another reason to retest on net-next.