diff mbox series

[RFC,3/5] dt-bindings: power: Add binding for MediaTek MT7988 topmisc power controller

Message ID 20250413085806.8544-4-linux@fw-web.de
State RFC
Headers show
Series Add mt7988 XS-PHY | expand

Commit Message

Frank Wunderlich April 13, 2025, 8:58 a.m. UTC
From: Frank Wunderlich <frank-w@public-files.de>

Topmisc is a systemcontroller used for xs-phy and ethernet on  mt7988.
Add binding for it.

Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
---
 .../power/mediatek,power-controller.yaml      | 35 +++++++++++++------
 1 file changed, 24 insertions(+), 11 deletions(-)

Comments

Rob Herring April 13, 2025, 10:42 a.m. UTC | #1
On Sun, 13 Apr 2025 10:58:03 +0200, Frank Wunderlich wrote:
> From: Frank Wunderlich <frank-w@public-files.de>
> 
> Topmisc is a systemcontroller used for xs-phy and ethernet on  mt7988.
> Add binding for it.
> 
> Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
> ---
>  .../power/mediatek,power-controller.yaml      | 35 +++++++++++++------
>  1 file changed, 24 insertions(+), 11 deletions(-)
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:
./Documentation/devicetree/bindings/power/mediatek,power-controller.yaml:27:9: [warning] wrong indentation: expected 10 but found 8 (indentation)
./Documentation/devicetree/bindings/power/mediatek,power-controller.yaml:28:11: [warning] wrong indentation: expected 12 but found 10 (indentation)
./Documentation/devicetree/bindings/power/mediatek,power-controller.yaml:39:9: [warning] wrong indentation: expected 10 but found 8 (indentation)
./Documentation/devicetree/bindings/power/mediatek,power-controller.yaml:40:11: [warning] wrong indentation: expected 12 but found 10 (indentation)

dtschema/dtc warnings/errors:

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20250413085806.8544-4-linux@fw-web.de

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
AngeloGioacchino Del Regno April 14, 2025, 10:25 a.m. UTC | #2
Il 13/04/25 10:58, Frank Wunderlich ha scritto:
> From: Frank Wunderlich <frank-w@public-files.de>
> 
> Topmisc is a systemcontroller used for xs-phy and ethernet on  mt7988.
> Add binding for it.

That's the wrong binding... check mfd/syscon.yaml :-)

P.S.: Is there any reset controller in topmisc? Any clock?
       If yes, syscon.yaml is also wrong, and you need a driver for that.
       Remember: If it turns out *later* that this has clk/resets and the
       bindings are already set for just a syscon, it's gonna be way harder!

Cheers,
Angelo

> 
> Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
> ---
>   .../power/mediatek,power-controller.yaml      | 35 +++++++++++++------
>   1 file changed, 24 insertions(+), 11 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/power/mediatek,power-controller.yaml b/Documentation/devicetree/bindings/power/mediatek,power-controller.yaml
> index 591a080ca3ff..60d2fc7963e5 100644
> --- a/Documentation/devicetree/bindings/power/mediatek,power-controller.yaml
> +++ b/Documentation/devicetree/bindings/power/mediatek,power-controller.yaml
> @@ -22,17 +22,27 @@ properties:
>       pattern: '^power-controller(@[0-9a-f]+)?$'
>   
>     compatible:
> -    enum:
> -      - mediatek,mt6735-power-controller
> -      - mediatek,mt6795-power-controller
> -      - mediatek,mt8167-power-controller
> -      - mediatek,mt8173-power-controller
> -      - mediatek,mt8183-power-controller
> -      - mediatek,mt8186-power-controller
> -      - mediatek,mt8188-power-controller
> -      - mediatek,mt8192-power-controller
> -      - mediatek,mt8195-power-controller
> -      - mediatek,mt8365-power-controller
> +    oneOf:
> +      - items:
> +        - enum:
> +          - mediatek,mt6735-power-controller
> +          - mediatek,mt6795-power-controller
> +          - mediatek,mt8167-power-controller
> +          - mediatek,mt8173-power-controller
> +          - mediatek,mt8183-power-controller
> +          - mediatek,mt8186-power-controller
> +          - mediatek,mt8188-power-controller
> +          - mediatek,mt8192-power-controller
> +          - mediatek,mt8195-power-controller
> +          - mediatek,mt8365-power-controller
> +      - items:
> +        - enum:
> +          - mediatek,mt7988-topmisc
> +        - const: syscon
> +        - const: mediatek,mt7988-power-controller
> +
> +  reg:
> +    maxItems: 1
>   
>     '#power-domain-cells':
>       const: 1
> @@ -43,6 +53,9 @@ properties:
>     '#size-cells':
>       const: 0
>   
> +  '#clock-cells':
> +    const: 1
> +
>   patternProperties:
>     "^power-domain@[0-9a-f]+$":
>       $ref: "#/$defs/power-domain-node"
Frank Wunderlich April 15, 2025, 9:52 a.m. UTC | #3
Hi Angelo,

Am 14. April 2025 12:25:23 MESZ schrieb AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>:
>Il 13/04/25 10:58, Frank Wunderlich ha scritto:
>> From: Frank Wunderlich <frank-w@public-files.de>
>> 
>> Topmisc is a systemcontroller used for xs-phy and ethernet on  mt7988.
>> Add binding for it.
>
>That's the wrong binding... check mfd/syscon.yaml :-)
>
>P.S.: Is there any reset controller in topmisc? Any clock?
>      If yes, syscon.yaml is also wrong, and you need a driver for that.
>      Remember: If it turns out *later* that this has clk/resets and the
>      bindings are already set for just a syscon, it's gonna be way harder!
>
>Cheers,
>Angelo

Ok based on the power-domain-cells property i guessed powercontroller is the right place.

But based on your suggestion i tried moving compatible to syscon binding and made dtbs_check here.

I can confirm dropping the unexpected properties reported by syscon binding (power-domain-cells,clock-cells,adress-cells and size-cells) are not needed for function (xsphy and ethernet).

For verifying that there are really no clocks/resets in topmisc (have not found it in public available register documents) i asked mtk (waiting for answer).

Also got it working without the syscon compatible by changing ethernet driver too (after this change xsphy was also working).

Eth:
https://github.com/frank-w/BPI-Router-Linux/commit/d866e648717800b6f6395ad36c38f9effcf0498d
Xsphy:
<https://github.com/frank-w/BPI-Router-Linux/commit/0121a94df99700487704ca056b210b13db07e90c>

regards Frank
AngeloGioacchino Del Regno April 15, 2025, 9:59 a.m. UTC | #4
Il 15/04/25 11:52, Frank Wunderlich ha scritto:
> Hi Angelo,
> 
> Am 14. April 2025 12:25:23 MESZ schrieb AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>:
>> Il 13/04/25 10:58, Frank Wunderlich ha scritto:
>>> From: Frank Wunderlich <frank-w@public-files.de>
>>>
>>> Topmisc is a systemcontroller used for xs-phy and ethernet on  mt7988.
>>> Add binding for it.
>>
>> That's the wrong binding... check mfd/syscon.yaml :-)
>>
>> P.S.: Is there any reset controller in topmisc? Any clock?
>>       If yes, syscon.yaml is also wrong, and you need a driver for that.
>>       Remember: If it turns out *later* that this has clk/resets and the
>>       bindings are already set for just a syscon, it's gonna be way harder!
>>
>> Cheers,
>> Angelo
> 
> Ok based on the power-domain-cells property i guessed powercontroller is the right place.

power-domain-cells, but without any power domain assignment, so that was wrong :)))

> 
> But based on your suggestion i tried moving compatible to syscon binding and made dtbs_check here.
> 
> I can confirm dropping the unexpected properties reported by syscon binding (power-domain-cells,clock-cells,adress-cells and size-cells) are not needed for function (xsphy and ethernet).
> 
> For verifying that there are really no clocks/resets in topmisc (have not found it in public available register documents) i asked mtk (waiting for answer).
> 
> Also got it working without the syscon compatible by changing ethernet driver too (after this change xsphy was also working).

Perfect, a bit of a cleanup and you're done, then!

Cheers!

> 
> Eth:
> https://github.com/frank-w/BPI-Router-Linux/commit/d866e648717800b6f6395ad36c38f9effcf0498d
> Xsphy:
> <https://github.com/frank-w/BPI-Router-Linux/commit/0121a94df99700487704ca056b210b13db07e90c>
> 
> regards Frank
Frank Wunderlich April 15, 2025, 11:33 a.m. UTC | #5
Am 15. April 2025 11:59:06 MESZ schrieb AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>:
>Il 15/04/25 11:52, Frank Wunderlich ha scritto:
>> Hi Angelo,
>> 
>> Am 14. April 2025 12:25:23 MESZ schrieb AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>:
>>> Il 13/04/25 10:58, Frank Wunderlich ha scritto:
>>>> From: Frank Wunderlich <frank-w@public-files.de>
>>>> 
>>>> Topmisc is a systemcontroller used for xs-phy and ethernet on  mt7988.
>>>> Add binding for it.
>>> 
>>> That's the wrong binding... check mfd/syscon.yaml :-)
>>> 
>>> P.S.: Is there any reset controller in topmisc? Any clock?
>>>       If yes, syscon.yaml is also wrong, and you need a driver for that.
>>>       Remember: If it turns out *later* that this has clk/resets and the
>>>       bindings are already set for just a syscon, it's gonna be way harder!
>>> 
>>> Cheers,
>>> Angelo
>> 
>> Ok based on the power-domain-cells property i guessed powercontroller is the right place.
>
>power-domain-cells, but without any power domain assignment, so that was wrong :)))
>
>> 
>> But based on your suggestion i tried moving compatible to syscon binding and made dtbs_check here.
>> 
>> I can confirm dropping the unexpected properties reported by syscon binding (power-domain-cells,clock-cells,adress-cells and size-cells) are not needed for function (xsphy and ethernet).
>> 
>> For verifying that there are really no clocks/resets in topmisc (have not found it in public available register documents) i asked mtk (waiting for answer).
>> 
>> Also got it working without the syscon compatible by changing ethernet driver too (after this change xsphy was also working).
>
>Perfect, a bit of a cleanup and you're done, then!

If i do not have to change/merge driver code...and i hope getting response from mtk soon.

>Cheers!
>
>> 
>> Eth:
>> https://github.com/frank-w/BPI-Router-Linux/commit/d866e648717800b6f6395ad36c38f9effcf0498d
>> Xsphy:
>> <https://github.com/frank-w/BPI-Router-Linux/commit/0121a94df99700487704ca056b210b13db07e90c>
>> 
>> regards Frank
>
>
>


regards Frank
Frank Wunderlich April 15, 2025, 2:03 p.m. UTC | #6
Am 15. April 2025 11:59:06 MESZ schrieb AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>:
>Il 15/04/25 11:52, Frank Wunderlich ha scritto:
>> Hi Angelo,
>> 
>> Am 14. April 2025 12:25:23 MESZ schrieb AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>:
>>> Il 13/04/25 10:58, Frank Wunderlich ha scritto:
>>>> From: Frank Wunderlich <frank-w@public-files.de>
>>>> 
>>>> Topmisc is a systemcontroller used for xs-phy and ethernet on  mt7988.
>>>> Add binding for it.
>>> 
>>> That's the wrong binding... check mfd/syscon.yaml :-)
>>> 
>>> P.S.: Is there any reset controller in topmisc? Any clock?
>>>       If yes, syscon.yaml is also wrong, and you need a driver for that.
>>>       Remember: If it turns out *later* that this has clk/resets and the
>>>       bindings are already set for just a syscon, it's gonna be way harder!
>>> 
>>> Cheers,
>>> Angelo
>> 
>> Ok based on the power-domain-cells property i guessed powercontroller is the right place.
>
>power-domain-cells, but without any power domain assignment, so that was wrong :)))
>
>> 
>> But based on your suggestion i tried moving compatible to syscon binding and made dtbs_check here.
>> 
>> I can confirm dropping the unexpected properties reported by syscon binding (power-domain-cells,clock-cells,adress-cells and size-cells) are not needed for function (xsphy and ethernet).
>> 
>> For verifying that there are really no clocks/resets in topmisc (have not found it in public available register documents) i asked mtk (waiting for answer).
>> 
>> Also got it working without the syscon compatible by changing ethernet driver too (after this change xsphy was also working).
>
>Perfect, a bit of a cleanup and you're done, then!
>
>Cheers!
>
>> 
>> Eth:
>> https://github.com/frank-w/BPI-Router-Linux/commit/d866e648717800b6f6395ad36c38f9effcf0498d
>> Xsphy:
>> <https://github.com/frank-w/BPI-Router-Linux/commit/0121a94df99700487704ca056b210b13db07e90c>
>> 
>> regards Frank
>
>
>

Got response from MTK and basicly topmisc contains a powercontroller (for cpu and internal 2g5) but currently not needed because ATF already switch this on. The second part is the pcs/phy muxing and 3rd some unneeded switches (where i have no detailed information). But no clocks or resets as these are handled in the connected platform driver (xsphy/ethernet).

So my original binding imho made more sense.

The syscon binding seems to need syscon fallback and shows error about unexpected "compatible" property. The binding itself does not contain any properties but references syscon-common where iiuc compatible property must have at least 2 items and requires the "syscon" fallback.

Mtk suggests splitting topmisc into the 2 blocks by only using the mux part as syscon with splitting the reg. If powerdomain really is needed then a second topmisc node could be added and bound to a new driver.

But this would need a bit more of testing.
regards Frank
AngeloGioacchino Del Regno April 15, 2025, 2:12 p.m. UTC | #7
Il 15/04/25 16:03, Frank Wunderlich ha scritto:
> Am 15. April 2025 11:59:06 MESZ schrieb AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>:
>> Il 15/04/25 11:52, Frank Wunderlich ha scritto:
>>> Hi Angelo,
>>>
>>> Am 14. April 2025 12:25:23 MESZ schrieb AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>:
>>>> Il 13/04/25 10:58, Frank Wunderlich ha scritto:
>>>>> From: Frank Wunderlich <frank-w@public-files.de>
>>>>>
>>>>> Topmisc is a systemcontroller used for xs-phy and ethernet on  mt7988.
>>>>> Add binding for it.
>>>>
>>>> That's the wrong binding... check mfd/syscon.yaml :-)
>>>>
>>>> P.S.: Is there any reset controller in topmisc? Any clock?
>>>>        If yes, syscon.yaml is also wrong, and you need a driver for that.
>>>>        Remember: If it turns out *later* that this has clk/resets and the
>>>>        bindings are already set for just a syscon, it's gonna be way harder!
>>>>
>>>> Cheers,
>>>> Angelo
>>>
>>> Ok based on the power-domain-cells property i guessed powercontroller is the right place.
>>
>> power-domain-cells, but without any power domain assignment, so that was wrong :)))
>>
>>>
>>> But based on your suggestion i tried moving compatible to syscon binding and made dtbs_check here.
>>>
>>> I can confirm dropping the unexpected properties reported by syscon binding (power-domain-cells,clock-cells,adress-cells and size-cells) are not needed for function (xsphy and ethernet).
>>>
>>> For verifying that there are really no clocks/resets in topmisc (have not found it in public available register documents) i asked mtk (waiting for answer).
>>>
>>> Also got it working without the syscon compatible by changing ethernet driver too (after this change xsphy was also working).
>>
>> Perfect, a bit of a cleanup and you're done, then!
>>
>> Cheers!
>>
>>>
>>> Eth:
>>> https://github.com/frank-w/BPI-Router-Linux/commit/d866e648717800b6f6395ad36c38f9effcf0498d
>>> Xsphy:
>>> <https://github.com/frank-w/BPI-Router-Linux/commit/0121a94df99700487704ca056b210b13db07e90c>
>>>
>>> regards Frank
>>
>>
>>
> 
> Got response from MTK and basicly topmisc contains a powercontroller (for cpu and internal 2g5) but currently not needed because ATF already switch this on. The second part is the pcs/phy muxing and 3rd some unneeded switches (where i have no detailed information). But no clocks or resets as these are handled in the connected platform driver (xsphy/ethernet).
> 
> So my original binding imho made more sense.
> 
> The syscon binding seems to need syscon fallback and shows error about unexpected "compatible" property. The binding itself does not contain any properties but references syscon-common where iiuc compatible property must have at least 2 items and requires the "syscon" fallback.
> 
> Mtk suggests splitting topmisc into the 2 blocks by only using the mux part as syscon with splitting the reg. If powerdomain really is needed then a second topmisc node could be added and bound to a new driver.
> 

I agree with MediaTek's suggestion. Split it, that just makes more sense.

Besides, if the first part (the power controller in topmisc) is managed by ATF it's
a good idea to avoid touching anything in there from the kernel :-)


> But this would need a bit more of testing.
> regards Frank
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/power/mediatek,power-controller.yaml b/Documentation/devicetree/bindings/power/mediatek,power-controller.yaml
index 591a080ca3ff..60d2fc7963e5 100644
--- a/Documentation/devicetree/bindings/power/mediatek,power-controller.yaml
+++ b/Documentation/devicetree/bindings/power/mediatek,power-controller.yaml
@@ -22,17 +22,27 @@  properties:
     pattern: '^power-controller(@[0-9a-f]+)?$'
 
   compatible:
-    enum:
-      - mediatek,mt6735-power-controller
-      - mediatek,mt6795-power-controller
-      - mediatek,mt8167-power-controller
-      - mediatek,mt8173-power-controller
-      - mediatek,mt8183-power-controller
-      - mediatek,mt8186-power-controller
-      - mediatek,mt8188-power-controller
-      - mediatek,mt8192-power-controller
-      - mediatek,mt8195-power-controller
-      - mediatek,mt8365-power-controller
+    oneOf:
+      - items:
+        - enum:
+          - mediatek,mt6735-power-controller
+          - mediatek,mt6795-power-controller
+          - mediatek,mt8167-power-controller
+          - mediatek,mt8173-power-controller
+          - mediatek,mt8183-power-controller
+          - mediatek,mt8186-power-controller
+          - mediatek,mt8188-power-controller
+          - mediatek,mt8192-power-controller
+          - mediatek,mt8195-power-controller
+          - mediatek,mt8365-power-controller
+      - items:
+        - enum:
+          - mediatek,mt7988-topmisc
+        - const: syscon
+        - const: mediatek,mt7988-power-controller
+
+  reg:
+    maxItems: 1
 
   '#power-domain-cells':
     const: 1
@@ -43,6 +53,9 @@  properties:
   '#size-cells':
     const: 0
 
+  '#clock-cells':
+    const: 1
+
 patternProperties:
   "^power-domain@[0-9a-f]+$":
     $ref: "#/$defs/power-domain-node"