* [PATCH] Add pci=assign-busses quirk to Dell Latitude D505 @ 2014-08-29 9:10 David Henningsson 2014-08-29 14:22 ` Bjorn Helgaas 0 siblings, 1 reply; 17+ messages in thread From: David Henningsson @ 2014-08-29 9:10 UTC (permalink / raw) To: bhelgaas, linux-pci; +Cc: 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 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH] Add pci=assign-busses quirk to Dell Latitude D505 2014-08-29 9:10 [PATCH] Add pci=assign-busses quirk to Dell Latitude D505 David Henningsson @ 2014-08-29 14:22 ` Bjorn Helgaas 2014-08-29 14:44 ` David Henningsson 0 siblings, 1 reply; 17+ messages in thread From: Bjorn Helgaas @ 2014-08-29 14:22 UTC (permalink / raw) To: David Henningsson; +Cc: linux-pci 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 > ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] Add pci=assign-busses quirk to Dell Latitude D505 2014-08-29 14:22 ` Bjorn Helgaas @ 2014-08-29 14:44 ` David Henningsson 2014-08-29 17:36 ` Bjorn Helgaas 0 siblings, 1 reply; 17+ messages in thread From: David Henningsson @ 2014-08-29 14:44 UTC (permalink / raw) To: Bjorn Helgaas; +Cc: linux-pci 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] Add pci=assign-busses quirk to Dell Latitude D505 2014-08-29 14:44 ` David Henningsson @ 2014-08-29 17:36 ` Bjorn Helgaas 2014-09-10 18:08 ` Bjorn Helgaas 0 siblings, 1 reply; 17+ messages in thread From: Bjorn Helgaas @ 2014-08-29 17:36 UTC (permalink / raw) To: David Henningsson; +Cc: linux-pci 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] Add pci=assign-busses quirk to Dell Latitude D505 2014-08-29 17:36 ` Bjorn Helgaas @ 2014-09-10 18:08 ` Bjorn Helgaas 2014-09-10 19:50 ` Andreas Noever 2014-09-11 8:13 ` David Henningsson 0 siblings, 2 replies; 17+ messages in thread From: Bjorn Helgaas @ 2014-09-10 18:08 UTC (permalink / raw) To: David Henningsson; +Cc: linux-pci, Andreas Noever [+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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] Add pci=assign-busses quirk to Dell Latitude D505 2014-09-10 18:08 ` Bjorn Helgaas @ 2014-09-10 19:50 ` Andreas Noever 2014-09-10 20:37 ` Bjorn Helgaas 2014-09-11 8:13 ` David Henningsson 1 sibling, 1 reply; 17+ messages in thread From: Andreas Noever @ 2014-09-10 19:50 UTC (permalink / raw) To: Bjorn Helgaas; +Cc: David Henningsson, linux-pci 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] Add pci=assign-busses quirk to Dell Latitude D505 2014-09-10 19:50 ` Andreas Noever @ 2014-09-10 20:37 ` Bjorn Helgaas 0 siblings, 0 replies; 17+ messages in thread From: Bjorn Helgaas @ 2014-09-10 20:37 UTC (permalink / raw) To: Andreas Noever; +Cc: David Henningsson, linux-pci 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] Add pci=assign-busses quirk to Dell Latitude D505 2014-09-10 18:08 ` Bjorn Helgaas 2014-09-10 19:50 ` Andreas Noever @ 2014-09-11 8:13 ` David Henningsson 2014-09-13 3:18 ` Bjorn Helgaas 1 sibling, 1 reply; 17+ messages in thread From: David Henningsson @ 2014-09-11 8:13 UTC (permalink / raw) To: Bjorn Helgaas; +Cc: linux-pci, Andreas Noever 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] Add pci=assign-busses quirk to Dell Latitude D505 2014-09-11 8:13 ` David Henningsson @ 2014-09-13 3:18 ` Bjorn Helgaas 2014-09-14 22:10 ` Andreas Noever 2014-09-19 17:04 ` Bjorn Helgaas 0 siblings, 2 replies; 17+ messages in thread From: Bjorn Helgaas @ 2014-09-13 3:18 UTC (permalink / raw) To: David Henningsson; +Cc: linux-pci, Andreas Noever 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); ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH] Add pci=assign-busses quirk to Dell Latitude D505 2014-09-13 3:18 ` Bjorn Helgaas @ 2014-09-14 22:10 ` Andreas Noever 2014-09-15 9:53 ` Wei Yang 2014-09-19 17:04 ` Bjorn Helgaas 1 sibling, 1 reply; 17+ messages in thread From: Andreas Noever @ 2014-09-14 22:10 UTC (permalink / raw) To: Bjorn Helgaas Cc: Andreas Noever, David Henningsson, linux-pci, Thomas Richter [+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 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH] Add pci=assign-busses quirk to Dell Latitude D505 2014-09-14 22:10 ` Andreas Noever @ 2014-09-15 9:53 ` Wei Yang 2014-09-15 10:04 ` Andreas Noever 2014-09-15 19:03 ` Bjorn Helgaas 0 siblings, 2 replies; 17+ messages in thread From: Wei Yang @ 2014-09-15 9:53 UTC (permalink / raw) To: Andreas Noever Cc: Bjorn Helgaas, David Henningsson, linux-pci, Thomas Richter, gwshan 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] Add pci=assign-busses quirk to Dell Latitude D505 2014-09-15 9:53 ` Wei Yang @ 2014-09-15 10:04 ` Andreas Noever 2014-09-16 1:37 ` Wei Yang 2014-09-15 19:03 ` Bjorn Helgaas 1 sibling, 1 reply; 17+ messages in thread From: Andreas Noever @ 2014-09-15 10:04 UTC (permalink / raw) To: Wei Yang Cc: Bjorn Helgaas, David Henningsson, linux-pci, Thomas Richter, gwshan 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 > ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] Add pci=assign-busses quirk to Dell Latitude D505 2014-09-15 10:04 ` Andreas Noever @ 2014-09-16 1:37 ` Wei Yang 2014-09-16 3:00 ` Gavin Shan 0 siblings, 1 reply; 17+ messages in thread From: Wei Yang @ 2014-09-16 1:37 UTC (permalink / raw) To: Andreas Noever Cc: Wei Yang, Bjorn Helgaas, David Henningsson, linux-pci, Thomas Richter, gwshan 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] Add pci=assign-busses quirk to Dell Latitude D505 2014-09-16 1:37 ` Wei Yang @ 2014-09-16 3:00 ` Gavin Shan 0 siblings, 0 replies; 17+ messages in thread From: Gavin Shan @ 2014-09-16 3:00 UTC (permalink / raw) To: Wei Yang Cc: Andreas Noever, Bjorn Helgaas, David Henningsson, linux-pci, Thomas Richter, gwshan 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] Add pci=assign-busses quirk to Dell Latitude D505 2014-09-15 9:53 ` Wei Yang 2014-09-15 10:04 ` Andreas Noever @ 2014-09-15 19:03 ` Bjorn Helgaas 2014-09-16 8:49 ` Wei Yang 1 sibling, 1 reply; 17+ messages in thread From: Bjorn Helgaas @ 2014-09-15 19:03 UTC (permalink / raw) To: Wei Yang Cc: Andreas Noever, David Henningsson, linux-pci, Thomas Richter, gwshan 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] Add pci=assign-busses quirk to Dell Latitude D505 2014-09-15 19:03 ` Bjorn Helgaas @ 2014-09-16 8:49 ` Wei Yang 0 siblings, 0 replies; 17+ messages in thread From: Wei Yang @ 2014-09-16 8:49 UTC (permalink / raw) To: Bjorn Helgaas Cc: Wei Yang, Andreas Noever, David Henningsson, linux-pci, Thomas Richter, gwshan 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] Add pci=assign-busses quirk to Dell Latitude D505 2014-09-13 3:18 ` Bjorn Helgaas 2014-09-14 22:10 ` Andreas Noever @ 2014-09-19 17:04 ` Bjorn Helgaas 1 sibling, 0 replies; 17+ messages in thread From: Bjorn Helgaas @ 2014-09-19 17:04 UTC (permalink / raw) To: David Henningsson; +Cc: linux-pci, Andreas Noever 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); ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2014-09-19 17:04 UTC | newest] Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2014-08-29 9:10 [PATCH] Add pci=assign-busses quirk to Dell Latitude D505 David Henningsson 2014-08-29 14:22 ` Bjorn Helgaas 2014-08-29 14:44 ` David Henningsson 2014-08-29 17:36 ` Bjorn Helgaas 2014-09-10 18:08 ` Bjorn Helgaas 2014-09-10 19:50 ` Andreas Noever 2014-09-10 20:37 ` Bjorn Helgaas 2014-09-11 8:13 ` David Henningsson 2014-09-13 3:18 ` Bjorn Helgaas 2014-09-14 22:10 ` Andreas Noever 2014-09-15 9:53 ` Wei Yang 2014-09-15 10:04 ` Andreas Noever 2014-09-16 1:37 ` Wei Yang 2014-09-16 3:00 ` Gavin Shan 2014-09-15 19:03 ` Bjorn Helgaas 2014-09-16 8:49 ` Wei Yang 2014-09-19 17:04 ` Bjorn Helgaas
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).