Re: [PATCH 1/2] arm64: dts: exynos: Add initial support for Exynos5515 SoC

> Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
> - [Medium] The GIC-400 node lacks the VGIC maintenance interrupt property.
> - [Medium] GIC CPU interface (GICC) memory region is incorrectly sized at 0x1000, which breaks VGIC support.
> - [Low] Node unit-addresses for `cpu1` and `gic` do not match their `reg` properties.
> - [Low] Redundant and deprecated custom function IDs are provided for PSCI 1.0.
> - [Low] Missing dt-bindings clock header `samsung,exynos5515-cmu.h`.
> --
>
> --- Patch [1]: [PATCH 1/2] arm64: dts: exynos: Add initial support for Exynos5515 SoC ---
> commit e8b991884f3a3fd45bff9e8dcdd76dcc3db06db9
> Author: Aiden Isik <[email protected]>
>
> arm64: dts: exynos: Add initial support for Exynos5515 SoC
>
> The patch adds initial device tree support for the Exynos5515 SoC, used in
> Samsung Galaxy Watch models. It adds CPUs, generic timer, watchdog, and
> various clock management units.
>
> > diff --git a/arch/arm64/boot/dts/exynos/exynos5515.dtsi b/arch/arm64/boot/dts/exynos/exynos5515.dtsi
> > new file mode 100644
> > index 0000000000000..5584c1a3aaf4f
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/exynos/exynos5515.dtsi
> > @@ -0,0 +1,186 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +/*
> > + * Samsung Exynos 5515 SoC device tree source
> > + *
> > + * Copyright (c) 2026 Aiden Isik <[email protected]>
> > + */
> > +
> > +#include <dt-bindings/clock/samsung,exynos5515-cmu.h>
>
> [Severity: Low]
> Is this header file missing? It appears it was not added in this patch or
> earlier in the series, which will cause the device tree compilation to fail.
>

This header is added in the series introducing clock driver support.

> > +#include <dt-bindings/interrupt-controller/arm-gic.h>
> > +
> > +/ {
> [ ... ]
> > +		cpu1: cpu@1 {
> > +			compatible = "arm,cortex-a55";
> > +			reg = <0x100>;
>
> [Severity: Low]
> Does the unit-address for cpu1 match the reg property? The device tree
> specification requires the unit-address to match the first address in the
> reg property, so should this be cpu@100?

I took this as a CPU index, but if any maintainers want it to be this way,
then I am happy to change it.

>
> > +			device_type = "cpu";
> > +			enable-method = "psci";
> > +		};
> > +	};
> [ ... ]
> > +	psci {
> > +		compatible = "arm,psci-1.0";
> > +		cpu_off = <0x84000002>;
> > +		cpu_on = <0xc4000003>;
> > +		cpu_suspend = <0xc4000001>;
>
> [Severity: Low]
> Are these custom function IDs necessary? Since compatible = "arm,psci-1.0"
> is used, the kernel relies on standard architecturally mandated function
> IDs, and explicitly defining cpu_off, cpu_on, and cpu_suspend will be ignored.

This is how it is done downstream and in other mainline Exynos SoCs from
memory. Again, if a maintainer wants it changed I am happy to.

>
> > +		method = "smc";
> > +	};
> [ ... ]
> > +		gic: interrupt-controller@15600000 {
>
> [Severity: Low]
> Does the unit-address for the gic node match its reg property? The first
> address in the reg block is 0x15601000, so should the node be named
> interrupt-controller@15601000?

Possibly, I will look into this.

>
> [Severity: Medium]
> Is the interrupts property for the VGIC maintenance interrupt missing from
> the gic device node? Without it, KVM virtualization cannot properly handle
> virtual interrupt maintenance.
>
> > +			compatible = "arm,gic-400";
> > +			reg = <0x15601000 0x1000>,
> > +			      <0x15602000 0x1000>,
>

Unsure.

> [Severity: Medium]
> Is the GICC memory region incorrectly sized at 0x1000? KVM VGIC
> initialization requires access to the GICC_DIR register at offset 0x1000 in
> the GICC region for EOI mode 1, so should this size be at least 0x2000?
>
> > +			      <0x15604000 0x2000>,
> > +			      <0x15606000 0x2000>;
>

Unsure.

> --
> Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
>

Kind regards,
- Aiden Isik