Re: [PATCH v4 3/3] arm64: dts: nuvoton: npcm845-evb: Add peripheral nodes

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Missing `/bits/ 8` for `fan-tach-ch` and `cooling-levels` causes incorrect 32-bit integer array emission, corrupting the thermal fan driver state.
- [High] The `tip_reserved` memory node lacks the `no-map` directive, causing speculative external aborts.
- [Medium] Incorrect Device Tree property name `spix-mode` prevents the FIU controller from enabling SPIX mode.
--

commit 011b4946d76e73a81f86b002033d53a6d052bd40
Author: Tomer Maimon <tmaimon77@gmail.com>

arm64: dts: nuvoton: npcm845-evb: Add peripheral nodes

Enable peripheral support for the NPCM845 evaluation board by wiring up
the SoC controller nodes to the board design.

> diff --git a/arch/arm64/boot/dts/nuvoton/nuvoton-npcm845-evb.dts b/arch/arm64/boot/dts/nuvoton/nuvoton-npcm845-evb.dts
> index 5edf5d13342da..ea92888d9b9e2 100644
> --- a/arch/arm64/boot/dts/nuvoton/nuvoton-npcm845-evb.dts
> +++ b/arch/arm64/boot/dts/nuvoton/nuvoton-npcm845-evb.dts

[ ... ]

> @@ -26,12 +62,389 @@ refclk: refclk-25mhz {
>  		clock-frequency = <25000000>;
>  		#clock-cells = <0>;
>  	};
> +
> +	reserved-memory {
> +		#address-cells = <2>;
> +		#size-cells = <2>;
> +		ranges;
> +
> +		tip_reserved: tip@0 {
> +			reg = <0x0 0x0 0x0 0x6200000>;

[Severity: High]
Does this reserved memory region need the no-map directive?

Without no-map, the kernel establishes a direct, cacheable linear mapping
over this region. Since this block is isolated by hardware firewalls for the
Trusted Information Processing firmware, could speculative execution or
prefetching by the CPU into this linear mapping trigger fatal synchronous
external aborts or SErrors?

> +		};
> +	};

[ ... ]

> +&fiux {
> +	spix-mode;

[Severity: Medium]
Is this property missing the vendor prefix?

The SPI driver for the NPCM FIU (drivers/spi/spi-npcm-fiu.c) appears to
check specifically for the "nuvoton,spix-mode" property. If it is only named
"spix-mode" here, will the driver fail to detect it and silently fall back
to standard mode?

> +};

[ ... ]

> +&pwm_fan {
> +	status = "okay";
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pwm0_pins &pwm1_pins
> +		&pwm2_pins &pwm3_pins
> +		&pwm4_pins &pwm5_pins
> +		&pwm6_pins &pwm7_pins
> +		&fanin0_pins &fanin1_pins
> +		&fanin2_pins &fanin3_pins
> +		&fanin4_pins &fanin5_pins
> +		&fanin6_pins &fanin7_pins>;
> +	#address-cells = <1>;
> +	#size-cells = <0>;
> +	fan@0 {
> +		reg = <0x00>;
> +		fan-tach-ch = <0x0 0x1>;
> +		cooling-levels = <127 255>;

[Severity: High]
Should these arrays be explicitly sized using /bits/ 8?

The Device Tree compiler natively emits 32-bit big-endian integers for this
syntax. However, if the driver (npcm750-pwm-fan.c) explicitly reads both
properties using of_property_read_u8_array(), an 8-byte payload like
<127 255> would be read as 8 individual u8 elements:
0, 0, 0, 127, 0, 0, 0, 255.

Does this result in registering intermediate thermal cooling states with a 0
duty cycle, inadvertently turning the fan off during active cooling requests?

> +	};

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260610160816.2828713-1-tmaimon77@gmail.com?part=3