# RGMII with internal RX and TX delays provided by the PHY,
# the MAC should not add the RX or TX delays in this case
- rgmii-id
The MAC should not add delays because the PHY does. But this implies
the PCB cannot also be adding delays because you would end up with two
sets of delays.
> Fixing MAC drivers to interpret the values without
> "id" to mean that there is a delay on the PCB will break existing Device Trees,
> so that's no good.
You need to look at it broken driver by broken driver. I _think_ the
Aspeed case can be fixed. Others we need to look at the details. Maybe
in some cases we do need to add a warning to the device tree binding
the driver is FUBAR and has special, broken meaning for 'rgmii'.
> As a first step, I'd update the docs to describe the intended behavior, but
> mention that some drivers implement it wrong.
Feel free to submit patches. However, please think about it. DT is OS
agnostic. DT describes the hardware, not configuration. Other OSes
might decide on a different policy about which of the MAC/PHY pair
implements the delay, since that is configuration, not a hardware
description.
> The question is how to deal with
> the wrong behavior going forward. I see the following options, but none spark
> joy for me...
>
> - Keep current driver behavior forever where fixing it would break existing
> Device Trees
> - Deprecate all existing "rgmii*" values because of their inconsistent
> implementation, come up with new ones. Seems like a huge pain to add support
> for in all MAC drivers and other code that deals with PHY modes...
Won't help anyway. If developers are getting it wrong now, why should
they suddenly start getting it right when all you are changing is the
name.
The real issue is that you can combine two bugs to get a working
system. It seems like few developers actually understand RGMII
delays. They don't spend time to understand it, they just try random
things until it works. As a result, they get systems where the MAC is
unknowingly adding delays, and 'rgmii' works, so job done. They don't
know or care it is wrong.
> - Introduce an additional DTS flag next to phy-mode to express that the
> phy-mode is supposed to be interpreted correctly
If you are going to do add any sort of property, it should be a bool
indicating the opposite, 'phy-mode-is-fubar'. Something which
developers will think about before copying into their DT.
> Do you have any suggestions?
Add a checkpatch.pl check. Any patch which adds a phy-mode !=
'rgmii-id' needs to have a comment on the previous line containing the
word PCB. That should at least help stop more broken DT being added.
Andrew
