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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <sdxhnqvdbcpmbp3l7hcnsrducpa5zrgbmkykwfluhrthqhznxi@6i4xiqrre3qg>
Date: Wed, 4 Sep 2024 08:34:36 +0200
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Nikunj Kela <quic_nkela@...cinc.com>
Cc: andersson@...nel.org, konradybcio@...nel.org, robh@...nel.org, 
	krzk+dt@...nel.org, conor+dt@...nel.org, rafael@...nel.org, viresh.kumar@...aro.org, 
	herbert@...dor.apana.org.au, davem@...emloft.net, sudeep.holla@....com, andi.shyti@...nel.org, 
	tglx@...utronix.de, will@...nel.org, robin.murphy@....com, joro@...tes.org, 
	jassisinghbrar@...il.com, lee@...nel.org, linus.walleij@...aro.org, amitk@...nel.org, 
	thara.gopinath@...il.com, broonie@...nel.org, cristian.marussi@....com, 
	rui.zhang@...el.com, lukasz.luba@....com, wim@...ux-watchdog.org, linux@...ck-us.net, 
	linux-arm-msm@...r.kernel.org, devicetree@...r.kernel.org, linux-kernel@...r.kernel.org, 
	linux-pm@...r.kernel.org, linux-crypto@...r.kernel.org, arm-scmi@...r.kernel.org, 
	linux-arm-kernel@...ts.infradead.org, linux-i2c@...r.kernel.org, iommu@...ts.linux.dev, 
	linux-gpio@...r.kernel.org, linux-serial@...r.kernel.org, linux-spi@...r.kernel.org, 
	linux-watchdog@...r.kernel.org, kernel@...cinc.com, quic_psodagud@...cinc.com, 
	Praveen Talari <quic_ptalari@...cinc.com>
Subject: Re: [PATCH v2 16/21] dt-bindings: spi: document support for SA8255p

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@...cinc.com>
> Signed-off-by: Nikunj Kela <quic_nkela@...cinc.com>
> ---
>  .../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