linux-kernel - Re: [PATCH v2 1/2] arm64: dts: nuvoton: npcm845: Add peripheral 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] [day] [month] [year] [list]
Message-ID: <9ed40c5439dc8ecaa7b265e21d3cf15d45415bee.camel@codeconstruct.com.au>
Date: Mon, 22 Sep 2025 13:21:59 +0930
From: Andrew Jeffery <andrew@...econstruct.com.au>
To: Tomer Maimon <tmaimon77@...il.com>
Cc: robh+dt@...nel.org, krzysztof.kozlowski+dt@...aro.org,
 conor+dt@...nel.org, 	avifishman70@...il.com, tali.perry1@...il.com,
 joel@....id.au, venture@...gle.com, 	yuenn@...gle.com,
 benjaminfair@...gle.com, openbmc@...ts.ozlabs.org, 
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 1/2] arm64: dts: nuvoton: npcm845: Add peripheral
 nodes

Hi Tomer,

On Sun, 2025-09-21 at 18:56 +0300, Tomer Maimon wrote:
> Hi Andrew,
> 
> Thanks for your comments.
> 
> On Wed, 10 Sept 2025 at 10:52, Andrew Jeffery
> <andrew@...econstruct.com.au> wrote:
> > 
> > Hi Tomer,
> > 
> > On Mon, 2025-09-08 at 15:59 +0300, Tomer Maimon wrote:
> > > Enable peripheral support for the Nuvoton NPCM845 SoC by adding
> > > device
> > > nodes for Ethernet controllers, MMC controller, SPI controllers,
> > > USB
> > > device controllers, random number generator, ADC, PWM-FAN
> > > controller,
> > > and I2C controllers. Include pinmux configurations for relevant
> > > peripherals to support hardware operation. Add an OP-TEE firmware
> > > node
> > > for secure services.
> > > 
> > > Signed-off-by: Tomer Maimon <tmaimon77@...il.com>
> > > ---
> > >  .../dts/nuvoton/nuvoton-common-npcm8xx.dtsi   | 702
> > > +++++++++++++++++-
> > >  .../boot/dts/nuvoton/nuvoton-npcm845.dtsi     |   7 +
> > >  2 files changed, 708 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/arch/arm64/boot/dts/nuvoton/nuvoton-common-
> > > npcm8xx.dtsi b/arch/arm64/boot/dts/nuvoton/nuvoton-common-
> > > npcm8xx.dtsi
> > > index 24133528b8e9..7f120da3310a 100644
> > > 
> > 
> > *snip*
> > 
> > > +               fiu1: spi@...02000 {
> > > +                       compatible = "nuvoton,npcm845-fiu";
> > > +                       #address-cells = <1>;
> > > +                       #size-cells = <0>;
> > > +                       reg = <0x0 0xfb002000 0x0 0x1000>;
> > > +                       reg-names = "control";
> > > +                       clocks =  <&clk NPCM8XX_CLK_SPI1>;
> > > +                       clock-names = "clk_spi1";
> > > +                       pinctrl-names = "default";
> > > +                       pinctrl-0 = <&spi1_pins>;
> > > +                       status = "disabled";
> > > +               };
> > > +
> > > +               fiu3: spi@...00000 {
> > > +                       compatible = "nuvoton,npcm845-fiu";
> > > +                       #address-cells = <1>;
> > > +                       #size-cells = <0>;
> > > +                       reg = <0x0 0xc0000000 0x0 0x1000>;
> > > +                       reg-names = "control";
> > > +                       clocks =  <&clk NPCM8XX_CLK_SPI3>;
> > > +                       clock-names = "clk_spi3";
> > > +                       pinctrl-names = "default";
> > > +                       pinctrl-0 = <&spi3_pins>;
> > > +                       status = "disabled";
> > > +               };
> I don't need to move the FIU3 node as ordered by ascending unit
> address since the FIU3 node is a part of the FIU's group, am I
> correct?

It's acceptable by the linked coding standard, but my preference is for
the consistency of ordering by unit address.

> > > +
> > > +               fiux: spi@...01000 {
> > > +                       compatible = "nuvoton,npcm845-fiu";
> > > +                       #address-cells = <1>;
> > > +                       #size-cells = <0>;
> > > +                       reg = <0x0 0xfb001000 0x0 0x1000>,
> > > +                       <0x0 0xf8000000 0x0 0x2000000>;
> > > +                       reg-names = "control", "memory";
> > > +                       clocks =  <&clk NPCM8XX_CLK_SPIX>;
> > > +                       clock-names = "clk_ahb";
> > > +                       status = "disabled";
> > > +               };
> > 
> > Can you please audit the patch (and the rest of the dtsi) to make
> > sure
> > all nodes are ordered by ascending unit address, as per the DTS
> > style
> > guide?
> > 
> > https://docs.kernel.org/devicetree/bindings/dts-coding-style.html#order-of-nodes
> > 
> > Andrew
> > 
> > > +
> > > +               mc: memory-controller@...24000 {
> > > +                       compatible = "nuvoton,npcm845-memory-
> > > controller";
> > > +                       reg = <0x0 0xf0824000 0x0 0x1000>;
> > > +                       interrupts = <GIC_SPI 25
> > > IRQ_TYPE_LEVEL_HIGH>;
> > > +               };
> > > +
> > 
> > *snip*
> 
> I see disorder in the upstream dtsi file, for example, PECI order. Do
> you suggest fixing it?

If you don't mind, yes please (... as a separate patch) :)

Andrew