Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <04506b2a-dbcb-0bb5-496e-35ccfc9cc18b@ti.com>
Date:   Mon, 19 Sep 2022 14:28:33 +0530
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.11.0
Subject: Re: [PATCH v2 3/3] PCI: endpoint: Use link_up() callback in place of
 LINK_UP notifier
Content-Language: en-US
To:     Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>,
        <lpieralisi@kernel.org>, <bhelgaas@google.com>
CC:     <kw@linux.com>, <robh@kernel.org>, <vidyas@nvidia.com>,
        <linux-pci@vger.kernel.org>, <linux-kernel@vger.kernel.org>
References: <20220910090508.61157-1-manivannan.sadhasivam@linaro.org>
 <20220910090508.61157-4-manivannan.sadhasivam@linaro.org>
From:   Kishon Vijay Abraham I <kishon@ti.com>
In-Reply-To: <20220910090508.61157-4-manivannan.sadhasivam@linaro.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
Precedence: bulk

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@linaro.org>
> ---
>  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