Re: [PATCH RFC 3/3] arm64: dts: qcom: sdm845-oneplus-fajita: Enable speaker codec

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Incorrect active polarity for the codec reset GPIO in the device tree, holding the hardware in reset.
- [Medium] The commit message incorrectly identifies the target device as 'OnePlus 6', while the patch actually modifies the 'OnePlus 6T' (Fajita) device tree.
--

--- Patch [3]: [PATCH RFC 3/3] arm64: dts: qcom: sdm845-oneplus-fajita: Enable speaker codec ---
commit 05cefbbcdc0e5b43fbfd650af4c52fe9dfdebf01
Author: David Heidelberg <david@ixit.cz>

arm64: dts: qcom: sdm845-oneplus-fajita: Enable speaker codec

This commit enables the NXP TFA9894 audio codec on the OnePlus 6T (Fajita)
device tree by adding the required I2C node and linking it to the speaker
playback DAI.

> OnePlus 6 uses NXP TFA9894 audio codec. Enable it.

[Severity: Medium]
Is the device name correct here? The patch modifies sdm845-oneplus-fajita.dts,
which corresponds to the OnePlus 6T. The OnePlus 6 (Enchilada) appears to use a
completely different codec (MAX98927). Should this specify OnePlus 6T instead?

> diff --git a/arch/arm64/boot/dts/qcom/sdm845-oneplus-fajita.dts b/arch/arm64/boot/dts/qcom/sdm845-oneplus-fajita.dts
> index 0542333a83579..cb08aab4740a4 100644
> --- a/arch/arm64/boot/dts/qcom/sdm845-oneplus-fajita.dts
> +++ b/arch/arm64/boot/dts/qcom/sdm845-oneplus-fajita.dts

[ ... ]

> @@ -40,7 +40,16 @@ &framebuffer {
>  };
>  
>  &i2c4 {
> -	/* nxp,tfa9894 @ 0x34 */
> +	clock-frequency = <400000>;
> +
> +	status = "okay";
> +
> +	tfa9894_codec: audio-codec@34 {
> +		compatible = "nxp,tfa9894";
> +		reg = <0x34>;
> +		reset-gpios = <&tlmm 69 GPIO_ACTIVE_HIGH>;

[Severity: High]
Does this GPIO polarity hold the codec in reset permanently?

Looking at tfa98xx_i2c_probe() in sound/soc/codecs/tfa98xx.c, the driver
clears the logical reset during boot:

sound/soc/codecs/tfa98xx.c:tfa98xx_i2c_probe() {
    ...
    gpiod_set_value_cansleep(reset_gpiod, 0);
    ...
}

Because the device tree specifies GPIO_ACTIVE_HIGH, deasserting the logical
reset (setting to 0) will drive the physical pin LOW. If the hardware actually
expects a physical HIGH signal to operate, this active-high configuration will
cause the subsequent I2C probe to fail.

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

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260802-tfa98xx-upstream-v1-0-4dd55cdd658a@ixit.cz?part=3