2024-06-06 07:27:46

by Manivannan Sadhasivam

[permalink] [raw]
Subject: [PATCH 0/5] PCI: endpoint: Add EPC 'deinit' event and dw_pcie_ep_linkdown() API

Hi,

This series includes patches that were left over from previous series [1] for
making the host reboot handling robust in endpoint framework.

When the above mentioned series got merged to pci/endpoint, we got a bug report
from LKP bot [2] and due to that the offending patches were dropped.

This series addressed the issue reported by the bot by adding the stub APIs in
include/pci/pci-epc.h and also removed the unused dwc wrapper as concluded in
[3].

Testing
=======

This series is tested on Qcom SM8450 based development board with 2 SM8450 SoCs
connected over PCIe.

- Mani

[1] https://lore.kernel.org/linux-pci/[email protected]/
[2] https://lore.kernel.org/linux-pci/[email protected]/
[3] https://lore.kernel.org/linux-pci/20240529141614.GA3293@thinkpad/

Signed-off-by: Manivannan Sadhasivam <[email protected]>
---
Manivannan Sadhasivam (5):
PCI: dwc: ep: Remove dw_pcie_ep_init_notify() wrapper
PCI: endpoint: Introduce 'epc_deinit' event and notify the EPF drivers
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: layerscape-ep: Use the generic dw_pcie_ep_linkdown() API to handle Link Down event

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 | 4 +-
drivers/pci/controller/dwc/pcie-artpec6.c | 2 +-
drivers/pci/controller/dwc/pcie-designware-ep.c | 116 +++++++++++++---------
drivers/pci/controller/dwc/pcie-designware-plat.c | 2 +-
drivers/pci/controller/dwc/pcie-designware.h | 10 +-
drivers/pci/controller/dwc/pcie-keembay.c | 2 +-
drivers/pci/controller/dwc/pcie-qcom-ep.c | 5 +-
drivers/pci/controller/dwc/pcie-rcar-gen4.c | 2 +-
drivers/pci/controller/dwc/pcie-tegra194.c | 3 +-
drivers/pci/controller/dwc/pcie-uniphier-ep.c | 2 +-
drivers/pci/endpoint/functions/pci-epf-mhi.c | 19 ++++
drivers/pci/endpoint/functions/pci-epf-test.c | 17 +++-
drivers/pci/endpoint/pci-epc-core.c | 25 +++++
include/linux/pci-epc.h | 13 +++
include/linux/pci-epf.h | 2 +
18 files changed, 162 insertions(+), 68 deletions(-)
---
base-commit: 7d96527bc16e46545739c6fe0ab6e4c915e9910e
change-id: 20240606-pci-deinit-2e6cdf1bd69f

Best regards,
--
Manivannan Sadhasivam <[email protected]>



2024-06-06 07:28:03

by Manivannan Sadhasivam

[permalink] [raw]
Subject: [PATCH 1/5] PCI: dwc: ep: Remove dw_pcie_ep_init_notify() wrapper

Currently dw_pcie_ep_init_notify() wrapper just calls pci_epc_init_notify()
directly. So this wrapper provides no benefit to the glue drivers.

So let's remove it and call pci_epc_init_notify() directly from glue
drivers.

Suggested-by: Bjorn Helgaas <[email protected]>
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-artpec6.c | 2 +-
drivers/pci/controller/dwc/pcie-designware-ep.c | 12 ------------
drivers/pci/controller/dwc/pcie-designware-plat.c | 2 +-
drivers/pci/controller/dwc/pcie-designware.h | 5 -----
drivers/pci/controller/dwc/pcie-keembay.c | 2 +-
drivers/pci/controller/dwc/pcie-qcom-ep.c | 2 +-
drivers/pci/controller/dwc/pcie-rcar-gen4.c | 2 +-
drivers/pci/controller/dwc/pcie-tegra194.c | 2 +-
drivers/pci/controller/dwc/pcie-uniphier-ep.c | 2 +-
13 files changed, 11 insertions(+), 28 deletions(-)

diff --git a/drivers/pci/controller/dwc/pci-dra7xx.c b/drivers/pci/controller/dwc/pci-dra7xx.c
index d2d17d37d3e0..e491d0ff3962 100644
--- a/drivers/pci/controller/dwc/pci-dra7xx.c
+++ b/drivers/pci/controller/dwc/pci-dra7xx.c
@@ -474,7 +474,7 @@ static int dra7xx_add_pcie_ep(struct dra7xx_pcie *dra7xx,
return ret;
}

- dw_pcie_ep_init_notify(ep);
+ pci_epc_init_notify(ep->epc);

return 0;
}
diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index 917c69edee1d..a876b8e6e741 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -1131,7 +1131,7 @@ static int imx6_add_pcie_ep(struct imx6_pcie *imx6_pcie,
return ret;
}

- dw_pcie_ep_init_notify(ep);
+ pci_epc_init_notify(ep->epc);

/* 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 d3a7d14ee685..ca1054f5c79a 100644
--- a/drivers/pci/controller/dwc/pci-keystone.c
+++ b/drivers/pci/controller/dwc/pci-keystone.c
@@ -1293,7 +1293,7 @@ static int ks_pcie_probe(struct platform_device *pdev)
goto err_ep_init;
}

- dw_pcie_ep_init_notify(&pci->ep);
+ pci_epc_init_notify(pci->ep.epc);

break;
default:
diff --git a/drivers/pci/controller/dwc/pci-layerscape-ep.c b/drivers/pci/controller/dwc/pci-layerscape-ep.c
index 7dde6d5fa4d8..35bb481564c7 100644
--- a/drivers/pci/controller/dwc/pci-layerscape-ep.c
+++ b/drivers/pci/controller/dwc/pci-layerscape-ep.c
@@ -286,7 +286,7 @@ static int __init ls_pcie_ep_probe(struct platform_device *pdev)
return ret;
}

- dw_pcie_ep_init_notify(&pci->ep);
+ pci_epc_init_notify(pci->ep.epc);

return ls_pcie_ep_interrupt_init(pcie, pdev);
}
diff --git a/drivers/pci/controller/dwc/pcie-artpec6.c b/drivers/pci/controller/dwc/pcie-artpec6.c
index a4630b92489b..dc8dd7f27b78 100644
--- a/drivers/pci/controller/dwc/pcie-artpec6.c
+++ b/drivers/pci/controller/dwc/pcie-artpec6.c
@@ -452,7 +452,7 @@ static int artpec6_pcie_probe(struct platform_device *pdev)
return ret;
}

- dw_pcie_ep_init_notify(&pci->ep);
+ pci_epc_init_notify(pci->ep.epc);

break;
default:
diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
index 47391d7d3a73..2e69f81baf99 100644
--- a/drivers/pci/controller/dwc/pcie-designware-ep.c
+++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
@@ -27,18 +27,6 @@ void dw_pcie_ep_linkup(struct dw_pcie_ep *ep)
}
EXPORT_SYMBOL_GPL(dw_pcie_ep_linkup);

-/**
- * 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;
-
- pci_epc_init_notify(epc);
-}
-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
diff --git a/drivers/pci/controller/dwc/pcie-designware-plat.c b/drivers/pci/controller/dwc/pcie-designware-plat.c
index 8490c5d6ff9f..771b9d9be077 100644
--- a/drivers/pci/controller/dwc/pcie-designware-plat.c
+++ b/drivers/pci/controller/dwc/pcie-designware-plat.c
@@ -154,7 +154,7 @@ static int dw_plat_pcie_probe(struct platform_device *pdev)
dw_pcie_ep_deinit(&pci->ep);
}

- dw_pcie_ep_init_notify(&pci->ep);
+ pci_epc_init_notify(pci->ep.epc);

break;
default:
diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index f8e5431a207b..49ae845a3662 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -670,7 +670,6 @@ static inline void __iomem *dw_pcie_own_conf_map_bus(struct pci_bus *bus,
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_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);
int dw_pcie_ep_raise_intx_irq(struct dw_pcie_ep *ep, u8 func_no);
@@ -698,10 +697,6 @@ static inline int dw_pcie_ep_init_registers(struct dw_pcie_ep *ep)
return 0;
}

-static inline void dw_pcie_ep_init_notify(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-keembay.c b/drivers/pci/controller/dwc/pcie-keembay.c
index 98bbc83182b4..278205db60a2 100644
--- a/drivers/pci/controller/dwc/pcie-keembay.c
+++ b/drivers/pci/controller/dwc/pcie-keembay.c
@@ -442,7 +442,7 @@ static int keembay_pcie_probe(struct platform_device *pdev)
return ret;
}

- dw_pcie_ep_init_notify(&pci->ep);
+ pci_epc_init_notify(pci->ep.epc);

break;
default:
diff --git a/drivers/pci/controller/dwc/pcie-qcom-ep.c b/drivers/pci/controller/dwc/pcie-qcom-ep.c
index 1ecf602c225a..4d2d7457dcb3 100644
--- a/drivers/pci/controller/dwc/pcie-qcom-ep.c
+++ b/drivers/pci/controller/dwc/pcie-qcom-ep.c
@@ -482,7 +482,7 @@ static int qcom_pcie_perst_deassert(struct dw_pcie *pci)
val &= ~PARF_MSTR_AXI_CLK_EN;
writel_relaxed(val, pcie_ep->parf + PARF_MHI_CLOCK_RESET_CTRL);

- dw_pcie_ep_init_notify(&pcie_ep->pci.ep);
+ pci_epc_init_notify(pcie_ep->pci.ep.epc);

/* Enable LTSSM */
val = readl_relaxed(pcie_ep->parf + PARF_LTSSM);
diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.c b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
index cfeccc2f9ee1..237a6a8818de 100644
--- a/drivers/pci/controller/dwc/pcie-rcar-gen4.c
+++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
@@ -437,7 +437,7 @@ 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);
+ pci_epc_init_notify(ep->epc);

