[PATCH V3 11/17] dt-bindings: arm: mediatek: Add #reset-cells property for MT8192-sys-clock

Rex-BC Chen posted 17 patches 2 years, 5 months ago
There is a newer version of this series
[PATCH V3 11/17] dt-bindings: arm: mediatek: Add #reset-cells property for MT8192-sys-clock
Posted by Rex-BC Chen 2 years, 5 months ago
We will use the infra_ao reset which is defined in mt8192-sys-clock.
The maximum value of reset-cells is 2. Therefore, we add this patch to
define it.

Signed-off-by: Rex-BC Chen <rex-bc.chen@mediatek.com>
---
 .../bindings/arm/mediatek/mediatek,mt8192-sys-clock.yaml       | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/mediatek/mediatek,mt8192-sys-clock.yaml b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mt8192-sys-clock.yaml
index 5705bcf1fe47..28ebcecc8258 100644
--- a/Documentation/devicetree/bindings/arm/mediatek/mediatek,mt8192-sys-clock.yaml
+++ b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mt8192-sys-clock.yaml
@@ -29,6 +29,9 @@ properties:
   '#clock-cells':
     const: 1
 
+  '#reset-cells':
+    maximum: 2
+
 required:
   - compatible
   - reg
-- 
2.18.0
Re: [PATCH V3 11/17] dt-bindings: arm: mediatek: Add #reset-cells property for MT8192-sys-clock
Posted by Krzysztof Kozlowski 2 years, 5 months ago
On 22/04/2022 08:01, Rex-BC Chen wrote:
> We will use the infra_ao reset which is defined in mt8192-sys-clock.
> The maximum value of reset-cells is 2. Therefore, we add this patch to
> define it.

Remove entire last sentence, does not make sense in the commit.

> 
> Signed-off-by: Rex-BC Chen <rex-bc.chen@mediatek.com>
> ---
>  .../bindings/arm/mediatek/mediatek,mt8192-sys-clock.yaml       | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/arm/mediatek/mediatek,mt8192-sys-clock.yaml b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mt8192-sys-clock.yaml
> index 5705bcf1fe47..28ebcecc8258 100644
> --- a/Documentation/devicetree/bindings/arm/mediatek/mediatek,mt8192-sys-clock.yaml
> +++ b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mt8192-sys-clock.yaml
> @@ -29,6 +29,9 @@ properties:
>    '#clock-cells':
>      const: 1
>  
> +  '#reset-cells':
> +    maximum: 2

Why this is a maximum? Usually this is const, so how do you use it (with
what values)?

> +
>  required:
>    - compatible
>    - reg


Best regards,
Krzysztof
Re: [PATCH V3 11/17] dt-bindings: arm: mediatek: Add #reset-cells property for MT8192-sys-clock
Posted by Rex-BC Chen 2 years, 5 months ago
On Sat, 2022-04-23 at 18:27 +0800, Krzysztof Kozlowski wrote:
> On 22/04/2022 08:01, Rex-BC Chen wrote:
> > We will use the infra_ao reset which is defined in mt8192-sys-
> > clock.
> > The maximum value of reset-cells is 2. Therefore, we add this patch
> > to
> > define it.
> 
> Remove entire last sentence, does not make sense in the commit.
> 

Hello Krzysztof,

Thanks for your review.
I will drop "Therefore, we add this patch to define it." and add more
detailed messages in next version.

> > 
> > Signed-off-by: Rex-BC Chen <rex-bc.chen@mediatek.com>
> > ---
> >  .../bindings/arm/mediatek/mediatek,mt8192-sys-clock.yaml       | 3
> > +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git
> > a/Documentation/devicetree/bindings/arm/mediatek/mediatek,mt8192-
> > sys-clock.yaml
> > b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mt8192-
> > sys-clock.yaml
> > index 5705bcf1fe47..28ebcecc8258 100644
> > ---
> > a/Documentation/devicetree/bindings/arm/mediatek/mediatek,mt8192-
> > sys-clock.yaml
> > +++
> > b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mt8192-
> > sys-clock.yaml
> > @@ -29,6 +29,9 @@ properties:
> >    '#clock-cells':
> >      const: 1
> >  
> > +  '#reset-cells':
> > +    maximum: 2
> 
> Why this is a maximum? Usually this is const, so how do you use it
> (with
> what values)?
> 
We need to let the driver compatible with previous setting in
drivers/clk/mediatek/reset.c

