Re: [PATCH v2 11/11] arm64: dts: qcom: sdm670-google: add sound card

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] dts: incorrect routing of 1.536MHz bit clock to AHB interconnect
--

commit dd0eb0dfbca83fed6d11405e093c6017f1613bfc
Author: Richard Acayan <mailingradian@gmail.com>

arm64: dts: qcom: sdm670-google: add sound card

This commit adds devicetree entries for the Pixel 3a sound card. It
configures the WCD codec, q6asm frontends, audio routing, and the
associated clocks for headset audio.

> diff --git a/arch/arm64/boot/dts/qcom/sdm670-google-common.dtsi b/arch/arm64/boot/dts/qcom/sdm670-google-common.dtsi
> index 73639b0271e9b..fadd3b5eca42a 100644
> --- a/arch/arm64/boot/dts/qcom/sdm670-google-common.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sdm670-google-common.dtsi
[ ... ]
> @@ -52,6 +53,14 @@ framebuffer: framebuffer@9c000000 {
>  		};
>  	};
>  
> +	clocks {
> +		int_cdc_bclk: int-cdc-bclk {
> +			compatible = "fixed-clock";
> +			#clock-cells = <0>;
> +			clock-frequency = <1536000>;
> +		};
> +	};
> +
[ ... ]
> @@ -528,6 +618,17 @@ rmi4_f12: rmi4-f12@12 {
>  	};
>  };
>  
> +&lpass_codec {
> +	clocks = <&int_cdc_bclk>,
> +		 <&q6afecc LPASS_CLK_ID_INT_MCLK_0 LPASS_CLK_ATTRIBUTE_COUPLE_NO>;
> +	clock-names = "ahbix-clk", "mclk";

[Severity: Medium]
Is it safe to supply a dummy 1.536 MHz bit clock as the AHB interconnect
clock here?

The underlying digital codec driver unconditionally requires the AHB clock
in msm8916_wcd_digital_get_clks():

	priv->ahbclk = devm_clk_get(dev, "ahbix-clk");
	if (IS_ERR(priv->ahbclk)) {
		dev_err(dev, "failed to get ahbix clk\n");
		return PTR_ERR(priv->ahbclk);
	}

By satisfying "ahbix-clk" with a dummy fixed clock, the hardware state is
misrepresented. The driver will successfully "enable" this dummy clock
and proceed to access registers.

If the actual AHB clock is gated by firmware or another driver, could
this result in register accesses without the bus being clocked,
triggering a bus stall and a synchronous external abort?

> +
> +	assigned-clocks = <&q6afecc LPASS_CLK_ID_INT_MCLK_0 LPASS_CLK_ATTRIBUTE_COUPLE_NO>;
> +	assigned-clock-rates = <9600000>;
> +
> +	status = "okay";
> +};

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261003175659.23536-1-mailingradian@gmail.com?part=11