netdev - 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: <5100ab9d-1b70-46fb-b3ba-d4bcff6d6870@microchip.com>
Date: Tue, 16 Apr 2024 11:02:47 +0000
From: <Parthiban.Veerasooran@...rochip.com>
To: <andrew@...n.ch>
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

Hi Andrew,

On 15/04/24 6:45 pm, Andrew Lunn wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> 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.
> 
Thanks for confirming my understanding.
> 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.
> 
OK.
> 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.
> 
I tried this approach and it works as expected. Means whenever there is 
a c45 register access, it directly uses the 
oa_tc6_read_c45()/oa_tc6_write_c45() functions. Herewith I have attached 
the patch 
(v4-0006-net-ethernet-oa_tc6-implement-internal-PHY-initia.patch) which 
has this new implementation for your reference. Is this you expected? 
Can you comment on this?
> 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.
> 
I tried this approach by setting up is_c45 flag when I use 
phy_read_mmd() function but ended up with the kernel call trace 
(c45_kernel_call_trace.png) attached here for your reference.
> We really do need to separate C45 registers from C45 protocol in the
> phylib core to cleanly solve this.
OK. In my opinion, until having the above implementation in the phylib 
core, shall we go with the first approach [1] I tried out as it is 
fulfilling our requirement? What do you think?

Best regards,
Parthiban V
> 
>         Andrew
> 

Download attachment "c45_kernel_call_trace.png" of type "image/png" (226309 bytes)

View attachment "v4-0006-net-ethernet-oa_tc6-implement-internal-PHY-initia.patch" of type "text/x-patch" (11260 bytes)