diff mbox series

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

Message ID 20180921090907.25937-3-govinds@codeaurora.org (mailing list archive)
State Not Applicable
Delegated to: Kalle Valo
Headers show
Series Enable ath10k wcn3990 wifi driver support on sdm845 | expand

Commit Message

Govind Singh Sept. 21, 2018, 9:09 a.m. UTC
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@codeaurora.org>
---
 arch/arm64/boot/dts/qcom/sdm845-mtp.dts |  7 +++++++
 arch/arm64/boot/dts/qcom/sdm845.dtsi    | 25 +++++++++++++++++++++++++
 2 files changed, 32 insertions(+)

Comments

kernel test robot Sept. 23, 2018, 2:35 a.m. UTC | #1
Hi Govind,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on agross/for-next]
[also build test ERROR on v4.19-rc4 next-20180921]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Govind-Singh/Enable-ath10k-wcn3990-wifi-driver-support-on-sdm845/20180923-061400
base:   https://git.kernel.org/pub/scm/linux/kernel/git/agross/linux.git for-next
config: arm64-defconfig (attached as .config)
compiler: aarch64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=7.2.0 make.cross ARCH=arm64 

All errors (new ones prefixed by >>):

   Error: arch/arm64/boot/dts/qcom/sdm845-mtp.dts:62.1-6 Label or path wifi not found
>> FATAL ERROR: Syntax error parsing input tree

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
kernel test robot Sept. 23, 2018, 6:04 a.m. UTC | #2
Hi Govind,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on agross/for-next]
[also build test ERROR on v4.19-rc4 next-20180921]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Govind-Singh/Enable-ath10k-wcn3990-wifi-driver-support-on-sdm845/20180923-061400
base:   https://git.kernel.org/pub/scm/linux/kernel/git/agross/linux.git for-next
config: arm64-allyesconfig (attached as .config)
compiler: aarch64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=7.2.0 make.cross ARCH=arm64 

All errors (new ones prefixed by >>):

>> Error: arch/arm64/boot/dts/qcom/sdm845-mtp.dts:62.1-6 Label or path wifi not found
   FATAL ERROR: Syntax error parsing input tree

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
Brian Norris Sept. 24, 2018, 5:11 p.m. UTC | #3
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@codeaurora.org>
> ---
>  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.

> +	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.)

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.

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
>
Govind Singh Sept. 25, 2018, 12:15 p.m. UTC | #4
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@codeaurora.org>
>> ---
>>  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
diff mbox series

Patch

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 {
+	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 {
+			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 */ >;
+		};
 	};
 };