diff mbox series

[02/12] pci: endpoint: do not export pci_epf_type_add_cfs()

Message ID 20230215032155.74993-3-damien.lemoal@opensource.wdc.com (mailing list archive)
State Superseded
Headers show
Series PCI endpoint fixes and improvements | expand

Commit Message

Damien Le Moal Feb. 15, 2023, 3:21 a.m. UTC 
pci_epf_type_add_cfs() is called only from pci_ep_cfs_add_type_group()
in drivers/pci/endpoint/pci-ep-cfs.c, so there is no need to export it
and function drivers should not call this function directly.

Remove the export for this function and move its declaration to the
internal header file drivers/pci/endpoint/pci-epf.h.

Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
---
 drivers/pci/endpoint/pci-ep-cfs.c   |  3 ++-
 drivers/pci/endpoint/pci-epf-core.c | 12 +++++-------
 drivers/pci/endpoint/pci-epf.h      | 14 ++++++++++++++
 include/linux/pci-epf.h             |  2 --
 4 files changed, 21 insertions(+), 10 deletions(-)
 create mode 100644 drivers/pci/endpoint/pci-epf.h

Comments

Manivannan Sadhasivam Feb. 16, 2023, 10:15 a.m. UTC | #1 
On Wed, Feb 15, 2023 at 12:21:45PM +0900, Damien Le Moal wrote:
> pci_epf_type_add_cfs() is called only from pci_ep_cfs_add_type_group()
> in drivers/pci/endpoint/pci-ep-cfs.c, so there is no need to export it
> and function drivers should not call this function directly.
> 

Where is the pci_ep_cfs_add_type_group() function defined?

