Re: [PATCH 10/10] arm64: dts: apple: Add minimal t604x (M4 Pro/Max) device trees

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] dts: Apple desktop Mac devices reference disabled simple-framebuffer power domains
- [Medium] dts: CPU cache sizes lack 0x prefix, causing them to be incorrectly parsed as decimal
- [Low] kbuild: Makefile entries for new dtbs are not sorted alphabetically
--

commit 12545a49c68dd3594dd8e8ff795d304076ce814b
Author: Yureka Lilian <yureka@cyberchaos.dev>

arm64: dts: apple: Add minimal t604x (M4 Pro/Max) device trees

This commit adds device trees for various Apple M4 Pro and M4 Max
MacBook Pro, Mac mini, and Mac Studio models. It defines the SoC
characteristics, CPU topology, power management controllers, and
framebuffer settings.

> diff --git a/arch/arm64/boot/dts/apple/Makefile b/arch/arm64/boot/dts/apple/Makefile
> index 4f31beeb11ac4..12fc777405b3f 100644
> --- a/arch/arm64/boot/dts/apple/Makefile
> +++ b/arch/arm64/boot/dts/apple/Makefile
> @@ -94,6 +94,12 @@ dtb-$(CONFIG_ARCH_APPLE) += t6031-j516c.dtb
>  dtb-$(CONFIG_ARCH_APPLE) += t6032-j575d.dtb
>  dtb-$(CONFIG_ARCH_APPLE) += t6034-j514m.dtb
>  dtb-$(CONFIG_ARCH_APPLE) += t6034-j516m.dtb
> +dtb-$(CONFIG_ARCH_APPLE) += t6040-j773s.dtb
> +dtb-$(CONFIG_ARCH_APPLE) += t6040-j614s.dtb
> +dtb-$(CONFIG_ARCH_APPLE) += t6040-j616s.dtb

[Severity: Low]
This isn't a bug, but device tree Makefile entries are typically kept in
alphabetical order.

Could these entries be resorted so that j614s and j616s come before j773s?

[ ... ]

> diff --git a/arch/arm64/boot/dts/apple/t6040-j773s.dts b/arch/arm64/boot/dts/apple/t6040-j773s.dts
> new file mode 100644
> index 0000000000000..751c29f60d9cf
> --- /dev/null
> +++ b/arch/arm64/boot/dts/apple/t6040-j773s.dts
[ ... ]
> +/ {
> +	model = "Apple Mac mini (M4 Pro, 2024)";
> +	compatible = "apple,j773s", "apple,t6040", "apple,arm-platform";
> +};
> +
> +&framebuffer0 {
> +	power-domains = <&ps_dispext0_cpu>;
> +};

[Severity: High]
Can this cause the simple-framebuffer driver probe to fail indefinitely?

The ps_dispext0_cpu power domain provider is explicitly marked with
status = "disabled" in t6041-pmgr.dtsi because it is managed by firmware.
The generic power domain framework will refuse to bind to a disabled
provider, leaving the system with no visible console output.

Should this use an enabled frontend domain like &ps_dispext0_fe instead?

[ ... ]

> diff --git a/arch/arm64/boot/dts/apple/t6041-j575c.dts b/arch/arm64/boot/dts/apple/t6041-j575c.dts
> new file mode 100644
> index 0000000000000..0bd50072e463b
> --- /dev/null
> +++ b/arch/arm64/boot/dts/apple/t6041-j575c.dts
[ ... ]
> +/ {
> +	model = "Apple Mac Studio (M4 Max, 2025)";
> +	compatible = "apple,j575c", "apple,t6041", "apple,arm-platform";
> +};
> +
> +&framebuffer0 {
> +	power-domains = <&ps_disp_cpu>;
> +};

[Severity: High]
Similarly, can this prevent the framebuffer from working on the Mac Studio?

The ps_disp_cpu domain is also disabled in t6041-pmgr.dtsi. Using an
enabled domain like &ps_disp_fe might be needed here to avoid the screen
staying black.

[ ... ]

> diff --git a/arch/arm64/boot/dts/apple/t6041.dtsi b/arch/arm64/boot/dts/apple/t6041.dtsi
> new file mode 100644
> index 0000000000000..258048167df63
> --- /dev/null
> +++ b/arch/arm64/boot/dts/apple/t6041.dtsi
[ ... ]
> +		cpu_e0: cpu@0 {
> +			device_type = "cpu";
> +			compatible = "apple,brava-e";
> +			reg = <0x0 0x0>;
> +			cpu-release-addr = <0 0>; /* To be filled by loader */
> +			enable-method = "spin-table";
> +
> +			d-cache-size = <10000>;
> +			i-cache-size = <20000>;
> +			next-level-cache = <&l2_cache_0>;
> +		};

[Severity: Medium]
Are these cache sizes intended to be parsed as decimal integers?

Without the 0x prefix, the device tree compiler reads these as exactly
10,000 bytes and 20,000 bytes. This will cause incorrect reporting of the
cache topology to userspace and could lead to suboptimal scheduling or
memory access patterns.

Should these be prefixed with 0x (e.g. <0x10000>) to represent 64 KB and
128 KB respectively?

[ ... ]

> +		cpu_p00: cpu@10100 {
> +			device_type = "cpu";
> +			compatible = "apple,brava-p";
> +			reg = <0x0 0x10100>;
> +			cpu-release-addr = <0 0>; /* To be filled by loader */
> +			enable-method = "spin-table";
> +
> +			d-cache-size = <20000>;
> +			i-cache-size = <30000>;
> +			next-level-cache = <&l2_cache_1>;
> +		};

[Severity: Medium]
Similar to the efficiency cores above, do these performance core cache sizes
need a 0x prefix to be parsed correctly as 128 KB and 192 KB?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261010-apple-m4-pro-max-initial-devicetrees-v1-0-781fb3f2d9f6@cyberchaos.dev?part=10