Re: [PATCH v2 1/5] dt-bindings: display/msm: dp-controller: Document Kaanapali

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] dt-bindings: qcom,kaanapali-dp omitted from existing conditional blocks
- [Medium] dt-bindings: missing additionalProperties in patternProperties
--

commit 98068de5d2fca0abb41ef1fecd6b1707d8f24f5d
Author: Balaji Kalkonda <balaji.kalkonda@oss.qualcomm.com>

dt-bindings: display/msm: dp-controller: Document Kaanapali

This commit adds device tree binding documentation for the DisplayPort
controller on Qualcomm Kaanapali. It introduces a new compatible string
and adds constraints for its five register ranges and six clocks.

> diff --git a/Documentation/devicetree/bindings/display/msm/dp-controller.yaml b/Documentation/devicetree/bindings/display/msm/dp-controller.yaml
> index 094a6383bb779..0240e3c1a6d9d 100644
> --- a/Documentation/devicetree/bindings/display/msm/dp-controller.yaml
> +++ b/Documentation/devicetree/bindings/display/msm/dp-controller.yaml
> @@ -19,6 +19,7 @@ properties:
>      oneOf:
>        - enum:
>            - qcom,glymur-dp
> +          - qcom,kaanapali-dp
>            - qcom,sa8775p-dp
>            - qcom,sc7180-dp
>            - qcom,sc7280-dp

[Severity: Medium]
Does this inadvertently restrict Kaanapali from declaring an aux-bus?

Because qcom,kaanapali-dp is omitted from the existing aux-bus conditional
block, it falls into the else clause which forces aux-bus: false:

      if:
        properties:
          compatible:
            contains:
              enum:
                - qcom,glymur-dp
                - qcom,sa8775p-dp
                - qcom,x1e80100-dp
      then:
        $ref: /schemas/sound/dai-common.yaml#
        oneOf:
          - required:
              - aux-bus
          - required:
              - "#sound-dai-cells"
      else:
        $ref: /schemas/sound/dai-common.yaml#
        properties:
          aux-bus: false

This incorrectly restricts valid device trees that declare an aux-bus.

[ ... ]

> @@ -290,6 +291,24 @@ allOf:
>            minItems: 6
>            maxItems: 6
>  
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - qcom,kaanapali-dp
> +    then:
> +      properties:
> +        reg:
> +          minItems: 5
> +          maxItems: 5
> +        clocks:
> +          minItems: 6
> +          maxItems: 6
> +        clock-names:
> +          minItems: 6
> +          maxItems: 6
> +

[Severity: Medium]
Could this conditional block be merged with the existing block that has
identical constraints?

The constraints specified here (5 reg items, 6 clocks) exactly match the
existing block for 2-stream MST platforms (qcom,sc8180x-dp, qcom,sdm845-dp,
qcom,sm8350-dp). Reusing the existing block would avoid duplicating schema
constraints.

> diff --git a/Documentation/devicetree/bindings/display/msm/qcom,kaanapali-mdss.yaml b/Documentation/devicetree/bindings/display/msm/qcom,kaanapali-mdss.yaml
> index 9f935defd6b12..83e3596ba0ed9 100644
> --- a/Documentation/devicetree/bindings/display/msm/qcom,kaanapali-mdss.yaml
> +++ b/Documentation/devicetree/bindings/display/msm/qcom,kaanapali-mdss.yaml
> @@ -48,6 +48,12 @@ patternProperties:
>        compatible:
>          const: qcom,kaanapali-dpu
>  
> +  "^displayport-controller@[0-9a-f]+$":
> +    type: object
> +    properties:
> +      compatible:
> +        const: qcom,kaanapali-dp
> +

[Severity: Medium]
Does this object definition need additionalProperties: true or
unevaluatedProperties: false?

The dtschema meta-schema requires one of these properties for object nodes
to dictate how unlisted properties are handled. Without it, make
dt_binding_check reports a meta-schema validation error.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261015-kaanapali_dp-v2-0-c15cb83d9140@oss.qualcomm.com?part=1