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

[Kishon Vijay Abraham I <kvijayab@amd.com>]

On 10/25/2022 8:21 PM, 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>

Acked-by: Kishon Vijay Abraham I <kishon@kernel.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 8bb60528f530..c0954fddcc14 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);
> +	list_for_each_entry(epf, &epc->pci_epf, list) {
> +		mutex_lock(&epf->lock);
> +		if (epf->event_ops && epf->event_ops->link_up)
> +			epf->event_ops->link_up(epf);
> +		mutex_unlock(&epf->lock);
> +	}
> +	mutex_unlock(&epc->list_lock);
>  }
>  EXPORT_SYMBOL_GPL(pci_epc_linkup);
>  
> @@ -784,7 +793,6 @@ __pci_epc_create(struct device *dev, const struct pci_epc_ops *ops,
>  	mutex_init(&epc->lock);
>  	mutex_init(&epc->list_lock);
>  	INIT_LIST_HEAD(&epc->pci_epf);
> -	ATOMIC_INIT_NOTIFIER_HEAD(&epc->notifier);
>  
>  	device_initialize(&epc->dev);
>  	epc->dev.class = pci_epc_class;
> diff --git a/include/linux/pci-epc.h b/include/linux/pci-epc.h
> index fe729dfe509b..301bb0e53707 100644
> --- a/include/linux/pci-epc.h
> +++ b/include/linux/pci-epc.h
> @@ -135,7 +135,6 @@ struct pci_epc_mem {
>   * @group: configfs group representing the PCI EPC device
>   * @lock: mutex to protect pci_epc ops
>   * @function_num_map: bitmap to manage physical function number
> - * @notifier: used to notify EPF of any EPC events (like linkup)
>   */
>  struct pci_epc {
>  	struct device			dev;
> @@ -151,7 +150,6 @@ struct pci_epc {
>  	/* mutex to protect against concurrent access of EP controller */
>  	struct mutex			lock;
>  	unsigned long			function_num_map;
> -	struct atomic_notifier_head	notifier;
>  };
>  
>  /**
> @@ -194,12 +192,6 @@ static inline void *epc_get_drvdata(struct pci_epc *epc)
>  	return dev_get_drvdata(&epc->dev);
>  }
>  
> -static inline int
> -pci_epc_register_notifier(struct pci_epc *epc, struct notifier_block *nb)
> -{
> -	return atomic_notifier_chain_register(&epc->notifier, nb);
> -}
> -
>  struct pci_epc *
>  __devm_pci_epc_create(struct device *dev, const struct pci_epc_ops *ops,
>  		      struct module *owner);
> diff --git a/include/linux/pci-epf.h b/include/linux/pci-epf.h
> index a06f3b4c8bee..bc613f0df7e3 100644
> --- a/include/linux/pci-epf.h
> +++ b/include/linux/pci-epf.h
> @@ -17,10 +17,6 @@
>  struct pci_epf;
>  enum pci_epc_interface_type;
>  
> -enum pci_notify_event {
> -	LINK_UP,
> -};
> -
>  enum pci_barno {
>  	NO_BAR = -1,
>  	BAR_0,
> @@ -74,9 +70,11 @@ struct pci_epf_ops {
>  /**
>   * struct pci_epf_event_ops - Callbacks for capturing the EPC events
>   * @core_init: Callback for the EPC initialization complete event
> + * @link_up: Callback for the EPC link up event
>   */
>  struct pci_epc_event_ops {
>  	int (*core_init)(struct pci_epf *epf);
> +	int (*link_up)(struct pci_epf *epf);
>  };
>  
>  /**
> @@ -135,7 +133,6 @@ struct pci_epf_bar {
>   * @driver: the EPF driver to which this EPF device is bound
>   * @id: Pointer to the EPF device ID
>   * @list: to add pci_epf as a list of PCI endpoint functions to pci_epc
> - * @nb: notifier block to notify EPF of any EPC events (like linkup)
>   * @lock: mutex to protect pci_epf_ops
>   * @sec_epc: the secondary EPC device to which this EPF device is bound
>   * @sec_epc_list: to add pci_epf as list of PCI endpoint functions to secondary
> @@ -164,7 +161,6 @@ struct pci_epf {
>  	struct pci_epf_driver	*driver;
>  	const struct pci_epf_device_id *id;
>  	struct list_head	list;
> -	struct notifier_block   nb;
>  	/* mutex to protect against concurrent access of pci_epf_ops */
>  	struct mutex		lock;
>