| Message ID | 329108a3-12d6-4ce4-9b28-b59f107120ba@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | microchip/micrel switch: replace MICREL_NO_EEE workaround | expand |
On Mon, Jan 06, 2025 at 02:23:36PM +0100, Heiner Kallweit wrote: > The integrated PHY's on all these switch types have the same PHY ID. > So we can assume that the issue is related to the PHY type, not the > switch type. After having disabled EEE for this PHY type, we can remove > the workaround code here. > > Note: On the fast ethernet models listed here the integrated PHY has > PHY ID 0x00221550, which is handled by PHY driver > "Micrel KSZ87XX Switch". This PHY driver doesn't handle flag > MICREL_NO_EEE, therefore setting the flag for these models > results in a no-op. > > Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> Reviewed-by: Andrew Lunn <andrew@lunn.ch> Andrew
On Mon, Jan 06, 2025 at 02:23:36PM +0100, Heiner Kallweit wrote: > The integrated PHY's on all these switch types have the same PHY ID. > So we can assume that the issue is related to the PHY type, not the > switch type. After having disabled EEE for this PHY type, we can remove > the workaround code here. > > Note: On the fast ethernet models listed here the integrated PHY has > PHY ID 0x00221550, which is handled by PHY driver > "Micrel KSZ87XX Switch". This PHY driver doesn't handle flag > MICREL_NO_EEE, therefore setting the flag for these models > results in a no-op. Yes, it feels like no one is using KSZ87XX switches with the kernel DSA driver. > Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> > --- > drivers/net/dsa/microchip/ksz_common.c | 25 ------------------------- > include/linux/micrel_phy.h | 1 - > 2 files changed, 26 deletions(-) > > diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c > index e3512e324..4871bb1fc 100644 > --- a/drivers/net/dsa/microchip/ksz_common.c > +++ b/drivers/net/dsa/microchip/ksz_common.c > @@ -3008,31 +3008,6 @@ static u32 ksz_get_phy_flags(struct dsa_switch *ds, int port) > if (!port) > return MICREL_KSZ8_P1_ERRATA; > break; > - case KSZ8567_CHIP_ID: > - /* KSZ8567R Errata DS80000752C Module 4 */ > - case KSZ8765_CHIP_ID: > - case KSZ8794_CHIP_ID: > - case KSZ8795_CHIP_ID: > - /* KSZ879x/KSZ877x/KSZ876x Errata DS80000687C Module 2 */ > - case KSZ9477_CHIP_ID: > - /* KSZ9477S Errata DS80000754A Module 4 */ > - case KSZ9567_CHIP_ID: > - /* KSZ9567S Errata DS80000756A Module 4 */ > - case KSZ9896_CHIP_ID: > - /* KSZ9896C Errata DS80000757A Module 3 */ > - case KSZ9897_CHIP_ID: > - case LAN9646_CHIP_ID: > - /* KSZ9897R Errata DS80000758C Module 4 */ > - /* Energy Efficient Ethernet (EEE) feature select must be manually disabled > - * The EEE feature is enabled by default, but it is not fully > - * operational. It must be manually disabled through register > - * controls. If not disabled, the PHY ports can auto-negotiate > - * to enable EEE, and this feature can cause link drops when > - * linked to another device supporting EEE. > - * > - * The same item appears in the errata for all switches above. > - */ I have two problems with current patch set: - dropped documentation, not all switches are officially broken, so keeping it documented is important. - not all KSZ9xxx based switches are officially broken. All 3 port switches are not broken but still match against the KSZ9477 PHY driver: KSZ8563_CHIP_ID - 0x00221631 KSZ9563_CHIP_ID - 0x00221631 KSZ9893_CHIP_ID - 0x00221631 Best Regards, Oleksij
> I have two problems with current patch set: > - dropped documentation, not all switches are officially broken, so > keeping it documented is important. > - not all KSZ9xxx based switches are officially broken. All 3 port > switches are not broken but still match against the KSZ9477 PHY > driver: > KSZ8563_CHIP_ID - 0x00221631 > KSZ9563_CHIP_ID - 0x00221631 > KSZ9893_CHIP_ID - 0x00221631 When you say "not broken", do you mean there is text in the errata which says they do really, truly, work, or there is simply no errata which says they are broken? Do you have these 3 ports switches and have tested them? It seems odd to me that the 3 port version should work. Why is it special? Andrew
On Tue, Jan 07, 2025 at 02:03:27PM +0100, Andrew Lunn wrote: > > I have two problems with current patch set: > > - dropped documentation, not all switches are officially broken, so > > keeping it documented is important. > > - not all KSZ9xxx based switches are officially broken. All 3 port > > switches are not broken but still match against the KSZ9477 PHY > > driver: > > KSZ8563_CHIP_ID - 0x00221631 > > KSZ9563_CHIP_ID - 0x00221631 > > KSZ9893_CHIP_ID - 0x00221631 > > When you say "not broken", do you mean there is text in the errata > which says they do really, truly, work, or there is simply no errata > which says they are broken? Do you have these 3 ports switches and > have tested them? There are multiple true conditions in this case: - Documentation claims EEE is supported - Errata documentation do not recommend to disable it - I have access to this variants, they are relatively common in industrial products. So far it seems to work without issues. > It seems odd to me that the 3 port version should work. Why is it > special? In my case, on KSZ9477 the EEE starts to have issues as soon as i start to use more then 2 ports. I assume, it is switch internal power domain integration issues and not directly related to actual PHYs. May be it is possible to handle it some how or disable EEE only for some ports, or not enable for more then 2 ports, but if vendor removes EEE from documentation I see it as officially not supported.
On 07.01.2025 08:42, Oleksij Rempel wrote: > On Mon, Jan 06, 2025 at 02:23:36PM +0100, Heiner Kallweit wrote: >> The integrated PHY's on all these switch types have the same PHY ID. >> So we can assume that the issue is related to the PHY type, not the >> switch type. After having disabled EEE for this PHY type, we can remove >> the workaround code here. >> >> Note: On the fast ethernet models listed here the integrated PHY has >> PHY ID 0x00221550, which is handled by PHY driver >> "Micrel KSZ87XX Switch". This PHY driver doesn't handle flag >> MICREL_NO_EEE, therefore setting the flag for these models >> results in a no-op. > > Yes, it feels like no one is using KSZ87XX switches with the kernel DSA > driver. > >> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> >> --- >> drivers/net/dsa/microchip/ksz_common.c | 25 ------------------------- >> include/linux/micrel_phy.h | 1 - >> 2 files changed, 26 deletions(-) >> >> diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c >> index e3512e324..4871bb1fc 100644 >> --- a/drivers/net/dsa/microchip/ksz_common.c >> +++ b/drivers/net/dsa/microchip/ksz_common.c >> @@ -3008,31 +3008,6 @@ static u32 ksz_get_phy_flags(struct dsa_switch *ds, int port) >> if (!port) >> return MICREL_KSZ8_P1_ERRATA; >> break; >> - case KSZ8567_CHIP_ID: >> - /* KSZ8567R Errata DS80000752C Module 4 */ >> - case KSZ8765_CHIP_ID: >> - case KSZ8794_CHIP_ID: >> - case KSZ8795_CHIP_ID: >> - /* KSZ879x/KSZ877x/KSZ876x Errata DS80000687C Module 2 */ >> - case KSZ9477_CHIP_ID: >> - /* KSZ9477S Errata DS80000754A Module 4 */ >> - case KSZ9567_CHIP_ID: >> - /* KSZ9567S Errata DS80000756A Module 4 */ >> - case KSZ9896_CHIP_ID: >> - /* KSZ9896C Errata DS80000757A Module 3 */ >> - case KSZ9897_CHIP_ID: >> - case LAN9646_CHIP_ID: >> - /* KSZ9897R Errata DS80000758C Module 4 */ >> - /* Energy Efficient Ethernet (EEE) feature select must be manually disabled >> - * The EEE feature is enabled by default, but it is not fully >> - * operational. It must be manually disabled through register >> - * controls. If not disabled, the PHY ports can auto-negotiate >> - * to enable EEE, and this feature can cause link drops when >> - * linked to another device supporting EEE. >> - * >> - * The same item appears in the errata for all switches above. >> - */ > > I have two problems with current patch set: > - dropped documentation, not all switches are officially broken, so > keeping it documented is important. > - not all KSZ9xxx based switches are officially broken. All 3 port > switches are not broken but still match against the KSZ9477 PHY > driver: > KSZ8563_CHIP_ID - 0x00221631 > KSZ9563_CHIP_ID - 0x00221631 > KSZ9893_CHIP_ID - 0x00221631 > OK, thanks for the clarification. Then we have to go another way. > Best Regards, > Oleksij
diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c index e3512e324..4871bb1fc 100644 --- a/drivers/net/dsa/microchip/ksz_common.c +++ b/drivers/net/dsa/microchip/ksz_common.c @@ -3008,31 +3008,6 @@ static u32 ksz_get_phy_flags(struct dsa_switch *ds, int port) if (!port) return MICREL_KSZ8_P1_ERRATA; break; - case KSZ8567_CHIP_ID: - /* KSZ8567R Errata DS80000752C Module 4 */ - case KSZ8765_CHIP_ID: - case KSZ8794_CHIP_ID: - case KSZ8795_CHIP_ID: - /* KSZ879x/KSZ877x/KSZ876x Errata DS80000687C Module 2 */ - case KSZ9477_CHIP_ID: - /* KSZ9477S Errata DS80000754A Module 4 */ - case KSZ9567_CHIP_ID: - /* KSZ9567S Errata DS80000756A Module 4 */ - case KSZ9896_CHIP_ID: - /* KSZ9896C Errata DS80000757A Module 3 */ - case KSZ9897_CHIP_ID: - case LAN9646_CHIP_ID: - /* KSZ9897R Errata DS80000758C Module 4 */ - /* Energy Efficient Ethernet (EEE) feature select must be manually disabled - * The EEE feature is enabled by default, but it is not fully - * operational. It must be manually disabled through register - * controls. If not disabled, the PHY ports can auto-negotiate - * to enable EEE, and this feature can cause link drops when - * linked to another device supporting EEE. - * - * The same item appears in the errata for all switches above. - */ - return MICREL_NO_EEE; } return 0; diff --git a/include/linux/micrel_phy.h b/include/linux/micrel_phy.h index 591bf5b5e..9af01bdd8 100644 --- a/include/linux/micrel_phy.h +++ b/include/linux/micrel_phy.h @@ -44,7 +44,6 @@ #define MICREL_PHY_50MHZ_CLK BIT(0) #define MICREL_PHY_FXEN BIT(1) #define MICREL_KSZ8_P1_ERRATA BIT(2) -#define MICREL_NO_EEE BIT(3) #define MICREL_KSZ9021_EXTREG_CTRL 0xB #define MICREL_KSZ9021_EXTREG_DATA_WRITE 0xC
The integrated PHY's on all these switch types have the same PHY ID. So we can assume that the issue is related to the PHY type, not the switch type. After having disabled EEE for this PHY type, we can remove the workaround code here. Note: On the fast ethernet models listed here the integrated PHY has PHY ID 0x00221550, which is handled by PHY driver "Micrel KSZ87XX Switch". This PHY driver doesn't handle flag MICREL_NO_EEE, therefore setting the flag for these models results in a no-op. Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> --- drivers/net/dsa/microchip/ksz_common.c | 25 ------------------------- include/linux/micrel_phy.h | 1 - 2 files changed, 26 deletions(-)