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.1On 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
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
>
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@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/
>
In that email I said you should have one node
clock-controller@ffef528000. Why did 0x50 get added to the address?
On 4/30/25 00:29, Stephen Boyd wrote:
> 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@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/
>>
>
> In that email I said you should have one node
> clock-controller@ffef528000. Why did 0x50 get added to the address?
Hi Stephen,
In the v2 version of the patchset, there was no reset controller yet, so
I thought your comment was made referring to that earlier version.
This representation clearly describes the hardware correctly, which is
the requirement for the Device Tree.
The manual, in section 5.4.1.6 VO_SUBSYS, describes the reset registers
starting at 0xFF_EF52_8000:
GPU_RST_CFG 0x00
DPU_RST_CFG 0x04
MIPI_DSI0_RST_CFG 0x8
MIPI_DSI1_RST_CFG 0xc
HDMI_RST_CFG 0x14
AXI4_VO_DW_AXI 0x18
X2H_X4_VOSYS_DW_AXI_X2H 0x20
And the clock registers for VO_SUBSYS, manual section 4.4.1.6 start at offset 0x50:
VOSYS_CLK_GATE 0x50
VOSYS_CLK_GATE1 0x54
VOSYS_DPU_CCLK_CFG0 0x64
TEST_CLK_FREQ_STAT 0xc4
TEST_CLK_CFG 0xc8
So I considered this back then and thought it was appropriate to divide
it into two nodes, as the reset node wasn't being considered at that
time.
When looking for the reference [1], I didn't notice if you corrected
yourself later, but I do remember considering the single-node approach
at the time.
>
Best regards,
--
Michal Wilczynski <m.wilczynski@samsung.com>
Quoting Michal Wilczynski (2025-04-30 00:52:29) > > In the v2 version of the patchset, there was no reset controller yet, so > I thought your comment was made referring to that earlier version. > This representation clearly describes the hardware correctly, which is > the requirement for the Device Tree. > > The manual, in section 5.4.1.6 VO_SUBSYS, describes the reset registers > starting at 0xFF_EF52_8000: > > GPU_RST_CFG 0x00 > DPU_RST_CFG 0x04 > MIPI_DSI0_RST_CFG 0x8 > MIPI_DSI1_RST_CFG 0xc > HDMI_RST_CFG 0x14 > AXI4_VO_DW_AXI 0x18 > X2H_X4_VOSYS_DW_AXI_X2H 0x20 > > And the clock registers for VO_SUBSYS, manual section 4.4.1.6 start at offset 0x50: > VOSYS_CLK_GATE 0x50 > VOSYS_CLK_GATE1 0x54 > VOSYS_DPU_CCLK_CFG0 0x64 > TEST_CLK_FREQ_STAT 0xc4 > TEST_CLK_CFG 0xc8 > > So I considered this back then and thought it was appropriate to divide > it into two nodes, as the reset node wasn't being considered at that > time. > > When looking for the reference [1], I didn't notice if you corrected > yourself later, but I do remember considering the single-node approach > at the time. > If the two register ranges don't overlap then this is probably OK. I imagine this is one device shipped by the hardware engineer, VO_SUBSYS, which happens to be a clock and reset controller. This is quite common, and we usually have one node with both #clock-cells and #reset-cells in it. Then we use the auxiliary bus to create the reset device from the clk driver with the same node. This helps match the device in the datasheet to the node and compatible in DT without making the compatible provider specific (clk or reset postfix). That's another reason why we usually have one node. DT doesn't describe software, i.e. the split between clk and reset frameworks may not exist in other operating systems. We don't want to put the software design decisions into the DT. It may also be that a device like this consumes shared power resources like clks or regulators that need to be enabled to drive the device, or an IOMMU is used to translate the register mappings. We wouldn't want to split the device in DT in that case so we can easily manage the power resources or memory mappings for the device. TL;DR: This is probably OK, but I'd be careful to not make it a thing.
On 5/6/25 23:30, Stephen Boyd wrote: > Quoting Michal Wilczynski (2025-04-30 00:52:29) >> >> In the v2 version of the patchset, there was no reset controller yet, so >> I thought your comment was made referring to that earlier version. >> This representation clearly describes the hardware correctly, which is >> the requirement for the Device Tree. >> >> The manual, in section 5.4.1.6 VO_SUBSYS, describes the reset registers >> starting at 0xFF_EF52_8000: >> >> GPU_RST_CFG 0x00 >> DPU_RST_CFG 0x04 >> MIPI_DSI0_RST_CFG 0x8 >> MIPI_DSI1_RST_CFG 0xc >> HDMI_RST_CFG 0x14 >> AXI4_VO_DW_AXI 0x18 >> X2H_X4_VOSYS_DW_AXI_X2H 0x20 >> >> And the clock registers for VO_SUBSYS, manual section 4.4.1.6 start at offset 0x50: >> VOSYS_CLK_GATE 0x50 >> VOSYS_CLK_GATE1 0x54 >> VOSYS_DPU_CCLK_CFG0 0x64 >> TEST_CLK_FREQ_STAT 0xc4 >> TEST_CLK_CFG 0xc8 >> >> So I considered this back then and thought it was appropriate to divide >> it into two nodes, as the reset node wasn't being considered at that >> time. >> >> When looking for the reference [1], I didn't notice if you corrected >> yourself later, but I do remember considering the single-node approach >> at the time. >> > > If the two register ranges don't overlap then this is probably OK. I > imagine this is one device shipped by the hardware engineer, VO_SUBSYS, > which happens to be a clock and reset controller. This is quite common, > and we usually have one node with both #clock-cells and #reset-cells in > it. Then we use the auxiliary bus to create the reset device from the > clk driver with the same node. This helps match the device in the > datasheet to the node and compatible in DT without making the compatible > provider specific (clk or reset postfix). > > That's another reason why we usually have one node. DT doesn't describe > software, i.e. the split between clk and reset frameworks may not exist > in other operating systems. We don't want to put the software design > decisions into the DT. > > It may also be that a device like this consumes shared power resources > like clks or regulators that need to be enabled to drive the device, or > an IOMMU is used to translate the register mappings. We wouldn't want to > split the device in DT in that case so we can easily manage the power > resources or memory mappings for the device. > > TL;DR: This is probably OK, but I'd be careful to not make it a thing. Thank you very much for the comprehensive explanation. Because the registers don’t overlap, it’s fine in this case. Since Drew also seem to agree, we can probably push these patches forward, while keeping in mind that for future SoCs it would be better to use a single node. > Best regards, -- Michal Wilczynski <m.wilczynski@samsung.com>
On Wed, May 07, 2025 at 12:04:01PM +0200, Michal Wilczynski wrote: > > > On 5/6/25 23:30, Stephen Boyd wrote: > > Quoting Michal Wilczynski (2025-04-30 00:52:29) > >> > >> In the v2 version of the patchset, there was no reset controller yet, so > >> I thought your comment was made referring to that earlier version. > >> This representation clearly describes the hardware correctly, which is > >> the requirement for the Device Tree. > >> > >> The manual, in section 5.4.1.6 VO_SUBSYS, describes the reset registers > >> starting at 0xFF_EF52_8000: > >> > >> GPU_RST_CFG 0x00 > >> DPU_RST_CFG 0x04 > >> MIPI_DSI0_RST_CFG 0x8 > >> MIPI_DSI1_RST_CFG 0xc > >> HDMI_RST_CFG 0x14 > >> AXI4_VO_DW_AXI 0x18 > >> X2H_X4_VOSYS_DW_AXI_X2H 0x20 > >> > >> And the clock registers for VO_SUBSYS, manual section 4.4.1.6 start at offset 0x50: > >> VOSYS_CLK_GATE 0x50 > >> VOSYS_CLK_GATE1 0x54 > >> VOSYS_DPU_CCLK_CFG0 0x64 > >> TEST_CLK_FREQ_STAT 0xc4 > >> TEST_CLK_CFG 0xc8 > >> > >> So I considered this back then and thought it was appropriate to divide > >> it into two nodes, as the reset node wasn't being considered at that > >> time. > >> > >> When looking for the reference [1], I didn't notice if you corrected > >> yourself later, but I do remember considering the single-node approach > >> at the time. > >> > > > > If the two register ranges don't overlap then this is probably OK. I > > imagine this is one device shipped by the hardware engineer, VO_SUBSYS, > > which happens to be a clock and reset controller. This is quite common, > > and we usually have one node with both #clock-cells and #reset-cells in > > it. Then we use the auxiliary bus to create the reset device from the > > clk driver with the same node. This helps match the device in the > > datasheet to the node and compatible in DT without making the compatible > > provider specific (clk or reset postfix). > > > > That's another reason why we usually have one node. DT doesn't describe > > software, i.e. the split between clk and reset frameworks may not exist > > in other operating systems. We don't want to put the software design > > decisions into the DT. > > > > It may also be that a device like this consumes shared power resources > > like clks or regulators that need to be enabled to drive the device, or > > an IOMMU is used to translate the register mappings. We wouldn't want to > > split the device in DT in that case so we can easily manage the power > > resources or memory mappings for the device. > > > > TL;DR: This is probably OK, but I'd be careful to not make it a thing. > > Thank you very much for the comprehensive explanation. Because the > registers don’t overlap, it’s fine in this case. Since Drew also seem to > agree, we can probably push these patches forward, while keeping in mind > that for future SoCs it would be better to use a single node. Yes, I think in this instance it makes sense to go ahead. I sent a pull request [1] to Stephen for my thead clk for-next tree with this series applied. Thanks, Drew [1] https://lore.kernel.org/all/aBus+Yc7kf%2FH2HE5@x1/
On Wed, Apr 30, 2025 at 09:52:29AM +0200, Michal Wilczynski wrote:
>
>
> On 4/30/25 00:29, Stephen Boyd wrote:
> > 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@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/
> >>
> >
> > In that email I said you should have one node
> > clock-controller@ffef528000. Why did 0x50 get added to the address?
>
> Hi Stephen,
> In the v2 version of the patchset, there was no reset controller yet, so
> I thought your comment was made referring to that earlier version.
> This representation clearly describes the hardware correctly, which is
> the requirement for the Device Tree.
>
> The manual, in section 5.4.1.6 VO_SUBSYS, describes the reset registers
> starting at 0xFF_EF52_8000:
>
> GPU_RST_CFG 0x00
> DPU_RST_CFG 0x04
> MIPI_DSI0_RST_CFG 0x8
> MIPI_DSI1_RST_CFG 0xc
> HDMI_RST_CFG 0x14
> AXI4_VO_DW_AXI 0x18
> X2H_X4_VOSYS_DW_AXI_X2H 0x20
>
> And the clock registers for VO_SUBSYS, manual section 4.4.1.6 start at offset 0x50:
> VOSYS_CLK_GATE 0x50
> VOSYS_CLK_GATE1 0x54
> VOSYS_DPU_CCLK_CFG0 0x64
> TEST_CLK_FREQ_STAT 0xc4
> TEST_CLK_CFG 0xc8
>
> So I considered this back then and thought it was appropriate to divide
> it into two nodes, as the reset node wasn't being considered at that
> time.
>
> When looking for the reference [1], I didn't notice if you corrected
> yourself later, but I do remember considering the single-node approach
> at the time.
>
> >
>
> Best regards,
> --
> Michal Wilczynski <m.wilczynski@samsung.com>
I chatted with Stephen on irc about setting up a thead clk branch and
sending pull requests to Stephen.
Stephen - are there changes in this series that you want to see in order
to give your Reviewed-by?
Thanks,
Drew
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>
© 2016 - 2026 Red Hat, Inc.