[PATCH] Add pci=assign-busses quirk to Dell Latitude D505

[PATCH] Add pci=assign-busses quirk to Dell Latitude D505

[David Henningsson]

Under a 3.16 based kernel (Ubuntu 3.16.0-031600-lowlatency),
CardBus is not working unless pci=assign-buses is added to the
kernel boot parameters.

Signed-off-by: David Henningsson <david.henningsson@canonical.com>
---
 arch/x86/pci/common.c | 8 ++++++++
 1 file changed, 8 insertions(+)

It was working OOTB on a 3.2 based kernel (Ubuntu 3.2.0-67-generic),
and I have not thoroughly bisected what made it stop working.
Still I'm suggesting to just a quirk to make it work regardless of
kernel - it's a ~8 year old laptop, so being a bit lazy about it is
probably okay, I hope. :-)

Extract from dmidecode:

Handle 0x0100, DMI type 1, 25 bytes
System Information
	Manufacturer: Dell Inc.
	Product Name: Latitude D505    

diff --git a/arch/x86/pci/common.c b/arch/x86/pci/common.c
index 059a76c..1849e39 100644
--- a/arch/x86/pci/common.c
+++ b/arch/x86/pci/common.c
@@ -262,6 +262,14 @@ static const struct dmi_system_id pciprobe_dmi_table[] = {
 			DMI_MATCH(DMI_PRODUCT_NAME, "SX20S"),
 		},
 	},
+	{
+		.callback = assign_all_busses,
+		.ident = "Dell Latitude D505 Laptop",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
+			DMI_MATCH(DMI_PRODUCT_NAME, "Latitude D505"),
+		},
+	},
 #endif		/* __i386__ */
 	{
 		.callback = set_bf_sort,
-- 
1.9.1

Re: [PATCH] Add pci=assign-busses quirk to Dell Latitude D505

[Bjorn Helgaas]

On Fri, Aug 29, 2014 at 3:10 AM, David Henningsson
<david.henningsson@canonical.com> wrote:
> Under a 3.16 based kernel (Ubuntu 3.16.0-031600-lowlatency),
> CardBus is not working unless pci=assign-buses is added to the
> kernel boot parameters.
>
> Signed-off-by: David Henningsson <david.henningsson@canonical.com>
> ---
>  arch/x86/pci/common.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> It was working OOTB on a 3.2 based kernel (Ubuntu 3.2.0-67-generic),
> and I have not thoroughly bisected what made it stop working.
> Still I'm suggesting to just a quirk to make it work regardless of
> kernel - it's a ~8 year old laptop, so being a bit lazy about it is
> probably okay, I hope. :-)

Please open a bugzilla and attach complete "lspci -vv" output and
dmesg logs from the working 3.2 kernel and a non-working current
kernel.

The quirk might be OK, but it would be better if we could make a
generic fix that would work on more machines than just this one.

>
> Extract from dmidecode:
>
> Handle 0x0100, DMI type 1, 25 bytes
> System Information
>         Manufacturer: Dell Inc.
>         Product Name: Latitude D505
>
> diff --git a/arch/x86/pci/common.c b/arch/x86/pci/common.c
> index 059a76c..1849e39 100644
> --- a/arch/x86/pci/common.c
> +++ b/arch/x86/pci/common.c
> @@ -262,6 +262,14 @@ static const struct dmi_system_id pciprobe_dmi_table[] = {
>                         DMI_MATCH(DMI_PRODUCT_NAME, "SX20S"),
>                 },
>         },
> +       {
> +               .callback = assign_all_busses,
> +               .ident = "Dell Latitude D505 Laptop",
> +               .matches = {
> +                       DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
> +                       DMI_MATCH(DMI_PRODUCT_NAME, "Latitude D505"),
> +               },
> +       },
>  #endif         /* __i386__ */
>         {
>                 .callback = set_bf_sort,
> --
> 1.9.1
>

Re: [PATCH] Add pci=assign-busses quirk to Dell Latitude D505

[David Henningsson]

On 2014-08-29 16:22, Bjorn Helgaas wrote:
> On Fri, Aug 29, 2014 at 3:10 AM, David Henningsson
> <david.henningsson@canonical.com> wrote:
>> Under a 3.16 based kernel (Ubuntu 3.16.0-031600-lowlatency),
>> CardBus is not working unless pci=assign-buses is added to the
>> kernel boot parameters.
>>
>> Signed-off-by: David Henningsson <david.henningsson@canonical.com>
>> ---
>>   arch/x86/pci/common.c | 8 ++++++++
>>   1 file changed, 8 insertions(+)
>>
>> It was working OOTB on a 3.2 based kernel (Ubuntu 3.2.0-67-generic),
>> and I have not thoroughly bisected what made it stop working.
>> Still I'm suggesting to just a quirk to make it work regardless of
>> kernel - it's a ~8 year old laptop, so being a bit lazy about it is
>> probably okay, I hope. :-)
>
> Please open a bugzilla and attach complete "lspci -vv" output and
> dmesg logs from the working 3.2 kernel and a non-working current
> kernel.
>
> The quirk might be OK, but it would be better if we could make a
> generic fix that would work on more machines than just this one.

All right, you can now find the requested information in this bug:

https://bugzilla.kernel.org/show_bug.cgi?id=83441

>
>>
>> Extract from dmidecode:
>>
>> Handle 0x0100, DMI type 1, 25 bytes
>> System Information
>>          Manufacturer: Dell Inc.
>>          Product Name: Latitude D505
>>
>> diff --git a/arch/x86/pci/common.c b/arch/x86/pci/common.c
>> index 059a76c..1849e39 100644
>> --- a/arch/x86/pci/common.c
>> +++ b/arch/x86/pci/common.c
>> @@ -262,6 +262,14 @@ static const struct dmi_system_id pciprobe_dmi_table[] = {
>>                          DMI_MATCH(DMI_PRODUCT_NAME, "SX20S"),
>>                  },
>>          },
>> +       {
>> +               .callback = assign_all_busses,
>> +               .ident = "Dell Latitude D505 Laptop",
>> +               .matches = {
>> +                       DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
>> +                       DMI_MATCH(DMI_PRODUCT_NAME, "Latitude D505"),
>> +               },
>> +       },
>>   #endif         /* __i386__ */
>>          {
>>                  .callback = set_bf_sort,
>> --
>> 1.9.1
>>
>

-- 
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic

Re: [PATCH] Add pci=assign-busses quirk to Dell Latitude D505

[Bjorn Helgaas]

On Fri, Aug 29, 2014 at 8:44 AM, David Henningsson
<david.henningsson@canonical.com> wrote:
>
>
> On 2014-08-29 16:22, Bjorn Helgaas wrote:
>>
>> On Fri, Aug 29, 2014 at 3:10 AM, David Henningsson
>> <david.henningsson@canonical.com> wrote:
>>>
>>> Under a 3.16 based kernel (Ubuntu 3.16.0-031600-lowlatency),
>>> CardBus is not working unless pci=assign-buses is added to the
>>> kernel boot parameters.
>>>
>>> Signed-off-by: David Henningsson <david.henningsson@canonical.com>
>>> ---
>>>   arch/x86/pci/common.c | 8 ++++++++
>>>   1 file changed, 8 insertions(+)
>>>
>>> It was working OOTB on a 3.2 based kernel (Ubuntu 3.2.0-67-generic),
>>> and I have not thoroughly bisected what made it stop working.
>>> Still I'm suggesting to just a quirk to make it work regardless of
>>> kernel - it's a ~8 year old laptop, so being a bit lazy about it is
>>> probably okay, I hope. :-)
>>
>>
>> Please open a bugzilla and attach complete "lspci -vv" output and
>> dmesg logs from the working 3.2 kernel and a non-working current
>> kernel.
>>
>> The quirk might be OK, but it would be better if we could make a
>> generic fix that would work on more machines than just this one.
>
>
> All right, you can now find the requested information in this bug:
>
> https://bugzilla.kernel.org/show_bug.cgi?id=83441

Thanks very much.

I'm going to resist adding a quirk as long as I can, because quirks
tend to paper over issues on individual systems without actually
fixing the underlying problem.  Unfortunately, I'm swamped at the
moment and won't be able to work on this for a while.  It would be
ideal if we could bisect this and pinpoint where the problem started.
Bisecting changes to drivers/pci would be sufficient.

