Message-ID: <85cc3b27-417e-4cf4-9f77-347a338c9d67@arinc9.com>
Date: Thu, 21 Sep 2023 21:21:40 +0300
From: Arınç ÜNAL <arinc.unal@...nc9.com>
To: Andrew Lunn <andrew@...n.ch>
Cc: Rob Herring <robh@...nel.org>, "David S. Miller" <davem@...emloft.net>,
 Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>,
 Paolo Abeni <pabeni@...hat.com>,
 Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
 Conor Dooley <conor+dt@...nel.org>,
 George McCollister <george.mccollister@...il.com>,
 Florian Fainelli <f.fainelli@...il.com>, Vladimir Oltean
 <olteanv@...il.com>, Kurt Kanzenbach <kurt@...utronix.de>,
 Matthias Brugger <matthias.bgg@...il.com>,
 AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>,
 Woojung Huh <woojung.huh@...rochip.com>, UNGLinuxDriver@...rochip.com,
 Linus Walleij <linus.walleij@...aro.org>,
 Alvin Šipraga <alsi@...g-olufsen.dk>,
 Clément Léger <clement.leger@...tlin.com>,
 Marcin Wojtas <mw@...ihalf.com>,
 "Russell King (Oracle)" <linux@...linux.org.uk>,
 Lars Povlsen <lars.povlsen@...rochip.com>,
 Steen Hegelund <Steen.Hegelund@...rochip.com>,
 Daniel Machon <daniel.machon@...rochip.com>,
 Radhey Shyam Pandey <radhey.shyam.pandey@....com>,
 Daniel Golle <daniel@...rotopia.org>, Landen Chao
 <Landen.Chao@...iatek.com>, DENG Qingfang <dqfext@...il.com>,
 Sean Wang <sean.wang@...iatek.com>,
 Geert Uytterhoeven <geert+renesas@...der.be>,
 Magnus Damm <magnus.damm@...il.com>,
 Maxime Chevallier <maxime.chevallier@...tlin.com>,
 Nicolas Ferre <nicolas.ferre@...rochip.com>,
 Claudiu Beznea <claudiu.beznea@...rochip.com>, Marek Vasut <marex@...x.de>,
 Claudiu Manoil <claudiu.manoil@....com>,
 Alexandre Belloni <alexandre.belloni@...tlin.com>,
 John Crispin <john@...ozen.org>, Madalin Bucur <madalin.bucur@....com>,
 Ioana Ciornei <ioana.ciornei@....com>, Lorenzo Bianconi
 <lorenzo@...nel.org>, Felix Fietkau <nbd@....name>,
 Horatiu Vultur <horatiu.vultur@...rochip.com>,
 Oleksij Rempel <linux@...pel-privat.de>,
 Alexandre Torgue <alexandre.torgue@...s.st.com>,
 Giuseppe Cavallaro <peppe.cavallaro@...com>,
 Jose Abreu <joabreu@...opsys.com>,
 Grygorii Strashko <grygorii.strashko@...com>, Sekhar Nori <nsekhar@...com>,
 Shyam Pandey <radhey.shyam.pandey@...inx.com>, mithat.guner@...ont.com,
 erkin.bozoglu@...ont.com, netdev@...r.kernel.org,
 devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
 linux-arm-kernel@...ts.infradead.org, linux-mediatek@...ts.infradead.org,
 linux-renesas-soc@...r.kernel.org
Subject: Re: [PATCH net-next v2 07/10] dt-bindings: net: enforce phylink
 bindings on certain ethernet controllers

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ç

Powered by blists - more mailing lists