Re: [PATCH v2 09/10] PCI/DOE: Make mailbox creation API private

[Ira Weiny <ira.weiny@intel.com>]

Lukas Wunner wrote:
> The PCI core has just been amended to create a pci_doe_mb struct for
> every DOE instance on device enumeration.  CXL (the only in-tree DOE
> user so far) has been migrated to use those mailboxes instead of
> creating its own.
> 
> That leaves pcim_doe_create_mb() and pci_doe_for_each_off() without any
> callers, so drop them.
> 
> pci_doe_supports_prot() is now only used internally, so declare it
> static.
> 
> pci_doe_flush_mb() and pci_doe_destroy_mb() are no longer used as
> callbacks for devm_add_action(), so refactor them to accept a
> struct pci_doe_mb pointer instead of a generic void pointer.
> 
> Because pci_doe_create_mb() is only called on device enumeration, i.e.
> before driver binding, the workqueue name never contains a driver name.
> So replace dev_driver_string() with dev_bus_name() when generating the
> workqueue name.
> 
> Tested-by: Ira Weiny <ira.weiny@intel.com>

Reviewed-by: Ira Weiny <ira.weiny@intel.com>

> Signed-off-by: Lukas Wunner <lukas@wunner.de>
> ---
>  .clang-format           |  1 -
>  drivers/pci/doe.c       | 52 ++++-------------------------------------
>  include/linux/pci-doe.h | 14 -----------
>  3 files changed, 5 insertions(+), 62 deletions(-)
> 
> diff --git a/.clang-format b/.clang-format
> index b62836419ea3..cb1c17c7fcc9 100644
> --- a/.clang-format
> +++ b/.clang-format
> @@ -520,7 +520,6 @@ ForEachMacros:
>    - 'of_property_for_each_string'
>    - 'of_property_for_each_u32'
>    - 'pci_bus_for_each_resource'
> -  - 'pci_doe_for_each_off'
>    - 'pcl_for_each_chunk'
>    - 'pcl_for_each_segment'
>    - 'pcm_for_each_format'
> diff --git a/drivers/pci/doe.c b/drivers/pci/doe.c
> index 06c57af05570..0263bcfdddd8 100644
> --- a/drivers/pci/doe.c
> +++ b/drivers/pci/doe.c
> @@ -414,10 +414,8 @@ static int pci_doe_cache_protocols(struct pci_doe_mb *doe_mb)
>  	return 0;
>  }
>  
> -static void pci_doe_flush_mb(void *mb)
> +static void pci_doe_flush_mb(struct pci_doe_mb *doe_mb)
>  {
> -	struct pci_doe_mb *doe_mb = mb;
> -
>  	/* Stop all pending work items from starting */
>  	set_bit(PCI_DOE_FLAG_DEAD, &doe_mb->flags);
>  
> @@ -457,7 +455,7 @@ static struct pci_doe_mb *pci_doe_create_mb(struct pci_dev *pdev,
>  	xa_init(&doe_mb->prots);
>  
>  	doe_mb->work_queue = alloc_ordered_workqueue("%s %s DOE [%x]", 0,
> -						dev_driver_string(&pdev->dev),
> +						dev_bus_name(&pdev->dev),
>  						pci_name(pdev),
>  						doe_mb->cap_offset);
>  	if (!doe_mb->work_queue) {
> @@ -501,56 +499,17 @@ static struct pci_doe_mb *pci_doe_create_mb(struct pci_dev *pdev,
>  /**
>   * pci_doe_destroy_mb() - Destroy a DOE mailbox object
>   *
> - * @ptr: Pointer to DOE mailbox
> + * @doe_mb: DOE mailbox
>   *
>   * Destroy all internal data structures created for the DOE mailbox.
>   */
> -static void pci_doe_destroy_mb(void *ptr)
> +static void pci_doe_destroy_mb(struct pci_doe_mb *doe_mb)
>  {
> -	struct pci_doe_mb *doe_mb = ptr;
> -
>  	xa_destroy(&doe_mb->prots);
>  	destroy_workqueue(doe_mb->work_queue);
>  	kfree(doe_mb);
>  }
>  
> -/**
> - * pcim_doe_create_mb() - Create a DOE mailbox object
> - *
> - * @pdev: PCI device to create the DOE mailbox for
> - * @cap_offset: Offset of the DOE mailbox
> - *
> - * Create a single mailbox object to manage the mailbox protocol at the
> - * cap_offset specified.  The mailbox will automatically be destroyed on
> - * driver unbinding from @pdev.
> - *
> - * RETURNS: created mailbox object on success
> - *	    ERR_PTR(-errno) on failure
> - */
> -struct pci_doe_mb *pcim_doe_create_mb(struct pci_dev *pdev, u16 cap_offset)
> -{
> -	struct pci_doe_mb *doe_mb;
> -	int rc;
> -
> -	doe_mb = pci_doe_create_mb(pdev, cap_offset);
> -	if (IS_ERR(doe_mb))
> -		return doe_mb;
> -
> -	rc = devm_add_action(&pdev->dev, pci_doe_destroy_mb, doe_mb);
> -	if (rc) {
> -		pci_doe_flush_mb(doe_mb);
> -		pci_doe_destroy_mb(doe_mb);
> -		return ERR_PTR(rc);
> -	}
> -
> -	rc = devm_add_action_or_reset(&pdev->dev, pci_doe_flush_mb, doe_mb);
> -	if (rc)
> -		return ERR_PTR(rc);
> -
> -	return doe_mb;
> -}
> -EXPORT_SYMBOL_GPL(pcim_doe_create_mb);
> -
>  /**
>   * pci_doe_supports_prot() - Return if the DOE instance supports the given
>   *			     protocol
> @@ -560,7 +519,7 @@ EXPORT_SYMBOL_GPL(pcim_doe_create_mb);
>   *
>   * RETURNS: True if the DOE mailbox supports the protocol specified
>   */
> -bool pci_doe_supports_prot(struct pci_doe_mb *doe_mb, u16 vid, u8 type)
> +static bool pci_doe_supports_prot(struct pci_doe_mb *doe_mb, u16 vid, u8 type)
>  {
>  	unsigned long index;
>  	void *entry;
> @@ -575,7 +534,6 @@ bool pci_doe_supports_prot(struct pci_doe_mb *doe_mb, u16 vid, u8 type)
>  
>  	return false;
>  }
> -EXPORT_SYMBOL_GPL(pci_doe_supports_prot);
>  
>  /**
>   * pci_doe_submit_task() - Submit a task to be processed by the state machine
> diff --git a/include/linux/pci-doe.h b/include/linux/pci-doe.h
> index d6192ee0ac07..1f14aed4354b 100644
> --- a/include/linux/pci-doe.h
> +++ b/include/linux/pci-doe.h
> @@ -15,20 +15,6 @@
>  
>  struct pci_doe_mb;
>  
> -/**
> - * pci_doe_for_each_off - Iterate each DOE capability
> - * @pdev: struct pci_dev to iterate
> - * @off: u16 of config space offset of each mailbox capability found
> - */
> -#define pci_doe_for_each_off(pdev, off) \
> -	for (off = pci_find_next_ext_capability(pdev, off, \
> -					PCI_EXT_CAP_ID_DOE); \
> -		off > 0; \
> -		off = pci_find_next_ext_capability(pdev, off, \
> -					PCI_EXT_CAP_ID_DOE))
> -
> -struct pci_doe_mb *pcim_doe_create_mb(struct pci_dev *pdev, u16 cap_offset);
> -bool pci_doe_supports_prot(struct pci_doe_mb *doe_mb, u16 vid, u8 type);
>  struct pci_doe_mb *pci_find_doe_mailbox(struct pci_dev *pdev, u16 vendor,
>  					u8 type);
>  
> -- 
> 2.39.1
>