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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] The fallback logic for missing delay properties incorrectly applies default delays to both RX and TX paths for RGMII_TXID and RGMII_RXID modes, breaking RGMII semantics.
- [High] Silent truncation of negative MDIO read errors due to `u16` variable typing causes the PHY reset polling loop to falsely report success.
- [High] The RGMII configuration is written to the PHY after the software reset, preventing the settings from taking effect.
--

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

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

This commit adds a new driver for the DAPU Telecom DAP8211R(I) Gigabit
Ethernet PHY. It supports RGMII delay configuration through device tree
properties and optional 180-degree TX clock phase shifting.

> diff --git a/drivers/net/phy/dap8211r.c b/drivers/net/phy/dap8211r.c
> new file mode 100644
> index 0000000000000..e6381fd8c98c2
> --- /dev/null
> +++ b/drivers/net/phy/dap8211r.c
[ ... ]
> +static int dap8211r_config_init(struct phy_device *phydev)
> +{
> +	struct device_node *phydev_node = phydev->mdio.dev.of_node;
> +	u16 set = 0, val = 0;

[Severity: High]
Does declaring val as u16 truncate negative error codes returned by 
dap8211r_read_ext() during polling? 

If dap8211r_read_ext() returns a negative error like -EIO, the truncated 
value will still have its high bit set, which might cause the polling 
loop below to exit early and falsely report success.

> +	int ret;
> +	s32 internal_delay;
> +
> +	if (of_property_read_bool(phydev_node, "dapu,tx-inverted-clk"))
> +		set |= DAP8211R_RGMII_CLK_INVERT;
> +
> +	internal_delay = phy_get_internal_delay(phydev, dap8211r_internal_delay,
> +						DAP8211R_DELAY_SIZE, true);
> +	if (internal_delay < 0) {
> +		if (phydev->interface == PHY_INTERFACE_MODE_RGMII)
> +			internal_delay = 0;
> +		else
> +			internal_delay = DAP8211R_DEFAULT_DEL_SEL;
> +	}

[Severity: High]
If the rx-internal-delay-ps property is missing and the interface mode is 
PHY_INTERFACE_MODE_RGMII_TXID, does this fallback logic incorrectly apply 
the default delay (DAP8211R_DEFAULT_DEL_SEL) to the RX path? 

For RGMII_TXID, should the RX delay instead be 0?

> +
> +	set |= FIELD_PREP(DAP8211R_RGMII_RX_DEL_MASK, internal_delay);
> +
> +	internal_delay = phy_get_internal_delay(phydev, dap8211r_internal_delay,
> +						DAP8211R_DELAY_SIZE, false);
> +	if (internal_delay < 0) {
> +		if (phydev->interface == PHY_INTERFACE_MODE_RGMII)
> +			internal_delay = DAP8211R_INIT_TX_DEL_VAL;
> +		else
> +			internal_delay = DAP8211R_DEFAULT_DEL_SEL;
> +	}

[Severity: High]
Similarly, if tx-internal-delay-ps is missing and the interface mode is 
PHY_INTERFACE_MODE_RGMII_RXID, will this incorrectly apply the default delay 
to the TX path when it should be 0?

> +
> +	set |= FIELD_PREP(DAP8211R_RGMII_TX_DEL_MASK, internal_delay);
> +
> +	ret = dap8211r_modify_ext(phydev, DAP8211R_PHY_CON, DAP8211R_PHY_SW_RST, 0);
> +	if (ret)
> +		return ret;
> +
> +	/* Wait for reset self-clear (max 200 us) */
> +	ret = read_poll_timeout(dap8211r_read_ext, val,
> +				(val & DAP8211R_PHY_SW_RST),
> +				20, 200, false, phydev, DAP8211R_PHY_CON);
> +	if (ret)
> +		return ret;
> +
> +	ret = dap8211r_modify_ext(phydev, DAP8211R_RGMII_CON, DAP8211R_RGMII_CONFIG_MASK, set);

[Severity: High]
Is the RGMII configuration written to the PHY after the software reset 
completes? 

The initialization comment mentions a software reset is used to apply 
settings. If we write the RGMII configuration after the reset, will the 
settings take effect, or will the PHY operate with uninitialized values?

> +	if (ret)
> +		return ret;
> +
> +	return 0;
> +}
[ ... ]

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