Re: [PATCH 6/6] arm64: dts: qcom: qcs6490-rubikpi3: Add audio support

[Hongyang Zhao]

Hi Konrad,

Thank you for your review.

> On 3/5/26 6:47 AM, Hongyang Zhao wrote:
> > Add audio support for the Thundercomm RubikPi3 board:
> > - Enable AudioReach via qcs6490-audioreach.dtsi
> > - Add ES8316 codec on I2C0 with MCLK from LPASS PRM and jack detection
> > - Add fixed 3.3V regulator for ES8316 power supply
> > - Add MI2S playback/capture dai-links for ES8316
> > - Add HDMI audio via LT9611 bridge on quaternary MI2S
> > - Add SPDIF TX/RX on tertiary MI2S exposed at the board 40‑pin header
> > - Add LPASS pin configurations for quaternary MI2S and LPI I2S1
> > 
> > Signed-off-by: Hongyang Zhao <hongyang.zhao@xxxxxxxxxxxxxxx>
> > ---
> 
> [...]
> 
> > +	spdif_tx: spdif-tx {
> > +		compatible = "linux,spdif-dit";
> > +		#sound-dai-cells = <0>;
> > +	};
> > +
> > +	spdif_rx: spdif-rx {
> 
> 'r' < 't', please swap them

I will swap them:
	spdif_rx: spdif-rx { ... };
	spdif_tx: spdif-tx { ... };

> 
> [...]
> 
> > +	mi2s-playback-dai-link {
> > +		link-name = "MI2S-LPAIF-RX-PRIMARY";
> > +		cpu {
> > +			sound-dai = <&q6apmbedai PRIMARY_MI2S_RX>;
> > +		};
> > +		codec {
> > +			sound-dai = <&es8316>;
> > +		};
> 
> 'co'dec < 'cp'u, please reshuffle
> 
> and leave a \n between the property (link-name) and the following subnode
> as well as between the individual subnodes (but not after the last one)

I will change this series to:
	mi2s-playback-dai-link {
		link-name = "MI2S-LPAIF-RX-PRIMARY";

		codec {
			sound-dai = <&es8316>;
		};

		cpu {
			sound-dai = <&q6apmbedai PRIMARY_MI2S_RX>;
		};

		platform {
			sound-dai = <&q6apm>;
		};
	};

> 
> [...]
> 
> > +&lpass_tlmm {
> > +	lpass_qua_mi2s_sclk: qua-mi2s-sclk-state {
> > +		pins = "gpio0";
> > +		function = "qua_mi2s_sclk";
> > +		drive-strength = <8>;
> > +		bias-disable;
> > +		output-high;
> > +	};
> > +
> > +	lpass_qua_mi2s_ws: qua-mi2s-ws-state {
> > +		pins = "gpio1";
> > +		function = "qua_mi2s_ws";
> > +		drive-strength = <8>;
> > +		output-high;
> > +	};
> > +
> > +	lpass_qua_mi2s_data0: qua-mi2s-data0-state {
> > +		pins = "gpio2";
> > +		function = "qua_mi2s_data";
> > +		drive-strength = <8>;
> > +		bias-disable;
> > +	};
> > +
> > +	lpass_qua_mi2s_data1: qua-mi2s-data1-state {
> > +		pins = "gpio3";
> > +		function = "qua_mi2s_data";
> > +		drive-strength = <8>;
> > +		bias-disable;
> > +	};
> 
> Because they have identical properties, you can squash GPIOs 2-4 into
> 'lpass_qua_mi2s_data' with:
> 
> gpios = "gpio2", "gpio3", "gpio4";
> 
> Konrad

I will revise it to:
	lpass_qua_mi2s_data: qua-mi2s-data-state {
		pins = "gpio2", "gpio3", "gpio4";
		function = "qua_mi2s_data";
		drive-strength = <8>;
		bias-disable;
	};

Thanks,
Hongyang