Message-ID: <5a1c5a4a-284e-47c6-af6f-cd95ac08b680@alliedtelesis.co.nz>
Date: Thu, 19 Jun 2025 02:47:57 +0000
From: Chris Packham <Chris.Packham@...iedtelesis.co.nz>
To: "markus.stockhausen@....de" <markus.stockhausen@....de>, 'Andrew Lunn'
	<andrew@...n.ch>
CC: "hkallweit1@...il.com" <hkallweit1@...il.com>, "linux@...linux.org.uk"
	<linux@...linux.org.uk>, "davem@...emloft.net" <davem@...emloft.net>,
	"edumazet@...gle.com" <edumazet@...gle.com>, "kuba@...nel.org"
	<kuba@...nel.org>, "pabeni@...hat.com" <pabeni@...hat.com>,
	"michael@...sekall.de" <michael@...sekall.de>, "daniel@...rotopia.org"
	<daniel@...rotopia.org>, "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: AW: [PATCH] net: phy: realtek: convert RTL8226-CG to c45 only


On 19/06/2025 09:27, Chris Packham wrote:
> Hi Markus,
>
> On 18/06/2025 18:03, markus.stockhausen@....de wrote:
>>> Von: Andrew Lunn <andrew@...n.ch>
>>> Gesendet: Dienstag, 17. Juni 2025 17:36
>>> An: Markus Stockhausen <markus.stockhausen@....de>
>>> Betreff: Re: [PATCH] net: phy: realtek: convert RTL8226-CG to c45 only
>>>
>>> On Tue, Jun 17, 2025 at 11:01:47AM -0400, Markus Stockhausen wrote:
>>>> The RTL8226-CG can be found on devices like the Zyxel XGS1210-12. 
>>>> These
>>>> are driven by a RTL9302B SoC that has active phy hardware polling in
>>>> the background.
>>> It would be a lot better to just turn that polling off.
> I have experimented with turning the polling off on my board but it 
> tends to make other switch things not work. The obvious one is the 
> LEDs but I also saw other switching functionality (e.g. packet 
> forwarding, fdb maintenance) stop working.
>> This is our challenge: 
>> https://elixir.bootlin.com/linux/v6.16-rc2/source/drivers/net/mdio/mdio-realtek-rtl9300.c#L366
>
> To be honest I'd never considered dynamically switching between c22 
> and c45. I've seen mdio controllers busses that can put in either mode 
> as a pinctrl type of operation and if seen PHYs that can be accessed 
> as either c22 or c45 and even support for c45 over c22. But I'd never 
> seen anything that needed to support both at the same time. The only 
> reason I put the restriction in was I didn't want the switch phy 
> polling unit interfering. I also don't have any (working [1]) Realtek 
> hardware with a c22 PHY to test on.
>
> I think that perhaps we don't need this restriction as the transfer 
> type is set via SMI_ACCESS_PHY_CTRL_1. I think we would however need 
> to reconcile the other feedback you had around tracking the page 
> number for the c22 accesses which has not been implemented.
>
> [1] - I do have a Zyxel board that will probably do the trick but so 
> far I've been unable to get it booting my images. I want to avoid 
> messing up the stock firmware so I haven't tried anything too invasive.
>
So I did another check. If I clear INTF_SEL bits in SMI_GLB_CTRL the 
switch will not detect the link status correctly. C45 MDIO access from 
the kernel seems to work regardless.

