diff mbox series

[v2,05/13] PCI: endpoint: Assign PCI domain number for endpoint controllers

Message ID 20240717-pci-qcom-hotplug-v2-5-71d304b817f8@linaro.org
State New
Headers show
Series PCI: qcom: Simulate PCIe hotplug using 'global' interrupt | expand

Commit Message

Manivannan Sadhasivam via B4 Relay July 17, 2024, 5:03 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.

If the architecture doesn't support CONFIG_PCI_DOMAINS_GENERIC (rare), then
currently a warning is thrown to indicate that the architecture specific
implementation is needed.

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

Comments

Konrad Dybcio July 18, 2024, 12:11 p.m. UTC | #1 
On 17.07.2024 7:03 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.
> 
> If the architecture doesn't support CONFIG_PCI_DOMAINS_GENERIC (rare), then
> currently a warning is thrown to indicate that the architecture specific
> implementation is needed.
> 
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> ---
>  drivers/pci/endpoint/pci-epc-core.c | 10 ++++++++++
>  include/linux/pci-epc.h             |  2 ++
>  2 files changed, 12 insertions(+)
> 
> diff --git a/drivers/pci/endpoint/pci-epc-core.c b/drivers/pci/endpoint/pci-epc-core.c
> index 84309dfe0c68..7fa81b91e762 100644
> --- a/drivers/pci/endpoint/pci-epc-core.c
> +++ b/drivers/pci/endpoint/pci-epc-core.c
> @@ -838,6 +838,9 @@ void pci_epc_destroy(struct pci_epc *epc)
>  {
>  	pci_ep_cfs_remove_epc_group(epc->group);
>  	device_unregister(&epc->dev);
> +
> +	if (IS_ENABLED(CONFIG_PCI_DOMAINS_GENERIC))
> +		pci_bus_release_domain_nr(NULL, &epc->dev);	

Shouldn't this be called before device_unregister? pci/remove.c
does that (via pci_remove_bus() in pci_remove_root_bus())

Konrad
Manivannan Sadhasivam July 18, 2024, 3:01 p.m. UTC | #2 
On Thu, Jul 18, 2024 at 02:11:00PM +0200, Konrad Dybcio wrote:
> On 17.07.2024 7:03 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.
> > 
> > If the architecture doesn't support CONFIG_PCI_DOMAINS_GENERIC (rare), then
> > currently a warning is thrown to indicate that the architecture specific
> > implementation is needed.
> > 
> > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > ---
> >  drivers/pci/endpoint/pci-epc-core.c | 10 ++++++++++
> >  include/linux/pci-epc.h             |  2 ++
> >  2 files changed, 12 insertions(+)
> > 
> > diff --git a/drivers/pci/endpoint/pci-epc-core.c b/drivers/pci/endpoint/pci-epc-core.c
> > index 84309dfe0c68..7fa81b91e762 100644
> > --- a/drivers/pci/endpoint/pci-epc-core.c
> > +++ b/drivers/pci/endpoint/pci-epc-core.c
> > @@ -838,6 +838,9 @@ void pci_epc_destroy(struct pci_epc *epc)
> >  {
> >  	pci_ep_cfs_remove_epc_group(epc->group);
> >  	device_unregister(&epc->dev);
> > +
> > +	if (IS_ENABLED(CONFIG_PCI_DOMAINS_GENERIC))
> > +		pci_bus_release_domain_nr(NULL, &epc->dev);	
> 
> Shouldn't this be called before device_unregister? pci/remove.c
> does that (via pci_remove_bus() in pci_remove_root_bus())
> 

No, the release should follow the allocation order in __pci_epc_create. As per
that, pci_bus_release_domain_nr() should come last.

- Mani
kernel test robot July 18, 2024, 3:04 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/20240718-010848
base:   91e3b24eb7d297d9d99030800ed96944b8652eaf
patch link:    https://lore.kernel.org/r/20240717-pci-qcom-hotplug-v2-5-71d304b817f8%40linaro.org
patch subject: [PATCH v2 05/13] PCI: endpoint: Assign PCI domain number for endpoint controllers
config: i386-buildonly-randconfig-004-20240718 (https://download.01.org/0day-ci/archive/20240718/202407182239.m0d1pKRB-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/20240718/202407182239.m0d1pKRB-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/202407182239.m0d1pKRB-lkp@intel.com/

All errors (new ones prefixed by >>):

>> drivers/pci/endpoint/pci-epc-core.c:843:3: error: call to undeclared function 'pci_bus_release_domain_nr'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     843 |                 pci_bus_release_domain_nr(NULL, &epc->dev);
         |                 ^
   drivers/pci/endpoint/pci-epc-core.c:843:3: note: did you mean 'pci_bus_release_busn_res'?
   include/linux/pci.h:1142:6: note: 'pci_bus_release_busn_res' declared here
    1142 | void pci_bus_release_busn_res(struct pci_bus *b);
         |      ^
   drivers/pci/endpoint/pci-epc-core.c:911:20: error: call to undeclared function 'pci_bus_find_domain_nr'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     911 |                 epc->domain_nr = pci_bus_find_domain_nr(NULL, dev);
         |                                  ^
   2 errors generated.


vim +/pci_bus_release_domain_nr +843 drivers/pci/endpoint/pci-epc-core.c

   830	
   831	/**
   832	 * pci_epc_destroy() - destroy the EPC device
   833	 * @epc: the EPC device that has to be destroyed
   834	 *
   835	 * Invoke to destroy the PCI EPC device
   836	 */
   837	void pci_epc_destroy(struct pci_epc *epc)
   838	{
   839		pci_ep_cfs_remove_epc_group(epc->group);
   840		device_unregister(&epc->dev);
   841	
   842		if (IS_ENABLED(CONFIG_PCI_DOMAINS_GENERIC))
 > 843			pci_bus_release_domain_nr(NULL, &epc->dev);
   844	}
   845	EXPORT_SYMBOL_GPL(pci_epc_destroy);
   846
Manivannan Sadhasivam July 18, 2024, 3:32 p.m. UTC | #4 
On Thu, Jul 18, 2024 at 11:04:04PM +0800, kernel test robot wrote:
> 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/20240718-010848
> base:   91e3b24eb7d297d9d99030800ed96944b8652eaf
> patch link:    https://lore.kernel.org/r/20240717-pci-qcom-hotplug-v2-5-71d304b817f8%40linaro.org
> patch subject: [PATCH v2 05/13] PCI: endpoint: Assign PCI domain number for endpoint controllers
> config: i386-buildonly-randconfig-004-20240718 (https://download.01.org/0day-ci/archive/20240718/202407182239.m0d1pKRB-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/20240718/202407182239.m0d1pKRB-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/202407182239.m0d1pKRB-lkp@intel.com/
> 
> All errors (new ones prefixed by >>):
> 
> >> drivers/pci/endpoint/pci-epc-core.c:843:3: error: call to undeclared function 'pci_bus_release_domain_nr'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
>      843 |                 pci_bus_release_domain_nr(NULL, &epc->dev);
>          |                 ^

Hmm, should've used #ifdef as compiler can still access the statements under
'if(IS_ENABLED())' condition.

- Mani

>    drivers/pci/endpoint/pci-epc-core.c:843:3: note: did you mean 'pci_bus_release_busn_res'?
>    include/linux/pci.h:1142:6: note: 'pci_bus_release_busn_res' declared here
>     1142 | void pci_bus_release_busn_res(struct pci_bus *b);
>          |      ^
>    drivers/pci/endpoint/pci-epc-core.c:911:20: error: call to undeclared function 'pci_bus_find_domain_nr'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
>      911 |                 epc->domain_nr = pci_bus_find_domain_nr(NULL, dev);
>          |                                  ^
>    2 errors generated.
> 
> 
> vim +/pci_bus_release_domain_nr +843 drivers/pci/endpoint/pci-epc-core.c
> 
>    830	
>    831	/**
>    832	 * pci_epc_destroy() - destroy the EPC device
>    833	 * @epc: the EPC device that has to be destroyed
>    834	 *
>    835	 * Invoke to destroy the PCI EPC device
>    836	 */
>    837	void pci_epc_destroy(struct pci_epc *epc)
>    838	{
>    839		pci_ep_cfs_remove_epc_group(epc->group);
>    840		device_unregister(&epc->dev);
>    841	
>    842		if (IS_ENABLED(CONFIG_PCI_DOMAINS_GENERIC))
>  > 843			pci_bus_release_domain_nr(NULL, &epc->dev);
>    844	}
>    845	EXPORT_SYMBOL_GPL(pci_epc_destroy);
>    846	
> 
> -- 
> 0-DAY CI Kernel Test Service
> https://github.com/intel/lkp-tests/wiki
kernel test robot July 18, 2024, 3:37 p.m. UTC | #5 
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/20240718-010848
base:   91e3b24eb7d297d9d99030800ed96944b8652eaf
patch link:    https://lore.kernel.org/r/20240717-pci-qcom-hotplug-v2-5-71d304b817f8%40linaro.org
patch subject: [PATCH v2 05/13] PCI: endpoint: Assign PCI domain number for endpoint controllers
config: i386-randconfig-002-20240718 (https://download.01.org/0day-ci/archive/20240718/202407182326.ZHNfCp4Z-lkp@intel.com/config)
compiler: gcc-13 (Ubuntu 13.2.0-4ubuntu3) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240718/202407182326.ZHNfCp4Z-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/202407182326.ZHNfCp4Z-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/pci/endpoint/pci-epc-core.c: In function 'pci_epc_destroy':
>> drivers/pci/endpoint/pci-epc-core.c:843:17: error: implicit declaration of function 'pci_bus_release_domain_nr'; did you mean 'pci_bus_release_busn_res'? [-Werror=implicit-function-declaration]
     843 |                 pci_bus_release_domain_nr(NULL, &epc->dev);
         |                 ^~~~~~~~~~~~~~~~~~~~~~~~~
         |                 pci_bus_release_busn_res
   drivers/pci/endpoint/pci-epc-core.c: In function '__pci_epc_create':
   drivers/pci/endpoint/pci-epc-core.c:911:34: error: implicit declaration of function 'pci_bus_find_domain_nr' [-Werror=implicit-function-declaration]
     911 |                 epc->domain_nr = pci_bus_find_domain_nr(NULL, dev);
         |                                  ^~~~~~~~~~~~~~~~~~~~~~
   cc1: some warnings being treated as errors


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

   830	
   831	/**
   832	 * pci_epc_destroy() - destroy the EPC device
   833	 * @epc: the EPC device that has to be destroyed
   834	 *
   835	 * Invoke to destroy the PCI EPC device
   836	 */
   837	void pci_epc_destroy(struct pci_epc *epc)
   838	{
   839		pci_ep_cfs_remove_epc_group(epc->group);
   840		device_unregister(&epc->dev);
   841	
   842		if (IS_ENABLED(CONFIG_PCI_DOMAINS_GENERIC))
 > 843			pci_bus_release_domain_nr(NULL, &epc->dev);
   844	}
   845	EXPORT_SYMBOL_GPL(pci_epc_destroy);
   846
diff mbox series

Patch

diff --git a/drivers/pci/endpoint/pci-epc-core.c b/drivers/pci/endpoint/pci-epc-core.c
index 84309dfe0c68..7fa81b91e762 100644
--- a/drivers/pci/endpoint/pci-epc-core.c
+++ b/drivers/pci/endpoint/pci-epc-core.c
@@ -838,6 +838,9 @@  void pci_epc_destroy(struct pci_epc *epc)
 {
 	pci_ep_cfs_remove_epc_group(epc->group);
 	device_unregister(&epc->dev);
+
+	if (IS_ENABLED(CONFIG_PCI_DOMAINS_GENERIC))
+		pci_bus_release_domain_nr(NULL, &epc->dev);
 }
 EXPORT_SYMBOL_GPL(pci_epc_destroy);
 
@@ -900,6 +903,13 @@  __pci_epc_create(struct device *dev, const struct pci_epc_ops *ops,
 	epc->dev.release = pci_epc_release;
 	epc->ops = ops;
 
+	/*
+	 * TODO: If the architecture doesn't support generic PCI domains, then
+	 * a custom implementation has to be used.
+	 */
+	if (!WARN_ON_ONCE(!IS_ENABLED(CONFIG_PCI_DOMAINS_GENERIC)))
+		epc->domain_nr = pci_bus_find_domain_nr(NULL, dev);
+
 	ret = dev_set_name(&epc->dev, "%s", dev_name(dev));
 	if (ret)
 		goto put_dev;
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;
 };