diff mbox series

[06/14] PCI: endpoint: Assign PCI domain number for endpoint controllers

Message ID 20240715-pci-qcom-hotplug-v1-6-5f3765cc873a@linaro.org (mailing list archive)
State Superseded
Headers show
Series PCI: qcom: Simulate PCIe hotplug using 'global' interrupt | expand

Commit Message

Manivannan Sadhasivam via B4 Relay July 15, 2024, 5:33 p.m. UTC 
From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

Right now, PCI endpoint subsystem doesn't assign PCI domain number for the
PCI endpoint controllers. But this domain number could be useful to the EPC
drivers to uniquely identify each controller based on the hardware instance
when there are multiple ones present in an SoC (even multiple RC/EP).

So let's make use of the existing pci_bus_find_domain_nr() API to allocate
domain numbers based on either Devicetree (linux,pci-domain) property or
dynamic domain number allocation scheme.

It should be noted that the domain number allocated by this API will be
based on both RC and EP controllers in a SoC. If the 'linux,pci-domain' DT
property is present, then the domain number represents the actual hardware
instance of the PCI endpoint controller. If not, then the domain number
will be allocated based on the PCI EP/RC controller probe order.

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 drivers/pci/endpoint/pci-epc-core.c | 1 +
 include/linux/pci-epc.h             | 2 ++
 2 files changed, 3 insertions(+)

Comments

Konrad Dybcio July 15, 2024, 8:02 p.m. UTC | #1 
On 15.07.2024 7:33 PM, Manivannan Sadhasivam via B4 Relay wrote:
> From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> 
> Right now, PCI endpoint subsystem doesn't assign PCI domain number for the
> PCI endpoint controllers. But this domain number could be useful to the EPC
> drivers to uniquely identify each controller based on the hardware instance
> when there are multiple ones present in an SoC (even multiple RC/EP).
> 
> So let's make use of the existing pci_bus_find_domain_nr() API to allocate
> domain numbers based on either Devicetree (linux,pci-domain) property or
> dynamic domain number allocation scheme.
> 
> It should be noted that the domain number allocated by this API will be
> based on both RC and EP controllers in a SoC. If the 'linux,pci-domain' DT
> property is present, then the domain number represents the actual hardware
> instance of the PCI endpoint controller. If not, then the domain number
> will be allocated based on the PCI EP/RC controller probe order.
> 
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> ---

The PCI counterpart does some error checking and requires
CONFIG_PCI_DOMAINS_GENERIC. Is that something that needs to be taken
care of here as well?

Konrad
Manivannan Sadhasivam July 16, 2024, 4:14 a.m. UTC | #2 
On Mon, Jul 15, 2024 at 10:02:18PM +0200, Konrad Dybcio wrote:
> On 15.07.2024 7:33 PM, Manivannan Sadhasivam via B4 Relay wrote:
> > From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > 
> > Right now, PCI endpoint subsystem doesn't assign PCI domain number for the
> > PCI endpoint controllers. But this domain number could be useful to the EPC
> > drivers to uniquely identify each controller based on the hardware instance
> > when there are multiple ones present in an SoC (even multiple RC/EP).
> > 
> > So let's make use of the existing pci_bus_find_domain_nr() API to allocate
> > domain numbers based on either Devicetree (linux,pci-domain) property or
> > dynamic domain number allocation scheme.
> > 
> > It should be noted that the domain number allocated by this API will be
> > based on both RC and EP controllers in a SoC. If the 'linux,pci-domain' DT
> > property is present, then the domain number represents the actual hardware
> > instance of the PCI endpoint controller. If not, then the domain number
> > will be allocated based on the PCI EP/RC controller probe order.
> > 
> > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > ---
> 
> The PCI counterpart does some error checking and requires
> CONFIG_PCI_DOMAINS_GENERIC. Is that something that needs to be taken
> care of here as well?
> 

Good catch. I excluded the Kconfig check initially during development as it was
selected by most of the architectures. But I clearly failed to revisit it later.

And yes, we do need to use the guard. I also missed freeing the domain during
epc_destroy() which I'll fix in next revision. Thanks!

- Mani
kernel test robot July 16, 2024, 3:41 p.m. UTC | #3 
Hi Manivannan,

kernel test robot noticed the following build errors:

[auto build test ERROR on 91e3b24eb7d297d9d99030800ed96944b8652eaf]

