linux-kernel - Re: [PATCH net-next v3 06/12] net: ethernet: oa_tc6: implement internal PHY initialization

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <3fc3b5c3-0750-4aff-ab26-240f4bc55236@lunn.ch>
Date: Mon, 15 Apr 2024 15:15:31 +0200
From: Andrew Lunn <andrew@...n.ch>
To: Parthiban.Veerasooran@...rochip.com
Cc: davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org,
	pabeni@...hat.com, horms@...nel.org, saeedm@...dia.com,
	anthony.l.nguyen@...el.com, netdev@...r.kernel.org,
	linux-kernel@...r.kernel.org, corbet@....net,
	linux-doc@...r.kernel.org, robh+dt@...nel.org,
	krzysztof.kozlowski+dt@...aro.org, conor+dt@...nel.org,
	devicetree@...r.kernel.org, Horatiu.Vultur@...rochip.com,
	ruanjinjie@...wei.com, Steen.Hegelund@...rochip.com,
	vladimir.oltean@....com, UNGLinuxDriver@...rochip.com,
	Thorsten.Kummermehr@...rochip.com, Pier.Beruto@...emi.com,
	Selvamani.Rajagopal@...emi.com, Nicolas.Ferre@...rochip.com,
	benjamin.bigler@...nformulastudent.ch
Subject: Re: [PATCH net-next v3 06/12] net: ethernet: oa_tc6: implement
 internal PHY initialization

On Fri, Apr 12, 2024 at 10:43:15AM +0000, Parthiban.Veerasooran@...rochip.com wrote:
> Hi Andrew,
> 
> After implementing the new APIs oa_tc6_mdiobus_read_c45() and 
> oa_tc6_mdiobus_write_c45(), I tried testing. But, for some reason these 
> APIs are never get called by phy_read_mmd() or phy_write_mmd() as those 
> APIs are checking for phydev->is_c45 flag for calling this new c45 APIs 
> which is not set in this case.
> 
> If the phydev is found via c22, phylib does not set phydev->is_c45, and 
> everything ends up going indirect.

We don't have a clean separation between C22/C45 register space and
C22/C45 MDIO bus protocols. As you said, if the PHY is discovered via
C22 bus protocol it assumes it uses C22 protocol.

Those PHYs which do support C45, and in particular, features which
need C45, all call genphy_c45_pma_read_abilities() in there
get_features function to add in C45 features. However, it will
continue using C45 over C22 because genphy_c45_pma_read_abilities()
only really says the device has C45 registers, not C45 protocol.

I can see two different things you can try.

1) set .read_mmd and .write_mmd in the PHY driver to phy_read_mmd()
   and phy_write_mmd(). That is not great however, since each vendor
   is likely to have their own PHY driver.

2) Add a helper to set is_c45. This however has side effects you might
   not want. e.g. auto-neg will be performed via C45, not C22. This
   might not matter for a T1 PHY where is think auto-neg is not
   actually used. But i don't see anything in TC6 which limits it to
   T1, i could well imagine a T2 or T4 PHY being used with full
   auto-neg.

We really do need to separate C45 registers from C45 protocol in the
phylib core to cleanly solve this.

       Andrew