Re: [PATCH v5 4/4] arm64: dts: nuvoton: npcm845-evb: Add peripheral nodes

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] The reserved memory region 'tip_reserved' for the TIP firmware lacks the 'no-map' property, which will cause memory corruption or speculative access crashes.
- [High] The `cooling-levels` property in the `&pwm_fan` sub-nodes is configured as a standard 32-bit integer array (`<127 255>`) instead of an 8-bit integer array (`/bits/ 8 <127 255>`).
- [Medium] Aliases 'fiu1' and 'fiu3' are cross-assigned to '&fiu3' and '&fiu1', contradicting their fixed-partition labels.
- [Low] The property 'snps,eee-force-disable' is undocumented and is not parsed by any kernel driver.
- [Low] Duplicate unit-address in `fiu0` flash partitions.
--

commit 157ec4fb1bc26d1a937aeb0fed0b00add10a7c95
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. Add board aliases, GPIO-backed
MDIO buses, reserved memory, flash partitions, and various controllers.

> diff --git a/arch/arm64/boot/dts/nuvoton/nuvoton-npcm845-evb.dts b/arch/arm64/boot/dts/nuvoton/nuvoton-npcm845-evb.dts
> index 5edf5d13342da..28dbb865d739d 100644
> --- a/arch/arm64/boot/dts/nuvoton/nuvoton-npcm845-evb.dts
> +++ b/arch/arm64/boot/dts/nuvoton/nuvoton-npcm845-evb.dts
> @@ -10,6 +10,42 @@ / {
>  
>  	aliases {
>  		serial0 = &serial0;
> +		ethernet1 = &gmac1;
[ ... ]
> +		mdio-gpio1 = &mdio1;
> +		fiu0 = &fiu0;
> +		fiu1 = &fiu3;
> +		fiu2 = &fiux;
> +		fiu3 = &fiu1;

[Severity: Medium]
Does this alias mapping intentionally cross-assign fiu1 to &fiu3 and fiu3 to
&fiu1?

Looking further down in the patch, the partition under &fiu1 is explicitly
labeled spi1-system1, and the partition under &fiu3 is labeled spi3-system1.
This swap could cause user-space confusion by mapping /dev/mtd1 to a partition
labeled spi3-system1 and /dev/mtd3 to a partition labeled spi1-system1.

[ ... ]
> +	reserved-memory {
> +		#address-cells = <2>;
> +		#size-cells = <2>;
> +		ranges;
> +
> +		tip_reserved: tip@0 {
> +			reg = <0x0 0x0 0x0 0x6200000>;
> +		};
> +	};

[Severity: High]
Without the no-map property, will the kernel map this 98MB reserved memory
region into the standard linear mapping with cacheable attributes?

Since this memory belongs to the TIP coprocessor, the ARM64 CPU might
aggressively perform speculative instruction prefetches and data loads here.
Could this lead to corrupted non-coherent working state via cache line
evictions, or trigger fatal asynchronous SErrors and synchronous external
aborts if the memory is hardware-protected by TrustZone?

[ ... ]
> +&gmac1 {
> +	phy-mode = "rgmii-id";
> +	snps,eee-force-disable;
> +	status = "okay";
> +};

[Severity: Low]
Is the snps,eee-force-disable property supported? It does not appear in the
snps,dwmac DT binding schema and seems to be ignored by the stmmac driver.
This appears to be a dead configuration that triggers a dt-schema violation.

[ ... ]
> +&fiu0 {
> +	status = "okay";
> +	flash@0 {
> +		compatible = "jedec,spi-nor";
> +		spi-rx-bus-width = <1>;
> +		reg = <0>;
> +		spi-max-frequency = <5000000>;
> +		partitions {
> +			compatible = "fixed-partitions";
> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +			bmc@0 {
> +				label = "bmc";
> +				reg = <0x00000000 0x04000000>;
> +			};
> +			u-boot@0 {
> +				label = "u-boot";
> +				reg = <0x00000000 0x007C0000>;
> +			};

[Severity: Low]
Both the bmc and u-boot partitions are using the @0 unit-address. Does this
result in a duplicate unit-address warning from the devicetree compiler?

[ ... ]
> +&pwm_fan {
> +	status = "okay";
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pwm0_pins &pwm1_pins
[ ... ]
> +	#address-cells = <1>;
> +	#size-cells = <0>;
> +	fan@0 {
> +		reg = <0x00>;
> +		fan-tach-ch = /bits/ 8 <0x0 0x1>;
> +		cooling-levels = <127 255>;
> +	};

[Severity: High]
Should the cooling-levels property be configured as an 8-bit array, matching
the fan-tach-ch format, such as /bits/ 8 <127 255>?

The Nuvoton PWM fan driver parses this property using
of_property_read_u8_array() and of_property_count_u8_elems().

Since the DT property is authored as two 32-bit big-endian integers, it
consumes 8 bytes total. The driver might incorrectly count 8 elements and
parse them as an array of [0, 0, 0, 127, 0, 0, 0, 255].

Could this corrupt the thermal control map and shut the fans off completely
at intermediate or critical temperatures?

[ ... ]

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