[PATCH v3 06/21] dt-bindings: power: mediatek: Add bindings for MediaTek SCPSYS

Tinghan Shen posted 21 patches 2 years, 2 months ago
Only 19 patches received!
There is a newer version of this series
[PATCH v3 06/21] dt-bindings: power: mediatek: Add bindings for MediaTek SCPSYS
Posted by Tinghan Shen 2 years, 2 months ago
The System Control Processor System (SCPSYS) has several power
management related tasks in the system. Add the bindings for it.

Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>
---
 .../bindings/mfd/mediatek,mt8195-scpsys.yaml  | 68 +++++++++++++++++++
 1 file changed, 68 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/mediatek,mt8195-scpsys.yaml

diff --git a/Documentation/devicetree/bindings/mfd/mediatek,mt8195-scpsys.yaml b/Documentation/devicetree/bindings/mfd/mediatek,mt8195-scpsys.yaml
new file mode 100644
index 000000000000..4117a6dbc19c
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/mediatek,mt8195-scpsys.yaml
@@ -0,0 +1,68 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mfd/mediatek,mt8195-scpsys.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: MediaTek System Control Processor System
+
+maintainers:
+  - MandyJH Liu <mandyjh.liu@mediatek.com>
+
+description:
+  MediaTek System Control Processor System (SCPSYS) has several
+  power management tasks. The tasks include MTCMOS power
+  domain control, thermal measurement, DVFS, etc.
+
+properties:
+  compatible:
+    items:
+      - enum:
+          - mediatek,mt8167-scpsys
+          - mediatek,mt8173-scpsys
+          - mediatek,mt8183-scpsys
+          - mediatek,mt8192-scpsys
+          - mediatek,mt8195-scpsys
+      - const: syscon
+      - const: simple-mfd
+
+  reg:
+    maxItems: 1
+
+patternProperties:
+  "^power-controller(@[0-9a-f]+)?$":
+    $ref: /schemas/power/mediatek,power-controller.yaml#
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/mt8195-clk.h>
+    #include <dt-bindings/power/mt8195-power.h>
+
+    syscon@10006000 {
+        compatible = "mediatek,mt8195-scpsys", "syscon", "simple-mfd";
+        reg = <0x10006000 0x100>;
+
+        spm: power-controller {
+            compatible = "mediatek,mt8195-power-controller";
+            #address-cells = <1>;
+            #size-cells = <0>;
+            #power-domain-cells = <1>;
+
+            /* sample of power domain nodes */
+            power-domain@MT8195_POWER_DOMAIN_PCIE_PHY {
+                    reg = <MT8195_POWER_DOMAIN_PCIE_PHY>;
+                    #power-domain-cells = <0>;
+            };
+
+            power-domain@MT8195_POWER_DOMAIN_SSUSB_PCIE_PHY {
+                    reg = <MT8195_POWER_DOMAIN_SSUSB_PCIE_PHY>;
+                    #power-domain-cells = <0>;
+            };
+        };
+    };
-- 
2.18.0
Re: [PATCH v3 06/21] dt-bindings: power: mediatek: Add bindings for MediaTek SCPSYS
Posted by Krzysztof Kozlowski 2 years, 2 months ago
On 20/07/2022 14:30, Tinghan Shen wrote:
> The System Control Processor System (SCPSYS) has several power
> management related tasks in the system. Add the bindings for it.
> 
> Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>
> ---
>  .../bindings/mfd/mediatek,mt8195-scpsys.yaml  | 68 +++++++++++++++++++
>  1 file changed, 68 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mfd/mediatek,mt8195-scpsys.yaml
> 
> diff --git a/Documentation/devicetree/bindings/mfd/mediatek,mt8195-scpsys.yaml b/Documentation/devicetree/bindings/mfd/mediatek,mt8195-scpsys.yaml
> new file mode 100644
> index 000000000000..4117a6dbc19c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/mediatek,mt8195-scpsys.yaml
> @@ -0,0 +1,68 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mfd/mediatek,mt8195-scpsys.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: MediaTek System Control Processor System
> +
> +maintainers:
> +  - MandyJH Liu <mandyjh.liu@mediatek.com>
> +
> +description:
> +  MediaTek System Control Processor System (SCPSYS) has several
> +  power management tasks. The tasks include MTCMOS power
> +  domain control, thermal measurement, DVFS, etc.
> +
> +properties:
> +  compatible:
> +    items:
> +      - enum:
> +          - mediatek,mt8167-scpsys
> +          - mediatek,mt8173-scpsys
> +          - mediatek,mt8183-scpsys
> +          - mediatek,mt8192-scpsys
> +          - mediatek,mt8195-scpsys
> +      - const: syscon
> +      - const: simple-mfd
> +
> +  reg:
> +    maxItems: 1
> +
> +patternProperties:
> +  "^power-controller(@[0-9a-f]+)?$":
> +    $ref: /schemas/power/mediatek,power-controller.yaml#

