[PATCH v2 0/5] Add support for Qualcomm QCA639x chips family

[PATCH v2 0/5] Add support for Qualcomm QCA639x chips family

[Dmitry Baryshkov]

Qualcomm QCA639x is a family of WiFi + Bluetooth chips, with BT part
being controlled through the UART and WiFi being present on PCIe
bus. Both blocks share common power sources wich should be turned on
before either of devices can be probed. Declare common 'qca639x' driver
providing a power domain to be used by both BT and WiFi parts.

Changes since v1:
 - Stopped using wildcard in the dts binding, stick to qcom,qca6390.
 - Stopped using pcie0_phy for qca639x power domain.
 - Describe root PCIe bridge in the dts and bind power domain to the
   bridge.
 - Add pci quirk to power up power domains connected to this bridge.

----------------------------------------------------------------
Dmitry Baryshkov (4):
      misc: qca639x: add support for QCA639x powerup sequence
      arm64: qcom: dts: qrb5165-rb5: add qca6391 power device
      pcie-qcom: provide a way to power up qca6390 chip on RB5 platform
      arm64: dtb: qcom: qrb5165-rb5: add bridge@0,0 to power up qca6391 chip

Manivannan Sadhasivam (1):
      arm64: dts: qcom: Add Bluetooth support on RB5

 arch/arm64/boot/dts/qcom/qrb5165-rb5.dts | 102 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 drivers/misc/Kconfig                     |  12 ++++++++++++
 drivers/misc/Makefile                    |   1 +
 drivers/misc/qcom-qca639x.c              | 164 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 drivers/pci/controller/dwc/pcie-qcom.c   |  21 ++++++++++++++++++++
 5 files changed, 300 insertions(+)
 create mode 100644 drivers/misc/qcom-qca639x.c

[PATCH v2 1/5] misc: qca639x: add support for QCA639x powerup sequence

[Dmitry Baryshkov]

Qualcomm QCA639x is a family of WiFi + Bluetooth SoCs, with BT part
being controlled through the UART and WiFi being present on PCIe
bus. Both blocks share common power sources. Add device driver handling
power sequencing of QCA6390/1.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/misc/Kconfig        |  12 +++
 drivers/misc/Makefile       |   1 +
 drivers/misc/qcom-qca639x.c | 164 ++++++++++++++++++++++++++++++++++++
 3 files changed, 177 insertions(+)
 create mode 100644 drivers/misc/qcom-qca639x.c

diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index e90c2524e46c..a14f67ab476c 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -255,6 +255,18 @@ config QCOM_FASTRPC
 	  applications DSP processor. Say M if you want to enable this
 	  module.
 
+config QCOM_QCA639X
+	tristate "Qualcomm QCA639x WiFi/Bluetooth module support"
+	depends on REGULATOR && PM_GENERIC_DOMAINS
+	help
+	  If you say yes to this option, support will be included for Qualcomm
+	  QCA639x family of WiFi and Bluetooth SoCs. Note, this driver supports
+	  only power control for this SoC, you still have to enable individual
+	  Bluetooth and WiFi drivers.
+
+	  Say M here if you want to include support for QCA639x chips as a
+	  module. This will build a module called "qcom-qca639x".
+
 config SGI_GRU
 	tristate "SGI GRU driver"
 	depends on X86_UV && SMP
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index f65e8b18ecd8..bf21d5b1189f 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -18,6 +18,7 @@ obj-$(CONFIG_TIFM_7XX1)       	+= tifm_7xx1.o
 obj-$(CONFIG_PHANTOM)		+= phantom.o
 obj-$(CONFIG_QCOM_COINCELL)	+= qcom-coincell.o
 obj-$(CONFIG_QCOM_FASTRPC)	+= fastrpc.o
+obj-$(CONFIG_QCOM_QCA639X)	+= qcom-qca639x.o
 obj-$(CONFIG_SENSORS_BH1770)	+= bh1770glc.o
 obj-$(CONFIG_SENSORS_APDS990X)	+= apds990x.o
 obj-$(CONFIG_ENCLOSURE_SERVICES) += enclosure.o
diff --git a/drivers/misc/qcom-qca639x.c b/drivers/misc/qcom-qca639x.c
new file mode 100644
index 000000000000..b34e08ff4a88
--- /dev/null
+++ b/drivers/misc/qcom-qca639x.c
@@ -0,0 +1,164 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2020, Linaro Limited
+ */
+#include <linux/delay.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/pinctrl/consumer.h>
+#include <linux/pinctrl/devinfo.h>
+#include <linux/platform_device.h>
+#include <linux/pm_domain.h>
+#include <linux/regulator/consumer.h>
+#include <linux/slab.h>
+
+#define MAX_NUM_REGULATORS	8
+
+static struct vreg {
+	const char *name;
+	unsigned int load_uA;
+} vregs[MAX_NUM_REGULATORS] = {
+	/* 2.0 V */
+	{ "vddpcie2", 15000 },
+	{ "vddrfa3", 400000 },
+
+	/* 0.95 V */
+	{ "vddaon", 100000 },
+	{ "vddpmu", 1250000 },
+	{ "vddrfa1", 200000 },
+
+	/* 1.35 V */
+	{ "vddrfa2", 400000 },
+	{ "vddpcie1", 35000 },
+
+	/* 1.8 V */
+	{ "vddio", 20000 },
+};
+
+struct qca639x_data {
+	struct regulator_bulk_data regulators[MAX_NUM_REGULATORS];
+	size_t num_vregs;
+	struct device *dev;
+	struct pinctrl_state *active_state;
+	struct generic_pm_domain pd;
+};
+
+#define domain_to_data(domain) container_of(domain, struct qca639x_data, pd)
+
+static int qca639x_power_on(struct generic_pm_domain *domain)
+{
+	struct qca639x_data *data = domain_to_data(domain);
+	int ret;
+
+	ret = regulator_bulk_enable(data->num_vregs, data->regulators);
+	if (ret) {
+		dev_err(data->dev, "Failed to enable regulators");
+		return ret;
+	}
+
+	/* Wait for 1ms before toggling enable pins. */
+	usleep_range(1000, 2000);
+
+	ret = pinctrl_select_state(data->dev->pins->p, data->active_state);
+	if (ret) {
+		dev_err(data->dev, "Failed to select active state");
+		return ret;
+	}
+
+	/* Wait for all power levels to stabilize */
+	usleep_range(6000, 7000);
+
+	return 0;
+}
+
+static int qca639x_power_off(struct generic_pm_domain *domain)
+{
+	struct qca639x_data *data = domain_to_data(domain);
+
+	pinctrl_select_default_state(data->dev);
+	regulator_bulk_disable(data->num_vregs, data->regulators);
+
+	return 0;
+}
+
+static int qca639x_probe(struct platform_device *pdev)
+{
+	struct qca639x_data *data;
+	struct device *dev = &pdev->dev;
+	int i, ret;
+
+	if (!dev->pins || IS_ERR_OR_NULL(dev->pins->default_state))
+		return -EINVAL;
+
+	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	data->dev = dev;
+	data->num_vregs = ARRAY_SIZE(vregs);
+
+	data->active_state = pinctrl_lookup_state(dev->pins->p, "active");
+	if (IS_ERR(data->active_state)) {
+		ret = PTR_ERR(data->active_state);
+		dev_err(dev, "Failed to get active_state: %d\n", ret);
+		return ret;
+	}
+
+	for (i = 0; i < data->num_vregs; i++)
+		data->regulators[i].supply = vregs[i].name;
+	ret = devm_regulator_bulk_get(dev, data->num_vregs, data->regulators);
+	if (ret < 0)
+		return ret;
+
+	for (i = 0; i < data->num_vregs; i++) {
+		ret = regulator_set_load(data->regulators[i].consumer, vregs[i].load_uA);
+		if (ret)
+			return ret;
+	}
+
+	data->pd.name = dev_name(dev);
+	data->pd.power_on = qca639x_power_on;
+	data->pd.power_off = qca639x_power_off;
+
+	ret = pm_genpd_init(&data->pd, NULL, true);
+	if (ret < 0)
+		return ret;
+
+	ret = of_genpd_add_provider_simple(dev->of_node, &data->pd);
+	if (ret < 0) {
+		pm_genpd_remove(&data->pd);
+		return ret;
+	}
+
+	platform_set_drvdata(pdev, data);
+
+	return 0;
+}
+
+static int qca639x_remove(struct platform_device *pdev)
+{
+	struct qca639x_data *data = platform_get_drvdata(pdev);
+
+	pm_genpd_remove(&data->pd);
+
+	return 0;
+}
+
+static const struct of_device_id qca639x_of_match[] = {
+	{ .compatible = "qcom,qca6390" },
+};
+
+static struct platform_driver qca639x_driver = {
+	.probe = qca639x_probe,
+	.remove = qca639x_remove,
+	.driver = {
+		.name = "qca639x",
+		.of_match_table = qca639x_of_match,
+	},
+};
+
+module_platform_driver(qca639x_driver);
+MODULE_AUTHOR("Dmitry Baryshkov <dmitry.baryshkov@linaro.org>");
+MODULE_DESCRIPTION("Power control for Qualcomm QCA639x BT/WiFi chip");
+MODULE_LICENSE("GPL v2");
-- 
2.29.2

