diff mbox series

[v5,11/12] riscv: dts: starfive: visionfive-2-v1.2a: Add gmac+phy's delay configuration

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

Checks

Context Check Description
netdev/tree_selection success Guessing tree name failed - patch did not apply

Commit Message

Guo Samin March 3, 2023, 8:59 a.m. UTC
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(+)

Comments

Emil Renner Berthing March 6, 2023, 1 p.m. UTC | #1
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
Guo Samin March 7, 2023, 1:43 a.m. UTC | #2
在 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
Emil Renner Berthing March 7, 2023, 12:40 p.m. UTC | #3
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
Guo Samin March 8, 2023, 3:01 a.m. UTC | #4
-------- 原始信息 --------
主题: 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 mbox series

Patch

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>;
+};