[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <0d4d40cc-9885-4933-a6d6-933e4705a68c@linaro.org>
Date: Wed, 4 Sep 2024 15:02:40 +0200
From: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To: Detlev Casanova <detlev.casanova@...labora.com>
Cc: linux-kernel@...r.kernel.org, Ulf Hansson <ulf.hansson@...aro.org>,
Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>, Jisheng Zhang <jszhang@...nel.org>,
linux-mmc@...r.kernel.org, devicetree@...r.kernel.org, kernel@...labora.com
Subject: Re: [PATCH 1/1] dt-bindings: mmc: Add support for rk3576 eMMC
On 04/09/2024 14:56, Detlev Casanova wrote:
> On Wednesday, 4 September 2024 02:00:27 EDT Krzysztof Kozlowski wrote:
>> On Tue, Sep 03, 2024 at 10:51:36AM -0400, Detlev Casanova wrote:
>>> The device is compatible with rk3588, so add an entry for the 2
>>> compatibles together.
>>>
>>> The rk3576 device has a power-domain that needs to be on for the eMMC to
>>> be used. Add it as a requirement.
>>>
>>> Signed-off-by: Detlev Casanova <detlev.casanova@...labora.com>
>>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
>>
>> NAK
>>
>> Drop fake tag. It is impossible to receive a review-tag from me on THE
>> FIRST version. I almost never provide reviews out of mailing lists.
>>
>> And since there is no changelog here and no versioning, this obviously
>> is not v2 or v3.
>
> That's because the patch was from another patchset[0]. Only this patch needed
> a rebase on the mmc tree, so I sent it separately. You reviewed it here [1].
>
> [0]: https://lore.kernel.org/all/010201919989e3de-60b56341-85e0-4869-89d1-362407c4f2ec-000000@eu-west-1.amazonses.com/
> [1]: https://lore.kernel.org/all/m5ua5jnbv4u36glqt2qrps35asuqfycxedgjrfhodi5bvs2r2h@xvy4qxt4gx74/
Hm, ok, changelog should explained this and the versioning should
continue, including history of this patch.
>
>>> ---
>>>
>>> .../bindings/mmc/snps,dwcmshc-sdhci.yaml | 34 ++++++++++++++-----
>>> 1 file changed, 26 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml
>>> b/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml index
>>> 80d50178d2e3..84a667f0c526 100644
>>> --- a/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml
>>> +++ b/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml
>>>
>>> @@ -12,14 +12,18 @@ maintainers:
>>> properties:
>>> compatible:
>>> - enum:
>>> - - rockchip,rk3568-dwcmshc
>>> - - rockchip,rk3588-dwcmshc
>>> - - snps,dwcmshc-sdhci
>>> - - sophgo,cv1800b-dwcmshc
>>> - - sophgo,sg2002-dwcmshc
>>> - - sophgo,sg2042-dwcmshc
>>> - - thead,th1520-dwcmshc
>>> + oneOf:
>>> + - items:
>>> + - const: rockchip,rk3576-dwcmshc
>>> + - const: rockchip,rk3588-dwcmshc
>>> + - enum:
>>> + - rockchip,rk3568-dwcmshc
>>> + - rockchip,rk3588-dwcmshc
>>> + - snps,dwcmshc-sdhci
>>> + - sophgo,cv1800b-dwcmshc
>>> + - sophgo,sg2002-dwcmshc
>>> + - sophgo,sg2042-dwcmshc
>>> + - thead,th1520-dwcmshc
>>>
>>> reg:
>>> maxItems: 1
>>>
>>> @@ -35,6 +39,9 @@ properties:
>>> minItems: 1
>>> maxItems: 5
>>>
>>> + power-domains:
>>> + maxItems: 1
>>> +
>>>
>>> resets:
>>> maxItems: 5
>>>
>>> @@ -97,6 +104,17 @@ allOf:
>>> - const: block
>>> - const: timer
>>>
>>> + - if:
>>> + properties:
>>> + compatible:
>>> + contains:
>>> + const: rockchip,rk3576-dwcmshc
>>> +
>>> + then:
>>> + properties:
>>> + power-domains:
>>> + minItems: 1
>>
>> Why minItems? This does not look right. I don't get what you are trying
>> to say here.
>
> I'm saying that for the rockchip,rk3576-dwcmshc compatible, 1 power-domain
> node has to be set.
The top-level property already says this. You need to disallow it for
other variants (:false).
Best regards,
Krzysztof
Powered by blists - more mailing lists