Re: [PATCH v2 2/5] dt-bindings: net: qca,ar803x: Add IPQ5018 Internal GE PHY support

[Rob Herring <robh@kernel.org>]

On Wed, May 28, 2025 at 08:59:45PM +0400, George Moussalem wrote:
> Hi Rob,
> 
> On 5/28/25 20:30, Rob Herring (Arm) wrote:
> > 
> > On Wed, 28 May 2025 18:45:48 +0400, George Moussalem wrote:
> > > Document the IPQ5018 Internal Gigabit Ethernet PHY found in the IPQ5018
> > > SoC. Its output pins provide an MDI interface to either an external
> > > switch in a PHY to PHY link scenario or is directly attached to an RJ45
> > > connector.
> > > 
> > > The PHY supports 10/100/1000 mbps link modes, CDT, auto-negotiation and
> > > 802.3az EEE.
> > > 
> > > For operation, the LDO controller found in the IPQ5018 SoC for which
> > > there is provision in the mdio-4019 driver. In addition, the PHY needs
> > > to take itself out of reset and enable the RX and TX clocks.
> > > 
> > > Two common archictures across IPQ5018 boards are:
> > > 1. IPQ5018 PHY --> MDI --> RJ45 connector
> > > 2. IPQ5018 PHY --> MDI --> External PHY
> > > In a phy to phy architecture, DAC values need to be set to accommodate
> > > for the short cable length. As such, add an optional boolean property so
> > > the driver sets the correct register values for the DAC accordingly.
> > > 
> > > Signed-off-by: George Moussalem <george.moussalem@outlook.com>
> > > ---
> > >   .../devicetree/bindings/net/qca,ar803x.yaml        | 52 +++++++++++++++++++++-
> > >   1 file changed, 51 insertions(+), 1 deletion(-)
> > > 
> > 
> > My bot found errors running 'make dt_binding_check' on your patch:
> > 
> > yamllint warnings/errors:
> > 
> > dtschema/dtc warnings/errors:
> > /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/qca,ar803x.example.dtb: ethernet-phy@7 (ethernet-phy-id004d.d0c0): clocks: [[4294967295, 36], [4294967295, 37]] is too long
> > 	from schema $id: http://devicetree.org/schemas/net/ethernet-phy.yaml#
> > 
> > doc reference errors (make refcheckdocs):
> > 
> > See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20250528-ipq5018-ge-phy-v2-2-dd063674c71c@outlook.com
> > 
> > The base for the series is generally the latest rc1. A different dependency
> > should be noted in *this* patch.
> > 
> > If you already ran 'make dt_binding_check' and didn't see the above
> > error(s), then make sure 'yamllint' is installed and dt-schema is up to
> > date:
> > 
> > pip3 install dtschema --upgrade
> > 
> > Please check and re-submit after running the above command yourself. Note
> > that DT_SCHEMA_FILES can be set to your schema file to speed up checking
> > your schema. However, it must be unset to test all examples with your schema.
> > 
> 
> 
> Really weird, I've checked this numerous times:
> 
> (myenv) george@sl2-ubuntu:~/src/linux-next$ make dt_binding_check
> DT_SCHEMA_FILES=qca,ar803x.yaml
>   SCHEMA  Documentation/devicetree/bindings/processed-schema.json
>   CHKDT   ./Documentation/devicetree/bindings
>   LINT    ./Documentation/devicetree/bindings
>   DTEX    Documentation/devicetree/bindings/net/qca,ar803x.example.dts
>   DTC [C] Documentation/devicetree/bindings/net/qca,ar803x.example.dtb
> (myenv) george@sl2-ubuntu:~/src/linux-next$ pip3 install dtschema --upgrade
> Requirement already satisfied: dtschema in
> /home/george/myenv/lib/python3.12/site-packages (2025.2)
> Requirement already satisfied: ruamel.yaml>0.15.69 in
> /home/george/myenv/lib/python3.12/site-packages (from dtschema) (0.18.10)
> Requirement already satisfied: jsonschema<4.18,>=4.1.2 in
> /home/george/myenv/lib/python3.12/site-packages (from dtschema) (4.17.3)
> Requirement already satisfied: rfc3987 in
> /home/george/myenv/lib/python3.12/site-packages (from dtschema) (1.3.8)
> Requirement already satisfied: pylibfdt in
> /home/george/myenv/lib/python3.12/site-packages (from dtschema) (1.7.2)
> Requirement already satisfied: attrs>=17.4.0 in
> /home/george/myenv/lib/python3.12/site-packages (from
> jsonschema<4.18,>=4.1.2->dtschema) (25.3.0)
> Requirement already satisfied: pyrsistent!=0.17.0,!=0.17.1,!=0.17.2,>=0.14.0
> in /home/george/myenv/lib/python3.12/site-packages (from
> jsonschema<4.18,>=4.1.2->dtschema) (0.20.0)
> Requirement already satisfied: ruamel.yaml.clib>=0.2.7 in
> /home/george/myenv/lib/python3.12/site-packages (from
> ruamel.yaml>0.15.69->dtschema) (0.2.12)
> (myenv) george@sl2-ubuntu:~/src/linux-next$ make dt_binding_check
> DT_SCHEMA_FILES=qca,ar803x.yaml
>   SCHEMA  Documentation/devicetree/bindings/processed-schema.json
>   CHKDT   ./Documentation/devicetree/bindings
>   LINT    ./Documentation/devicetree/bindings
>   DTEX    Documentation/devicetree/bindings/net/qca,ar803x.example.dts
>   DTC [C] Documentation/devicetree/bindings/net/qca,ar803x.example.dtb
> 
> I only found the same errors when removing the DT_SCHEMA_FILES property.

Correct.

> Is that because ethernet-phy.yaml is a catch-all based on the pattern on the
> compatible property (assuming my understanding is correct)? How would we get
> around that without modifying ethernet-phy.yaml only for this particular PHY
> (with a condition)? This PHY needs to enable two clocks and the restriction
> is on 1.

It's kind of a mess since ethernet phys didn't have compatibles 
frequently and then there was resistance to adding compatibles. You know 
we don't need compatibles because phys are discoverable and all. Well, 
except for everything we keep adding for them in DT like clocks...

We probably need to split out common phy properties to its own schema. 
And then add a schema just for phys with no compatible string (so 
'select' needs to match on $nodename with ethernet-phy as now, but also 
have 'not: { required: [compatible] }'. And then a schema for the 
'generic' phys with just ethernet-phy-ieee802.3-c22 or 
ethernet-phy-ieee802.3-c45. Then we'll have to look at what to do with 
ones with "^ethernet-phy-id[a-f0-9]{4}\\.[a-f0-9]{4}$" compatibles. 
Probably, we need to add specific id's to the generic schema or in their 
own schemas.

Or we can just change clocks in ethernet-phys.yaml to:

minItems: 1
maxItems: 2

And kick that can down the road...

Rob