Re: [PATCH 19/28] arm64: dts: apple: t8103-j293: Add speaker/headset jack nodes

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] dt-bindings: apple: t8103-j293: ALSA probe failure due to shared shutdown GPIO
- [Low] dt-bindings: apple: t8103-j293: Missing required regulator supplies for cs42l83
- [Low] dt-bindings: apple: t8103-j293: Unit address used without reg property
- [Low] dt-bindings: apple: t8103-j293: Redundant duplicate &i2c2 node override
--

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

arm64: dts: apple: t8103-j293: 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 found in the M1 MacBook Pro. It also
advertises them to the machine driver via the sound node.

> diff --git a/arch/arm64/boot/dts/apple/t8103-j293.dts b/arch/arm64/boot/dts/apple/t8103-j293.dts
> index 1d074b9e60183..24d0b512fac4a 100644
> --- a/arch/arm64/boot/dts/apple/t8103-j293.dts
> +++ b/arch/arm64/boot/dts/apple/t8103-j293.dts
[ ... ]
> +&i2c1 {
> +	status = "okay";
> +
> +	speaker_left_rear: codec@31 {
> +		compatible = "ti,tas5770l", "ti,tas2770";
> +		reg = <0x31>;
> +		interrupts-extended = <&pinctrl_ap 182 IRQ_TYPE_LEVEL_LOW>;
> +		shutdown-gpios = <&pinctrl_ap 181 GPIO_ACTIVE_HIGH>;

[Severity: High]
Does sharing the same shutdown-gpios across all four TAS2770 codecs prevent
ALSA from probing?

During I2C enumeration, speaker_left_rear requests the GPIO exclusively and
drives it LOW (shutdown state). The subsequent three codecs fail to request
it with -EBUSY and silently fall back to a NULL GPIO handle.

When ALSA later probes speaker_left_front first, it skips pulling the
hardware out of shutdown due to the NULL handle. It then issues I2C writes
which are NAKed because the line is still held in shutdown by
speaker_left_rear, causing the entire sound card registration to abort.

> +		#sound-dai-cells = <0>;
> +		sound-name-prefix = "Left Rear";
> +		ti,imon-slot-no = <8>;
> +		ti,vmon-slot-no = <10>;
> +	};
[ ... ]
> +&i2c2 {
> +	status = "okay";

[Severity: Low]
Is this &i2c2 override redundant? The base device tree already overrides
&i2c2 to set status = "okay" earlier in the file.

> +
> +	jack_codec: codec@48 {
> +		compatible = "cirrus,cs42l83";
> +		reg = <0x48>;

[Severity: Low]
Are the required regulator supplies missing for the cs42l83 codec?

The cirrus,cs42l42.yaml bindings require VP-supply, VCP-supply,
VD_FILT-supply, VL-supply, and VA-supply, which appear to be entirely
omitted here.

> +		interrupts-extended = <&pinctrl_ap 183 IRQ_TYPE_LEVEL_LOW>;
> +		reset-gpios = <&pinctrl_nub 11 GPIO_ACTIVE_HIGH>;
> +		#sound-dai-cells = <0>;
[ ... ]
> +		dai-link@0 {

[Severity: Low]
Should this node include a reg = <0>; property? Per device tree syntax
rules, any node with a unit address (@0) must have a matching reg property.

> +			dai-tdm-idle-mode-0 = "pulldown";
> +			dai-tdm-idle-mode-2 = "pulldown";
> +
> +			link-name = "Speakers";
[ ... ]
> +		dai-link@1 {

[Severity: Low]
Should this node include a reg = <1>; property?

> +			link-name = "Headphone Jack";
> +
> +			codec {

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260920-macaudio-v1-0-741cc20a74e5@gmail.com?part=19