Messages in this thread | | | Subject | Re: [RFC] net: dsa: qca8k: implement rgmii-id mode | From | Michal Vokáč <> | Date | Mon, 18 Feb 2019 14:31:43 +0100 |
| |
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
| |