Re: [PATCH v2 11/21] arm64: dts: apple: t8103-j313: Add speaker/headset jack nodes

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] sound: missing tas5770l driver binding prevents audio probe
- [High] sound: shared shutdown-gpios cause power desynchronization and suspend failure
--

commit f49e30a50ba190f34e911ebbdc8c79e90f63cee5
Author: James Calligeros <jcalligeros99@gmail.com>

arm64: dts: apple: t8103-j313: Add speaker/headset jack nodes

This commit adds device tree nodes for the Apple-specific variants of the TI
TAS2770 and Cirrus CS42L42 audio codecs on the M1 MacBook Air. It connects
them to the machine driver via the sound node.

> diff --git a/arch/arm64/boot/dts/apple/t8103-j313.dts b/arch/arm64/boot/dts/apple/t8103-j313.dts
> index f8b2b1637b9dc..cdda63ca8b349 100644
> --- a/arch/arm64/boot/dts/apple/t8103-j313.dts
> +++ b/arch/arm64/boot/dts/apple/t8103-j313.dts
[ ... ]
> +/* Audio codecs */
> +&i2c1 {
> +	status = "okay";
> +
> +	speaker_left: codec@31 {
> +		compatible = "ti,tas5770l";

[Severity: High]
Will this node fail to probe because the driver lacks a binding for
ti,tas5770l?

Looking at the driver in sound/soc/codecs/tas2770.c, tas2770_of_match[] only
includes the ti,tas2770 string:

sound/soc/codecs/tas2770.c:tas2770_of_match[] = {
	{ .compatible = "ti,tas2770" },
	{ }
};

Since there is no fallback string provided in the device tree for either
speaker_left or speaker_right, will the ALSA machine driver defer
indefinitely waiting for these speaker codecs to bind?

> +		reg = <0x31>;
> +		interrupts-extended = <&pinctrl_ap 182 IRQ_TYPE_LEVEL_LOW>;
> +		shutdown-gpios = <&pinctrl_ap 181 GPIO_ACTIVE_HIGH>;

[Severity: High]
Can this shared shutdown-gpios configuration cause a system suspend failure?

Since both speaker_left and speaker_right map to the exact same physical
shutdown-gpios line, the second codec to probe will fail to acquire it.
Looking at tas2770_parse_dt() in sound/soc/codecs/tas2770.c, the driver
requests the GPIO exclusively:

sound/soc/codecs/tas2770.c:tas2770_parse_dt() {
    ...
	tas2770->sdz_gpio = devm_gpiod_get_optional(dev, "shutdown", GPIOD_OUT_LOW);
	if (IS_ERR(tas2770->sdz_gpio)) {
		if (PTR_ERR(tas2770->sdz_gpio) == -EPROBE_DEFER)
			return -EPROBE_DEFER;

		tas2770->sdz_gpio = NULL;
	}
    ...
}

The second codec will get -EBUSY and silently fall back to sdz_gpio = NULL,
leaving it without control of its power state.

During system suspend, the first codec will pull the physical GPIO low,
cutting power to both chips. When the second codec's suspend callback
subsequently runs, it will attempt an I2C write (TAS2770_PWR_CTRL_SHUTDOWN)
to a chip that is already physically powered off.

Could this I2C timeout or NACK return an error that fails the component
suspend, thereby aborting the overall system sleep transition?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261004-macaudio-v2-0-7b47ed6335a8@gmail.com?part=11