Re: [PATCH 5/5] arm64: dts: qcom: motorola-dubai: Add sound card with aw88261 speakers

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] dt-bindings: aw88261 audio channel assignment inverted
- [Low] commit-message: inaccurate description of power-domains removal
--

commit df15644ca6ae81e8d6c78e4e8358dd5c5c68d8f0
Author: Val Packett <val@packett.cool>
arm64: dts: qcom: motorola-dubai: Add sound card with aw88261 speakers

This commit enables the sndcard node with MultiMedia frontends, a USB offload
RX link, and two aw88261 speaker amplifiers. It also fixes the audiocc
compatible string and removes power-domains from a node.

> While here, use the correct compatible for audiocc for this phone,
> and remove references to power-domains from a 'reserved' node.

[Severity: Low]
Is this description accurate? The patch code removes the power-domains
property from lpass_audiocc, which is a clock controller, not a reserved
memory node.

> diff --git a/arch/arm64/boot/dts/qcom/sm7325-motorola-dubai-common.dtsi b/arch/arm64/boot/dts/qcom/sm7325-motorola-dubai-common.dtsi
> index 3860ab8508a09..1e1c195de1af1 100644
> --- a/arch/arm64/boot/dts/qcom/sm7325-motorola-dubai-common.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm7325-motorola-dubai-common.dtsi

[ ... ]

> @@ -699,6 +701,40 @@ &gpu_zap_shader {
>  	firmware-name = "qcom/sm7325/motorola/dubai/a660_zap.mbn";
>  };
>  
> +&i2c1 {
> +	clock-frequency = <100000>;
> +
> +	status = "okay";
> +
> +	/* Bottom speaker */
> +	aw88261_r: audio-codec@34 {
> +		compatible = "awinic,aw88261";
> +		reg = <0x34>;
> +
> +		dvdd-supply = <&vreg_l18b_1p8>;
> +		sound-name-prefix = "Amplifier R";
> +		firmware-name = "qcom/sm7325/motorola/dubai/aw88261_acf.bin";
> +
> +		awinic,audio-channel = <0>;

[Severity: Medium]
Does this assign the right physical speaker to the left audio channel?
Channel 0 is typically the left channel, which contradicts the
"Amplifier R" and "Bottom speaker" designation. This might result
in inverted stereo audio playback.

> +
> +		#sound-dai-cells = <0>;
> +	};
> +
> +	/* Top speaker / ear speaker */
> +	aw88261_l: audio-codec@35 {
> +		compatible = "awinic,aw88261";
> +		reg = <0x35>;
> +
> +		dvdd-supply = <&vreg_l18b_1p8>;
> +		sound-name-prefix = "Amplifier L";
> +		firmware-name = "qcom/sm7325/motorola/dubai/aw88261_acf.bin";
> +
> +		awinic,audio-channel = <1>;

[Severity: Medium]
Similarly, does this assign the left physical speaker to the right audio
channel? Assigning 1 (typically the right channel) to the top/left
speaker contradicts its physical placement and name.

> +
> +		#sound-dai-cells = <0>;
> +	};
> +};

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261016073157.611956-1-val@packett.cool?part=5