diff mbox series

[v2,5/6] dt-bindings: arm: mediatek: Add missing power-domains property

Message ID 20221221034407.19605-6-allen-kh.cheng@mediatek.com
State New
Headers
Series Add ADSP power domains controller support for MT8192 |

Commit Message

Allen-KH Cheng Dec. 21, 2022, 3:44 a.m. UTC 
  The "mediatek,mt8192-scp_adsp" binding requires a power domain to be
specified.

Fixes: 4a803990aeb1 ("dt-bindings: ARM: Mediatek: Add new document bindings of MT8192 clock")
Signed-off-by: Allen-KH Cheng <allen-kh.cheng@mediatek.com>
---
 .../arm/mediatek/mediatek,mt8192-clock.yaml     | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

Comments

Krzysztof Kozlowski Dec. 21, 2022, 8:18 a.m. UTC | #1 
On 21/12/2022 04:44, Allen-KH Cheng wrote:
> The "mediatek,mt8192-scp_adsp" binding requires a power domain to be
> specified.

That's not true. Before this patch, how does the binding require a power
domain? Please show me the part of binding which requires it.

Best regards,
Krzysztof
AngeloGioacchino Del Regno Dec. 21, 2022, 10:22 a.m. UTC | #2 
Il 21/12/22 04:44, Allen-KH Cheng ha scritto:
> The "mediatek,mt8192-scp_adsp" binding requires a power domain to be
> specified.
> 
> Fixes: 4a803990aeb1 ("dt-bindings: ARM: Mediatek: Add new document bindings of MT8192 clock")
> Signed-off-by: Allen-KH Cheng <allen-kh.cheng@mediatek.com>
> ---
>   .../arm/mediatek/mediatek,mt8192-clock.yaml     | 17 +++++++++++++++++
>   1 file changed, 17 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/arm/mediatek/mediatek,mt8192-clock.yaml b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mt8192-clock.yaml
> index b57cc2e69efb..ce8dd2bfb533 100644
> --- a/Documentation/devicetree/bindings/arm/mediatek/mediatek,mt8192-clock.yaml
> +++ b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mt8192-clock.yaml
> @@ -40,6 +40,9 @@ properties:
>     reg:
>       maxItems: 1
>   
> +  power-domains:
> +    maxItems: 1
> +
>     '#clock-cells':
>       const: 1
>   
> @@ -47,13 +50,27 @@ required:
>     - compatible
>     - reg
>   
> +allOf:

allOf is unnecessary here.

Regards,
Angelo
Krzysztof Kozlowski Dec. 21, 2022, 10:33 a.m. UTC | #3 
On 21/12/2022 11:22, AngeloGioacchino Del Regno wrote:
> Il 21/12/22 04:44, Allen-KH Cheng ha scritto:
>> The "mediatek,mt8192-scp_adsp" binding requires a power domain to be
>> specified.
>>
>> Fixes: 4a803990aeb1 ("dt-bindings: ARM: Mediatek: Add new document bindings of MT8192 clock")
>> Signed-off-by: Allen-KH Cheng <allen-kh.cheng@mediatek.com>
>> ---
>>   .../arm/mediatek/mediatek,mt8192-clock.yaml     | 17 +++++++++++++++++
>>   1 file changed, 17 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/arm/mediatek/mediatek,mt8192-clock.yaml b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mt8192-clock.yaml
>> index b57cc2e69efb..ce8dd2bfb533 100644
>> --- a/Documentation/devicetree/bindings/arm/mediatek/mediatek,mt8192-clock.yaml
>> +++ b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mt8192-clock.yaml
>> @@ -40,6 +40,9 @@ properties:
>>     reg:
>>       maxItems: 1
>>   
>> +  power-domains:
>> +    maxItems: 1
>> +
>>     '#clock-cells':
>>       const: 1
>>   
>> @@ -47,13 +50,27 @@ required:
>>     - compatible
>>     - reg
>>   
>> +allOf:
> 
> allOf is unnecessary here.