return ret;
}
diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c
index 93f5433c5c55..432ed9d9a463 100644
--- a/drivers/pci/controller/dwc/pcie-tegra194.c
+++ b/drivers/pci/controller/dwc/pcie-tegra194.c
@@ -1903,7 +1903,7 @@ static void pex_ep_event_pex_rst_deassert(struct tegra_pcie_dw *pcie)
goto fail_init_complete;
}

- dw_pcie_ep_init_notify(ep);
+ pci_epc_init_notify(ep->epc);

/* Program the private control to allow sending LTR upstream */
if (pcie->of_data->has_ltr_req_fix) {
diff --git a/drivers/pci/controller/dwc/pcie-uniphier-ep.c b/drivers/pci/controller/dwc/pcie-uniphier-ep.c
index a2b844268e28..d6e73811216e 100644
--- a/drivers/pci/controller/dwc/pcie-uniphier-ep.c
+++ b/drivers/pci/controller/dwc/pcie-uniphier-ep.c
@@ -410,7 +410,7 @@ static int uniphier_pcie_ep_probe(struct platform_device *pdev)
return ret;
}

- dw_pcie_ep_init_notify(&priv->pci.ep);
+ pci_epc_init_notify(priv->pci.ep.epc);

return 0;
}

--
2.25.1


2024-06-06 07:28:22

by Manivannan Sadhasivam

[permalink] [raw]
Subject: [PATCH 2/5] PCI: endpoint: Introduce 'epc_deinit' event and notify the EPF drivers

As like the 'epc_init' event, that is used to signal the EPF drivers about
the EPC initialization, let's introduce 'epc_deinit' event that is used to
signal EPC deinitialization.

The EPC deinitialization applies only when any sort of fundamental reset
is supported by the endpoint controller as per the PCIe spec.

Reference: PCIe Base spec v5.0, sections 4.2.4.9.1 and 6.6.1.

Currently, some EPC drivers like pcie-qcom-ep and pcie-tegra194 support
PERST# as the fundamental reset. So the 'deinit' event will be notified to
the EPF drivers when PERST# assert happens in the above mentioned EPC
drivers.

The EPF drivers, on receiving the event through the epc_deinit() callback
should reset the EPF state machine and also cleanup any configuration that
got affected by the fundamental reset like BAR, DMA etc...

This change also warrants skipping the cleanups in unbind() if already done
in epc_deinit().

Reviewed-by: Niklas Cassel <[email protected]>
Signed-off-by: Manivannan Sadhasivam <[email protected]>
---
drivers/pci/controller/dwc/pcie-designware-ep.c | 1 -
drivers/pci/controller/dwc/pcie-qcom-ep.c | 1 +
drivers/pci/controller/dwc/pcie-tegra194.c | 1 +
drivers/pci/endpoint/functions/pci-epf-mhi.c | 19 +++++++++++++++++++
drivers/pci/endpoint/functions/pci-epf-test.c | 17 +++++++++++++++--
drivers/pci/endpoint/pci-epc-core.c | 25 +++++++++++++++++++++++++
include/linux/pci-epc.h | 13 +++++++++++++
include/linux/pci-epf.h | 2 ++
8 files changed, 76 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
index 2e69f81baf99..78d5fc72c9cb 100644
--- a/drivers/pci/controller/dwc/pcie-designware-ep.c
+++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
@@ -620,7 +620,6 @@ void dw_pcie_ep_cleanup(struct dw_pcie_ep *ep)
struct dw_pcie *pci = to_dw_pcie_from_ep(ep);

dw_pcie_edma_remove(pci);
- ep->epc->init_complete = false;
}
EXPORT_SYMBOL_GPL(dw_pcie_ep_cleanup);

diff --git a/drivers/pci/controller/dwc/pcie-qcom-ep.c b/drivers/pci/controller/dwc/pcie-qcom-ep.c
index 4d2d7457dcb3..2324e56c9bfc 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;
}

+ pci_epc_deinit_notify(pci->ep.epc);
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 432ed9d9a463..4ca7404246a3 100644
--- a/drivers/pci/controller/dwc/pcie-tegra194.c
+++ b/drivers/pci/controller/dwc/pcie-tegra194.c
@@ -1715,6 +1715,7 @@ 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);

+ pci_epc_deinit_notify(pcie->pci.ep.epc);
dw_pcie_ep_cleanup(&pcie->pci.ep);

reset_control_assert(pcie->core_rst);
diff --git a/drivers/pci/endpoint/functions/pci-epf-mhi.c b/drivers/pci/endpoint/functions/pci-epf-mhi.c
index 205c02953f25..5832989e55e8 100644
--- a/drivers/pci/endpoint/functions/pci-epf-mhi.c
+++ b/drivers/pci/endpoint/functions/pci-epf-mhi.c
@@ -764,6 +764,24 @@ static int pci_epf_mhi_epc_init(struct pci_epf *epf)
return 0;
}

