| Message ID | 20220422060152.13534-12-rex-bc.chen@mediatek.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | Cleanup MediaTek clk reset drivers and support MT8192/MT8195 | expand |
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
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
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
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
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
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(+)