> Remove the export for this function and move its declaration to the
> internal header file drivers/pci/endpoint/pci-epf.h.
> 
> Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
> ---
>  drivers/pci/endpoint/pci-ep-cfs.c   |  3 ++-
>  drivers/pci/endpoint/pci-epf-core.c | 12 +++++-------
>  drivers/pci/endpoint/pci-epf.h      | 14 ++++++++++++++
>  include/linux/pci-epf.h             |  2 --
>  4 files changed, 21 insertions(+), 10 deletions(-)
>  create mode 100644 drivers/pci/endpoint/pci-epf.h
> 
> diff --git a/drivers/pci/endpoint/pci-ep-cfs.c b/drivers/pci/endpoint/pci-ep-cfs.c
> index 1fb31f07199f..62b3b9e306fa 100644
> --- a/drivers/pci/endpoint/pci-ep-cfs.c
> +++ b/drivers/pci/endpoint/pci-ep-cfs.c
> @@ -11,9 +11,10 @@
>  #include <linux/slab.h>
>  
>  #include <linux/pci-epc.h>
> -#include <linux/pci-epf.h>
>  #include <linux/pci-ep-cfs.h>
>  
> +#include "pci-epf.h"
> +
>  static DEFINE_IDR(functions_idr);
>  static DEFINE_MUTEX(functions_mutex);
>  static struct config_group *functions_group;
> diff --git a/drivers/pci/endpoint/pci-epf-core.c b/drivers/pci/endpoint/pci-epf-core.c
> index 9ed556936f48..db121a58a586 100644
> --- a/drivers/pci/endpoint/pci-epf-core.c
> +++ b/drivers/pci/endpoint/pci-epf-core.c
> @@ -12,24 +12,23 @@
>  #include <linux/module.h>
>  
>  #include <linux/pci-epc.h>
> -#include <linux/pci-epf.h>
>  #include <linux/pci-ep-cfs.h>
>  
> +#include "pci-epf.h"
> +
>  static DEFINE_MUTEX(pci_epf_mutex);
>  
>  static struct bus_type pci_epf_bus_type;
>  static const struct device_type pci_epf_type;
>  
>  /**
> - * pci_epf_type_add_cfs() - Help function drivers to expose function specific
> - *                          attributes in configfs
> + * pci_epf_type_add_cfs() - Get a function driver specific attribute group.
>   * @epf: the EPF device that has to be configured using configfs
>   * @group: the parent configfs group (corresponding to entries in
>   *         pci_epf_device_id)
>   *
> - * Invoke to expose function specific attributes in configfs. If the function
> - * driver does not have anything to expose (attributes configured by user),
> - * return NULL.
> + * Called from pci_ep_cfs_add_type_group() when the function is created.
> + * If the function driver does not have anything to expose, return NULL.
>   */
>  struct config_group *pci_epf_type_add_cfs(struct pci_epf *epf,
>  					  struct config_group *group)
> @@ -50,7 +49,6 @@ struct config_group *pci_epf_type_add_cfs(struct pci_epf *epf,
>  
>  	return epf_type_group;
>  }
> -EXPORT_SYMBOL_GPL(pci_epf_type_add_cfs);
>  
>  /**
>   * pci_epf_unbind() - Notify the function driver that the binding between the
> diff --git a/drivers/pci/endpoint/pci-epf.h b/drivers/pci/endpoint/pci-epf.h
> new file mode 100644
> index 000000000000..b2f351afd623
> --- /dev/null
> +++ b/drivers/pci/endpoint/pci-epf.h

When there is already a pci-epf.h header available, creating one more even
under different location will create ambiguity. Please rename it as internal.h
or something relevant.

Thanks,
Mani

> @@ -0,0 +1,14 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * PCI Endpoint *Function* (EPF) internal header file
> + */
> +
> +#ifndef PCI_EPF_H
> +#define PCI_EPF_H
> +
> +#include <linux/pci-epf.h>
> +
> +struct config_group *pci_epf_type_add_cfs(struct pci_epf *epf,
> +					  struct config_group *group);
> +
> +#endif /* PCI_EPF_H */
> diff --git a/include/linux/pci-epf.h b/include/linux/pci-epf.h
> index 009a07147c61..b89cd8515073 100644
> --- a/include/linux/pci-epf.h
> +++ b/include/linux/pci-epf.h
> @@ -209,8 +209,6 @@ void pci_epf_free_space(struct pci_epf *epf, void *addr, enum pci_barno bar,
>  			enum pci_epc_interface_type type);
>  int pci_epf_bind(struct pci_epf *epf);
>  void pci_epf_unbind(struct pci_epf *epf);
> -struct config_group *pci_epf_type_add_cfs(struct pci_epf *epf,
> -					  struct config_group *group);
>  int pci_epf_add_vepf(struct pci_epf *epf_pf, struct pci_epf *epf_vf);
>  void pci_epf_remove_vepf(struct pci_epf *epf_pf, struct pci_epf *epf_vf);
>  #endif /* __LINUX_PCI_EPF_H */
> -- 
> 2.39.1
>
Damien Le Moal Feb. 16, 2023, 12:33 p.m. UTC | #2 
On 2/16/23 19:15, Manivannan Sadhasivam wrote:
> On Wed, Feb 15, 2023 at 12:21:45PM +0900, Damien Le Moal wrote:
>> pci_epf_type_add_cfs() is called only from pci_ep_cfs_add_type_group()
>> in drivers/pci/endpoint/pci-ep-cfs.c, so there is no need to export it
>> and function drivers should not call this function directly.
>>
> 
> Where is the pci_ep_cfs_add_type_group() function defined?

In patch 1.