+static void pci_epf_mhi_epc_deinit(struct pci_epf *epf)
+{
+ struct pci_epf_mhi *epf_mhi = epf_get_drvdata(epf);
+ const struct pci_epf_mhi_ep_info *info = epf_mhi->info;
+ struct pci_epf_bar *epf_bar = &epf->bar[info->bar_num];
+ struct mhi_ep_cntrl *mhi_cntrl = &epf_mhi->mhi_cntrl;
+ struct pci_epc *epc = epf->epc;
+
+ if (mhi_cntrl->mhi_dev) {
+ mhi_ep_power_down(mhi_cntrl);
+ if (info->flags & MHI_EPF_USE_DMA)
+ pci_epf_mhi_dma_deinit(epf_mhi);
+ mhi_ep_unregister_controller(mhi_cntrl);
+ }
+
+ pci_epc_clear_bar(epc, epf->func_no, epf->vfunc_no, epf_bar);
+}
+
static int pci_epf_mhi_link_up(struct pci_epf *epf)
{
struct pci_epf_mhi *epf_mhi = epf_get_drvdata(epf);
@@ -898,6 +916,7 @@ static void pci_epf_mhi_unbind(struct pci_epf *epf)

static const struct pci_epc_event_ops pci_epf_mhi_event_ops = {
.epc_init = pci_epf_mhi_epc_init,
+ .epc_deinit = pci_epf_mhi_epc_deinit,
.link_up = pci_epf_mhi_link_up,
.link_down = pci_epf_mhi_link_down,
.bus_master_enable = pci_epf_mhi_bus_master_enable,
diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c
index e771be7512a1..7c2ed6eae53a 100644
--- a/drivers/pci/endpoint/functions/pci-epf-test.c
+++ b/drivers/pci/endpoint/functions/pci-epf-test.c
@@ -782,6 +782,15 @@ static int pci_epf_test_epc_init(struct pci_epf *epf)
return 0;
}

+static void pci_epf_test_epc_deinit(struct pci_epf *epf)
+{
+ struct pci_epf_test *epf_test = epf_get_drvdata(epf);
+
+ cancel_delayed_work(&epf_test->cmd_handler);
+ pci_epf_test_clean_dma_chan(epf_test);
+ pci_epf_test_clear_bar(epf);
+}
+
static int pci_epf_test_link_up(struct pci_epf *epf)
{
struct pci_epf_test *epf_test = epf_get_drvdata(epf);
@@ -803,6 +812,7 @@ static int pci_epf_test_link_down(struct pci_epf *epf)

static const struct pci_epc_event_ops pci_epf_test_event_ops = {
.epc_init = pci_epf_test_epc_init,
+ .epc_deinit = pci_epf_test_epc_deinit,
.link_up = pci_epf_test_link_up,
.link_down = pci_epf_test_link_down,
};
@@ -905,10 +915,13 @@ static int pci_epf_test_bind(struct pci_epf *epf)
static void pci_epf_test_unbind(struct pci_epf *epf)
{
struct pci_epf_test *epf_test = epf_get_drvdata(epf);
+ struct pci_epc *epc = epf->epc;

cancel_delayed_work(&epf_test->cmd_handler);
- pci_epf_test_clean_dma_chan(epf_test);
- pci_epf_test_clear_bar(epf);
+ if (epc->init_complete) {
+ pci_epf_test_clean_dma_chan(epf_test);
+ pci_epf_test_clear_bar(epf);
+ }
pci_epf_test_free_space(epf);
}

diff --git a/drivers/pci/endpoint/pci-epc-core.c b/drivers/pci/endpoint/pci-epc-core.c
index 56b60330355d..47a91dcb07d7 100644
--- a/drivers/pci/endpoint/pci-epc-core.c
+++ b/drivers/pci/endpoint/pci-epc-core.c
@@ -774,6 +774,31 @@ void pci_epc_notify_pending_init(struct pci_epc *epc, struct pci_epf *epf)
}
EXPORT_SYMBOL_GPL(pci_epc_notify_pending_init);

+/**
+ * pci_epc_deinit_notify() - Notify the EPF device about EPC deinitialization
+ * @epc: the EPC device whose deinitialization is completed
+ *
+ * Invoke to notify the EPF device that the EPC deinitialization is completed.
+ */
+void pci_epc_deinit_notify(struct pci_epc *epc)
+{
+ struct pci_epf *epf;
+
+ if (IS_ERR_OR_NULL(epc))
+ return;
+
+ mutex_lock(&epc->list_lock);
+ list_for_each_entry(epf, &epc->pci_epf, list) {
+ mutex_lock(&epf->lock);
+ if (epf->event_ops && epf->event_ops->epc_deinit)
+ epf->event_ops->epc_deinit(epf);
+ mutex_unlock(&epf->lock);
+ }
+ epc->init_complete = false;
+ mutex_unlock(&epc->list_lock);
+}
+EXPORT_SYMBOL_GPL(pci_epc_deinit_notify);
+
/**
* pci_epc_bus_master_enable_notify() - Notify the EPF device that the EPC
* device has received the Bus Master
diff --git a/include/linux/pci-epc.h b/include/linux/pci-epc.h
index 11115cd0fe5b..85bdf2adb760 100644
--- a/include/linux/pci-epc.h
+++ b/include/linux/pci-epc.h
@@ -197,6 +197,8 @@ struct pci_epc_features {

#define to_pci_epc(device) container_of((device), struct pci_epc, dev)

+#ifdef CONFIG_PCI_ENDPOINT
+
#define pci_epc_create(dev, ops) \
__pci_epc_create((dev), (ops), THIS_MODULE)
#define devm_pci_epc_create(dev, ops) \
@@ -226,6 +228,7 @@ void pci_epc_linkup(struct pci_epc *epc);
void pci_epc_linkdown(struct pci_epc *epc);
void pci_epc_init_notify(struct pci_epc *epc);
void pci_epc_notify_pending_init(struct pci_epc *epc, struct pci_epf *epf);
+void pci_epc_deinit_notify(struct pci_epc *epc);
void pci_epc_bus_master_enable_notify(struct pci_epc *epc);
void pci_epc_remove_epf(struct pci_epc *epc, struct pci_epf *epf,
enum pci_epc_interface_type type);
@@ -272,4 +275,14 @@ void __iomem *pci_epc_mem_alloc_addr(struct pci_epc *epc,
phys_addr_t *phys_addr, size_t size);
void pci_epc_mem_free_addr(struct pci_epc *epc, phys_addr_t phys_addr,
void __iomem *virt_addr, size_t size);
+
+#else
+static inline void pci_epc_init_notify(struct pci_epc *epc)
+{
+}
+
+static inline void pci_epc_deinit_notify(struct pci_epc *epc)
+{
+}
+#endif /* CONFIG_PCI_ENDPOINT */
#endif /* __LINUX_PCI_EPC_H */
diff --git a/include/linux/pci-epf.h b/include/linux/pci-epf.h
index dc759eb7157c..0639d4dc8986 100644
--- a/include/linux/pci-epf.h
+++ b/include/linux/pci-epf.h
@@ -71,12 +71,14 @@ struct pci_epf_ops {
/**
* struct pci_epc_event_ops - Callbacks for capturing the EPC events
* @epc_init: Callback for the EPC initialization complete event
+ * @epc_deinit: Callback for the EPC deinitialization event
* @link_up: Callback for the EPC link up event
* @link_down: Callback for the EPC link down event
* @bus_master_enable: Callback for the EPC Bus Master Enable event
*/
struct pci_epc_event_ops {
int (*epc_init)(struct pci_epf *epf);
+ void (*epc_deinit)(struct pci_epf *epf);
int (*link_up)(struct pci_epf *epf);
int (*link_down)(struct pci_epf *epf);
int (*bus_master_enable)(struct pci_epf *epf);

--
2.25.1


2024-06-06 07:28:47

by Manivannan Sadhasivam

[permalink] [raw]
Subject: [PATCH 4/5] PCI: qcom-ep: Use the generic dw_pcie_ep_linkdown() API to handle Link Down event

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.

Reviewed-by: Niklas Cassel <[email protected]>
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 2324e56c9bfc..02a2a871a91f 100644
--- a/drivers/pci/controller/dwc/pcie-qcom-ep.c
+++ b/drivers/pci/controller/dwc/pcie-qcom-ep.c
@@ -641,7 +641,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 Bus Master Enable event\n");
pcie_ep->link_status = QCOM_PCIE_EP_LINK_ENABLED;

--
2.25.1


2024-06-06 07:29:19

by Manivannan Sadhasivam

[permalink] [raw]
Subject: [PATCH 5/5] PCI: layerscape-ep: Use the generic dw_pcie_ep_linkdown() API to handle Link Down event

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.

Reported-by: Bjorn Helgaas <[email protected]>
Closes: https://lore.kernel.org/linux-pci/20240528195539.GA458945@bhelgaas/
Signed-off-by: Manivannan Sadhasivam <[email protected]>
---
drivers/pci/controller/dwc/pci-layerscape-ep.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/controller/dwc/pci-layerscape-ep.c b/drivers/pci/controller/dwc/pci-layerscape-ep.c
index 35bb481564c7..a4a800699f89 100644
--- a/drivers/pci/controller/dwc/pci-layerscape-ep.c
+++ b/drivers/pci/controller/dwc/pci-layerscape-ep.c
@@ -104,7 +104,7 @@ static irqreturn_t ls_pcie_ep_event_handler(int irq, void *dev_id)
dev_dbg(pci->dev, "Link up\n");
} else if (val & PEX_PF0_PME_MES_DR_LDD) {
dev_dbg(pci->dev, "Link down\n");
- pci_epc_linkdown(pci->ep.epc);
+ dw_pcie_ep_linkdown(&pci->ep);
} else if (val & PEX_PF0_PME_MES_DR_HRD) {
dev_dbg(pci->dev, "Hot reset\n");
}

--
2.25.1


2024-06-06 07:30:23

by Manivannan Sadhasivam

[permalink] [raw]
Subject: [PATCH 3/5] PCI: dwc: ep: Add a generic dw_pcie_ep_linkdown() API to handle Link Down event

As per the PCIe base spec r5.0, section 5.2, Link Down event can happen
under any of the following circumstances:

1. Fundamental/Hot reset
2. Link disable transmission by upstream component
3. Moving from L2/L3 to L0

In those cases, Link Down causes some non-sticky DWC registers to loose the
state (like REBAR, 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.

NOTE: For the sake of code organization, move the dw_pcie_ep_linkup()
definition just above dw_pcie_ep_linkdown().

Reviewed-by: Niklas Cassel <[email protected]>
Signed-off-by: Manivannan Sadhasivam <[email protected]>
---
drivers/pci/controller/dwc/pcie-designware-ep.c | 103 ++++++++++++++++--------
drivers/pci/controller/dwc/pcie-designware.h | 5 ++
2 files changed, 73 insertions(+), 35 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
index 78d5fc72c9cb..09ad6f7b5095 100644
--- a/drivers/pci/controller/dwc/pcie-designware-ep.c
+++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
@@ -15,18 +15,6 @@
#include <linux/pci-epc.h>
#include <linux/pci-epf.h>

-/**
- * 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;
-
- pci_epc_linkup(epc);
-}
-EXPORT_SYMBOL_GPL(dw_pcie_ep_linkup);
-
/**
* dw_pcie_ep_get_func_from_ep - Get the struct dw_pcie_ep_func corresponding to
* the endpoint function
@@ -661,6 +649,34 @@ 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;
+ unsigned int nbars;
+ u32 reg, i;
+
+ offset = dw_pcie_ep_find_ext_capability(pci, PCI_EXT_CAP_ID_REBAR);
+
+ 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;
+
+ /*
+ * PCIe r6.0, sec 7.8.6.2 require us to support at least one
+ * size in the range from 1 MB to 512 GB. Advertise support
+ * for 1 MB BAR size only.
+ */
+ for (i = 0; i < nbars; i++, offset += PCI_REBAR_CTRL)
+ dw_pcie_writel_dbi(pci, offset + PCI_REBAR_CAP, 0x0);
+ }
+
+ dw_pcie_setup(pci);
+ dw_pcie_dbi_ro_wr_dis(pci);
+}
+
/**
* dw_pcie_ep_init_registers - Initialize DWC EP specific registers
* @ep: DWC EP device
@@ -675,13 +691,11 @@ int dw_pcie_ep_init_registers(struct dw_pcie_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;
+ u32 ptm_cap_base, reg;
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;
@@ -744,25 +758,8 @@ 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;
-
- /*
- * PCIe r6.0, sec 7.8.6.2 require us to support at least one
- * size in the range from 1 MB to 512 GB. Advertise support
- * for 1 MB BAR size only.
- */
- for (i = 0; i < nbars; i++, offset += PCI_REBAR_CTRL)
- dw_pcie_writel_dbi(pci, offset + PCI_REBAR_CAP, BIT(4));
- }
-
/*
* PTM responder capability can be disabled only after disabling
* PTM root capability.
@@ -779,8 +776,7 @@ int dw_pcie_ep_init_registers(struct dw_pcie_ep *ep)
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;

@@ -791,6 +787,43 @@ 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;
+
+ pci_epc_linkup(epc);
+}
+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);
+ 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);
+
/**
* dw_pcie_ep_init - Initialize the endpoint device
* @ep: DWC EP device
diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index 49ae845a3662..89f9046af7eb 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_deinit(struct dw_pcie_ep *ep);
@@ -687,6 +688,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


2024-06-06 10:31:26

by Siddharth Vadapalli

[permalink] [raw]
Subject: Re: [PATCH 1/5] PCI: dwc: ep: Remove dw_pcie_ep_init_notify() wrapper

On Thu, Jun 06, 2024 at 12:56:34PM +0530, Manivannan Sadhasivam wrote:
> Currently dw_pcie_ep_init_notify() wrapper just calls pci_epc_init_notify()
> directly. So this wrapper provides no benefit to the glue drivers.
>
> So let's remove it and call pci_epc_init_notify() directly from glue
> drivers.
>
> Suggested-by: Bjorn Helgaas <[email protected]>
> Signed-off-by: Manivannan Sadhasivam <[email protected]>

Reviewed-by: Siddharth Vadapalli <[email protected]>

[...]

Regards,
Siddharth.

2024-06-06 11:05:02

by Siddharth Vadapalli

[permalink] [raw]
Subject: Re: [PATCH 2/5] PCI: endpoint: Introduce 'epc_deinit' event and notify the EPF drivers

On Thu, Jun 06, 2024 at 12:56:35PM +0530, Manivannan Sadhasivam wrote:
> As like the 'epc_init' event, that is used to signal the EPF drivers about
> the EPC initialization, let's introduce 'epc_deinit' event that is used to
> signal EPC deinitialization.
>
> The EPC deinitialization applies only when any sort of fundamental reset
> is supported by the endpoint controller as per the PCIe spec.
>
> Reference: PCIe Base spec v5.0, sections 4.2.4.9.1 and 6.6.1.
>
> Currently, some EPC drivers like pcie-qcom-ep and pcie-tegra194 support
> PERST# as the fundamental reset. So the 'deinit' event will be notified to
> the EPF drivers when PERST# assert happens in the above mentioned EPC
> drivers.
>
> The EPF drivers, on receiving the event through the epc_deinit() callback
> should reset the EPF state machine and also cleanup any configuration that
> got affected by the fundamental reset like BAR, DMA etc...
>
> This change also warrants skipping the cleanups in unbind() if already done
> in epc_deinit().
>
> Reviewed-by: Niklas Cassel <[email protected]>
> Signed-off-by: Manivannan Sadhasivam <[email protected]>

Reviewed-by: Siddharth Vadapalli <[email protected]>

[...]

Regards,
Siddharth.

2024-06-06 11:21:59

by Siddharth Vadapalli

[permalink] [raw]
Subject: Re: [PATCH 3/5] PCI: dwc: ep: Add a generic dw_pcie_ep_linkdown() API to handle Link Down event

On Thu, Jun 06, 2024 at 12:56:36PM +0530, Manivannan Sadhasivam wrote:
> As per the PCIe base spec r5.0, section 5.2, Link Down event can happen
> under any of the following circumstances:
>
> 1. Fundamental/Hot reset
> 2. Link disable transmission by upstream component
> 3. Moving from L2/L3 to L0
>
> In those cases, Link Down causes some non-sticky DWC registers to loose the
> state (like REBAR, 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.
>
> NOTE: For the sake of code organization, move the dw_pcie_ep_linkup()
> definition just above dw_pcie_ep_linkdown().
>
> Reviewed-by: Niklas Cassel <[email protected]>
> Signed-off-by: Manivannan Sadhasivam <[email protected]>

This patch already seems to be present in linux-next:
https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git/commit/?id=3d2e425263e2674713220379ad04e925efdb731d&h=next

Other patches in this series also seem to be merged.

[...]

Regards,
Siddharth.

2024-06-06 21:08:42

by Frank Li

[permalink] [raw]
Subject: Re: [PATCH 1/5] PCI: dwc: ep: Remove dw_pcie_ep_init_notify() wrapper

On Thu, Jun 06, 2024 at 12:56:34PM +0530, Manivannan Sadhasivam wrote:
> Currently dw_pcie_ep_init_notify() wrapper just calls pci_epc_init_notify()
> directly. So this wrapper provides no benefit to the glue drivers.
>
> So let's remove it and call pci_epc_init_notify() directly from glue
> drivers.
>
> Suggested-by: Bjorn Helgaas <[email protected]>
> 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-artpec6.c | 2 +-
> drivers/pci/controller/dwc/pcie-designware-ep.c | 12 ------------
> drivers/pci/controller/dwc/pcie-designware-plat.c | 2 +-
> drivers/pci/controller/dwc/pcie-designware.h | 5 -----
> drivers/pci/controller/dwc/pcie-keembay.c | 2 +-
> drivers/pci/controller/dwc/pcie-qcom-ep.c | 2 +-
> drivers/pci/controller/dwc/pcie-rcar-gen4.c | 2 +-
> drivers/pci/controller/dwc/pcie-tegra194.c | 2 +-
> drivers/pci/controller/dwc/pcie-uniphier-ep.c | 2 +-
> 13 files changed, 11 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pci-dra7xx.c b/drivers/pci/controller/dwc/pci-dra7xx.c
> index d2d17d37d3e0..e491d0ff3962 100644
> --- a/drivers/pci/controller/dwc/pci-dra7xx.c
> +++ b/drivers/pci/controller/dwc/pci-dra7xx.c
> @@ -474,7 +474,7 @@ static int dra7xx_add_pcie_ep(struct dra7xx_pcie *dra7xx,
> return ret;
> }
>
> - dw_pcie_ep_init_notify(ep);
> + pci_epc_init_notify(ep->epc);
>
> return 0;
> }
> diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> index 917c69edee1d..a876b8e6e741 100644
> --- a/drivers/pci/controller/dwc/pci-imx6.c
> +++ b/drivers/pci/controller/dwc/pci-imx6.c
> @@ -1131,7 +1131,7 @@ static int imx6_add_pcie_ep(struct imx6_pcie *imx6_pcie,
> return ret;
> }
>
> - dw_pcie_ep_init_notify(ep);
> + pci_epc_init_notify(ep->epc);
>
> /* 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 d3a7d14ee685..ca1054f5c79a 100644
> --- a/drivers/pci/controller/dwc/pci-keystone.c
> +++ b/drivers/pci/controller/dwc/pci-keystone.c
> @@ -1293,7 +1293,7 @@ static int ks_pcie_probe(struct platform_device *pdev)
> goto err_ep_init;
> }
>
> - dw_pcie_ep_init_notify(&pci->ep);
> + pci_epc_init_notify(pci->ep.epc);
>
> break;
> default:
> diff --git a/drivers/pci/controller/dwc/pci-layerscape-ep.c b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> index 7dde6d5fa4d8..35bb481564c7 100644
> --- a/drivers/pci/controller/dwc/pci-layerscape-ep.c
> +++ b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> @@ -286,7 +286,7 @@ static int __init ls_pcie_ep_probe(struct platform_device *pdev)
> return ret;
> }
>
> - dw_pcie_ep_init_notify(&pci->ep);
> + pci_epc_init_notify(pci->ep.epc);
>
> return ls_pcie_ep_interrupt_init(pcie, pdev);
> }
> diff --git a/drivers/pci/controller/dwc/pcie-artpec6.c b/drivers/pci/controller/dwc/pcie-artpec6.c
> index a4630b92489b..dc8dd7f27b78 100644
> --- a/drivers/pci/controller/dwc/pcie-artpec6.c
> +++ b/drivers/pci/controller/dwc/pcie-artpec6.c
> @@ -452,7 +452,7 @@ static int artpec6_pcie_probe(struct platform_device *pdev)
> return ret;
> }
>
> - dw_pcie_ep_init_notify(&pci->ep);
> + pci_epc_init_notify(pci->ep.epc);
>
> break;
> default:
> diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
> index 47391d7d3a73..2e69f81baf99 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
> @@ -27,18 +27,6 @@ void dw_pcie_ep_linkup(struct dw_pcie_ep *ep)
> }
> EXPORT_SYMBOL_GPL(dw_pcie_ep_linkup);
>
> -/**
> - * 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;
> -
> - pci_epc_init_notify(epc);
> -}
> -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
> diff --git a/drivers/pci/controller/dwc/pcie-designware-plat.c b/drivers/pci/controller/dwc/pcie-designware-plat.c
> index 8490c5d6ff9f..771b9d9be077 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-plat.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-plat.c
> @@ -154,7 +154,7 @@ static int dw_plat_pcie_probe(struct platform_device *pdev)
> dw_pcie_ep_deinit(&pci->ep);
> }
>
> - dw_pcie_ep_init_notify(&pci->ep);
> + pci_epc_init_notify(pci->ep.epc);
>
> break;
> default:
> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> index f8e5431a207b..49ae845a3662 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.h
> +++ b/drivers/pci/controller/dwc/pcie-designware.h
> @@ -670,7 +670,6 @@ static inline void __iomem *dw_pcie_own_conf_map_bus(struct pci_bus *bus,
> 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_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);
> int dw_pcie_ep_raise_intx_irq(struct dw_pcie_ep *ep, u8 func_no);
> @@ -698,10 +697,6 @@ static inline int dw_pcie_ep_init_registers(struct dw_pcie_ep *ep)
> return 0;
> }
>
> -static inline void dw_pcie_ep_init_notify(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-keembay.c b/drivers/pci/controller/dwc/pcie-keembay.c
> index 98bbc83182b4..278205db60a2 100644
> --- a/drivers/pci/controller/dwc/pcie-keembay.c
> +++ b/drivers/pci/controller/dwc/pcie-keembay.c
> @@ -442,7 +442,7 @@ static int keembay_pcie_probe(struct platform_device *pdev)
> return ret;
> }
>
> - dw_pcie_ep_init_notify(&pci->ep);
> + pci_epc_init_notify(pci->ep.epc);
>
> break;
> default:
> diff --git a/drivers/pci/controller/dwc/pcie-qcom-ep.c b/drivers/pci/controller/dwc/pcie-qcom-ep.c
> index 1ecf602c225a..4d2d7457dcb3 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom-ep.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom-ep.c
> @@ -482,7 +482,7 @@ static int qcom_pcie_perst_deassert(struct dw_pcie *pci)
> val &= ~PARF_MSTR_AXI_CLK_EN;
> writel_relaxed(val, pcie_ep->parf + PARF_MHI_CLOCK_RESET_CTRL);
>
> - dw_pcie_ep_init_notify(&pcie_ep->pci.ep);
> + pci_epc_init_notify(pcie_ep->pci.ep.epc);
>
> /* Enable LTSSM */
> val = readl_relaxed(pcie_ep->parf + PARF_LTSSM);
> diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.c b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> index cfeccc2f9ee1..237a6a8818de 100644
> --- a/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> +++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> @@ -437,7 +437,7 @@ 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);
> + pci_epc_init_notify(ep->epc);
>
> return ret;
> }
> diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c
> index 93f5433c5c55..432ed9d9a463 100644
> --- a/drivers/pci/controller/dwc/pcie-tegra194.c
> +++ b/drivers/pci/controller/dwc/pcie-tegra194.c
> @@ -1903,7 +1903,7 @@ static void pex_ep_event_pex_rst_deassert(struct tegra_pcie_dw *pcie)
> goto fail_init_complete;
> }
>
> - dw_pcie_ep_init_notify(ep);
> + pci_epc_init_notify(ep->epc);
>
> /* Program the private control to allow sending LTR upstream */
> if (pcie->of_data->has_ltr_req_fix) {
> diff --git a/drivers/pci/controller/dwc/pcie-uniphier-ep.c b/drivers/pci/controller/dwc/pcie-uniphier-ep.c
> index a2b844268e28..d6e73811216e 100644
> --- a/drivers/pci/controller/dwc/pcie-uniphier-ep.c
> +++ b/drivers/pci/controller/dwc/pcie-uniphier-ep.c
> @@ -410,7 +410,7 @@ static int uniphier_pcie_ep_probe(struct platform_device *pdev)
> return ret;
> }
>
> - dw_pcie_ep_init_notify(&priv->pci.ep);
> + pci_epc_init_notify(priv->pci.ep.epc);
>
> return 0;
> }
>
> --
> 2.25.1
>

