Re: [PATCH 2/4] dts: arm64/sdm845: Add WCN3990 WLAN module device node

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

 



Hi Brian,

On 2018-09-24 22:41, Brian Norris wrote:
Hi,

On Fri, Sep 21, 2018 at 02:39:05PM +0530, Govind Singh wrote:
Add device node for the ath10k SNOC platform driver probe
and add resources required for WCN3990 on SDM845 soc.

Signed-off-by: Govind Singh <govinds@xxxxxxxxxxxxxx>
---
 arch/arm64/boot/dts/qcom/sdm845-mtp.dts |  7 +++++++
arch/arm64/boot/dts/qcom/sdm845.dtsi | 25 +++++++++++++++++++++++++
 2 files changed, 32 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sdm845-mtp.dts b/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
index eedfaf8922e2..4de57f7df93c 100644
--- a/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
+++ b/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
@@ -440,3 +440,10 @@
 		bias-pull-up;
 	};
 };
+
+&wifi {

As the bots have already told you, you missed adding a 'wifi' label to
sdm845.dtsi, so this reference doesn't compile.


Yes, I will correct this in next revision.

+	vdd-0.8-cx-mx-supply = <&vreg_l5a_0p8>;
+	vdd-1.8-xo-supply = <&vreg_l7a_1p8>;
+	vdd-1.3-rfa-supply = <&vreg_l17a_1p3>;
+	vdd-3.3-ch0-supply = <&vreg_l25a_3p3>;
+};
diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
index e080072cdfdb..b0b107641e74 100644
--- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
@@ -86,6 +86,11 @@
 			reg = <0 0x86200000 0 0x2d00000>;
 			no-map;
 		};
+
+		wlan_msa_mem: reserved-memory@96700000 {
+			no-map;
+			reg = <0 0x96700000 0 0x100000>;
+		};
 	};

 	cpus {
@@ -1200,5 +1205,25 @@
 				status = "disabled";
 			};
 		};
+
+		qcom,wifi {

That's not a legit node name. Nodes should (a) have a generic name and
(b) include a unit address when the node contains a 'reg' property. [1]

So, how about 'wifi@18800000'? (And include the 'wifi:' label, since you
want to refer to it in other files.)


I am fine with 'wifi@18800000'.

Also, this seems like the kind of device that should remain 'status =
"disabled"' in the top-level DTSI, and be overridden with 'status =
"okay"' in the child DTS.


Sure, i will populate 'status = "disabled"' in next revision, which can be enabled from child dtsi
as part of separate change.

Brian

[1] See:
https://elinux.org/Device_Tree_Usage#Node_Names
https://elinux.org/images/c/cf/Power_ePAPR_APPROVED_v1.1.pdf

+			compatible = "qcom,wcn3990-wifi";
+			reg = <0x18800000 0x800000>;
+			reg-names = "membase";
+			memory-region = <&wlan_msa_mem>;
+			interrupts =
+				<0 413 0 /* CE0 */ >,
+				<0 414 0 /* CE1 */ >,
+				<0 415 0 /* CE2 */ >,
+				<0 416 0 /* CE3 */ >,
+				<0 417 0 /* CE4 */ >,
+				<0 418 0 /* CE5 */ >,
+				<0 420 0 /* CE6 */ >,
+				<0 421 0 /* CE7 */ >,
+				<0 422 0 /* CE8 */ >,
+				<0 423 0 /* CE9 */ >,
+				<0 424 0 /* CE10 */ >,
+				<0 425 0 /* CE11 */ >;
+		};
 	};
 };
--
2.17.0


Thanks,
Govind



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux