Re: [PATCH net-next 3/9] dt-bindings: net: qcom,ethqos: add qcom,shikra-ethqos compatible

[netdev-bot+sashiko@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider:

Critical: 0 · High: 0 · Medium: 1 · Low: 1

- [Medium] The new `qcom,shikra-ethqos` conditional in
  Documentation/devicetree/bindings/net/qcom,ethqos.yaml enforces only…
- [Low] Reported concern: the binding documents `qcom,shikra-ethqos` (a
  standalone compatible with no fallback), two new clocks (`axi-noc`,…

--

dt-bindings: net: qcom,ethqos: add qcom,shikra-ethqos compatible

Adds the qcom,shikra-ethqos compatible to the ethqos and snps,dwmac
bindings, extends the clock list with axi-noc and pcie-tile-axi-noc, and
adds an if/else block constraining Shikra to six clocks while requiring
operating-points-v2.

> diff --git a/Documentation/devicetree/bindings/net/qcom,ethqos.yaml b/Documentation/devicetree/bindings/net/qcom,ethqos.yaml
> index 423959cb928d9..ad2cc706fa740 100644
> --- a/Documentation/devicetree/bindings/net/qcom,ethqos.yaml
> +++ b/Documentation/devicetree/bindings/net/qcom,ethqos.yaml

[ ... ]

> @@ -32,6 +29,7 @@ properties:
>            - qcom,qcs404-ethqos
>            - qcom,sa8775p-ethqos
>            - qcom,sc8280xp-ethqos
> +          - qcom,shikra-ethqos
>            - qcom,sm8150-ethqos

[Severity: Low]

This is a standalone compatible with no fallback, and at this point in the
series nothing consumes it, the two new NOC clocks, or the mandatory OPP
table.

Checking the end of the series, this is addressed by the later patches:
"net: stmmac: qcom-ethqos: add per-platform NOC clock voting" adds
qcom_ethqos_init_noc_clks() with devm_clk_bulk_get() and
devm_pm_opp_of_add_table(), and "net: stmmac: qcom-ethqos: add Shikra EMAC
support" adds shikra_noc_clks[] plus:

	{ .compatible = "qcom,shikra-ethqos", .data = &shikra_data},

to qcom_ethqos_match[], so the compatible does bind by the end of the
series. Noting it only for ordering visibility.

[ ... ]

> @@ -81,6 +83,11 @@ properties:
>        - const: cpu-mac
>        - const: mac-mem
>  
> +  operating-points-v2: true
> +
> +  opp-table:
> +    type: object
> +

[Severity: Medium]

The OPP properties are added here, but power-domains is never mentioned
anywhere in qcom,ethqos.yaml. It is only permitted through the inherited
$ref to snps,dwmac.yaml, which defines:

  power-domains:
    maxItems: 1

The commit message says the OPP table exists "to vote VDD_CX to SVS when
the NOC clocks are enabled" through required-opps. Can that vote take
effect without a power-domains phandle on the node?

A per-OPP required-opps entry reaches genpd only via:

drivers/base/power/common.c:dev_pm_domain_set_performance_state() {
	if (dev->pm_domain && dev->pm_domain->set_performance_state)
		return dev->pm_domain->set_performance_state(dev, state);

	return 0;
}

With no power-domains phandle, dev->pm_domain is NULL and this returns 0
without doing anything, so dev_pm_opp_set_rate() ->  _set_required_opps()
-> _set_opp_level() silently drops the CX corner while the two 120 MHz NOC
clocks run.

> @@ -92,6 +99,41 @@ required:
>    - clock-names
>    - reg-names
>  
> +allOf:
> +  - $ref: snps,dwmac.yaml#
> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: qcom,shikra-ethqos
> +    then:
> +      properties:
> +        clocks:
> +          minItems: 6
> +        clock-names:
> +          items:
> +            - const: stmmaceth
> +            - const: pclk
> +            - const: ptp_ref
> +            - const: rgmii
> +            - const: axi-noc
> +            - const: pcie-tile-axi-noc
> +      required:
> +        - operating-points-v2

[Severity: Medium]

Should power-domains also be listed in this required block, next to
operating-points-v2?

As written, a Shikra node carrying operating-points-v2 and an OPP table
full of required-opps but no power-domains = <&rpmhpd RPMHPD_CX> passes
dt_binding_check and dtbs_check cleanly, and the schema emits no
diagnostic for the missing phandle.

Grepping drivers/net/ethernet/stmicro/stmmac/ at the end of the series
returns no pm_domain references, so the device tree is the only place the
CX dependency can be expressed.

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260904-shikra_ethernet-v1-0-a50765996035%40oss.qualcomm.com