Add a mandatory reset property for the TH1520 VO clock controller that
handles the GPU clocks. This reset line controls the GPU CLKGEN reset,
which is required for proper GPU clock operation.

The reset property is only required for the "thead,th1520-clk-vo"
compatible, as it specifically handles the GPU-related clocks.

Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com>
---
 .../bindings/clock/thead,th1520-clk-ap.yaml      | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/Documentation/devicetree/bindings/clock/thead,th1520-clk-ap.yaml b/Documentation/devicetree/bindings/clock/thead,th1520-clk-ap.yaml
index 9d058c00ab3d..6ea8202718d0 100644
--- a/Documentation/devicetree/bindings/clock/thead,th1520-clk-ap.yaml
+++ b/Documentation/devicetree/bindings/clock/thead,th1520-clk-ap.yaml
@@ -40,6 +40,12 @@ properties:
             (integer PLL) typically running at 792 MHz (FOUTPOSTDIV), with
             a maximum FOUTVCO of 2376 MHz.
 
+  resets:
+    maxItems: 1
+    description:
+      Required for "thead,th1520-clk-vo". This reset line controls the
+      GPU CLKGEN reset which is required for proper GPU clock operation.
+
   "#clock-cells":
     const: 1
     description:
@@ -51,6 +57,16 @@ required:
   - clocks
   - "#clock-cells"
 
+allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: thead,th1520-clk-vo
+    then:
+      required:
+        - resets
+
 additionalProperties: false
 
 examples:
-- 
2.34.1

On Wed, Feb 19, 2025 at 03:02:27PM +0100, Michal Wilczynski wrote:
> Add a mandatory reset property for the TH1520 VO clock controller that
> handles the GPU clocks. This reset line controls the GPU CLKGEN reset,
> which is required for proper GPU clock operation.
> 
> The reset property is only required for the "thead,th1520-clk-vo"
> compatible, as it specifically handles the GPU-related clocks.
> 
> Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com>
> ---
>  .../bindings/clock/thead,th1520-clk-ap.yaml      | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/clock/thead,th1520-clk-ap.yaml b/Documentation/devicetree/bindings/clock/thead,th1520-clk-ap.yaml
> index 9d058c00ab3d..6ea8202718d0 100644
> --- a/Documentation/devicetree/bindings/clock/thead,th1520-clk-ap.yaml
> +++ b/Documentation/devicetree/bindings/clock/thead,th1520-clk-ap.yaml
> @@ -40,6 +40,12 @@ properties:
>              (integer PLL) typically running at 792 MHz (FOUTPOSTDIV), with
>              a maximum FOUTVCO of 2376 MHz.
>  
> +  resets:
> +    maxItems: 1
> +    description:
> +      Required for "thead,th1520-clk-vo". This reset line controls the

You just added the compatible in other patch, so are you saying you
added knowingly incomplete code?

No, this must be squashed.

> +      GPU CLKGEN reset which is required for proper GPU clock operation.
> +
>    "#clock-cells":
>      const: 1
>      description:
> @@ -51,6 +57,16 @@ required:
>    - clocks
>    - "#clock-cells"
>  
> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: thead,th1520-clk-vo
> +    then:
> +      required:
> +        - resets

else:
? What's there? Also reset or no?

Best regards,
Krzysztof

On 2/21/25 10:11, Krzysztof Kozlowski wrote:
> On Wed, Feb 19, 2025 at 03:02:27PM +0100, Michal Wilczynski wrote:
>> Add a mandatory reset property for the TH1520 VO clock controller that
>> handles the GPU clocks. This reset line controls the GPU CLKGEN reset,
>> which is required for proper GPU clock operation.
>>
>> The reset property is only required for the "thead,th1520-clk-vo"
>> compatible, as it specifically handles the GPU-related clocks.
>>
>> Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com>
>> ---
>>  .../bindings/clock/thead,th1520-clk-ap.yaml      | 16 ++++++++++++++++
>>  1 file changed, 16 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/clock/thead,th1520-clk-ap.yaml b/Documentation/devicetree/bindings/clock/thead,th1520-clk-ap.yaml
>> index 9d058c00ab3d..6ea8202718d0 100644
>> --- a/Documentation/devicetree/bindings/clock/thead,th1520-clk-ap.yaml
>> +++ b/Documentation/devicetree/bindings/clock/thead,th1520-clk-ap.yaml
>> @@ -40,6 +40,12 @@ properties:
>>              (integer PLL) typically running at 792 MHz (FOUTPOSTDIV), with
>>              a maximum FOUTVCO of 2376 MHz.
>>  
>> +  resets:
>> +    maxItems: 1
>> +    description:
>> +      Required for "thead,th1520-clk-vo". This reset line controls the
> 
> You just added the compatible in other patch, so are you saying you
> added knowingly incomplete code?
> 
> No, this must be squashed.
> 
>> +      GPU CLKGEN reset which is required for proper GPU clock operation.
>> +
>>    "#clock-cells":
>>      const: 1
>>      description:
>> @@ -51,6 +57,16 @@ required:
>>    - clocks
>>    - "#clock-cells"
>>  
>> +allOf:
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            const: thead,th1520-clk-vo
>> +    then:
>> +      required:
>> +        - resets
> 
> else:
> ? What's there? Also reset or no?

If the else: case the reset is not required, as it's only required in
the th1520clk-vo, so there is no need for else:.

> 
> Best regards,
> Krzysztof
> 
>

On 03/03/2025 09:42, Michal Wilczynski wrote:
>>> +allOf:
>>> +  - if:
>>> +      properties:
>>> +        compatible:
>>> +          contains:
>>> +            const: thead,th1520-clk-vo
>>> +    then:
>>> +      required:
>>> +        - resets
>>
>> else:
>> ? What's there? Also reset or no?
> 
> If the else: case the reset is not required, as it's only required in
> the th1520clk-vo, so there is no need for else:.
That's not the question. I know it is not required, I can read code.
What is in the hardware?

Best regards,
Krzysztof

On 3/3/25 09:52, Krzysztof Kozlowski wrote:
> On 03/03/2025 09:42, Michal Wilczynski wrote:
>>>> +allOf:
>>>> +  - if:
>>>> +      properties:
>>>> +        compatible:
>>>> +          contains:
>>>> +            const: thead,th1520-clk-vo
>>>> +    then:
>>>> +      required:
>>>> +        - resets
>>>
>>> else:
>>> ? What's there? Also reset or no?
>>
>> If the else: case the reset is not required, as it's only required in
>> the th1520clk-vo, so there is no need for else:.
> That's not the question. I know it is not required, I can read code.
> What is in the hardware?

I noticed the register SW_GMAC1_GRST_N in section 5.4.2.2.66 of the
manual (GMAC1_SWRST [2]), which indicates a GMAC1 CLKGEN soft reset.
Although this could theoretically reset part of the AP clock, it is not
actually used by the AP clock driver or needed for initialization.

> 
> Best regards,
> Krzysztof
>

On 03/03/2025 10:55, Michal Wilczynski wrote:
> 
> 
> On 3/3/25 09:52, Krzysztof Kozlowski wrote:
>> On 03/03/2025 09:42, Michal Wilczynski wrote:
>>>>> +allOf:
>>>>> +  - if:
>>>>> +      properties:
>>>>> +        compatible:
>>>>> +          contains:
>>>>> +            const: thead,th1520-clk-vo
>>>>> +    then:
>>>>> +      required:
>>>>> +        - resets
>>>>
>>>> else:
>>>> ? What's there? Also reset or no?
>>>
>>> If the else: case the reset is not required, as it's only required in
>>> the th1520clk-vo, so there is no need for else:.
>> That's not the question. I know it is not required, I can read code.
>> What is in the hardware?
> 
> I noticed the register SW_GMAC1_GRST_N in section 5.4.2.2.66 of the
> manual (GMAC1_SWRST [2]), which indicates a GMAC1 CLKGEN soft reset.
> Although this could theoretically reset part of the AP clock, it is not
> actually used by the AP clock driver or needed for initialization.

Thanks, this answers here.

Best regards,
Krzysztof