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
>
> > Please do test
>
> Sure, will do. Just let me know when you have something ready.
> Thank you!
Sending it shortly and ccing you as well
Thanks
--
~Vinod
