linux-kernel - Re: [PATCH v28 2/3] reset: npcm: register npcm8xx clock auxiliary bus device

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: <9a9de8bd-d864-4e29-89b2-91db8aea8ce5@roeck-us.net>
Date: Mon, 17 Mar 2025 07:09:25 -0700
From: Guenter Roeck <linux@...ck-us.net>
To: Tomer Maimon <tmaimon77@...il.com>
Cc: mturquette@...libre.com, sboyd@...nel.org, p.zabel@...gutronix.de,
 robh+dt@...nel.org, krzysztof.kozlowski+dt@...aro.org,
 tali.perry1@...il.com, joel@....id.au, venture@...gle.com, yuenn@...gle.com,
 benjaminfair@...gle.com, openbmc@...ts.ozlabs.org,
 linux-clk@...r.kernel.org, linux-kernel@...r.kernel.org,
 devicetree@...r.kernel.org
Subject: Re: [PATCH v28 2/3] reset: npcm: register npcm8xx clock auxiliary bus
 device

Hi Tomer,

On 3/17/25 03:39, Tomer Maimon wrote:
> Hi Guenter,
> 
> Yes, of course, it works in real hardware.
> The modification was made since the reset and clock share the same
> register memory region.
> 
> To enable the clock change needs to be done in the device tree as
> follows (we are planning to send these change patches soon):
> 
> diff -Naur a/arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi
> b/arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi
> --- a/arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi
> 2025-02-26 16:20:39.000000000 +0200
> +++ b/arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi
> 2025-03-17 12:29:17.876551537 +0200
> @@ -47,19 +47,16 @@
>                  interrupt-parent = <&gic>;
>                  ranges;
> 
> -               rstc: reset-controller@...01000 {
> +               clk: rstc: reset-controller@...01000 {
>                          compatible = "nuvoton,npcm845-reset";
> -                       reg = <0x0 0xf0801000 0x0 0x78>;
> -                       #reset-cells = <2>;
> +                       reg = <0x0 0xf0801000 0x0 0xC4>;
>                          nuvoton,sysgcr = <&gcr>;
> -               };
> -
> -               clk: clock-controller@...01000 {
> -                       compatible = "nuvoton,npcm845-clk";
> +                       #reset-cells = <2>;
> +                       clocks = <&refclk>;
>                          #clock-cells = <1>;
> -                       reg = <0x0 0xf0801000 0x0 0x1000>;
>                  };
> 
> +
>                  apb {
>                          #address-cells = <1>;
>                          #size-cells = <1>;
> diff -Naur a/arch/arm64/boot/dts/nuvoton/nuvoton-npcm845-evb.dts
> b/arch/arm64/boot/dts/nuvoton/nuvoton-npcm845-evb.dts
> --- a/arch/arm64/boot/dts/nuvoton/nuvoton-npcm845-evb.dts
> 2025-02-26 16:20:39.000000000 +0200
> +++ b/arch/arm64/boot/dts/nuvoton/nuvoton-npcm845-evb.dts
> 2025-03-17 12:24:52.293171764 +0200
> @@ -19,6 +19,13 @@
>          memory@0 {
>                  reg = <0x0 0x0 0x0 0x40000000>;
>          };
> +
> +       refclk: refclk-25mhz {
> +               compatible = "fixed-clock";
> +               clock-output-names = "ref";
> +               clock-frequency = <25000000>;
> +               #clock-cells = <0>;
> +       };
>   };
> 
>   &serial0 {
> 
> Is it better to modify the reset driver with your suggestion or change
> the device tree?
> 

My assumption was that the devicetree file is correct, and that it would match
the devicetree file in the actual devices. I since noticed that the file is
widely incomplete when comparing it with the various downstream versions,
so that was obviously wrong. Also, my change seemed odd, but then I did
not know how such situations are supposed to be handled.

Also, it looks like the devicetree file needs to be changed anyway unless something
else is wrong, because booting Linux still stalls later. Presumably that is because
the reference clock is missing in the upstream devicetree file (the serial port clock
frequency is reported as 0). Given this, fixing the devicetree files seems to be the
way to go.

Thanks,
Guenter