[PATCH v2 0/4] qrb2210-rb1: HDMI/I2S audio playback support

[PATCH v2 0/4] qrb2210-rb1: HDMI/I2S audio playback support

[Alexey Klimov]

This series adds a feature to playback/output audio via HDMI
on the Qualcomm RB1 board. Since RB1 and RB2 are very similar
to each and other and most likely use the same mainboard therefore
this series is pretty much a rework of the similar patchset for RB2.

Changes since v1:
- added tags;
- patch that adds sndcard compatble is moved into separate series
and also one new patch is added there;
here: https://lore.kernel.org/linux-sound/20251007-qrb2210-qcm2290-sndcard-v1-0-8222141bca79@linaro.org/
- resorted cpu,codec,platform subnodes of HDMI DAI link
(as asked by Konrad);
- apt => APR in "arm64: dts: qcom: qcm2290: add apr and its services"
(as suggested by Dmitry);
- resorted pins in LPASS TLMM node (as asked by Konrad);

Previous version:
https://lore.kernel.org/linux-sound/20250302-rb1_hdmi_sound_first-v1-0-81a87ae1503c@linaro.org/

Signed-off-by: Alexey Klimov <alexey.klimov@linaro.org>
---
Alexey Klimov (4):
      dt-bindings: pinctrl: qcom,sm6115-lpass-lpi: add QCM2290 compatible
      arm64: dts: qcom: qcm2290: add APR and its services
      arm64: dts: qcom: qcm2290: add LPASS LPI pin controller
      arm64: dts: qcom: qrb2210-rb1: add HDMI/I2S audio playback support

 .../pinctrl/qcom,sm6115-lpass-lpi-pinctrl.yaml     |   8 +-
 arch/arm64/boot/dts/qcom/qcm2290.dtsi              | 113 +++++++++++++++++++++
 arch/arm64/boot/dts/qcom/qrb2210-rb1.dts           |  55 ++++++++++
 3 files changed, 175 insertions(+), 1 deletion(-)
---
base-commit: 4a71531471926e3c391665ee9c42f4e0295a4585
change-id: 20251007-rb1_hdmi_audio-1ab3cd93fc9e
prerequisite-message-id: <20251007-qrb2210-qcm2290-sndcard-v1-0-8222141bca79@linaro.org>
prerequisite-patch-id: 0644af1b677a6f0aa2cf85736ccdfc2495a4c9d4
prerequisite-patch-id: 986319766f2f9e41cc702af7f6ebbb764f5117da

Best regards,
-- 
Alexey Klimov <alexey.klimov@linaro.org>

[PATCH v2 1/4] dt-bindings: pinctrl: qcom,sm6115-lpass-lpi: add QCM2290 compatible

[Alexey Klimov]

Add a compatible for the LPASS LPI pin controller on QCM2290. It seems
to be compatible with sm6115 LPASS pinctrl.

Cc: Konrad Dybcio <konradybcio@kernel.org>
Cc: Srinivas Kandagatla <srini@kernel.org>
Acked-by: Rob Herring (Arm) <robh@kernel.org>
Signed-off-by: Alexey Klimov <alexey.klimov@linaro.org>
---
 .../bindings/pinctrl/qcom,sm6115-lpass-lpi-pinctrl.yaml           | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/pinctrl/qcom,sm6115-lpass-lpi-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/qcom,sm6115-lpass-lpi-pinctrl.yaml
index f4cf2ce86fcd425b7522918442de25c836bf24f0..ec1cd684753ad22050ae349a2e7acbb2545a9070 100644
--- a/Documentation/devicetree/bindings/pinctrl/qcom,sm6115-lpass-lpi-pinctrl.yaml
+++ b/Documentation/devicetree/bindings/pinctrl/qcom,sm6115-lpass-lpi-pinctrl.yaml
@@ -16,7 +16,13 @@ description:
 
 properties:
   compatible:
-    const: qcom,sm6115-lpass-lpi-pinctrl
+    oneOf:
+      - enum:
+          - qcom,sm6115-lpass-lpi-pinctrl
+      - items:
+          - enum:
+              - qcom,qcm2290-lpass-lpi-pinctrl
+          - const: qcom,sm6115-lpass-lpi-pinctrl
 
   reg:
     items:

-- 
2.47.3

[PATCH v2 2/4] arm64: dts: qcom: qcm2290: add APR and its services

