linux-kernel - Re: [PATCH net-next v11] 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 for Android: free password hash cracker in your pocket

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

Message-ID: <d260e3d1-20e9-42f6-89b6-e646b8107bc0@alliedtelesis.co.nz>
Date: Sun, 16 Mar 2025 20:11:29 +0000
From: Chris Packham <Chris.Packham@...iedtelesis.co.nz>
To: Andrew Lunn <andrew@...n.ch>
CC: "hkallweit1@...il.com" <hkallweit1@...il.com>, "linux@...linux.org.uk"
	<linux@...linux.org.uk>, "davem@...emloft.net" <davem@...emloft.net>,
	"edumazet@...gle.com" <edumazet@...gle.com>, "kuba@...nel.org"
	<kuba@...nel.org>, "pabeni@...hat.com" <pabeni@...hat.com>,
	"sander@...nheule.net" <sander@...nheule.net>, "markus.stockhausen@....de"
	<markus.stockhausen@....de>, "daniel@...rotopia.org" <daniel@...rotopia.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [PATCH net-next v11] net: mdio: Add RTL9300 MDIO driver


On 15/03/2025 04:26, Andrew Lunn wrote:
>> +static int rtl9300_mdiobus_probe_one(struct device *dev, struct rtl9300_mdio_priv *priv,
>> +				     struct fwnode_handle *node)
>> +{
>> +	struct rtl9300_mdio_chan *chan;
>> +	struct fwnode_handle *child;
>> +	struct mii_bus *bus;
>> +	u32 mdio_bus;
>> +	int err;
>> +
>> +	err = fwnode_property_read_u32(node, "reg", &mdio_bus);
>> +	if (err)
>> +		return err;
>> +
>> +	/* The MDIO interfaces are either in GPHY (i.e. clause 22) or 10GPHY
>> +	 * mode (i.e. clause 45).
> I still need more clarification about this. Is this solely about the
> polling? Or does an interface in C22 mode go horribly wrong when asked
> to do a C45 bus transaction?

It's just the polling. I haven't seen any sign of the bus getting into a 
bad state when using the wrong transaction type.

>> +	bus->name = "Realtek Switch MDIO Bus";
>> +	bus->read = rtl9300_mdio_read_c22;
>> +	bus->write = rtl9300_mdio_write_c22;
>> +	bus->read_c45 = rtl9300_mdio_read_c45;
>> +	bus->write_c45 =  rtl9300_mdio_write_c45;
> You are providing C45 bus methods, independent of the interface
> mode. So when accessing EEE registers in C45 address space, C45 bus
> transactions are going to be used, even on an MDIO interface using C22
> mode. Does this work? Can you actually do both C22 and C45 bus
> transactions independent of the interface mode?
I'm not actually sure if I can mix transactions but it doesn't seem to 
do any harm.

Initially I planned to only supply one of the function pairs depending 
on the mode but I left this in because of this:

https://web.git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/phy/phy.c#n337

I've written myself a little test app that uses SIOCGMIIREG/SIOCSMIIREG 
to exercise the MDIO accesses. It uses SIOCGMIIPHY to look up the MDIO 
address of the PHY attached to the netdev but because of that 
fallthrough looking up the PHY address for a C45 PHY will fail with 
-EOPNOTSUPP.

I've squinted at that code and can't decide if it's a bug or intended. 
It seems to be there to validate if the PHY is actually present and will 
works for C22 because mii_data->reg_num will come through the ioctl (or 
because 0 is a valid register) . It won't work for C45 because 
SIOCGMIIPHY has no way of supplying the MMD device.


>
> 	Andrew