Re: [PATCH 1/3] dt-bindings: display: msm: Allow two MDSS power domains

[sashiko-bot@kernel.org]

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 <yongxing.mou@oss.qualcomm.com>

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/20260720-msm_gdsc2-v1-0-4687866d6cb0@oss.qualcomm.com?part=1