Re: [PATCH v2 1/5] misc: qca639x: add support for QCA639x powerup sequence

[Kalle Valo]

Dmitry Baryshkov <dmitry.baryshkov@linaro.org> writes:

> Qualcomm QCA639x is a family of WiFi + Bluetooth SoCs, with BT part
> being controlled through the UART and WiFi being present on PCIe
> bus. Both blocks share common power sources. Add device driver handling
> power sequencing of QCA6390/1.
>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>  drivers/misc/Kconfig        |  12 +++
>  drivers/misc/Makefile       |   1 +
>  drivers/misc/qcom-qca639x.c | 164 ++++++++++++++++++++++++++++++++++++
>  3 files changed, 177 insertions(+)
>  create mode 100644 drivers/misc/qcom-qca639x.c
>
> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
> index e90c2524e46c..a14f67ab476c 100644
> --- a/drivers/misc/Kconfig
> +++ b/drivers/misc/Kconfig
> @@ -255,6 +255,18 @@ config QCOM_FASTRPC
>  	  applications DSP processor. Say M if you want to enable this
>  	  module.
>  
> +config QCOM_QCA639X
> +	tristate "Qualcomm QCA639x WiFi/Bluetooth module support"
> +	depends on REGULATOR && PM_GENERIC_DOMAINS
> +	help
> +	  If you say yes to this option, support will be included for Qualcomm
> +	  QCA639x family of WiFi and Bluetooth SoCs. Note, this driver supports
> +	  only power control for this SoC, you still have to enable individual
> +	  Bluetooth and WiFi drivers.
> +
> +	  Say M here if you want to include support for QCA639x chips as a
> +	  module. This will build a module called "qcom-qca639x".

Is this is something you need on ARM platforms? As on x86 this is
definitely not needed, for example it's enough to load ath11k_pci to get
QCA6390 Wi-Fi working. I think the documentation should be clarified
where this QCOM_QCA639X is needed (and it's not needed on normal PCI
devices).

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

[PATCH v2 2/5] arm64: qcom: dts: qrb5165-rb5: add qca6391 power device

[Dmitry Baryshkov]

Add qca6391 to device tree as a way to provide power domain to WiFi and
BT parts of the chip.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 arch/arm64/boot/dts/qcom/qrb5165-rb5.dts | 61 ++++++++++++++++++++++++
 1 file changed, 61 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts b/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts
index 8aebc3660b11..2b0c1cc9333b 100644
--- a/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts
+++ b/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts
@@ -151,6 +151,23 @@ vreg_s4a_1p8: vreg-s4a-1p8 {
 		regulator-max-microvolt = <1800000>;
 		regulator-always-on;
 	};
