[PATCH v1 00/16] Add driver nodes for MT8195 SoC

[PATCH v1 00/16] Add driver nodes for MT8195 SoC

[Tinghan Shen]

Add driver nodes for MT8195 SoC.

Patchset 12 depends on 
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git/commit/?id=009b21f392759ca7be91bc4be9d9534f6cee2878

Jason-JH.Lin (2):
  arm64: dts: mt8195: Add gce node
  arm64: dts: mt8195: Add display node for vdosys0

Tinghan Shen (10):
  dt-bindings: iommu: mediatek: Increase max interrupt number
  dt-bindings: memory: mediatek: Update condition for mt8195 smi node
  dt-bindings: power: mediatek: Refine multiple level power domain nodes
  arm64: dts: mt8195: Disable watchdog external reset signal
  arm64: dts: mt8195: Add vdosys and vppsys clock nodes
  arm64: dts: mt8195: Add power domains controller
  arm64: dts: mt8195: Add spmi node
  arm64: dts: mt8195: Add scp node
  arm64: dts: mt8195: Add audio related nodes
  arm64: dts: mt8195: Add iommu and smi nodes

Trevor Wu (1):
  arm64: dts: mt8195: Specify audio reset controller

Tzung-Bi Shih (1):
  arm64: dts: mt8195: Disable I2C0 node

YC Hung (1):
  arm64: dts: mt8195: Add adsp node and adsp mailbox nodes

YT Lee (1):
  arm64: dts: mt8195: Add cpufreq node

 .../bindings/iommu/mediatek,iommu.yaml        |   12 +-
 .../mediatek,smi-common.yaml                  |   10 +-
 .../power/mediatek,power-controller.yaml      |  132 +-
 arch/arm64/boot/dts/mediatek/mt8195.dtsi      | 1073 ++++++++++++++++-
 4 files changed, 1104 insertions(+), 123 deletions(-)

-- 
2.18.0

[PATCH v1 01/16] dt-bindings: iommu: mediatek: Increase max interrupt number

[Tinghan Shen]

mt8195 infra iommu has max 5 interrupts.

Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>
---
 .../devicetree/bindings/iommu/mediatek,iommu.yaml    | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml b/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml
index 2ae3bbad7f1a..27eb9f6aa3ce 100644
--- a/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml
+++ b/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml
@@ -91,7 +91,8 @@ properties:
     maxItems: 1
 
   interrupts:
-    maxItems: 1
+    minItems: 1
+    maxItems: 5
 
   clocks:
     items:
@@ -175,9 +176,18 @@ allOf:
               const: mediatek,mt8195-iommu-infra
 
     then:
+      properties:
+        interrupts:
+          maxItems: 1
+
       required:
         - mediatek,larbs
 
+    else:
+      properties:
+        interrupts:
+          maxItems: 5
+
 additionalProperties: false
 
 examples:
-- 
2.18.0

Re: [PATCH v1 01/16] dt-bindings: iommu: mediatek: Increase max interrupt number

[Rob Herring]

On Mon, Jul 04, 2022 at 06:00:13PM +0800, Tinghan Shen wrote:
> mt8195 infra iommu has max 5 interrupts.
> 
> Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>
> ---
>  .../devicetree/bindings/iommu/mediatek,iommu.yaml    | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml b/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml
> index 2ae3bbad7f1a..27eb9f6aa3ce 100644
> --- a/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml
> +++ b/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml
> @@ -91,7 +91,8 @@ properties:
>      maxItems: 1
>  
>    interrupts:
> -    maxItems: 1
> +    minItems: 1
> +    maxItems: 5
>  
>    clocks:
>      items:
> @@ -175,9 +176,18 @@ allOf:
>                const: mediatek,mt8195-iommu-infra
>  
>      then:
> +      properties:
> +        interrupts:
> +          maxItems: 1
> +
>        required:
>          - mediatek,larbs
>  
> +    else:
> +      properties:
> +        interrupts:
> +          maxItems: 5

5 is already the max.

minItems: 5

Re: [PATCH v1 01/16] dt-bindings: iommu: mediatek: Increase max interrupt number

[Tinghan Shen]

On Tue, 2022-07-05 at 14:49 -0600, Rob Herring wrote:
> On Mon, Jul 04, 2022 at 06:00:13PM +0800, Tinghan Shen wrote:
> > mt8195 infra iommu has max 5 interrupts.
> > 
> > Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>
> > ---
> >  .../devicetree/bindings/iommu/mediatek,iommu.yaml    | 12 +++++++++++-
> >  1 file changed, 11 insertions(+), 1 deletion(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml
> > b/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml
> > index 2ae3bbad7f1a..27eb9f6aa3ce 100644
> > --- a/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml
> > +++ b/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml
> > @@ -91,7 +91,8 @@ properties:
> >      maxItems: 1
> >  
> >    interrupts:
> > -    maxItems: 1
> > +    minItems: 1
> > +    maxItems: 5
> >  
> >    clocks:
> >      items:
> > @@ -175,9 +176,18 @@ allOf:
> >                const: mediatek,mt8195-iommu-infra
> >  
> >      then:
> > +      properties:
> > +        interrupts:
> > +          maxItems: 1
> > +
> >        required:
> >          - mediatek,larbs
> >  
> > +    else:
> > +      properties:
> > +        interrupts:
> > +          maxItems: 5
> 
> 5 is already the max.
> 
> minItems: 5
> 

Ok, I'll update in the next version.

Thanks,
TingHan

[PATCH v1 02/16] dt-bindings: memory: mediatek: Update condition for mt8195 smi node

[Tinghan Shen]

The max clock items for the dts node with compatible
'mediatek,mt8195-smi-sub-common' should be 3.

However, the dtbs_check of such node will get following message,
arch/arm64/boot/dts/mediatek/mt8195-evb.dtb: smi@14010000: clock-names: ['apb', 'smi', 'gals0'] is too long
         From schema: Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml

Remove the last 'else' checking to fix this error.

Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>
---
 .../memory-controllers/mediatek,smi-common.yaml        | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml b/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml
index a98b359bf909..e5f553e2e12a 100644
--- a/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml
+++ b/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml
@@ -143,7 +143,15 @@ allOf:
             - const: gals0
             - const: gals1
 
-    else:  # for gen2 HW that don't have gals
+  - if:  # for gen2 HW that don't have gals
+      properties:
+        compatible:
+          enum:
+            - mediatek,mt2712-smi-common
+            - mediatek,mt8167-smi-common
+            - mediatek,mt8173-smi-common
+
+    then:
       properties:
         clocks:
           minItems: 2
-- 
2.18.0

Re: [PATCH v1 02/16] dt-bindings: memory: mediatek: Update condition for mt8195 smi node

[AngeloGioacchino Del Regno]

Il 04/07/22 12:00, Tinghan Shen ha scritto:
> The max clock items for the dts node with compatible
> 'mediatek,mt8195-smi-sub-common' should be 3.
> 
> However, the dtbs_check of such node will get following message,
> arch/arm64/boot/dts/mediatek/mt8195-evb.dtb: smi@14010000: clock-names: ['apb', 'smi', 'gals0'] is too long
>           From schema: Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml
> 
> Remove the last 'else' checking to fix this error.
> 
> Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>
> ---
>   .../memory-controllers/mediatek,smi-common.yaml        | 10 +++++++++-
>   1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml b/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml
> index a98b359bf909..e5f553e2e12a 100644
> --- a/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml
> +++ b/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml
> @@ -143,7 +143,15 @@ allOf:
>               - const: gals0
>               - const: gals1
>   
> -    else:  # for gen2 HW that don't have gals
> +  - if:  # for gen2 HW that don't have gals
> +      properties:
> +        compatible:
> +          enum:
> +            - mediatek,mt2712-smi-common

MT6795 also doesn't have any GALS, please add it in here.

Regards,
Angelo

> +            - mediatek,mt8167-smi-common
> +            - mediatek,mt8173-smi-common
> +
> +    then:
>         properties:
>           clocks:
>             minItems: 2

Re: [PATCH v1 02/16] dt-bindings: memory: mediatek: Update condition for mt8195 smi node

[Tinghan Shen]

On Mon, 2022-07-04 at 12:25 +0200, AngeloGioacchino Del Regno wrote:
> Il 04/07/22 12:00, Tinghan Shen ha scritto:
> > The max clock items for the dts node with compatible
> > 'mediatek,mt8195-smi-sub-common' should be 3.
> > 
> > However, the dtbs_check of such node will get following message,
> > arch/arm64/boot/dts/mediatek/mt8195-evb.dtb: smi@14010000: clock-names: ['apb', 'smi', 'gals0']
> > is too long
> >           From schema: Documentation/devicetree/bindings/memory-controllers/mediatek,smi-
> > common.yaml
> > 
> > Remove the last 'else' checking to fix this error.
> > 
> > Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>
> > ---
> >   .../memory-controllers/mediatek,smi-common.yaml        | 10 +++++++++-
> >   1 file changed, 9 insertions(+), 1 deletion(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml
> > b/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml
> > index a98b359bf909..e5f553e2e12a 100644
> > --- a/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml
> > +++ b/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml
> > @@ -143,7 +143,15 @@ allOf:
> >               - const: gals0
> >               - const: gals1
> >   
> > -    else:  # for gen2 HW that don't have gals
> > +  - if:  # for gen2 HW that don't have gals
> > +      properties:
> > +        compatible:
> > +          enum:
> > +            - mediatek,mt2712-smi-common
> 
> MT6795 also doesn't have any GALS, please add it in here.

Ok, I'll update it in next version.

Thanks,
TingHan

Re: [PATCH v1 02/16] dt-bindings: memory: mediatek: Update condition for mt8195 smi node

[Krzysztof Kozlowski]

On 04/07/2022 12:00, Tinghan Shen wrote:
> The max clock items for the dts node with compatible
> 'mediatek,mt8195-smi-sub-common' should be 3.
> 
> However, the dtbs_check of such node will get following message,
> arch/arm64/boot/dts/mediatek/mt8195-evb.dtb: smi@14010000: clock-names: ['apb', 'smi', 'gals0'] is too long
>          From schema: Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml
> 
> Remove the last 'else' checking to fix this error.

Missing fixes tag.

> 
> Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>
> ---
>  .../memory-controllers/mediatek,smi-common.yaml        | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml b/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml
> index a98b359bf909..e5f553e2e12a 100644
> --- a/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml
> +++ b/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml
> @@ -143,7 +143,15 @@ allOf:
>              - const: gals0
>              - const: gals1
>  
> -    else:  # for gen2 HW that don't have gals
> +  - if:  # for gen2 HW that don't have gals
> +      properties:
> +        compatible:
> +          enum:
> +            - mediatek,mt2712-smi-common
> +            - mediatek,mt8167-smi-common
> +            - mediatek,mt8173-smi-common
> +

Without looking at the code, it's impossible to understand what you are
doing here. The commit msg says one, but you are doing something else.

Write commit msg explaining what you want to achieve and what you are doing.


Best regards,
Krzysztof

Re: [PATCH v1 02/16] dt-bindings: memory: mediatek: Update condition for mt8195 smi node

[Tinghan Shen]

On Mon, 2022-07-04 at 14:36 +0200, Krzysztof Kozlowski wrote:
> On 04/07/2022 12:00, Tinghan Shen wrote:
> > The max clock items for the dts node with compatible
> > 'mediatek,mt8195-smi-sub-common' should be 3.
> > 
> > However, the dtbs_check of such node will get following message,
> > arch/arm64/boot/dts/mediatek/mt8195-evb.dtb: smi@14010000: clock-names: ['apb', 'smi', 'gals0']
> > is too long
> >          From schema: Documentation/devicetree/bindings/memory-controllers/mediatek,smi-
> > common.yaml
> > 
> > Remove the last 'else' checking to fix this error.
> 
> Missing fixes tag.
> 
> > 
> > Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>
> > ---
> >  .../memory-controllers/mediatek,smi-common.yaml        | 10 +++++++++-
> >  1 file changed, 9 insertions(+), 1 deletion(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml
> > b/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml
> > index a98b359bf909..e5f553e2e12a 100644
> > --- a/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml
> > +++ b/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml
> > @@ -143,7 +143,15 @@ allOf:
> >              - const: gals0
> >              - const: gals1
> >  
> > -    else:  # for gen2 HW that don't have gals
> > +  - if:  # for gen2 HW that don't have gals
> > +      properties:
> > +        compatible:
> > +          enum:
> > +            - mediatek,mt2712-smi-common
> > +            - mediatek,mt8167-smi-common
> > +            - mediatek,mt8173-smi-common
> > +
> 
> Without looking at the code, it's impossible to understand what you are
> doing here. The commit msg says one, but you are doing something else.
> 
> Write commit msg explaining what you want to achieve and what you are doing.
> 
> 
> Best regards,
> Krzysztof

Ok, I'll update in next version.

Thanks,
TingHan

Re: [PATCH v1 02/16] dt-bindings: memory: mediatek: Update condition for mt8195 smi node

[Matthias Brugger]

On 04/07/2022 14:36, Krzysztof Kozlowski wrote:
> On 04/07/2022 12:00, Tinghan Shen wrote:
>> The max clock items for the dts node with compatible
>> 'mediatek,mt8195-smi-sub-common' should be 3.
>>
>> However, the dtbs_check of such node will get following message,
>> arch/arm64/boot/dts/mediatek/mt8195-evb.dtb: smi@14010000: clock-names: ['apb', 'smi', 'gals0'] is too long
>>           From schema: Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml
>>
>> Remove the last 'else' checking to fix this error.
> 
> Missing fixes tag.
> 

 From my understanding, fixes tags are for patches that fix bugs (hw is not 
working etc) and not a warning message from dtbs_check. So my point of view 
would be to not add a fixes tag here.

Regards,
Matthias

>>
>> Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>
>> ---
>>   .../memory-controllers/mediatek,smi-common.yaml        | 10 +++++++++-
>>   1 file changed, 9 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml b/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml
>> index a98b359bf909..e5f553e2e12a 100644
>> --- a/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml
>> +++ b/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml
>> @@ -143,7 +143,15 @@ allOf:
>>               - const: gals0
>>               - const: gals1
>>   
>> -    else:  # for gen2 HW that don't have gals
>> +  - if:  # for gen2 HW that don't have gals
>> +      properties:
>> +        compatible:
>> +          enum:
>> +            - mediatek,mt2712-smi-common
>> +            - mediatek,mt8167-smi-common
>> +            - mediatek,mt8173-smi-common
>> +
> 
> Without looking at the code, it's impossible to understand what you are
> doing here. The commit msg says one, but you are doing something else.
> 
> Write commit msg explaining what you want to achieve and what you are doing.
> 
> 
> Best regards,
> Krzysztof

Re: [PATCH v1 02/16] dt-bindings: memory: mediatek: Update condition for mt8195 smi node

[Krzysztof Kozlowski]

On 06/07/2022 15:48, Matthias Brugger wrote:
> 
> 
> On 04/07/2022 14:36, Krzysztof Kozlowski wrote:
>> On 04/07/2022 12:00, Tinghan Shen wrote:
>>> The max clock items for the dts node with compatible
>>> 'mediatek,mt8195-smi-sub-common' should be 3.
>>>
>>> However, the dtbs_check of such node will get following message,
>>> arch/arm64/boot/dts/mediatek/mt8195-evb.dtb: smi@14010000: clock-names: ['apb', 'smi', 'gals0'] is too long
>>>           From schema: Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml
>>>
>>> Remove the last 'else' checking to fix this error.
>>
>> Missing fixes tag.
>>
> 
>  From my understanding, fixes tags are for patches that fix bugs (hw is not 
> working etc) and not a warning message from dtbs_check. So my point of view 
> would be to not add a fixes tag here.

Not conforming to bindings is also a bug. Missing properties or wrong
properties, even if hardware is working, is still a bug. If such bug is
not visible now in Linux, might be visible later in the future or
visible in different OS (DTS are used by other systems and pieces of
software like bootloaders). Limiting this only to Linux and to current
version (hardware still works) is OK for Linux drivers, but not for DTS.

Therefore Fixes tag in general is applicable. Of course maybe to this
one not really, maybe this is too trivial, or whatever, so I do not
insist. But I insist on the principle - reasonable dtbs_check warnings
are like compiler warnings - bugs which have to be fixed.


Best regards,
Krzysztof

Re: [PATCH v1 02/16] dt-bindings: memory: mediatek: Update condition for mt8195 smi node

[Matthias Brugger]

On 06/07/2022 16:38, Krzysztof Kozlowski wrote:
> On 06/07/2022 15:48, Matthias Brugger wrote:
>>
>>
>> On 04/07/2022 14:36, Krzysztof Kozlowski wrote:
>>> On 04/07/2022 12:00, Tinghan Shen wrote:
>>>> The max clock items for the dts node with compatible
>>>> 'mediatek,mt8195-smi-sub-common' should be 3.
>>>>
>>>> However, the dtbs_check of such node will get following message,
>>>> arch/arm64/boot/dts/mediatek/mt8195-evb.dtb: smi@14010000: clock-names: ['apb', 'smi', 'gals0'] is too long
>>>>            From schema: Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml
>>>>
>>>> Remove the last 'else' checking to fix this error.
>>>
>>> Missing fixes tag.
>>>
>>
>>   From my understanding, fixes tags are for patches that fix bugs (hw is not
>> working etc) and not a warning message from dtbs_check. So my point of view
>> would be to not add a fixes tag here.
> 
> Not conforming to bindings is also a bug. Missing properties or wrong
> properties, even if hardware is working, is still a bug. If such bug is
> not visible now in Linux, might be visible later in the future or
> visible in different OS (DTS are used by other systems and pieces of
> software like bootloaders). Limiting this only to Linux and to current
> version (hardware still works) is OK for Linux drivers, but not for DTS.
> 

If a wrong DTS breaks software, then it's worth a fixes tag, especially for the 
DTS part, we could argue about the bindings part, but in that case it would be 
probably OK.

> Therefore Fixes tag in general is applicable. Of course maybe to this
> one not really, maybe this is too trivial, or whatever, so I do not
> insist. But I insist on the principle - reasonable dtbs_check warnings
> are like compiler warnings - bugs which have to be fixed.
> 

I'm not arguing that these things shouldn't be fixed, but that they are worth 
being back-ported to the stable branches.

Regards,
Matthias

[PATCH v1 03/16] dt-bindings: power: mediatek: Refine multiple level power domain nodes

[Tinghan Shen]

Extract duplicated properties and support more levels of power
domain nodes.

This change fix following error when do dtbs_check,
    arch/arm64/boot/dts/mediatek/mt8195-evb.dtb: power-controller: power-domain@15:power-domain@16:power-domain@18: 'power-domain@19', 'power-domain@20', 'power-domain@21' do not match any of the regexes: 'pinctrl-[0-9]+'
	 From schema: Documentation/devicetree/bindings/power/mediatek,power-controller.yaml

Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>
---
 .../power/mediatek,power-controller.yaml      | 132 ++----------------
 1 file changed, 12 insertions(+), 120 deletions(-)

diff --git a/Documentation/devicetree/bindings/power/mediatek,power-controller.yaml b/Documentation/devicetree/bindings/power/mediatek,power-controller.yaml
index 135c6f722091..09a537a802b8 100644
--- a/Documentation/devicetree/bindings/power/mediatek,power-controller.yaml
+++ b/Documentation/devicetree/bindings/power/mediatek,power-controller.yaml
@@ -39,8 +39,17 @@ properties:
   '#size-cells':
     const: 0
 
+required:
+  - compatible
+
+additionalProperties: false
+
 patternProperties:
   "^power-domain@[0-9a-f]+$":
+    $ref: "#/$defs/power-domain-node"
+
+$defs:
+  power-domain-node:
     type: object
     description: |
       Represents the power domains within the power controller node as documented
@@ -98,127 +107,10 @@ patternProperties:
         $ref: /schemas/types.yaml#/definitions/phandle
         description: phandle to the device containing the SMI register range.
 
-    patternProperties:
-      "^power-domain@[0-9a-f]+$":
-        type: object
-        description: |
-          Represents a power domain child within a power domain parent node.
-
-        properties:
-
-          '#power-domain-cells':
-            description:
-              Must be 0 for nodes representing a single PM domain and 1 for nodes
-              providing multiple PM domains.
-
-          '#address-cells':
-            const: 1
-
-          '#size-cells':
-            const: 0
-
-          reg:
-            maxItems: 1
-
-          clocks:
-            description: |
-              A number of phandles to clocks that need to be enabled during domain
-              power-up sequencing.
-
-          clock-names:
-            description: |
-              List of names of clocks, in order to match the power-up sequencing
-              for each power domain we need to group the clocks by name. BASIC
-              clocks need to be enabled before enabling the corresponding power
-              domain, and should not have a '-' in their name (i.e mm, mfg, venc).
-              SUSBYS clocks need to be enabled before releasing the bus protection,
-              and should contain a '-' in their name (i.e mm-0, isp-0, cam-0).
-
-              In order to follow properly the power-up sequencing, the clocks must
-              be specified by order, adding first the BASIC clocks followed by the
-              SUSBSYS clocks.
-
-          domain-supply:
-            description: domain regulator supply.
-
-          mediatek,infracfg:
-            $ref: /schemas/types.yaml#/definitions/phandle
-            description: phandle to the device containing the INFRACFG register range.
-
-          mediatek,smi:
-            $ref: /schemas/types.yaml#/definitions/phandle
-            description: phandle to the device containing the SMI register range.
-
-        patternProperties:
-          "^power-domain@[0-9a-f]+$":
-            type: object
-            description: |
-              Represents a power domain child within a power domain parent node.
-
-            properties:
+      required:
+        - reg
 
-              '#power-domain-cells':
-                description:
-                  Must be 0 for nodes representing a single PM domain and 1 for nodes
-                  providing multiple PM domains.
-
-              '#address-cells':
-                const: 1
-
-              '#size-cells':
-                const: 0
-
-              reg:
-                maxItems: 1
-
-              clocks:
-                description: |
-                  A number of phandles to clocks that need to be enabled during domain
-                  power-up sequencing.
-
-              clock-names:
-                description: |
-                  List of names of clocks, in order to match the power-up sequencing
-                  for each power domain we need to group the clocks by name. BASIC
-                  clocks need to be enabled before enabling the corresponding power
-                  domain, and should not have a '-' in their name (i.e mm, mfg, venc).
-                  SUSBYS clocks need to be enabled before releasing the bus protection,
-                  and should contain a '-' in their name (i.e mm-0, isp-0, cam-0).
-
-                  In order to follow properly the power-up sequencing, the clocks must
-                  be specified by order, adding first the BASIC clocks followed by the
-                  SUSBSYS clocks.
-
-              domain-supply:
-                description: domain regulator supply.
-
-              mediatek,infracfg:
-                $ref: /schemas/types.yaml#/definitions/phandle
-                description: phandle to the device containing the INFRACFG register range.
-
-              mediatek,smi:
-                $ref: /schemas/types.yaml#/definitions/phandle
-                description: phandle to the device containing the SMI register range.
-
-            required:
-              - reg
-
-            additionalProperties: false
-
-        required:
-          - reg
-
-        additionalProperties: false
-
-    required:
-      - reg
-
-    additionalProperties: false
-
-required:
-  - compatible
-
-additionalProperties: false
+      additionalProperties: false
 
 examples:
   - |
-- 
2.18.0

Re: [PATCH v1 03/16] dt-bindings: power: mediatek: Refine multiple level power domain nodes

[Rob Herring]

On Mon, Jul 04, 2022 at 06:00:15PM +0800, Tinghan Shen wrote:
> Extract duplicated properties and support more levels of power
> domain nodes.
> 
> This change fix following error when do dtbs_check,
>     arch/arm64/boot/dts/mediatek/mt8195-evb.dtb: power-controller: power-domain@15:power-domain@16:power-domain@18: 'power-domain@19', 'power-domain@20', 'power-domain@21' do not match any of the regexes: 'pinctrl-[0-9]+'
> 	 From schema: Documentation/devicetree/bindings/power/mediatek,power-controller.yaml
> 
> Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>
> ---
>  .../power/mediatek,power-controller.yaml      | 132 ++----------------
>  1 file changed, 12 insertions(+), 120 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/power/mediatek,power-controller.yaml b/Documentation/devicetree/bindings/power/mediatek,power-controller.yaml
> index 135c6f722091..09a537a802b8 100644
> --- a/Documentation/devicetree/bindings/power/mediatek,power-controller.yaml
> +++ b/Documentation/devicetree/bindings/power/mediatek,power-controller.yaml
> @@ -39,8 +39,17 @@ properties:
>    '#size-cells':
>      const: 0
>  
> +required:
> +  - compatible
> +
> +additionalProperties: false
> +
>  patternProperties:
>    "^power-domain@[0-9a-f]+$":
> +    $ref: "#/$defs/power-domain-node"
> +
> +$defs:
> +  power-domain-node:
>      type: object
>      description: |
>        Represents the power domains within the power controller node as documented
> @@ -98,127 +107,10 @@ patternProperties:
>          $ref: /schemas/types.yaml#/definitions/phandle
>          description: phandle to the device containing the SMI register range.
>  
> -    patternProperties:
> -      "^power-domain@[0-9a-f]+$":
> -        type: object
> -        description: |
> -          Represents a power domain child within a power domain parent node.
> -
> -        properties:
> -
> -          '#power-domain-cells':
> -            description:
> -              Must be 0 for nodes representing a single PM domain and 1 for nodes
> -              providing multiple PM domains.
> -
> -          '#address-cells':
> -            const: 1
> -
> -          '#size-cells':
> -            const: 0
> -
> -          reg:
> -            maxItems: 1
> -
> -          clocks:
> -            description: |
> -              A number of phandles to clocks that need to be enabled during domain
> -              power-up sequencing.
> -
> -          clock-names:
> -            description: |
> -              List of names of clocks, in order to match the power-up sequencing
> -              for each power domain we need to group the clocks by name. BASIC
> -              clocks need to be enabled before enabling the corresponding power
> -              domain, and should not have a '-' in their name (i.e mm, mfg, venc).
> -              SUSBYS clocks need to be enabled before releasing the bus protection,
> -              and should contain a '-' in their name (i.e mm-0, isp-0, cam-0).
> -
> -              In order to follow properly the power-up sequencing, the clocks must
> -              be specified by order, adding first the BASIC clocks followed by the
> -              SUSBSYS clocks.
> -
> -          domain-supply:
> -            description: domain regulator supply.
> -
> -          mediatek,infracfg:
> -            $ref: /schemas/types.yaml#/definitions/phandle
> -            description: phandle to the device containing the INFRACFG register range.
> -
> -          mediatek,smi:
> -            $ref: /schemas/types.yaml#/definitions/phandle
> -            description: phandle to the device containing the SMI register range.
> -
> -        patternProperties:
> -          "^power-domain@[0-9a-f]+$":
> -            type: object
> -            description: |
> -              Represents a power domain child within a power domain parent node.
> -
> -            properties:
> +      required:
> +        - reg
>  
> -              '#power-domain-cells':
> -                description:
> -                  Must be 0 for nodes representing a single PM domain and 1 for nodes
> -                  providing multiple PM domains.
> -
> -              '#address-cells':
> -                const: 1
> -
> -              '#size-cells':
> -                const: 0
> -
> -              reg:
> -                maxItems: 1
> -
> -              clocks:
> -                description: |
> -                  A number of phandles to clocks that need to be enabled during domain
> -                  power-up sequencing.
> -
> -              clock-names:
> -                description: |
> -                  List of names of clocks, in order to match the power-up sequencing
> -                  for each power domain we need to group the clocks by name. BASIC
> -                  clocks need to be enabled before enabling the corresponding power
> -                  domain, and should not have a '-' in their name (i.e mm, mfg, venc).
> -                  SUSBYS clocks need to be enabled before releasing the bus protection,
> -                  and should contain a '-' in their name (i.e mm-0, isp-0, cam-0).
> -
> -                  In order to follow properly the power-up sequencing, the clocks must
> -                  be specified by order, adding first the BASIC clocks followed by the
> -                  SUSBSYS clocks.
> -
> -              domain-supply:
> -                description: domain regulator supply.
> -
> -              mediatek,infracfg:
> -                $ref: /schemas/types.yaml#/definitions/phandle
> -                description: phandle to the device containing the INFRACFG register range.
> -
> -              mediatek,smi:
> -                $ref: /schemas/types.yaml#/definitions/phandle
> -                description: phandle to the device containing the SMI register range.
> -
> -            required:
> -              - reg
> -
> -            additionalProperties: false
> -
> -        required:
> -          - reg
> -
> -        additionalProperties: false
> -
> -    required:
> -      - reg
> -
> -    additionalProperties: false
> -
> -required:
> -  - compatible
> -
> -additionalProperties: false
> +      additionalProperties: false

You now aren't checking more than 1 level because you have defined 
'additionalProperties' to be a DT property. Check the indentation.

You need this in $defs/power-domain-node to recurse:

    additionalProperties:
      $ref: #/$defs/power-domain-node

>  
>  examples:
>    - |
> -- 
> 2.18.0
> 
> 

Re: [PATCH v1 03/16] dt-bindings: power: mediatek: Refine multiple level power domain nodes

[Tinghan Shen]

On Tue, 2022-07-05 at 14:57 -0600, Rob Herring wrote:
> On Mon, Jul 04, 2022 at 06:00:15PM +0800, Tinghan Shen wrote:
> > Extract duplicated properties and support more levels of power
> > domain nodes.
> > 
> > This change fix following error when do dtbs_check,
> >     arch/arm64/boot/dts/mediatek/mt8195-evb.dtb: power-controller: power-domain@15:
> > power-domain@16:power-domain@18: 'power-domain@19', 'power-domain@20', 'power-domain@21' do not
> > match any of the regexes: 'pinctrl-[0-9]+'
> > 	 From schema: Documentation/devicetree/bindings/power/mediatek,power-controller.yaml
> > 
> > Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>
> > ---
> >  .../power/mediatek,power-controller.yaml      | 132 ++----------------
> >  1 file changed, 12 insertions(+), 120 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/power/mediatek,power-controller.yaml
> > b/Documentation/devicetree/bindings/power/mediatek,power-controller.yaml
> > index 135c6f722091..09a537a802b8 100644
> > --- a/Documentation/devicetree/bindings/power/mediatek,power-controller.yaml
> > +++ b/Documentation/devicetree/bindings/power/mediatek,power-controller.yaml
> > @@ -39,8 +39,17 @@ properties:
> >    '#size-cells':
> >      const: 0
> >  
> > +required:
> > +  - compatible
> > +
> > +additionalProperties: false
> > +
> >  patternProperties:
> >    "^power-domain@[0-9a-f]+$":
> > +    $ref: "#/$defs/power-domain-node"
> > +
> > +$defs:
> > +  power-domain-node:
> >      type: object
> >      description: |
> >        Represents the power domains within the power controller node as documented
> > @@ -98,127 +107,10 @@ patternProperties:
> >          $ref: /schemas/types.yaml#/definitions/phandle
> >          description: phandle to the device containing the SMI register range.
> >  
> > -    patternProperties:
> > -      "^power-domain@[0-9a-f]+$":
> > -        type: object
> > -        description: |
> > -          Represents a power domain child within a power domain parent node.
> > -
> > -        properties:
> > -
> > -          '#power-domain-cells':
> > -            description:
> > -              Must be 0 for nodes representing a single PM domain and 1 for nodes
> > -              providing multiple PM domains.
> > -
> > -          '#address-cells':
> > -            const: 1
> > -
> > -          '#size-cells':
> > -            const: 0
> > -
> > -          reg:
> > -            maxItems: 1
> > -
> > -          clocks:
> > -            description: |
> > -              A number of phandles to clocks that need to be enabled during domain
> > -              power-up sequencing.
> > -
> > -          clock-names:
> > -            description: |
> > -              List of names of clocks, in order to match the power-up sequencing
> > -              for each power domain we need to group the clocks by name. BASIC
> > -              clocks need to be enabled before enabling the corresponding power
> > -              domain, and should not have a '-' in their name (i.e mm, mfg, venc).
> > -              SUSBYS clocks need to be enabled before releasing the bus protection,
> > -              and should contain a '-' in their name (i.e mm-0, isp-0, cam-0).
> > -
> > -              In order to follow properly the power-up sequencing, the clocks must
> > -              be specified by order, adding first the BASIC clocks followed by the
> > -              SUSBSYS clocks.
> > -
> > -          domain-supply:
> > -            description: domain regulator supply.
> > -
> > -          mediatek,infracfg:
> > -            $ref: /schemas/types.yaml#/definitions/phandle
> > -            description: phandle to the device containing the INFRACFG register range.
> > -
> > -          mediatek,smi:
> > -            $ref: /schemas/types.yaml#/definitions/phandle
> > -            description: phandle to the device containing the SMI register range.
> > -
> > -        patternProperties:
> > -          "^power-domain@[0-9a-f]+$":
> > -            type: object
> > -            description: |
> > -              Represents a power domain child within a power domain parent node.
> > -
> > -            properties:
> > +      required:
> > +        - reg
> >  
> > -              '#power-domain-cells':
> > -                description:
> > -                  Must be 0 for nodes representing a single PM domain and 1 for nodes
> > -                  providing multiple PM domains.
> > -
> > -              '#address-cells':
> > -                const: 1
> > -
> > -              '#size-cells':
> > -                const: 0
> > -
> > -              reg:
> > -                maxItems: 1
> > -
> > -              clocks:
> > -                description: |
> > -                  A number of phandles to clocks that need to be enabled during domain
> > -                  power-up sequencing.
> > -
> > -              clock-names:
> > -                description: |
> > -                  List of names of clocks, in order to match the power-up sequencing
> > -                  for each power domain we need to group the clocks by name. BASIC
> > -                  clocks need to be enabled before enabling the corresponding power
> > -                  domain, and should not have a '-' in their name (i.e mm, mfg, venc).
> > -                  SUSBYS clocks need to be enabled before releasing the bus protection,
> > -                  and should contain a '-' in their name (i.e mm-0, isp-0, cam-0).
> > -
> > -                  In order to follow properly the power-up sequencing, the clocks must
> > -                  be specified by order, adding first the BASIC clocks followed by the
> > -                  SUSBSYS clocks.
> > -
> > -              domain-supply:
> > -                description: domain regulator supply.
> > -
> > -              mediatek,infracfg:
> > -                $ref: /schemas/types.yaml#/definitions/phandle
> > -                description: phandle to the device containing the INFRACFG register range.
> > -
> > -              mediatek,smi:
> > -                $ref: /schemas/types.yaml#/definitions/phandle
> > -                description: phandle to the device containing the SMI register range.
> > -
> > -            required:
> > -              - reg
> > -
> > -            additionalProperties: false
> > -
> > -        required:
> > -          - reg
> > -
> > -        additionalProperties: false
> > -
> > -    required:
> > -      - reg
> > -
> > -    additionalProperties: false
> > -
> > -required:
> > -  - compatible
> > -
> > -additionalProperties: false
> > +      additionalProperties: false
> $ make dt_binding_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/power/mediatek,power-
> controller.yaml O=out
> make[1]: Entering directory '/proj/mtk15399/upstream-cros/src/third_party/kernel/v5.10/out'
>   LINT    Documentation/devicetree/bindings
>   CHKDT   Documentation/devicetree/bindings/processed-schema.json
>   SCHEMA  Documentation/devicetree/bindings/processed-schema.json
> /proj/mtk15399/upstream-
> cros/src/third_party/kernel/v5.10/Documentation/devicetree/bindings/iio/temperature/adi,ltc2983.ya
> ml: ignoring, error in schema: patternProperties: ^thermistor@: properties: adi,excitation-
> current-nanoamp
> /proj/mtk15399/upstream-
> cros/src/third_party/kernel/v5.10/Documentation/devicetree/bindings/display/tegra/nvidia,tegra124-
> sor.yaml: ignoring, error in schema: allOf: 1: if: not: properties
> /proj/mtk15399/upstream-
> cros/src/third_party/kernel/v5.10/Documentation/devicetree/bindings/mfd/mps,mp2629.yaml: ignoring,
> error parsing file
>   DTEX    Documentation/devicetree/bindings/power/mediatek,power-controller.example.dts
>   DTC     Documentation/devicetree/bindings/power/mediatek,power-controller.example.dtb
>   CHECK   Documentation/devicetree/bindings/power/mediatek,power-controller.example.dtb
> Traceback (most recent call last):
>   File "/proj/mtk15399/.venv/py3.9/bin/dt-validate", line 173, in <module>
>     testtree = dtschema.load(filename, line_number=args.line_number)
>   File "/proj/mtk15399/.venv/py3.9/lib/python3.9/site-packages/dtschema/lib.py", line 913, in load
>     return [ dtschema.dtb.fdt_unflatten(f.read()) ]
>   File "/proj/mtk15399/.venv/py3.9/lib/python3.9/site-packages/dtschema/dtb.py", line 463, in
> fdt_unflatten
>     p = dtschema.get_prop_types()
>   File "/proj/mtk15399/.venv/py3.9/lib/python3.9/site-packages/dtschema/lib.py", line 892, in
> get_prop_types
>     props = dtschema.extract_types(schema_cache)
>   File "/proj/mtk15399/.venv/py3.9/lib/python3.9/site-packages/dtschema/lib.py", line 883, in
> extract_types
>     _extract_subschema_types(props, sch, sch)
>   File "/proj/mtk15399/.venv/py3.9/lib/python3.9/site-packages/dtschema/lib.py", line 874, in
> _extract_subschema_types
> 
> [...snip...]
> 
>   File "/proj/mtk15399/.venv/py3.9/lib/python3.9/site-packages/dtschema/lib.py", line 803, in
> _extract_prop_type
>     _extract_prop_type(props, schema, propname, subschema)
>   File "/proj/mtk15399/.venv/py3.9/lib/python3.9/site-packages/dtschema/lib.py", line 794, in
> _extract_prop_type
>     _extract_subschema_types(props, schema, subschema)
>   File "/proj/mtk15399/.venv/py3.9/lib/python3.9/site-packages/dtschema/lib.py", line 874, in
> _extract_subschema_types
>     _extract_prop_type(props, schema, p, v)
>   File "/proj/mtk15399/.venv/py3.9/lib/python3.9/site-packages/dtschema/lib.py", line 790, in
> _extract_prop_type
>     if not isinstance(subschema, dict):
> RecursionError: maximum recursion depth exceeded while calling a Python object
> make[1]: Leaving directory '/proj/mtk15399/upstream-cros/src/third_party/kernel/v5.10/out'
> You now aren't checking more than 1 level because you have defined 
> 'additionalProperties' to be a DT property. Check the indentation.
> 
> You need this in $defs/power-domain-node to recurse:
> 
>     additionalProperties:
>       $ref: #/$defs/power-domain-node
Hi Rob,

I get the following error after adding the 'additionalProperties' to $defs/power-domain-node.
The same error occurs when I run dt_binding_check on power/renesas,sysc-rmobile.yaml, which has the
similar property.

$ make dt_binding_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/power/mediatek,power-
controller.yaml O=out
make[1]: Entering directory '/test/upstream-cros/src/third_party/kernel/v5.10/out'
  LINT    Documentation/devicetree/bindings
  CHKDT   Documentation/devicetree/bindings/processed-schema.json
  SCHEMA  Documentation/devicetree/bindings/processed-schema.json
/test/upstream-
cros/src/third_party/kernel/v5.10/Documentation/devicetree/bindings/iio/temperature/adi,ltc2983.yaml
: ignoring, error in schema: patternProperties: ^thermistor@: properties: adi,excitation-current-
nanoamp
/test/upstream-
cros/src/third_party/kernel/v5.10/Documentation/devicetree/bindings/display/tegra/nvidia,tegra124-
sor.yaml: ignoring, error in schema: allOf: 1: if: not: properties
/test/upstream-
cros/src/third_party/kernel/v5.10/Documentation/devicetree/bindings/mfd/mps,mp2629.yaml: ignoring,
error parsing file
  DTEX    Documentation/devicetree/bindings/power/mediatek,power-controller.example.dts
  DTC     Documentation/devicetree/bindings/power/mediatek,power-controller.example.dtb
  CHECK   Documentation/devicetree/bindings/power/mediatek,power-controller.example.dtb
Traceback (most recent call last):
  File "/test/.venv/py3.9/bin/dt-validate", line 173, in <module>
    testtree = dtschema.load(filename, line_number=args.line_number)
  File "/test/.venv/py3.9/lib/python3.9/site-packages/dtschema/lib.py", line 913, in load
    return [ dtschema.dtb.fdt_unflatten(f.read()) ]
  File "/test/.venv/py3.9/lib/python3.9/site-packages/dtschema/dtb.py", line 463, in fdt_unflatten
    p = dtschema.get_prop_types()
  File "/test/.venv/py3.9/lib/python3.9/site-packages/dtschema/lib.py", line 892, in get_prop_types
    props = dtschema.extract_types(schema_cache)
  File "/test/.venv/py3.9/lib/python3.9/site-packages/dtschema/lib.py", line 883, in extract_types
    _extract_subschema_types(props, sch, sch)
  File "/test/.venv/py3.9/lib/python3.9/site-packages/dtschema/lib.py", line 874, in
_extract_subschema_types

[...snip...]

  File "/test/.venv/py3.9/lib/python3.9/site-packages/dtschema/lib.py", line 803, in
_extract_prop_type
    _extract_prop_type(props, schema, propname, subschema)
  File "/test/.venv/py3.9/lib/python3.9/site-packages/dtschema/lib.py", line 794, in
_extract_prop_type
    _extract_subschema_types(props, schema, subschema)
  File "/test/.venv/py3.9/lib/python3.9/site-packages/dtschema/lib.py", line 874, in
_extract_subschema_types
    _extract_prop_type(props, schema, p, v)
  File "/test/.venv/py3.9/lib/python3.9/site-packages/dtschema/lib.py", line 790, in
_extract_prop_type
    if not isinstance(subschema, dict):
RecursionError: maximum recursion depth exceeded while calling a Python object
make[1]: Leaving directory '/test/upstream-cros/src/third_party/kernel/v5.10/out'


Best Regards,
TingHan

Re: [PATCH v1 03/16] dt-bindings: power: mediatek: Refine multiple level power domain nodes

[Rob Herring]

On Wed, Jul 06, 2022 at 02:19:08PM +0800, Tinghan Shen wrote:
> On Tue, 2022-07-05 at 14:57 -0600, Rob Herring wrote:
> > On Mon, Jul 04, 2022 at 06:00:15PM +0800, Tinghan Shen wrote:
> > > Extract duplicated properties and support more levels of power
> > > domain nodes.
> > > 
> > > This change fix following error when do dtbs_check,
> > >     arch/arm64/boot/dts/mediatek/mt8195-evb.dtb: power-controller: power-domain@15:
> > > power-domain@16:power-domain@18: 'power-domain@19', 'power-domain@20', 'power-domain@21' do not
> > > match any of the regexes: 'pinctrl-[0-9]+'
> > > 	 From schema: Documentation/devicetree/bindings/power/mediatek,power-controller.yaml
> > > 
> > > Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>
> > > ---
> > >  .../power/mediatek,power-controller.yaml      | 132 ++----------------
> > >  1 file changed, 12 insertions(+), 120 deletions(-)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/power/mediatek,power-controller.yaml
> > > b/Documentation/devicetree/bindings/power/mediatek,power-controller.yaml
> > > index 135c6f722091..09a537a802b8 100644
> > > --- a/Documentation/devicetree/bindings/power/mediatek,power-controller.yaml
> > > +++ b/Documentation/devicetree/bindings/power/mediatek,power-controller.yaml
> > > @@ -39,8 +39,17 @@ properties:
> > >    '#size-cells':
> > >      const: 0
> > >  
> > > +required:
> > > +  - compatible
> > > +
> > > +additionalProperties: false
> > > +
> > >  patternProperties:
> > >    "^power-domain@[0-9a-f]+$":
> > > +    $ref: "#/$defs/power-domain-node"
> > > +
> > > +$defs:
> > > +  power-domain-node:
> > >      type: object
> > >      description: |
> > >        Represents the power domains within the power controller node as documented
> > > @@ -98,127 +107,10 @@ patternProperties:
> > >          $ref: /schemas/types.yaml#/definitions/phandle
> > >          description: phandle to the device containing the SMI register range.
> > >  
> > > -    patternProperties:
> > > -      "^power-domain@[0-9a-f]+$":
> > > -        type: object
> > > -        description: |
> > > -          Represents a power domain child within a power domain parent node.
> > > -
> > > -        properties:
> > > -
> > > -          '#power-domain-cells':
> > > -            description:
> > > -              Must be 0 for nodes representing a single PM domain and 1 for nodes
> > > -              providing multiple PM domains.
> > > -
> > > -          '#address-cells':
> > > -            const: 1
> > > -
> > > -          '#size-cells':
> > > -            const: 0
> > > -
> > > -          reg:
> > > -            maxItems: 1
> > > -
> > > -          clocks:
> > > -            description: |
> > > -              A number of phandles to clocks that need to be enabled during domain
> > > -              power-up sequencing.
> > > -
> > > -          clock-names:
> > > -            description: |
> > > -              List of names of clocks, in order to match the power-up sequencing
> > > -              for each power domain we need to group the clocks by name. BASIC
> > > -              clocks need to be enabled before enabling the corresponding power
> > > -              domain, and should not have a '-' in their name (i.e mm, mfg, venc).
> > > -              SUSBYS clocks need to be enabled before releasing the bus protection,
> > > -              and should contain a '-' in their name (i.e mm-0, isp-0, cam-0).
> > > -
> > > -              In order to follow properly the power-up sequencing, the clocks must
> > > -              be specified by order, adding first the BASIC clocks followed by the
> > > -              SUSBSYS clocks.
> > > -
> > > -          domain-supply:
> > > -            description: domain regulator supply.
> > > -
> > > -          mediatek,infracfg:
> > > -            $ref: /schemas/types.yaml#/definitions/phandle
> > > -            description: phandle to the device containing the INFRACFG register range.
> > > -
> > > -          mediatek,smi:
> > > -            $ref: /schemas/types.yaml#/definitions/phandle
> > > -            description: phandle to the device containing the SMI register range.
> > > -
> > > -        patternProperties:
> > > -          "^power-domain@[0-9a-f]+$":
> > > -            type: object
> > > -            description: |
> > > -              Represents a power domain child within a power domain parent node.
> > > -
> > > -            properties:
> > > +      required:
> > > +        - reg
> > >  
> > > -              '#power-domain-cells':
> > > -                description:
> > > -                  Must be 0 for nodes representing a single PM domain and 1 for nodes
> > > -                  providing multiple PM domains.
> > > -
> > > -              '#address-cells':
> > > -                const: 1
> > > -
> > > -              '#size-cells':
> > > -                const: 0
> > > -
> > > -              reg:
> > > -                maxItems: 1
> > > -
> > > -              clocks:
> > > -                description: |
> > > -                  A number of phandles to clocks that need to be enabled during domain
> > > -                  power-up sequencing.
> > > -
> > > -              clock-names:
> > > -                description: |
> > > -                  List of names of clocks, in order to match the power-up sequencing
> > > -                  for each power domain we need to group the clocks by name. BASIC
> > > -                  clocks need to be enabled before enabling the corresponding power
> > > -                  domain, and should not have a '-' in their name (i.e mm, mfg, venc).
> > > -                  SUSBYS clocks need to be enabled before releasing the bus protection,
> > > -                  and should contain a '-' in their name (i.e mm-0, isp-0, cam-0).
> > > -
> > > -                  In order to follow properly the power-up sequencing, the clocks must
> > > -                  be specified by order, adding first the BASIC clocks followed by the
> > > -                  SUSBSYS clocks.
> > > -
> > > -              domain-supply:
> > > -                description: domain regulator supply.
> > > -
> > > -              mediatek,infracfg:
> > > -                $ref: /schemas/types.yaml#/definitions/phandle
> > > -                description: phandle to the device containing the INFRACFG register range.
> > > -
> > > -              mediatek,smi:
> > > -                $ref: /schemas/types.yaml#/definitions/phandle
> > > -                description: phandle to the device containing the SMI register range.
> > > -
> > > -            required:
> > > -              - reg
> > > -
> > > -            additionalProperties: false
> > > -
> > > -        required:
> > > -          - reg
> > > -
> > > -        additionalProperties: false
> > > -
> > > -    required:
> > > -      - reg
> > > -
> > > -    additionalProperties: false
> > > -
> > > -required:
> > > -  - compatible
> > > -
> > > -additionalProperties: false
> > > +      additionalProperties: false
> > You now aren't checking more than 1 level because you have defined 
> > 'additionalProperties' to be a DT property. Check the indentation.
> > 
> > You need this in $defs/power-domain-node to recurse:
> > 
> >     additionalProperties:
> >       $ref: #/$defs/power-domain-node
> Hi Rob,
> 
> I get the following error after adding the 'additionalProperties' to $defs/power-domain-node.
> The same error occurs when I run dt_binding_check on power/renesas,sysc-rmobile.yaml, which has the
> similar property.
> 
> $ make dt_binding_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/power/mediatek,power-
> controller.yaml O=out
> make[1]: Entering directory '/test/upstream-cros/src/third_party/kernel/v5.10/out'
>   LINT    Documentation/devicetree/bindings
>   CHKDT   Documentation/devicetree/bindings/processed-schema.json
>   SCHEMA  Documentation/devicetree/bindings/processed-schema.json
> /test/upstream-
> cros/src/third_party/kernel/v5.10/Documentation/devicetree/bindings/iio/temperature/adi,ltc2983.yaml
> : ignoring, error in schema: patternProperties: ^thermistor@: properties: adi,excitation-current-
> nanoamp
> /test/upstream-
> cros/src/third_party/kernel/v5.10/Documentation/devicetree/bindings/display/tegra/nvidia,tegra124-
> sor.yaml: ignoring, error in schema: allOf: 1: if: not: properties
> /test/upstream-
> cros/src/third_party/kernel/v5.10/Documentation/devicetree/bindings/mfd/mps,mp2629.yaml: ignoring,
> error parsing file
>   DTEX    Documentation/devicetree/bindings/power/mediatek,power-controller.example.dts
>   DTC     Documentation/devicetree/bindings/power/mediatek,power-controller.example.dtb
>   CHECK   Documentation/devicetree/bindings/power/mediatek,power-controller.example.dtb
> Traceback (most recent call last):
>   File "/test/.venv/py3.9/bin/dt-validate", line 173, in <module>
>     testtree = dtschema.load(filename, line_number=args.line_number)
>   File "/test/.venv/py3.9/lib/python3.9/site-packages/dtschema/lib.py", line 913, in load
>     return [ dtschema.dtb.fdt_unflatten(f.read()) ]
>   File "/test/.venv/py3.9/lib/python3.9/site-packages/dtschema/dtb.py", line 463, in fdt_unflatten
>     p = dtschema.get_prop_types()
>   File "/test/.venv/py3.9/lib/python3.9/site-packages/dtschema/lib.py", line 892, in get_prop_types
>     props = dtschema.extract_types(schema_cache)
>   File "/test/.venv/py3.9/lib/python3.9/site-packages/dtschema/lib.py", line 883, in extract_types
>     _extract_subschema_types(props, sch, sch)
>   File "/test/.venv/py3.9/lib/python3.9/site-packages/dtschema/lib.py", line 874, in
> _extract_subschema_types
> 
> [...snip...]
> 
>   File "/test/.venv/py3.9/lib/python3.9/site-packages/dtschema/lib.py", line 803, in
> _extract_prop_type
>     _extract_prop_type(props, schema, propname, subschema)
>   File "/test/.venv/py3.9/lib/python3.9/site-packages/dtschema/lib.py", line 794, in
> _extract_prop_type
>     _extract_subschema_types(props, schema, subschema)
>   File "/test/.venv/py3.9/lib/python3.9/site-packages/dtschema/lib.py", line 874, in
> _extract_subschema_types
>     _extract_prop_type(props, schema, p, v)
>   File "/test/.venv/py3.9/lib/python3.9/site-packages/dtschema/lib.py", line 790, in
> _extract_prop_type
>     if not isinstance(subschema, dict):
> RecursionError: maximum recursion depth exceeded while calling a Python object
> make[1]: Leaving directory '/test/upstream-cros/src/third_party/kernel/v5.10/out'

Okay, I think you need something more like this that doesn't recurse 
infinitely:

patternProperties:
  "^power-domain@[0-9a-f]+$":
    $ref: #/$defs/power-domain-node

    unevaluatedProperties:
      $ref: #/$defs/power-domain-node

If you need a 3rd level of nodes:
      unevaluatedProperties:
        $ref: #/$defs/power-domain-node


Rob

Re: [PATCH v1 03/16] dt-bindings: power: mediatek: Refine multiple level power domain nodes

[Tinghan Shen]

Hi Rob,
> 
> Okay, I think you need something more like this that doesn't recurse 
> infinitely:
> 
> patternProperties:
>   "^power-domain@[0-9a-f]+$":
>     $ref: #/$defs/power-domain-node
> 
>     unevaluatedProperties:
>       $ref: #/$defs/power-domain-node
> 
> If you need a 3rd level of nodes:
>       unevaluatedProperties:
>         $ref: #/$defs/power-domain-node
> 
> 
> Rob

After some test, your 1st suggestion works.

The infinite error is introduced from my changes and affect the result of power/renesas,sysc-
rmobile.yaml. The 'additionalProperties' being defined as a DT property is the root of this error.
After fix the indentation, the error is gone.

I'll update the yaml as your 1st suggestion in next version.

Thanks,
TingHan

[PATCH v1 04/16] arm64: dts: mt8195: Disable watchdog external reset signal

[Tinghan Shen]

Disable external output reset signal in first round of watchdog reset
to reserve wdt reset reason for debugging watchdog issue.

Signed-off-by: Fengquan Chen <fengquan.chen@mediatek.com>
Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>
---
 arch/arm64/boot/dts/mediatek/mt8195.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt8195.dtsi b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
index 066c14989708..436687ba826f 100644
--- a/arch/arm64/boot/dts/mediatek/mt8195.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
@@ -327,6 +327,7 @@
 		watchdog: watchdog@10007000 {
 			compatible = "mediatek,mt8195-wdt",
 				     "mediatek,mt6589-wdt";
+			mediatek,disable-extrst;
 			reg = <0 0x10007000 0 0x100>;
 		};
 
-- 
2.18.0

Re: [PATCH v1 04/16] arm64: dts: mt8195: Disable watchdog external reset signal

[AngeloGioacchino Del Regno]

Il 04/07/22 12:00, Tinghan Shen ha scritto:
> Disable external output reset signal in first round of watchdog reset
> to reserve wdt reset reason for debugging watchdog issue.

If my understanding of the commit decription is right, then we can clarify
that with something like: "[...] for debugging eventual watchdog issues".

Otherwise, if this implies that disable-extrst is needed to avoid losing
the reset reason stored in the WDT, you could say something like:

"Disable external output reset signal in the first round of watchdog reset
to avoid losing the reset reason stored in the watchdog registers"

After which:

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

> 
> Signed-off-by: Fengquan Chen <fengquan.chen@mediatek.com>
> Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>
> ---
>   arch/arm64/boot/dts/mediatek/mt8195.dtsi | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm64/boot/dts/mediatek/mt8195.dtsi b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
> index 066c14989708..436687ba826f 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8195.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
> @@ -327,6 +327,7 @@
>   		watchdog: watchdog@10007000 {
>   			compatible = "mediatek,mt8195-wdt",
>   				     "mediatek,mt6589-wdt";
> +			mediatek,disable-extrst;
>   			reg = <0 0x10007000 0 0x100>;
>   		};
>   

Re: [PATCH v1 04/16] arm64: dts: mt8195: Disable watchdog external reset signal

[Tinghan Shen]

On Mon, 2022-07-04 at 12:30 +0200, AngeloGioacchino Del Regno wrote:
> Il 04/07/22 12:00, Tinghan Shen ha scritto:
> > Disable external output reset signal in first round of watchdog reset
> > to reserve wdt reset reason for debugging watchdog issue.
> 
> If my understanding of the commit decription is right, then we can clarify
> that with something like: "[...] for debugging eventual watchdog issues".
> 
> Otherwise, if this implies that disable-extrst is needed to avoid losing
> the reset reason stored in the WDT, you could say something like:
> 
> "Disable external output reset signal in the first round of watchdog reset
> to avoid losing the reset reason stored in the watchdog registers"
> 
> After which:
> 
> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> 

Ok, I'll update it in next version.

Thanks,
TingHan

[PATCH v1 05/16] arm64: dts: mt8195: Disable I2C0 node

[Tinghan Shen]

From: Tzung-Bi Shih <tzungbi@chromium.org>

The I2C0 node doesn't need to be enabled in dtsi.

Signed-off-by: Tzung-Bi Shih <tzungbi@chromium.org>
Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>
---
 arch/arm64/boot/dts/mediatek/mt8195.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/mediatek/mt8195.dtsi b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
index 436687ba826f..8032b839dfe8 100644
--- a/arch/arm64/boot/dts/mediatek/mt8195.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
@@ -829,7 +829,7 @@
 			clock-names = "main", "dma";
 			#address-cells = <1>;
 			#size-cells = <0>;
-			status = "okay";
+			status = "disabled";
 		};
 
 		i2c1: i2c@11e01000 {
-- 
2.18.0

Re: [PATCH v1 05/16] arm64: dts: mt8195: Disable I2C0 node

[AngeloGioacchino Del Regno]

Il 04/07/22 12:00, Tinghan Shen ha scritto:
> From: Tzung-Bi Shih <tzungbi@chromium.org>
> 
> The I2C0 node doesn't need to be enabled in dtsi.
> 

"The I2C0 node should not be enabled globally, as usage is board dependant:
disable it in dtsi."

after which...

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

> Signed-off-by: Tzung-Bi Shih <tzungbi@chromium.org>
> Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>
> ---
>   arch/arm64/boot/dts/mediatek/mt8195.dtsi | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/mediatek/mt8195.dtsi b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
> index 436687ba826f..8032b839dfe8 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8195.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
> @@ -829,7 +829,7 @@
>   			clock-names = "main", "dma";
>   			#address-cells = <1>;
>   			#size-cells = <0>;
> -			status = "okay";
> +			status = "disabled";
>   		};
>   
>   		i2c1: i2c@11e01000 {

[PATCH v1 06/16] arm64: dts: mt8195: Add cpufreq node

[Tinghan Shen]

From: YT Lee <yt.lee@mediatek.corp-partner.google.com>

Add cpufreq node for mt8195.

Signed-off-by: YT Lee <yt.lee@mediatek.corp-partner.google.com>
Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>
---
 arch/arm64/boot/dts/mediatek/mt8195.dtsi | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt8195.dtsi b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
index 8032b839dfe8..900aaa16f862 100644
--- a/arch/arm64/boot/dts/mediatek/mt8195.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
@@ -26,6 +26,7 @@
 			compatible = "arm,cortex-a55";
 			reg = <0x000>;
 			enable-method = "psci";
+			performance-domains = <&performance 0>;
 			clock-frequency = <1701000000>;
 			capacity-dmips-mhz = <578>;
 			cpu-idle-states = <&cpu_off_l &cluster_off_l>;
@@ -38,6 +39,7 @@
 			compatible = "arm,cortex-a55";
 			reg = <0x100>;
 			enable-method = "psci";
+			performance-domains = <&performance 0>;
 			clock-frequency = <1701000000>;
 			capacity-dmips-mhz = <578>;
 			cpu-idle-states = <&cpu_off_l &cluster_off_l>;
@@ -50,6 +52,7 @@
 			compatible = "arm,cortex-a55";
 			reg = <0x200>;
 			enable-method = "psci";
+			performance-domains = <&performance 0>;
 			clock-frequency = <1701000000>;
 			capacity-dmips-mhz = <578>;
 			cpu-idle-states = <&cpu_off_l &cluster_off_l>;
@@ -62,6 +65,7 @@
 			compatible = "arm,cortex-a55";
 			reg = <0x300>;
 			enable-method = "psci";
+			performance-domains = <&performance 0>;
 			clock-frequency = <1701000000>;
 			capacity-dmips-mhz = <578>;
 			cpu-idle-states = <&cpu_off_l &cluster_off_l>;
@@ -74,6 +78,7 @@
 			compatible = "arm,cortex-a78";
 			reg = <0x400>;
 			enable-method = "psci";
+			performance-domains = <&performance 1>;
 			clock-frequency = <2171000000>;
 			capacity-dmips-mhz = <1024>;
 			cpu-idle-states = <&cpu_off_b &cluster_off_b>;
@@ -86,6 +91,7 @@
 			compatible = "arm,cortex-a78";
 			reg = <0x500>;
 			enable-method = "psci";
+			performance-domains = <&performance 1>;
 			clock-frequency = <2171000000>;
 			capacity-dmips-mhz = <1024>;
 			cpu-idle-states = <&cpu_off_b &cluster_off_b>;
@@ -98,6 +104,7 @@
 			compatible = "arm,cortex-a78";
 			reg = <0x600>;
 			enable-method = "psci";
+			performance-domains = <&performance 1>;
 			clock-frequency = <2171000000>;
 			capacity-dmips-mhz = <1024>;
 			cpu-idle-states = <&cpu_off_b &cluster_off_b>;
@@ -110,6 +117,7 @@
 			compatible = "arm,cortex-a78";
 			reg = <0x700>;
 			enable-method = "psci";
+			performance-domains = <&performance 1>;
 			clock-frequency = <2171000000>;
 			capacity-dmips-mhz = <1024>;
 			cpu-idle-states = <&cpu_off_b &cluster_off_b>;
@@ -231,6 +239,12 @@
 		clock-output-names = "clk32k";
 	};
 
+	performance: performance-controller@11bc10 {
+		compatible = "mediatek,cpufreq-hw";
+		reg = <0 0x0011bc10 0 0x120>, <0 0x0011bd30 0 0x120>;
+		#performance-domain-cells = <1>;
+	};
+
 	pmu-a55 {
 		compatible = "arm,cortex-a55-pmu";
 		interrupt-parent = <&gic>;
-- 
2.18.0

Re: [PATCH v1 06/16] arm64: dts: mt8195: Add cpufreq node

[AngeloGioacchino Del Regno]

Il 04/07/22 12:00, Tinghan Shen ha scritto:
> From: YT Lee <yt.lee@mediatek.corp-partner.google.com>
> 
> Add cpufreq node for mt8195.
> 
> Signed-off-by: YT Lee <yt.lee@mediatek.corp-partner.google.com>
> Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

[PATCH v1 07/16] arm64: dts: mt8195: Add vdosys and vppsys clock nodes

[Tinghan Shen]

Add display clock nodes.

Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>
---
 arch/arm64/boot/dts/mediatek/mt8195.dtsi | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt8195.dtsi b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
index 900aaa16f862..8d59a7da3271 100644
--- a/arch/arm64/boot/dts/mediatek/mt8195.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
@@ -983,6 +983,12 @@
 			#clock-cells = <1>;
 		};
 
+		vppsys0: clock-controller@14000000 {
+			compatible = "mediatek,mt8195-vppsys0";
+			reg = <0 0x14000000 0 0x1000>;
+			#clock-cells = <1>;
+		};
+
 		wpesys: clock-controller@14e00000 {
 			compatible = "mediatek,mt8195-wpesys";
 			reg = <0 0x14e00000 0 0x1000>;
@@ -1001,6 +1007,12 @@
 			#clock-cells = <1>;
 		};
 
+		vppsys1: clock-controller@14f00000 {
+			compatible = "mediatek,mt8195-vppsys1";
+			reg = <0 0x14f00000 0 0x1000>;
+			#clock-cells = <1>;
+		};
+
 		imgsys: clock-controller@15000000 {
 			compatible = "mediatek,mt8195-imgsys";
 			reg = <0 0x15000000 0 0x1000>;
@@ -1108,5 +1120,17 @@
 			reg = <0 0x1b000000 0 0x1000>;
 			#clock-cells = <1>;
 		};
+
+		vdosys0: syscon@1c01a000 {
+			compatible = "mediatek,mt8195-mmsys", "syscon";
+			reg = <0 0x1c01a000 0 0x1000>;
+			#clock-cells = <1>;
+		};
+
+		vdosys1: syscon@1c100000 {
+			compatible = "mediatek,mt8195-mmsys", "syscon";
+			reg = <0 0x1c100000 0 0x1000>;
+			#clock-cells = <1>;
+		};
 	};
 };
-- 
2.18.0

Re: [PATCH v1 07/16] arm64: dts: mt8195: Add vdosys and vppsys clock nodes

[AngeloGioacchino Del Regno]

Il 04/07/22 12:00, Tinghan Shen ha scritto:
> Add display clock nodes.
> 
> Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

[PATCH v1 08/16] arm64: dts: mt8195: Add power domains controller

[Tinghan Shen]

Add power domains controller node for mt8195.

Signed-off-by: Weiyi Lu <weiyi.lu@mediatek.com>
Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>
---
 arch/arm64/boot/dts/mediatek/mt8195.dtsi | 327 +++++++++++++++++++++++
 1 file changed, 327 insertions(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt8195.dtsi b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
index 8d59a7da3271..d52e140d9271 100644
--- a/arch/arm64/boot/dts/mediatek/mt8195.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
@@ -10,6 +10,7 @@
 #include <dt-bindings/interrupt-controller/irq.h>
 #include <dt-bindings/phy/phy.h>
 #include <dt-bindings/pinctrl/mt8195-pinfunc.h>
+#include <dt-bindings/power/mt8195-power.h>
 
 / {
 	compatible = "mediatek,mt8195";
@@ -338,6 +339,332 @@
 			#interrupt-cells = <2>;
 		};
 
+		scpsys: syscon@10006000 {
+			compatible = "syscon", "simple-mfd";
+			reg = <0 0x10006000 0 0x1000>;
+			#power-domain-cells = <1>;
+
+			/* System Power Manager */
+			spm: power-controller {
+				compatible = "mediatek,mt8195-power-controller";
+				#address-cells = <1>;
+				#size-cells = <0>;
+				#power-domain-cells = <1>;
+
+				/* power domain of the SoC */
+				mfg0: power-domain@MT8195_POWER_DOMAIN_MFG0 {
+					reg = <MT8195_POWER_DOMAIN_MFG0>;
+					#address-cells = <1>;
+					#size-cells = <0>;
+					#power-domain-cells = <1>;
+
+					power-domain@MT8195_POWER_DOMAIN_MFG1 {
+						reg = <MT8195_POWER_DOMAIN_MFG1>;
+						clocks = <&apmixedsys CLK_APMIXED_MFGPLL>;
+						clock-names = "mfg";
+						mediatek,infracfg = <&infracfg_ao>;
+						#address-cells = <1>;
+						#size-cells = <0>;
+						#power-domain-cells = <1>;
+
+						power-domain@MT8195_POWER_DOMAIN_MFG2 {
+							reg = <MT8195_POWER_DOMAIN_MFG2>;
+							#power-domain-cells = <0>;
+						};
+
+						power-domain@MT8195_POWER_DOMAIN_MFG3 {
+							reg = <MT8195_POWER_DOMAIN_MFG3>;
+							#power-domain-cells = <0>;
+						};
+
+						power-domain@MT8195_POWER_DOMAIN_MFG4 {
+							reg = <MT8195_POWER_DOMAIN_MFG4>;
+							#power-domain-cells = <0>;
+						};
+
+						power-domain@MT8195_POWER_DOMAIN_MFG5 {
+							reg = <MT8195_POWER_DOMAIN_MFG5>;
+							#power-domain-cells = <0>;
+						};
+
+						power-domain@MT8195_POWER_DOMAIN_MFG6 {
+							reg = <MT8195_POWER_DOMAIN_MFG6>;
+							#power-domain-cells = <0>;
+						};
+					};
+				};
+
+				power-domain@MT8195_POWER_DOMAIN_VPPSYS0 {
+					reg = <MT8195_POWER_DOMAIN_VPPSYS0>;
+					clocks = <&topckgen CLK_TOP_VPP>,
+						 <&topckgen CLK_TOP_CAM>,
+						 <&topckgen CLK_TOP_CCU>,
+						 <&topckgen CLK_TOP_IMG>,
+						 <&topckgen CLK_TOP_VENC>,
+						 <&topckgen CLK_TOP_VDEC>,
+						 <&topckgen CLK_TOP_WPE_VPP>,
+						 <&topckgen CLK_TOP_CFG_VPP0>,
+						 <&vppsys0 CLK_VPP0_SMI_COMMON>,
+						 <&vppsys0 CLK_VPP0_GALS_VDO0_LARB0>,
+						 <&vppsys0 CLK_VPP0_GALS_VDO0_LARB1>,
+						 <&vppsys0 CLK_VPP0_GALS_VENCSYS>,
+						 <&vppsys0 CLK_VPP0_GALS_VENCSYS_CORE1>,
+						 <&vppsys0 CLK_VPP0_GALS_INFRA>,
+						 <&vppsys0 CLK_VPP0_GALS_CAMSYS>,
+						 <&vppsys0 CLK_VPP0_GALS_VPP1_LARB5>,
+						 <&vppsys0 CLK_VPP0_GALS_VPP1_LARB6>,
+						 <&vppsys0 CLK_VPP0_SMI_REORDER>,
+						 <&vppsys0 CLK_VPP0_SMI_IOMMU>,
+						 <&vppsys0 CLK_VPP0_GALS_IMGSYS_CAMSYS>,
+						 <&vppsys0 CLK_VPP0_GALS_EMI0_EMI1>,
+						 <&vppsys0 CLK_VPP0_SMI_SUB_COMMON_REORDER>,
+						 <&vppsys0 CLK_VPP0_SMI_RSI>,
+						 <&vppsys0 CLK_VPP0_SMI_COMMON_LARB4>,
+						 <&vppsys0 CLK_VPP0_GALS_VDEC_VDEC_CORE1>,
+						 <&vppsys0 CLK_VPP0_GALS_VPP1_WPE>,
+						 <&vppsys0 CLK_VPP0_GALS_VDO0_VDO1_VENCSYS_CORE1>;
+					clock-names = "vppsys", "vppsys1", "vppsys2", "vppsys3",
+						      "vppsys4", "vppsys5", "vppsys6", "vppsys7",
+						      "vppsys0-0", "vppsys0-1", "vppsys0-2", "vppsys0-3",
+						      "vppsys0-4", "vppsys0-5", "vppsys0-6", "vppsys0-7",
+						      "vppsys0-8", "vppsys0-9", "vppsys0-10", "vppsys0-11",
+						      "vppsys0-12", "vppsys0-13", "vppsys0-14",
+						      "vppsys0-15", "vppsys0-16", "vppsys0-17",
+						      "vppsys0-18";
+					mediatek,infracfg = <&infracfg_ao>;
+					#address-cells = <1>;
+					#size-cells = <0>;
+					#power-domain-cells = <1>;
+
+					power-domain@MT8195_POWER_DOMAIN_VDEC1 {
+						reg = <MT8195_POWER_DOMAIN_VDEC1>;
+						clocks = <&vdecsys CLK_VDEC_LARB1>;
+						clock-names = "vdec1-0";
+						mediatek,infracfg = <&infracfg_ao>;
+						#power-domain-cells = <0>;
+					};
+
+					power-domain@MT8195_POWER_DOMAIN_VENC_CORE1 {
+						reg = <MT8195_POWER_DOMAIN_VENC_CORE1>;
+						mediatek,infracfg = <&infracfg_ao>;
+						#power-domain-cells = <0>;
+					};
+
+					power-domain@MT8195_POWER_DOMAIN_VDOSYS0 {
+						reg = <MT8195_POWER_DOMAIN_VDOSYS0>;
+						clocks = <&topckgen CLK_TOP_CFG_VDO0>,
+							 <&vdosys0 CLK_VDO0_SMI_GALS>,
+							 <&vdosys0 CLK_VDO0_SMI_COMMON>,
+							 <&vdosys0 CLK_VDO0_SMI_EMI>,
+							 <&vdosys0 CLK_VDO0_SMI_IOMMU>,
+							 <&vdosys0 CLK_VDO0_SMI_LARB>,
+							 <&vdosys0 CLK_VDO0_SMI_RSI>;
+						clock-names = "vdosys0", "vdosys0-0", "vdosys0-1",
+							      "vdosys0-2", "vdosys0-3",
+							      "vdosys0-4", "vdosys0-5";
+						mediatek,infracfg = <&infracfg_ao>;
+						#address-cells = <1>;
+						#size-cells = <0>;
+						#power-domain-cells = <1>;
+
+						power-domain@MT8195_POWER_DOMAIN_VPPSYS1 {
+							reg = <MT8195_POWER_DOMAIN_VPPSYS1>;
+							clocks = <&topckgen CLK_TOP_CFG_VPP1>,
+								 <&vppsys1 CLK_VPP1_VPPSYS1_GALS>,
+								 <&vppsys1 CLK_VPP1_VPPSYS1_LARB>;
+							clock-names = "vppsys1", "vppsys1-0",
+								      "vppsys1-1";
+							mediatek,infracfg = <&infracfg_ao>;
+							#power-domain-cells = <0>;
+						};
+
+						power-domain@MT8195_POWER_DOMAIN_WPESYS {
+							reg = <MT8195_POWER_DOMAIN_WPESYS>;
+							clocks = <&wpesys CLK_WPE_SMI_LARB7>,
+								 <&wpesys CLK_WPE_SMI_LARB8>,
+								 <&wpesys CLK_WPE_SMI_LARB7_P>,
+								 <&wpesys CLK_WPE_SMI_LARB8_P>;
+							clock-names = "wepsys-0", "wepsys-1", "wepsys-2",
+								      "wepsys-3";
+							mediatek,infracfg = <&infracfg_ao>;
+							#power-domain-cells = <0>;
+						};
+
+						power-domain@MT8195_POWER_DOMAIN_VDEC0 {
+							reg = <MT8195_POWER_DOMAIN_VDEC0>;
+							clocks = <&vdecsys_soc CLK_VDEC_SOC_LARB1>;
+							clock-names = "vdec0-0";
+							mediatek,infracfg = <&infracfg_ao>;
+							#power-domain-cells = <0>;
+						};
+
+						power-domain@MT8195_POWER_DOMAIN_VDEC2 {
+							reg = <MT8195_POWER_DOMAIN_VDEC2>;
+							clocks = <&vdecsys_core1 CLK_VDEC_CORE1_LARB1>;
+							clock-names = "vdec2-0";
+							mediatek,infracfg = <&infracfg_ao>;
+							#power-domain-cells = <0>;
+						};
+
+						power-domain@MT8195_POWER_DOMAIN_VENC {
+							reg = <MT8195_POWER_DOMAIN_VENC>;
+							mediatek,infracfg = <&infracfg_ao>;
+							#power-domain-cells = <0>;
+						};
+
+						power-domain@MT8195_POWER_DOMAIN_VDOSYS1 {
+							reg = <MT8195_POWER_DOMAIN_VDOSYS1>;
+							clocks = <&topckgen CLK_TOP_CFG_VDO1>,
+								 <&vdosys1 CLK_VDO1_SMI_LARB2>,
+								 <&vdosys1 CLK_VDO1_SMI_LARB3>,
+								 <&vdosys1 CLK_VDO1_GALS>;
+							clock-names = "vdosys1", "vdosys1-0",
+								      "vdosys1-1", "vdosys1-2";
+							mediatek,infracfg = <&infracfg_ao>;
+							#address-cells = <1>;
+							#size-cells = <0>;
+							#power-domain-cells = <1>;
+
+							power-domain@MT8195_POWER_DOMAIN_DP_TX {
+								reg = <MT8195_POWER_DOMAIN_DP_TX>;
+								mediatek,infracfg = <&infracfg_ao>;
+								#power-domain-cells = <0>;
+							};
+
+							power-domain@MT8195_POWER_DOMAIN_EPD_TX {
+								reg = <MT8195_POWER_DOMAIN_EPD_TX>;
+								mediatek,infracfg = <&infracfg_ao>;
+								#power-domain-cells = <0>;
+							};
+
+							power-domain@MT8195_POWER_DOMAIN_HDMI_TX {
+								reg = <MT8195_POWER_DOMAIN_HDMI_TX>;
+								clocks = <&topckgen CLK_TOP_HDMI_APB>;
+								clock-names = "hdmi_tx";
+								#power-domain-cells = <0>;
+							};
+						};
+
+						power-domain@MT8195_POWER_DOMAIN_IMG {
+							reg = <MT8195_POWER_DOMAIN_IMG>;
+							clocks = <&imgsys CLK_IMG_LARB9>,
+								 <&imgsys CLK_IMG_GALS>;
+							clock-names = "img-0", "img-1";
+							mediatek,infracfg = <&infracfg_ao>;
+							#address-cells = <1>;
+							#size-cells = <0>;
+							#power-domain-cells = <1>;
+
+							power-domain@MT8195_POWER_DOMAIN_DIP {
+								reg = <MT8195_POWER_DOMAIN_DIP>;
+								#power-domain-cells = <0>;
+							};
+
+							power-domain@MT8195_POWER_DOMAIN_IPE {
+								reg = <MT8195_POWER_DOMAIN_IPE>;
+								clocks = <&topckgen CLK_TOP_IPE>,
+									 <&imgsys CLK_IMG_IPE>,
+									 <&ipesys CLK_IPE_SMI_LARB12>;
+								clock-names = "ipe", "ipe-0", "ipe-1";
+								mediatek,infracfg = <&infracfg_ao>;
+								#power-domain-cells = <0>;
+							};
+						};
+
+						power-domain@MT8195_POWER_DOMAIN_CAM {
+							reg = <MT8195_POWER_DOMAIN_CAM>;
+							clocks = <&camsys CLK_CAM_LARB13>,
+								 <&camsys CLK_CAM_LARB14>,
+								 <&camsys CLK_CAM_CAM2MM0_GALS>,
+								 <&camsys CLK_CAM_CAM2MM1_GALS>,
+								 <&camsys CLK_CAM_CAM2SYS_GALS>;
+							clock-names = "cam-0", "cam-1", "cam-2", "cam-3",
+								      "cam-4";
+							mediatek,infracfg = <&infracfg_ao>;
+							#address-cells = <1>;
+							#size-cells = <0>;
+							#power-domain-cells = <1>;
+
+							power-domain@MT8195_POWER_DOMAIN_CAM_RAWA {
+								reg = <MT8195_POWER_DOMAIN_CAM_RAWA>;
+								#power-domain-cells = <0>;
+							};
+
+							power-domain@MT8195_POWER_DOMAIN_CAM_RAWB {
+								reg = <MT8195_POWER_DOMAIN_CAM_RAWB>;
+								#power-domain-cells = <0>;
+							};
+
+							power-domain@MT8195_POWER_DOMAIN_CAM_MRAW {
+								reg = <MT8195_POWER_DOMAIN_CAM_MRAW>;
+								#power-domain-cells = <0>;
+							};
+						};
+					};
+				};
+
+				power-domain@MT8195_POWER_DOMAIN_PCIE_MAC_P0 {
+					reg = <MT8195_POWER_DOMAIN_PCIE_MAC_P0>;
+					mediatek,infracfg = <&infracfg_ao>;
+					#power-domain-cells = <0>;
+				};
+
+				power-domain@MT8195_POWER_DOMAIN_PCIE_MAC_P1 {
+					reg = <MT8195_POWER_DOMAIN_PCIE_MAC_P1>;
+					mediatek,infracfg = <&infracfg_ao>;
+					#power-domain-cells = <0>;
+				};
+
+				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>;
+				};
+
+				power-domain@MT8195_POWER_DOMAIN_CSI_RX_TOP {
+					reg = <MT8195_POWER_DOMAIN_CSI_RX_TOP>;
+					clocks = <&topckgen CLK_TOP_SENINF>,
+						 <&topckgen CLK_TOP_SENINF2>;
+					clock-names = "csi_rx_top", "csi_rx_top1";
+					#power-domain-cells = <0>;
+				};
+
+				power-domain@MT8195_POWER_DOMAIN_ETHER {
+					reg = <MT8195_POWER_DOMAIN_ETHER>;
+					clocks = <&pericfg_ao CLK_PERI_AO_ETHERNET_MAC>;
+					clock-names = "ether";
+					#power-domain-cells = <0>;
+				};
+
+				power-domain@MT8195_POWER_DOMAIN_ADSP {
+					reg = <MT8195_POWER_DOMAIN_ADSP>;
+					clocks = <&topckgen CLK_TOP_ADSP>,
+						 <&topckgen CLK_TOP_AUDIO_LOCAL_BUS>;
+					clock-names = "adsp", "adsp1";
+					#address-cells = <1>;
+					#size-cells = <0>;
+					mediatek,infracfg = <&infracfg_ao>;
+					#power-domain-cells = <1>;
+
+					power-domain@MT8195_POWER_DOMAIN_AUDIO {
+						reg = <MT8195_POWER_DOMAIN_AUDIO>;
+						clocks = <&topckgen CLK_TOP_A1SYS_HP>,
+							 <&topckgen CLK_TOP_AUD_INTBUS>,
+							 <&topckgen CLK_TOP_AUDIO_LOCAL_BUS>,
+							 <&infracfg_ao CLK_INFRA_AO_AUDIO_26M_B>;
+						clock-names = "audio", "audio1", "audio2",
+							      "audio3";
+						mediatek,infracfg = <&infracfg_ao>;
+						#power-domain-cells = <0>;
+					};
+				};
+			};
+		};
+
 		watchdog: watchdog@10007000 {
 			compatible = "mediatek,mt8195-wdt",
 				     "mediatek,mt6589-wdt";
-- 
2.18.0

Re: [PATCH v1 08/16] arm64: dts: mt8195: Add power domains controller

[AngeloGioacchino Del Regno]

Il 04/07/22 12:00, Tinghan Shen ha scritto:
> Add power domains controller node for mt8195.
> 
> Signed-off-by: Weiyi Lu <weiyi.lu@mediatek.com>
> Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

Re: [PATCH v1 08/16] arm64: dts: mt8195: Add power domains controller

[Krzysztof Kozlowski]

On 04/07/2022 12:00, Tinghan Shen wrote:
> Add power domains controller node for mt8195.
> 
> Signed-off-by: Weiyi Lu <weiyi.lu@mediatek.com>
> Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>
> ---
>  arch/arm64/boot/dts/mediatek/mt8195.dtsi | 327 +++++++++++++++++++++++
>  1 file changed, 327 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/mediatek/mt8195.dtsi b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
> index 8d59a7da3271..d52e140d9271 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8195.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
> @@ -10,6 +10,7 @@
>  #include <dt-bindings/interrupt-controller/irq.h>
>  #include <dt-bindings/phy/phy.h>
>  #include <dt-bindings/pinctrl/mt8195-pinfunc.h>
> +#include <dt-bindings/power/mt8195-power.h>
>  
>  / {
>  	compatible = "mediatek,mt8195";
> @@ -338,6 +339,332 @@
>  			#interrupt-cells = <2>;
>  		};
>  
> +		scpsys: syscon@10006000 {
> +			compatible = "syscon", "simple-mfd";

These compatibles cannot be alone.

> +			reg = <0 0x10006000 0 0x1000>;
> +			#power-domain-cells = <1>;

If it is simple MFD, then probably it is not a power domain provider.
Decide.

Best regards,
Krzysztof

Re: [PATCH v1 08/16] arm64: dts: mt8195: Add power domains controller

[Tinghan Shen]

Hi Krzysztof,

After discussing your message with our power team, 
we realized that we need your help to ensure we fully understand you.

On Mon, 2022-07-04 at 14:38 +0200, Krzysztof Kozlowski wrote:
> On 04/07/2022 12:00, Tinghan Shen wrote:
> > Add power domains controller node for mt8195.
> > 
> > Signed-off-by: Weiyi Lu <weiyi.lu@mediatek.com>
> > Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>
> > ---
> >  arch/arm64/boot/dts/mediatek/mt8195.dtsi | 327 +++++++++++++++++++++++
> >  1 file changed, 327 insertions(+)
> > 
> > diff --git a/arch/arm64/boot/dts/mediatek/mt8195.dtsi b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
> > index 8d59a7da3271..d52e140d9271 100644
> > --- a/arch/arm64/boot/dts/mediatek/mt8195.dtsi
> > +++ b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
> > @@ -10,6 +10,7 @@
> >  #include <dt-bindings/interrupt-controller/irq.h>
> >  #include <dt-bindings/phy/phy.h>
> >  #include <dt-bindings/pinctrl/mt8195-pinfunc.h>
> > +#include <dt-bindings/power/mt8195-power.h>
> >  
> >  / {
> >  	compatible = "mediatek,mt8195";
> > @@ -338,6 +339,332 @@
> >  			#interrupt-cells = <2>;
> >  		};
> >  
> > +		scpsys: syscon@10006000 {
> > +			compatible = "syscon", "simple-mfd";
> 
> These compatibles cannot be alone.

the scpsys sub node has the compatible of the power domain driver.
do you suggest that the compatible in the sub node should move to here?

> > +			reg = <0 0x10006000 0 0x1000>;
> > +			#power-domain-cells = <1>;
> 
> If it is simple MFD, then probably it is not a power domain provider.
> Decide.

this MFD device is the power controller on mt8195. Some features need 
to do some operations on registers in this node. We think that implement 
the operation of these registers as the MFD device can provide flexibility 
for future use. We want to clarify if you're saying that an MFD device 
cannot be a power domain provider.



Best regards,
TingHan

Re: [PATCH v1 08/16] arm64: dts: mt8195: Add power domains controller

[Krzysztof Kozlowski]

On 06/07/2022 14:00, Tinghan Shen wrote:
> Hi Krzysztof,
> 
> After discussing your message with our power team, 
> we realized that we need your help to ensure we fully understand you.
> 
> On Mon, 2022-07-04 at 14:38 +0200, Krzysztof Kozlowski wrote:
>> On 04/07/2022 12:00, Tinghan Shen wrote:
>>> Add power domains controller node for mt8195.
>>>
>>> Signed-off-by: Weiyi Lu <weiyi.lu@mediatek.com>
>>> Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>
>>> ---
>>>  arch/arm64/boot/dts/mediatek/mt8195.dtsi | 327 +++++++++++++++++++++++
>>>  1 file changed, 327 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/mediatek/mt8195.dtsi b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
>>> index 8d59a7da3271..d52e140d9271 100644
>>> --- a/arch/arm64/boot/dts/mediatek/mt8195.dtsi
>>> +++ b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
>>> @@ -10,6 +10,7 @@
>>>  #include <dt-bindings/interrupt-controller/irq.h>
>>>  #include <dt-bindings/phy/phy.h>
>>>  #include <dt-bindings/pinctrl/mt8195-pinfunc.h>
>>> +#include <dt-bindings/power/mt8195-power.h>
>>>  
>>>  / {
>>>  	compatible = "mediatek,mt8195";
>>> @@ -338,6 +339,332 @@
>>>  			#interrupt-cells = <2>;
>>>  		};
>>>  
>>> +		scpsys: syscon@10006000 {
>>> +			compatible = "syscon", "simple-mfd";
>>
>> These compatibles cannot be alone.
> 
> the scpsys sub node has the compatible of the power domain driver.
> do you suggest that the compatible in the sub node should move to here?

Not necessarily, depends. You have here device node representing system
registers. They need they own compatibles, just like everywhere in the
kernel (except the broken cases...).

Whether this should be compatible of power-domain driver, it depends
what this device node is. I don't know, I don't have your datasheets or
your architecture diagrams...

> 
>>> +			reg = <0 0x10006000 0 0x1000>;
>>> +			#power-domain-cells = <1>;
>>
>> If it is simple MFD, then probably it is not a power domain provider.
>> Decide.
> 
> this MFD device is the power controller on mt8195. 

Then it is not a simple MFD but a power controller. Do not use
"simple-mfd" compatible.

> Some features need 
> to do some operations on registers in this node. We think that implement 
> the operation of these registers as the MFD device can provide flexibility 
> for future use. We want to clarify if you're saying that an MFD device 
> cannot be a power domain provider.

MFD device is Linuxism, so it has nothing to do here. I am talking only
about simple-mfd. simple-mfd is a simple device only instantiating
children and not providing anything to anyone. Neither to children. This
 the most important part. The children do not depend on anything from
simple-mfd device. For example simple-mfd device can be shut down
(gated) and children should still operate. Being a power domain
controller, contradicts this usually.

Best regards,
Krzysztof

Re: [PATCH v1 08/16] arm64: dts: mt8195: Add power domains controller

[AngeloGioacchino Del Regno]

Il 06/07/22 17:18, Krzysztof Kozlowski ha scritto:
> On 06/07/2022 14:00, Tinghan Shen wrote:
>> Hi Krzysztof,
>>
>> After discussing your message with our power team,
>> we realized that we need your help to ensure we fully understand you.
>>
>> On Mon, 2022-07-04 at 14:38 +0200, Krzysztof Kozlowski wrote:
>>> On 04/07/2022 12:00, Tinghan Shen wrote:
>>>> Add power domains controller node for mt8195.
>>>>
>>>> Signed-off-by: Weiyi Lu <weiyi.lu@mediatek.com>
>>>> Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>
>>>> ---
>>>>   arch/arm64/boot/dts/mediatek/mt8195.dtsi | 327 +++++++++++++++++++++++
>>>>   1 file changed, 327 insertions(+)
>>>>
>>>> diff --git a/arch/arm64/boot/dts/mediatek/mt8195.dtsi b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
>>>> index 8d59a7da3271..d52e140d9271 100644
>>>> --- a/arch/arm64/boot/dts/mediatek/mt8195.dtsi
>>>> +++ b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
>>>> @@ -10,6 +10,7 @@
>>>>   #include <dt-bindings/interrupt-controller/irq.h>
>>>>   #include <dt-bindings/phy/phy.h>
>>>>   #include <dt-bindings/pinctrl/mt8195-pinfunc.h>
>>>> +#include <dt-bindings/power/mt8195-power.h>
>>>>   
>>>>   / {
>>>>   	compatible = "mediatek,mt8195";
>>>> @@ -338,6 +339,332 @@
>>>>   			#interrupt-cells = <2>;
>>>>   		};
>>>>   
>>>> +		scpsys: syscon@10006000 {
>>>> +			compatible = "syscon", "simple-mfd";
>>>
>>> These compatibles cannot be alone.
>>
>> the scpsys sub node has the compatible of the power domain driver.
>> do you suggest that the compatible in the sub node should move to here?
> 
> Not necessarily, depends. You have here device node representing system
> registers. They need they own compatibles, just like everywhere in the
> kernel (except the broken cases...).
> 
> Whether this should be compatible of power-domain driver, it depends
> what this device node is. I don't know, I don't have your datasheets or
> your architecture diagrams...
> 
>>
>>>> +			reg = <0 0x10006000 0 0x1000>;
>>>> +			#power-domain-cells = <1>;
>>>
>>> If it is simple MFD, then probably it is not a power domain provider.
>>> Decide.
>>
>> this MFD device is the power controller on mt8195.
> 
> Then it is not a simple MFD but a power controller. Do not use
> "simple-mfd" compatible.
> 
>> Some features need
>> to do some operations on registers in this node. We think that implement
>> the operation of these registers as the MFD device can provide flexibility
>> for future use. We want to clarify if you're saying that an MFD device
>> cannot be a power domain provider.
> 
> MFD device is Linuxism, so it has nothing to do here. I am talking only
> about simple-mfd. simple-mfd is a simple device only instantiating
> children and not providing anything to anyone. Neither to children. This
>   the most important part. The children do not depend on anything from
> simple-mfd device. For example simple-mfd device can be shut down
> (gated) and children should still operate. Being a power domain
> controller, contradicts this usually.
> 

If my interpretation of this issue is right, I have pushed a solution for it.
Krzysztof, Matthias, can you please check [1] and give feedback, so that
Tinghan can rewrite this commit ASAP?

Reason is - I need the MT8195 devicetree to be complete to push the remaining
pieces for Tomato Chromebooks, of course.

[1]: https://patchwork.kernel.org/project/linux-mediatek/list/?series=658527

Thanks a lot,
Angelo

Re: [PATCH v1 08/16] arm64: dts: mt8195: Add power domains controller

[Krzysztof Kozlowski]

On 12/07/2022 10:17, AngeloGioacchino Del Regno wrote:
> Il 06/07/22 17:18, Krzysztof Kozlowski ha scritto:
>> On 06/07/2022 14:00, Tinghan Shen wrote:
>>> Hi Krzysztof,
>>>
>>> After discussing your message with our power team,
>>> we realized that we need your help to ensure we fully understand you.
>>>
>>> On Mon, 2022-07-04 at 14:38 +0200, Krzysztof Kozlowski wrote:
>>>> On 04/07/2022 12:00, Tinghan Shen wrote:
>>>>> Add power domains controller node for mt8195.
>>>>>
>>>>> Signed-off-by: Weiyi Lu <weiyi.lu@mediatek.com>
>>>>> Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>
>>>>> ---
>>>>>   arch/arm64/boot/dts/mediatek/mt8195.dtsi | 327 +++++++++++++++++++++++
>>>>>   1 file changed, 327 insertions(+)
>>>>>
>>>>> diff --git a/arch/arm64/boot/dts/mediatek/mt8195.dtsi b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
>>>>> index 8d59a7da3271..d52e140d9271 100644
>>>>> --- a/arch/arm64/boot/dts/mediatek/mt8195.dtsi
>>>>> +++ b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
>>>>> @@ -10,6 +10,7 @@
>>>>>   #include <dt-bindings/interrupt-controller/irq.h>
>>>>>   #include <dt-bindings/phy/phy.h>
>>>>>   #include <dt-bindings/pinctrl/mt8195-pinfunc.h>
>>>>> +#include <dt-bindings/power/mt8195-power.h>
>>>>>   
>>>>>   / {
>>>>>   	compatible = "mediatek,mt8195";
>>>>> @@ -338,6 +339,332 @@
>>>>>   			#interrupt-cells = <2>;
>>>>>   		};
>>>>>   
>>>>> +		scpsys: syscon@10006000 {
>>>>> +			compatible = "syscon", "simple-mfd";
>>>>
>>>> These compatibles cannot be alone.
>>>
>>> the scpsys sub node has the compatible of the power domain driver.
>>> do you suggest that the compatible in the sub node should move to here?
>>
>> Not necessarily, depends. You have here device node representing system
>> registers. They need they own compatibles, just like everywhere in the
>> kernel (except the broken cases...).
>>
>> Whether this should be compatible of power-domain driver, it depends
>> what this device node is. I don't know, I don't have your datasheets or
>> your architecture diagrams...
>>
>>>
>>>>> +			reg = <0 0x10006000 0 0x1000>;
>>>>> +			#power-domain-cells = <1>;
>>>>
>>>> If it is simple MFD, then probably it is not a power domain provider.
>>>> Decide.
>>>
>>> this MFD device is the power controller on mt8195.
>>
>> Then it is not a simple MFD but a power controller. Do not use
>> "simple-mfd" compatible.
>>
>>> Some features need
>>> to do some operations on registers in this node. We think that implement
>>> the operation of these registers as the MFD device can provide flexibility
>>> for future use. We want to clarify if you're saying that an MFD device
>>> cannot be a power domain provider.
>>
>> MFD device is Linuxism, so it has nothing to do here. I am talking only
>> about simple-mfd. simple-mfd is a simple device only instantiating
>> children and not providing anything to anyone. Neither to children. This
>>   the most important part. The children do not depend on anything from
>> simple-mfd device. For example simple-mfd device can be shut down
>> (gated) and children should still operate. Being a power domain
>> controller, contradicts this usually.
>>
> 
> If my interpretation of this issue is right, I have pushed a solution for it.
> Krzysztof, Matthias, can you please check [1] and give feedback, so that
> Tinghan can rewrite this commit ASAP?
> 
> Reason is - I need the MT8195 devicetree to be complete to push the remaining
> pieces for Tomato Chromebooks, of course.
> 
> [1]: https://patchwork.kernel.org/project/linux-mediatek/list/?series=658527

I have two or three similar discussions, so maybe I lost the context,
but I don't understand how your fix is matching real hardware.

In the patchset here, Tinghan claimed that power domain controller is a
child of 10006000. 10006000 is also a power domain controller. This was
explicitly described by the DTS code.

Now you abandon this hierarchy in favor of syscon. If the hierarchy was
correct, your patchset does not match the hardware, so it's a no-go.
Describe the hardware.

However maybe this patch did not make any sense and there is no
relationship parent-child... so what do you guys send here? Bunch of
hacks and work-arounds?

Your DTS should reflect the hardware, not some hacks.

Best regards,
Krzysztof

Re: [PATCH v1 08/16] arm64: dts: mt8195: Add power domains controller

[AngeloGioacchino Del Regno]

Il 12/07/22 10:37, Krzysztof Kozlowski ha scritto:
> On 12/07/2022 10:17, AngeloGioacchino Del Regno wrote:
>> Il 06/07/22 17:18, Krzysztof Kozlowski ha scritto:
>>> On 06/07/2022 14:00, Tinghan Shen wrote:
>>>> Hi Krzysztof,
>>>>
>>>> After discussing your message with our power team,
>>>> we realized that we need your help to ensure we fully understand you.
>>>>
>>>> On Mon, 2022-07-04 at 14:38 +0200, Krzysztof Kozlowski wrote:
>>>>> On 04/07/2022 12:00, Tinghan Shen wrote:
>>>>>> Add power domains controller node for mt8195.
>>>>>>
>>>>>> Signed-off-by: Weiyi Lu <weiyi.lu@mediatek.com>
>>>>>> Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>
>>>>>> ---
>>>>>>    arch/arm64/boot/dts/mediatek/mt8195.dtsi | 327 +++++++++++++++++++++++
>>>>>>    1 file changed, 327 insertions(+)
>>>>>>
>>>>>> diff --git a/arch/arm64/boot/dts/mediatek/mt8195.dtsi b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
>>>>>> index 8d59a7da3271..d52e140d9271 100644
>>>>>> --- a/arch/arm64/boot/dts/mediatek/mt8195.dtsi
>>>>>> +++ b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
>>>>>> @@ -10,6 +10,7 @@
>>>>>>    #include <dt-bindings/interrupt-controller/irq.h>
>>>>>>    #include <dt-bindings/phy/phy.h>
>>>>>>    #include <dt-bindings/pinctrl/mt8195-pinfunc.h>
>>>>>> +#include <dt-bindings/power/mt8195-power.h>
>>>>>>    
>>>>>>    / {
>>>>>>    	compatible = "mediatek,mt8195";
>>>>>> @@ -338,6 +339,332 @@
>>>>>>    			#interrupt-cells = <2>;
>>>>>>    		};
>>>>>>    
>>>>>> +		scpsys: syscon@10006000 {
>>>>>> +			compatible = "syscon", "simple-mfd";
>>>>>
>>>>> These compatibles cannot be alone.
>>>>
>>>> the scpsys sub node has the compatible of the power domain driver.
>>>> do you suggest that the compatible in the sub node should move to here?
>>>
>>> Not necessarily, depends. You have here device node representing system
>>> registers. They need they own compatibles, just like everywhere in the
>>> kernel (except the broken cases...).
>>>
>>> Whether this should be compatible of power-domain driver, it depends
>>> what this device node is. I don't know, I don't have your datasheets or
>>> your architecture diagrams...
>>>
>>>>
>>>>>> +			reg = <0 0x10006000 0 0x1000>;
>>>>>> +			#power-domain-cells = <1>;
>>>>>
>>>>> If it is simple MFD, then probably it is not a power domain provider.
>>>>> Decide.
>>>>
>>>> this MFD device is the power controller on mt8195.
>>>
>>> Then it is not a simple MFD but a power controller. Do not use
>>> "simple-mfd" compatible.
>>>
>>>> Some features need
>>>> to do some operations on registers in this node. We think that implement
>>>> the operation of these registers as the MFD device can provide flexibility
>>>> for future use. We want to clarify if you're saying that an MFD device
>>>> cannot be a power domain provider.
>>>
>>> MFD device is Linuxism, so it has nothing to do here. I am talking only
>>> about simple-mfd. simple-mfd is a simple device only instantiating
>>> children and not providing anything to anyone. Neither to children. This
>>>    the most important part. The children do not depend on anything from
>>> simple-mfd device. For example simple-mfd device can be shut down
>>> (gated) and children should still operate. Being a power domain
>>> controller, contradicts this usually.
>>>
>>
>> If my interpretation of this issue is right, I have pushed a solution for it.
>> Krzysztof, Matthias, can you please check [1] and give feedback, so that
>> Tinghan can rewrite this commit ASAP?
>>
>> Reason is - I need the MT8195 devicetree to be complete to push the remaining
>> pieces for Tomato Chromebooks, of course.
>>
>> [1]: https://patchwork.kernel.org/project/linux-mediatek/list/?series=658527
> 
> I have two or three similar discussions, so maybe I lost the context,
> but I don't understand how your fix is matching real hardware.
> 
> In the patchset here, Tinghan claimed that power domain controller is a
> child of 10006000. 10006000 is also a power domain controller. This was
> explicitly described by the DTS code.
> 
> Now you abandon this hierarchy in favor of syscon. If the hierarchy was
> correct, your patchset does not match the hardware, so it's a no-go.
> Describe the hardware.
> 
> However maybe this patch did not make any sense and there is no
> relationship parent-child... so what do you guys send here? Bunch of
> hacks and work-arounds?
> 

For how I get it, hardware side, the SPM (System Power Manager) resides inside
of the SCPSYS block (consequently, in that iospace).

As Matthias pointed out earlier, SCPSYS provides more functionality, but the
only one that's currently implemented upstream is the System Power Manager,
responsible for managing the MTCMOS (power domains).

In any case, now I'm a little confused on how to proceed, can you please give
some suggestion?

Thanks,
Angelo

> Your DTS should reflect the hardware, not some hacks.
> 
> Best regards,
> Krzysztof

Re: [PATCH v1 08/16] arm64: dts: mt8195: Add power domains controller

[Krzysztof Kozlowski]

On 12/07/2022 10:53, AngeloGioacchino Del Regno wrote:
> Il 12/07/22 10:37, Krzysztof Kozlowski ha scritto:
>> On 12/07/2022 10:17, AngeloGioacchino Del Regno wrote:
>>> Il 06/07/22 17:18, Krzysztof Kozlowski ha scritto:
>>>> On 06/07/2022 14:00, Tinghan Shen wrote:
>>>>> Hi Krzysztof,
>>>>>
>>>>> After discussing your message with our power team,
>>>>> we realized that we need your help to ensure we fully understand you.
>>>>>
>>>>> On Mon, 2022-07-04 at 14:38 +0200, Krzysztof Kozlowski wrote:
>>>>>> On 04/07/2022 12:00, Tinghan Shen wrote:
>>>>>>> Add power domains controller node for mt8195.
>>>>>>>
>>>>>>> Signed-off-by: Weiyi Lu <weiyi.lu@mediatek.com>
>>>>>>> Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>
>>>>>>> ---
>>>>>>>    arch/arm64/boot/dts/mediatek/mt8195.dtsi | 327 +++++++++++++++++++++++
>>>>>>>    1 file changed, 327 insertions(+)
>>>>>>>
>>>>>>> diff --git a/arch/arm64/boot/dts/mediatek/mt8195.dtsi b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
>>>>>>> index 8d59a7da3271..d52e140d9271 100644
>>>>>>> --- a/arch/arm64/boot/dts/mediatek/mt8195.dtsi
>>>>>>> +++ b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
>>>>>>> @@ -10,6 +10,7 @@
>>>>>>>    #include <dt-bindings/interrupt-controller/irq.h>
>>>>>>>    #include <dt-bindings/phy/phy.h>
>>>>>>>    #include <dt-bindings/pinctrl/mt8195-pinfunc.h>
>>>>>>> +#include <dt-bindings/power/mt8195-power.h>
>>>>>>>    
>>>>>>>    / {
>>>>>>>    	compatible = "mediatek,mt8195";
>>>>>>> @@ -338,6 +339,332 @@
>>>>>>>    			#interrupt-cells = <2>;
>>>>>>>    		};
>>>>>>>    
>>>>>>> +		scpsys: syscon@10006000 {
>>>>>>> +			compatible = "syscon", "simple-mfd";
>>>>>>
>>>>>> These compatibles cannot be alone.
>>>>>
>>>>> the scpsys sub node has the compatible of the power domain driver.
>>>>> do you suggest that the compatible in the sub node should move to here?
>>>>
>>>> Not necessarily, depends. You have here device node representing system
>>>> registers. They need they own compatibles, just like everywhere in the
>>>> kernel (except the broken cases...).
>>>>
>>>> Whether this should be compatible of power-domain driver, it depends
>>>> what this device node is. I don't know, I don't have your datasheets or
>>>> your architecture diagrams...
>>>>
>>>>>
>>>>>>> +			reg = <0 0x10006000 0 0x1000>;
>>>>>>> +			#power-domain-cells = <1>;
>>>>>>
>>>>>> If it is simple MFD, then probably it is not a power domain provider.
>>>>>> Decide.
>>>>>
>>>>> this MFD device is the power controller on mt8195.
>>>>
>>>> Then it is not a simple MFD but a power controller. Do not use
>>>> "simple-mfd" compatible.
>>>>
>>>>> Some features need
>>>>> to do some operations on registers in this node. We think that implement
>>>>> the operation of these registers as the MFD device can provide flexibility
>>>>> for future use. We want to clarify if you're saying that an MFD device
>>>>> cannot be a power domain provider.
>>>>
>>>> MFD device is Linuxism, so it has nothing to do here. I am talking only
>>>> about simple-mfd. simple-mfd is a simple device only instantiating
>>>> children and not providing anything to anyone. Neither to children. This
>>>>    the most important part. The children do not depend on anything from
>>>> simple-mfd device. For example simple-mfd device can be shut down
>>>> (gated) and children should still operate. Being a power domain
>>>> controller, contradicts this usually.
>>>>
>>>
>>> If my interpretation of this issue is right, I have pushed a solution for it.
>>> Krzysztof, Matthias, can you please check [1] and give feedback, so that
>>> Tinghan can rewrite this commit ASAP?
>>>
>>> Reason is - I need the MT8195 devicetree to be complete to push the remaining
>>> pieces for Tomato Chromebooks, of course.
>>>
>>> [1]: https://patchwork.kernel.org/project/linux-mediatek/list/?series=658527
>>
>> I have two or three similar discussions, so maybe I lost the context,
>> but I don't understand how your fix is matching real hardware.
>>
>> In the patchset here, Tinghan claimed that power domain controller is a
>> child of 10006000. 10006000 is also a power domain controller. This was
>> explicitly described by the DTS code.
>>
>> Now you abandon this hierarchy in favor of syscon. If the hierarchy was
>> correct, your patchset does not match the hardware, so it's a no-go.
>> Describe the hardware.
>>
>> However maybe this patch did not make any sense and there is no
>> relationship parent-child... so what do you guys send here? Bunch of
>> hacks and work-arounds?
>>
> 
> For how I get it, hardware side, the SPM (System Power Manager) resides inside
> of the SCPSYS block (consequently, in that iospace).
> 
> As Matthias pointed out earlier, SCPSYS provides more functionality, but the
> only one that's currently implemented upstream is the System Power Manager,
> responsible for managing the MTCMOS (power domains).
> 
> In any case, now I'm a little confused on how to proceed, can you please give
> some suggestion?

You should make SCPSYS (0x10006000, AFAIU) a proper driver with its own
compatible (followed by syscon if needed), even if now it is almost
empty stub. The driver should populate OF children and then you can
embed SPM inside and reference to parent's regmap. No need for
simple-mfd. Later the SCPSYS can grow, if you ever need it.



Best regards,
Krzysztof

Re: [PATCH v1 08/16] arm64: dts: mt8195: Add power domains controller

[AngeloGioacchino Del Regno]

Il 12/07/22 11:03, Krzysztof Kozlowski ha scritto:
> On 12/07/2022 10:53, AngeloGioacchino Del Regno wrote:
>> Il 12/07/22 10:37, Krzysztof Kozlowski ha scritto:
>>> On 12/07/2022 10:17, AngeloGioacchino Del Regno wrote:
>>>> Il 06/07/22 17:18, Krzysztof Kozlowski ha scritto:
>>>>> On 06/07/2022 14:00, Tinghan Shen wrote:
>>>>>> Hi Krzysztof,
>>>>>>
>>>>>> After discussing your message with our power team,
>>>>>> we realized that we need your help to ensure we fully understand you.
>>>>>>
>>>>>> On Mon, 2022-07-04 at 14:38 +0200, Krzysztof Kozlowski wrote:
>>>>>>> On 04/07/2022 12:00, Tinghan Shen wrote:
>>>>>>>> Add power domains controller node for mt8195.
>>>>>>>>
>>>>>>>> Signed-off-by: Weiyi Lu <weiyi.lu@mediatek.com>
>>>>>>>> Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>
>>>>>>>> ---
>>>>>>>>     arch/arm64/boot/dts/mediatek/mt8195.dtsi | 327 +++++++++++++++++++++++
>>>>>>>>     1 file changed, 327 insertions(+)
>>>>>>>>
>>>>>>>> diff --git a/arch/arm64/boot/dts/mediatek/mt8195.dtsi b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
>>>>>>>> index 8d59a7da3271..d52e140d9271 100644
>>>>>>>> --- a/arch/arm64/boot/dts/mediatek/mt8195.dtsi
>>>>>>>> +++ b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
>>>>>>>> @@ -10,6 +10,7 @@
>>>>>>>>     #include <dt-bindings/interrupt-controller/irq.h>
>>>>>>>>     #include <dt-bindings/phy/phy.h>
>>>>>>>>     #include <dt-bindings/pinctrl/mt8195-pinfunc.h>
>>>>>>>> +#include <dt-bindings/power/mt8195-power.h>
>>>>>>>>     
>>>>>>>>     / {
>>>>>>>>     	compatible = "mediatek,mt8195";
>>>>>>>> @@ -338,6 +339,332 @@
>>>>>>>>     			#interrupt-cells = <2>;
>>>>>>>>     		};
>>>>>>>>     
>>>>>>>> +		scpsys: syscon@10006000 {
>>>>>>>> +			compatible = "syscon", "simple-mfd";
>>>>>>>
>>>>>>> These compatibles cannot be alone.
>>>>>>
>>>>>> the scpsys sub node has the compatible of the power domain driver.
>>>>>> do you suggest that the compatible in the sub node should move to here?
>>>>>
>>>>> Not necessarily, depends. You have here device node representing system
>>>>> registers. They need they own compatibles, just like everywhere in the
>>>>> kernel (except the broken cases...).
>>>>>
>>>>> Whether this should be compatible of power-domain driver, it depends
>>>>> what this device node is. I don't know, I don't have your datasheets or
>>>>> your architecture diagrams...
>>>>>
>>>>>>
>>>>>>>> +			reg = <0 0x10006000 0 0x1000>;
>>>>>>>> +			#power-domain-cells = <1>;
>>>>>>>
>>>>>>> If it is simple MFD, then probably it is not a power domain provider.
>>>>>>> Decide.
>>>>>>
>>>>>> this MFD device is the power controller on mt8195.
>>>>>
>>>>> Then it is not a simple MFD but a power controller. Do not use
>>>>> "simple-mfd" compatible.
>>>>>
>>>>>> Some features need
>>>>>> to do some operations on registers in this node. We think that implement
>>>>>> the operation of these registers as the MFD device can provide flexibility
>>>>>> for future use. We want to clarify if you're saying that an MFD device
>>>>>> cannot be a power domain provider.
>>>>>
>>>>> MFD device is Linuxism, so it has nothing to do here. I am talking only
>>>>> about simple-mfd. simple-mfd is a simple device only instantiating
>>>>> children and not providing anything to anyone. Neither to children. This
>>>>>     the most important part. The children do not depend on anything from
>>>>> simple-mfd device. For example simple-mfd device can be shut down
>>>>> (gated) and children should still operate. Being a power domain
>>>>> controller, contradicts this usually.
>>>>>
>>>>
>>>> If my interpretation of this issue is right, I have pushed a solution for it.
>>>> Krzysztof, Matthias, can you please check [1] and give feedback, so that
>>>> Tinghan can rewrite this commit ASAP?
>>>>
>>>> Reason is - I need the MT8195 devicetree to be complete to push the remaining
>>>> pieces for Tomato Chromebooks, of course.
>>>>
>>>> [1]: https://patchwork.kernel.org/project/linux-mediatek/list/?series=658527
>>>
>>> I have two or three similar discussions, so maybe I lost the context,
>>> but I don't understand how your fix is matching real hardware.
>>>
>>> In the patchset here, Tinghan claimed that power domain controller is a
>>> child of 10006000. 10006000 is also a power domain controller. This was
>>> explicitly described by the DTS code.
>>>
>>> Now you abandon this hierarchy in favor of syscon. If the hierarchy was
>>> correct, your patchset does not match the hardware, so it's a no-go.
>>> Describe the hardware.
>>>
>>> However maybe this patch did not make any sense and there is no
>>> relationship parent-child... so what do you guys send here? Bunch of
>>> hacks and work-arounds?
>>>
>>
>> For how I get it, hardware side, the SPM (System Power Manager) resides inside
>> of the SCPSYS block (consequently, in that iospace).
>>
>> As Matthias pointed out earlier, SCPSYS provides more functionality, but the
>> only one that's currently implemented upstream is the System Power Manager,
>> responsible for managing the MTCMOS (power domains).
>>
>> In any case, now I'm a little confused on how to proceed, can you please give
>> some suggestion?
> 
> You should make SCPSYS (0x10006000, AFAIU) a proper driver with its own
> compatible (followed by syscon if needed), even if now it is almost
> empty stub. The driver should populate OF children and then you can
> embed SPM inside and reference to parent's regmap. No need for
> simple-mfd. Later the SCPSYS can grow, if you ever need it.
> 
> 

I see an issue with such approach: the SCPSYS doesn't have a mailbox, doesn't
need power management from the Linux side, doesn't have any register to check
HW revision, it's always online (hence it doesn't need Linux to boot it), it
doesn't need any root clock, nor regulator, nor mmu context, and there's no
retrievable "boot log" of any sort.

Hence, a driver with its own compatible would be an empty stub forever: it's
not going to get any "scpsys root handling" at all, because there's none to do.

Digging through some downstream kernels, the only other functionality that I
can find in the SCPSYS is a MODULE_RESET (which is used to reset the SCP System)
and a INFRA_IRQ_SET, used to set "wake locks" on the AP and CONNSYS (modem).

Both of these may only be used in the SCP mailbox driver (which is *not* SCPSYS)
to perform an ipi_send operation (but currently we simply en/disable the clock
and that's enough), or to perform a reset and firmware reload of the SCP (but
currently we're simply powering off and back on: this may change in the future).

So, at the end of the day, we would end up having a copy of simple-pm-bus and
nothing else, which doesn't look like being optimal at all.

My own vision is that either using syscon (as shown in the series that you've
checked), keeping "simple-mfd", or changing it to "simple-bus" (whatever) is
the cleanest (and best approach) - please otherwise explain why having such
a practically forever-stub driver (practically, a copy of simple-pm-bus.c)
would be beneficial in any way.

Regards,
Angelo

> 
> Best regards,
> Krzysztof

Re: [PATCH v1 08/16] arm64: dts: mt8195: Add power domains controller

[Krzysztof Kozlowski]

On 12/07/2022 12:33, AngeloGioacchino Del Regno wrote:
> Il 12/07/22 11:03, Krzysztof Kozlowski ha scritto:
>> On 12/07/2022 10:53, AngeloGioacchino Del Regno wrote:
>>> Il 12/07/22 10:37, Krzysztof Kozlowski ha scritto:
>>>> On 12/07/2022 10:17, AngeloGioacchino Del Regno wrote:
>>>>> Il 06/07/22 17:18, Krzysztof Kozlowski ha scritto:
>>>>>> On 06/07/2022 14:00, Tinghan Shen wrote:
>>>>>>> Hi Krzysztof,
>>>>>>>
>>>>>>> After discussing your message with our power team,
>>>>>>> we realized that we need your help to ensure we fully understand you.
>>>>>>>
>>>>>>> On Mon, 2022-07-04 at 14:38 +0200, Krzysztof Kozlowski wrote:
>>>>>>>> On 04/07/2022 12:00, Tinghan Shen wrote:
>>>>>>>>> Add power domains controller node for mt8195.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Weiyi Lu <weiyi.lu@mediatek.com>
>>>>>>>>> Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>
>>>>>>>>> ---
>>>>>>>>>     arch/arm64/boot/dts/mediatek/mt8195.dtsi | 327 +++++++++++++++++++++++
>>>>>>>>>     1 file changed, 327 insertions(+)
>>>>>>>>>
>>>>>>>>> diff --git a/arch/arm64/boot/dts/mediatek/mt8195.dtsi b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
>>>>>>>>> index 8d59a7da3271..d52e140d9271 100644
>>>>>>>>> --- a/arch/arm64/boot/dts/mediatek/mt8195.dtsi
>>>>>>>>> +++ b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
>>>>>>>>> @@ -10,6 +10,7 @@
>>>>>>>>>     #include <dt-bindings/interrupt-controller/irq.h>
>>>>>>>>>     #include <dt-bindings/phy/phy.h>
>>>>>>>>>     #include <dt-bindings/pinctrl/mt8195-pinfunc.h>
>>>>>>>>> +#include <dt-bindings/power/mt8195-power.h>
>>>>>>>>>     
>>>>>>>>>     / {
>>>>>>>>>     	compatible = "mediatek,mt8195";
>>>>>>>>> @@ -338,6 +339,332 @@
>>>>>>>>>     			#interrupt-cells = <2>;
>>>>>>>>>     		};
>>>>>>>>>     
>>>>>>>>> +		scpsys: syscon@10006000 {
>>>>>>>>> +			compatible = "syscon", "simple-mfd";
>>>>>>>>
>>>>>>>> These compatibles cannot be alone.
>>>>>>>
>>>>>>> the scpsys sub node has the compatible of the power domain driver.
>>>>>>> do you suggest that the compatible in the sub node should move to here?
>>>>>>
>>>>>> Not necessarily, depends. You have here device node representing system
>>>>>> registers. They need they own compatibles, just like everywhere in the
>>>>>> kernel (except the broken cases...).
>>>>>>
>>>>>> Whether this should be compatible of power-domain driver, it depends
>>>>>> what this device node is. I don't know, I don't have your datasheets or
>>>>>> your architecture diagrams...
>>>>>>
>>>>>>>
>>>>>>>>> +			reg = <0 0x10006000 0 0x1000>;
>>>>>>>>> +			#power-domain-cells = <1>;
>>>>>>>>
>>>>>>>> If it is simple MFD, then probably it is not a power domain provider.
>>>>>>>> Decide.
>>>>>>>
>>>>>>> this MFD device is the power controller on mt8195.
>>>>>>
>>>>>> Then it is not a simple MFD but a power controller. Do not use
>>>>>> "simple-mfd" compatible.
>>>>>>
>>>>>>> Some features need
>>>>>>> to do some operations on registers in this node. We think that implement
>>>>>>> the operation of these registers as the MFD device can provide flexibility
>>>>>>> for future use. We want to clarify if you're saying that an MFD device
>>>>>>> cannot be a power domain provider.
>>>>>>
>>>>>> MFD device is Linuxism, so it has nothing to do here. I am talking only
>>>>>> about simple-mfd. simple-mfd is a simple device only instantiating
>>>>>> children and not providing anything to anyone. Neither to children. This
>>>>>>     the most important part. The children do not depend on anything from
>>>>>> simple-mfd device. For example simple-mfd device can be shut down
>>>>>> (gated) and children should still operate. Being a power domain
>>>>>> controller, contradicts this usually.
>>>>>>
>>>>>
>>>>> If my interpretation of this issue is right, I have pushed a solution for it.
>>>>> Krzysztof, Matthias, can you please check [1] and give feedback, so that
>>>>> Tinghan can rewrite this commit ASAP?
>>>>>
>>>>> Reason is - I need the MT8195 devicetree to be complete to push the remaining
>>>>> pieces for Tomato Chromebooks, of course.
>>>>>
>>>>> [1]: https://patchwork.kernel.org/project/linux-mediatek/list/?series=658527
>>>>
>>>> I have two or three similar discussions, so maybe I lost the context,
>>>> but I don't understand how your fix is matching real hardware.
>>>>
>>>> In the patchset here, Tinghan claimed that power domain controller is a
>>>> child of 10006000. 10006000 is also a power domain controller. This was
>>>> explicitly described by the DTS code.
>>>>
>>>> Now you abandon this hierarchy in favor of syscon. If the hierarchy was
>>>> correct, your patchset does not match the hardware, so it's a no-go.
>>>> Describe the hardware.
>>>>
>>>> However maybe this patch did not make any sense and there is no
>>>> relationship parent-child... so what do you guys send here? Bunch of
>>>> hacks and work-arounds?
>>>>
>>>
>>> For how I get it, hardware side, the SPM (System Power Manager) resides inside
>>> of the SCPSYS block (consequently, in that iospace).
>>>
>>> As Matthias pointed out earlier, SCPSYS provides more functionality, but the
>>> only one that's currently implemented upstream is the System Power Manager,
>>> responsible for managing the MTCMOS (power domains).
>>>
>>> In any case, now I'm a little confused on how to proceed, can you please give
>>> some suggestion?
>>
>> You should make SCPSYS (0x10006000, AFAIU) a proper driver with its own
>> compatible (followed by syscon if needed), even if now it is almost
>> empty stub. The driver should populate OF children and then you can
>> embed SPM inside and reference to parent's regmap. No need for
>> simple-mfd. Later the SCPSYS can grow, if you ever need it.
>>
>>
> 
> I see an issue with such approach: the SCPSYS doesn't have a mailbox, doesn't
> need power management from the Linux side, doesn't have any register to check
> HW revision, it's always online (hence it doesn't need Linux to boot it), it
> doesn't need any root clock, nor regulator, nor mmu context, and there's no
> retrievable "boot log" of any sort.

No problems, there are other drivers who do not need any resources,
except address space.

> 
> Hence, a driver with its own compatible would be an empty stub forever: it's
> not going to get any "scpsys root handling" at all, because there's none to do.

But it is a power domain provider, so you need to embed it in some
dirver, don't you?


> Digging through some downstream kernels, the only other functionality that I
> can find in the SCPSYS is a MODULE_RESET (which is used to reset the SCP System)
> and a INFRA_IRQ_SET, used to set "wake locks" on the AP and CONNSYS (modem).

So why was power domain provider added to SCPSYS? Guys, I don't know
your architecture, so I deduct it based on pieces of DTS code I see.

> 
> Both of these may only be used in the SCP mailbox driver (which is *not* SCPSYS)
> to perform an ipi_send operation (but currently we simply en/disable the clock
> and that's enough), or to perform a reset and firmware reload of the SCP (but
> currently we're simply powering off and back on: this may change in the future).
> 
> So, at the end of the day, we would end up having a copy of simple-pm-bus and
> nothing else, which doesn't look like being optimal at all.

No, because you need that power domain driver, don't you? If you don't
need power domain provider/driver, why the heck this is there:

+		scpsys: syscon@10006000 {
+			compatible = "syscon", "simple-mfd";
+			reg = <0 0x10006000 0 0x1000>;
+			#power-domain-cells = <1>;
                        ^^^^^^^^^^^^^^^^^
Entire discussion started from this.

> 
> My own vision is that either using syscon (as shown in the series that you've
> checked), keeping "simple-mfd", or changing it to "simple-bus" (whatever) is
> the cleanest (and best approach) - please otherwise explain why having such

Again, simple-mfd is just MFD, not a power domain provider.

simple-bus should not have it's own address space, so combining it with
syscon is rather wrong.

https://lore.kernel.org/linux-devicetree/Ynq52E93mcTXcw9H@robh.at.kernel.org/

> a practically forever-stub driver (practically, a copy of simple-pm-bus.c)
> would be beneficial in any way.
> 

Of course not, but your DTS is saying it is not a stub.

Best regards,
Krzysztof

Re: [PATCH v1 08/16] arm64: dts: mt8195: Add power domains controller

[AngeloGioacchino Del Regno]

Il 12/07/22 14:47, Krzysztof Kozlowski ha scritto:
> On 12/07/2022 12:33, AngeloGioacchino Del Regno wrote:
>> Il 12/07/22 11:03, Krzysztof Kozlowski ha scritto:
>>> On 12/07/2022 10:53, AngeloGioacchino Del Regno wrote:
>>>> Il 12/07/22 10:37, Krzysztof Kozlowski ha scritto:
>>>>> On 12/07/2022 10:17, AngeloGioacchino Del Regno wrote:
>>>>>> Il 06/07/22 17:18, Krzysztof Kozlowski ha scritto:
>>>>>>> On 06/07/2022 14:00, Tinghan Shen wrote:
>>>>>>>> Hi Krzysztof,
>>>>>>>>
>>>>>>>> After discussing your message with our power team,
>>>>>>>> we realized that we need your help to ensure we fully understand you.
>>>>>>>>
>>>>>>>> On Mon, 2022-07-04 at 14:38 +0200, Krzysztof Kozlowski wrote:
>>>>>>>>> On 04/07/2022 12:00, Tinghan Shen wrote:
>>>>>>>>>> Add power domains controller node for mt8195.
>>>>>>>>>>
>>>>>>>>>> Signed-off-by: Weiyi Lu <weiyi.lu@mediatek.com>
>>>>>>>>>> Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>
>>>>>>>>>> ---
>>>>>>>>>>      arch/arm64/boot/dts/mediatek/mt8195.dtsi | 327 +++++++++++++++++++++++
>>>>>>>>>>      1 file changed, 327 insertions(+)
>>>>>>>>>>
>>>>>>>>>> diff --git a/arch/arm64/boot/dts/mediatek/mt8195.dtsi b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
>>>>>>>>>> index 8d59a7da3271..d52e140d9271 100644
>>>>>>>>>> --- a/arch/arm64/boot/dts/mediatek/mt8195.dtsi
>>>>>>>>>> +++ b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
>>>>>>>>>> @@ -10,6 +10,7 @@
>>>>>>>>>>      #include <dt-bindings/interrupt-controller/irq.h>
>>>>>>>>>>      #include <dt-bindings/phy/phy.h>
>>>>>>>>>>      #include <dt-bindings/pinctrl/mt8195-pinfunc.h>
>>>>>>>>>> +#include <dt-bindings/power/mt8195-power.h>
>>>>>>>>>>      
>>>>>>>>>>      / {
>>>>>>>>>>      	compatible = "mediatek,mt8195";
>>>>>>>>>> @@ -338,6 +339,332 @@
>>>>>>>>>>      			#interrupt-cells = <2>;
>>>>>>>>>>      		};
>>>>>>>>>>      
>>>>>>>>>> +		scpsys: syscon@10006000 {
>>>>>>>>>> +			compatible = "syscon", "simple-mfd";
>>>>>>>>>
>>>>>>>>> These compatibles cannot be alone.
>>>>>>>>
>>>>>>>> the scpsys sub node has the compatible of the power domain driver.
>>>>>>>> do you suggest that the compatible in the sub node should move to here?
>>>>>>>
>>>>>>> Not necessarily, depends. You have here device node representing system
>>>>>>> registers. They need they own compatibles, just like everywhere in the
>>>>>>> kernel (except the broken cases...).
>>>>>>>
>>>>>>> Whether this should be compatible of power-domain driver, it depends
>>>>>>> what this device node is. I don't know, I don't have your datasheets or
>>>>>>> your architecture diagrams...
>>>>>>>
>>>>>>>>
>>>>>>>>>> +			reg = <0 0x10006000 0 0x1000>;
>>>>>>>>>> +			#power-domain-cells = <1>;
>>>>>>>>>
>>>>>>>>> If it is simple MFD, then probably it is not a power domain provider.
>>>>>>>>> Decide.
>>>>>>>>
>>>>>>>> this MFD device is the power controller on mt8195.
>>>>>>>
>>>>>>> Then it is not a simple MFD but a power controller. Do not use
>>>>>>> "simple-mfd" compatible.
>>>>>>>
>>>>>>>> Some features need
>>>>>>>> to do some operations on registers in this node. We think that implement
>>>>>>>> the operation of these registers as the MFD device can provide flexibility
>>>>>>>> for future use. We want to clarify if you're saying that an MFD device
>>>>>>>> cannot be a power domain provider.
>>>>>>>
>>>>>>> MFD device is Linuxism, so it has nothing to do here. I am talking only
>>>>>>> about simple-mfd. simple-mfd is a simple device only instantiating
>>>>>>> children and not providing anything to anyone. Neither to children. This
>>>>>>>      the most important part. The children do not depend on anything from
>>>>>>> simple-mfd device. For example simple-mfd device can be shut down
>>>>>>> (gated) and children should still operate. Being a power domain
>>>>>>> controller, contradicts this usually.
>>>>>>>
>>>>>>
>>>>>> If my interpretation of this issue is right, I have pushed a solution for it.
>>>>>> Krzysztof, Matthias, can you please check [1] and give feedback, so that
>>>>>> Tinghan can rewrite this commit ASAP?
>>>>>>
>>>>>> Reason is - I need the MT8195 devicetree to be complete to push the remaining
>>>>>> pieces for Tomato Chromebooks, of course.
>>>>>>
>>>>>> [1]: https://patchwork.kernel.org/project/linux-mediatek/list/?series=658527
>>>>>
>>>>> I have two or three similar discussions, so maybe I lost the context,
>>>>> but I don't understand how your fix is matching real hardware.
>>>>>
>>>>> In the patchset here, Tinghan claimed that power domain controller is a
>>>>> child of 10006000. 10006000 is also a power domain controller. This was
>>>>> explicitly described by the DTS code.
>>>>>
>>>>> Now you abandon this hierarchy in favor of syscon. If the hierarchy was
>>>>> correct, your patchset does not match the hardware, so it's a no-go.
>>>>> Describe the hardware.
>>>>>
>>>>> However maybe this patch did not make any sense and there is no
>>>>> relationship parent-child... so what do you guys send here? Bunch of
>>>>> hacks and work-arounds?
>>>>>
>>>>
>>>> For how I get it, hardware side, the SPM (System Power Manager) resides inside
>>>> of the SCPSYS block (consequently, in that iospace).
>>>>
>>>> As Matthias pointed out earlier, SCPSYS provides more functionality, but the
>>>> only one that's currently implemented upstream is the System Power Manager,
>>>> responsible for managing the MTCMOS (power domains).
>>>>
>>>> In any case, now I'm a little confused on how to proceed, can you please give
>>>> some suggestion?
>>>
>>> You should make SCPSYS (0x10006000, AFAIU) a proper driver with its own
>>> compatible (followed by syscon if needed), even if now it is almost
>>> empty stub. The driver should populate OF children and then you can
>>> embed SPM inside and reference to parent's regmap. No need for
>>> simple-mfd. Later the SCPSYS can grow, if you ever need it.
>>>
>>>
>>
>> I see an issue with such approach: the SCPSYS doesn't have a mailbox, doesn't
>> need power management from the Linux side, doesn't have any register to check
>> HW revision, it's always online (hence it doesn't need Linux to boot it), it
>> doesn't need any root clock, nor regulator, nor mmu context, and there's no
>> retrievable "boot log" of any sort.
> 
> No problems, there are other drivers who do not need any resources,
> except address space.
> 
>>
>> Hence, a driver with its own compatible would be an empty stub forever: it's
>> not going to get any "scpsys root handling" at all, because there's none to do.
> 
> But it is a power domain provider, so you need to embed it in some
> dirver, don't you?
> 
> 
>> Digging through some downstream kernels, the only other functionality that I
>> can find in the SCPSYS is a MODULE_RESET (which is used to reset the SCP System)
>> and a INFRA_IRQ_SET, used to set "wake locks" on the AP and CONNSYS (modem).
> 
> So why was power domain provider added to SCPSYS? Guys, I don't know
> your architecture, so I deduct it based on pieces of DTS code I see.
> 
>>
>> Both of these may only be used in the SCP mailbox driver (which is *not* SCPSYS)
>> to perform an ipi_send operation (but currently we simply en/disable the clock
>> and that's enough), or to perform a reset and firmware reload of the SCP (but
>> currently we're simply powering off and back on: this may change in the future).
>>
>> So, at the end of the day, we would end up having a copy of simple-pm-bus and
>> nothing else, which doesn't look like being optimal at all.
> 
> No, because you need that power domain driver, don't you? If you don't
> need power domain provider/driver, why the heck this is there:
> 
> +		scpsys: syscon@10006000 {
> +			compatible = "syscon", "simple-mfd";
> +			reg = <0 0x10006000 0 0x1000>;
> +			#power-domain-cells = <1>;
>                          ^^^^^^^^^^^^^^^^^
> Entire discussion started from this.
> 

Is this all a huge misunderstanding? It probably is, at least partially.

That node shouldn't have any #power-domain-cells, the only PD is the SPM node
(mediatek,mt8195-power-controller), not the scpsys parent! Ugh...

>>
>> My own vision is that either using syscon (as shown in the series that you've
>> checked), keeping "simple-mfd", or changing it to "simple-bus" (whatever) is
>> the cleanest (and best approach) - please otherwise explain why having such
> 
> Again, simple-mfd is just MFD, not a power domain provider.
> 
> simple-bus should not have it's own address space, so combining it with
> syscon is rather wrong.
> 
> https://lore.kernel.org/linux-devicetree/Ynq52E93mcTXcw9H@robh.at.kernel.org/
> 
>> a practically forever-stub driver (practically, a copy of simple-pm-bus.c)
>> would be beneficial in any way.
>>
> 
> Of course not, but your DTS is saying it is not a stub.
> 
> Best regards,
> Krzysztof

Re: [PATCH v1 08/16] arm64: dts: mt8195: Add power domains controller

[Krzysztof Kozlowski]

On 12/07/2022 14:54, AngeloGioacchino Del Regno wrote:
> Il 12/07/22 14:47, Krzysztof Kozlowski ha scritto:
>> On 12/07/2022 12:33, AngeloGioacchino Del Regno wrote:
>>> Il 12/07/22 11:03, Krzysztof Kozlowski ha scritto:
>>>> On 12/07/2022 10:53, AngeloGioacchino Del Regno wrote:
>>>>> Il 12/07/22 10:37, Krzysztof Kozlowski ha scritto:
>>>>>> On 12/07/2022 10:17, AngeloGioacchino Del Regno wrote:
>>>>>>> Il 06/07/22 17:18, Krzysztof Kozlowski ha scritto:
>>>>>>>> On 06/07/2022 14:00, Tinghan Shen wrote:
>>>>>>>>> Hi Krzysztof,
>>>>>>>>>
>>>>>>>>> After discussing your message with our power team,
>>>>>>>>> we realized that we need your help to ensure we fully understand you.
>>>>>>>>>
>>>>>>>>> On Mon, 2022-07-04 at 14:38 +0200, Krzysztof Kozlowski wrote:
>>>>>>>>>> On 04/07/2022 12:00, Tinghan Shen wrote:
>>>>>>>>>>> Add power domains controller node for mt8195.
>>>>>>>>>>>
>>>>>>>>>>> Signed-off-by: Weiyi Lu <weiyi.lu@mediatek.com>
>>>>>>>>>>> Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>
>>>>>>>>>>> ---
>>>>>>>>>>>      arch/arm64/boot/dts/mediatek/mt8195.dtsi | 327 +++++++++++++++++++++++
>>>>>>>>>>>      1 file changed, 327 insertions(+)
>>>>>>>>>>>
>>>>>>>>>>> diff --git a/arch/arm64/boot/dts/mediatek/mt8195.dtsi b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
>>>>>>>>>>> index 8d59a7da3271..d52e140d9271 100644
>>>>>>>>>>> --- a/arch/arm64/boot/dts/mediatek/mt8195.dtsi
>>>>>>>>>>> +++ b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
>>>>>>>>>>> @@ -10,6 +10,7 @@
>>>>>>>>>>>      #include <dt-bindings/interrupt-controller/irq.h>
>>>>>>>>>>>      #include <dt-bindings/phy/phy.h>
>>>>>>>>>>>      #include <dt-bindings/pinctrl/mt8195-pinfunc.h>
>>>>>>>>>>> +#include <dt-bindings/power/mt8195-power.h>
>>>>>>>>>>>      
>>>>>>>>>>>      / {
>>>>>>>>>>>      	compatible = "mediatek,mt8195";
>>>>>>>>>>> @@ -338,6 +339,332 @@
>>>>>>>>>>>      			#interrupt-cells = <2>;
>>>>>>>>>>>      		};
>>>>>>>>>>>      
>>>>>>>>>>> +		scpsys: syscon@10006000 {
>>>>>>>>>>> +			compatible = "syscon", "simple-mfd";
>>>>>>>>>>
>>>>>>>>>> These compatibles cannot be alone.
>>>>>>>>>
>>>>>>>>> the scpsys sub node has the compatible of the power domain driver.
>>>>>>>>> do you suggest that the compatible in the sub node should move to here?
>>>>>>>>
>>>>>>>> Not necessarily, depends. You have here device node representing system
>>>>>>>> registers. They need they own compatibles, just like everywhere in the
>>>>>>>> kernel (except the broken cases...).
>>>>>>>>
>>>>>>>> Whether this should be compatible of power-domain driver, it depends
>>>>>>>> what this device node is. I don't know, I don't have your datasheets or
>>>>>>>> your architecture diagrams...
>>>>>>>>
>>>>>>>>>
>>>>>>>>>>> +			reg = <0 0x10006000 0 0x1000>;
>>>>>>>>>>> +			#power-domain-cells = <1>;
>>>>>>>>>>
>>>>>>>>>> If it is simple MFD, then probably it is not a power domain provider.
>>>>>>>>>> Decide.
>>>>>>>>>
>>>>>>>>> this MFD device is the power controller on mt8195.
>>>>>>>>
>>>>>>>> Then it is not a simple MFD but a power controller. Do not use
>>>>>>>> "simple-mfd" compatible.
>>>>>>>>
>>>>>>>>> Some features need
>>>>>>>>> to do some operations on registers in this node. We think that implement
>>>>>>>>> the operation of these registers as the MFD device can provide flexibility
>>>>>>>>> for future use. We want to clarify if you're saying that an MFD device
>>>>>>>>> cannot be a power domain provider.
>>>>>>>>
>>>>>>>> MFD device is Linuxism, so it has nothing to do here. I am talking only
>>>>>>>> about simple-mfd. simple-mfd is a simple device only instantiating
>>>>>>>> children and not providing anything to anyone. Neither to children. This
>>>>>>>>      the most important part. The children do not depend on anything from
>>>>>>>> simple-mfd device. For example simple-mfd device can be shut down
>>>>>>>> (gated) and children should still operate. Being a power domain
>>>>>>>> controller, contradicts this usually.
>>>>>>>>
>>>>>>>
>>>>>>> If my interpretation of this issue is right, I have pushed a solution for it.
>>>>>>> Krzysztof, Matthias, can you please check [1] and give feedback, so that
>>>>>>> Tinghan can rewrite this commit ASAP?
>>>>>>>
>>>>>>> Reason is - I need the MT8195 devicetree to be complete to push the remaining
>>>>>>> pieces for Tomato Chromebooks, of course.
>>>>>>>
>>>>>>> [1]: https://patchwork.kernel.org/project/linux-mediatek/list/?series=658527
>>>>>>
>>>>>> I have two or three similar discussions, so maybe I lost the context,
>>>>>> but I don't understand how your fix is matching real hardware.
>>>>>>
>>>>>> In the patchset here, Tinghan claimed that power domain controller is a
>>>>>> child of 10006000. 10006000 is also a power domain controller. This was
>>>>>> explicitly described by the DTS code.
>>>>>>
>>>>>> Now you abandon this hierarchy in favor of syscon. If the hierarchy was
>>>>>> correct, your patchset does not match the hardware, so it's a no-go.
>>>>>> Describe the hardware.
>>>>>>
>>>>>> However maybe this patch did not make any sense and there is no
>>>>>> relationship parent-child... so what do you guys send here? Bunch of
>>>>>> hacks and work-arounds?
>>>>>>
>>>>>
>>>>> For how I get it, hardware side, the SPM (System Power Manager) resides inside
>>>>> of the SCPSYS block (consequently, in that iospace).
>>>>>
>>>>> As Matthias pointed out earlier, SCPSYS provides more functionality, but the
>>>>> only one that's currently implemented upstream is the System Power Manager,
>>>>> responsible for managing the MTCMOS (power domains).
>>>>>
>>>>> In any case, now I'm a little confused on how to proceed, can you please give
>>>>> some suggestion?
>>>>
>>>> You should make SCPSYS (0x10006000, AFAIU) a proper driver with its own
>>>> compatible (followed by syscon if needed), even if now it is almost
>>>> empty stub. The driver should populate OF children and then you can
>>>> embed SPM inside and reference to parent's regmap. No need for
>>>> simple-mfd. Later the SCPSYS can grow, if you ever need it.
>>>>
>>>>
>>>
>>> I see an issue with such approach: the SCPSYS doesn't have a mailbox, doesn't
>>> need power management from the Linux side, doesn't have any register to check
>>> HW revision, it's always online (hence it doesn't need Linux to boot it), it
>>> doesn't need any root clock, nor regulator, nor mmu context, and there's no
>>> retrievable "boot log" of any sort.
>>
>> No problems, there are other drivers who do not need any resources,
>> except address space.
>>
>>>
>>> Hence, a driver with its own compatible would be an empty stub forever: it's
>>> not going to get any "scpsys root handling" at all, because there's none to do.
>>
>> But it is a power domain provider, so you need to embed it in some
>> dirver, don't you?
>>
>>
>>> Digging through some downstream kernels, the only other functionality that I
>>> can find in the SCPSYS is a MODULE_RESET (which is used to reset the SCP System)
>>> and a INFRA_IRQ_SET, used to set "wake locks" on the AP and CONNSYS (modem).
>>
>> So why was power domain provider added to SCPSYS? Guys, I don't know
>> your architecture, so I deduct it based on pieces of DTS code I see.
>>
>>>
>>> Both of these may only be used in the SCP mailbox driver (which is *not* SCPSYS)
>>> to perform an ipi_send operation (but currently we simply en/disable the clock
>>> and that's enough), or to perform a reset and firmware reload of the SCP (but
>>> currently we're simply powering off and back on: this may change in the future).
>>>
>>> So, at the end of the day, we would end up having a copy of simple-pm-bus and
>>> nothing else, which doesn't look like being optimal at all.
>>
>> No, because you need that power domain driver, don't you? If you don't
>> need power domain provider/driver, why the heck this is there:
>>
>> +		scpsys: syscon@10006000 {
>> +			compatible = "syscon", "simple-mfd";
>> +			reg = <0 0x10006000 0 0x1000>;
>> +			#power-domain-cells = <1>;
>>                          ^^^^^^^^^^^^^^^^^
>> Entire discussion started from this.
>>
> 
> Is this all a huge misunderstanding? It probably is, at least partially.
> 
> That node shouldn't have any #power-domain-cells, the only PD is the SPM node
> (mediatek,mt8195-power-controller), not the scpsys parent! Ugh...

My comment was quite clear I think:

  > +			#power-domain-cells = <1>;
  If it is simple MFD, then probably it is not a power domain provider.
  Decide.

and you keep telling me that it is a power domain provider and MFD and
something more.

Anyway neither syscon nor simple-mfd can be without specific compatible.

Best regards,
Krzysztof

Re: [PATCH v1 08/16] arm64: dts: mt8195: Add power domains controller

[AngeloGioacchino Del Regno]

Il 12/07/22 14:58, Krzysztof Kozlowski ha scritto:
> On 12/07/2022 14:54, AngeloGioacchino Del Regno wrote:
>> Il 12/07/22 14:47, Krzysztof Kozlowski ha scritto:
>>> On 12/07/2022 12:33, AngeloGioacchino Del Regno wrote:
>>>> Il 12/07/22 11:03, Krzysztof Kozlowski ha scritto:
>>>>> On 12/07/2022 10:53, AngeloGioacchino Del Regno wrote:
>>>>>> Il 12/07/22 10:37, Krzysztof Kozlowski ha scritto:
>>>>>>> On 12/07/2022 10:17, AngeloGioacchino Del Regno wrote:
>>>>>>>> Il 06/07/22 17:18, Krzysztof Kozlowski ha scritto:
>>>>>>>>> On 06/07/2022 14:00, Tinghan Shen wrote:
>>>>>>>>>> Hi Krzysztof,
>>>>>>>>>>
>>>>>>>>>> After discussing your message with our power team,
>>>>>>>>>> we realized that we need your help to ensure we fully understand you.
>>>>>>>>>>
>>>>>>>>>> On Mon, 2022-07-04 at 14:38 +0200, Krzysztof Kozlowski wrote:
>>>>>>>>>>> On 04/07/2022 12:00, Tinghan Shen wrote:
>>>>>>>>>>>> Add power domains controller node for mt8195.
>>>>>>>>>>>>
>>>>>>>>>>>> Signed-off-by: Weiyi Lu <weiyi.lu@mediatek.com>
>>>>>>>>>>>> Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>
>>>>>>>>>>>> ---
>>>>>>>>>>>>       arch/arm64/boot/dts/mediatek/mt8195.dtsi | 327 +++++++++++++++++++++++
>>>>>>>>>>>>       1 file changed, 327 insertions(+)
>>>>>>>>>>>>
>>>>>>>>>>>> diff --git a/arch/arm64/boot/dts/mediatek/mt8195.dtsi b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
>>>>>>>>>>>> index 8d59a7da3271..d52e140d9271 100644
>>>>>>>>>>>> --- a/arch/arm64/boot/dts/mediatek/mt8195.dtsi
>>>>>>>>>>>> +++ b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
>>>>>>>>>>>> @@ -10,6 +10,7 @@
>>>>>>>>>>>>       #include <dt-bindings/interrupt-controller/irq.h>
>>>>>>>>>>>>       #include <dt-bindings/phy/phy.h>
>>>>>>>>>>>>       #include <dt-bindings/pinctrl/mt8195-pinfunc.h>
>>>>>>>>>>>> +#include <dt-bindings/power/mt8195-power.h>
>>>>>>>>>>>>       
>>>>>>>>>>>>       / {
>>>>>>>>>>>>       	compatible = "mediatek,mt8195";
>>>>>>>>>>>> @@ -338,6 +339,332 @@
>>>>>>>>>>>>       			#interrupt-cells = <2>;
>>>>>>>>>>>>       		};
>>>>>>>>>>>>       
>>>>>>>>>>>> +		scpsys: syscon@10006000 {
>>>>>>>>>>>> +			compatible = "syscon", "simple-mfd";
>>>>>>>>>>>
>>>>>>>>>>> These compatibles cannot be alone.
>>>>>>>>>>
>>>>>>>>>> the scpsys sub node has the compatible of the power domain driver.
>>>>>>>>>> do you suggest that the compatible in the sub node should move to here?
>>>>>>>>>
>>>>>>>>> Not necessarily, depends. You have here device node representing system
>>>>>>>>> registers. They need they own compatibles, just like everywhere in the
>>>>>>>>> kernel (except the broken cases...).
>>>>>>>>>
>>>>>>>>> Whether this should be compatible of power-domain driver, it depends
>>>>>>>>> what this device node is. I don't know, I don't have your datasheets or
>>>>>>>>> your architecture diagrams...
>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>>> +			reg = <0 0x10006000 0 0x1000>;
>>>>>>>>>>>> +			#power-domain-cells = <1>;
>>>>>>>>>>>
>>>>>>>>>>> If it is simple MFD, then probably it is not a power domain provider.
>>>>>>>>>>> Decide.
>>>>>>>>>>
>>>>>>>>>> this MFD device is the power controller on mt8195.
>>>>>>>>>
>>>>>>>>> Then it is not a simple MFD but a power controller. Do not use
>>>>>>>>> "simple-mfd" compatible.
>>>>>>>>>
>>>>>>>>>> Some features need
>>>>>>>>>> to do some operations on registers in this node. We think that implement
>>>>>>>>>> the operation of these registers as the MFD device can provide flexibility
>>>>>>>>>> for future use. We want to clarify if you're saying that an MFD device
>>>>>>>>>> cannot be a power domain provider.
>>>>>>>>>
>>>>>>>>> MFD device is Linuxism, so it has nothing to do here. I am talking only
>>>>>>>>> about simple-mfd. simple-mfd is a simple device only instantiating
>>>>>>>>> children and not providing anything to anyone. Neither to children. This
>>>>>>>>>       the most important part. The children do not depend on anything from
>>>>>>>>> simple-mfd device. For example simple-mfd device can be shut down
>>>>>>>>> (gated) and children should still operate. Being a power domain
>>>>>>>>> controller, contradicts this usually.
>>>>>>>>>
>>>>>>>>
>>>>>>>> If my interpretation of this issue is right, I have pushed a solution for it.
>>>>>>>> Krzysztof, Matthias, can you please check [1] and give feedback, so that
>>>>>>>> Tinghan can rewrite this commit ASAP?
>>>>>>>>
>>>>>>>> Reason is - I need the MT8195 devicetree to be complete to push the remaining
>>>>>>>> pieces for Tomato Chromebooks, of course.
>>>>>>>>
>>>>>>>> [1]: https://patchwork.kernel.org/project/linux-mediatek/list/?series=658527
>>>>>>>
>>>>>>> I have two or three similar discussions, so maybe I lost the context,
>>>>>>> but I don't understand how your fix is matching real hardware.
>>>>>>>
>>>>>>> In the patchset here, Tinghan claimed that power domain controller is a
>>>>>>> child of 10006000. 10006000 is also a power domain controller. This was
>>>>>>> explicitly described by the DTS code.
>>>>>>>
>>>>>>> Now you abandon this hierarchy in favor of syscon. If the hierarchy was
>>>>>>> correct, your patchset does not match the hardware, so it's a no-go.
>>>>>>> Describe the hardware.
>>>>>>>
>>>>>>> However maybe this patch did not make any sense and there is no
>>>>>>> relationship parent-child... so what do you guys send here? Bunch of
>>>>>>> hacks and work-arounds?
>>>>>>>
>>>>>>
>>>>>> For how I get it, hardware side, the SPM (System Power Manager) resides inside
>>>>>> of the SCPSYS block (consequently, in that iospace).
>>>>>>
>>>>>> As Matthias pointed out earlier, SCPSYS provides more functionality, but the
>>>>>> only one that's currently implemented upstream is the System Power Manager,
>>>>>> responsible for managing the MTCMOS (power domains).
>>>>>>
>>>>>> In any case, now I'm a little confused on how to proceed, can you please give
>>>>>> some suggestion?
>>>>>
>>>>> You should make SCPSYS (0x10006000, AFAIU) a proper driver with its own
>>>>> compatible (followed by syscon if needed), even if now it is almost
>>>>> empty stub. The driver should populate OF children and then you can
>>>>> embed SPM inside and reference to parent's regmap. No need for
>>>>> simple-mfd. Later the SCPSYS can grow, if you ever need it.
>>>>>
>>>>>
>>>>
>>>> I see an issue with such approach: the SCPSYS doesn't have a mailbox, doesn't
>>>> need power management from the Linux side, doesn't have any register to check
>>>> HW revision, it's always online (hence it doesn't need Linux to boot it), it
>>>> doesn't need any root clock, nor regulator, nor mmu context, and there's no
>>>> retrievable "boot log" of any sort.
>>>
>>> No problems, there are other drivers who do not need any resources,
>>> except address space.
>>>
>>>>
>>>> Hence, a driver with its own compatible would be an empty stub forever: it's
>>>> not going to get any "scpsys root handling" at all, because there's none to do.
>>>
>>> But it is a power domain provider, so you need to embed it in some
>>> dirver, don't you?
>>>
>>>
>>>> Digging through some downstream kernels, the only other functionality that I
>>>> can find in the SCPSYS is a MODULE_RESET (which is used to reset the SCP System)
>>>> and a INFRA_IRQ_SET, used to set "wake locks" on the AP and CONNSYS (modem).
>>>
>>> So why was power domain provider added to SCPSYS? Guys, I don't know
>>> your architecture, so I deduct it based on pieces of DTS code I see.
>>>
>>>>
>>>> Both of these may only be used in the SCP mailbox driver (which is *not* SCPSYS)
>>>> to perform an ipi_send operation (but currently we simply en/disable the clock
>>>> and that's enough), or to perform a reset and firmware reload of the SCP (but
>>>> currently we're simply powering off and back on: this may change in the future).
>>>>
>>>> So, at the end of the day, we would end up having a copy of simple-pm-bus and
>>>> nothing else, which doesn't look like being optimal at all.
>>>
>>> No, because you need that power domain driver, don't you? If you don't
>>> need power domain provider/driver, why the heck this is there:
>>>
>>> +		scpsys: syscon@10006000 {
>>> +			compatible = "syscon", "simple-mfd";
>>> +			reg = <0 0x10006000 0 0x1000>;
>>> +			#power-domain-cells = <1>;
>>>                           ^^^^^^^^^^^^^^^^^
>>> Entire discussion started from this.
>>>
>>
>> Is this all a huge misunderstanding? It probably is, at least partially.
>>
>> That node shouldn't have any #power-domain-cells, the only PD is the SPM node
>> (mediatek,mt8195-power-controller), not the scpsys parent! Ugh...
> 
> My comment was quite clear I think:
> 
>    > +			#power-domain-cells = <1>;
>    If it is simple MFD, then probably it is not a power domain provider.
>    Decide.

Yes it was quite clear. It's entirely my fault for misreading that part and
I'm truly sorry for that.

> 
> and you keep telling me that it is a power domain provider and MFD and
> something more.
> 
> Anyway neither syscon nor simple-mfd can be without specific compatible.
> 

I believe, at this point, that adding a compatible like "mediatek,scpsys" in
mfd/syscon.yaml should do?


> Best regards,
> Krzysztof

Re: [PATCH v1 08/16] arm64: dts: mt8195: Add power domains controller

[Krzysztof Kozlowski]

On 12/07/2022 15:03, AngeloGioacchino Del Regno wrote:
>> and you keep telling me that it is a power domain provider and MFD and
>> something more.
>>
>> Anyway neither syscon nor simple-mfd can be without specific compatible.
>>
> 
> I believe, at this point, that adding a compatible like "mediatek,scpsys" in
> mfd/syscon.yaml should do?

Yes. Or dedicated file, like other mediatek syscons.


Best regards,
Krzysztof

Re: [PATCH v1 08/16] arm64: dts: mt8195: Add power domains controller

[Matthias Brugger]

On 04/07/2022 14:38, Krzysztof Kozlowski wrote:
> On 04/07/2022 12:00, Tinghan Shen wrote:
>> Add power domains controller node for mt8195.
>>
>> Signed-off-by: Weiyi Lu <weiyi.lu@mediatek.com>
>> Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>
>> ---
>>   arch/arm64/boot/dts/mediatek/mt8195.dtsi | 327 +++++++++++++++++++++++
>>   1 file changed, 327 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/mediatek/mt8195.dtsi b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
>> index 8d59a7da3271..d52e140d9271 100644
>> --- a/arch/arm64/boot/dts/mediatek/mt8195.dtsi
>> +++ b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
>> @@ -10,6 +10,7 @@
>>   #include <dt-bindings/interrupt-controller/irq.h>
>>   #include <dt-bindings/phy/phy.h>
>>   #include <dt-bindings/pinctrl/mt8195-pinfunc.h>
>> +#include <dt-bindings/power/mt8195-power.h>
>>   
>>   / {
>>   	compatible = "mediatek,mt8195";
>> @@ -338,6 +339,332 @@
>>   			#interrupt-cells = <2>;
>>   		};
>>   
>> +		scpsys: syscon@10006000 {
>> +			compatible = "syscon", "simple-mfd";
> 
> These compatibles cannot be alone.
> 

You mean we would need something like "mediatek,scpsys" as dummy compatible 
that's not bound to any driver?

>> +			reg = <0 0x10006000 0 0x1000>;
>> +			#power-domain-cells = <1>;
> 
> If it is simple MFD, then probably it is not a power domain provider.
> Decide.

The SCPSYS IP block of MediaTek SoCs group several functionality, one is the 
power domain controller. Others are not yet implemented, but defining the scpsys 
as a MFD will give us the possibility to do so in the future.

Regards,
Matthias

> 
> Best regards,
> Krzysztof

Re: [PATCH v1 08/16] arm64: dts: mt8195: Add power domains controller

[Krzysztof Kozlowski]

On 06/07/2022 15:41, Matthias Brugger wrote:
> 
> 
> On 04/07/2022 14:38, Krzysztof Kozlowski wrote:
>> On 04/07/2022 12:00, Tinghan Shen wrote:
>>> Add power domains controller node for mt8195.
>>>
>>> Signed-off-by: Weiyi Lu <weiyi.lu@mediatek.com>
>>> Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>
>>> ---
>>>   arch/arm64/boot/dts/mediatek/mt8195.dtsi | 327 +++++++++++++++++++++++
>>>   1 file changed, 327 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/mediatek/mt8195.dtsi b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
>>> index 8d59a7da3271..d52e140d9271 100644
>>> --- a/arch/arm64/boot/dts/mediatek/mt8195.dtsi
>>> +++ b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
>>> @@ -10,6 +10,7 @@
>>>   #include <dt-bindings/interrupt-controller/irq.h>
>>>   #include <dt-bindings/phy/phy.h>
>>>   #include <dt-bindings/pinctrl/mt8195-pinfunc.h>
>>> +#include <dt-bindings/power/mt8195-power.h>
>>>   
>>>   / {
>>>   	compatible = "mediatek,mt8195";
>>> @@ -338,6 +339,332 @@
>>>   			#interrupt-cells = <2>;
>>>   		};
>>>   
>>> +		scpsys: syscon@10006000 {
>>> +			compatible = "syscon", "simple-mfd";
>>
>> These compatibles cannot be alone.
>>
> 
> You mean we would need something like "mediatek,scpsys" as dummy compatible 
> that's not bound to any driver?

Yes. syscon (and simple-mfd) must always come with a specific compatible.

> 
>>> +			reg = <0 0x10006000 0 0x1000>;
>>> +			#power-domain-cells = <1>;
>>
>> If it is simple MFD, then probably it is not a power domain provider.
>> Decide.
> 
> The SCPSYS IP block of MediaTek SoCs group several functionality, one is the 
> power domain controller. Others are not yet implemented, but defining the scpsys 
> as a MFD will give us the possibility to do so in the future.

No, quite the opposite. Having simple-mfd prevents you from implementing
it correctly later as a driver, because you cannot remove it. It would
be ABI break.

It's fine to have one block being a simple MFD having several children,
but then it's not a power controller. Children could be such power
controller, but not simple-mfd. Rob explained this several times:
https://lore.kernel.org/all/YXhINE00HG6hbQI4@robh.at.kernel.org/
https://lore.kernel.org/all/20220701000959.GA3588170-robh@kernel.org/


Best regards,
Krzysztof

[PATCH v1 09/16] arm64: dts: mt8195: Add spmi node

[Tinghan Shen]

Add spmi node to mt8195.

Signed-off-by: Henry Chen <henryc.chen@mediatek.com>
Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>
---
 arch/arm64/boot/dts/mediatek/mt8195.dtsi | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt8195.dtsi b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
index d52e140d9271..456612d9d508 100644
--- a/arch/arm64/boot/dts/mediatek/mt8195.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
@@ -698,6 +698,21 @@
 			assigned-clock-parents = <&topckgen CLK_TOP_ULPOSC1_D10>;
 		};
 
+		spmi: spmi@10027000 {
+			compatible = "mediatek,mt8195-spmi";
+			reg = <0 0x10027000 0 0x000e00>,
+			      <0 0x10029000 0 0x000100>;
+			reg-names = "pmif", "spmimst";
+			clocks = <&infracfg_ao CLK_INFRA_AO_PMIC_AP>,
+				 <&infracfg_ao CLK_INFRA_AO_PMIC_TMR>,
+				 <&topckgen CLK_TOP_SPMI_M_MST>;
+			clock-names = "pmif_sys_ck",
+				      "pmif_tmr_ck",
+				      "spmimst_clk_mux";
+			assigned-clocks = <&topckgen CLK_TOP_PWRAP_ULPOSC>;
+			assigned-clock-parents = <&topckgen CLK_TOP_ULPOSC1_D10>;
+		};
+
 		scp_adsp: clock-controller@10720000 {
 			compatible = "mediatek,mt8195-scp_adsp";
 			reg = <0 0x10720000 0 0x1000>;
-- 
2.18.0

Re: [PATCH v1 09/16] arm64: dts: mt8195: Add spmi node

[AngeloGioacchino Del Regno]

Il 04/07/22 12:00, Tinghan Shen ha scritto:
> Add spmi node to mt8195.
> 
> Signed-off-by: Henry Chen <henryc.chen@mediatek.com>
> Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

[PATCH v1 10/16] arm64: dts: mt8195: Add scp node

[Tinghan Shen]

Add scp node for mt8195.

Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>
---
 arch/arm64/boot/dts/mediatek/mt8195.dtsi | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt8195.dtsi b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
index 456612d9d508..543bb719a445 100644
--- a/arch/arm64/boot/dts/mediatek/mt8195.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
@@ -713,6 +713,16 @@
 			assigned-clock-parents = <&topckgen CLK_TOP_ULPOSC1_D10>;
 		};
 
+		scp: scp@10500000 {
+			compatible = "mediatek,mt8195-scp";
+			reg = <0 0x10500000 0 0x100000>,
+			      <0 0x10720000 0 0xe0000>,
+			      <0 0x10700000 0 0x8000>;
+			reg-names = "sram", "cfg", "l1tcm";
+			interrupts = <GIC_SPI 462 IRQ_TYPE_LEVEL_HIGH 0>;
+			status = "disabled";
+		};
+
 		scp_adsp: clock-controller@10720000 {
 			compatible = "mediatek,mt8195-scp_adsp";
 			reg = <0 0x10720000 0 0x1000>;
-- 
2.18.0

Re: [PATCH v1 10/16] arm64: dts: mt8195: Add scp node

[AngeloGioacchino Del Regno]

Il 04/07/22 12:00, Tinghan Shen ha scritto:
> Add scp node for mt8195.
> 
> Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

[PATCH v1 11/16] arm64: dts: mt8195: Add audio related nodes

[Tinghan Shen]

Add audio related nodes for mt8195.

Signed-off-by: Trevor Wu <trevor.wu@mediatek.com>
Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>
---
 arch/arm64/boot/dts/mediatek/mt8195.dtsi | 58 ++++++++++++++++++++++++
 1 file changed, 58 insertions(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt8195.dtsi b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
index 543bb719a445..1776f5dcde03 100644
--- a/arch/arm64/boot/dts/mediatek/mt8195.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
@@ -226,6 +226,17 @@
 		       <&cpu4>, <&cpu5>, <&cpu6>, <&cpu7>;
 	};
 
+	dmic_codec: dmic-codec {
+		compatible = "dmic-codec";
+		num-channels = <2>;
+		wakeup-delay-ms = <50>;
+	};
+
+	sound: mt8195-sound {
+		mediatek,platform = <&afe>;
+		status = "disabled";
+	};
+
 	clk26m: oscillator-26m {
 		compatible = "fixed-clock";
 		#clock-cells = <0>;
@@ -729,6 +740,53 @@
 			#clock-cells = <1>;
 		};
 
+		afe: mt8195-afe-pcm@10890000 {
+			compatible = "mediatek,mt8195-audio";
+			reg = <0 0x10890000 0 0x10000>;
+			mediatek,topckgen = <&topckgen>;
+			power-domains = <&spm MT8195_POWER_DOMAIN_AUDIO>;
+			interrupts = <GIC_SPI 822 IRQ_TYPE_LEVEL_HIGH 0>;
+			clocks = <&clk26m>,
+				<&apmixedsys CLK_APMIXED_APLL1>,
+				<&apmixedsys CLK_APMIXED_APLL2>,
+				<&topckgen CLK_TOP_APLL12_DIV0>,
+				<&topckgen CLK_TOP_APLL12_DIV1>,
+				<&topckgen CLK_TOP_APLL12_DIV2>,
+				<&topckgen CLK_TOP_APLL12_DIV3>,
+				<&topckgen CLK_TOP_APLL12_DIV9>,
+				<&topckgen CLK_TOP_A1SYS_HP>,
+				<&topckgen CLK_TOP_AUD_INTBUS>,
+				<&topckgen CLK_TOP_AUDIO_H>,
+				<&topckgen CLK_TOP_AUDIO_LOCAL_BUS>,
+				<&topckgen CLK_TOP_DPTX_MCK>,
+				<&topckgen CLK_TOP_I2SO1_MCK>,
+				<&topckgen CLK_TOP_I2SO2_MCK>,
+				<&topckgen CLK_TOP_I2SI1_MCK>,
+				<&topckgen CLK_TOP_I2SI2_MCK>,
+				<&infracfg_ao CLK_INFRA_AO_AUDIO_26M_B>,
+				<&scp_adsp CLK_SCP_ADSP_AUDIODSP>;
+			clock-names = "clk26m",
+				"apll1_ck",
+				"apll2_ck",
+				"apll12_div0",
+				"apll12_div1",
+				"apll12_div2",
+				"apll12_div3",
+				"apll12_div9",
+				"a1sys_hp_sel",
+				"aud_intbus_sel",
+				"audio_h_sel",
+				"audio_local_bus_sel",
+				"dptx_m_sel",
+				"i2so1_m_sel",
+				"i2so2_m_sel",
+				"i2si1_m_sel",
+				"i2si2_m_sel",
+				"infra_ao_audio_26m_b",
+				"scp_adsp_audiodsp";
+			status = "disabled";
+		};
+
 		uart0: serial@11001100 {
 			compatible = "mediatek,mt8195-uart",
 				     "mediatek,mt6577-uart";
-- 
2.18.0

Re: [PATCH v1 11/16] arm64: dts: mt8195: Add audio related nodes

[AngeloGioacchino Del Regno]

Il 04/07/22 12:00, Tinghan Shen ha scritto:
> Add audio related nodes for mt8195.
> 
> Signed-off-by: Trevor Wu <trevor.wu@mediatek.com>
> Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

[PATCH v1 12/16] arm64: dts: mt8195: Add adsp node and adsp mailbox nodes

[Tinghan Shen]

From: YC Hung <yc.hung@mediatek.corp-partner.google.com>

Add adsp node and adsp mailbox nodes for mt8195.

Signed-off-by: YC Hung <yc.hung@mediatek.corp-partner.google.com>
Signed-off-by: Allen-KH Cheng <allen-kh.cheng@mediatek.corp-partner.google.com>
Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>
---
 arch/arm64/boot/dts/mediatek/mt8195.dtsi | 37 ++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt8195.dtsi b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
index 1776f5dcde03..0e5614108c12 100644
--- a/arch/arm64/boot/dts/mediatek/mt8195.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
@@ -740,6 +740,43 @@
 			#clock-cells = <1>;
 		};
 
+		adsp: adsp@10803000 {
+			compatible = "mediatek,mt8195-dsp";
+			reg = <0 0x10803000 0 0x1000>,
+			      <0 0x10840000 0 0x40000>;
+			reg-names = "cfg", "sram";
+			clocks = <&topckgen CLK_TOP_ADSP>,
+				 <&clk26m>,
+				 <&topckgen CLK_TOP_AUDIO_LOCAL_BUS>,
+				 <&topckgen CLK_TOP_MAINPLL_D7_D2>,
+				 <&scp_adsp CLK_SCP_ADSP_AUDIODSP>,
+				 <&topckgen CLK_TOP_AUDIO_H>;
+			clock-names = "adsp_sel",
+				 "clk26m_ck",
+				 "audio_local_bus",
+				 "mainpll_d7_d2",
+				 "scp_adsp_audiodsp",
+				 "audio_h";
+			power-domains = <&spm MT8195_POWER_DOMAIN_ADSP>;
+			mbox-names = "rx", "tx";
+			mboxes = <&adsp_mailbox0>, <&adsp_mailbox1>;
+			status = "disabled";
+		};
+
+		adsp_mailbox0: mailbox@10816000 {
+			compatible = "mediatek,mt8195-adsp-mbox";
+			#mbox-cells = <0>;
+			reg = <0 0x10816000 0 0x1000>;
+			interrupts = <GIC_SPI 702 IRQ_TYPE_LEVEL_HIGH 0>;
+		};
+
+		adsp_mailbox1: mailbox@10817000 {
+			compatible = "mediatek,mt8195-adsp-mbox";
+			#mbox-cells = <0>;
+			reg = <0 0x10817000 0 0x1000>;
+			interrupts = <GIC_SPI 703 IRQ_TYPE_LEVEL_HIGH 0>;
+		};
+
 		afe: mt8195-afe-pcm@10890000 {
 			compatible = "mediatek,mt8195-audio";
 			reg = <0 0x10890000 0 0x10000>;
-- 
2.18.0

[PATCH v1 13/16] arm64: dts: mt8195: Specify audio reset controller

[Tinghan Shen]

From: Trevor Wu <trevor.wu@mediatek.com>

Specify audio reset controller for audio hardware resetting.

Signed-off-by: Trevor Wu <trevor.wu@mediatek.com>
Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>
---
 arch/arm64/boot/dts/mediatek/mt8195.dtsi | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt8195.dtsi b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
index 0e5614108c12..618fb2fa195a 100644
--- a/arch/arm64/boot/dts/mediatek/mt8195.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
@@ -681,6 +681,7 @@
 				     "mediatek,mt6589-wdt";
 			mediatek,disable-extrst;
 			reg = <0 0x10007000 0 0x100>;
+			#reset-cells = <1>;
 		};
 
 		apmixedsys: syscon@1000c000 {
@@ -783,6 +784,8 @@
 			mediatek,topckgen = <&topckgen>;
 			power-domains = <&spm MT8195_POWER_DOMAIN_AUDIO>;
 			interrupts = <GIC_SPI 822 IRQ_TYPE_LEVEL_HIGH 0>;
+			resets = <&watchdog 14>;
+			reset-names = "audiosys";
 			clocks = <&clk26m>,
 				<&apmixedsys CLK_APMIXED_APLL1>,
 				<&apmixedsys CLK_APMIXED_APLL2>,
-- 
2.18.0

Re: [PATCH v1 13/16] arm64: dts: mt8195: Specify audio reset controller

[AngeloGioacchino Del Regno]

Il 04/07/22 12:00, Tinghan Shen ha scritto:
> From: Trevor Wu <trevor.wu@mediatek.com>
> 
> Specify audio reset controller for audio hardware resetting.
> 
> Signed-off-by: Trevor Wu <trevor.wu@mediatek.com>
> Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

[PATCH v1 14/16] arm64: dts: mt8195: Add iommu and smi nodes

[Tinghan Shen]

Add iommu nodes and smi nodes for mt8195.

Signed-off-by: Yong Wu <yong.wu@mediatek.com>
Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>
---
 arch/arm64/boot/dts/mediatek/mt8195.dtsi | 451 +++++++++++++++++++++++
 1 file changed, 451 insertions(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt8195.dtsi b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
index 618fb2fa195a..cb2b79dc08d1 100644
--- a/arch/arm64/boot/dts/mediatek/mt8195.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
@@ -8,6 +8,7 @@
 #include <dt-bindings/clock/mt8195-clk.h>
 #include <dt-bindings/interrupt-controller/arm-gic.h>
 #include <dt-bindings/interrupt-controller/irq.h>
+#include <dt-bindings/memory/mt8195-memory-port.h>
 #include <dt-bindings/phy/phy.h>
 #include <dt-bindings/pinctrl/mt8195-pinfunc.h>
 #include <dt-bindings/power/mt8195-power.h>
@@ -725,6 +726,19 @@
 			assigned-clock-parents = <&topckgen CLK_TOP_ULPOSC1_D10>;
 		};
 
+		iommu_infra: infra-iommu@10315000 {
+			compatible = "mediatek,mt8195-iommu-infra";
+			reg = <0 0x10315000 0 0x5000>;
+			interrupts = <GIC_SPI 795 IRQ_TYPE_LEVEL_HIGH 0>,
+				     <GIC_SPI 796 IRQ_TYPE_LEVEL_HIGH 0>,
+				     <GIC_SPI 797 IRQ_TYPE_LEVEL_HIGH 0>,
+				     <GIC_SPI 798 IRQ_TYPE_LEVEL_HIGH 0>,
+				     <GIC_SPI 799 IRQ_TYPE_LEVEL_HIGH 0>;
+			clocks = <&clk26m>;
+			clock-names = "bclk";
+			#iommu-cells = <1>;
+		};
+
 		scp: scp@10500000 {
 			compatible = "mediatek,mt8195-scp";
 			reg = <0 0x10500000 0 0x100000>,
@@ -1439,6 +1453,64 @@
 			#clock-cells = <1>;
 		};
 
+		smi_sub_common_vpp0_vpp1_2x1: smi@14010000 {
+			compatible = "mediatek,mt8195-smi-sub-common";
+			reg = <0 0x14010000 0 0x1000>;
+			clocks = <&vppsys0 CLK_VPP0_GALS_VPP1_WPE>,
+			       <&vppsys0 CLK_VPP0_GALS_VPP1_WPE>,
+			       <&vppsys0 CLK_VPP0_GALS_VPP1_WPE>;
+			clock-names = "apb", "smi", "gals0";
+			mediatek,smi = <&smi_common_vpp>;
+			power-domains = <&spm MT8195_POWER_DOMAIN_VPPSYS0>;
+		};
+
+		smi_sub_common_vdec_vpp0_2x1: smi@14011000 {
+			compatible = "mediatek,mt8195-smi-sub-common";
+			reg = <0 0x14011000 0 0x1000>;
+			clocks = <&vppsys0 CLK_VPP0_GALS_VDEC_VDEC_CORE1>,
+				 <&vppsys0 CLK_VPP0_GALS_VDEC_VDEC_CORE1>,
+				 <&vppsys0 CLK_VPP0_GALS_VDEC_VDEC_CORE1>;
+			clock-names = "apb", "smi", "gals0";
+			mediatek,smi = <&smi_common_vpp>;
+			power-domains = <&spm MT8195_POWER_DOMAIN_VPPSYS0>;
+		};
+
+		smi_common_vpp: smi@14012000 {
+			compatible = "mediatek,mt8195-smi-common-vpp";
+			reg = <0 0x14012000 0 0x1000>;
+			clocks = <&vppsys0 CLK_VPP0_SMI_COMMON_LARB4>,
+			       <&vppsys0 CLK_VPP0_SMI_COMMON_LARB4>,
+			       <&vppsys0 CLK_VPP0_SMI_RSI>,
+			       <&vppsys0 CLK_VPP0_SMI_RSI>;
+			clock-names = "apb", "smi", "gals0", "gals1";
+			power-domains = <&spm MT8195_POWER_DOMAIN_VPPSYS0>;
+		};
+
+		larb4: larb@14013000 {
+			compatible = "mediatek,mt8195-smi-larb";
+			reg = <0 0x14013000 0 0x1000>;
+			mediatek,larb-id = <4>;
+			mediatek,smi = <&smi_sub_common_vpp0_vpp1_2x1>;
+			clocks = <&vppsys0 CLK_VPP0_GALS_VPP1_WPE>,
+			       <&vppsys0 CLK_VPP0_SMI_COMMON_LARB4>;
+			clock-names = "apb", "smi";
+			power-domains = <&spm MT8195_POWER_DOMAIN_VPPSYS0>;
+		};
+
+		iommu_vpp: iommu@14018000 {
+			compatible = "mediatek,mt8195-iommu-vpp";
+			reg = <0 0x14018000 0 0x1000>;
+			mediatek,larbs = <&larb1 &larb3 &larb4 &larb6 &larb8
+					  &larb12 &larb14 &larb16 &larb18
+					  &larb20 &larb22 &larb23 &larb26
+					  &larb27>;
+			interrupts = <GIC_SPI 594 IRQ_TYPE_LEVEL_HIGH 0>;
+			clocks = <&vppsys0 CLK_VPP0_SMI_IOMMU>;
+			clock-names = "bclk";
+			#iommu-cells = <1>;
+			power-domains = <&spm MT8195_POWER_DOMAIN_VPPSYS0>;
+		};
+
 		wpesys: clock-controller@14e00000 {
 			compatible = "mediatek,mt8195-wpesys";
 			reg = <0 0x14e00000 0 0x1000>;
@@ -1457,24 +1529,116 @@
 			#clock-cells = <1>;
 		};
 
+		larb7: larb@14e04000 {
+			compatible = "mediatek,mt8195-smi-larb";
+			reg = <0 0x14e04000 0 0x1000>;
+			mediatek,larb-id = <7>;
+			mediatek,smi = <&smi_common_vdo>;
+			clocks = <&wpesys CLK_WPE_SMI_LARB7>,
+				 <&wpesys CLK_WPE_SMI_LARB7>;
+			clock-names = "apb", "smi";
+			power-domains = <&spm MT8195_POWER_DOMAIN_WPESYS>;
+		};
+
+		larb8: larb@14e05000 {
+			compatible = "mediatek,mt8195-smi-larb";
+			reg = <0 0x14e05000 0 0x1000>;
+			mediatek,larb-id = <8>;
+			mediatek,smi = <&smi_common_vpp>;
+			clocks = <&wpesys CLK_WPE_SMI_LARB8>,
+			       <&wpesys CLK_WPE_SMI_LARB8>,
+			       <&vppsys0 CLK_VPP0_GALS_VPP1_WPE>;
+			clock-names = "apb", "smi", "gals";
+			power-domains = <&spm MT8195_POWER_DOMAIN_WPESYS>;
+		};
+
 		vppsys1: clock-controller@14f00000 {
 			compatible = "mediatek,mt8195-vppsys1";
 			reg = <0 0x14f00000 0 0x1000>;
 			#clock-cells = <1>;
 		};
 
+		larb5: larb@14f02000 {
+			compatible = "mediatek,mt8195-smi-larb";
+			reg = <0 0x14f02000 0 0x1000>;
+			mediatek,larb-id = <5>;
+			mediatek,smi = <&smi_common_vdo>;
+			clocks = <&vppsys1 CLK_VPP1_VPPSYS1_LARB>,
+			       <&vppsys1 CLK_VPP1_VPPSYS1_GALS>,
+			       <&vppsys0 CLK_VPP0_GALS_VPP1_LARB5>;
+			clock-names = "apb", "smi", "gals";
+			power-domains = <&spm MT8195_POWER_DOMAIN_VPPSYS1>;
+		};
+
+		larb6: larb@14f03000 {
+			compatible = "mediatek,mt8195-smi-larb";
+			reg = <0 0x14f03000 0 0x1000>;
+			mediatek,larb-id = <6>;
+			mediatek,smi = <&smi_sub_common_vpp0_vpp1_2x1>;
+			clocks = <&vppsys1 CLK_VPP1_VPPSYS1_LARB>,
+			       <&vppsys1 CLK_VPP1_VPPSYS1_GALS>,
+			       <&vppsys0 CLK_VPP0_GALS_VPP1_LARB6>;
+			clock-names = "apb", "smi", "gals";
+			power-domains = <&spm MT8195_POWER_DOMAIN_VPPSYS1>;
+		};
+
 		imgsys: clock-controller@15000000 {
 			compatible = "mediatek,mt8195-imgsys";
 			reg = <0 0x15000000 0 0x1000>;
 			#clock-cells = <1>;
 		};
 
+		larb9: larb@15001000 {
+			compatible = "mediatek,mt8195-smi-larb";
+			reg = <0 0x15001000 0 0x1000>;
+			mediatek,larb-id = <9>;
+			mediatek,smi = <&smi_sub_common_img1_3x1>;
+			clocks = <&imgsys CLK_IMG_LARB9>,
+				 <&imgsys CLK_IMG_LARB9>,
+				 <&imgsys CLK_IMG_GALS>;
+			clock-names = "apb", "smi", "gals";
+			power-domains = <&spm MT8195_POWER_DOMAIN_IMG>;
+		};
+
+		smi_sub_common_img0_3x1: smi@15002000 {
+			compatible = "mediatek,mt8195-smi-sub-common";
+			reg = <0 0x15002000 0 0x1000>;
+			clocks = <&imgsys CLK_IMG_IPE>,
+				 <&imgsys CLK_IMG_IPE>,
+				 <&vppsys0 CLK_VPP0_GALS_IMGSYS_CAMSYS>;
+			clock-names = "apb", "smi", "gals0";
+			mediatek,smi = <&smi_common_vpp>;
+			power-domains = <&spm MT8195_POWER_DOMAIN_IMG>;
+		};
+
+		smi_sub_common_img1_3x1: smi@15003000 {
+			compatible = "mediatek,mt8195-smi-sub-common";
+			reg = <0 0x15003000 0 0x1000>;
+			clocks = <&imgsys CLK_IMG_LARB9>,
+				 <&imgsys CLK_IMG_LARB9>,
+				 <&imgsys CLK_IMG_GALS>;
+			clock-names = "apb", "smi", "gals0";
+			mediatek,smi = <&smi_common_vdo>;
+			power-domains = <&spm MT8195_POWER_DOMAIN_IMG>;
+		};
+
 		imgsys1_dip_top: clock-controller@15110000 {
 			compatible = "mediatek,mt8195-imgsys1_dip_top";
 			reg = <0 0x15110000 0 0x1000>;
 			#clock-cells = <1>;
 		};
 
+		larb10: larb@15120000 {
+			compatible = "mediatek,mt8195-smi-larb";
+			reg = <0 0x15120000 0 0x1000>;
+			mediatek,larb-id = <10>;
+			mediatek,smi = <&smi_sub_common_img1_3x1>;
+			clocks = <&imgsys CLK_IMG_DIP0>,
+			       <&imgsys1_dip_top CLK_IMG1_DIP_TOP_LARB10>;
+			clock-names = "apb", "smi";
+			power-domains = <&spm MT8195_POWER_DOMAIN_DIP>;
+		};
+
 		imgsys1_dip_nr: clock-controller@15130000 {
 			compatible = "mediatek,mt8195-imgsys1_dip_nr";
 			reg = <0 0x15130000 0 0x1000>;
@@ -1487,18 +1651,129 @@
 			#clock-cells = <1>;
 		};
 
+		larb11: larb@15230000 {
+			compatible = "mediatek,mt8195-smi-larb";
+			reg = <0 0x15230000 0 0x1000>;
+			mediatek,larb-id = <11>;
+			mediatek,smi = <&smi_sub_common_img1_3x1>;
+			clocks = <&imgsys CLK_IMG_WPE0>,
+			       <&imgsys1_wpe CLK_IMG1_WPE_LARB11>;
+			clock-names = "apb", "smi";
+			power-domains = <&spm MT8195_POWER_DOMAIN_DIP>;
+		};
+
 		ipesys: clock-controller@15330000 {
 			compatible = "mediatek,mt8195-ipesys";
 			reg = <0 0x15330000 0 0x1000>;
 			#clock-cells = <1>;
 		};
 
+		larb12: larb@15340000 {
+			compatible = "mediatek,mt8195-smi-larb";
+			reg = <0 0x15340000 0 0x1000>;
+			mediatek,larb-id = <12>;
+			mediatek,smi = <&smi_sub_common_img0_3x1>;
+			clocks = <&ipesys CLK_IPE_SMI_LARB12>,
+				 <&ipesys CLK_IPE_SMI_LARB12>;
+			clock-names = "apb", "smi";
+			power-domains = <&spm MT8195_POWER_DOMAIN_IPE>;
+		};
+
 		camsys: clock-controller@16000000 {
 			compatible = "mediatek,mt8195-camsys";
 			reg = <0 0x16000000 0 0x1000>;
 			#clock-cells = <1>;
 		};
 
+		larb13: larb@16001000 {
+			compatible = "mediatek,mt8195-smi-larb";
+			reg = <0 0x16001000 0 0x1000>;
+			mediatek,larb-id = <13>;
+			mediatek,smi = <&smi_sub_common_cam_4x1>;
+			clocks = <&camsys CLK_CAM_LARB13>,
+			       <&camsys CLK_CAM_LARB13>,
+			       <&camsys CLK_CAM_CAM2MM0_GALS>;
+			clock-names = "apb", "smi", "gals";
+			power-domains = <&spm MT8195_POWER_DOMAIN_CAM>;
+		};
+
+		larb14: larb@16002000 {
+			compatible = "mediatek,mt8195-smi-larb";
+			reg = <0 0x16002000 0 0x1000>;
+			mediatek,larb-id = <14>;
+			mediatek,smi = <&smi_sub_common_cam_7x1>;
+			clocks = <&camsys CLK_CAM_LARB14>,
+				 <&camsys CLK_CAM_LARB14>;
+			clock-names = "apb", "smi";
+			power-domains = <&spm MT8195_POWER_DOMAIN_CAM>;
+		};
+
+		smi_sub_common_cam_4x1: smi@16004000 {
+			compatible = "mediatek,mt8195-smi-sub-common";
+			reg = <0 0x16004000 0 0x1000>;
+			clocks = <&camsys CLK_CAM_LARB13>,
+				 <&camsys CLK_CAM_LARB13>,
+				 <&camsys CLK_CAM_CAM2MM0_GALS>;
+			clock-names = "apb", "smi", "gals0";
+			mediatek,smi = <&smi_common_vdo>;
+			power-domains = <&spm MT8195_POWER_DOMAIN_CAM>;
+		};
+
+		smi_sub_common_cam_7x1: smi@16005000 {
+			compatible = "mediatek,mt8195-smi-sub-common";
+			reg = <0 0x16005000 0 0x1000>;
+			clocks = <&camsys CLK_CAM_LARB14>,
+				 <&camsys CLK_CAM_CAM2MM1_GALS>,
+				 <&vppsys0 CLK_VPP0_GALS_IMGSYS_CAMSYS>;
+			clock-names = "apb", "smi", "gals0";
+			mediatek,smi = <&smi_common_vpp>;
+			power-domains = <&spm MT8195_POWER_DOMAIN_CAM>;
+		};
+
+		larb16: larb@16012000 {
+			compatible = "mediatek,mt8195-smi-larb";
+			reg = <0 0x16012000 0 0x1000>;
+			mediatek,larb-id = <16>;
+			mediatek,smi = <&smi_sub_common_cam_7x1>;
+			clocks = <&camsys_rawa CLK_CAM_RAWA_LARBX>,
+				 <&camsys_rawa CLK_CAM_RAWA_LARBX>;
+			clock-names = "apb", "smi";
+			power-domains = <&spm MT8195_POWER_DOMAIN_CAM_RAWA>;
+		};
+
+		larb17: larb@16013000 {
+			compatible = "mediatek,mt8195-smi-larb";
+			reg = <0 0x16013000 0 0x1000>;
+			mediatek,larb-id = <17>;
+			mediatek,smi = <&smi_sub_common_cam_4x1>;
+			clocks = <&camsys_yuva CLK_CAM_YUVA_LARBX>,
+				 <&camsys_yuva CLK_CAM_YUVA_LARBX>;
+			clock-names = "apb", "smi";
+			power-domains = <&spm MT8195_POWER_DOMAIN_CAM_RAWA>;
+		};
+
+		larb27: larb@16014000 {
+			compatible = "mediatek,mt8195-smi-larb";
+			reg = <0 0x16014000 0 0x1000>;
+			mediatek,larb-id = <27>;
+			mediatek,smi = <&smi_sub_common_cam_7x1>;
+			clocks = <&camsys_rawb CLK_CAM_RAWB_LARBX>,
+				 <&camsys_rawb CLK_CAM_RAWB_LARBX>;
+			clock-names = "apb", "smi";
+			power-domains = <&spm MT8195_POWER_DOMAIN_CAM_RAWB>;
+		};
+
+		larb28: larb@16015000 {
+			compatible = "mediatek,mt8195-smi-larb";
+			reg = <0 0x16015000 0 0x1000>;
+			mediatek,larb-id = <28>;
+			mediatek,smi = <&smi_sub_common_cam_4x1>;
+			clocks = <&camsys_yuvb CLK_CAM_YUVB_LARBX>,
+				 <&camsys_yuvb CLK_CAM_YUVB_LARBX>;
+			clock-names = "apb", "smi";
+			power-domains = <&spm MT8195_POWER_DOMAIN_CAM_RAWB>;
+		};
+
 		camsys_rawa: clock-controller@1604f000 {
 			compatible = "mediatek,mt8195-camsys_rawa";
 			reg = <0 0x1604f000 0 0x1000>;
@@ -1529,24 +1804,103 @@
 			#clock-cells = <1>;
 		};
 
+		larb25: larb@16141000 {
+			compatible = "mediatek,mt8195-smi-larb";
+			reg = <0 0x16141000 0 0x1000>;
+			mediatek,larb-id = <25>;
+			mediatek,smi = <&smi_sub_common_cam_4x1>;
+			clocks = <&camsys CLK_CAM_LARB13>,
+				 <&camsys_mraw CLK_CAM_MRAW_LARBX>,
+				 <&camsys CLK_CAM_CAM2MM0_GALS>;
+			clock-names = "apb", "smi", "gals";
+			power-domains = <&spm MT8195_POWER_DOMAIN_CAM_MRAW>;
+		};
+
+		larb26: larb@16142000 {
+			compatible = "mediatek,mt8195-smi-larb";
+			reg = <0 0x16142000 0 0x1000>;
+			mediatek,larb-id = <26>;
+			mediatek,smi = <&smi_sub_common_cam_7x1>;
+			clocks = <&camsys_mraw CLK_CAM_MRAW_LARBX>,
+				 <&camsys_mraw CLK_CAM_MRAW_LARBX>;
+			clock-names = "apb", "smi";
+			power-domains = <&spm MT8195_POWER_DOMAIN_CAM_MRAW>;
+
+		};
+
 		ccusys: clock-controller@17200000 {
 			compatible = "mediatek,mt8195-ccusys";
 			reg = <0 0x17200000 0 0x1000>;
 			#clock-cells = <1>;
 		};
 
+		larb18: larb@17201000 {
+			compatible = "mediatek,mt8195-smi-larb";
+			reg = <0 0x17201000 0 0x1000>;
+			mediatek,larb-id = <18>;
+			mediatek,smi = <&smi_sub_common_cam_7x1>;
+			clocks = <&ccusys CLK_CCU_LARB18>,
+				 <&ccusys CLK_CCU_LARB18>;
+			clock-names = "apb", "smi";
+			power-domains = <&spm MT8195_POWER_DOMAIN_CAM>;
+		};
+
+		larb24: larb@1800d000 {
+			compatible = "mediatek,mt8195-smi-larb";
+			reg = <0 0x1800d000 0 0x1000>;
+			mediatek,larb-id = <24>;
+			mediatek,smi = <&smi_common_vdo>;
+			clocks = <&vdecsys_soc CLK_VDEC_SOC_LARB1>,
+				 <&vdecsys_soc CLK_VDEC_SOC_LARB1>;
+			clock-names = "apb", "smi";
+			power-domains = <&spm MT8195_POWER_DOMAIN_VDEC0>;
+		};
+
+		larb23: larb@1800e000 {
+			compatible = "mediatek,mt8195-smi-larb";
+			reg = <0 0x1800e000 0 0x1000>;
+			mediatek,larb-id = <23>;
+			mediatek,smi = <&smi_sub_common_vdec_vpp0_2x1>;
+			clocks = <&vppsys0 CLK_VPP0_GALS_VDEC_VDEC_CORE1>,
+				 <&vdecsys_soc CLK_VDEC_SOC_LARB1>;
+			clock-names = "apb", "smi";
+			power-domains = <&spm MT8195_POWER_DOMAIN_VDEC0>;
+		};
+
 		vdecsys_soc: clock-controller@1800f000 {
 			compatible = "mediatek,mt8195-vdecsys_soc";
 			reg = <0 0x1800f000 0 0x1000>;
 			#clock-cells = <1>;
 		};
 
+		larb21: larb@1802e000 {
+			compatible = "mediatek,mt8195-smi-larb";
+			reg = <0 0x1802e000 0 0x1000>;
+			mediatek,larb-id = <21>;
+			mediatek,smi = <&smi_common_vdo>;
+			clocks = <&vdecsys CLK_VDEC_LARB1>,
+				 <&vdecsys CLK_VDEC_LARB1>;
+			clock-names = "apb", "smi";
+			power-domains = <&spm MT8195_POWER_DOMAIN_VDEC1>;
+		};
+
 		vdecsys: clock-controller@1802f000 {
 			compatible = "mediatek,mt8195-vdecsys";
 			reg = <0 0x1802f000 0 0x1000>;
 			#clock-cells = <1>;
 		};
 
+		larb22: larb@1803e000 {
+			compatible = "mediatek,mt8195-smi-larb";
+			reg = <0 0x1803e000 0 0x1000>;
+			mediatek,larb-id = <22>;
+			mediatek,smi = <&smi_sub_common_vdec_vpp0_2x1>;
+			clocks = <&vppsys0 CLK_VPP0_GALS_VDEC_VDEC_CORE1>,
+				 <&vdecsys_core1 CLK_VDEC_CORE1_LARB1>;
+			clock-names = "apb", "smi";
+			power-domains = <&spm MT8195_POWER_DOMAIN_VDEC2>;
+		};
+
 		vdecsys_core1: clock-controller@1803f000 {
 			compatible = "mediatek,mt8195-vdecsys_core1";
 			reg = <0 0x1803f000 0 0x1000>;
@@ -1565,6 +1919,17 @@
 			#clock-cells = <1>;
 		};
 
+		larb19: larb@1a010000 {
+			compatible = "mediatek,mt8195-smi-larb";
+			reg = <0 0x1a010000 0 0x1000>;
+			mediatek,larb-id = <19>;
+			mediatek,smi = <&smi_common_vdo>;
+			clocks = <&vencsys CLK_VENC_VENC>,
+				 <&vencsys CLK_VENC_GALS>;
+			clock-names = "apb", "smi";
+			power-domains = <&spm MT8195_POWER_DOMAIN_VENC>;
+		};
+
 		vencsys_core1: clock-controller@1b000000 {
 			compatible = "mediatek,mt8195-vencsys_core1";
 			reg = <0 0x1b000000 0 0x1000>;
@@ -1577,10 +1942,96 @@
 			#clock-cells = <1>;
 		};
 
+		larb20: larb@1b010000 {
+			compatible = "mediatek,mt8195-smi-larb";
+			reg = <0 0x1b010000 0 0x1000>;
+			mediatek,larb-id = <20>;
+			mediatek,smi = <&smi_common_vpp>;
+			clocks = <&vencsys_core1 CLK_VENC_CORE1_LARB>,
+				 <&vencsys_core1 CLK_VENC_CORE1_GALS>,
+				 <&vppsys0 CLK_VPP0_GALS_VDO0_VDO1_VENCSYS_CORE1>;
+			clock-names = "apb", "smi", "gals";
+			power-domains = <&spm MT8195_POWER_DOMAIN_VENC_CORE1>;
+		};
+
+		larb0: larb@1c018000 {
+			compatible = "mediatek,mt8195-smi-larb";
+			reg = <0 0x1c018000 0 0x1000>;
+			mediatek,larb-id = <0>;
+			mediatek,smi = <&smi_common_vdo>;
+			clocks = <&vdosys0 CLK_VDO0_SMI_LARB>,
+				 <&vdosys0 CLK_VDO0_SMI_LARB>,
+				 <&vppsys0 CLK_VPP0_GALS_VDO0_LARB0>;
+			clock-names = "apb", "smi", "gals";
+			power-domains = <&spm MT8195_POWER_DOMAIN_VDOSYS0>;
+		};
+
+		larb1: larb@1c019000 {
+			compatible = "mediatek,mt8195-smi-larb";
+			reg = <0 0x1c019000 0 0x1000>;
+			mediatek,larb-id = <1>;
+			mediatek,smi = <&smi_common_vpp>;
+			clocks = <&vdosys0 CLK_VDO0_SMI_LARB>,
+				 <&vppsys0 CLK_VPP0_GALS_VDO0_VDO1_VENCSYS_CORE1>,
+				 <&vppsys0 CLK_VPP0_GALS_VDO0_LARB1>;
+			clock-names = "apb", "smi", "gals";
+			power-domains = <&spm MT8195_POWER_DOMAIN_VDOSYS0>;
+		};
+
 		vdosys1: syscon@1c100000 {
 			compatible = "mediatek,mt8195-mmsys", "syscon";
 			reg = <0 0x1c100000 0 0x1000>;
 			#clock-cells = <1>;
 		};
+
+		smi_common_vdo: smi@1c01b000 {
+			compatible = "mediatek,mt8195-smi-common-vdo";
+			reg = <0 0x1c01b000 0 0x1000>;
+			clocks = <&vdosys0 CLK_VDO0_SMI_COMMON>,
+				 <&vdosys0 CLK_VDO0_SMI_EMI>,
+				 <&vdosys0 CLK_VDO0_SMI_RSI>,
+				 <&vdosys0 CLK_VDO0_SMI_GALS>;
+			clock-names = "apb", "smi", "gals0", "gals1";
+			power-domains = <&spm MT8195_POWER_DOMAIN_VDOSYS0>;
+
+		};
+
+		iommu_vdo: iommu@1c01f000 {
+			compatible = "mediatek,mt8195-iommu-vdo";
+			reg = <0 0x1c01f000 0 0x1000>;
+			mediatek,larbs = <&larb0 &larb2 &larb5 &larb7 &larb9
+					  &larb10 &larb11 &larb13 &larb17
+					  &larb19 &larb21 &larb24 &larb25
+					  &larb28>;
+			interrupts = <GIC_SPI 669 IRQ_TYPE_LEVEL_HIGH 0>;
+			#iommu-cells = <1>;
+			clocks = <&vdosys0 CLK_VDO0_SMI_IOMMU>;
+			clock-names = "bclk";
+			power-domains = <&spm MT8195_POWER_DOMAIN_VDOSYS0>;
+		};
+
+		larb2: larb@1c102000 {
+			compatible = "mediatek,mt8195-smi-larb";
+			reg = <0 0x1c102000 0 0x1000>;
+			mediatek,larb-id = <2>;
+			mediatek,smi = <&smi_common_vdo>;
+			clocks = <&vdosys1 CLK_VDO1_SMI_LARB2>,
+				 <&vdosys1 CLK_VDO1_SMI_LARB2>,
+				 <&vdosys1 CLK_VDO1_GALS>;
+			clock-names = "apb", "smi", "gals";
+			power-domains = <&spm MT8195_POWER_DOMAIN_VDOSYS1>;
+		};
+
+		larb3: larb@1c103000 {
+			compatible = "mediatek,mt8195-smi-larb";
+			reg = <0 0x1c103000 0 0x1000>;
+			mediatek,larb-id = <3>;
+			mediatek,smi = <&smi_common_vpp>;
+			clocks = <&vdosys1 CLK_VDO1_SMI_LARB3>,
+				 <&vdosys1 CLK_VDO1_GALS>,
+				 <&vppsys0 CLK_VPP0_GALS_VDO0_VDO1_VENCSYS_CORE1>;
+			clock-names = "apb", "smi", "gals";
+			power-domains = <&spm MT8195_POWER_DOMAIN_VDOSYS1>;
+		};
 	};
 };
-- 
2.18.0

Re: [PATCH v1 14/16] arm64: dts: mt8195: Add iommu and smi nodes

[AngeloGioacchino Del Regno]

Il 04/07/22 12:00, Tinghan Shen ha scritto:
> Add iommu nodes and smi nodes for mt8195.
> 
> Signed-off-by: Yong Wu <yong.wu@mediatek.com>
> Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

Re: [PATCH v1 15/16] arm64: dts: mt8195: Add gce node

[AngeloGioacchino Del Regno]

Il 04/07/22 12:00, Tinghan Shen ha scritto:
> From: "Jason-JH.Lin" <jason-jh.lin@mediatek.com>
> 
> Add gce node and gce alias to mt8195 device tree.
> 
> Signed-off-by: Jason-JH.Lin <jason-jh.lin@mediatek.com>
> Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

Re: [PATCH v1 16/16] arm64: dts: mt8195: Add display node for vdosys0

[AngeloGioacchino Del Regno]

Il 04/07/22 12:00, Tinghan Shen ha scritto:
> From: "Jason-JH.Lin" <jason-jh.lin@mediatek.com>
> 
> Add display node for vdosys0 of mt8195.
> 
> Signed-off-by: Jason-JH.Lin <jason-jh.lin@mediatek.com>
> Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>
> ---
>   arch/arm64/boot/dts/mediatek/mt8195.dtsi | 109 +++++++++++++++++++++++
>   1 file changed, 109 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/mediatek/mt8195.dtsi b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
> index 724c6ca837b6..faea8ef33e5a 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8195.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
> @@ -1961,6 +1961,7 @@
>   		vdosys0: syscon@1c01a000 {
>   			compatible = "mediatek,mt8195-mmsys", "syscon";
>   			reg = <0 0x1c01a000 0 0x1000>;
> +			mboxes = <&gce0 0 CMDQ_THR_PRIO_4>;
>   			#clock-cells = <1>;
>   		};
>   
> @@ -1976,6 +1977,114 @@
>   			power-domains = <&spm MT8195_POWER_DOMAIN_VENC_CORE1>;
>   		};
>   
> +		ovl0: ovl@1c000000 {
> +			compatible = "mediatek,mt8195-disp-ovl",
> +				     "mediatek,mt8183-disp-ovl";

This fits in one line, please fix, here and all of the other instances of that.

> +			reg = <0 0x1c000000 0 0x1000>;
> +			interrupts = <GIC_SPI 636 IRQ_TYPE_LEVEL_HIGH 0>;
> +			power-domains = <&spm MT8195_POWER_DOMAIN_VDOSYS0>;
> +			clocks = <&vdosys0 CLK_VDO0_DISP_OVL0>;
> +			iommus = <&iommu_vdo M4U_PORT_L0_DISP_OVL0_RDMA0>;
> +			mediatek,gce-client-reg =
> +				 <&gce0 SUBSYS_1c00XXXX 0x0000 0x1000>;

Same for gce-client-reg.

Regards,
Angelo

Re: [PATCH v1 16/16] arm64: dts: mt8195: Add display node for vdosys0

[Krzysztof Kozlowski]

On 04/07/2022 12:00, Tinghan Shen wrote:
> From: "Jason-JH.Lin" <jason-jh.lin@mediatek.com>
> 
> Add display node for vdosys0 of mt8195.
> 
> Signed-off-by: Jason-JH.Lin <jason-jh.lin@mediatek.com>
> Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>
> ---
>  arch/arm64/boot/dts/mediatek/mt8195.dtsi | 109 +++++++++++++++++++++++
>  1 file changed, 109 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/mediatek/mt8195.dtsi b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
> index 724c6ca837b6..faea8ef33e5a 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8195.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
> @@ -1961,6 +1961,7 @@
>  		vdosys0: syscon@1c01a000 {
>  			compatible = "mediatek,mt8195-mmsys", "syscon";
>  			reg = <0 0x1c01a000 0 0x1000>;
> +			mboxes = <&gce0 0 CMDQ_THR_PRIO_4>;
>  			#clock-cells = <1>;
>  		};
>  
> @@ -1976,6 +1977,114 @@
>  			power-domains = <&spm MT8195_POWER_DOMAIN_VENC_CORE1>;
>  		};
>  
> +		ovl0: ovl@1c000000 {
> +			compatible = "mediatek,mt8195-disp-ovl",
> +				     "mediatek,mt8183-disp-ovl";
> +			reg = <0 0x1c000000 0 0x1000>;
> +			interrupts = <GIC_SPI 636 IRQ_TYPE_LEVEL_HIGH 0>;
> +			power-domains = <&spm MT8195_POWER_DOMAIN_VDOSYS0>;
> +			clocks = <&vdosys0 CLK_VDO0_DISP_OVL0>;
> +			iommus = <&iommu_vdo M4U_PORT_L0_DISP_OVL0_RDMA0>;
> +			mediatek,gce-client-reg =
> +				 <&gce0 SUBSYS_1c00XXXX 0x0000 0x1000>;
> +		};
> +
> +		rdma0: rdma@1c002000 {
> +			compatible = "mediatek,mt8195-disp-rdma";
> +			reg = <0 0x1c002000 0 0x1000>;
> +			interrupts = <GIC_SPI 638 IRQ_TYPE_LEVEL_HIGH 0>;
> +			power-domains = <&spm MT8195_POWER_DOMAIN_VDOSYS0>;
> +			clocks = <&vdosys0 CLK_VDO0_DISP_RDMA0>;
> +			iommus = <&iommu_vdo M4U_PORT_L0_DISP_RDMA0>;
> +			mediatek,gce-client-reg =
> +				 <&gce0 SUBSYS_1c00XXXX 0x2000 0x1000>;
> +		};
> +
> +		color0: color@1c003000 {
> +			compatible = "mediatek,mt8195-disp-color",
> +				     "mediatek,mt8173-disp-color";
> +			reg = <0 0x1c003000 0 0x1000>;
> +			interrupts = <GIC_SPI 639 IRQ_TYPE_LEVEL_HIGH 0>;
> +			power-domains = <&spm MT8195_POWER_DOMAIN_VDOSYS0>;
> +			clocks = <&vdosys0 CLK_VDO0_DISP_COLOR0>;
> +			mediatek,gce-client-reg =
> +				 <&gce0 SUBSYS_1c00XXXX 0x3000 0x1000>;
> +		};
> +
> +		ccorr0: ccorr@1c004000 {
> +			compatible = "mediatek,mt8195-disp-ccorr",
> +				     "mediatek,mt8192-disp-ccorr";
> +			reg = <0 0x1c004000 0 0x1000>;
> +			interrupts = <GIC_SPI 640 IRQ_TYPE_LEVEL_HIGH 0>;
> +			power-domains = <&spm MT8195_POWER_DOMAIN_VDOSYS0>;
> +			clocks = <&vdosys0 CLK_VDO0_DISP_CCORR0>;
> +			mediatek,gce-client-reg =
> +				 <&gce0 SUBSYS_1c00XXXX 0x4000 0x1000>;
> +		};
> +
> +		aal0: aal@1c005000 {
> +			compatible = "mediatek,mt8195-disp-aal",
> +				     "mediatek,mt8183-disp-aal";
> +			reg = <0 0x1c005000 0 0x1000>;
> +			interrupts = <GIC_SPI 641 IRQ_TYPE_LEVEL_HIGH 0>;
> +			power-domains = <&spm MT8195_POWER_DOMAIN_VDOSYS0>;
> +			clocks = <&vdosys0 CLK_VDO0_DISP_AAL0>;
> +			mediatek,gce-client-reg =
> +				 <&gce0 SUBSYS_1c00XXXX 0x5000 0x1000>;
> +		};
> +
> +		gamma0: gamma@1c006000 {
> +			compatible = "mediatek,mt8195-disp-gamma",
> +				     "mediatek,mt8183-disp-gamma";
> +			reg = <0 0x1c006000 0 0x1000>;
> +			interrupts = <GIC_SPI 642 IRQ_TYPE_LEVEL_HIGH 0>;
> +			power-domains = <&spm MT8195_POWER_DOMAIN_VDOSYS0>;
> +			clocks = <&vdosys0 CLK_VDO0_DISP_GAMMA0>;
> +			mediatek,gce-client-reg =
> +				 <&gce0 SUBSYS_1c00XXXX 0x6000 0x1000>;
> +		};
> +
> +		dither0: dither@1c007000 {
> +			compatible = "mediatek,mt8195-disp-dither",
> +				     "mediatek,mt8183-disp-dither";
> +			reg = <0 0x1c007000 0 0x1000>;
> +			interrupts = <GIC_SPI 643 IRQ_TYPE_LEVEL_HIGH 0>;
> +			power-domains = <&spm MT8195_POWER_DOMAIN_VDOSYS0>;
> +			clocks = <&vdosys0 CLK_VDO0_DISP_DITHER0>;
> +			mediatek,gce-client-reg =
> +				 <&gce0 SUBSYS_1c00XXXX 0x7000 0x1000>;
> +		};
> +
> +		dsc0: dsc@1c009000 {
> +			compatible = "mediatek,mt8195-disp-dsc";
> +			reg = <0 0x1c009000 0 0x1000>;
> +			interrupts = <GIC_SPI 645 IRQ_TYPE_LEVEL_HIGH 0>;
> +			power-domains = <&spm MT8195_POWER_DOMAIN_VDOSYS0>;
> +			clocks = <&vdosys0 CLK_VDO0_DSC_WRAP0>;
> +			mediatek,gce-client-reg =
> +				 <&gce0 SUBSYS_1c00XXXX 0x9000 0x1000>;
> +		};
> +
> +		merge0: merge0@1c014000 {

Generic node name.

> +			compatible = "mediatek,mt8195-disp-merge";
> +			reg = <0 0x1c014000 0 0x1000>;
> +			interrupts = <GIC_SPI 656 IRQ_TYPE_LEVEL_HIGH 0>;
> +			power-domains = <&spm MT8195_POWER_DOMAIN_VDOSYS0>;
> +			clocks = <&vdosys0 CLK_VDO0_VPP_MERGE0>;
> +			mediatek,gce-client-reg =
> +				 <&gce0 SUBSYS_1c01XXXX 0x4000 0x1000>;
> +		};
> +
> +		mutex: mutex0@1c016000 {

Generic node name.

> +			compatible = "mediatek,mt8195-disp-mutex";
> +			reg = <0 0x1c016000 0 0x1000>;
> +			interrupts = <GIC_SPI 658 IRQ_TYPE_LEVEL_HIGH 0>;
> +			power-domains = <&spm MT8195_POWER_DOMAIN_VDOSYS0>;
> +			clocks = <&vdosys0 CLK_VDO0_DISP_MUTEX0>;
> +			mediatek,gce-events =
> +				 <CMDQ_EVENT_VDO0_DISP_STREAM_DONE_0>;
> +		};
> +
>  		larb0: larb@1c018000 {
>  			compatible = "mediatek,mt8195-smi-larb";
>  			reg = <0 0x1c018000 0 0x1000>;


Best regards,
Krzysztof