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

[Lukasz Majewski <lukma@denx.de>]

[-- Attachment #1: Type: text/plain, Size: 2865 bytes --]

Hi Oleksij,

> On Wed, Aug 30, 2023 at 01:51:51PM +0200, Lukasz Majewski wrote:
> > Hi Oleksij,  
> 
> > It looks like the most optimal solution would be the one proposed by
> > Tristam:
> > https://www.spinics.net/lists/netdev/msg932044.html  
> 
> In this case, please add the reason why it would work on this HW and
> will not break by any changes in PHYlib or micrel.c driver.
> 

The ksz9477_config_cpu_port() is called from ksz_setup. In this
function we would clear 7.60 MMD register for EEE advertisement.

Only after the switch initialization, the phy code reads 7.60 register
for each port and on that basis decides if the EEE is supported or not.

(And only then ksz9477_get_features() is executed. Finally
ksz_get_phy_flags() is called.

> If I remember it correctly, in KSZ9477 variants, if you write to EEE
> advertisement register, it will affect the state of a EEE capability
> register. Which break IEEE 802.3 specification and the reason why
> ksz9477_get_features() actually exist. But can be used as workaround
> if it is written early enough before PHYlib tried to read EEE
> capability register.
> 
> Please confirm my assumption by applying your workaround and testing
> it with ethtool --show-eee lanX.
> 

# ethtool --show-eee lan1
EEE Settings for lan1:
        EEE status: disabled
        Tx LPI: 0 (us)
        Supported EEE link modes:  100baseT/Full 
                                   1000baseT/Full 
        Advertised EEE link modes:  Not reported
        Link partner advertised EEE link modes:  Not reported
# 
# ethtool --show-eee lan2
EEE Settings for lan2:
        EEE status: disabled
        Tx LPI: 0 (us)
        Supported EEE link modes:  100baseT/Full 
                                   1000baseT/Full 
        Advertised EEE link modes:  Not reported
        Link partner advertised EEE link modes:  Not reported


> It should be commented in the code with all kind of warnings:
> Don't move!!! We use one bug to workaround another bug!!!

As I've pointed out in the previous e-mail. This kind of bug cannot be
easily fixed with modifying flags in ksz_get_phy_flags() as this
function is called after ksz9477_get_features().

I'm open for ideas to do it right...

> If PHYlib
> start scanning PHYs before this code is executed, then thing may
> break!!

Is it possible that PHY lib will start scanning for phys before the DSA
switch IC is probed with SPI?

KSZ9477-EVB DTS is not using "mdio" node - i.e. the "old" scheme for DSA
switch is used.

> 
> ... it is broken as hell....
> 


Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]