+
+	qca6391: qca6391 {
+		compatible = "qcom,qca6390";
+		#power-domain-cells = <0>;
+
+		vddaon-supply = <&vreg_s6a_0p95>;
+		vddpmu-supply = <&vreg_s2f_0p95>;
+		vddrfa1-supply = <&vreg_s2f_0p95>;
+		vddrfa2-supply = <&vreg_s8c_1p3>;
+		vddrfa3-supply = <&vreg_s5a_1p9>;
+		vddpcie1-supply = <&vreg_s8c_1p3>;
+		vddpcie2-supply = <&vreg_s5a_1p9>;
+		vddio-supply = <&vreg_s4a_1p8>;
+		pinctrl-names = "default", "active";
+		pinctrl-0 = <&wlan_default_state &bt_default_state>;
+		pinctrl-1 = <&wlan_active_state &bt_active_state>;
+	};
 };
 
 &adsp {
@@ -1013,6 +1030,28 @@ &tlmm {
 		"HST_WLAN_UART_TX",
 		"HST_WLAN_UART_RX";
 
+	bt_default_state: bt-default-state {
+		bt-en {
+			pins = "gpio21";
+			function = "gpio";
+
+			drive-strength = <16>;
+			output-low;
+			bias-pull-up;
+		};
+	};
+
+	bt_active_state: bt-active-state {
+		bt-en {
+			pins = "gpio21";
+			function = "gpio";
+
+			drive-strength = <16>;
+			output-high;
+			bias-pull-up;
+		};
+	};
+
 	lt9611_irq_pin: lt9611-irq {
 		pins = "gpio63";
 		function = "gpio";
@@ -1119,6 +1158,28 @@ sdc2_card_det_n: sd-card-det-n {
 		function = "gpio";
 		bias-pull-up;
 	};
+
+	wlan_default_state: wlan-default-state {
+		wlan-en {
+			pins = "gpio20";
+			function = "gpio";
+
+			drive-strength = <16>;
+			output-low;
+			bias-pull-up;
+		};
+	};
+
+	wlan_active_state: wlan-active-state {
+		wlan-en {
+			pins = "gpio20";
+			function = "gpio";
+
+			drive-strength = <16>;
+			output-high;
+			bias-pull-up;
+		};
+	};
 };
 
 &uart12 {
-- 
2.29.2

Re: [PATCH v2 2/5] arm64: qcom: dts: qrb5165-rb5: add qca6391 power device

[Bjorn Andersson]

On Thu 28 Jan 11:52 CST 2021, Dmitry Baryshkov wrote:

> Add qca6391 to device tree as a way to provide power domain to WiFi and
> BT parts of the chip.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>  arch/arm64/boot/dts/qcom/qrb5165-rb5.dts | 61 ++++++++++++++++++++++++
>  1 file changed, 61 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts b/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts
> index 8aebc3660b11..2b0c1cc9333b 100644
> --- a/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts
> +++ b/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts
> @@ -151,6 +151,23 @@ vreg_s4a_1p8: vreg-s4a-1p8 {
>  		regulator-max-microvolt = <1800000>;
>  		regulator-always-on;
>  	};
> +
> +	qca6391: qca6391 {
> +		compatible = "qcom,qca6390";
> +		#power-domain-cells = <0>;
> +
> +		vddaon-supply = <&vreg_s6a_0p95>;
> +		vddpmu-supply = <&vreg_s2f_0p95>;
> +		vddrfa1-supply = <&vreg_s2f_0p95>;
> +		vddrfa2-supply = <&vreg_s8c_1p3>;
> +		vddrfa3-supply = <&vreg_s5a_1p9>;
> +		vddpcie1-supply = <&vreg_s8c_1p3>;
> +		vddpcie2-supply = <&vreg_s5a_1p9>;
> +		vddio-supply = <&vreg_s4a_1p8>;
> +		pinctrl-names = "default", "active";
> +		pinctrl-0 = <&wlan_default_state &bt_default_state>;
> +		pinctrl-1 = <&wlan_active_state &bt_active_state>;

I dislike the use of pinctrl states for toggling the level of the gpio
and would prefer that you use the gpio binding and api for this instead.

> +	};
>  };
>  
>  &adsp {
> @@ -1013,6 +1030,28 @@ &tlmm {
>  		"HST_WLAN_UART_TX",
>  		"HST_WLAN_UART_RX";
>  
> +	bt_default_state: bt-default-state {

Are you sure you need to drive the BT_EN pin in order to have WiFi
working? On QCA6174 I believe the "WL_EN" was actually RESET_N and BT_EN
was actually "blueooth enable" - so we wired that in the bluetooth node
instead.

> +		bt-en {
> +			pins = "gpio21";
> +			function = "gpio";
> +
> +			drive-strength = <16>;
> +			output-low;
> +			bias-pull-up;
> +		};
> +	};
> +
> +	bt_active_state: bt-active-state {
> +		bt-en {
> +			pins = "gpio21";
> +			function = "gpio";
> +
> +			drive-strength = <16>;
> +			output-high;
> +			bias-pull-up;
> +		};
> +	};
> +
>  	lt9611_irq_pin: lt9611-irq {
>  		pins = "gpio63";
>  		function = "gpio";
> @@ -1119,6 +1158,28 @@ sdc2_card_det_n: sd-card-det-n {
>  		function = "gpio";
>  		bias-pull-up;
>  	};
> +
> +	wlan_default_state: wlan-default-state {

JFYI. You don't need this "dummy" subnode, you can put the properties
directly in the state node.

Regards,
Bjorn

> +		wlan-en {
> +			pins = "gpio20";
> +			function = "gpio";
> +
> +			drive-strength = <16>;
> +			output-low;
> +			bias-pull-up;
> +		};
> +	};
> +
> +	wlan_active_state: wlan-active-state {
> +		wlan-en {
> +			pins = "gpio20";
> +			function = "gpio";
> +
> +			drive-strength = <16>;
> +			output-high;
> +			bias-pull-up;
> +		};
> +	};
>  };
>  
>  &uart12 {
> -- 
> 2.29.2
> 

Re: [PATCH v2 2/5] arm64: qcom: dts: qrb5165-rb5: add qca6391 power device

[Manivannan Sadhasivam]

On Fri, Jan 29, 2021 at 09:47:37PM -0600, Bjorn Andersson wrote:
> On Thu 28 Jan 11:52 CST 2021, Dmitry Baryshkov wrote:
> 
> > Add qca6391 to device tree as a way to provide power domain to WiFi and
> > BT parts of the chip.
> > 
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > ---
> >  arch/arm64/boot/dts/qcom/qrb5165-rb5.dts | 61 ++++++++++++++++++++++++
> >  1 file changed, 61 insertions(+)
> > 
> > diff --git a/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts b/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts
> > index 8aebc3660b11..2b0c1cc9333b 100644
> > --- a/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts
> > +++ b/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts
> > @@ -151,6 +151,23 @@ vreg_s4a_1p8: vreg-s4a-1p8 {
> >  		regulator-max-microvolt = <1800000>;
> >  		regulator-always-on;
> >  	};
> > +
> > +	qca6391: qca6391 {
> > +		compatible = "qcom,qca6390";
> > +		#power-domain-cells = <0>;
> > +
> > +		vddaon-supply = <&vreg_s6a_0p95>;
> > +		vddpmu-supply = <&vreg_s2f_0p95>;
> > +		vddrfa1-supply = <&vreg_s2f_0p95>;
> > +		vddrfa2-supply = <&vreg_s8c_1p3>;
> > +		vddrfa3-supply = <&vreg_s5a_1p9>;
> > +		vddpcie1-supply = <&vreg_s8c_1p3>;
> > +		vddpcie2-supply = <&vreg_s5a_1p9>;
> > +		vddio-supply = <&vreg_s4a_1p8>;
> > +		pinctrl-names = "default", "active";
> > +		pinctrl-0 = <&wlan_default_state &bt_default_state>;
> > +		pinctrl-1 = <&wlan_active_state &bt_active_state>;
> 
> I dislike the use of pinctrl states for toggling the level of the gpio
> and would prefer that you use the gpio binding and api for this instead.
> 
> > +	};
> >  };
> >  
> >  &adsp {
> > @@ -1013,6 +1030,28 @@ &tlmm {
> >  		"HST_WLAN_UART_TX",
> >  		"HST_WLAN_UART_RX";
> >  
> > +	bt_default_state: bt-default-state {
> 
> Are you sure you need to drive the BT_EN pin in order to have WiFi
> working? On QCA6174 I believe the "WL_EN" was actually RESET_N and BT_EN
> was actually "blueooth enable" - so we wired that in the bluetooth node
> instead.
> 

IIRC, that's the case. We need both BT_EN and WL_EN for proper bringup of the
chip.

Thanks,
Mani

[PATCH v2 3/5] pcie-qcom: provide a way to power up qca6390 chip on RB5 platform

[Dmitry Baryshkov]

Some Qualcomm platforms require to power up an external device before
probing the PCI bus. E.g. on RB5 platform the QCA6390 WiFi/BT chip needs
to be powered up before PCIe0 bus is probed. Add a quirk to the
respective PCIe root bridge to attach to the power domain if one is
required, so that the QCA chip is started before scanning the PCIe bus.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/pci/controller/dwc/pcie-qcom.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index ab21aa01c95d..eb73c8540d4d 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -20,6 +20,7 @@
 #include <linux/of_device.h>
 #include <linux/of_gpio.h>
 #include <linux/pci.h>
+#include <linux/pm_domain.h>
 #include <linux/pm_runtime.h>
 #include <linux/platform_device.h>
 #include <linux/phy/phy.h>
@@ -1568,6 +1569,26 @@ DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_QCOM, 0x0302, qcom_fixup_class);
 DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_QCOM, 0x1000, qcom_fixup_class);
 DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_QCOM, 0x1001, qcom_fixup_class);
 
+static void qcom_fixup_power(struct pci_dev *dev)
+{
+	int ret;
+	struct pcie_port *pp = dev->bus->sysdata;
+	struct dw_pcie *pci;
+
+	if (!pci_is_root_bus(dev->bus))
+		return;
+
+	ret = dev_pm_domain_attach(&dev->dev, true);
+	if (ret < 0 || !dev->dev.pm_domain)
+		return;
+
+	pci = to_dw_pcie_from_pp(pp);
+	dev_info(&dev->dev, "Bus powered up, waiting for link to come up\n");
+
+	dw_pcie_wait_for_link(pci);
+}
+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_QCOM, 0x010b, qcom_fixup_power);
+
 static struct platform_driver qcom_pcie_driver = {
 	.probe = qcom_pcie_probe,
 	.driver = {
-- 
2.29.2

Re: [PATCH v2 3/5] pcie-qcom: provide a way to power up qca6390 chip on RB5 platform

[Dmitry Baryshkov]

On 28/01/2021 22:26, Rob Herring wrote:
> On Thu, Jan 28, 2021 at 11:52 AM Dmitry Baryshkov
> <dmitry.baryshkov@linaro.org> wrote:
>>
>> Some Qualcomm platforms require to power up an external device before
>> probing the PCI bus. E.g. on RB5 platform the QCA6390 WiFi/BT chip needs
>> to be powered up before PCIe0 bus is probed. Add a quirk to the
>> respective PCIe root bridge to attach to the power domain if one is
>> required, so that the QCA chip is started before scanning the PCIe bus.
> 
> This is solving a generic problem in a specific driver. It needs to be
> solved for any PCI host and any device.

Ack. I see your point here.

As this would require porting code from powerpc/spark of-pci code and 
changing pcie port driver to apply power supply before bus probing 
happens, I'd also ask for the comments from PCI maintainers. Will that 
solution be acceptable to you?


> 
>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>> ---
>>   drivers/pci/controller/dwc/pcie-qcom.c | 21 +++++++++++++++++++++
>>   1 file changed, 21 insertions(+)
>>
>> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
>> index ab21aa01c95d..eb73c8540d4d 100644
>> --- a/drivers/pci/controller/dwc/pcie-qcom.c
>> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
>> @@ -20,6 +20,7 @@
>>   #include <linux/of_device.h>
>>   #include <linux/of_gpio.h>
>>   #include <linux/pci.h>
>> +#include <linux/pm_domain.h>
>>   #include <linux/pm_runtime.h>
>>   #include <linux/platform_device.h>
>>   #include <linux/phy/phy.h>
>> @@ -1568,6 +1569,26 @@ DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_QCOM, 0x0302, qcom_fixup_class);
>>   DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_QCOM, 0x1000, qcom_fixup_class);
>>   DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_QCOM, 0x1001, qcom_fixup_class);
>>
>> +static void qcom_fixup_power(struct pci_dev *dev)
>> +{
>> +       int ret;
>> +       struct pcie_port *pp = dev->bus->sysdata;
>> +       struct dw_pcie *pci;
>> +
>> +       if (!pci_is_root_bus(dev->bus))
>> +               return;
>> +
>> +       ret = dev_pm_domain_attach(&dev->dev, true);
>> +       if (ret < 0 || !dev->dev.pm_domain)
>> +               return;
>> +
>> +       pci = to_dw_pcie_from_pp(pp);
>> +       dev_info(&dev->dev, "Bus powered up, waiting for link to come up\n");
>> +
>> +       dw_pcie_wait_for_link(pci);
>> +}
>> +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_QCOM, 0x010b, qcom_fixup_power);
>> +
>>   static struct platform_driver qcom_pcie_driver = {
>>          .probe = qcom_pcie_probe,
>>          .driver = {
>> --
>> 2.29.2
>>


-- 
With best wishes
Dmitry

Re: [PATCH v2 3/5] pcie-qcom: provide a way to power up qca6390 chip on RB5 platform

[Bjorn Helgaas]

On Fri, Jan 29, 2021 at 06:45:21AM +0300, Dmitry Baryshkov wrote:
> On 28/01/2021 22:26, Rob Herring wrote:
> > On Thu, Jan 28, 2021 at 11:52 AM Dmitry Baryshkov
> > <dmitry.baryshkov@linaro.org> wrote:
> > > 
> > > Some Qualcomm platforms require to power up an external device before
> > > probing the PCI bus. E.g. on RB5 platform the QCA6390 WiFi/BT chip needs
> > > to be powered up before PCIe0 bus is probed. Add a quirk to the
> > > respective PCIe root bridge to attach to the power domain if one is
> > > required, so that the QCA chip is started before scanning the PCIe bus.
> > 
> > This is solving a generic problem in a specific driver. It needs to be
> > solved for any PCI host and any device.
> 
> Ack. I see your point here.
> 
> As this would require porting code from powerpc/spark of-pci code and
> changing pcie port driver to apply power supply before bus probing happens,
> I'd also ask for the comments from PCI maintainers. Will that solution be
> acceptable to you?

I can't say without seeing the code.  I don't know enough about this
scenario to envision how it might look.

I guess the QCA6390 is a PCIe device?  Why does it need to be powered
up before probing?  Shouldn't we get a link-up interrupt when it is
powered up so we could probe it then?

Nit: when changing any file, please take a look at the commit history
and make yours match, e.g.,

  pcie-qcom: provide a way to power up qca6390 chip on RB5 platform

does not look like:

  PCI: qcom: Add support for configuring BDF to SID mapping for SM8250
  PCI: qcom: Add SM8250 SoC support
  PCI: qcom: Make sure PCIe is reset before init for rev 2.1.0
  PCI: qcom: Replace define with standard value
  PCI: qcom: Support pci speed set for ipq806x
  PCI: qcom: Add ipq8064 rev2 variant

Also, if you capitalize it as "QCA6390" in the commit log, do it the
same in the subject.

> > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > > ---
> > >   drivers/pci/controller/dwc/pcie-qcom.c | 21 +++++++++++++++++++++
> > >   1 file changed, 21 insertions(+)
> > > 
> > > diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> > > index ab21aa01c95d..eb73c8540d4d 100644
> > > --- a/drivers/pci/controller/dwc/pcie-qcom.c
> > > +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> > > @@ -20,6 +20,7 @@
> > >   #include <linux/of_device.h>
> > >   #include <linux/of_gpio.h>
> > >   #include <linux/pci.h>
> > > +#include <linux/pm_domain.h>
> > >   #include <linux/pm_runtime.h>
> > >   #include <linux/platform_device.h>
> > >   #include <linux/phy/phy.h>
> > > @@ -1568,6 +1569,26 @@ DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_QCOM, 0x0302, qcom_fixup_class);
> > >   DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_QCOM, 0x1000, qcom_fixup_class);
> > >   DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_QCOM, 0x1001, qcom_fixup_class);
> > > 
> > > +static void qcom_fixup_power(struct pci_dev *dev)
> > > +{
> > > +       int ret;
> > > +       struct pcie_port *pp = dev->bus->sysdata;
> > > +       struct dw_pcie *pci;
> > > +
> > > +       if (!pci_is_root_bus(dev->bus))
> > > +               return;
> > > +
> > > +       ret = dev_pm_domain_attach(&dev->dev, true);
> > > +       if (ret < 0 || !dev->dev.pm_domain)
> > > +               return;
> > > +
> > > +       pci = to_dw_pcie_from_pp(pp);
> > > +       dev_info(&dev->dev, "Bus powered up, waiting for link to come up\n");
> > > +
> > > +       dw_pcie_wait_for_link(pci);
> > > +}
> > > +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_QCOM, 0x010b, qcom_fixup_power);
> > > +
> > >   static struct platform_driver qcom_pcie_driver = {
> > >          .probe = qcom_pcie_probe,
> > >          .driver = {
> > > --
> > > 2.29.2
> > > 
> 
> 
> -- 
> With best wishes
> Dmitry

Re: [PATCH v2 3/5] pcie-qcom: provide a way to power up qca6390 chip on RB5 platform

[Dmitry Baryshkov]

On Sat, 30 Jan 2021 at 00:50, Bjorn Helgaas <helgaas@kernel.org> wrote:
>
> On Fri, Jan 29, 2021 at 06:45:21AM +0300, Dmitry Baryshkov wrote:
> > On 28/01/2021 22:26, Rob Herring wrote:
> > > On Thu, Jan 28, 2021 at 11:52 AM Dmitry Baryshkov
> > > <dmitry.baryshkov@linaro.org> wrote:
> > > >
> > > > Some Qualcomm platforms require to power up an external device before
> > > > probing the PCI bus. E.g. on RB5 platform the QCA6390 WiFi/BT chip needs
> > > > to be powered up before PCIe0 bus is probed. Add a quirk to the
> > > > respective PCIe root bridge to attach to the power domain if one is
> > > > required, so that the QCA chip is started before scanning the PCIe bus.
> > >
> > > This is solving a generic problem in a specific driver. It needs to be
> > > solved for any PCI host and any device.
> >
> > Ack. I see your point here.
> >
> > As this would require porting code from powerpc/spark of-pci code and
> > changing pcie port driver to apply power supply before bus probing happens,
> > I'd also ask for the comments from PCI maintainers. Will that solution be
> > acceptable to you?
>
> I can't say without seeing the code.  I don't know enough about this
> scenario to envision how it might look.
>
> I guess the QCA6390 is a PCIe device?  Why does it need to be powered
> up before probing?  Shouldn't we get a link-up interrupt when it is
> powered up so we could probe it then?

Not quite. QCA6390 is a multifunction device, with PCIe and serial
parts. It has internal power regulators which once enabled will
powerup the PCIe, serial and radio parts. There is no need to manage
regulators. Once enabled they will automatically handle device
suspend/resume, etc.

I'm not sure about link-up interrupt. I've just lightly tested using
PCIe HP on this port, getting no interrupts from it.
If I manually rescan the bus after enabling the qca6390 device (e.g.
via sysfs), it gets enabled, but then I see PCIe link errors (most
probably because the PCIe link was not retrained after the device
comes up).

> Nit: when changing any file, please take a look at the commit history
> and make yours match, e.g.,
>
>   pcie-qcom: provide a way to power up qca6390 chip on RB5 platform
>
> does not look like:

Ack.


-- 
With best wishes
Dmitry

Re: [PATCH v2 3/5] pcie-qcom: provide a way to power up qca6390 chip on RB5 platform

[Bjorn Helgaas]

On Sat, Jan 30, 2021 at 01:19:10AM +0300, Dmitry Baryshkov wrote:
> On Sat, 30 Jan 2021 at 00:50, Bjorn Helgaas <helgaas@kernel.org> wrote:
> > On Fri, Jan 29, 2021 at 06:45:21AM +0300, Dmitry Baryshkov wrote:
> > > On 28/01/2021 22:26, Rob Herring wrote:
> > > > On Thu, Jan 28, 2021 at 11:52 AM Dmitry Baryshkov
> > > > <dmitry.baryshkov@linaro.org> wrote:
> > > > >
> > > > > Some Qualcomm platforms require to power up an external device before
> > > > > probing the PCI bus. E.g. on RB5 platform the QCA6390 WiFi/BT chip needs
> > > > > to be powered up before PCIe0 bus is probed. Add a quirk to the
> > > > > respective PCIe root bridge to attach to the power domain if one is
> > > > > required, so that the QCA chip is started before scanning the PCIe bus.
> > > >
> > > > This is solving a generic problem in a specific driver. It needs to be
> > > > solved for any PCI host and any device.
> > >
> > > Ack. I see your point here.
> > >
> > > As this would require porting code from powerpc/spark of-pci code and
> > > changing pcie port driver to apply power supply before bus probing happens,
> > > I'd also ask for the comments from PCI maintainers. Will that solution be
> > > acceptable to you?
> >
> > I can't say without seeing the code.  I don't know enough about this
> > scenario to envision how it might look.
> >
> > I guess the QCA6390 is a PCIe device?  Why does it need to be powered
> > up before probing?  Shouldn't we get a link-up interrupt when it is
> > powered up so we could probe it then?
> 
> Not quite. QCA6390 is a multifunction device, with PCIe and serial
> parts. It has internal power regulators which once enabled will
> powerup the PCIe, serial and radio parts. There is no need to manage
> regulators. Once enabled they will automatically handle device
> suspend/resume, etc.

OK, just elaborate on this a bit in the commit log so the topology
makes sense to someone with no prior knowledge of QCA6390.

Bjorn

Re: [PATCH v2 3/5] pcie-qcom: provide a way to power up qca6390 chip on RB5 platform

[Bjorn Andersson]

On Fri 29 Jan 16:19 CST 2021, Dmitry Baryshkov wrote:

> On Sat, 30 Jan 2021 at 00:50, Bjorn Helgaas <helgaas@kernel.org> wrote:
> >
> > On Fri, Jan 29, 2021 at 06:45:21AM +0300, Dmitry Baryshkov wrote:
> > > On 28/01/2021 22:26, Rob Herring wrote:
> > > > On Thu, Jan 28, 2021 at 11:52 AM Dmitry Baryshkov
> > > > <dmitry.baryshkov@linaro.org> wrote:
> > > > >
> > > > > Some Qualcomm platforms require to power up an external device before
> > > > > probing the PCI bus. E.g. on RB5 platform the QCA6390 WiFi/BT chip needs
> > > > > to be powered up before PCIe0 bus is probed. Add a quirk to the
> > > > > respective PCIe root bridge to attach to the power domain if one is
> > > > > required, so that the QCA chip is started before scanning the PCIe bus.
> > > >
> > > > This is solving a generic problem in a specific driver. It needs to be
> > > > solved for any PCI host and any device.
> > >
> > > Ack. I see your point here.
> > >
> > > As this would require porting code from powerpc/spark of-pci code and
> > > changing pcie port driver to apply power supply before bus probing happens,
> > > I'd also ask for the comments from PCI maintainers. Will that solution be
> > > acceptable to you?
> >
> > I can't say without seeing the code.  I don't know enough about this
> > scenario to envision how it might look.
> >
> > I guess the QCA6390 is a PCIe device?  Why does it need to be powered
> > up before probing?  Shouldn't we get a link-up interrupt when it is
> > powered up so we could probe it then?
> 
> Not quite. QCA6390 is a multifunction device, with PCIe and serial
> parts. It has internal power regulators which once enabled will
> powerup the PCIe, serial and radio parts. There is no need to manage
> regulators. Once enabled they will automatically handle device
> suspend/resume, etc.
> 

So what you're saying is that if either the PCI controller or bluetooth
driver probes these regulators will be turned on, indefinitely?

If so, why do we need a driver to turn them on, rather than just mark
them as always-on?

What's the timing requirement wrt regulators vs WL_EN/BT_EN?

Regards,
Bjorn A

> I'm not sure about link-up interrupt. I've just lightly tested using
> PCIe HP on this port, getting no interrupts from it.
> If I manually rescan the bus after enabling the qca6390 device (e.g.
> via sysfs), it gets enabled, but then I see PCIe link errors (most
> probably because the PCIe link was not retrained after the device
> comes up).
> 
> > Nit: when changing any file, please take a look at the commit history
> > and make yours match, e.g.,
> >
> >   pcie-qcom: provide a way to power up qca6390 chip on RB5 platform
> >
> > does not look like:
> 
> Ack.
> 
> 
> -- 
> With best wishes
> Dmitry

Re: [PATCH v2 3/5] pcie-qcom: provide a way to power up qca6390 chip on RB5 platform

[Dmitry Baryshkov]

On Sat, 30 Jan 2021 at 06:53, Bjorn Andersson
<bjorn.andersson@linaro.org> wrote:
>
> On Fri 29 Jan 16:19 CST 2021, Dmitry Baryshkov wrote:
>
> > On Sat, 30 Jan 2021 at 00:50, Bjorn Helgaas <helgaas@kernel.org> wrote:
> > >
> > > On Fri, Jan 29, 2021 at 06:45:21AM +0300, Dmitry Baryshkov wrote:
> > > > On 28/01/2021 22:26, Rob Herring wrote:
> > > > > On Thu, Jan 28, 2021 at 11:52 AM Dmitry Baryshkov
> > > > > <dmitry.baryshkov@linaro.org> wrote:
> > > > > >
> > > > > > Some Qualcomm platforms require to power up an external device before
> > > > > > probing the PCI bus. E.g. on RB5 platform the QCA6390 WiFi/BT chip needs
> > > > > > to be powered up before PCIe0 bus is probed. Add a quirk to the
> > > > > > respective PCIe root bridge to attach to the power domain if one is
> > > > > > required, so that the QCA chip is started before scanning the PCIe bus.
> > > > >
> > > > > This is solving a generic problem in a specific driver. It needs to be
> > > > > solved for any PCI host and any device.
> > > >
> > > > Ack. I see your point here.
> > > >
> > > > As this would require porting code from powerpc/spark of-pci code and
> > > > changing pcie port driver to apply power supply before bus probing happens,
> > > > I'd also ask for the comments from PCI maintainers. Will that solution be
> > > > acceptable to you?
> > >
> > > I can't say without seeing the code.  I don't know enough about this
> > > scenario to envision how it might look.
> > >
> > > I guess the QCA6390 is a PCIe device?  Why does it need to be powered
> > > up before probing?  Shouldn't we get a link-up interrupt when it is
> > > powered up so we could probe it then?
> >
> > Not quite. QCA6390 is a multifunction device, with PCIe and serial
> > parts. It has internal power regulators which once enabled will
> > powerup the PCIe, serial and radio parts. There is no need to manage
> > regulators. Once enabled they will automatically handle device
> > suspend/resume, etc.
> >
>
> So what you're saying is that if either the PCI controller or bluetooth
> driver probes these regulators will be turned on, indefinitely?
>
> If so, why do we need a driver to turn them on, rather than just mark
> them as always-on?
>
> What's the timing requirement wrt regulators vs WL_EN/BT_EN?

According to the documentation I have, they must be enabled right
after enabling powering the chip and they must stay enabled all the
time.




-- 
With best wishes
Dmitry

Re: [PATCH v2 3/5] pcie-qcom: provide a way to power up qca6390 chip on RB5 platform

[Bjorn Andersson]

On Sat 30 Jan 10:14 CST 2021, Dmitry Baryshkov wrote:

> On Sat, 30 Jan 2021 at 06:53, Bjorn Andersson
> <bjorn.andersson@linaro.org> wrote:
> >
> > On Fri 29 Jan 16:19 CST 2021, Dmitry Baryshkov wrote:
> >
> > > On Sat, 30 Jan 2021 at 00:50, Bjorn Helgaas <helgaas@kernel.org> wrote:
> > > >
> > > > On Fri, Jan 29, 2021 at 06:45:21AM +0300, Dmitry Baryshkov wrote:
> > > > > On 28/01/2021 22:26, Rob Herring wrote:
> > > > > > On Thu, Jan 28, 2021 at 11:52 AM Dmitry Baryshkov
> > > > > > <dmitry.baryshkov@linaro.org> wrote:
> > > > > > >
> > > > > > > Some Qualcomm platforms require to power up an external device before
> > > > > > > probing the PCI bus. E.g. on RB5 platform the QCA6390 WiFi/BT chip needs
> > > > > > > to be powered up before PCIe0 bus is probed. Add a quirk to the
> > > > > > > respective PCIe root bridge to attach to the power domain if one is
> > > > > > > required, so that the QCA chip is started before scanning the PCIe bus.
> > > > > >
> > > > > > This is solving a generic problem in a specific driver. It needs to be
> > > > > > solved for any PCI host and any device.
> > > > >
> > > > > Ack. I see your point here.
> > > > >
> > > > > As this would require porting code from powerpc/spark of-pci code and
> > > > > changing pcie port driver to apply power supply before bus probing happens,
> > > > > I'd also ask for the comments from PCI maintainers. Will that solution be
> > > > > acceptable to you?
> > > >
> > > > I can't say without seeing the code.  I don't know enough about this
> > > > scenario to envision how it might look.
> > > >
> > > > I guess the QCA6390 is a PCIe device?  Why does it need to be powered
> > > > up before probing?  Shouldn't we get a link-up interrupt when it is
> > > > powered up so we could probe it then?
> > >
> > > Not quite. QCA6390 is a multifunction device, with PCIe and serial
> > > parts. It has internal power regulators which once enabled will
> > > powerup the PCIe, serial and radio parts. There is no need to manage
> > > regulators. Once enabled they will automatically handle device
> > > suspend/resume, etc.
> > >
> >
> > So what you're saying is that if either the PCI controller or bluetooth
> > driver probes these regulators will be turned on, indefinitely?
> >
> > If so, why do we need a driver to turn them on, rather than just mark
> > them as always-on?
> >
> > What's the timing requirement wrt regulators vs WL_EN/BT_EN?
> 
> According to the documentation I have, they must be enabled right
> after enabling powering the chip and they must stay enabled all the
> time.
> 

So presumably just marking these things always-on and flipping the GPIO
statically won't be good enough due to the lack of control over the
timing.

This really do look like a simplified case of what we see with the
PCIe attached modems, where similar requirements are provided, but also
the ability to perform a device specific reset sequence in case the
hardware has locked up. I'm slightly worried about the ability of
extending your power-domain model to handle the restart operation
though.

Regards,
Bjorn

Re: [PATCH v2 3/5] pcie-qcom: provide a way to power up qca6390 chip on RB5 platform

[Rob Herring]

On Tue, Feb 2, 2021 at 1:48 PM Bjorn Andersson
<bjorn.andersson@linaro.org> wrote:
>
> On Sat 30 Jan 10:14 CST 2021, Dmitry Baryshkov wrote:
>
> > On Sat, 30 Jan 2021 at 06:53, Bjorn Andersson
> > <bjorn.andersson@linaro.org> wrote:
> > >
> > > On Fri 29 Jan 16:19 CST 2021, Dmitry Baryshkov wrote:
> > >
> > > > On Sat, 30 Jan 2021 at 00:50, Bjorn Helgaas <helgaas@kernel.org> wrote:
> > > > >
> > > > > On Fri, Jan 29, 2021 at 06:45:21AM +0300, Dmitry Baryshkov wrote:
> > > > > > On 28/01/2021 22:26, Rob Herring wrote:
> > > > > > > On Thu, Jan 28, 2021 at 11:52 AM Dmitry Baryshkov
> > > > > > > <dmitry.baryshkov@linaro.org> wrote:
> > > > > > > >
> > > > > > > > Some Qualcomm platforms require to power up an external device before
> > > > > > > > probing the PCI bus. E.g. on RB5 platform the QCA6390 WiFi/BT chip needs
> > > > > > > > to be powered up before PCIe0 bus is probed. Add a quirk to the
> > > > > > > > respective PCIe root bridge to attach to the power domain if one is
> > > > > > > > required, so that the QCA chip is started before scanning the PCIe bus.
> > > > > > >
> > > > > > > This is solving a generic problem in a specific driver. It needs to be
> > > > > > > solved for any PCI host and any device.
> > > > > >
> > > > > > Ack. I see your point here.
> > > > > >
> > > > > > As this would require porting code from powerpc/spark of-pci code and
> > > > > > changing pcie port driver to apply power supply before bus probing happens,
> > > > > > I'd also ask for the comments from PCI maintainers. Will that solution be
> > > > > > acceptable to you?
> > > > >
> > > > > I can't say without seeing the code.  I don't know enough about this
> > > > > scenario to envision how it might look.
> > > > >
> > > > > I guess the QCA6390 is a PCIe device?  Why does it need to be powered
> > > > > up before probing?  Shouldn't we get a link-up interrupt when it is
> > > > > powered up so we could probe it then?
> > > >
> > > > Not quite. QCA6390 is a multifunction device, with PCIe and serial
> > > > parts. It has internal power regulators which once enabled will
> > > > powerup the PCIe, serial and radio parts. There is no need to manage
> > > > regulators. Once enabled they will automatically handle device
> > > > suspend/resume, etc.
> > > >
> > >
> > > So what you're saying is that if either the PCI controller or bluetooth
> > > driver probes these regulators will be turned on, indefinitely?
> > >
> > > If so, why do we need a driver to turn them on, rather than just mark
> > > them as always-on?
> > >
> > > What's the timing requirement wrt regulators vs WL_EN/BT_EN?
> >
> > According to the documentation I have, they must be enabled right
> > after enabling powering the chip and they must stay enabled all the
> > time.
> >
>
> So presumably just marking these things always-on and flipping the GPIO
> statically won't be good enough due to the lack of control over the
> timing.
>
> This really do look like a simplified case of what we see with the
> PCIe attached modems, where similar requirements are provided, but also
> the ability to perform a device specific reset sequence in case the
> hardware has locked up. I'm slightly worried about the ability of
> extending your power-domain model to handle the restart operation
> though.

I think this is an abuse of 'power-domains'. Just define the
regulators in both WiFi and BT nodes and have each driver enable them.
They're refcounted. If that's still not enough control over the power
sequencing, then create a 3rd entity to do it, but that doesn't need
to leak into DT. You already have all the information you need.

Rob

Re: [PATCH v2 3/5] pcie-qcom: provide a way to power up qca6390 chip on RB5 platform

[Bjorn Andersson]

On Tue 02 Feb 15:37 CST 2021, Rob Herring wrote:

> On Tue, Feb 2, 2021 at 1:48 PM Bjorn Andersson
> <bjorn.andersson@linaro.org> wrote:
> >
> > On Sat 30 Jan 10:14 CST 2021, Dmitry Baryshkov wrote:
> >
> > > On Sat, 30 Jan 2021 at 06:53, Bjorn Andersson
> > > <bjorn.andersson@linaro.org> wrote:
> > > >
> > > > On Fri 29 Jan 16:19 CST 2021, Dmitry Baryshkov wrote:
> > > >
> > > > > On Sat, 30 Jan 2021 at 00:50, Bjorn Helgaas <helgaas@kernel.org> wrote:
> > > > > >
> > > > > > On Fri, Jan 29, 2021 at 06:45:21AM +0300, Dmitry Baryshkov wrote:
> > > > > > > On 28/01/2021 22:26, Rob Herring wrote:
> > > > > > > > On Thu, Jan 28, 2021 at 11:52 AM Dmitry Baryshkov
> > > > > > > > <dmitry.baryshkov@linaro.org> wrote:
> > > > > > > > >
> > > > > > > > > Some Qualcomm platforms require to power up an external device before
> > > > > > > > > probing the PCI bus. E.g. on RB5 platform the QCA6390 WiFi/BT chip needs
> > > > > > > > > to be powered up before PCIe0 bus is probed. Add a quirk to the
> > > > > > > > > respective PCIe root bridge to attach to the power domain if one is
> > > > > > > > > required, so that the QCA chip is started before scanning the PCIe bus.
> > > > > > > >
> > > > > > > > This is solving a generic problem in a specific driver. It needs to be
> > > > > > > > solved for any PCI host and any device.
> > > > > > >
> > > > > > > Ack. I see your point here.
> > > > > > >
> > > > > > > As this would require porting code from powerpc/spark of-pci code and
> > > > > > > changing pcie port driver to apply power supply before bus probing happens,
> > > > > > > I'd also ask for the comments from PCI maintainers. Will that solution be
> > > > > > > acceptable to you?
> > > > > >
> > > > > > I can't say without seeing the code.  I don't know enough about this
> > > > > > scenario to envision how it might look.
> > > > > >
> > > > > > I guess the QCA6390 is a PCIe device?  Why does it need to be powered
> > > > > > up before probing?  Shouldn't we get a link-up interrupt when it is
> > > > > > powered up so we could probe it then?
> > > > >
> > > > > Not quite. QCA6390 is a multifunction device, with PCIe and serial
> > > > > parts. It has internal power regulators which once enabled will
> > > > > powerup the PCIe, serial and radio parts. There is no need to manage
> > > > > regulators. Once enabled they will automatically handle device
> > > > > suspend/resume, etc.
> > > > >
> > > >
> > > > So what you're saying is that if either the PCI controller or bluetooth
> > > > driver probes these regulators will be turned on, indefinitely?
> > > >
> > > > If so, why do we need a driver to turn them on, rather than just mark
> > > > them as always-on?
> > > >
> > > > What's the timing requirement wrt regulators vs WL_EN/BT_EN?
> > >
> > > According to the documentation I have, they must be enabled right
> > > after enabling powering the chip and they must stay enabled all the
> > > time.
> > >
> >
> > So presumably just marking these things always-on and flipping the GPIO
> > statically won't be good enough due to the lack of control over the
> > timing.
> >
> > This really do look like a simplified case of what we see with the
> > PCIe attached modems, where similar requirements are provided, but also
> > the ability to perform a device specific reset sequence in case the
> > hardware has locked up. I'm slightly worried about the ability of
> > extending your power-domain model to handle the restart operation
> > though.
> 
> I think this is an abuse of 'power-domains'. Just define the
> regulators in both WiFi and BT nodes and have each driver enable them.
> They're refcounted. If that's still not enough control over the power
> sequencing, then create a 3rd entity to do it, but that doesn't need
> to leak into DT. You already have all the information you need.
> 

As Dmitry explained he still need to pull the two GPIOs high after
enabling the regulators, but before scanning the PCI or serdev buses.

I was thinking something along the lines you suggest, but I've not been
able to come up with something that will guarantee the ordering of the
events.

Regards,
Bjorn

Re: [PATCH v2 3/5] pcie-qcom: provide a way to power up qca6390 chip on RB5 platform

[Dmitry Baryshkov]

On Wed, 3 Feb 2021 at 00:37, Rob Herring <robh+dt@kernel.org> wrote:
>
> On Tue, Feb 2, 2021 at 1:48 PM Bjorn Andersson
> <bjorn.andersson@linaro.org> wrote:
> >
> > On Sat 30 Jan 10:14 CST 2021, Dmitry Baryshkov wrote:
> >
> > > On Sat, 30 Jan 2021 at 06:53, Bjorn Andersson
> > > <bjorn.andersson@linaro.org> wrote:
> > > >
> > > > On Fri 29 Jan 16:19 CST 2021, Dmitry Baryshkov wrote:
> > > >
> > > > > On Sat, 30 Jan 2021 at 00:50, Bjorn Helgaas <helgaas@kernel.org> wrote:
> > > > > >
> > > > > > On Fri, Jan 29, 2021 at 06:45:21AM +0300, Dmitry Baryshkov wrote:
> > > > > > > On 28/01/2021 22:26, Rob Herring wrote:
> > > > > > > > On Thu, Jan 28, 2021 at 11:52 AM Dmitry Baryshkov
> > > > > > > > <dmitry.baryshkov@linaro.org> wrote:
> > > > > > > > >
> > > > > > > > > Some Qualcomm platforms require to power up an external device before
> > > > > > > > > probing the PCI bus. E.g. on RB5 platform the QCA6390 WiFi/BT chip needs
> > > > > > > > > to be powered up before PCIe0 bus is probed. Add a quirk to the
> > > > > > > > > respective PCIe root bridge to attach to the power domain if one is
> > > > > > > > > required, so that the QCA chip is started before scanning the PCIe bus.
> > > > > > > >
> > > > > > > > This is solving a generic problem in a specific driver. It needs to be
> > > > > > > > solved for any PCI host and any device.
> > > > > > >
> > > > > > > Ack. I see your point here.
> > > > > > >
> > > > > > > As this would require porting code from powerpc/spark of-pci code and
> > > > > > > changing pcie port driver to apply power supply before bus probing happens,
> > > > > > > I'd also ask for the comments from PCI maintainers. Will that solution be
> > > > > > > acceptable to you?
> > > > > >
> > > > > > I can't say without seeing the code.  I don't know enough about this
> > > > > > scenario to envision how it might look.
> > > > > >
> > > > > > I guess the QCA6390 is a PCIe device?  Why does it need to be powered
> > > > > > up before probing?  Shouldn't we get a link-up interrupt when it is
> > > > > > powered up so we could probe it then?
> > > > >
> > > > > Not quite. QCA6390 is a multifunction device, with PCIe and serial
> > > > > parts. It has internal power regulators which once enabled will
> > > > > powerup the PCIe, serial and radio parts. There is no need to manage
> > > > > regulators. Once enabled they will automatically handle device
> > > > > suspend/resume, etc.
> > > > >
> > > >
> > > > So what you're saying is that if either the PCI controller or bluetooth
> > > > driver probes these regulators will be turned on, indefinitely?
> > > >
> > > > If so, why do we need a driver to turn them on, rather than just mark
> > > > them as always-on?
> > > >
> > > > What's the timing requirement wrt regulators vs WL_EN/BT_EN?
> > >
> > > According to the documentation I have, they must be enabled right
> > > after enabling powering the chip and they must stay enabled all the
> > > time.
> > >
> >
> > So presumably just marking these things always-on and flipping the GPIO
> > statically won't be good enough due to the lack of control over the
> > timing.
> >
> > This really do look like a simplified case of what we see with the
> > PCIe attached modems, where similar requirements are provided, but also
> > the ability to perform a device specific reset sequence in case the
> > hardware has locked up. I'm slightly worried about the ability of
> > extending your power-domain model to handle the restart operation
> > though.
>
> I think this is an abuse of 'power-domains'. Just define the
> regulators in both WiFi and BT nodes and have each driver enable them.

I think it is too late to enable regulators in the WiFi driver. Even
if I modify the/pci code to register devices basing on the OF nodes
(like we do for PPC and Sparc), necessary link training/hotplug
handling should happen outside of the WiFi driver.

> They're refcounted. If that's still not enough control over the power
> sequencing, then create a 3rd entity to do it, but that doesn't need
> to leak into DT. You already have all the information you need.

From my point of view the proposed design (with three nodes) exactly
represents the hardware: the power-handling part, the WiFi part and
the BT part. If you don't like the power domains, would regulators be
better from your point of view? The "power" device providing a
regulator to be used by the child nodes. For the BT part the regulator
is fine, while for the WiFi...

The major problem with regulators in this case is that they are
typically enabled by the device driver itself, rather than by parent
device/bus code. And in the WiFi driver case the WiFi chip should be
already up and running before probing the ath11k driver.

Maybe it would still be better to take a step back and just introduce
'vcc-child-supply' entry to the pcie-qcom device tree node or to the
PCIe bridge node? This would also cover cases of PCIe mezzanine boards
when the on-mezzanine devices are visible through the PCIe bus, but
the mezzanine is powered by a separate voltage regulator. It would not
be possible to describe child devices in the device tree node, but
rather it would be possible to describe that there might be devices
behind the PCIe bridge, they must be powered on using a referenced
regulator. This starts to sound like a kind of PCI hotplug.

This would result in the following nodes:

pcie0: pci@1c0000 {
   compatible = "qcom,pcie-sm8250";
   [....]
   bridge@0,0 {
        compatible = "pci17cb,010b", "linux,regulator-hotplug";
        [....]
        vcc-children-supply = <&qca6390>;
        /* known WiFi card */
   };
};

pce1: pci@1c08000 {
   compatible = "qcom,pcie-sm8250";
   [....]
   bridge@1,0 {
        compatible = "pci17cb,010b", "linux,regulator-hotplug";
        [....]
        vcc-children-supply = <&vdc_gpio13>;
        /* unpredictable devices on the mezzanine card */
   };
};

-- 
With best wishes
Dmitry

Re: [PATCH v2 3/5] pcie-qcom: provide a way to power up qca6390 chip on RB5 platform

[Rob Herring]

On Thu, Jan 28, 2021 at 9:45 PM Dmitry Baryshkov
<dmitry.baryshkov@linaro.org> wrote:
>
> On 28/01/2021 22:26, Rob Herring wrote:
> > On Thu, Jan 28, 2021 at 11:52 AM Dmitry Baryshkov
> > <dmitry.baryshkov@linaro.org> wrote:
> >>
> >> Some Qualcomm platforms require to power up an external device before
> >> probing the PCI bus. E.g. on RB5 platform the QCA6390 WiFi/BT chip needs
> >> to be powered up before PCIe0 bus is probed. Add a quirk to the
> >> respective PCIe root bridge to attach to the power domain if one is
> >> required, so that the QCA chip is started before scanning the PCIe bus.
> >
> > This is solving a generic problem in a specific driver. It needs to be
> > solved for any PCI host and any device.
>
> Ack. I see your point here.
>
> As this would require porting code from powerpc/spark of-pci code and
> changing pcie port driver to apply power supply before bus probing
> happens, I'd also ask for the comments from PCI maintainers. Will that
> solution be acceptable to you?

Oh good, something exists. :)

FYI, there's another similar case needing this that just popped up[1].

Rob

[1] https://lore.kernel.org/linux-pci/20210129173057.30288c9d@coco.lan/

Re: [PATCH v2 3/5] pcie-qcom: provide a way to power up qca6390 chip on RB5 platform

[Konrad Dybcio]

Hi, bumping the discussion as solving this is crucial for Wi-Fi and Bluetooth to work on a good number (which will probably increase with time) of Qualcomm devices, including newer phones.


Konrad

[PATCH v2 4/5] arm64: dtb: qcom: qrb5165-rb5: add bridge@0,0 to power up qca6391 chip

[Dmitry Baryshkov]

If QCA6391 chip (connected to PCIe0) is not powered at the PCIe probe
time, PCIe0 bus probe will timeout and the device will not be detected.
So use qca6391 as pcie0's bridge power-domain.  This allows us to make
sure that QCA6391 chip is powered on before PCIe0 probe happens.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 arch/arm64/boot/dts/qcom/qrb5165-rb5.dts | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts b/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts
index 2b0c1cc9333b..b39a9729395f 100644
--- a/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts
+++ b/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts
@@ -581,6 +581,18 @@ &pcie0 {
 	wake-gpio = <&tlmm 81 GPIO_ACTIVE_HIGH>;
 	pinctrl-names = "default";
 	pinctrl-0 = <&pcie0_default_state>;
+
+	bridge@0,0 {
+		compatible = "pci17cb,010b";
+                reg = <0 0 0 0 0>;
+
+                #address-cells = <3>;
+                #size-cells = <2>;
+                #interrupt-cells = <1>;
+
+		/* Power on QCA639x chip sitting behind this bridge. */
+		power-domains = <&qca6391>;
+	};
 };
 
 &pcie0_phy {
-- 
2.29.2

Re: [PATCH v2 4/5] arm64: dtb: qcom: qrb5165-rb5: add bridge@0,0 to power up qca6391 chip

[Dmitry Baryshkov]

On 28/01/2021 22:21, Rob Herring wrote:
> On Thu, Jan 28, 2021 at 11:52 AM Dmitry Baryshkov
> <dmitry.baryshkov@linaro.org> wrote:
>>
>> If QCA6391 chip (connected to PCIe0) is not powered at the PCIe probe
>> time, PCIe0 bus probe will timeout and the device will not be detected.
>> So use qca6391 as pcie0's bridge power-domain.  This allows us to make
>> sure that QCA6391 chip is powered on before PCIe0 probe happens.
>>
>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>> ---
>>   arch/arm64/boot/dts/qcom/qrb5165-rb5.dts | 12 ++++++++++++
>>   1 file changed, 12 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts b/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts
>> index 2b0c1cc9333b..b39a9729395f 100644
>> --- a/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts
>> +++ b/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts
>> @@ -581,6 +581,18 @@ &pcie0 {
>>          wake-gpio = <&tlmm 81 GPIO_ACTIVE_HIGH>;
>>          pinctrl-names = "default";
>>          pinctrl-0 = <&pcie0_default_state>;
>> +
>> +       bridge@0,0 {
>> +               compatible = "pci17cb,010b";
>> +                reg = <0 0 0 0 0>;
>> +
>> +                #address-cells = <3>;
>> +                #size-cells = <2>;
>> +                #interrupt-cells = <1>;
>> +
>> +               /* Power on QCA639x chip sitting behind this bridge. */
>> +               power-domains = <&qca6391>;
> 
> This all must be in a child node of the bridge representing the wifi
> device.

Ack

> And all the regulators in the &qca6391 node should just be in
> the child node here. The indirection is pointless from a DT
> perspective.

It is not an indirection. The qca6391 node is shared between WiFi 
sitting on PCIe and BT sitting on serial port. One can not say that BT 
is powered by WiFi or vice versa. Thus there is a need for separate 
'power domain' node.


-- 
With best wishes
Dmitry

[PATCH v2 5/5] arm64: dts: qcom: Add Bluetooth support on RB5

[Dmitry Baryshkov]

From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

Add Bluetooth support on RB5 using the onboard QCA6391 WLAN+BT chipset.

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
[DB: added qca6391 power domain, removed s2f regulator]
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 arch/arm64/boot/dts/qcom/qrb5165-rb5.dts | 29 ++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts b/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts
index b39a9729395f..c65c13994a86 100644
--- a/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts
+++ b/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts
@@ -19,6 +19,7 @@ / {
 	compatible = "qcom,qrb5165-rb5", "qcom,sm8250";
 
 	aliases {
+		hsuart0 = &uart6;
 		serial0 = &uart12;
 		sdhc2 = &sdhc_2;
 	};
@@ -689,6 +690,26 @@ &pm8150_rtc {
 	status = "okay";
 };
 
+&qup_uart6_default {
+	ctsrx {
+		pins = "gpio16", "gpio19";
+		drive-strength = <2>;
+		bias-disable;
+	};
+
+	rts {
+		pins = "gpio17";
+		drive-strength = <2>;
+		bias-disable;
+	};
+
+	tx {
+		pins = "gpio18";
+		drive-strength = <2>;
+		bias-pull-up;
+	};
+};
+
 &qupv3_id_0 {
 	status = "okay";
 };
@@ -1194,6 +1215,14 @@ wlan-en {
 	};
 };
 
+&uart6 {
+	status = "okay";
+	bluetooth {
+		compatible = "qcom,qca6390-bt";
+		power-domains = <&qca6391>;
+	};
+};
+
 &uart12 {
 	status = "okay";
 };
-- 
2.29.2

Re: [PATCH v2 5/5] arm64: dts: qcom: Add Bluetooth support on RB5

[Dmitry Baryshkov]

On 28/01/2021 22:15, Rob Herring wrote:
> On Thu, Jan 28, 2021 at 11:52 AM Dmitry Baryshkov
> <dmitry.baryshkov@linaro.org> wrote:
>>
>> From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
>>
>> Add Bluetooth support on RB5 using the onboard QCA6391 WLAN+BT chipset.
>>
>> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
>> [DB: added qca6391 power domain, removed s2f regulator]
>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>> ---
>>   arch/arm64/boot/dts/qcom/qrb5165-rb5.dts | 29 ++++++++++++++++++++++++
>>   1 file changed, 29 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts b/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts
>> index b39a9729395f..c65c13994a86 100644
>> --- a/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts
>> +++ b/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts
>> @@ -19,6 +19,7 @@ / {
>>          compatible = "qcom,qrb5165-rb5", "qcom,sm8250";
>>
>>          aliases {
>> +               hsuart0 = &uart6;
> 
> Serial devices should be 'serialN'. Don't add custom aliases.

Ack

> 
>>                  serial0 = &uart12;
>>                  sdhc2 = &sdhc_2;
> 
> BTW, this should be dropped too.

Ack

> 
>>          };
>> @@ -689,6 +690,26 @@ &pm8150_rtc {
>>          status = "okay";
>>   };
>>
>> +&qup_uart6_default {
>> +       ctsrx {
>> +               pins = "gpio16", "gpio19";
>> +               drive-strength = <2>;
>> +               bias-disable;
>> +       };
>> +
>> +       rts {
>> +               pins = "gpio17";
>> +               drive-strength = <2>;
>> +               bias-disable;
>> +       };
>> +
>> +       tx {
>> +               pins = "gpio18";
>> +               drive-strength = <2>;
>> +               bias-pull-up;
>> +       };
>> +};
>> +
>>   &qupv3_id_0 {
>>          status = "okay";
>>   };
>> @@ -1194,6 +1215,14 @@ wlan-en {
>>          };
>>   };
>>
>> +&uart6 {
>> +       status = "okay";
>> +       bluetooth {
>> +               compatible = "qcom,qca6390-bt";
>> +               power-domains = <&qca6391>;
>> +       };
>> +};
>> +
>>   &uart12 {
>>          status = "okay";
>>   };
>> --
>> 2.29.2
>>


-- 
With best wishes
Dmitry

Re: [PATCH v2 5/5] arm64: dts: qcom: Add Bluetooth support on RB5

[Manivannan Sadhasivam]

On Thu, Jan 28, 2021 at 01:15:22PM -0600, Rob Herring wrote:
> On Thu, Jan 28, 2021 at 11:52 AM Dmitry Baryshkov
> <dmitry.baryshkov@linaro.org> wrote:
> >
> > From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> >
> > Add Bluetooth support on RB5 using the onboard QCA6391 WLAN+BT chipset.
> >
> > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > [DB: added qca6391 power domain, removed s2f regulator]
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > ---
> >  arch/arm64/boot/dts/qcom/qrb5165-rb5.dts | 29 ++++++++++++++++++++++++
> >  1 file changed, 29 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts b/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts
> > index b39a9729395f..c65c13994a86 100644
> > --- a/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts
> > +++ b/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts
> > @@ -19,6 +19,7 @@ / {
> >         compatible = "qcom,qrb5165-rb5", "qcom,sm8250";
> >
> >         aliases {
> > +               hsuart0 = &uart6;
> 
> Serial devices should be 'serialN'. Don't add custom aliases.
> 

Sorry, this is needed by the serial driver:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/tty/serial/qcom_geni_serial.c#n1364

Thanks,
Mani

> >                 serial0 = &uart12;
> >                 sdhc2 = &sdhc_2;
> 
> BTW, this should be dropped too.
> 
> >         };
> > @@ -689,6 +690,26 @@ &pm8150_rtc {
> >         status = "okay";
> >  };
> >
> > +&qup_uart6_default {
> > +       ctsrx {
> > +               pins = "gpio16", "gpio19";
> > +               drive-strength = <2>;
> > +               bias-disable;
> > +       };
> > +
> > +       rts {
> > +               pins = "gpio17";
> > +               drive-strength = <2>;
> > +               bias-disable;
> > +       };
> > +
> > +       tx {
> > +               pins = "gpio18";
> > +               drive-strength = <2>;
> > +               bias-pull-up;
> > +       };
> > +};
> > +
> >  &qupv3_id_0 {
> >         status = "okay";
> >  };
> > @@ -1194,6 +1215,14 @@ wlan-en {
> >         };
> >  };
> >
> > +&uart6 {
> > +       status = "okay";
> > +       bluetooth {
> > +               compatible = "qcom,qca6390-bt";
> > +               power-domains = <&qca6391>;
> > +       };
> > +};
> > +
> >  &uart12 {
> >         status = "okay";
> >  };
> > --
> > 2.29.2
> >

Re: [PATCH v2 3/5] pcie-qcom: provide a way to power up qca6390 chip on RB5 platform

[Yassine Oudjana]

On Thu, Jan 28, 2021 at 11:52 AM Dmitry Baryshkov
<dmitry.baryshkov@linaro.org> wrote:
>
> Some Qualcomm platforms require to power up an external device before
> probing the PCI bus. E.g. on RB5 platform the QCA6390 WiFi/BT chip needs
> to be powered up before PCIe0 bus is probed. Add a quirk to the
> respective PCIe root bridge to attach to the power domain if one is
> required, so that the QCA chip is started before scanning the PCIe bus.

I'm lead to believe that this is also needed for QCA6174. I tried taking
the driver you wrote and changing regulators to match the ones used by QCA6174,
but I found that it gets probed after PCIe, which makes it get -EPROBE_DEFER
when it tries to attach to the power domain, which it doesn't handle it and
just proceeds to scan the bus and not find the chip as usual.

Re: [PATCH v2 3/5] pcie-qcom: provide a way to power up qca6390 chip on RB5 platform

[Dmitry Baryshkov]

On Sat, 30 Jan 2021 at 11:04, Yassine Oudjana <y.oudjana@protonmail.com> wrote:
>
> On Thu, Jan 28, 2021 at 11:52 AM Dmitry Baryshkov
> <dmitry.baryshkov@linaro.org> wrote:
> >
> > Some Qualcomm platforms require to power up an external device before
> > probing the PCI bus. E.g. on RB5 platform the QCA6390 WiFi/BT chip needs
> > to be powered up before PCIe0 bus is probed. Add a quirk to the
> > respective PCIe root bridge to attach to the power domain if one is
> > required, so that the QCA chip is started before scanning the PCIe bus.
>
> I'm lead to believe that this is also needed for QCA6174. I tried taking
> the driver you wrote and changing regulators to match the ones used by QCA6174,
> but I found that it gets probed after PCIe, which makes it get -EPROBE_DEFER
> when it tries to attach to the power domain, which it doesn't handle it and
> just proceeds to scan the bus and not find the chip as usual.

As a quick hack, you can try using this power domain for the pci_phy
(see my v1). Rob declined this approach, but it is useful to check if
QCA6174 wakes up properly and deferring PCIe probing till the chip is
powered up.



-- 
With best wishes
Dmitry