linux-kernel - Re: [PATCH net-next v9] 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: <b506b6e9-d5c3-4927-ab2d-e3a241513082@alliedtelesis.co.nz>
Date: Mon, 10 Mar 2025 02:07:26 +0000
From: Chris Packham <Chris.Packham@...iedtelesis.co.nz>
To: Daniel Golle <daniel@...rotopia.org>
CC: "andrew@...n.ch" <andrew@...n.ch>, "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>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "netdev@...r.kernel.org"
	<netdev@...r.kernel.org>
Subject: Re: [PATCH net-next v9] net: mdio: Add RTL9300 MDIO driver

Hi Daniel,

On 10/03/2025 14:31, Daniel Golle wrote:
> Hi Chris,
>
> On Mon, Mar 10, 2025 at 12:25:36PM +1300, Chris Packham wrote:
>> Add a driver for the MDIO controller on the RTL9300 family of Ethernet
>> switches with integrated SoC. There are 4 physical SMI interfaces on the
>> RTL9300 however access is done using the switch ports. The driver takes
>> the MDIO bus hierarchy from the DTS and uses this to configure the
>> switch ports so they are associated with the correct PHY. This mapping
>> is also used when dealing with software requests from phylib.
>>
>> Signed-off-by: Chris Packham <chris.packham@...iedtelesis.co.nz>
>> ---
>> ...
>> +static int rtl9300_mdio_read_c22(struct mii_bus *bus, int phy_id, int regnum)
>> +{
>> +	struct rtl9300_mdio_chan *chan = bus->priv;
>> +	struct rtl9300_mdio_priv *priv;
>> +	struct regmap *regmap;
>> +	int port;
>> +	u32 val;
>> +	int err;
>> +
>> +	priv = chan->priv;
>> +	regmap = priv->regmap;
>> +
>> +	port = rtl9300_mdio_phy_to_port(bus, phy_id);
>> +	if (port < 0)
>> +		return port;
>> +
>> +	mutex_lock(&priv->lock);
>> +	err = rtl9300_mdio_wait_ready(priv);
>> +	if (err)
>> +		goto out_err;
>> +
>> +	err = regmap_write(regmap, SMI_ACCESS_PHY_CTRL_2, FIELD_PREP(PHY_CTRL_INDATA, port));
>> +	if (err)
>> +		goto out_err;
>> +
>> +	val = FIELD_PREP(PHY_CTRL_REG_ADDR, regnum) |
>> +	      FIELD_PREP(PHY_CTRL_PARK_PAGE, 0x1f) |
>> +	      FIELD_PREP(PHY_CTRL_MAIN_PAGE, 0xfff) |
>> +	      PHY_CTRL_READ | PHY_CTRL_TYPE_C22 | PHY_CTRL_CMD;
> Using "raw" access to the PHY and thereby bypassing the MDIO
> controller's support for hardware-assisted page access is problematic.
> The MDIO controller also polls all PHYs status in hardware and hence
> be aware of the currently selected page. Using raw access to switch
> the page of a PHY "behind the back" of the hardware polling mechanism
> results in in occassional havoc on link status changes in case Linux'
> reading the phy status overlaps with the hardware polling.
> This is esp. when using RealTek's 2.5GBit/s PHYs which require using
> paged access in their read_status() function.
>
> Markus Stockhausen (already in Cc) has implemented a nice solution to
> this problem, including documentation, see
> https://git.openwrt.org/?p=openwrt/openwrt.git;a=blob;f=target/linux/realtek/files-6.6/drivers/net/ethernet/rtl838x_eth.c;h=4b79090696e341ed1e432a7ec5c0f7f92776f0e1;hb=HEAD#l1631

I read that code/comment a few times and I must admit I still don't 
quite get it. Part of the problem might be that my hardware platform is 
using C45 PHYs and that's what I've been testing with. The C22 support 
is based on my reading of the datasheet and some of what I can glean 
from openwrt (although I completely missed that comment when I read 
through the driver the first time).

> Including a similar mechanism in this driver for C22 read and write
> operations would be my advise, so hardware-assisted access to the PHY
> pages is always used, and hence the hardware polling mechanism is aware
> of the currently selected page.

So far upstream Linux doesn't have generic paged PHY register functions. 
It sounds like that'd be a prerequisite for this.

> Other than that the driver looks really good now, and will allow using
> existing RealTek PHY drivers independently of whether they are used with
> RealTek's switch SoCs or with non-RealTek systems -- this has always
> been a big issue with OpenWrt's current implementation and I look
> forward to use this driver instead asap ;)