Re: [PATCH v7 1/3] dt-bindings: mmc: sdhci-msm: Add ICE phandle

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Incomplete DT binding constraint: The schema disallows the embedded `ice` register map when the `qcom,ice` phandle is present, but fails to disallow the embedded `ice` clock. This allows device trees to mistakenly specify both the `qcom,ice` phandle and the legacy `ice` clock on the SDHCI node, passing schema validation while double-modeling the clock resource.
--

commit 1a4e3c2586694f366860741fba23e497632a71fa
Author: Neeraj Soni <neeraj.soni@oss.qualcomm.com>

dt-bindings: mmc: sdhci-msm: Add ICE phandle

This commit introduces the qcom,ice property to reference the Inline Crypto
Engine hardware block via a phandle. When this property is present, it
updates the allOf conditional blocks to disallow the embedded register
region in the SDHCI node to prevent double-modeling. Older SoCs without
this block remain unaffected.

Link: https://lore.kernel.org/r/20260310113557.348502-2-neeraj.soni@oss.qualcomm.com

> diff --git a/Documentation/devicetree/bindings/mmc/qcom,sdhci-msm.yaml b/Documentation/devicetree/bindings/mmc/qcom,sdhci-msm.yaml
> index bd558a11b7929..b3fcc1673c106 100644
> --- a/Documentation/devicetree/bindings/mmc/qcom,sdhci-msm.yaml
> +++ b/Documentation/devicetree/bindings/mmc/qcom,sdhci-msm.yaml

[ ... ]

> @@ -198,35 +203,69 @@ allOf:
>              enum:
>                - qcom,sdhci-msm-v4
>      then:
> -      properties:
> -        reg:
> -          minItems: 2
> -          items:
> -            - description: Host controller register map
> -            - description: SD Core register map
> -            - description: CQE register map
> -            - description: Inline Crypto Engine register map
> -        reg-names:
> -          minItems: 2
> -          items:
> -            - const: hc
> -            - const: core
> -            - const: cqhci
> -            - const: ice
> +      if:
> +        required:
> +          - qcom,ice
> +      then:
> +        properties:
> +          reg:
> +            minItems: 2
> +            items:
> +              - description: Host controller register map
> +              - description: SD Core register map
> +              - description: CQE register map
> +          reg-names:
> +            minItems: 2
> +            items:
> +              - const: hc
> +              - const: core
> +              - const: cqhci

[Severity: Medium]
Does this regression allow device trees to mistakenly specify the legacy "ice"
clock on the SDHCI node even when the "qcom,ice" phandle is present?

The commit message states the intent is to avoid double-modeling of the
Inline Crypto Engine block, and the allOf conditional block correctly
restricts the "ice" register map here. However, the schema doesn't seem to
disallow the embedded "ice" clock in clock-names when "qcom,ice" is provided.

Should there be corresponding restrictions for clock-names here to fully
prevent double-modeling the clock resource?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260608041650.541502-1-neeraj.soni@oss.qualcomm.com?part=1