Re: [PATCH] spi: dt-bindings: mediatek,spi-mtk-nor: Add clock bindings for mt8189

[Krzysztof Kozlowski]

On Thu, Mar 05, 2026 at 10:27:36AM +0800, Meiker Gao wrote:
> Update mediatek,spi-mtk-nor.yaml to add conditional clock and
> clock-names bindings for the mt8189-nor platform. The mt8189-nor
> controller requires five specific clocks and corresponding clock-names
> ("spi", "sf", "axi_f", "axi_h", "axi_p"). This change enforces these
> requirements in the device tree binding schema.
> 
> For other platforms, the minimum number of clocks and clock-names
> remains unchanged. The patch also adds an example for mt8189-nor,
> illustrating the new clock configuration.
> 
> This update ensures correct hardware description and validation for
> mt8189-nor, improving compatibility and reducing configuration errors.
> 
> Signed-off-by: Meiker Gao <ot_meiker.gao@xxxxxxxxxxxx>
> 
> ---

Not tested so limited review.

>  .../bindings/spi/mediatek,spi-mtk-nor.yaml    | 78 ++++++++++++++++---
>  1 file changed, 68 insertions(+), 10 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/spi/mediatek,spi-mtk-nor.yaml b/Documentation/devicetree/bindings/spi/mediatek,spi-mtk-nor.yaml
> index a453996c13f2..7e551f2cb52c 100644
> --- a/Documentation/devicetree/bindings/spi/mediatek,spi-mtk-nor.yaml
> +++ b/Documentation/devicetree/bindings/spi/mediatek,spi-mtk-nor.yaml
> @@ -17,8 +17,6 @@ description: |
>    for devices other than SPI NOR flash due to limited transfer
>    capability of this controller.
>  
> -allOf:
> -  - $ref: /schemas/spi/spi-controller.yaml#
>  
>  properties:
>    compatible:
> @@ -39,6 +37,7 @@ properties:
>        - items:
>            - enum:
>                - mediatek,mt8188-nor
> +              - mediatek,mt8189-nor
>            - const: mediatek,mt8186-nor
>  
>    reg:
> @@ -56,14 +55,12 @@ properties:
>                       design, so this is optional.
>        - description: clock used for controller axi slave bus.
>                       this depends on hardware design, so it is optional.
> -
> -  clock-names:
> -    minItems: 2
> -    items:
> -      - const: spi
> -      - const: sf
> -      - const: axi
> -      - const: axi_s
> +      - description: clock used for controller axi fast bus (axi_f).
> +                     Required for new platforms, such as mt8189.
> +      - description: clock used for controller axi high-speed bus (axi_h).
> +                     Required for new platforms, such as mt8189.
> +      - description: clock used for controller axi peripheral bus (axi_p).
> +                     Required for new platforms, such as mt8189.

No, I don't understand why you remove names. This is not expected.
Please also read writing schema and writing bindings docs explaining how
to organize properties with variable lengths. There is plenty of
examples to follow.

>  
>  required:
>    - compatible
> @@ -71,6 +68,42 @@ required:
>    - clocks
>    - clock-names
>  
> +allOf:
> +  - $ref: /schemas/spi/spi-controller.yaml#
> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: mediatek,mt8189-nor
> +    then:
> +      properties:
> +        clocks:
> +          minItems: 5
> +          maxItems: 5


> +        clock-names:
> +          minItems: 5
> +          maxItems: 5

Pointess. Drop.

> +          items:
> +            - const: spi
> +            - const: sf
> +            - const: axi_f
> +            - const: axi_h
> +            - const: axi_p
> +    else:
> +      properties:
> +        clocks:
> +          minItems: 2
> +          maxItems: 2

ABI change without explanation. Don't mix multiple changes into one
commit.

> +        clock-names:
> +          minItems: 2
> +          maxItems: 2
> +          items:
> +            - const: spi
> +            - const: sf
> +            - const: axi
> +            - const: axi_s

Plain wrong. So either 2 or 4.

> +
>  unevaluatedProperties: false
>  
>  examples:
> @@ -97,3 +130,28 @@ examples:
>          };
>        };
>      };
> +
> +  - |
> +    #include <dt-bindings/clock/mt8189-clk.h>
> +
> +    soc {
> +      #address-cells = <2>;
> +      #size-cells = <2>;
> +
> +      nor_flash: spi@1100d000 {

No, drop entire example.

Best regards,
Krzysztof