On Wed, Jun 09, 2021 at 02:22:00PM +0530, Manivannan Sadhasivam wrote:
> On Sat, Jun 05, 2021 at 10:07:15PM -0500, Bjorn Andersson wrote:
> > On Thu 03 Jun 05:38 CDT 2021, Manivannan Sadhasivam wrote:
> >
> > > Add driver support for Qualcomm PCIe Endpoint controller driver based on
> > > the Designware core with added Qualcomm specific wrapper around the
> > > core. The driver support is very basic such that it supports only
> > > enumeration, PCIe read/write, and MSI. There is no ASPM and PM support
> > > for now but these will be added later.
> > >
> > > The driver is capable of using the PERST# and WAKE# side-band GPIOs for
> > > operation and written on top of the DWC PCI framework.
> > >
> > > Co-developed-by: Siddartha Mohanadoss <smohanad@xxxxxxxxxxxxxx>
> > > Signed-off-by: Siddartha Mohanadoss <smohanad@xxxxxxxxxxxxxx>
> > > [mani: restructured the driver and fixed several bugs for upstream]
> > > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@xxxxxxxxxx>
> >
> > Really nice to see this working!
> >
> > > ---
> > > drivers/pci/controller/dwc/Kconfig | 10 +
> > > drivers/pci/controller/dwc/Makefile | 1 +
> > > drivers/pci/controller/dwc/pcie-qcom-ep.c | 780 ++++++++++++++++++++++
> > > 3 files changed, 791 insertions(+)
> > > create mode 100644 drivers/pci/controller/dwc/pcie-qcom-ep.c
> > >
> > > diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
> > > index 423d35872ce4..32e735b1fd85 100644
> > > --- a/drivers/pci/controller/dwc/Kconfig
> > > +++ b/drivers/pci/controller/dwc/Kconfig
> > > @@ -180,6 +180,16 @@ config PCIE_QCOM
> > > PCIe controller uses the DesignWare core plus Qualcomm-specific
> > > hardware wrappers.
> > >
> > > +config PCIE_QCOM_EP
> > > + bool "Qualcomm PCIe controller - Endpoint mode"
> > > + depends on OF && (ARCH_QCOM || COMPILE_TEST)
> > > + depends on PCI_ENDPOINT
> > > + select PCIE_DW_EP
> > > + help
> > > + Say Y here to enable support for the PCIe controllers on Qualcomm SoCs
> > > + to work in endpoint mode. The PCIe controller uses the DesignWare core
> > > + plus Qualcomm-specific hardware wrappers.
> > > +
> > > config PCIE_ARMADA_8K
> > > bool "Marvell Armada-8K PCIe controller"
> > > depends on ARCH_MVEBU || COMPILE_TEST
> > > diff --git a/drivers/pci/controller/dwc/Makefile b/drivers/pci/controller/dwc/Makefile
> > > index eca805c1a023..abb27642d46b 100644
> > > --- a/drivers/pci/controller/dwc/Makefile
> > > +++ b/drivers/pci/controller/dwc/Makefile
> > > @@ -12,6 +12,7 @@ obj-$(CONFIG_PCI_KEYSTONE) += pci-keystone.o
> > > obj-$(CONFIG_PCI_LAYERSCAPE) += pci-layerscape.o
> > > obj-$(CONFIG_PCI_LAYERSCAPE_EP) += pci-layerscape-ep.o
> > > obj-$(CONFIG_PCIE_QCOM) += pcie-qcom.o
> > > +obj-$(CONFIG_PCIE_QCOM_EP) += pcie-qcom-ep.o
> > > obj-$(CONFIG_PCIE_ARMADA_8K) += pcie-armada8k.o
> > > obj-$(CONFIG_PCIE_ARTPEC6) += pcie-artpec6.o
> > > obj-$(CONFIG_PCIE_INTEL_GW) += pcie-intel-gw.o
> > > diff --git a/drivers/pci/controller/dwc/pcie-qcom-ep.c b/drivers/pci/controller/dwc/pcie-qcom-ep.c
> > > new file mode 100644
> > > index 000000000000..b68511bacc2a
> > > --- /dev/null
> > > +++ b/drivers/pci/controller/dwc/pcie-qcom-ep.c
> > > @@ -0,0 +1,780 @@
[...]
> > > +static int qcom_pcie_ep_enable_irq_resources(struct platform_device *pdev,
> > > + struct qcom_pcie_ep *pcie_ep)
> > > +{
> > > + int irq, ret;
> > > +
> > > + irq = platform_get_irq_byname(pdev, "global");
> > > + if (irq < 0) {
> > > + dev_err(&pdev->dev, "Failed to get Global IRQ\n");
> > > + return irq;
> > > + }
> > > +
> > > + ret = devm_request_threaded_irq(&pdev->dev, irq, NULL,
> > > + qcom_pcie_ep_global_threaded_irq,
> > > + IRQF_TRIGGER_HIGH | IRQF_ONESHOT,
> >
> > Leave out the trigger and rely on DT.
> >
>
> Okay
>
> > > + "global_irq", pcie_ep);
> > > + if (ret) {
> > > + dev_err(&pdev->dev, "Failed to request Global IRQ\n");
> > > + return ret;
> > > + }
> > > +
> > > + pcie_ep->perst_irq = gpiod_to_irq(pcie_ep->reset);
> > > + irq_set_status_flags(pcie_ep->perst_irq, IRQ_NOAUTOEN);
> >
> > Is the global interrupt needed for dw_pcie_ep_init()? Or could this
> > simply be done when things are ready to handle the interrupts?
> >
>
> No it is not needed. I can move this after dw_pcie_ep_init().
>
I don't know why but I'm seeing issues when this gets called after
dw_pcie_ep_init(). So I'm keeping this function as it is for now.
Thanks,
Mani
