| Message ID | 20251007-rb1_hdmi_audio-v2-3-821b6a705e4c@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | qrb2210-rb1: HDMI/I2S audio playback support | expand |
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
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.
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. >
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.
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)?
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>
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
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
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>;
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(+)