| Message ID | 20220825213943.2342050-1-romain.naour@smile.fr (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [1/2] net: dsa: microchip: add KSZ9896 switch support | expand |
On Thu, Aug 25, 2022 at 11:39:42PM +0200, Romain Naour wrote: > 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> > Signed-off-by: Romain Naour <romain.naour@smile.fr> Two signed-off-by from the same person is unusual :-) > --- > It seems that the KSZ9896 support has been sent to the kernel netdev > mailing list a while ago but got lost after initial review: > https://www.spinics.net/lists/netdev/msg554771.html I'm not sure saying it got lost is true. It looks more like the issues pointed out were never addressed. > 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. This looks sufficiently different that i don't think we need Tristram's Signed-off-by as well. I don't know these chips well enough to do a detailed review. Andrew
Hi, Le 26/08/2022 à 00:04, Andrew Lunn a écrit : > On Thu, Aug 25, 2022 at 11:39:42PM +0200, Romain Naour wrote: >> 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> >> Signed-off-by: Romain Naour <romain.naour@smile.fr> > > Two signed-off-by from the same person is unusual :-) Indeed, but my customer (skf) asked me to use the skf.com address for the patch but I use my smile.fr (my employer) git/email setup for mailing list. > >> --- >> It seems that the KSZ9896 support has been sent to the kernel netdev >> mailing list a while ago but got lost after initial review: >> https://www.spinics.net/lists/netdev/msg554771.html > > I'm not sure saying it got lost is true. It looks more like the issues > pointed out were never addressed. It seems the initial KSZ9896 support was in the same patch with other changes that were addressed later by followup patches. > >> 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. > > This looks sufficiently different that i don't think we need > Tristram's Signed-off-by as well. > > I don't know these chips well enough to do a detailed review. Thanks for your feedback! Best regards, Romain > > Andrew
On Fri, 26 Aug 2022 09:31:00 +0200 Romain Naour wrote: > >> Signed-off-by: Romain Naour <romain.naour@skf.com> > >> Signed-off-by: Romain Naour <romain.naour@smile.fr> > > > > Two signed-off-by from the same person is unusual :-) > > Indeed, but my customer (skf) asked me to use the skf.com address for the patch > but I use my smile.fr (my employer) git/email setup for mailing list. It's pretty common to use a different email server than what's on the From and S-o-b lines. You can drop the second S-o-b.
Hi, Le 27/08/2022 à 02:49, Jakub Kicinski a écrit : > On Fri, 26 Aug 2022 09:31:00 +0200 Romain Naour wrote: >>>> Signed-off-by: Romain Naour <romain.naour@skf.com> >>>> Signed-off-by: Romain Naour <romain.naour@smile.fr> >>> >>> Two signed-off-by from the same person is unusual :-) >> >> Indeed, but my customer (skf) asked me to use the skf.com address for the patch >> but I use my smile.fr (my employer) git/email setup for mailing list. > > It's pretty common to use a different email server than what's > on the From and S-o-b lines. You can drop the second S-o-b. Thanks for clarification. Should resend the series now or wait for a review? Best regards, Romain
On Mon, 29 Aug 2022 09:20:06 +0200 Romain Naour wrote: > > It's pretty common to use a different email server than what's > > on the From and S-o-b lines. You can drop the second S-o-b. > > Thanks for clarification. > > Should resend the series now or wait for a review? Resend, please. Looking quickly at the code it's interesting to see the compatible already listed in the DT schema but the driver not supporting it. Is this common in the embedded world? That's orthogonal to your patch, I'm just curious.
Hi, Le 30/08/2022 à 07:07, Jakub Kicinski a écrit : > On Mon, 29 Aug 2022 09:20:06 +0200 Romain Naour wrote: >>> It's pretty common to use a different email server than what's >>> on the From and S-o-b lines. You can drop the second S-o-b. >> >> Thanks for clarification. >> >> Should resend the series now or wait for a review? > > Resend, please. ok. > > Looking quickly at the code it's interesting to see the compatible > already listed in the DT schema but the driver not supporting it. > Is this common in the embedded world? > > That's orthogonal to your patch, I'm just curious. I was surprised too about the compatible being listed in the DT schema but being missing in the driver. AFAIK, the DT schema was merged in 2019 (kernel 5.1) but the corresponding driver patch didn't pass the review and no further patch was sent on the mailing list for the KSZ9896. It's true that the KSZ9896 is really close to the KSZ9897 (only one CPU port is missing) but the driver reject this device due to a mismatch in the check between the device id and the compatible. I tried to workaround this check but the driver really need to know how many port are really present on the ethernet switch otherwise there is a kernel oops. Best regards, Romain
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");