>>> Extract from dmidecode:
>>>
>>> Handle 0x0100, DMI type 1, 25 bytes
>>> System Information
>>>          Manufacturer: Dell Inc.
>>>          Product Name: Latitude D505
>>>
>>> diff --git a/arch/x86/pci/common.c b/arch/x86/pci/common.c
>>> index 059a76c..1849e39 100644
>>> --- a/arch/x86/pci/common.c
>>> +++ b/arch/x86/pci/common.c
>>> @@ -262,6 +262,14 @@ static const struct dmi_system_id
>>> pciprobe_dmi_table[] = {
>>>                          DMI_MATCH(DMI_PRODUCT_NAME, "SX20S"),
>>>                  },
>>>          },
>>> +       {
>>> +               .callback = assign_all_busses,
>>> +               .ident = "Dell Latitude D505 Laptop",
>>> +               .matches = {
>>> +                       DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
>>> +                       DMI_MATCH(DMI_PRODUCT_NAME, "Latitude D505"),
>>> +               },
>>> +       },
>>>   #endif         /* __i386__ */
>>>          {
>>>                  .callback = set_bf_sort,
>>> --
>>> 1.9.1
>>>
>>
>
> --
> David Henningsson, Canonical Ltd.
> https://launchpad.net/~diwic

Re: [PATCH] Add pci=assign-busses quirk to Dell Latitude D505

[Bjorn Helgaas]

[+cc Andreas]

On Fri, Aug 29, 2014 at 11:36 AM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> On Fri, Aug 29, 2014 at 8:44 AM, David Henningsson
> <david.henningsson@canonical.com> wrote:
>>
>>
>> On 2014-08-29 16:22, Bjorn Helgaas wrote:
>>>
>>> On Fri, Aug 29, 2014 at 3:10 AM, David Henningsson
>>> <david.henningsson@canonical.com> wrote:
>>>>
>>>> Under a 3.16 based kernel (Ubuntu 3.16.0-031600-lowlatency),
>>>> CardBus is not working unless pci=assign-buses is added to the
>>>> kernel boot parameters.
>>>>
>>>> Signed-off-by: David Henningsson <david.henningsson@canonical.com>
>>>> ---
>>>>   arch/x86/pci/common.c | 8 ++++++++
>>>>   1 file changed, 8 insertions(+)
>>>>
>>>> It was working OOTB on a 3.2 based kernel (Ubuntu 3.2.0-67-generic),
>>>> and I have not thoroughly bisected what made it stop working.
>>>> Still I'm suggesting to just a quirk to make it work regardless of
>>>> kernel - it's a ~8 year old laptop, so being a bit lazy about it is
>>>> probably okay, I hope. :-)
>>>
>>>
>>> Please open a bugzilla and attach complete "lspci -vv" output and
>>> dmesg logs from the working 3.2 kernel and a non-working current
>>> kernel.
>>>
>>> The quirk might be OK, but it would be better if we could make a
>>> generic fix that would work on more machines than just this one.
>>
>>
>> All right, you can now find the requested information in this bug:
>>
>> https://bugzilla.kernel.org/show_bug.cgi?id=83441
>
> Thanks very much.
>
> I'm going to resist adding a quirk as long as I can, because quirks
> tend to paper over issues on individual systems without actually
> fixing the underlying problem.  Unfortunately, I'm swamped at the
> moment and won't be able to work on this for a while.  It would be
> ideal if we could bisect this and pinpoint where the problem started.
> Bisecting changes to drivers/pci would be sufficient.
>
>>>> Extract from dmidecode:
>>>>
>>>> Handle 0x0100, DMI type 1, 25 bytes
>>>> System Information
>>>>          Manufacturer: Dell Inc.
>>>>          Product Name: Latitude D505
>>>>
>>>> diff --git a/arch/x86/pci/common.c b/arch/x86/pci/common.c
>>>> index 059a76c..1849e39 100644
>>>> --- a/arch/x86/pci/common.c
>>>> +++ b/arch/x86/pci/common.c
>>>> @@ -262,6 +262,14 @@ static const struct dmi_system_id
>>>> pciprobe_dmi_table[] = {
>>>>                          DMI_MATCH(DMI_PRODUCT_NAME, "SX20S"),
>>>>                  },
>>>>          },
>>>> +       {
>>>> +               .callback = assign_all_busses,
>>>> +               .ident = "Dell Latitude D505 Laptop",
>>>> +               .matches = {
>>>> +                       DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
>>>> +                       DMI_MATCH(DMI_PRODUCT_NAME, "Latitude D505"),
>>>> +               },
>>>> +       },
>>>>   #endif         /* __i386__ */
>>>>          {
>>>>                  .callback = set_bf_sort,

v3.16 fails with:

  pci 0000:01:01.0: can't allocate child bus 01 from [bus 01]

which Andreas added with fc1b253141b3 ("PCI: Don't scan random busses
in pci_scan_bridge()").  But I don't understand that code well enough
to know whether this commit is actually the cause of the problem.

And I also haven't figured out how "pci=assign-busses" makes a difference.

David, would you mind turning on CONFIG_DYNAMIC_DEBUG=y in your v3.16
kernel, adding  'dyndbg="file probe.c +p"' to your boot arguments, and
collecting another dmesg log?

Bjorn

Re: [PATCH] Add pci=assign-busses quirk to Dell Latitude D505

[Andreas Noever]

On Wed, Sep 10, 2014 at 8:08 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> [+cc Andreas]
>
> On Fri, Aug 29, 2014 at 11:36 AM, Bjorn Helgaas <bhelgaas@google.com> wrote:
>> On Fri, Aug 29, 2014 at 8:44 AM, David Henningsson
>> <david.henningsson@canonical.com> wrote:
>>>
>>>
>>> On 2014-08-29 16:22, Bjorn Helgaas wrote:
>>>>
>>>> On Fri, Aug 29, 2014 at 3:10 AM, David Henningsson
>>>> <david.henningsson@canonical.com> wrote:
>>>>>
>>>>> Under a 3.16 based kernel (Ubuntu 3.16.0-031600-lowlatency),
>>>>> CardBus is not working unless pci=assign-buses is added to the
>>>>> kernel boot parameters.
>>>>>
>>>>> Signed-off-by: David Henningsson <david.henningsson@canonical.com>
>>>>> ---
>>>>>   arch/x86/pci/common.c | 8 ++++++++
>>>>>   1 file changed, 8 insertions(+)
>>>>>
>>>>> It was working OOTB on a 3.2 based kernel (Ubuntu 3.2.0-67-generic),
>>>>> and I have not thoroughly bisected what made it stop working.
>>>>> Still I'm suggesting to just a quirk to make it work regardless of
>>>>> kernel - it's a ~8 year old laptop, so being a bit lazy about it is
>>>>> probably okay, I hope. :-)
>>>>
>>>>
>>>> Please open a bugzilla and attach complete "lspci -vv" output and
>>>> dmesg logs from the working 3.2 kernel and a non-working current
>>>> kernel.
>>>>
>>>> The quirk might be OK, but it would be better if we could make a
>>>> generic fix that would work on more machines than just this one.
>>>
>>>
>>> All right, you can now find the requested information in this bug:
>>>
>>> https://bugzilla.kernel.org/show_bug.cgi?id=83441
>>
>> Thanks very much.
>>
>> I'm going to resist adding a quirk as long as I can, because quirks
>> tend to paper over issues on individual systems without actually
>> fixing the underlying problem.  Unfortunately, I'm swamped at the
>> moment and won't be able to work on this for a while.  It would be
>> ideal if we could bisect this and pinpoint where the problem started.
>> Bisecting changes to drivers/pci would be sufficient.
>>
>>>>> Extract from dmidecode:
>>>>>
>>>>> Handle 0x0100, DMI type 1, 25 bytes
>>>>> System Information
>>>>>          Manufacturer: Dell Inc.
>>>>>          Product Name: Latitude D505
>>>>>
>>>>> diff --git a/arch/x86/pci/common.c b/arch/x86/pci/common.c
>>>>> index 059a76c..1849e39 100644
>>>>> --- a/arch/x86/pci/common.c
>>>>> +++ b/arch/x86/pci/common.c
>>>>> @@ -262,6 +262,14 @@ static const struct dmi_system_id
>>>>> pciprobe_dmi_table[] = {
>>>>>                          DMI_MATCH(DMI_PRODUCT_NAME, "SX20S"),
>>>>>                  },
>>>>>          },
>>>>> +       {
>>>>> +               .callback = assign_all_busses,
>>>>> +               .ident = "Dell Latitude D505 Laptop",
>>>>> +               .matches = {
>>>>> +                       DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
>>>>> +                       DMI_MATCH(DMI_PRODUCT_NAME, "Latitude D505"),
>>>>> +               },
>>>>> +       },
>>>>>   #endif         /* __i386__ */
>>>>>          {
>>>>>                  .callback = set_bf_sort,
>
> v3.16 fails with:
>
>   pci 0000:01:01.0: can't allocate child bus 01 from [bus 01]
>
> which Andreas added with fc1b253141b3 ("PCI: Don't scan random busses
> in pci_scan_bridge()").  But I don't understand that code well enough
> to know whether this commit is actually the cause of the problem.
Looks like my commit is at fault. The parent bridge 00:1e has
secondary=subordinate=1 assigned by the BIOS. My commit then refuses
to assign a secondary bus to 01:01 because the parent's bus window has
been exhausted.

Before fc1b253141b3, we ignored the parent's bus window and simply
created a new bus #2. If bus #2 had already existed elsewhere then we
would have rescanned it, which was the (potential) problem patch was
trying to address. Much later yenta_socket fixes the parent bridge.
See http://lxr.free-electrons.com/source/drivers/pcmcia/yenta_socket.c#L1077

Could move the yenta quirk to pci_scan_bridge or trigger a rescan from
the yenta driver after the fixup is done?


> And I also haven't figured out how "pci=assign-busses" makes a difference.
I'd guess that with pci=assign-busses we set size the parent bridge correctly.


> David, would you mind turning on CONFIG_DYNAMIC_DEBUG=y in your v3.16
> kernel, adding  'dyndbg="file probe.c +p"' to your boot arguments, and
> collecting another dmesg log?
>
> Bjorn

Re: [PATCH] Add pci=assign-busses quirk to Dell Latitude D505

[Bjorn Helgaas]

On Wed, Sep 10, 2014 at 1:50 PM, Andreas Noever
<andreas.noever@gmail.com> wrote:
> On Wed, Sep 10, 2014 at 8:08 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
>> [+cc Andreas]
>>
>> On Fri, Aug 29, 2014 at 11:36 AM, Bjorn Helgaas <bhelgaas@google.com> wrote:
>>> On Fri, Aug 29, 2014 at 8:44 AM, David Henningsson
>>> <david.henningsson@canonical.com> wrote:
>>>>
>>>>
>>>> On 2014-08-29 16:22, Bjorn Helgaas wrote:
>>>>>
>>>>> On Fri, Aug 29, 2014 at 3:10 AM, David Henningsson
>>>>> <david.henningsson@canonical.com> wrote:
>>>>>>
>>>>>> Under a 3.16 based kernel (Ubuntu 3.16.0-031600-lowlatency),
>>>>>> CardBus is not working unless pci=assign-buses is added to the
>>>>>> kernel boot parameters.
>>>>>>
>>>>>> Signed-off-by: David Henningsson <david.henningsson@canonical.com>
>>>>>> ---
>>>>>>   arch/x86/pci/common.c | 8 ++++++++
>>>>>>   1 file changed, 8 insertions(+)
>>>>>>
>>>>>> It was working OOTB on a 3.2 based kernel (Ubuntu 3.2.0-67-generic),
>>>>>> and I have not thoroughly bisected what made it stop working.
>>>>>> Still I'm suggesting to just a quirk to make it work regardless of
>>>>>> kernel - it's a ~8 year old laptop, so being a bit lazy about it is
>>>>>> probably okay, I hope. :-)
>>>>>
>>>>>
>>>>> Please open a bugzilla and attach complete "lspci -vv" output and
>>>>> dmesg logs from the working 3.2 kernel and a non-working current
>>>>> kernel.
>>>>>
>>>>> The quirk might be OK, but it would be better if we could make a
>>>>> generic fix that would work on more machines than just this one.
>>>>
>>>>
>>>> All right, you can now find the requested information in this bug:
>>>>
>>>> https://bugzilla.kernel.org/show_bug.cgi?id=83441
>>>
>>> Thanks very much.
>>>
>>> I'm going to resist adding a quirk as long as I can, because quirks
>>> tend to paper over issues on individual systems without actually
>>> fixing the underlying problem.  Unfortunately, I'm swamped at the
>>> moment and won't be able to work on this for a while.  It would be
>>> ideal if we could bisect this and pinpoint where the problem started.
>>> Bisecting changes to drivers/pci would be sufficient.
>>>
>>>>>> Extract from dmidecode:
>>>>>>
>>>>>> Handle 0x0100, DMI type 1, 25 bytes
>>>>>> System Information
>>>>>>          Manufacturer: Dell Inc.
>>>>>>          Product Name: Latitude D505
>>>>>>
>>>>>> diff --git a/arch/x86/pci/common.c b/arch/x86/pci/common.c
>>>>>> index 059a76c..1849e39 100644
>>>>>> --- a/arch/x86/pci/common.c
>>>>>> +++ b/arch/x86/pci/common.c
>>>>>> @@ -262,6 +262,14 @@ static const struct dmi_system_id
>>>>>> pciprobe_dmi_table[] = {
>>>>>>                          DMI_MATCH(DMI_PRODUCT_NAME, "SX20S"),
>>>>>>                  },
>>>>>>          },
>>>>>> +       {
>>>>>> +               .callback = assign_all_busses,
>>>>>> +               .ident = "Dell Latitude D505 Laptop",
>>>>>> +               .matches = {
>>>>>> +                       DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
>>>>>> +                       DMI_MATCH(DMI_PRODUCT_NAME, "Latitude D505"),
>>>>>> +               },
>>>>>> +       },
>>>>>>   #endif         /* __i386__ */
>>>>>>          {
>>>>>>                  .callback = set_bf_sort,
>>
>> v3.16 fails with:
>>
>>   pci 0000:01:01.0: can't allocate child bus 01 from [bus 01]
>>
>> which Andreas added with fc1b253141b3 ("PCI: Don't scan random busses
>> in pci_scan_bridge()").  But I don't understand that code well enough
>> to know whether this commit is actually the cause of the problem.
> Looks like my commit is at fault. The parent bridge 00:1e has
> secondary=subordinate=1 assigned by the BIOS. My commit then refuses
> to assign a secondary bus to 01:01 because the parent's bus window has
> been exhausted.
>
> Before fc1b253141b3, we ignored the parent's bus window and simply
> created a new bus #2. If bus #2 had already existed elsewhere then we
> would have rescanned it, which was the (potential) problem patch was
> trying to address. Much later yenta_socket fixes the parent bridge.
> See http://lxr.free-electrons.com/source/drivers/pcmcia/yenta_socket.c#L1077
>
> Could move the yenta quirk to pci_scan_bridge or trigger a rescan from
> the yenta driver after the fixup is done?

I vote for somehow integrating yenta_fixup_parent_bridge() into
pci_scan_bridge().  The code in yenta_fixup_parent_bridge() looks
pretty generic; I don't see anything that seems yenta-specific.  So
it'd be nice to have the fixup in the generic code.

Bjorn

Re: [PATCH] Add pci=assign-busses quirk to Dell Latitude D505

[David Henningsson]

On 2014-09-10 20:08, Bjorn Helgaas wrote:
> [+cc Andreas]
> v3.16 fails with:
>
>    pci 0000:01:01.0: can't allocate child bus 01 from [bus 01]
>
> which Andreas added with fc1b253141b3 ("PCI: Don't scan random busses
> in pci_scan_bridge()").  But I don't understand that code well enough
> to know whether this commit is actually the cause of the problem.
>
> And I also haven't figured out how "pci=assign-busses" makes a difference.
>
> David, would you mind turning on CONFIG_DYNAMIC_DEBUG=y in your v3.16
> kernel, adding  'dyndbg="file probe.c +p"' to your boot arguments, and
> collecting another dmesg log?

Thanks for looking at this (and sorry for not having done a bisect yet).

Anyhow, a new dmesg log has now been posted at kernel bugzilla #83441. 
It has a few more lines starting with "pci_bus" but nothing that really 
stands out AFAICT.


-- 
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic

Re: [PATCH] Add pci=assign-busses quirk to Dell Latitude D505

[Bjorn Helgaas]

On Thu, Sep 11, 2014 at 10:13:09AM +0200, David Henningsson wrote:
> 
> 
> On 2014-09-10 20:08, Bjorn Helgaas wrote:
> >[+cc Andreas]
> >v3.16 fails with:
> >
> >   pci 0000:01:01.0: can't allocate child bus 01 from [bus 01]
> >
> >which Andreas added with fc1b253141b3 ("PCI: Don't scan random busses
> >in pci_scan_bridge()").  But I don't understand that code well enough
> >to know whether this commit is actually the cause of the problem.
> >
> >And I also haven't figured out how "pci=assign-busses" makes a difference.
> >
> >David, would you mind turning on CONFIG_DYNAMIC_DEBUG=y in your v3.16
> >kernel, adding  'dyndbg="file probe.c +p"' to your boot arguments, and
> >collecting another dmesg log?
> 
> Thanks for looking at this (and sorry for not having done a bisect yet).
> 
> Anyhow, a new dmesg log has now been posted at kernel bugzilla
> #83441. It has a few more lines starting with "pci_bus" but nothing
> that really stands out AFAICT.

We need to make progress on this regression before v3.17.  David, can you
test the following revert (on top of v3.17-rc2)?  If it works, I plan to
apply it for v3.17 (unless we have a better solution, of course).

Bjorn


commit 413db4234ebbb9750f01180c04965ad3a5e18986
Author: Bjorn Helgaas <bhelgaas@google.com>
Date:   Fri Sep 12 20:46:09 2014 -0600

    Revert "PCI: Don't scan random busses in pci_scan_bridge()"
    
    This reverts commit fc1b253141b3 ("PCI: Don't scan random busses in
    pci_scan_bridge()") because it breaks CardBus on some machines.
    
    David tested a Dell Latitude D505 that worked like this prior to
    fc1b253141b3:
    
        pci 0000:00:1e.0: PCI bridge to [bus 01]
        pci 0000:01:01.0: CardBus bridge to [bus 02-05]
    
    Note that the 01:01.0 CardBus bridge has a bus number aperture of
    [bus 02-05], but those buses are all outside the 00:1e.0 PCI bridge bus
    number aperture, so accesses to buses 02-05 never reach CardBus.  This is
    later patched up by yenta_fixup_parent_bridge(), which changes the
    subordinate bus number of the 00:1e.0 PCI bridge:
    
        pci_bus 0000:01: Raising subordinate bus# of parent bus (#01) from #01 to #05
    
    With fc1b253141b3, pci_scan_bridge() just fails immediately when it notices
    that we can't allocate a valid secondary bus number for the CardBus bridge,
    and CardBus doesn't work at all:
    
        pci 0000:01:01.0: can't allocate child bus 01 from [bus 01]
    
    I'd prefer to fix this by integrating the yenta_fixup_parent_bridge() logic
    into pci_scan_bridge() so we fix the bus number apertures up front.  But
    that's a bigger effort, and I want to fix the regression while we're
    working on it.
    
    Link: https://bugzilla.kernel.org/show_bug.cgi?id=83441
    Link: http://lkml.kernel.org/r/1409303414-5196-1-git-send-email-david.henningsson@canonical.com
    Reported-by: David Henningsson <david.henningsson@canonical.com>
    Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
    CC: stable@vger.kernel.org	# v3.15+

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index e3cf8a2e6292..7c8ca351beae 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -838,16 +838,12 @@ int pci_scan_bridge(struct pci_bus *bus, struct pci_dev *dev, int max, int pass)
 			goto out;
 		}
 
-		if (max >= bus->busn_res.end) {
-			dev_warn(&dev->dev, "can't allocate child bus %02x from %pR\n",
-				 max, &bus->busn_res);
-			goto out;
-		}
-
 		/* Clear errors */
 		pci_write_config_word(dev, PCI_STATUS, 0xffff);
 
-		/* The bus will already exist if we are rescanning */
+		/* Prevent assigning a bus number that already exists.
+		 * This can happen when a bridge is hot-plugged, so in
+		 * this case we only re-scan this bus. */
 		child = pci_find_bus(pci_domain_nr(bus), max+1);
 		if (!child) {
 			child = pci_add_new_bus(bus, dev, max+1);

Re: [PATCH] Add pci=assign-busses quirk to Dell Latitude D505

[Andreas Noever]

[+cc Thomas, your regression has probably the same cause]

This looks a like it is going to be a little more complicated than anticipated.

pci_scan_bus has two main branches. One for "broken" hardware and cardbus and
another one for bridges whose configuration looks sane.

David's working 3.2 Kernel does the following:
pci 0000:00:1e.0: PCI bridge to [bus 01-01] (subtractive decode)
pci 0000:01:01.0: bridge configuration invalid ([bus 00-00]), reconfiguring

The 00:1e bridge is deemed sane and the 01:01 (cardbus) bridge below it is
deemed broken. pci_scan_bridge than assigns bus [2-5] to 01:01 without touching
1e (so we just got 4 imaginary busses). We just print a warning:
pci_bus 0000:02: [bus 02-05] partially hidden behind transparent bridge 0000:01 [bus 01-01]

After returning to 00:1e (in the sane branch) we also do not update our
subordinate and just return. Later yenta_socket sees that this is nuts and
carefully increases the subordinate of 00:1e.

My patch series for 3.15 was trying to make sure that pci_scan_bus does not
overstep its resources. So now we now refuse to create bus 2 under [01-01]
(fc1b2531 PCI: Don't scan random busses in pci_scan_bridge()) and a little
later we would forbid the cardbus code in pci_scan_bridge from reserving 4 more
busses (1820ffdc PCI: Make sure bus number resources stay within their parents
bounds). At least these two would need to be reverted.

As an alternative the following patch tries grow the bus window, if necessary
by growing its parents bus window (recursively). This should make the yenta
fixup unnecessary. I have tested the patch with normal pci bridges + setpci,
since I don't have access to a cardbus system.

So if you have time please test this and/or try to revert the two mentioned
commits.

Thanks,
Andreas
---
 drivers/pci/probe.c | 45 ++++++++++++++++++++++++++++++++++++---------
 1 file changed, 36 insertions(+), 9 deletions(-)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index e3cf8a2..81dd668 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -740,6 +740,30 @@ struct pci_bus *pci_add_new_bus(struct pci_bus *parent, struct pci_dev *dev,
 }
 EXPORT_SYMBOL(pci_add_new_bus);
 
+int pci_grow_bus(struct pci_bus *bus, int bus_max)
+{
+	struct resource *res = &bus->busn_res;
+	struct resource old_res = *res;
+	int ret;
+	if (res->end >= bus_max)
+		return 0;
+	if (!bus->self || pci_is_root_bus(bus)) {
+		dev_printk(KERN_DEBUG, &bus->dev,
+			   "root busn_res %pR cannot grow\n", &res);
+		return -EBUSY;
+	}
+	ret = pci_grow_bus(bus->parent, bus_max);
+	if (ret)
+		return ret;
+	ret = adjust_resource(res, res->start, bus_max - res->start + 1);
+	dev_printk(KERN_DEBUG, &bus->dev,
+			"busn_res: %pR end %s grown to %02x\n",
+			&old_res, ret ? "cannot be" : "has", bus_max);
+
+	pci_write_config_byte(bus->self, PCI_SUBORDINATE_BUS, bus_max);
+	return ret;
+}
+
 /*
  * If it's a bridge, configure it and scan the bus behind it.
  * For CardBus bridges, we don't scan behind as the devices will
@@ -814,12 +838,11 @@ int pci_scan_bridge(struct pci_bus *bus, struct pci_dev *dev, int max, int pass)
 		}
 
 		cmax = pci_scan_child_bus(child);
-		if (cmax > subordinate)
-			dev_warn(&dev->dev, "bridge has subordinate %02x but max busn %02x\n",
-				 subordinate, cmax);
-		/* subordinate should equal child->busn_res.end */
-		if (subordinate > max)
-			max = subordinate;
+		if (cmax > child->busn_res.end)
+			dev_warn(&dev->dev, "bridge has bus resource %pR but max busn %02x\n",
+				 &child->busn_res, cmax);
+		if (child->busn_res.end > max)
+			max = child->busn_res.end;
 	} else {
 		/*
 		 * We need to assign a number to this bus which we always
@@ -840,8 +863,10 @@ int pci_scan_bridge(struct pci_bus *bus, struct pci_dev *dev, int max, int pass)
 
 		if (max >= bus->busn_res.end) {
 			dev_warn(&dev->dev, "can't allocate child bus %02x from %pR\n",
-				 max, &bus->busn_res);
-			goto out;
+				 max + 1, &bus->busn_res);
+			/* Try to resize bus */
+			if (pci_grow_bus(bus, max + 1))
+				goto out;
 		}
 
 		/* Clear errors */
@@ -908,6 +933,7 @@ int pci_scan_bridge(struct pci_bus *bus, struct pci_dev *dev, int max, int pass)
 					break;
 				}
 			}
+			dev_info(&dev->dev, "adding %02x bus numbers for cardbus\n", i);
 			max += i;
 		}
 		/*
@@ -916,7 +942,8 @@ int pci_scan_bridge(struct pci_bus *bus, struct pci_dev *dev, int max, int pass)
 		if (max > bus->busn_res.end) {
 			dev_warn(&dev->dev, "max busn %02x is outside %pR\n",
 				 max, &bus->busn_res);
-			max = bus->busn_res.end;
+			if (pci_grow_bus(bus, max))
+				max = bus->busn_res.end;
 		}
 		pci_bus_update_busn_res_end(child, max);
 		pci_write_config_byte(dev, PCI_SUBORDINATE_BUS, max);
-- 
2.1.0

Re: [PATCH] Add pci=assign-busses quirk to Dell Latitude D505

[Wei Yang]

On Mon, Sep 15, 2014 at 12:10:27AM +0200, Andreas Noever wrote:
>[+cc Thomas, your regression has probably the same cause]
>
>This looks a like it is going to be a little more complicated than anticipated.
>
>pci_scan_bus has two main branches. One for "broken" hardware and cardbus and
>another one for bridges whose configuration looks sane.
>
>David's working 3.2 Kernel does the following:
>pci 0000:00:1e.0: PCI bridge to [bus 01-01] (subtractive decode)
>pci 0000:01:01.0: bridge configuration invalid ([bus 00-00]), reconfiguring
>
>The 00:1e bridge is deemed sane and the 01:01 (cardbus) bridge below it is
>deemed broken. pci_scan_bridge than assigns bus [2-5] to 01:01 without touching
>1e (so we just got 4 imaginary busses). We just print a warning:
>pci_bus 0000:02: [bus 02-05] partially hidden behind transparent bridge 0000:01 [bus 01-01]
>
>After returning to 00:1e (in the sane branch) we also do not update our
>subordinate and just return. Later yenta_socket sees that this is nuts and
>carefully increases the subordinate of 00:1e.
>
>My patch series for 3.15 was trying to make sure that pci_scan_bus does not
>overstep its resources. So now we now refuse to create bus 2 under [01-01]
>(fc1b2531 PCI: Don't scan random busses in pci_scan_bridge()) and a little
>later we would forbid the cardbus code in pci_scan_bridge from reserving 4 more
>busses (1820ffdc PCI: Make sure bus number resources stay within their parents
>bounds). At least these two would need to be reverted.
>
>As an alternative the following patch tries grow the bus window, if necessary
>by growing its parents bus window (recursively). This should make the yenta
>fixup unnecessary. I have tested the patch with normal pci bridges + setpci,
>since I don't have access to a cardbus system.
>
>So if you have time please test this and/or try to revert the two mentioned
>commits.
>
>Thanks,
>Andreas
>---
> drivers/pci/probe.c | 45 ++++++++++++++++++++++++++++++++++++---------
> 1 file changed, 36 insertions(+), 9 deletions(-)
>
>diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
>index e3cf8a2..81dd668 100644
>--- a/drivers/pci/probe.c
>+++ b/drivers/pci/probe.c
>@@ -740,6 +740,30 @@ struct pci_bus *pci_add_new_bus(struct pci_bus *parent, struct pci_dev *dev,
> }
> EXPORT_SYMBOL(pci_add_new_bus);
>
>+int pci_grow_bus(struct pci_bus *bus, int bus_max)
>+{
>+	struct resource *res = &bus->busn_res;
>+	struct resource old_res = *res;
>+	int ret;
>+	if (res->end >= bus_max)
>+		return 0;
>+	if (!bus->self || pci_is_root_bus(bus)) {
>+		dev_printk(KERN_DEBUG, &bus->dev,
>+			   "root busn_res %pR cannot grow\n", &res);
>+		return -EBUSY;
>+	}
>+	ret = pci_grow_bus(bus->parent, bus_max);
>+	if (ret)
>+		return ret;
>+	ret = adjust_resource(res, res->start, bus_max - res->start + 1);
>+	dev_printk(KERN_DEBUG, &bus->dev,
>+			"busn_res: %pR end %s grown to %02x\n",
>+			&old_res, ret ? "cannot be" : "has", bus_max);
>+

If adjust_resource fails, I think we can't write the bus_max into the bridge
register.

>+	pci_write_config_byte(bus->self, PCI_SUBORDINATE_BUS, bus_max);
>+	return ret;
>+}
>+
> /*
>  * If it's a bridge, configure it and scan the bus behind it.
>  * For CardBus bridges, we don't scan behind as the devices will
>@@ -814,12 +838,11 @@ int pci_scan_bridge(struct pci_bus *bus, struct pci_dev *dev, int max, int pass)
> 		}
>
> 		cmax = pci_scan_child_bus(child);
>-		if (cmax > subordinate)
>-			dev_warn(&dev->dev, "bridge has subordinate %02x but max busn %02x\n",
>-				 subordinate, cmax);
>-		/* subordinate should equal child->busn_res.end */
>-		if (subordinate > max)
>-			max = subordinate;
>+		if (cmax > child->busn_res.end)
>+			dev_warn(&dev->dev, "bridge has bus resource %pR but max busn %02x\n",
>+				 &child->busn_res, cmax);
>+		if (child->busn_res.end > max)
>+			max = child->busn_res.end;

By a quick look, I don't see some place change the busn_res.end. And in the
pci_bus_insert_busn_res(), the child->busn_res.end is set to subordinate. So I
don't see the reason you change it.

Do I miss something?

> 	} else {
> 		/*
> 		 * We need to assign a number to this bus which we always
>@@ -840,8 +863,10 @@ int pci_scan_bridge(struct pci_bus *bus, struct pci_dev *dev, int max, int pass)
>
> 		if (max >= bus->busn_res.end) {
> 			dev_warn(&dev->dev, "can't allocate child bus %02x from %pR\n",
>-				 max, &bus->busn_res);
>-			goto out;
>+				 max + 1, &bus->busn_res);
>+			/* Try to resize bus */
>+			if (pci_grow_bus(bus, max + 1))
>+				goto out;

On some platforms, like powerpc, we have some limitations of the bus number a
bridge could have. Sometimes, we need the start bus number to be power 2
aligned.

Hmm... hope I don't make a mistake, add Gavin in CC list. Gavin, is my
understanding correct?

> 		}
>
> 		/* Clear errors */
>@@ -908,6 +933,7 @@ int pci_scan_bridge(struct pci_bus *bus, struct pci_dev *dev, int max, int pass)
> 					break;
> 				}
> 			}
>+			dev_info(&dev->dev, "adding %02x bus numbers for cardbus\n", i);
> 			max += i;
> 		}
> 		/*
>@@ -916,7 +942,8 @@ int pci_scan_bridge(struct pci_bus *bus, struct pci_dev *dev, int max, int pass)
> 		if (max > bus->busn_res.end) {
> 			dev_warn(&dev->dev, "max busn %02x is outside %pR\n",
> 				 max, &bus->busn_res);
>-			max = bus->busn_res.end;
>+			if (pci_grow_bus(bus, max))
>+				max = bus->busn_res.end;
> 		}
> 		pci_bus_update_busn_res_end(child, max);
> 		pci_write_config_byte(dev, PCI_SUBORDINATE_BUS, max);
>-- 
>2.1.0
>
>--
>To unsubscribe from this list: send the line "unsubscribe linux-pci" in
>the body of a message to majordomo@vger.kernel.org
>More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Richard Yang
Help you, Help me

Re: [PATCH] Add pci=assign-busses quirk to Dell Latitude D505

[Andreas Noever]

On Mon, Sep 15, 2014 at 11:53 AM, Wei Yang <weiyang@linux.vnet.ibm.com> wrote:
> On Mon, Sep 15, 2014 at 12:10:27AM +0200, Andreas Noever wrote:
>>[+cc Thomas, your regression has probably the same cause]
>>
>>This looks a like it is going to be a little more complicated than anticipated.
>>
>>pci_scan_bus has two main branches. One for "broken" hardware and cardbus and
>>another one for bridges whose configuration looks sane.
>>
>>David's working 3.2 Kernel does the following:
>>pci 0000:00:1e.0: PCI bridge to [bus 01-01] (subtractive decode)
>>pci 0000:01:01.0: bridge configuration invalid ([bus 00-00]), reconfiguring
>>
>>The 00:1e bridge is deemed sane and the 01:01 (cardbus) bridge below it is
>>deemed broken. pci_scan_bridge than assigns bus [2-5] to 01:01 without touching
>>1e (so we just got 4 imaginary busses). We just print a warning:
>>pci_bus 0000:02: [bus 02-05] partially hidden behind transparent bridge 0000:01 [bus 01-01]
>>
>>After returning to 00:1e (in the sane branch) we also do not update our
>>subordinate and just return. Later yenta_socket sees that this is nuts and
>>carefully increases the subordinate of 00:1e.
>>
>>My patch series for 3.15 was trying to make sure that pci_scan_bus does not
>>overstep its resources. So now we now refuse to create bus 2 under [01-01]
>>(fc1b2531 PCI: Don't scan random busses in pci_scan_bridge()) and a little
>>later we would forbid the cardbus code in pci_scan_bridge from reserving 4 more
>>busses (1820ffdc PCI: Make sure bus number resources stay within their parents
>>bounds). At least these two would need to be reverted.
>>
>>As an alternative the following patch tries grow the bus window, if necessary
>>by growing its parents bus window (recursively). This should make the yenta
>>fixup unnecessary. I have tested the patch with normal pci bridges + setpci,
>>since I don't have access to a cardbus system.
>>
>>So if you have time please test this and/or try to revert the two mentioned
>>commits.
>>
>>Thanks,
>>Andreas
>>---
>> drivers/pci/probe.c | 45 ++++++++++++++++++++++++++++++++++++---------
>> 1 file changed, 36 insertions(+), 9 deletions(-)
>>
>>diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
>>index e3cf8a2..81dd668 100644
>>--- a/drivers/pci/probe.c
>>+++ b/drivers/pci/probe.c
>>@@ -740,6 +740,30 @@ struct pci_bus *pci_add_new_bus(struct pci_bus *parent, struct pci_dev *dev,
>> }
>> EXPORT_SYMBOL(pci_add_new_bus);
>>
>>+int pci_grow_bus(struct pci_bus *bus, int bus_max)
>>+{
>>+      struct resource *res = &bus->busn_res;
>>+      struct resource old_res = *res;
>>+      int ret;
>>+      if (res->end >= bus_max)
>>+              return 0;
>>+      if (!bus->self || pci_is_root_bus(bus)) {
>>+              dev_printk(KERN_DEBUG, &bus->dev,
>>+                         "root busn_res %pR cannot grow\n", &res);
>>+              return -EBUSY;
>>+      }
>>+      ret = pci_grow_bus(bus->parent, bus_max);
>>+      if (ret)
>>+              return ret;
>>+      ret = adjust_resource(res, res->start, bus_max - res->start + 1);
>>+      dev_printk(KERN_DEBUG, &bus->dev,
>>+                      "busn_res: %pR end %s grown to %02x\n",
>>+                      &old_res, ret ? "cannot be" : "has", bus_max);
>>+
>
> If adjust_resource fails, I think we can't write the bus_max into the bridge
> register.
Right, there is an if statement missing. Thanks.

>>+      pci_write_config_byte(bus->self, PCI_SUBORDINATE_BUS, bus_max);
>>+      return ret;
>>+}
>>+
>> /*
>>  * If it's a bridge, configure it and scan the bus behind it.
>>  * For CardBus bridges, we don't scan behind as the devices will
>>@@ -814,12 +838,11 @@ int pci_scan_bridge(struct pci_bus *bus, struct pci_dev *dev, int max, int pass)
>>               }
>>
>>               cmax = pci_scan_child_bus(child);
>>-              if (cmax > subordinate)
>>-                      dev_warn(&dev->dev, "bridge has subordinate %02x but max busn %02x\n",
>>-                               subordinate, cmax);
>>-              /* subordinate should equal child->busn_res.end */
>>-              if (subordinate > max)
>>-                      max = subordinate;
>>+              if (cmax > child->busn_res.end)
>>+                      dev_warn(&dev->dev, "bridge has bus resource %pR but max busn %02x\n",
>>+                               &child->busn_res, cmax);
>>+              if (child->busn_res.end > max)
>>+                      max = child->busn_res.end;
>
> By a quick look, I don't see some place change the busn_res.end. And in the
> pci_bus_insert_busn_res(), the child->busn_res.end is set to subordinate. So I
> don't see the reason you change it.
adjust_resource in pci_grow_bus changes busn_res.end. The value in
'subordinate' is then stale at this point.

