linux-kernel - Re: 回覆: 回覆: [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: <81567190-a683-4542-a530-0fb419f5f9be@linux.ibm.com>
Date: Fri, 10 Jan 2025 08:54:46 -0600
From: Ninad Palsule <ninad@...ux.ibm.com>
To: Andrew Lunn <andrew@...n.ch>, Jacky Chou <jacky_chou@...eedtech.com>
Cc: "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: Re: 回覆: 回覆: [PATCH v2 05/10] ARM: dts: aspeed: system1: Add RGMII support

Hello Andrew & Jacky,

On 1/10/25 08:04, Andrew Lunn wrote:
>> 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.
>
> 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?

Jacky,

Here are the values on my system and those are expected that means 
u-boot is setting correct value?

# devmem 0x1E6E2340 32
0x9028A410
# devmem 0x1E6E2348 32
0x00410410
# devmem 0x1E6E234c 32
0x00410410

# devmem 0x1E6E2350 32
0x40104145
# devmem 0x1E6E2358 32
0x00104145
# devmem 0x1E6E235c 32
0x00104145

>
> 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.

Andrew,

Do we need updates on this description. It doesn't talk about external 
PCB level delays?

https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/net/ethernet-controller.yaml#L77-L90

This is what you explained:

MAC driver reads following phy-mode from device tree. 95% of mac driver 
directly
  pass it to PHY through phy_connect.

rgmii - PCB has long clock lines so delay is added by PCB
         On this mode PHY does nothing.
rgmii-id - PCB doesn't add delay. Either MAC or PHY needs to add the delay
            Add delays in both directions. Some PHY may not add delay in 
that
            case MAC needs to add the delay mask rgmii-id to rgmii.
rgmii-rxid - If there is an extra long TX clock line, but not RX clock,
              you would use rgmii-rxid
rgmii-txid - When there is an extra long RX clock line on the PCB, but not
              the TX clock line, you would use rgmii-txid

Regards,

Ninad

>
> 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.
>
> 	Andrew
>