[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1ce84668-5f21-5e62-6a02-be7c6e79012e@linaro.org>
Date: Wed, 16 Nov 2022 12:17:31 +0100
From: Konrad Dybcio <konrad.dybcio@...aro.org>
To: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
Andy Gross <agross@...nel.org>,
Bjorn Andersson <andersson@...nel.org>,
Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
linux-arm-msm@...r.kernel.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org
Cc: Srinivas Kandagatla <srinivas.kandagatla@...aro.org>
Subject: Re: [PATCH v3 2/3] arm64: dts: qcom: sm8450: add Soundwire and LPASS
On 16/11/2022 11:40, Krzysztof Kozlowski wrote:
> On 16/11/2022 11:20, Konrad Dybcio wrote:
>>
>>
>> On 16/11/2022 11:13, Krzysztof Kozlowski wrote:
>>> From: Srinivas Kandagatla <srinivas.kandagatla@...aro.org>
>>>
>>> Add Soundwire controllers, Low Power Audio SubSystem (LPASS) devices and
>>> LPASS pin controller.
>>>
>>> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@...aro.org>
>>> Co-developed-by: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
>>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
>>>
>>> ---
>>>
>>> Changes since v2:
>>> 1. Use lower-case hex.
>>>
>>> Changes since v1:
>>> 1. Whitespace cleanups.
>>> 2. Correct include - do not use deprecated one.
>>> ---
>>> arch/arm64/boot/dts/qcom/sm8450.dtsi | 295 +++++++++++++++++++++++++++
>>> 1 file changed, 295 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/qcom/sm8450.dtsi b/arch/arm64/boot/dts/qcom/sm8450.dtsi
>>> index 4b0a1eee8bd9..747440d0445a 100644
>>> --- a/arch/arm64/boot/dts/qcom/sm8450.dtsi
>>> +++ b/arch/arm64/boot/dts/qcom/sm8450.dtsi
>>> @@ -15,6 +15,7 @@
>>> #include <dt-bindings/interconnect/qcom,sm8450.h>
>>> #include <dt-bindings/soc/qcom,gpr.h>
>>> #include <dt-bindings/soc/qcom,rpmh-rsc.h>
>>> +#include <dt-bindings/sound/qcom,q6dsp-lpass-ports.h>
>>> #include <dt-bindings/thermal/thermal.h>
>>>
>>> / {
>>> @@ -2097,6 +2098,212 @@ compute-cb@3 {
>>> };
>>> };
>>>
>>> + wsa2macro: codec@...0000 {
>>> + compatible = "qcom,sm8450-lpass-wsa-macro";
>>> + reg = <0 0x031e0000 0 0x1000>;
>>> + clocks = <&q6prmcc LPASS_CLK_ID_WSA_CORE_TX_MCLK LPASS_CLK_ATTRIBUTE_COUPLE_NO>,
>>> + <&q6prmcc LPASS_CLK_ID_RX_CORE_MCLK2_2X_MCLK LPASS_CLK_ATTRIBUTE_COUPLE_NO>,
>>> + <&q6prmcc LPASS_HW_MACRO_VOTE LPASS_CLK_ATTRIBUTE_COUPLE_NO>,
>>> + <&q6prmcc LPASS_HW_DCODEC_VOTE LPASS_CLK_ATTRIBUTE_COUPLE_NO>,
>>> + <&vamacro>;
>>> + clock-names = "mclk", "npl", "macro", "dcodec", "fsgen";
>>> + assigned-clocks = <&q6prmcc LPASS_CLK_ID_WSA_CORE_TX_MCLK LPASS_CLK_ATTRIBUTE_COUPLE_NO>,
>>> + <&q6prmcc LPASS_CLK_ID_RX_CORE_MCLK2_2X_MCLK LPASS_CLK_ATTRIBUTE_COUPLE_NO>;
>>> + assigned-clock-rates = <19200000>, <19200000>;
>>> +
>>> + #clock-cells = <0>;
>>> + clock-output-names = "wsa2-mclk";
>>> + #sound-dai-cells = <1>;
>> I think I'm being a bit too picky, but #-cells could go as the last
>> bunch of properties.
>
> I was thinking about this as well, but some of other codecs which are
> very similar (also "macro") do not have pinctrls and this makes them
> unified with additions at the end.
>
> Are you sure you still prefer alphabetical order?
Thinking about it, IMO it makes sense to add things "roughly where they
belong", a.k.a if there was a codec that for whatever reason also
required a power domain, we would stick it somewhere in the middle and
not at the end..
>
>>
>>
>>> +
>>> + pinctrl-names = "default";
>>> + pinctrl-0 = <&wsa2_swr_active>;
>>> + };
>>> +
>>> + /* WSA2 */
>>> + swr4: soundwire-controller@...0000 {
>>> + reg = <0 0x031f0000 0 0x2000>;
>>> + compatible = "qcom,soundwire-v1.7.0";
>>> + interrupts = <GIC_SPI 171 IRQ_TYPE_LEVEL_HIGH>;
>>> + clocks = <&wsa2macro>;
>>> + clock-names = "iface";
>>> +
>>> + qcom,din-ports = <2>;
>>> + qcom,dout-ports = <6>;
>>> +
>>> + qcom,ports-sinterval-low = /bits/ 8 <0x07 0x1f 0x3f 0x07 0x1f 0x3f 0x0f 0x0f>;
>>> + qcom,ports-offset1 = /bits/ 8 <0x01 0x02 0x0c 0x06 0x12 0x0d 0x07 0x0a>;
>>> + qcom,ports-offset2 = /bits/ 8 <0xff 0x00 0x1f 0xff 0x00 0x1f 0x00 0x00>;
>>> + qcom,ports-hstart = /bits/ 8 <0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff>;
>>> + qcom,ports-hstop = /bits/ 8 <0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff>;
>>> + qcom,ports-word-length = /bits/ 8 <0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff>;
>>> + qcom,ports-block-pack-mode = /bits/ 8 <0xff 0xff 0x01 0xff 0xff 0x01 0xff 0xff>;
>>> + qcom,ports-block-group-count = /bits/ 8 <0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff>;
>>> + qcom,ports-lane-control = /bits/ 8 <0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff>;
>>> +
>>> + #sound-dai-cells = <1>;
>>> + #address-cells = <2>;
>>> + #size-cells = <0>;
>>> + };
>>> +
>>> + rxmacro: codec@...0000 {
>>> + compatible = "qcom,sm8450-lpass-rx-macro";
>>> + reg = <0 0x3200000 0 0x1000>;
>>> + clocks = <&q6prmcc LPASS_CLK_ID_RX_CORE_TX_MCLK LPASS_CLK_ATTRIBUTE_COUPLE_NO>,
>>> + <&q6prmcc LPASS_CLK_ID_RX_CORE_MCLK2_2X_MCLK LPASS_CLK_ATTRIBUTE_COUPLE_NO>,
>>> + <&q6prmcc LPASS_HW_MACRO_VOTE LPASS_CLK_ATTRIBUTE_COUPLE_NO>,
>>> + <&q6prmcc LPASS_HW_DCODEC_VOTE LPASS_CLK_ATTRIBUTE_COUPLE_NO>,
>>> + <&vamacro>;
>>> + clock-names = "mclk", "npl", "macro", "dcodec", "fsgen";
>>> +
>>> + assigned-clocks = <&q6prmcc LPASS_CLK_ID_RX_CORE_TX_MCLK LPASS_CLK_ATTRIBUTE_COUPLE_NO>,
>>> + <&q6prmcc LPASS_CLK_ID_RX_CORE_MCLK2_2X_MCLK LPASS_CLK_ATTRIBUTE_COUPLE_NO>;
>>> + assigned-clock-rates = <19200000>, <19200000>;
>>> +
>>> + #clock-cells = <0>;
>>> + clock-output-names = "mclk";
>>> + #sound-dai-cells = <1>;
>>> +
>>> + pinctrl-names = "default";
>>> + pinctrl-0 = <&rx_swr_active>;
>>> + };
>>> +
>>> + swr1: soundwire-controller@...0000 {
>>> + reg = <0 0x3210000 0 0x2000>;
>>> + compatible = "qcom,soundwire-v1.7.0";
>> Some nodes have reg and compatible flipped.
>
> Ack
>
>>
>>> + interrupts = <GIC_SPI 155 IRQ_TYPE_LEVEL_HIGH>;
>>> + clocks = <&rxmacro>;
>>> + clock-names = "iface";
>>> + label = "RX";
>>> + qcom,din-ports = <0>;
>>> + qcom,dout-ports = <5>;
>>> +
>
>
> (...)
>
>>> apps_smmu: iommu@...00000 {
>>> compatible = "qcom,sm8450-smmu-500", "arm,mmu-500";
>>> reg = <0 0x15000000 0 0x100000>;
>>> @@ -3507,6 +3799,9 @@ lpass_ag_noc: interconnect@...0000 {
>>> };
>>> };
>>>
>>> + sound: sound {
>>> + };
>> You asked another folk working on sa8540p to not include sound in the
>> SoC dtsi.
>
> I asked not to put it in soc node. It can be in DTSI, but not under soc.
Oh okay, then I misread that, sorry.
Konrad
>
> Best regards,
> Krzysztof
>
Powered by blists - more mailing lists