There are two use cases in our reset driver:
(Refer to [1])

1. #reset-cells = <1>
   When we input the argument, the older driver
use is to calculate  
   bank and bit by different method. From the implementation of
   reset_xlate(), we can see if the argument number is 1, it will
   return directly.

2. #reset-cells = <2>
   The input arguments is offset and bit. When we input two arguments,
   we can use reset_xlate() to calculate the corresponding id to assert
   and deassert.

[1]:
https://lore.kernel.org/all/20220422060152.13534-10-rex-bc.chen@mediatek.com/

If it's acceptable, I will add this in commit message.

BRs,
Rex
> > +
> >  required:
> >    - compatible
> >    - reg
> 
> 
> Best regards,
> Krzysztof
Re: [PATCH V3 11/17] dt-bindings: arm: mediatek: Add #reset-cells property for MT8192-sys-clock
Posted by Krzysztof Kozlowski 2 years, 5 months ago
On 25/04/2022 04:37, Rex-BC Chen wrote:
>>> +  '#reset-cells':
>>> +    maximum: 2
>>
>> Why this is a maximum? Usually this is const, so how do you use it
>> (with
>> what values)?
>>
> We need to let the driver compatible with previous setting in
> drivers/clk/mediatek/reset.c

Then it should be enum [1, 2], because '0' is not valid.

> There are two use cases in our reset driver:
> (Refer to [1])
> 
> 1. #reset-cells = <1>
>    When we input the argument, the older driver
> use is to calculate  
>    bank and bit by different method. From the implementation of
>    reset_xlate(), we can see if the argument number is 1, it will
>    return directly.

I understand this is an old binding with older compatibles, so this
should be restricted per each variant (allOf:if:then)... but wait, old
binding did not allow any reset-cells! You add an entirely new binding
property and already want to support some older (deprecated?) way.

> 
> 2. #reset-cells = <2>
>    The input arguments is offset and bit. When we input two arguments,
>    we can use reset_xlate() to calculate the corresponding id to assert
>    and deassert.
> 
> [1]:
> https://lore.kernel.org/all/20220422060152.13534-10-rex-bc.chen@mediatek.com/
> 
> If it's acceptable, I will add this in commit message.


Best regards,
Krzysztof
Re: [PATCH V3 11/17] dt-bindings: arm: mediatek: Add #reset-cells property for MT8192-sys-clock
Posted by Rex-BC Chen 2 years, 4 months ago
On Mon, 2022-04-25 at 15:44 +0800, Krzysztof Kozlowski wrote:
> On 25/04/2022 04:37, Rex-BC Chen wrote:
> > > > +  '#reset-cells':
> > > > +    maximum: 2
> > > 
> > > Why this is a maximum? Usually this is const, so how do you use
> > > it
> > > (with
> > > what values)?
> > > 
> > 
> > We need to let the driver compatible with previous setting in
> > drivers/clk/mediatek/reset.c
> 
> Then it should be enum [1, 2], because '0' is not valid.
> 
> > There are two use cases in our reset driver:
> > (Refer to [1])
> > 
> > 1. #reset-cells = <1>
> >    When we input the argument, the older driver
> > use is to calculate  
> >    bank and bit by different method. From the implementation of
> >    reset_xlate(), we can see if the argument number is 1, it will
> >    return directly.
> 
> I understand this is an old binding with older compatibles, so this
> should be restricted per each variant (allOf:if:then)... but wait,
> old
> binding did not allow any reset-cells! You add an entirely new
> binding
> property and already want to support some older (deprecated?) way.
> 
> > 
> > 2. #reset-cells = <2>
> >    The input arguments is offset and bit. When we input two
> > arguments,
> >    we can use reset_xlate() to calculate the corresponding id to
> > assert
> >    and deassert.
> > 
> > [1]:
> > 
https://urldefense.com/v3/__https://lore.kernel.org/all/20220422060152.13534-10-rex-bc.chen@mediatek.com/__;!!CTRNKA9wMg0ARbw!0U0Yrp6WQxZ0YNMjaLJbAdq6Zyc524B4CY57-TP7QJ5FoSkCM72VI7mHJyWa1SZCnYTK$
> >  
> > 
> > If it's acceptable, I will add this in commit message.
> 
> 
> Best regards,
> Krzysztof

Hello Krzysztof,

I will let #reset-cells = <1> in next version and abandon the
modification of reset_xlate().

Thanks!

BRs,
Rex