[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <7224b8bb-2f59-3b06-b4c3-0b86a6e4703f@alliedtelesis.co.nz>
Date: Mon, 28 Mar 2022 23:41:33 +0000
From: Chris Packham <Chris.Packham@...iedtelesis.co.nz>
To: Krzysztof Kozlowski <krzk@...nel.org>,
"ulf.hansson@...aro.org" <ulf.hansson@...aro.org>,
"robh+dt@...nel.org" <robh+dt@...nel.org>,
"huziji@...vell.com" <huziji@...vell.com>,
"andrew@...n.ch" <andrew@...n.ch>,
"gregory.clement@...tlin.com" <gregory.clement@...tlin.com>,
"sebastian.hesselbarth@...il.com" <sebastian.hesselbarth@...il.com>
CC: "linux-mmc@...r.kernel.org" <linux-mmc@...r.kernel.org>,
"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH v3 3/3] dt-bindings: mmc: xenon: add marvell,sdhci-xenon
compatible
On 26/03/22 01:04, Krzysztof Kozlowski wrote:
> 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. :)
The in-tree uses all have both. I'll remove it from the enum.
>
> 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.
OK will squash and add an explanation for v4.
>
> Best regards,
> Krzysztof
Powered by blists - more mailing lists