[Alexey Klimov]

Add APR (asynchronous packet router) node and its associated services
required to enable audio on QRB2210 RB1 board.

Cc: Srinivas Kandagatla <srini@kernel.org>
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Signed-off-by: Alexey Klimov <alexey.klimov@linaro.org>
---
 arch/arm64/boot/dts/qcom/qcm2290.dtsi | 72 +++++++++++++++++++++++++++++++++++
 1 file changed, 72 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/qcm2290.dtsi b/arch/arm64/boot/dts/qcom/qcm2290.dtsi
index 08141b41de2462ce91896fd84644413fa46ac047..7303aff33814f256a2ea09a71a640db466370eff 100644
--- a/arch/arm64/boot/dts/qcom/qcm2290.dtsi
+++ b/arch/arm64/boot/dts/qcom/qcm2290.dtsi
@@ -17,6 +17,8 @@
 #include <dt-bindings/interconnect/qcom,qcm2290.h>
 #include <dt-bindings/interconnect/qcom,rpm-icc.h>
 #include <dt-bindings/power/qcom-rpmpd.h>
+#include <dt-bindings/soc/qcom,apr.h>
+#include <dt-bindings/sound/qcom,q6asm.h>
 
 / {
 	interrupt-parent = <&intc>;
@@ -2077,6 +2079,76 @@ glink-edge {
 				label = "lpass";
 				qcom,remote-pid = <2>;
 				mboxes = <&apcs_glb 8>;
+
+				apr {
+					compatible = "qcom,apr-v2";
+					qcom,glink-channels = "apr_audio_svc";
+					qcom,domain = <APR_DOMAIN_ADSP>;
+					#address-cells = <1>;
+					#size-cells = <0>;
+
+					service@3 {
+						reg = <APR_SVC_ADSP_CORE>;
+						compatible = "qcom,q6core";
+						qcom,protection-domain = "avs/audio",
+									 "msm/adsp/audio_pd";
+					};
+
+					q6afe: service@4 {
+						compatible = "qcom,q6afe";
+						reg = <APR_SVC_AFE>;
+						qcom,protection-domain = "avs/audio",
+									 "msm/adsp/audio_pd";
+						q6afedai: dais {
+							compatible = "qcom,q6afe-dais";
+							#address-cells = <1>;
+							#size-cells = <0>;
+							#sound-dai-cells = <1>;
+						};
+
+						q6afecc: clock-controller {
+							compatible = "qcom,q6afe-clocks";
+							#clock-cells = <2>;
+						};
+					};
+
+					q6asm: service@7 {
+						compatible = "qcom,q6asm";
+						reg = <APR_SVC_ASM>;
+						qcom,protection-domain = "avs/audio",
+									 "msm/adsp/audio_pd";
+						q6asmdai: dais {
+							compatible = "qcom,q6asm-dais";
+							#address-cells = <1>;
+							#size-cells = <0>;
+							#sound-dai-cells = <1>;
+							iommus = <&apps_smmu 0x1c1 0x0>;
+
+							dai@0 {
+								reg = <MSM_FRONTEND_DAI_MULTIMEDIA1>;
+							};
+
+							dai@1 {
+								reg = <MSM_FRONTEND_DAI_MULTIMEDIA2>;
+							};
+
+							dai@2 {
+								reg = <MSM_FRONTEND_DAI_MULTIMEDIA3>;
+							};
+						};
+					};
+
+					q6adm: service@8 {
+						compatible = "qcom,q6adm";
+						reg = <APR_SVC_ADM>;
+						qcom,protection-domain = "avs/audio",
+									 "msm/adsp/audio_pd";
+						q6routing: routing {
+							compatible = "qcom,q6adm-routing";
+							#sound-dai-cells = <0>;
+						};
+					};
+				};
 			};
 		};
 

-- 
2.47.3

[PATCH v2 3/4] arm64: dts: qcom: qcm2290: add LPASS LPI pin controller

[Alexey Klimov]

Add the Low Power Audio SubSystem Low Power Island (LPASS LPI) pin
controller device node required for audio subsystem on Qualcomm
QRB2210 RB1. QRB2210 is based on qcm2290 which is based on sm6115.

While at this, also add description of lpi_i2s2 pins (active state)
required for audio playback via HDMI/I2S.

