Re: [PATCH RESEND 2/2] dt-bindings: memory-controllers: mtk-smi: Add support for mt8189

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Mismatch between device tree binding and driver regarding the number of required clocks for `mediatek,mt8189-smi-sub-common`.
- [Low] The new compatible string `mediatek,mt8189-smi-common` is omitted from the device tree binding conditional blocks, bypassing clock validation.
--

--- Patch [2]: [PATCH RESEND 2/2] dt-bindings: memory-controllers: mtk-smi: Add support for mt8189 ---
commit cc036d50e071efdc0f20a8e8886395bb02c7e74e
Author: Congcong Yao <congcong.yao@mediatek.com>

dt-bindings: memory-controllers: mtk-smi: Add support for mt8189

This commit adds device tree binding descriptions for the SMI larb, common,
and sub-common hardware components on MediaTek mt8189.

> diff --git a/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml b/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml
> index 0762e0ff66ef0..4e1deeff92b14 100644
> --- a/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml
> +++ b/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml
[ ... ]
> @@ -40,6 +55,8 @@ properties:
>            - mediatek,mt8186-smi-common
>            - mediatek,mt8188-smi-common-vdo
>            - mediatek,mt8188-smi-common-vpp
> +          - mediatek,mt8189-smi-common

[Severity: Low]
Should mediatek,mt8189-smi-common be added to the conditional block for
gen2 hardware that lacks GALS support?

Looking at the rest of the binding, there is an allOf conditional block that
enforces a strict 2-clock constraint for compatible strings without GALS:

  - if:  # for gen2 HW that don't have gals
      properties:
        compatible:
          enum:
            - mediatek,mt2712-smi-common
            - mediatek,mt6795-smi-common
            - mediatek,mt8167-smi-common
            - mediatek,mt8173-smi-common

    then:
      properties:
        clocks:
          minItems: 2
          maxItems: 2

Since the driver platform data for mt8189-smi-common lacks GALS support,
omitting it from this conditional block bypasses the strict clock validation.

> +          - mediatek,mt8189-smi-sub-common
>            - mediatek,mt8192-smi-common
[ ... ]
> @@ -108,6 +125,7 @@ allOf:
>          compatible:
>            contains:
>              enum:
> +              - mediatek,mt8189-smi-sub-common

[Severity: Medium]
Is there a mismatch between this binding and the driver platform data
regarding the number of required clocks?

This schema requires 3 clocks (apb, smi, gals0) for the
mediatek,mt8189-smi-sub-common hardware in this block:

  - if:  # only for sub common
      properties:
        compatible:
          contains:
            enum:
              - mediatek,mt8189-smi-sub-common
              - mediatek,mt8195-smi-sub-common
    then:
      required:
        - mediatek,smi
      properties:
        clocks:
          minItems: 3
          maxItems: 3

However, the corresponding driver platform data in drivers/memory/mtk-smi.c
omits the has_gals flag:

static const struct mtk_smi_common_plat mtk_smi_sub_common_mt8189 = {
	.type     = MTK_SMI_GEN2_SUB_COMM,
};

Because has_gals is false, the driver will only configure and enable 2 clocks.
Does the hardware actually require gals0 to be configured, or is the schema
overly restrictive by enforcing 3 clocks?

>                - mediatek,mt8195-smi-sub-common
>      then:
>        required:

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260722-8189-smi-support-v1-0-b528a4fd878c@mediatek.com?part=2