| Message ID | 20230102105038.8074-3-johan+linaro@kernel.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | arm64: dts: qcom: disable x13s sound + cleanups | expand |
On 2.01.2023 11:50, Johan Hovold wrote: > The sound nodes in the SoC dtsi should be disabled by default. > > Signed-off-by: Johan Hovold <johan+linaro@kernel.org> > --- Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org> Konrad > arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 16 ++++++++++++++++ > 1 file changed, 16 insertions(+) > > diff --git a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi > index ed1e2bee86ee..c1ce2d7b3675 100644 > --- a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi > +++ b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi > @@ -1733,6 +1733,8 @@ rxmacro: rxmacro@3200000 { > > pinctrl-names = "default"; > pinctrl-0 = <&rx_swr_default>; > + > + status = "disabled"; > }; > > /* RX */ > @@ -1760,6 +1762,8 @@ swr1: soundwire-controller@3210000 { > #sound-dai-cells = <1>; > #address-cells = <2>; > #size-cells = <0>; > + > + status = "disabled"; > }; > > txmacro: txmacro@3220000 { > @@ -1783,6 +1787,8 @@ txmacro: txmacro@3220000 { > #address-cells = <2>; > #size-cells = <2>; > #sound-dai-cells = <1>; > + > + status = "disabled"; > }; > > wsamacro: codec@3240000 { > @@ -1804,6 +1810,8 @@ wsamacro: codec@3240000 { > > pinctrl-names = "default"; > pinctrl-0 = <&wsa_swr_default>; > + > + status = "disabled"; > }; > > /* WSA */ > @@ -1830,6 +1838,8 @@ swr0: soundwire-controller@3250000 { > #sound-dai-cells = <1>; > #address-cells = <2>; > #size-cells = <0>; > + > + status = "disabled"; > }; > > /* TX */ > @@ -1858,6 +1868,8 @@ swr2: soundwire-controller@3330000 { > qcom,ports-word-length = /bits/ 8 <0xff 0x00 0xff 0xff>; > qcom,ports-block-group-count = /bits/ 8 <0xff 0xff 0xff 0xff>; > qcom,ports-lane-control = /bits/ 8 <0x00 0x01 0x00 0x00>; > + > + status = "disabled"; > }; > > vamacro: codec@3370000 { > @@ -1874,6 +1886,8 @@ vamacro: codec@3370000 { > #clock-cells = <0>; > clock-output-names = "fsgen"; > #sound-dai-cells = <1>; > + > + status = "disabled"; > }; > > lpass_tlmm: pinctrl@33c0000 { > @@ -1888,6 +1902,8 @@ lpass_tlmm: pinctrl@33c0000 { > <&q6prmcc LPASS_HW_DCODEC_VOTE LPASS_CLK_ATTRIBUTE_COUPLE_NO>; > clock-names = "core", "audio"; > > + status = "disabled"; > + > tx_swr_default: tx-swr-default-state { > clk-pins { > pins = "gpio0";
On 02/01/2023 11:50, Johan Hovold wrote: > The sound nodes in the SoC dtsi should be disabled by default. > > Signed-off-by: Johan Hovold <johan+linaro@kernel.org> > --- > arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 16 ++++++++++++++++ > 1 file changed, 16 insertions(+) > > diff --git a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi > index ed1e2bee86ee..c1ce2d7b3675 100644 > --- a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi > +++ b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi > @@ -1733,6 +1733,8 @@ rxmacro: rxmacro@3200000 { > > pinctrl-names = "default"; > pinctrl-0 = <&rx_swr_default>; > + > + status = "disabled"; There is no reason this should be disabled by default. No external (per board) configuration is needed and by default SoC components should be enabled if they do not need anything from the board. > }; > > /* RX */ > @@ -1760,6 +1762,8 @@ swr1: soundwire-controller@3210000 { > #sound-dai-cells = <1>; > #address-cells = <2>; > #size-cells = <0>; > + > + status = "disabled"; For soundwires disabling makes sense - these are busses so they need to be explicitly enabled and populated with children. > }; > > txmacro: txmacro@3220000 { > @@ -1783,6 +1787,8 @@ txmacro: txmacro@3220000 { > #address-cells = <2>; > #size-cells = <2>; > #sound-dai-cells = <1>; > + > + status = "disabled"; > }; > > wsamacro: codec@3240000 { > @@ -1804,6 +1810,8 @@ wsamacro: codec@3240000 { > > pinctrl-names = "default"; > pinctrl-0 = <&wsa_swr_default>; > + > + status = "disabled"; > }; > > /* WSA */ > @@ -1830,6 +1838,8 @@ swr0: soundwire-controller@3250000 { > #sound-dai-cells = <1>; > #address-cells = <2>; > #size-cells = <0>; > + > + status = "disabled"; > }; > > /* TX */ > @@ -1858,6 +1868,8 @@ swr2: soundwire-controller@3330000 { > qcom,ports-word-length = /bits/ 8 <0xff 0x00 0xff 0xff>; > qcom,ports-block-group-count = /bits/ 8 <0xff 0xff 0xff 0xff>; > qcom,ports-lane-control = /bits/ 8 <0x00 0x01 0x00 0x00>; > + > + status = "disabled"; > }; > > vamacro: codec@3370000 { > @@ -1874,6 +1886,8 @@ vamacro: codec@3370000 { > #clock-cells = <0>; > clock-output-names = "fsgen"; > #sound-dai-cells = <1>; > + > + status = "disabled"; vamacro needs supply from the board so this one also makes sense. Best regards, Krzysztof
On Mon, Jan 02, 2023 at 01:29:38PM +0100, Krzysztof Kozlowski wrote: > On 02/01/2023 11:50, Johan Hovold wrote: > > The sound nodes in the SoC dtsi should be disabled by default. > > > > Signed-off-by: Johan Hovold <johan+linaro@kernel.org> > > --- > > arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 16 ++++++++++++++++ > > 1 file changed, 16 insertions(+) > > > > diff --git a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi > > index ed1e2bee86ee..c1ce2d7b3675 100644 > > --- a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi > > +++ b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi > > @@ -1733,6 +1733,8 @@ rxmacro: rxmacro@3200000 { > > > > pinctrl-names = "default"; > > pinctrl-0 = <&rx_swr_default>; > > + > > + status = "disabled"; > > There is no reason this should be disabled by default. No external (per > board) configuration is needed and by default SoC components should be > enabled if they do not need anything from the board. This node is one of the nodes for which driver support is not yet in place so that's one reason for at least disabling it temporarily. Since all other pinconfig lives in the board dts, if we decide to move also the sound pinconfig then then that may be a second. Johan
On Mon, Jan 02, 2023 at 04:15:42PM +0100, Johan Hovold wrote: > On Mon, Jan 02, 2023 at 01:29:38PM +0100, Krzysztof Kozlowski wrote: > > On 02/01/2023 11:50, Johan Hovold wrote: > > > The sound nodes in the SoC dtsi should be disabled by default. > > > > > > Signed-off-by: Johan Hovold <johan+linaro@kernel.org> > > > --- > > > arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 16 ++++++++++++++++ > > > 1 file changed, 16 insertions(+) > > > > > > diff --git a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi > > > index ed1e2bee86ee..c1ce2d7b3675 100644 > > > --- a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi > > > +++ b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi > > > @@ -1733,6 +1733,8 @@ rxmacro: rxmacro@3200000 { > > > > > > pinctrl-names = "default"; > > > pinctrl-0 = <&rx_swr_default>; > > > + > > > + status = "disabled"; > > > > There is no reason this should be disabled by default. No external (per > > board) configuration is needed and by default SoC components should be > > enabled if they do not need anything from the board. > > This node is one of the nodes for which driver support is not yet in > place so that's one reason for at least disabling it temporarily. > > Since all other pinconfig lives in the board dts, if we decide to move > also the sound pinconfig then then that may be a second. Also note that these depend on q6prmcc which is in turn depends on remoteproc_adsp being enabled by the x13s dts. So keeping them disabled by default seems justified. Johan
On 2.01.2023 18:17, Johan Hovold wrote: > On Mon, Jan 02, 2023 at 04:15:42PM +0100, Johan Hovold wrote: >> On Mon, Jan 02, 2023 at 01:29:38PM +0100, Krzysztof Kozlowski wrote: >>> On 02/01/2023 11:50, Johan Hovold wrote: >>>> The sound nodes in the SoC dtsi should be disabled by default. >>>> >>>> Signed-off-by: Johan Hovold <johan+linaro@kernel.org> >>>> --- >>>> arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 16 ++++++++++++++++ >>>> 1 file changed, 16 insertions(+) >>>> >>>> diff --git a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi >>>> index ed1e2bee86ee..c1ce2d7b3675 100644 >>>> --- a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi >>>> +++ b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi >>>> @@ -1733,6 +1733,8 @@ rxmacro: rxmacro@3200000 { >>>> >>>> pinctrl-names = "default"; >>>> pinctrl-0 = <&rx_swr_default>; >>>> + >>>> + status = "disabled"; >>> >>> There is no reason this should be disabled by default. No external (per >>> board) configuration is needed and by default SoC components should be >>> enabled if they do not need anything from the board. >> >> This node is one of the nodes for which driver support is not yet in >> place so that's one reason for at least disabling it temporarily. >> >> Since all other pinconfig lives in the board dts, if we decide to move >> also the sound pinconfig then then that may be a second. > > Also note that these depend on q6prmcc which is in turn depends on > remoteproc_adsp being enabled by the x13s dts. > > So keeping them disabled by default seems justified. This I agree with, having sound-related nodes enabled by default results in a big chunk of "deferred probe pending" spam when you boot with adsp disabled (for example when you don't have fw for your board in your rootfs).. Konrad > > Johan
diff --git a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi index ed1e2bee86ee..c1ce2d7b3675 100644 --- a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi +++ b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi @@ -1733,6 +1733,8 @@ rxmacro: rxmacro@3200000 { pinctrl-names = "default"; pinctrl-0 = <&rx_swr_default>; + + status = "disabled"; }; /* RX */ @@ -1760,6 +1762,8 @@ swr1: soundwire-controller@3210000 { #sound-dai-cells = <1>; #address-cells = <2>; #size-cells = <0>; + + status = "disabled"; }; txmacro: txmacro@3220000 { @@ -1783,6 +1787,8 @@ txmacro: txmacro@3220000 { #address-cells = <2>; #size-cells = <2>; #sound-dai-cells = <1>; + + status = "disabled"; }; wsamacro: codec@3240000 { @@ -1804,6 +1810,8 @@ wsamacro: codec@3240000 { pinctrl-names = "default"; pinctrl-0 = <&wsa_swr_default>; + + status = "disabled"; }; /* WSA */ @@ -1830,6 +1838,8 @@ swr0: soundwire-controller@3250000 { #sound-dai-cells = <1>; #address-cells = <2>; #size-cells = <0>; + + status = "disabled"; }; /* TX */ @@ -1858,6 +1868,8 @@ swr2: soundwire-controller@3330000 { qcom,ports-word-length = /bits/ 8 <0xff 0x00 0xff 0xff>; qcom,ports-block-group-count = /bits/ 8 <0xff 0xff 0xff 0xff>; qcom,ports-lane-control = /bits/ 8 <0x00 0x01 0x00 0x00>; + + status = "disabled"; }; vamacro: codec@3370000 { @@ -1874,6 +1886,8 @@ vamacro: codec@3370000 { #clock-cells = <0>; clock-output-names = "fsgen"; #sound-dai-cells = <1>; + + status = "disabled"; }; lpass_tlmm: pinctrl@33c0000 { @@ -1888,6 +1902,8 @@ lpass_tlmm: pinctrl@33c0000 { <&q6prmcc LPASS_HW_DCODEC_VOTE LPASS_CLK_ATTRIBUTE_COUPLE_NO>; clock-names = "core", "audio"; + status = "disabled"; + tx_swr_default: tx-swr-default-state { clk-pins { pins = "gpio0";
The sound nodes in the SoC dtsi should be disabled by default. Signed-off-by: Johan Hovold <johan+linaro@kernel.org> --- arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)