Message-ID: <CAJq09z7j_gNbUcYDWXjzUNAXat-+EyryFJFEqpVG-jPcY4ZmmQ@mail.gmail.com>
Date: Thu, 7 Dec 2023 16:50:12 -0300
From: Luiz Angelo Daros de Luca <luizluca@...il.com>
To: Vladimir Oltean <olteanv@...il.com>
Cc: Alvin Šipraga <ALSI@...g-olufsen.dk>, 
	Krzysztof Kozlowski <krzk@...nel.org>, "linus.walleij@...aro.org" <linus.walleij@...aro.org>, 
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>, "andrew@...n.ch" <andrew@...n.ch>, 
	"f.fainelli@...il.com" <f.fainelli@...il.com>, "davem@...emloft.net" <davem@...emloft.net>, 
	"edumazet@...gle.com" <edumazet@...gle.com>, "kuba@...nel.org" <kuba@...nel.org>, 
	"pabeni@...hat.com" <pabeni@...hat.com>, "arinc.unal@...nc9.com" <arinc.unal@...nc9.com>
Subject: Re: [net-next 2/2] net: dsa: realtek: load switch variants on demand

> On Mon, Nov 27, 2023 at 07:24:16PM -0300, Luiz Angelo Daros de Luca wrote:
> > The "realtek_smi_setup_mdio()" used in setup_interface isn't really
> > necessary (like it happens in realtek-mdio). It could be used (or not)
> > by both interfaces. The "realtek,smi-mdio" compatible string is
> > misleading, indicating it might be something specific to the SMI
> > interface HW while it is just how the driver was implemented. A
> > "realtek,slave_mdio" or "realtek,user_mii" would be better.
>
> The compatible string is about picking a driver for a device. It is
> supposed to uniquely describe the register layout and functionality of
> that IP block, not its functional role in the kernel. "slave_mdio" and
> "user_mii" are too ingrained with "this MDIO controller gives access to
> internal PHY ports of DSA slave ports".
>
> Even if the MDIO controller doesn't currently have its own struct device
> and driver, you'd have to think of the fact that it could, when picking
> an appropriate compatible string.
>
> If you have very specific information that the MDIO controller is based on
> some reusable/licensable IP block and there were no modifications made
> to it, you could use that compatible string.
>
> Otherwise, another sensible choice would be "realtek,<precise-soc-name>-mdio",
> because it leaves room for future extensions with other compatible
> strings, more generic or just as specific, that all bind to the same
> driver.
>
> It's always good to start being too specific rather than too generic,
> because a future Realtek switch might have a different IP block for its
> MDIO controller. Then a driver for your existing "realtek,smi-mdio" or
> "realtek,slave_mdio" or "realtek,user_mii" compatible string sounds like
> it could handle it, but it can't.
>
> You can always add secondary compatible strings to a node, but changing
> the existing one breaks the ABI between the kernel and the DT.

HI Vladmir,

We discussed something about that in the past:

https://lkml.kernel.org/netdev/20220630200423.tieprdu5fpabflj7@bang-olufsen.dk/T/#m04e6cf7d0c1f18b02c2bf40266ada915b1f02f3d

The code is able to handle only a single node and binding docs say it
should be named "mdio". The compatible string wasn't a requirement
since the beginning and I don't think it is worth it to rename the
compatible string. I suggest we simply switch to
of_get_child_by_name() and look for a node named "mdio". If that node
is not found, we can still look for the old compatible string
(backwards compatibility) and probably warn the "user" (targeting not
the end-user but the one creating the DT for a new device).

I don't know how to handle the binding docs as the compatible string
is still a requirement for older kernel versions. Is it ok to update
the device-tree bindings docs in such a way it would break old
drivers? Or should we keep it there until the last LTS kernel
requiring it reaches EOL? As device-tree bindings docs should not
consider how the driver was implemented, I think it would be strange
to have a note like "required by kernel up to 6.x".

Regards,

Luiz

Powered by blists - more mailing lists