Re: [PATCH v2 3/3] PCI: endpoint: Use link_up() callback in place of LINK_UP notifier

From: Kishon Vijay Abraham I
Date: Mon Sep 19 2022 - 04:58:58 EST


Hi Mani,

On 10/09/22 14:35, Manivannan Sadhasivam wrote:
> As a part of the transition towards callback mechanism for signalling the
> events from EPC to EPF, let's use the link_up() callback in the place of
> the LINK_UP notifier. This also removes the notifier support completely
> from the PCI endpoint framework.
>
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@xxxxxxxxxx>
> ---
> drivers/pci/endpoint/functions/pci-epf-test.c | 33 ++++++-------------
> drivers/pci/endpoint/pci-epc-core.c | 12 +++++--
> include/linux/pci-epc.h | 8 -----
> include/linux/pci-epf.h | 8 ++---
> 4 files changed, 22 insertions(+), 39 deletions(-)
>
> diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c
> index 868de17e1ad2..f75045f2dee3 100644
> --- a/drivers/pci/endpoint/functions/pci-epf-test.c
> +++ b/drivers/pci/endpoint/functions/pci-epf-test.c
> @@ -826,30 +826,21 @@ static int pci_epf_test_core_init(struct pci_epf *epf)
> return 0;
> }
>
> -static const struct pci_epc_event_ops pci_epf_test_event_ops = {
> - .core_init = pci_epf_test_core_init,
> -};
> -
> -static int pci_epf_test_notifier(struct notifier_block *nb, unsigned long val,
> - void *data)
> +int pci_epf_test_link_up(struct pci_epf *epf)
> {
> - struct pci_epf *epf = container_of(nb, struct pci_epf, nb);
> struct pci_epf_test *epf_test = epf_get_drvdata(epf);
>
> - switch (val) {
> - case LINK_UP:
> - queue_delayed_work(kpcitest_workqueue, &epf_test->cmd_handler,
> - msecs_to_jiffies(1));
> - break;
> -
> - default:
> - dev_err(&epf->dev, "Invalid EPF test notifier event\n");
> - return NOTIFY_BAD;
> - }
> + queue_delayed_work(kpcitest_workqueue, &epf_test->cmd_handler,
> + msecs_to_jiffies(1));
>
> - return NOTIFY_OK;
> + return 0;
> }
>
> +static const struct pci_epc_event_ops pci_epf_test_event_ops = {
> + .core_init = pci_epf_test_core_init,
> + .link_up = pci_epf_test_link_up,
> +};
> +
> static int pci_epf_test_alloc_space(struct pci_epf *epf)
> {
> struct pci_epf_test *epf_test = epf_get_drvdata(epf);
> @@ -976,12 +967,8 @@ static int pci_epf_test_bind(struct pci_epf *epf)
> if (ret)
> epf_test->dma_supported = false;
>
> - if (linkup_notifier || core_init_notifier) {
> - epf->nb.notifier_call = pci_epf_test_notifier;
> - pci_epc_register_notifier(epc, &epf->nb);
> - } else {
> + if (!linkup_notifier && !core_init_notifier)
> queue_work(kpcitest_workqueue, &epf_test->cmd_handler.work);
> - }
>
> return 0;
> }
> diff --git a/drivers/pci/endpoint/pci-epc-core.c b/drivers/pci/endpoint/pci-epc-core.c
> index ba54f17ae06f..5dac1496cf16 100644
> --- a/drivers/pci/endpoint/pci-epc-core.c
> +++ b/drivers/pci/endpoint/pci-epc-core.c
> @@ -690,10 +690,19 @@ EXPORT_SYMBOL_GPL(pci_epc_remove_epf);
> */
> void pci_epc_linkup(struct pci_epc *epc)
> {
> + struct pci_epf *epf;
> +
> if (!epc || IS_ERR(epc))
> return;
>
> - atomic_notifier_call_chain(&epc->notifier, LINK_UP, NULL);
> + mutex_lock(&epc->list_lock);

This will break pci-dra7xx which invokes pci_epc_linkup() in interrupt
context.

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/pci/controller/dwc/pci-dra7xx.c#n332

dra7xx_pcie_irq_handler()
|
|
dw_pcie_ep_linkup()
|
|
pci_epc_linkup()
|
|
mutex_lock()

Thanks,
Kishon