[PATCH v2 03/19] dt-bindings: power: mediatek: Add bindings for MediaTek SCPSYS

Tinghan Shen posted 19 patches 2 years, 2 months ago
Only 17 patches received!
There is a newer version of this series
[PATCH v2 03/19] 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,scpsys.yaml         | 62 +++++++++++++++++++
 1 file changed, 62 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/mediatek,scpsys.yaml

diff --git a/Documentation/devicetree/bindings/mfd/mediatek,scpsys.yaml b/Documentation/devicetree/bindings/mfd/mediatek,scpsys.yaml
new file mode 100644
index 000000000000..a8b9220f2f27
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/mediatek,scpsys.yaml
@@ -0,0 +1,62 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mfd/mediatek,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:
+      - const: mediatek,scpsys
+      - const: syscon
+      - const: simple-mfd
+
+  reg:
+    maxItems: 1
+
+  power-controller:
+    $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,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 v2 03/19] dt-bindings: power: mediatek: Add bindings for MediaTek SCPSYS
Posted by Rob Herring 2 years, 2 months ago
On Thu, Jul 14, 2022 at 08:28:21PM +0800, Tinghan Shen wrote:
> The System Control Processor System (SCPSYS) has several power
> management related tasks in the system. Add the bindings for it.

Please coordinate your work:

https://lore.kernel.org/linux-arm-kernel/20220718180654.GA3260460-robh@kernel.org/

> 
> Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>
> ---
>  .../bindings/mfd/mediatek,scpsys.yaml         | 62 +++++++++++++++++++
>  1 file changed, 62 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mfd/mediatek,scpsys.yaml
Re: [PATCH v2 03/19] dt-bindings: power: mediatek: Add bindings for MediaTek SCPSYS
Posted by Krzysztof Kozlowski 2 years, 2 months ago
On 14/07/2022 14:28, 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,scpsys.yaml         | 62 +++++++++++++++++++
>  1 file changed, 62 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mfd/mediatek,scpsys.yaml
> 
> diff --git a/Documentation/devicetree/bindings/mfd/mediatek,scpsys.yaml b/Documentation/devicetree/bindings/mfd/mediatek,scpsys.yaml
> new file mode 100644
> index 000000000000..a8b9220f2f27
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/mediatek,scpsys.yaml
> @@ -0,0 +1,62 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mfd/mediatek,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:
> +      - const: mediatek,scpsys
> +      - const: syscon
> +      - const: simple-mfd
> +
> +  reg:
> +    maxItems: 1
> +
> +  power-controller:
> +    $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,scpsys", "syscon", "simple-mfd";

This should be a SoC-specific compatible (and filename).

