Re: [Patch v7 4/7] PCI/ACPI: Add interface acpi_pci_root_create()

[Liviu Dudau <Liviu.Dudau@arm.com>]

On Wed, Oct 21, 2015 at 07:49:13PM +0800, Jiang Liu wrote:
> On 2015/10/21 19:27, Tomasz Nowicki wrote:
> > On 21.10.2015 13:02, Liviu Dudau wrote:
> >> On Wed, Oct 21, 2015 at 11:57:53AM +0200, Tomasz Nowicki wrote:
> >>> On 14.10.2015 08:29, Jiang Liu wrote:
> >>>> Introduce common interface acpi_pci_root_create() and related data
> >>>> structures to create PCI root bus for ACPI PCI host bridges. It will
> >>>> be used to kill duplicated arch specific code for IA64 and x86. It may
> >>>> also help ARM64 in future.
> >>>>
> >>>> Reviewed-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> >>>> Tested-by: Tony Luck <tony.luck@intel.com>
> >>>> Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
> >>>> Signed-off-by: Liu Jiang <jiang.liu@linux.intel.com>
> >>>> ---
> >>>>   drivers/acpi/pci_root.c  |  204
> >>>> ++++++++++++++++++++++++++++++++++++++++++++++
> >>>>   include/linux/pci-acpi.h |   24 ++++++
> >>>>   2 files changed, 228 insertions(+)
> >>>>
> >>>
> >>> [...]
> >>>
> >>>> +
> >>>> +struct pci_bus *acpi_pci_root_create(struct acpi_pci_root *root,
> >>>> +                     struct acpi_pci_root_ops *ops,
> >>>> +                     struct acpi_pci_root_info *info,
> >>>> +                     void *sysdata)
> >>>> +{
> >>>> +    int ret, busnum = root->secondary.start;
> >>>> +    struct acpi_device *device = root->device;
> >>>> +    int node = acpi_get_node(device->handle);
> >>>> +    struct pci_bus *bus;
> >>>> +
> >>>> +    info->root = root;
> >>>> +    info->bridge = device;
> >>>> +    info->ops = ops;
> >>>> +    INIT_LIST_HEAD(&info->resources);
> >>>> +    snprintf(info->name, sizeof(info->name), "PCI Bus %04x:%02x",
> >>>> +         root->segment, busnum);
> >>>> +
> >>>> +    if (ops->init_info && ops->init_info(info))
> >>>> +        goto out_release_info;
> >>>> +    if (ops->prepare_resources)
> >>>> +        ret = ops->prepare_resources(info);
> >>>> +    else
> >>>> +        ret = acpi_pci_probe_root_resources(info);
> >>>> +    if (ret < 0)
> >>>> +        goto out_release_info;
> >>>> +
> >>>> +    pci_acpi_root_add_resources(info);
> >>>> +    pci_add_resource(&info->resources, &root->secondary);
> >>>> +    bus = pci_create_root_bus(NULL, busnum, ops->pci_ops,
> >>>> +                  sysdata, &info->resources);
> >>>
> >>> Thank a lot for this cleanup!!
> >>>
> >>> I recall you already considered passing segment (domain nr) to
> >>> pci_create_root_bus, right? Can you please remind me why we gave up
> >>> on this?
> >>>
> >>> I am asking because currently I can not find the way to retrieve domain
> >>> number from pci_bus_assign_domain_nr (for those platforms which choose
> >>> PCI_DOMAINS_GENERIC and want to use segment nr from MCFG table) which
> >>> is the
> >>> part of pci_create_root_bus.
> >>
> >> Not sure I fully understand your question, but
> >> pci_bus_assign_domain_nr() will
> >> put the assigned domain number in bus->domain_nr if you chose
> >> PCI_DOMAINS_GENERIC.
> >> Do you want to override that value with the segment nr from MCFG?
> >>
> > 
> > Let me give ACPI ARM64 example:
> > 
> > 1. We parse MCFG table and get segment nr assigned to root bridge
> > 2. Then PCI host bridge calls acpi_pci_root_create ->
> > pci_create_root_bus -> pci_bus_assign_domain_nr
> > 3. At this point we cannot get segment nr for ACPI
> > 
> > So I would like to assign MCFG segment nr to bus->domain_nr being in
> > pci_bus_assign_domain_nr giving we have scenario above.
> Please use sysdata for that, IA64 and x86 are making use of sysdata
> to store such information:
> struct pci_sysdata {
>         int             domain;         /* PCI domain */
>         int             node;           /* NUMA node */
> #ifdef CONFIG_ACPI
>         struct acpi_device *companion;  /* ACPI companion device */
> #endif
> #ifdef CONFIG_X86_64
>         void            *iommu;         /* IOMMU private data */
> #endif
> };
> 
> Hanjun once tried to introduce struct pci_sysdata for ARM64, but
> seems it has been rejected.

For good reason! There is a lot of duplication between different arches notion
of the pci_sysdata and they can be moved into pci_bus or pci_host_bridge
structures. The goal is to get rid of multiple pci_sysdata structures, not
to add more.

Best regards,
Liviu

> 
> > 
> > Thanks,
> > Tomasz
> > -- 
> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at  http://www.tux.org/lkml/
> 

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