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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <07073382-df51-4064-9802-cdbfcf732523@lunn.ch>
Date: Thu, 19 Dec 2024 10:40:01 +0100
From: Andrew Lunn <andrew@...n.ch>
To: Luiz Angelo Daros de Luca <luizluca@...il.com>
Cc: Chris Packham <chris.packham@...iedtelesis.co.nz>, lee@...nel.org,
	robh@...nel.org, krzk+dt@...nel.org, conor+dt@...nel.org,
	andrew+netdev@...n.ch, davem@...emloft.net, edumazet@...gle.com,
	kuba@...nel.org, pabeni@...hat.com, tsbogend@...ha.franken.de,
	hkallweit1@...il.com, linux@...linux.org.uk,
	markus.stockhausen@....de, devicetree@...r.kernel.org,
	linux-kernel@...r.kernel.org, netdev@...r.kernel.org,
	linux-mips@...r.kernel.org
Subject: Re: [PATCH v2 4/4] net: mdio: Add RTL9300 MDIO driver

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