Re: [PATCH v2 6/6] arm64: dts: apple: Initial t8122 (M3) device trees

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] The `pmgr` node's register size is too small to cover its child node `ps_disp_cpu`, causing out-of-bounds regmap access.
- [Medium] The `ps_sio_adma` (Audio DMA) power domain has an incorrect dependency on the Fast PWM (`ps_fpwm0`) power domain.
- [Medium] The `aic` (Apple Interrupt Controller) node has overlapping memory regions for `core` and `event`.
--

commit bef1d69fa7e259eaf8dc310010b063110b9823c0
Author: Janne Grunau <j@jannau.net>

arm64: dts: apple: Initial t8122 (M3) device trees

This commit adds minimal device trees for all t8122 based devices. It
includes support for the iMac and MacBook Air/Pro, with a basic set of
devices like CPU cores, interrupt controller, and power states.

> diff --git a/arch/arm64/boot/dts/apple/t8122-pmgr.dtsi b/arch/arm64/boot/dts/apple/t8122-pmgr.dtsi
> new file mode 100644
> index 0000000000000..64093792e0ade
> --- /dev/null
> +++ b/arch/arm64/boot/dts/apple/t8122-pmgr.dtsi

[ ... ]

> +	ps_sio_adma: power-controller@310 {
> +		compatible = "apple,t8122-pmgr-pwrstate", "apple,t8103-pmgr-pwrstate";
> +		reg = <0x310 4>;
> +		#power-domain-cells = <0>;
> +		#reset-cells = <0>;
> +		label = "sio_adma";
> +		power-domains = <&ps_fpwm0>;

Is it intentional that the Audio DMA power domain depends on the Fast PWM
domain? This looks like it might be a copy-paste error that will force the
PWM domain to power on alongside the audio DMA, while leaving the audio DMA
missing its actual required parent power domain.

[ ... ]

> +	ps_disp_cpu: power-controller@10000 {
> +		compatible = "apple,t8122-pmgr-pwrstate", "apple,t8103-pmgr-pwrstate";
> +		reg = <0x10000 4>;

Could this regmap access cause an out-of-bounds error?

In t8122.dtsi, the pmgr node is defined with a size of 0x10000, which sets
the syscon regmap's max_register to 0xFFFF. This power controller is declared
at offset 0x10000, which falls strictly out of bounds and might be rejected
by the regmap subsystem with an -EINVAL error.

> +		#power-domain-cells = <0>;
> +		#reset-cells = <0>;
> +		label = "disp_cpu";
> +		power-domains = <&ps_disp_fe>;
> +		apple,min-state = <4>;
> +	};

[ ... ]

> diff --git a/arch/arm64/boot/dts/apple/t8122.dtsi b/arch/arm64/boot/dts/apple/t8122.dtsi
> new file mode 100644
> index 0000000000000..2a042b6fbebcd
> --- /dev/null
> +++ b/arch/arm64/boot/dts/apple/t8122.dtsi

[ ... ]

> +		aic: interrupt-controller@2d1000000 {
> +			compatible = "apple,t8122-aic3";
> +			#interrupt-cells = <3>;
> +			interrupt-controller;
> +			reg = <0x2 0xd1000000 0x0 0x184000>,
> +				<0x2 0xd1040000 0x0 0x4>;

Is it expected for the core and event memory regions to overlap here?

The first region spans from 0xd1000000 to 0xd1184000, which completely
subsumes the second region located at 0xd1040000. Overlapping windows violate
standard device tree conventions for distinct hardware blocks. Could 0x184000
be a typo for 0x40000?

> +			reg-names = "core", "event";
> +			power-domains = <&ps_aic>;

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260505-apple-m3-initial-devicetrees-v2-0-b0c2f3519e0e@jannau.net?part=6