Re: [PATCH net-next v2 07/10] dt-bindings: net: enforce phylink bindings on certain ethernet controllers

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 22.09.2023 02:29, Andrew Lunn wrote:
On Thu, Sep 21, 2023 at 09:21:40PM +0300, Arınç ÜNAL wrote:
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.

That would not help. The hardware supported all 4 RGMII modes. So
listing all four in the dt-binding would be correct. But the driver
for the hardware had a bug, and so silently ignored one of the
modes. That then masked the bugs in board DT files.

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.

Which is what did happen. But it took a while to find all those broken
boards.  For a period of time, we had regressions.

Bugs happen. It is a fact of life. But we want those bugs to be easy
to find as possible. If we force DT writers to add properties which
the driver never uses, they are going to be bugs in those
properties. And those bugs are not going to be easy to find, and quite
likely, they will only be found a long time after they are added. We
should not be adding unused properties and bugs just to keep a yaml
checker happy.

Understood. Then I will refrain from enforcing link descriptions altogether
as I can't justify enforcing them on ethernet controllers selectively
because certain Linux drivers don't make use of the link properties and may
expose bugs if used.

The link properties are still allowed for all ethernet controllers though,
just not enforced. But I suppose it's better to optionally find the bug
instead of causing a regression then finding the bug.

Arınç



[Index of Archives]     [Linux Samsung SOC]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux