Re: [PATCH net-next v6 3/3] net: phy: add DAPU Telecom DAP8211R(I) Gigabit Ethernet PHY driver

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] RGMII configuration is written after the PHY software reset, preventing delay settings from taking effect.
--

--- Patch [3]: [PATCH net-next v6 3/3] net: phy: add DAPU Telecom DAP8211R(I) Gigabit Ethernet PHY driver ---
commit 0de15848a30c891806ee2f58213d5a3b6475e080
Author: Artem Shimko <a.shimko.dev@gmail.com>

net: phy: add DAPU Telecom DAP8211R(I) Gigabit Ethernet PHY driver

Add a new PHY driver for the DAPU Telecom DAP8211R(I) Gigabit Ethernet PHY.
This driver implements extended register access and provides device tree
support for RGMII delay configuration.

> diff --git a/drivers/net/phy/dap8211r.c b/drivers/net/phy/dap8211r.c
> new file mode 100644
> --- /dev/null
> +++ b/drivers/net/phy/dap8211r.c

[ ... ]

> +static int dap8211r_config_init(struct phy_device *phydev)
> +{
> +	u16 set = 0;

[ ... ]

> +	set |= FIELD_PREP(DAP8211R_RGMII_RX_DEL_MASK, rx_internal_delay);
> +	set |= FIELD_PREP(DAP8211R_RGMII_TX_DEL_MASK, tx_internal_delay);
> +
> +	ret = dap8211r_modify_ext(phydev, DAP8211R_PHY_CON, DAP8211R_PHY_SW_RST, 0);
> +	if (ret)
> +		return ret;
> +
> +	/* Wait for reset self-clear (from low active to high) */
> +	ret = read_poll_timeout(dap8211r_read_ext, val,
> +				(val & DAP8211R_PHY_SW_RST),
> +				20, 200, false, phydev, DAP8211R_PHY_CON);
> +	if (ret)
> +		return ret;
> +	if (val < 0)
> +		return val;
> +
> +	ret = dap8211r_modify_ext(phydev, DAP8211R_RGMII_CON, DAP8211R_RGMII_CONFIG_MASK, set);

[Severity: High]
Will these delay settings take effect? The function documentation notes that a
software reset is required to apply the settings, but the RGMII configuration
is written to DAP8211R_RGMII_CON after the software reset has already been
issued and completed.

Should dap8211r_modify_ext() for DAP8211R_RGMII_CON be called before
triggering DAP8211R_PHY_SW_RST so the PHY latches the new delay values?

> +	if (ret)
> +		return ret;
> +
> +	return 0;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260805085540.452260-1-a.shimko.dev@gmail.com?part=3