Re: [Alternative 2][PATCH] ACPI / PCI: Set root bridge ACPI handle in advance

["Rafael J. Wysocki" <rjw@sisk.pl>]

On Thursday, January 03, 2013 06:00:38 PM Bjorn Helgaas wrote:
> On Thu, Jan 03, 2013 at 11:56:55PM +0100, Rafael J. Wysocki wrote:
> 
> > OK, I now have sent no less than three working version of the patch that fixes
> > the current code which _is_ insane.  You haven't even responded to the last
> > one, but for the first two the reason why you didn't like them was something
> > similar to "it may conflict with some future changes I'm planning".  Well,
> > that might be used to reject prety much any change and I'm not considering it
> > as a good enough reason for blocking a fix.  Sorry about that.
> 
> I think your memory is faulty.  My response to the first
> (https://lkml.org/lkml/2012/12/20/407) was "Thanks for cleaning this up, I
> have an interface concern, here's an outline of a possible alternative."
> 
> My response to the second (https://lkml.org/lkml/2012/12/26/72) was "I like
> this much better, Acked-by: Bjorn Helgaas."  Then Yinghai noticed the issue
> with non-ACPI host bridges, and you abandoned that approach.

I thought it was helpelss, but I was clearly wrong.  I should have spent more
time on figuring out why it failed, so thank for taking the time to do that.

> I took a few days of vacation, then spent the better part of yesterday
> exploring the reasons why x86 and ia64 don't use the "parent" argument when
> several other arches do, and worked up a patch
> (https://lkml.org/lkml/2013/1/2/285).  It turned out to have a fatal flaw,
> but was done in good faith.

I know.

> It's true I haven't responded to the third one, posted about 12 hours ago.

Oh, that's a simple patch. ;-)

But you're right, I should be more patient.  Sorry about that.

> I still like the approach of the second patch.  What would you think
> of the following incremental change to it?

I'm fine with it.

> I did reproduce Yinghai's
> issue with non-ACPI host bridges, and this change resolves it for me.

If you don't mind, I'll fold the patch below into

https://patchwork.kernel.org/patch/1910181/

and post the complete patch.

Thanks,
Rafael


> diff -u b/arch/x86/pci/acpi.c b/arch/x86/pci/acpi.c
> --- b/arch/x86/pci/acpi.c
> +++ b/arch/x86/pci/acpi.c
> @@ -522,6 +522,7 @@
>  	sd = &info->sd;
>  	sd->domain = domain;
>  	sd->node = node;
> +	sd->acpi_handle = device->handle;
>  	/*
>  	 * Maybe the desired pci bus has been already scanned. In such case
>  	 * it is unnecessary to scan the pci bus with the given domain,busnum.
> @@ -596,9 +597,8 @@
>  int pcibios_root_bridge_prepare(struct pci_host_bridge *bridge)
>  {
>  	struct pci_sysdata *sd = bridge->bus->sysdata;
> -	struct pci_root_info *info = container_of(sd, struct pci_root_info, sd);
>  
> -	ACPI_HANDLE_SET(&bridge->dev, info->bridge->handle);
> +	ACPI_HANDLE_SET(&bridge->dev, sd->acpi_handle);
>  	return 0;
>  }
>  
> --- a/arch/x86/include/asm/pci.h
> +++ b/arch/x86/include/asm/pci.h
> @@ -14,6 +14,7 @@
>  struct pci_sysdata {
>  	int		domain;		/* PCI domain */
>  	int		node;		/* NUMA node */
> +	void		*acpi_handle;
>  #ifdef CONFIG_X86_64
>  	void		*iommu;		/* IOMMU private data */
>  #endif
> 
-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.