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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <c46de621e098b7873a00c1af4ca550a1@kernel.org>
Date: Tue, 06 May 2025 14:30:27 -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-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.