>
> Do I miss something?
>
>>       } else {
>>               /*
>>                * We need to assign a number to this bus which we always
>>@@ -840,8 +863,10 @@ int pci_scan_bridge(struct pci_bus *bus, struct pci_dev *dev, int max, int pass)
>>
>>               if (max >= bus->busn_res.end) {
>>                       dev_warn(&dev->dev, "can't allocate child bus %02x from %pR\n",
>>-                               max, &bus->busn_res);
>>-                      goto out;
>>+                               max + 1, &bus->busn_res);
>>+                      /* Try to resize bus */
>>+                      if (pci_grow_bus(bus, max + 1))
>>+                              goto out;
>
> On some platforms, like powerpc, we have some limitations of the bus number a
> bridge could have. Sometimes, we need the start bus number to be power 2
> aligned.
>
> Hmm... hope I don't make a mistake, add Gavin in CC list. Gavin, is my
> understanding correct?
Hm I have not seen any code that would enforce this. Is it possible to
do pci=assign-busses on PowerPC?

>>               }
>>
>>               /* Clear errors */
>>@@ -908,6 +933,7 @@ int pci_scan_bridge(struct pci_bus *bus, struct pci_dev *dev, int max, int pass)
>>                                       break;
>>                               }
>>                       }
>>+                      dev_info(&dev->dev, "adding %02x bus numbers for cardbus\n", i);
>>                       max += i;
>>               }
>>               /*
>>@@ -916,7 +942,8 @@ int pci_scan_bridge(struct pci_bus *bus, struct pci_dev *dev, int max, int pass)
>>               if (max > bus->busn_res.end) {
>>                       dev_warn(&dev->dev, "max busn %02x is outside %pR\n",
>>                                max, &bus->busn_res);
>>-                      max = bus->busn_res.end;
>>+                      if (pci_grow_bus(bus, max))
>>+                              max = bus->busn_res.end;
>>               }
>>               pci_bus_update_busn_res_end(child, max);
>>               pci_write_config_byte(dev, PCI_SUBORDINATE_BUS, max);
>>--
>>2.1.0
>>
>>--
>>To unsubscribe from this list: send the line "unsubscribe linux-pci" in
>>the body of a message to majordomo@vger.kernel.org
>>More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
> --
> Richard Yang
> Help you, Help me
>

Re: [PATCH] Add pci=assign-busses quirk to Dell Latitude D505

[Bjorn Helgaas]

On Mon, Sep 15, 2014 at 05:53:05PM +0800, Wei Yang wrote:
> On Mon, Sep 15, 2014 at 12:10:27AM +0200, Andreas Noever wrote:
> ...

> >@@ -840,8 +863,10 @@ int pci_scan_bridge(struct pci_bus *bus, struct pci_dev *dev, int max, int pass)
> >
> > 		if (max >= bus->busn_res.end) {
> > 			dev_warn(&dev->dev, "can't allocate child bus %02x from %pR\n",
> >-				 max, &bus->busn_res);
> >-			goto out;
> >+				 max + 1, &bus->busn_res);
> >+			/* Try to resize bus */
> >+			if (pci_grow_bus(bus, max + 1))
> >+				goto out;
> 
> On some platforms, like powerpc, we have some limitations of the bus number a
> bridge could have. Sometimes, we need the start bus number to be power 2
> aligned.

Huh.  I have to admit that I'm getting tired of all the powerpc-specific
PCI hacks.  It's hard enough to get this stuff working on hardware that
conforms to the spec, and scattering pcibios_*() hooks around makes the
code even harder to follow.

What would happen if powerpc used PCI_PROBE_ONLY?  Do you really depend on
the PCI core to configure anything for you, or does your firmware set
everything up the way it needs to be?

Changing bridge configuration seems like something we should avoid under
PCI_PROBE_ONLY (I haven't read Andreas' patch in detail, so I don't know if
that's how it works).  If PCI_PROBE_ONLY would work for powerpc, then we
wouldn't have an issue here.

Bjorn

Re: [PATCH] Add pci=assign-busses quirk to Dell Latitude D505

[Wei Yang]

On Mon, Sep 15, 2014 at 12:04:22PM +0200, Andreas Noever wrote:
>On Mon, Sep 15, 2014 at 11:53 AM, Wei Yang <weiyang@linux.vnet.ibm.com> wrote:
>> On Mon, Sep 15, 2014 at 12:10:27AM +0200, Andreas Noever wrote:
>>>[+cc Thomas, your regression has probably the same cause]
>>>
>>>This looks a like it is going to be a little more complicated than anticipated.
>>>
>>>pci_scan_bus has two main branches. One for "broken" hardware and cardbus and
>>>another one for bridges whose configuration looks sane.
>>>
>>>David's working 3.2 Kernel does the following:
>>>pci 0000:00:1e.0: PCI bridge to [bus 01-01] (subtractive decode)
>>>pci 0000:01:01.0: bridge configuration invalid ([bus 00-00]), reconfiguring
>>>
>>>The 00:1e bridge is deemed sane and the 01:01 (cardbus) bridge below it is
>>>deemed broken. pci_scan_bridge than assigns bus [2-5] to 01:01 without touching
>>>1e (so we just got 4 imaginary busses). We just print a warning:
>>>pci_bus 0000:02: [bus 02-05] partially hidden behind transparent bridge 0000:01 [bus 01-01]
>>>
>>>After returning to 00:1e (in the sane branch) we also do not update our
>>>subordinate and just return. Later yenta_socket sees that this is nuts and
>>>carefully increases the subordinate of 00:1e.
>>>
>>>My patch series for 3.15 was trying to make sure that pci_scan_bus does not
>>>overstep its resources. So now we now refuse to create bus 2 under [01-01]
>>>(fc1b2531 PCI: Don't scan random busses in pci_scan_bridge()) and a little
>>>later we would forbid the cardbus code in pci_scan_bridge from reserving 4 more
>>>busses (1820ffdc PCI: Make sure bus number resources stay within their parents
>>>bounds). At least these two would need to be reverted.
>>>
>>>As an alternative the following patch tries grow the bus window, if necessary
>>>by growing its parents bus window (recursively). This should make the yenta
>>>fixup unnecessary. I have tested the patch with normal pci bridges + setpci,
>>>since I don't have access to a cardbus system.
>>>
>>>So if you have time please test this and/or try to revert the two mentioned
>>>commits.
>>>
>>>Thanks,
>>>Andreas
>>>---
>>> drivers/pci/probe.c | 45 ++++++++++++++++++++++++++++++++++++---------
>>> 1 file changed, 36 insertions(+), 9 deletions(-)
>>>
>>>diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
>>>index e3cf8a2..81dd668 100644
>>>--- a/drivers/pci/probe.c
>>>+++ b/drivers/pci/probe.c
>>>@@ -740,6 +740,30 @@ struct pci_bus *pci_add_new_bus(struct pci_bus *parent, struct pci_dev *dev,
>>> }
>>> EXPORT_SYMBOL(pci_add_new_bus);
>>>
>>>+int pci_grow_bus(struct pci_bus *bus, int bus_max)
>>>+{
>>>+      struct resource *res = &bus->busn_res;
>>>+      struct resource old_res = *res;
>>>+      int ret;
>>>+      if (res->end >= bus_max)
>>>+              return 0;
>>>+      if (!bus->self || pci_is_root_bus(bus)) {
>>>+              dev_printk(KERN_DEBUG, &bus->dev,
>>>+                         "root busn_res %pR cannot grow\n", &res);
>>>+              return -EBUSY;
>>>+      }
>>>+      ret = pci_grow_bus(bus->parent, bus_max);
>>>+      if (ret)
>>>+              return ret;
>>>+      ret = adjust_resource(res, res->start, bus_max - res->start + 1);
>>>+      dev_printk(KERN_DEBUG, &bus->dev,
>>>+                      "busn_res: %pR end %s grown to %02x\n",
>>>+                      &old_res, ret ? "cannot be" : "has", bus_max);
>>>+
>>
>> If adjust_resource fails, I think we can't write the bus_max into the bridge
>> register.
>Right, there is an if statement missing. Thanks.
>
>>>+      pci_write_config_byte(bus->self, PCI_SUBORDINATE_BUS, bus_max);
>>>+      return ret;
>>>+}
>>>+
>>> /*
>>>  * If it's a bridge, configure it and scan the bus behind it.
>>>  * For CardBus bridges, we don't scan behind as the devices will
>>>@@ -814,12 +838,11 @@ int pci_scan_bridge(struct pci_bus *bus, struct pci_dev *dev, int max, int pass)
>>>               }
>>>
>>>               cmax = pci_scan_child_bus(child);
>>>-              if (cmax > subordinate)
>>>-                      dev_warn(&dev->dev, "bridge has subordinate %02x but max busn %02x\n",
>>>-                               subordinate, cmax);
>>>-              /* subordinate should equal child->busn_res.end */
>>>-              if (subordinate > max)
>>>-                      max = subordinate;
>>>+              if (cmax > child->busn_res.end)
>>>+                      dev_warn(&dev->dev, "bridge has bus resource %pR but max busn %02x\n",
>>>+                               &child->busn_res, cmax);
>>>+              if (child->busn_res.end > max)
>>>+                      max = child->busn_res.end;
>>
>> By a quick look, I don't see some place change the busn_res.end. And in the
>> pci_bus_insert_busn_res(), the child->busn_res.end is set to subordinate. So I
>> don't see the reason you change it.
>adjust_resource in pci_grow_bus changes busn_res.end. The value in
>'subordinate' is then stale at this point.
>

Yep, I see it. Thanks

>>
>> Do I miss something?
>>
>>>       } else {
>>>               /*
>>>                * We need to assign a number to this bus which we always
>>>@@ -840,8 +863,10 @@ int pci_scan_bridge(struct pci_bus *bus, struct pci_dev *dev, int max, int pass)
>>>
>>>               if (max >= bus->busn_res.end) {
>>>                       dev_warn(&dev->dev, "can't allocate child bus %02x from %pR\n",
>>>-                               max, &bus->busn_res);
>>>-                      goto out;
>>>+                               max + 1, &bus->busn_res);
>>>+                      /* Try to resize bus */
>>>+                      if (pci_grow_bus(bus, max + 1))
>>>+                              goto out;
>>
>> On some platforms, like powerpc, we have some limitations of the bus number a
>> bridge could have. Sometimes, we need the start bus number to be power 2
>> aligned.
>>
>> Hmm... hope I don't make a mistake, add Gavin in CC list. Gavin, is my
>> understanding correct?
>Hm I have not seen any code that would enforce this. Is it possible to
>do pci=assign-busses on PowerPC?
>

No code in kernel, while we assign the bus number in firmware.

Usually we don't re-assigned the bus numbers at kernel.

>>>               }
>>>
>>>               /* Clear errors */
>>>@@ -908,6 +933,7 @@ int pci_scan_bridge(struct pci_bus *bus, struct pci_dev *dev, int max, int pass)
>>>                                       break;
>>>                               }
>>>                       }
>>>+                      dev_info(&dev->dev, "adding %02x bus numbers for cardbus\n", i);
>>>                       max += i;
>>>               }
>>>               /*
>>>@@ -916,7 +942,8 @@ int pci_scan_bridge(struct pci_bus *bus, struct pci_dev *dev, int max, int pass)
>>>               if (max > bus->busn_res.end) {
>>>                       dev_warn(&dev->dev, "max busn %02x is outside %pR\n",
>>>                                max, &bus->busn_res);
>>>-                      max = bus->busn_res.end;
>>>+                      if (pci_grow_bus(bus, max))
>>>+                              max = bus->busn_res.end;
>>>               }
>>>               pci_bus_update_busn_res_end(child, max);
>>>               pci_write_config_byte(dev, PCI_SUBORDINATE_BUS, max);
>>>--
>>>2.1.0
>>>
>>>--
>>>To unsubscribe from this list: send the line "unsubscribe linux-pci" in
>>>the body of a message to majordomo@vger.kernel.org
>>>More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>> --
>> Richard Yang
>> Help you, Help me
>>
>--
>To unsubscribe from this list: send the line "unsubscribe linux-pci" in
>the body of a message to majordomo@vger.kernel.org
>More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Richard Yang
Help you, Help me

Re: [PATCH] Add pci=assign-busses quirk to Dell Latitude D505

[Gavin Shan]

On Tue, Sep 16, 2014 at 09:37:04AM +0800, Wei Yang wrote:
>On Mon, Sep 15, 2014 at 12:04:22PM +0200, Andreas Noever wrote:
>>On Mon, Sep 15, 2014 at 11:53 AM, Wei Yang <weiyang@linux.vnet.ibm.com> wrote:
>>> On Mon, Sep 15, 2014 at 12:10:27AM +0200, Andreas Noever wrote:
>>>>[+cc Thomas, your regression has probably the same cause]
>>>>
>>>>This looks a like it is going to be a little more complicated than anticipated.
>>>>
>>>>pci_scan_bus has two main branches. One for "broken" hardware and cardbus and
>>>>another one for bridges whose configuration looks sane.
>>>>
>>>>David's working 3.2 Kernel does the following:
>>>>pci 0000:00:1e.0: PCI bridge to [bus 01-01] (subtractive decode)
>>>>pci 0000:01:01.0: bridge configuration invalid ([bus 00-00]), reconfiguring
>>>>
>>>>The 00:1e bridge is deemed sane and the 01:01 (cardbus) bridge below it is
>>>>deemed broken. pci_scan_bridge than assigns bus [2-5] to 01:01 without touching
>>>>1e (so we just got 4 imaginary busses). We just print a warning:
>>>>pci_bus 0000:02: [bus 02-05] partially hidden behind transparent bridge 0000:01 [bus 01-01]
>>>>
>>>>After returning to 00:1e (in the sane branch) we also do not update our
>>>>subordinate and just return. Later yenta_socket sees that this is nuts and
>>>>carefully increases the subordinate of 00:1e.
>>>>
>>>>My patch series for 3.15 was trying to make sure that pci_scan_bus does not
>>>>overstep its resources. So now we now refuse to create bus 2 under [01-01]
>>>>(fc1b2531 PCI: Don't scan random busses in pci_scan_bridge()) and a little
>>>>later we would forbid the cardbus code in pci_scan_bridge from reserving 4 more
>>>>busses (1820ffdc PCI: Make sure bus number resources stay within their parents
>>>>bounds). At least these two would need to be reverted.
>>>>
>>>>As an alternative the following patch tries grow the bus window, if necessary
>>>>by growing its parents bus window (recursively). This should make the yenta
>>>>fixup unnecessary. I have tested the patch with normal pci bridges + setpci,
>>>>since I don't have access to a cardbus system.
>>>>
>>>>So if you have time please test this and/or try to revert the two mentioned
>>>>commits.
>>>>
>>>>Thanks,
>>>>Andreas
>>>>---
>>>> drivers/pci/probe.c | 45 ++++++++++++++++++++++++++++++++++++---------
>>>> 1 file changed, 36 insertions(+), 9 deletions(-)
>>>>
>>>>diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
>>>>index e3cf8a2..81dd668 100644
>>>>--- a/drivers/pci/probe.c
>>>>+++ b/drivers/pci/probe.c
>>>>@@ -740,6 +740,30 @@ struct pci_bus *pci_add_new_bus(struct pci_bus *parent, struct pci_dev *dev,
>>>> }
>>>> EXPORT_SYMBOL(pci_add_new_bus);
>>>>
>>>>+int pci_grow_bus(struct pci_bus *bus, int bus_max)
>>>>+{
>>>>+      struct resource *res = &bus->busn_res;
>>>>+      struct resource old_res = *res;
>>>>+      int ret;
>>>>+      if (res->end >= bus_max)
>>>>+              return 0;
>>>>+      if (!bus->self || pci_is_root_bus(bus)) {
>>>>+              dev_printk(KERN_DEBUG, &bus->dev,
>>>>+                         "root busn_res %pR cannot grow\n", &res);
>>>>+              return -EBUSY;
>>>>+      }
>>>>+      ret = pci_grow_bus(bus->parent, bus_max);
>>>>+      if (ret)
>>>>+              return ret;
>>>>+      ret = adjust_resource(res, res->start, bus_max - res->start + 1);
>>>>+      dev_printk(KERN_DEBUG, &bus->dev,
>>>>+                      "busn_res: %pR end %s grown to %02x\n",
>>>>+                      &old_res, ret ? "cannot be" : "has", bus_max);
>>>>+
>>>
>>> If adjust_resource fails, I think we can't write the bus_max into the bridge
>>> register.
>>Right, there is an if statement missing. Thanks.
>>
>>>>+      pci_write_config_byte(bus->self, PCI_SUBORDINATE_BUS, bus_max);
>>>>+      return ret;
>>>>+}
>>>>+
>>>> /*
>>>>  * If it's a bridge, configure it and scan the bus behind it.
>>>>  * For CardBus bridges, we don't scan behind as the devices will
>>>>@@ -814,12 +838,11 @@ int pci_scan_bridge(struct pci_bus *bus, struct pci_dev *dev, int max, int pass)
>>>>               }
>>>>
>>>>               cmax = pci_scan_child_bus(child);
>>>>-              if (cmax > subordinate)
>>>>-                      dev_warn(&dev->dev, "bridge has subordinate %02x but max busn %02x\n",
>>>>-                               subordinate, cmax);
>>>>-              /* subordinate should equal child->busn_res.end */
>>>>-              if (subordinate > max)
>>>>-                      max = subordinate;
>>>>+              if (cmax > child->busn_res.end)
>>>>+                      dev_warn(&dev->dev, "bridge has bus resource %pR but max busn %02x\n",
>>>>+                               &child->busn_res, cmax);
>>>>+              if (child->busn_res.end > max)
>>>>+                      max = child->busn_res.end;
>>>
>>> By a quick look, I don't see some place change the busn_res.end. And in the
>>> pci_bus_insert_busn_res(), the child->busn_res.end is set to subordinate. So I
>>> don't see the reason you change it.
>>adjust_resource in pci_grow_bus changes busn_res.end. The value in
>>'subordinate' is then stale at this point.
>>
>
>Yep, I see it. Thanks
>
>>>
>>> Do I miss something?
>>>
>>>>       } else {
>>>>               /*
>>>>                * We need to assign a number to this bus which we always
>>>>@@ -840,8 +863,10 @@ int pci_scan_bridge(struct pci_bus *bus, struct pci_dev *dev, int max, int pass)
>>>>
>>>>               if (max >= bus->busn_res.end) {
>>>>                       dev_warn(&dev->dev, "can't allocate child bus %02x from %pR\n",
>>>>-                               max, &bus->busn_res);
>>>>-                      goto out;
>>>>+                               max + 1, &bus->busn_res);
>>>>+                      /* Try to resize bus */
>>>>+                      if (pci_grow_bus(bus, max + 1))
>>>>+                              goto out;
>>>
>>> On some platforms, like powerpc, we have some limitations of the bus number a
>>> bridge could have. Sometimes, we need the start bus number to be power 2
>>> aligned.
>>>
>>> Hmm... hope I don't make a mistake, add Gavin in CC list. Gavin, is my
>>> understanding correct?
>>Hm I have not seen any code that would enforce this. Is it possible to
>>do pci=assign-busses on PowerPC?
>>
>
>No code in kernel, while we assign the bus number in firmware.
>
>Usually we don't re-assigned the bus numbers at kernel.
>

Yes, you're correct. But we don't have to have the limitation on P8: Each
PCI bridge has downstream 2^N spanning buses. That means PCI bridge on P8
can take any bus numbers assigned by firmware/kernel if I'm correct. Actually,
it's P7 box specific problem when we have a PCIe-to-PCI bridge and have to put
everything behind the bridge into one PE. If we don't have PCI domain behind
PCIe-to-PCI bridge on P7 box, we needn't consider the limitation.

Another reason why we don't expect kernel to reassign bus numbers is related
to hotplug and EEH. Currently, we don't reconfigure things for (RID mapping,
IO and MMIO mapping to PE#). So the PCI bus numbers assigned to one particular
PCI bridge is expected to be fixed. I guess we can improve it when having a
chance looking at current implementation so that things will be reconfigured
during hotplug.

>>>>               }
>>>>
>>>>               /* Clear errors */
>>>>@@ -908,6 +933,7 @@ int pci_scan_bridge(struct pci_bus *bus, struct pci_dev *dev, int max, int pass)
>>>>                                       break;
>>>>                               }
>>>>                       }
>>>>+                      dev_info(&dev->dev, "adding %02x bus numbers for cardbus\n", i);
>>>>                       max += i;
>>>>               }
>>>>               /*
>>>>@@ -916,7 +942,8 @@ int pci_scan_bridge(struct pci_bus *bus, struct pci_dev *dev, int max, int pass)
>>>>               if (max > bus->busn_res.end) {
>>>>                       dev_warn(&dev->dev, "max busn %02x is outside %pR\n",
>>>>                                max, &bus->busn_res);
>>>>-                      max = bus->busn_res.end;
>>>>+                      if (pci_grow_bus(bus, max))
>>>>+                              max = bus->busn_res.end;
>>>>               }
>>>>               pci_bus_update_busn_res_end(child, max);
>>>>               pci_write_config_byte(dev, PCI_SUBORDINATE_BUS, max);

Thanks,
Gavin

Re: [PATCH] Add pci=assign-busses quirk to Dell Latitude D505

[Wei Yang]

On Mon, Sep 15, 2014 at 01:03:19PM -0600, Bjorn Helgaas wrote:
>On Mon, Sep 15, 2014 at 05:53:05PM +0800, Wei Yang wrote:
>> On Mon, Sep 15, 2014 at 12:10:27AM +0200, Andreas Noever wrote:
>> ...
>
>> >@@ -840,8 +863,10 @@ int pci_scan_bridge(struct pci_bus *bus, struct pci_dev *dev, int max, int pass)
>> >
>> > 		if (max >= bus->busn_res.end) {
>> > 			dev_warn(&dev->dev, "can't allocate child bus %02x from %pR\n",
>> >-				 max, &bus->busn_res);
>> >-			goto out;
>> >+				 max + 1, &bus->busn_res);
>> >+			/* Try to resize bus */
>> >+			if (pci_grow_bus(bus, max + 1))
>> >+				goto out;
>> 
>> On some platforms, like powerpc, we have some limitations of the bus number a
>> bridge could have. Sometimes, we need the start bus number to be power 2
>> aligned.
>
>Huh.  I have to admit that I'm getting tired of all the powerpc-specific
>PCI hacks.  It's hard enough to get this stuff working on hardware that
>conforms to the spec, and scattering pcibios_*() hooks around makes the
>code even harder to follow.
>

Yep, those powerpc-specific things are not friendly :-(

I feel very sad it brings a lot difficulties to maintain it in linux mainline.
Sorry to bring so many trouble to you.

>What would happen if powerpc used PCI_PROBE_ONLY?  Do you really depend on
>the PCI core to configure anything for you, or does your firmware set
>everything up the way it needs to be?
>

Hmm... I had a try with PCI_PROBE_ONLY set, sounds can't bring up the machine.
Currently, we rely on the kernel to assign devices' resources. When
PCI_PROBE_ONLY is set, device resources will not be setup properly.

>Changing bridge configuration seems like something we should avoid under
>PCI_PROBE_ONLY (I haven't read Andreas' patch in detail, so I don't know if
>that's how it works).  If PCI_PROBE_ONLY would work for powerpc, then we
>wouldn't have an issue here.
>
>Bjorn

-- 
Richard Yang
Help you, Help me

Re: [PATCH] Add pci=assign-busses quirk to Dell Latitude D505

[Bjorn Helgaas]

On Fri, Sep 12, 2014 at 09:18:18PM -0600, Bjorn Helgaas wrote:
> On Thu, Sep 11, 2014 at 10:13:09AM +0200, David Henningsson wrote:
> > 
> > 
> > On 2014-09-10 20:08, Bjorn Helgaas wrote:
> > >[+cc Andreas]
> > >v3.16 fails with:
> > >
> > >   pci 0000:01:01.0: can't allocate child bus 01 from [bus 01]
> > >
> > >which Andreas added with fc1b253141b3 ("PCI: Don't scan random busses
> > >in pci_scan_bridge()").  But I don't understand that code well enough
> > >to know whether this commit is actually the cause of the problem.
> > >
> > >And I also haven't figured out how "pci=assign-busses" makes a difference.
> > >
> > >David, would you mind turning on CONFIG_DYNAMIC_DEBUG=y in your v3.16
> > >kernel, adding  'dyndbg="file probe.c +p"' to your boot arguments, and
> > >collecting another dmesg log?
> > 
> > Thanks for looking at this (and sorry for not having done a bisect yet).
> > 
> > Anyhow, a new dmesg log has now been posted at kernel bugzilla
> > #83441. It has a few more lines starting with "pci_bus" but nothing
> > that really stands out AFAICT.
> 
> We need to make progress on this regression before v3.17.  David, can you
> test the following revert (on top of v3.17-rc2)?  If it works, I plan to
> apply it for v3.17 (unless we have a better solution, of course).

David reported at https://bugzilla.kernel.org/show_bug.cgi?id=83441#c7 that
the revert below did fix the problem, so I'm going to apply it for v3.17.

I'd prefer to use something like Andreas' pci_grow_bus() patch, but I don't
think it's going to be ready in time for v3.17.  I hope we can polish that
for v3.18 or v3.19.

Bjorn

> commit 413db4234ebbb9750f01180c04965ad3a5e18986
> Author: Bjorn Helgaas <bhelgaas@google.com>
> Date:   Fri Sep 12 20:46:09 2014 -0600
> 
>     Revert "PCI: Don't scan random busses in pci_scan_bridge()"
>     
>     This reverts commit fc1b253141b3 ("PCI: Don't scan random busses in
>     pci_scan_bridge()") because it breaks CardBus on some machines.
>     
>     David tested a Dell Latitude D505 that worked like this prior to
>     fc1b253141b3:
>     
>         pci 0000:00:1e.0: PCI bridge to [bus 01]
>         pci 0000:01:01.0: CardBus bridge to [bus 02-05]
>     
>     Note that the 01:01.0 CardBus bridge has a bus number aperture of
>     [bus 02-05], but those buses are all outside the 00:1e.0 PCI bridge bus
>     number aperture, so accesses to buses 02-05 never reach CardBus.  This is
>     later patched up by yenta_fixup_parent_bridge(), which changes the
>     subordinate bus number of the 00:1e.0 PCI bridge:
>     
>         pci_bus 0000:01: Raising subordinate bus# of parent bus (#01) from #01 to #05
>     
>     With fc1b253141b3, pci_scan_bridge() just fails immediately when it notices
>     that we can't allocate a valid secondary bus number for the CardBus bridge,
>     and CardBus doesn't work at all:
>     
>         pci 0000:01:01.0: can't allocate child bus 01 from [bus 01]
>     
>     I'd prefer to fix this by integrating the yenta_fixup_parent_bridge() logic
>     into pci_scan_bridge() so we fix the bus number apertures up front.  But
>     that's a bigger effort, and I want to fix the regression while we're
>     working on it.
>     
>     Link: https://bugzilla.kernel.org/show_bug.cgi?id=83441
>     Link: http://lkml.kernel.org/r/1409303414-5196-1-git-send-email-david.henningsson@canonical.com
>     Reported-by: David Henningsson <david.henningsson@canonical.com>
>     Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
>     CC: stable@vger.kernel.org	# v3.15+
> 
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index e3cf8a2e6292..7c8ca351beae 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -838,16 +838,12 @@ int pci_scan_bridge(struct pci_bus *bus, struct pci_dev *dev, int max, int pass)
>  			goto out;
>  		}
>  
> -		if (max >= bus->busn_res.end) {
> -			dev_warn(&dev->dev, "can't allocate child bus %02x from %pR\n",
> -				 max, &bus->busn_res);
> -			goto out;
> -		}
> -
>  		/* Clear errors */
>  		pci_write_config_word(dev, PCI_STATUS, 0xffff);
>  
> -		/* The bus will already exist if we are rescanning */
> +		/* Prevent assigning a bus number that already exists.
> +		 * This can happen when a bridge is hot-plugged, so in
> +		 * this case we only re-scan this bus. */
>  		child = pci_find_bus(pci_domain_nr(bus), max+1);
>  		if (!child) {
>  			child = pci_add_new_bus(bus, dev, max+1);