url:    https://github.com/intel-lab-lkp/linux/commits/Manivannan-Sadhasivam-via-B4-Relay/PCI-qcom-ep-Drop-the-redundant-masking-of-global-IRQ-events/20240716-014703
base:   91e3b24eb7d297d9d99030800ed96944b8652eaf
patch link:    https://lore.kernel.org/r/20240715-pci-qcom-hotplug-v1-6-5f3765cc873a%40linaro.org
patch subject: [PATCH 06/14] PCI: endpoint: Assign PCI domain number for endpoint controllers
config: i386-randconfig-001-20240716 (https://download.01.org/0day-ci/archive/20240716/202407162357.A9pxRKuo-lkp@intel.com/config)
compiler: clang version 18.1.5 (https://github.com/llvm/llvm-project 617a15a9eac96088ae5e9134248d8236e34b91b1)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240716/202407162357.A9pxRKuo-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202407162357.A9pxRKuo-lkp@intel.com/

All errors (new ones prefixed by >>):

>> drivers/pci/endpoint/pci-epc-core.c:902:19: error: call to undeclared function 'pci_bus_find_domain_nr'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     902 |         epc->domain_nr = pci_bus_find_domain_nr(NULL, dev);
         |                          ^
   1 error generated.


vim +/pci_bus_find_domain_nr +902 drivers/pci/endpoint/pci-epc-core.c

   866	
   867	/**
   868	 * __pci_epc_create() - create a new endpoint controller (EPC) device
   869	 * @dev: device that is creating the new EPC
   870	 * @ops: function pointers for performing EPC operations
   871	 * @owner: the owner of the module that creates the EPC device
   872	 *
   873	 * Invoke to create a new EPC device and add it to pci_epc class.
   874	 */
   875	struct pci_epc *
   876	__pci_epc_create(struct device *dev, const struct pci_epc_ops *ops,
   877			 struct module *owner)
   878	{
   879		int ret;
   880		struct pci_epc *epc;
   881	
   882		if (WARN_ON(!dev)) {
   883			ret = -EINVAL;
   884			goto err_ret;
   885		}
   886	
   887		epc = kzalloc(sizeof(*epc), GFP_KERNEL);
   888		if (!epc) {
   889			ret = -ENOMEM;
   890			goto err_ret;
   891		}
   892	
   893		mutex_init(&epc->lock);
   894		mutex_init(&epc->list_lock);
   895		INIT_LIST_HEAD(&epc->pci_epf);
   896	
   897		device_initialize(&epc->dev);
   898		epc->dev.class = &pci_epc_class;
   899		epc->dev.parent = dev;
   900		epc->dev.release = pci_epc_release;
   901		epc->ops = ops;
 > 902		epc->domain_nr = pci_bus_find_domain_nr(NULL, dev);
   903	
   904		ret = dev_set_name(&epc->dev, "%s", dev_name(dev));
   905		if (ret)
   906			goto put_dev;
   907	
   908		ret = device_add(&epc->dev);
   909		if (ret)
   910			goto put_dev;
   911	
   912		epc->group = pci_ep_cfs_add_epc_group(dev_name(dev));
   913	
   914		return epc;
   915	
   916	put_dev:
   917		put_device(&epc->dev);
   918	
   919	err_ret:
   920		return ERR_PTR(ret);
   921	}
   922	EXPORT_SYMBOL_GPL(__pci_epc_create);
   923
kernel test robot July 16, 2024, 4:47 p.m. UTC | #4 
Hi Manivannan,

kernel test robot noticed the following build errors:

[auto build test ERROR on 91e3b24eb7d297d9d99030800ed96944b8652eaf]

url:    https://github.com/intel-lab-lkp/linux/commits/Manivannan-Sadhasivam-via-B4-Relay/PCI-qcom-ep-Drop-the-redundant-masking-of-global-IRQ-events/20240716-014703
base:   91e3b24eb7d297d9d99030800ed96944b8652eaf
patch link:    https://lore.kernel.org/r/20240715-pci-qcom-hotplug-v1-6-5f3765cc873a%40linaro.org
patch subject: [PATCH 06/14] PCI: endpoint: Assign PCI domain number for endpoint controllers
config: i386-randconfig-011-20240716 (https://download.01.org/0day-ci/archive/20240717/202407170032.WTPexEVV-lkp@intel.com/config)
compiler: gcc-8 (Ubuntu 8.4.0-3ubuntu2) 8.4.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240717/202407170032.WTPexEVV-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202407170032.WTPexEVV-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/pci/endpoint/pci-epc-core.c: In function '__pci_epc_create':
>> drivers/pci/endpoint/pci-epc-core.c:902:19: error: implicit declaration of function 'pci_bus_find_domain_nr'; did you mean 'pci_bus_find_capability'? [-Werror=implicit-function-declaration]
     epc->domain_nr = pci_bus_find_domain_nr(NULL, dev);
                      ^~~~~~~~~~~~~~~~~~~~~~
                      pci_bus_find_capability
   cc1: some warnings being treated as errors


vim +902 drivers/pci/endpoint/pci-epc-core.c

   866	
   867	/**
   868	 * __pci_epc_create() - create a new endpoint controller (EPC) device
   869	 * @dev: device that is creating the new EPC
   870	 * @ops: function pointers for performing EPC operations
   871	 * @owner: the owner of the module that creates the EPC device
   872	 *
   873	 * Invoke to create a new EPC device and add it to pci_epc class.
   874	 */
   875	struct pci_epc *
   876	__pci_epc_create(struct device *dev, const struct pci_epc_ops *ops,
   877			 struct module *owner)
   878	{
   879		int ret;
   880		struct pci_epc *epc;
   881	
   882		if (WARN_ON(!dev)) {
   883			ret = -EINVAL;
   884			goto err_ret;
   885		}
   886	
   887		epc = kzalloc(sizeof(*epc), GFP_KERNEL);
   888		if (!epc) {
   889			ret = -ENOMEM;
   890			goto err_ret;
   891		}
   892	
   893		mutex_init(&epc->lock);
   894		mutex_init(&epc->list_lock);
   895		INIT_LIST_HEAD(&epc->pci_epf);
   896	
   897		device_initialize(&epc->dev);
   898		epc->dev.class = &pci_epc_class;
   899		epc->dev.parent = dev;
   900		epc->dev.release = pci_epc_release;
   901		epc->ops = ops;
 > 902		epc->domain_nr = pci_bus_find_domain_nr(NULL, dev);
   903	
   904		ret = dev_set_name(&epc->dev, "%s", dev_name(dev));
   905		if (ret)
   906			goto put_dev;
   907	
   908		ret = device_add(&epc->dev);
   909		if (ret)
   910			goto put_dev;
   911	
   912		epc->group = pci_ep_cfs_add_epc_group(dev_name(dev));
   913	
   914		return epc;
   915	
   916	put_dev:
   917		put_device(&epc->dev);
   918	
   919	err_ret:
   920		return ERR_PTR(ret);
   921	}
   922	EXPORT_SYMBOL_GPL(__pci_epc_create);
   923
diff mbox series

Patch

diff --git a/drivers/pci/endpoint/pci-epc-core.c b/drivers/pci/endpoint/pci-epc-core.c
index 84309dfe0c68..7e8bf4ac003a 100644
--- a/drivers/pci/endpoint/pci-epc-core.c
+++ b/drivers/pci/endpoint/pci-epc-core.c
@@ -899,6 +899,7 @@  __pci_epc_create(struct device *dev, const struct pci_epc_ops *ops,
 	epc->dev.parent = dev;
 	epc->dev.release = pci_epc_release;
 	epc->ops = ops;
+	epc->domain_nr = pci_bus_find_domain_nr(NULL, dev);
 
 	ret = dev_set_name(&epc->dev, "%s", dev_name(dev));
 	if (ret)
diff --git a/include/linux/pci-epc.h b/include/linux/pci-epc.h
index 85bdf2adb760..8e3dcac55dcd 100644
--- a/include/linux/pci-epc.h
+++ b/include/linux/pci-epc.h
@@ -128,6 +128,7 @@  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
+ * @domain_nr: PCI domain number of the endpoint controller
  * @init_complete: flag to indicate whether the EPC initialization is complete
  *                 or not
  */
@@ -145,6 +146,7 @@  struct pci_epc {
 	/* mutex to protect against concurrent access of EP controller */
 	struct mutex			lock;
 	unsigned long			function_num_map;
+	int				domain_nr;
 	bool				init_complete;
 };