Cc: Srinivas Kandagatla <srini@kernel.org>
Signed-off-by: Alexey Klimov <alexey.klimov@linaro.org>
---
 arch/arm64/boot/dts/qcom/qcm2290.dtsi | 41 +++++++++++++++++++++++++++++++++++
 1 file changed, 41 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/qcm2290.dtsi b/arch/arm64/boot/dts/qcom/qcm2290.dtsi
index 7303aff33814f256a2ea09a71a640db466370eff..02eee364e618358e5ead78c2b97fde12a57ede0a 100644
--- a/arch/arm64/boot/dts/qcom/qcm2290.dtsi
+++ b/arch/arm64/boot/dts/qcom/qcm2290.dtsi
@@ -19,6 +19,7 @@
 #include <dt-bindings/power/qcom-rpmpd.h>
 #include <dt-bindings/soc/qcom,apr.h>
 #include <dt-bindings/sound/qcom,q6asm.h>
+#include <dt-bindings/sound/qcom,q6dsp-lpass-ports.h>
 
 / {
 	interrupt-parent = <&intc>;
@@ -673,6 +674,46 @@ data-pins {
 			};
 		};
 
+		lpass_tlmm: pinctrl@a7c0000 {
+			compatible = "qcom,qcm2290-lpass-lpi-pinctrl",
+				     "qcom,sm6115-lpass-lpi-pinctrl";
+			reg = <0x0 0x0a7c0000 0x0 0x20000>,
+			      <0x0 0x0a950000 0x0 0x10000>;
+
+			clocks = <&q6afecc LPASS_HW_DCODEC_VOTE LPASS_CLK_ATTRIBUTE_COUPLE_NO>;
+			clock-names = "audio";
+
+			gpio-controller;
+			#gpio-cells = <2>;
+			gpio-ranges = <&lpass_tlmm 0 0 19>;
+
+			lpi_i2s2_active: lpi-i2s2-active-state {
+				data-pins {
+					pins = "gpio12";
+					function = "i2s2_data";
+					bias-disable;
+					drive-strength = <8>;
+					output-high;
+				};
+
+				sck-pins {
+					pins = "gpio10";
+					function = "i2s2_clk";
+					bias-disable;
+					drive-strength = <8>;
+					output-high;
+				};
+
+				ws-pins {
+					pins = "gpio11";
+					function = "i2s2_ws";
+					bias-disable;
+					drive-strength = <8>;
+					output-high;
+				};
+			};
+		};
+
 		gcc: clock-controller@1400000 {
 			compatible = "qcom,gcc-qcm2290";
 			reg = <0x0 0x01400000 0x0 0x1f0000>;

-- 
2.47.3

[PATCH v2 4/4] arm64: dts: qcom: qrb2210-rb1: add HDMI/I2S audio playback support

[Alexey Klimov]

Add sound node and aDSP-related pieces to enable HDMI+I2S audio playback
support on Qualcomm QR2210 RB1 board. That is the only sound output
supported for now.

The audio playback is verified using the following commands:
amixer -c0 cset iface=MIXER,name='SEC_MI2S_RX Audio Mixer MultiMedia1' 1
aplay -D hw:0,0 /usr/share/sounds/alsa/Front_Center.wav

Cc: Srinivas Kandagatla <srini@kernel.org>
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Signed-off-by: Alexey Klimov <alexey.klimov@linaro.org>
---
 arch/arm64/boot/dts/qcom/qrb2210-rb1.dts | 55 ++++++++++++++++++++++++++++++++
 1 file changed, 55 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/qrb2210-rb1.dts b/arch/arm64/boot/dts/qcom/qrb2210-rb1.dts
index 67ba508e92ba1f0ef3bcf8b248eae125de059869..4ab5d3379ed22b43675284af40eb7f9304fbe1ef 100644
--- a/arch/arm64/boot/dts/qcom/qrb2210-rb1.dts
+++ b/arch/arm64/boot/dts/qcom/qrb2210-rb1.dts
@@ -188,6 +188,53 @@ vph_pwr: regulator-vph-pwr {
 		regulator-always-on;
 		regulator-boot-on;
 	};
+
+	sound {
+		compatible = "qcom,qrb2210-rb1-sndcard", "qcom,qrb2210-sndcard";
+		pinctrl-0 = <&lpi_i2s2_active>;
+		pinctrl-names = "default";
+		model = "Qualcomm-RB1-WSA8815-Speakers-DMIC0";
+
+		mm1-dai-link {
+			link-name = "MultiMedia1";
+
+			cpu {
+				sound-dai = <&q6asmdai MSM_FRONTEND_DAI_MULTIMEDIA1>;
+			};
+		};
+
+		mm2-dai-link {
+			link-name = "MultiMedia2";
+
+			cpu {
+				sound-dai = <&q6asmdai MSM_FRONTEND_DAI_MULTIMEDIA2>;
+			};
+		};
+
+		mm3-dai-link {
+			link-name = "MultiMedia3";
+
+			cpu {
+				sound-dai = <&q6asmdai MSM_FRONTEND_DAI_MULTIMEDIA3>;
+			};
+		};
+
+		hdmi-i2s-dai-link {
+			link-name = "HDMI/I2S Playback";
+
+			codec {
+				sound-dai = <&lt9611_codec 0>;
+			};
+
+			cpu {
+				sound-dai = <&q6afedai SECONDARY_MI2S_RX>;
+			};
+
+			platform {
+				sound-dai = <&q6routing>;
+			};
+		};
+	};
 };
 
 &cpu_pd0 {
@@ -323,6 +370,14 @@ &pm4125_vbus {
 	status = "okay";
 };
 
+/* SECONDARY I2S uses 1 I2S SD Line for audio on LT9611UXC HDMI Bridge */
+&q6afedai {
+	dai@18 {
+		reg = <SECONDARY_MI2S_RX>;
+		qcom,sd-lines = <0>;
+	};
+};
+
 &qupv3_id_0 {
 	status = "okay";
 };

-- 
2.47.3

Re: [PATCH v2 3/4] arm64: dts: qcom: qcm2290: add LPASS LPI pin controller

[Konrad Dybcio]

On 10/7/25 4:03 AM, Alexey Klimov wrote:
> Add the Low Power Audio SubSystem Low Power Island (LPASS LPI) pin
> controller device node required for audio subsystem on Qualcomm
> QRB2210 RB1. QRB2210 is based on qcm2290 which is based on sm6115.
> 
> While at this, also add description of lpi_i2s2 pins (active state)
> required for audio playback via HDMI/I2S.
> 
> Cc: Srinivas Kandagatla <srini@kernel.org>
> Signed-off-by: Alexey Klimov <alexey.klimov@linaro.org>
> ---

[...]

> +			lpi_i2s2_active: lpi-i2s2-active-state {
> +				data-pins {
> +					pins = "gpio12";
> +					function = "i2s2_data";
> +					bias-disable;
> +					drive-strength = <8>;
> +					output-high;

I.. doubt output-high is what you want?

> +				};
> +
> +				sck-pins {
> +					pins = "gpio10";
> +					function = "i2s2_clk";
> +					bias-disable;
> +					drive-strength = <8>;
> +					output-high;
> +				};
> +
> +				ws-pins {
> +					pins = "gpio11";

Please re-sort pin entries by the gpio index

https://docs.kernel.org/devicetree/bindings/dts-coding-style.html#order-of-nodes

Konrad

Re: [PATCH v2 3/4] arm64: dts: qcom: qcm2290: add LPASS LPI pin controller

[Alexey Klimov]

On Tue Oct 7, 2025 at 1:39 PM BST, Konrad Dybcio wrote:
> On 10/7/25 4:03 AM, Alexey Klimov wrote:
>> Add the Low Power Audio SubSystem Low Power Island (LPASS LPI) pin
>> controller device node required for audio subsystem on Qualcomm
>> QRB2210 RB1. QRB2210 is based on qcm2290 which is based on sm6115.
>> 
>> While at this, also add description of lpi_i2s2 pins (active state)
>> required for audio playback via HDMI/I2S.
>> 
>> Cc: Srinivas Kandagatla <srini@kernel.org>
>> Signed-off-by: Alexey Klimov <alexey.klimov@linaro.org>
>> ---
>
> [...]
>
>> +			lpi_i2s2_active: lpi-i2s2-active-state {
>> +				data-pins {
>> +					pins = "gpio12";
>> +					function = "i2s2_data";
>> +					bias-disable;
>> +					drive-strength = <8>;
>> +					output-high;
>
> I.. doubt output-high is what you want?

Why? Or is it because of some in-kernel gpiod?

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/qcom/sm4250.dtsi#n66

>> +				};
>> +
>> +				sck-pins {
>> +					pins = "gpio10";
>> +					function = "i2s2_clk";
>> +					bias-disable;
>> +					drive-strength = <8>;
>> +					output-high;
>> +				};
>> +
>> +				ws-pins {
>> +					pins = "gpio11";
>
> Please re-sort pin entries by the gpio index
>
> https://docs.kernel.org/devicetree/bindings/dts-coding-style.html#order-of-nodes

Ok.

Thanks,
Alexey.

Re: [PATCH v2 1/4] dt-bindings: pinctrl: qcom,sm6115-lpass-lpi: add QCM2290 compatible

[Linus Walleij]

On Tue, Oct 7, 2025 at 4:03 AM Alexey Klimov <alexey.klimov@linaro.org> wrote:

> Add a compatible for the LPASS LPI pin controller on QCM2290. It seems
> to be compatible with sm6115 LPASS pinctrl.
>
> Cc: Konrad Dybcio <konradybcio@kernel.org>
> Cc: Srinivas Kandagatla <srini@kernel.org>
> Acked-by: Rob Herring (Arm) <robh@kernel.org>
> Signed-off-by: Alexey Klimov <alexey.klimov@linaro.org>

This patch 1/4 applied to the pinctrl tree,
please funnel the rest through the SoC tree.

Yours,
Linus Walleij

Re: [PATCH v2 3/4] arm64: dts: qcom: qcm2290: add LPASS LPI pin controller

[Bjorn Andersson]

On Fri, Oct 10, 2025 at 01:29:38PM +0100, Alexey Klimov wrote:
> On Tue Oct 7, 2025 at 1:39 PM BST, Konrad Dybcio wrote:
> > On 10/7/25 4:03 AM, Alexey Klimov wrote:
> >> Add the Low Power Audio SubSystem Low Power Island (LPASS LPI) pin
> >> controller device node required for audio subsystem on Qualcomm
> >> QRB2210 RB1. QRB2210 is based on qcm2290 which is based on sm6115.
> >> 
> >> While at this, also add description of lpi_i2s2 pins (active state)
> >> required for audio playback via HDMI/I2S.
> >> 
> >> Cc: Srinivas Kandagatla <srini@kernel.org>
> >> Signed-off-by: Alexey Klimov <alexey.klimov@linaro.org>
> >> ---
> >
> > [...]
> >
> >> +			lpi_i2s2_active: lpi-i2s2-active-state {
> >> +				data-pins {
> >> +					pins = "gpio12";
> >> +					function = "i2s2_data";
> >> +					bias-disable;
> >> +					drive-strength = <8>;
> >> +					output-high;
> >
> > I.. doubt output-high is what you want?
> 
> Why? Or is it because of some in-kernel gpiod?
> 

What does "output-high" mean for a non-gpio function?

Regards,
Bjorn

> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/qcom/sm4250.dtsi#n66
> 
> >> +				};
> >> +
> >> +				sck-pins {
> >> +					pins = "gpio10";
> >> +					function = "i2s2_clk";
> >> +					bias-disable;
> >> +					drive-strength = <8>;
> >> +					output-high;
> >> +				};
> >> +
> >> +				ws-pins {
> >> +					pins = "gpio11";
> >
> > Please re-sort pin entries by the gpio index
> >
> > https://docs.kernel.org/devicetree/bindings/dts-coding-style.html#order-of-nodes
> 
> Ok.
> 
> Thanks,
> Alexey.
> 

Re: [PATCH v2 3/4] arm64: dts: qcom: qcm2290: add LPASS LPI pin controller

[Alexey Klimov]

On Fri Oct 17, 2025 at 11:42 PM BST, Bjorn Andersson wrote:
> On Fri, Oct 10, 2025 at 01:29:38PM +0100, Alexey Klimov wrote:
>> On Tue Oct 7, 2025 at 1:39 PM BST, Konrad Dybcio wrote:
>> > On 10/7/25 4:03 AM, Alexey Klimov wrote:
>> >> Add the Low Power Audio SubSystem Low Power Island (LPASS LPI) pin
>> >> controller device node required for audio subsystem on Qualcomm
>> >> QRB2210 RB1. QRB2210 is based on qcm2290 which is based on sm6115.
>> >> 
>> >> While at this, also add description of lpi_i2s2 pins (active state)
>> >> required for audio playback via HDMI/I2S.
>> >> 
>> >> Cc: Srinivas Kandagatla <srini@kernel.org>
>> >> Signed-off-by: Alexey Klimov <alexey.klimov@linaro.org>
>> >> ---
>> >
>> > [...]
>> >
>> >> +			lpi_i2s2_active: lpi-i2s2-active-state {
>> >> +				data-pins {
>> >> +					pins = "gpio12";
>> >> +					function = "i2s2_data";
>> >> +					bias-disable;
>> >> +					drive-strength = <8>;
>> >> +					output-high;
>> >
>> > I.. doubt output-high is what you want?
>> 
>> Why? Or is it because of some in-kernel gpiod?
>> 
>
> What does "output-high" mean for a non-gpio function?

This is not efficient. It will be more useful to go straight to
the point.

This description of pins was taken from Qualcomm downstream code
and the similar patch was applied (see provided URL in the prev email).

The same problem here and no response:
https://lore.kernel.org/linux-arm-msm/d8e202fb-f494-4b5e-bfb9-c94c1fd3af4d@linaro.org/

After looking at
https://lore.kernel.org/linux-arm-msm/39ebaf4e-e91f-4568-8de6-9fc1f805a1e4@oss.qualcomm.com/
I guess the output-high should removed at all (if it is the case) but it
will be more efficient to provide that as a feedback rather than vague
responses.

Back to your question -- does it matter here if it is gpio or non-gpio
function?

Best regards,
Alexey.

Re: [PATCH v2 3/4] arm64: dts: qcom: qcm2290: add LPASS LPI pin controller

[Dmitry Baryshkov]

On Tue, Oct 21, 2025 at 01:56:09PM +0100, Alexey Klimov wrote:
> On Fri Oct 17, 2025 at 11:42 PM BST, Bjorn Andersson wrote:
> > On Fri, Oct 10, 2025 at 01:29:38PM +0100, Alexey Klimov wrote:
> >> On Tue Oct 7, 2025 at 1:39 PM BST, Konrad Dybcio wrote:
> >> > On 10/7/25 4:03 AM, Alexey Klimov wrote:
> >> >> Add the Low Power Audio SubSystem Low Power Island (LPASS LPI) pin
> >> >> controller device node required for audio subsystem on Qualcomm
> >> >> QRB2210 RB1. QRB2210 is based on qcm2290 which is based on sm6115.
> >> >> 
> >> >> While at this, also add description of lpi_i2s2 pins (active state)
> >> >> required for audio playback via HDMI/I2S.
> >> >> 
> >> >> Cc: Srinivas Kandagatla <srini@kernel.org>
> >> >> Signed-off-by: Alexey Klimov <alexey.klimov@linaro.org>
> >> >> ---
> >> >
> >> > [...]
> >> >
> >> >> +			lpi_i2s2_active: lpi-i2s2-active-state {
> >> >> +				data-pins {
> >> >> +					pins = "gpio12";
> >> >> +					function = "i2s2_data";
> >> >> +					bias-disable;
> >> >> +					drive-strength = <8>;
> >> >> +					output-high;
> >> >
> >> > I.. doubt output-high is what you want?
> >> 
> >> Why? Or is it because of some in-kernel gpiod?
> >> 
> >
> > What does "output-high" mean for a non-gpio function?
> 
> This is not efficient. It will be more useful to go straight to
> the point.

It is efficient. It makes everybody think about it (and ask the same
question in future) instead of just depending on maintainers words.

> This description of pins was taken from Qualcomm downstream code
> and the similar patch was applied (see provided URL in the prev email).

And we all know that downstream can be buggy, incomplete, etc.

> Back to your question -- does it matter here if it is gpio or non-gpio
> function?

It does. The I2S data pin is supposed to be toggled in some way by a
certain IP core. What would it mean if we program output-high? Will the
pin still be toggled (by the function) or stay pulled up (because of the
output being programmed)?

-- 
With best wishes
Dmitry

Re: [PATCH v2 3/4] arm64: dts: qcom: qcm2290: add LPASS LPI pin controller

[Srinivas Kandagatla]

On 10/21/25 2:03 PM, Dmitry Baryshkov wrote:
> On Tue, Oct 21, 2025 at 01:56:09PM +0100, Alexey Klimov wrote:
>> On Fri Oct 17, 2025 at 11:42 PM BST, Bjorn Andersson wrote:
>>> On Fri, Oct 10, 2025 at 01:29:38PM +0100, Alexey Klimov wrote:
>>>> On Tue Oct 7, 2025 at 1:39 PM BST, Konrad Dybcio wrote:
>>>>> On 10/7/25 4:03 AM, Alexey Klimov wrote:
>>>>>> Add the Low Power Audio SubSystem Low Power Island (LPASS LPI) pin
>>>>>> controller device node required for audio subsystem on Qualcomm
>>>>>> QRB2210 RB1. QRB2210 is based on qcm2290 which is based on sm6115.
>>>>>>
>>>>>> While at this, also add description of lpi_i2s2 pins (active state)
>>>>>> required for audio playback via HDMI/I2S.
>>>>>>
>>>>>> Cc: Srinivas Kandagatla <srini@kernel.org>
>>>>>> Signed-off-by: Alexey Klimov <alexey.klimov@linaro.org>
>>>>>> ---
>>>>>
>>>>> [...]
>>>>>
>>>>>> +			lpi_i2s2_active: lpi-i2s2-active-state {
>>>>>> +				data-pins {
>>>>>> +					pins = "gpio12";
>>>>>> +					function = "i2s2_data";
>>>>>> +					bias-disable;
>>>>>> +					drive-strength = <8>;
>>>>>> +					output-high;
>>>>>
>>>>> I.. doubt output-high is what you want?
>>>>
>>>> Why? Or is it because of some in-kernel gpiod?
>>>>
>>>
>>> What does "output-high" mean for a non-gpio function?
>>
>> This is not efficient. It will be more useful to go straight to
>> the point.
> 
> It is efficient. It makes everybody think about it (and ask the same
> question in future) instead of just depending on maintainers words.
> 
>> This description of pins was taken from Qualcomm downstream code
>> and the similar patch was applied (see provided URL in the prev email).
> 
> And we all know that downstream can be buggy, incomplete, etc.
> 
>> Back to your question -- does it matter here if it is gpio or non-gpio
>> function?
> 
> It does. The I2S data pin is supposed to be toggled in some way by a
> certain IP core. What would it mean if we program output-high? Will the
> pin still be toggled (by the function) or stay pulled up (because of the
> output being programmed)?
I2S lines are configured in push-pull mode which means that the lines
are driven high and low actively, am not sure why output-high is needed
an what it means here as these lines are actively driven by the controller.

@Alexey, what issues do you see without this?

Am not sure if pinctrl driver even cares about this if we are in alt mode.


--srini>

Re: [PATCH v2 3/4] arm64: dts: qcom: qcm2290: add LPASS LPI pin controller

[Alexey Klimov]

On Tue Oct 21, 2025 at 2:03 PM BST, Dmitry Baryshkov wrote:
> On Tue, Oct 21, 2025 at 01:56:09PM +0100, Alexey Klimov wrote:
>> On Fri Oct 17, 2025 at 11:42 PM BST, Bjorn Andersson wrote:
>> > On Fri, Oct 10, 2025 at 01:29:38PM +0100, Alexey Klimov wrote:
>> >> On Tue Oct 7, 2025 at 1:39 PM BST, Konrad Dybcio wrote:
>> >> > On 10/7/25 4:03 AM, Alexey Klimov wrote:
>> >> >> Add the Low Power Audio SubSystem Low Power Island (LPASS LPI) pin
>> >> >> controller device node required for audio subsystem on Qualcomm
>> >> >> QRB2210 RB1. QRB2210 is based on qcm2290 which is based on sm6115.
>> >> >> 
>> >> >> While at this, also add description of lpi_i2s2 pins (active state)
>> >> >> required for audio playback via HDMI/I2S.
>> >> >> 
>> >> >> Cc: Srinivas Kandagatla <srini@kernel.org>
>> >> >> Signed-off-by: Alexey Klimov <alexey.klimov@linaro.org>
>> >> >> ---
>> >> >
>> >> > [...]
>> >> >
>> >> >> +			lpi_i2s2_active: lpi-i2s2-active-state {
>> >> >> +				data-pins {
>> >> >> +					pins = "gpio12";
>> >> >> +					function = "i2s2_data";
>> >> >> +					bias-disable;
>> >> >> +					drive-strength = <8>;
>> >> >> +					output-high;
>> >> >
>> >> > I.. doubt output-high is what you want?
>> >> 
>> >> Why? Or is it because of some in-kernel gpiod?
>> >> 
>> >
>> > What does "output-high" mean for a non-gpio function?
>> 
>> This is not efficient. It will be more useful to go straight to
>> the point.
>
> It is efficient. It makes everybody think about it (and ask the same
> question in future) instead of just depending on maintainers words.

No. Imagine instead of this email you've got smth like this:
"I.. doubt efficient is what you want?"

>> This description of pins was taken from Qualcomm downstream code
>> and the similar patch was applied (see provided URL in the prev email).
>
> And we all know that downstream can be buggy, incomplete, etc.
>
>> Back to your question -- does it matter here if it is gpio or non-gpio
>> function?
>
> It does. The I2S data pin is supposed to be toggled in some way by a
> certain IP core. What would it mean if we program output-high? Will the
> pin still be toggled (by the function) or stay pulled up (because of the
> output being programmed)?

Here the topic was switched/replaced. And the other referenced email
suggests that they don't want to see output property regardless.

Anyway, as it was indicated in my previous email the problem they
were trying to refer to was kinda understood.

Thanks,
Alexey

Re: [PATCH v2 3/4] arm64: dts: qcom: qcm2290: add LPASS LPI pin controller

[Alexey Klimov]

On Tue Oct 21, 2025 at 2:13 PM BST, Srinivas Kandagatla wrote:
>
>
> On 10/21/25 2:03 PM, Dmitry Baryshkov wrote:
>> On Tue, Oct 21, 2025 at 01:56:09PM +0100, Alexey Klimov wrote:
>>> On Fri Oct 17, 2025 at 11:42 PM BST, Bjorn Andersson wrote:
>>>> On Fri, Oct 10, 2025 at 01:29:38PM +0100, Alexey Klimov wrote:
>>>>> On Tue Oct 7, 2025 at 1:39 PM BST, Konrad Dybcio wrote:
>>>>>> On 10/7/25 4:03 AM, Alexey Klimov wrote:
>>>>>>> Add the Low Power Audio SubSystem Low Power Island (LPASS LPI) pin
>>>>>>> controller device node required for audio subsystem on Qualcomm
>>>>>>> QRB2210 RB1. QRB2210 is based on qcm2290 which is based on sm6115.
>>>>>>>
>>>>>>> While at this, also add description of lpi_i2s2 pins (active state)
>>>>>>> required for audio playback via HDMI/I2S.
>>>>>>>
>>>>>>> Cc: Srinivas Kandagatla <srini@kernel.org>
>>>>>>> Signed-off-by: Alexey Klimov <alexey.klimov@linaro.org>
>>>>>>> ---
>>>>>>
>>>>>> [...]
>>>>>>
>>>>>>> +			lpi_i2s2_active: lpi-i2s2-active-state {
>>>>>>> +				data-pins {
>>>>>>> +					pins = "gpio12";
>>>>>>> +					function = "i2s2_data";
>>>>>>> +					bias-disable;
>>>>>>> +					drive-strength = <8>;
>>>>>>> +					output-high;
>>>>>>
>>>>>> I.. doubt output-high is what you want?
>>>>>
>>>>> Why? Or is it because of some in-kernel gpiod?
>>>>>
>>>>
>>>> What does "output-high" mean for a non-gpio function?
>>>
>>> This is not efficient. It will be more useful to go straight to
>>> the point.
>> 
>> It is efficient. It makes everybody think about it (and ask the same
>> question in future) instead of just depending on maintainers words.
>> 
>>> This description of pins was taken from Qualcomm downstream code
>>> and the similar patch was applied (see provided URL in the prev email).
>> 
>> And we all know that downstream can be buggy, incomplete, etc.
>> 
>>> Back to your question -- does it matter here if it is gpio or non-gpio
>>> function?
>> 
>> It does. The I2S data pin is supposed to be toggled in some way by a
>> certain IP core. What would it mean if we program output-high? Will the
>> pin still be toggled (by the function) or stay pulled up (because of the
>> output being programmed)?
> I2S lines are configured in push-pull mode which means that the lines
> are driven high and low actively, am not sure why output-high is needed
> an what it means here as these lines are actively driven by the controller.
>
> @Alexey, what issues do you see without this?
>
> Am not sure if pinctrl driver even cares about this if we are in alt mode.

No issues. Not sure why the problem (if it exists) wasn't reported or
fixed, say, for sm4250.

Thanks,
Alexey