[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <9ce45e7c1769a25ea1abfaeac9aefcfb@kernel.org>
Date: Tue, 29 Apr 2025 15:29:45 -0700
From: Stephen Boyd <sboyd@...nel.org>
To: Drew Fustini <drew@...7.com>, Michal Wilczynski <m.wilczynski@...sung.com>
Cc: mturquette@...libre.com, robh@...nel.org, krzk+dt@...nel.org, conor+dt@...nel.org, guoren@...nel.org, wefu@...hat.com, paul.walmsley@...ive.com, palmer@...belt.com, aou@...s.berkeley.edu, alex@...ti.fr, jszhang@...nel.org, p.zabel@...gutronix.de, m.szyprowski@...sung.com, linux-clk@...r.kernel.org, devicetree@...r.kernel.org, linux-kernel@...r.kernel.org, linux-riscv@...ts.infradead.org
Subject: Re: [PATCH v7 3/3] riscv: dts: thead: Add device tree VO clock controller
Quoting Michal Wilczynski (2025-04-07 08:30:43)
> On 4/5/25 01:16, Drew Fustini wrote:
> >> 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@...f010000 {
> >> #clock-cells = <1>;
> >> };
> >>
> >> + clk_vo: clock-controller@...f528050 {
> >> + 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/
>
In that email I said you should have one node
clock-controller@...f528000. Why did 0x50 get added to the address?
Powered by blists - more mailing lists