linux-kernel - 回覆: 回覆: 回覆: [PATCH v2 05/10] ARM: dts: aspeed: system1: Add RGMII support

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:
 <SEYPR06MB5134D71B324B6A0094ED45419D1F2@SEYPR06MB5134.apcprd06.prod.outlook.com>
Date: Mon, 13 Jan 2025 06:18:42 +0000
From: Jacky Chou <jacky_chou@...eedtech.com>
To: Andrew Lunn <andrew@...n.ch>
CC: Ninad Palsule <ninad@...ux.ibm.com>, "andrew+netdev@...n.ch"
	<andrew+netdev@...n.ch>, "andrew@...econstruct.com.au"
	<andrew@...econstruct.com.au>, "conor+dt@...nel.org" <conor+dt@...nel.org>,
	"davem@...emloft.net" <davem@...emloft.net>, "devicetree@...r.kernel.org"
	<devicetree@...r.kernel.org>, "eajames@...ux.ibm.com"
	<eajames@...ux.ibm.com>, "edumazet@...gle.com" <edumazet@...gle.com>,
	"joel@....id.au" <joel@....id.au>, "krzk+dt@...nel.org" <krzk+dt@...nel.org>,
	"kuba@...nel.org" <kuba@...nel.org>, "linux-arm-kernel@...ts.infradead.org"
	<linux-arm-kernel@...ts.infradead.org>, "linux-aspeed@...ts.ozlabs.org"
	<linux-aspeed@...ts.ozlabs.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "minyard@....org" <minyard@....org>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"openipmi-developer@...ts.sourceforge.net"
	<openipmi-developer@...ts.sourceforge.net>, "pabeni@...hat.com"
	<pabeni@...hat.com>, "ratbert@...aday-tech.com" <ratbert@...aday-tech.com>,
	"robh@...nel.org" <robh@...nel.org>
Subject:
 回覆: 回覆: 回覆: [PATCH v2 05/10] ARM: dts: aspeed: system1: Add RGMII support

Hi Andrew

Thank you for your reply.

> > Agree. You are right. This part is used to create a gated clock.
> > We will configure these RGMII delay in bootloader like U-boot.
> > Therefore, here does not configure delay again.
> 
> > Because AST2600 MAC1/2 RGMII delay setting in scu region is combined
> > to one 32-bit register,
> > MAC3/4 is also. I will also use 'aliase' to get MAC index to set delay in scu.
> >
> > // aspeed-g6.dtsi
> > aliases {
> > 		..........
> > 		mac0 = &mac0;
> > 		mac1 = &mac1;
> > 		mac2 = &mac2;
> > 		mac4 = &mac3;
> > 	};
> 
> I would avoid that, because they are under control of the DT developer. You
> sometimes seen the order changed in the hope of changing the interface
> names, rather than use a udev script, or systemd naming scheme.
> 
> The physical address of each interface is well known and fixed? Are they the
> same for all ASTxxxx devices? I would hard code them into the driver to
> identify the instance.

The physical address of each interface is all different in all aspeed device.
And they are fixed and known. I can use the address to distinguish the interface.

> 
> But first we need to fix what is broken with the existing DT phy-modes etc.
> 
> What is the reset default of these SCU registers? 0? So we can tell if the
> bootloader has modified it and inserted a delay?
> 
> What i think you need to do is during probe of the MAC driver, compare
> phy-mode and how the delays are configured in hardware. If the delays in
> hardware are 0, assume phy-mode is correct and use it. If the delays are not 0,
> compare them with phy-mode. If the delays and phy-mode agree, use them. If
> they disagree, assume phy-mode is wrong, issue a dev_warn() that the DT blob
> is out of date, and modify phy-mode to match the delays in the hardware,
> including a good explanation of what is going on in the commit message to
> help those with out of tree DT files. And then patch all the in tree DT files to
> use the correct phy-mode.

Agree. I think this method is good. The RGMII delay reset value in SCU is zero.
I can use the reset value to know if the RGMII delay is configured.
If the values are not match the phy-mode, print warning message and apply the 
RGMII delay property if there is in MAC node of dts.

> 
> Please double check my logic, just to make sure it is correct. If i have it correct,
> it should be backwards compatible. The one feature you loose out on is when
> the bootloader sets the wrong delays and you want phy-mode to actually
> override it.
> 
> When AST2700 comes along, you can skip all this, get it right from the start
> and not need this workaround.

Thanks for your friendly reminder.

Thanks,
Jacky