1. Patchew
  2. linux
  3. [PATCH v7 0/3] Add T-HEAD TH1520 VO clock support for LicheePi 4A GPU enablement
  4. View patch

[PATCH v7 3/3] riscv: dts: thead: Add device tree VO clock controller

Michal Wilczynski posted 3 patches 1 week ago
[PATCH v7 3/3] riscv: dts: thead: Add device tree VO clock controller
Posted by Michal Wilczynski 1 week ago 
VO clocks reside in a different address space from the AP clocks on the
T-HEAD SoC. Add the device tree node of a clock-controller to handle
VO address space as well.

Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com>
---
 arch/riscv/boot/dts/thead/th1520.dtsi | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/riscv/boot/dts/thead/th1520.dtsi b/arch/riscv/boot/dts/thead/th1520.dtsi
index 527336417765..d4cba0713cab 100644
--- a/arch/riscv/boot/dts/thead/th1520.dtsi
+++ b/arch/riscv/boot/dts/thead/th1520.dtsi
@@ -489,6 +489,13 @@ clk: clock-controller@ffef010000 {
 			#clock-cells = <1>;
 		};
 
+		clk_vo: clock-controller@ffef528050 {
+			compatible = "thead,th1520-clk-vo";
+			reg = <0xff 0xef528050 0x0 0xfb0>;
+			clocks = <&clk CLK_VIDEO_PLL>;
+			#clock-cells = <1>;
+		};
+
 		dmac0: dma-controller@ffefc00000 {
 			compatible = "snps,axi-dma-1.01a";
 			reg = <0xff 0xefc00000 0x0 0x1000>;
-- 
2.34.1
Re: [PATCH v7 3/3] riscv: dts: thead: Add device tree VO clock controller
Posted by Drew Fustini 5 days, 23 hours ago 
On Thu, Apr 03, 2025 at 11:44:25AM +0200, Michal Wilczynski wrote:
> VO clocks reside in a different address space from the AP clocks on the
> T-HEAD SoC. Add the device tree node of a clock-controller to handle
> VO address space as well.
> 
> Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com>
> ---
>  arch/riscv/boot/dts/thead/th1520.dtsi | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/arch/riscv/boot/dts/thead/th1520.dtsi b/arch/riscv/boot/dts/thead/th1520.dtsi
> index 527336417765..d4cba0713cab 100644
> --- a/arch/riscv/boot/dts/thead/th1520.dtsi
> +++ b/arch/riscv/boot/dts/thead/th1520.dtsi
> @@ -489,6 +489,13 @@ clk: clock-controller@ffef010000 {
>  			#clock-cells = <1>;
>  		};
>  
> +		clk_vo: clock-controller@ffef528050 {
> +			compatible = "thead,th1520-clk-vo";
> +			reg = <0xff 0xef528050 0x0 0xfb0>;

Thanks for your patch. It is great to have more of the clocks supported
upstream.

The TH1520 System User Manual shows 0xFF_EF52_8000 for VO_SUBSYS on page
205. Is there a reason you decided to use 0xFF_EF52_8050 as the base?

I see on page 213 that the first register for VO_SUBSYS starts with
VOSYS_CLK_GATE at offset 0x50. I figure you did this to have the
CCU_GATE macros use offset of 0x0 instead 0x50.

I kind of think the reg property using the actual base address
(0xFF_EF52_8000) makes more sense as that's a closer match to the tables
in the manual. But I don't have a strong preference if you think think
using 0xef528050 makes the CCU_GATE macros easier to read.

-Drew
Re: [PATCH v7 3/3] riscv: dts: thead: Add device tree VO clock controller
Posted by Michal Wilczynski 3 days, 7 hours ago 

On 4/5/25 01:16, Drew Fustini wrote:
> On Thu, Apr 03, 2025 at 11:44:25AM +0200, Michal Wilczynski wrote:
>> VO clocks reside in a different address space from the AP clocks on the
>> T-HEAD SoC. Add the device tree node of a clock-controller to handle
>> VO address space as well.
>>
>> Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com>
>> ---
>>  arch/riscv/boot/dts/thead/th1520.dtsi | 7 +++++++
>>  1 file changed, 7 insertions(+)
>>
>> diff --git a/arch/riscv/boot/dts/thead/th1520.dtsi b/arch/riscv/boot/dts/thead/th1520.dtsi
>> index 527336417765..d4cba0713cab 100644
>> --- a/arch/riscv/boot/dts/thead/th1520.dtsi
>> +++ b/arch/riscv/boot/dts/thead/th1520.dtsi
>> @@ -489,6 +489,13 @@ clk: clock-controller@ffef010000 {
>>  			#clock-cells = <1>;
>>  		};
>>  
>> +		clk_vo: clock-controller@ffef528050 {
>> +			compatible = "thead,th1520-clk-vo";
>> +			reg = <0xff 0xef528050 0x0 0xfb0>;
> 
> Thanks for your patch. It is great to have more of the clocks supported
> upstream.
> 
> The TH1520 System User Manual shows 0xFF_EF52_8000 for VO_SUBSYS on page
> 205. Is there a reason you decided to use 0xFF_EF52_8050 as the base?
> 
> I see on page 213 that the first register for VO_SUBSYS starts with
> VOSYS_CLK_GATE at offset 0x50. I figure you did this to have the
> CCU_GATE macros use offset of 0x0 instead 0x50.
> 
> I kind of think the reg property using the actual base address
> (0xFF_EF52_8000) makes more sense as that's a closer match to the tables
> in the manual. But I don't have a strong preference if you think think
> using 0xef528050 makes the CCU_GATE macros easier to read.

Thank you for your comment.

This was discussed some time ago. The main issue was that the address
space was fragmented between clocks and resets. Initially, I proposed
using syscon as a way to abstract this, but the idea wasn't particularly
well received.

So at the start of the 0xFF_EF52_8000 there is a reset register GPU_RST_CFG
I need for resetting the GPU.

For reference, here's the earlier discussion: [1]

[1] - https://lore.kernel.org/all/1b05b11b2a8287c0ff4b6bdd079988c7.sboyd@kernel.org/

Regards,
Michał

> 
> -Drew
>
Re: [PATCH v7 3/3] riscv: dts: thead: Add device tree VO clock controller
Posted by Drew Fustini 3 days, 4 hours ago 
On Mon, Apr 07, 2025 at 05:30:43PM +0200, Michal Wilczynski wrote:
> 
> 
> On 4/5/25 01:16, Drew Fustini wrote:
> > On Thu, Apr 03, 2025 at 11:44:25AM +0200, Michal Wilczynski wrote:
> >> VO clocks reside in a different address space from the AP clocks on the
> >> T-HEAD SoC. Add the device tree node of a clock-controller to handle
> >> VO address space as well.
> >>
> >> Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com>
> >> ---
> >>  arch/riscv/boot/dts/thead/th1520.dtsi | 7 +++++++
> >>  1 file changed, 7 insertions(+)
> >>
> >> diff --git a/arch/riscv/boot/dts/thead/th1520.dtsi b/arch/riscv/boot/dts/thead/th1520.dtsi
> >> index 527336417765..d4cba0713cab 100644
> >> --- a/arch/riscv/boot/dts/thead/th1520.dtsi
> >> +++ b/arch/riscv/boot/dts/thead/th1520.dtsi
> >> @@ -489,6 +489,13 @@ clk: clock-controller@ffef010000 {
> >>  			#clock-cells = <1>;
> >>  		};
> >>  
> >> +		clk_vo: clock-controller@ffef528050 {
> >> +			compatible = "thead,th1520-clk-vo";
> >> +			reg = <0xff 0xef528050 0x0 0xfb0>;
> > 
> > Thanks for your patch. It is great to have more of the clocks supported
> > upstream.
> > 
> > The TH1520 System User Manual shows 0xFF_EF52_8000 for VO_SUBSYS on page
> > 205. Is there a reason you decided to use 0xFF_EF52_8050 as the base?
> > 
> > I see on page 213 that the first register for VO_SUBSYS starts with
> > VOSYS_CLK_GATE at offset 0x50. I figure you did this to have the
> > CCU_GATE macros use offset of 0x0 instead 0x50.
> > 
> > I kind of think the reg property using the actual base address
> > (0xFF_EF52_8000) makes more sense as that's a closer match to the tables
> > in the manual. But I don't have a strong preference if you think think
> > using 0xef528050 makes the CCU_GATE macros easier to read.
> 
> Thank you for your comment.
> 
> This was discussed some time ago. The main issue was that the address
> space was fragmented between clocks and resets. Initially, I proposed
> using syscon as a way to abstract this, but the idea wasn't particularly
> well received.
> 
> So at the start of the 0xFF_EF52_8000 there is a reset register GPU_RST_CFG
> I need for resetting the GPU.
> 
> For reference, here's the earlier discussion: [1]
> 
> [1] - https://lore.kernel.org/all/1b05b11b2a8287c0ff4b6bdd079988c7.sboyd@kernel.org/

Thanks for the explanation.

Reviewed-by: Drew Fustini <drew@pdp7.com>

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.