Hello,
This series is the continuation of previous work by Vidya Sagar [1] to fix the
issues related to accessing DBI register space before completing the core
initialization in some EP platforms like Tegra194/234 and Qcom EP.
Since Vidya is busy, I took over the series based on his consent (off-list
discussion).
NOTE
====
Based on the comments received in v7 [2], I've heavily modified the series
to fix several other issues reported by Bjorn and Niklas. One noticeable
change is getting rid of the 'core_init_notifer' flag added to differentiate
between glue drivers requiring refclk from host and drivers getting refclk
locally.
By getting rid of this flag, now both the DWC EP driver and the EPF drivers
can use a single flow and need not distinguish between the glue drivers.
We can also get rid of the 'link_up_notifier' flag in the future by following
the same convention.
Testing
=======
I've tested the series on Qcom SM8450 based dev board that depends on refclk
from host with EPF_MHI driver. It'd be good to test this series on platforms
that generate refclk locally and also with EPF_TEST driver.
- Mani
[1] https://lore.kernel.org/linux-pci/[email protected]/
[2] https://lore.kernel.org/linux-pci/[email protected]/
Changes in v8:
- Rebased on top of v6.8-rc1
- Removed the deinit callback from struct dw_pcie_ep_ops
- Renamed dw_pcie_ep_exit() to dw_pcie_ep_deinit()
- Introduced dw_pcie_ep_cleanup() API for drivers supporting PERST#
- Renamed dw_pcie_ep_init_complete() to dw_pcie_ep_init_registers()
- Called dw_pcie_ep_init_registers() API directly from all glue drivers
- Removed "core_init_notifier" flag
- Added a generic dw_pcie_ep_linkdown() API to handle LINK_DOWN event and used
it in qcom driver
- Added Kernel-doc comments for DWC EP APIs
Changes in v7:
- Rebased on top of v6.7-rc1
- Kept the current dw_pcie_ep_init_complete() API instead of renaming it to
dw_pcie_ep_init_late(), since changing the name causes a slight ambiguity.
- Splitted the change that moves pci_epc_init_notify() inside
dw_pcie_ep_init_notify() to help bisecting and also to avoid build issue.
- Added a new patch that moves pci_epc_init_notify() inside
dw_pcie_ep_init_notify().
- Took over the authorship and dropped the previous Ack as the patches are
heavily modified.
Changes in v6:
- Rebased on top of pci/next (6e2fca71e187)
- removed ep_init_late() callback as it is no longer necessary
For previous changelog, please refer [1].
Signed-off-by: Manivannan Sadhasivam <[email protected]>
---
Manivannan Sadhasivam (10):
PCI: dwc: ep: Remove deinit() callback from struct dw_pcie_ep_ops
PCI: dwc: ep: Rename dw_pcie_ep_exit() to dw_pcie_ep_deinit()
PCI: dwc: ep: Introduce dw_pcie_ep_cleanup() API for drivers supporting PERST#
PCI: dwc: ep: Fix DBI access failure for drivers requiring refclk from host
PCI: dwc: ep: Rename dw_pcie_ep_init_complete() to dw_pcie_ep_init_registers()
PCI: dwc: ep: Call dw_pcie_ep_init_registers() API directly from all glue drivers
PCI: dwc: ep: Remove "core_init_notifier" flag
PCI: dwc: ep: Add a generic dw_pcie_ep_linkdown() API to handle LINK_DOWN event
PCI: qcom-ep: Use the generic dw_pcie_ep_linkdown() API to handle LINK_DOWN event
PCI: dwc: ep: Add Kernel-doc comments for APIs
drivers/pci/controller/dwc/pci-dra7xx.c | 9 +
drivers/pci/controller/dwc/pci-imx6.c | 10 +
drivers/pci/controller/dwc/pci-keystone.c | 11 +
drivers/pci/controller/dwc/pci-layerscape-ep.c | 9 +
drivers/pci/controller/dwc/pcie-designware-ep.c | 307 +++++++++++++++-------
drivers/pci/controller/dwc/pcie-designware-plat.c | 11 +
drivers/pci/controller/dwc/pcie-designware.h | 19 +-
drivers/pci/controller/dwc/pcie-qcom-ep.c | 6 +-
drivers/pci/controller/dwc/pcie-rcar-gen4.c | 28 +-
drivers/pci/controller/dwc/pcie-tegra194.c | 5 +-
drivers/pci/controller/dwc/pcie-uniphier-ep.c | 15 +-
drivers/pci/endpoint/functions/pci-epf-test.c | 18 +-
include/linux/pci-epc.h | 3 -
13 files changed, 321 insertions(+), 130 deletions(-)
---
base-commit: 6613476e225e090cc9aad49be7fa504e290dd33d
change-id: 20240224-pci-dbi-rework-b2e99a62930c
Best regards,
--
Manivannan Sadhasivam <[email protected]>
deinit() callback was solely introduced for the pcie-rcar-gen4 driver where
it is used to do platform specific resource deallocation. And this callback
is called right at the end of the dw_pcie_ep_exit() API. So it doesn't
matter whether it is called within or outside of dw_pcie_ep_exit() API.
So let's remove this callback and directly call rcar_gen4_pcie_ep_deinit()
in pcie-rcar-gen4 driver to do resource deallocation after the completion
of dw_pcie_ep_exit() API in rcar_gen4_remove_dw_pcie_ep().
This simplifies the DWC layer.
Signed-off-by: Manivannan Sadhasivam <[email protected]>
---
drivers/pci/controller/dwc/pcie-designware-ep.c | 9 +--------
drivers/pci/controller/dwc/pcie-designware.h | 1 -
drivers/pci/controller/dwc/pcie-rcar-gen4.c | 14 ++++++++------
3 files changed, 9 insertions(+), 15 deletions(-)
diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
index 5befed2dc02b..d305f9b4cdfe 100644
--- a/drivers/pci/controller/dwc/pcie-designware-ep.c
+++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
@@ -575,9 +575,6 @@ void dw_pcie_ep_exit(struct dw_pcie_ep *ep)
epc->mem->window.page_size);
pci_epc_mem_exit(epc);
-
- if (ep->ops->deinit)
- ep->ops->deinit(ep);
}
EXPORT_SYMBOL_GPL(dw_pcie_ep_exit);
@@ -738,7 +735,7 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
ep->page_size);
if (ret < 0) {
dev_err(dev, "Failed to initialize address space\n");
- goto err_ep_deinit;
+ return ret;
}
ep->msi_mem = pci_epc_mem_alloc_addr(epc, &ep->msi_mem_phys,
@@ -775,10 +772,6 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
err_exit_epc_mem:
pci_epc_mem_exit(epc);
-err_ep_deinit:
- if (ep->ops->deinit)
- ep->ops->deinit(ep);
-
return ret;
}
EXPORT_SYMBOL_GPL(dw_pcie_ep_init);
diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index 26dae4837462..ab7431a37209 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -333,7 +333,6 @@ struct dw_pcie_rp {
struct dw_pcie_ep_ops {
void (*pre_init)(struct dw_pcie_ep *ep);
void (*init)(struct dw_pcie_ep *ep);
- void (*deinit)(struct dw_pcie_ep *ep);
int (*raise_irq)(struct dw_pcie_ep *ep, u8 func_no,
unsigned int type, u16 interrupt_num);
const struct pci_epc_features* (*get_features)(struct dw_pcie_ep *ep);
diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.c b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
index e9166619b1f9..ac97d594ea47 100644
--- a/drivers/pci/controller/dwc/pcie-rcar-gen4.c
+++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
@@ -352,11 +352,8 @@ static void rcar_gen4_pcie_ep_init(struct dw_pcie_ep *ep)
dw_pcie_ep_reset_bar(pci, bar);
}
-static void rcar_gen4_pcie_ep_deinit(struct dw_pcie_ep *ep)
+static void rcar_gen4_pcie_ep_deinit(struct rcar_gen4_pcie *rcar)
{
- struct dw_pcie *dw = to_dw_pcie_from_ep(ep);
- struct rcar_gen4_pcie *rcar = to_rcar_gen4_pcie(dw);
-
writel(0, rcar->base + PCIEDMAINTSTSEN);
rcar_gen4_pcie_common_deinit(rcar);
}
@@ -408,7 +405,6 @@ static unsigned int rcar_gen4_pcie_ep_get_dbi2_offset(struct dw_pcie_ep *ep,
static const struct dw_pcie_ep_ops pcie_ep_ops = {
.pre_init = rcar_gen4_pcie_ep_pre_init,
.init = rcar_gen4_pcie_ep_init,
- .deinit = rcar_gen4_pcie_ep_deinit,
.raise_irq = rcar_gen4_pcie_ep_raise_irq,
.get_features = rcar_gen4_pcie_ep_get_features,
.get_dbi_offset = rcar_gen4_pcie_ep_get_dbi_offset,
@@ -418,18 +414,24 @@ static const struct dw_pcie_ep_ops pcie_ep_ops = {
static int rcar_gen4_add_dw_pcie_ep(struct rcar_gen4_pcie *rcar)
{
struct dw_pcie_ep *ep = &rcar->dw.ep;
+ int ret;
if (!IS_ENABLED(CONFIG_PCIE_RCAR_GEN4_EP))
return -ENODEV;
ep->ops = &pcie_ep_ops;
- return dw_pcie_ep_init(ep);
+ ret = dw_pcie_ep_init(ep);
+ if (ret)
+ rcar_gen4_pcie_ep_deinit(rcar);
+
+ return ret;
}
static void rcar_gen4_remove_dw_pcie_ep(struct rcar_gen4_pcie *rcar)
{
dw_pcie_ep_exit(&rcar->dw.ep);
+ rcar_gen4_pcie_ep_deinit(rcar);
}
/* Common */
--
2.25.1
dw_pcie_ep_exit() API is undoing what the dw_pcie_ep_init() API has done
already (at least partly). But the API name dw_pcie_ep_exit() is not quite
reflecting that. So let's rename it to dw_pcie_ep_deinit() to make the
purpose of this API clear. This also aligns with the DWC host driver.
Signed-off-by: Manivannan Sadhasivam <[email protected]>
---
drivers/pci/controller/dwc/pcie-designware-ep.c | 4 ++--
drivers/pci/controller/dwc/pcie-designware.h | 4 ++--
drivers/pci/controller/dwc/pcie-rcar-gen4.c | 2 +-
3 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
index d305f9b4cdfe..2b11290aab4c 100644
--- a/drivers/pci/controller/dwc/pcie-designware-ep.c
+++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
@@ -564,7 +564,7 @@ int dw_pcie_ep_raise_msix_irq(struct dw_pcie_ep *ep, u8 func_no,
return 0;
}
-void dw_pcie_ep_exit(struct dw_pcie_ep *ep)
+void dw_pcie_ep_deinit(struct dw_pcie_ep *ep)
{
struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
struct pci_epc *epc = ep->epc;
@@ -576,7 +576,7 @@ void dw_pcie_ep_exit(struct dw_pcie_ep *ep)
pci_epc_mem_exit(epc);
}
-EXPORT_SYMBOL_GPL(dw_pcie_ep_exit);
+EXPORT_SYMBOL_GPL(dw_pcie_ep_deinit);
static unsigned int dw_pcie_ep_find_ext_capability(struct dw_pcie *pci, int cap)
{
diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index ab7431a37209..61465203bb60 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -671,7 +671,7 @@ void dw_pcie_ep_linkup(struct dw_pcie_ep *ep);
int dw_pcie_ep_init(struct dw_pcie_ep *ep);
int dw_pcie_ep_init_complete(struct dw_pcie_ep *ep);
void dw_pcie_ep_init_notify(struct dw_pcie_ep *ep);
-void dw_pcie_ep_exit(struct dw_pcie_ep *ep);
+void dw_pcie_ep_deinit(struct dw_pcie_ep *ep);
int dw_pcie_ep_raise_intx_irq(struct dw_pcie_ep *ep, u8 func_no);
int dw_pcie_ep_raise_msi_irq(struct dw_pcie_ep *ep, u8 func_no,
u8 interrupt_num);
@@ -701,7 +701,7 @@ static inline void dw_pcie_ep_init_notify(struct dw_pcie_ep *ep)
{
}
-static inline void dw_pcie_ep_exit(struct dw_pcie_ep *ep)
+static inline void dw_pcie_ep_deinit(struct dw_pcie_ep *ep)
{
}
diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.c b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
index ac97d594ea47..9d9d22e367bb 100644
--- a/drivers/pci/controller/dwc/pcie-rcar-gen4.c
+++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
@@ -430,7 +430,7 @@ static int rcar_gen4_add_dw_pcie_ep(struct rcar_gen4_pcie *rcar)
static void rcar_gen4_remove_dw_pcie_ep(struct rcar_gen4_pcie *rcar)
{
- dw_pcie_ep_exit(&rcar->dw.ep);
+ dw_pcie_ep_deinit(&rcar->dw.ep);
rcar_gen4_pcie_ep_deinit(rcar);
}
--
2.25.1
For DWC glue drivers supporting PERST# (currently Qcom and Tegra194), some
of the DWC resources like eDMA should be cleaned up during the PERST#
assert time.
So let's introduce a dw_pcie_ep_cleanup() API that could be called by these
drivers to cleanup the DWC specific resources. Currently, it just removes
eDMA.
Reported-by: Niklas Cassel <[email protected]>
Closes: https://lore.kernel.org/linux-pci/ZWYmX8Y%2F7Q9WMxES@x1-carbon
Signed-off-by: Manivannan Sadhasivam <[email protected]>
---
drivers/pci/controller/dwc/pcie-designware-ep.c | 11 +++++++++--
drivers/pci/controller/dwc/pcie-designware.h | 5 +++++
drivers/pci/controller/dwc/pcie-qcom-ep.c | 1 +
drivers/pci/controller/dwc/pcie-tegra194.c | 2 ++
4 files changed, 17 insertions(+), 2 deletions(-)
diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
index 2b11290aab4c..1205bfba8310 100644
--- a/drivers/pci/controller/dwc/pcie-designware-ep.c
+++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
@@ -564,12 +564,19 @@ int dw_pcie_ep_raise_msix_irq(struct dw_pcie_ep *ep, u8 func_no,
return 0;
}
-void dw_pcie_ep_deinit(struct dw_pcie_ep *ep)
+void dw_pcie_ep_cleanup(struct dw_pcie_ep *ep)
{
struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
- struct pci_epc *epc = ep->epc;
dw_pcie_edma_remove(pci);
+}
+EXPORT_SYMBOL_GPL(dw_pcie_ep_cleanup);
+
+void dw_pcie_ep_deinit(struct dw_pcie_ep *ep)
+{
+ struct pci_epc *epc = ep->epc;
+
+ dw_pcie_ep_cleanup(ep);
pci_epc_mem_free_addr(epc, ep->msi_mem_phys, ep->msi_mem,
epc->mem->window.page_size);
diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index 61465203bb60..351d2fe3ea4d 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -672,6 +672,7 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep);
int dw_pcie_ep_init_complete(struct dw_pcie_ep *ep);
void dw_pcie_ep_init_notify(struct dw_pcie_ep *ep);
void dw_pcie_ep_deinit(struct dw_pcie_ep *ep);
+void dw_pcie_ep_cleanup(struct dw_pcie_ep *ep);
int dw_pcie_ep_raise_intx_irq(struct dw_pcie_ep *ep, u8 func_no);
int dw_pcie_ep_raise_msi_irq(struct dw_pcie_ep *ep, u8 func_no,
u8 interrupt_num);
@@ -705,6 +706,10 @@ static inline void dw_pcie_ep_deinit(struct dw_pcie_ep *ep)
{
}
+static inline void dw_pcie_ep_cleanup(struct dw_pcie_ep *ep)
+{
+}
+
static inline int dw_pcie_ep_raise_intx_irq(struct dw_pcie_ep *ep, u8 func_no)
{
return 0;
diff --git a/drivers/pci/controller/dwc/pcie-qcom-ep.c b/drivers/pci/controller/dwc/pcie-qcom-ep.c
index 36e5e80cd22f..59b1c0110288 100644
--- a/drivers/pci/controller/dwc/pcie-qcom-ep.c
+++ b/drivers/pci/controller/dwc/pcie-qcom-ep.c
@@ -507,6 +507,7 @@ static void qcom_pcie_perst_assert(struct dw_pcie *pci)
return;
}
+ dw_pcie_ep_cleanup(&pci->ep);
qcom_pcie_disable_resources(pcie_ep);
pcie_ep->link_status = QCOM_PCIE_EP_LINK_DISABLED;
}
diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c
index 7afa9e9aabe2..68bfeed3429b 100644
--- a/drivers/pci/controller/dwc/pcie-tegra194.c
+++ b/drivers/pci/controller/dwc/pcie-tegra194.c
@@ -1715,6 +1715,8 @@ static void pex_ep_event_pex_rst_assert(struct tegra_pcie_dw *pcie)
if (ret)
dev_err(pcie->dev, "Failed to go Detect state: %d\n", ret);
+ dw_pcie_ep_cleanup(&pcie->pci.ep);
+
reset_control_assert(pcie->core_rst);
tegra_pcie_disable_phy(pcie);
--
2.25.1
The DWC glue drivers requiring an active reference clock from the PCIe host
for initializing their PCIe EP core, set a flag called 'core_init_notifier'
to let DWC driver know that these drivers need a special attention during
initialization. In these drivers, access to the hw registers (like DBI)
before receiving the active refclk from host will result in access failure
and also could cause a whole system hang.
But the current DWC EP driver doesn't honor the requirements of the drivers
setting 'core_init_notifier' flag and tries to access the DBI registers
during dw_pcie_ep_init(). This causes the system hang for glue drivers such
as Tegra194 and Qcom EP as they depend on refclk from host and have set the
above mentioned flag.
To workaround this issue, users of the affected platforms have to maintain
the dependency with the PCIe host by booting the PCIe EP after host boot.
But this won't provide a good user experience, since PCIe EP is _one_ of
the features of those platforms and it doesn't make sense to delay the
whole platform booting due to PCIe requiring active refclk.
So to fix this issue, let's move all the DBI access from
dw_pcie_ep_init() in the DWC EP driver to the dw_pcie_ep_init_complete()
API. This API will only be called by the drivers setting
'core_init_notifier' flag once refclk is received from host. For the rest
of the drivers that gets the refclk locally, this API will be called
within dw_pcie_ep_init().
Fixes: e966f7390da9 ("PCI: dwc: Refactor core initialization code for EP mode")
Co-developed-by: Vidya Sagar <[email protected]>
Signed-off-by: Vidya Sagar <[email protected]>
Signed-off-by: Manivannan Sadhasivam <[email protected]>
---
drivers/pci/controller/dwc/pcie-designware-ep.c | 120 ++++++++++++++----------
1 file changed, 71 insertions(+), 49 deletions(-)
diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
index 1205bfba8310..99d66b0fa59b 100644
--- a/drivers/pci/controller/dwc/pcie-designware-ep.c
+++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
@@ -606,11 +606,16 @@ static unsigned int dw_pcie_ep_find_ext_capability(struct dw_pcie *pci, int cap)
int dw_pcie_ep_init_complete(struct dw_pcie_ep *ep)
{
struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
+ struct dw_pcie_ep_func *ep_func;
+ struct device *dev = pci->dev;
+ struct pci_epc *epc = ep->epc;
unsigned int offset, ptm_cap_base;
unsigned int nbars;
u8 hdr_type;
+ u8 func_no;
+ int i, ret;
+ void *addr;
u32 reg;
- int i;
hdr_type = dw_pcie_readb_dbi(pci, PCI_HEADER_TYPE) &
PCI_HEADER_TYPE_MASK;
@@ -621,6 +626,58 @@ int dw_pcie_ep_init_complete(struct dw_pcie_ep *ep)
return -EIO;
}
+ dw_pcie_version_detect(pci);
+
+ dw_pcie_iatu_detect(pci);
+
+ ret = dw_pcie_edma_detect(pci);
+ if (ret)
+ return ret;
+
+ if (!ep->ib_window_map) {
+ ep->ib_window_map = devm_bitmap_zalloc(dev, pci->num_ib_windows,
+ GFP_KERNEL);
+ if (!ep->ib_window_map)
+ goto err_remove_edma;
+ }
+
+ if (!ep->ob_window_map) {
+ ep->ob_window_map = devm_bitmap_zalloc(dev, pci->num_ob_windows,
+ GFP_KERNEL);
+ if (!ep->ob_window_map)
+ goto err_remove_edma;
+ }
+
+ if (!ep->outbound_addr) {
+ addr = devm_kcalloc(dev, pci->num_ob_windows, sizeof(phys_addr_t),
+ GFP_KERNEL);
+ if (!addr)
+ goto err_remove_edma;
+ ep->outbound_addr = addr;
+ }
+
+ for (func_no = 0; func_no < epc->max_functions; func_no++) {
+
+ ep_func = dw_pcie_ep_get_func_from_ep(ep, func_no);
+ if (ep_func)
+ continue;
+
+ ep_func = devm_kzalloc(dev, sizeof(*ep_func), GFP_KERNEL);
+ if (!ep_func)
+ goto err_remove_edma;
+
+ ep_func->func_no = func_no;
+ ep_func->msi_cap = dw_pcie_ep_find_capability(ep, func_no,
+ PCI_CAP_ID_MSI);
+ ep_func->msix_cap = dw_pcie_ep_find_capability(ep, func_no,
+ PCI_CAP_ID_MSIX);
+
+ list_add_tail(&ep_func->list, &ep->func_list);
+ }
+
+ if (ep->ops->init)
+ ep->ops->init(ep);
+
offset = dw_pcie_ep_find_ext_capability(pci, PCI_EXT_CAP_ID_REBAR);
ptm_cap_base = dw_pcie_ep_find_ext_capability(pci, PCI_EXT_CAP_ID_PTM);
@@ -655,14 +712,17 @@ int dw_pcie_ep_init_complete(struct dw_pcie_ep *ep)
dw_pcie_dbi_ro_wr_dis(pci);
return 0;
+
+err_remove_edma:
+ dw_pcie_edma_remove(pci);
+
+ return ret;
}
EXPORT_SYMBOL_GPL(dw_pcie_ep_init_complete);
int dw_pcie_ep_init(struct dw_pcie_ep *ep)
{
int ret;
- void *addr;
- u8 func_no;
struct resource *res;
struct pci_epc *epc;
struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
@@ -670,7 +730,6 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
struct platform_device *pdev = to_platform_device(dev);
struct device_node *np = dev->of_node;
const struct pci_epc_features *epc_features;
- struct dw_pcie_ep_func *ep_func;
INIT_LIST_HEAD(&ep->func_list);
@@ -688,26 +747,6 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
if (ep->ops->pre_init)
ep->ops->pre_init(ep);
- dw_pcie_version_detect(pci);
-
- dw_pcie_iatu_detect(pci);
-
- ep->ib_window_map = devm_bitmap_zalloc(dev, pci->num_ib_windows,
- GFP_KERNEL);
- if (!ep->ib_window_map)
- return -ENOMEM;
-
- ep->ob_window_map = devm_bitmap_zalloc(dev, pci->num_ob_windows,
- GFP_KERNEL);
- if (!ep->ob_window_map)
- return -ENOMEM;
-
- addr = devm_kcalloc(dev, pci->num_ob_windows, sizeof(phys_addr_t),
- GFP_KERNEL);
- if (!addr)
- return -ENOMEM;
- ep->outbound_addr = addr;
-
epc = devm_pci_epc_create(dev, &epc_ops);
if (IS_ERR(epc)) {
dev_err(dev, "Failed to create epc device\n");
@@ -721,23 +760,6 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
if (ret < 0)
epc->max_functions = 1;
- for (func_no = 0; func_no < epc->max_functions; func_no++) {
- ep_func = devm_kzalloc(dev, sizeof(*ep_func), GFP_KERNEL);
- if (!ep_func)
- return -ENOMEM;
-
- ep_func->func_no = func_no;
- ep_func->msi_cap = dw_pcie_ep_find_capability(ep, func_no,
- PCI_CAP_ID_MSI);
- ep_func->msix_cap = dw_pcie_ep_find_capability(ep, func_no,
- PCI_CAP_ID_MSIX);
-
- list_add_tail(&ep_func->list, &ep->func_list);
- }
-
- if (ep->ops->init)
- ep->ops->init(ep);
-
ret = pci_epc_mem_init(epc, ep->phys_base, ep->addr_size,
ep->page_size);
if (ret < 0) {
@@ -753,25 +775,25 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
goto err_exit_epc_mem;
}
- ret = dw_pcie_edma_detect(pci);
- if (ret)
- goto err_free_epc_mem;
-
if (ep->ops->get_features) {
epc_features = ep->ops->get_features(ep);
if (epc_features->core_init_notifier)
return 0;
}
+ /*
+ * NOTE:- Avoid accessing the hardware (Ex:- DBI space) before this
+ * step as platforms that implement 'core_init_notifier' feature may
+ * not have the hardware ready (i.e. core initialized) for access
+ * (Ex: tegra194). Any hardware access on such platforms result
+ * in system hang.
+ */
ret = dw_pcie_ep_init_complete(ep);
if (ret)
- goto err_remove_edma;
+ goto err_free_epc_mem;
return 0;
-err_remove_edma:
- dw_pcie_edma_remove(pci);
-
err_free_epc_mem:
pci_epc_mem_free_addr(epc, ep->msi_mem_phys, ep->msi_mem,
epc->mem->window.page_size);
--
2.25.1
The goal of the dw_pcie_ep_init_complete() API is to initialize the DWC
specific registers post registering the controller with the EP framework.
But the naming doesn't reflect its functionality and causes confusion. So,
let's rename it to dw_pcie_ep_init_registers() to make it clear that it
initializes the DWC specific registers.
Signed-off-by: Manivannan Sadhasivam <[email protected]>
---
drivers/pci/controller/dwc/pcie-designware-ep.c | 6 +++---
drivers/pci/controller/dwc/pcie-designware.h | 4 ++--
drivers/pci/controller/dwc/pcie-qcom-ep.c | 2 +-
drivers/pci/controller/dwc/pcie-tegra194.c | 2 +-
4 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
index 99d66b0fa59b..ed1f2afd830a 100644
--- a/drivers/pci/controller/dwc/pcie-designware-ep.c
+++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
@@ -603,7 +603,7 @@ static unsigned int dw_pcie_ep_find_ext_capability(struct dw_pcie *pci, int cap)
return 0;
}
-int dw_pcie_ep_init_complete(struct dw_pcie_ep *ep)
+int dw_pcie_ep_init_registers(struct dw_pcie_ep *ep)
{
struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
struct dw_pcie_ep_func *ep_func;
@@ -718,7 +718,7 @@ int dw_pcie_ep_init_complete(struct dw_pcie_ep *ep)
return ret;
}
-EXPORT_SYMBOL_GPL(dw_pcie_ep_init_complete);
+EXPORT_SYMBOL_GPL(dw_pcie_ep_init_registers);
int dw_pcie_ep_init(struct dw_pcie_ep *ep)
{
@@ -788,7 +788,7 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
* (Ex: tegra194). Any hardware access on such platforms result
* in system hang.
*/
- ret = dw_pcie_ep_init_complete(ep);
+ ret = dw_pcie_ep_init_registers(ep);
if (ret)
goto err_free_epc_mem;
diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index 351d2fe3ea4d..f8e5431a207b 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -669,7 +669,7 @@ static inline void __iomem *dw_pcie_own_conf_map_bus(struct pci_bus *bus,
#ifdef CONFIG_PCIE_DW_EP
void dw_pcie_ep_linkup(struct dw_pcie_ep *ep);
int dw_pcie_ep_init(struct dw_pcie_ep *ep);
-int dw_pcie_ep_init_complete(struct dw_pcie_ep *ep);
+int dw_pcie_ep_init_registers(struct dw_pcie_ep *ep);
void dw_pcie_ep_init_notify(struct dw_pcie_ep *ep);
void dw_pcie_ep_deinit(struct dw_pcie_ep *ep);
void dw_pcie_ep_cleanup(struct dw_pcie_ep *ep);
@@ -693,7 +693,7 @@ static inline int dw_pcie_ep_init(struct dw_pcie_ep *ep)
return 0;
}
-static inline int dw_pcie_ep_init_complete(struct dw_pcie_ep *ep)
+static inline int dw_pcie_ep_init_registers(struct dw_pcie_ep *ep)
{
return 0;
}
diff --git a/drivers/pci/controller/dwc/pcie-qcom-ep.c b/drivers/pci/controller/dwc/pcie-qcom-ep.c
index 59b1c0110288..3697b4a944cc 100644
--- a/drivers/pci/controller/dwc/pcie-qcom-ep.c
+++ b/drivers/pci/controller/dwc/pcie-qcom-ep.c
@@ -463,7 +463,7 @@ static int qcom_pcie_perst_deassert(struct dw_pcie *pci)
PARF_INT_ALL_LINK_UP | PARF_INT_ALL_EDMA;
writel_relaxed(val, pcie_ep->parf + PARF_INT_ALL_MASK);
- ret = dw_pcie_ep_init_complete(&pcie_ep->pci.ep);
+ ret = dw_pcie_ep_init_registers(&pcie_ep->pci.ep);
if (ret) {
dev_err(dev, "Failed to complete initialization: %d\n", ret);
goto err_disable_resources;
diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c
index 68bfeed3429b..264ee76bf008 100644
--- a/drivers/pci/controller/dwc/pcie-tegra194.c
+++ b/drivers/pci/controller/dwc/pcie-tegra194.c
@@ -1897,7 +1897,7 @@ static void pex_ep_event_pex_rst_deassert(struct tegra_pcie_dw *pcie)
val = (upper_32_bits(ep->msi_mem_phys) & MSIX_ADDR_MATCH_HIGH_OFF_MASK);
dw_pcie_writel_dbi(pci, MSIX_ADDR_MATCH_HIGH_OFF, val);
- ret = dw_pcie_ep_init_complete(ep);
+ ret = dw_pcie_ep_init_registers(ep);
if (ret) {
dev_err(dev, "Failed to complete initialization: %d\n", ret);
goto fail_init_complete;
--
2.25.1
Currently, dw_pcie_ep_init_registers() API is directly called by the glue
drivers requiring active refclk from host. But for the other drivers, it is
getting called implicitly by dw_pcie_ep_init(). This is due to the fact
that this API initializes DWC EP specific registers and that requires an
active refclk (either from host or generated locally by endpoint itsef).
But, this causes a discrepancy among the glue drivers. So to avoid this
confusion, let's call this API directly from all glue drivers irrespective
of refclk dependency. Only difference here is that the drivers requiring
refclk from host will call this API only after the refclk is received and
other drivers without refclk dependency will call this API right after
dw_pcie_ep_init().
This change will also allow us to remove the "core_init_notifier" flag in
the later commits.
Signed-off-by: Manivannan Sadhasivam <[email protected]>
---
drivers/pci/controller/dwc/pci-dra7xx.c | 7 +++++++
drivers/pci/controller/dwc/pci-imx6.c | 8 ++++++++
drivers/pci/controller/dwc/pci-keystone.c | 9 +++++++++
drivers/pci/controller/dwc/pci-layerscape-ep.c | 7 +++++++
drivers/pci/controller/dwc/pcie-designware-ep.c | 22 ----------------------
drivers/pci/controller/dwc/pcie-designware-plat.c | 9 +++++++++
drivers/pci/controller/dwc/pcie-rcar-gen4.c | 12 +++++++++++-
drivers/pci/controller/dwc/pcie-uniphier-ep.c | 13 ++++++++++++-
8 files changed, 63 insertions(+), 24 deletions(-)
diff --git a/drivers/pci/controller/dwc/pci-dra7xx.c b/drivers/pci/controller/dwc/pci-dra7xx.c
index 0e406677060d..395042b29ffc 100644
--- a/drivers/pci/controller/dwc/pci-dra7xx.c
+++ b/drivers/pci/controller/dwc/pci-dra7xx.c
@@ -467,6 +467,13 @@ static int dra7xx_add_pcie_ep(struct dra7xx_pcie *dra7xx,
return ret;
}
+ ret = dw_pcie_ep_init_registers(ep);
+ if (ret) {
+ dev_err(dev, "Failed to initialize DWC endpoint registers\n");
+ dw_pcie_ep_deinit(ep);
+ return ret;
+ }
+
return 0;
}
diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index dc2c036ab28c..bfcafa440ddb 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -1136,6 +1136,14 @@ static int imx6_add_pcie_ep(struct imx6_pcie *imx6_pcie,
dev_err(dev, "failed to initialize endpoint\n");
return ret;
}
+
+ ret = dw_pcie_ep_init_registers(ep);
+ if (ret) {
+ dev_err(dev, "Failed to initialize DWC endpoint registers\n");
+ dw_pcie_ep_deinit(ep);
+ return ret;
+ }
+
/* Start LTSSM. */
imx6_pcie_ltssm_enable(dev);
diff --git a/drivers/pci/controller/dwc/pci-keystone.c b/drivers/pci/controller/dwc/pci-keystone.c
index c0c62533a3f1..8392894ed286 100644
--- a/drivers/pci/controller/dwc/pci-keystone.c
+++ b/drivers/pci/controller/dwc/pci-keystone.c
@@ -1286,6 +1286,13 @@ static int ks_pcie_probe(struct platform_device *pdev)
ret = dw_pcie_ep_init(&pci->ep);
if (ret < 0)
goto err_get_sync;
+
+ ret = dw_pcie_ep_init_registers(&pci->ep);
+ if (ret < 0) {
+ dev_err(dev, "Failed to initialize DWC endpoint registers\n");
+ goto err_ep_init;
+ }
+
break;
default:
dev_err(dev, "INVALID device type %d\n", mode);
@@ -1295,6 +1302,8 @@ static int ks_pcie_probe(struct platform_device *pdev)
return 0;
+err_ep_init:
+ dw_pcie_ep_deinit(&pci->ep);
err_get_sync:
pm_runtime_put(dev);
pm_runtime_disable(dev);
diff --git a/drivers/pci/controller/dwc/pci-layerscape-ep.c b/drivers/pci/controller/dwc/pci-layerscape-ep.c
index 2e398494e7c0..b712fdd06549 100644
--- a/drivers/pci/controller/dwc/pci-layerscape-ep.c
+++ b/drivers/pci/controller/dwc/pci-layerscape-ep.c
@@ -276,6 +276,13 @@ static int __init ls_pcie_ep_probe(struct platform_device *pdev)
if (ret)
return ret;
+ ret = dw_pcie_ep_init_registers(&pci->ep);
+ if (ret) {
+ dev_err(dev, "Failed to initialize DWC endpoint registers\n");
+ dw_pcie_ep_deinit(&pci->ep);
+ return ret;
+ }
+
return ls_pcie_ep_interrupt_init(pcie, pdev);
}
diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
index ed1f2afd830a..278bdc9b2269 100644
--- a/drivers/pci/controller/dwc/pcie-designware-ep.c
+++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
@@ -729,7 +729,6 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
struct device *dev = pci->dev;
struct platform_device *pdev = to_platform_device(dev);
struct device_node *np = dev->of_node;
- const struct pci_epc_features *epc_features;
INIT_LIST_HEAD(&ep->func_list);
@@ -775,29 +774,8 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
goto err_exit_epc_mem;
}
- if (ep->ops->get_features) {
- epc_features = ep->ops->get_features(ep);
- if (epc_features->core_init_notifier)
- return 0;
- }
-
- /*
- * NOTE:- Avoid accessing the hardware (Ex:- DBI space) before this
- * step as platforms that implement 'core_init_notifier' feature may
- * not have the hardware ready (i.e. core initialized) for access
- * (Ex: tegra194). Any hardware access on such platforms result
- * in system hang.
- */
- ret = dw_pcie_ep_init_registers(ep);
- if (ret)
- goto err_free_epc_mem;
-
return 0;
-err_free_epc_mem:
- pci_epc_mem_free_addr(epc, ep->msi_mem_phys, ep->msi_mem,
- epc->mem->window.page_size);
-
err_exit_epc_mem:
pci_epc_mem_exit(epc);
diff --git a/drivers/pci/controller/dwc/pcie-designware-plat.c b/drivers/pci/controller/dwc/pcie-designware-plat.c
index 778588b4be70..ca9b22e654cd 100644
--- a/drivers/pci/controller/dwc/pcie-designware-plat.c
+++ b/drivers/pci/controller/dwc/pcie-designware-plat.c
@@ -145,6 +145,15 @@ static int dw_plat_pcie_probe(struct platform_device *pdev)
pci->ep.ops = &pcie_ep_ops;
ret = dw_pcie_ep_init(&pci->ep);
+ if (ret)
+ return ret;
+
+ ret = dw_pcie_ep_init_registers(&pci->ep);
+ if (ret) {
+ dev_err(dev, "Failed to initialize DWC endpoint registers\n");
+ dw_pcie_ep_deinit(&pci->ep);
+ }
+
break;
default:
dev_err(dev, "INVALID device type %d\n", dw_plat_pcie->mode);
diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.c b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
index 9d9d22e367bb..fb7c03639a53 100644
--- a/drivers/pci/controller/dwc/pcie-rcar-gen4.c
+++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
@@ -414,6 +414,7 @@ static const struct dw_pcie_ep_ops pcie_ep_ops = {
static int rcar_gen4_add_dw_pcie_ep(struct rcar_gen4_pcie *rcar)
{
struct dw_pcie_ep *ep = &rcar->dw.ep;
+ struct device *dev = rcar->dw.dev;
int ret;
if (!IS_ENABLED(CONFIG_PCIE_RCAR_GEN4_EP))
@@ -422,8 +423,17 @@ static int rcar_gen4_add_dw_pcie_ep(struct rcar_gen4_pcie *rcar)
ep->ops = &pcie_ep_ops;
ret = dw_pcie_ep_init(ep);
- if (ret)
+ if (ret) {
rcar_gen4_pcie_ep_deinit(rcar);
+ return ret;
+ }
+
+ ret = dw_pcie_ep_init_registers(ep);
+ if (ret) {
+ dev_err(dev, "Failed to initialize DWC endpoint registers\n");
+ dw_pcie_ep_deinit(ep);
+ rcar_gen4_pcie_ep_deinit(rcar);
+ }
return ret;
}
diff --git a/drivers/pci/controller/dwc/pcie-uniphier-ep.c b/drivers/pci/controller/dwc/pcie-uniphier-ep.c
index 3fced0d3e851..82ccaea089be 100644
--- a/drivers/pci/controller/dwc/pcie-uniphier-ep.c
+++ b/drivers/pci/controller/dwc/pcie-uniphier-ep.c
@@ -399,7 +399,18 @@ static int uniphier_pcie_ep_probe(struct platform_device *pdev)
return ret;
priv->pci.ep.ops = &uniphier_pcie_ep_ops;
- return dw_pcie_ep_init(&priv->pci.ep);
+ ret = dw_pcie_ep_init(&priv->pci.ep);
+ if (ret)
+ return ret;
+
+ ret = dw_pcie_ep_init_registers(&priv->pci.ep);
+ if (ret) {
+ dev_err(dev, "Failed to initialize DWC endpoint registers\n");
+ dw_pcie_ep_deinit(&priv->pci.ep);
+ return ret;
+ }
+
+ return 0;
}
static const struct uniphier_pcie_ep_soc_data uniphier_pro5_data = {
--
2.25.1
"core_init_notifier" flag is set by the glue drivers requiring refclk from
the host to complete the DWC core initialization. Also, those drivers will
send a notification to the EPF drivers once the initialization is fully
completed using the pci_epc_init_notify() API. Only then, the EPF drivers
will start functioning.
For the rest of the drivers generating refclk locally, EPF drivers will
start functioning post binding with them. EPF drivers rely on the
'core_init_notifier' flag to differentiate between the drivers.
Unfortunately, this creates two different flows for the EPF drivers.
So to avoid that, let's get rid of the "core_init_notifier" flag and follow
a single initialization flow for the EPF drivers. This is done by calling
the dw_pcie_ep_init_notify() from all glue drivers after the completion of
dw_pcie_ep_init_registers() API. This will allow all the glue drivers to
send the notification to the EPF drivers once the initialization is fully
completed.
Only difference here is that, the drivers requiring refclk from host will
send the notification once refclk is received, while others will send it
during probe time itself.
Signed-off-by: Manivannan Sadhasivam <[email protected]>
---
drivers/pci/controller/dwc/pci-dra7xx.c | 2 ++
drivers/pci/controller/dwc/pci-imx6.c | 2 ++
drivers/pci/controller/dwc/pci-keystone.c | 2 ++
drivers/pci/controller/dwc/pci-layerscape-ep.c | 2 ++
drivers/pci/controller/dwc/pcie-designware-plat.c | 2 ++
drivers/pci/controller/dwc/pcie-qcom-ep.c | 1 -
drivers/pci/controller/dwc/pcie-rcar-gen4.c | 2 ++
drivers/pci/controller/dwc/pcie-tegra194.c | 1 -
drivers/pci/controller/dwc/pcie-uniphier-ep.c | 2 ++
drivers/pci/endpoint/functions/pci-epf-test.c | 18 +++++-------------
include/linux/pci-epc.h | 3 ---
11 files changed, 19 insertions(+), 18 deletions(-)
diff --git a/drivers/pci/controller/dwc/pci-dra7xx.c b/drivers/pci/controller/dwc/pci-dra7xx.c
index 395042b29ffc..d2d17d37d3e0 100644
--- a/drivers/pci/controller/dwc/pci-dra7xx.c
+++ b/drivers/pci/controller/dwc/pci-dra7xx.c
@@ -474,6 +474,8 @@ static int dra7xx_add_pcie_ep(struct dra7xx_pcie *dra7xx,
return ret;
}
+ dw_pcie_ep_init_notify(ep);
+
return 0;
}
diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index bfcafa440ddb..894b5de76e3a 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -1144,6 +1144,8 @@ static int imx6_add_pcie_ep(struct imx6_pcie *imx6_pcie,
return ret;
}
+ dw_pcie_ep_init_notify(ep);
+
/* Start LTSSM. */
imx6_pcie_ltssm_enable(dev);
diff --git a/drivers/pci/controller/dwc/pci-keystone.c b/drivers/pci/controller/dwc/pci-keystone.c
index 8392894ed286..1d00c5fa14ce 100644
--- a/drivers/pci/controller/dwc/pci-keystone.c
+++ b/drivers/pci/controller/dwc/pci-keystone.c
@@ -1293,6 +1293,8 @@ static int ks_pcie_probe(struct platform_device *pdev)
goto err_ep_init;
}
+ dw_pcie_ep_init_notify(&pci->ep);
+
break;
default:
dev_err(dev, "INVALID device type %d\n", mode);
diff --git a/drivers/pci/controller/dwc/pci-layerscape-ep.c b/drivers/pci/controller/dwc/pci-layerscape-ep.c
index b712fdd06549..c513598a46d7 100644
--- a/drivers/pci/controller/dwc/pci-layerscape-ep.c
+++ b/drivers/pci/controller/dwc/pci-layerscape-ep.c
@@ -283,6 +283,8 @@ static int __init ls_pcie_ep_probe(struct platform_device *pdev)
return ret;
}
+ dw_pcie_ep_init_notify(&pci->ep);
+
return ls_pcie_ep_interrupt_init(pcie, pdev);
}
diff --git a/drivers/pci/controller/dwc/pcie-designware-plat.c b/drivers/pci/controller/dwc/pcie-designware-plat.c
index ca9b22e654cd..8490c5d6ff9f 100644
--- a/drivers/pci/controller/dwc/pcie-designware-plat.c
+++ b/drivers/pci/controller/dwc/pcie-designware-plat.c
@@ -154,6 +154,8 @@ static int dw_plat_pcie_probe(struct platform_device *pdev)
dw_pcie_ep_deinit(&pci->ep);
}
+ dw_pcie_ep_init_notify(&pci->ep);
+
break;
default:
dev_err(dev, "INVALID device type %d\n", dw_plat_pcie->mode);
diff --git a/drivers/pci/controller/dwc/pcie-qcom-ep.c b/drivers/pci/controller/dwc/pcie-qcom-ep.c
index 3697b4a944cc..2fb8c15e7a91 100644
--- a/drivers/pci/controller/dwc/pcie-qcom-ep.c
+++ b/drivers/pci/controller/dwc/pcie-qcom-ep.c
@@ -775,7 +775,6 @@ static void qcom_pcie_ep_init_debugfs(struct qcom_pcie_ep *pcie_ep)
static const struct pci_epc_features qcom_pcie_epc_features = {
.linkup_notifier = true,
- .core_init_notifier = true,
.msi_capable = true,
.msix_capable = false,
.align = SZ_4K,
diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.c b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
index fb7c03639a53..0448928017f3 100644
--- a/drivers/pci/controller/dwc/pcie-rcar-gen4.c
+++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
@@ -435,6 +435,8 @@ static int rcar_gen4_add_dw_pcie_ep(struct rcar_gen4_pcie *rcar)
rcar_gen4_pcie_ep_deinit(rcar);
}
+ dw_pcie_ep_init_notify(ep);
+
return ret;
}
diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c
index 264ee76bf008..e02deb31a72d 100644
--- a/drivers/pci/controller/dwc/pcie-tegra194.c
+++ b/drivers/pci/controller/dwc/pcie-tegra194.c
@@ -2006,7 +2006,6 @@ static int tegra_pcie_ep_raise_irq(struct dw_pcie_ep *ep, u8 func_no,
static const struct pci_epc_features tegra_pcie_epc_features = {
.linkup_notifier = true,
- .core_init_notifier = true,
.msi_capable = false,
.msix_capable = false,
.reserved_bar = 1 << BAR_2 | 1 << BAR_3 | 1 << BAR_4 | 1 << BAR_5,
diff --git a/drivers/pci/controller/dwc/pcie-uniphier-ep.c b/drivers/pci/controller/dwc/pcie-uniphier-ep.c
index 82ccaea089be..eb1d79fdb1f1 100644
--- a/drivers/pci/controller/dwc/pcie-uniphier-ep.c
+++ b/drivers/pci/controller/dwc/pcie-uniphier-ep.c
@@ -410,6 +410,8 @@ static int uniphier_pcie_ep_probe(struct platform_device *pdev)
return ret;
}
+ dw_pcie_ep_init_notify(&priv->pci.ep);
+
return 0;
}
diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c
index 18c80002d3bd..fc0282b0d626 100644
--- a/drivers/pci/endpoint/functions/pci-epf-test.c
+++ b/drivers/pci/endpoint/functions/pci-epf-test.c
@@ -753,6 +753,7 @@ static int pci_epf_test_core_init(struct pci_epf *epf)
const struct pci_epc_features *epc_features;
struct pci_epc *epc = epf->epc;
struct device *dev = &epf->dev;
+ bool linkup_notifier = false;
bool msix_capable = false;
bool msi_capable = true;
int ret;
@@ -795,6 +796,10 @@ static int pci_epf_test_core_init(struct pci_epf *epf)
}
}
+ linkup_notifier = epc_features->linkup_notifier;
+ if (!linkup_notifier)
+ queue_work(kpcitest_workqueue, &epf_test->cmd_handler.work);
+
return 0;
}
@@ -901,8 +906,6 @@ static int pci_epf_test_bind(struct pci_epf *epf)
const struct pci_epc_features *epc_features;
enum pci_barno test_reg_bar = BAR_0;
struct pci_epc *epc = epf->epc;
- bool linkup_notifier = false;
- bool core_init_notifier = false;
if (WARN_ON_ONCE(!epc))
return -EINVAL;
@@ -913,8 +916,6 @@ static int pci_epf_test_bind(struct pci_epf *epf)
return -EOPNOTSUPP;
}
- linkup_notifier = epc_features->linkup_notifier;
- core_init_notifier = epc_features->core_init_notifier;
test_reg_bar = pci_epc_get_first_free_bar(epc_features);
if (test_reg_bar < 0)
return -EINVAL;
@@ -927,21 +928,12 @@ static int pci_epf_test_bind(struct pci_epf *epf)
if (ret)
return ret;
- if (!core_init_notifier) {
- ret = pci_epf_test_core_init(epf);
- if (ret)
- return ret;
- }
-
epf_test->dma_supported = true;
ret = pci_epf_test_init_dma_chan(epf_test);
if (ret)
epf_test->dma_supported = false;
- if (!linkup_notifier && !core_init_notifier)
- queue_work(kpcitest_workqueue, &epf_test->cmd_handler.work);
-
return 0;
}
diff --git a/include/linux/pci-epc.h b/include/linux/pci-epc.h
index 40ea18f5aa02..03d22aed5ac6 100644
--- a/include/linux/pci-epc.h
+++ b/include/linux/pci-epc.h
@@ -148,8 +148,6 @@ struct pci_epc {
/**
* struct pci_epc_features - features supported by a EPC device per function
* @linkup_notifier: indicate if the EPC device can notify EPF driver on link up
- * @core_init_notifier: indicate cores that can notify about their availability
- * for initialization
* @msi_capable: indicate if the endpoint function has MSI capability
* @msix_capable: indicate if the endpoint function has MSI-X capability
* @reserved_bar: bitmap to indicate reserved BAR unavailable to function driver
@@ -159,7 +157,6 @@ struct pci_epc {
*/
struct pci_epc_features {
unsigned int linkup_notifier : 1;
- unsigned int core_init_notifier : 1;
unsigned int msi_capable : 1;
unsigned int msix_capable : 1;
u8 reserved_bar;
--
2.25.1
The PCIe link can go to LINK_DOWN state in one of the following scenarios:
1. Fundamental (PERST#)/hot/warm reset
2. Link transition from L2/L3 to L0
In those cases, LINK_DOWN causes some non-sticky DWC registers to loose the
state (like REBAR, PTM_CAP etc...). So the drivers need to reinitialize
them to function properly once the link comes back again.
This is not a problem for drivers supporting PERST# IRQ, since they can
reinitialize the registers in the PERST# IRQ callback. But for the drivers
not supporting PERST#, there is no way they can reinitialize the registers
other than relying on LINK_DOWN IRQ received when the link goes down. So
let's add a DWC generic API dw_pcie_ep_linkdown() that reinitializes the
non-sticky registers and also notifies the EPF drivers about link going
down.
This API can also be used by the drivers supporting PERST# to handle the
scenario (2) mentioned above.
Signed-off-by: Manivannan Sadhasivam <[email protected]>
---
drivers/pci/controller/dwc/pcie-designware-ep.c | 111 ++++++++++++++----------
drivers/pci/controller/dwc/pcie-designware.h | 5 ++
2 files changed, 72 insertions(+), 44 deletions(-)
diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
index 278bdc9b2269..fed4c2936c78 100644
--- a/drivers/pci/controller/dwc/pcie-designware-ep.c
+++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
@@ -14,14 +14,6 @@
#include <linux/pci-epc.h>
#include <linux/pci-epf.h>
-void dw_pcie_ep_linkup(struct dw_pcie_ep *ep)
-{
- struct pci_epc *epc = ep->epc;
-
- pci_epc_linkup(epc);
-}
-EXPORT_SYMBOL_GPL(dw_pcie_ep_linkup);
-
void dw_pcie_ep_init_notify(struct dw_pcie_ep *ep)
{
struct pci_epc *epc = ep->epc;
@@ -603,19 +595,56 @@ static unsigned int dw_pcie_ep_find_ext_capability(struct dw_pcie *pci, int cap)
return 0;
}
+static void dw_pcie_ep_init_non_sticky_registers(struct dw_pcie *pci)
+{
+ unsigned int offset, ptm_cap_base;
+ unsigned int nbars;
+ u32 reg, i;
+
+ offset = dw_pcie_ep_find_ext_capability(pci, PCI_EXT_CAP_ID_REBAR);
+ ptm_cap_base = dw_pcie_ep_find_ext_capability(pci, PCI_EXT_CAP_ID_PTM);
+
+ dw_pcie_dbi_ro_wr_en(pci);
+
+ if (offset) {
+ reg = dw_pcie_readl_dbi(pci, offset + PCI_REBAR_CTRL);
+ nbars = (reg & PCI_REBAR_CTRL_NBAR_MASK) >>
+ PCI_REBAR_CTRL_NBAR_SHIFT;
+
+ for (i = 0; i < nbars; i++, offset += PCI_REBAR_CTRL)
+ dw_pcie_writel_dbi(pci, offset + PCI_REBAR_CAP, 0x0);
+ }
+
+ /*
+ * PTM responder capability can be disabled only after disabling
+ * PTM root capability.
+ */
+ if (ptm_cap_base) {
+ dw_pcie_dbi_ro_wr_en(pci);
+ reg = dw_pcie_readl_dbi(pci, ptm_cap_base + PCI_PTM_CAP);
+ reg &= ~PCI_PTM_CAP_ROOT;
+ dw_pcie_writel_dbi(pci, ptm_cap_base + PCI_PTM_CAP, reg);
+
+ reg = dw_pcie_readl_dbi(pci, ptm_cap_base + PCI_PTM_CAP);
+ reg &= ~(PCI_PTM_CAP_RES | PCI_PTM_GRANULARITY_MASK);
+ dw_pcie_writel_dbi(pci, ptm_cap_base + PCI_PTM_CAP, reg);
+ dw_pcie_dbi_ro_wr_dis(pci);
+ }
+
+ dw_pcie_setup(pci);
+ dw_pcie_dbi_ro_wr_dis(pci);
+}
+
int dw_pcie_ep_init_registers(struct dw_pcie_ep *ep)
{
struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
struct dw_pcie_ep_func *ep_func;
struct device *dev = pci->dev;
struct pci_epc *epc = ep->epc;
- unsigned int offset, ptm_cap_base;
- unsigned int nbars;
u8 hdr_type;
u8 func_no;
- int i, ret;
void *addr;
- u32 reg;
+ int ret;
hdr_type = dw_pcie_readb_dbi(pci, PCI_HEADER_TYPE) &
PCI_HEADER_TYPE_MASK;
@@ -678,38 +707,7 @@ int dw_pcie_ep_init_registers(struct dw_pcie_ep *ep)
if (ep->ops->init)
ep->ops->init(ep);
- offset = dw_pcie_ep_find_ext_capability(pci, PCI_EXT_CAP_ID_REBAR);
- ptm_cap_base = dw_pcie_ep_find_ext_capability(pci, PCI_EXT_CAP_ID_PTM);
-
- dw_pcie_dbi_ro_wr_en(pci);
-
- if (offset) {
- reg = dw_pcie_readl_dbi(pci, offset + PCI_REBAR_CTRL);
- nbars = (reg & PCI_REBAR_CTRL_NBAR_MASK) >>
- PCI_REBAR_CTRL_NBAR_SHIFT;
-
- for (i = 0; i < nbars; i++, offset += PCI_REBAR_CTRL)
- dw_pcie_writel_dbi(pci, offset + PCI_REBAR_CAP, 0x0);
- }
-
- /*
- * PTM responder capability can be disabled only after disabling
- * PTM root capability.
- */
- if (ptm_cap_base) {
- dw_pcie_dbi_ro_wr_en(pci);
- reg = dw_pcie_readl_dbi(pci, ptm_cap_base + PCI_PTM_CAP);
- reg &= ~PCI_PTM_CAP_ROOT;
- dw_pcie_writel_dbi(pci, ptm_cap_base + PCI_PTM_CAP, reg);
-
- reg = dw_pcie_readl_dbi(pci, ptm_cap_base + PCI_PTM_CAP);
- reg &= ~(PCI_PTM_CAP_RES | PCI_PTM_GRANULARITY_MASK);
- dw_pcie_writel_dbi(pci, ptm_cap_base + PCI_PTM_CAP, reg);
- dw_pcie_dbi_ro_wr_dis(pci);
- }
-
- dw_pcie_setup(pci);
- dw_pcie_dbi_ro_wr_dis(pci);
+ dw_pcie_ep_init_non_sticky_registers(pci);
return 0;
@@ -720,6 +718,31 @@ int dw_pcie_ep_init_registers(struct dw_pcie_ep *ep)
}
EXPORT_SYMBOL_GPL(dw_pcie_ep_init_registers);
+void dw_pcie_ep_linkup(struct dw_pcie_ep *ep)
+{
+ struct pci_epc *epc = ep->epc;
+
+ pci_epc_linkup(epc);
+}
+EXPORT_SYMBOL_GPL(dw_pcie_ep_linkup);
+
+void dw_pcie_ep_linkdown(struct dw_pcie_ep *ep)
+{
+ struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
+ struct pci_epc *epc = ep->epc;
+
+ /*
+ * Initialize the non-sticky DWC registers as they would've reset post
+ * LINK_DOWN. This is specifically needed for drivers not supporting
+ * PERST# as they have no way to reinitialize the registers before the
+ * link comes back again.
+ */
+ dw_pcie_ep_init_non_sticky_registers(pci);
+
+ pci_epc_linkdown(epc);
+}
+EXPORT_SYMBOL_GPL(dw_pcie_ep_linkdown);
+
int dw_pcie_ep_init(struct dw_pcie_ep *ep)
{
int ret;
diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index f8e5431a207b..152969545b0a 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -668,6 +668,7 @@ static inline void __iomem *dw_pcie_own_conf_map_bus(struct pci_bus *bus,
#ifdef CONFIG_PCIE_DW_EP
void dw_pcie_ep_linkup(struct dw_pcie_ep *ep);
+void dw_pcie_ep_linkdown(struct dw_pcie_ep *ep);
int dw_pcie_ep_init(struct dw_pcie_ep *ep);
int dw_pcie_ep_init_registers(struct dw_pcie_ep *ep);
void dw_pcie_ep_init_notify(struct dw_pcie_ep *ep);
@@ -688,6 +689,10 @@ static inline void dw_pcie_ep_linkup(struct dw_pcie_ep *ep)
{
}
+static inline void dw_pcie_ep_linkdown(struct dw_pcie_ep *ep)
+{
+}
+
static inline int dw_pcie_ep_init(struct dw_pcie_ep *ep)
{
return 0;
--
2.25.1
Now that the API is available, let's make use of it. It also handles the
reinitialization of DWC non-sticky registers in addition to sending the
notification to EPF drivers.
Signed-off-by: Manivannan Sadhasivam <[email protected]>
---
drivers/pci/controller/dwc/pcie-qcom-ep.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/pci/controller/dwc/pcie-qcom-ep.c b/drivers/pci/controller/dwc/pcie-qcom-ep.c
index 2fb8c15e7a91..4e45bc4bca45 100644
--- a/drivers/pci/controller/dwc/pcie-qcom-ep.c
+++ b/drivers/pci/controller/dwc/pcie-qcom-ep.c
@@ -640,7 +640,7 @@ static irqreturn_t qcom_pcie_ep_global_irq_thread(int irq, void *data)
if (FIELD_GET(PARF_INT_ALL_LINK_DOWN, status)) {
dev_dbg(dev, "Received Linkdown event\n");
pcie_ep->link_status = QCOM_PCIE_EP_LINK_DOWN;
- pci_epc_linkdown(pci->ep.epc);
+ dw_pcie_ep_linkdown(&pci->ep);
} else if (FIELD_GET(PARF_INT_ALL_BME, status)) {
dev_dbg(dev, "Received BME event. Link is enabled!\n");
pcie_ep->link_status = QCOM_PCIE_EP_LINK_ENABLED;
--
2.25.1
All of the APIs are missing the Kernel-doc comments. Hence, add them.
Signed-off-by: Manivannan Sadhasivam <[email protected]>
---
drivers/pci/controller/dwc/pcie-designware-ep.c | 92 +++++++++++++++++++++++++
1 file changed, 92 insertions(+)
diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
index fed4c2936c78..cdcb33a279db 100644
--- a/drivers/pci/controller/dwc/pcie-designware-ep.c
+++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
@@ -14,6 +14,11 @@
#include <linux/pci-epc.h>
#include <linux/pci-epf.h>
+/**
+ * dw_pcie_ep_init_notify - Notify EPF drivers about EPC initialization
+ * complete
+ * @ep: DWC EP device
+ */
void dw_pcie_ep_init_notify(struct dw_pcie_ep *ep)
{
struct pci_epc *epc = ep->epc;
@@ -22,6 +27,14 @@ void dw_pcie_ep_init_notify(struct dw_pcie_ep *ep)
}
EXPORT_SYMBOL_GPL(dw_pcie_ep_init_notify);
+/**
+ * dw_pcie_ep_get_func_from_ep - Get the struct dw_pcie_ep_func corresponding to
+ * the endpoint function
+ * @ep: DWC EP device
+ * @func_no: Function number of the endpoint device
+ *
+ * Return: struct dw_pcie_ep_func if success, NULL otherwise.
+ */
struct dw_pcie_ep_func *
dw_pcie_ep_get_func_from_ep(struct dw_pcie_ep *ep, u8 func_no)
{
@@ -52,6 +65,11 @@ static void __dw_pcie_ep_reset_bar(struct dw_pcie *pci, u8 func_no,
dw_pcie_dbi_ro_wr_dis(pci);
}
+/**
+ * dw_pcie_ep_reset_bar - Reset endpoint BAR
+ * @pci: DWC PCI device
+ * @bar: BAR number of the endpoint
+ */
void dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum pci_barno bar)
{
u8 func_no, funcs;
@@ -431,6 +449,13 @@ static const struct pci_epc_ops epc_ops = {
.get_features = dw_pcie_ep_get_features,
};
+/**
+ * dw_pcie_ep_raise_intx_irq - Raise INTx IRQ to the host
+ * @ep: DWC EP device
+ * @func_no: Function number of the endpoint
+ *
+ * Return: 0 if success, errono otherwise.
+ */
int dw_pcie_ep_raise_intx_irq(struct dw_pcie_ep *ep, u8 func_no)
{
struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
@@ -442,6 +467,14 @@ int dw_pcie_ep_raise_intx_irq(struct dw_pcie_ep *ep, u8 func_no)
}
EXPORT_SYMBOL_GPL(dw_pcie_ep_raise_intx_irq);
+/**
+ * dw_pcie_ep_raise_msi_irq - Raise MSI IRQ to the host
+ * @ep: DWC EP device
+ * @func_no: Function number of the endpoint
+ * @interrupt_num: Interrupt number to be raised
+ *
+ * Return: 0 if success, errono otherwise.
+ */
int dw_pcie_ep_raise_msi_irq(struct dw_pcie_ep *ep, u8 func_no,
u8 interrupt_num)
{
@@ -490,6 +523,15 @@ int dw_pcie_ep_raise_msi_irq(struct dw_pcie_ep *ep, u8 func_no,
}
EXPORT_SYMBOL_GPL(dw_pcie_ep_raise_msi_irq);
+/**
+ * dw_pcie_ep_raise_msix_irq_doorbell - Raise MSIX to the host using Doorbell
+ * method
+ * @ep: DWC EP device
+ * @func_no: Function number of the endpoint device
+ * @interrupt_num: Interrupt number to be raised
+ *
+ * Return: 0 if success, errno otherwise.
+ */
int dw_pcie_ep_raise_msix_irq_doorbell(struct dw_pcie_ep *ep, u8 func_no,
u16 interrupt_num)
{
@@ -509,6 +551,14 @@ int dw_pcie_ep_raise_msix_irq_doorbell(struct dw_pcie_ep *ep, u8 func_no,
return 0;
}
+/**
+ * dw_pcie_ep_raise_msix_irq - Raise MSIX to the host
+ * @ep: DWC EP device
+ * @func_no: Function number of the endpoint device
+ * @interrupt_num: Interrupt number to be raised
+ *
+ * Return: 0 if success, errno otherwise.
+ */
int dw_pcie_ep_raise_msix_irq(struct dw_pcie_ep *ep, u8 func_no,
u16 interrupt_num)
{
@@ -556,6 +606,12 @@ int dw_pcie_ep_raise_msix_irq(struct dw_pcie_ep *ep, u8 func_no,
return 0;
}
+/**
+ * dw_pcie_ep_cleanup - Cleanup DWC EP resources
+ * @ep: DWC EP device
+ *
+ * Cleans up the DWC EP specific resources like eDMA etc...
+ */
void dw_pcie_ep_cleanup(struct dw_pcie_ep *ep)
{
struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
@@ -564,6 +620,13 @@ void dw_pcie_ep_cleanup(struct dw_pcie_ep *ep)
}
EXPORT_SYMBOL_GPL(dw_pcie_ep_cleanup);
+/**
+ * dw_pcie_ep_deinit - Deinitialize the endpoint device
+ * @ep: DWC EP device
+ *
+ * Deinitialize the endpoint device. EPC device is not destroyed since that will
+ * taken care by Devres.
+ */
void dw_pcie_ep_deinit(struct dw_pcie_ep *ep)
{
struct pci_epc *epc = ep->epc;
@@ -635,6 +698,14 @@ static void dw_pcie_ep_init_non_sticky_registers(struct dw_pcie *pci)
dw_pcie_dbi_ro_wr_dis(pci);
}
+/**
+ * dw_pcie_ep_init_registers - Initialize DWC EP specific registers
+ * @ep: DWC EP device
+ *
+ * Initialize the registers (CSRs) specific to DWC EP. This API should be called
+ * only when the endpoint receives an active refclk (either from host or
+ * generated locally).
+ */
int dw_pcie_ep_init_registers(struct dw_pcie_ep *ep)
{
struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
@@ -718,6 +789,10 @@ int dw_pcie_ep_init_registers(struct dw_pcie_ep *ep)
}
EXPORT_SYMBOL_GPL(dw_pcie_ep_init_registers);
+/**
+ * dw_pcie_ep_linkup - Notify EPF drivers about link up event
+ * @ep: DWC EP device
+ */
void dw_pcie_ep_linkup(struct dw_pcie_ep *ep)
{
struct pci_epc *epc = ep->epc;
@@ -726,6 +801,14 @@ void dw_pcie_ep_linkup(struct dw_pcie_ep *ep)
}
EXPORT_SYMBOL_GPL(dw_pcie_ep_linkup);
+/**
+ * dw_pcie_ep_linkdown - Notify EPF drivers about link down event
+ * @ep: DWC EP device
+ *
+ * Non-sticky registers are also initialized before sending the notification to
+ * the EPF drivers. This is needed since the registers need to be initialized
+ * before the link comes back again.
+ */
void dw_pcie_ep_linkdown(struct dw_pcie_ep *ep)
{
struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
@@ -743,6 +826,15 @@ void dw_pcie_ep_linkdown(struct dw_pcie_ep *ep)
}
EXPORT_SYMBOL_GPL(dw_pcie_ep_linkdown);
+/**
+ * dw_pcie_ep_init - Initialize the endpoint device
+ * @ep: DWC EP device
+ *
+ * Initialize the endpoint device. Allocate resources and create the EPC
+ * device with the endpoint framework.
+ *
+ * Return: 0 if success, errno otherwise.
+ */
int dw_pcie_ep_init(struct dw_pcie_ep *ep)
{
int ret;
--
2.25.1
On Sat, Feb 24, 2024 at 12:24:07PM +0530, Manivannan Sadhasivam wrote:
> deinit() callback was solely introduced for the pcie-rcar-gen4 driver where
> it is used to do platform specific resource deallocation. And this callback
> is called right at the end of the dw_pcie_ep_exit() API. So it doesn't
> matter whether it is called within or outside of dw_pcie_ep_exit() API.
>
> So let's remove this callback and directly call rcar_gen4_pcie_ep_deinit()
> in pcie-rcar-gen4 driver to do resource deallocation after the completion
> of dw_pcie_ep_exit() API in rcar_gen4_remove_dw_pcie_ep().
>
> This simplifies the DWC layer.
>
> Signed-off-by: Manivannan Sadhasivam <[email protected]>
Reviewed-by: Frank Li <[email protected]>
> ---
> drivers/pci/controller/dwc/pcie-designware-ep.c | 9 +--------
> drivers/pci/controller/dwc/pcie-designware.h | 1 -
> drivers/pci/controller/dwc/pcie-rcar-gen4.c | 14 ++++++++------
> 3 files changed, 9 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
> index 5befed2dc02b..d305f9b4cdfe 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
> @@ -575,9 +575,6 @@ void dw_pcie_ep_exit(struct dw_pcie_ep *ep)
> epc->mem->window.page_size);
>
> pci_epc_mem_exit(epc);
> -
> - if (ep->ops->deinit)
> - ep->ops->deinit(ep);
> }
> EXPORT_SYMBOL_GPL(dw_pcie_ep_exit);
>
> @@ -738,7 +735,7 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
> ep->page_size);
> if (ret < 0) {
> dev_err(dev, "Failed to initialize address space\n");
> - goto err_ep_deinit;
> + return ret;
> }
>
> ep->msi_mem = pci_epc_mem_alloc_addr(epc, &ep->msi_mem_phys,
> @@ -775,10 +772,6 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
> err_exit_epc_mem:
> pci_epc_mem_exit(epc);
>
> -err_ep_deinit:
> - if (ep->ops->deinit)
> - ep->ops->deinit(ep);
> -
> return ret;
> }
> EXPORT_SYMBOL_GPL(dw_pcie_ep_init);
> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> index 26dae4837462..ab7431a37209 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.h
> +++ b/drivers/pci/controller/dwc/pcie-designware.h
> @@ -333,7 +333,6 @@ struct dw_pcie_rp {
> struct dw_pcie_ep_ops {
> void (*pre_init)(struct dw_pcie_ep *ep);
> void (*init)(struct dw_pcie_ep *ep);
> - void (*deinit)(struct dw_pcie_ep *ep);
> int (*raise_irq)(struct dw_pcie_ep *ep, u8 func_no,
> unsigned int type, u16 interrupt_num);
> const struct pci_epc_features* (*get_features)(struct dw_pcie_ep *ep);
> diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.c b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> index e9166619b1f9..ac97d594ea47 100644
> --- a/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> +++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> @@ -352,11 +352,8 @@ static void rcar_gen4_pcie_ep_init(struct dw_pcie_ep *ep)
> dw_pcie_ep_reset_bar(pci, bar);
> }
>
> -static void rcar_gen4_pcie_ep_deinit(struct dw_pcie_ep *ep)
> +static void rcar_gen4_pcie_ep_deinit(struct rcar_gen4_pcie *rcar)
> {
> - struct dw_pcie *dw = to_dw_pcie_from_ep(ep);
> - struct rcar_gen4_pcie *rcar = to_rcar_gen4_pcie(dw);
> -
> writel(0, rcar->base + PCIEDMAINTSTSEN);
> rcar_gen4_pcie_common_deinit(rcar);
> }
> @@ -408,7 +405,6 @@ static unsigned int rcar_gen4_pcie_ep_get_dbi2_offset(struct dw_pcie_ep *ep,
> static const struct dw_pcie_ep_ops pcie_ep_ops = {
> .pre_init = rcar_gen4_pcie_ep_pre_init,
> .init = rcar_gen4_pcie_ep_init,
> - .deinit = rcar_gen4_pcie_ep_deinit,
> .raise_irq = rcar_gen4_pcie_ep_raise_irq,
> .get_features = rcar_gen4_pcie_ep_get_features,
> .get_dbi_offset = rcar_gen4_pcie_ep_get_dbi_offset,
> @@ -418,18 +414,24 @@ static const struct dw_pcie_ep_ops pcie_ep_ops = {
> static int rcar_gen4_add_dw_pcie_ep(struct rcar_gen4_pcie *rcar)
> {
> struct dw_pcie_ep *ep = &rcar->dw.ep;
> + int ret;
>
> if (!IS_ENABLED(CONFIG_PCIE_RCAR_GEN4_EP))
> return -ENODEV;
>
> ep->ops = &pcie_ep_ops;
>
> - return dw_pcie_ep_init(ep);
> + ret = dw_pcie_ep_init(ep);
> + if (ret)
> + rcar_gen4_pcie_ep_deinit(rcar);
> +
> + return ret;
> }
>
> static void rcar_gen4_remove_dw_pcie_ep(struct rcar_gen4_pcie *rcar)
> {
> dw_pcie_ep_exit(&rcar->dw.ep);
> + rcar_gen4_pcie_ep_deinit(rcar);
> }
>
> /* Common */
>
> --
> 2.25.1
>
On Sat, Feb 24, 2024 at 12:24:08PM +0530, Manivannan Sadhasivam wrote:
> dw_pcie_ep_exit() API is undoing what the dw_pcie_ep_init() API has done
> already (at least partly). But the API name dw_pcie_ep_exit() is not quite
> reflecting that. So let's rename it to dw_pcie_ep_deinit() to make the
> purpose of this API clear. This also aligns with the DWC host driver.
>
> Signed-off-by: Manivannan Sadhasivam <[email protected]>
Reviewed-by: Frank Li <[email protected]>
> ---
> drivers/pci/controller/dwc/pcie-designware-ep.c | 4 ++--
> drivers/pci/controller/dwc/pcie-designware.h | 4 ++--
> drivers/pci/controller/dwc/pcie-rcar-gen4.c | 2 +-
> 3 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
> index d305f9b4cdfe..2b11290aab4c 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
> @@ -564,7 +564,7 @@ int dw_pcie_ep_raise_msix_irq(struct dw_pcie_ep *ep, u8 func_no,
> return 0;
> }
>
> -void dw_pcie_ep_exit(struct dw_pcie_ep *ep)
> +void dw_pcie_ep_deinit(struct dw_pcie_ep *ep)
> {
> struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> struct pci_epc *epc = ep->epc;
> @@ -576,7 +576,7 @@ void dw_pcie_ep_exit(struct dw_pcie_ep *ep)
>
> pci_epc_mem_exit(epc);
> }
> -EXPORT_SYMBOL_GPL(dw_pcie_ep_exit);
> +EXPORT_SYMBOL_GPL(dw_pcie_ep_deinit);
>
> static unsigned int dw_pcie_ep_find_ext_capability(struct dw_pcie *pci, int cap)
> {
> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> index ab7431a37209..61465203bb60 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.h
> +++ b/drivers/pci/controller/dwc/pcie-designware.h
> @@ -671,7 +671,7 @@ void dw_pcie_ep_linkup(struct dw_pcie_ep *ep);
> int dw_pcie_ep_init(struct dw_pcie_ep *ep);
> int dw_pcie_ep_init_complete(struct dw_pcie_ep *ep);
> void dw_pcie_ep_init_notify(struct dw_pcie_ep *ep);
> -void dw_pcie_ep_exit(struct dw_pcie_ep *ep);
> +void dw_pcie_ep_deinit(struct dw_pcie_ep *ep);
> int dw_pcie_ep_raise_intx_irq(struct dw_pcie_ep *ep, u8 func_no);
> int dw_pcie_ep_raise_msi_irq(struct dw_pcie_ep *ep, u8 func_no,
> u8 interrupt_num);
> @@ -701,7 +701,7 @@ static inline void dw_pcie_ep_init_notify(struct dw_pcie_ep *ep)
> {
> }
>
> -static inline void dw_pcie_ep_exit(struct dw_pcie_ep *ep)
> +static inline void dw_pcie_ep_deinit(struct dw_pcie_ep *ep)
> {
> }
>
> diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.c b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> index ac97d594ea47..9d9d22e367bb 100644
> --- a/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> +++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> @@ -430,7 +430,7 @@ static int rcar_gen4_add_dw_pcie_ep(struct rcar_gen4_pcie *rcar)
>
> static void rcar_gen4_remove_dw_pcie_ep(struct rcar_gen4_pcie *rcar)
> {
> - dw_pcie_ep_exit(&rcar->dw.ep);
> + dw_pcie_ep_deinit(&rcar->dw.ep);
> rcar_gen4_pcie_ep_deinit(rcar);
> }
>
>
> --
> 2.25.1
>
On Sat, Feb 24, 2024 at 12:24:09PM +0530, Manivannan Sadhasivam wrote:
> For DWC glue drivers supporting PERST# (currently Qcom and Tegra194), some
> of the DWC resources like eDMA should be cleaned up during the PERST#
> assert time.
>
> So let's introduce a dw_pcie_ep_cleanup() API that could be called by these
> drivers to cleanup the DWC specific resources. Currently, it just removes
> eDMA.
>
> Reported-by: Niklas Cassel <[email protected]>
> Closes: https://lore.kernel.org/linux-pci/ZWYmX8Y%2F7Q9WMxES@x1-carbon
> Signed-off-by: Manivannan Sadhasivam <[email protected]>
Reviewed-by: Frank Li <[email protected]>
> ---
> drivers/pci/controller/dwc/pcie-designware-ep.c | 11 +++++++++--
> drivers/pci/controller/dwc/pcie-designware.h | 5 +++++
> drivers/pci/controller/dwc/pcie-qcom-ep.c | 1 +
> drivers/pci/controller/dwc/pcie-tegra194.c | 2 ++
> 4 files changed, 17 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
> index 2b11290aab4c..1205bfba8310 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
> @@ -564,12 +564,19 @@ int dw_pcie_ep_raise_msix_irq(struct dw_pcie_ep *ep, u8 func_no,
> return 0;
> }
>
> -void dw_pcie_ep_deinit(struct dw_pcie_ep *ep)
> +void dw_pcie_ep_cleanup(struct dw_pcie_ep *ep)
> {
> struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> - struct pci_epc *epc = ep->epc;
>
> dw_pcie_edma_remove(pci);
> +}
> +EXPORT_SYMBOL_GPL(dw_pcie_ep_cleanup);
> +
> +void dw_pcie_ep_deinit(struct dw_pcie_ep *ep)
> +{
> + struct pci_epc *epc = ep->epc;
> +
> + dw_pcie_ep_cleanup(ep);
>
> pci_epc_mem_free_addr(epc, ep->msi_mem_phys, ep->msi_mem,
> epc->mem->window.page_size);
> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> index 61465203bb60..351d2fe3ea4d 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.h
> +++ b/drivers/pci/controller/dwc/pcie-designware.h
> @@ -672,6 +672,7 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep);
> int dw_pcie_ep_init_complete(struct dw_pcie_ep *ep);
> void dw_pcie_ep_init_notify(struct dw_pcie_ep *ep);
> void dw_pcie_ep_deinit(struct dw_pcie_ep *ep);
> +void dw_pcie_ep_cleanup(struct dw_pcie_ep *ep);
> int dw_pcie_ep_raise_intx_irq(struct dw_pcie_ep *ep, u8 func_no);
> int dw_pcie_ep_raise_msi_irq(struct dw_pcie_ep *ep, u8 func_no,
> u8 interrupt_num);
> @@ -705,6 +706,10 @@ static inline void dw_pcie_ep_deinit(struct dw_pcie_ep *ep)
> {
> }
>
> +static inline void dw_pcie_ep_cleanup(struct dw_pcie_ep *ep)
> +{
> +}
> +
> static inline int dw_pcie_ep_raise_intx_irq(struct dw_pcie_ep *ep, u8 func_no)
> {
> return 0;
> diff --git a/drivers/pci/controller/dwc/pcie-qcom-ep.c b/drivers/pci/controller/dwc/pcie-qcom-ep.c
> index 36e5e80cd22f..59b1c0110288 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom-ep.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom-ep.c
> @@ -507,6 +507,7 @@ static void qcom_pcie_perst_assert(struct dw_pcie *pci)
> return;
> }
>
> + dw_pcie_ep_cleanup(&pci->ep);
> qcom_pcie_disable_resources(pcie_ep);
> pcie_ep->link_status = QCOM_PCIE_EP_LINK_DISABLED;
> }
> diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c
> index 7afa9e9aabe2..68bfeed3429b 100644
> --- a/drivers/pci/controller/dwc/pcie-tegra194.c
> +++ b/drivers/pci/controller/dwc/pcie-tegra194.c
> @@ -1715,6 +1715,8 @@ static void pex_ep_event_pex_rst_assert(struct tegra_pcie_dw *pcie)
> if (ret)
> dev_err(pcie->dev, "Failed to go Detect state: %d\n", ret);
>
> + dw_pcie_ep_cleanup(&pcie->pci.ep);
> +
> reset_control_assert(pcie->core_rst);
>
> tegra_pcie_disable_phy(pcie);
>
> --
> 2.25.1
>
On Sat, Feb 24, 2024 at 12:24:10PM +0530, Manivannan Sadhasivam wrote:
> The DWC glue drivers requiring an active reference clock from the PCIe host
> for initializing their PCIe EP core, set a flag called 'core_init_notifier'
> to let DWC driver know that these drivers need a special attention during
> initialization. In these drivers, access to the hw registers (like DBI)
> before receiving the active refclk from host will result in access failure
> and also could cause a whole system hang.
>
> But the current DWC EP driver doesn't honor the requirements of the drivers
> setting 'core_init_notifier' flag and tries to access the DBI registers
> during dw_pcie_ep_init(). This causes the system hang for glue drivers such
> as Tegra194 and Qcom EP as they depend on refclk from host and have set the
> above mentioned flag.
>
> To workaround this issue, users of the affected platforms have to maintain
> the dependency with the PCIe host by booting the PCIe EP after host boot.
> But this won't provide a good user experience, since PCIe EP is _one_ of
> the features of those platforms and it doesn't make sense to delay the
> whole platform booting due to PCIe requiring active refclk.
>
> So to fix this issue, let's move all the DBI access from
> dw_pcie_ep_init() in the DWC EP driver to the dw_pcie_ep_init_complete()
> API. This API will only be called by the drivers setting
> 'core_init_notifier' flag once refclk is received from host. For the rest
> of the drivers that gets the refclk locally, this API will be called
> within dw_pcie_ep_init().
>
> Fixes: e966f7390da9 ("PCI: dwc: Refactor core initialization code for EP mode")
> Co-developed-by: Vidya Sagar <[email protected]>
> Signed-off-by: Vidya Sagar <[email protected]>
> Signed-off-by: Manivannan Sadhasivam <[email protected]>
Reviewed-by: Frank Li <[email protected]>
> ---
> drivers/pci/controller/dwc/pcie-designware-ep.c | 120 ++++++++++++++----------
> 1 file changed, 71 insertions(+), 49 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
> index 1205bfba8310..99d66b0fa59b 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
> @@ -606,11 +606,16 @@ static unsigned int dw_pcie_ep_find_ext_capability(struct dw_pcie *pci, int cap)
> int dw_pcie_ep_init_complete(struct dw_pcie_ep *ep)
> {
> struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> + struct dw_pcie_ep_func *ep_func;
> + struct device *dev = pci->dev;
> + struct pci_epc *epc = ep->epc;
> unsigned int offset, ptm_cap_base;
> unsigned int nbars;
> u8 hdr_type;
> + u8 func_no;
> + int i, ret;
> + void *addr;
> u32 reg;
> - int i;
>
> hdr_type = dw_pcie_readb_dbi(pci, PCI_HEADER_TYPE) &
> PCI_HEADER_TYPE_MASK;
> @@ -621,6 +626,58 @@ int dw_pcie_ep_init_complete(struct dw_pcie_ep *ep)
> return -EIO;
> }
>
> + dw_pcie_version_detect(pci);
> +
> + dw_pcie_iatu_detect(pci);
> +
> + ret = dw_pcie_edma_detect(pci);
> + if (ret)
> + return ret;
> +
> + if (!ep->ib_window_map) {
> + ep->ib_window_map = devm_bitmap_zalloc(dev, pci->num_ib_windows,
> + GFP_KERNEL);
> + if (!ep->ib_window_map)
> + goto err_remove_edma;
> + }
> +
> + if (!ep->ob_window_map) {
> + ep->ob_window_map = devm_bitmap_zalloc(dev, pci->num_ob_windows,
> + GFP_KERNEL);
> + if (!ep->ob_window_map)
> + goto err_remove_edma;
> + }
> +
> + if (!ep->outbound_addr) {
> + addr = devm_kcalloc(dev, pci->num_ob_windows, sizeof(phys_addr_t),
> + GFP_KERNEL);
> + if (!addr)
> + goto err_remove_edma;
> + ep->outbound_addr = addr;
> + }
> +
> + for (func_no = 0; func_no < epc->max_functions; func_no++) {
> +
> + ep_func = dw_pcie_ep_get_func_from_ep(ep, func_no);
> + if (ep_func)
> + continue;
> +
> + ep_func = devm_kzalloc(dev, sizeof(*ep_func), GFP_KERNEL);
> + if (!ep_func)
> + goto err_remove_edma;
> +
> + ep_func->func_no = func_no;
> + ep_func->msi_cap = dw_pcie_ep_find_capability(ep, func_no,
> + PCI_CAP_ID_MSI);
> + ep_func->msix_cap = dw_pcie_ep_find_capability(ep, func_no,
> + PCI_CAP_ID_MSIX);
> +
> + list_add_tail(&ep_func->list, &ep->func_list);
> + }
> +
> + if (ep->ops->init)
> + ep->ops->init(ep);
> +
> offset = dw_pcie_ep_find_ext_capability(pci, PCI_EXT_CAP_ID_REBAR);
> ptm_cap_base = dw_pcie_ep_find_ext_capability(pci, PCI_EXT_CAP_ID_PTM);
>
> @@ -655,14 +712,17 @@ int dw_pcie_ep_init_complete(struct dw_pcie_ep *ep)
> dw_pcie_dbi_ro_wr_dis(pci);
>
> return 0;
> +
> +err_remove_edma:
> + dw_pcie_edma_remove(pci);
> +
> + return ret;
> }
> EXPORT_SYMBOL_GPL(dw_pcie_ep_init_complete);
>
> int dw_pcie_ep_init(struct dw_pcie_ep *ep)
> {
> int ret;
> - void *addr;
> - u8 func_no;
> struct resource *res;
> struct pci_epc *epc;
> struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> @@ -670,7 +730,6 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
> struct platform_device *pdev = to_platform_device(dev);
> struct device_node *np = dev->of_node;
> const struct pci_epc_features *epc_features;
> - struct dw_pcie_ep_func *ep_func;
>
> INIT_LIST_HEAD(&ep->func_list);
>
> @@ -688,26 +747,6 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
> if (ep->ops->pre_init)
> ep->ops->pre_init(ep);
>
> - dw_pcie_version_detect(pci);
> -
> - dw_pcie_iatu_detect(pci);
> -
> - ep->ib_window_map = devm_bitmap_zalloc(dev, pci->num_ib_windows,
> - GFP_KERNEL);
> - if (!ep->ib_window_map)
> - return -ENOMEM;
> -
> - ep->ob_window_map = devm_bitmap_zalloc(dev, pci->num_ob_windows,
> - GFP_KERNEL);
> - if (!ep->ob_window_map)
> - return -ENOMEM;
> -
> - addr = devm_kcalloc(dev, pci->num_ob_windows, sizeof(phys_addr_t),
> - GFP_KERNEL);
> - if (!addr)
> - return -ENOMEM;
> - ep->outbound_addr = addr;
> -
> epc = devm_pci_epc_create(dev, &epc_ops);
> if (IS_ERR(epc)) {
> dev_err(dev, "Failed to create epc device\n");
> @@ -721,23 +760,6 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
> if (ret < 0)
> epc->max_functions = 1;
>
> - for (func_no = 0; func_no < epc->max_functions; func_no++) {
> - ep_func = devm_kzalloc(dev, sizeof(*ep_func), GFP_KERNEL);
> - if (!ep_func)
> - return -ENOMEM;
> -
> - ep_func->func_no = func_no;
> - ep_func->msi_cap = dw_pcie_ep_find_capability(ep, func_no,
> - PCI_CAP_ID_MSI);
> - ep_func->msix_cap = dw_pcie_ep_find_capability(ep, func_no,
> - PCI_CAP_ID_MSIX);
> -
> - list_add_tail(&ep_func->list, &ep->func_list);
> - }
> -
> - if (ep->ops->init)
> - ep->ops->init(ep);
> -
> ret = pci_epc_mem_init(epc, ep->phys_base, ep->addr_size,
> ep->page_size);
> if (ret < 0) {
> @@ -753,25 +775,25 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
> goto err_exit_epc_mem;
> }
>
> - ret = dw_pcie_edma_detect(pci);
> - if (ret)
> - goto err_free_epc_mem;
> -
> if (ep->ops->get_features) {
> epc_features = ep->ops->get_features(ep);
> if (epc_features->core_init_notifier)
> return 0;
> }
>
> + /*
> + * NOTE:- Avoid accessing the hardware (Ex:- DBI space) before this
> + * step as platforms that implement 'core_init_notifier' feature may
> + * not have the hardware ready (i.e. core initialized) for access
> + * (Ex: tegra194). Any hardware access on such platforms result
> + * in system hang.
> + */
> ret = dw_pcie_ep_init_complete(ep);
> if (ret)
> - goto err_remove_edma;
> + goto err_free_epc_mem;
>
> return 0;
>
> -err_remove_edma:
> - dw_pcie_edma_remove(pci);
> -
> err_free_epc_mem:
> pci_epc_mem_free_addr(epc, ep->msi_mem_phys, ep->msi_mem,
> epc->mem->window.page_size);
>
> --
> 2.25.1
>
On Sat, Feb 24, 2024 at 12:24:11PM +0530, Manivannan Sadhasivam wrote:
> The goal of the dw_pcie_ep_init_complete() API is to initialize the DWC
> specific registers post registering the controller with the EP framework.
>
> But the naming doesn't reflect its functionality and causes confusion. So,
> let's rename it to dw_pcie_ep_init_registers() to make it clear that it
> initializes the DWC specific registers.
>
> Signed-off-by: Manivannan Sadhasivam <[email protected]>
Reviewed-by: Frank Li <[email protected]>
> ---
> drivers/pci/controller/dwc/pcie-designware-ep.c | 6 +++---
> drivers/pci/controller/dwc/pcie-designware.h | 4 ++--
> drivers/pci/controller/dwc/pcie-qcom-ep.c | 2 +-
> drivers/pci/controller/dwc/pcie-tegra194.c | 2 +-
> 4 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
> index 99d66b0fa59b..ed1f2afd830a 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
> @@ -603,7 +603,7 @@ static unsigned int dw_pcie_ep_find_ext_capability(struct dw_pcie *pci, int cap)
> return 0;
> }
>
> -int dw_pcie_ep_init_complete(struct dw_pcie_ep *ep)
> +int dw_pcie_ep_init_registers(struct dw_pcie_ep *ep)
> {
> struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> struct dw_pcie_ep_func *ep_func;
> @@ -718,7 +718,7 @@ int dw_pcie_ep_init_complete(struct dw_pcie_ep *ep)
>
> return ret;
> }
> -EXPORT_SYMBOL_GPL(dw_pcie_ep_init_complete);
> +EXPORT_SYMBOL_GPL(dw_pcie_ep_init_registers);
>
> int dw_pcie_ep_init(struct dw_pcie_ep *ep)
> {
> @@ -788,7 +788,7 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
> * (Ex: tegra194). Any hardware access on such platforms result
> * in system hang.
> */
> - ret = dw_pcie_ep_init_complete(ep);
> + ret = dw_pcie_ep_init_registers(ep);
> if (ret)
> goto err_free_epc_mem;
>
> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> index 351d2fe3ea4d..f8e5431a207b 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.h
> +++ b/drivers/pci/controller/dwc/pcie-designware.h
> @@ -669,7 +669,7 @@ static inline void __iomem *dw_pcie_own_conf_map_bus(struct pci_bus *bus,
> #ifdef CONFIG_PCIE_DW_EP
> void dw_pcie_ep_linkup(struct dw_pcie_ep *ep);
> int dw_pcie_ep_init(struct dw_pcie_ep *ep);
> -int dw_pcie_ep_init_complete(struct dw_pcie_ep *ep);
> +int dw_pcie_ep_init_registers(struct dw_pcie_ep *ep);
> void dw_pcie_ep_init_notify(struct dw_pcie_ep *ep);
> void dw_pcie_ep_deinit(struct dw_pcie_ep *ep);
> void dw_pcie_ep_cleanup(struct dw_pcie_ep *ep);
> @@ -693,7 +693,7 @@ static inline int dw_pcie_ep_init(struct dw_pcie_ep *ep)
> return 0;
> }
>
> -static inline int dw_pcie_ep_init_complete(struct dw_pcie_ep *ep)
> +static inline int dw_pcie_ep_init_registers(struct dw_pcie_ep *ep)
> {
> return 0;
> }
> diff --git a/drivers/pci/controller/dwc/pcie-qcom-ep.c b/drivers/pci/controller/dwc/pcie-qcom-ep.c
> index 59b1c0110288..3697b4a944cc 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom-ep.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom-ep.c
> @@ -463,7 +463,7 @@ static int qcom_pcie_perst_deassert(struct dw_pcie *pci)
> PARF_INT_ALL_LINK_UP | PARF_INT_ALL_EDMA;
> writel_relaxed(val, pcie_ep->parf + PARF_INT_ALL_MASK);
>
> - ret = dw_pcie_ep_init_complete(&pcie_ep->pci.ep);
> + ret = dw_pcie_ep_init_registers(&pcie_ep->pci.ep);
> if (ret) {
> dev_err(dev, "Failed to complete initialization: %d\n", ret);
> goto err_disable_resources;
> diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c
> index 68bfeed3429b..264ee76bf008 100644
> --- a/drivers/pci/controller/dwc/pcie-tegra194.c
> +++ b/drivers/pci/controller/dwc/pcie-tegra194.c
> @@ -1897,7 +1897,7 @@ static void pex_ep_event_pex_rst_deassert(struct tegra_pcie_dw *pcie)
> val = (upper_32_bits(ep->msi_mem_phys) & MSIX_ADDR_MATCH_HIGH_OFF_MASK);
> dw_pcie_writel_dbi(pci, MSIX_ADDR_MATCH_HIGH_OFF, val);
>
> - ret = dw_pcie_ep_init_complete(ep);
> + ret = dw_pcie_ep_init_registers(ep);
> if (ret) {
> dev_err(dev, "Failed to complete initialization: %d\n", ret);
> goto fail_init_complete;
>
> --
> 2.25.1
>
On Sat, Feb 24, 2024 at 12:24:12PM +0530, Manivannan Sadhasivam wrote:
> Currently, dw_pcie_ep_init_registers() API is directly called by the glue
> drivers requiring active refclk from host. But for the other drivers, it is
> getting called implicitly by dw_pcie_ep_init(). This is due to the fact
> that this API initializes DWC EP specific registers and that requires an
> active refclk (either from host or generated locally by endpoint itsef).
>
> But, this causes a discrepancy among the glue drivers. So to avoid this
> confusion, let's call this API directly from all glue drivers irrespective
> of refclk dependency. Only difference here is that the drivers requiring
> refclk from host will call this API only after the refclk is received and
> other drivers without refclk dependency will call this API right after
> dw_pcie_ep_init().
>
> This change will also allow us to remove the "core_init_notifier" flag in
> the later commits.
>
> Signed-off-by: Manivannan Sadhasivam <[email protected]>
> ---
> drivers/pci/controller/dwc/pci-dra7xx.c | 7 +++++++
> drivers/pci/controller/dwc/pci-imx6.c | 8 ++++++++
> drivers/pci/controller/dwc/pci-keystone.c | 9 +++++++++
> drivers/pci/controller/dwc/pci-layerscape-ep.c | 7 +++++++
> drivers/pci/controller/dwc/pcie-designware-ep.c | 22 ----------------------
> drivers/pci/controller/dwc/pcie-designware-plat.c | 9 +++++++++
> drivers/pci/controller/dwc/pcie-rcar-gen4.c | 12 +++++++++++-
> drivers/pci/controller/dwc/pcie-uniphier-ep.c | 13 ++++++++++++-
> 8 files changed, 63 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pci-dra7xx.c b/drivers/pci/controller/dwc/pci-dra7xx.c
> index 0e406677060d..395042b29ffc 100644
> --- a/drivers/pci/controller/dwc/pci-dra7xx.c
> +++ b/drivers/pci/controller/dwc/pci-dra7xx.c
> @@ -467,6 +467,13 @@ static int dra7xx_add_pcie_ep(struct dra7xx_pcie *dra7xx,
> return ret;
> }
>
> + ret = dw_pcie_ep_init_registers(ep);
> + if (ret) {
> + dev_err(dev, "Failed to initialize DWC endpoint registers\n");
> + dw_pcie_ep_deinit(ep);
> + return ret;
> + }
> +
> return 0;
> }
>
> diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> index dc2c036ab28c..bfcafa440ddb 100644
> --- a/drivers/pci/controller/dwc/pci-imx6.c
> +++ b/drivers/pci/controller/dwc/pci-imx6.c
> @@ -1136,6 +1136,14 @@ static int imx6_add_pcie_ep(struct imx6_pcie *imx6_pcie,
> dev_err(dev, "failed to initialize endpoint\n");
> return ret;
> }
> +
> + ret = dw_pcie_ep_init_registers(ep);
> + if (ret) {
> + dev_err(dev, "Failed to initialize DWC endpoint registers\n");
> + dw_pcie_ep_deinit(ep);
> + return ret;
> + }
> +
> /* Start LTSSM. */
> imx6_pcie_ltssm_enable(dev);
>
> diff --git a/drivers/pci/controller/dwc/pci-keystone.c b/drivers/pci/controller/dwc/pci-keystone.c
> index c0c62533a3f1..8392894ed286 100644
> --- a/drivers/pci/controller/dwc/pci-keystone.c
> +++ b/drivers/pci/controller/dwc/pci-keystone.c
> @@ -1286,6 +1286,13 @@ static int ks_pcie_probe(struct platform_device *pdev)
> ret = dw_pcie_ep_init(&pci->ep);
> if (ret < 0)
> goto err_get_sync;
> +
> + ret = dw_pcie_ep_init_registers(&pci->ep);
> + if (ret < 0) {
> + dev_err(dev, "Failed to initialize DWC endpoint registers\n");
> + goto err_ep_init;
> + }
> +
> break;
> default:
> dev_err(dev, "INVALID device type %d\n", mode);
> @@ -1295,6 +1302,8 @@ static int ks_pcie_probe(struct platform_device *pdev)
>
> return 0;
>
> +err_ep_init:
> + dw_pcie_ep_deinit(&pci->ep);
> err_get_sync:
> pm_runtime_put(dev);
> pm_runtime_disable(dev);
> diff --git a/drivers/pci/controller/dwc/pci-layerscape-ep.c b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> index 2e398494e7c0..b712fdd06549 100644
> --- a/drivers/pci/controller/dwc/pci-layerscape-ep.c
> +++ b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> @@ -276,6 +276,13 @@ static int __init ls_pcie_ep_probe(struct platform_device *pdev)
> if (ret)
> return ret;
>
> + ret = dw_pcie_ep_init_registers(&pci->ep);
> + if (ret) {
> + dev_err(dev, "Failed to initialize DWC endpoint registers\n");
> + dw_pcie_ep_deinit(&pci->ep);
> + return ret;
> + }
> +
> return ls_pcie_ep_interrupt_init(pcie, pdev);
> }
>
> diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
> index ed1f2afd830a..278bdc9b2269 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
> @@ -729,7 +729,6 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
> struct device *dev = pci->dev;
> struct platform_device *pdev = to_platform_device(dev);
> struct device_node *np = dev->of_node;
> - const struct pci_epc_features *epc_features;
>
> INIT_LIST_HEAD(&ep->func_list);
>
> @@ -775,29 +774,8 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
> goto err_exit_epc_mem;
> }
>
> - if (ep->ops->get_features) {
> - epc_features = ep->ops->get_features(ep);
> - if (epc_features->core_init_notifier)
> - return 0;
> - }
why remove this check?
Frank
> -
> - /*
> - * NOTE:- Avoid accessing the hardware (Ex:- DBI space) before this
> - * step as platforms that implement 'core_init_notifier' feature may
> - * not have the hardware ready (i.e. core initialized) for access
> - * (Ex: tegra194). Any hardware access on such platforms result
> - * in system hang.
> - */
> - ret = dw_pcie_ep_init_registers(ep);
> - if (ret)
> - goto err_free_epc_mem;
> -
> return 0;
>
> -err_free_epc_mem:
> - pci_epc_mem_free_addr(epc, ep->msi_mem_phys, ep->msi_mem,
> - epc->mem->window.page_size);
> -
> err_exit_epc_mem:
> pci_epc_mem_exit(epc);
>
> diff --git a/drivers/pci/controller/dwc/pcie-designware-plat.c b/drivers/pci/controller/dwc/pcie-designware-plat.c
> index 778588b4be70..ca9b22e654cd 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-plat.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-plat.c
> @@ -145,6 +145,15 @@ static int dw_plat_pcie_probe(struct platform_device *pdev)
>
> pci->ep.ops = &pcie_ep_ops;
> ret = dw_pcie_ep_init(&pci->ep);
> + if (ret)
> + return ret;
> +
> + ret = dw_pcie_ep_init_registers(&pci->ep);
> + if (ret) {
> + dev_err(dev, "Failed to initialize DWC endpoint registers\n");
> + dw_pcie_ep_deinit(&pci->ep);
> + }
> +
> break;
> default:
> dev_err(dev, "INVALID device type %d\n", dw_plat_pcie->mode);
> diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.c b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> index 9d9d22e367bb..fb7c03639a53 100644
> --- a/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> +++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> @@ -414,6 +414,7 @@ static const struct dw_pcie_ep_ops pcie_ep_ops = {
> static int rcar_gen4_add_dw_pcie_ep(struct rcar_gen4_pcie *rcar)
> {
> struct dw_pcie_ep *ep = &rcar->dw.ep;
> + struct device *dev = rcar->dw.dev;
> int ret;
>
> if (!IS_ENABLED(CONFIG_PCIE_RCAR_GEN4_EP))
> @@ -422,8 +423,17 @@ static int rcar_gen4_add_dw_pcie_ep(struct rcar_gen4_pcie *rcar)
> ep->ops = &pcie_ep_ops;
>
> ret = dw_pcie_ep_init(ep);
> - if (ret)
> + if (ret) {
> rcar_gen4_pcie_ep_deinit(rcar);
> + return ret;
> + }
> +
> + ret = dw_pcie_ep_init_registers(ep);
> + if (ret) {
> + dev_err(dev, "Failed to initialize DWC endpoint registers\n");
> + dw_pcie_ep_deinit(ep);
> + rcar_gen4_pcie_ep_deinit(rcar);
> + }
>
> return ret;
> }
> diff --git a/drivers/pci/controller/dwc/pcie-uniphier-ep.c b/drivers/pci/controller/dwc/pcie-uniphier-ep.c
> index 3fced0d3e851..82ccaea089be 100644
> --- a/drivers/pci/controller/dwc/pcie-uniphier-ep.c
> +++ b/drivers/pci/controller/dwc/pcie-uniphier-ep.c
> @@ -399,7 +399,18 @@ static int uniphier_pcie_ep_probe(struct platform_device *pdev)
> return ret;
>
> priv->pci.ep.ops = &uniphier_pcie_ep_ops;
> - return dw_pcie_ep_init(&priv->pci.ep);
> + ret = dw_pcie_ep_init(&priv->pci.ep);
> + if (ret)
> + return ret;
> +
> + ret = dw_pcie_ep_init_registers(&priv->pci.ep);
> + if (ret) {
> + dev_err(dev, "Failed to initialize DWC endpoint registers\n");
> + dw_pcie_ep_deinit(&priv->pci.ep);
> + return ret;
> + }
> +
> + return 0;
> }
>
> static const struct uniphier_pcie_ep_soc_data uniphier_pro5_data = {
>
> --
> 2.25.1
>
On Sat, Feb 24, 2024 at 12:24:13PM +0530, Manivannan Sadhasivam wrote:
> "core_init_notifier" flag is set by the glue drivers requiring refclk from
> the host to complete the DWC core initialization. Also, those drivers will
> send a notification to the EPF drivers once the initialization is fully
> completed using the pci_epc_init_notify() API. Only then, the EPF drivers
> will start functioning.
>
> For the rest of the drivers generating refclk locally, EPF drivers will
> start functioning post binding with them. EPF drivers rely on the
> 'core_init_notifier' flag to differentiate between the drivers.
> Unfortunately, this creates two different flows for the EPF drivers.
>
> So to avoid that, let's get rid of the "core_init_notifier" flag and follow
> a single initialization flow for the EPF drivers. This is done by calling
> the dw_pcie_ep_init_notify() from all glue drivers after the completion of
> dw_pcie_ep_init_registers() API. This will allow all the glue drivers to
> send the notification to the EPF drivers once the initialization is fully
> completed.
>
> Only difference here is that, the drivers requiring refclk from host will
> send the notification once refclk is received, while others will send it
> during probe time itself.
>
> Signed-off-by: Manivannan Sadhasivam <[email protected]>
Reviewed-by: Frank Li <[email protected]>
> ---
> drivers/pci/controller/dwc/pci-dra7xx.c | 2 ++
> drivers/pci/controller/dwc/pci-imx6.c | 2 ++
> drivers/pci/controller/dwc/pci-keystone.c | 2 ++
> drivers/pci/controller/dwc/pci-layerscape-ep.c | 2 ++
> drivers/pci/controller/dwc/pcie-designware-plat.c | 2 ++
> drivers/pci/controller/dwc/pcie-qcom-ep.c | 1 -
> drivers/pci/controller/dwc/pcie-rcar-gen4.c | 2 ++
> drivers/pci/controller/dwc/pcie-tegra194.c | 1 -
> drivers/pci/controller/dwc/pcie-uniphier-ep.c | 2 ++
> drivers/pci/endpoint/functions/pci-epf-test.c | 18 +++++-------------
> include/linux/pci-epc.h | 3 ---
> 11 files changed, 19 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pci-dra7xx.c b/drivers/pci/controller/dwc/pci-dra7xx.c
> index 395042b29ffc..d2d17d37d3e0 100644
> --- a/drivers/pci/controller/dwc/pci-dra7xx.c
> +++ b/drivers/pci/controller/dwc/pci-dra7xx.c
> @@ -474,6 +474,8 @@ static int dra7xx_add_pcie_ep(struct dra7xx_pcie *dra7xx,
> return ret;
> }
>
> + dw_pcie_ep_init_notify(ep);
> +
> return 0;
> }
>
> diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> index bfcafa440ddb..894b5de76e3a 100644
> --- a/drivers/pci/controller/dwc/pci-imx6.c
> +++ b/drivers/pci/controller/dwc/pci-imx6.c
> @@ -1144,6 +1144,8 @@ static int imx6_add_pcie_ep(struct imx6_pcie *imx6_pcie,
> return ret;
> }
>
> + dw_pcie_ep_init_notify(ep);
> +
> /* Start LTSSM. */
> imx6_pcie_ltssm_enable(dev);
>
> diff --git a/drivers/pci/controller/dwc/pci-keystone.c b/drivers/pci/controller/dwc/pci-keystone.c
> index 8392894ed286..1d00c5fa14ce 100644
> --- a/drivers/pci/controller/dwc/pci-keystone.c
> +++ b/drivers/pci/controller/dwc/pci-keystone.c
> @@ -1293,6 +1293,8 @@ static int ks_pcie_probe(struct platform_device *pdev)
> goto err_ep_init;
> }
>
> + dw_pcie_ep_init_notify(&pci->ep);
> +
> break;
> default:
> dev_err(dev, "INVALID device type %d\n", mode);
> diff --git a/drivers/pci/controller/dwc/pci-layerscape-ep.c b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> index b712fdd06549..c513598a46d7 100644
> --- a/drivers/pci/controller/dwc/pci-layerscape-ep.c
> +++ b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> @@ -283,6 +283,8 @@ static int __init ls_pcie_ep_probe(struct platform_device *pdev)
> return ret;
> }
>
> + dw_pcie_ep_init_notify(&pci->ep);
> +
> return ls_pcie_ep_interrupt_init(pcie, pdev);
> }
>
> diff --git a/drivers/pci/controller/dwc/pcie-designware-plat.c b/drivers/pci/controller/dwc/pcie-designware-plat.c
> index ca9b22e654cd..8490c5d6ff9f 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-plat.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-plat.c
> @@ -154,6 +154,8 @@ static int dw_plat_pcie_probe(struct platform_device *pdev)
> dw_pcie_ep_deinit(&pci->ep);
> }
>
> + dw_pcie_ep_init_notify(&pci->ep);
> +
> break;
> default:
> dev_err(dev, "INVALID device type %d\n", dw_plat_pcie->mode);
> diff --git a/drivers/pci/controller/dwc/pcie-qcom-ep.c b/drivers/pci/controller/dwc/pcie-qcom-ep.c
> index 3697b4a944cc..2fb8c15e7a91 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom-ep.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom-ep.c
> @@ -775,7 +775,6 @@ static void qcom_pcie_ep_init_debugfs(struct qcom_pcie_ep *pcie_ep)
>
> static const struct pci_epc_features qcom_pcie_epc_features = {
> .linkup_notifier = true,
> - .core_init_notifier = true,
> .msi_capable = true,
> .msix_capable = false,
> .align = SZ_4K,
> diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.c b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> index fb7c03639a53..0448928017f3 100644
> --- a/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> +++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> @@ -435,6 +435,8 @@ static int rcar_gen4_add_dw_pcie_ep(struct rcar_gen4_pcie *rcar)
> rcar_gen4_pcie_ep_deinit(rcar);
> }
>
> + dw_pcie_ep_init_notify(ep);
> +
> return ret;
> }
>
> diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c
> index 264ee76bf008..e02deb31a72d 100644
> --- a/drivers/pci/controller/dwc/pcie-tegra194.c
> +++ b/drivers/pci/controller/dwc/pcie-tegra194.c
> @@ -2006,7 +2006,6 @@ static int tegra_pcie_ep_raise_irq(struct dw_pcie_ep *ep, u8 func_no,
>
> static const struct pci_epc_features tegra_pcie_epc_features = {
> .linkup_notifier = true,
> - .core_init_notifier = true,
> .msi_capable = false,
> .msix_capable = false,
> .reserved_bar = 1 << BAR_2 | 1 << BAR_3 | 1 << BAR_4 | 1 << BAR_5,
> diff --git a/drivers/pci/controller/dwc/pcie-uniphier-ep.c b/drivers/pci/controller/dwc/pcie-uniphier-ep.c
> index 82ccaea089be..eb1d79fdb1f1 100644
> --- a/drivers/pci/controller/dwc/pcie-uniphier-ep.c
> +++ b/drivers/pci/controller/dwc/pcie-uniphier-ep.c
> @@ -410,6 +410,8 @@ static int uniphier_pcie_ep_probe(struct platform_device *pdev)
> return ret;
> }
>
> + dw_pcie_ep_init_notify(&priv->pci.ep);
> +
> return 0;
> }
>
> diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c
> index 18c80002d3bd..fc0282b0d626 100644
> --- a/drivers/pci/endpoint/functions/pci-epf-test.c
> +++ b/drivers/pci/endpoint/functions/pci-epf-test.c
> @@ -753,6 +753,7 @@ static int pci_epf_test_core_init(struct pci_epf *epf)
> const struct pci_epc_features *epc_features;
> struct pci_epc *epc = epf->epc;
> struct device *dev = &epf->dev;
> + bool linkup_notifier = false;
> bool msix_capable = false;
> bool msi_capable = true;
> int ret;
> @@ -795,6 +796,10 @@ static int pci_epf_test_core_init(struct pci_epf *epf)
> }
> }
>
> + linkup_notifier = epc_features->linkup_notifier;
> + if (!linkup_notifier)
> + queue_work(kpcitest_workqueue, &epf_test->cmd_handler.work);
> +
> return 0;
> }
>
> @@ -901,8 +906,6 @@ static int pci_epf_test_bind(struct pci_epf *epf)
> const struct pci_epc_features *epc_features;
> enum pci_barno test_reg_bar = BAR_0;
> struct pci_epc *epc = epf->epc;
> - bool linkup_notifier = false;
> - bool core_init_notifier = false;
>
> if (WARN_ON_ONCE(!epc))
> return -EINVAL;
> @@ -913,8 +916,6 @@ static int pci_epf_test_bind(struct pci_epf *epf)
> return -EOPNOTSUPP;
> }
>
> - linkup_notifier = epc_features->linkup_notifier;
> - core_init_notifier = epc_features->core_init_notifier;
> test_reg_bar = pci_epc_get_first_free_bar(epc_features);
> if (test_reg_bar < 0)
> return -EINVAL;
> @@ -927,21 +928,12 @@ static int pci_epf_test_bind(struct pci_epf *epf)
> if (ret)
> return ret;
>
> - if (!core_init_notifier) {
> - ret = pci_epf_test_core_init(epf);
> - if (ret)
> - return ret;
> - }
> -
> epf_test->dma_supported = true;
>
> ret = pci_epf_test_init_dma_chan(epf_test);
> if (ret)
> epf_test->dma_supported = false;
>
> - if (!linkup_notifier && !core_init_notifier)
> - queue_work(kpcitest_workqueue, &epf_test->cmd_handler.work);
> -
> return 0;
> }
>
> diff --git a/include/linux/pci-epc.h b/include/linux/pci-epc.h
> index 40ea18f5aa02..03d22aed5ac6 100644
> --- a/include/linux/pci-epc.h
> +++ b/include/linux/pci-epc.h
> @@ -148,8 +148,6 @@ struct pci_epc {
> /**
> * struct pci_epc_features - features supported by a EPC device per function
> * @linkup_notifier: indicate if the EPC device can notify EPF driver on link up
> - * @core_init_notifier: indicate cores that can notify about their availability
> - * for initialization
> * @msi_capable: indicate if the endpoint function has MSI capability
> * @msix_capable: indicate if the endpoint function has MSI-X capability
> * @reserved_bar: bitmap to indicate reserved BAR unavailable to function driver
> @@ -159,7 +157,6 @@ struct pci_epc {
> */
> struct pci_epc_features {
> unsigned int linkup_notifier : 1;
> - unsigned int core_init_notifier : 1;
> unsigned int msi_capable : 1;
> unsigned int msix_capable : 1;
> u8 reserved_bar;
>
> --
> 2.25.1
>
On Sat, Feb 24, 2024 at 12:24:14PM +0530, Manivannan Sadhasivam wrote:
> The PCIe link can go to LINK_DOWN state in one of the following scenarios:
>
> 1. Fundamental (PERST#)/hot/warm reset
> 2. Link transition from L2/L3 to L0
From L0 to L2/l3
>
> In those cases, LINK_DOWN causes some non-sticky DWC registers to loose the
> state (like REBAR, PTM_CAP etc...). So the drivers need to reinitialize
> them to function properly once the link comes back again.
>
> This is not a problem for drivers supporting PERST# IRQ, since they can
> reinitialize the registers in the PERST# IRQ callback. But for the drivers
> not supporting PERST#, there is no way they can reinitialize the registers
> other than relying on LINK_DOWN IRQ received when the link goes down. So
> let's add a DWC generic API dw_pcie_ep_linkdown() that reinitializes the
> non-sticky registers and also notifies the EPF drivers about link going
> down.
>
> This API can also be used by the drivers supporting PERST# to handle the
> scenario (2) mentioned above.
>
> Signed-off-by: Manivannan Sadhasivam <[email protected]>
> ---
> drivers/pci/controller/dwc/pcie-designware-ep.c | 111 ++++++++++++++----------
> drivers/pci/controller/dwc/pcie-designware.h | 5 ++
> 2 files changed, 72 insertions(+), 44 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
> index 278bdc9b2269..fed4c2936c78 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
> @@ -14,14 +14,6 @@
> #include <linux/pci-epc.h>
> #include <linux/pci-epf.h>
>
> -void dw_pcie_ep_linkup(struct dw_pcie_ep *ep)
> -{
> - struct pci_epc *epc = ep->epc;
> -
> - pci_epc_linkup(epc);
> -}
> -EXPORT_SYMBOL_GPL(dw_pcie_ep_linkup);
> -
No sure why git remove this block and add these back.
> void dw_pcie_ep_init_notify(struct dw_pcie_ep *ep)
> {
> struct pci_epc *epc = ep->epc;
> @@ -603,19 +595,56 @@ static unsigned int dw_pcie_ep_find_ext_capability(struct dw_pcie *pci, int cap)
> return 0;
> }
>
> +static void dw_pcie_ep_init_non_sticky_registers(struct dw_pcie *pci)
> +{
> + unsigned int offset, ptm_cap_base;
> + unsigned int nbars;
> + u32 reg, i;
> +
> + offset = dw_pcie_ep_find_ext_capability(pci, PCI_EXT_CAP_ID_REBAR);
> + ptm_cap_base = dw_pcie_ep_find_ext_capability(pci, PCI_EXT_CAP_ID_PTM);
> +
> + dw_pcie_dbi_ro_wr_en(pci);
> +
> + if (offset) {
> + reg = dw_pcie_readl_dbi(pci, offset + PCI_REBAR_CTRL);
> + nbars = (reg & PCI_REBAR_CTRL_NBAR_MASK) >>
> + PCI_REBAR_CTRL_NBAR_SHIFT;
> +
> + for (i = 0; i < nbars; i++, offset += PCI_REBAR_CTRL)
> + dw_pcie_writel_dbi(pci, offset + PCI_REBAR_CAP, 0x0);
> + }
> +
> + /*
> + * PTM responder capability can be disabled only after disabling
> + * PTM root capability.
> + */
> + if (ptm_cap_base) {
> + dw_pcie_dbi_ro_wr_en(pci);
> + reg = dw_pcie_readl_dbi(pci, ptm_cap_base + PCI_PTM_CAP);
> + reg &= ~PCI_PTM_CAP_ROOT;
> + dw_pcie_writel_dbi(pci, ptm_cap_base + PCI_PTM_CAP, reg);
> +
> + reg = dw_pcie_readl_dbi(pci, ptm_cap_base + PCI_PTM_CAP);
> + reg &= ~(PCI_PTM_CAP_RES | PCI_PTM_GRANULARITY_MASK);
> + dw_pcie_writel_dbi(pci, ptm_cap_base + PCI_PTM_CAP, reg);
> + dw_pcie_dbi_ro_wr_dis(pci);
> + }
> +
> + dw_pcie_setup(pci);
> + dw_pcie_dbi_ro_wr_dis(pci);
> +}
> +
> int dw_pcie_ep_init_registers(struct dw_pcie_ep *ep)
> {
> struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> struct dw_pcie_ep_func *ep_func;
> struct device *dev = pci->dev;
> struct pci_epc *epc = ep->epc;
> - unsigned int offset, ptm_cap_base;
> - unsigned int nbars;
> u8 hdr_type;
> u8 func_no;
> - int i, ret;
> void *addr;
> - u32 reg;
> + int ret;
>
> hdr_type = dw_pcie_readb_dbi(pci, PCI_HEADER_TYPE) &
> PCI_HEADER_TYPE_MASK;
> @@ -678,38 +707,7 @@ int dw_pcie_ep_init_registers(struct dw_pcie_ep *ep)
> if (ep->ops->init)
> ep->ops->init(ep);
>
> - offset = dw_pcie_ep_find_ext_capability(pci, PCI_EXT_CAP_ID_REBAR);
> - ptm_cap_base = dw_pcie_ep_find_ext_capability(pci, PCI_EXT_CAP_ID_PTM);
> -
> - dw_pcie_dbi_ro_wr_en(pci);
> -
> - if (offset) {
> - reg = dw_pcie_readl_dbi(pci, offset + PCI_REBAR_CTRL);
> - nbars = (reg & PCI_REBAR_CTRL_NBAR_MASK) >>
> - PCI_REBAR_CTRL_NBAR_SHIFT;
> -
> - for (i = 0; i < nbars; i++, offset += PCI_REBAR_CTRL)
> - dw_pcie_writel_dbi(pci, offset + PCI_REBAR_CAP, 0x0);
> - }
> -
> - /*
> - * PTM responder capability can be disabled only after disabling
> - * PTM root capability.
> - */
> - if (ptm_cap_base) {
> - dw_pcie_dbi_ro_wr_en(pci);
> - reg = dw_pcie_readl_dbi(pci, ptm_cap_base + PCI_PTM_CAP);
> - reg &= ~PCI_PTM_CAP_ROOT;
> - dw_pcie_writel_dbi(pci, ptm_cap_base + PCI_PTM_CAP, reg);
> -
> - reg = dw_pcie_readl_dbi(pci, ptm_cap_base + PCI_PTM_CAP);
> - reg &= ~(PCI_PTM_CAP_RES | PCI_PTM_GRANULARITY_MASK);
> - dw_pcie_writel_dbi(pci, ptm_cap_base + PCI_PTM_CAP, reg);
> - dw_pcie_dbi_ro_wr_dis(pci);
> - }
> -
> - dw_pcie_setup(pci);
> - dw_pcie_dbi_ro_wr_dis(pci);
> + dw_pcie_ep_init_non_sticky_registers(pci);
>
> return 0;
>
> @@ -720,6 +718,31 @@ int dw_pcie_ep_init_registers(struct dw_pcie_ep *ep)
> }
> EXPORT_SYMBOL_GPL(dw_pcie_ep_init_registers);
>
> +void dw_pcie_ep_linkup(struct dw_pcie_ep *ep)
> +{
> + struct pci_epc *epc = ep->epc;
> +
> + pci_epc_linkup(epc);
> +}
> +EXPORT_SYMBOL_GPL(dw_pcie_ep_linkup);
> +
> +void dw_pcie_ep_linkdown(struct dw_pcie_ep *ep)
> +{
> + struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> + struct pci_epc *epc = ep->epc;
> +
> + /*
> + * Initialize the non-sticky DWC registers as they would've reset post
> + * LINK_DOWN. This is specifically needed for drivers not supporting
> + * PERST# as they have no way to reinitialize the registers before the
> + * link comes back again.
> + */
> + dw_pcie_ep_init_non_sticky_registers(pci);
> +
> + pci_epc_linkdown(epc);
> +}
> +EXPORT_SYMBOL_GPL(dw_pcie_ep_linkdown);
> +
> int dw_pcie_ep_init(struct dw_pcie_ep *ep)
> {
> int ret;
> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> index f8e5431a207b..152969545b0a 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.h
> +++ b/drivers/pci/controller/dwc/pcie-designware.h
> @@ -668,6 +668,7 @@ static inline void __iomem *dw_pcie_own_conf_map_bus(struct pci_bus *bus,
>
> #ifdef CONFIG_PCIE_DW_EP
> void dw_pcie_ep_linkup(struct dw_pcie_ep *ep);
> +void dw_pcie_ep_linkdown(struct dw_pcie_ep *ep);
> int dw_pcie_ep_init(struct dw_pcie_ep *ep);
> int dw_pcie_ep_init_registers(struct dw_pcie_ep *ep);
> void dw_pcie_ep_init_notify(struct dw_pcie_ep *ep);
> @@ -688,6 +689,10 @@ static inline void dw_pcie_ep_linkup(struct dw_pcie_ep *ep)
> {
> }
>
> +static inline void dw_pcie_ep_linkdown(struct dw_pcie_ep *ep)
> +{
> +}
> +
> static inline int dw_pcie_ep_init(struct dw_pcie_ep *ep)
> {
> return 0;
>
> --
> 2.25.1
>
On Sat, Feb 24, 2024 at 12:24:15PM +0530, Manivannan Sadhasivam wrote:
> Now that the API is available, let's make use of it. It also handles the
> reinitialization of DWC non-sticky registers in addition to sending the
> notification to EPF drivers.
>
> Signed-off-by: Manivannan Sadhasivam <[email protected]>
> ---
> drivers/pci/controller/dwc/pcie-qcom-ep.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-qcom-ep.c b/drivers/pci/controller/dwc/pcie-qcom-ep.c
> index 2fb8c15e7a91..4e45bc4bca45 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom-ep.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom-ep.c
> @@ -640,7 +640,7 @@ static irqreturn_t qcom_pcie_ep_global_irq_thread(int irq, void *data)
> if (FIELD_GET(PARF_INT_ALL_LINK_DOWN, status)) {
> dev_dbg(dev, "Received Linkdown event\n");
> pcie_ep->link_status = QCOM_PCIE_EP_LINK_DOWN;
> - pci_epc_linkdown(pci->ep.epc);
> + dw_pcie_ep_linkdown(&pci->ep);
Suppose pci_epc_linkdown() will call dw_pcie_ep_linkdown() ?
why need direct call dw_pcie_ep_linkdown() here?
Frank
> } else if (FIELD_GET(PARF_INT_ALL_BME, status)) {
> dev_dbg(dev, "Received BME event. Link is enabled!\n");
> pcie_ep->link_status = QCOM_PCIE_EP_LINK_ENABLED;
>
> --
> 2.25.1
>
On Sat, Feb 24, 2024 at 12:24:16PM +0530, Manivannan Sadhasivam wrote:
> All of the APIs are missing the Kernel-doc comments. Hence, add them.
>
> Signed-off-by: Manivannan Sadhasivam <[email protected]>
Reviewed-by: Frank Li <[email protected]>
> ---
> drivers/pci/controller/dwc/pcie-designware-ep.c | 92 +++++++++++++++++++++++++
> 1 file changed, 92 insertions(+)
>
> diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
> index fed4c2936c78..cdcb33a279db 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
> @@ -14,6 +14,11 @@
> #include <linux/pci-epc.h>
> #include <linux/pci-epf.h>
>
> +/**
> + * dw_pcie_ep_init_notify - Notify EPF drivers about EPC initialization
> + * complete
> + * @ep: DWC EP device
> + */
> void dw_pcie_ep_init_notify(struct dw_pcie_ep *ep)
> {
> struct pci_epc *epc = ep->epc;
> @@ -22,6 +27,14 @@ void dw_pcie_ep_init_notify(struct dw_pcie_ep *ep)
> }
> EXPORT_SYMBOL_GPL(dw_pcie_ep_init_notify);
>
> +/**
> + * dw_pcie_ep_get_func_from_ep - Get the struct dw_pcie_ep_func corresponding to
> + * the endpoint function
> + * @ep: DWC EP device
> + * @func_no: Function number of the endpoint device
> + *
> + * Return: struct dw_pcie_ep_func if success, NULL otherwise.
> + */
> struct dw_pcie_ep_func *
> dw_pcie_ep_get_func_from_ep(struct dw_pcie_ep *ep, u8 func_no)
> {
> @@ -52,6 +65,11 @@ static void __dw_pcie_ep_reset_bar(struct dw_pcie *pci, u8 func_no,
> dw_pcie_dbi_ro_wr_dis(pci);
> }
>
> +/**
> + * dw_pcie_ep_reset_bar - Reset endpoint BAR
> + * @pci: DWC PCI device
> + * @bar: BAR number of the endpoint
> + */
> void dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum pci_barno bar)
> {
> u8 func_no, funcs;
> @@ -431,6 +449,13 @@ static const struct pci_epc_ops epc_ops = {
> .get_features = dw_pcie_ep_get_features,
> };
>
> +/**
> + * dw_pcie_ep_raise_intx_irq - Raise INTx IRQ to the host
> + * @ep: DWC EP device
> + * @func_no: Function number of the endpoint
> + *
> + * Return: 0 if success, errono otherwise.
> + */
> int dw_pcie_ep_raise_intx_irq(struct dw_pcie_ep *ep, u8 func_no)
> {
> struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> @@ -442,6 +467,14 @@ int dw_pcie_ep_raise_intx_irq(struct dw_pcie_ep *ep, u8 func_no)
> }
> EXPORT_SYMBOL_GPL(dw_pcie_ep_raise_intx_irq);
>
> +/**
> + * dw_pcie_ep_raise_msi_irq - Raise MSI IRQ to the host
> + * @ep: DWC EP device
> + * @func_no: Function number of the endpoint
> + * @interrupt_num: Interrupt number to be raised
> + *
> + * Return: 0 if success, errono otherwise.
> + */
> int dw_pcie_ep_raise_msi_irq(struct dw_pcie_ep *ep, u8 func_no,
> u8 interrupt_num)
> {
> @@ -490,6 +523,15 @@ int dw_pcie_ep_raise_msi_irq(struct dw_pcie_ep *ep, u8 func_no,
> }
> EXPORT_SYMBOL_GPL(dw_pcie_ep_raise_msi_irq);
>
> +/**
> + * dw_pcie_ep_raise_msix_irq_doorbell - Raise MSIX to the host using Doorbell
> + * method
> + * @ep: DWC EP device
> + * @func_no: Function number of the endpoint device
> + * @interrupt_num: Interrupt number to be raised
> + *
> + * Return: 0 if success, errno otherwise.
> + */
> int dw_pcie_ep_raise_msix_irq_doorbell(struct dw_pcie_ep *ep, u8 func_no,
> u16 interrupt_num)
> {
> @@ -509,6 +551,14 @@ int dw_pcie_ep_raise_msix_irq_doorbell(struct dw_pcie_ep *ep, u8 func_no,
> return 0;
> }
>
> +/**
> + * dw_pcie_ep_raise_msix_irq - Raise MSIX to the host
> + * @ep: DWC EP device
> + * @func_no: Function number of the endpoint device
> + * @interrupt_num: Interrupt number to be raised
> + *
> + * Return: 0 if success, errno otherwise.
> + */
> int dw_pcie_ep_raise_msix_irq(struct dw_pcie_ep *ep, u8 func_no,
> u16 interrupt_num)
> {
> @@ -556,6 +606,12 @@ int dw_pcie_ep_raise_msix_irq(struct dw_pcie_ep *ep, u8 func_no,
> return 0;
> }
>
> +/**
> + * dw_pcie_ep_cleanup - Cleanup DWC EP resources
> + * @ep: DWC EP device
> + *
> + * Cleans up the DWC EP specific resources like eDMA etc...
> + */
> void dw_pcie_ep_cleanup(struct dw_pcie_ep *ep)
> {
> struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> @@ -564,6 +620,13 @@ void dw_pcie_ep_cleanup(struct dw_pcie_ep *ep)
> }
> EXPORT_SYMBOL_GPL(dw_pcie_ep_cleanup);
>
> +/**
> + * dw_pcie_ep_deinit - Deinitialize the endpoint device
> + * @ep: DWC EP device
> + *
> + * Deinitialize the endpoint device. EPC device is not destroyed since that will
> + * taken care by Devres.
> + */
> void dw_pcie_ep_deinit(struct dw_pcie_ep *ep)
> {
> struct pci_epc *epc = ep->epc;
> @@ -635,6 +698,14 @@ static void dw_pcie_ep_init_non_sticky_registers(struct dw_pcie *pci)
> dw_pcie_dbi_ro_wr_dis(pci);
> }
>
> +/**
> + * dw_pcie_ep_init_registers - Initialize DWC EP specific registers
> + * @ep: DWC EP device
> + *
> + * Initialize the registers (CSRs) specific to DWC EP. This API should be called
> + * only when the endpoint receives an active refclk (either from host or
> + * generated locally).
> + */
> int dw_pcie_ep_init_registers(struct dw_pcie_ep *ep)
> {
> struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> @@ -718,6 +789,10 @@ int dw_pcie_ep_init_registers(struct dw_pcie_ep *ep)
> }
> EXPORT_SYMBOL_GPL(dw_pcie_ep_init_registers);
>
> +/**
> + * dw_pcie_ep_linkup - Notify EPF drivers about link up event
> + * @ep: DWC EP device
> + */
> void dw_pcie_ep_linkup(struct dw_pcie_ep *ep)
> {
> struct pci_epc *epc = ep->epc;
> @@ -726,6 +801,14 @@ void dw_pcie_ep_linkup(struct dw_pcie_ep *ep)
> }
> EXPORT_SYMBOL_GPL(dw_pcie_ep_linkup);
>
> +/**
> + * dw_pcie_ep_linkdown - Notify EPF drivers about link down event
> + * @ep: DWC EP device
> + *
> + * Non-sticky registers are also initialized before sending the notification to
> + * the EPF drivers. This is needed since the registers need to be initialized
> + * before the link comes back again.
> + */
> void dw_pcie_ep_linkdown(struct dw_pcie_ep *ep)
> {
> struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> @@ -743,6 +826,15 @@ void dw_pcie_ep_linkdown(struct dw_pcie_ep *ep)
> }
> EXPORT_SYMBOL_GPL(dw_pcie_ep_linkdown);
>
> +/**
> + * dw_pcie_ep_init - Initialize the endpoint device
> + * @ep: DWC EP device
> + *
> + * Initialize the endpoint device. Allocate resources and create the EPC
> + * device with the endpoint framework.
> + *
> + * Return: 0 if success, errno otherwise.
> + */
> int dw_pcie_ep_init(struct dw_pcie_ep *ep)
> {
> int ret;
>
> --
> 2.25.1
>
On Mon, Feb 26, 2024 at 12:18:18PM -0500, Frank Li wrote:
> On Sat, Feb 24, 2024 at 12:24:14PM +0530, Manivannan Sadhasivam wrote:
> > The PCIe link can go to LINK_DOWN state in one of the following scenarios:
> >
> > 1. Fundamental (PERST#)/hot/warm reset
> > 2. Link transition from L2/L3 to L0
>
> From L0 to L2/l3
>
I don't understand what you mean here. Link down won't happen while moving from
L0 to L2/L3, it is the opposite.
> >
> > In those cases, LINK_DOWN causes some non-sticky DWC registers to loose the
> > state (like REBAR, PTM_CAP etc...). So the drivers need to reinitialize
> > them to function properly once the link comes back again.
> >
> > This is not a problem for drivers supporting PERST# IRQ, since they can
> > reinitialize the registers in the PERST# IRQ callback. But for the drivers
> > not supporting PERST#, there is no way they can reinitialize the registers
> > other than relying on LINK_DOWN IRQ received when the link goes down. So
> > let's add a DWC generic API dw_pcie_ep_linkdown() that reinitializes the
> > non-sticky registers and also notifies the EPF drivers about link going
> > down.
> >
> > This API can also be used by the drivers supporting PERST# to handle the
> > scenario (2) mentioned above.
> >
> > Signed-off-by: Manivannan Sadhasivam <[email protected]>
> > ---
> > drivers/pci/controller/dwc/pcie-designware-ep.c | 111 ++++++++++++++----------
> > drivers/pci/controller/dwc/pcie-designware.h | 5 ++
> > 2 files changed, 72 insertions(+), 44 deletions(-)
> >
> > diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
> > index 278bdc9b2269..fed4c2936c78 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
> > +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
> > @@ -14,14 +14,6 @@
> > #include <linux/pci-epc.h>
> > #include <linux/pci-epf.h>
> >
> > -void dw_pcie_ep_linkup(struct dw_pcie_ep *ep)
> > -{
> > - struct pci_epc *epc = ep->epc;
> > -
> > - pci_epc_linkup(epc);
> > -}
> > -EXPORT_SYMBOL_GPL(dw_pcie_ep_linkup);
> > -
>
> No sure why git remove this block and add these back.
>
Because, we are adding dw_pcie_ep_linkdown() API way below and it makes sense to
move this API also to keep it ordered. Maybe I should've described it in commit
message.
- Mani
>
> > void dw_pcie_ep_init_notify(struct dw_pcie_ep *ep)
> > {
> > struct pci_epc *epc = ep->epc;
> > @@ -603,19 +595,56 @@ static unsigned int dw_pcie_ep_find_ext_capability(struct dw_pcie *pci, int cap)
> > return 0;
> > }
> >
> > +static void dw_pcie_ep_init_non_sticky_registers(struct dw_pcie *pci)
> > +{
> > + unsigned int offset, ptm_cap_base;
> > + unsigned int nbars;
> > + u32 reg, i;
> > +
> > + offset = dw_pcie_ep_find_ext_capability(pci, PCI_EXT_CAP_ID_REBAR);
> > + ptm_cap_base = dw_pcie_ep_find_ext_capability(pci, PCI_EXT_CAP_ID_PTM);
> > +
> > + dw_pcie_dbi_ro_wr_en(pci);
> > +
> > + if (offset) {
> > + reg = dw_pcie_readl_dbi(pci, offset + PCI_REBAR_CTRL);
> > + nbars = (reg & PCI_REBAR_CTRL_NBAR_MASK) >>
> > + PCI_REBAR_CTRL_NBAR_SHIFT;
> > +
> > + for (i = 0; i < nbars; i++, offset += PCI_REBAR_CTRL)
> > + dw_pcie_writel_dbi(pci, offset + PCI_REBAR_CAP, 0x0);
> > + }
> > +
> > + /*
> > + * PTM responder capability can be disabled only after disabling
> > + * PTM root capability.
> > + */
> > + if (ptm_cap_base) {
> > + dw_pcie_dbi_ro_wr_en(pci);
> > + reg = dw_pcie_readl_dbi(pci, ptm_cap_base + PCI_PTM_CAP);
> > + reg &= ~PCI_PTM_CAP_ROOT;
> > + dw_pcie_writel_dbi(pci, ptm_cap_base + PCI_PTM_CAP, reg);
> > +
> > + reg = dw_pcie_readl_dbi(pci, ptm_cap_base + PCI_PTM_CAP);
> > + reg &= ~(PCI_PTM_CAP_RES | PCI_PTM_GRANULARITY_MASK);
> > + dw_pcie_writel_dbi(pci, ptm_cap_base + PCI_PTM_CAP, reg);
> > + dw_pcie_dbi_ro_wr_dis(pci);
> > + }
> > +
> > + dw_pcie_setup(pci);
> > + dw_pcie_dbi_ro_wr_dis(pci);
> > +}
> > +
> > int dw_pcie_ep_init_registers(struct dw_pcie_ep *ep)
> > {
> > struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> > struct dw_pcie_ep_func *ep_func;
> > struct device *dev = pci->dev;
> > struct pci_epc *epc = ep->epc;
> > - unsigned int offset, ptm_cap_base;
> > - unsigned int nbars;
> > u8 hdr_type;
> > u8 func_no;
> > - int i, ret;
> > void *addr;
> > - u32 reg;
> > + int ret;
> >
> > hdr_type = dw_pcie_readb_dbi(pci, PCI_HEADER_TYPE) &
> > PCI_HEADER_TYPE_MASK;
> > @@ -678,38 +707,7 @@ int dw_pcie_ep_init_registers(struct dw_pcie_ep *ep)
> > if (ep->ops->init)
> > ep->ops->init(ep);
> >
> > - offset = dw_pcie_ep_find_ext_capability(pci, PCI_EXT_CAP_ID_REBAR);
> > - ptm_cap_base = dw_pcie_ep_find_ext_capability(pci, PCI_EXT_CAP_ID_PTM);
> > -
> > - dw_pcie_dbi_ro_wr_en(pci);
> > -
> > - if (offset) {
> > - reg = dw_pcie_readl_dbi(pci, offset + PCI_REBAR_CTRL);
> > - nbars = (reg & PCI_REBAR_CTRL_NBAR_MASK) >>
> > - PCI_REBAR_CTRL_NBAR_SHIFT;
> > -
> > - for (i = 0; i < nbars; i++, offset += PCI_REBAR_CTRL)
> > - dw_pcie_writel_dbi(pci, offset + PCI_REBAR_CAP, 0x0);
> > - }
> > -
> > - /*
> > - * PTM responder capability can be disabled only after disabling
> > - * PTM root capability.
> > - */
> > - if (ptm_cap_base) {
> > - dw_pcie_dbi_ro_wr_en(pci);
> > - reg = dw_pcie_readl_dbi(pci, ptm_cap_base + PCI_PTM_CAP);
> > - reg &= ~PCI_PTM_CAP_ROOT;
> > - dw_pcie_writel_dbi(pci, ptm_cap_base + PCI_PTM_CAP, reg);
> > -
> > - reg = dw_pcie_readl_dbi(pci, ptm_cap_base + PCI_PTM_CAP);
> > - reg &= ~(PCI_PTM_CAP_RES | PCI_PTM_GRANULARITY_MASK);
> > - dw_pcie_writel_dbi(pci, ptm_cap_base + PCI_PTM_CAP, reg);
> > - dw_pcie_dbi_ro_wr_dis(pci);
> > - }
> > -
> > - dw_pcie_setup(pci);
> > - dw_pcie_dbi_ro_wr_dis(pci);
> > + dw_pcie_ep_init_non_sticky_registers(pci);
> >
> > return 0;
> >
> > @@ -720,6 +718,31 @@ int dw_pcie_ep_init_registers(struct dw_pcie_ep *ep)
> > }
> > EXPORT_SYMBOL_GPL(dw_pcie_ep_init_registers);
> >
> > +void dw_pcie_ep_linkup(struct dw_pcie_ep *ep)
> > +{
> > + struct pci_epc *epc = ep->epc;
> > +
> > + pci_epc_linkup(epc);
> > +}
> > +EXPORT_SYMBOL_GPL(dw_pcie_ep_linkup);
> > +
> > +void dw_pcie_ep_linkdown(struct dw_pcie_ep *ep)
> > +{
> > + struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> > + struct pci_epc *epc = ep->epc;
> > +
> > + /*
> > + * Initialize the non-sticky DWC registers as they would've reset post
> > + * LINK_DOWN. This is specifically needed for drivers not supporting
> > + * PERST# as they have no way to reinitialize the registers before the
> > + * link comes back again.
> > + */
> > + dw_pcie_ep_init_non_sticky_registers(pci);
> > +
> > + pci_epc_linkdown(epc);
> > +}
> > +EXPORT_SYMBOL_GPL(dw_pcie_ep_linkdown);
> > +
> > int dw_pcie_ep_init(struct dw_pcie_ep *ep)
> > {
> > int ret;
> > diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> > index f8e5431a207b..152969545b0a 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware.h
> > +++ b/drivers/pci/controller/dwc/pcie-designware.h
> > @@ -668,6 +668,7 @@ static inline void __iomem *dw_pcie_own_conf_map_bus(struct pci_bus *bus,
> >
> > #ifdef CONFIG_PCIE_DW_EP
> > void dw_pcie_ep_linkup(struct dw_pcie_ep *ep);
> > +void dw_pcie_ep_linkdown(struct dw_pcie_ep *ep);
> > int dw_pcie_ep_init(struct dw_pcie_ep *ep);
> > int dw_pcie_ep_init_registers(struct dw_pcie_ep *ep);
> > void dw_pcie_ep_init_notify(struct dw_pcie_ep *ep);
> > @@ -688,6 +689,10 @@ static inline void dw_pcie_ep_linkup(struct dw_pcie_ep *ep)
> > {
> > }
> >
> > +static inline void dw_pcie_ep_linkdown(struct dw_pcie_ep *ep)
> > +{
> > +}
> > +
> > static inline int dw_pcie_ep_init(struct dw_pcie_ep *ep)
> > {
> > return 0;
> >
> > --
> > 2.25.1
> >
--
மணிவண்ணன் சதாசிவம்
On Mon, Feb 26, 2024 at 12:04:33PM -0500, Frank Li wrote:
> On Sat, Feb 24, 2024 at 12:24:12PM +0530, Manivannan Sadhasivam wrote:
> > Currently, dw_pcie_ep_init_registers() API is directly called by the glue
> > drivers requiring active refclk from host. But for the other drivers, it is
> > getting called implicitly by dw_pcie_ep_init(). This is due to the fact
> > that this API initializes DWC EP specific registers and that requires an
> > active refclk (either from host or generated locally by endpoint itsef).
> >
> > But, this causes a discrepancy among the glue drivers. So to avoid this
> > confusion, let's call this API directly from all glue drivers irrespective
> > of refclk dependency. Only difference here is that the drivers requiring
> > refclk from host will call this API only after the refclk is received and
> > other drivers without refclk dependency will call this API right after
> > dw_pcie_ep_init().
> >
> > This change will also allow us to remove the "core_init_notifier" flag in
> > the later commits.
> >
> > Signed-off-by: Manivannan Sadhasivam <[email protected]>
> > ---
> > drivers/pci/controller/dwc/pci-dra7xx.c | 7 +++++++
> > drivers/pci/controller/dwc/pci-imx6.c | 8 ++++++++
> > drivers/pci/controller/dwc/pci-keystone.c | 9 +++++++++
> > drivers/pci/controller/dwc/pci-layerscape-ep.c | 7 +++++++
> > drivers/pci/controller/dwc/pcie-designware-ep.c | 22 ----------------------
> > drivers/pci/controller/dwc/pcie-designware-plat.c | 9 +++++++++
> > drivers/pci/controller/dwc/pcie-rcar-gen4.c | 12 +++++++++++-
> > drivers/pci/controller/dwc/pcie-uniphier-ep.c | 13 ++++++++++++-
> > 8 files changed, 63 insertions(+), 24 deletions(-)
[...]
> > diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
> > index ed1f2afd830a..278bdc9b2269 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
> > +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
> > @@ -729,7 +729,6 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
> > struct device *dev = pci->dev;
> > struct platform_device *pdev = to_platform_device(dev);
> > struct device_node *np = dev->of_node;
> > - const struct pci_epc_features *epc_features;
> >
> > INIT_LIST_HEAD(&ep->func_list);
> >
> > @@ -775,29 +774,8 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
> > goto err_exit_epc_mem;
> > }
> >
> > - if (ep->ops->get_features) {
> > - epc_features = ep->ops->get_features(ep);
> > - if (epc_features->core_init_notifier)
> > - return 0;
> > - }
>
> why remove this check?
>
There is no point in keeping this check since we are removing the call to
dw_pcie_ep_init_registers() below. But I should've described this change in the
commit message.
- Mani
--
மணிவண்ணன் சதாசிவம்
On Mon, Feb 26, 2024 at 12:20:41PM -0500, Frank Li wrote:
> On Sat, Feb 24, 2024 at 12:24:15PM +0530, Manivannan Sadhasivam wrote:
> > Now that the API is available, let's make use of it. It also handles the
> > reinitialization of DWC non-sticky registers in addition to sending the
> > notification to EPF drivers.
> >
> > Signed-off-by: Manivannan Sadhasivam <[email protected]>
> > ---
> > drivers/pci/controller/dwc/pcie-qcom-ep.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/pci/controller/dwc/pcie-qcom-ep.c b/drivers/pci/controller/dwc/pcie-qcom-ep.c
> > index 2fb8c15e7a91..4e45bc4bca45 100644
> > --- a/drivers/pci/controller/dwc/pcie-qcom-ep.c
> > +++ b/drivers/pci/controller/dwc/pcie-qcom-ep.c
> > @@ -640,7 +640,7 @@ static irqreturn_t qcom_pcie_ep_global_irq_thread(int irq, void *data)
> > if (FIELD_GET(PARF_INT_ALL_LINK_DOWN, status)) {
> > dev_dbg(dev, "Received Linkdown event\n");
> > pcie_ep->link_status = QCOM_PCIE_EP_LINK_DOWN;
> > - pci_epc_linkdown(pci->ep.epc);
> > + dw_pcie_ep_linkdown(&pci->ep);
>
> Suppose pci_epc_linkdown() will call dw_pcie_ep_linkdown() ?
> why need direct call dw_pcie_ep_linkdown() here?
>
I've already justified this in the commit message. Here is the excerpt:
"It also handles the reinitialization of DWC non-sticky registers in addition
to sending the notification to EPF drivers."
- Mani
--
மணிவண்ணன் சதாசிவம்
On Tue, Feb 27, 2024 at 06:00:24PM +0530, Manivannan Sadhasivam wrote:
> On Mon, Feb 26, 2024 at 12:18:18PM -0500, Frank Li wrote:
> > On Sat, Feb 24, 2024 at 12:24:14PM +0530, Manivannan Sadhasivam wrote:
> > > The PCIe link can go to LINK_DOWN state in one of the following scenarios:
> > >
> > > 1. Fundamental (PERST#)/hot/warm reset
> > > 2. Link transition from L2/L3 to L0
> >
> > From L0 to L2/l3
> >
>
> I don't understand what you mean here. Link down won't happen while moving from
> L0 to L2/L3, it is the opposite.
Strange, why there are not linkdown from L0 to L2/l3. But have linkdown
from L2/l3 to L0? when linkup happen? Any document show these?
Frank
>
> > >
> > > In those cases, LINK_DOWN causes some non-sticky DWC registers to loose the
> > > state (like REBAR, PTM_CAP etc...). So the drivers need to reinitialize
> > > them to function properly once the link comes back again.
> > >
> > > This is not a problem for drivers supporting PERST# IRQ, since they can
> > > reinitialize the registers in the PERST# IRQ callback. But for the drivers
> > > not supporting PERST#, there is no way they can reinitialize the registers
> > > other than relying on LINK_DOWN IRQ received when the link goes down. So
> > > let's add a DWC generic API dw_pcie_ep_linkdown() that reinitializes the
> > > non-sticky registers and also notifies the EPF drivers about link going
> > > down.
> > >
> > > This API can also be used by the drivers supporting PERST# to handle the
> > > scenario (2) mentioned above.
> > >
> > > Signed-off-by: Manivannan Sadhasivam <[email protected]>
> > > ---
> > > drivers/pci/controller/dwc/pcie-designware-ep.c | 111 ++++++++++++++----------
> > > drivers/pci/controller/dwc/pcie-designware.h | 5 ++
> > > 2 files changed, 72 insertions(+), 44 deletions(-)
> > >
> > > diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
> > > index 278bdc9b2269..fed4c2936c78 100644
> > > --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
> > > +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
> > > @@ -14,14 +14,6 @@
> > > #include <linux/pci-epc.h>
> > > #include <linux/pci-epf.h>
> > >
> > > -void dw_pcie_ep_linkup(struct dw_pcie_ep *ep)
> > > -{
> > > - struct pci_epc *epc = ep->epc;
> > > -
> > > - pci_epc_linkup(epc);
> > > -}
> > > -EXPORT_SYMBOL_GPL(dw_pcie_ep_linkup);
> > > -
> >
> > No sure why git remove this block and add these back.
> >
>
> Because, we are adding dw_pcie_ep_linkdown() API way below and it makes sense to
> move this API also to keep it ordered. Maybe I should've described it in commit
> message.
>
> - Mani
>
> >
> > > void dw_pcie_ep_init_notify(struct dw_pcie_ep *ep)
> > > {
> > > struct pci_epc *epc = ep->epc;
> > > @@ -603,19 +595,56 @@ static unsigned int dw_pcie_ep_find_ext_capability(struct dw_pcie *pci, int cap)
> > > return 0;
> > > }
> > >
> > > +static void dw_pcie_ep_init_non_sticky_registers(struct dw_pcie *pci)
> > > +{
> > > + unsigned int offset, ptm_cap_base;
> > > + unsigned int nbars;
> > > + u32 reg, i;
> > > +
> > > + offset = dw_pcie_ep_find_ext_capability(pci, PCI_EXT_CAP_ID_REBAR);
> > > + ptm_cap_base = dw_pcie_ep_find_ext_capability(pci, PCI_EXT_CAP_ID_PTM);
> > > +
> > > + dw_pcie_dbi_ro_wr_en(pci);
> > > +
> > > + if (offset) {
> > > + reg = dw_pcie_readl_dbi(pci, offset + PCI_REBAR_CTRL);
> > > + nbars = (reg & PCI_REBAR_CTRL_NBAR_MASK) >>
> > > + PCI_REBAR_CTRL_NBAR_SHIFT;
> > > +
> > > + for (i = 0; i < nbars; i++, offset += PCI_REBAR_CTRL)
> > > + dw_pcie_writel_dbi(pci, offset + PCI_REBAR_CAP, 0x0);
> > > + }
> > > +
> > > + /*
> > > + * PTM responder capability can be disabled only after disabling
> > > + * PTM root capability.
> > > + */
> > > + if (ptm_cap_base) {
> > > + dw_pcie_dbi_ro_wr_en(pci);
> > > + reg = dw_pcie_readl_dbi(pci, ptm_cap_base + PCI_PTM_CAP);
> > > + reg &= ~PCI_PTM_CAP_ROOT;
> > > + dw_pcie_writel_dbi(pci, ptm_cap_base + PCI_PTM_CAP, reg);
> > > +
> > > + reg = dw_pcie_readl_dbi(pci, ptm_cap_base + PCI_PTM_CAP);
> > > + reg &= ~(PCI_PTM_CAP_RES | PCI_PTM_GRANULARITY_MASK);
> > > + dw_pcie_writel_dbi(pci, ptm_cap_base + PCI_PTM_CAP, reg);
> > > + dw_pcie_dbi_ro_wr_dis(pci);
> > > + }
> > > +
> > > + dw_pcie_setup(pci);
> > > + dw_pcie_dbi_ro_wr_dis(pci);
> > > +}
> > > +
> > > int dw_pcie_ep_init_registers(struct dw_pcie_ep *ep)
> > > {
> > > struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> > > struct dw_pcie_ep_func *ep_func;
> > > struct device *dev = pci->dev;
> > > struct pci_epc *epc = ep->epc;
> > > - unsigned int offset, ptm_cap_base;
> > > - unsigned int nbars;
> > > u8 hdr_type;
> > > u8 func_no;
> > > - int i, ret;
> > > void *addr;
> > > - u32 reg;
> > > + int ret;
> > >
> > > hdr_type = dw_pcie_readb_dbi(pci, PCI_HEADER_TYPE) &
> > > PCI_HEADER_TYPE_MASK;
> > > @@ -678,38 +707,7 @@ int dw_pcie_ep_init_registers(struct dw_pcie_ep *ep)
> > > if (ep->ops->init)
> > > ep->ops->init(ep);
> > >
> > > - offset = dw_pcie_ep_find_ext_capability(pci, PCI_EXT_CAP_ID_REBAR);
> > > - ptm_cap_base = dw_pcie_ep_find_ext_capability(pci, PCI_EXT_CAP_ID_PTM);
> > > -
> > > - dw_pcie_dbi_ro_wr_en(pci);
> > > -
> > > - if (offset) {
> > > - reg = dw_pcie_readl_dbi(pci, offset + PCI_REBAR_CTRL);
> > > - nbars = (reg & PCI_REBAR_CTRL_NBAR_MASK) >>
> > > - PCI_REBAR_CTRL_NBAR_SHIFT;
> > > -
> > > - for (i = 0; i < nbars; i++, offset += PCI_REBAR_CTRL)
> > > - dw_pcie_writel_dbi(pci, offset + PCI_REBAR_CAP, 0x0);
> > > - }
> > > -
> > > - /*
> > > - * PTM responder capability can be disabled only after disabling
> > > - * PTM root capability.
> > > - */
> > > - if (ptm_cap_base) {
> > > - dw_pcie_dbi_ro_wr_en(pci);
> > > - reg = dw_pcie_readl_dbi(pci, ptm_cap_base + PCI_PTM_CAP);
> > > - reg &= ~PCI_PTM_CAP_ROOT;
> > > - dw_pcie_writel_dbi(pci, ptm_cap_base + PCI_PTM_CAP, reg);
> > > -
> > > - reg = dw_pcie_readl_dbi(pci, ptm_cap_base + PCI_PTM_CAP);
> > > - reg &= ~(PCI_PTM_CAP_RES | PCI_PTM_GRANULARITY_MASK);
> > > - dw_pcie_writel_dbi(pci, ptm_cap_base + PCI_PTM_CAP, reg);
> > > - dw_pcie_dbi_ro_wr_dis(pci);
> > > - }
> > > -
> > > - dw_pcie_setup(pci);
> > > - dw_pcie_dbi_ro_wr_dis(pci);
> > > + dw_pcie_ep_init_non_sticky_registers(pci);
> > >
> > > return 0;
> > >
> > > @@ -720,6 +718,31 @@ int dw_pcie_ep_init_registers(struct dw_pcie_ep *ep)
> > > }
> > > EXPORT_SYMBOL_GPL(dw_pcie_ep_init_registers);
> > >
> > > +void dw_pcie_ep_linkup(struct dw_pcie_ep *ep)
> > > +{
> > > + struct pci_epc *epc = ep->epc;
> > > +
> > > + pci_epc_linkup(epc);
> > > +}
> > > +EXPORT_SYMBOL_GPL(dw_pcie_ep_linkup);
> > > +
> > > +void dw_pcie_ep_linkdown(struct dw_pcie_ep *ep)
> > > +{
> > > + struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> > > + struct pci_epc *epc = ep->epc;
> > > +
> > > + /*
> > > + * Initialize the non-sticky DWC registers as they would've reset post
> > > + * LINK_DOWN. This is specifically needed for drivers not supporting
> > > + * PERST# as they have no way to reinitialize the registers before the
> > > + * link comes back again.
> > > + */
> > > + dw_pcie_ep_init_non_sticky_registers(pci);
> > > +
> > > + pci_epc_linkdown(epc);
> > > +}
> > > +EXPORT_SYMBOL_GPL(dw_pcie_ep_linkdown);
> > > +
> > > int dw_pcie_ep_init(struct dw_pcie_ep *ep)
> > > {
> > > int ret;
> > > diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> > > index f8e5431a207b..152969545b0a 100644
> > > --- a/drivers/pci/controller/dwc/pcie-designware.h
> > > +++ b/drivers/pci/controller/dwc/pcie-designware.h
> > > @@ -668,6 +668,7 @@ static inline void __iomem *dw_pcie_own_conf_map_bus(struct pci_bus *bus,
> > >
> > > #ifdef CONFIG_PCIE_DW_EP
> > > void dw_pcie_ep_linkup(struct dw_pcie_ep *ep);
> > > +void dw_pcie_ep_linkdown(struct dw_pcie_ep *ep);
> > > int dw_pcie_ep_init(struct dw_pcie_ep *ep);
> > > int dw_pcie_ep_init_registers(struct dw_pcie_ep *ep);
> > > void dw_pcie_ep_init_notify(struct dw_pcie_ep *ep);
> > > @@ -688,6 +689,10 @@ static inline void dw_pcie_ep_linkup(struct dw_pcie_ep *ep)
> > > {
> > > }
> > >
> > > +static inline void dw_pcie_ep_linkdown(struct dw_pcie_ep *ep)
> > > +{
> > > +}
> > > +
> > > static inline int dw_pcie_ep_init(struct dw_pcie_ep *ep)
> > > {
> > > return 0;
> > >
> > > --
> > > 2.25.1
> > >
>
> --
> மணிவண்ணன் சதாசிவம்
On Tue, Feb 27, 2024 at 05:51:41PM +0530, Manivannan Sadhasivam wrote:
> On Mon, Feb 26, 2024 at 12:04:33PM -0500, Frank Li wrote:
> > On Sat, Feb 24, 2024 at 12:24:12PM +0530, Manivannan Sadhasivam wrote:
> > > Currently, dw_pcie_ep_init_registers() API is directly called by the glue
> > > drivers requiring active refclk from host. But for the other drivers, it is
> > > getting called implicitly by dw_pcie_ep_init(). This is due to the fact
> > > that this API initializes DWC EP specific registers and that requires an
> > > active refclk (either from host or generated locally by endpoint itsef).
> > >
> > > But, this causes a discrepancy among the glue drivers. So to avoid this
> > > confusion, let's call this API directly from all glue drivers irrespective
> > > of refclk dependency. Only difference here is that the drivers requiring
> > > refclk from host will call this API only after the refclk is received and
> > > other drivers without refclk dependency will call this API right after
> > > dw_pcie_ep_init().
> > >
> > > This change will also allow us to remove the "core_init_notifier" flag in
> > > the later commits.
> > >
> > > Signed-off-by: Manivannan Sadhasivam <[email protected]>
> > > ---
> > > drivers/pci/controller/dwc/pci-dra7xx.c | 7 +++++++
> > > drivers/pci/controller/dwc/pci-imx6.c | 8 ++++++++
> > > drivers/pci/controller/dwc/pci-keystone.c | 9 +++++++++
> > > drivers/pci/controller/dwc/pci-layerscape-ep.c | 7 +++++++
> > > drivers/pci/controller/dwc/pcie-designware-ep.c | 22 ----------------------
> > > drivers/pci/controller/dwc/pcie-designware-plat.c | 9 +++++++++
> > > drivers/pci/controller/dwc/pcie-rcar-gen4.c | 12 +++++++++++-
> > > drivers/pci/controller/dwc/pcie-uniphier-ep.c | 13 ++++++++++++-
> > > 8 files changed, 63 insertions(+), 24 deletions(-)
>
> [...]
>
> > > diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
> > > index ed1f2afd830a..278bdc9b2269 100644
> > > --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
> > > +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
> > > @@ -729,7 +729,6 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
> > > struct device *dev = pci->dev;
> > > struct platform_device *pdev = to_platform_device(dev);
> > > struct device_node *np = dev->of_node;
> > > - const struct pci_epc_features *epc_features;
> > >
> > > INIT_LIST_HEAD(&ep->func_list);
> > >
> > > @@ -775,29 +774,8 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
> > > goto err_exit_epc_mem;
> > > }
> > >
> > > - if (ep->ops->get_features) {
> > > - epc_features = ep->ops->get_features(ep);
> > > - if (epc_features->core_init_notifier)
> > > - return 0;
> > > - }
> >
> > why remove this check?
> >
>
> There is no point in keeping this check since we are removing the call to
> dw_pcie_ep_init_registers() below. But I should've described this change in the
> commit message.
Sperated patch will be helpful. This clean up does not related with other
change.
Frank
>
> - Mani
>
> --
> மணிவண்ணன் சதாசிவம்
On Tue, Feb 27, 2024 at 06:02:30PM +0530, Manivannan Sadhasivam wrote:
> On Mon, Feb 26, 2024 at 12:20:41PM -0500, Frank Li wrote:
> > On Sat, Feb 24, 2024 at 12:24:15PM +0530, Manivannan Sadhasivam wrote:
> > > Now that the API is available, let's make use of it. It also handles the
> > > reinitialization of DWC non-sticky registers in addition to sending the
> > > notification to EPF drivers.
> > >
> > > Signed-off-by: Manivannan Sadhasivam <[email protected]>
> > > ---
> > > drivers/pci/controller/dwc/pcie-qcom-ep.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/pci/controller/dwc/pcie-qcom-ep.c b/drivers/pci/controller/dwc/pcie-qcom-ep.c
> > > index 2fb8c15e7a91..4e45bc4bca45 100644
> > > --- a/drivers/pci/controller/dwc/pcie-qcom-ep.c
> > > +++ b/drivers/pci/controller/dwc/pcie-qcom-ep.c
> > > @@ -640,7 +640,7 @@ static irqreturn_t qcom_pcie_ep_global_irq_thread(int irq, void *data)
> > > if (FIELD_GET(PARF_INT_ALL_LINK_DOWN, status)) {
> > > dev_dbg(dev, "Received Linkdown event\n");
> > > pcie_ep->link_status = QCOM_PCIE_EP_LINK_DOWN;
> > > - pci_epc_linkdown(pci->ep.epc);
> > > + dw_pcie_ep_linkdown(&pci->ep);
> >
> > Suppose pci_epc_linkdown() will call dw_pcie_ep_linkdown() ?
> > why need direct call dw_pcie_ep_linkdown() here?
> >
>
> I've already justified this in the commit message. Here is the excerpt:
>
> "It also handles the reinitialization of DWC non-sticky registers in addition
> to sending the notification to EPF drivers."
API function name is too similar. It is hard to know difference from API
naming. It'd better to know what function do from function name.
Frank
>
> - Mani
>
> --
> மணிவண்ணன் சதாசிவம்
On Tue, Feb 27, 2024 at 12:28:41PM -0500, Frank Li wrote:
> On Tue, Feb 27, 2024 at 05:51:41PM +0530, Manivannan Sadhasivam wrote:
> > On Mon, Feb 26, 2024 at 12:04:33PM -0500, Frank Li wrote:
> > > On Sat, Feb 24, 2024 at 12:24:12PM +0530, Manivannan Sadhasivam wrote:
> > > > Currently, dw_pcie_ep_init_registers() API is directly called by the glue
> > > > drivers requiring active refclk from host. But for the other drivers, it is
> > > > getting called implicitly by dw_pcie_ep_init(). This is due to the fact
> > > > that this API initializes DWC EP specific registers and that requires an
> > > > active refclk (either from host or generated locally by endpoint itsef).
> > > >
> > > > But, this causes a discrepancy among the glue drivers. So to avoid this
> > > > confusion, let's call this API directly from all glue drivers irrespective
> > > > of refclk dependency. Only difference here is that the drivers requiring
> > > > refclk from host will call this API only after the refclk is received and
> > > > other drivers without refclk dependency will call this API right after
> > > > dw_pcie_ep_init().
> > > >
> > > > This change will also allow us to remove the "core_init_notifier" flag in
> > > > the later commits.
> > > >
> > > > Signed-off-by: Manivannan Sadhasivam <[email protected]>
> > > > ---
> > > > drivers/pci/controller/dwc/pci-dra7xx.c | 7 +++++++
> > > > drivers/pci/controller/dwc/pci-imx6.c | 8 ++++++++
> > > > drivers/pci/controller/dwc/pci-keystone.c | 9 +++++++++
> > > > drivers/pci/controller/dwc/pci-layerscape-ep.c | 7 +++++++
> > > > drivers/pci/controller/dwc/pcie-designware-ep.c | 22 ----------------------
> > > > drivers/pci/controller/dwc/pcie-designware-plat.c | 9 +++++++++
> > > > drivers/pci/controller/dwc/pcie-rcar-gen4.c | 12 +++++++++++-
> > > > drivers/pci/controller/dwc/pcie-uniphier-ep.c | 13 ++++++++++++-
> > > > 8 files changed, 63 insertions(+), 24 deletions(-)
> >
> > [...]
> >
> > > > diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
> > > > index ed1f2afd830a..278bdc9b2269 100644
> > > > --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
> > > > +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
> > > > @@ -729,7 +729,6 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
> > > > struct device *dev = pci->dev;
> > > > struct platform_device *pdev = to_platform_device(dev);
> > > > struct device_node *np = dev->of_node;
> > > > - const struct pci_epc_features *epc_features;
> > > >
> > > > INIT_LIST_HEAD(&ep->func_list);
> > > >
> > > > @@ -775,29 +774,8 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
> > > > goto err_exit_epc_mem;
> > > > }
> > > >
> > > > - if (ep->ops->get_features) {
> > > > - epc_features = ep->ops->get_features(ep);
> > > > - if (epc_features->core_init_notifier)
> > > > - return 0;
> > > > - }
> > >
> > > why remove this check?
> > >
> >
> > There is no point in keeping this check since we are removing the call to
> > dw_pcie_ep_init_registers() below. But I should've described this change in the
> > commit message.
>
> Sperated patch will be helpful. This clean up does not related with other
> change.
>
Well this is not a generic cleanup that could be moved to a separate patch. Due
to the changes in this patch, the use of the flag becomes redundant. So it has
to removed here itself.
- Mani
--
மணிவண்ணன் சதாசிவம்
On Tue, Feb 27, 2024 at 12:34:15PM -0500, Frank Li wrote:
> On Tue, Feb 27, 2024 at 06:02:30PM +0530, Manivannan Sadhasivam wrote:
> > On Mon, Feb 26, 2024 at 12:20:41PM -0500, Frank Li wrote:
> > > On Sat, Feb 24, 2024 at 12:24:15PM +0530, Manivannan Sadhasivam wrote:
> > > > Now that the API is available, let's make use of it. It also handles the
> > > > reinitialization of DWC non-sticky registers in addition to sending the
> > > > notification to EPF drivers.
> > > >
> > > > Signed-off-by: Manivannan Sadhasivam <[email protected]>
> > > > ---
> > > > drivers/pci/controller/dwc/pcie-qcom-ep.c | 2 +-
> > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/pci/controller/dwc/pcie-qcom-ep.c b/drivers/pci/controller/dwc/pcie-qcom-ep.c
> > > > index 2fb8c15e7a91..4e45bc4bca45 100644
> > > > --- a/drivers/pci/controller/dwc/pcie-qcom-ep.c
> > > > +++ b/drivers/pci/controller/dwc/pcie-qcom-ep.c
> > > > @@ -640,7 +640,7 @@ static irqreturn_t qcom_pcie_ep_global_irq_thread(int irq, void *data)
> > > > if (FIELD_GET(PARF_INT_ALL_LINK_DOWN, status)) {
> > > > dev_dbg(dev, "Received Linkdown event\n");
> > > > pcie_ep->link_status = QCOM_PCIE_EP_LINK_DOWN;
> > > > - pci_epc_linkdown(pci->ep.epc);
> > > > + dw_pcie_ep_linkdown(&pci->ep);
> > >
> > > Suppose pci_epc_linkdown() will call dw_pcie_ep_linkdown() ?
> > > why need direct call dw_pcie_ep_linkdown() here?
> > >
> >
> > I've already justified this in the commit message. Here is the excerpt:
> >
> > "It also handles the reinitialization of DWC non-sticky registers in addition
> > to sending the notification to EPF drivers."
>
> API function name is too similar. It is hard to know difference from API
> naming. It'd better to know what function do from function name.
>
In reality we cannot name a function based on everything it does. The naming is
mostly based on what is the primary motive of the API and here it is handling
Link down event. Maybe dw_pcie_ep_handle_linkdown() would be an apt one, but
that's out of scope of this series (since changing that would also require
changes to other similar APIs).
- Mani
> Frank
> >
> > - Mani
> >
> > --
> > மணிவண்ணன் சதாசிவம்
--
மணிவண்ணன் சதாசிவம்
On Tue, Feb 27, 2024 at 12:26:05PM -0500, Frank Li wrote:
> On Tue, Feb 27, 2024 at 06:00:24PM +0530, Manivannan Sadhasivam wrote:
> > On Mon, Feb 26, 2024 at 12:18:18PM -0500, Frank Li wrote:
> > > On Sat, Feb 24, 2024 at 12:24:14PM +0530, Manivannan Sadhasivam wrote:
> > > > The PCIe link can go to LINK_DOWN state in one of the following scenarios:
> > > >
> > > > 1. Fundamental (PERST#)/hot/warm reset
> > > > 2. Link transition from L2/L3 to L0
> > >
> > > From L0 to L2/l3
> > >
> >
> > I don't understand what you mean here. Link down won't happen while moving from
> > L0 to L2/L3, it is the opposite.
>
> Strange, why there are not linkdown from L0 to L2/l3. But have linkdown
> from L2/l3 to L0? when linkup happen? Any document show these?
>
Refer PCIe Spec 5.0, Figure 5-1 Link Power Management State Flow Diagram.
- Mani
> Frank
>
> >
> > > >
> > > > In those cases, LINK_DOWN causes some non-sticky DWC registers to loose the
> > > > state (like REBAR, PTM_CAP etc...). So the drivers need to reinitialize
> > > > them to function properly once the link comes back again.
> > > >
> > > > This is not a problem for drivers supporting PERST# IRQ, since they can
> > > > reinitialize the registers in the PERST# IRQ callback. But for the drivers
> > > > not supporting PERST#, there is no way they can reinitialize the registers
> > > > other than relying on LINK_DOWN IRQ received when the link goes down. So
> > > > let's add a DWC generic API dw_pcie_ep_linkdown() that reinitializes the
> > > > non-sticky registers and also notifies the EPF drivers about link going
> > > > down.
> > > >
> > > > This API can also be used by the drivers supporting PERST# to handle the
> > > > scenario (2) mentioned above.
> > > >
> > > > Signed-off-by: Manivannan Sadhasivam <[email protected]>
> > > > ---
> > > > drivers/pci/controller/dwc/pcie-designware-ep.c | 111 ++++++++++++++----------
> > > > drivers/pci/controller/dwc/pcie-designware.h | 5 ++
> > > > 2 files changed, 72 insertions(+), 44 deletions(-)
> > > >
> > > > diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
> > > > index 278bdc9b2269..fed4c2936c78 100644
> > > > --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
> > > > +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
> > > > @@ -14,14 +14,6 @@
> > > > #include <linux/pci-epc.h>
> > > > #include <linux/pci-epf.h>
> > > >
> > > > -void dw_pcie_ep_linkup(struct dw_pcie_ep *ep)
> > > > -{
> > > > - struct pci_epc *epc = ep->epc;
> > > > -
> > > > - pci_epc_linkup(epc);
> > > > -}
> > > > -EXPORT_SYMBOL_GPL(dw_pcie_ep_linkup);
> > > > -
> > >
> > > No sure why git remove this block and add these back.
> > >
> >
> > Because, we are adding dw_pcie_ep_linkdown() API way below and it makes sense to
> > move this API also to keep it ordered. Maybe I should've described it in commit
> > message.
> >
> > - Mani
> >
> > >
> > > > void dw_pcie_ep_init_notify(struct dw_pcie_ep *ep)
> > > > {
> > > > struct pci_epc *epc = ep->epc;
> > > > @@ -603,19 +595,56 @@ static unsigned int dw_pcie_ep_find_ext_capability(struct dw_pcie *pci, int cap)
> > > > return 0;
> > > > }
> > > >
> > > > +static void dw_pcie_ep_init_non_sticky_registers(struct dw_pcie *pci)
> > > > +{
> > > > + unsigned int offset, ptm_cap_base;
> > > > + unsigned int nbars;
> > > > + u32 reg, i;
> > > > +
> > > > + offset = dw_pcie_ep_find_ext_capability(pci, PCI_EXT_CAP_ID_REBAR);
> > > > + ptm_cap_base = dw_pcie_ep_find_ext_capability(pci, PCI_EXT_CAP_ID_PTM);
> > > > +
> > > > + dw_pcie_dbi_ro_wr_en(pci);
> > > > +
> > > > + if (offset) {
> > > > + reg = dw_pcie_readl_dbi(pci, offset + PCI_REBAR_CTRL);
> > > > + nbars = (reg & PCI_REBAR_CTRL_NBAR_MASK) >>
> > > > + PCI_REBAR_CTRL_NBAR_SHIFT;
> > > > +
> > > > + for (i = 0; i < nbars; i++, offset += PCI_REBAR_CTRL)
> > > > + dw_pcie_writel_dbi(pci, offset + PCI_REBAR_CAP, 0x0);
> > > > + }
> > > > +
> > > > + /*
> > > > + * PTM responder capability can be disabled only after disabling
> > > > + * PTM root capability.
> > > > + */
> > > > + if (ptm_cap_base) {
> > > > + dw_pcie_dbi_ro_wr_en(pci);
> > > > + reg = dw_pcie_readl_dbi(pci, ptm_cap_base + PCI_PTM_CAP);
> > > > + reg &= ~PCI_PTM_CAP_ROOT;
> > > > + dw_pcie_writel_dbi(pci, ptm_cap_base + PCI_PTM_CAP, reg);
> > > > +
> > > > + reg = dw_pcie_readl_dbi(pci, ptm_cap_base + PCI_PTM_CAP);
> > > > + reg &= ~(PCI_PTM_CAP_RES | PCI_PTM_GRANULARITY_MASK);
> > > > + dw_pcie_writel_dbi(pci, ptm_cap_base + PCI_PTM_CAP, reg);
> > > > + dw_pcie_dbi_ro_wr_dis(pci);
> > > > + }
> > > > +
> > > > + dw_pcie_setup(pci);
> > > > + dw_pcie_dbi_ro_wr_dis(pci);
> > > > +}
> > > > +
> > > > int dw_pcie_ep_init_registers(struct dw_pcie_ep *ep)
> > > > {
> > > > struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> > > > struct dw_pcie_ep_func *ep_func;
> > > > struct device *dev = pci->dev;
> > > > struct pci_epc *epc = ep->epc;
> > > > - unsigned int offset, ptm_cap_base;
> > > > - unsigned int nbars;
> > > > u8 hdr_type;
> > > > u8 func_no;
> > > > - int i, ret;
> > > > void *addr;
> > > > - u32 reg;
> > > > + int ret;
> > > >
> > > > hdr_type = dw_pcie_readb_dbi(pci, PCI_HEADER_TYPE) &
> > > > PCI_HEADER_TYPE_MASK;
> > > > @@ -678,38 +707,7 @@ int dw_pcie_ep_init_registers(struct dw_pcie_ep *ep)
> > > > if (ep->ops->init)
> > > > ep->ops->init(ep);
> > > >
> > > > - offset = dw_pcie_ep_find_ext_capability(pci, PCI_EXT_CAP_ID_REBAR);
> > > > - ptm_cap_base = dw_pcie_ep_find_ext_capability(pci, PCI_EXT_CAP_ID_PTM);
> > > > -
> > > > - dw_pcie_dbi_ro_wr_en(pci);
> > > > -
> > > > - if (offset) {
> > > > - reg = dw_pcie_readl_dbi(pci, offset + PCI_REBAR_CTRL);
> > > > - nbars = (reg & PCI_REBAR_CTRL_NBAR_MASK) >>
> > > > - PCI_REBAR_CTRL_NBAR_SHIFT;
> > > > -
> > > > - for (i = 0; i < nbars; i++, offset += PCI_REBAR_CTRL)
> > > > - dw_pcie_writel_dbi(pci, offset + PCI_REBAR_CAP, 0x0);
> > > > - }
> > > > -
> > > > - /*
> > > > - * PTM responder capability can be disabled only after disabling
> > > > - * PTM root capability.
> > > > - */
> > > > - if (ptm_cap_base) {
> > > > - dw_pcie_dbi_ro_wr_en(pci);
> > > > - reg = dw_pcie_readl_dbi(pci, ptm_cap_base + PCI_PTM_CAP);
> > > > - reg &= ~PCI_PTM_CAP_ROOT;
> > > > - dw_pcie_writel_dbi(pci, ptm_cap_base + PCI_PTM_CAP, reg);
> > > > -
> > > > - reg = dw_pcie_readl_dbi(pci, ptm_cap_base + PCI_PTM_CAP);
> > > > - reg &= ~(PCI_PTM_CAP_RES | PCI_PTM_GRANULARITY_MASK);
> > > > - dw_pcie_writel_dbi(pci, ptm_cap_base + PCI_PTM_CAP, reg);
> > > > - dw_pcie_dbi_ro_wr_dis(pci);
> > > > - }
> > > > -
> > > > - dw_pcie_setup(pci);
> > > > - dw_pcie_dbi_ro_wr_dis(pci);
> > > > + dw_pcie_ep_init_non_sticky_registers(pci);
> > > >
> > > > return 0;
> > > >
> > > > @@ -720,6 +718,31 @@ int dw_pcie_ep_init_registers(struct dw_pcie_ep *ep)
> > > > }
> > > > EXPORT_SYMBOL_GPL(dw_pcie_ep_init_registers);
> > > >
> > > > +void dw_pcie_ep_linkup(struct dw_pcie_ep *ep)
> > > > +{
> > > > + struct pci_epc *epc = ep->epc;
> > > > +
> > > > + pci_epc_linkup(epc);
> > > > +}
> > > > +EXPORT_SYMBOL_GPL(dw_pcie_ep_linkup);
> > > > +
> > > > +void dw_pcie_ep_linkdown(struct dw_pcie_ep *ep)
> > > > +{
> > > > + struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> > > > + struct pci_epc *epc = ep->epc;
> > > > +
> > > > + /*
> > > > + * Initialize the non-sticky DWC registers as they would've reset post
> > > > + * LINK_DOWN. This is specifically needed for drivers not supporting
> > > > + * PERST# as they have no way to reinitialize the registers before the
> > > > + * link comes back again.
> > > > + */
> > > > + dw_pcie_ep_init_non_sticky_registers(pci);
> > > > +
> > > > + pci_epc_linkdown(epc);
> > > > +}
> > > > +EXPORT_SYMBOL_GPL(dw_pcie_ep_linkdown);
> > > > +
> > > > int dw_pcie_ep_init(struct dw_pcie_ep *ep)
> > > > {
> > > > int ret;
> > > > diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> > > > index f8e5431a207b..152969545b0a 100644
> > > > --- a/drivers/pci/controller/dwc/pcie-designware.h
> > > > +++ b/drivers/pci/controller/dwc/pcie-designware.h
> > > > @@ -668,6 +668,7 @@ static inline void __iomem *dw_pcie_own_conf_map_bus(struct pci_bus *bus,
> > > >
> > > > #ifdef CONFIG_PCIE_DW_EP
> > > > void dw_pcie_ep_linkup(struct dw_pcie_ep *ep);
> > > > +void dw_pcie_ep_linkdown(struct dw_pcie_ep *ep);
> > > > int dw_pcie_ep_init(struct dw_pcie_ep *ep);
> > > > int dw_pcie_ep_init_registers(struct dw_pcie_ep *ep);
> > > > void dw_pcie_ep_init_notify(struct dw_pcie_ep *ep);
> > > > @@ -688,6 +689,10 @@ static inline void dw_pcie_ep_linkup(struct dw_pcie_ep *ep)
> > > > {
> > > > }
> > > >
> > > > +static inline void dw_pcie_ep_linkdown(struct dw_pcie_ep *ep)
> > > > +{
> > > > +}
> > > > +
> > > > static inline int dw_pcie_ep_init(struct dw_pcie_ep *ep)
> > > > {
> > > > return 0;
> > > >
> > > > --
> > > > 2.25.1
> > > >
> >
> > --
> > மணிவண்ணன் சதாசிவம்
--
மணிவண்ணன் சதாசிவம்
Hello Mani,
On Sat, Feb 24, 2024 at 12:24:13PM +0530, Manivannan Sadhasivam wrote:
> "core_init_notifier" flag is set by the glue drivers requiring refclk from
> the host to complete the DWC core initialization. Also, those drivers will
> send a notification to the EPF drivers once the initialization is fully
> completed using the pci_epc_init_notify() API. Only then, the EPF drivers
> will start functioning.
>
> For the rest of the drivers generating refclk locally, EPF drivers will
> start functioning post binding with them. EPF drivers rely on the
> 'core_init_notifier' flag to differentiate between the drivers.
> Unfortunately, this creates two different flows for the EPF drivers.
>
> So to avoid that, let's get rid of the "core_init_notifier" flag and follow
> a single initialization flow for the EPF drivers. This is done by calling
> the dw_pcie_ep_init_notify() from all glue drivers after the completion of
> dw_pcie_ep_init_registers() API. This will allow all the glue drivers to
> send the notification to the EPF drivers once the initialization is fully
> completed.
>
> Only difference here is that, the drivers requiring refclk from host will
> send the notification once refclk is received, while others will send it
> during probe time itself.
>
> Signed-off-by: Manivannan Sadhasivam <[email protected]>
> ---
> drivers/pci/controller/dwc/pci-dra7xx.c | 2 ++
> drivers/pci/controller/dwc/pci-imx6.c | 2 ++
> drivers/pci/controller/dwc/pci-keystone.c | 2 ++
> drivers/pci/controller/dwc/pci-layerscape-ep.c | 2 ++
> drivers/pci/controller/dwc/pcie-designware-plat.c | 2 ++
> drivers/pci/controller/dwc/pcie-qcom-ep.c | 1 -
> drivers/pci/controller/dwc/pcie-rcar-gen4.c | 2 ++
> drivers/pci/controller/dwc/pcie-tegra194.c | 1 -
> drivers/pci/controller/dwc/pcie-uniphier-ep.c | 2 ++
> drivers/pci/endpoint/functions/pci-epf-test.c | 18 +++++-------------
> include/linux/pci-epc.h | 3 ---
pcie-artpec6.c:static const struct dw_pcie_ep_ops pcie_ep_ops = {
pcie-keembay.c:static const struct dw_pcie_ep_ops keembay_pcie_ep_ops = {
Where is the love for these drivers? ;)
Kind regards,
Niklas
On Sat, Feb 24, 2024 at 12:24:09PM +0530, Manivannan Sadhasivam wrote:
> For DWC glue drivers supporting PERST# (currently Qcom and Tegra194), some
> of the DWC resources like eDMA should be cleaned up during the PERST#
> assert time.
>
> So let's introduce a dw_pcie_ep_cleanup() API that could be called by these
> drivers to cleanup the DWC specific resources. Currently, it just removes
> eDMA.
>
> Reported-by: Niklas Cassel <[email protected]>
> Closes: https://lore.kernel.org/linux-pci/ZWYmX8Y%2F7Q9WMxES@x1-carbon
> Signed-off-by: Manivannan Sadhasivam <[email protected]>
> ---
> drivers/pci/controller/dwc/pcie-designware-ep.c | 11 +++++++++--
> drivers/pci/controller/dwc/pcie-designware.h | 5 +++++
> drivers/pci/controller/dwc/pcie-qcom-ep.c | 1 +
> drivers/pci/controller/dwc/pcie-tegra194.c | 2 ++
> 4 files changed, 17 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
> index 2b11290aab4c..1205bfba8310 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
> @@ -564,12 +564,19 @@ int dw_pcie_ep_raise_msix_irq(struct dw_pcie_ep *ep, u8 func_no,
> return 0;
> }
>
> -void dw_pcie_ep_deinit(struct dw_pcie_ep *ep)
> +void dw_pcie_ep_cleanup(struct dw_pcie_ep *ep)
> {
> struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> - struct pci_epc *epc = ep->epc;
>
> dw_pcie_edma_remove(pci);
Hello Mani,
In this message:
https://lore.kernel.org/linux-pci/20240130062938.GB32821@thinkpad/
You mentioned that you were going to clean up the BARs.
(Like I wrote in that thread, I really think that we should merge a fix for
the broken "do we have a saved value from find_first_zero_bit() in the array",
by using a "if (!saved_value[bar])", when find_first_zero_bit() returns zero.)
However, regardless of that, I do not see that this series (neither
dw_pcie_ep_cleanup(), nor dw_pcie_ep_linkdown()), calls any function which
will clean up the BARs.
Since e.g. qcom-ep.c does a reset_control_assert() during perst
assert/deassert, which should clear sticky registers, I think that
you should let dw_pcie_ep_cleanup() clean up the BARs using
dw_pcie_ep_clear_bar().
Kind regards,
Niklas
> +}
> +EXPORT_SYMBOL_GPL(dw_pcie_ep_cleanup);
> +
> +void dw_pcie_ep_deinit(struct dw_pcie_ep *ep)
> +{
> + struct pci_epc *epc = ep->epc;
> +
> + dw_pcie_ep_cleanup(ep);
>
> pci_epc_mem_free_addr(epc, ep->msi_mem_phys, ep->msi_mem,
> epc->mem->window.page_size);
> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> index 61465203bb60..351d2fe3ea4d 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.h
> +++ b/drivers/pci/controller/dwc/pcie-designware.h
> @@ -672,6 +672,7 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep);
> int dw_pcie_ep_init_complete(struct dw_pcie_ep *ep);
> void dw_pcie_ep_init_notify(struct dw_pcie_ep *ep);
> void dw_pcie_ep_deinit(struct dw_pcie_ep *ep);
> +void dw_pcie_ep_cleanup(struct dw_pcie_ep *ep);
> int dw_pcie_ep_raise_intx_irq(struct dw_pcie_ep *ep, u8 func_no);
> int dw_pcie_ep_raise_msi_irq(struct dw_pcie_ep *ep, u8 func_no,
> u8 interrupt_num);
> @@ -705,6 +706,10 @@ static inline void dw_pcie_ep_deinit(struct dw_pcie_ep *ep)
> {
> }
>
> +static inline void dw_pcie_ep_cleanup(struct dw_pcie_ep *ep)
> +{
> +}
> +
> static inline int dw_pcie_ep_raise_intx_irq(struct dw_pcie_ep *ep, u8 func_no)
> {
> return 0;
> diff --git a/drivers/pci/controller/dwc/pcie-qcom-ep.c b/drivers/pci/controller/dwc/pcie-qcom-ep.c
> index 36e5e80cd22f..59b1c0110288 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom-ep.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom-ep.c
> @@ -507,6 +507,7 @@ static void qcom_pcie_perst_assert(struct dw_pcie *pci)
> return;
> }
>
> + dw_pcie_ep_cleanup(&pci->ep);
> qcom_pcie_disable_resources(pcie_ep);
> pcie_ep->link_status = QCOM_PCIE_EP_LINK_DISABLED;
> }
> diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c
> index 7afa9e9aabe2..68bfeed3429b 100644
> --- a/drivers/pci/controller/dwc/pcie-tegra194.c
> +++ b/drivers/pci/controller/dwc/pcie-tegra194.c
> @@ -1715,6 +1715,8 @@ static void pex_ep_event_pex_rst_assert(struct tegra_pcie_dw *pcie)
> if (ret)
> dev_err(pcie->dev, "Failed to go Detect state: %d\n", ret);
>
> + dw_pcie_ep_cleanup(&pcie->pci.ep);
> +
> reset_control_assert(pcie->core_rst);
>
> tegra_pcie_disable_phy(pcie);
>
> --
> 2.25.1
>
Hello Mani,
On Sat, Feb 24, 2024 at 12:24:16PM +0530, Manivannan Sadhasivam wrote:
> All of the APIs are missing the Kernel-doc comments. Hence, add them.
>
> Signed-off-by: Manivannan Sadhasivam <[email protected]>
> ---
> drivers/pci/controller/dwc/pcie-designware-ep.c | 92 +++++++++++++++++++++++++
> 1 file changed, 92 insertions(+)
>
> diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
> index fed4c2936c78..cdcb33a279db 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
(snip)
> @@ -556,6 +606,12 @@ int dw_pcie_ep_raise_msix_irq(struct dw_pcie_ep *ep, u8 func_no,
> return 0;
> }
>
> +/**
> + * dw_pcie_ep_cleanup - Cleanup DWC EP resources
> + * @ep: DWC EP device
> + *
> + * Cleans up the DWC EP specific resources like eDMA etc...
I think that you should mention that this is only for glue drivers that
use PERST# handling, so that other glue drivers do no start using it :)
> + */
> void dw_pcie_ep_cleanup(struct dw_pcie_ep *ep)
> {
> struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> @@ -564,6 +620,13 @@ void dw_pcie_ep_cleanup(struct dw_pcie_ep *ep)
> }
> EXPORT_SYMBOL_GPL(dw_pcie_ep_cleanup);
Kind regards,
Niklas
On Thu, Feb 29, 2024 at 12:23:16PM +0100, Niklas Cassel wrote:
> Hello Mani,
>
> On Sat, Feb 24, 2024 at 12:24:13PM +0530, Manivannan Sadhasivam wrote:
> > "core_init_notifier" flag is set by the glue drivers requiring refclk from
> > the host to complete the DWC core initialization. Also, those drivers will
> > send a notification to the EPF drivers once the initialization is fully
> > completed using the pci_epc_init_notify() API. Only then, the EPF drivers
> > will start functioning.
> >
> > For the rest of the drivers generating refclk locally, EPF drivers will
> > start functioning post binding with them. EPF drivers rely on the
> > 'core_init_notifier' flag to differentiate between the drivers.
> > Unfortunately, this creates two different flows for the EPF drivers.
> >
> > So to avoid that, let's get rid of the "core_init_notifier" flag and follow
> > a single initialization flow for the EPF drivers. This is done by calling
> > the dw_pcie_ep_init_notify() from all glue drivers after the completion of
> > dw_pcie_ep_init_registers() API. This will allow all the glue drivers to
> > send the notification to the EPF drivers once the initialization is fully
> > completed.
> >
> > Only difference here is that, the drivers requiring refclk from host will
> > send the notification once refclk is received, while others will send it
> > during probe time itself.
> >
> > Signed-off-by: Manivannan Sadhasivam <[email protected]>
> > ---
> > drivers/pci/controller/dwc/pci-dra7xx.c | 2 ++
> > drivers/pci/controller/dwc/pci-imx6.c | 2 ++
> > drivers/pci/controller/dwc/pci-keystone.c | 2 ++
> > drivers/pci/controller/dwc/pci-layerscape-ep.c | 2 ++
> > drivers/pci/controller/dwc/pcie-designware-plat.c | 2 ++
> > drivers/pci/controller/dwc/pcie-qcom-ep.c | 1 -
> > drivers/pci/controller/dwc/pcie-rcar-gen4.c | 2 ++
> > drivers/pci/controller/dwc/pcie-tegra194.c | 1 -
> > drivers/pci/controller/dwc/pcie-uniphier-ep.c | 2 ++
> > drivers/pci/endpoint/functions/pci-epf-test.c | 18 +++++-------------
> > include/linux/pci-epc.h | 3 ---
>
> pcie-artpec6.c:static const struct dw_pcie_ep_ops pcie_ep_ops = {
> pcie-keembay.c:static const struct dw_pcie_ep_ops keembay_pcie_ep_ops = {
>
> Where is the love for these drivers? ;)
>
Ah, my grep skills got exposed :(
Will fix them.
- Mani
--
மணிவண்ணன் சதாசிவம்
On Thu, Feb 29, 2024 at 01:40:29PM +0100, Niklas Cassel wrote:
> On Sat, Feb 24, 2024 at 12:24:09PM +0530, Manivannan Sadhasivam wrote:
> > For DWC glue drivers supporting PERST# (currently Qcom and Tegra194), some
> > of the DWC resources like eDMA should be cleaned up during the PERST#
> > assert time.
> >
> > So let's introduce a dw_pcie_ep_cleanup() API that could be called by these
> > drivers to cleanup the DWC specific resources. Currently, it just removes
> > eDMA.
> >
> > Reported-by: Niklas Cassel <[email protected]>
> > Closes: https://lore.kernel.org/linux-pci/ZWYmX8Y%2F7Q9WMxES@x1-carbon
> > Signed-off-by: Manivannan Sadhasivam <[email protected]>
> > ---
> > drivers/pci/controller/dwc/pcie-designware-ep.c | 11 +++++++++--
> > drivers/pci/controller/dwc/pcie-designware.h | 5 +++++
> > drivers/pci/controller/dwc/pcie-qcom-ep.c | 1 +
> > drivers/pci/controller/dwc/pcie-tegra194.c | 2 ++
> > 4 files changed, 17 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
> > index 2b11290aab4c..1205bfba8310 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
> > +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
> > @@ -564,12 +564,19 @@ int dw_pcie_ep_raise_msix_irq(struct dw_pcie_ep *ep, u8 func_no,
> > return 0;
> > }
> >
> > -void dw_pcie_ep_deinit(struct dw_pcie_ep *ep)
> > +void dw_pcie_ep_cleanup(struct dw_pcie_ep *ep)
> > {
> > struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> > - struct pci_epc *epc = ep->epc;
> >
> > dw_pcie_edma_remove(pci);
>
> Hello Mani,
>
> In this message:
> https://lore.kernel.org/linux-pci/20240130062938.GB32821@thinkpad/
>
> You mentioned that you were going to clean up the BARs.
Yes, I did and it is still in my queue.
> (Like I wrote in that thread, I really think that we should merge a fix for
> the broken "do we have a saved value from find_first_zero_bit() in the array",
> by using a "if (!saved_value[bar])", when find_first_zero_bit() returns zero.)
>
Hmm, yeah that logic is flawed. Let me take another look.
> However, regardless of that, I do not see that this series (neither
> dw_pcie_ep_cleanup(), nor dw_pcie_ep_linkdown()), calls any function which
> will clean up the BARs.
>
> Since e.g. qcom-ep.c does a reset_control_assert() during perst
> assert/deassert, which should clear sticky registers, I think that
> you should let dw_pcie_ep_cleanup() clean up the BARs using
> dw_pcie_ep_clear_bar().
>
As I mentioned earlier, it is the job of the EPF drivers to clear the BARs since
they allocate them. I'm trying to reduce the implicit resetting wherever we
could.
The proper fix is to add the LINK_DOWN callback to EPF drivers and do cleanup.
I'm planning to submit a series for that after this one.
- Mani
--
மணிவண்ணன் சதாசிவம்
On Thu, Feb 29, 2024 at 01:56:23PM +0100, Niklas Cassel wrote:
> Hello Mani,
>
> On Sat, Feb 24, 2024 at 12:24:16PM +0530, Manivannan Sadhasivam wrote:
> > All of the APIs are missing the Kernel-doc comments. Hence, add them.
> >
> > Signed-off-by: Manivannan Sadhasivam <[email protected]>
> > ---
> > drivers/pci/controller/dwc/pcie-designware-ep.c | 92 +++++++++++++++++++++++++
> > 1 file changed, 92 insertions(+)
> >
> > diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
> > index fed4c2936c78..cdcb33a279db 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
> > +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
>
> (snip)
>
> > @@ -556,6 +606,12 @@ int dw_pcie_ep_raise_msix_irq(struct dw_pcie_ep *ep, u8 func_no,
> > return 0;
> > }
> >
> > +/**
> > + * dw_pcie_ep_cleanup - Cleanup DWC EP resources
> > + * @ep: DWC EP device
> > + *
> > + * Cleans up the DWC EP specific resources like eDMA etc...
>
> I think that you should mention that this is only for glue drivers that
> use PERST# handling, so that other glue drivers do no start using it :)
>
You are right. Will add the wording.
- Mani
--
மணிவண்ணன் சதாசிவம்
On Mon, Mar 04, 2024 at 01:47:13PM +0530, Manivannan Sadhasivam wrote:
> On Thu, Feb 29, 2024 at 01:40:29PM +0100, Niklas Cassel wrote:
> > On Sat, Feb 24, 2024 at 12:24:09PM +0530, Manivannan Sadhasivam wrote:
> >
> > Since e.g. qcom-ep.c does a reset_control_assert() during perst
> > assert/deassert, which should clear sticky registers, I think that
> > you should let dw_pcie_ep_cleanup() clean up the BARs using
> > dw_pcie_ep_clear_bar().
> >
>
> As I mentioned earlier, it is the job of the EPF drivers to clear the BARs since
> they allocate them. I'm trying to reduce the implicit resetting wherever we
> could.
>
> The proper fix is to add the LINK_DOWN callback to EPF drivers and do cleanup.
> I'm planning to submit a series for that after this one.
Currently, pci-epf-test allocates memory for the BARs in .bind().
Likewise it frees the memory for the BARs in .unbind().
AFAICT, most iATU registers, and most BAR registers are sticky registers,
so they will not get reset on link down.
(The currently selected BAR size, in case of Resizable BAR is an exception.)
That means that even on link down, we do not need to free the memory,
or change the iATU settings. (This applies to all drivers.)
However, on PERST (for the drivers call dw_pcie_ep_cleanup()), they call
reset_control_assert(), so they will clear sticky registers, which means
that they need to at least re-write the iATU and BAR registers.
(I guess they could free + allocate the memory for the BARs again,
but I don't think that is strictly necessary.)
That is why I suggested that you call dw_pcie_ep_clear_bar() from
dw_pcie_ep_cleanup().
If you free the memory for the BARs in link_down() (this callback exists
for many drivers, even drivers without a PERST handler), where are you
supposted to alloc the memory for the BARs again?
Allocating them at link_up() is too late (because as soon as the link is
up, the host is allowed to enumerate the EP BARs.) The proper place is to
allocate them when receiving PERST, but not all drivers have a PERST handler.
(My understanding is that 1) PERST assert 2) PERST deassert 3) link is up.)
unbind() undos what was done in bind(), so shouldn't link_down() undo what was
done in link_up()? With that logic, if you move the alloc to .core_init(),
should we perhaps have a .core_deinit() callback for EPF drivers?
(I guess only drivers which perform a reset during PERST would call this.)
But considering that free+alloc is not strictly needed, why not just keep
the allocation + free in .bind()/.unbind() ?
(To avoid the need to create a .core_deinit()), and let dw_pcie_ep_cleanup()
call dw_pcie_ep_clear_bar() ?
I guess my point is that it seems a bit pointless for drivers that do not
clear sticky registers to free+alloc memory on link down, for no good
reason. (Memory might get fragmented over time, so it might not be possible
to perform a big allocation after the device has been running for a really
long time.)
So I'm thinking that we either
1) Keep the alloc/free in bind/unbind, and let dw_pcie_ep_cleanup() call
dw_pcie_ep_clear_bar(),
or
2) Introduce a .deinit_core() callback which will free the BARs.
(Because I don't see how you will (re-)allocate memory for all drivers
if you free the memory in link_down().)
Kind regards,
Niklas
On Mon, Mar 04, 2024 at 11:51:04AM +0100, Niklas Cassel wrote:
> On Mon, Mar 04, 2024 at 01:47:13PM +0530, Manivannan Sadhasivam wrote:
> > On Thu, Feb 29, 2024 at 01:40:29PM +0100, Niklas Cassel wrote:
> > > On Sat, Feb 24, 2024 at 12:24:09PM +0530, Manivannan Sadhasivam wrote:
> > >
> > > Since e.g. qcom-ep.c does a reset_control_assert() during perst
> > > assert/deassert, which should clear sticky registers, I think that
> > > you should let dw_pcie_ep_cleanup() clean up the BARs using
> > > dw_pcie_ep_clear_bar().
> > >
> >
> > As I mentioned earlier, it is the job of the EPF drivers to clear the BARs since
> > they allocate them. I'm trying to reduce the implicit resetting wherever we
> > could.
> >
> > The proper fix is to add the LINK_DOWN callback to EPF drivers and do cleanup.
> > I'm planning to submit a series for that after this one.
>
> Currently, pci-epf-test allocates memory for the BARs in .bind().
> Likewise it frees the memory for the BARs in .unbind().
>
> AFAICT, most iATU registers, and most BAR registers are sticky registers,
> so they will not get reset on link down.
> (The currently selected BAR size, in case of Resizable BAR is an exception.)
>
> That means that even on link down, we do not need to free the memory,
> or change the iATU settings. (This applies to all drivers.)
>
>
>
> However, on PERST (for the drivers call dw_pcie_ep_cleanup()), they call
> reset_control_assert(), so they will clear sticky registers, which means
> that they need to at least re-write the iATU and BAR registers.
> (I guess they could free + allocate the memory for the BARs again,
> but I don't think that is strictly necessary.)
> That is why I suggested that you call dw_pcie_ep_clear_bar() from
> dw_pcie_ep_cleanup().
>
Sorry, I keep assuming the flow w.r.t PERST# supported platforms :/
My bad!
>
>
> If you free the memory for the BARs in link_down() (this callback exists
> for many drivers, even drivers without a PERST handler), where are you
> supposted to alloc the memory for the BARs again?
>
> Allocating them at link_up() is too late (because as soon as the link is
> up, the host is allowed to enumerate the EP BARs.) The proper place is to
> allocate them when receiving PERST, but not all drivers have a PERST handler.
>
> (My understanding is that 1) PERST assert 2) PERST deassert 3) link is up.)
>
>
>
> unbind() undos what was done in bind(), so shouldn't link_down() undo what was
> done in link_up()? With that logic, if you move the alloc to .core_init(),
> should we perhaps have a .core_deinit() callback for EPF drivers?
> (I guess only drivers which perform a reset during PERST would call this.)
>
> But considering that free+alloc is not strictly needed, why not just keep
> the allocation + free in .bind()/.unbind() ?
> (To avoid the need to create a .core_deinit()), and let dw_pcie_ep_cleanup()
> call dw_pcie_ep_clear_bar() ?
>
> I guess my point is that it seems a bit pointless for drivers that do not
> clear sticky registers to free+alloc memory on link down, for no good
> reason. (Memory might get fragmented over time, so it might not be possible
> to perform a big allocation after the device has been running for a really
> long time.)
>
>
>
> So I'm thinking that we either
> 1) Keep the alloc/free in bind/unbind, and let dw_pcie_ep_cleanup() call
> dw_pcie_ep_clear_bar(),
> or
> 2) Introduce a .deinit_core() callback which will free the BARs.
> (Because I don't see how you will (re-)allocate memory for all drivers
> if you free the memory in link_down().)
>
I think option 2 is the better solution. In my view, calling
dw_pcie_ep_clear_bar() from EPC drivers is a layering violation since the
allocation happens from EPF drivers.
So clearing the BARs during the deinit() callback that gets called when PERST#
assert happens is the way to go.
- Mani
--
மணிவண்ணன் சதாசிவம்