>>  /**
>>   * pci_epf_unbind() - Notify the function driver that the binding between the
>> diff --git a/drivers/pci/endpoint/pci-epf.h b/drivers/pci/endpoint/pci-epf.h
>> new file mode 100644
>> index 000000000000..b2f351afd623
>> --- /dev/null
>> +++ b/drivers/pci/endpoint/pci-epf.h
> 
> When there is already a pci-epf.h header available, creating one more even
> under different location will create ambiguity. Please rename it as internal.h
> or something relevant.

OK.
diff mbox series

Patch

diff --git a/drivers/pci/endpoint/pci-ep-cfs.c b/drivers/pci/endpoint/pci-ep-cfs.c
index 1fb31f07199f..62b3b9e306fa 100644
--- a/drivers/pci/endpoint/pci-ep-cfs.c
+++ b/drivers/pci/endpoint/pci-ep-cfs.c
@@ -11,9 +11,10 @@ 
 #include <linux/slab.h>
 
 #include <linux/pci-epc.h>
-#include <linux/pci-epf.h>
 #include <linux/pci-ep-cfs.h>
 
+#include "pci-epf.h"
+
 static DEFINE_IDR(functions_idr);
 static DEFINE_MUTEX(functions_mutex);
 static struct config_group *functions_group;
diff --git a/drivers/pci/endpoint/pci-epf-core.c b/drivers/pci/endpoint/pci-epf-core.c
index 9ed556936f48..db121a58a586 100644
--- a/drivers/pci/endpoint/pci-epf-core.c
+++ b/drivers/pci/endpoint/pci-epf-core.c
@@ -12,24 +12,23 @@ 
 #include <linux/module.h>
 
 #include <linux/pci-epc.h>
-#include <linux/pci-epf.h>
 #include <linux/pci-ep-cfs.h>
 
+#include "pci-epf.h"
+
 static DEFINE_MUTEX(pci_epf_mutex);
 
 static struct bus_type pci_epf_bus_type;
 static const struct device_type pci_epf_type;
 
 /**
- * pci_epf_type_add_cfs() - Help function drivers to expose function specific
- *                          attributes in configfs
+ * pci_epf_type_add_cfs() - Get a function driver specific attribute group.
  * @epf: the EPF device that has to be configured using configfs
  * @group: the parent configfs group (corresponding to entries in
  *         pci_epf_device_id)
  *
- * Invoke to expose function specific attributes in configfs. If the function
- * driver does not have anything to expose (attributes configured by user),
- * return NULL.
+ * Called from pci_ep_cfs_add_type_group() when the function is created.
+ * If the function driver does not have anything to expose, return NULL.
  */
 struct config_group *pci_epf_type_add_cfs(struct pci_epf *epf,
 					  struct config_group *group)
@@ -50,7 +49,6 @@  struct config_group *pci_epf_type_add_cfs(struct pci_epf *epf,
 
 	return epf_type_group;
 }
-EXPORT_SYMBOL_GPL(pci_epf_type_add_cfs);
 
 /**
  * pci_epf_unbind() - Notify the function driver that the binding between the
diff --git a/drivers/pci/endpoint/pci-epf.h b/drivers/pci/endpoint/pci-epf.h
new file mode 100644
index 000000000000..b2f351afd623
--- /dev/null
+++ b/drivers/pci/endpoint/pci-epf.h
@@ -0,0 +1,14 @@ 
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * PCI Endpoint *Function* (EPF) internal header file
+ */
+
+#ifndef PCI_EPF_H
+#define PCI_EPF_H
+
+#include <linux/pci-epf.h>
+
+struct config_group *pci_epf_type_add_cfs(struct pci_epf *epf,
+					  struct config_group *group);
+
+#endif /* PCI_EPF_H */
diff --git a/include/linux/pci-epf.h b/include/linux/pci-epf.h
index 009a07147c61..b89cd8515073 100644
--- a/include/linux/pci-epf.h
+++ b/include/linux/pci-epf.h
@@ -209,8 +209,6 @@  void pci_epf_free_space(struct pci_epf *epf, void *addr, enum pci_barno bar,
 			enum pci_epc_interface_type type);
 int pci_epf_bind(struct pci_epf *epf);
 void pci_epf_unbind(struct pci_epf *epf);
-struct config_group *pci_epf_type_add_cfs(struct pci_epf *epf,
-					  struct config_group *group);
 int pci_epf_add_vepf(struct pci_epf *epf_pf, struct pci_epf *epf_vf);
 void pci_epf_remove_vepf(struct pci_epf *epf_pf, struct pci_epf *epf_vf);
 #endif /* __LINUX_PCI_EPF_H */