Re: [PATCH 1/3] dt-bindings: net: qca,ipq5018: Split IPQ5018 PHY bindings from ar803x

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Schema Validation Bypass Failure and Binding Incompleteness
- [High] Broken Schema Validation for AR803x PHYs due to Removed Selection Criteria
--

commit 48327af0b67eba3af6e8fa934ae07a6c6cb94d54
Author: George Moussalem <george.moussalem@outlook.com>

dt-bindings: net: qca,ipq5018: Split IPQ5018 PHY bindings from ar803x

This commit moves the Qualcomm IPQ5018 Internal PHY definition into a dedicated
file. It isolates the binding from qca,ar803x.yaml in an attempt to support
both RX and TX clocks, as the core ethernet-phy.yaml restricts clocks to one.

> diff --git a/Documentation/devicetree/bindings/net/qca,ar803x.yaml b/Documentation/devicetree/bindings/net/qca,ar803x.yaml
> index 7ae5110e7aa2c..3acd09f0da863 100644
> --- a/Documentation/devicetree/bindings/net/qca,ar803x.yaml
> +++ b/Documentation/devicetree/bindings/net/qca,ar803x.yaml

[ ... ]

>  properties:
> -  compatible:
> -    enum:
> -      - ethernet-phy-id004d.d0c0
> -
>    qca,clk-out-frequency:
>      description: Clock output frequency in Hertz.
>      $ref: /schemas/types.yaml#/definitions/uint32

[Severity: High]
With the compatible property removed, how will dt-schema match this binding
to device nodes?

Because device tree schema validation relies on the compatible property
to automatically generate selection conditions, removing it leaves the schema
with no way to match device nodes.

This causes qca,ar803x.yaml to be silently skipped during validation,
breaking validation for properties like qca,clk-out-frequency for all
downstream AR803x PHY configurations.

[ ... ]

> diff --git a/Documentation/devicetree/bindings/net/qca,ipq5018.yaml b/Documentation/devicetree/bindings/net/qca,ipq5018.yaml
> new file mode 100644
> index 0000000000000..7cdc96f6d389f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/qca,ipq5018.yaml

[ ... ]

> +properties:
> +  compatible:
> +    const: ethernet-phy-id004d.d0c0
> +
> +  reg:
> +    const: 7  # This PHY is always at MDIO address 7 in the IPQ5018 SoC
> +
> +  clocks:
> +    items:
> +      - description: RX clock
> +      - description: TX clock

[Severity: High]
The commit message mentions splitting this file to bypass the 1-clock
restriction in ethernet-phy.yaml, but does this actually prevent the core
schema from applying?

Since the device tree node is named ethernet-phy@7 (as seen in the examples),
dt-schema will still automatically apply the core ethernet-phy.yaml schema
due to its select logic on the node name. Validation will therefore still
fail for having two clocks.

[ ... ]

> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - clock-names
> +  - resets
> +
> +additionalProperties: false

[Severity: High]
Does this block standard Ethernet PHY properties?

Because additionalProperties is set to false without a reference to the
core ethernet-phy.yaml schema, any standard PHY properties (such as
max-speed or interrupts) added to a device tree will be incorrectly
rejected.

The standard approach is to modify ethernet-phy.yaml to allow multiple
clocks, include the $ref to it here, and use unevaluatedProperties: false.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260601-ipq5018-gephy-clocks-v1-0-2df8287712c3@outlook.com?part=1