Message-ID: <CAP_9mL6utQjN_2EZ4vs3K8jzcxHxvKWNTNEXZ9fAx4HuA=DNXA@mail.gmail.com>
Date: Wed, 8 Oct 2025 01:29:57 +0800
From: jjm2473 <jjm2473@...il.com>
To: Andrew Lunn <andrew@...n.ch>
Cc: robh@...nel.org, krzk+dt@...nel.org, conor+dt@...nel.org, heiko@...ech.de, 
	quentin.schulz@...rry.de, kever.yang@...k-chips.com, naoki@...xa.com, 
	honyuenkwun@...il.com, inindev@...il.com, ivan8215145640@...il.com, 
	neil.armstrong@...aro.org, mani@...nel.org, dsimic@...jaro.org, 
	pbrobinson@...il.com, alchark@...il.com, didi.debian@...ow.org, 
	jbx6244@...il.com, devicetree@...r.kernel.org, 
	linux-arm-kernel@...ts.infradead.org, linux-rockchip@...ts.infradead.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 3/3] arm64: dts: rockchip: add LinkEase EasePi R1

Andrew Lunn <andrew@...n.ch> 于2025年10月7日周二 22:57写道：
>
> On Tue, Oct 07, 2025 at 10:32:26PM +0800, jjm2473 wrote:
> > Andrew Lunn <andrew@...n.ch> 于2025年10月6日周一 23:51写道：
> > >
> > > > +&gmac0 {
> > > > +     phy-mode = "rgmii";
> > >
> > > Did i really miss this patch series in its earlier version, or did you
> > > ignore me?
> > >
> > > https://elixir.bootlin.com/linux/v6.15/source/Documentation/devicetree/bindings/net/ethernet-controller.yaml#L287
> > >
> > > > +     tx_delay = <0x3c>;
> > > > +     rx_delay = <0x2f>;
> > >
> > > Please change it to rgmii-id, and smaller tx/rx_delay values. Or show
> > > us the schematics which clearly show extra long clock lines.
> > >
> > > > +/* Micro SD card slot is not mounted */
> > > > +&sdmmc0 {
> > > > +     max-frequency = <150000000>;
> > > > +     no-sdio;
> > > > +     no-mmc;
> > > > +     bus-width = <4>;
> > > > +     cap-mmc-highspeed;
> > > > +     cap-sd-highspeed;
> > > > +     disable-wp;
> > > > +     pinctrl-names = "default";
> > > > +     pinctrl-0 = <&sdmmc0_bus4 &sdmmc0_clk &sdmmc0_cmd &sdmmc0_det>;
> > > > +     vmmc-supply = <&vcc3v3_sd>;
> > > > +     vqmmc-supply = <&vccio_sd>;
> > > > +     status = "disabled";
> > > > +};
> > > > +
> > > > +/* Wifi module is not mounted */
> > > > +&sdmmc2 {
> > >
> > > What do you mean by "not mounted"?
> > >
> > > Often you would say "not populated", to indicate the PCB has all the
> > > tracks in place, but the chip has simply not been soldered in place.
> > >
> > > Or is there a connector here, and nothing plugged into the connector?
> > >
> > >    Andrew
> >
> > Andrew:
> >  Hello! I ran `./scripts/get_maintainer.pl
> > patches-v4/v4-0003-arm64-dts-rockchip-add-LinkEase-EasePi-R1.patch`
> > to get maintainer list, and got:
> > ```
> > Rob Herring <robh@...nel.org> (maintainer:OPEN FIRMWARE AND FLATTENED
> > DEVICE TREE BINDINGS)
> > Krzysztof Kozlowski <krzk+dt@...nel.org> (maintainer:OPEN FIRMWARE AND
> > FLATTENED DEVICE TREE BINDINGS,commit_signer:3/41=7%)
> > Conor Dooley <conor+dt@...nel.org> (maintainer:OPEN FIRMWARE AND
> > FLATTENED DEVICE TREE BINDINGS)
> > Heiko Stuebner <heiko@...ech.de> (maintainer:ARM/Rockchip SoC
> > support,commit_signer:43/41=100%,authored:4/41=10%,added_lines:12/117=10%,commit_signer:5/6=83%)
> > Quentin Schulz <quentin.schulz@...rry.de>
> > (commit_signer:10/41=24%,authored:8/41=20%,added_lines:63/117=54%)
> > Dragan Simic <dsimic@...jaro.org> (commit_signer:5/41=12%,commit_signer:1/6=17%)
> > FUKAUMI Naoki <naoki@...xa.com>
> > (commit_signer:3/41=7%,authored:3/41=7%,removed_lines:1/1=100%)
> > Peter Robinson <pbrobinson@...il.com>
> > (added_lines:9/117=8%,commit_signer:1/6=17%,authored:1/6=17%)
> > Alexey Charkov <alchark@...il.com> (added_lines:6/117=5%)
> > Diederik de Haas <didi.debian@...ow.org>
> > (commit_signer:4/6=67%,authored:3/6=50%)
> > Liangbin Lian <jjm2473@...il.com> (commit_signer:1/6=17%,authored:1/6=17%)
> > Johan Jonker <jbx6244@...il.com> (authored:1/6=17%)
> > devicetree@...r.kernel.org (open list:OPEN FIRMWARE AND FLATTENED
> > DEVICE TREE BINDINGS)
> > linux-arm-kernel@...ts.infradead.org (moderated list:ARM/Rockchip SoC support)
> > linux-rockchip@...ts.infradead.org (open list:ARM/Rockchip SoC support)
> > linux-kernel@...r.kernel.org (open list)
> > ```
> > your email address is not listed above.
>
> What i eventually found is that you posted v3 separately, and then
> threaded v4 to v2, which makes no sense.
>
> Please always start a new thread for each patchset.
>
> > > What do you mean by "not mounted"?
> > >
> > > Often you would say "not populated", to indicate the PCB has all the
> > > tracks in place, but the chip has simply not been soldered in place.
> > >
> > > Or is there a connector here, and nothing plugged into the connector?
> >
> > The chip/slot has not been soldered. So here should be "not
> > populated", forgive my poor English.
>
> Thanks for the clarification. I'm not sure it is worth adding these DT
> properties. When a new board is produced which does populate these
> devices, you are going to need a new .dts file. So you can put the
> properties into that new file.
>
> >
> > > Please change it to rgmii-id, and smaller tx/rx_delay values. Or show
> > > us the schematics which clearly show extra long clock lines.
> >
> > In fact, the RTL8211F's RXDLY and TXDLY signals are both pulled low,
> > just like the Banana Pi BPI-R2 Pro, so the configuration is also referenced:
> > https://elixir.bootlin.com/linux/v6.15/source/arch/arm64/boot/dts/rockchip/rk3568-bpi-r2-pro.dts#L237
>
> Pull low makes no difference to the 2ns RGMII delays.
>
> > The tx_delay and rx_delay values were obtained using Rockchip's
> > automatic scanning tool:
> > https://github.com/istoreos/istoreos/blob/54746dfdb5bd34d1f281cf41d1d1620d0c3ee686/target/linux/rockchip/files/drivers/net/ethernet/stmicro/stmmac/dwmac-rk-tool.c
> > https://gitlab.com/firefly-linux/docs/-/blob/rk356x/firefly/Common/GMAC/Rockchip_Developer_Guide_Linux_GMAC_RGMII_Delayline_EN.pdf
> > https://github.com/axlrose/rkdocs/blob/main/Common/GMAC/Rockchip_Developer_Guide_Linux_GMAC_RGMII_Delayline_EN.pdf
>
> Vendors get things wrong, including this. 'rgmii' means the PCB adds
> the 2ns delay. Nearly every Rockchip board follows Rockchip broken
> vendor recommendations, and then i come along, point out how it is
> wrong, and ask for it to be fixed, before being merged to Mainline.
>
>         Andrew

Andrew:
 Hello!

>
> What i eventually found is that you posted v3 separately, and then
> threaded v4 to v2, which makes no sense.
>

This is v2 link
https://lore.kernel.org/all/20250925092037.13582-1-jjm2473@gmail.com/
.
I don't see 'v4' in there. I have no idea why you see 'v4', can you
please share a link?

This is v3 link
https://lore.kernel.org/all/20250929065714.27741-1-jjm2473@gmail.com/
.
I don't see threading issue.
I use `git send-email --to '***' --cc '***' patches-v3` to send email,
should be OK.
(`patches-v3` is a folder contains patches generaterated by `git
format-patch --base=master --cover-letter -v3 HEAD -3 -o patches-v3`).

> When a new board is produced which does populate these
> devices, you are going to need a new .dts file. So you can put the
> properties into that new file.

These two nodes just describe the gpio and regulator found in the schematic.
If some users solder these connectors or modules themselves,
they only need to change the status to ok and they can use them.
If this will cause confusion, I can delete these two nodes.

> Vendors get things wrong, including this. 'rgmii' means the PCB adds
> the 2ns delay. Nearly every Rockchip board follows Rockchip broken
> vendor recommendations, and then i come along, point out how it is
> wrong, and ask for it to be fixed, before being merged to Mainline.

I will try `rgmii-id` and rescan {tx|rx}_delay, just like
https://lore.kernel.org/all/20250925092923.2184187-3-heiko@sntech.de/

I also notice that you suggest use {tx|rx}-internal-delay-ps instead
of  {tx|rx}_delay in
https://lore.kernel.org/all/e4d3127b-c879-4931-9ea0-de7449bc508c@lunn.ch/ ,
but I think this depends on stmmac driver.

  Liangbin Lian

Powered by blists - more mailing lists