Re: [PATCH net-next v11 3/7] dt-bindings: net: qcom: document the ethqos device for SCMI-based systems

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The SCMI PHY driver fails to apply its power domain performance state dynamically when `phy_set_mode` is called on an already powered-on PHY, causing link speed to stall.
- [Medium] Device tree bindings missed update for `power-domains` maxItems constraints, allowing 2 power domains on hardware variants that only support 1.
--

commit 53c301c3f426ecddb2b45080a9a52eeda51a523e
Author: Bartosz Golaszewski <brgl@kernel.org>

dt-bindings: net: qcom: document the ethqos device for SCMI-based systems

Describe the firmware-managed variant of the QCom DesignWare MAC. As the
properties here differ a lot from the HLOS-managed variant, lets put it
in a separate file. Since we need to update the maximum number of power
domains, let's update existing bindings referencing the top-level
snps,dwmac.yaml and limit their maxItems for power-domains to 1.

> diff --git a/Documentation/devicetree/bindings/net/qcom,sa8255p-ethqos.yaml b/Documentation/devicetree/bindings/net/qcom,sa8255p-ethqos.yaml
> new file mode 100644
> index 0000000000000..5158481e5e5b0
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/qcom,sa8255p-ethqos.yaml
[ ... ]
> +  phys:
> +    maxItems: 1
> +
> +  phy-names:
> +    items:
> +      - const: serdes

[Severity: High]
The SCMI PHY driver fails to apply its power domain performance state
dynamically when phy_set_mode is called on an already powered-on PHY. Will
this cause the link speed to stall?

If a link negotiates 2.5Gbps dynamically via phylink, but the PHY was
originally powered on in 1Gbps SGMII mode, the link speed stalls at 1000Mbps
because the PHY's SCMI performance state is not elevated to SPEED_2500.

The performance state is stored but not applied if the device is already
powered on:

drivers/phy/qualcomm/phy-qcom-sgmii-eth-scmi.c:qcom_dwmac_sgmii_phy_scmi_set_mode() {
    ...
	priv->perf_state = (submode == PHY_INTERFACE_MODE_2500BASEX) ?
			   SPEED_2500 : SPEED_1000;

	return 0;
}

The state is only sent to the GENPD core during power on, which executes too
early to capture runtime 2.5Gbps mode transitions:

drivers/phy/qualcomm/phy-qcom-sgmii-eth-scmi.c:qcom_dwmac_sgmii_phy_scmi_power_on() {
    ...
	ret = dev_pm_genpd_set_performance_state(dev, priv->perf_state);
    ...
}

This is triggered when phylink resolves the speed and invokes phy_set_mode_ext
on the already powered-on PHY:

drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c:ethqos_mac_finish_serdes() {
    ...
		ret = phy_set_mode_ext(ethqos->serdes_phy, PHY_MODE_ETHERNET,
				       interface);
    ...
}

> diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> index 2449311c6d28e..d3aff1df3070d 100644
> --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
[ ... ]
> @@ -185,7 +186,8 @@ properties:
>            - const: ahb
>  
>    power-domains:
> -    maxItems: 1
> +    minItems: 1
> +    maxItems: 2

[Severity: Medium]
When the common schema is relaxed to allow 2 power domains here, it affects
all dependent schemas that reference it. Could this unintentionally allow
maxItems: 2 in spacemit,k3-dwmac.yaml and nuvoton,ma35d1-dwmac.yaml?

These schemas reference snps,dwmac.yaml# in their allOf block but lack a
power-domains constraint override:

Documentation/devicetree/bindings/net/spacemit,k3-dwmac.yaml
allOf:
  - $ref: snps,dwmac.yaml#

Documentation/devicetree/bindings/net/nuvoton,ma35d1-dwmac.yaml
allOf:
  - $ref: snps,dwmac.yaml#

Since they only support 1 power domain, dt_binding_check might now
incorrectly pass device trees specifying two power domains for these
hardware variants.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260629-qcom-sa8255p-emac-v11-0-1b7fb95b51f9@oss.qualcomm.com?part=3