All of lore.kernel.org
 help / color / mirror / Atom feed
From: Bjorn Helgaas <bhelgaas@google.com>
To: David Henningsson <david.henningsson@canonical.com>
Cc: "linux-pci@vger.kernel.org" <linux-pci@vger.kernel.org>,
	Andreas Noever <andreas.noever@gmail.com>
Subject: Re: [PATCH] Add pci=assign-busses quirk to Dell Latitude D505
Date: Fri, 12 Sep 2014 21:18:18 -0600	[thread overview]
Message-ID: <20140913031818.GA25656@google.com> (raw)
In-Reply-To: <54115995.5020107@canonical.com>

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);

  reply	other threads:[~2014-09-13  3:18 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20140913031818.GA25656@google.com \
    --to=bhelgaas@google.com \
    --cc=andreas.noever@gmail.com \
    --cc=david.henningsson@canonical.com \
    --cc=linux-pci@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.