Re: [PATCH v3 1/9] dt-bindings: phy: Document ST STM32MP25 USB2-FEMTO PHY

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Low] The `reg` property is defined in the schema and used in the example but is missing from the `required` list.
- [Low] The example node name 'usb-phy@2400' violates the generic PHY schema pattern.
- [Low] The '#clock-cells' property lacks documentation for the clock indices.
--

--- Patch [1]: [PATCH v3 1/9] dt-bindings: phy: Document ST STM32MP25 USB2-FEMTO PHY ---
commit fee849432713aad415e73ed872e6de622206bee8
Author: Pankaj Dev <[email protected]>

dt-bindings: phy: Document ST STM32MP25 USB2-FEMTO PHY

This commit documents the USB2 phy for STM32MP25 USB2 controllers. It introduces
bindings for two slightly different phys present in the STM32MP25 SoC using
separate compatible strings.

> diff --git a/Documentation/devicetree/bindings/phy/st,stm32-usb2phy.yaml b/Documentation/devicetree/bindings/phy/st,stm32-usb2phy.yaml
> new file mode 100644
> index 0000000000000..cc5044790beea
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/st,stm32-usb2phy.yaml
> @@ -0,0 +1,67 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---

[ ... ]

> +properties:
> +  compatible:
> +    enum:
> +      - st,stm32mp25-usb2phy1
> +      - st,stm32mp25-usb2phy2
> +
> +  "#clock-cells":
> +    const: 1
> +

[Severity: Low]
This sets #clock-cells to 1, meaning consumers will need to pass an index,
but there doesn't appear to be any documentation of the valid indices (e.g., 
0 for PLL, 1 for CMN) in the binding or a dt-bindings header file.

Could this be updated to document what index corresponds to which clock so
device tree authors aren't forced to rely on undocumented magic numbers?

> +  "#phy-cells":
> +    const: 0
> +
> +  reg:
> +    maxItems: 1

[ ... ]

> +required:
> +  - compatible
> +  - "#clock-cells"
> +  - "#phy-cells"
> +  - clocks
> +  - resets
> +  - vdd33-supply

[Severity: Low]
The reg property is defined in the schema and used in the example below,
but it is missing from this list.

If it is omitted here, device tree nodes without a register offset can
silently pass schema validation, even though the driver will fail to probe.

Should the reg property be included in this required list?

> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/st,stm32mp25-rcc.h>
> +    #include <dt-bindings/reset/st,stm32mp25-rcc.h>
> +
> +    usb-phy@2400 {

[Severity: Low]
Generic phy schema patterns in dtschema restrict node names for phy
providers to match the regular expression ^phy(@.*)?$.

Will this node name trigger warnings when validating the example during a 
dt_binding_check?

> +        compatible = "st,stm32mp25-usb2phy1";
> +        reg = <0x2400 0x24>;
> +        #phy-cells = <0>;
> +        #clock-cells = <1>;
> +        clocks = <&rcc CK_KER_USB2PHY1>;
> +        resets = <&rcc USB2PHY1_R>;
> +        vdd33-supply = <&vdd33usb>;
> +    };

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1

-- 
linux-phy mailing list
[email protected]
https://lists.infradead.org/mailman/listinfo/linux-phy