Re: [PATCH net-next v5 07/11] net: pcs: xpcs: add Rockchip RK3568 platform glue driver

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Incorrect MMD write remapping for MII vendor registers breaks configuration for ports 1, 2, and 3.
- [High] Calculation of `eee_mult_fact` overflows the 32-bit `unsigned long` on 32-bit architectures, leading to completely incorrect EEE configurations.
- [Medium] Acquiring `device_lock()` of another device from within a probe path causes a lockdep warning (recursive locking).
--

commit ba02c9523dcac3b326b325ec63d9a16255030d26
Author: Coia Prant <coiaprant@gmail.com>
net: pcs: xpcs: add Rockchip RK3568 platform glue driver

This commit adds the Rockchip RK3568 platform glue driver for the Synopsys
DesignWare XPCS. It creates a virtual MDIO bus to translate standard MDIO
operations into APB3 register accesses with address remapping.

Link: https://dl.radxa.com/rock3/docs/hw/datasheet/Rockchip%20RK3568%20TRM%20Part2%20V1.1-20210301.pdf (Page 2078)

> diff --git a/drivers/net/pcs/pcs-xpcs-rk.c b/drivers/net/pcs/pcs-xpcs-rk.c
> new file mode 100644
> index 0000000000000..7fd1408c044cc
> --- /dev/null
> +++ b/drivers/net/pcs/pcs-xpcs-rk.c

[ ... ]

> +static int xpcs_rk_mdio_write_remapping(int addr, int dev, int reg)
> +{
> +	switch (dev) {
> +	case MDIO_MMD_PMAPMD:
> +		return ROCKCHIP_MMD_PMAPMD;
> +	case MDIO_MMD_VEND2:
> +		break;
> +	default:
> +		return -ENXIO;
> +	}
> +
> +	/* Writable only on MII */
> +	switch (reg) {
> +	case DW_VR_MII_AN_CTRL:
> +	case DW_VR_MII_AN_INTR_STS:
> +	case DW_VR_MII_EEE_MCTRL0:
> +	case DW_VR_MII_EEE_MCTRL1:
> +	case DW_VR_MII_DIG_CTRL2:
> +		return ROCKCHIP_MMD_MII;
> +	default:
> +		break;
> +	}

[Severity: High]
Does this break the configuration for ports 1, 2, and 3? This block intercepts
writes to these specific MII vendor registers and unconditionally routes them
to MMD 7 (port 0), ignoring the port address mapping. 

If these registers are modified via the MDIO bus for ports greater than 0
during initialization, the auto-negotiation and EEE configurations might be
applied to port 0 instead, potentially breaking the network links for the
secondary ports.

[ ... ]

> +static int xpcs_rk_init_clk(struct dw_xpcs_rk *pxpcs)
> +{

[ ... ]

> +	/*
> +	 * Compute the multiplier for the EEE clock so that
> +	 * clk_eee_period * (mult_fact + 1) falls within 80..120 ns.
> +	 *
> +	 * On RK3568, clk_xpcs_eee is muxed between gpll200 (200 MHz, 5 ns)
> +	 * and cpll125 (125 MHz, 8 ns), selected by CRU_CLKSEL_CON29 bit 13.
> +	 * The reset value is 0 (200 MHz), but derive the value at runtime to
> +	 * stay correct if the mux is changed by a board.
> +	 */
> +	pxpcs->eee_mult_fact = DIV_ROUND_CLOSEST(100 * clk_get_rate(pxpcs->eee_clk),
> +						 1000000000UL) - 1;

[Severity: High]
Can this calculation overflow on 32-bit architectures? clk_get_rate() returns
an unsigned long. On 32-bit architectures (like an ARM32 kernel or during a 
COMPILE_TEST build), the multiplication of 100 * 125,000,000 equals 
12,500,000,000, which exceeds the 32-bit maximum (4,294,967,295). 

This overflow could result in completely incorrect hardware timings for the
EEE configuration.

[ ... ]

> +struct dw_xpcs *xpcs_rk_create(struct device *dev, struct device_node *np)
> +{

[ ... ]

> +	pdev = of_find_device_by_node(pcs_np);
> +	of_node_put(pcs_np);
> +	if (!pdev)
> +		return ERR_PTR(-EPROBE_DEFER);
> +
> +	device_lock(&pdev->dev);
> +	pxpcs = platform_get_drvdata(pdev);
> +	if (!pxpcs || !pxpcs->bus) {
> +		device_unlock(&pdev->dev);

[Severity: Medium]
Will this trigger a lockdep warning for recursive locking? The MAC driver's 
probe already holds its own dev->mutex via really_probe() when it invokes 
xpcs_rk_create(), and this code then attempts to acquire the XPCS device's 
dev->mutex. 

Since all default device locks share the same lockdep class, could this
un-annotated nesting cause a false-positive lockdep splat that pollutes
kernel logs (and panics if panic_on_warn is enabled)?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260913185849.907479-1-coiaprant@gmail.com?part=7

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy