Re: [PATCH RFC 6/6] arm64: dts: qcom: sdm845-google: Add basic audio support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The `reset-gpios` property for the CS35L36 codecs incorrectly specifies `GPIO_ACTIVE_HIGH` to work around a logical bug in the CS35L36 driver.
--

commit 380e434eaeea6507428e5e193d27e7812369af1c
Author: David Heidelberg <david@ixit.cz>

arm64: dts: qcom: sdm845-google: Add basic audio support

This commit introduces audio support for the sdm845-google devices. It adds
the sound card and wires up two CS35L36 audio codecs for the top and bottom
speakers.

> diff --git a/arch/arm64/boot/dts/qcom/sdm845-google-common.dtsi b/arch/arm64/boot/dts/qcom/sdm845-google-common.dtsi
> index b2dc0327dff85..d00fee7ebd424 100644
> --- a/arch/arm64/boot/dts/qcom/sdm845-google-common.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sdm845-google-common.dtsi

[ ... ]

> @@ -409,9 +411,53 @@ nfc@28 {
>  };
>  
>  &i2c12 {
> -	/* Bottom spkr (right) CS35L36 @ 40 */
> +	status = "okay";
> +
> +	/* CS35L36, Bottom Speaker */
> +	cs35l36_bottom: audio-codec@40 {
> +		compatible = "cirrus,cs35l36";
> +		reg = <0x40>;
> +		reset-gpios = <&tlmm 112 GPIO_ACTIVE_HIGH>;

[Severity: Low]
Is this reset-gpios polarity intentionally set to GPIO_ACTIVE_HIGH to work
around the cs35l36 driver bug?

Since the CS35L36 hardware reset pin is active-low, shouldn't devicetree
accurately describe the hardware rather than encode a software workaround?

The driver in sound/soc/codecs/cs35l36.c currently treats the GPIO as an
enable signal, requesting it with GPIOD_OUT_LOW and writing a logical 1
to run. Setting GPIO_ACTIVE_HIGH in the DT translates this into a physical
high voltage, taking the device out of reset. 

Would it be better to specify this as active-low to match the hardware and
address the bug in the driver instead?

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

[ ... ]

> +	/* CS35L36, Top Speaker */
> +	cs35l36_top: audio-codec@41 {
> +		compatible = "cirrus,cs35l36";
> +		reg = <0x41>;
> +		reset-gpios = <&tlmm 75 GPIO_ACTIVE_HIGH>;

[Severity: Low]
Are we encoding the same driver workaround in the devicetree here for the
top speaker?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260705-pixel3-audio-v1-0-3b66f33859f1@ixit.cz?part=6