Re: [PATCH 2/2] arm64: dts: axiado: Add initial support for AX3005 SoC and eval board

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] All secondary CPUs share the exact same `cpu-release-addr` for the SMP spin-table.
- [Medium] The ARMv8 architectural timer and GIC maintenance interrupts are incorrectly configured as active-high.
- [Low] Mismatch between the memory node's unit address and its `reg` base address.
- [Low] Missing `clock-frequency` property in the `timer` node despite indicating firmware did not configure registers.
--

commit 9c7369abb8e810cca148d4bf6cc3f8266fdfad15
Author: Swark Yang <syang@axiado.com>
Subject: arm64: dts: axiado: Add initial support for AX3005 SoC and eval board

This commit adds initial device tree support for the AX3005 SoC and its
evaluation board. The AX3005 is a multi-core SoC featuring 4 Cortex-A53
cores, and this adds the CPUs, timer, GPIO, UART, I2C, I3C, SPI and USB
controllers.

Link: None

> diff --git a/arch/arm64/boot/dts/axiado/ax3005-evk.dts b/arch/arm64/boot/dts/axiado/ax3005-evk.dts
> new file mode 100644
> --- /dev/null
> +++ b/arch/arm64/boot/dts/axiado/ax3005-evk.dts
[ ... ]
> +	memory@0 {
> +		device_type = "memory";
> +		/* Cortex-A53 will use following memory map */
> +		reg = <0x0 0x81000000 0x0 0x7f000000>;
> +	};

[Severity: Low]
Should this unit address match the base address? The memory node specifies a
base address of 0x81000000 in its reg property, but the unit address is @0.
This will cause a dtc compilation warning for unit_address_vs_reg.

[ ... ]
> diff --git a/arch/arm64/boot/dts/axiado/ax3005.dtsi b/arch/arm64/boot/dts/axiado/ax3005.dtsi
> new file mode 100644
> --- /dev/null
> +++ b/arch/arm64/boot/dts/axiado/ax3005.dtsi
[ ... ]
> +		cpu1: cpu@1 {
> +			compatible = "arm,cortex-a53";
> +			device_type = "cpu";
> +			reg = <0x0 0x1>;
> +			enable-method = "spin-table";
> +			cpu-release-addr = <0x0 0x80002fa0>;

[Severity: High]
Are all secondary CPUs supposed to share the exact same cpu-release-addr?

If cpu1, cpu2, and cpu3 all poll 0x80002fa0, the first wake event will
trigger all of them simultaneously. Will this cause concurrent jumps into
secondary_startup and corrupt the stack during SMP bring-up since they would
all attempt to consume the single cpu_boot_task?

[ ... ]
> +	timer {
> +		compatible = "arm,armv8-timer";
> +		interrupt-parent = <&gic500>;
> +		interrupts = <GIC_PPI 13 IRQ_TYPE_LEVEL_HIGH>,
> +			     <GIC_PPI 14 IRQ_TYPE_LEVEL_HIGH>,
> +			     <GIC_PPI 11 IRQ_TYPE_LEVEL_HIGH>,
> +			     <GIC_PPI 10 IRQ_TYPE_LEVEL_HIGH>;

[Severity: Medium]
Should the ARMv8 architectural timer PPIs be active-low? The generic timer
output signals usually generate active-low level-sensitive interrupts
(IRQ_TYPE_LEVEL_LOW), but these are defined as IRQ_TYPE_LEVEL_HIGH here.

> +		arm,cpu-registers-not-fw-configured;

[Severity: Low]
Does this timer node need a clock-frequency property?

The arm,cpu-registers-not-fw-configured property indicates firmware did not
initialize the timer frequency register. If CNTFRQ_EL0 is actually
unconfigured, omitting the fallback frequency can cause boot warnings or
failure in the timer subsystem.

> +	};
[ ... ]
> +		gic500: interrupt-controller@40400000 {
> +			compatible = "arm,gic-v3";
> +			reg = <0x0 0x40400000 0x0 0x10000>,
> +			      <0x0 0x40500000 0x0 0xc0000>;
> +			#address-cells = <2>;
> +			#size-cells = <2>;
> +			ranges;
> +			#interrupt-cells = <3>;
> +			interrupt-controller;
> +			#redistributor-regions = <1>;
> +			interrupts = <GIC_PPI 9 IRQ_TYPE_LEVEL_HIGH>;

[Severity: Medium]
Does the GICv3 VGIC maintenance interrupt need to be active-low? Similar to
the timer interrupts, the architecture specifies this as an active-low
level-sensitive interrupt, but it is declared as IRQ_TYPE_LEVEL_HIGH here.

> +		};

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260624-upstream-axiado-ax3005-upstream-v1-0-c05bd0bc9124@axiado.com?part=2