We talked that unit address might be useful but it was with an
assumption that you will actually use it. I think you don't use it, so
it is kind of meaningless now... unless you plan to use it?

> +
> +required:
> +  - compatible
> +  - reg
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/mt8195-clk.h>
> +    #include <dt-bindings/power/mt8195-power.h>
> +
> +    syscon@10006000 {
> +        compatible = "mediatek,mt8195-scpsys", "syscon", "simple-mfd";
> +        reg = <0x10006000 0x100>;
> +
> +        spm: power-controller {
> +            compatible = "mediatek,mt8195-power-controller";
> +            #address-cells = <1>;
> +            #size-cells = <0>;
> +            #power-domain-cells = <1>;
> +
> +            /* sample of power domain nodes */
> +            power-domain@MT8195_POWER_DOMAIN_PCIE_PHY {
> +                    reg = <MT8195_POWER_DOMAIN_PCIE_PHY>;

Wrong indentation.


Best regards,
Krzysztof
Re: [PATCH v3 06/21] dt-bindings: power: mediatek: Add bindings for MediaTek SCPSYS
Posted by Tinghan Shen 2 years, 2 months ago
On Wed, 2022-07-20 at 19:35 +0200, Krzysztof Kozlowski wrote:
> On 20/07/2022 14:30, Tinghan Shen wrote:
> > The System Control Processor System (SCPSYS) has several power
> > management related tasks in the system. Add the bindings for it.
> > 
> > Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>
> > ---
> >  .../bindings/mfd/mediatek,mt8195-scpsys.yaml  | 68 +++++++++++++++++++
> >  1 file changed, 68 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/mfd/mediatek,mt8195-scpsys.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/mfd/mediatek,mt8195-scpsys.yaml
> > b/Documentation/devicetree/bindings/mfd/mediatek,mt8195-scpsys.yaml
> > new file mode 100644
> > index 000000000000..4117a6dbc19c
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mfd/mediatek,mt8195-scpsys.yaml
> > @@ -0,0 +1,68 @@
> > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> > +%YAML 1.2
> > +---
> > +$id: 
> > https://urldefense.com/v3/__http://devicetree.org/schemas/mfd/mediatek,mt8195-scpsys.yaml*__;Iw!!CTRNKA9wMg0ARbw!y63E-9qbCW_vyn8RrsCCs7YvZ7NxKqFT7l8C0ZZirEW95Ec0ce3lwegsSq51wrjtz8GjVSpkK-omCBP5CKx1l0WESQ$
> >  
> > +$schema: 
> > https://urldefense.com/v3/__http://devicetree.org/meta-schemas/core.yaml*__;Iw!!CTRNKA9wMg0ARbw!y63E-9qbCW_vyn8RrsCCs7YvZ7NxKqFT7l8C0ZZirEW95Ec0ce3lwegsSq51wrjtz8GjVSpkK-omCBP5CKzzr9p82Q$
> >  
> > +
> > +title: MediaTek System Control Processor System
> > +
> > +maintainers:
> > +  - MandyJH Liu <mandyjh.liu@mediatek.com>
> > +
> > +description:
> > +  MediaTek System Control Processor System (SCPSYS) has several
> > +  power management tasks. The tasks include MTCMOS power
> > +  domain control, thermal measurement, DVFS, etc.
> > +
> > +properties:
> > +  compatible:
> > +    items:
> > +      - enum:
> > +          - mediatek,mt8167-scpsys
> > +          - mediatek,mt8173-scpsys
> > +          - mediatek,mt8183-scpsys
> > +          - mediatek,mt8192-scpsys
> > +          - mediatek,mt8195-scpsys
> > +      - const: syscon
> > +      - const: simple-mfd
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +patternProperties:
> > +  "^power-controller(@[0-9a-f]+)?$":
> > +    $ref: /schemas/power/mediatek,power-controller.yaml#
> 
> We talked that unit address might be useful but it was with an
> assumption that you will actually use it. I think you don't use it, so
> it is kind of meaningless now... unless you plan to use it?

I tried to add the offset in the node name, but the binding check reports this message.
 	
    power-controller@0: node has a unit name, but no reg or ranges property

After considering the fact of mt8195 power controller HW resides in scpsys and the current power
domain driver doesn't support parsing the register address seperated from scpsys, I decide to
keep the power controller node as in v2 to pass the binding check and compatible with driver.

> 
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/clock/mt8195-clk.h>
> > +    #include <dt-bindings/power/mt8195-power.h>
> > +
> > +    syscon@10006000 {
> > +        compatible = "mediatek,mt8195-scpsys", "syscon", "simple-mfd";
> > +        reg = <0x10006000 0x100>;
> > +
> > +        spm: power-controller {
> > +            compatible = "mediatek,mt8195-power-controller";
> > +            #address-cells = <1>;
> > +            #size-cells = <0>;
> > +            #power-domain-cells = <1>;
> > +
> > +            /* sample of power domain nodes */
> > +            power-domain@MT8195_POWER_DOMAIN_PCIE_PHY {
> > +                    reg = <MT8195_POWER_DOMAIN_PCIE_PHY>;
> 
> Wrong indentation.

I missed checking this. I'll udpate in the next version.

Thanks,
TingHan
Re: [PATCH v3 06/21] dt-bindings: power: mediatek: Add bindings for MediaTek SCPSYS
Posted by Krzysztof Kozlowski 2 years, 2 months ago
On 21/07/2022 05:05, Tinghan Shen wrote:
> On Wed, 2022-07-20 at 19:35 +0200, Krzysztof Kozlowski wrote:
>> On 20/07/2022 14:30, Tinghan Shen wrote:
>>> The System Control Processor System (SCPSYS) has several power
>>> management related tasks in the system. Add the bindings for it.
>>>
>>> Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>
>>> ---
>>>  .../bindings/mfd/mediatek,mt8195-scpsys.yaml  | 68 +++++++++++++++++++
>>>  1 file changed, 68 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/mfd/mediatek,mt8195-scpsys.yaml
>>>
>>> diff --git a/Documentation/devicetree/bindings/mfd/mediatek,mt8195-scpsys.yaml
>>> b/Documentation/devicetree/bindings/mfd/mediatek,mt8195-scpsys.yaml
>>> new file mode 100644
>>> index 000000000000..4117a6dbc19c
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/mfd/mediatek,mt8195-scpsys.yaml
>>> @@ -0,0 +1,68 @@
>>> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
>>> +%YAML 1.2
>>> +---
>>> +$id: 
>>> https://urldefense.com/v3/__http://devicetree.org/schemas/mfd/mediatek,mt8195-scpsys.yaml*__;Iw!!CTRNKA9wMg0ARbw!y63E-9qbCW_vyn8RrsCCs7YvZ7NxKqFT7l8C0ZZirEW95Ec0ce3lwegsSq51wrjtz8GjVSpkK-omCBP5CKx1l0WESQ$
>>>  
>>> +$schema: 
>>> https://urldefense.com/v3/__http://devicetree.org/meta-schemas/core.yaml*__;Iw!!CTRNKA9wMg0ARbw!y63E-9qbCW_vyn8RrsCCs7YvZ7NxKqFT7l8C0ZZirEW95Ec0ce3lwegsSq51wrjtz8GjVSpkK-omCBP5CKzzr9p82Q$
>>>  
>>> +
>>> +title: MediaTek System Control Processor System
>>> +
>>> +maintainers:
>>> +  - MandyJH Liu <mandyjh.liu@mediatek.com>
>>> +
>>> +description:
>>> +  MediaTek System Control Processor System (SCPSYS) has several
>>> +  power management tasks. The tasks include MTCMOS power
>>> +  domain control, thermal measurement, DVFS, etc.
>>> +
>>> +properties:
>>> +  compatible:
>>> +    items:
>>> +      - enum:
>>> +          - mediatek,mt8167-scpsys
>>> +          - mediatek,mt8173-scpsys
>>> +          - mediatek,mt8183-scpsys
>>> +          - mediatek,mt8192-scpsys
>>> +          - mediatek,mt8195-scpsys
>>> +      - const: syscon
>>> +      - const: simple-mfd
>>> +
>>> +  reg:
>>> +    maxItems: 1
>>> +
>>> +patternProperties:
>>> +  "^power-controller(@[0-9a-f]+)?$":
>>> +    $ref: /schemas/power/mediatek,power-controller.yaml#
>>
>> We talked that unit address might be useful but it was with an
>> assumption that you will actually use it. I think you don't use it, so
>> it is kind of meaningless now... unless you plan to use it?
> 
> I tried to add the offset in the node name, but the binding check reports this message.
>  	
>     power-controller@0: node has a unit name, but no reg or ranges property

Why would you add unit address without reg? What point would it solve?

> After considering the fact of mt8195 power controller HW resides in scpsys and the current power
> domain driver doesn't support parsing the register address seperated from scpsys, I decide to
> keep the power controller node as in v2 to pass the binding check and compatible with driver.

Then adding unit addresses in bindings has no point.


Best regards,
Krzysztof
Re: [PATCH v3 06/21] dt-bindings: power: mediatek: Add bindings for MediaTek SCPSYS
Posted by Lee Jones 2 years, 2 months ago
On Wed, 20 Jul 2022, Tinghan Shen wrote:

> The System Control Processor System (SCPSYS) has several power
> management related tasks in the system. Add the bindings for it.
> 
> Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>

Why have you ignored my review comments from v2?

> ---
>  .../bindings/mfd/mediatek,mt8195-scpsys.yaml  | 68 +++++++++++++++++++
>  1 file changed, 68 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mfd/mediatek,mt8195-scpsys.yaml
> 
> diff --git a/Documentation/devicetree/bindings/mfd/mediatek,mt8195-scpsys.yaml b/Documentation/devicetree/bindings/mfd/mediatek,mt8195-scpsys.yaml
> new file mode 100644
> index 000000000000..4117a6dbc19c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/mediatek,mt8195-scpsys.yaml
> @@ -0,0 +1,68 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mfd/mediatek,mt8195-scpsys.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: MediaTek System Control Processor System
> +
> +maintainers:
> +  - MandyJH Liu <mandyjh.liu@mediatek.com>
> +
> +description:
> +  MediaTek System Control Processor System (SCPSYS) has several
> +  power management tasks. The tasks include MTCMOS power
> +  domain control, thermal measurement, DVFS, etc.
> +
> +properties:
> +  compatible:
> +    items:
> +      - enum:
> +          - mediatek,mt8167-scpsys
> +          - mediatek,mt8173-scpsys
> +          - mediatek,mt8183-scpsys
> +          - mediatek,mt8192-scpsys
> +          - mediatek,mt8195-scpsys
> +      - const: syscon
> +      - const: simple-mfd
> +
> +  reg:
> +    maxItems: 1
> +
> +patternProperties:
> +  "^power-controller(@[0-9a-f]+)?$":
> +    $ref: /schemas/power/mediatek,power-controller.yaml#
> +
> +required:
> +  - compatible
> +  - reg
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/mt8195-clk.h>
> +    #include <dt-bindings/power/mt8195-power.h>
> +
> +    syscon@10006000 {
> +        compatible = "mediatek,mt8195-scpsys", "syscon", "simple-mfd";
> +        reg = <0x10006000 0x100>;
> +
> +        spm: power-controller {
> +            compatible = "mediatek,mt8195-power-controller";
> +            #address-cells = <1>;
> +            #size-cells = <0>;
> +            #power-domain-cells = <1>;
> +
> +            /* sample of power domain nodes */
> +            power-domain@MT8195_POWER_DOMAIN_PCIE_PHY {
> +                    reg = <MT8195_POWER_DOMAIN_PCIE_PHY>;
> +                    #power-domain-cells = <0>;
> +            };
> +
> +            power-domain@MT8195_POWER_DOMAIN_SSUSB_PCIE_PHY {
> +                    reg = <MT8195_POWER_DOMAIN_SSUSB_PCIE_PHY>;
> +                    #power-domain-cells = <0>;
> +            };
> +        };
> +    };

-- 
Lee Jones [李琼斯]
Principal Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog
Re: [PATCH v3 06/21] dt-bindings: power: mediatek: Add bindings for MediaTek SCPSYS
Posted by Krzysztof Kozlowski 2 years, 2 months ago
On 20/07/2022 15:31, Lee Jones wrote:
> On Wed, 20 Jul 2022, Tinghan Shen wrote:
> 
>> The System Control Processor System (SCPSYS) has several power
>> management related tasks in the system. Add the bindings for it.
>>
>> Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>
> 
> Why have you ignored my review comments from v2?

If you refer whether the binding is needed at all, then the answer is
yes, because this is not simple syscons but a device with children,
which we want to parse/match.

Anyway Tinghan should respond to you about way how he proceeds...

Best regards,
Krzysztof
Re: [PATCH v3 06/21] dt-bindings: power: mediatek: Add bindings for MediaTek SCPSYS
Posted by Lee Jones 2 years, 2 months ago
On Wed, 20 Jul 2022, Krzysztof Kozlowski wrote:

> On 20/07/2022 15:31, Lee Jones wrote:
> > On Wed, 20 Jul 2022, Tinghan Shen wrote:
> > 
> >> The System Control Processor System (SCPSYS) has several power
> >> management related tasks in the system. Add the bindings for it.
> >>
> >> Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>
> > 
> > Why have you ignored my review comments from v2?
> 
> If you refer whether the binding is needed at all, then the answer is
> yes, because this is not simple syscons but a device with children,
> which we want to parse/match.

This part is fine.

> Anyway Tinghan should respond to you about way how he proceeds...

This would be nice, yes.

I was referring to the submit line, which nearly made me miss it, again.

-- 
Lee Jones [李琼斯]
Principal Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog
Re: [PATCH v3 06/21] dt-bindings: power: mediatek: Add bindings for MediaTek SCPSYS
Posted by Tinghan Shen 2 years, 2 months ago
On Wed, 2022-07-20 at 19:37 +0200, Krzysztof Kozlowski wrote:
> On 20/07/2022 15:31, Lee Jones wrote:
> > On Wed, 20 Jul 2022, Tinghan Shen wrote:
> > 
> > > The System Control Processor System (SCPSYS) has several power
> > > management related tasks in the system. Add the bindings for it.
> > > 
> > > Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>
> > 
> > Why have you ignored my review comments from v2?
> 
> If you refer whether the binding is needed at all, then the answer is
> yes, because this is not simple syscons but a device with children,
> which we want to parse/match.
> 
> Anyway Tinghan should respond to you about way how he proceeds...
> 
> Best regards,
> Krzysztof

Hi Lee,

I'm so sorry for not answering your feedback. I misunderstood it
that you're waiting the review result of DT maintainers.

This binding is added because of the discussion of the scpsys node[1].
I first looked at mfd/syscon.yaml to see if I could use it.
I found that it doesn't have the 'simple-mfd' compatible, so I decide to add a new one.
It's because I need the 'simple-mfd' compatible to parse child nodes.


[1] https://lore.kernel.org/all/f7eee4e8-05fa-4c83-9168-64e5ea4c510f@linaro.org/



Best regards,
TingHan