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

On Fri, Jan 29, 2021 at 06:45:21AM +0300, Dmitry Baryshkov wrote:

On 28/01/2021 22:26, Rob Herring wrote:

quoted

On Thu, Jan 28, 2021 at 11:52 AM Dmitry Baryshkov
[off-list ref] wrote:

quoted

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.

quoted

quoted

Signed-off-by: Dmitry Baryshkov <redacted>
---
  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