diff mbox series

dt-bindings: mmc: Make comment on wakeup-source less confusing

Message ID 20221122111124.6828-3-cniedermaier@dh-electronics.com
State Superseded
Headers show
Series dt-bindings: mmc: Make comment on wakeup-source less confusing | expand

Commit Message

Christoph Niedermaier Nov. 22, 2022, 11:11 a.m. UTC
The current comment on wakeup-source is a bit confusing, because it isn't
clear at first sight which property is actually deprecated.
Change the comment to one that is less confusing.

Signed-off-by: Christoph Niedermaier <cniedermaier@dh-electronics.com>
---
Cc: Ulf Hansson <ulf.hansson@linaro.org>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
Cc: Marek Vasut <marex@denx.de>
Cc: kernel@dh-electronics.com
Cc: linux-mmc@vger.kernel.org
Cc: devicetree@vger.kernel.org
To: linux-kernel@vger.kernel.org
---
 Documentation/devicetree/bindings/mmc/mmc-controller.yaml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Ulf Hansson Nov. 29, 2022, 12:06 p.m. UTC | #1
On Tue, 22 Nov 2022 at 12:43, Christoph Niedermaier
<cniedermaier@dh-electronics.com> wrote:
>
> The current comment on wakeup-source is a bit confusing, because it isn't
> clear at first sight which property is actually deprecated.
> Change the comment to one that is less confusing.
>
> Signed-off-by: Christoph Niedermaier <cniedermaier@dh-electronics.com>
> ---
> Cc: Ulf Hansson <ulf.hansson@linaro.org>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
> Cc: Marek Vasut <marex@denx.de>
> Cc: kernel@dh-electronics.com
> Cc: linux-mmc@vger.kernel.org
> Cc: devicetree@vger.kernel.org
> To: linux-kernel@vger.kernel.org
> ---
>  Documentation/devicetree/bindings/mmc/mmc-controller.yaml | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/mmc/mmc-controller.yaml b/Documentation/devicetree/bindings/mmc/mmc-controller.yaml
> index 802e3ca8be4d..a921442c6c1d 100644
> --- a/Documentation/devicetree/bindings/mmc/mmc-controller.yaml
> +++ b/Documentation/devicetree/bindings/mmc/mmc-controller.yaml
> @@ -293,7 +293,7 @@ properties:
>      description:
>        SDIO only. Preserves card power during a suspend/resume cycle.
>
> -  # Deprecated: enable-sdio-wakeup
> +  # Use wakeup-source instead of the deprecated enable-sdio-wakeup
>    wakeup-source:
>      $ref: /schemas/types.yaml#/definitions/flag
>      description:

Rob, Krzysztof - do we have a preferred pattern for how to express
deprecated bindings - or is the above okay to you?

Kind regards
Uffe
Krzysztof Kozlowski Nov. 29, 2022, 12:33 p.m. UTC | #2
On 29/11/2022 13:06, Ulf Hansson wrote:
> On Tue, 22 Nov 2022 at 12:43, Christoph Niedermaier
> <cniedermaier@dh-electronics.com> wrote:
>>
>> The current comment on wakeup-source is a bit confusing, because it isn't
>> clear at first sight which property is actually deprecated.
>> Change the comment to one that is less confusing.
>>
>> Signed-off-by: Christoph Niedermaier <cniedermaier@dh-electronics.com>
>> ---
>> Cc: Ulf Hansson <ulf.hansson@linaro.org>
>> Cc: Rob Herring <robh+dt@kernel.org>
>> Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
>> Cc: Marek Vasut <marex@denx.de>
>> Cc: kernel@dh-electronics.com
>> Cc: linux-mmc@vger.kernel.org
>> Cc: devicetree@vger.kernel.org
>> To: linux-kernel@vger.kernel.org
>> ---
>>  Documentation/devicetree/bindings/mmc/mmc-controller.yaml | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/mmc/mmc-controller.yaml b/Documentation/devicetree/bindings/mmc/mmc-controller.yaml
>> index 802e3ca8be4d..a921442c6c1d 100644
>> --- a/Documentation/devicetree/bindings/mmc/mmc-controller.yaml
>> +++ b/Documentation/devicetree/bindings/mmc/mmc-controller.yaml
>> @@ -293,7 +293,7 @@ properties:
>>      description:
>>        SDIO only. Preserves card power during a suspend/resume cycle.
>>
>> -  # Deprecated: enable-sdio-wakeup
>> +  # Use wakeup-source instead of the deprecated enable-sdio-wakeup
>>    wakeup-source:
>>      $ref: /schemas/types.yaml#/definitions/flag
>>      description:
> 
> Rob, Krzysztof - do we have a preferred pattern for how to express
> deprecated bindings - or is the above okay to you?