> +        reg = <0x10006000 0x100>;
> +
> +        spm: power-controller {

I think you created before less-portable, quite constrained bindings for
power controller. You now require that mt8195-power-controller is always
a child of some parent device which will share its regmap/MMIO with it.

And what if in your next block there is no scpsys block and power
controller is the scpsys alone? It's not possible with your bindings.

Wouldn't it be better to assign some address space to the
power-controller (now as an offset from scpsys)?

This is just wondering (Rockchip did the same...) and not a blocker as
power-controller bindings are done.

Best regards,
Krzysztof
Re: [PATCH v2 03/19] dt-bindings: power: mediatek: Add bindings for MediaTek SCPSYS
Posted by Tinghan Shen 2 years, 2 months ago
Hi Krzysztof,

On Fri, 2022-07-15 at 09:57 +0200, Krzysztof Kozlowski wrote:
> On 14/07/2022 14:28, 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,scpsys.yaml         | 62 +++++++++++++++++++
> >  1 file changed, 62 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/mfd/mediatek,scpsys.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/mfd/mediatek,scpsys.yaml
> > b/Documentation/devicetree/bindings/mfd/mediatek,scpsys.yaml
> > new file mode 100644
> > index 000000000000..a8b9220f2f27
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mfd/mediatek,scpsys.yaml
> > @@ -0,0 +1,62 @@
> > +# 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,scpsys.yaml*__;Iw!!CTRNKA9wMg0ARbw!1TUl-dhD0p8qh3rYVk8RtfoKEP88jg8OADMd19qP6siBCQHhFnHWCgsyUqiETyBzxw8$
> >  
> > +$schema: 
> > https://urldefense.com/v3/__http://devicetree.org/meta-schemas/core.yaml*__;Iw!!CTRNKA9wMg0ARbw!1TUl-dhD0p8qh3rYVk8RtfoKEP88jg8OADMd19qP6siBCQHhFnHWCgsyUqiEJQmakAI$
> >  
> > +
> > +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:
> > +      - const: mediatek,scpsys
> > +      - const: syscon
> > +      - const: simple-mfd
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  power-controller:
> > +    $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,scpsys", "syscon", "simple-mfd";
> 
> This should be a SoC-specific compatible (and filename).

Ok. I think that you mean "mediatek,mt8195-scpsys".
I'll update it in next version.

> 
> > +        reg = <0x10006000 0x100>;
> > +
> > +        spm: power-controller {
> 
> I think you created before less-portable, quite constrained bindings for
> power controller. You now require that mt8195-power-controller is always
> a child of some parent device which will share its regmap/MMIO with it.
> 
> And what if in your next block there is no scpsys block and power
> controller is the scpsys alone? It's not possible with your bindings.

Do you mean a power controller node that looks like this?

scpsys: power-controller@10006000 {
	compatible = "mediatek,mt6797-scpsys";
	#power-domain-cells = <1>;

	// ...
};

> 
> Wouldn't it be better to assign some address space to the
> power-controller (now as an offset from scpsys)?

Is this mean adding an offset after the node name?

spm: power-controller@0 {
                     ^^

> 
> This is just wondering (Rockchip did the same...) and not a blocker as
> power-controller bindings are done.
> 
> Best regards,
> Krzysztof


Thanks,
TingHan
Re: [PATCH v2 03/19] dt-bindings: power: mediatek: Add bindings for MediaTek SCPSYS
Posted by Krzysztof Kozlowski 2 years, 2 months ago
On 19/07/2022 10:17, Tinghan Shen wrote:
>>> +    syscon@10006000 {
>>> +        compatible = "mediatek,scpsys", "syscon", "simple-mfd";
>>
>> This should be a SoC-specific compatible (and filename).
> 
> Ok. I think that you mean "mediatek,mt8195-scpsys".
> I'll update it in next version.

Yes.

> 
>>
>>> +        reg = <0x10006000 0x100>;
>>> +
>>> +        spm: power-controller {
>>
>> I think you created before less-portable, quite constrained bindings for
>> power controller. You now require that mt8195-power-controller is always
>> a child of some parent device which will share its regmap/MMIO with it.
>>
>> And what if in your next block there is no scpsys block and power
>> controller is the scpsys alone? It's not possible with your bindings.
> 
> Do you mean a power controller node that looks like this?
> 
> scpsys: power-controller@10006000 {
> 	compatible = "mediatek,mt6797-scpsys";
> 	#power-domain-cells = <1>;
> 
> 	// ...
> };

Yes, I mean, with an unit address.

> 
>>
>> Wouldn't it be better to assign some address space to the
>> power-controller (now as an offset from scpsys)?
> 
> Is this mean adding an offset after the node name?
> 
> spm: power-controller@0 {

This or above. I think it does not matter for the bindings - it's an
implementation detail, whether you give to the child absolute SoC
address or you give an bus-specific (scpsys) sub-address/offset.

The point is that you have an unit address, thus in the future this
could be a device node separate from scpsys.

Best regards,
Krzysztof
Re: [PATCH v2 03/19] dt-bindings: power: mediatek: Add bindings for MediaTek SCPSYS
Posted by Lee Jones 2 years, 2 months ago
Subject line should be 'mfd', rather than 'power'.

On Thu, 14 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>
> ---
>  .../bindings/mfd/mediatek,scpsys.yaml         | 62 +++++++++++++++++++
>  1 file changed, 62 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mfd/mediatek,scpsys.yaml
> 
> diff --git a/Documentation/devicetree/bindings/mfd/mediatek,scpsys.yaml b/Documentation/devicetree/bindings/mfd/mediatek,scpsys.yaml
> new file mode 100644
> index 000000000000..a8b9220f2f27
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/mediatek,scpsys.yaml
> @@ -0,0 +1,62 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mfd/mediatek,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:
> +      - const: mediatek,scpsys
> +      - const: syscon
> +      - const: simple-mfd
> +
> +  reg:
> +    maxItems: 1
> +
> +  power-controller:
> +    $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,scpsys", "syscon", "simple-mfd";

Not sure you need bindings for this.  Seems overkill.

I'll let the DT guys have the final say though.

> +        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