Messages in this thread | | | Date | Thu, 21 Sep 2023 21:21:40 +0300 | Subject | Re: [PATCH net-next v2 07/10] dt-bindings: net: enforce phylink bindings on certain ethernet controllers | From | Arınç ÜNAL <> |
| |
On 21.09.2023 16:00, Andrew Lunn wrote: >> - Link descriptions must be required on ethernet controllers. We don't care >> whether some Linux driver can or cannot find the PHY or set up a fixed >> link without looking at the devicetree. > > That can lead to future surprises, and breakage. > > Something which is not used is not tested, and so sometimes wrong, and > nobody knows. Say the driver is extended to a new device and actually > does need to use this never before used information. You then find it > is wrong, and you get a regression. > > We have had issues like this before. There are four rgmii phy-link > modes. We have had PHY drivers which ignored one of those modes, it > silently accepted it, but did not change the hardware to actually use > that mode. The PHY continues to use its reset defaults or strapping, > and it worked. A lot of device trees ended up using this mode. And it > was not the same as reset defaults/strapping. > > And then somebody needed that fourth mode, and made it actually > work. And all those boards wrongly using that mode broke. > > The lesson i learned from that episode is that anything in device tree > must actually be used and tested.
It looks like the root cause here was the lack of dt-bindings to only allow the phy-mode values the hardware supports. What I see here is the driver change should've been tested on all different hardware the driver controls then the improper describing of hardware on the devicetree source file addressed.
If a devicetree change that ensures proper describing of hardware is found to break a driver in the future, then that exposes a bug on the driver and the driver will have to be fixed. I don't see this upholding writing dt-bindings that ensures proper describing of the hardware.
> >> Although I see dsa.yaml and dsa-port.yaml mostly consist of describing an >> ethernet switch with CPU port(s), there're properties that are specific to >> DSA, such as dsa,member on dsa.yaml and dsa-tag-protocol and label on >> dsa-port.yaml. > > I would say dsa,member does describe the hardware. It provides two > bits of information: > > Which cluster of switches does this switch belong to. You probably can > derive it using the DSA links between switches, which is also a > hardware property. But having it in device tree makes it simpler. > > Which switch is this within a cluster. You need to be able to say: > Send this frame out Port X of switch Y. How does a switch know it is > Y? It could be strapping, which is clearly a hardware property. > > dsa-tag-protocol is similar to phy-mode. It tells you the protocol > running between the CPU port and the SoC master interface. You often > can imply it, but again, it could be determined by strapping on the > switch. > > label is an interesting one, and probably would not be accepted if it > was proposed now. But it has been around a long time. It also does > describe the hardware, it is what is printed on the case next to the > RJ45. To make the user experience simpler, we then try to make the > linux interface name match the label on the case.
Looks like we can incorporate dsa.port and dsa-port.yaml into ethernet-switch.yaml and ethernet-switch-port.yaml with adjustments.
Arınç
| |