All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Kyle Switch" <kyle.switch@motor-comm.com>
Cc: devicetree@vger.kernel.org, conor+dt@kernel.org, robh@kernel.org
Subject: Re: [PATCH v16 2/2] dt-bindings: net: Document Motorcomm YT8824 PHY package
Date: Fri, 11 Sep 2026 07:47:03 +0000	[thread overview]
Message-ID: <20260911074704.235ED1F00893@smtp.kernel.org> (raw)
In-Reply-To: <20260910074624.2426899-3-kyle.switch@motor-comm.com>

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] Misuse of a standard hardware PHY ID string as the compatible string for an Ethernet PHY package container node.
- [Low] Mismatch between the node name unit-address and its 'reg' property in the example.
- [Low] The YAML binding completely omits the 'required' block, failing to enforce mandatory properties.
--

commit 086492b3eff200332b4ab4eddc5f5f7362981d4f
Author: Kyle Switch <kyle.switch@motor-comm.com>

dt-bindings: net: Document Motorcomm YT8824 PHY package

Motorcomm YT8824 Ethernet PHY is a PHY package of 4 PHY-s. This commit
documents the required property to make the PHY package correctly configure
and work.

> diff --git a/Documentation/devicetree/bindings/net/motorcomm,yt8824.yaml b/Documentation/devicetree/bindings/net/motorcomm,yt8824.yaml
> new file mode 100644
> index 0000000000000..9907a4a7e63e8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/motorcomm,yt8824.yaml
[ ... ]
> +properties:
> +  compatible:
> +    enum:
> +      - ethernet-phy-id4f51.e8b8

[Severity: Medium]
Does this misuse a standard hardware PHY ID string as the compatible string
for an Ethernet PHY package container node?

dt-schema selects bindings based on the compatible property. Because
ethernet-phy-id4f51.e8b8 is an ID format strictly reserved for individual
hardware PHY child nodes (as defined by the pattern in ethernet-phy.yaml),
using it for the parent package container causes dt-schema to incorrectly
apply this package schema to individual child PHY nodes when running make
dt_binding_check on device trees containing these PHYs.

> +
> +  motorcomm,interface-mode:
> +    description: |
> +      PHY package interface mode.
> +    $ref: /schemas/types.yaml#/definitions/string
> +    enum:
> +      - internal
> +      - usxgmii
> +    default: internal
> +
> +unevaluatedProperties: false
> +

[Severity: Low]
Does this YAML binding completely omit the required block, failing to enforce
mandatory properties?

Standard Device Tree schema design mandates that mandatory properties,
specifically compatible, be explicitly listed in a required block. Without
it, the schema validation will not enforce the presence of the compatible
property on matching nodes, leading to incomplete validation.

> +examples:
> +  - |
> +    mdio {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        ethernet-phy-package@0 {
> +            #address-cells = <1>;
> +            #size-cells = <0>;
> +            compatible = "ethernet-phy-id4f51.e8b8";
> +            reg = <9>;

[Severity: Low]
Is there a mismatch between the node name unit-address and its reg property
in this example?

The example block defines the node as ethernet-phy-package@0 but specifies
reg = <9>. The Device Tree specification requires that the unit address in
the node name (the part after @) must exactly match the first address
specified in the reg property. This will cause a unit_address_vs_reg or
simple_bus_reg warning during compilation and validation.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260910074624.2426899-1-kyle.switch@motor-comm.com?part=2

  reply	other threads:[~2026-09-11  7:47 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-10  7:46 [PATCH v16 0/2] net: phy: Add driver for Motorcomm Quad 2.5GbE phy Kyle Switch
2026-09-10  7:46 ` [PATCH v16 1/2] " Kyle Switch
2026-09-11  7:47   ` sashiko-bot
2026-09-10  7:46 ` [PATCH v16 2/2] dt-bindings: net: Document Motorcomm YT8824 PHY package Kyle Switch
2026-09-11  7:47   ` sashiko-bot [this message]
2026-09-11  8:44   ` Krzysztof Kozlowski
2026-09-12  5:20     ` Kyle Switch
2026-09-11 11:52   ` Andrew Lunn
2026-09-12  5:16     ` Kyle Switch

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260911074704.235ED1F00893@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=kyle.switch@motor-comm.com \
    --cc=robh@kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.