Re: [PATCH 16/17] arm64: dts: imx952-evk: Add pinmux and mark conflicting devices for MQS1

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The patch silently modifies the `&sai1` node to append SoC-level clock definitions (`pll8k` and `pll11k`) in a board-specific `.dts` file, violating Devicetree abstractions and the commit message's stated scope.
--

--- Patch [16]: [PATCH 16/17] arm64: dts: imx952-evk: Add pinmux and mark conflicting devices for MQS1 ---
commit 8ad36d874036617823df79dfb507cb8569fd6f0b
Author: Chancel Liu <[email protected]>

arm64: dts: imx952-evk: Add pinmux and mark conflicting devices for MQS1

This commit adds the MQS1 pinctrl group and labels the conflicting device and
sound card so the shared imx9-mqs overlay can disable them.

> diff --git a/arch/arm64/boot/dts/freescale/imx952-evk.dts b/arch/arm64/boot/dts/freescale/imx952-evk.dts
> index e9d735dc2a51b..522c6cb9abf28 100644
> --- a/arch/arm64/boot/dts/freescale/imx952-evk.dts
> +++ b/arch/arm64/boot/dts/freescale/imx952-evk.dts

[ ... ]

> @@ -519,11 +519,21 @@ &micfil {
>  	status = "okay";
>  };
>  
> +&mqs1 {
> +	pinctrl-0 = <&pinctrl_mqs1>, <&mqs1_enable>;
> +	pinctrl-names = "default";
> +};
> +
>  &mu7 {
>  	status = "okay";
>  };
>  
>  &sai1 {
> +	clocks = <&scmi_clk IMX952_CLK_BUSAON>, <&clk_dummy>,
> +		 <&scmi_clk IMX952_CLK_SAI1>, <&clk_dummy>,
> +		 <&clk_dummy>, <&scmi_clk IMX952_CLK_AUDIOPLL1>,
> +		 <&scmi_clk IMX952_CLK_AUDIOPLL2>;
> +	clock-names = "bus", "mclk0", "mclk1", "mclk2", "mclk3", "pll8k", "pll11k";

[Severity: Medium]
Is it intentional to place these SoC-level clock definitions in the
board-specific .dts file rather than the base SoC .dtsi file?

The added clocks (IMX952_CLK_AUDIOPLL1 and IMX952_CLK_AUDIOPLL2) appear to be
internal SoC clocks. Wiring these to the sai1 peripheral seems like an
SoC-level integration detail, and placing it here might force other boards
using sai1 to duplicate this wiring.

Additionally, this change is not mentioned in the commit message. Could the
commit message be updated to document this, or should this modification be
split into a separate patch?

>  	assigned-clocks = <&scmi_clk IMX952_CLK_AUDIOPLL1_VCO>,
>  			  <&scmi_clk IMX952_CLK_AUDIOPLL2_VCO>,
>  			  <&scmi_clk IMX952_CLK_AUDIOPLL1>,

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