Re: [PATCH v3 2/3] arm64: dts: qcom: sm8450: add Soundwire and LPASS

[Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>]

On 16/11/2022 12:17, Konrad Dybcio wrote:
> 
> 
> 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@linaro.org>
>>>>
>>>> Add Soundwire controllers, Low Power Audio SubSystem (LPASS) devices and
>>>> LPASS pin controller.
>>>>
>>>> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>>>> Co-developed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>>>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.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@31e0000 {
>>>> +			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..

OK, I'll re-order these.

Best regards,
Krzysztof