Messages in this thread | | | Date | Sun, 24 Mar 2019 21:17:34 +0100 | From | Andrew Lunn <> | Subject | Re: [BISECTED, REGRESSION] Broken networking on MIPS/OCTEON EdgeRouter Lite |
| |
On Fri, Mar 22, 2019 at 11:41:20PM +0200, Aaro Koskinen wrote: > Hi, > > On Fri, Mar 22, 2019 at 10:25:57PM +0100, Andrew Lunn wrote: > > > The OCTEON HW code knows only about RGMII. And looking at > > > octeon ethernet staging driver it does phy connect always with > > > PHY_INTERFACE_MODE_GMII. I did some experimentation, and it seems that > > > with PHY_INTERFACE_MODE_RGMII_RXID it starts to work.. In the DT we have > > > for ethernet for this board: > > > > > > rx-delay = <0>; > > > tx-delay = <0x10>; > > > > These are not PHY properties. > > > > Looking at the code, it looks like these control delays the MAC > > inserts. I don't see a binding document for these properties, so i've > > no idea what 0x10 means. Before this driver moves out of staging, > > these values should be changed to be in ns. > > Documentation/devicetree/bindings/net/cavium-pip.txt
Hi Aaro
Ah, sorry, missed that.
- rx-delay: Delay value for RGMII receive clock. Optional. Disabled if 0. Value range is 1-31, and mapping to the actual delay varies depending on HW.
- tx-delay: Delay value for RGMII transmit clock. Optional. Disabled if 0. Value range is 1-31, and mapping to the actual delay varies depending on HW.
I'm surprised this made it passed review. We try to avoid having DT poke magic values into registers. That is what this appears to be, since it is unclear what the value actually means.
It is also good to state what happens if the property is not present. It often means it defaults to zero. But this implementation just leaves the value alone. So to be on the safe side, the DT blob should probably have these properties, so the behaviour is well defined.
Andrew
| |