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

[Srinivas Kandagatla <srinivas.kandagatla@oss.qualcomm.com>]

On 2/10/26 10:07 AM, Konrad Dybcio wrote:
> 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)

there isn't, there is only Ear, LIne, HP and MIc.
> 
> [...]
> 
>> +		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..
It will be via UCM.

> 
>> +			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)
sure.

> 
> [...]
> 
>> +&spmi_bus {
>> +	pmic@0 {
>> +		pmic4125_codec: audio-codec@f000{
> 
> This definition should definitely live in pm4125.dtsi!

agreed.

> 
> 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