2024-06-06 21:08:45

by Frank Li

[permalink] [raw]
Subject: Re: [PATCH 5/5] PCI: layerscape-ep: Use the generic dw_pcie_ep_linkdown() API to handle Link Down event

On Thu, Jun 06, 2024 at 12:56:38PM +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.
>
> Reported-by: Bjorn Helgaas <[email protected]>
> Closes: https://lore.kernel.org/linux-pci/20240528195539.GA458945@bhelgaas/
> Signed-off-by: Manivannan Sadhasivam <[email protected]>

Reviewed-by: Frank Li <[email protected]>

> ---
> drivers/pci/controller/dwc/pci-layerscape-ep.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/pci/controller/dwc/pci-layerscape-ep.c b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> index 35bb481564c7..a4a800699f89 100644
> --- a/drivers/pci/controller/dwc/pci-layerscape-ep.c
> +++ b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> @@ -104,7 +104,7 @@ static irqreturn_t ls_pcie_ep_event_handler(int irq, void *dev_id)
> dev_dbg(pci->dev, "Link up\n");
> } else if (val & PEX_PF0_PME_MES_DR_LDD) {
> dev_dbg(pci->dev, "Link down\n");
> - pci_epc_linkdown(pci->ep.epc);
> + dw_pcie_ep_linkdown(&pci->ep);
> } else if (val & PEX_PF0_PME_MES_DR_HRD) {
> dev_dbg(pci->dev, "Hot reset\n");
> }
>
> --
> 2.25.1
>

