netdev - 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: <c0b653ea-3fe0-4bdb-9681-bf4e3ef1364a@linux.ibm.com>
Date: Thu, 9 Jan 2025 08:25:28 -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,

On 1/9/25 07:21, Andrew Lunn wrote:
> On Thu, Jan 09, 2025 at 10:33:20AM +0000, Jacky Chou wrote:
>> Hi Andrew,
>>
>>>> There are around 11 boards in Aspeed SOC with phy-mode set to "rgmii"
>>>> (some of them are mac0&1 and others are mac2&3). "rgmii-rxid" is only
>>> mine.
>>>> No one in aspeed SOC using "rgmii-id".
>>> O.K, so we have to be careful how we fix this. But the fact they are all equally
>>> broken might help here.
>>>
>>>>> Humm, interesting. Looking at ftgmac100.c, i don't see where you
>>>>> configure the RGMII delays in the MAC?
>>> This is going to be important. How are delays configured if they are not in the
>>> MAC driver?
>> The RGMII delay is adjusted on clk-ast2600 driver. Please refer to the following link.
>> https://github.com/AspeedTech-BMC/linux/blob/f52a0cf7c475dc576482db46759e2d854c1f36e4/drivers/clk/clk-ast2600.c#L1008
> O.K. So in your vendor tree, you have additional DT properties
> mac1-clk-delay, mac2-clk-delay, mac3-clk-delay. Which is fine, you can
> do whatever you want in your vendor tree, it is all open source.
>
> But for mainline, this will not be accepted. We have standard
> properties defined for configuring MAC delays in picoseconds:
>
>          rx-internal-delay-ps:
>            description:
>              RGMII Receive Clock Delay defined in pico seconds. This is used for
>              controllers that have configurable RX internal delays. If this
>              property is present then the MAC applies the RX delay.
>          tx-internal-delay-ps:
>            description:
>              RGMII Transmit Clock Delay defined in pico seconds. This is used for
>              controllers that have configurable TX internal delays. If this
>              property is present then the MAC applies the TX delay.
>
>
> You need to use these, and in the MAC driver, not a clock driver. That
> is also part of the issue. Your MAC driver looks correct, it just
> silently passes phy-mode to the PHY just like every other MAC
> driver. But you have some code hidden away in the clock controller
> which adds the delays. If this was in the MAC driver, where it should
> be, this broken behaviour would of been found earlier.
>
> So, looking at mainline, i see where you create a gated clock. But
> what i do not see is where you set the delays.
>
> How does this work in mainline? Is there more hidden code somewhere
> setting the ASPEED_MAC12_CLK_DLY register?

I think the code already exist in the mainline: 
https://github.com/torvalds/linux/blob/master/drivers/clk/clk-ast2600.c#L595

It is configuring SCU register in the ast2600 SOC to introduce delays. 
The mac is part of the SOC.

Regards,

Ninad

>
> 	Andrew