Re: [PATCH 2/2] arm64: dts: exynos: Add initial support for Samsung Galaxy Watch FE

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] Unaligned `no-map` simple-framebuffer region causes memory attribute aliasing.
- [Medium] Invalid usage of `reserved-memory` for memory-mapped devices and regions outside system RAM.
- [Low] Missing unit address in the `misc-rmem` device tree node.
- [Low] Mathematical error in memory calculation in commit message and comments.
--

--- Patch [2]: [PATCH 2/2] arm64: dts: exynos: Add initial support for Samsung Galaxy Watch FE ---
commit 6f30a8a96fda7068a12749f38925083c1838f92d
Author: Aiden Isik <aidenisik@member.fsf.org>

arm64: dts: exynos: Add initial support for Samsung Galaxy Watch FE

This commit adds initial device tree support for the Samsung Galaxy Watch FE,
including bootloader framebuffer and memory definitions.

> Only the lowest 490MiB and highest 512MiB of memory are used for now.
> The 488MiB between the addresses of 0x9ea00000 and 0xbe680000 is excluded

[Severity: Low]
This isn't a bug, but the math in the commit message seems slightly off.

0xbe680000 - 0x9ea00000 is 0x1fc80000, which is 508.5 MiB, not 488 MiB.

> diff --git a/arch/arm64/boot/dts/exynos/exynos5515-lucky7.dts b/arch/arm64/boot/dts/exynos/exynos5515-lucky7.dts
> index 0000000000000..f91ea7e44cf86
> --- /dev/null
> +++ b/arch/arm64/boot/dts/exynos/exynos5515-lucky7.dts
> @@ -0,0 +1,106 @@
[ ... ]
> +	memory@80000000 {
> +		/*
> +		 * Use the lowest 490MiB and highest 512MiB of memory.
> +		 * The 488MiB between the addresses of 0x9ea00000 and

[Severity: Low]
This isn't a bug, but similarly to the commit message, the calculated memory
size here is 508.5 MiB, not 488 MiB.

> +		 * 0xbe680000 is currently unused due to any memory accesses
> +		 * in several ranges between those addresses causing the core
> +		 * performing the accesses to stall.
> +		 */
> +		reg = <0x0 0x80000000 0x1ea00000>,
[ ... ]
> +	reserved-memory {
> +		ranges;
> +		#address-cells = <2>;
> +		#size-cells = <1>;
> +
> +		framebuffer@9e0cf000 {
> +			reg = <0x0 0x9e0cf000 (396 * 396 * 4)>;
> +			no-map;
> +		};

[Severity: High]
Does this unaligned size combined with no-map cause memory attribute aliasing
on arm64?

The region has an unaligned size of 0x99240 (627264 bytes). The no-map
property drops this exact byte range from the kernel's linear map, leaving the
remainder of the page (0x9e168240-0x9e169000) to be mapped as Normal Cacheable
memory.

When the simple-framebuffer driver calls ioremap_wc(), it maps the entire page
at 0x9e168000 as Write-Combine (Normal Non-Cacheable), creating an attribute
alias.

> +
> +		misc-rmem {
> +			/*
> +			 * Some of these regions belong to memory-mapped
> +			 * devices, and will be assigned to drivers as support
> +			 * is added.
> +			 *
> +			 * Other regions belong to the TrustZone firmware
> +			 * and should never be touched.
> +			 */
> +			reg = <0x0 0x87000000 0x200000>,

[Severity: Medium]
Is it valid to use reserved-memory for memory-mapped devices and regions
outside system RAM?

The Devicetree specification dictates that reserved-memory regions MUST be
contained within a defined /memory node. The misc-rmem node includes
addresses (e.g., 0xa0000000, 0xb0000000) that fall within the
0x9ea00000-0xc0000000 range explicitly omitted from the /memory node.

Additionally, since these are MMIO regions, shouldn't they be described as
device nodes on a bus rather than reserved RAM?

[Severity: Low]
This isn't a bug, but this node is missing the unit address @87000000
to match the first reg entry, which will generate a unit_address_vs_reg dtc
compiler warning.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260817-for-next-lucky7-v1-0-42ffb2b71ba2@member.fsf.org?part=2