linux-kernel - Re: [RFC PATCH v1 12/14] riscv: dts: Add Video Output clock and syscon regmap nodes

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: <2b6ffc5a-4021-459d-ae4c-b2c472960c60@kernel.org>
Date: Tue, 3 Dec 2024 16:50:34 +0100
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Michal Wilczynski <m.wilczynski@...sung.com>, mturquette@...libre.com,
 sboyd@...nel.org, robh@...nel.org, krzk+dt@...nel.org, conor+dt@...nel.org,
 drew@...7.com, guoren@...nel.org, wefu@...hat.com, jassisinghbrar@...il.com,
 paul.walmsley@...ive.com, palmer@...belt.com, aou@...s.berkeley.edu,
 frank.binns@...tec.com, matt.coster@...tec.com,
 maarten.lankhorst@...ux.intel.com, mripard@...nel.org, tzimmermann@...e.de,
 airlied@...il.com, simona@...ll.ch, ulf.hansson@...aro.org,
 jszhang@...nel.org, m.szyprowski@...sung.com
Cc: linux-clk@...r.kernel.org, devicetree@...r.kernel.org,
 linux-kernel@...r.kernel.org, linux-riscv@...ts.infradead.org,
 dri-devel@...ts.freedesktop.org, linux-pm@...r.kernel.org
Subject: Re: [RFC PATCH v1 12/14] riscv: dts: Add Video Output clock and
 syscon regmap nodes

On 03/12/2024 14:41, Michal Wilczynski wrote:
> The address space controlling the Video Output (VO) subsystem clocks
> also contains control registers for GPU resets. To properly synchronize
> access to this shared address space, create a syscon Device Tree node
> for the VO registers and reference it in the clock controller node.
> 
> This change ensures coordinated access to the VO registers between the
> clock controller and other drivers, preventing conflicts and maintaining
> system stability.
> 
> Signed-off-by: Michal Wilczynski <m.wilczynski@...sung.com>
> ---
>  arch/riscv/boot/dts/thead/th1520.dtsi | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/arch/riscv/boot/dts/thead/th1520.dtsi b/arch/riscv/boot/dts/thead/th1520.dtsi
> index dc2d554b4a71..39d39059160d 100644
> --- a/arch/riscv/boot/dts/thead/th1520.dtsi
> +++ b/arch/riscv/boot/dts/thead/th1520.dtsi
> @@ -489,6 +489,18 @@ clk: clock-controller@...f010000 {
>  			#clock-cells = <1>;
>  		};
>  
> +		vosys_clk: clock-controller {

Missing address space. You cannot have here nodes without unit address.

It does not look like you tested the DTS against bindings. Please run
`make dtbs_check W=1` (see
Documentation/devicetree/bindings/writing-schema.rst or
https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/
for instructions).

> +			compatible = "thead,th1520-clk-vo";
> +			thead,vosys-regmap = <&vosys_reg>;
> +			#clock-cells = <1>;
> +		};
> +
> +		vosys_reg: vosys@...f528000 {

Node names should be generic. See also an explanation and list of
examples (not exhaustive) in DT specification:
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation



> +			compatible = "thead,th1520-vosys", "syscon";
> +			reg = <0xff 0xef528000 0x0 0x1000>;
> +			status = "okay";

Where is it disabled? Drop.

Best regards,
Krzysztof