Re: [Linaro-acpi] [PATCH V3 13/21] pci, acpi: Provide generic way to assign bus domain number.

On 21.01.2016 19:22, Lorenzo Pieralisi wrote:

Hi Tomasz,

On Wed, Jan 13, 2016 at 02:20:59PM +0100, Tomasz Nowicki wrote:

[...]

...

@@ -4796,14 +4797,34 @@ void pci_bus_assign_domain_nr(struct pci_bus *bus, struct device *parent) * API and update the use_dt_domains value to keep track of method we * are using to assign domain numbers (use_dt_domains = 0). *

• * IF ACPI, we expect non-DT method (use_dt_domains == -1)
  

• * and call _SEG method for corresponding host bridge device.
  

• * If _SEG method does not exist, following ACPI spec (6.5.6)
  

• * all PCI buses belong to domain 0.
  

• *
  

  • All other combinations imply we have a platform that is trying

• * to mix domain numbers obtained from DT and pci_get_new_domain_nr(),
  

• * which is a recipe for domain mishandling and it is prevented by
  

• * invalidating the domain value (domain = -1) and printing a
  

• * corresponding error.
  

• * to mix domain numbers obtained from DT, ACPI and
  

• * pci_get_new_domain_nr(), which is a recipe for domain mishandling and
  

• * it is prevented by invalidating the domain value (domain = -1) and
  

• * printing a corresponding error.
  

  */
• if (domain >= 0 && use_dt_domains) { use_dt_domains = 1;

+#ifdef CONFIG_ACPI

• } else if (!acpi_disabled && use_dt_domains == -1) {

• struct acpi_device *acpi_dev = to_acpi_device(parent);
  

• unsigned long long segment = 0;
  

• acpi_status status;
  

• status = acpi_evaluate_integer(acpi_dev->handle,
  

• 			       METHOD_NAME__SEG, NULL,
  

• 			       &segment);
  

• if (ACPI_FAILURE(status) && status != AE_NOT_FOUND)
  

• 	dev_err(&acpi_dev->dev,  "can't evaluate _SEG\n");
  

• domain = segment;
  

+#endif

I think you can reshuffle a bit the code to make it easier to follow.

How about this ? (on top of mainline, I just compiled it do not take it verbatim):

-- >8 -- diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index d1a7105..467a316 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -7,6 +7,7 @@

• Copyright 1997 -- 2000 Martin Mares mj@ucw.cz

*/

+#include <linux/acpi.h> #include <linux/kernel.h> #include <linux/delay.h> #include <linux/init.h> @@ -4769,7 +4770,27 @@ int pci_get_new_domain_nr(void) }

#ifdef CONFIG_PCI_DOMAINS_GENERIC -void pci_bus_assign_domain_nr(struct pci_bus *bus, struct device *parent) +/* Feel free to add this to drivers/acpi as a helper */ +#ifdef CONFIG_ACPI +int pci_bus_acpi_domain_nr(struct device *parent) +{

• struct acpi_device *acpi_dev = to_acpi_device(parent);
• unsigned long long segment = 0;
• acpi_status status;
• status = acpi_evaluate_integer(acpi_dev->handle,

• 		       METHOD_NAME__SEG, NULL,
  

• 		       &segment);
  

• if (ACPI_FAILURE(status) && status != AE_NOT_FOUND)

• dev_err(&acpi_dev->dev,  "can't evaluate _SEG\n");
  

• return segment;

+} +#else +int pci_bus_acpi_domain_nr(struct device *parent) { return -1; } +#endif

+int pci_bus_of_domain_nr(struct device *parent) { static int use_dt_domains = -1; int domain = of_get_pci_domain_nr(parent->of_node); @@ -4811,7 +4832,13 @@ void pci_bus_assign_domain_nr(struct pci_bus *bus, struct device *parent) domain = -1; }

• bus->domain_nr = domain;

• return domain;

+}

+void pci_bus_assign_domain_nr(struct pci_bus *bus, struct device *parent) +{

• bus->domain_nr = acpi_disabled ? pci_bus_of_domain_nr(parent) :

• 			 pci_bus_acpi_domain_nr(parent);
  

  } #endif #endif

It looks nice, I will integrate your patch. Thanks!

Tomasz