Re: [PATCH v3 2/3] dt-bindings: phy: airoha: Document support for AN7583 Gen2 PCIe PHY

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Low] The binding title incorrectly refers to 'AN7581' instead of 'AN7583'.
- [Low] The register sizes in the example are off-by-one, incorrectly using the maximum offset instead of the byte count.
- [Low] The binding filename 'airoha,an7583-pcie-gen2-phy.yaml' does not match the compatible string 'airoha,an7583-pcie-phy'.
--

commit a30f9e597cd1c12059c1c3ca3b0550d5e5800f4d
Author: Christian Marangi <ansuelsmth@gmail.com>
Subject: dt-bindings: phy: airoha: Document support for AN7583 Gen2 PCIe PHY

This commit documents support for the AN7583 Gen2 PCIe PHY, which is used
to make the Gen2 PCIe port work. It adds the required registers to configure
the PCIe PHY and provides a usage example.

> diff --git a/Documentation/devicetree/bindings/phy/airoha,an7583-pcie-gen2-phy.yaml b/Documentation/devicetree/bindings/phy/airoha,an7583-pcie-gen2-phy.yaml
> new file mode 100644
> index 0000000000000..f31d838871fc1
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/airoha,an7583-pcie-gen2-phy.yaml
> @@ -0,0 +1,63 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/phy/airoha,an7583-pcie-gen2-phy.yaml#

[Severity: Low]
Should the filename (and consequently the $id path) match the defined
compatible string airoha,an7583-pcie-phy? The extraneous 'gen2-' in the
filename causes it to diverge from the compatible string, which typically
triggers a schema validation warning when running dt_binding_check.

> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Airoha AN7581 SoC PCIe Gen2 PHY

[Severity: Low]
Does this title incorrectly refer to AN7581 instead of the intended target
AN7583?

[ ... ]

> +examples:
> +  - |
> +    phy@1fa5f000 {
> +        compatible = "airoha,an7583-pcie-phy";
> +        reg = <0x1fa5f000 0xff>,
> +              <0x1fa5e000 0x3ff>,
> +              <0x1fa5a000 0x8ff>;

[Severity: Low]
Are these register sizes in the example off-by-one? The sizes 0xff, 0x3ff,
and 0x8ff (255, 1023, and 2303 bytes) are not multiples of 4, meaning the
final byte of the last 32-bit register is excluded from the memory region.

A developer copying this example verbatim might create an invalid device
tree that could cause out-of-bounds accesses when the driver reads the final
register.

> +        reg-names = "ana", "pma", "dig";
> +
> +        phys = <&serdes_phy>;
> +
> +        #phy-cells = <0>;
> +    };

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260827131551.26725-1-ansuelsmth@gmail.com?part=2

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy