On Fri, Mar 22, 2024 at 05:10:17PM +0100, Niklas Cassel wrote: > On Thu, Mar 14, 2024 at 08:53:45PM +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 '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 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 deinit(). > > > > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@xxxxxxxxxx> > > --- > > 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 | 1 + > > include/linux/pci-epf.h | 2 ++ > > 7 files changed, 64 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/pci/controller/dwc/pcie-qcom-ep.c b/drivers/pci/controller/dwc/pcie-qcom-ep.c > > index 50b1635e3cbb..e4b742355d57 100644 > > --- a/drivers/pci/controller/dwc/pcie-qcom-ep.c > > +++ b/drivers/pci/controller/dwc/pcie-qcom-ep.c > > @@ -501,6 +501,7 @@ static void qcom_pcie_perst_assert(struct dw_pcie *pci) > > { > > struct qcom_pcie_ep *pcie_ep = to_pcie_ep(pci); > > > > + pci_epc_deinit_notify(pci->ep.epc); > > Why not just move pci_epc_deinit_notify() in to dw_pcie_ep_cleanup() ? > (So that we don't need to add pci_epc_deinit_notify() to all EPC drivers > with PERST?) > This is mostly done to keep similarity with dw_pcie_ep_init_notify(). Even though it is a helper, it explicitly says that the function sends init notification. Otherwise, it will confuse developers on who is calling the deinit_notify(). I believe there is already enough mess to confuse the newcomers ;) - Mani > Regardless: > Reviewed-by: Niklas Cassel <cassel@xxxxxxxxxx> > > > 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 e02deb31a72d..3e6e08b321fb 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 4e4300efd9d7..83de96119718 100644 > > --- a/drivers/pci/endpoint/functions/pci-epf-mhi.c > > +++ b/drivers/pci/endpoint/functions/pci-epf-mhi.c > > @@ -748,6 +748,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); > > @@ -882,6 +900,7 @@ static void pci_epf_mhi_unbind(struct pci_epf *epf) > > > > static const struct pci_epc_event_ops pci_epf_mhi_epc_event_ops = { > > .init = pci_epf_mhi_epc_init, > > + .deinit = pci_epf_mhi_epc_deinit, > > }; > > > > static const struct pci_epc_bus_event_ops pci_epf_mhi_bus_event_ops = { > > diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c > > index 8f1e0cb08814..84cd47ebac22 100644 > > --- a/drivers/pci/endpoint/functions/pci-epf-test.c > > +++ b/drivers/pci/endpoint/functions/pci-epf-test.c > > @@ -804,6 +804,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); > > @@ -816,6 +825,7 @@ static int pci_epf_test_link_up(struct pci_epf *epf) > > > > static const struct pci_epc_event_ops pci_epf_test_epc_event_ops = { > > .init = pci_epf_test_epc_init, > > + .deinit = pci_epf_test_epc_deinit, > > }; > > > > static const struct pci_epc_bus_event_ops pci_epf_test_bus_event_ops = { > > @@ -954,10 +964,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 5a522b2842e2..26378a9a56a7 100644 > > --- a/drivers/pci/endpoint/pci-epc-core.c > > +++ b/drivers/pci/endpoint/pci-epc-core.c > > @@ -779,6 +779,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->epc_event_ops && epf->epc_event_ops->deinit) > > + epf->epc_event_ops->deinit(epf); > > + mutex_unlock(&epf->lock); > > + } > > + epc->init_complete = false; > > + mutex_unlock(&epc->list_lock); > > +} > > +EXPORT_SYMBOL_GPL(pci_epc_deinit_notify); > > + > > /** > > * pci_epc_bme_notify() - Notify the EPF device that the EPC device has received > > * the BME event from the Root complex > > diff --git a/include/linux/pci-epc.h b/include/linux/pci-epc.h > > index adee6dbe4e45..976b2212e872 100644 > > --- a/include/linux/pci-epc.h > > +++ b/include/linux/pci-epc.h > > @@ -199,6 +199,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_bme_notify(struct pci_epc *epc); > > void pci_epc_remove_epf(struct pci_epc *epc, struct pci_epf *epf, > > enum pci_epc_interface_type type); > > diff --git a/include/linux/pci-epf.h b/include/linux/pci-epf.h > > index ff8304e72f8e..52f69eaf505d 100644 > > --- a/include/linux/pci-epf.h > > +++ b/include/linux/pci-epf.h > > @@ -70,9 +70,11 @@ struct pci_epf_ops { > > /** > > * struct pci_epc_event_ops - Callbacks for capturing the EPC specific events > > * @init: Callback for the EPC initialization event > > + * @deinit: Callback for the EPC deinitialization event > > */ > > struct pci_epc_event_ops { > > int (*init)(struct pci_epf *epf); > > + void (*deinit)(struct pci_epf *epf); > > }; > > > > /** > > > > -- > > 2.25.1 > > -- மணிவண்ணன் சதாசிவம்