lkml.org 
[lkml]   [2015]   [Oct]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [Linaro-acpi] [PATCH V1 10/11] pci, acpi: Provide generic way to assign bus domain number.
From
Date
Hi Liviu,

On 28.10.2015 12:38, Liviu.Dudau@arm.com wrote:
> On Tue, Oct 27, 2015 at 05:38:41PM +0100, Tomasz Nowicki wrote:
>> Architectures which support PCI_DOMAINS_GENERIC (like ARM64)
>> cannot call pci_bus_assign_domain_nr along ACPI PCI host bridge
>> initialization since this function needs valid parent device reference
>> to be able to retrieve domain number (aka segment).
>>
>> We can omit that blocker and pass down host bridge device via
>> pci_create_root_bus parameter and then be able to evaluate _SEG method
>> being in pci_bus_assign_domain_nr.
>>
>> Note that _SEG method is optional, therefore _SEG absence means
>> that all PCI buses belong to domain 0.
>>
>> Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
>> ---
>> drivers/acpi/pci_root.c | 2 +-
>> drivers/pci/pci.c | 32 +++++++++++++++++++++++++++-----
>> 2 files changed, 28 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
>> index 850d7bf..e682dc6 100644
>> --- a/drivers/acpi/pci_root.c
>> +++ b/drivers/acpi/pci_root.c
>> @@ -839,7 +839,7 @@ struct pci_bus *acpi_pci_root_create(struct acpi_pci_root *root,
>>
>> pci_acpi_root_add_resources(info);
>> pci_add_resource(&info->resources, &root->secondary);
>> - bus = pci_create_root_bus(NULL, busnum, ops->pci_ops,
>> + bus = pci_create_root_bus(&device->dev, busnum, ops->pci_ops,
>> sysdata, &info->resources);
>
> Not sure this change should be in this patch, I don't see the relation.
>
> To put it differently: I think the patch should introduce the retrieval of the
> domain number from _SEG method and leave the passing of a valid host bridge device
> to a more appropriate patch.

I wanted to highlight that ACPI kernel using PCI_DOMAINS_GENERIC needs
to have both in place:
1. Obtaining domain from _SEG method
2. And host bridge device reference for which we can call _SEG.
But you are right, it will be more clear if I split up patch and
describe it in separate changelog.

>
>
>> if (!bus)
>> goto out_release_info;
>> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
>> index 6a9a111..17d1857 100644
>> --- a/drivers/pci/pci.c
>> +++ b/drivers/pci/pci.c
>> @@ -25,6 +25,7 @@
>> #include <linux/device.h>
>> #include <linux/pm_runtime.h>
>> #include <linux/pci_hotplug.h>
>> +#include <linux/acpi.h>
>> #include <asm-generic/pci-bridge.h>
>> #include <asm/setup.h>
>> #include "pci.h"
>> @@ -4501,7 +4502,7 @@ int pci_get_new_domain_nr(void)
>> void pci_bus_assign_domain_nr(struct pci_bus *bus, struct device *parent)
>> {
>> static int use_dt_domains = -1;
>> - int domain = of_get_pci_domain_nr(parent->of_node);
>> + int domain;
>>
>> /*
>> * Check DT domain and use_dt_domains values.
>> @@ -4523,14 +4524,35 @@ 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.
>> */
>> +
>> + domain = of_get_pci_domain_nr(parent->of_node);
>
> Not sure what you've got here by splitting the original line into two other than an increase
> in the change count.

Yes, it does not make sense to split the original line. I will fix that.

>
> Otherwise, it looks sensible.
>
> Reviewed-by: Liviu Dudau <Liviu.Dudau@arm.com>

Thanks Liviu!

Regards,
Tomasz


\
 
 \ /
  Last update: 2015-10-28 14:01    [W:0.238 / U:0.396 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site