On 19/12/2024 22:40, Andrew Lunn wrote:
On Thu, Dec 19, 2024 at 01:46:41AM -0300, Luiz Angelo Daros de Luca wrote:
Hello Chris,
+++ b/drivers/net/mdio/mdio-realtek-rtl.c
I wonder if the name might be dubious in the future with other realtek
products with MDIO. Realtek is quite a large company with many
products. Would a version/model/family/usage in that name help a far
future reader to identify what this file is about?
Isnt rtl the family name? Or would you prefer mdio-realtek-rtl9300.c?
Yes my intention was that "rtl" was the family name. I'm happy to change
to rtl9300.
I suspect this probably will be compatible with the rtl9310. I've just
received a RTL9313 based board so will probably start looking at that in
the new year.
+static int realtek_mdio_wait_ready(struct realtek_mdio_priv *priv)
All those realtek_mdio_* prefix might collide with realtek_mdio_* from
drivers/net/dsa/realtek/realtek-mdio.c. This realtek_mdio_* is about a
Realtek SoC MDIO interface with the switch. The other realtek_mdio_*
is about the interface (MDIO or SMI) between (the other vendor) SoC
and the switch. I don't know if the maintainers are OK with it but
listing those symbols in alphabetic order from both sources might be
confusing.
rtl9300_ as a prefix?
I'd happily change to rtl_ or rtl9300_
+static const struct of_device_id realtek_mdio_ids[] = {
+ { .compatible = "realtek,rtl9301-mdio" },
+ { .compatible = "realtek,rtl9302b-mdio" },
+ { .compatible = "realtek,rtl9302c-mdio" },
+ { .compatible = "realtek,rtl9303-mdio" },
Do these different compatible strings really matter? AFAIK, compatible
are not for listing all supported models/variants but to describe
devices that have a different behavior and indicating that (with
different strings) is needed to decide how the driver will work. If
the driver does not use which compatible was set, it might indicate
that we don't really need 4 compatible but 1.
It can be useful when we initially think they are compatible, but
later find out they are not, and we need different behaviour.
The way I've written the dt-binding any board should include
"realtek,rtl9301-mdio" and may also include one of
"realtek,rtl9302b-mdio", "realtek,rtl9302c-mdio",
"realtek,rtl9303-mdio". For the MDIO driver the specific chip could
possibly tell us the maximum SMI bus number. Unfortunately I've only got
a block diagram of the RTL9302C, I know that does have 4 SMI interfaces,
the others may have fewer. Things would probably work fine for now with
just "realtek,rtl9301-mdio" but is there any harm in including the others?
