Re: [PATCH v2 4/4] net: mdio: Add RTL9300 MDIO driver

[Andrew Lunn]

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?

> > +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?

> > +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.

FYI: Please trim the text when replying.

	Andrew