Re: [PATCH v2 1/2] arm64: dts: nuvoton: npcm845: Add peripheral nodes

Tomer Maimon
2025-09-21T15:56:29Z

Hi Andrew,

Thanks for your comments.

On Wed, 10 Sept 2025 at 10:52, Andrew Jeffery
<andrew@codeconstruct.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@gmail.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@fb002000 {
> > + 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@c0000000 {
> > + 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?
> > +
> > + fiux: spi@fb001000 {
> > + 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@f0824000 {
> > + 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?

peci: peci-controller@100000 {
compatible = "nuvoton,npcm845-peci";
reg = <0x100000 0x1000>;
interrupts = <GIC_SPI 6 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&clk NPCM8XX_CLK_APB3>;
cmd-timeout-ms = <1000>;
status = "disabled";
};

timer0: timer@8000 {
compatible = "nuvoton,npcm845-timer";
interrupts = <GIC_SPI 32 IRQ_TYPE_LEVEL_HIGH>;
reg = <0x8000 0x1C>;
clocks = <&clk NPCM8XX_CLK_REFCLK>;
clock-names = "refclk";
};

Thanks,

Tomer