linux-kernel - Re: [PATCH v3 3/3] dt-bindings: mmc: xenon: add marvell,sdhci-xenon compatible

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: <475d2af6-38cd-47a1-61b2-6b1298bf505d@kernel.org>
Date:   Fri, 25 Mar 2022 13:04:11 +0100
From:   Krzysztof Kozlowski <krzk@...nel.org>
To:     Chris Packham <chris.packham@...iedtelesis.co.nz>,
        ulf.hansson@...aro.org, robh+dt@...nel.org, huziji@...vell.com,
        andrew@...n.ch, gregory.clement@...tlin.com,
        sebastian.hesselbarth@...il.com
Cc:     linux-mmc@...r.kernel.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH v3 3/3] dt-bindings: mmc: xenon: add marvell,sdhci-xenon
 compatible

On 25/03/2022 01:07, Chris Packham wrote:
> The armada-37xx SoC dtsi includes this as a compatible string. Add it to
> the dt-binding.
> 
> Signed-off-by: Chris Packham <chris.packham@...iedtelesis.co.nz>
> ---
> 
> Notes:
>     Changes in v3:
>     - new
> 
>  .../devicetree/bindings/mmc/marvell,xenon-sdhci.yaml          | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.yaml b/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.yaml
> index 326ac3fa36b5..776bed5046fa 100644
> --- a/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.yaml
> +++ b/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.yaml
> @@ -31,6 +31,10 @@ properties:
>            - const: marvell,armada-ap807-sdhci
>            - const: marvell,armada-ap806-sdhci
>  
> +      - items:
> +          - const: marvell,armada-3700-sdhci
> +          - const: marvell,sdhci-xenon

Do you know of any usages of marvell,armada-3700-sdhci alone (without
sdhci-xenon)? If not, it should be removed from the enum before (the one
added in your patch #2). It does not look correct to have it both as
standalone compatible and one compatible with sdhci-xenon. Either it is
compatible with sdhci-xenon or not. :)

I suggested before to make this change here as separate patch, but I
think I missed the fact that it is simple correction of
armada-3700-sdhci compatible. Such simple corrections can be done within
same patch as conversion, with explanation in commit msg (which was
missing in your v1).

To avoid unnecessary patch changes you could squash it with v1 and
include this in the commit msg (actually extend it to say that you are
correcting the 3700-sdhci compatible), or create patch #2 that way:

+    oneOf:
+      - enum:
+          - marvell,armada-cp110-sdhci
+          - marvell,armada-ap806-sdhci
+      - items:
+          - const: marvell,armada-ap807-sdhci
+          - const: marvell,armada-ap806-sdhci
+      - items:
+          - marvell,armada-3700-sdhci

so now you will only add xenon here. But then it is so small patch that
with explanation we usually include it in conversion.

Best regards,
Krzysztof