Linux-ACPI Archive on lore.kernel.org
 help / color / Atom feed
From: Yinghai Lu <yinghai@kernel.org>
To: Bjorn Helgaas <bhelgaas@google.com>,
	"Rafael J. Wysocki" <rjw@sisk.pl>,
	Jiang Liu <jiang.liu@huawei.com>
Cc: Gavin Guo <tuffkidtt@gmail.com>,
	linux-pci@vger.kernel.org, linux-acpi@vger.kernel.org,
	linux-kernel@vger.kernel.org, Yinghai Lu <yinghai@kernel.org>
Subject: [PATCH -v3] PCI, ACPI, hotplug: Fix BUS_CHECK event handle on root bridge
Date: Thu, 25 Apr 2013 18:46:38 -0700
Message-ID: <1366940798-15268-1-git-send-email-yinghai@kernel.org> (raw)
In-Reply-To: <CAAh6nkn2WxQAcmtLM1bu66Ngn+BheJNfoAUMN=dO9eY2HXi7YQ@mail.gmail.com>

Gavin found that acpiphp does not handle hotplug anymore even after
now we have acpiphp built-in preparing for v3.10.

Bjorn analyzed bootlog, he found that 
    acpiphp: ACPI Hot Plug PCI Controller Driver version: 0.5
    ACPI: PCI Root Bridge [PCI0] (domain 0000 [bus 00-3e])
    \_SB_.PCI0:_OSC invalid UUID
    acpiphp: Slot [1] registered
    acpiphp: Slot [1-1] registered
   acpi root: \_SB_.PCI0 notify handler is installed
    _handle_hotplug_event_root: Bus check notify on \_SB_.PCI0
    _handle_hotplug_event_root: Bus check notify on \_SB_.PCI0
And that means:
   So we should be using acpiphp, which you do have built in statically,
   and it found a couple slots.  And we did get two bus check notifies on
   \_SB_.PCI0, so we *should* be re-enumerating PCI bus 0000:00.  But it
   looks like we're handling this as a host bridge hotplug event instead
   of a PCI device hotplug.  My guess is that
   handle_root_bridge_insertion() does nothing because the PCI0 ACPI
   device already exists, though I would expect to see the "acpi device
   exists..." in your dmesg log if this were the case.

Also according to Rafael and Bjorn, it is perfect fine that we
should enumerate bus by sending event to root bridge after hotadd
device to slots under that root bridge or children bridges.

It turns out that it is regression caused by
| commit 668192b678201d2fff27c6cc76bb003c1ec4a52a
| Author: Yinghai Lu <yinghai@kernel.org>
| Date:   Mon Jan 21 13:20:48 2013 -0800
|
|    PCI: acpiphp: Move host bridge hotplug to pci_root.c

We should check slots when BUS_CHECK is sent to root bridge acpi handle.

Restore the old behavoir by calling acpi_check_bridge and check_sub_bridge
in acpiphp.

Jiang Liu acctually have simimar patch but it forgets calling
acpi_check_bridge() for system that have slots on root bus directly.
That is still valid, as in QEMU we still have that slots on bus 0 at
least. But my first version patch wrongly check if root bridge exists
before check_sub_bridge for children bridges. 

-v2: Don't check bridge for acpi_walk_namespace with check_sub_bridges.
     also put back bridge reference.
-v3: More changelog and etc.

Reported-by: Gavin Guo <tuffkidtt@gmail.com>
Tested-by: Gavin Guo <tuffkidtt@gmail.com>
Signed-off-by: Yinghai Lu <yinghai@kernel.org>

---
 drivers/acpi/pci_root.c            |    2 ++
 drivers/pci/hotplug/acpiphp_glue.c |   14 ++++++++++++++
 include/linux/pci-acpi.h           |    2 ++
 3 files changed, 18 insertions(+)

Index: linux-2.6/drivers/acpi/pci_root.c
===================================================================
--- linux-2.6.orig/drivers/acpi/pci_root.c
+++ linux-2.6/drivers/acpi/pci_root.c
@@ -643,6 +643,8 @@ static void _handle_hotplug_event_root(s
 				 (char *)buffer.pointer);
 		if (!root)
 			handle_root_bridge_insertion(handle);
+		else
+			acpiphp_check_host_bridge(handle);
 
 		break;
 
Index: linux-2.6/drivers/pci/hotplug/acpiphp_glue.c
===================================================================
--- linux-2.6.orig/drivers/pci/hotplug/acpiphp_glue.c
+++ linux-2.6/drivers/pci/hotplug/acpiphp_glue.c
@@ -950,6 +950,20 @@ check_sub_bridges(acpi_handle handle, u3
 	return AE_OK ;
 }
 
+void acpiphp_check_host_bridge(acpi_handle handle)
+{
+	struct acpiphp_bridge *bridge;
+
+	bridge = acpiphp_handle_to_bridge(handle);
+	if (bridge) {
+		acpiphp_check_bridge(bridge);
+		put_bridge(bridge);
+	}
+
+	acpi_walk_namespace(ACPI_TYPE_DEVICE, handle,
+		ACPI_UINT32_MAX, check_sub_bridges, NULL, NULL, NULL);
+}
+
 static void _handle_hotplug_event_bridge(struct work_struct *work)
 {
 	struct acpiphp_bridge *bridge;
Index: linux-2.6/include/linux/pci-acpi.h
===================================================================
--- linux-2.6.orig/include/linux/pci-acpi.h
+++ linux-2.6/include/linux/pci-acpi.h
@@ -60,11 +60,13 @@ static inline void acpi_pci_slot_remove(
 void acpiphp_init(void);
 void acpiphp_enumerate_slots(struct pci_bus *bus, acpi_handle handle);
 void acpiphp_remove_slots(struct pci_bus *bus);
+void acpiphp_check_host_bridge(acpi_handle handle);
 #else
 static inline void acpiphp_init(void) { }
 static inline void acpiphp_enumerate_slots(struct pci_bus *bus,
 					   acpi_handle handle) { }
 static inline void acpiphp_remove_slots(struct pci_bus *bus) { }
+static inline void acpiphp_check_host_bridge(acpi_handle handle) { }
 #endif
 
 #else	/* CONFIG_ACPI */

       reply index

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <CAAh6nkn2WxQAcmtLM1bu66Ngn+BheJNfoAUMN=dO9eY2HXi7YQ@mail.gmail.com>
2013-04-26  1:46 ` Yinghai Lu [this message]
2013-04-26 10:58   ` Rafael J. Wysocki
2013-04-29 16:19   ` Jiang Liu
2013-05-10 17:09     ` 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=1366940798-15268-1-git-send-email-yinghai@kernel.org \
    --to=yinghai@kernel.org \
    --cc=bhelgaas@google.com \
    --cc=jiang.liu@huawei.com \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=rjw@sisk.pl \
    --cc=tuffkidtt@gmail.com \
    /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

Linux-ACPI Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-acpi/0 linux-acpi/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-acpi linux-acpi/ https://lore.kernel.org/linux-acpi \
		linux-acpi@vger.kernel.org
	public-inbox-index linux-acpi

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-acpi


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git