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

[Drew Fustini]

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@xxxxxxxxxxx>
> ---
>  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