diff mbox series

[6/9] arm64: dts: qcom: sc8280xp-crd: enable WiFi controller

Message ID 20221110103558.12690-7-johan+linaro@kernel.org
State New
Headers
Series arm64: dts: qcom: sc8280xp/sa8540p: add support for PCIe |

Commit Message

Johan Hovold Nov. 10, 2022, 10:35 a.m. UTC 
  Enable the Qualcomm QCNFA765 Wireless Network Adapter connected to
PCIe4.

Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 arch/arm64/boot/dts/qcom/sc8280xp-crd.dts | 65 +++++++++++++++++++++++
 1 file changed, 65 insertions(+)

Comments

Manivannan Sadhasivam Nov. 10, 2022, 11:35 a.m. UTC | #1 
On Thu, Nov 10, 2022 at 11:35:55AM +0100, Johan Hovold wrote:
> Enable the Qualcomm QCNFA765 Wireless Network Adapter connected to
> PCIe4.
> 
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> ---
>  arch/arm64/boot/dts/qcom/sc8280xp-crd.dts | 65 +++++++++++++++++++++++
>  1 file changed, 65 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts b/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts
> index 5b9e37a16f9f..ab5b0aadeead 100644
> --- a/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts
> +++ b/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts
> @@ -81,6 +81,22 @@ vreg_misc_3p3: regulator-misc-3p3 {
>  		regulator-always-on;
>  	};
>  
> +	vreg_wlan: regulator-wlan {
> +		compatible = "regulator-fixed";
> +
> +		regulator-name = "VCC_WLAN_3R9";
> +		regulator-min-microvolt = <3900000>;
> +		regulator-max-microvolt = <3900000>;
> +
> +		gpio = <&pmr735a_gpios 1 GPIO_ACTIVE_HIGH>;
> +		enable-active-high;
> +
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&hastings_reg_en>;

Hastings is the family name of QCA639x WLAN chipsets. I don't think it would be
applicable here. Please use "wlan_reg_en" as that matches the convention used
throughout this file.

Thanks,
Mani

> +
> +		regulator-boot-on;
> +	};
> +
>  	vreg_wwan: regulator-wwan {
>  		compatible = "regulator-fixed";
>  
> @@ -246,6 +262,25 @@ &pcie3a_phy {
>  	status = "okay";
>  };
>  
> +&pcie4 {
> +	perst-gpios = <&tlmm 141 GPIO_ACTIVE_LOW>;
> +	wake-gpios = <&tlmm 139 GPIO_ACTIVE_LOW>;
> +
> +	vddpe-3v3-supply = <&vreg_wlan>;
> +
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pcie4_default>;
> +
> +	status = "okay";
> +};
> +
> +&pcie4_phy {
> +	vdda-phy-supply = <&vreg_l6d>;
> +	vdda-pll-supply = <&vreg_l4d>;
> +
> +	status = "okay";
> +};
> +
>  &pmc8280c_lpg {
>  	status = "okay";
>  };
> @@ -445,6 +480,13 @@ edp_bl_pwm: edp-bl-pwm-state {
>  	};
>  };
>  
> +&pmr735a_gpios {
> +	hastings_reg_en: hastings-reg-en-state {
> +		pins = "gpio1";
> +		function = "normal";
> +	};
> +};
> +
>  &tlmm {
>  	gpio-reserved-ranges = <74 6>, <83 4>, <125 2>, <128 2>, <154 7>;
>  
> @@ -521,6 +563,29 @@ wake-n-pins {
>  		};
>  	};
>  
> +	pcie4_default: pcie4-default-state {
> +		clkreq-n-pins {
> +			pins = "gpio140";
> +			function = "pcie4_clkreq";
> +			drive-strength = <2>;
> +			bias-pull-up;
> +		};
> +
> +		perst-n-pins {
> +			pins = "gpio141";
> +			function = "gpio";
> +			drive-strength = <2>;
> +			bias-pull-down;
> +		};
> +
> +		wake-n-pins {
> +			pins = "gpio139";
> +			function = "gpio";
> +			drive-strength = <2>;
> +			bias-pull-up;
> +		};
> +	};
> +
>  	qup0_i2c4_default: qup0-i2c4-default-state {
>  		pins = "gpio171", "gpio172";
>  		function = "qup4";
> -- 
> 2.37.4
>
Johan Hovold Nov. 11, 2022, 4:27 p.m. UTC | #2 
On Thu, Nov 10, 2022 at 05:05:13PM +0530, Manivannan Sadhasivam wrote:
> On Thu, Nov 10, 2022 at 11:35:55AM +0100, Johan Hovold wrote:
> > Enable the Qualcomm QCNFA765 Wireless Network Adapter connected to
> > PCIe4.
> > 
> > Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> > ---
> >  arch/arm64/boot/dts/qcom/sc8280xp-crd.dts | 65 +++++++++++++++++++++++
> >  1 file changed, 65 insertions(+)
> > 
> > diff --git a/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts b/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts
> > index 5b9e37a16f9f..ab5b0aadeead 100644
> > --- a/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts
> > +++ b/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts
> > @@ -81,6 +81,22 @@ vreg_misc_3p3: regulator-misc-3p3 {
> >  		regulator-always-on;
> >  	};
> >  
> > +	vreg_wlan: regulator-wlan {
> > +		compatible = "regulator-fixed";
> > +
> > +		regulator-name = "VCC_WLAN_3R9";
> > +		regulator-min-microvolt = <3900000>;
> > +		regulator-max-microvolt = <3900000>;
> > +
> > +		gpio = <&pmr735a_gpios 1 GPIO_ACTIVE_HIGH>;
> > +		enable-active-high;
> > +
> > +		pinctrl-names = "default";
> > +		pinctrl-0 = <&hastings_reg_en>;
> 
> Hastings is the family name of QCA639x WLAN chipsets. I don't think it would be
> applicable here. Please use "wlan_reg_en" as that matches the convention used
> throughout this file.

The pin name here comes from the schematics, which is what we should use
for naming when we can.

Johan
Bjorn Andersson Nov. 11, 2022, 8:40 p.m. UTC | #3 
On Fri, Nov 11, 2022 at 05:27:46PM +0100, Johan Hovold wrote:
> On Thu, Nov 10, 2022 at 05:05:13PM +0530, Manivannan Sadhasivam wrote:
> > On Thu, Nov 10, 2022 at 11:35:55AM +0100, Johan Hovold wrote:
> > > Enable the Qualcomm QCNFA765 Wireless Network Adapter connected to
> > > PCIe4.
> > > 
> > > Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> > > ---
> > >  arch/arm64/boot/dts/qcom/sc8280xp-crd.dts | 65 +++++++++++++++++++++++
> > >  1 file changed, 65 insertions(+)
> > > 
> > > diff --git a/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts b/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts
> > > index 5b9e37a16f9f..ab5b0aadeead 100644
> > > --- a/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts
> > > +++ b/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts
> > > @@ -81,6 +81,22 @@ vreg_misc_3p3: regulator-misc-3p3 {
> > >  		regulator-always-on;
> > >  	};
> > >  
> > > +	vreg_wlan: regulator-wlan {
> > > +		compatible = "regulator-fixed";
> > > +
> > > +		regulator-name = "VCC_WLAN_3R9";
> > > +		regulator-min-microvolt = <3900000>;
> > > +		regulator-max-microvolt = <3900000>;
> > > +
> > > +		gpio = <&pmr735a_gpios 1 GPIO_ACTIVE_HIGH>;
> > > +		enable-active-high;
> > > +
> > > +		pinctrl-names = "default";
> > > +		pinctrl-0 = <&hastings_reg_en>;
> > 
> > Hastings is the family name of QCA639x WLAN chipsets. I don't think it would be
> > applicable here. Please use "wlan_reg_en" as that matches the convention used
> > throughout this file.
> 
> The pin name here comes from the schematics, which is what we should use
> for naming when we can.
> 

Following the naming in the schematics is the right thing to do.

Regards,
Bjorn
Manivannan Sadhasivam Nov. 12, 2022, 2:39 p.m. UTC | #4 
On Fri, Nov 11, 2022 at 02:40:21PM -0600, Bjorn Andersson wrote:
> On Fri, Nov 11, 2022 at 05:27:46PM +0100, Johan Hovold wrote:
> > On Thu, Nov 10, 2022 at 05:05:13PM +0530, Manivannan Sadhasivam wrote:
> > > On Thu, Nov 10, 2022 at 11:35:55AM +0100, Johan Hovold wrote:
> > > > Enable the Qualcomm QCNFA765 Wireless Network Adapter connected to
> > > > PCIe4.
> > > > 
> > > > Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> > > > ---
> > > >  arch/arm64/boot/dts/qcom/sc8280xp-crd.dts | 65 +++++++++++++++++++++++
> > > >  1 file changed, 65 insertions(+)
> > > > 
> > > > diff --git a/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts b/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts
> > > > index 5b9e37a16f9f..ab5b0aadeead 100644
> > > > --- a/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts
> > > > +++ b/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts
> > > > @@ -81,6 +81,22 @@ vreg_misc_3p3: regulator-misc-3p3 {
> > > >  		regulator-always-on;
> > > >  	};
> > > >  
> > > > +	vreg_wlan: regulator-wlan {
> > > > +		compatible = "regulator-fixed";
> > > > +
> > > > +		regulator-name = "VCC_WLAN_3R9";
> > > > +		regulator-min-microvolt = <3900000>;
> > > > +		regulator-max-microvolt = <3900000>;
> > > > +
> > > > +		gpio = <&pmr735a_gpios 1 GPIO_ACTIVE_HIGH>;
> > > > +		enable-active-high;
> > > > +
> > > > +		pinctrl-names = "default";
> > > > +		pinctrl-0 = <&hastings_reg_en>;
> > > 
> > > Hastings is the family name of QCA639x WLAN chipsets. I don't think it would be
> > > applicable here. Please use "wlan_reg_en" as that matches the convention used
> > > throughout this file.
> > 
> > The pin name here comes from the schematics, which is what we should use
> > for naming when we can.

If hastings is what mentioned in the schematics then it is fine (I can see that
now). For a moment I thought it came from downstream...

Thanks,
Mani

> > 
> 
> Following the naming in the schematics is the right thing to do.
> 
> Regards,
> Bjorn
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts b/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts
index 5b9e37a16f9f..ab5b0aadeead 100644
--- a/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts
+++ b/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts
@@ -81,6 +81,22 @@  vreg_misc_3p3: regulator-misc-3p3 {
 		regulator-always-on;
 	};
 
+	vreg_wlan: regulator-wlan {
+		compatible = "regulator-fixed";
+
+		regulator-name = "VCC_WLAN_3R9";
+		regulator-min-microvolt = <3900000>;
+		regulator-max-microvolt = <3900000>;
+
+		gpio = <&pmr735a_gpios 1 GPIO_ACTIVE_HIGH>;
+		enable-active-high;
+
+		pinctrl-names = "default";
+		pinctrl-0 = <&hastings_reg_en>;
+
+		regulator-boot-on;
+	};
+
 	vreg_wwan: regulator-wwan {
 		compatible = "regulator-fixed";
 
@@ -246,6 +262,25 @@  &pcie3a_phy {
 	status = "okay";
 };
 
+&pcie4 {
+	perst-gpios = <&tlmm 141 GPIO_ACTIVE_LOW>;
+	wake-gpios = <&tlmm 139 GPIO_ACTIVE_LOW>;
+
+	vddpe-3v3-supply = <&vreg_wlan>;
+
+	pinctrl-names = "default";
+	pinctrl-0 = <&pcie4_default>;
+
+	status = "okay";
+};
+
+&pcie4_phy {
+	vdda-phy-supply = <&vreg_l6d>;
+	vdda-pll-supply = <&vreg_l4d>;
+
+	status = "okay";
+};
+
 &pmc8280c_lpg {
 	status = "okay";
 };
@@ -445,6 +480,13 @@  edp_bl_pwm: edp-bl-pwm-state {
 	};
 };
 
+&pmr735a_gpios {
+	hastings_reg_en: hastings-reg-en-state {
+		pins = "gpio1";
+		function = "normal";
+	};
+};
+
 &tlmm {
 	gpio-reserved-ranges = <74 6>, <83 4>, <125 2>, <128 2>, <154 7>;
 
@@ -521,6 +563,29 @@  wake-n-pins {
 		};
 	};
 
+	pcie4_default: pcie4-default-state {
+		clkreq-n-pins {
+			pins = "gpio140";
+			function = "pcie4_clkreq";
+			drive-strength = <2>;
+			bias-pull-up;
+		};
+
+		perst-n-pins {
+			pins = "gpio141";
+			function = "gpio";
+			drive-strength = <2>;
+			bias-pull-down;
+		};
+
+		wake-n-pins {
+			pins = "gpio139";
+			function = "gpio";
+			drive-strength = <2>;
+			bias-pull-up;
+		};
+	};
+
 	qup0_i2c4_default: qup0-i2c4-default-state {
 		pins = "gpio171", "gpio172";
 		function = "qup4";