lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<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

Powered by Openwall GNU/*/Linux Powered by OpenVZ