Re: [PATCH v2 16/21] dt-bindings: spi: document support for SA8255p

[Krzysztof Kozlowski <krzk@xxxxxxxxxx>]

On Tue, Sep 03, 2024 at 03:02:35PM -0700, Nikunj Kela wrote:
> Add compatible representing spi support on SA8255p.
> 
> Clocks and interconnects are being configured in firmware VM
> on SA8255p platform, therefore making them optional.
> 

Please use standard email subjects, so with the PATCH keyword in the
title.  helps here to create proper versioned patches.
Another useful tool is b4. Skipping the PATCH keyword makes filtering of
emails more difficult thus making the review process less convenient.


> CC: Praveen Talari <quic_ptalari@xxxxxxxxxxx>
> Signed-off-by: Nikunj Kela <quic_nkela@xxxxxxxxxxx>
> ---
>  .../bindings/spi/qcom,spi-geni-qcom.yaml      | 60 +++++++++++++++++--
>  1 file changed, 56 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/spi/qcom,spi-geni-qcom.yaml b/Documentation/devicetree/bindings/spi/qcom,spi-geni-qcom.yaml
> index 2e20ca313ec1..75b52c0a7440 100644
> --- a/Documentation/devicetree/bindings/spi/qcom,spi-geni-qcom.yaml
> +++ b/Documentation/devicetree/bindings/spi/qcom,spi-geni-qcom.yaml
> @@ -25,10 +25,45 @@ description:
>  
>  allOf:
>    - $ref: /schemas/spi/spi-controller.yaml#
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: qcom,sa8255p-geni-spi

Not much improved. All my previous (v1) and other patch (i2c) comments
apply.

> +    then:
> +      required:
> +        - power-domains
> +        - power-domain-names
> +
> +      properties:
> +        power-domains:
> +          minItems: 2
> +
> +    else:
> +      required:
> +        - clocks
> +        - clock-names
> +
> +      properties:
> +        power-domains:
> +          maxItems: 1
> +
> +        interconnects:
> +          minItems: 2
> +          maxItems: 3
> +
> +        interconnect-names:
> +          minItems: 2
> +          items:
> +            - const: qup-core
> +            - const: qup-config
> +            - const: qup-memory
>  
>  properties:
>    compatible:
> -    const: qcom,geni-spi
> +    enum:
> +      - qcom,geni-spi
> +      - qcom,sa8255p-geni-spi

You have entire commit msg to explain why this device's programming
model is not compatible with existing generic compatible which must
cover all variants (because it is crazy generic).

Best regards,
Krzysztof