Hi Lukasz,

On Tue, Aug 29, 2023 at 10:35:33AM +0200, Lukasz Majewski wrote:

Hi Vladimir,

quoted

On Fri, Aug 25, 2023 at 06:48:41PM +0000, Tristram.Ha@microchip.com
wrote:

quoted

quoted

quoted

IMHO adding functions to MMD modification would facilitate
further development (for example LED setup).  

We already have some KSZ9477 specific initialization done in the
Micrel PHY driver under drivers/net/phy/micrel.c, can we converge
on the PHY driver which has a reasonable amount of infrastructure
for dealing with workarounds, indirect or direct MMD accesses
etc.?  

Actually the internal PHY used in the KSZ9897/KSZ9477/KSZ9893
switches are special and only used inside those switches.  Putting
all the switch related code in Micrel PHY driver does not really
help.  When the switch is reset all those PHY registers need to be
set again, but the PHY driver only executes those code during PHY
initialization.  I do not know if there is a good way to tell the
PHY to re-initialize again.  

Suppose there was a method to tell the PHY driver to re-initialize
itself. What would be the key points in which the DSA switch driver
would need to trigger that method? Where is the switch reset at
runtime?

Tristam has explained why adding the internal switch PHY errata to
generic PHY code is not optimal.

Yes, and I didn't understand that explanation, so I asked a
clarification question.

If adding MMD generic code is a problem - then I'm fine with just
clearing proper bits with just two indirect writes in the
drivers/net/dsa/microchip/ksz9477.c

I would also prefer to keep the separate ksz9477_errata() function, so
we could add other errata code there.

Just informative - without this patch the KSZ9477-EVB board's network
is useless when the other peer has EEE enabled by default (like almost
all non managed ETH switches).

No, adding direct PHY MMD access code to the ksz9477 switch driver is
not even the biggest problem - even though, IIUC, the "workaround" to
disable EEE advertisement could be moved to ksz9477_get_features() in
drivers/net/phy/micrel.c, where phydev->supported_eee could be cleared.

The biggest problem that I see is that Oleksij Rempel has "just" added
EEE support to the KSZ9477 earlier this year, with an ack from Arun
Ramadoss: 69d3b36ca045 ("net: dsa: microchip: enable EEE support").
I'm not understanding why the erratum wasn't a discussion topic then.

I am currently on vacation and won't be able to look very deeply into
the problem, but IIUC, your patch undoes that work, and so, it needs an
ACK from Oleksij.