Re: [PATCH 0/2] net: phy: broadcom: RGMII delays fixes

[Vladimir Oltean]

Hi Andrew,

On Thu, 3 Oct 2019 at 22:06, Andrew Lunn <andrew@xxxxxxx> wrote:
>
> On Thu, Oct 03, 2019 at 11:55:40AM -0700, Florian Fainelli wrote:
> > Hi Andrew,
> >
> > On 10/3/19 11:51 AM, Andrew Lunn wrote:
> > > On Thu, Oct 03, 2019 at 11:43:50AM -0700, Florian Fainelli wrote:
> > >> Hi all,
> > >>
> > >> This patch series fixes the BCM54210E RGMII delay configuration which
> > >> could only have worked in a PHY_INTERFACE_MODE_RGMII configuration.
> > >
> > > Hi Florian
> > >
> > > So any DT blob which incorrectly uses one of the other RGMII modes is
> > > now going to break, where as before it was ignored.
> >
> > Potentially yes. There is a precedent with the at803x PHY driver
>
> Hi Florian
>
> Yes that was an interesting learning experience. I'm not sure we want
> to do that again. A lot of devices broken, and a lot of people were
> unhappy.
>
> If we are looking at a similar scale of breakage, i think i would
> prefer to add a broadcom,bcm54210e-phy-mode property in the DT which
> if present would override the phy_interface_t passed to the driver.
>
>    Andrew

What is the breakage concern here?
The driver was unconditionally clearing the RGMII delays. Therefore,
any board that needed them would have noticed really fast, IMO. That
should include people who configure 'rgmii-id' in the DT in the hope
that it would solve some problems.
The typical RGMII delay breakage is not realizing you need Linux to
enable RGMII delays (perhaps due to strapping) and specifying plain
"rgmii" in the phy-mode, then somebody 'fixing' those and disabling
them.
But in this case, the only breakage would be "hmmm, let's just enable
RGMII delays everywhere. So it works with rgmii-id on both the PHY and
the MAC side of things? Great, time for lunch!". I just hope that did
not happen. And maybe even if it did, AFAIK the BCM54xx skews the data
lines by 1.9 ns (unfortunately not configurable), that leaves an extra
~2.1 ns of timing budget until the next RGMII clock transition,
considering a 50% duty cycle? Maybe that's enough to fit the MAC's
RGMII I/O buffer delays without breaking anything, and then it doesn't
really matter?

Regards,
-Vladimir