Re: [PATCH 3/4] arm64: dts: arduino-imola: add support for sound

[Konrad Dybcio]

On 2/9/26 3:24 PM, Srinivas Kandagatla wrote:
> Add support for sound on Arduino UNO Q board, which includes
> - Headset playback and record.
> - Lineout

Looking at the images, I can't see a speaker there..

And looking at https://docs.arduino.cc/resources/pinouts/ABX00162-full-pinout.pdf
I *thiiink* there's no I2S out (which is a little surprising)

[...]

> +		hph-playback-dai-link {
> +			link-name = "HPH Playback";
> +			cpu {
> +				sound-dai = <&q6afedai RX_CODEC_DMA_RX_0>;
> +			};
> +
> +			platform {
> +				sound-dai = <&q6routing>;
> +			};
> +
> +			codec {
> +				sound-dai = <&pmic4125_codec 0>, <&swr1 0>, <&rxmacro 0>;
> +			};
> +		};
> +
> +		lo-playback-dai-link {
> +			link-name = "LO Playback";

"Line Out", "Headphones", etc.? I know if it's user-visible..

> +			cpu {
> +				sound-dai = <&q6afedai RX_CODEC_DMA_RX_0>;
> +			};
> +
> +			platform {
> +				sound-dai = <&q6routing>;
> +			};
> +
> +			codec {
> +				sound-dai = <&pmic4125_codec 0>, <&swr1 0>, <&rxmacro 0>;
> +			};
> +		};
> +
> +		ear-playback-dai-link {
> +			link-name = "Ear Playback";
> +			cpu {

Let's uniformly keep a newline between the last property and the following
subnodes, and let's sort all these nodes alphabetically (i.e. both under
/sound and codec/cpu/platform within them)

[...]

> +&spmi_bus {
> +	pmic@0 {
> +		pmic4125_codec: audio-codec@f000{

This definition should definitely live in pm4125.dtsi!

missing space before '{'

> +			compatible = "qcom,pm4125-codec";
> +			reg =<0xf000>;

missing space after '='

> +			vdd-io-supply = <&pm4125_l15>;
> +			vdd-cp-supply = <&pm4125_s4>;
> +			vdd-pa-vpos-supply = <&pm4125_s4>;
> +
> +			vdd-mic-bias-supply = <&pm4125_l22>;
> +			qcom,micbias1-microvolt = <1800000>;
> +			qcom,micbias2-microvolt = <1800000>;
> +			qcom,micbias3-microvolt = <1800000>;
> +
> +			qcom,rx-device = <&pm4125_rx>;
> +			qcom,tx-device = <&pm4125_tx>;
> +			#sound-dai-cells = <1>;
> +		};
> +	};
> +};
> +
> +&swr0 {
> +	pinctrl-0 = <&lpass_tx_swr_active>;
> +	pinctrl-names = "default";
> +	status = "okay";

Please add a \n before status

otherwise this looks ok

Konrad