lkml.org 
[lkml]   [2019]   [Feb]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [RFC] net: dsa: qca8k: implement rgmii-id mode
From
Date
On 18. 02. 19 14:03, Vinod Koul wrote:
> On 18-02-19, 12:54, Michal Vokáč wrote:
>> On 18. 02. 19 11:45, Vinod Koul wrote:
>>> On 15-02-19, 16:23, Andrew Lunn wrote:
>>>> On Fri, Feb 15, 2019 at 04:01:08PM +0100, Michal Vokáč wrote:
>>>>> Hi,
>>>>>
>>>>> networking on my boards [1], which are currently in linux-next, suddently
>>>>> stopped working. I tracked it down to this commit 5ecdd77c61c8 ("net: dsa:
>>>>> qca8k: disable delay for RGMII mode") [2].
>>>>>
>>>>> So I think the rgmii-id mode is obviously needed in my case.
>>>>> I was able to find a couple drivers that read tx/rx-delay or
>>>>> tx/rx-internal-delay from device tree. Namely:
>>>>>
>>>>> drivers/net/ethernet/apm/xgene/xgene_enet_main.c
>>>>> drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c
>>>>> drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
>>>>> drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
>>>>> drivers/net/phy/dp83867.c
>>>>>
>>>>> I would appreciate any hints how to add similar function to qca8k driver
>>>>> if that is the correct way to go. Can I take some of the above mentioned
>>>>> drivers as a good example for that? How should the binding look like?
>>>>>
>>>>> I would expect something like this:
>>>>>
>>>>> switch@0 {
>>>>> compatible = "qca,qca8334";
>>>>> reg = <0>;
>>>>>
>>>>> switch_ports: ports {
>>>>> #address-cells = <1>;
>>>>> #size-cells = <0>;
>>>>>
>>>>> ethphy0: port@0 {
>>>>> reg = <0>;
>>>>> label = "cpu";
>>>>> phy-mode = "rgmii-id";
>>>>> qca,tx-delay = <3>;
>>>>> qca,rx-delay = <3>;
>>>>> ethernet = <&fec>;
>>>>> };
>>>>
>>>> Hi Michal
>>>>
>>>> Your submission used:
>>>>
>>>> + ethphy0: port@0 {
>>>> + reg = <0>;
>>>> + label = "cpu";
>>>> + phy-mode = "rgmii";
>>>> + ethernet = <&fec>;
>>>> +
>>>> + fixed-link {
>>>> + speed = <1000>;
>>>> + full-duplex;
>>>> + };
>>>> + };
>>>>
>>>> This is good. If you have a fixed-link you can pass a phy-mode.
>>
>> Yes, I am using fixed-link and the plan was to implement the rgmii-id
>> mode.
>>
>>>>
>>>> The comment that was removed was:
>>>>
>>>> - /* According to the datasheet, RGMII delay is enabled through
>>>> - * PORT5_PAD_CTRL for all ports, rather than individual port
>>>> - * registers
>>>> - */
>>>>
>>>> Is it possible to enable delays per port? Ideally, you want to enable
>>>> delays for just selected ports. Add another case for
>>>> PHY_INTERFACE_MODE_RGMII_ID to enable the delays.
>>
>> I am still trying to collect all the relevant notes and bits from the
>> horrible docs to understand how this is done. The problem is different
>> parts and different versions of the documentation provide diffrerent
>> details.
>>
>> For example the QCA8334 model does not have PORT5 and so that registers
>> are not documented. Though some application note document says:
>>
>> """
>> The MAC0 timing control is in the Port0 PAD Mode Control Register (offset 0x0004):
>>
>> 1. RGMII timing delay for the output path of QCA8337(N) is enabled by 0x8[24].
>> Set 1 to add 2ns delay in 1000 mode for all RGMII interfaces.
>>
>> 2. Bit [21:20]: select the delay time for the output path in 10/100 mode.
>>
>> 3. Bit 25: enable the timing delay for the input path of QCA8337(N) in 1000 mode.
>>
>> 4. Bit [23:22]: select the delay time for the input path in 1000 mode.
>> 00: 0.2ns
>> 01: 1.2ns
>> 10: 2.1ns
>> 11: 3.1ns
>> """
>>
>> That is in line with the removed comment. The bit 24 at address 0x8 is
>> used to enable/disable delays globally. And obviously it is needed on the
>> QCA8334 model as well even though it should not have the PORT5.
>>
>>> In the hindsight I should not have removed the comment, let me ressurect
>>> that as well as add handling of the RGMII modes...
>>
>> OK, thanks.
>
> Since it used to work for you before, you need older code in RGMII_ID
> mode, I have added that along with the comment

Sure, that is OK for me. I just thought about implementing something more
generic with the option to set the delay value, even though I do not really
need it.

Thank you,
Michal

\
 
 \ /
  Last update: 2019-02-18 14:32    [W:0.060 / U:0.456 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site