If you mean that "if:" can be without it, then it is better to have
allOf. It grows often, so you avoid useless indentation change later.


Best regards,
Krzysztof
AngeloGioacchino Del Regno Dec. 21, 2022, 10:34 a.m. UTC | #4 
Il 21/12/22 11:33, Krzysztof Kozlowski ha scritto:
> On 21/12/2022 11:22, AngeloGioacchino Del Regno wrote:
>> Il 21/12/22 04:44, Allen-KH Cheng ha scritto:
>>> The "mediatek,mt8192-scp_adsp" binding requires a power domain to be
>>> specified.
>>>
>>> Fixes: 4a803990aeb1 ("dt-bindings: ARM: Mediatek: Add new document bindings of MT8192 clock")
>>> Signed-off-by: Allen-KH Cheng <allen-kh.cheng@mediatek.com>
>>> ---
>>>    .../arm/mediatek/mediatek,mt8192-clock.yaml     | 17 +++++++++++++++++
>>>    1 file changed, 17 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/arm/mediatek/mediatek,mt8192-clock.yaml b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mt8192-clock.yaml
>>> index b57cc2e69efb..ce8dd2bfb533 100644
>>> --- a/Documentation/devicetree/bindings/arm/mediatek/mediatek,mt8192-clock.yaml
>>> +++ b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mt8192-clock.yaml
>>> @@ -40,6 +40,9 @@ properties:
>>>      reg:
>>>        maxItems: 1
>>>    
>>> +  power-domains:
>>> +    maxItems: 1
>>> +
>>>      '#clock-cells':
>>>        const: 1
>>>    
>>> @@ -47,13 +50,27 @@ required:
>>>      - compatible
>>>      - reg
>>>    
>>> +allOf:
>>
>> allOf is unnecessary here.
> 
> If you mean that "if:" can be without it, then it is better to have
> allOf. It grows often, so you avoid useless indentation change later.
> 
> 

Yes, that's what I mean. I've suggested so because I don't expect this list
to grow in the future.

Regards,
Angelo
Chen-Yu Tsai Dec. 29, 2022, 10:15 a.m. UTC | #5 
On Wed, Dec 21, 2022 at 4:18 PM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 21/12/2022 04:44, Allen-KH Cheng wrote:
> > The "mediatek,mt8192-scp_adsp" binding requires a power domain to be
> > specified.
>
> That's not true. Before this patch, how does the binding require a power
> domain? Please show me the part of binding which requires it.

Maybe this should be reworded to something like the following?

<--- cut
The SCP_ADSP clock controller has a power domain dependency that was not
described properly. Add it to the binding.
<--- cut

This was discovered when I was reworking the clock drivers. The clocks
in this controller were being turned off by the clock core, which would
result in the system locking up. MediaTek said this was due to the power
domain.

Regards
ChenYu
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/arm/mediatek/mediatek,mt8192-clock.yaml b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mt8192-clock.yaml
index b57cc2e69efb..ce8dd2bfb533 100644
--- a/Documentation/devicetree/bindings/arm/mediatek/mediatek,mt8192-clock.yaml
+++ b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mt8192-clock.yaml
@@ -40,6 +40,9 @@  properties:
   reg:
     maxItems: 1
 
+  power-domains:
+    maxItems: 1
+
   '#clock-cells':
     const: 1
 
@@ -47,13 +50,27 @@  required:
   - compatible
   - reg
 
+allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - mediatek,mt8192-scp_adsp
+    then:
+      required:
+        - power-domains
+
 additionalProperties: false
 
 examples:
   - |
+    #include <dt-bindings/power/mt8192-power.h>
+
     scp_adsp: clock-controller@10720000 {
         compatible = "mediatek,mt8192-scp_adsp";
         reg = <0x10720000 0x1000>;
+        power-domains = <&spm MT8192_POWER_DOMAIN_ADSP>;
         #clock-cells = <1>;
     };