Yes, we have:
  deprecated: true

I don't get why this is in response to LED patch? I entirely ignored
this thread because of it.


Best regards,
Krzysztof
Krzysztof Kozlowski Nov. 29, 2022, 12:35 p.m. UTC | #3
On 22/11/2022 12:11, Christoph Niedermaier wrote:
> The current comment on wakeup-source is a bit confusing, because it isn't
> clear at first sight which property is actually deprecated.
> Change the comment to one that is less confusing.

It's still confusing - you mention some non-existing property. I don't
think it's worth to touch it. If you insist, drop entire comment...

> 
> Signed-off-by: Christoph Niedermaier <cniedermaier@dh-electronics.com>
> ---
> Cc: Ulf Hansson <ulf.hansson@linaro.org>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
> Cc: Marek Vasut <marex@denx.de>
> Cc: kernel@dh-electronics.com
> Cc: linux-mmc@vger.kernel.org
> Cc: devicetree@vger.kernel.org
> To: linux-kernel@vger.kernel.org
> ---
>  Documentation/devicetree/bindings/mmc/mmc-controller.yaml | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/mmc/mmc-controller.yaml b/Documentation/devicetree/bindings/mmc/mmc-controller.yaml
> index 802e3ca8be4d..a921442c6c1d 100644
> --- a/Documentation/devicetree/bindings/mmc/mmc-controller.yaml
> +++ b/Documentation/devicetree/bindings/mmc/mmc-controller.yaml
> @@ -293,7 +293,7 @@ properties:
>      description:
>        SDIO only. Preserves card power during a suspend/resume cycle.
>  
> -  # Deprecated: enable-sdio-wakeup
> +  # Use wakeup-source instead of the deprecated enable-sdio-wakeup
>    wakeup-source:
>      $ref: /schemas/types.yaml#/definitions/flag
>      description:

Best regards,
Krzysztof
Ulf Hansson Nov. 29, 2022, 3:30 p.m. UTC | #4
On Tue, 29 Nov 2022 at 13:36, Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 22/11/2022 12:11, Christoph Niedermaier wrote:
> > The current comment on wakeup-source is a bit confusing, because it isn't
> > clear at first sight which property is actually deprecated.
> > Change the comment to one that is less confusing.
>
> It's still confusing - you mention some non-existing property. I don't
> think it's worth to touch it. If you insist, drop entire comment...

Hmm, it doesn't look that easy to me. The "enable-sdio-wakeup" is
indeed a deprecated wakeup property [1], but it seems like it's not
really described somewhere. I wouldn't mind removing it (as it seems
to add some confusions), but in that case it needs to be entirely
removed from the DT docs, right?

Another option, would be to add a proper description of the property
and mark it with "deprecated: true".

>
> >
> > Signed-off-by: Christoph Niedermaier <cniedermaier@dh-electronics.com>
> > ---
> > Cc: Ulf Hansson <ulf.hansson@linaro.org>
> > Cc: Rob Herring <robh+dt@kernel.org>
> > Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
> > Cc: Marek Vasut <marex@denx.de>
> > Cc: kernel@dh-electronics.com
> > Cc: linux-mmc@vger.kernel.org
> > Cc: devicetree@vger.kernel.org
> > To: linux-kernel@vger.kernel.org
> > ---
> >  Documentation/devicetree/bindings/mmc/mmc-controller.yaml | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/Documentation/devicetree/bindings/mmc/mmc-controller.yaml b/Documentation/devicetree/bindings/mmc/mmc-controller.yaml
> > index 802e3ca8be4d..a921442c6c1d 100644
> > --- a/Documentation/devicetree/bindings/mmc/mmc-controller.yaml
> > +++ b/Documentation/devicetree/bindings/mmc/mmc-controller.yaml
> > @@ -293,7 +293,7 @@ properties:
> >      description:
> >        SDIO only. Preserves card power during a suspend/resume cycle.
> >
> > -  # Deprecated: enable-sdio-wakeup
> > +  # Use wakeup-source instead of the deprecated enable-sdio-wakeup
> >    wakeup-source:
> >      $ref: /schemas/types.yaml#/definitions/flag
> >      description:

Kind regards
Uffe

[1]
Documentation/devicetree/bindings/power/wakeup-source.txt
Marek Vasut Nov. 29, 2022, 3:37 p.m. UTC | #5
On 11/29/22 16:30, Ulf Hansson wrote:
> On Tue, 29 Nov 2022 at 13:36, Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
>>
>> On 22/11/2022 12:11, Christoph Niedermaier wrote:
>>> The current comment on wakeup-source is a bit confusing, because it isn't
>>> clear at first sight which property is actually deprecated.
>>> Change the comment to one that is less confusing.
>>
>> It's still confusing - you mention some non-existing property. I don't
>> think it's worth to touch it. If you insist, drop entire comment...
> 
> Hmm, it doesn't look that easy to me. The "enable-sdio-wakeup" is
> indeed a deprecated wakeup property [1], but it seems like it's not
> really described somewhere. I wouldn't mind removing it (as it seems
> to add some confusions), but in that case it needs to be entirely
> removed from the DT docs, right?
> 
> Another option, would be to add a proper description of the property
> and mark it with "deprecated: true".

There are no in-tree DTs which use the property, so maybe we should just 
drop it altogether from the bindings ?

next-20221128$ git grep enable-sdio-wakeup
Documentation/devicetree/bindings/mmc/mmc-controller.yaml:  # 
Deprecated: enable-sdio-wakeup
Documentation/devicetree/bindings/power/wakeup-source.txt:1. 
"enable-sdio-wakeup" 
Documentation/devicetree/bindings/mmc/mmc.txt
drivers/mmc/core/host.c:            device_property_read_bool(dev, 
"enable-sdio-wakeup")) /* legacy */
drivers/mmc/host/sdhci-pltfm.c:     device_property_read_bool(dev, 
"enable-sdio-wakeup")) /* legacy */
Krzysztof Kozlowski Nov. 29, 2022, 3:59 p.m. UTC | #6
On 29/11/2022 16:30, Ulf Hansson wrote:
> On Tue, 29 Nov 2022 at 13:36, Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
>>
>> On 22/11/2022 12:11, Christoph Niedermaier wrote:
>>> The current comment on wakeup-source is a bit confusing, because it isn't
>>> clear at first sight which property is actually deprecated.
>>> Change the comment to one that is less confusing.
>>
>> It's still confusing - you mention some non-existing property. I don't
>> think it's worth to touch it. If you insist, drop entire comment...
> 
> Hmm, it doesn't look that easy to me. The "enable-sdio-wakeup" is
> indeed a deprecated wakeup property [1], but it seems like it's not
> really described somewhere. I wouldn't mind removing it (as it seems
> to add some confusions), but in that case it needs to be entirely
> removed from the DT docs, right?

Yes, from the bindings and from DTSes. It's already gone from
mmc-controller.yaml bindings, so basically we already did the most of
such removal.

> 
> Another option, would be to add a proper description of the property
> and mark it with "deprecated: true".


Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/mmc/mmc-controller.yaml b/Documentation/devicetree/bindings/mmc/mmc-controller.yaml
index 802e3ca8be4d..a921442c6c1d 100644
--- a/Documentation/devicetree/bindings/mmc/mmc-controller.yaml
+++ b/Documentation/devicetree/bindings/mmc/mmc-controller.yaml
@@ -293,7 +293,7 @@  properties:
     description:
       SDIO only. Preserves card power during a suspend/resume cycle.
 
-  # Deprecated: enable-sdio-wakeup
+  # Use wakeup-source instead of the deprecated enable-sdio-wakeup
   wakeup-source:
     $ref: /schemas/types.yaml#/definitions/flag
     description: