netdev - Re: [PATCH net-next v4 09/11] net: dsa: realtek: migrate user_mii

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: <20240129151906.a6oeyh7qyq7c3ow4@skbuf>
Date: Mon, 29 Jan 2024 17:19:06 +0200
From: Vladimir Oltean <olteanv@...il.com>
To: Luiz Angelo Daros de Luca <luizluca@...il.com>
Cc: netdev@...r.kernel.org, linus.walleij@...aro.org, alsi@...g-olufsen.dk,
	andrew@...n.ch, f.fainelli@...il.com, davem@...emloft.net,
	edumazet@...gle.com, kuba@...nel.org, pabeni@...hat.com,
	arinc.unal@...nc9.com, ansuelsmth@...il.com
Subject: Re: [PATCH net-next v4 09/11] net: dsa: realtek: migrate
 user_mii_bus setup to realtek-dsa

On Sun, Jan 28, 2024 at 11:49:42PM -0300, Luiz Angelo Daros de Luca wrote:
> Using mii_bus will also prevent an easy way for the driver to query
> those registers (although not used anymore after ds_switch_ops
> .phy_read/write are gone)

Exactly, there is no other remaining call to priv->ops->phy_read() and
priv->ops->phy_write(), so their prototypes can be tailored such that
they need no extra adapter.

> I guess the best approach is to append something that identifies the
> other mdio bus, for example ":user_mii". The result is something like
> this:
> 
> mdio-bus:1d
> mdio-bus:1d:user_mii:00
> mdio-bus:1d:user_mii:01
> ...
> 
> Or, for SMI:
> 
> switch:user_mii:00
> switch:user_mii:01
> ...

This looks good.

> 
> It is good enough for me as these switches have only one MDIO bus.
> 
> We could also bring up some kind of a general suggestion for naming
> user_mii buses. In that case, we should be prepared for multiple mdio
> buses and the mdio node name+@...t (%pOFP) might be appropriate. We
> would get something like this:
> 
> mdio-bus:1d:mdio:00
> mdio-bus:1d:mdio:01
> 
> Or, for SMI:
> 
> switch:mdio:00
> switch:mdio:01
> 
> If there are multiple MDIO buses, it will be mdio@N (not tested).
> 
> mdio-bus:1d:mdio@0:00
> mdio-bus:1d:mdio@0:01
> ...
> mdio-bus:1d:mdio@1:00
> mdio-bus:1d:mdio@1:01
> ...

SJA1110 has 2 MDIO buses and they are named:

	snprintf(bus->id, MII_BUS_ID_SIZE, "%s-base-tx",
		 dev_name(priv->ds->dev));
	snprintf(bus->id, MII_BUS_ID_SIZE, "%s-base-t1",
		 dev_name(priv->ds->dev));

which I think is more descriptive, because in its case, the indices in
"mdio@0" and "mdio@1" are arbitrary numbers.

I don't think we'll find a way to unify the naming convention across the
board. Let's use dev_name(dev) + "-some-driver-specific-qualifier" here,
and hopefully also as a convention from now on.

> I also considered %pOFf but it gives a more verbose device name
> without adding too much useful information:
> 
> !ethernet@...00000!mdio-bus!switch1@29:00
> !ethernet@...00000!mdio-bus!switch1@29:01
> !ethernet@...00000!mdio-bus!switch1@29:02
> 
> And I'm reluctant to add those "!" as they may not play nice with some
> non-ideal scripts reading sysfs. I would, at least, replace them with
> ":" .

I'm also not in love with the exclamation marks that the sysfs code has
to use, to replace the forward slashes that can't be represented in the
filesystem.