Re: [PATCH 13/15] arm64: dts: st: support Engicam MicroGEA-STM32MP257-RMM board

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Incorrect clock override on the parent `&sai1` node.
- [Low] Redundant redeclaration of labeled sub-nodes 'sai1a' and 'sai1b'.
--

commit 33b586ab74c039ad01865906f417ac15ced15e98
Author: Dario Binacchi <dario.binacchi@amarulasolutions.com>

arm64: dts: st: support Engicam MicroGEA-STM32MP257-RMM board

This commit adds device tree support for the Engicam MicroGEA-STM32MP257-RMM
board. It configures peripherals including eMMC, DRAM, CAN, LEDs, LCD panel,
SD card, audio codec, and buzzer.

> diff --git a/arch/arm64/boot/dts/st/stm32mp257-engicam-microgea-rmm.dts b/arch/arm64/boot/dts/st/stm32mp257-engicam-microgea-rmm.dts
> new file mode 100644
> index 0000000000000..1d5e023b3b442
> --- /dev/null
> +++ b/arch/arm64/boot/dts/st/stm32mp257-engicam-microgea-rmm.dts

[ ... ]

> +&sai1 {
> +	pinctrl-names = "default", "sleep";
> +	pinctrl-0 = <&sai1a_pins_a>, <&sai1b_pins_a>;
> +	pinctrl-1 = <&sai1a_sleep_pins_a>, <&sai1b_sleep_pins_a>;
> +	status = "okay";
> +	clocks = <&rcc CK_KER_SAI1>;

Does overriding the clocks property here remove the peripheral bus clock
reference? The base SoC device tree defines this as clocks = <&rcc CK_BUS_SAI1>
with clock-names = "pclk".

If the bus clock is omitted, the clock framework might disable it, potentially
leading to bus faults when the st,stm32mp25-sai driver accesses the registers.

Should the kernel clock (CK_KER_SAI1) be provided only to the sai1a and sai1b
sub-nodes instead?

> +
> +	sai1a: audio-controller@40290004 {

This isn't a bug, but does redefining the entire node structure and redeclaring
the label for sai1a (and sai1b below) clutter the tree and potentially trigger
duplicate label warnings?

The idiomatic approach is usually to reference the existing labeled nodes
directly at the root level using &sai1a and &sai1b.

> +		#clock-cells = <0>;
> +		dma-names = "tx";
> +		status = "okay";
> +
> +		sai1a_port: port {
> +			sai1a_endpoint: endpoint {
> +				remote-endpoint = <&sgtl5000_tx_endpoint>;
> +				dai-format = "i2s";
> +				mclk-fs = <512>;
> +			};
> +		};
> +	};
> +
> +	sai1b: audio-controller@40290024 {
> +		dma-names = "rx";
> +		st,sync = <&sai1a 2>;
> +		clocks = <&rcc CK_KER_SAI1>, <&sai1a>;
> +		clock-names = "sai_ck", "MCLK";
> +		status = "okay";

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260518143150.3138712-1-dario.binacchi@amarulasolutions.com?part=13