linux-kernel - Re: [PATCH net-next v4 4/4] net: ftgmac100: Add RGMII delay support for AST2600

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] [day] [month] [year] [list]

Message-ID: <1c2ace4e-f3bb-4efa-a621-53c3711f46cb@lunn.ch>
Date: Wed, 26 Nov 2025 00:49:57 +0100
From: Andrew Lunn <andrew@...n.ch>
To: Jacky Chou <jacky_chou@...eedtech.com>
Cc: Andrew Lunn <andrew+netdev@...n.ch>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Rob Herring <robh@...nel.org>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>,
	Po-Yu Chuang <ratbert@...aday-tech.com>,
	Joel Stanley <joel@....id.au>,
	Andrew Jeffery <andrew@...econstruct.com.au>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-arm-kernel@...ts.infradead.org" <linux-arm-kernel@...ts.infradead.org>,
	"linux-aspeed@...ts.ozlabs.org" <linux-aspeed@...ts.ozlabs.org>,
	"taoren@...a.com" <taoren@...a.com>
Subject: Re: [PATCH net-next v4 4/4] net: ftgmac100: Add RGMII delay support
 for AST2600

> I try to summary in the following informations that I understand.
> 
> 1. with rx-internal-delay-ps OR tx-internal-delay-ps OR both
> 
>   Use "rx/tx-internal-delay-ps" property to configure RGMII delay at MAC side
>   Pass "phy-mode" to PHY driver by calling of_phy_get_and_connect()

Yes, since they are new properties, you can assume the phy-mode is
correct for these delays. We just need to watch out for DT developers
setting these delays to 2000ps and 'rgmii', which would be against the
guidelines.

> 2. withour rx-internal-delay-ps AND tx-internal-delay-ps
> 
>   If "phy-mode" is 'rgmii-rxid' or 'rgmii-txid':
> 	Keep original delay
> 	Print Warning message
> 	  "Update 'phy-mode' to rgmii-id and add 'rx/tx-internal-delay-ps'"
> 
> There are FOUR conditions in delay configuration:
> 'X' means RGMII delay setting from bootloader
> A: 7500 <= X <= 8000, 0 <= X <= 500
> B: 500 < X < 1500
> C: 1500 <= X <= 2500
> 	Mean "Enable RGMII delay" at MAC side
> D: 2500 < X < 7500
> 
>   If "phy-mode" is 'rgmii':
> 	Condition A:
> 		Keep original delay
> 		Update "phy-mode" to 'rgmii-id'
> 		Print Information message
> 			"Forced 'phy-mode' to rgmii-id"

So 0 <= X <= 500 is a small tuning value, so yes, is correct.

> 	Condition B and D
> 		Keep original delay
> 		Print Warning message
> 	  		"Update 'phy-mode' to rgmii-id and add 'rx/tx-internal-delay-ps'"

Yes.

> 	Condition C:
> 		Disable RGMII delay at MAC side
> 		Update "phy-mode" to 'rgmii-id'
> 		Print Warning message
> 	  		"Update 'phy-mode' to rgmii-id and add 'rx/tx-internal-delay-ps'"

'rx/tx-internal-delay-ps are probably not required in this case, the
2ns from the PHY is probably sufficient.

> 
>   If "phy-mode" is 'rgmii-id':
> 	Condition A:
> 		Keep original delay
> 		Keep "phy-mode" to 'rgmii-id'
> 	Condition B and D
> 		Keep original delay
> 		Print Warning message
> 	  		"Update 'phy-mode' to rgmii-id and add 'rx/tx-internal-delay-ps'"
> 	Condition C:
> 		Disable RGMII delay at MAC side
> 		Update "phy-mode" to 'rgmii-id'
> 		Print Warning message
> 	  		"Update 'phy-mode' to rgmii-id and add 'rx/tx-internal-delay-ps'"
> 

These look correct.

How many different boards do you have you can test with? Do you only
have access to RDKs? Or do you have a test farm of customer boards for
regression testing. I would throw the patchset at as many boards as
you can to make sure there are no regressions.

> Because the driver may need to update the "phy-mode" of dts, it need to add
> CONFIG_OF_DYNAMIC in ftgma100 of Kconfig.

I don't think you need this. At least, i would not patch the DT blob.

You are only fixing 2600. 2700 will be correct from day 1. You don't
need any of this code for the 2700. The 2500 also does not need any of
this, from what i have seen of the 2500. I've not looked at 2400, but
i also assume none of this is needed there.

The current ftgmac100_probe() is very complex. So i would pull it
apart into helpers. It looks like the ncsi is generic across all
versions. So that can be put into a helper. I would then probably have
helpers for 2400/2500, 2600, and sometime in the future 2700. In the
2600, i would look at replacing the of_phy_get_and_connect() with a
call to of_get_phy_mode() and of_phy_connect(), changing the interface
value passed to of_phy_connect() as needed.

	Andrew