2024-06-06 21:11:02

by Frank Li

[permalink] [raw]
Subject: Re: [PATCH 2/5] PCI: endpoint: Introduce 'epc_deinit' event and notify the EPF drivers

On Thu, Jun 06, 2024 at 12:56:35PM +0530, Manivannan Sadhasivam wrote:
> As like the 'epc_init' event, that is used to signal the EPF drivers about
> the EPC initialization, let's introduce 'epc_deinit' event that is used to
> signal EPC deinitialization.
>
> The EPC deinitialization applies only when any sort of fundamental reset
> is supported by the endpoint controller as per the PCIe spec.
>
> Reference: PCIe Base spec v5.0, sections 4.2.4.9.1 and 6.6.1.
>
> Currently, some EPC drivers like pcie-qcom-ep and pcie-tegra194 support
> PERST# as the fundamental reset. So the 'deinit' event will be notified to
> the EPF drivers when PERST# assert happens in the above mentioned EPC
> drivers.
>
> The EPF drivers, on receiving the event through the epc_deinit() callback
> should reset the EPF state machine and also cleanup any configuration that
> got affected by the fundamental reset like BAR, DMA etc...
>
> This change also warrants skipping the cleanups in unbind() if already done
> in epc_deinit().
>
> Reviewed-by: Niklas Cassel <[email protected]>
> Signed-off-by: Manivannan Sadhasivam <[email protected]>

Reviewed-by: Frank Li <[email protected]>

