Re: [PATCH 2/2] arm64: dts: socfpga: agilex7-gen2: Add initial device tree

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 7 potential issue(s) to consider:
- [Critical] Missing mandatory ARM architected timer (`arm,armv8-timer`) node.
- [High] The ATF (ARM Trusted Firmware) reserved memory is improperly declared as a shared DMA pool.
- [Medium] Missing Energy Aware Scheduling (EAS) capacity and topology maps for heterogeneous big.LITTLE CPUs.
- [Medium] Missing UART/serial console and `chosen` node.
- [Medium] Missing VGIC maintenance interrupt in the GICv3 node.
- [Low] Invalid combination of `reg` and `alignment` properties in `reserved-memory` nodes.
- [Low] Incorrect `#address-cells` for 64-bit CPUs.
--

commit 686f550501dd7f94cdc969838594ed32ff4cbd42
Author: Nazim Amirul <muhammad.nazim.amirul.nazle.asmade@altera.com>

arm64: dts: socfpga: agilex7-gen2: Add initial device tree

This patch adds the initial device tree support for the Intel SoCFPGA Agilex7
Gen2 platform. It includes the SoC DTSI and the SoCDK board DTS, featuring
a heterogeneous CPU cluster with Cortex-A520 and Cortex-A720 cores.

> diff --git a/arch/arm64/boot/dts/intel/socfpga_agilex7_gen2.dtsi b/arch/arm64/boot/dts/intel/socfpga_agilex7_gen2.dtsi
> new file mode 100644
> index 0000000000000..e3b777408f9a6
> --- /dev/null
> +++ b/arch/arm64/boot/dts/intel/socfpga_agilex7_gen2.dtsi
> @@ -0,0 +1,119 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (C) 2026, Altera Corporation
> + */
> +/dts-v1/;
> +#include <dt-bindings/interrupt-controller/arm-gic.h>
> +#include <dt-bindings/interrupt-controller/irq.h>
> +
> +/ {
> +	compatible = "intel,socfpga-agilex7-gen2";
> +	#address-cells = <2>;
> +	#size-cells = <2>;

[Severity: Critical]
Is it intentional to omit the arm,armv8-timer node from the root level? Without
the architectural timer node, the system may fail to establish a clocksource
and clockevents, which typically leads to an indefinite hang during early boot
when timer_probe() fails.

[Severity: Medium]
Does this platform need a chosen node and UART serial console configured at
the root level? Omitting them usually results in no interactive console or
kernel boot logs over serial, which makes early boot validation very difficult.

> +
> +	reserved-memory {
> +		#address-cells = <2>;
> +		#size-cells = <2>;
> +		ranges;
> +
> +		atf_reserved: atf@80000000 {
> +			compatible = "shared-dma-pool";

[Severity: High]
Does declaring the ATF reserved memory as a shared-dma-pool expose the secure
EL3 memory region to the kernel's DMA allocator? If the kernel resolves a DMA
allocation to this region and a device writes to it, it could overwrite the
TrustZone firmware, potentially triggering a secure access violation (SError).

> +			reg = <0x0 0x80000000 0x0 0x100000>;
> +			alignment = <0x1000>;

[Severity: Low]
Is it necessary to include the alignment property when reg is already
explicitly provided? Device tree schema validation typically rejects combining
reg and alignment in reserved-memory nodes, as the kernel parser typically
ignores alignment when reg is present.

> +			no-map;
> +		};
> +
> +		service_reserved: svcbuffer@80100000 {
> +			compatible = "shared-dma-pool";
> +			reg = <0x0 0x80100000 0x0 0xf00000>;
> +			alignment = <0x1000>;

[Severity: Low]
Similar to the ATF reserved node, is the alignment property needed here when a
reg property is explicitly defined?

> +			no-map;
> +		};
> +	};
> +
> +	cpus {
> +		#address-cells = <1>;

[Severity: Low]
Should #address-cells be 2 for 64-bit ARM CPUs? While it might work if the
Aff3 field is zero, setting it to 1 truncates the MPIDR_EL1 representation to
32 bits and violates ARM64 device tree binding conventions.

> +		#size-cells = <0>;
> +
> +		cpu0: cpu@0 {
> +			compatible = "arm,cortex-a520";
> +			device_type = "cpu";
> +			enable-method = "psci";
> +			reg = <0x0>;
> +		};

[ ... ]

> +		cpu3: cpu@300 {
> +			compatible = "arm,cortex-a720";

[Severity: Medium]
Are the Energy Aware Scheduling (EAS) capacity and topology maps missing for
these heterogeneous cores? Without them, the scheduler might treat the
Cortex-A520 and Cortex-A720 as having identical performance capabilities,
leading to suboptimal task placement.

> +			device_type = "cpu";
> +			enable-method = "psci";
> +			reg = <0x300>;
> +		};
> +	};
> +
> +	psci {
> +		compatible = "arm,psci-0.2";
> +		method = "smc";
> +	};
> +
> +	intc: interrupt-controller@7000000 {
> +		compatible = "arm,gic-v3";
> +		reg = <0x0 0x7000000 0x0 0x10000>,
> +		      <0x0 0x7080000 0x0 0x100000>;
> +		ranges;
> +		#interrupt-cells = <3>;
> +		#address-cells = <2>;
> +		#size-cells = <2>;
> +		interrupt-controller;
> +		#redistributor-regions = <1>;
> +		redistributor-stride = <0x0 0x40000>;

[Severity: Medium]
Does the GICv3 node need to declare a VGIC maintenance interrupt? Without this
interrupt, the KVM hypervisor might not receive maintenance interrupts,
which can break virtualized interrupt state management for guest VMs.

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260623111716.16690-1-muhammad.nazim.amirul.nazle.asmade@altera.com?part=2