This is using the Realtek u-boot to do some HW init and my as yet 
unpublished switchdev driver for the RTL9300. Something somewhere needs 
to configure SMI_GLB_CTRL so the switch will get the port link status 
correctly. It doesn't have to be the mdio driver, if I remove that code 
completely everything still works (it's using the SMI_GLB_CTRL value 
that has been put there by Realtek's U-Boot).

>>>> As soon as this is active and set to c45 most c22
>>>> register accesses are blocked and will stop working. Convert the
>>>> phy to a c45-only function set.
>>>>
>>>> For documentation purposes some register extracts that where taken to
>>>> verify proper detection.
>>> Please could you show us the output from ethtool before/after.
>>>
>>>> PHY_ID_MATCH_EXACT(0x001cc838),
>>>>           .name           = "RTL8226-CG 2.5Gbps PHY",
>>>> -        .get_features   = rtl822x_get_features,
>>> You can see this calls genphy_read_abilities(phydev) at the end, so
>>> reading information about 10/100/1G speeds, using the standard C22
>>> registers.
>>>
>>>> -        .config_aneg    = rtl822x_config_aneg,
>>>> -        .read_status    = rtl822x_read_status,
>>>> -        .suspend        = genphy_suspend,
>>>> -        .resume         = rtlgen_resume,
>>>> +        .soft_reset     = rtl822x_c45_soft_reset,
>>>> +        .get_features   = rtl822x_c45_get_features,
>>> This only calls genphy_c45_pma_read_abilities(). So i expect 10/100/1G
>>> is missing.
>> I had to patch the mdio driver to make the existing RTL8226 phy 
>> driver work
>> with
>> It. So whenever a c22 command is sent it will toggle the protocol. I 
>> do not
>> believe
>> that this is what it was designed for but maybe Chris has some more
>> experience.
>
> I don't know one way or the other if swapping between c22 and c45 is a 
> thing that the hardware allows. It's not something the driver I wrote 
> supports but it could possibly be added by removing the check and 
> unconditionally supplying the required read/write functions. To play 
> nicely with the PPU it would need to track the PHY page accesses like 
> the openWRT implementation does.
>
>>
>> Output with patched bus:
>>
>> [   49.552627] toggle bus 1 from c45 to c22 to read port 24 page 0 
>> register
>> 1
>> [   49.560663] toggle bus 1 from c22 to c45
>> ...
>> # ethtool lan9
>> Settings for lan9:
>>          Supported ports: [ TP MII ]
>>          Supported link modes:   10baseT/Half 10baseT/Full
>>                                  100baseT/Half 100baseT/Full
>>                                  1000baseT/Full
>>                                  2500baseT/Full
>>          Supported pause frame use: Symmetric Receive-only
>>          Supports auto-negotiation: Yes
>>          Supported FEC modes: Not reported
>>          Advertised link modes:  10baseT/Half 10baseT/Full
>>                                  100baseT/Half 100baseT/Full
>>                                  1000baseT/Full
>>                                  2500baseT/Full
>>          Advertised pause frame use: Symmetric Receive-only
>>          Advertised auto-negotiation: Yes
>>          Advertised FEC modes: Not reported
>>          Speed: Unknown!
>>          Duplex: Unknown! (255)
>>          Port: Twisted Pair
>>          PHYAD: 24
>>          Transceiver: external
>>          Auto-negotiation: on
>>          MDI-X: Unknown
>>          Supports Wake-on: d
>>          Wake-on: d
>>          Link detected: no
>>
>> The RTL8226 seems to support proper MDIO_PMA_EXTABLE flags.
>> So genphy_c45_pma_read_abilities() can conveniently call
>> genphy_c45_pma_read_ext_abilities() and 10/100/1000 is
>> populated right.
>>
>> Outputs with patched driver:
>>
>> # ethtool lan9
>> Settings for lan9:
>>          Supported ports: [ TP ]
>>          Supported link modes:   10baseT/Half 10baseT/Full
>>                                  100baseT/Half 100baseT/Full
>>                                  1000baseT/Full
>>                                  2500baseT/Full
>>          Supported pause frame use: Symmetric Receive-only
>>          Supports auto-negotiation: Yes
>>          Supported FEC modes: Not reported
>>          Advertised link modes:  10baseT/Half 10baseT/Full
>>                                  100baseT/Half 100baseT/Full
>>                                  1000baseT/Full
>>                                  2500baseT/Full
>>          Advertised pause frame use: Symmetric Receive-only
>>          Advertised auto-negotiation: Yes
>>          Advertised FEC modes: Not reported
>>          Speed: Unknown!
>>          Duplex: Unknown! (255)
>>          Port: Twisted Pair
>>          PHYAD: 24
>>          Transceiver: external
>>          Auto-negotiation: on
>>          MDI-X: Unknown
>>          Supports Wake-on: d
>>          Wake-on: d
>>          Link detected: no
>>
>> Markus

Powered by blists - more mailing lists