Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider: - [Medium] Relaxing the global maximum number of `power-domains` in `mdss-common.yaml` without restricting legacy schemas allows invalid device trees to pass validation. - [Medium] Missing `power-domain-names` in the `required` list allows incomplete device trees to pass validation. - [Medium] Adding an items array to `power-domains` implicitly sets `minItems: 2`, breaking backward compatibility for existing device trees. --
--- Patch [1]: [PATCH 1/3] dt-bindings: display: msm: Allow two MDSS power domains --- commit 3afe59087bba460fdef9d8666ba4cb4a97273b44 Author: Yongxing Mou <[email protected]> dt-bindings: display: msm: Allow two MDSS power domains Kaanapali (SM8750) and Glymur use two display power domains. CORE_GDSC powers the main display hardware while INT2_GDSC powers a subset of SSPP blocks. Allow the MDSS bindings to describe both power domains and their corresponding power-domain-names values. > diff --git a/Documentation/devicetree/bindings/display/msm/mdss-common.yaml > b/Documentation/devicetree/bindings/display/msm/mdss-common.yaml > index c6305a6e03346..c0b650ffd9ef8 100644 > --- a/Documentation/devicetree/bindings/display/msm/mdss-common.yaml > +++ b/Documentation/devicetree/bindings/display/msm/mdss-common.yaml > @@ -31,7 +31,8 @@ properties: > const: mdss > > power-domains: > - maxItems: 1 > + minItems: 1 > + maxItems: 2 [Severity: Medium] When we relax the global maximum number of power-domains here, does this allow invalid device trees to pass validation for legacy platforms? For example, schemas like qcom,sm8550-mdss.yaml include this common schema but only have 1 hardware power domain. Without explicitly overriding maxItems to 1 in those legacy schemas, they might silently accept invalid device trees with 2 power domains. > > clocks: > minItems: 2 > diff --git > a/Documentation/devicetree/bindings/display/msm/qcom,glymur-mdss.yaml > b/Documentation/devicetree/bindings/display/msm/qcom,glymur-mdss.yaml > index 64dde43373ac7..685bedb26a124 100644 > --- a/Documentation/devicetree/bindings/display/msm/qcom,glymur-mdss.yaml > +++ b/Documentation/devicetree/bindings/display/msm/qcom,glymur-mdss.yaml [ ... ] > @@ -38,6 +38,16 @@ properties: > - const: mdp0-mem > - const: cpu-cfg > > + power-domains: > + items: > + - description: MDSS core GDSC power domain > + - description: MDSS INT2 GDSC power domain [Severity: Medium] Does adding this items array implicitly set minItems to 2 during dt-schema evaluation? If so, this might break backward compatibility for older device trees that only specify a single power domain. Should minItems: 1 be explicitly specified here? > + > + power-domain-names: > + items: > + - const: core > + - const: int2 [Severity: Medium] If a device tree specifies 2 power domains but omits power-domain-names, will it pass schema validation? The driver might fail to probe at runtime when attempting to attach power domains by name. Should power-domain-names be conditionally required in the required list (using an if-then block) when multiple power domains are provided? > + > patternProperties: > "^display-controller@[0-9a-f]+$": > type: object > diff --git > a/Documentation/devicetree/bindings/display/msm/qcom,kaanapali-mdss.yaml > b/Documentation/devicetree/bindings/display/msm/qcom,kaanapali-mdss.yaml > index 9f935defd6b12..088010f464c3d 100644 > --- a/Documentation/devicetree/bindings/display/msm/qcom,kaanapali-mdss.yaml > +++ b/Documentation/devicetree/bindings/display/msm/qcom,kaanapali-mdss.yaml [ ... ] > @@ -40,6 +40,16 @@ properties: > - const: mdp0-mem > - const: cpu-cfg > > + power-domains: > + items: > + - description: MDSS core GDSC power domain > + - description: MDSS INT2 GDSC power domain [Severity: Medium] Similar to the glymur binding, does this items array implicitly set minItems to 2 and break backward compatibility? > + > + power-domain-names: > + items: > + - const: core > + - const: int2 [Severity: Medium] Also similar to the glymur binding, should power-domain-names be conditionally required to ensure device trees with two power domains provide the necessary names for the driver? > + > patternProperties: > "^display-controller@[0-9a-f]+$": > type: object -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
