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