> ---
> drivers/pci/controller/dwc/pcie-designware-ep.c | 1 -
> drivers/pci/controller/dwc/pcie-qcom-ep.c | 1 +
> drivers/pci/controller/dwc/pcie-tegra194.c | 1 +
> drivers/pci/endpoint/functions/pci-epf-mhi.c | 19 +++++++++++++++++++
> drivers/pci/endpoint/functions/pci-epf-test.c | 17 +++++++++++++++--
> drivers/pci/endpoint/pci-epc-core.c | 25 +++++++++++++++++++++++++
> include/linux/pci-epc.h | 13 +++++++++++++
> include/linux/pci-epf.h | 2 ++
> 8 files changed, 76 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
> index 2e69f81baf99..78d5fc72c9cb 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
> @@ -620,7 +620,6 @@ void dw_pcie_ep_cleanup(struct dw_pcie_ep *ep)
> struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
>
> dw_pcie_edma_remove(pci);
> - ep->epc->init_complete = false;
> }
> EXPORT_SYMBOL_GPL(dw_pcie_ep_cleanup);
>
> diff --git a/drivers/pci/controller/dwc/pcie-qcom-ep.c b/drivers/pci/controller/dwc/pcie-qcom-ep.c
> index 4d2d7457dcb3..2324e56c9bfc 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;
> }
>
> + pci_epc_deinit_notify(pci->ep.epc);
> 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 432ed9d9a463..4ca7404246a3 100644
> --- a/drivers/pci/controller/dwc/pcie-tegra194.c
> +++ b/drivers/pci/controller/dwc/pcie-tegra194.c
> @@ -1715,6 +1715,7 @@ 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);
>
> + pci_epc_deinit_notify(pcie->pci.ep.epc);
> dw_pcie_ep_cleanup(&pcie->pci.ep);
>
> reset_control_assert(pcie->core_rst);
> diff --git a/drivers/pci/endpoint/functions/pci-epf-mhi.c b/drivers/pci/endpoint/functions/pci-epf-mhi.c
> index 205c02953f25..5832989e55e8 100644
> --- a/drivers/pci/endpoint/functions/pci-epf-mhi.c
> +++ b/drivers/pci/endpoint/functions/pci-epf-mhi.c
> @@ -764,6 +764,24 @@ static int pci_epf_mhi_epc_init(struct pci_epf *epf)
> return 0;
> }
>
> +static void pci_epf_mhi_epc_deinit(struct pci_epf *epf)
> +{
> + struct pci_epf_mhi *epf_mhi = epf_get_drvdata(epf);
> + const struct pci_epf_mhi_ep_info *info = epf_mhi->info;
> + struct pci_epf_bar *epf_bar = &epf->bar[info->bar_num];
> + struct mhi_ep_cntrl *mhi_cntrl = &epf_mhi->mhi_cntrl;
> + struct pci_epc *epc = epf->epc;
> +
> + if (mhi_cntrl->mhi_dev) {
> + mhi_ep_power_down(mhi_cntrl);
> + if (info->flags & MHI_EPF_USE_DMA)
> + pci_epf_mhi_dma_deinit(epf_mhi);
> + mhi_ep_unregister_controller(mhi_cntrl);
> + }
> +
> + pci_epc_clear_bar(epc, epf->func_no, epf->vfunc_no, epf_bar);
> +}
> +
> static int pci_epf_mhi_link_up(struct pci_epf *epf)
> {
> struct pci_epf_mhi *epf_mhi = epf_get_drvdata(epf);
> @@ -898,6 +916,7 @@ static void pci_epf_mhi_unbind(struct pci_epf *epf)
>
> static const struct pci_epc_event_ops pci_epf_mhi_event_ops = {
> .epc_init = pci_epf_mhi_epc_init,
> + .epc_deinit = pci_epf_mhi_epc_deinit,
> .link_up = pci_epf_mhi_link_up,
> .link_down = pci_epf_mhi_link_down,
> .bus_master_enable = pci_epf_mhi_bus_master_enable,
> diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c
> index e771be7512a1..7c2ed6eae53a 100644
> --- a/drivers/pci/endpoint/functions/pci-epf-test.c
> +++ b/drivers/pci/endpoint/functions/pci-epf-test.c
> @@ -782,6 +782,15 @@ static int pci_epf_test_epc_init(struct pci_epf *epf)
> return 0;
> }
>
> +static void pci_epf_test_epc_deinit(struct pci_epf *epf)
> +{
> + struct pci_epf_test *epf_test = epf_get_drvdata(epf);
> +
> + cancel_delayed_work(&epf_test->cmd_handler);
> + pci_epf_test_clean_dma_chan(epf_test);
> + pci_epf_test_clear_bar(epf);
> +}
> +
> static int pci_epf_test_link_up(struct pci_epf *epf)
> {
> struct pci_epf_test *epf_test = epf_get_drvdata(epf);
> @@ -803,6 +812,7 @@ static int pci_epf_test_link_down(struct pci_epf *epf)
>
> static const struct pci_epc_event_ops pci_epf_test_event_ops = {
> .epc_init = pci_epf_test_epc_init,
> + .epc_deinit = pci_epf_test_epc_deinit,
> .link_up = pci_epf_test_link_up,
> .link_down = pci_epf_test_link_down,
> };
> @@ -905,10 +915,13 @@ static int pci_epf_test_bind(struct pci_epf *epf)
> static void pci_epf_test_unbind(struct pci_epf *epf)
> {
> struct pci_epf_test *epf_test = epf_get_drvdata(epf);
> + struct pci_epc *epc = epf->epc;
>
> cancel_delayed_work(&epf_test->cmd_handler);
> - pci_epf_test_clean_dma_chan(epf_test);
> - pci_epf_test_clear_bar(epf);
> + if (epc->init_complete) {
> + pci_epf_test_clean_dma_chan(epf_test);
> + pci_epf_test_clear_bar(epf);
> + }
> pci_epf_test_free_space(epf);
> }
>
> diff --git a/drivers/pci/endpoint/pci-epc-core.c b/drivers/pci/endpoint/pci-epc-core.c
> index 56b60330355d..47a91dcb07d7 100644
> --- a/drivers/pci/endpoint/pci-epc-core.c
> +++ b/drivers/pci/endpoint/pci-epc-core.c
> @@ -774,6 +774,31 @@ void pci_epc_notify_pending_init(struct pci_epc *epc, struct pci_epf *epf)
> }
> EXPORT_SYMBOL_GPL(pci_epc_notify_pending_init);
>
> +/**
> + * pci_epc_deinit_notify() - Notify the EPF device about EPC deinitialization
> + * @epc: the EPC device whose deinitialization is completed
> + *
> + * Invoke to notify the EPF device that the EPC deinitialization is completed.
> + */
> +void pci_epc_deinit_notify(struct pci_epc *epc)
> +{
> + struct pci_epf *epf;
> +
> + if (IS_ERR_OR_NULL(epc))
> + return;
> +
> + mutex_lock(&epc->list_lock);
> + list_for_each_entry(epf, &epc->pci_epf, list) {
> + mutex_lock(&epf->lock);
> + if (epf->event_ops && epf->event_ops->epc_deinit)
> + epf->event_ops->epc_deinit(epf);
> + mutex_unlock(&epf->lock);
> + }
> + epc->init_complete = false;
> + mutex_unlock(&epc->list_lock);
> +}
> +EXPORT_SYMBOL_GPL(pci_epc_deinit_notify);
> +
> /**
> * pci_epc_bus_master_enable_notify() - Notify the EPF device that the EPC
> * device has received the Bus Master
> diff --git a/include/linux/pci-epc.h b/include/linux/pci-epc.h
> index 11115cd0fe5b..85bdf2adb760 100644
> --- a/include/linux/pci-epc.h
> +++ b/include/linux/pci-epc.h
> @@ -197,6 +197,8 @@ struct pci_epc_features {
>
> #define to_pci_epc(device) container_of((device), struct pci_epc, dev)
>
> +#ifdef CONFIG_PCI_ENDPOINT
> +
> #define pci_epc_create(dev, ops) \
> __pci_epc_create((dev), (ops), THIS_MODULE)
> #define devm_pci_epc_create(dev, ops) \
> @@ -226,6 +228,7 @@ void pci_epc_linkup(struct pci_epc *epc);
> void pci_epc_linkdown(struct pci_epc *epc);
> void pci_epc_init_notify(struct pci_epc *epc);
> void pci_epc_notify_pending_init(struct pci_epc *epc, struct pci_epf *epf);
> +void pci_epc_deinit_notify(struct pci_epc *epc);
> void pci_epc_bus_master_enable_notify(struct pci_epc *epc);
> void pci_epc_remove_epf(struct pci_epc *epc, struct pci_epf *epf,
> enum pci_epc_interface_type type);
> @@ -272,4 +275,14 @@ void __iomem *pci_epc_mem_alloc_addr(struct pci_epc *epc,
> phys_addr_t *phys_addr, size_t size);
> void pci_epc_mem_free_addr(struct pci_epc *epc, phys_addr_t phys_addr,
> void __iomem *virt_addr, size_t size);
> +
> +#else
> +static inline void pci_epc_init_notify(struct pci_epc *epc)
> +{
> +}
> +
> +static inline void pci_epc_deinit_notify(struct pci_epc *epc)
> +{
> +}
> +#endif /* CONFIG_PCI_ENDPOINT */
> #endif /* __LINUX_PCI_EPC_H */
> diff --git a/include/linux/pci-epf.h b/include/linux/pci-epf.h
> index dc759eb7157c..0639d4dc8986 100644
> --- a/include/linux/pci-epf.h
> +++ b/include/linux/pci-epf.h
> @@ -71,12 +71,14 @@ struct pci_epf_ops {
> /**
> * struct pci_epc_event_ops - Callbacks for capturing the EPC events
> * @epc_init: Callback for the EPC initialization complete event
> + * @epc_deinit: Callback for the EPC deinitialization event
> * @link_up: Callback for the EPC link up event
> * @link_down: Callback for the EPC link down event
> * @bus_master_enable: Callback for the EPC Bus Master Enable event
> */
> struct pci_epc_event_ops {
> int (*epc_init)(struct pci_epf *epf);
> + void (*epc_deinit)(struct pci_epf *epf);
> int (*link_up)(struct pci_epf *epf);
> int (*link_down)(struct pci_epf *epf);
> int (*bus_master_enable)(struct pci_epf *epf);
>
> --
> 2.25.1
>

2024-06-07 09:22:07

by Niklas Cassel

[permalink] [raw]
Subject: Re: [PATCH 3/5] PCI: dwc: ep: Add a generic dw_pcie_ep_linkdown() API to handle Link Down event

On Thu, Jun 06, 2024 at 12:56:36PM +0530, Manivannan Sadhasivam wrote:
> As per the PCIe base spec r5.0, section 5.2, Link Down event can happen
> under any of the following circumstances:
>
> 1. Fundamental/Hot reset
> 2. Link disable transmission by upstream component
> 3. Moving from L2/L3 to L0
>
> In those cases, Link Down causes some non-sticky DWC registers to loose the
> state (like REBAR, 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.
>
> NOTE: For the sake of code organization, move the dw_pcie_ep_linkup()
> definition just above dw_pcie_ep_linkdown().
>
> Reviewed-by: Niklas Cassel <[email protected]>
> Signed-off-by: Manivannan Sadhasivam <[email protected]>
> ---

Like Siddharth reported, this patch is already in pci/next.

2024-06-07 09:22:32

by Niklas Cassel

[permalink] [raw]
Subject: Re: [PATCH 4/5] PCI: qcom-ep: Use the generic dw_pcie_ep_linkdown() API to handle Link Down event

On Thu, Jun 06, 2024 at 12:56:37PM +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.
>
> Reviewed-by: Niklas Cassel <[email protected]>
> 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 2324e56c9bfc..02a2a871a91f 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom-ep.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom-ep.c
> @@ -641,7 +641,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 Bus Master Enable event\n");
> pcie_ep->link_status = QCOM_PCIE_EP_LINK_ENABLED;
>
> --
> 2.25.1
>

Like Siddharth reported, this patch is already in pci/next.

2024-06-07 09:23:16

by Niklas Cassel

[permalink] [raw]
Subject: Re: [PATCH 5/5] PCI: layerscape-ep: Use the generic dw_pcie_ep_linkdown() API to handle Link Down event

On Thu, Jun 06, 2024 at 12:56:38PM +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.
>
> Reported-by: Bjorn Helgaas <[email protected]>
> Closes: https://lore.kernel.org/linux-pci/20240528195539.GA458945@bhelgaas/
> Signed-off-by: Manivannan Sadhasivam <[email protected]>
> ---
> drivers/pci/controller/dwc/pci-layerscape-ep.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/pci/controller/dwc/pci-layerscape-ep.c b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> index 35bb481564c7..a4a800699f89 100644
> --- a/drivers/pci/controller/dwc/pci-layerscape-ep.c
> +++ b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> @@ -104,7 +104,7 @@ static irqreturn_t ls_pcie_ep_event_handler(int irq, void *dev_id)
> dev_dbg(pci->dev, "Link up\n");
> } else if (val & PEX_PF0_PME_MES_DR_LDD) {
> dev_dbg(pci->dev, "Link down\n");
> - pci_epc_linkdown(pci->ep.epc);
> + dw_pcie_ep_linkdown(&pci->ep);
> } else if (val & PEX_PF0_PME_MES_DR_HRD) {
> dev_dbg(pci->dev, "Hot reset\n");
> }
>
> --
> 2.25.1
>

Reviewed-by: Niklas Cassel <[email protected]>

2024-06-07 09:27:24

by Niklas Cassel

[permalink] [raw]
Subject: Re: [PATCH 1/5] PCI: dwc: ep: Remove dw_pcie_ep_init_notify() wrapper

On Thu, Jun 06, 2024 at 12:56:34PM +0530, Manivannan Sadhasivam wrote:
> Currently dw_pcie_ep_init_notify() wrapper just calls pci_epc_init_notify()
> directly. So this wrapper provides no benefit to the glue drivers.
>
> So let's remove it and call pci_epc_init_notify() directly from glue
> drivers.
>
> Suggested-by: Bjorn Helgaas <[email protected]>
> Signed-off-by: Manivannan Sadhasivam <[email protected]>
> ---

Reviewed-by: Niklas Cassel <[email protected]>

2024-06-07 09:32:07

by Niklas Cassel

[permalink] [raw]
Subject: Re: [PATCH 0/5] PCI: endpoint: Add EPC 'deinit' event and dw_pcie_ep_linkdown() API

On Thu, Jun 06, 2024 at 12:56:33PM +0530, Manivannan Sadhasivam wrote:
> Hi,
>
> This series includes patches that were left over from previous series [1] for
> making the host reboot handling robust in endpoint framework.
>
> When the above mentioned series got merged to pci/endpoint, we got a bug report
> from LKP bot [2] and due to that the offending patches were dropped.
>
> This series addressed the issue reported by the bot by adding the stub APIs in
> include/pci/pci-epc.h and also removed the unused dwc wrapper as concluded in
> [3].
>
> Testing
> =======
>
> This series is tested on Qcom SM8450 based development board with 2 SM8450 SoCs
> connected over PCIe.
>
> - Mani
>
> [1] https://lore.kernel.org/linux-pci/[email protected]/
> [2] https://lore.kernel.org/linux-pci/[email protected]/
> [3] https://lore.kernel.org/linux-pci/20240529141614.GA3293@thinkpad/
>
> Signed-off-by: Manivannan Sadhasivam <[email protected]>
> ---
> Manivannan Sadhasivam (5):
> PCI: dwc: ep: Remove dw_pcie_ep_init_notify() wrapper
> PCI: endpoint: Introduce 'epc_deinit' event and notify the EPF drivers
> 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: layerscape-ep: Use the generic dw_pcie_ep_linkdown() API to handle Link Down event
>
> 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 | 4 +-
> drivers/pci/controller/dwc/pcie-artpec6.c | 2 +-
> drivers/pci/controller/dwc/pcie-designware-ep.c | 116 +++++++++++++---------
> drivers/pci/controller/dwc/pcie-designware-plat.c | 2 +-
> drivers/pci/controller/dwc/pcie-designware.h | 10 +-
> drivers/pci/controller/dwc/pcie-keembay.c | 2 +-
> drivers/pci/controller/dwc/pcie-qcom-ep.c | 5 +-
> drivers/pci/controller/dwc/pcie-rcar-gen4.c | 2 +-
> drivers/pci/controller/dwc/pcie-tegra194.c | 3 +-
> drivers/pci/controller/dwc/pcie-uniphier-ep.c | 2 +-
> drivers/pci/endpoint/functions/pci-epf-mhi.c | 19 ++++
> drivers/pci/endpoint/functions/pci-epf-test.c | 17 +++-
> drivers/pci/endpoint/pci-epc-core.c | 25 +++++
> include/linux/pci-epc.h | 13 +++
> include/linux/pci-epf.h | 2 +
> 18 files changed, 162 insertions(+), 68 deletions(-)
> ---
> base-commit: 7d96527bc16e46545739c6fe0ab6e4c915e9910e
> change-id: 20240606-pci-deinit-2e6cdf1bd69f
>
> Best regards,
> --
> Manivannan Sadhasivam <[email protected]>
>

Considering certain dependency patches have been merged to
pci/endpoint and other dependency patches have been merged to
pci/controller/dwc, perhaps it is best if you split this series:

Series 1 based on pci/endpoint: with patch 1/5 and 2/5.
Series 2 based on pci/controller/dwc: with patch 5/5.

Just a friendly suggestion to make Bjorn's life easier
(and grease the path of your patch) ;) [0]


Kind regards,
Niklas

[0]: https://lore.kernel.org/linux-pci/[email protected]/
(I still have this in my bookmarks :P)

2024-06-07 12:12:57

by Manivannan Sadhasivam

[permalink] [raw]
Subject: Re: [PATCH 0/5] PCI: endpoint: Add EPC 'deinit' event and dw_pcie_ep_linkdown() API

On Fri, Jun 07, 2024 at 11:31:28AM +0200, Niklas Cassel wrote:
> On Thu, Jun 06, 2024 at 12:56:33PM +0530, Manivannan Sadhasivam wrote:
> > Hi,
> >
> > This series includes patches that were left over from previous series [1] for
> > making the host reboot handling robust in endpoint framework.
> >
> > When the above mentioned series got merged to pci/endpoint, we got a bug report
> > from LKP bot [2] and due to that the offending patches were dropped.
> >
> > This series addressed the issue reported by the bot by adding the stub APIs in
> > include/pci/pci-epc.h and also removed the unused dwc wrapper as concluded in
> > [3].
> >
> > Testing
> > =======
> >
> > This series is tested on Qcom SM8450 based development board with 2 SM8450 SoCs
> > connected over PCIe.
> >
> > - Mani
> >
> > [1] https://lore.kernel.org/linux-pci/[email protected]/
> > [2] https://lore.kernel.org/linux-pci/[email protected]/
> > [3] https://lore.kernel.org/linux-pci/20240529141614.GA3293@thinkpad/
> >
> > Signed-off-by: Manivannan Sadhasivam <[email protected]>
> > ---
> > Manivannan Sadhasivam (5):
> > PCI: dwc: ep: Remove dw_pcie_ep_init_notify() wrapper
> > PCI: endpoint: Introduce 'epc_deinit' event and notify the EPF drivers
> > 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: layerscape-ep: Use the generic dw_pcie_ep_linkdown() API to handle Link Down event
> >
> > 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 | 4 +-
> > drivers/pci/controller/dwc/pcie-artpec6.c | 2 +-
> > drivers/pci/controller/dwc/pcie-designware-ep.c | 116 +++++++++++++---------
> > drivers/pci/controller/dwc/pcie-designware-plat.c | 2 +-
> > drivers/pci/controller/dwc/pcie-designware.h | 10 +-
> > drivers/pci/controller/dwc/pcie-keembay.c | 2 +-
> > drivers/pci/controller/dwc/pcie-qcom-ep.c | 5 +-
> > drivers/pci/controller/dwc/pcie-rcar-gen4.c | 2 +-
> > drivers/pci/controller/dwc/pcie-tegra194.c | 3 +-
> > drivers/pci/controller/dwc/pcie-uniphier-ep.c | 2 +-
> > drivers/pci/endpoint/functions/pci-epf-mhi.c | 19 ++++
> > drivers/pci/endpoint/functions/pci-epf-test.c | 17 +++-
> > drivers/pci/endpoint/pci-epc-core.c | 25 +++++
> > include/linux/pci-epc.h | 13 +++
> > include/linux/pci-epf.h | 2 +
> > 18 files changed, 162 insertions(+), 68 deletions(-)
> > ---
> > base-commit: 7d96527bc16e46545739c6fe0ab6e4c915e9910e
> > change-id: 20240606-pci-deinit-2e6cdf1bd69f
> >
> > Best regards,
> > --
> > Manivannan Sadhasivam <[email protected]>
> >
>
> Considering certain dependency patches have been merged to
> pci/endpoint and other dependency patches have been merged to
> pci/controller/dwc, perhaps it is best if you split this series:
>
> Series 1 based on pci/endpoint: with patch 1/5 and 2/5.
> Series 2 based on pci/controller/dwc: with patch 5/5.
>

Thanks Niklas! I didn't check the 'dwc' branch, so ended up posting patches 3/5
and 4/5 again.

Bjorn, if you are OK with this series, I can go ahead and apply patches 1/5 and
2/5 to 'pci/endpoint' and bank on Krzysztof to handle 5/5.

- Mani

--
மணிவண்ணன் சதாசிவம்

2024-06-10 06:53:48

by Manivannan Sadhasivam

[permalink] [raw]
Subject: Re: [PATCH 0/5] PCI: endpoint: Add EPC 'deinit' event and dw_pcie_ep_linkdown() API

On Thu, Jun 06, 2024 at 12:56:33PM +0530, Manivannan Sadhasivam wrote:
> Hi,
>
> This series includes patches that were left over from previous series [1] for
> making the host reboot handling robust in endpoint framework.
>
> When the above mentioned series got merged to pci/endpoint, we got a bug report
> from LKP bot [2] and due to that the offending patches were dropped.
>
> This series addressed the issue reported by the bot by adding the stub APIs in
> include/pci/pci-epc.h and also removed the unused dwc wrapper as concluded in
> [3].
>
> Testing
> =======
>
> This series is tested on Qcom SM8450 based development board with 2 SM8450 SoCs
> connected over PCIe.
>
> - Mani
>

Applied patch 2/5 to pci/endpoint! Krzysztof, please apply patches 1/5 and 5/5
to controller/dwc (patches 3/5 and 4/5 are already applied by you).

- Mani

> [1] https://lore.kernel.org/linux-pci/[email protected]/
> [2] https://lore.kernel.org/linux-pci/[email protected]/
> [3] https://lore.kernel.org/linux-pci/20240529141614.GA3293@thinkpad/
>
> Signed-off-by: Manivannan Sadhasivam <[email protected]>
> ---
> Manivannan Sadhasivam (5):
> PCI: dwc: ep: Remove dw_pcie_ep_init_notify() wrapper
> PCI: endpoint: Introduce 'epc_deinit' event and notify the EPF drivers
> 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: layerscape-ep: Use the generic dw_pcie_ep_linkdown() API to handle Link Down event
>
> 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 | 4 +-
> drivers/pci/controller/dwc/pcie-artpec6.c | 2 +-
> drivers/pci/controller/dwc/pcie-designware-ep.c | 116 +++++++++++++---------
> drivers/pci/controller/dwc/pcie-designware-plat.c | 2 +-
> drivers/pci/controller/dwc/pcie-designware.h | 10 +-
> drivers/pci/controller/dwc/pcie-keembay.c | 2 +-
> drivers/pci/controller/dwc/pcie-qcom-ep.c | 5 +-
> drivers/pci/controller/dwc/pcie-rcar-gen4.c | 2 +-
> drivers/pci/controller/dwc/pcie-tegra194.c | 3 +-
> drivers/pci/controller/dwc/pcie-uniphier-ep.c | 2 +-
> drivers/pci/endpoint/functions/pci-epf-mhi.c | 19 ++++
> drivers/pci/endpoint/functions/pci-epf-test.c | 17 +++-
> drivers/pci/endpoint/pci-epc-core.c | 25 +++++
> include/linux/pci-epc.h | 13 +++
> include/linux/pci-epf.h | 2 +
> 18 files changed, 162 insertions(+), 68 deletions(-)
> ---
> base-commit: 7d96527bc16e46545739c6fe0ab6e4c915e9910e
> change-id: 20240606-pci-deinit-2e6cdf1bd69f
>
> Best regards,
> --
> Manivannan Sadhasivam <[email protected]>
>

--
மணிவண்ணன் சதாசிவம்

2024-06-11 22:06:57

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [PATCH 2/5] PCI: endpoint: Introduce 'epc_deinit' event and notify the EPF drivers

On Thu, Jun 06, 2024 at 12:56:35PM +0530, Manivannan Sadhasivam wrote:
> As like the 'epc_init' event, that is used to signal the EPF drivers about
> the EPC initialization, let's introduce 'epc_deinit' event that is used to
> signal EPC deinitialization.
>
> The EPC deinitialization applies only when any sort of fundamental reset
> is supported by the endpoint controller as per the PCIe spec.
>
> Reference: PCIe Base spec v5.0, sections 4.2.4.9.1 and 6.6.1.

PCIe r6.0, sec 4.2.5.9.1 and 6.6.1.

(Not 4.2.4.9.1, which no longer exists in r6.x)

> Currently, some EPC drivers like pcie-qcom-ep and pcie-tegra194 support
> PERST# as the fundamental reset. So the 'deinit' event will be notified to
> the EPF drivers when PERST# assert happens in the above mentioned EPC
> drivers.
>
> The EPF drivers, on receiving the event through the epc_deinit() callback
> should reset the EPF state machine and also cleanup any configuration that
> got affected by the fundamental reset like BAR, DMA etc...
>
> This change also warrants skipping the cleanups in unbind() if already done
> in epc_deinit().
>
> Reviewed-by: Niklas Cassel <[email protected]>
> Signed-off-by: Manivannan Sadhasivam <[email protected]>

2024-06-12 04:36:43

by Manivannan Sadhasivam

[permalink] [raw]
Subject: Re: [PATCH 2/5] PCI: endpoint: Introduce 'epc_deinit' event and notify the EPF drivers

On Tue, Jun 11, 2024 at 05:06:40PM -0500, Bjorn Helgaas wrote:
> On Thu, Jun 06, 2024 at 12:56:35PM +0530, Manivannan Sadhasivam wrote:
> > As like the 'epc_init' event, that is used to signal the EPF drivers about
> > the EPC initialization, let's introduce 'epc_deinit' event that is used to
> > signal EPC deinitialization.
> >
> > The EPC deinitialization applies only when any sort of fundamental reset
> > is supported by the endpoint controller as per the PCIe spec.
> >
> > Reference: PCIe Base spec v5.0, sections 4.2.4.9.1 and 6.6.1.
>
> PCIe r6.0, sec 4.2.5.9.1 and 6.6.1.
>
> (Not 4.2.4.9.1, which no longer exists in r6.x)
>

Ammended the commit in pci/endpoint, thanks!

- Mani

> > Currently, some EPC drivers like pcie-qcom-ep and pcie-tegra194 support
> > PERST# as the fundamental reset. So the 'deinit' event will be notified to
> > the EPF drivers when PERST# assert happens in the above mentioned EPC
> > drivers.
> >
> > The EPF drivers, on receiving the event through the epc_deinit() callback
> > should reset the EPF state machine and also cleanup any configuration that
> > got affected by the fundamental reset like BAR, DMA etc...
> >
> > This change also warrants skipping the cleanups in unbind() if already done
> > in epc_deinit().
> >
> > Reviewed-by: Niklas Cassel <[email protected]>
> > Signed-off-by: Manivannan Sadhasivam <[email protected]>

--
மணிவண்ணன் சதாசிவம்

2024-06-13 15:49:25

by Markus Elfring

[permalink] [raw]
Subject: Re: [PATCH 2/5] PCI: endpoint: Introduce 'epc_deinit' event and notify the EPF drivers


> +++ b/drivers/pci/endpoint/pci-epc-core.c

> +void pci_epc_deinit_notify(struct pci_epc *epc)
> +{

> + mutex_lock(&epc->list_lock);
> + list_for_each_entry(epf, &epc->pci_epf, list) {
> + mutex_lock(&epf->lock);
> + if (epf->event_ops && epf->event_ops->epc_deinit)
> + epf->event_ops->epc_deinit(epf);
> + mutex_unlock(&epf->lock);
> + }
> + epc->init_complete = false;
> + mutex_unlock(&epc->list_lock);
> +}


Would you become interested to apply lock guards?
https://elixir.bootlin.com/linux/v6.10-rc3/source/include/linux/mutex.h#L196

Regards,
Markus