Message ID | 20230303085928.4535-12-samin.guo@starfivetech.com (mailing list archive) |
---|---|
State | Superseded |
Headers | show |
Series | Add Ethernet driver for StarFive JH7110 SoC | expand |
Context | Check | Description |
---|---|---|
conchuod/tree_selection | fail | Failed to apply to next/pending-fixes or riscv/for-next |
On Fri, 3 Mar 2023 at 10:01, Samin Guo <samin.guo@starfivetech.com> wrote: > v1.2A gmac0 uses motorcomm YT8531(rgmii-id) PHY, and needs delay > configurations. > > v1.2A gmac1 uses motorcomm YT8512(rmii) PHY, and needs to > switch rx and rx to external clock sources. > > Signed-off-by: Samin Guo <samin.guo@starfivetech.com> > --- > .../starfive/jh7110-starfive-visionfive-2-v1.2a.dts | 13 +++++++++++++ > 1 file changed, 13 insertions(+) > > diff --git a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-v1.2a.dts b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-v1.2a.dts > index 4af3300f3cf3..205a13d8c8b1 100644 > --- a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-v1.2a.dts > +++ b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-v1.2a.dts > @@ -11,3 +11,16 @@ > model = "StarFive VisionFive 2 v1.2A"; > compatible = "starfive,visionfive-2-v1.2a", "starfive,jh7110"; > }; > + > +&gmac1 { > + phy-mode = "rmii"; > + assigned-clocks = <&syscrg JH7110_SYSCLK_GMAC1_TX>, > + <&syscrg JH7110_SYSCLK_GMAC1_RX>; > + assigned-clock-parents = <&syscrg JH7110_SYSCLK_GMAC1_RMII_RTX>, > + <&syscrg JH7110_SYSCLK_GMAC1_RMII_RTX>; > +}; > + > +&phy0 { > + rx-internal-delay-ps = <1900>; > + tx-internal-delay-ps = <1350>; > +}; Here you're not specifying the internal delays for phy1 which means it defaults to 1950ps for both rx and tx. Is that right or did you mean to set them to 0 like the v1.3b phy1? Also your u-boot seems to set what the linux phy driver calls motorcomm,keep-pll-enabled and motorcomm,auto-sleep-disabled for all the phys. Did you leave those out on purpose? > -- > 2.17.1 > > > _______________________________________________ > linux-riscv mailing list > linux-riscv@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-riscv
在 2023/3/6 21:00:19, Emil Renner Berthing 写道: > On Fri, 3 Mar 2023 at 10:01, Samin Guo <samin.guo@starfivetech.com> wrote: >> v1.2A gmac0 uses motorcomm YT8531(rgmii-id) PHY, and needs delay >> configurations. >> >> v1.2A gmac1 uses motorcomm YT8512(rmii) PHY, and needs to >> switch rx and rx to external clock sources. >> >> Signed-off-by: Samin Guo <samin.guo@starfivetech.com> >> --- >> .../starfive/jh7110-starfive-visionfive-2-v1.2a.dts | 13 +++++++++++++ >> 1 file changed, 13 insertions(+) >> >> diff --git a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-v1.2a.dts b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-v1.2a.dts >> index 4af3300f3cf3..205a13d8c8b1 100644 >> --- a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-v1.2a.dts >> +++ b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-v1.2a.dts >> @@ -11,3 +11,16 @@ >> model = "StarFive VisionFive 2 v1.2A"; >> compatible = "starfive,visionfive-2-v1.2a", "starfive,jh7110"; >> }; >> + >> +&gmac1 { >> + phy-mode = "rmii"; >> + assigned-clocks = <&syscrg JH7110_SYSCLK_GMAC1_TX>, >> + <&syscrg JH7110_SYSCLK_GMAC1_RX>; >> + assigned-clock-parents = <&syscrg JH7110_SYSCLK_GMAC1_RMII_RTX>, >> + <&syscrg JH7110_SYSCLK_GMAC1_RMII_RTX>; >> +}; >> + >> +&phy0 { >> + rx-internal-delay-ps = <1900>; >> + tx-internal-delay-ps = <1350>; >> +}; > > Here you're not specifying the internal delays for phy1 which means it > defaults to 1950ps for both rx and tx. Is that right or did you mean > to set them to 0 like the v1.3b phy1? Hi, emil, usually, only 1000M (rgmii) needs to configure the delay, and 100M(rmii) does not. > > Also your u-boot seems to set what the linux phy driver calls > motorcomm,keep-pll-enabled and motorcomm,auto-sleep-disabled for all > the phys. Did you leave those out on purpose? Hi, Emil, We did configure motorcomm,auto-sleep-disabled for yt8512 in uboot, but Yutai upstream's Linux driver only yt8521/yt8531 supports this property. Yt8512 is a Generic PHY driver and does not support the configuration of motorcomm,auto-sleep-disabled and motorcomm,keep-pll-enabled. And without configuring these two attributes, vf2-1.2a gmac1 also works normally. Best regards, Samin > >> -- >> 2.17.1 >> >> >> _______________________________________________ >> linux-riscv mailing list >> linux-riscv@lists.infradead.org >> http://lists.infradead.org/mailman/listinfo/linux-riscv
On Tue, 7 Mar 2023 at 02:43, Guo Samin <samin.guo@starfivetech.com> wrote: > 在 2023/3/6 21:00:19, Emil Renner Berthing 写道: > > On Fri, 3 Mar 2023 at 10:01, Samin Guo <samin.guo@starfivetech.com> wrote: > >> v1.2A gmac0 uses motorcomm YT8531(rgmii-id) PHY, and needs delay > >> configurations. > >> > >> v1.2A gmac1 uses motorcomm YT8512(rmii) PHY, and needs to > >> switch rx and rx to external clock sources. > >> > >> Signed-off-by: Samin Guo <samin.guo@starfivetech.com> > >> --- > >> .../starfive/jh7110-starfive-visionfive-2-v1.2a.dts | 13 +++++++++++++ > >> 1 file changed, 13 insertions(+) > >> > >> diff --git a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-v1.2a.dts b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-v1.2a.dts > >> index 4af3300f3cf3..205a13d8c8b1 100644 > >> --- a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-v1.2a.dts > >> +++ b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-v1.2a.dts > >> @@ -11,3 +11,16 @@ > >> model = "StarFive VisionFive 2 v1.2A"; > >> compatible = "starfive,visionfive-2-v1.2a", "starfive,jh7110"; > >> }; > >> + > >> +&gmac1 { > >> + phy-mode = "rmii"; > >> + assigned-clocks = <&syscrg JH7110_SYSCLK_GMAC1_TX>, > >> + <&syscrg JH7110_SYSCLK_GMAC1_RX>; > >> + assigned-clock-parents = <&syscrg JH7110_SYSCLK_GMAC1_RMII_RTX>, > >> + <&syscrg JH7110_SYSCLK_GMAC1_RMII_RTX>; > >> +}; > >> + > >> +&phy0 { > >> + rx-internal-delay-ps = <1900>; > >> + tx-internal-delay-ps = <1350>; > >> +}; > > > > Here you're not specifying the internal delays for phy1 which means it > > defaults to 1950ps for both rx and tx. Is that right or did you mean > > to set them to 0 like the v1.3b phy1? > > Hi, emil, usually, only 1000M (rgmii) needs to configure the delay, and 100M(rmii) does not. Ah, I see. > > Also your u-boot seems to set what the linux phy driver calls > > motorcomm,keep-pll-enabled and motorcomm,auto-sleep-disabled for all > > the phys. Did you leave those out on purpose? > > Hi, Emil, We did configure motorcomm,auto-sleep-disabled for yt8512 in uboot, > but Yutai upstream's Linux driver only yt8521/yt8531 supports this property. I'm confused. Is Yutai also Frank Sae? Because he is the one who added support for the yt8531 upstream. > Yt8512 is a Generic PHY driver and does not support the configuration of > motorcomm,auto-sleep-disabled and motorcomm,keep-pll-enabled. Right phy1 of the 1.2a might use a different phy, but I'm also talking about phy0 and the v1.3b which does use the yt8531 right? > And without configuring these two attributes, vf2-1.2a gmac1 also works normally. Yes, but what I'm worried about is that it only works because u-boot initialises the PHYs and ethernet may stop working if you're using a different bootloader or Linux gains support for resetting the PHYs before use. > > Best regards, > Samin > > > >> -- > >> 2.17.1 > >> > >> > >> _______________________________________________ > >> linux-riscv mailing list > >> linux-riscv@lists.infradead.org > >> http://lists.infradead.org/mailman/listinfo/linux-riscv > > -- > Best regards, > Samin
-------- 原始信息 -------- 主题: Re: [PATCH v5 11/12] riscv: dts: starfive: visionfive-2-v1.2a: Add gmac+phy's delay configuration From: Emil Renner Berthing <emil.renner.berthing@canonical.com> 收件人: Guo Samin <samin.guo@starfivetech.com> 日期: 2023/3/7 > On Tue, 7 Mar 2023 at 02:43, Guo Samin <samin.guo@starfivetech.com> wrote: >> 在 2023/3/6 21:00:19, Emil Renner Berthing 写道: >>> On Fri, 3 Mar 2023 at 10:01, Samin Guo <samin.guo@starfivetech.com> wrote: >>>> v1.2A gmac0 uses motorcomm YT8531(rgmii-id) PHY, and needs delay >>>> configurations. >>>> >>>> v1.2A gmac1 uses motorcomm YT8512(rmii) PHY, and needs to >>>> switch rx and rx to external clock sources. >>>> >>>> Signed-off-by: Samin Guo <samin.guo@starfivetech.com> >>>> --- >>>> .../starfive/jh7110-starfive-visionfive-2-v1.2a.dts | 13 +++++++++++++ >>>> 1 file changed, 13 insertions(+) >>>> >>>> diff --git a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-v1.2a.dts b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-v1.2a.dts >>>> index 4af3300f3cf3..205a13d8c8b1 100644 >>>> --- a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-v1.2a.dts >>>> +++ b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-v1.2a.dts >>>> @@ -11,3 +11,16 @@ >>>> model = "StarFive VisionFive 2 v1.2A"; >>>> compatible = "starfive,visionfive-2-v1.2a", "starfive,jh7110"; >>>> }; >>>> + >>>> +&gmac1 { >>>> + phy-mode = "rmii"; >>>> + assigned-clocks = <&syscrg JH7110_SYSCLK_GMAC1_TX>, >>>> + <&syscrg JH7110_SYSCLK_GMAC1_RX>; >>>> + assigned-clock-parents = <&syscrg JH7110_SYSCLK_GMAC1_RMII_RTX>, >>>> + <&syscrg JH7110_SYSCLK_GMAC1_RMII_RTX>; >>>> +}; >>>> + >>>> +&phy0 { >>>> + rx-internal-delay-ps = <1900>; >>>> + tx-internal-delay-ps = <1350>; >>>> +}; >>> >>> Here you're not specifying the internal delays for phy1 which means it >>> defaults to 1950ps for both rx and tx. Is that right or did you mean >>> to set them to 0 like the v1.3b phy1? >> >> Hi, emil, usually, only 1000M (rgmii) needs to configure the delay, and 100M(rmii) does not. > > Ah, I see. > >>> Also your u-boot seems to set what the linux phy driver calls >>> motorcomm,keep-pll-enabled and motorcomm,auto-sleep-disabled for all >>> the phys. Did you leave those out on purpose? >> >> Hi, Emil, We did configure motorcomm,auto-sleep-disabled for yt8512 in uboot, >> but Yutai upstream's Linux driver only yt8521/yt8531 supports this property. > > I'm confused. Is Yutai also Frank Sae? Because he is the one who added > support for the yt8531 upstream. My fault , Frank Sae is from Motorcomm, also known as Yutai. yt8531 ==> Yutai 8531 > >> Yt8512 is a Generic PHY driver and does not support the configuration of >> motorcomm,auto-sleep-disabled and motorcomm,keep-pll-enabled. > > Right phy1 of the 1.2a might use a different phy, but I'm also talking > about phy0 and the v1.3b which does use the yt8531 right? Right: v1.3b: gmac0:yt8531 gmac1:yt8531 v1.2a: gmac0:yt8531 gmac1:yt8512 > >> And without configuring these two attributes, vf2-1.2a gmac1 also works normally. > > Yes, but what I'm worried about is that it only works because u-boot > initialises the PHYs and ethernet may stop working if you're using a > different bootloader or Linux gains support for resetting the PHYs > before use. > I have tested that in uboot, use the sd card to start Linux, do not run network programs (do not use uboot to initialize phy and gmac), and the network works normally in Linux. Best regards, Samin >> >> Best regards, >> Samin >>> >>>> -- >>>> 2.17.1 >>>> >>>> >>>> _______________________________________________ >>>> linux-riscv mailing list >>>> linux-riscv@lists.infradead.org >>>> http://lists.infradead.org/mailman/listinfo/linux-riscv >> >> -- >> Best regards, >> Samin
diff --git a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-v1.2a.dts b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-v1.2a.dts index 4af3300f3cf3..205a13d8c8b1 100644 --- a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-v1.2a.dts +++ b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-v1.2a.dts @@ -11,3 +11,16 @@ model = "StarFive VisionFive 2 v1.2A"; compatible = "starfive,visionfive-2-v1.2a", "starfive,jh7110"; }; + +&gmac1 { + phy-mode = "rmii"; + assigned-clocks = <&syscrg JH7110_SYSCLK_GMAC1_TX>, + <&syscrg JH7110_SYSCLK_GMAC1_RX>; + assigned-clock-parents = <&syscrg JH7110_SYSCLK_GMAC1_RMII_RTX>, + <&syscrg JH7110_SYSCLK_GMAC1_RMII_RTX>; +}; + +&phy0 { + rx-internal-delay-ps = <1900>; + tx-internal-delay-ps = <1350>; +};
v1.2A gmac0 uses motorcomm YT8531(rgmii-id) PHY, and needs delay configurations. v1.2A gmac1 uses motorcomm YT8512(rmii) PHY, and needs to switch rx and rx to external clock sources. Signed-off-by: Samin Guo <samin.guo@starfivetech.com> --- .../starfive/jh7110-starfive-visionfive-2-v1.2a.dts | 13 +++++++++++++ 1 file changed, 13 insertions(+)