Re: [PATCH v6 1/3] arm64: dts: qcom: sc7280: Add nodes for va tx and rx macros and external codecs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 4/12/2022 1:23 AM, Matthias Kaehlcke wrote:
Thanks for your time Matthias!!!
On Mon, Apr 11, 2022 at 07:32:33PM +0530, Srinivasa Rao Mandadapu wrote:
On 4/6/2022 3:05 AM, Matthias Kaehlcke wrote:
Thanks for your time Matthias!!!
On Tue, Apr 05, 2022 at 04:43:38PM +0530, Srinivasa Rao Mandadapu wrote:
SC7280 has VA, TX and RX macros with SoundWire Controllers to attach with
codecs like WCD938x, max98360a using soundwire masters and i2s bus.
Add these nodes for sc7280 based platforms audio use case.
Add tlmm gpio property in wcd938x node for switching CTIA/OMTP Headset.

Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@xxxxxxxxxxx>
Co-developed-by: Venkata Prasad Potturu <quic_potturu@xxxxxxxxxxx>
Signed-off-by: Venkata Prasad Potturu <quic_potturu@xxxxxxxxxxx>
---
A change log would be helpful for reviewers
Change log added in cover letter.
 From my perspective as a reviewer I find it more practical to have a
change log per patch.
Okay. will do accordingly from next time.

diff --git a/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi b/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi
index de646d9..c6a04c3 100644
--- a/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi
@@ -20,6 +20,14 @@
   #include "sc7280-chrome-common.dtsi"
   / {
+	max98360a: audio-codec-0 {
+		compatible = "maxim,max98360a";
+		pinctrl-names = "default";
+		pinctrl-0 = <&amp_en>;
+		sdmode-gpios = <&tlmm 63 GPIO_ACTIVE_HIGH>;
+		#sound-dai-cells = <0>;
+	};
+
This node shouldn't be at top but with the other device nodes, in
alphabetical order, i.e. before 'pwmleds'.
Actually we are sorting as per node name. Hence kept it here. As per
previous reviewer comments, sorted accordingly.

Please suggest better approach.
True, I forgot the node names are used for sorting, not the labels.
Okay.

diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
index db74fc3..78ec84c 100644
--- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
@@ -822,6 +822,127 @@
   			#power-domain-cells = <1>;
   		};
+		rxmacro: codec@3200000 {
These node are not at the correct position. They should be sorted by
address and hence be inserted between 'lpasscc@3000000' and
'interconnect@3c40000'.
Actually we are sorting as per node(codec) name. Hence kept it here. As per
previous reviewer comments, sorted accordingly.
Could you provide a pointer to those comments?
Sorry. it seems we misunderstood.

My understanding is that we are sorting by node name when nodes don't have
addresses or for overrides in board files/snippets, however the nodes under
'soc@0' are sorted by address.

Another nit: 'rx_macro, tx_macro, va_macro' instead of the labels without
underscore? Not really important, but maybe slightly more readable and
would match the 'spelling' of the compatible strings.
Okay. Will change accordingly!!!



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux