Re: [PATCH v17 1/2] dt-bindings: phy: qcom: Add CSI2 C-PHY/DPHY schema

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] Missing `maxItems: 1` constraint on `endpoint@1` `data-lanes`.
- [Low] Mismatch in `required-opps` array length relative to `power-domains` in the example.
- [Low] Type mismatch constraint on `clock-lanes` using a scalar `const`.
--

commit be1ee8982134944fca33ceaa85eed2383dcf7ea6
Author: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

dt-bindings: phy: qcom: Add CSI2 C-PHY/DPHY schema

This commit adds a base schema for the MIPI CSI2 PHYs on Qualcomm SoCs.
The PHY supports both C-PHY and D-PHY operation, as well as a combo-mode
that allows for two sensors to be connected to one PHY.

> diff --git a/Documentation/devicetree/bindings/phy/qcom,x1e80100-csi2-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,x1e80100-csi2-phy.yaml
> new file mode 100644
> index 0000000000000..993271c42fa06
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/qcom,x1e80100-csi2-phy.yaml
[ ... ]
> +        allOf:
> +          - if:
> +              required:
> +                - endpoint@1
> +            then:
> +              properties:
> +                endpoint@0:
> +                  properties:
> +                    clock-lanes:
> +                      const: 0

[Severity: Low]
Will this pass dtbs_check validation? 

Because the clock-lanes property uses a -lanes suffix, dt-schema evaluates
the payload from the device tree as an array. Using a scalar "const: 0"
checks for an integer, which can cause a type mismatch. 

Should this be written as an array constraint, such as "items: - const: 0"?

> +                    data-lanes:
> +                      maxItems: 2
> +                endpoint@1:
> +                  properties:
> +                    clock-lanes:
> +                      const: 3

[Severity: Low]
Similarly, should this use an array item constraint rather than a scalar
constant to prevent a validation type mismatch with the clock-lanes array?

> +                    data-lanes:
> +                      items:
> +                        - const: 4

[Severity: Medium]
Is it necessary to add "maxItems: 1" here to properly restrict the property
to a single data lane?

Since maxItems is inherited as 4 from the base definition, a configuration
like data-lanes = <4 2> might falsely pass schema validation without an
explicit maxItems override.

> +              required:
> +                - endpoint@0
> +            else:
> +              patternProperties:
> +                "^endpoint(@0)?$":
> +                  properties:
> +                    clock-lanes:
> +                      const: 0

[Severity: Low]
As with the other clock-lanes definitions above, does this scalar constant
need to be updated to match the array evaluation of dt-schema?

[ ... ]
> +        csiphy_opp_table: opp-table {
> +            compatible = "operating-points-v2";
> +
> +            opp-300000000 {
> +                opp-hz = /bits/ 64 <300000000>;
> +                required-opps = <&rpmhpd_opp_low_svs_d1>,
> +                                <&rpmhpd_opp_low_svs_d1>;

[Severity: Low]
Does this required-opps array length match the power-domains definition
provided earlier in the example?

The example block defines 3 power-domains ("top", "mmcx", "mx") but only
provides 2 entries here. In the Linux PM domain framework, required-opps
maps 1:1 to power-domains by index, which leaves the scalable mx domain
without an OPP mapping. 

Should this be padded to 3 entries (e.g., using <0> for the non-scalable
GDSC)?

> +            };
> +        };
> +    };

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260903-x1e-csi2-phy-v17-0-26606fa9a039@linaro.org?part=1

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