Re: [PATCH 2/2] net: dsa: microchip: Provide Module 4 KSZ9477 errata (DS80000754C)

From: Oleksij Rempel
Date: Tue Aug 29 2023 - 10:43:28 EST


On Tue, Aug 29, 2023 at 02:38:29PM +0200, Lukasz Majewski wrote:
> Hi Oleksij,

...

> Hence, I would prefer to apply the Errata and then somebody, who would
> like to enable EEE can try if it works for him.

ok.

> IMHO, code to fix erratas shall be added unconditionally, without any
> "freedom of choic

This claim is not consistent with the patch. To make it without ability
to enable EEE, you will need to clear all eee_supported bits.
If this HW is really so broken, then it is the we how it should be
fixed.

> > Beside, are you able to reproduce this issue?
>
> Yes, I can reproduce the issue. I do use two Microchip's development
> boards (KSZ9477-EVB [1]) connected together to test HSR as well as
> communication with HOST PC.

I use KSZ9477-EVB as well.

> The network on this board without this patch is not usable (continually
> I do encounter link up/downs).

My test setup runs currently about two hours. It had 4 link drops on LAN3 and
none on other ports. Swapping cables connected to LAN2 and LAN3 still let the
LAN3 sometimes drop the connection. So far, for example LAN2 works stable and
this is probably the reason why I have not seen this issue before.
After disabling EEE on LAN3 I start getting drops on LAN2.

> Please be also aware, that this errata fix is (implicitly I think)
> already present in the kernel:
> https://elixir.bootlin.com/linux/latest/source/drivers/net/phy/micrel.c#L1804
>
> However, the execution order of PHY/DSA functions with newest mainline
> makes it not working any more (I've described it in details in the
> earlier mail to Vladimir).

Ok, since it was already not advertised by default, I have nothing against
having default policy to not advertise EEE for this switch.

On other hand, since this functionality is not listed as supported by the
KSZ9477 datasheet (No word about IEEE 802.3az Energy Efficient Ethernet (EEE))
compared to KSZ8565R datasheet (where EEE support is listed) and it is
confirmed to work not stable enough, then it should be disabled properly.
The phydev->supported_eee should be cleared. See ksz9477_get_features().


Regards,
Oleksij
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |