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] [day] [month] [year] [list]

Message-ID: <CAP6Zq1h2bsODnSR6kiVmtueqbjOtEShu_=EYHtw65SPGaX+bgA@mail.gmail.com>
Date: Tue, 18 Mar 2025 08:33:43 +0200
From: Tomer Maimon <tmaimon77@...il.com>
To: Guenter Roeck <linux@...ck-us.net>
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 Guenter,

Thanks a lot for your recommendations and sorry for the inconvenience.

We will fix the device tree and send the patch soon.

Best regards,

Tomer

On Mon, 17 Mar 2025 at 16:09, Guenter Roeck <linux@...ck-us.net> wrote:
>
> 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
>