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

[Guo 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@xxxxxxxxxxxxx>
收件人: Guo Samin <samin.guo@xxxxxxxxxxxxxxxx>
日期: 2023/3/7

> On Tue, 7 Mar 2023 at 02:43, Guo Samin <samin.guo@xxxxxxxxxxxxxxxx> wrote:
>> 在 2023/3/6 21:00:19, Emil Renner Berthing 写道:
>>> On Fri, 3 Mar 2023 at 10:01, Samin Guo <samin.guo@xxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxx>
>>>> ---
>>>>  .../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@xxxxxxxxxxxxxxxxxxx
>>>> http://lists.infradead.org/mailman/listinfo/linux-riscv
>>
>> --
>> Best regards,
>> Samin

-- 
Best regards,
Samin