Re: [PATCH v3 10/21] arm64: dts: apple: t8103-j274: Add speaker/headset jack nodes

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] arm64: dts: apple: duplicate regulator node names merge into single node
- [Low] arm64: dts: apple: t8103-j274: duplicate &i2c2 node reference
--

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

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

This commit adds Device Tree nodes for the TI TAS5770L speaker codec on
i2c1, the Cirrus CS42L83 headset jack codec on i2c2, and the top-level
sound node for the M1 Mac mini (J274).

> diff --git a/arch/arm64/boot/dts/apple/t8103-j274.dts b/arch/arm64/boot/dts/apple/t8103-j274.dts
> index 1212852251e07..150d778565f0c 100644
> --- a/arch/arm64/boot/dts/apple/t8103-j274.dts
> +++ b/arch/arm64/boot/dts/apple/t8103-j274.dts
> @@ -75,4 +75,76 @@ &i2c2 {
>  	status = "okay";
>  };
>  
> +/* Audio codecs */
> +&i2c1 {
> +	status = "okay";
> +
> +	speaker: codec@31 {
> +		compatible = "ti,tas5770l";
> +		reg = <0x31>;
> +		interrupts-extended = <&pinctrl_ap 182 IRQ_TYPE_LEVEL_LOW>;
> +		shutdown-gpios = <&pinctrl_ap 181 GPIO_ACTIVE_HIGH>;
> +		#sound-dai-cells = <0>;
> +		ti,imon-slot-no = <0>;
> +		ti,vmon-slot-no = <2>;
> +	};
> +};
> +
> +&i2c2 {
> +	status = "okay";

[Severity: Low]
Does this duplicate the existing &i2c2 block right above? Since
arch/arm64/boot/dts/apple/t8103-j274.dts already has an &i2c2 block
setting status = "okay", could jack_codec be placed inside that existing
block (or the earlier stub removed) to avoid referencing &i2c2 twice in
the same file?

> +
> +	jack_codec: codec@48 {
> +		compatible = "cirrus,cs42l83";
> +		reg = <0x48>;
> +		interrupts-extended = <&pinctrl_ap 183 IRQ_TYPE_LEVEL_LOW>;
> +		reset-gpios = <&pinctrl_nub 11 GPIO_ACTIVE_HIGH>;
> +		#sound-dai-cells = <0>;
> +		sound-name-prefix = "Jack";
> +		cirrus,ts-inv = <1>;
> +		VA-supply = <&jack_va_reg>;
> +		VP-supply = <&jack_vp_reg>;
> +		VCP-supply = <&jack_vcp_reg>;
> +		VD_FILT-supply = <&jack_vd_reg>;
> +		VL-supply = <&jack_vl_reg>;
> +	};
> +};

[Severity: Medium]
Do all five regulator supplies in jack_codec end up resolving to a single
merged /regulator node in the compiled DTB?

In arch/arm64/boot/dts/apple/t8103-jxxx.dtsi (included by t8103-j274.dts
after t8103.dtsi), all five fixed regulators under the root node / use the
identical node name regulator:

	jack_va_reg: regulator {
		compatible = "regulator-fixed";
		regulator-name = "jack-va";
	};

	/* CS42L83 VCP and VCP_FILT */
	jack_vcp_reg: regulator {
		compatible = "regulator-fixed";
		regulator-name = "jack-vcp";
	};

	jack_vl_reg: regulator {
		compatible = "regulator-fixed";
		regulator-name = "jack-vl";
	};

	/* VD_FILT */
	jack_vd_reg: regulator {
		compatible = "regulator-fixed";
		regulator-name = "jack-vd";
	};

	jack_vp_reg: regulator {
		compatible = "regulator-fixed";
		regulator-name = "jack-vp";
	};

When dtc merges the root node / from t8103-jxxx.dtsi into the existing
root node from t8103.dtsi via merge_nodes() in scripts/dtc/livetree.c, the
first regulator child (jack_va_reg) is added to old_node->children, and
each of the remaining four regulator children matches that node name and
merges into it before check_duplicate_node_names() runs.

As a result, four of the five regulators (jack-va, jack-vcp, jack-vl,
jack-vd) are overwritten, leaving a single /regulator node with
regulator-name = "jack-vp" and all five labels pointing to the same
phandle. Should the regulator nodes in t8103-jxxx.dtsi be given distinct
node names such as regulator-jack-va, regulator-jack-vcp,
regulator-jack-vl, regulator-jack-vd, and regulator-jack-vp?

[ ... ]

>
> -- 
> 2.51.2
>

 base-commit: 74612d0b448fe745e80cda87fe49ac3c102ccc1f
-- 
2.51.2

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261018-macaudio-v3-0-abdff7741dea@gmail.com?part=10