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

[Chris Packham]

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 at alliedtelesis.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