[PATCH v2 00/37] PCI, x86: pci root bus hotplug support

[PATCH v2 00/37] PCI, x86: pci root bus hotplug support

[Yinghai Lu]

will add pci_stop_and_remove_bus() to support remove bus in
/sys/devices/pci.../pci_bus/...

To rescan root, need to
echo 1 > /sys/bus/pci/rescan

It supports acpi path root bus and legacy root bus.

This patcheset include some IOMM and dmar and pnpacpi fix with device refcount leaking.
	also include some bus remove/rescan cleanup.

The patches need to apply to pci/for-linus and pci/linux-next
and [PATCH 00/36] PCI: pci_host_bridge related cleanup and busn_alloc

could get from
        git://git.kernel.org/pub/scm/linux/kernel/git/yinghai/linux-yinghai.git for-pci-root-bus-hotplug

-v2: Address most of Bjorn's review, except still keeping .../pci_bus/.../remove
     Separate pci root hot add support from acpiphp to pci_root_hp
     add hot removal to pci_root_hp

Thanks

Yinghai

Ashok Raj (1):
  ACPI: Enable SCI_EMULATE to manually simulate physical hotplug testing.

Yinghai Lu (36):
  IOMMU: Update dmar units devices list during hotplug
  PNPACPI: Fix device ref leaking in acpi_pnp_match
  IOMMU: Fix tboot force iommu logic
  x86, PCI: Fix non acpi path pci_sysdata leaking with release_fn
  PCI: Separate out pci_assign_unassigned_bus_resources()
  PCI: Move back pci_rescan_bus()
  PCI: pci_bus_size_bridges() should not size own bridge
  PCI: Use __pci_bus_size_bridges() directly in pci_assign_unassigned_bus_resources()
  PCI, sysfs: Use device_type and attr_groups with pci dev
  PCI, sysfs: create rescan_bridge under /sys/.../pci/devices/... for pci bridges
  PCI: Add pci_bus_add_single_device()
  PCI: Make pci_rescan_bus_bridge_resize() use pci_scan_bridge instead
  PCI: Clean up rescan_bus_bridge_resize()
  PCI: Rescan bus or bridge using callback method too
  PCI, sysfs: Clean up rescan/remove with scheule_callback
  PCI: Move pci_stop_and_remove_behind_bridge() down
  PCI: Add __pci_remove_bus_devices()
  PCI: Use list_for_each_entry_safe instead of list_for_each_safe
  PCI: Add pci_stop_and_remove_bus()
  PCI: Add pci bus removal through /sys/.../pci_bus/.../remove
  PCI, ACPI: Make acpi_pci_root_remove remove pci root bus too
  PCI, acpiphp: remove hot-add support of pci host bridge
  PCI, ACPI: Add pci_root_hp hot add hotplug notification
  PCI, ACPI: Add pci_root_hp hot removal notification support.
  PCI, ACPI: Add alloc_acpi_hp_work()
  PCI, acpiphp: Use acpi_hp_work
  PCI, pci_root_hp: Use acpi_hp_work
  PCI, ACPI: Make kacpi_hotplug_wq static
  PCI, ACPI: Add acpi_pci_root_rescan()
  PCI: Add __pci_scan_root_bus() that can skip bus_add
  x86, PCI: add __pci_scan_root_bus_on_node() that can skip bus_add
  x86, PCI: add __pcibios_scan_specific_bus that can skip bus_add
  x86, PCI: Add pcibios_root_rescan()
  x86, PCI: Add arch version pci_root_rescan()
  PCI: Make /sys/bus/pci/rescan rescan root
  PCI, sysfs: Prepare to kill pci device rescan

 Documentation/ABI/testing/sysfs-bus-pci    |   18 ++
 Documentation/feature-removal-schedule.txt |    9 +
 arch/x86/include/asm/pci.h                 |    4 +
 arch/x86/pci/common.c                      |   35 +++-
 arch/x86/pci/legacy.c                      |   44 ++++-
 drivers/acpi/Kconfig                       |   10 +
 drivers/acpi/Makefile                      |    2 +
 drivers/acpi/bus.c                         |    2 +
 drivers/acpi/internal.h                    |    6 +
 drivers/acpi/osl.c                         |   24 ++-
 drivers/acpi/pci_root.c                    |   14 ++
 drivers/acpi/pci_root_hp.c                 |  287 ++++++++++++++++++++++++++++
 drivers/acpi/sci_emu.c                     |  141 ++++++++++++++
 drivers/iommu/intel-iommu.c                |  194 +++++++++++++++++--
 drivers/pci/bus.c                          |   39 ++++
 drivers/pci/hotplug/acpiphp.h              |    9 +-
 drivers/pci/hotplug/acpiphp_glue.c         |  148 +++------------
 drivers/pci/hotplug/cpci_hotplug_pci.c     |    2 +-
 drivers/pci/hotplug/shpchp_pci.c           |    2 +-
 drivers/pci/pci-sysfs.c                    |  136 ++++++++++++--
 drivers/pci/pci.h                          |    1 +
 drivers/pci/probe.c                        |   43 ++++-
 drivers/pci/remove.c                       |   84 ++++++---
 drivers/pci/setup-bus.c                    |   50 ++----
 drivers/pnp/pnpacpi/core.c                 |    7 +-
 include/acpi/acpiosxf.h                    |    9 +-
 include/linux/dmar.h                       |    4 +
 include/linux/pci-acpi.h                   |    7 +
 include/linux/pci.h                        |    7 +
 29 files changed, 1089 insertions(+), 249 deletions(-)
 create mode 100644 drivers/acpi/pci_root_hp.c
 create mode 100644 drivers/acpi/sci_emu.c

-- 
1.7.7

[PATCH v2 01/37] IOMMU: Update dmar units devices list during hotplug

[Yinghai Lu]

When do pci remove/rescan on system that have more iommus, got

[  894.089745] Set context mapping for c4:00.0
[  894.110890] mpt2sas3: Allocated physical memory: size(4293 kB)
[  894.112556] mpt2sas3: Current Controller Queue Depth(1883), Max Controller Queue Depth(2144)
[  894.127278] mpt2sas3: Scatter Gather Elements per IO(128)
[  894.361295] DRHD: handling fault status reg 2
[  894.364053] DMAR:[DMA Read] Request device [c4:00.0] fault addr fffbe000
[  894.364056] DMAR:[fault reason 02] Present bit in context entry is cl

It turns out when remove/rescan, pci dev will be freed and will get another
new dev. But drhd units still keep old one... so dmar_find_matched_drhd_unit
will return wrong drhd and iommu for the device that is not on first iommu.

So need to update devices in drhd_units during pci remove/rescan.

Could save domain/bus/device/function aside in the list and according that
info restore new dev to drhd_units later.
Then dmar_find_matched_drdh_unit and device_to_iommu could return right drhd
and iommu.

Add remove_dev_from_drhd/restore_dev_to_drhd functions to do the real work.
call them in device ADD_DEVICE and UNBOUND_DRIVER

Need to do the samething to atsr. (expose dmar_atsr_units and add
atsru->segment)

After patch, will have right iommu for the new dev and will not get DMAR
error anymore.

-v2: add pci_dev_put/pci_dev_get to make refcount consistent.
-v3: fix one left over CONFIG_DMAR
-v4: pass pci_dev *dev in save_dev_dmaru()/get_dev_dmaru() according to Bjorn.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
Cc: David Woodhouse <dwmw2@infradead.org>
Cc: Vinod Koul <vinod.koul@intel.com>
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: iommu@lists.linux-foundation.org
---
 drivers/iommu/intel-iommu.c |  188 ++++++++++++++++++++++++++++++++++++++++---
 include/linux/dmar.h        |    4 +
 2 files changed, 180 insertions(+), 12 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index c9c6053..49eb412 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -665,6 +665,158 @@ static struct intel_iommu *device_to_iommu(int segment, u8 bus, u8 devfn)
 	return NULL;
 }
 
+#ifdef CONFIG_HOTPLUG
+struct dev_dmaru {
+	struct list_head list;
+	void *dmaru;
+	int index;
+	int segment;
+	unsigned char bus;
+	unsigned int devfn;
+};
+
+static int save_dev_dmaru(struct pci_dev *dev, void *dmaru,
+			  int index, struct list_head *lh)
+{
+	struct dev_dmaru *m;
+
+	m = kzalloc(sizeof(*m), GFP_KERNEL);
+	if (!m)
+		return -ENOMEM;
+
+	m->segment = pci_domain_nr(dev->bus);
+	m->bus     = dev->bus->number;
+	m->devfn   = dev->devfn;
+	m->dmaru   = dmaru;
+	m->index   = index;
+
+	list_add(&m->list, lh);
+
+	return 0;
+}
+
+static void *get_dev_dmaru(struct pci_dev *dev, int *index,
+				struct list_head *lh)
+{
+	int segment = pci_domain_nr(dev->bus);
+	unsigned char bus = dev->bus->number;
+	unsigned int devfn = dev->devfn;
+	struct dev_dmaru *m;
+	void *dmaru = NULL;
+
+	list_for_each_entry(m, lh, list) {
+		if (m->segment == segment &&
+		    m->bus == bus && m->devfn == devfn) {
+			*index = m->index;
+			dmaru  = m->dmaru;
+			list_del(&m->list);
+			kfree(m);
+			break;
+		}
+	}
+
+	return dmaru;
+}
+
+static LIST_HEAD(saved_dev_drhd_list);
+
+static void remove_dev_from_drhd(struct pci_dev *dev)
+{
+	struct dmar_drhd_unit *drhd = NULL;
+	int segment = pci_domain_nr(dev->bus);
+	int i;
+
+	for_each_drhd_unit(drhd) {
+		if (drhd->ignored)
+			continue;
+		if (segment != drhd->segment)
+			continue;
+
+		for (i = 0; i < drhd->devices_cnt; i++) {
+			if (drhd->devices[i] == dev) {
+				/* save it at first if it is in drhd */
+				save_dev_dmaru(dev, drhd, i,
+						&saved_dev_drhd_list);
+				/* always remove it */
+				pci_dev_put(dev);
+				drhd->devices[i] = NULL;
+				return;
+			}
+		}
+	}
+}
+
+static void restore_dev_to_drhd(struct pci_dev *dev)
+{
+	struct dmar_drhd_unit *drhd = NULL;
+	int i;
+
+	/* find the stored drhd */
+	drhd = get_dev_dmaru(dev, &i, &saved_dev_drhd_list);
+	/* restore that into drhd */
+	if (drhd)
+		drhd->devices[i] = pci_dev_get(dev);
+}
+#else
+static void remove_dev_from_drhd(struct pci_dev *dev)
+{
+}
+
+static void restore_dev_to_drhd(struct pci_dev *dev)
+{
+}
+#endif
+
+static LIST_HEAD(dmar_atsr_units);
+
+#if defined(CONFIG_INTEL_IOMMU) && defined(CONFIG_HOTPLUG)
+static LIST_HEAD(saved_dev_atsr_list);
+
+static void remove_dev_from_atsr(struct pci_dev *dev)
+{
+	struct dmar_atsr_unit *atsr = NULL;
+	int segment = pci_domain_nr(dev->bus);
+	int i;
+
+	for_each_atsr_unit(atsr) {
+		if (segment != atsr->segment)
+			continue;
+
+		for (i = 0; i < atsr->devices_cnt; i++) {
+			if (atsr->devices[i] == dev) {
+				/* save it at first if it is in drhd */
+				save_dev_dmaru(dev, atsr, i,
+						&saved_dev_atsr_list);
+				/* always remove it */
+				pci_dev_put(dev);
+				atsr->devices[i] = NULL;
+				return;
+			}
+		}
+	}
+}
+
+static void restore_dev_to_atsr(struct pci_dev *dev)
+{
+	struct dmar_atsr_unit *atsr = NULL;
+	int i;
+
+	/* find the stored atsr */
+	atsr = get_dev_dmaru(dev, &i, &saved_dev_atsr_list);
+	/* restore that into atsr */
+	if (atsr)
+		atsr->devices[i] = pci_dev_get(dev);
+}
+#else
+static void remove_dev_from_atsr(struct pci_dev *dev)
+{
+}
+
+static void restore_dev_to_atsr(struct pci_dev *dev)
+{
+}
+#endif
+
 static void domain_flush_cache(struct dmar_domain *domain,
 			       void *addr, int size)
 {
@@ -3460,8 +3612,6 @@ rmrr_parse_dev(struct dmar_rmrr_unit *rmrru)
 	return ret;
 }
 
-static LIST_HEAD(dmar_atsr_units);
-
 int __init dmar_parse_one_atsr(struct acpi_dmar_header *hdr)
 {
 	struct acpi_dmar_atsr *atsr;
@@ -3474,6 +3624,7 @@ int __init dmar_parse_one_atsr(struct acpi_dmar_header *hdr)
 
 	atsru->hdr = hdr;
 	atsru->include_all = atsr->flags & 0x1;
+	atsru->segment = atsr->segment;
 
 	list_add(&atsru->list, &dmar_atsr_units);
 
@@ -3505,16 +3656,13 @@ int dmar_find_matched_atsr_unit(struct pci_dev *dev)
 {
 	int i;
 	struct pci_bus *bus;
-	struct acpi_dmar_atsr *atsr;
 	struct dmar_atsr_unit *atsru;
 
 	dev = pci_physfn(dev);
 
-	list_for_each_entry(atsru, &dmar_atsr_units, list) {
-		atsr = container_of(atsru->hdr, struct acpi_dmar_atsr, header);
-		if (atsr->segment == pci_domain_nr(dev->bus))
+	list_for_each_entry(atsru, &dmar_atsr_units, list)
+		if (atsru->segment == pci_domain_nr(dev->bus))
 			goto found;
-	}
 
 	return 0;
 
@@ -3574,20 +3722,36 @@ static int device_notifier(struct notifier_block *nb,
 	struct pci_dev *pdev = to_pci_dev(dev);
 	struct dmar_domain *domain;
 
-	if (iommu_no_mapping(dev))
+	if (unlikely(dev->bus != &pci_bus_type))
 		return 0;
 
-	domain = find_domain(pdev);
-	if (!domain)
-		return 0;
+	switch (action) {
+	case BUS_NOTIFY_UNBOUND_DRIVER:
+		if (iommu_no_mapping(dev))
+			goto out;
+
+		if (iommu_pass_through)
+			goto out;
+
+		domain = find_domain(pdev);
+		if (!domain)
+			goto out;
 
-	if (action == BUS_NOTIFY_UNBOUND_DRIVER && !iommu_pass_through) {
 		domain_remove_one_dev_info(domain, pdev);
 
 		if (!(domain->flags & DOMAIN_FLAG_VIRTUAL_MACHINE) &&
 		    !(domain->flags & DOMAIN_FLAG_STATIC_IDENTITY) &&
 		    list_empty(&domain->devices))
 			domain_exit(domain);
+out:
+		remove_dev_from_drhd(pdev);
+		remove_dev_from_atsr(pdev);
+
+		break;
+	case BUS_NOTIFY_ADD_DEVICE:
+		restore_dev_to_drhd(pdev);
+		restore_dev_to_atsr(pdev);
+		break;
 	}
 
 	return 0;
diff --git a/include/linux/dmar.h b/include/linux/dmar.h
index 731a609..57e1375 100644
--- a/include/linux/dmar.h
+++ b/include/linux/dmar.h
@@ -236,9 +236,13 @@ struct dmar_atsr_unit {
 	struct acpi_dmar_header *hdr;	/* ACPI header */
 	struct pci_dev **devices;	/* target devices */
 	int devices_cnt;		/* target device count */
+	u16 segment;			/* PCI domain */
 	u8 include_all:1;		/* include all ports */
 };
 
+#define for_each_atsr_unit(atsr) \
+	list_for_each_entry(atsr, &dmar_atsr_units, list)
+
 int dmar_parse_rmrr_atsr_dev(void);
 extern int dmar_parse_one_rmrr(struct acpi_dmar_header *header);
 extern int dmar_parse_one_atsr(struct acpi_dmar_header *header);
-- 
1.7.7

[PATCH v2 02/37] PNPACPI: Fix device ref leaking in acpi_pnp_match

[Yinghai Lu]

During testing pci root bus removal, found some root bus bridge is not freed.

If booting with pnpacpi=off, those hostbridge could be freed without problem.

It turns out that some devices reference are not released during acpi_pnp_match.

That match should not hold one device ref during every calling.

Add put_device calling before returning.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
Cc: stable@kernel.org
Cc: Len Brown <lenb@kernel.org>
Cc: Adam Belay <abelay@mit.edu>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: linux-acpi@vger.kernel.org
---
 drivers/pnp/pnpacpi/core.c |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/drivers/pnp/pnpacpi/core.c b/drivers/pnp/pnpacpi/core.c
index b00c176..d21e8f5 100644
--- a/drivers/pnp/pnpacpi/core.c
+++ b/drivers/pnp/pnpacpi/core.c
@@ -321,9 +321,14 @@ static int __init acpi_pnp_match(struct device *dev, void *_pnp)
 {
 	struct acpi_device *acpi = to_acpi_device(dev);
 	struct pnp_dev *pnp = _pnp;
+	struct device *physical_device;
+
+	physical_device = acpi_get_physical_device(acpi->handle);
+	if (physical_device)
+		put_device(physical_device);
 
 	/* true means it matched */
-	return !acpi_get_physical_device(acpi->handle)
+	return !physical_device
 	    && compare_pnp_id(pnp->id, acpi_device_hid(acpi));
 }
 
-- 
1.7.7

Re: [PATCH v2 02/37] PNPACPI: Fix device ref leaking in acpi_pnp_match

[Bjorn Helgaas]

On Sat, Mar 10, 2012 at 12:00 AM, Yinghai Lu <yinghai@kernel.org> wrote:
> During testing pci root bus removal, found some root bus bridge is not freed.
>
> If booting with pnpacpi=off, those hostbridge could be freed without problem.
>
> It turns out that some devices reference are not released during acpi_pnp_match.
>
> That match should not hold one device ref during every calling.
>
> Add put_device calling before returning.

Changelog doesn't fit in 80 columns when using "git log".  I mentioned
this before; please try it yourself.

> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> Cc: stable@kernel.org
> Cc: Len Brown <lenb@kernel.org>
> Cc: Adam Belay <abelay@mit.edu>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: linux-acpi@vger.kernel.org
> ---
>  drivers/pnp/pnpacpi/core.c |    7 ++++++-
>  1 files changed, 6 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/pnp/pnpacpi/core.c b/drivers/pnp/pnpacpi/core.c
> index b00c176..d21e8f5 100644
> --- a/drivers/pnp/pnpacpi/core.c
> +++ b/drivers/pnp/pnpacpi/core.c
> @@ -321,9 +321,14 @@ static int __init acpi_pnp_match(struct device *dev, void *_pnp)
>  {
>        struct acpi_device *acpi = to_acpi_device(dev);
>        struct pnp_dev *pnp = _pnp;
> +       struct device *physical_device;
> +
> +       physical_device = acpi_get_physical_device(acpi->handle);
> +       if (physical_device)
> +               put_device(physical_device);
>
>        /* true means it matched */
> -       return !acpi_get_physical_device(acpi->handle)
> +       return !physical_device
>            && compare_pnp_id(pnp->id, acpi_device_hid(acpi));
>  }
>
> --
> 1.7.7
>

[PATCH v2 03/37] IOMMU: Fix tboot force iommu logic

[Yinghai Lu]

Should check dmar_disabled just after tboot_force_iommu.

Otherwise when tboot is not used, and intel_iommu=off, and nointrmap
still get dmar_table_init() called. that will cause some get_device
calling, and it will have some device refcount leaking.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
Cc: David Woodhouse <dwmw2@infradead.org>
Cc: iommu@lists.linux-foundation.org
---
 drivers/iommu/intel-iommu.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 49eb412..7c71a3a 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -3768,6 +3768,9 @@ int __init intel_iommu_init(void)
 	/* VT-d is required for a TXT/tboot launch, so enforce that */
 	force_on = tboot_force_iommu();
 
+	if (no_iommu || dmar_disabled)
+		return -ENODEV;
+
 	if (dmar_table_init()) {
 		if (force_on)
 			panic("tboot: Failed to initialize DMAR table\n");
@@ -3780,9 +3783,6 @@ int __init intel_iommu_init(void)
 		return 	-ENODEV;
 	}
 
-	if (no_iommu || dmar_disabled)
-		return -ENODEV;
-
 	if (iommu_init_mempool()) {
 		if (force_on)
 			panic("tboot: Failed to initialize iommu memory\n");
-- 
1.7.7

[PATCH v2 04/37] x86, PCI: Fix non acpi path pci_sysdata leaking with release_fn

[Yinghai Lu]

Non acpi path, or root is not probed from acpi, during root bus removal
will get warning about leaking from pci_scan_bus_on_node.

Fix it with setting pci_host_bridge release function.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
 arch/x86/pci/common.c |   17 +++++++++++------
 1 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/arch/x86/pci/common.c b/arch/x86/pci/common.c
index 8e04ec5..35931f6 100644
--- a/arch/x86/pci/common.c
+++ b/arch/x86/pci/common.c
@@ -618,17 +618,19 @@ int pci_ext_cfg_avail(struct pci_dev *dev)
 		return 0;
 }
 
+static void release_pci_sysdata(struct pci_host_bridge *bridge)
+{
+	struct pci_sysdata *sd = bridge->release_data;
+
+	kfree(sd);
+}
+
 struct pci_bus * __devinit pci_scan_bus_on_node(int busno, struct pci_ops *ops, int node)
 {
 	LIST_HEAD(resources);
 	struct pci_bus *bus = NULL;
 	struct pci_sysdata *sd;
 
-	/*
-	 * Allocate per-root-bus (not per bus) arch-specific data.
-	 * TODO: leak; this memory is never freed.
-	 * It's arguable whether it's worth the trouble to care.
-	 */
 	sd = kzalloc(sizeof(*sd), GFP_KERNEL);
 	if (!sd) {
 		printk(KERN_ERR "PCI: OOM, skipping PCI bus %02x\n", busno);
@@ -638,7 +640,10 @@ struct pci_bus * __devinit pci_scan_bus_on_node(int busno, struct pci_ops *ops,
 	x86_pci_root_bus_resources(busno, &resources);
 	printk(KERN_DEBUG "PCI: Probing PCI hardware (bus %02x)\n", busno);
 	bus = pci_scan_root_bus(NULL, busno, ops, sd, &resources);
-	if (!bus) {
+	if (bus)
+		pci_set_host_bridge_release(to_pci_host_bridge(bus->bridge),
+					release_pci_sysdata, sd);
+	else {
 		pci_free_resource_list(&resources);
 		kfree(sd);
 	}
-- 
1.7.7

[PATCH v2 05/37] PCI: Separate out pci_assign_unassigned_bus_resources()

[Yinghai Lu]

It is main portion of pci_rescan_bus().

Separate it out and need to use it for pci root bus rescan later.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
 drivers/pci/setup-bus.c |   32 ++++++++++++++++++--------------
 include/linux/pci.h     |    1 +
 2 files changed, 19 insertions(+), 14 deletions(-)

diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index 8fa2d4b..6443c81 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -1488,25 +1488,12 @@ enable_all:
 }
 EXPORT_SYMBOL_GPL(pci_assign_unassigned_bridge_resources);
 
-#ifdef CONFIG_HOTPLUG
-/**
- * pci_rescan_bus - scan a PCI bus for devices.
- * @bus: PCI bus to scan
- *
- * Scan a PCI bus and child buses for new devices, adds them,
- * and enables them.
- *
- * Returns the max number of subordinate bus discovered.
- */
-unsigned int __ref pci_rescan_bus(struct pci_bus *bus)
+void pci_assign_unassigned_bus_resources(struct pci_bus *bus)
 {
-	unsigned int max;
 	struct pci_dev *dev;
 	LIST_HEAD(add_list); /* list of resources that
 					want additional resources */
 
-	max = pci_scan_child_bus(bus);
-
 	down_read(&pci_bus_sem);
 	list_for_each_entry(dev, &bus->devices, bus_list)
 		if (dev->hdr_type == PCI_HEADER_TYPE_BRIDGE ||
@@ -1519,6 +1506,23 @@ unsigned int __ref pci_rescan_bus(struct pci_bus *bus)
 	BUG_ON(!list_empty(&add_list));
 
 	pci_enable_bridges(bus);
+}
+#ifdef CONFIG_HOTPLUG
+/**
+ * pci_rescan_bus - scan a PCI bus for devices.
+ * @bus: PCI bus to scan
+ *
+ * Scan a PCI bus and child buses for new devices, adds them,
+ * and enables them.
+ *
+ * Returns the max number of subordinate bus discovered.
+ */
+unsigned int __ref pci_rescan_bus(struct pci_bus *bus)
+{
+	unsigned int max;
+
+	max = pci_scan_child_bus(bus);
+	pci_assign_unassigned_bus_resources(bus);
 	pci_bus_add_devices(bus);
 
 	return max;
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 40606c1..634bbf5 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -909,6 +909,7 @@ void pci_bus_size_bridges(struct pci_bus *bus);
 int pci_claim_resource(struct pci_dev *, int);
 void pci_assign_unassigned_resources(void);
 void pci_assign_unassigned_bridge_resources(struct pci_dev *bridge);
+void pci_assign_unassigned_bus_resources(struct pci_bus *bus);
 void pdev_enable_device(struct pci_dev *);
 int pci_enable_resources(struct pci_dev *, int mask);
 void pci_fixup_irqs(u8 (*)(struct pci_dev *, u8 *),
-- 
1.7.7

[PATCH v2 06/37] PCI: Move back pci_rescan_bus()

[Yinghai Lu]

Now we have pci_assign_unassigned_bus_resources() in place.

So move back pci_rescan_bus to probe.c where it should be.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
 drivers/pci/probe.c     |   21 +++++++++++++++++++++
 drivers/pci/setup-bus.c |   22 ----------------------
 2 files changed, 21 insertions(+), 22 deletions(-)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 67071b0..0d779f6 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -2054,6 +2054,27 @@ unsigned int __ref pci_rescan_bus_bridge_resize(struct pci_dev *bridge)
 	return max;
 }
 
+/**
+ * pci_rescan_bus - scan a PCI bus for devices.
+ * @bus: PCI bus to scan
+ *
+ * Scan a PCI bus and child buses for new devices, adds them,
+ * and enables them.
+ *
+ * Returns the max number of subordinate bus discovered.
+ */
+unsigned int __ref pci_rescan_bus(struct pci_bus *bus)
+{
+	unsigned int max;
+
+	max = pci_scan_child_bus(bus);
+	pci_assign_unassigned_bus_resources(bus);
+	pci_bus_add_devices(bus);
+
+	return max;
+}
+EXPORT_SYMBOL_GPL(pci_rescan_bus);
+
 EXPORT_SYMBOL(pci_add_new_bus);
 EXPORT_SYMBOL(pci_scan_slot);
 EXPORT_SYMBOL(pci_scan_bridge);
diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index 6443c81..f35a679 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -1507,25 +1507,3 @@ void pci_assign_unassigned_bus_resources(struct pci_bus *bus)
 
 	pci_enable_bridges(bus);
 }
-#ifdef CONFIG_HOTPLUG
-/**
- * pci_rescan_bus - scan a PCI bus for devices.
- * @bus: PCI bus to scan
- *
- * Scan a PCI bus and child buses for new devices, adds them,
- * and enables them.
- *
- * Returns the max number of subordinate bus discovered.
- */
-unsigned int __ref pci_rescan_bus(struct pci_bus *bus)
-{
-	unsigned int max;
-
-	max = pci_scan_child_bus(bus);
-	pci_assign_unassigned_bus_resources(bus);
-	pci_bus_add_devices(bus);
-
-	return max;
-}
-EXPORT_SYMBOL_GPL(pci_rescan_bus);
-#endif
-- 
1.7.7

[PATCH v2 07/37] PCI: pci_bus_size_bridges() should not size own bridge

[Yinghai Lu]

During checking acpiphp code, found following sequence:
        pci_bus_size_bridges(bus);
        pci_bus_assign_resources(bus);
	pci_enable_bridges(bus);

The problem is that when bus is not root bus, pci_bus_size_bridges would also size
own pci bridge bus->self if that bridge resource is not inserted before.
but later pci_bus_assign_resources will not allocate those size bridge res.

So try make it less confusing, let it does not include self sizing.
and add with_self parameter info __pci_bus_size_bridge()

Fixes caller in pci hotplug componets.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
 drivers/pci/hotplug/acpiphp_glue.c     |    2 +-
 drivers/pci/hotplug/cpci_hotplug_pci.c |    2 +-
 drivers/pci/hotplug/shpchp_pci.c       |    2 +-
 drivers/pci/setup-bus.c                |   24 +++++++++++++++---------
 include/linux/pci.h                    |    1 +
 5 files changed, 19 insertions(+), 12 deletions(-)

diff --git a/drivers/pci/hotplug/acpiphp_glue.c b/drivers/pci/hotplug/acpiphp_glue.c
index 806c44f..10b2122 100644
--- a/drivers/pci/hotplug/acpiphp_glue.c
+++ b/drivers/pci/hotplug/acpiphp_glue.c
@@ -816,7 +816,7 @@ static int __ref enable_device(struct acpiphp_slot *slot)
 			    dev->hdr_type == PCI_HEADER_TYPE_CARDBUS) {
 				max = pci_scan_bridge(bus, dev, max, pass);
 				if (pass && dev->subordinate)
-					pci_bus_size_bridges(dev->subordinate);
+					pci_bus_size_bridges_with_self(dev->subordinate);
 			}
 		}
 	}
diff --git a/drivers/pci/hotplug/cpci_hotplug_pci.c b/drivers/pci/hotplug/cpci_hotplug_pci.c
index ae853cc..4ef80ad 100644
--- a/drivers/pci/hotplug/cpci_hotplug_pci.c
+++ b/drivers/pci/hotplug/cpci_hotplug_pci.c
@@ -313,7 +313,7 @@ int __ref cpci_configure_slot(struct slot *slot)
 				continue;
 			}
 			child->subordinate = pci_do_scan_bus(child);
-			pci_bus_size_bridges(child);
+			pci_bus_size_bridges_with_self(child);
 		}
 		pci_dev_put(dev);
 	}
diff --git a/drivers/pci/hotplug/shpchp_pci.c b/drivers/pci/hotplug/shpchp_pci.c
index df7e4bf..d92807b 100644
--- a/drivers/pci/hotplug/shpchp_pci.c
+++ b/drivers/pci/hotplug/shpchp_pci.c
@@ -85,7 +85,7 @@ int __ref shpchp_configure_device(struct slot *p_slot)
 				continue;
 			}
 			child->subordinate = pci_do_scan_bus(child);
-			pci_bus_size_bridges(child);
+			pci_bus_size_bridges_with_self(child);
 		}
 		pci_configure_slot(dev);
 		pci_dev_put(dev);
diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index f35a679..ec2026c 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -982,8 +982,8 @@ handle_done:
 	;
 }
 
-void __ref __pci_bus_size_bridges(struct pci_bus *bus,
-			struct list_head *realloc_head)
+static void __ref __pci_bus_size_bridges(struct pci_bus *bus,
+			struct list_head *realloc_head, bool with_self)
 {
 	struct pci_dev *dev;
 	unsigned long mask, prefmask;
@@ -1001,13 +1001,13 @@ void __ref __pci_bus_size_bridges(struct pci_bus *bus,
 
 		case PCI_CLASS_BRIDGE_PCI:
 		default:
-			__pci_bus_size_bridges(b, realloc_head);
+			__pci_bus_size_bridges(b, realloc_head, true);
 			break;
 		}
 	}
 
-	/* The root bus? */
-	if (!bus->self)
+	/* Is root bus or not wanted? */
+	if (!bus->self || !with_self)
 		return;
 
 	switch (bus->self->class >> 8) {
@@ -1047,9 +1047,15 @@ void __ref __pci_bus_size_bridges(struct pci_bus *bus,
 	}
 }
 
+void __ref pci_bus_size_bridges_with_self(struct pci_bus *bus)
+{
+	__pci_bus_size_bridges(bus, NULL, true);
+}
+EXPORT_SYMBOL(pci_bus_size_bridges_with_self);
+
 void __ref pci_bus_size_bridges(struct pci_bus *bus)
 {
-	__pci_bus_size_bridges(bus, NULL);
+	__pci_bus_size_bridges(bus, NULL, false);
 }
 EXPORT_SYMBOL(pci_bus_size_bridges);
 
@@ -1362,7 +1368,7 @@ again:
 	/* Depth first, calculate sizes and alignments of all
 	   subordinate buses. */
 	list_for_each_entry(bus, &pci_root_buses, node)
-		__pci_bus_size_bridges(bus, add_list);
+		__pci_bus_size_bridges(bus, add_list, false);
 
 	/* Depth last, allocate resources and update the hardware. */
 	list_for_each_entry(bus, &pci_root_buses, node)
@@ -1439,7 +1445,7 @@ void pci_assign_unassigned_bridge_resources(struct pci_dev *bridge)
 				  IORESOURCE_PREFETCH;
 
 again:
-	__pci_bus_size_bridges(parent, &add_list);
+	__pci_bus_size_bridges(parent, &add_list, true);
 	__pci_bridge_assign_resources(bridge, &add_list, &fail_head);
 	BUG_ON(!list_empty(&add_list));
 	tried_times++;
@@ -1500,7 +1506,7 @@ void pci_assign_unassigned_bus_resources(struct pci_bus *bus)
 		    dev->hdr_type == PCI_HEADER_TYPE_CARDBUS)
 			if (dev->subordinate)
 				__pci_bus_size_bridges(dev->subordinate,
-							 &add_list);
+						 &add_list, true);
 	up_read(&pci_bus_sem);
 	__pci_bus_assign_resources(bus, &add_list, NULL);
 	BUG_ON(!list_empty(&add_list));
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 634bbf5..87dceca 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -906,6 +906,7 @@ int pci_vpd_truncate(struct pci_dev *dev, size_t size);
 resource_size_t pcibios_retrieve_fw_addr(struct pci_dev *dev, int idx);
 void pci_bus_assign_resources(const struct pci_bus *bus);
 void pci_bus_size_bridges(struct pci_bus *bus);
+void pci_bus_size_bridges_with_self(struct pci_bus *bus);
 int pci_claim_resource(struct pci_dev *, int);
 void pci_assign_unassigned_resources(void);
 void pci_assign_unassigned_bridge_resources(struct pci_dev *bridge);
-- 
1.7.7

Re: [PATCH v2 07/37] PCI: pci_bus_size_bridges() should not size own bridge

[Bjorn Helgaas]

On Sat, Mar 10, 2012 at 12:00 AM, Yinghai Lu <yinghai@kernel.org> wrote:
> During checking acpiphp code, found following sequence:
>        pci_bus_size_bridges(bus);
>        pci_bus_assign_resources(bus);
>        pci_enable_bridges(bus);

One of these lines is indented with tab, the others with spaces, which
makes it not line up when using "git log".  Try it sometime.

> The problem is that when bus is not root bus, pci_bus_size_bridges would also size
> own pci bridge bus->self if that bridge resource is not inserted before.
> but later pci_bus_assign_resources will not allocate those size bridge res.
>
> So try make it less confusing, let it does not include self sizing.
> and add with_self parameter info __pci_bus_size_bridge()

You failed to make this less confusing.  Adding words to function
names, adding "__" prefixes, adding flags to say "include self" or not
-- they all make things hard to read.

> Fixes caller in pci hotplug componets.

"components"

>
> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> ---
>  drivers/pci/hotplug/acpiphp_glue.c     |    2 +-
>  drivers/pci/hotplug/cpci_hotplug_pci.c |    2 +-
>  drivers/pci/hotplug/shpchp_pci.c       |    2 +-
>  drivers/pci/setup-bus.c                |   24 +++++++++++++++---------
>  include/linux/pci.h                    |    1 +
>  5 files changed, 19 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/pci/hotplug/acpiphp_glue.c b/drivers/pci/hotplug/acpiphp_glue.c
> index 806c44f..10b2122 100644
> --- a/drivers/pci/hotplug/acpiphp_glue.c
> +++ b/drivers/pci/hotplug/acpiphp_glue.c
> @@ -816,7 +816,7 @@ static int __ref enable_device(struct acpiphp_slot *slot)
>                            dev->hdr_type == PCI_HEADER_TYPE_CARDBUS) {
>                                max = pci_scan_bridge(bus, dev, max, pass);
>                                if (pass && dev->subordinate)
> -                                       pci_bus_size_bridges(dev->subordinate);
> +                                       pci_bus_size_bridges_with_self(dev->subordinate);
>                        }
>                }
>        }
> diff --git a/drivers/pci/hotplug/cpci_hotplug_pci.c b/drivers/pci/hotplug/cpci_hotplug_pci.c
> index ae853cc..4ef80ad 100644
> --- a/drivers/pci/hotplug/cpci_hotplug_pci.c
> +++ b/drivers/pci/hotplug/cpci_hotplug_pci.c
> @@ -313,7 +313,7 @@ int __ref cpci_configure_slot(struct slot *slot)
>                                continue;
>                        }
>                        child->subordinate = pci_do_scan_bus(child);
> -                       pci_bus_size_bridges(child);
> +                       pci_bus_size_bridges_with_self(child);
>                }
>                pci_dev_put(dev);
>        }
> diff --git a/drivers/pci/hotplug/shpchp_pci.c b/drivers/pci/hotplug/shpchp_pci.c
> index df7e4bf..d92807b 100644
> --- a/drivers/pci/hotplug/shpchp_pci.c
> +++ b/drivers/pci/hotplug/shpchp_pci.c
> @@ -85,7 +85,7 @@ int __ref shpchp_configure_device(struct slot *p_slot)
>                                continue;
>                        }
>                        child->subordinate = pci_do_scan_bus(child);
> -                       pci_bus_size_bridges(child);
> +                       pci_bus_size_bridges_with_self(child);
>                }
>                pci_configure_slot(dev);
>                pci_dev_put(dev);
> diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
> index f35a679..ec2026c 100644
> --- a/drivers/pci/setup-bus.c
> +++ b/drivers/pci/setup-bus.c
> @@ -982,8 +982,8 @@ handle_done:
>        ;
>  }
>
> -void __ref __pci_bus_size_bridges(struct pci_bus *bus,
> -                       struct list_head *realloc_head)
> +static void __ref __pci_bus_size_bridges(struct pci_bus *bus,
> +                       struct list_head *realloc_head, bool with_self)
>  {
>        struct pci_dev *dev;
>        unsigned long mask, prefmask;
> @@ -1001,13 +1001,13 @@ void __ref __pci_bus_size_bridges(struct pci_bus *bus,
>
>                case PCI_CLASS_BRIDGE_PCI:
>                default:
> -                       __pci_bus_size_bridges(b, realloc_head);
> +                       __pci_bus_size_bridges(b, realloc_head, true);
>                        break;
>                }
>        }
>
> -       /* The root bus? */
> -       if (!bus->self)
> +       /* Is root bus or not wanted? */
> +       if (!bus->self || !with_self)
>                return;
>
>        switch (bus->self->class >> 8) {
> @@ -1047,9 +1047,15 @@ void __ref __pci_bus_size_bridges(struct pci_bus *bus,
>        }
>  }
>
> +void __ref pci_bus_size_bridges_with_self(struct pci_bus *bus)
> +{
> +       __pci_bus_size_bridges(bus, NULL, true);
> +}
> +EXPORT_SYMBOL(pci_bus_size_bridges_with_self);
> +
>  void __ref pci_bus_size_bridges(struct pci_bus *bus)
>  {
> -       __pci_bus_size_bridges(bus, NULL);
> +       __pci_bus_size_bridges(bus, NULL, false);
>  }
>  EXPORT_SYMBOL(pci_bus_size_bridges);
>
> @@ -1362,7 +1368,7 @@ again:
>        /* Depth first, calculate sizes and alignments of all
>           subordinate buses. */
>        list_for_each_entry(bus, &pci_root_buses, node)
> -               __pci_bus_size_bridges(bus, add_list);
> +               __pci_bus_size_bridges(bus, add_list, false);
>
>        /* Depth last, allocate resources and update the hardware. */
>        list_for_each_entry(bus, &pci_root_buses, node)
> @@ -1439,7 +1445,7 @@ void pci_assign_unassigned_bridge_resources(struct pci_dev *bridge)
>                                  IORESOURCE_PREFETCH;
>
>  again:
> -       __pci_bus_size_bridges(parent, &add_list);
> +       __pci_bus_size_bridges(parent, &add_list, true);
>        __pci_bridge_assign_resources(bridge, &add_list, &fail_head);
>        BUG_ON(!list_empty(&add_list));
>        tried_times++;
> @@ -1500,7 +1506,7 @@ void pci_assign_unassigned_bus_resources(struct pci_bus *bus)
>                    dev->hdr_type == PCI_HEADER_TYPE_CARDBUS)
>                        if (dev->subordinate)
>                                __pci_bus_size_bridges(dev->subordinate,
> -                                                        &add_list);
> +                                                &add_list, true);
>        up_read(&pci_bus_sem);
>        __pci_bus_assign_resources(bus, &add_list, NULL);
>        BUG_ON(!list_empty(&add_list));
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 634bbf5..87dceca 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -906,6 +906,7 @@ int pci_vpd_truncate(struct pci_dev *dev, size_t size);
>  resource_size_t pcibios_retrieve_fw_addr(struct pci_dev *dev, int idx);
>  void pci_bus_assign_resources(const struct pci_bus *bus);
>  void pci_bus_size_bridges(struct pci_bus *bus);
> +void pci_bus_size_bridges_with_self(struct pci_bus *bus);
>  int pci_claim_resource(struct pci_dev *, int);
>  void pci_assign_unassigned_resources(void);
>  void pci_assign_unassigned_bridge_resources(struct pci_dev *bridge);
> --
> 1.7.7
>

[PATCH v2 08/37] PCI: Use __pci_bus_size_bridges() directly in pci_assign_unassigned_bus_resources()

[Yinghai Lu]

Now __pci_bus_size_bridges() will not include self bridge.
So we don't need to have our own version in
 pci_assign_unassigned_bus_resources anymore.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
 drivers/pci/setup-bus.c |    8 +-------
 1 files changed, 1 insertions(+), 7 deletions(-)

diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index ec2026c..d9a5500 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -1496,17 +1496,11 @@ EXPORT_SYMBOL_GPL(pci_assign_unassigned_bridge_resources);
 
 void pci_assign_unassigned_bus_resources(struct pci_bus *bus)
 {
-	struct pci_dev *dev;
 	LIST_HEAD(add_list); /* list of resources that
 					want additional resources */
 
 	down_read(&pci_bus_sem);
-	list_for_each_entry(dev, &bus->devices, bus_list)
-		if (dev->hdr_type == PCI_HEADER_TYPE_BRIDGE ||
-		    dev->hdr_type == PCI_HEADER_TYPE_CARDBUS)
-			if (dev->subordinate)
-				__pci_bus_size_bridges(dev->subordinate,
-						 &add_list, true);
+	__pci_bus_size_bridges(bus, &add_list, false);
 	up_read(&pci_bus_sem);
 	__pci_bus_assign_resources(bus, &add_list, NULL);
 	BUG_ON(!list_empty(&add_list));
-- 
1.7.7

[PATCH v2 09/37] PCI, sysfs: Use device_type and attr_groups with pci dev

[Yinghai Lu]

We want to create rescan in sys only for pci bridge instead of all pci dev.

We could use attribute_groups/is_visible method to do that.

Now pci dev does not use device type yet. So add pci_dev_type to take
attr_groups with it.

Add pci_dev_bridge_attrs_are_visible() to control attr_bridge_group only create
attr for bridge.

This is the framework related change, later could attr to bridge_attr_group,
to make those attr only show up on pci bridge device.

Also We could add more attr groups with is_visible to reduce messness in
	pci_create_sysfs_dev_files. ( at least for boot_vga one )

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
 drivers/pci/pci-sysfs.c |   30 ++++++++++++++++++++++++++++++
 drivers/pci/pci.h       |    1 +
 drivers/pci/probe.c     |    1 +
 3 files changed, 32 insertions(+), 0 deletions(-)

diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index a55e248..39d15e6 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -1335,3 +1335,33 @@ static int __init pci_sysfs_init(void)
 }
 
 late_initcall(pci_sysfs_init);
+
+static struct attribute *pci_dev_bridge_attrs[] = {
+	NULL,
+};
+
+static umode_t pci_dev_bridge_attrs_are_visible(struct kobject *kobj,
+						struct attribute *a, int n)
+{
+	struct device *dev = container_of(kobj, struct device, kobj);
+	struct pci_dev *pdev = to_pci_dev(dev);
+
+	if (!pdev->subordinate)
+		return 0;
+
+	return a->mode;
+}
+
+static struct attribute_group pci_dev_bridge_attr_group = {
+	.attrs = pci_dev_bridge_attrs,
+	.is_visible = pci_dev_bridge_attrs_are_visible,
+};
+
+static const struct attribute_group *pci_dev_attr_groups[] = {
+	&pci_dev_bridge_attr_group,
+	NULL,
+};
+
+struct device_type pci_dev_type = {
+	.groups = pci_dev_attr_groups,
+};
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index e494347..13b1159 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -162,6 +162,7 @@ static inline int pci_no_d1d2(struct pci_dev *dev)
 }
 extern struct device_attribute pci_dev_attrs[];
 extern struct device_attribute pcibus_dev_attrs[];
+extern struct device_type pci_dev_type;
 #ifdef CONFIG_HOTPLUG
 extern struct bus_attribute pci_bus_attrs[];
 #else
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 0d779f6..2f5f728 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1138,6 +1138,7 @@ int pci_setup_device(struct pci_dev *dev)
 	dev->sysdata = dev->bus->sysdata;
 	dev->dev.parent = dev->bus->bridge;
 	dev->dev.bus = &pci_bus_type;
+	dev->dev.type = &pci_dev_type;
 	dev->hdr_type = hdr_type & 0x7f;
 	dev->multifunction = !!(hdr_type & 0x80);
 	dev->error_state = pci_channel_io_normal;
-- 
1.7.7

[PATCH v2 10/37] PCI, sysfs: create rescan_bridge under /sys/.../pci/devices/... for pci bridges

[Yinghai Lu]

Current code will create rescan for every pci device under parent bus.
that is not right. the device is already there, there is no reason to rescan it.

We could have rescan for pci bridges. less confusing.

Need to move rescan attr to pci dev bridge attribute group.
And We should rescan bridge's secondary bus instead of primary bus.

-v3: Use device_type for pci dev.
-v4: Seperate pci device type change out
-v5: add rescan_bridge for bridge type, and still keep the old rescan.
	may remove the old rescan later.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
Cc: Randy Dunlap <rdunlap@xenotime.net>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 Documentation/ABI/testing/sysfs-bus-pci |   10 ++++++++++
 drivers/pci/pci-sysfs.c                 |   24 ++++++++++++++++++++++++
 2 files changed, 34 insertions(+), 0 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-bus-pci b/Documentation/ABI/testing/sysfs-bus-pci
index 34f5110..95f0f37 100644
--- a/Documentation/ABI/testing/sysfs-bus-pci
+++ b/Documentation/ABI/testing/sysfs-bus-pci
@@ -111,6 +111,16 @@ Description:
 		from this part of the device tree.
 		Depends on CONFIG_HOTPLUG.
 
+What:		/sys/bus/pci/devices/.../rescan_bridge
+Date:		February 2012
+Contact:	Linux PCI developers <linux-pci@vger.kernel.org>
+Description:
+		Writing a non-zero value to this attribute will
+		force a rescan of the bridge and all child buses, and
+		re-discover devices removed earlier from this part of
+		the device tree.
+		Depends on CONFIG_HOTPLUG.
+
 What:		/sys/bus/pci/devices/.../reset
 Date:		July 2009
 Contact:	Michael S. Tsirkin <mst@redhat.com>
diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index 39d15e6..d5c8ffb 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -325,6 +325,27 @@ dev_rescan_store(struct device *dev, struct device_attribute *attr,
 	return count;
 }
 
+static ssize_t
+dev_bridge_rescan_store(struct device *dev, struct device_attribute *attr,
+		 const char *buf, size_t count)
+{
+	unsigned long val;
+	struct pci_dev *pdev = to_pci_dev(dev);
+
+	if (kstrtoul(buf, 0, &val) < 0)
+		return -EINVAL;
+
+	if (val) {
+		mutex_lock(&pci_remove_rescan_mutex);
+		pci_rescan_bus(pdev->subordinate);
+		mutex_unlock(&pci_remove_rescan_mutex);
+	}
+	return count;
+}
+
+static struct device_attribute pci_dev_bridge_rescan_attr =
+	__ATTR(rescan_bridge, (S_IWUSR|S_IWGRP), NULL, dev_bridge_rescan_store);
+
 static void remove_callback(struct device *dev)
 {
 	struct pci_dev *pdev = to_pci_dev(dev);
@@ -1337,6 +1358,9 @@ static int __init pci_sysfs_init(void)
 late_initcall(pci_sysfs_init);
 
 static struct attribute *pci_dev_bridge_attrs[] = {
+#ifdef CONFIG_HOTPLUG
+	&pci_dev_bridge_rescan_attr.attr,
+#endif
 	NULL,
 };
 
-- 
1.7.7

Re: [PATCH v2 10/37] PCI, sysfs: create rescan_bridge under /sys/.../pci/devices/... for pci bridges

[Bjorn Helgaas]

On Sat, Mar 10, 2012 at 12:00 AM, Yinghai Lu <yinghai@kernel.org> wrote:
> Current code will create rescan for every pci device under parent bus.
> that is not right. the device is already there, there is no reason to rescan it.

"The device" (not "the device").

> We could have rescan for pci bridges. less confusing.
>
> Need to move rescan attr to pci dev bridge attribute group.
> And We should rescan bridge's secondary bus instead of primary bus.

"And we" (not "And We").

I think the idea of this patch makes sense.

You might want to consider having this actually read the bridge config
again.  That way, if the user manually changed the secondary bus
number or the bridge windows using setpci or something, this rescan
could use that new config if possible.

I still hate the fact that we will have three different rescan files
(four if you count /sys/bus/pci/rescan), but I don't have a solution
to offer:
    device/rescan
    device/rescan_bridge
    bus/rescan

> -v3: Use device_type for pci dev.
> -v4: Seperate pci device type change out
> -v5: add rescan_bridge for bridge type, and still keep the old rescan.
>        may remove the old rescan later.
>
> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> Cc: Randy Dunlap <rdunlap@xenotime.net>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
>  Documentation/ABI/testing/sysfs-bus-pci |   10 ++++++++++
>  drivers/pci/pci-sysfs.c                 |   24 ++++++++++++++++++++++++
>  2 files changed, 34 insertions(+), 0 deletions(-)
>
> diff --git a/Documentation/ABI/testing/sysfs-bus-pci b/Documentation/ABI/testing/sysfs-bus-pci
> index 34f5110..95f0f37 100644
> --- a/Documentation/ABI/testing/sysfs-bus-pci
> +++ b/Documentation/ABI/testing/sysfs-bus-pci
> @@ -111,6 +111,16 @@ Description:
>                from this part of the device tree.
>                Depends on CONFIG_HOTPLUG.
>
> +What:          /sys/bus/pci/devices/.../rescan_bridge
> +Date:          February 2012
> +Contact:       Linux PCI developers <linux-pci@vger.kernel.org>
> +Description:
> +               Writing a non-zero value to this attribute will
> +               force a rescan of the bridge and all child buses, and
> +               re-discover devices removed earlier from this part of
> +               the device tree.
> +               Depends on CONFIG_HOTPLUG.
> +
>  What:          /sys/bus/pci/devices/.../reset
>  Date:          July 2009
>  Contact:       Michael S. Tsirkin <mst@redhat.com>
> diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
> index 39d15e6..d5c8ffb 100644
> --- a/drivers/pci/pci-sysfs.c
> +++ b/drivers/pci/pci-sysfs.c
> @@ -325,6 +325,27 @@ dev_rescan_store(struct device *dev, struct device_attribute *attr,
>        return count;
>  }
>
> +static ssize_t
> +dev_bridge_rescan_store(struct device *dev, struct device_attribute *attr,
> +                const char *buf, size_t count)
> +{
> +       unsigned long val;
> +       struct pci_dev *pdev = to_pci_dev(dev);
> +
> +       if (kstrtoul(buf, 0, &val) < 0)
> +               return -EINVAL;
> +
> +       if (val) {
> +               mutex_lock(&pci_remove_rescan_mutex);
> +               pci_rescan_bus(pdev->subordinate);
> +               mutex_unlock(&pci_remove_rescan_mutex);
> +       }
> +       return count;
> +}
> +
> +static struct device_attribute pci_dev_bridge_rescan_attr =
> +       __ATTR(rescan_bridge, (S_IWUSR|S_IWGRP), NULL, dev_bridge_rescan_store);
> +
>  static void remove_callback(struct device *dev)
>  {
>        struct pci_dev *pdev = to_pci_dev(dev);
> @@ -1337,6 +1358,9 @@ static int __init pci_sysfs_init(void)
>  late_initcall(pci_sysfs_init);
>
>  static struct attribute *pci_dev_bridge_attrs[] = {
> +#ifdef CONFIG_HOTPLUG
> +       &pci_dev_bridge_rescan_attr.attr,
> +#endif
>        NULL,
>  };
>
> --
> 1.7.7
>

Re: [PATCH v2 10/37] PCI, sysfs: create rescan_bridge under /sys/.../pci/devices/... for pci bridges

[Yinghai Lu]

On Mon, Mar 12, 2012 at 7:55 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> On Sat, Mar 10, 2012 at 12:00 AM, Yinghai Lu <yinghai@kernel.org> wrote:
>> Current code will create rescan for every pci device under parent bus.
>> that is not right. the device is already there, there is no reason to rescan it.
>
> "The device" (not "the device").

ok

>
>> We could have rescan for pci bridges. less confusing.
>>
>> Need to move rescan attr to pci dev bridge attribute group.
>> And We should rescan bridge's secondary bus instead of primary bus.
>
> "And we" (not "And We").

ok

>
> I think the idea of this patch makes sense.
>
> You might want to consider having this actually read the bridge config
> again.  That way, if the user manually changed the secondary bus
> number or the bridge windows using setpci or something, this rescan
> could use that new config if possible.

yes, in the following patches, change that to
pci_rescan_bus_bridge_resize()

and

will be
unsigned int __ref pci_rescan_bus_bridge_resize(struct pci_dev *bridge)
{
        unsigned int max = 0;
        int pass;
        struct pci_bus *bus = bridge->bus;

        for (pass = 0; pass < 2; pass++)
                max = pci_scan_bridge(bus, bridge, max, pass);

        pci_assign_unassigned_bridge_resources(bridge);

        pci_bus_add_single_device(bridge);

at last pci_scan_bridge will check the bus register in the bridge.

>
> I still hate the fact that we will have three different rescan files
> (four if you count /sys/bus/pci/rescan), but I don't have a solution
> to offer:
>    device/rescan
>    device/rescan_bridge
>    bus/rescan

looks like we only need
/sys/devices/pci.../device/rescan_bridge
/sys/bus/pci/bus/rescan

device rescan and bus rescan should be removed.

Yinghai

[PATCH v2 11/37] PCI: Add pci_bus_add_single_device()

[Yinghai Lu]

Will use it to pci_bus_bridge_scan_resize() to make bridge will
have pci_bus directory created correctly.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
 drivers/pci/bus.c   |   39 +++++++++++++++++++++++++++++++++++++++
 include/linux/pci.h |    1 +
 2 files changed, 40 insertions(+), 0 deletions(-)

diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c
index 4ce5ef2..1eb7944 100644
--- a/drivers/pci/bus.c
+++ b/drivers/pci/bus.c
@@ -255,6 +255,45 @@ void pci_bus_add_devices(const struct pci_bus *bus)
 	}
 }
 
+void pci_bus_add_single_device(struct pci_dev *dev)
+{
+	struct pci_bus *child;
+	int retval;
+
+	/* Skip already-added devices */
+	if (!dev->is_added) {
+		retval = pci_bus_add_device(dev);
+		if (retval)
+			dev_err(&dev->dev, "Error adding device, continuing\n");
+	}
+
+	BUG_ON(!dev->is_added);
+
+	child = dev->subordinate;
+	/*
+	 * If there is an unattached subordinate bus, attach
+	 * it and then scan for unattached PCI devices.
+	 */
+	if (child) {
+		if (list_empty(&child->node)) {
+			down_write(&pci_bus_sem);
+			list_add_tail(&child->node, &dev->bus->children);
+			up_write(&pci_bus_sem);
+		}
+		pci_bus_add_devices(child);
+
+		/*
+		 * register the bus with sysfs as the parent is now
+		 * properly registered.
+		 */
+		if (!child->is_added) {
+			retval = pci_bus_add_child(child);
+			if (retval)
+				dev_err(&dev->dev, "Error adding bus, continuing\n");
+		}
+	}
+}
+
 void pci_enable_bridges(struct pci_bus *bus)
 {
 	struct pci_dev *dev;
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 87dceca..860091f 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -665,6 +665,7 @@ void pcibios_bus_to_resource(struct pci_dev *dev, struct resource *res,
 void pcibios_scan_specific_bus(int busn);
 extern struct pci_bus *pci_find_bus(int domain, int busnr);
 void pci_bus_add_devices(const struct pci_bus *bus);
+void pci_bus_add_single_device(struct pci_dev *dev);
 struct pci_bus *pci_scan_bus_parented(struct device *parent, int bus,
 				      struct pci_ops *ops, void *sysdata);
 struct pci_bus *pci_scan_bus(int bus, struct pci_ops *ops, void *sysdata);
-- 
1.7.7

[PATCH v2 12/37] PCI: Make pci_rescan_bus_bridge_resize() use pci_scan_bridge instead

[Yinghai Lu]

So after remove all children and then using setpci change bus register and rescan
bridge could use new set bus number.

Otherwise need to rescan parent bus, it would have too much overhead.

also need to use pci_bus_add_single_device to make sure new change bus have directory
 /sys/../.../pci_bus.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
 drivers/pci/probe.c |   10 ++++++----
 1 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 2f5f728..8512e0d 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -2043,14 +2043,16 @@ EXPORT_SYMBOL(pci_scan_bus);
  */
 unsigned int __ref pci_rescan_bus_bridge_resize(struct pci_dev *bridge)
 {
-	unsigned int max;
-	struct pci_bus *bus = bridge->subordinate;
+	unsigned int max = 0;
+	int pass;
+	struct pci_bus *bus = bridge->bus;
 
-	max = pci_scan_child_bus(bus);
+	for (pass = 0; pass < 2; pass++)
+		max = pci_scan_bridge(bus, bridge, max, pass);
 
 	pci_assign_unassigned_bridge_resources(bridge);
 
-	pci_bus_add_devices(bus);
+	pci_bus_add_single_device(bridge);
 
 	return max;
 }
-- 
1.7.7

[PATCH v2 13/37] PCI: Clean up rescan_bus_bridge_resize()

[Yinghai Lu]

Should only use it with bridge instead of bus.
It could get new subordinate. so can not use it with reguar bus.

In that case, user may need to make sure all children devices get removed
already before rescan.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
 drivers/pci/pci-sysfs.c |    7 ++-----
 1 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index d5c8ffb..2049b2f 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -337,7 +337,7 @@ dev_bridge_rescan_store(struct device *dev, struct device_attribute *attr,
 
 	if (val) {
 		mutex_lock(&pci_remove_rescan_mutex);
-		pci_rescan_bus(pdev->subordinate);
+		pci_rescan_bus_bridge_resize(pdev);
 		mutex_unlock(&pci_remove_rescan_mutex);
 	}
 	return count;
@@ -387,10 +387,7 @@ dev_bus_rescan_store(struct device *dev, struct device_attribute *attr,
 
 	if (val) {
 		mutex_lock(&pci_remove_rescan_mutex);
-		if (!pci_is_root_bus(bus) && list_empty(&bus->devices))
-			pci_rescan_bus_bridge_resize(bus->self);
-		else
-			pci_rescan_bus(bus);
+		pci_rescan_bus(bus);
 		mutex_unlock(&pci_remove_rescan_mutex);
 	}
 	return count;
-- 
1.7.7

[PATCH v2 14/37] PCI: Rescan bus or bridge using callback method too

[Yinghai Lu]

Just like removal.

Because We could add new bus under the bridges...

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
 drivers/pci/pci-sysfs.c |   43 +++++++++++++++++++++++++++++++------------
 1 files changed, 31 insertions(+), 12 deletions(-)

diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index 2049b2f..1794508 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -325,21 +325,31 @@ dev_rescan_store(struct device *dev, struct device_attribute *attr,
 	return count;
 }
 
+static void bridge_rescan_callback(struct device *dev)
+{
+	struct pci_dev *pdev = to_pci_dev(dev);
+
+	mutex_lock(&pci_remove_rescan_mutex);
+	pci_rescan_bus_bridge_resize(pdev);
+	mutex_unlock(&pci_remove_rescan_mutex);
+}
+
 static ssize_t
 dev_bridge_rescan_store(struct device *dev, struct device_attribute *attr,
 		 const char *buf, size_t count)
 {
+	int ret = 0;
 	unsigned long val;
-	struct pci_dev *pdev = to_pci_dev(dev);
 
 	if (kstrtoul(buf, 0, &val) < 0)
 		return -EINVAL;
 
-	if (val) {
-		mutex_lock(&pci_remove_rescan_mutex);
-		pci_rescan_bus_bridge_resize(pdev);
-		mutex_unlock(&pci_remove_rescan_mutex);
-	}
+	if (val)
+		ret = device_schedule_callback(dev, bridge_rescan_callback);
+
+	if (ret)
+		count = ret;
+
 	return count;
 }
 
@@ -375,21 +385,30 @@ remove_store(struct device *dev, struct device_attribute *dummy,
 	return count;
 }
 
+static void bus_rescan_callback(struct device *dev)
+{
+	struct pci_bus *bus = to_pci_bus(dev);
+
+	mutex_lock(&pci_remove_rescan_mutex);
+	pci_rescan_bus(bus);
+	mutex_unlock(&pci_remove_rescan_mutex);
+}
 static ssize_t
 dev_bus_rescan_store(struct device *dev, struct device_attribute *attr,
 		 const char *buf, size_t count)
 {
+	int ret = 0;
 	unsigned long val;
-	struct pci_bus *bus = to_pci_bus(dev);
 
 	if (strict_strtoul(buf, 0, &val) < 0)
 		return -EINVAL;
 
-	if (val) {
-		mutex_lock(&pci_remove_rescan_mutex);
-		pci_rescan_bus(bus);
-		mutex_unlock(&pci_remove_rescan_mutex);
-	}
+	if (val)
+		ret = device_schedule_callback(dev, bus_rescan_callback);
+
+	if (ret)
+		count = ret;
+
 	return count;
 }
 
-- 
1.7.7

[PATCH v2 15/37] PCI, sysfs: Clean up rescan/remove with scheule_callback

[Yinghai Lu]

Change to use three return according to Bjorn.

Suggested-by: Bjorn Helgaas <bhelgaas@google.com>
Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
 drivers/pci/pci-sysfs.c |   37 ++++++++++++++++++++-----------------
 1 files changed, 20 insertions(+), 17 deletions(-)

diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index 1794508..4697afe 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -338,17 +338,17 @@ static ssize_t
 dev_bridge_rescan_store(struct device *dev, struct device_attribute *attr,
 		 const char *buf, size_t count)
 {
-	int ret = 0;
+	int err;
 	unsigned long val;
 
 	if (kstrtoul(buf, 0, &val) < 0)
 		return -EINVAL;
+	if (!val)
+		return count;
 
-	if (val)
-		ret = device_schedule_callback(dev, bridge_rescan_callback);
-
-	if (ret)
-		count = ret;
+	err = device_schedule_callback(dev, bridge_rescan_callback);
+	if (err)
+		return err;
 
 	return count;
 }
@@ -369,7 +369,7 @@ static ssize_t
 remove_store(struct device *dev, struct device_attribute *dummy,
 	     const char *buf, size_t count)
 {
-	int ret = 0;
+	int err;
 	unsigned long val;
 
 	if (strict_strtoul(buf, 0, &val) < 0)
@@ -378,10 +378,13 @@ remove_store(struct device *dev, struct device_attribute *dummy,
 	/* An attribute cannot be unregistered by one of its own methods,
 	 * so we have to use this roundabout approach.
 	 */
-	if (val)
-		ret = device_schedule_callback(dev, remove_callback);
-	if (ret)
-		count = ret;
+	if (!val)
+		return count;
+
+	err = device_schedule_callback(dev, remove_callback);
+	if (err)
+		return err;
+
 	return count;
 }
 
@@ -397,17 +400,17 @@ static ssize_t
 dev_bus_rescan_store(struct device *dev, struct device_attribute *attr,
 		 const char *buf, size_t count)
 {
-	int ret = 0;
+	int err;
 	unsigned long val;
 
 	if (strict_strtoul(buf, 0, &val) < 0)
 		return -EINVAL;
+	if (!val)
+		return count;
 
-	if (val)
-		ret = device_schedule_callback(dev, bus_rescan_callback);
-
-	if (ret)
-		count = ret;
+	err = device_schedule_callback(dev, bus_rescan_callback);
+	if (err)
+		return err;
 
 	return count;
 }
-- 
1.7.7

[PATCH v2 16/37] PCI: Move pci_stop_and_remove_behind_bridge() down

[Yinghai Lu]

Later it will use pci_stop_bus_devices instead of pci_stop_behind_bridge.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
 drivers/pci/remove.c |   30 +++++++++++++++---------------
 1 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/drivers/pci/remove.c b/drivers/pci/remove.c
index 04a4861..243d59b 100644
--- a/drivers/pci/remove.c
+++ b/drivers/pci/remove.c
@@ -129,21 +129,6 @@ static void pci_stop_behind_bridge(struct pci_dev *dev)
 			pci_stop_bus_device(pci_dev_b(l));
 }
 
-/**
- * pci_stop_and_remove_behind_bridge - stop and remove all devices behind
- *					 a PCI bridge
- * @dev: PCI bridge device
- *
- * Remove all devices on the bus, except for the parent bridge.
- * This also removes any child buses, and any devices they may
- * contain in a depth-first manner.
- */
-void pci_stop_and_remove_behind_bridge(struct pci_dev *dev)
-{
-	pci_stop_behind_bridge(dev);
-	__pci_remove_behind_bridge(dev);
-}
-
 static void pci_stop_bus_devices(struct pci_bus *bus)
 {
 	struct list_head *l, *n;
@@ -163,6 +148,21 @@ static void pci_stop_bus_devices(struct pci_bus *bus)
 }
 
 /**
+ * pci_stop_and_remove_behind_bridge - stop and remove all devices behind
+ *					 a PCI bridge
+ * @dev: PCI bridge device
+ *
+ * Remove all devices on the bus, except for the parent bridge.
+ * This also removes any child buses, and any devices they may
+ * contain in a depth-first manner.
+ */
+void pci_stop_and_remove_behind_bridge(struct pci_dev *dev)
+{
+	pci_stop_behind_bridge(dev);
+	__pci_remove_behind_bridge(dev);
+}
+
+/**
  * pci_stop_bus_device - stop a PCI device and any children
  * @dev: the device to stop
  *
-- 
1.7.7

[PATCH v2 17/37] PCI: Add __pci_remove_bus_devices()

[Yinghai Lu]

Will use it with pci_stop_and_remove_bus later.

Also remove __pci_remove_behind_bridge and pci_stop_behind_bridge.

They are same except one take bridge and one take bus.

We already have pci_stop_bus_devices().

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
 drivers/pci/remove.c |   28 +++++++++++-----------------
 1 files changed, 11 insertions(+), 17 deletions(-)

diff --git a/drivers/pci/remove.c b/drivers/pci/remove.c
index 243d59b..62c348c 100644
--- a/drivers/pci/remove.c
+++ b/drivers/pci/remove.c
@@ -78,7 +78,7 @@ void pci_remove_bus(struct pci_bus *pci_bus)
 }
 EXPORT_SYMBOL(pci_remove_bus);
 
-static void __pci_remove_behind_bridge(struct pci_dev *dev);
+static void __pci_remove_bus_devices(struct pci_bus *bus);
 /**
  * pci_stop_and_remove_bus_device - remove a PCI device and any children
  * @dev: the device to remove
@@ -96,7 +96,7 @@ void __pci_remove_bus_device(struct pci_dev *dev)
 	if (dev->subordinate) {
 		struct pci_bus *b = dev->subordinate;
 
-		__pci_remove_behind_bridge(dev);
+		__pci_remove_bus_devices(b);
 		pci_remove_bus(b);
 		dev->subordinate = NULL;
 	}
@@ -111,22 +111,12 @@ void pci_stop_and_remove_bus_device(struct pci_dev *dev)
 	__pci_remove_bus_device(dev);
 }
 
-static void __pci_remove_behind_bridge(struct pci_dev *dev)
+static void __pci_remove_bus_devices(struct pci_bus *bus)
 {
 	struct list_head *l, *n;
 
-	if (dev->subordinate)
-		list_for_each_safe(l, n, &dev->subordinate->devices)
-			__pci_remove_bus_device(pci_dev_b(l));
-}
-
-static void pci_stop_behind_bridge(struct pci_dev *dev)
-{
-	struct list_head *l, *n;
-
-	if (dev->subordinate)
-		list_for_each_safe(l, n, &dev->subordinate->devices)
-			pci_stop_bus_device(pci_dev_b(l));
+	list_for_each_safe(l, n, &bus->devices)
+		__pci_remove_bus_device(pci_dev_b(l));
 }
 
 static void pci_stop_bus_devices(struct pci_bus *bus)
@@ -158,8 +148,12 @@ static void pci_stop_bus_devices(struct pci_bus *bus)
  */
 void pci_stop_and_remove_behind_bridge(struct pci_dev *dev)
 {
-	pci_stop_behind_bridge(dev);
-	__pci_remove_behind_bridge(dev);
+	struct pci_bus *bus = dev->subordinate;
+
+	if (bus) {
+		pci_stop_bus_devices(bus);
+		__pci_remove_bus_devices(bus);
+	}
 }
 
 /**
-- 
1.7.7

[PATCH v2 18/37] PCI: Use list_for_each_entry_safe instead of list_for_each_safe

[Yinghai Lu]

So could skip using pci_dev_b().

Suggested-by: Bjorn Helgaas <bhelgaas@google.com>
Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
 drivers/pci/remove.c |   12 +++++-------
 1 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/pci/remove.c b/drivers/pci/remove.c
index 62c348c..d4fdf7b 100644
--- a/drivers/pci/remove.c
+++ b/drivers/pci/remove.c
@@ -113,15 +113,15 @@ void pci_stop_and_remove_bus_device(struct pci_dev *dev)
 
 static void __pci_remove_bus_devices(struct pci_bus *bus)
 {
-	struct list_head *l, *n;
+	struct pci_dev *dev, *tmp;
 
-	list_for_each_safe(l, n, &bus->devices)
-		__pci_remove_bus_device(pci_dev_b(l));
+	list_for_each_entry_safe(dev, tmp, &bus->devices, bus_list)
+		__pci_remove_bus_device(dev);
 }
 
 static void pci_stop_bus_devices(struct pci_bus *bus)
 {
-	struct list_head *l, *n;
+	struct pci_dev *dev, *tmp;
 
 	/*
 	 * VFs could be removed by pci_stop_and_remove_bus_device() in the
@@ -131,10 +131,8 @@ static void pci_stop_bus_devices(struct pci_bus *bus)
 	 * We can iterate the list backwards to get prev valid PF instead
 	 *  of removed VF.
 	 */
-	list_for_each_prev_safe(l, n, &bus->devices) {
-		struct pci_dev *dev = pci_dev_b(l);
+	list_for_each_entry_safe_reverse(dev, tmp, &bus->devices, bus_list)
 		pci_stop_bus_device(dev);
-	}
 }
 
 /**
-- 
1.7.7

[PATCH v2 19/37] PCI: Add pci_stop_and_remove_bus()

[Yinghai Lu]

It supports both pci root bus and pci bus under pci bridge.

-v2: clear pci_bridge's subordinate.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
 drivers/pci/remove.c |   32 ++++++++++++++++++++++++++++++++
 include/linux/pci.h  |    1 +
 2 files changed, 33 insertions(+), 0 deletions(-)

diff --git a/drivers/pci/remove.c b/drivers/pci/remove.c
index d4fdf7b..9ffc071 100644
--- a/drivers/pci/remove.c
+++ b/drivers/pci/remove.c
@@ -170,6 +170,38 @@ void pci_stop_bus_device(struct pci_dev *dev)
 	pci_stop_dev(dev);
 }
 
+static void pci_stop_host_bridge(struct pci_host_bridge *bridge)
+{
+	device_unregister(&bridge->dev);
+}
+/*
+ * it will support pci root bus too, in that case we need
+ *  stop and remove host bridge
+ */
+void pci_stop_and_remove_bus(struct pci_bus *bus)
+{
+	struct pci_host_bridge *host_bridge = NULL;
+	struct pci_dev *pci_bridge = NULL;
+
+	pci_stop_bus_devices(bus);
+
+	if (pci_is_root_bus(bus)) {
+		host_bridge = to_pci_host_bridge(bus->bridge);
+		pci_stop_host_bridge(host_bridge);
+	} else
+		pci_bridge = bus->self;
+
+	__pci_remove_bus_devices(bus);
+
+	pci_remove_bus(bus);
+
+	if (host_bridge)
+		host_bridge->bus = NULL;
+
+	if (pci_bridge)
+		pci_bridge->subordinate = NULL;
+}
+
 EXPORT_SYMBOL(pci_stop_and_remove_bus_device);
 EXPORT_SYMBOL(pci_stop_and_remove_behind_bridge);
 EXPORT_SYMBOL_GPL(pci_stop_bus_device);
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 860091f..8874b90 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -702,6 +702,7 @@ extern void pci_dev_put(struct pci_dev *dev);
 extern void pci_remove_bus(struct pci_bus *b);
 extern void __pci_remove_bus_device(struct pci_dev *dev);
 extern void pci_stop_and_remove_bus_device(struct pci_dev *dev);
+void pci_stop_and_remove_bus(struct pci_bus *bus);
 extern void pci_stop_bus_device(struct pci_dev *dev);
 void pci_setup_cardbus(struct pci_bus *bus);
 extern void pci_sort_breadthfirst(void);
-- 
1.7.7

Re: [PATCH v2 19/37] PCI: Add pci_stop_and_remove_bus()

[Jiang Liu]

On 03/10/2012 03:00 PM, Yinghai Lu wrote:
> It supports both pci root bus and pci bus under pci bridge.
> 
> -v2: clear pci_bridge's subordinate.
> 
> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> ---
>  drivers/pci/remove.c |   32 ++++++++++++++++++++++++++++++++
>  include/linux/pci.h  |    1 +
>  2 files changed, 33 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/pci/remove.c b/drivers/pci/remove.c
> index d4fdf7b..9ffc071 100644
> --- a/drivers/pci/remove.c
> +++ b/drivers/pci/remove.c
> @@ -170,6 +170,38 @@ void pci_stop_bus_device(struct pci_dev *dev)
>  	pci_stop_dev(dev);
>  }
>  
> +static void pci_stop_host_bridge(struct pci_host_bridge *bridge)
> +{
> +	device_unregister(&bridge->dev);
> +}
> +/*
> + * it will support pci root bus too, in that case we need
> + *  stop and remove host bridge
> + */
> +void pci_stop_and_remove_bus(struct pci_bus *bus)
> +{
> +	struct pci_host_bridge *host_bridge = NULL;
> +	struct pci_dev *pci_bridge = NULL;
> +
> +	pci_stop_bus_devices(bus);
> +
> +	if (pci_is_root_bus(bus)) {
> +		host_bridge = to_pci_host_bridge(bus->bridge);
> +		pci_stop_host_bridge(host_bridge);
> +	} else
> +		pci_bridge = bus->self;
> +
> +	__pci_remove_bus_devices(bus);
> +
> +	pci_remove_bus(bus);
> +
It seems that the bus and host_bridge structures may have already been freed here,
so should not access host_bridge structure any more.
> +	if (host_bridge)
> +		host_bridge->bus = NULL;
> +
> +	if (pci_bridge)
> +		pci_bridge->subordinate = NULL;
> +}
> +
>  EXPORT_SYMBOL(pci_stop_and_remove_bus_device);
>  EXPORT_SYMBOL(pci_stop_and_remove_behind_bridge);
>  EXPORT_SYMBOL_GPL(pci_stop_bus_device);
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 860091f..8874b90 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -702,6 +702,7 @@ extern void pci_dev_put(struct pci_dev *dev);
>  extern void pci_remove_bus(struct pci_bus *b);
>  extern void __pci_remove_bus_device(struct pci_dev *dev);
>  extern void pci_stop_and_remove_bus_device(struct pci_dev *dev);
> +void pci_stop_and_remove_bus(struct pci_bus *bus);
>  extern void pci_stop_bus_device(struct pci_dev *dev);
>  void pci_setup_cardbus(struct pci_bus *bus);
>  extern void pci_sort_breadthfirst(void);

Re: [PATCH v2 19/37] PCI: Add pci_stop_and_remove_bus()

[Yinghai Lu]

>> +void pci_stop_and_remove_bus(struct pci_bus *bus)
>> +{
>> +     struct pci_host_bridge *host_bridge = NULL;
>> +     struct pci_dev *pci_bridge = NULL;
>> +
>> +     pci_stop_bus_devices(bus);
>> +
>> +     if (pci_is_root_bus(bus)) {
>> +             host_bridge = to_pci_host_bridge(bus->bridge);
>> +             pci_stop_host_bridge(host_bridge);
>> +     } else
>> +             pci_bridge = bus->self;
>> +
>> +     __pci_remove_bus_devices(bus);
>> +
>> +     pci_remove_bus(bus);
>> +
> It seems that the bus and host_bridge structures may have already been freed here,
> so should not access host_bridge structure any more.
>> +     if (host_bridge)
>> +             host_bridge->bus = NULL;
>> +

not really.

Yinghai

[PATCH v2 20/37] PCI: Add pci bus removal through /sys/.../pci_bus/.../remove

[Yinghai Lu]

it supports both pci root bus and pci bus under pci bridge.

-v2: Change to three returns way in dev_bus_remove_store.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
Cc: Randy Dunlap <rdunlap@xenotime.net>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: linux-doc@vger.kernel.org
---
 Documentation/ABI/testing/sysfs-bus-pci |    8 ++++++++
 drivers/pci/pci-sysfs.c                 |   30 ++++++++++++++++++++++++++++++
 2 files changed, 38 insertions(+), 0 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-bus-pci b/Documentation/ABI/testing/sysfs-bus-pci
index 95f0f37..22392de 100644
--- a/Documentation/ABI/testing/sysfs-bus-pci
+++ b/Documentation/ABI/testing/sysfs-bus-pci
@@ -92,6 +92,14 @@ Description:
 		hot-remove the PCI device and any of its children.
 		Depends on CONFIG_HOTPLUG.
 
+What:		/sys/bus/pci/devices/.../pci_bus/.../remove
+Date:		March 2012
+Contact:	Linux PCI developers <linux-pci@vger.kernel.org>
+Description:
+		Writing a non-zero value to this attribute will
+		hot-remove the PCI bus and any of its children.
+		Depends on CONFIG_HOTPLUG.
+
 What:		/sys/bus/pci/devices/.../pci_bus/.../rescan
 Date:		May 2011
 Contact:	Linux PCI developers <linux-pci@vger.kernel.org>
diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index 4697afe..4d122cb 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -388,6 +388,35 @@ remove_store(struct device *dev, struct device_attribute *dummy,
 	return count;
 }
 
+static void bus_remove_callback(struct device *dev)
+{
+	struct pci_bus *bus = to_pci_bus(dev);
+
+	mutex_lock(&pci_remove_rescan_mutex);
+	pci_stop_and_remove_bus(bus);
+	mutex_unlock(&pci_remove_rescan_mutex);
+}
+static ssize_t
+dev_bus_remove_store(struct device *dev, struct device_attribute *attr,
+		 const char *buf, size_t count)
+{
+	int err;
+	unsigned long val;
+
+	if (kstrtoul(buf, 0, &val) < 0)
+		return -EINVAL;
+
+	if (!val)
+		return count;
+
+	err = device_schedule_callback(dev, bus_remove_callback);
+
+	if (err)
+		return err;
+
+	return count;
+}
+
 static void bus_rescan_callback(struct device *dev)
 {
 	struct pci_bus *bus = to_pci_bus(dev);
@@ -447,6 +476,7 @@ struct device_attribute pci_dev_attrs[] = {
 struct device_attribute pcibus_dev_attrs[] = {
 #ifdef CONFIG_HOTPLUG
 	__ATTR(rescan, (S_IWUSR|S_IWGRP), NULL, dev_bus_rescan_store),
+	__ATTR(remove, (S_IWUSR|S_IWGRP), NULL, dev_bus_remove_store),
 #endif
 	__ATTR(cpuaffinity, S_IRUGO, pci_bus_show_cpumaskaffinity, NULL),
 	__ATTR(cpulistaffinity, S_IRUGO, pci_bus_show_cpulistaffinity, NULL),
-- 
1.7.7

Re: [PATCH v2 20/37] PCI: Add pci bus removal through /sys/.../pci_bus/.../remove

[Bjorn Helgaas]

On Sat, Mar 10, 2012 at 12:00 AM, Yinghai Lu <yinghai@kernel.org> wrote:
> it supports both pci root bus and pci bus under pci bridge.
>
> -v2: Change to three returns way in dev_bus_remove_store.
>
> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> Cc: Randy Dunlap <rdunlap@xenotime.net>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: linux-doc@vger.kernel.org
> ---
>  Documentation/ABI/testing/sysfs-bus-pci |    8 ++++++++
>  drivers/pci/pci-sysfs.c                 |   30 ++++++++++++++++++++++++++++++
>  2 files changed, 38 insertions(+), 0 deletions(-)
>
> diff --git a/Documentation/ABI/testing/sysfs-bus-pci b/Documentation/ABI/testing/sysfs-bus-pci
> index 95f0f37..22392de 100644
> --- a/Documentation/ABI/testing/sysfs-bus-pci
> +++ b/Documentation/ABI/testing/sysfs-bus-pci
> @@ -92,6 +92,14 @@ Description:
>                hot-remove the PCI device and any of its children.
>                Depends on CONFIG_HOTPLUG.
>
> +What:          /sys/bus/pci/devices/.../pci_bus/.../remove
> +Date:          March 2012
> +Contact:       Linux PCI developers <linux-pci@vger.kernel.org>
> +Description:
> +               Writing a non-zero value to this attribute will
> +               hot-remove the PCI bus and any of its children.
> +               Depends on CONFIG_HOTPLUG.

I don't think this interface makes sense.  This stops every device on
the bus, removes the bus, unregisters the host bridge device if we're
removing a root bus.

I think we should just have a "remove *bridge*" interface.  It makes
no sense to me to remove a bus but leave the bridge, and it makes the
code quite complicated.

The point of hotplug is to add and remove devices.  We'll get
interrupts saying "please remove this device" or "please rescan the
bus behind this bridge because something might have changed."

If we're removing a host bridge, the platform, e.g., ACPI, will tell
us about the bridge that is going away.  The notification is attached
to the *bridge*, not the bus it leads to.

The expected mechanism on x86 is an SCI from the platform, but if we
need a sysfs way to start host bridge removal, I think it should be
something like this:

    echo 1 > /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/remove

That corresponds to the way ACPI works, and it corresponds to the way
we manually remove PCI devices.

>  What:          /sys/bus/pci/devices/.../pci_bus/.../rescan
>  Date:          May 2011
>  Contact:       Linux PCI developers <linux-pci@vger.kernel.org>
> diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
> index 4697afe..4d122cb 100644
> --- a/drivers/pci/pci-sysfs.c
> +++ b/drivers/pci/pci-sysfs.c
> @@ -388,6 +388,35 @@ remove_store(struct device *dev, struct device_attribute *dummy,
>        return count;
>  }
>
> +static void bus_remove_callback(struct device *dev)
> +{
> +       struct pci_bus *bus = to_pci_bus(dev);
> +
> +       mutex_lock(&pci_remove_rescan_mutex);
> +       pci_stop_and_remove_bus(bus);
> +       mutex_unlock(&pci_remove_rescan_mutex);
> +}
> +static ssize_t
> +dev_bus_remove_store(struct device *dev, struct device_attribute *attr,
> +                const char *buf, size_t count)
> +{
> +       int err;
> +       unsigned long val;
> +
> +       if (kstrtoul(buf, 0, &val) < 0)
> +               return -EINVAL;
> +
> +       if (!val)
> +               return count;
> +
> +       err = device_schedule_callback(dev, bus_remove_callback);
> +
> +       if (err)
> +               return err;
> +
> +       return count;
> +}
> +
>  static void bus_rescan_callback(struct device *dev)
>  {
>        struct pci_bus *bus = to_pci_bus(dev);
> @@ -447,6 +476,7 @@ struct device_attribute pci_dev_attrs[] = {
>  struct device_attribute pcibus_dev_attrs[] = {
>  #ifdef CONFIG_HOTPLUG
>        __ATTR(rescan, (S_IWUSR|S_IWGRP), NULL, dev_bus_rescan_store),
> +       __ATTR(remove, (S_IWUSR|S_IWGRP), NULL, dev_bus_remove_store),
>  #endif
>        __ATTR(cpuaffinity, S_IRUGO, pci_bus_show_cpumaskaffinity, NULL),
>        __ATTR(cpulistaffinity, S_IRUGO, pci_bus_show_cpulistaffinity, NULL),
> --
> 1.7.7
>

Re: [PATCH v2 20/37] PCI: Add pci bus removal through /sys/.../pci_bus/.../remove

[Yinghai Lu]

On Mon, Mar 12, 2012 at 8:10 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> On Sat, Mar 10, 2012 at 12:00 AM, Yinghai Lu <yinghai@kernel.org> wrote:
>> it supports both pci root bus and pci bus under pci bridge.
>>
>> -v2: Change to three returns way in dev_bus_remove_store.
>>
>> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
>> Cc: Randy Dunlap <rdunlap@xenotime.net>
>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>> Cc: linux-doc@vger.kernel.org
>> ---
>>  Documentation/ABI/testing/sysfs-bus-pci |    8 ++++++++
>>  drivers/pci/pci-sysfs.c                 |   30 ++++++++++++++++++++++++++++++
>>  2 files changed, 38 insertions(+), 0 deletions(-)
>>
>> diff --git a/Documentation/ABI/testing/sysfs-bus-pci b/Documentation/ABI/testing/sysfs-bus-pci
>> index 95f0f37..22392de 100644
>> --- a/Documentation/ABI/testing/sysfs-bus-pci
>> +++ b/Documentation/ABI/testing/sysfs-bus-pci
>> @@ -92,6 +92,14 @@ Description:
>>                hot-remove the PCI device and any of its children.
>>                Depends on CONFIG_HOTPLUG.
>>
>> +What:          /sys/bus/pci/devices/.../pci_bus/.../remove
>> +Date:          March 2012
>> +Contact:       Linux PCI developers <linux-pci@vger.kernel.org>
>> +Description:
>> +               Writing a non-zero value to this attribute will
>> +               hot-remove the PCI bus and any of its children.
>> +               Depends on CONFIG_HOTPLUG.
>
> I don't think this interface makes sense.  This stops every device on
> the bus, removes the bus, unregisters the host bridge device if we're
> removing a root bus.

it is some kind of handy. otherwise if there is several devices on the
bus, will need to
remove the devices one by one.

>
> I think we should just have a "remove *bridge*" interface.  It makes
> no sense to me to remove a bus but leave the bridge, and it makes the
> code quite complicated.

I want to remove all devices under the bridge and then rescan the
bridge. then if the bridge get removed.
then we have to rescan bridge's parent bus.

>
> The point of hotplug is to add and remove devices.  We'll get
> interrupts saying "please remove this device" or "please rescan the
> bus behind this bridge because something might have changed."
>
> If we're removing a host bridge, the platform, e.g., ACPI, will tell
> us about the bridge that is going away.  The notification is attached
> to the *bridge*, not the bus it leads to.
>
> The expected mechanism on x86 is an SCI from the platform, but if we
> need a sysfs way to start host bridge removal, I think it should be
> something like this:
>
>    echo 1 > /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/remove

for that,  let just use
echo "\_SB.PCI0 3" > /proc/acpi/sci/notify

Yinghai

Re: [PATCH v2 20/37] PCI: Add pci bus removal through /sys/.../pci_bus/.../remove

[Bjorn Helgaas]

On Mon, Mar 12, 2012 at 11:54 PM, Yinghai Lu <yinghai@kernel.org> wrote:
> On Mon, Mar 12, 2012 at 8:10 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
>> On Sat, Mar 10, 2012 at 12:00 AM, Yinghai Lu <yinghai@kernel.org> wrote:
>>> it supports both pci root bus and pci bus under pci bridge.
>>>
>>> -v2: Change to three returns way in dev_bus_remove_store.
>>>
>>> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
>>> Cc: Randy Dunlap <rdunlap@xenotime.net>
>>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>>> Cc: linux-doc@vger.kernel.org
>>> ---
>>>  Documentation/ABI/testing/sysfs-bus-pci |    8 ++++++++
>>>  drivers/pci/pci-sysfs.c                 |   30 ++++++++++++++++++++++++++++++
>>>  2 files changed, 38 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/Documentation/ABI/testing/sysfs-bus-pci b/Documentation/ABI/testing/sysfs-bus-pci
>>> index 95f0f37..22392de 100644
>>> --- a/Documentation/ABI/testing/sysfs-bus-pci
>>> +++ b/Documentation/ABI/testing/sysfs-bus-pci
>>> @@ -92,6 +92,14 @@ Description:
>>>                hot-remove the PCI device and any of its children.
>>>                Depends on CONFIG_HOTPLUG.
>>>
>>> +What:          /sys/bus/pci/devices/.../pci_bus/.../remove
>>> +Date:          March 2012
>>> +Contact:       Linux PCI developers <linux-pci@vger.kernel.org>
>>> +Description:
>>> +               Writing a non-zero value to this attribute will
>>> +               hot-remove the PCI bus and any of its children.
>>> +               Depends on CONFIG_HOTPLUG.
>>
>> I don't think this interface makes sense.  This stops every device on
>> the bus, removes the bus, unregisters the host bridge device if we're
>> removing a root bus.
>
> it is some kind of handy. otherwise if there is several devices on the
> bus, will need to
> remove the devices one by one.

It might be handy, but that's not enough.  You're doing a sysfs
operation in the PCI namespace.  It should not have effects outside
that namespace.  Host bridges are by definition outside the PCI
namespace.

>> I think we should just have a "remove *bridge*" interface.  It makes
>> no sense to me to remove a bus but leave the bridge, and it makes the
>> code quite complicated.
>
> I want to remove all devices under the bridge and then rescan the
> bridge. then if the bridge get removed.
> then we have to rescan bridge's parent bus.
>
>>
>> The point of hotplug is to add and remove devices.  We'll get
>> interrupts saying "please remove this device" or "please rescan the
>> bus behind this bridge because something might have changed."
>>
>> If we're removing a host bridge, the platform, e.g., ACPI, will tell
>> us about the bridge that is going away.  The notification is attached
>> to the *bridge*, not the bus it leads to.
>>
>> The expected mechanism on x86 is an SCI from the platform, but if we
>> need a sysfs way to start host bridge removal, I think it should be
>> something like this:
>>
>>    echo 1 > /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/remove
>
> for that,  let just use
> echo "\_SB.PCI0 3" > /proc/acpi/sci/notify

That's a totally broken user interface.  There's no way for the user
to look at the sysfs tree and figure out how to do that.  The ACPI
namespace names like "PCI0" are platform-dependent internal things and
should not be part of any user interface.

I don't object to /proc/acpi/sci/notify.  That looks like a useful way
to inject notification events that would normally come from the
hardware/BIOS.  But it's a development tool, not something that can be
part of the normal user interface.

Bjorn

[PATCH v2 21/37] PCI, ACPI: Make acpi_pci_root_remove remove pci root bus too

[Yinghai Lu]

It will call new added pci_stop_and_remove_bus() to stop/remove pci root bus.

Also checking if that pci_root_bus get removed already in bus remove in /sys

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
Cc: Len Brown <lenb@kernel.org>
Cc: linux-acpi@vger.kernel.org
---
 drivers/acpi/pci_root.c |   14 ++++++++++++++
 1 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
index 7aff631..b38e347 100644
--- a/drivers/acpi/pci_root.c
+++ b/drivers/acpi/pci_root.c
@@ -643,10 +643,24 @@ static int acpi_pci_root_remove(struct acpi_device *device, int type)
 {
 	struct acpi_pci_root *root = acpi_driver_data(device);
 
+	/* that root bus could be removed already */
+	if (!pci_find_bus(root->segment, root->secondary.start)) {
+		dev_printk(KERN_DEBUG, &device->dev,
+		  "freeing acpi_pci_root, but pci root bus was removed before");
+		goto out;
+	}
+
 	device_set_run_wake(root->bus->bridge, false);
 	pci_acpi_remove_bus_pm_notifier(device);
 
+	dev_printk(KERN_DEBUG, &device->dev,
+		"freeing acpi_pci_root, will remove pci root bus at first");
+	pci_stop_and_remove_bus(root->bus);
+
+out:
+	list_del(&root->node);
 	kfree(root);
+
 	return 0;
 }
 
-- 
1.7.7

Re: [PATCH v2 21/37] PCI, ACPI: Make acpi_pci_root_remove remove pci root bus too

[Bjorn Helgaas]

On Sat, Mar 10, 2012 at 12:00 AM, Yinghai Lu <yinghai@kernel.org> wrote:
> It will call new added pci_stop_and_remove_bus() to stop/remove pci root bus.
>
> Also checking if that pci_root_bus get removed already in bus remove in /sys
>
> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> Cc: Len Brown <lenb@kernel.org>
> Cc: linux-acpi@vger.kernel.org
> ---
>  drivers/acpi/pci_root.c |   14 ++++++++++++++
>  1 files changed, 14 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
> index 7aff631..b38e347 100644
> --- a/drivers/acpi/pci_root.c
> +++ b/drivers/acpi/pci_root.c
> @@ -643,10 +643,24 @@ static int acpi_pci_root_remove(struct acpi_device *device, int type)
>  {
>        struct acpi_pci_root *root = acpi_driver_data(device);
>
> +       /* that root bus could be removed already */
> +       if (!pci_find_bus(root->segment, root->secondary.start)) {
> +               dev_printk(KERN_DEBUG, &device->dev,
> +                 "freeing acpi_pci_root, but pci root bus was removed before");
> +               goto out;

I don't think we should handle the root bus separately from the host
bridge, as I mentioned before.  If we handle them together, checks
like this won't be needed.

> +       }
> +
>        device_set_run_wake(root->bus->bridge, false);
>        pci_acpi_remove_bus_pm_notifier(device);
>
> +       dev_printk(KERN_DEBUG, &device->dev,
> +               "freeing acpi_pci_root, will remove pci root bus at first");
> +       pci_stop_and_remove_bus(root->bus);
> +
> +out:
> +       list_del(&root->node);
>        kfree(root);
> +
>        return 0;
>  }
>
> --
> 1.7.7
>

[PATCH v2 22/37] PCI, acpiphp: remove hot-add support of pci host bridge

[Yinghai Lu]

It causes confusing.

We may only need acpi hp for pci host bridge.

Split host bridge hot-add support to another file, and keep acpiphp simple.

Also remove not used res_lock in the struct.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
 drivers/pci/hotplug/acpiphp.h      |    9 +---
 drivers/pci/hotplug/acpiphp_glue.c |  104 +++++++-----------------------------
 2 files changed, 21 insertions(+), 92 deletions(-)

diff --git a/drivers/pci/hotplug/acpiphp.h b/drivers/pci/hotplug/acpiphp.h
index 7722108..1a62e7b 100644
--- a/drivers/pci/hotplug/acpiphp.h
+++ b/drivers/pci/hotplug/acpiphp.h
@@ -79,18 +79,15 @@ struct acpiphp_bridge {
 	/* Ejectable PCI-to-PCI bridge (PCI bridge and PCI function) */
 	struct acpiphp_func *func;
 
-	int type;
 	int nr_slots;
 
 	u32 flags;
 
-	/* This bus (host bridge) or Secondary bus (PCI-to-PCI bridge) */
+	/* Secondary bus (PCI-to-PCI bridge) */
 	struct pci_bus *pci_bus;
 
 	/* PCI-to-PCI bridge device */
 	struct pci_dev *pci_dev;
-
-	spinlock_t res_lock;
 };
 
 
@@ -148,10 +145,6 @@ struct acpiphp_attention_info
 /* PCI bus bridge HID */
 #define ACPI_PCI_HOST_HID		"PNP0A03"
 
-/* PCI BRIDGE type */
-#define BRIDGE_TYPE_HOST		0
-#define BRIDGE_TYPE_P2P			1
-
 /* ACPI _STA method value (ignore bit 4; battery present) */
 #define ACPI_STA_PRESENT		(0x00000001)
 #define ACPI_STA_ENABLED		(0x00000002)
diff --git a/drivers/pci/hotplug/acpiphp_glue.c b/drivers/pci/hotplug/acpiphp_glue.c
index 10b2122..1ca82ae 100644
--- a/drivers/pci/hotplug/acpiphp_glue.c
+++ b/drivers/pci/hotplug/acpiphp_glue.c
@@ -283,7 +283,7 @@ static int detect_ejectable_slots(acpi_handle handle)
 	return found;
 }
 
-/* initialize miscellaneous stuff for both root and PCI-to-PCI bridge */
+/* initialize miscellaneous stuff for PCI-to-PCI bridge */
 static void init_bridge_misc(struct acpiphp_bridge *bridge)
 {
 	acpi_status status;
@@ -300,25 +300,21 @@ static void init_bridge_misc(struct acpiphp_bridge *bridge)
 	}
 
 	/* install notify handler */
-	if (bridge->type != BRIDGE_TYPE_HOST) {
-		if ((bridge->flags & BRIDGE_HAS_EJ0) && bridge->func) {
-			status = acpi_remove_notify_handler(bridge->func->handle,
-						ACPI_SYSTEM_NOTIFY,
-						handle_hotplug_event_func);
-			if (ACPI_FAILURE(status))
-				err("failed to remove notify handler\n");
-		}
-		status = acpi_install_notify_handler(bridge->handle,
-					     ACPI_SYSTEM_NOTIFY,
-					     handle_hotplug_event_bridge,
-					     bridge);
-
-		if (ACPI_FAILURE(status)) {
-			err("failed to register interrupt notify handler\n");
-		}
+	if ((bridge->flags & BRIDGE_HAS_EJ0) && bridge->func) {
+		status = acpi_remove_notify_handler(bridge->func->handle,
+					ACPI_SYSTEM_NOTIFY,
+					handle_hotplug_event_func);
+		if (ACPI_FAILURE(status))
+			err("failed to remove notify handler\n");
 	}
-}
+	status = acpi_install_notify_handler(bridge->handle,
+				     ACPI_SYSTEM_NOTIFY,
+				     handle_hotplug_event_bridge,
+				     bridge);
 
+	if (ACPI_FAILURE(status))
+		err("failed to register interrupt notify handler\n");
+}
 
 /* find acpiphp_func from acpiphp_bridge */
 static struct acpiphp_func *acpiphp_bridge_handle_to_function(acpi_handle handle)
@@ -375,28 +371,6 @@ static inline void config_p2p_bridge_flags(struct acpiphp_bridge *bridge)
 	}
 }
 
-
-/* allocate and initialize host bridge data structure */
-static void add_host_bridge(acpi_handle *handle)
-{
-	struct acpiphp_bridge *bridge;
-	struct acpi_pci_root *root = acpi_pci_find_root(handle);
-
-	bridge = kzalloc(sizeof(struct acpiphp_bridge), GFP_KERNEL);
-	if (bridge == NULL)
-		return;
-
-	bridge->type = BRIDGE_TYPE_HOST;
-	bridge->handle = handle;
-
-	bridge->pci_bus = root->bus;
-
-	spin_lock_init(&bridge->res_lock);
-
-	init_bridge_misc(bridge);
-}
-
-
 /* allocate and initialize PCI-to-PCI bridge data structure */
 static void add_p2p_bridge(acpi_handle *handle)
 {
@@ -408,7 +382,6 @@ static void add_p2p_bridge(acpi_handle *handle)
 		return;
 	}
 
-	bridge->type = BRIDGE_TYPE_P2P;
 	bridge->handle = handle;
 	config_p2p_bridge_flags(bridge);
 
@@ -425,7 +398,6 @@ static void add_p2p_bridge(acpi_handle *handle)
 	 * (which we access during module unload).
 	 */
 	get_device(&bridge->pci_bus->dev);
-	spin_lock_init(&bridge->res_lock);
 
 	init_bridge_misc(bridge);
 	return;
@@ -485,12 +457,6 @@ static int add_bridge(acpi_handle handle)
 			return 0;
 	}
 
-	/* check if this bridge has ejectable slots */
-	if (detect_ejectable_slots(handle) > 0) {
-		dbg("found PCI host-bus bridge with hot-pluggable slots\n");
-		add_host_bridge(handle);
-	}
-
 	/* search P2P bridges under this host bridge */
 	status = acpi_walk_namespace(ACPI_TYPE_DEVICE, handle, (u32)1,
 				     find_p2p_bridge, NULL, NULL, NULL);
@@ -524,8 +490,7 @@ static void cleanup_bridge(struct acpiphp_bridge *bridge)
 	if (ACPI_FAILURE(status))
 		err("failed to remove notify handler\n");
 
-	if ((bridge->type != BRIDGE_TYPE_HOST) &&
-	    ((bridge->flags & BRIDGE_HAS_EJ0) && bridge->func)) {
+	if ((bridge->flags & BRIDGE_HAS_EJ0) && bridge->func) {
 		status = acpi_install_notify_handler(bridge->func->handle,
 						ACPI_SYSTEM_NOTIFY,
 						handle_hotplug_event_func,
@@ -1078,15 +1043,10 @@ static void acpiphp_sanitize_bus(struct pci_bus *bus)
 static int acpiphp_configure_bridge (acpi_handle handle)
 {
 	struct pci_bus *bus;
+	struct pci_dev *pdev = acpi_get_pci_dev(handle);
 
-	if (acpi_is_root_bridge(handle)) {
-		struct acpi_pci_root *root = acpi_pci_find_root(handle);
-		bus = root->bus;
-	} else {
-		struct pci_dev *pdev = acpi_get_pci_dev(handle);
-		bus = pdev->subordinate;
-		pci_dev_put(pdev);
-	}
+	bus = pdev->subordinate;
+	pci_dev_put(pdev);
 
 	pci_bus_size_bridges(bus);
 	pci_bus_assign_resources(bus);
@@ -1249,8 +1209,7 @@ static void _handle_hotplug_event_bridge(struct work_struct *work)
 	case ACPI_NOTIFY_EJECT_REQUEST:
 		/* request device eject */
 		dbg("%s: Device eject notify on %s\n", __func__, objname);
-		if ((bridge->type != BRIDGE_TYPE_HOST) &&
-		    (bridge->flags & BRIDGE_HAS_EJ0)) {
+		if (bridge->flags & BRIDGE_HAS_EJ0) {
 			struct acpiphp_slot *slot;
 			slot = bridge->func->slot;
 			if (!acpiphp_disable_slot(slot))
@@ -1381,21 +1340,6 @@ static void handle_hotplug_event_func(acpi_handle handle, u32 type,
 			      _handle_hotplug_event_func);
 }
 
-static acpi_status
-find_root_bridges(acpi_handle handle, u32 lvl, void *context, void **rv)
-{
-	int *count = (int *)context;
-
-	if (!acpi_is_root_bridge(handle))
-		return AE_OK;
-
-	(*count)++;
-	acpi_install_notify_handler(handle, ACPI_SYSTEM_NOTIFY,
-				    handle_hotplug_event_bridge, NULL);
-
-	return AE_OK ;
-}
-
 static struct acpi_pci_driver acpi_pci_hp_driver = {
 	.add =		add_bridge,
 	.remove =	remove_bridge,
@@ -1406,15 +1350,7 @@ static struct acpi_pci_driver acpi_pci_hp_driver = {
  */
 int __init acpiphp_glue_init(void)
 {
-	int num = 0;
-
-	acpi_walk_namespace(ACPI_TYPE_DEVICE, ACPI_ROOT_OBJECT,
-			ACPI_UINT32_MAX, find_root_bridges, NULL, &num, NULL);
-
-	if (num <= 0)
-		return -1;
-	else
-		acpi_pci_register_driver(&acpi_pci_hp_driver);
+	acpi_pci_register_driver(&acpi_pci_hp_driver);
 
 	return 0;
 }
-- 
1.7.7

Re: [PATCH v2 22/37] PCI, acpiphp: remove hot-add support of pci host bridge

[Bjorn Helgaas]

On Sat, Mar 10, 2012 at 12:00 AM, Yinghai Lu <yinghai@kernel.org> wrote:
> It causes confusing.

"causes confusion."

> We may only need acpi hp for pci host bridge.
>
> Split host bridge hot-add support to another file, and keep acpiphp simple.

I think it's a good thing to remove host bridge hotplug from acpiphp;
thanks for doing that.

However, I think you removed it here, but didn't add it elsewhere
until a different patch.  That means we can't bisect across this.  It
would be better to do the entire move in a single patch.

> Also remove not used res_lock in the struct.
>
> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> ---
>  drivers/pci/hotplug/acpiphp.h      |    9 +---
>  drivers/pci/hotplug/acpiphp_glue.c |  104 +++++++-----------------------------
>  2 files changed, 21 insertions(+), 92 deletions(-)
>
> diff --git a/drivers/pci/hotplug/acpiphp.h b/drivers/pci/hotplug/acpiphp.h
> index 7722108..1a62e7b 100644
> --- a/drivers/pci/hotplug/acpiphp.h
> +++ b/drivers/pci/hotplug/acpiphp.h
> @@ -79,18 +79,15 @@ struct acpiphp_bridge {
>        /* Ejectable PCI-to-PCI bridge (PCI bridge and PCI function) */
>        struct acpiphp_func *func;
>
> -       int type;
>        int nr_slots;
>
>        u32 flags;
>
> -       /* This bus (host bridge) or Secondary bus (PCI-to-PCI bridge) */
> +       /* Secondary bus (PCI-to-PCI bridge) */
>        struct pci_bus *pci_bus;
>
>        /* PCI-to-PCI bridge device */
>        struct pci_dev *pci_dev;
> -
> -       spinlock_t res_lock;
>  };
>
>
> @@ -148,10 +145,6 @@ struct acpiphp_attention_info
>  /* PCI bus bridge HID */
>  #define ACPI_PCI_HOST_HID              "PNP0A03"
>
> -/* PCI BRIDGE type */
> -#define BRIDGE_TYPE_HOST               0
> -#define BRIDGE_TYPE_P2P                        1
> -
>  /* ACPI _STA method value (ignore bit 4; battery present) */
>  #define ACPI_STA_PRESENT               (0x00000001)
>  #define ACPI_STA_ENABLED               (0x00000002)
> diff --git a/drivers/pci/hotplug/acpiphp_glue.c b/drivers/pci/hotplug/acpiphp_glue.c
> index 10b2122..1ca82ae 100644
> --- a/drivers/pci/hotplug/acpiphp_glue.c
> +++ b/drivers/pci/hotplug/acpiphp_glue.c
> @@ -283,7 +283,7 @@ static int detect_ejectable_slots(acpi_handle handle)
>        return found;
>  }
>
> -/* initialize miscellaneous stuff for both root and PCI-to-PCI bridge */
> +/* initialize miscellaneous stuff for PCI-to-PCI bridge */
>  static void init_bridge_misc(struct acpiphp_bridge *bridge)
>  {
>        acpi_status status;
> @@ -300,25 +300,21 @@ static void init_bridge_misc(struct acpiphp_bridge *bridge)
>        }
>
>        /* install notify handler */
> -       if (bridge->type != BRIDGE_TYPE_HOST) {
> -               if ((bridge->flags & BRIDGE_HAS_EJ0) && bridge->func) {
> -                       status = acpi_remove_notify_handler(bridge->func->handle,
> -                                               ACPI_SYSTEM_NOTIFY,
> -                                               handle_hotplug_event_func);
> -                       if (ACPI_FAILURE(status))
> -                               err("failed to remove notify handler\n");
> -               }
> -               status = acpi_install_notify_handler(bridge->handle,
> -                                            ACPI_SYSTEM_NOTIFY,
> -                                            handle_hotplug_event_bridge,
> -                                            bridge);
> -
> -               if (ACPI_FAILURE(status)) {
> -                       err("failed to register interrupt notify handler\n");
> -               }
> +       if ((bridge->flags & BRIDGE_HAS_EJ0) && bridge->func) {
> +               status = acpi_remove_notify_handler(bridge->func->handle,
> +                                       ACPI_SYSTEM_NOTIFY,
> +                                       handle_hotplug_event_func);
> +               if (ACPI_FAILURE(status))
> +                       err("failed to remove notify handler\n");
>        }
> -}
> +       status = acpi_install_notify_handler(bridge->handle,
> +                                    ACPI_SYSTEM_NOTIFY,
> +                                    handle_hotplug_event_bridge,
> +                                    bridge);
>
> +       if (ACPI_FAILURE(status))
> +               err("failed to register interrupt notify handler\n");
> +}
>
>  /* find acpiphp_func from acpiphp_bridge */
>  static struct acpiphp_func *acpiphp_bridge_handle_to_function(acpi_handle handle)
> @@ -375,28 +371,6 @@ static inline void config_p2p_bridge_flags(struct acpiphp_bridge *bridge)
>        }
>  }
>
> -
> -/* allocate and initialize host bridge data structure */
> -static void add_host_bridge(acpi_handle *handle)
> -{
> -       struct acpiphp_bridge *bridge;
> -       struct acpi_pci_root *root = acpi_pci_find_root(handle);
> -
> -       bridge = kzalloc(sizeof(struct acpiphp_bridge), GFP_KERNEL);
> -       if (bridge == NULL)
> -               return;
> -
> -       bridge->type = BRIDGE_TYPE_HOST;
> -       bridge->handle = handle;
> -
> -       bridge->pci_bus = root->bus;
> -
> -       spin_lock_init(&bridge->res_lock);
> -
> -       init_bridge_misc(bridge);
> -}
> -
> -
>  /* allocate and initialize PCI-to-PCI bridge data structure */
>  static void add_p2p_bridge(acpi_handle *handle)
>  {
> @@ -408,7 +382,6 @@ static void add_p2p_bridge(acpi_handle *handle)
>                return;
>        }
>
> -       bridge->type = BRIDGE_TYPE_P2P;
>        bridge->handle = handle;
>        config_p2p_bridge_flags(bridge);
>
> @@ -425,7 +398,6 @@ static void add_p2p_bridge(acpi_handle *handle)
>         * (which we access during module unload).
>         */
>        get_device(&bridge->pci_bus->dev);
> -       spin_lock_init(&bridge->res_lock);
>
>        init_bridge_misc(bridge);
>        return;
> @@ -485,12 +457,6 @@ static int add_bridge(acpi_handle handle)
>                        return 0;
>        }
>
> -       /* check if this bridge has ejectable slots */
> -       if (detect_ejectable_slots(handle) > 0) {
> -               dbg("found PCI host-bus bridge with hot-pluggable slots\n");
> -               add_host_bridge(handle);
> -       }
> -
>        /* search P2P bridges under this host bridge */
>        status = acpi_walk_namespace(ACPI_TYPE_DEVICE, handle, (u32)1,
>                                     find_p2p_bridge, NULL, NULL, NULL);
> @@ -524,8 +490,7 @@ static void cleanup_bridge(struct acpiphp_bridge *bridge)
>        if (ACPI_FAILURE(status))
>                err("failed to remove notify handler\n");
>
> -       if ((bridge->type != BRIDGE_TYPE_HOST) &&
> -           ((bridge->flags & BRIDGE_HAS_EJ0) && bridge->func)) {
> +       if ((bridge->flags & BRIDGE_HAS_EJ0) && bridge->func) {
>                status = acpi_install_notify_handler(bridge->func->handle,
>                                                ACPI_SYSTEM_NOTIFY,
>                                                handle_hotplug_event_func,
> @@ -1078,15 +1043,10 @@ static void acpiphp_sanitize_bus(struct pci_bus *bus)
>  static int acpiphp_configure_bridge (acpi_handle handle)
>  {
>        struct pci_bus *bus;
> +       struct pci_dev *pdev = acpi_get_pci_dev(handle);
>
> -       if (acpi_is_root_bridge(handle)) {
> -               struct acpi_pci_root *root = acpi_pci_find_root(handle);
> -               bus = root->bus;
> -       } else {
> -               struct pci_dev *pdev = acpi_get_pci_dev(handle);
> -               bus = pdev->subordinate;
> -               pci_dev_put(pdev);
> -       }
> +       bus = pdev->subordinate;
> +       pci_dev_put(pdev);
>
>        pci_bus_size_bridges(bus);
>        pci_bus_assign_resources(bus);
> @@ -1249,8 +1209,7 @@ static void _handle_hotplug_event_bridge(struct work_struct *work)
>        case ACPI_NOTIFY_EJECT_REQUEST:
>                /* request device eject */
>                dbg("%s: Device eject notify on %s\n", __func__, objname);
> -               if ((bridge->type != BRIDGE_TYPE_HOST) &&
> -                   (bridge->flags & BRIDGE_HAS_EJ0)) {
> +               if (bridge->flags & BRIDGE_HAS_EJ0) {
>                        struct acpiphp_slot *slot;
>                        slot = bridge->func->slot;
>                        if (!acpiphp_disable_slot(slot))
> @@ -1381,21 +1340,6 @@ static void handle_hotplug_event_func(acpi_handle handle, u32 type,
>                              _handle_hotplug_event_func);
>  }
>
> -static acpi_status
> -find_root_bridges(acpi_handle handle, u32 lvl, void *context, void **rv)
> -{
> -       int *count = (int *)context;
> -
> -       if (!acpi_is_root_bridge(handle))
> -               return AE_OK;
> -
> -       (*count)++;
> -       acpi_install_notify_handler(handle, ACPI_SYSTEM_NOTIFY,
> -                                   handle_hotplug_event_bridge, NULL);
> -
> -       return AE_OK ;
> -}
> -
>  static struct acpi_pci_driver acpi_pci_hp_driver = {
>        .add =          add_bridge,
>        .remove =       remove_bridge,
> @@ -1406,15 +1350,7 @@ static struct acpi_pci_driver acpi_pci_hp_driver = {
>  */
>  int __init acpiphp_glue_init(void)
>  {
> -       int num = 0;
> -
> -       acpi_walk_namespace(ACPI_TYPE_DEVICE, ACPI_ROOT_OBJECT,
> -                       ACPI_UINT32_MAX, find_root_bridges, NULL, &num, NULL);
> -
> -       if (num <= 0)
> -               return -1;
> -       else
> -               acpi_pci_register_driver(&acpi_pci_hp_driver);
> +       acpi_pci_register_driver(&acpi_pci_hp_driver);
>
>        return 0;
>  }
> --
> 1.7.7
>

Re: [PATCH v2 22/37] PCI, acpiphp: remove hot-add support of pci host bridge

[Yinghai Lu]

On Mon, Mar 12, 2012 at 8:14 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> On Sat, Mar 10, 2012 at 12:00 AM, Yinghai Lu <yinghai@kernel.org> wrote:
>> It causes confusing.
>
> "causes confusion."
>
>> We may only need acpi hp for pci host bridge.
>>
>> Split host bridge hot-add support to another file, and keep acpiphp simple.
>
> I think it's a good thing to remove host bridge hotplug from acpiphp;
> thanks for doing that.
>
> However, I think you removed it here, but didn't add it elsewhere
> until a different patch.  That means we can't bisect across this.  It
> would be better to do the entire move in a single patch.

do you mean merge patch 22 and 23 together?

Re: [PATCH v2 22/37] PCI, acpiphp: remove hot-add support of pci host bridge

[Bjorn Helgaas]

On Mon, Mar 12, 2012 at 11:58 PM, Yinghai Lu <yinghai@kernel.org> wrote:
> On Mon, Mar 12, 2012 at 8:14 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
>> On Sat, Mar 10, 2012 at 12:00 AM, Yinghai Lu <yinghai@kernel.org> wrote:
>>> It causes confusing.
>>
>> "causes confusion."
>>
>>> We may only need acpi hp for pci host bridge.
>>>
>>> Split host bridge hot-add support to another file, and keep acpiphp simple.
>>
>> I think it's a good thing to remove host bridge hotplug from acpiphp;
>> thanks for doing that.
>>
>> However, I think you removed it here, but didn't add it elsewhere
>> until a different patch.  That means we can't bisect across this.  It
>> would be better to do the entire move in a single patch.
>
> do you mean merge patch 22 and 23 together?

Yes, I think so.  You should have a single "move," not separate
patches that "remove" then "add."

[PATCH v2 23/37] PCI, ACPI: Add pci_root_hp hot add hotplug notification

[Yinghai Lu]

It is from acpiphp_glue.c

How to use it?
Find out root bus number to acpi root name mapping from dmesg or /sys

  echo "\_SB.PCIB 0" > /proc/acpi/sci/notify
to add it back

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
Cc: Len Brown <lenb@kernel.org>
Cc: linux-acpi@vger.kernel.org
---
 drivers/acpi/Makefile      |    1 +
 drivers/acpi/pci_root_hp.c |  237 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 238 insertions(+), 0 deletions(-)
 create mode 100644 drivers/acpi/pci_root_hp.c

diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
index 1567028..bc6e53f 100644
--- a/drivers/acpi/Makefile
+++ b/drivers/acpi/Makefile
@@ -36,6 +36,7 @@ acpi-y				+= processor_core.o
 acpi-y				+= ec.o
 acpi-$(CONFIG_ACPI_DOCK)	+= dock.o
 acpi-y				+= pci_root.o pci_link.o pci_irq.o pci_bind.o
+acpi-$(CONFIG_HOTPLUG)		+= pci_root_hp.o
 acpi-y				+= power.o
 acpi-y				+= event.o
 acpi-y				+= sysfs.o
diff --git a/drivers/acpi/pci_root_hp.c b/drivers/acpi/pci_root_hp.c
new file mode 100644
index 0000000..dc11e81
--- /dev/null
+++ b/drivers/acpi/pci_root_hp.c
@@ -0,0 +1,237 @@
+/*
+ * Separated from drivers/pci/hotplug/acpiphp_glue.c
+ *	only support root bridge
+ */
+
+#include <linux/init.h>
+#include <linux/module.h>
+
+#include <linux/kernel.h>
+#include <linux/pci.h>
+#include <linux/mutex.h>
+#include <linux/slab.h>
+#include <linux/acpi.h>
+
+static LIST_HEAD(acpi_root_bridge_list);
+struct acpi_root_bridge {
+	struct list_head list;
+	acpi_handle handle;
+	u32 flags;
+};
+
+/* bridge flags */
+#define ROOT_BRIDGE_HAS_EJ0	(0x00000002)
+#define ROOT_BRIDGE_HAS_PS3	(0x00000080)
+
+#define ACPI_STA_FUNCTIONING	(0x00000008)
+
+static struct acpi_root_bridge *acpi_root_handle_to_bridge(acpi_handle handle)
+{
+	struct acpi_root_bridge *bridge;
+
+	list_for_each_entry(bridge, &acpi_root_bridge_list, list)
+		if (bridge->handle == handle)
+			return bridge;
+
+	return NULL;
+}
+
+/* allocate and initialize host bridge data structure */
+static void add_acpi_root_bridge(acpi_handle handle)
+{
+	struct acpi_root_bridge *bridge;
+	acpi_handle dummy_handle;
+	acpi_status status;
+
+	/* if the bridge doesn't have _STA, we assume it is always there */
+	status = acpi_get_handle(handle, "_STA", &dummy_handle);
+	if (ACPI_SUCCESS(status)) {
+		unsigned long long tmp;
+
+		status = acpi_evaluate_integer(handle, "_STA", NULL, &tmp);
+		if (ACPI_FAILURE(status)) {
+			printk(KERN_DEBUG "%s: _STA evaluation failure\n",
+						 __func__);
+			return;
+		}
+		if ((tmp & ACPI_STA_FUNCTIONING) == 0)
+			/* don't register this object */
+			return;
+	}
+
+	bridge = kzalloc(sizeof(struct acpi_root_bridge), GFP_KERNEL);
+	if (!bridge)
+		return;
+
+	bridge->handle = handle;
+
+	if (ACPI_SUCCESS(acpi_get_handle(handle, "_EJ0", &dummy_handle)))
+		bridge->flags |= ROOT_BRIDGE_HAS_EJ0;
+	if (ACPI_SUCCESS(acpi_get_handle(handle, "_PS3", &dummy_handle)))
+		bridge->flags |= ROOT_BRIDGE_HAS_PS3;
+
+	list_add(&bridge->list, &acpi_root_bridge_list);
+}
+
+struct acpi_root_hp_work {
+	struct work_struct work;
+	acpi_handle handle;
+	u32 type;
+	void *context;
+};
+
+static void alloc_acpi_root_hp_work(acpi_handle handle, u32 type,
+					void *context,
+					void (*func)(struct work_struct *work))
+{
+	struct acpi_root_hp_work *hp_work;
+	int ret;
+
+	hp_work = kmalloc(sizeof(*hp_work), GFP_KERNEL);
+	if (!hp_work)
+		return;
+
+	hp_work->handle = handle;
+	hp_work->type = type;
+	hp_work->context = context;
+
+	INIT_WORK(&hp_work->work, func);
+	ret = queue_work(kacpi_hotplug_wq, &hp_work->work);
+	if (!ret)
+		kfree(hp_work);
+}
+
+/* Program resources in newly inserted bridge */
+static void acpi_root_configure_bridge(acpi_handle handle)
+{
+	struct acpi_pci_root *root = acpi_pci_find_root(handle);
+
+	pci_assign_unassigned_bus_resources(root->bus);
+}
+
+static void handle_root_bridge_insertion(acpi_handle handle)
+{
+	struct acpi_device *device, *pdevice;
+	acpi_handle phandle;
+	int ret_val;
+
+	acpi_get_parent(handle, &phandle);
+	if (acpi_bus_get_device(phandle, &pdevice)) {
+		printk(KERN_DEBUG "no parent device, assuming NULL\n");
+		pdevice = NULL;
+	}
+	if (!acpi_bus_get_device(handle, &device)) {
+		/* check if  pci root_bus is removed */
+		struct acpi_pci_root *root = acpi_driver_data(device);
+		if (pci_find_bus(root->segment, root->secondary.start))
+			return;
+
+		printk(KERN_DEBUG "bus exists... trim\n");
+		/* this shouldn't be in here, so remove
+		 * the bus then re-add it...
+		 */
+		ret_val = acpi_bus_trim(device, 1);
+		printk(KERN_DEBUG "acpi_bus_trim return %x\n", ret_val);
+	}
+	if (acpi_bus_add(&device, pdevice, handle, ACPI_BUS_TYPE_DEVICE)) {
+		printk(KERN_ERR "cannot add bridge to acpi list\n");
+		return;
+	}
+	acpi_root_configure_bridge(handle);
+	if (acpi_bus_start(device))
+		printk(KERN_ERR "cannot start bridge\n");
+}
+
+static void _handle_hotplug_event_root(struct work_struct *work)
+{
+	struct acpi_root_bridge *bridge;
+	char objname[64];
+	struct acpi_buffer buffer = { .length = sizeof(objname),
+				      .pointer = objname };
+	struct acpi_root_hp_work *hp_work;
+	acpi_handle handle;
+	u32 type;
+
+	hp_work = container_of(work, struct acpi_root_hp_work, work);
+	handle = hp_work->handle;
+	type = hp_work->type;
+
+	bridge = acpi_root_handle_to_bridge(handle);
+
+	acpi_get_name(handle, ACPI_FULL_PATHNAME, &buffer);
+
+	switch (type) {
+	case ACPI_NOTIFY_BUS_CHECK:
+		/* bus enumerate */
+		printk(KERN_DEBUG "%s: Bus check notify on %s\n", __func__,
+				 objname);
+		if (!bridge) {
+			handle_root_bridge_insertion(handle);
+			add_acpi_root_bridge(handle);
+		}
+
+		break;
+
+	case ACPI_NOTIFY_DEVICE_CHECK:
+		/* device check */
+		printk(KERN_DEBUG "%s: Device check notify on %s\n", __func__,
+				 objname);
+		if (!bridge) {
+			handle_root_bridge_insertion(handle);
+			add_acpi_root_bridge(handle);
+		}
+		break;
+
+	default:
+		printk(KERN_WARNING "notify_handler: unknown event type 0x%x for %s\n",
+				 type, objname);
+		break;
+	}
+
+	kfree(hp_work); /* allocated in handle_hotplug_event_bridge */
+}
+
+static void handle_hotplug_event_root(acpi_handle handle, u32 type,
+					void *context)
+{
+	alloc_acpi_root_hp_work(handle, type, context,
+				_handle_hotplug_event_root);
+}
+
+static acpi_status __init
+find_root_bridges(acpi_handle handle, u32 lvl, void *context, void **rv)
+{
+	char objname[64];
+	struct acpi_buffer buffer = { .length = sizeof(objname),
+				      .pointer = objname };
+	int *count = (int *)context;
+
+	if (!acpi_is_root_bridge(handle))
+		return AE_OK;
+
+	(*count)++;
+
+	acpi_get_name(handle, ACPI_FULL_PATHNAME, &buffer);
+
+	acpi_install_notify_handler(handle, ACPI_SYSTEM_NOTIFY,
+				handle_hotplug_event_root, NULL);
+	printk(KERN_DEBUG "acpi root: %s notify handler installed\n", objname);
+
+	add_acpi_root_bridge(handle);
+
+	return AE_OK;
+}
+
+static int __init acpi_pci_root_hp_init(void)
+{
+	int num = 0;
+
+	acpi_walk_namespace(ACPI_TYPE_DEVICE, ACPI_ROOT_OBJECT,
+		ACPI_UINT32_MAX, find_root_bridges, NULL, &num, NULL);
+
+	printk(KERN_DEBUG "Found %d acpi root devices\n", num);
+
+	return 0;
+}
+
+subsys_initcall(acpi_pci_root_hp_init);
-- 
1.7.7

Re: [PATCH v2 23/37] PCI, ACPI: Add pci_root_hp hot add hotplug notification

[Bjorn Helgaas]

On Sat, Mar 10, 2012 at 12:00 AM, Yinghai Lu <yinghai@kernel.org> wrote:
> It is from acpiphp_glue.c
>
> How to use it?
> Find out root bus number to acpi root name mapping from dmesg or /sys
>
>  echo "\_SB.PCIB 0" > /proc/acpi/sci/notify
> to add it back

Nope.  That might be a way to exercise this for debug purposes (and it
only works when /proc/acpi/sci/notify is compiled in), but we
definitely don't want normal users to deal with ACPI pathnames like
this.

Most of this functionality belongs in the ACPI core, not here.  The
core should be handling these notify events and turning them into
.add() and .remove() calls to the driver.

But since the ACPI core doesn't do that yet, we have to do it in the
driver, like we do for CPU, memory, etc.  That part is fine.  But I
think it at least belongs *in* the driver, i.e., in pci_root.c, not in
a new pci_root_hp.c file.

There definitely should not be another acpi_walk_namespace() and
another list of ACPI host bridges.  The core *does* already walk the
namespace and call acpi_pci_root_add() for us.  And the driver already
has a list of host bridges (acpi_pci_roots).

> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> Cc: Len Brown <lenb@kernel.org>
> Cc: linux-acpi@vger.kernel.org
> ---
>  drivers/acpi/Makefile      |    1 +
>  drivers/acpi/pci_root_hp.c |  237 ++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 238 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/acpi/pci_root_hp.c
>
> diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
> index 1567028..bc6e53f 100644
> --- a/drivers/acpi/Makefile
> +++ b/drivers/acpi/Makefile
> @@ -36,6 +36,7 @@ acpi-y                                += processor_core.o
>  acpi-y                         += ec.o
>  acpi-$(CONFIG_ACPI_DOCK)       += dock.o
>  acpi-y                         += pci_root.o pci_link.o pci_irq.o pci_bind.o
> +acpi-$(CONFIG_HOTPLUG)         += pci_root_hp.o
>  acpi-y                         += power.o
>  acpi-y                         += event.o
>  acpi-y                         += sysfs.o
> diff --git a/drivers/acpi/pci_root_hp.c b/drivers/acpi/pci_root_hp.c
> new file mode 100644
> index 0000000..dc11e81
> --- /dev/null
> +++ b/drivers/acpi/pci_root_hp.c
> @@ -0,0 +1,237 @@
> +/*
> + * Separated from drivers/pci/hotplug/acpiphp_glue.c
> + *     only support root bridge
> + */
> +
> +#include <linux/init.h>
> +#include <linux/module.h>
> +
> +#include <linux/kernel.h>
> +#include <linux/pci.h>
> +#include <linux/mutex.h>
> +#include <linux/slab.h>
> +#include <linux/acpi.h>
> +
> +static LIST_HEAD(acpi_root_bridge_list);
> +struct acpi_root_bridge {
> +       struct list_head list;
> +       acpi_handle handle;
> +       u32 flags;
> +};
> +
> +/* bridge flags */
> +#define ROOT_BRIDGE_HAS_EJ0    (0x00000002)
> +#define ROOT_BRIDGE_HAS_PS3    (0x00000080)
> +
> +#define ACPI_STA_FUNCTIONING   (0x00000008)
> +
> +static struct acpi_root_bridge *acpi_root_handle_to_bridge(acpi_handle handle)
> +{
> +       struct acpi_root_bridge *bridge;
> +
> +       list_for_each_entry(bridge, &acpi_root_bridge_list, list)
> +               if (bridge->handle == handle)
> +                       return bridge;
> +
> +       return NULL;
> +}
> +
> +/* allocate and initialize host bridge data structure */
> +static void add_acpi_root_bridge(acpi_handle handle)
> +{
> +       struct acpi_root_bridge *bridge;
> +       acpi_handle dummy_handle;
> +       acpi_status status;
> +
> +       /* if the bridge doesn't have _STA, we assume it is always there */
> +       status = acpi_get_handle(handle, "_STA", &dummy_handle);
> +       if (ACPI_SUCCESS(status)) {
> +               unsigned long long tmp;
> +
> +               status = acpi_evaluate_integer(handle, "_STA", NULL, &tmp);
> +               if (ACPI_FAILURE(status)) {
> +                       printk(KERN_DEBUG "%s: _STA evaluation failure\n",
> +                                                __func__);
> +                       return;
> +               }
> +               if ((tmp & ACPI_STA_FUNCTIONING) == 0)
> +                       /* don't register this object */
> +                       return;
> +       }
> +
> +       bridge = kzalloc(sizeof(struct acpi_root_bridge), GFP_KERNEL);
> +       if (!bridge)
> +               return;
> +
> +       bridge->handle = handle;
> +
> +       if (ACPI_SUCCESS(acpi_get_handle(handle, "_EJ0", &dummy_handle)))
> +               bridge->flags |= ROOT_BRIDGE_HAS_EJ0;
> +       if (ACPI_SUCCESS(acpi_get_handle(handle, "_PS3", &dummy_handle)))
> +               bridge->flags |= ROOT_BRIDGE_HAS_PS3;
> +
> +       list_add(&bridge->list, &acpi_root_bridge_list);
> +}
> +
> +struct acpi_root_hp_work {
> +       struct work_struct work;
> +       acpi_handle handle;
> +       u32 type;
> +       void *context;
> +};
> +
> +static void alloc_acpi_root_hp_work(acpi_handle handle, u32 type,
> +                                       void *context,
> +                                       void (*func)(struct work_struct *work))
> +{
> +       struct acpi_root_hp_work *hp_work;
> +       int ret;
> +
> +       hp_work = kmalloc(sizeof(*hp_work), GFP_KERNEL);
> +       if (!hp_work)
> +               return;
> +
> +       hp_work->handle = handle;
> +       hp_work->type = type;
> +       hp_work->context = context;
> +
> +       INIT_WORK(&hp_work->work, func);
> +       ret = queue_work(kacpi_hotplug_wq, &hp_work->work);
> +       if (!ret)
> +               kfree(hp_work);
> +}
> +
> +/* Program resources in newly inserted bridge */
> +static void acpi_root_configure_bridge(acpi_handle handle)
> +{
> +       struct acpi_pci_root *root = acpi_pci_find_root(handle);
> +
> +       pci_assign_unassigned_bus_resources(root->bus);
> +}
> +
> +static void handle_root_bridge_insertion(acpi_handle handle)
> +{
> +       struct acpi_device *device, *pdevice;
> +       acpi_handle phandle;
> +       int ret_val;
> +
> +       acpi_get_parent(handle, &phandle);
> +       if (acpi_bus_get_device(phandle, &pdevice)) {
> +               printk(KERN_DEBUG "no parent device, assuming NULL\n");
> +               pdevice = NULL;
> +       }
> +       if (!acpi_bus_get_device(handle, &device)) {
> +               /* check if  pci root_bus is removed */
> +               struct acpi_pci_root *root = acpi_driver_data(device);
> +               if (pci_find_bus(root->segment, root->secondary.start))
> +                       return;
> +
> +               printk(KERN_DEBUG "bus exists... trim\n");
> +               /* this shouldn't be in here, so remove
> +                * the bus then re-add it...
> +                */
> +               ret_val = acpi_bus_trim(device, 1);
> +               printk(KERN_DEBUG "acpi_bus_trim return %x\n", ret_val);
> +       }
> +       if (acpi_bus_add(&device, pdevice, handle, ACPI_BUS_TYPE_DEVICE)) {
> +               printk(KERN_ERR "cannot add bridge to acpi list\n");
> +               return;
> +       }
> +       acpi_root_configure_bridge(handle);
> +       if (acpi_bus_start(device))
> +               printk(KERN_ERR "cannot start bridge\n");
> +}
> +
> +static void _handle_hotplug_event_root(struct work_struct *work)
> +{
> +       struct acpi_root_bridge *bridge;
> +       char objname[64];
> +       struct acpi_buffer buffer = { .length = sizeof(objname),
> +                                     .pointer = objname };
> +       struct acpi_root_hp_work *hp_work;
> +       acpi_handle handle;
> +       u32 type;
> +
> +       hp_work = container_of(work, struct acpi_root_hp_work, work);
> +       handle = hp_work->handle;
> +       type = hp_work->type;
> +
> +       bridge = acpi_root_handle_to_bridge(handle);
> +
> +       acpi_get_name(handle, ACPI_FULL_PATHNAME, &buffer);
> +
> +       switch (type) {
> +       case ACPI_NOTIFY_BUS_CHECK:
> +               /* bus enumerate */
> +               printk(KERN_DEBUG "%s: Bus check notify on %s\n", __func__,
> +                                objname);
> +               if (!bridge) {
> +                       handle_root_bridge_insertion(handle);
> +                       add_acpi_root_bridge(handle);
> +               }
> +
> +               break;
> +
> +       case ACPI_NOTIFY_DEVICE_CHECK:
> +               /* device check */
> +               printk(KERN_DEBUG "%s: Device check notify on %s\n", __func__,
> +                                objname);
> +               if (!bridge) {
> +                       handle_root_bridge_insertion(handle);
> +                       add_acpi_root_bridge(handle);
> +               }
> +               break;
> +
> +       default:
> +               printk(KERN_WARNING "notify_handler: unknown event type 0x%x for %s\n",
> +                                type, objname);
> +               break;
> +       }
> +
> +       kfree(hp_work); /* allocated in handle_hotplug_event_bridge */
> +}
> +
> +static void handle_hotplug_event_root(acpi_handle handle, u32 type,
> +                                       void *context)
> +{
> +       alloc_acpi_root_hp_work(handle, type, context,
> +                               _handle_hotplug_event_root);
> +}
> +
> +static acpi_status __init
> +find_root_bridges(acpi_handle handle, u32 lvl, void *context, void **rv)
> +{
> +       char objname[64];
> +       struct acpi_buffer buffer = { .length = sizeof(objname),
> +                                     .pointer = objname };
> +       int *count = (int *)context;
> +
> +       if (!acpi_is_root_bridge(handle))
> +               return AE_OK;
> +
> +       (*count)++;
> +
> +       acpi_get_name(handle, ACPI_FULL_PATHNAME, &buffer);
> +
> +       acpi_install_notify_handler(handle, ACPI_SYSTEM_NOTIFY,
> +                               handle_hotplug_event_root, NULL);
> +       printk(KERN_DEBUG "acpi root: %s notify handler installed\n", objname);
> +
> +       add_acpi_root_bridge(handle);
> +
> +       return AE_OK;
> +}
> +
> +static int __init acpi_pci_root_hp_init(void)
> +{
> +       int num = 0;
> +
> +       acpi_walk_namespace(ACPI_TYPE_DEVICE, ACPI_ROOT_OBJECT,
> +               ACPI_UINT32_MAX, find_root_bridges, NULL, &num, NULL);
> +
> +       printk(KERN_DEBUG "Found %d acpi root devices\n", num);
> +
> +       return 0;
> +}
> +
> +subsys_initcall(acpi_pci_root_hp_init);
> --
> 1.7.7
>

Re: [PATCH v2 23/37] PCI, ACPI: Add pci_root_hp hot add hotplug notification

[Yinghai Lu]

On Mon, Mar 12, 2012 at 8:22 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> On Sat, Mar 10, 2012 at 12:00 AM, Yinghai Lu <yinghai@kernel.org> wrote:
>> It is from acpiphp_glue.c
>>
>> How to use it?
>> Find out root bus number to acpi root name mapping from dmesg or /sys
>>
>>  echo "\_SB.PCIB 0" > /proc/acpi/sci/notify
>> to add it back
>
> Nope.  That might be a way to exercise this for debug purposes (and it
> only works when /proc/acpi/sci/notify is compiled in), but we
> definitely don't want normal users to deal with ACPI pathnames like
> this.

before this patch set, only way is: Users really put one root bus in
and press buton etc to trigger notification
to get it work.

>
> Most of this functionality belongs in the ACPI core, not here.  The
> core should be handling these notify events and turning them into
> .add() and .remove() calls to the driver.
>
> But since the ACPI core doesn't do that yet, we have to do it in the
> driver, like we do for CPU, memory, etc.  That part is fine.  But I
> think it at least belongs *in* the driver, i.e., in pci_root.c, not in
> a new pci_root_hp.c file.

don't want to pollute pci_root.c.

in separated file, we can use CONFIG_HOTPLUG in Makefile instead of pci_root.c

>
> There definitely should not be another acpi_walk_namespace() and
> another list of ACPI host bridges.  The core *does* already walk the
> namespace and call acpi_pci_root_add() for us.  And the driver already
> has a list of host bridges (acpi_pci_roots).

no, we may not have acpi_pci_root there.

Re: [PATCH v2 23/37] PCI, ACPI: Add pci_root_hp hot add hotplug notification

[Bjorn Helgaas]

On Tue, Mar 13, 2012 at 12:03 AM, Yinghai Lu <yinghai@kernel.org> wrote:
> On Mon, Mar 12, 2012 at 8:22 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
>> On Sat, Mar 10, 2012 at 12:00 AM, Yinghai Lu <yinghai@kernel.org> wrote:
>>> It is from acpiphp_glue.c
>>>
>>> How to use it?
>>> Find out root bus number to acpi root name mapping from dmesg or /sys
>>>
>>>  echo "\_SB.PCIB 0" > /proc/acpi/sci/notify
>>> to add it back
>>
>> Nope.  That might be a way to exercise this for debug purposes (and it
>> only works when /proc/acpi/sci/notify is compiled in), but we
>> definitely don't want normal users to deal with ACPI pathnames like
>> this.
>
> before this patch set, only way is: Users really put one root bus in
> and press buton etc to trigger notification
> to get it work.
>
>>
>> Most of this functionality belongs in the ACPI core, not here.  The
>> core should be handling these notify events and turning them into
>> .add() and .remove() calls to the driver.
>>
>> But since the ACPI core doesn't do that yet, we have to do it in the
>> driver, like we do for CPU, memory, etc.  That part is fine.  But I
>> think it at least belongs *in* the driver, i.e., in pci_root.c, not in
>> a new pci_root_hp.c file.
>
> don't want to pollute pci_root.c.
>
> in separated file, we can use CONFIG_HOTPLUG in Makefile instead of pci_root.c

I don't buy this argument.  Most of this functionality should someday
be in the ACPI core.  Until then, it is intimately connected to the
stuff in pci_root.c.  It is not "pollution."

>> There definitely should not be another acpi_walk_namespace() and
>> another list of ACPI host bridges.  The core *does* already walk the
>> namespace and call acpi_pci_root_add() for us.  And the driver already
>> has a list of host bridges (acpi_pci_roots).
>
> no, we may not have acpi_pci_root there.

I don't understand what you're saying here.  I think you should be
basically implementing the "turn notify events into add/remove calls"
functionality.  This is what *should* be in the ACPI core, but isn't
yet.  If you put this in the driver, and hopefully follow the style
already used by the processor/memory/etc. drivers, it will be easier
to someday move it into the ACPI core.

Bjorn

[PATCH v2 24/37] PCI, ACPI: Add pci_root_hp hot removal notification support.

[Yinghai Lu]

Add missing hot_remove support for root device.

How to use it?
Find out root bus number to acpi root name mapping from dmesg or /sys

  echo "\_SB.PCIB 3" > /proc/acpi/sci/notify
to remove root bus

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
Cc: Len Brown <lenb@kernel.org>
Cc: linux-acpi@vger.kernel.org
---
 drivers/acpi/pci_root_hp.c |   61 ++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 61 insertions(+), 0 deletions(-)

diff --git a/drivers/acpi/pci_root_hp.c b/drivers/acpi/pci_root_hp.c
index dc11e81..f5585f5 100644
--- a/drivers/acpi/pci_root_hp.c
+++ b/drivers/acpi/pci_root_hp.c
@@ -73,6 +73,12 @@ static void add_acpi_root_bridge(acpi_handle handle)
 	list_add(&bridge->list, &acpi_root_bridge_list);
 }
 
+static void remove_acpi_root_bridge(struct acpi_root_bridge *bridge)
+{
+	list_del(&bridge->list);
+	kfree(bridge);
+}
+
 struct acpi_root_hp_work {
 	struct work_struct work;
 	acpi_handle handle;
@@ -142,6 +148,55 @@ static void handle_root_bridge_insertion(acpi_handle handle)
 		printk(KERN_ERR "cannot start bridge\n");
 }
 
+static int acpi_root_evaluate_object(acpi_handle handle, char *cmd, int val)
+{
+	acpi_status status;
+	struct acpi_object_list arg_list;
+	union acpi_object arg;
+
+	arg_list.count = 1;
+	arg_list.pointer = &arg;
+	arg.type = ACPI_TYPE_INTEGER;
+	arg.integer.value = val;
+
+	status = acpi_evaluate_object(handle, cmd, &arg_list, NULL);
+	if (ACPI_FAILURE(status)) {
+		printk(KERN_WARNING "%s: %s to %d failed\n",
+				 __func__, cmd, val);
+		return -1;
+	}
+
+	return 0;
+}
+
+static void handle_root_bridge_removal(acpi_handle handle,
+		 struct acpi_root_bridge *bridge)
+{
+	u32 flags = 0;
+	struct acpi_device *device;
+
+	if (bridge) {
+		flags = bridge->flags;
+		remove_acpi_root_bridge(bridge);
+	}
+
+	if (!acpi_bus_get_device(handle, &device)) {
+		int ret_val = acpi_bus_trim(device, 1);
+
+		printk(KERN_DEBUG "acpi_bus_trim return %x\n", ret_val);
+	}
+
+	if (flags & ROOT_BRIDGE_HAS_PS3) {
+		acpi_status status;
+
+		status = acpi_evaluate_object(handle, "_PS3", NULL, NULL);
+		if (ACPI_FAILURE(status))
+			printk(KERN_WARNING "%s: _PS3 failed\n", __func__);
+	}
+	if (flags & ROOT_BRIDGE_HAS_EJ0)
+		acpi_root_evaluate_object(handle, "_EJ0", 1);
+}
+
 static void _handle_hotplug_event_root(struct work_struct *work)
 {
 	struct acpi_root_bridge *bridge;
@@ -182,6 +237,12 @@ static void _handle_hotplug_event_root(struct work_struct *work)
 		}
 		break;
 
+	case ACPI_NOTIFY_EJECT_REQUEST:
+		/* request device eject */
+		printk(KERN_DEBUG "%s: Device eject notify on %s\n", __func__,
+				 objname);
+		handle_root_bridge_removal(handle, bridge);
+		break;
 	default:
 		printk(KERN_WARNING "notify_handler: unknown event type 0x%x for %s\n",
 				 type, objname);
-- 
1.7.7

Re: [PATCH v2 24/37] PCI, ACPI: Add pci_root_hp hot removal notification support.

[Bjorn Helgaas]

On Sat, Mar 10, 2012 at 12:00 AM, Yinghai Lu <yinghai@kernel.org> wrote:
> Add missing hot_remove support for root device.
>
> How to use it?
> Find out root bus number to acpi root name mapping from dmesg or /sys
>
>  echo "\_SB.PCIB 3" > /proc/acpi/sci/notify
> to remove root bus

Same comments as before: this is definitely NOT the way users should
use this.  If you want to do that for debugging the ACPI notification
path, fine, but the normal way is for the platform to generate this
notification.  I think the normal way for users to manually remove an
ACPI device should be something like:

    echo 1 > /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/remove

just like we do for PCI devices.

> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> Cc: Len Brown <lenb@kernel.org>
> Cc: linux-acpi@vger.kernel.org
> ---
>  drivers/acpi/pci_root_hp.c |   61 ++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 61 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/acpi/pci_root_hp.c b/drivers/acpi/pci_root_hp.c
> index dc11e81..f5585f5 100644
> --- a/drivers/acpi/pci_root_hp.c
> +++ b/drivers/acpi/pci_root_hp.c
> @@ -73,6 +73,12 @@ static void add_acpi_root_bridge(acpi_handle handle)
>        list_add(&bridge->list, &acpi_root_bridge_list);
>  }
>
> +static void remove_acpi_root_bridge(struct acpi_root_bridge *bridge)
> +{
> +       list_del(&bridge->list);
> +       kfree(bridge);
> +}
> +
>  struct acpi_root_hp_work {
>        struct work_struct work;
>        acpi_handle handle;
> @@ -142,6 +148,55 @@ static void handle_root_bridge_insertion(acpi_handle handle)
>                printk(KERN_ERR "cannot start bridge\n");
>  }
>
> +static int acpi_root_evaluate_object(acpi_handle handle, char *cmd, int val)
> +{
> +       acpi_status status;
> +       struct acpi_object_list arg_list;
> +       union acpi_object arg;
> +
> +       arg_list.count = 1;
> +       arg_list.pointer = &arg;
> +       arg.type = ACPI_TYPE_INTEGER;
> +       arg.integer.value = val;
> +
> +       status = acpi_evaluate_object(handle, cmd, &arg_list, NULL);
> +       if (ACPI_FAILURE(status)) {
> +               printk(KERN_WARNING "%s: %s to %d failed\n",
> +                                __func__, cmd, val);
> +               return -1;
> +       }
> +
> +       return 0;

This function really has nothing to do with host bridges.  Are you
sure there's no generic function you can use instead?

> +}
> +
> +static void handle_root_bridge_removal(acpi_handle handle,
> +                struct acpi_root_bridge *bridge)
> +{
> +       u32 flags = 0;
> +       struct acpi_device *device;
> +
> +       if (bridge) {
> +               flags = bridge->flags;
> +               remove_acpi_root_bridge(bridge);
> +       }
> +
> +       if (!acpi_bus_get_device(handle, &device)) {
> +               int ret_val = acpi_bus_trim(device, 1);
> +
> +               printk(KERN_DEBUG "acpi_bus_trim return %x\n", ret_val);
> +       }
> +
> +       if (flags & ROOT_BRIDGE_HAS_PS3) {
> +               acpi_status status;
> +
> +               status = acpi_evaluate_object(handle, "_PS3", NULL, NULL);
> +               if (ACPI_FAILURE(status))
> +                       printk(KERN_WARNING "%s: _PS3 failed\n", __func__);
> +       }
> +       if (flags & ROOT_BRIDGE_HAS_EJ0)
> +               acpi_root_evaluate_object(handle, "_EJ0", 1);
> +}
> +
>  static void _handle_hotplug_event_root(struct work_struct *work)
>  {
>        struct acpi_root_bridge *bridge;
> @@ -182,6 +237,12 @@ static void _handle_hotplug_event_root(struct work_struct *work)
>                }
>                break;
>
> +       case ACPI_NOTIFY_EJECT_REQUEST:
> +               /* request device eject */
> +               printk(KERN_DEBUG "%s: Device eject notify on %s\n", __func__,
> +                                objname);
> +               handle_root_bridge_removal(handle, bridge);
> +               break;
>        default:
>                printk(KERN_WARNING "notify_handler: unknown event type 0x%x for %s\n",
>                                 type, objname);
> --
> 1.7.7
>

Re: [PATCH v2 24/37] PCI, ACPI: Add pci_root_hp hot removal notification support.

[Yinghai Lu]

On Mon, Mar 12, 2012 at 8:25 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> On Sat, Mar 10, 2012 at 12:00 AM, Yinghai Lu <yinghai@kernel.org> wrote:
>> Add missing hot_remove support for root device.
>>
>> How to use it?
>> Find out root bus number to acpi root name mapping from dmesg or /sys
>>
>>  echo "\_SB.PCIB 3" > /proc/acpi/sci/notify
>> to remove root bus
>
> Same comments as before: this is definitely NOT the way users should
> use this.  If you want to do that for debugging the ACPI notification
> path, fine, but the normal way is for the platform to generate this
> notification.  I think the normal way for users to manually remove an
> ACPI device should be something like:
>
>    echo 1 > /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/remove
>
> just like we do for PCI devices.

notify way looks more like really physical hotadd/remove.

pci bus/remove and other is not really, because when you are using
/sys, you may still need to hold
one reference to bus or device.

>
>> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
>> Cc: Len Brown <lenb@kernel.org>
>> Cc: linux-acpi@vger.kernel.org
>> ---
>>  drivers/acpi/pci_root_hp.c |   61 ++++++++++++++++++++++++++++++++++++++++++++
>>  1 files changed, 61 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/acpi/pci_root_hp.c b/drivers/acpi/pci_root_hp.c
>> index dc11e81..f5585f5 100644
>> --- a/drivers/acpi/pci_root_hp.c
>> +++ b/drivers/acpi/pci_root_hp.c
>> @@ -73,6 +73,12 @@ static void add_acpi_root_bridge(acpi_handle handle)
>>        list_add(&bridge->list, &acpi_root_bridge_list);
>>  }
>>
>> +static void remove_acpi_root_bridge(struct acpi_root_bridge *bridge)
>> +{
>> +       list_del(&bridge->list);
>> +       kfree(bridge);
>> +}
>> +
>>  struct acpi_root_hp_work {
>>        struct work_struct work;
>>        acpi_handle handle;
>> @@ -142,6 +148,55 @@ static void handle_root_bridge_insertion(acpi_handle handle)
>>                printk(KERN_ERR "cannot start bridge\n");
>>  }
>>
>> +static int acpi_root_evaluate_object(acpi_handle handle, char *cmd, int val)
>> +{
>> +       acpi_status status;
>> +       struct acpi_object_list arg_list;
>> +       union acpi_object arg;
>> +
>> +       arg_list.count = 1;
>> +       arg_list.pointer = &arg;
>> +       arg.type = ACPI_TYPE_INTEGER;
>> +       arg.integer.value = val;
>> +
>> +       status = acpi_evaluate_object(handle, cmd, &arg_list, NULL);
>> +       if (ACPI_FAILURE(status)) {
>> +               printk(KERN_WARNING "%s: %s to %d failed\n",
>> +                                __func__, cmd, val);
>> +               return -1;
>> +       }
>> +
>> +       return 0;
>
> This function really has nothing to do with host bridges.  Are you
> sure there's no generic function you can use instead?

seems not.

Re: [PATCH v2 24/37] PCI, ACPI: Add pci_root_hp hot removal notification support.

[Bjorn Helgaas]

On Tue, Mar 13, 2012 at 12:06 AM, Yinghai Lu <yinghai@kernel.org> wrote:
> On Mon, Mar 12, 2012 at 8:25 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
>> On Sat, Mar 10, 2012 at 12:00 AM, Yinghai Lu <yinghai@kernel.org> wrote:
>>> Add missing hot_remove support for root device.
>>>
>>> How to use it?
>>> Find out root bus number to acpi root name mapping from dmesg or /sys
>>>
>>>  echo "\_SB.PCIB 3" > /proc/acpi/sci/notify
>>> to remove root bus
>>
>> Same comments as before: this is definitely NOT the way users should
>> use this.  If you want to do that for debugging the ACPI notification
>> path, fine, but the normal way is for the platform to generate this
>> notification.  I think the normal way for users to manually remove an
>> ACPI device should be something like:
>>
>>    echo 1 > /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/remove
>>
>> just like we do for PCI devices.
>
> notify way looks more like really physical hotadd/remove.

Sure, the 'echo "\_SB.PCIB 3" > /proc/acpi/sci/notify' way looks more
like physical hotadd/remove because it is basically injecting the
notify event that the hardware/BIOS should be generating itself.  But
it is platform-specific and therefore unacceptable as a general user
interface.

Using /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/remove would be a
more generic way.  The PNP0A08:00/remove implementation would
basically turn into an acpi_pci_root_remove() call, just like the
'echo "\_SB.PCIB 3" > /proc/acpi/sci/notify' does.

Both ways should exercise the same code path.  The only difference is
that using /proc/acpi/sci/notify should go through a little more of
the ACPI notify path before reaching acpi_pci_root_remove().

> pci bus/remove and other is not really, because when you are using
> /sys, you may still need to hold
> one reference to bus or device.
>
>>
>>> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
>>> Cc: Len Brown <lenb@kernel.org>
>>> Cc: linux-acpi@vger.kernel.org
>>> ---
>>>  drivers/acpi/pci_root_hp.c |   61 ++++++++++++++++++++++++++++++++++++++++++++
>>>  1 files changed, 61 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/drivers/acpi/pci_root_hp.c b/drivers/acpi/pci_root_hp.c
>>> index dc11e81..f5585f5 100644
>>> --- a/drivers/acpi/pci_root_hp.c
>>> +++ b/drivers/acpi/pci_root_hp.c
>>> @@ -73,6 +73,12 @@ static void add_acpi_root_bridge(acpi_handle handle)
>>>        list_add(&bridge->list, &acpi_root_bridge_list);
>>>  }
>>>
>>> +static void remove_acpi_root_bridge(struct acpi_root_bridge *bridge)
>>> +{
>>> +       list_del(&bridge->list);
>>> +       kfree(bridge);
>>> +}
>>> +
>>>  struct acpi_root_hp_work {
>>>        struct work_struct work;
>>>        acpi_handle handle;
>>> @@ -142,6 +148,55 @@ static void handle_root_bridge_insertion(acpi_handle handle)
>>>                printk(KERN_ERR "cannot start bridge\n");
>>>  }
>>>
>>> +static int acpi_root_evaluate_object(acpi_handle handle, char *cmd, int val)
>>> +{
>>> +       acpi_status status;
>>> +       struct acpi_object_list arg_list;
>>> +       union acpi_object arg;
>>> +
>>> +       arg_list.count = 1;
>>> +       arg_list.pointer = &arg;
>>> +       arg.type = ACPI_TYPE_INTEGER;
>>> +       arg.integer.value = val;
>>> +
>>> +       status = acpi_evaluate_object(handle, cmd, &arg_list, NULL);
>>> +       if (ACPI_FAILURE(status)) {
>>> +               printk(KERN_WARNING "%s: %s to %d failed\n",
>>> +                                __func__, cmd, val);
>>> +               return -1;
>>> +       }
>>> +
>>> +       return 0;
>>
>> This function really has nothing to do with host bridges.  Are you
>> sure there's no generic function you can use instead?
>
> seems not.

Re: [PATCH v2 24/37] PCI, ACPI: Add pci_root_hp hot removal notification support.

[Yinghai Lu]

On Thu, Mar 15, 2012 at 10:53 AM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> Sure, the 'echo "\_SB.PCIB 3" > /proc/acpi/sci/notify' way looks more
> like physical hotadd/remove because it is basically injecting the
> notify event that the hardware/BIOS should be generating itself.  But
> it is platform-specific and therefore unacceptable as a general user
> interface.
>
> Using /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/remove would be a
> more generic way.  The PNP0A08:00/remove implementation would
> basically turn into an acpi_pci_root_remove() call, just like the
> 'echo "\_SB.PCIB 3" > /proc/acpi/sci/notify' does.

if the users can not find _SB.PCIB etc, they may not find
LNXSYSTM:00/device:00/PNP0A08:00.

so /sys/class/pci_bus/0000:80/remove is more easy for them?

Yinghai

Re: [PATCH v2 24/37] PCI, ACPI: Add pci_root_hp hot removal notification support.

[Bjorn Helgaas]

On Thu, Mar 15, 2012 at 12:01 PM, Yinghai Lu <yinghai@kernel.org> wrote:
> On Thu, Mar 15, 2012 at 10:53 AM, Bjorn Helgaas <bhelgaas@google.com> wrote:
>> Sure, the 'echo "\_SB.PCIB 3" > /proc/acpi/sci/notify' way looks more
>> like physical hotadd/remove because it is basically injecting the
>> notify event that the hardware/BIOS should be generating itself.  But
>> it is platform-specific and therefore unacceptable as a general user
>> interface.
>>
>> Using /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/remove would be a
>> more generic way.  The PNP0A08:00/remove implementation would
>> basically turn into an acpi_pci_root_remove() call, just like the
>> 'echo "\_SB.PCIB 3" > /proc/acpi/sci/notify' does.
>
> if the users can not find _SB.PCIB etc, they may not find
> LNXSYSTM:00/device:00/PNP0A08:00.
>
> so /sys/class/pci_bus/0000:80/remove is more easy for them?

I don't think so.  LNXSYSTM:00/... is the canonical machine
description on x86.  I think there should probably be a link from the
PCI device to the upstream bridge.  The upstream bridge could be
either a P2P bridge or a host bridge.

Bjorn

[PATCH v2 25/37] PCI, ACPI: Add alloc_acpi_hp_work()

[Yinghai Lu]

Will use it with acpiphp and pci_root_hp events handling

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
Cc: Len Brown <lenb@kernel.org>
Cc: linux-acpi@vger.kernel.org
---
 drivers/acpi/osl.c      |   21 +++++++++++++++++++++
 include/acpi/acpiosxf.h |    9 +++++++++
 2 files changed, 30 insertions(+), 0 deletions(-)

diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
index 412a1e0..fdcf4a5 100644
--- a/drivers/acpi/osl.c
+++ b/drivers/acpi/osl.c
@@ -1641,3 +1641,24 @@ acpi_status acpi_os_terminate(void)
 
 	return AE_OK;
 }
+
+void alloc_acpi_hp_work(acpi_handle handle, u32 type, void *context,
+			void (*func)(struct work_struct *work))
+{
+	struct acpi_hp_work *hp_work;
+	int ret;
+
+	hp_work = kmalloc(sizeof(*hp_work), GFP_KERNEL);
+	if (!hp_work)
+		return;
+
+	hp_work->handle = handle;
+	hp_work->type = type;
+	hp_work->context = context;
+
+	INIT_WORK(&hp_work->work, func);
+	ret = queue_work(kacpi_hotplug_wq, &hp_work->work);
+	if (!ret)
+		kfree(hp_work);
+}
+EXPORT_SYMBOL(alloc_acpi_hp_work);
diff --git a/include/acpi/acpiosxf.h b/include/acpi/acpiosxf.h
index 7c9aebe..62326ae 100644
--- a/include/acpi/acpiosxf.h
+++ b/include/acpi/acpiosxf.h
@@ -191,6 +191,15 @@ void acpi_os_fixed_event_count(u32 fixed_event_number);
  */
 extern struct workqueue_struct *kacpi_hotplug_wq;
 
+struct acpi_hp_work {
+	struct work_struct work;
+	acpi_handle handle;
+	u32 type;
+	void *context;
+};
+void alloc_acpi_hp_work(acpi_handle handle, u32 type, void *context,
+			void (*func)(struct work_struct *work));
+
 acpi_thread_id acpi_os_get_thread_id(void);
 
 acpi_status
-- 
1.7.7

[PATCH v2 26/37] PCI, acpiphp: Use acpi_hp_work

[Yinghai Lu]

Remove local defined acpiphp_hp_work.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
Cc: Len Brown <lenb@kernel.org>
Cc: linux-acpi@vger.kernel.org
---
 drivers/pci/hotplug/acpiphp_glue.c |   42 +++++------------------------------
 1 files changed, 6 insertions(+), 36 deletions(-)

diff --git a/drivers/pci/hotplug/acpiphp_glue.c b/drivers/pci/hotplug/acpiphp_glue.c
index 1ca82ae..9f1a195 100644
--- a/drivers/pci/hotplug/acpiphp_glue.c
+++ b/drivers/pci/hotplug/acpiphp_glue.c
@@ -1118,34 +1118,6 @@ check_sub_bridges(acpi_handle handle, u32 lvl, void *context, void **rv)
 	return AE_OK ;
 }
 
-struct acpiphp_hp_work {
-	struct work_struct work;
-	acpi_handle handle;
-	u32 type;
-	void *context;
-};
-
-static void alloc_acpiphp_hp_work(acpi_handle handle, u32 type,
-				  void *context,
-				  void (*func)(struct work_struct *work))
-{
-	struct acpiphp_hp_work *hp_work;
-	int ret;
-
-	hp_work = kmalloc(sizeof(*hp_work), GFP_KERNEL);
-	if (!hp_work)
-		return;
-
-	hp_work->handle = handle;
-	hp_work->type = type;
-	hp_work->context = context;
-
-	INIT_WORK(&hp_work->work, func);
-	ret = queue_work(kacpi_hotplug_wq, &hp_work->work);
-	if (!ret)
-		kfree(hp_work);
-}
-
 static void _handle_hotplug_event_bridge(struct work_struct *work)
 {
 	struct acpiphp_bridge *bridge;
@@ -1154,11 +1126,11 @@ static void _handle_hotplug_event_bridge(struct work_struct *work)
 				      .pointer = objname };
 	struct acpi_device *device;
 	int num_sub_bridges = 0;
-	struct acpiphp_hp_work *hp_work;
+	struct acpi_hp_work *hp_work;
 	acpi_handle handle;
 	u32 type;
 
-	hp_work = container_of(work, struct acpiphp_hp_work, work);
+	hp_work = container_of(work, struct acpi_hp_work, work);
 	handle = hp_work->handle;
 	type = hp_work->type;
 
@@ -1260,8 +1232,7 @@ static void handle_hotplug_event_bridge(acpi_handle handle, u32 type,
 	 * For now just re-add this work to the kacpi_hotplug_wq so we
 	 * don't deadlock on hotplug actions.
 	 */
-	alloc_acpiphp_hp_work(handle, type, context,
-			      _handle_hotplug_event_bridge);
+	alloc_acpi_hp_work(handle, type, context, _handle_hotplug_event_bridge);
 }
 
 static void _handle_hotplug_event_func(struct work_struct *work)
@@ -1270,12 +1241,12 @@ static void _handle_hotplug_event_func(struct work_struct *work)
 	char objname[64];
 	struct acpi_buffer buffer = { .length = sizeof(objname),
 				      .pointer = objname };
-	struct acpiphp_hp_work *hp_work;
+	struct acpi_hp_work *hp_work;
 	acpi_handle handle;
 	u32 type;
 	void *context;
 
-	hp_work = container_of(work, struct acpiphp_hp_work, work);
+	hp_work = container_of(work, struct acpi_hp_work, work);
 	handle = hp_work->handle;
 	type = hp_work->type;
 	context = hp_work->context;
@@ -1336,8 +1307,7 @@ static void handle_hotplug_event_func(acpi_handle handle, u32 type,
 	 * For now just re-add this work to the kacpi_hotplug_wq so we
 	 * don't deadlock on hotplug actions.
 	 */
-	alloc_acpiphp_hp_work(handle, type, context,
-			      _handle_hotplug_event_func);
+	alloc_acpi_hp_work(handle, type, context, _handle_hotplug_event_func);
 }
 
 static struct acpi_pci_driver acpi_pci_hp_driver = {
-- 
1.7.7

[PATCH v2 27/37] PCI, pci_root_hp: Use acpi_hp_work

[Yinghai Lu]

Remove local copy: acpi_root_hp_work

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
Cc: Len Brown <lenb@kernel.org>
Cc: linux-acpi@vger.kernel.org
---
 drivers/acpi/pci_root_hp.c |   34 +++-------------------------------
 1 files changed, 3 insertions(+), 31 deletions(-)

diff --git a/drivers/acpi/pci_root_hp.c b/drivers/acpi/pci_root_hp.c
index f5585f5..b453181 100644
--- a/drivers/acpi/pci_root_hp.c
+++ b/drivers/acpi/pci_root_hp.c
@@ -79,34 +79,6 @@ static void remove_acpi_root_bridge(struct acpi_root_bridge *bridge)
 	kfree(bridge);
 }
 
-struct acpi_root_hp_work {
-	struct work_struct work;
-	acpi_handle handle;
-	u32 type;
-	void *context;
-};
-
-static void alloc_acpi_root_hp_work(acpi_handle handle, u32 type,
-					void *context,
-					void (*func)(struct work_struct *work))
-{
-	struct acpi_root_hp_work *hp_work;
-	int ret;
-
-	hp_work = kmalloc(sizeof(*hp_work), GFP_KERNEL);
-	if (!hp_work)
-		return;
-
-	hp_work->handle = handle;
-	hp_work->type = type;
-	hp_work->context = context;
-
-	INIT_WORK(&hp_work->work, func);
-	ret = queue_work(kacpi_hotplug_wq, &hp_work->work);
-	if (!ret)
-		kfree(hp_work);
-}
-
 /* Program resources in newly inserted bridge */
 static void acpi_root_configure_bridge(acpi_handle handle)
 {
@@ -203,11 +175,11 @@ static void _handle_hotplug_event_root(struct work_struct *work)
 	char objname[64];
 	struct acpi_buffer buffer = { .length = sizeof(objname),
 				      .pointer = objname };
-	struct acpi_root_hp_work *hp_work;
+	struct acpi_hp_work *hp_work;
 	acpi_handle handle;
 	u32 type;
 
-	hp_work = container_of(work, struct acpi_root_hp_work, work);
+	hp_work = container_of(work, struct acpi_hp_work, work);
 	handle = hp_work->handle;
 	type = hp_work->type;
 
@@ -255,7 +227,7 @@ static void _handle_hotplug_event_root(struct work_struct *work)
 static void handle_hotplug_event_root(acpi_handle handle, u32 type,
 					void *context)
 {
-	alloc_acpi_root_hp_work(handle, type, context,
+	alloc_acpi_hp_work(handle, type, context,
 				_handle_hotplug_event_root);
 }
 
-- 
1.7.7

[PATCH v2 28/37] PCI, ACPI: Make kacpi_hotplug_wq static

[Yinghai Lu]

No external user anymore.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
Cc: Len Brown <lenb@kernel.org>
Cc: linux-acpi@vger.kernel.org
---
 drivers/acpi/osl.c      |    3 +--
 include/acpi/acpiosxf.h |    2 --
 2 files changed, 1 insertions(+), 4 deletions(-)

diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
index fdcf4a5..09be602 100644
--- a/drivers/acpi/osl.c
+++ b/drivers/acpi/osl.c
@@ -81,8 +81,7 @@ static acpi_osd_handler acpi_irq_handler;
 static void *acpi_irq_context;
 static struct workqueue_struct *kacpid_wq;
 static struct workqueue_struct *kacpi_notify_wq;
-struct workqueue_struct *kacpi_hotplug_wq;
-EXPORT_SYMBOL(kacpi_hotplug_wq);
+static struct workqueue_struct *kacpi_hotplug_wq;
 
 /*
  * This list of permanent mappings is for memory that may be accessed from
diff --git a/include/acpi/acpiosxf.h b/include/acpi/acpiosxf.h
index 62326ae..2326622 100644
--- a/include/acpi/acpiosxf.h
+++ b/include/acpi/acpiosxf.h
@@ -189,8 +189,6 @@ void acpi_os_fixed_event_count(u32 fixed_event_number);
 /*
  * Threads and Scheduling
  */
-extern struct workqueue_struct *kacpi_hotplug_wq;
-
 struct acpi_hp_work {
 	struct work_struct work;
 	acpi_handle handle;
-- 
1.7.7

[PATCH v2 29/37] PCI, ACPI: Add acpi_pci_root_rescan()

[Yinghai Lu]

It will rescan all acpi pci root if related pci root bus get removed before.

Signed-off-by: Yinghai <yinghai@kernel.org>
Cc: Len Brown <lenb@kernel.org>
Cc: linux-acpi@vger.kernel.org
---
 drivers/acpi/pci_root_hp.c |   17 +++++++++++++++++
 include/linux/pci-acpi.h   |    7 +++++++
 2 files changed, 24 insertions(+), 0 deletions(-)

diff --git a/drivers/acpi/pci_root_hp.c b/drivers/acpi/pci_root_hp.c
index b453181..ca73d51 100644
--- a/drivers/acpi/pci_root_hp.c
+++ b/drivers/acpi/pci_root_hp.c
@@ -268,3 +268,20 @@ static int __init acpi_pci_root_hp_init(void)
 }
 
 subsys_initcall(acpi_pci_root_hp_init);
+
+static acpi_status
+rescan_root_bridge(acpi_handle handle, u32 lvl, void *context, void **rv)
+{
+	if (!acpi_is_root_bridge(handle))
+		return AE_OK;
+
+	handle_root_bridge_insertion(handle);
+
+	return AE_OK;
+}
+
+void acpi_pci_root_rescan(void)
+{
+	acpi_walk_namespace(ACPI_TYPE_DEVICE, ACPI_ROOT_OBJECT,
+		ACPI_UINT32_MAX, rescan_root_bridge, NULL, NULL, NULL);
+}
diff --git a/include/linux/pci-acpi.h b/include/linux/pci-acpi.h
index 4462350..ac93634 100644
--- a/include/linux/pci-acpi.h
+++ b/include/linux/pci-acpi.h
@@ -35,6 +35,13 @@ static inline acpi_handle acpi_pci_get_bridge_handle(struct pci_bus *pbus)
 	return acpi_get_pci_rootbridge_handle(pci_domain_nr(pbus),
 					      pbus->number);
 }
+
+void acpi_pci_root_rescan(void);
+
+#else
+
+static inline void acpi_pci_root_rescan(void) { }
+
 #endif
 
 #ifdef CONFIG_ACPI_APEI
-- 
1.7.7

Re: [PATCH v2 29/37] PCI, ACPI: Add acpi_pci_root_rescan()

[Bjorn Helgaas]

On Sat, Mar 10, 2012 at 12:00 AM, Yinghai Lu <yinghai@kernel.org> wrote:
> It will rescan all acpi pci root if related pci root bus get removed before.

I think this is a bad idea.  The ACPI core already walks the entire
ACPI namespace, finding devices and binding drivers to them.

Walking the whole thing again looking for "hotplug" things we can do
is just a hack.  Maybe you want it for debugging, but I don't want it
in Linux.

> Signed-off-by: Yinghai <yinghai@kernel.org>
> Cc: Len Brown <lenb@kernel.org>
> Cc: linux-acpi@vger.kernel.org
> ---
>  drivers/acpi/pci_root_hp.c |   17 +++++++++++++++++
>  include/linux/pci-acpi.h   |    7 +++++++
>  2 files changed, 24 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/acpi/pci_root_hp.c b/drivers/acpi/pci_root_hp.c
> index b453181..ca73d51 100644
> --- a/drivers/acpi/pci_root_hp.c
> +++ b/drivers/acpi/pci_root_hp.c
> @@ -268,3 +268,20 @@ static int __init acpi_pci_root_hp_init(void)
>  }
>
>  subsys_initcall(acpi_pci_root_hp_init);
> +
> +static acpi_status
> +rescan_root_bridge(acpi_handle handle, u32 lvl, void *context, void **rv)
> +{
> +       if (!acpi_is_root_bridge(handle))
> +               return AE_OK;
> +
> +       handle_root_bridge_insertion(handle);
> +
> +       return AE_OK;
> +}
> +
> +void acpi_pci_root_rescan(void)
> +{
> +       acpi_walk_namespace(ACPI_TYPE_DEVICE, ACPI_ROOT_OBJECT,
> +               ACPI_UINT32_MAX, rescan_root_bridge, NULL, NULL, NULL);
> +}
> diff --git a/include/linux/pci-acpi.h b/include/linux/pci-acpi.h
> index 4462350..ac93634 100644
> --- a/include/linux/pci-acpi.h
> +++ b/include/linux/pci-acpi.h
> @@ -35,6 +35,13 @@ static inline acpi_handle acpi_pci_get_bridge_handle(struct pci_bus *pbus)
>        return acpi_get_pci_rootbridge_handle(pci_domain_nr(pbus),
>                                              pbus->number);
>  }
> +
> +void acpi_pci_root_rescan(void);
> +
> +#else
> +
> +static inline void acpi_pci_root_rescan(void) { }
> +
>  #endif
>
>  #ifdef CONFIG_ACPI_APEI
> --
> 1.7.7
>

[PATCH v2 30/37] PCI: Add __pci_scan_root_bus() that can skip bus_add

[Yinghai Lu]

So could insert pus pci_assign_unassigned_bus_resources() before do bus_add.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
 drivers/pci/probe.c |   13 ++++++++++---
 include/linux/pci.h |    3 +++
 2 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 8512e0d..0ca213c 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1953,8 +1953,9 @@ void pci_bus_release_busn_res(struct pci_bus *b)
 			res, ret ? "can not be" : "is");
 }
 
-struct pci_bus * __devinit pci_scan_root_bus(struct device *parent, int bus,
-		struct pci_ops *ops, void *sysdata, struct list_head *resources)
+struct pci_bus * __devinit __pci_scan_root_bus(struct device *parent, int bus,
+		struct pci_ops *ops, void *sysdata, struct list_head *resources,
+		bool bus_add)
 {
 	struct pci_bus *b;
 	struct pci_host_bridge_window *window, *n;
@@ -1985,9 +1986,15 @@ struct pci_bus * __devinit pci_scan_root_bus(struct device *parent, int bus,
 	if (!found)
 		pci_bus_update_busn_res_end(b, b->subordinate);
 
-	pci_bus_add_devices(b);
+	if (bus_add)
+		pci_bus_add_devices(b);
 	return b;
 }
+struct pci_bus * __devinit pci_scan_root_bus(struct device *parent, int bus,
+		struct pci_ops *ops, void *sysdata, struct list_head *resources)
+{
+	return __pci_scan_root_bus(parent, bus, ops, sysdata, resources, true);
+}
 EXPORT_SYMBOL(pci_scan_root_bus);
 
 /* Deprecated; use pci_scan_root_bus() instead */
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 8874b90..ec8c4cf 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -675,6 +675,9 @@ struct pci_bus *pci_create_root_bus(struct device *parent, int bus,
 void pci_bus_insert_busn_res(struct pci_bus *b, int bus, int busmax);
 void pci_bus_update_busn_res_end(struct pci_bus *b, int busmax);
 void pci_bus_release_busn_res(struct pci_bus *b);
+struct pci_bus * __devinit __pci_scan_root_bus(struct device *parent, int bus,
+		struct pci_ops *ops, void *sysdata, struct list_head *resources,
+		bool bus_add);
 struct pci_bus * __devinit pci_scan_root_bus(struct device *parent, int bus,
 					     struct pci_ops *ops, void *sysdata,
 					     struct list_head *resources);
-- 
1.7.7

[PATCH v2 31/37] x86, PCI: add __pci_scan_root_bus_on_node() that can skip bus_add

[Yinghai Lu]

So could insert pus pci_assign_unassigned_bus_resources() before do bus_add.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
 arch/x86/include/asm/pci.h |    2 ++
 arch/x86/pci/common.c      |   11 ++++++++---
 2 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/arch/x86/include/asm/pci.h b/arch/x86/include/asm/pci.h
index df75d07..bb015c7 100644
--- a/arch/x86/include/asm/pci.h
+++ b/arch/x86/include/asm/pci.h
@@ -24,6 +24,8 @@ extern int noioapicquirk;
 extern int noioapicreroute;
 
 /* scan a bus after allocating a pci_sysdata for it */
+struct pci_bus * __devinit __pci_scan_bus_on_node(int busno,
+			 struct pci_ops *ops, int node, bool bus_add);
 extern struct pci_bus *pci_scan_bus_on_node(int busno, struct pci_ops *ops,
 					    int node);
 extern struct pci_bus *pci_scan_bus_with_sysdata(int busno);
diff --git a/arch/x86/pci/common.c b/arch/x86/pci/common.c
index 35931f6..2ebdc231 100644
--- a/arch/x86/pci/common.c
+++ b/arch/x86/pci/common.c
@@ -625,7 +625,8 @@ static void release_pci_sysdata(struct pci_host_bridge *bridge)
 	kfree(sd);
 }
 
-struct pci_bus * __devinit pci_scan_bus_on_node(int busno, struct pci_ops *ops, int node)
+struct pci_bus * __devinit __pci_scan_bus_on_node(int busno,
+			 struct pci_ops *ops, int node, bool bus_add)
 {
 	LIST_HEAD(resources);
 	struct pci_bus *bus = NULL;
@@ -639,7 +640,7 @@ struct pci_bus * __devinit pci_scan_bus_on_node(int busno, struct pci_ops *ops,
 	sd->node = node;
 	x86_pci_root_bus_resources(busno, &resources);
 	printk(KERN_DEBUG "PCI: Probing PCI hardware (bus %02x)\n", busno);
-	bus = pci_scan_root_bus(NULL, busno, ops, sd, &resources);
+	bus = __pci_scan_root_bus(NULL, busno, ops, sd, &resources, bus_add);
 	if (bus)
 		pci_set_host_bridge_release(to_pci_host_bridge(bus->bridge),
 					release_pci_sysdata, sd);
@@ -650,7 +651,11 @@ struct pci_bus * __devinit pci_scan_bus_on_node(int busno, struct pci_ops *ops,
 
 	return bus;
 }
-
+struct pci_bus * __devinit pci_scan_bus_on_node(int busno,
+			 struct pci_ops *ops, int node)
+{
+	return __pci_scan_bus_on_node(busno, ops, node, true);
+}
 struct pci_bus * __devinit pci_scan_bus_with_sysdata(int busno)
 {
 	return pci_scan_bus_on_node(busno, &pci_root_ops, -1);
-- 
1.7.7

[PATCH v2 32/37] x86, PCI: add __pcibios_scan_specific_bus that can skip bus_add

[Yinghai Lu]

So could insert pci_assign_unassigned_bus_resources() before do bus_add.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
 arch/x86/pci/legacy.c |   19 +++++++++++++------
 1 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/arch/x86/pci/legacy.c b/arch/x86/pci/legacy.c
index a1df191..aab0e41 100644
--- a/arch/x86/pci/legacy.c
+++ b/arch/x86/pci/legacy.c
@@ -34,25 +34,32 @@ int __init pci_legacy_init(void)
 	return 0;
 }
 
-void __devinit pcibios_scan_specific_bus(int busn)
+static __devinit struct pci_bus *__pcibios_scan_specific_bus(int busn,
+					 bool bus_add)
 {
 	int devfn;
 	long node;
 	u32 l;
 
-	if (pci_find_bus(0, busn))
-		return;
-
 	node = get_mp_bus_to_node(busn);
 	for (devfn = 0; devfn < 256; devfn += 8) {
 		if (!raw_pci_read(0, busn, devfn, PCI_VENDOR_ID, 2, &l) &&
 		    l != 0x0000 && l != 0xffff) {
 			DBG("Found device at %02x:%02x [%04x]\n", busn, devfn, l);
 			printk(KERN_INFO "PCI: Discovered peer bus %02x\n", busn);
-			pci_scan_bus_on_node(busn, &pci_root_ops, node);
-			return;
+			return __pci_scan_bus_on_node(busn, &pci_root_ops, node,
+					 bus_add);
 		}
 	}
+
+	return NULL;
+}
+void __devinit pcibios_scan_specific_bus(int busn)
+{
+	if (pci_find_bus(0, busn))
+		return;
+
+	__pcibios_scan_specific_bus(busn, true);
 }
 EXPORT_SYMBOL_GPL(pcibios_scan_specific_bus);
 
-- 
1.7.7

[PATCH v2 33/37] x86, PCI: Add pcibios_root_rescan()

[Yinghai Lu]

Need use it to rescan root bus that was not added via acpi probe.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
 arch/x86/include/asm/pci.h |    2 ++
 arch/x86/pci/legacy.c      |   25 +++++++++++++++++++++++++
 2 files changed, 27 insertions(+), 0 deletions(-)

diff --git a/arch/x86/include/asm/pci.h b/arch/x86/include/asm/pci.h
index bb015c7..332c191 100644
--- a/arch/x86/include/asm/pci.h
+++ b/arch/x86/include/asm/pci.h
@@ -30,6 +30,8 @@ extern struct pci_bus *pci_scan_bus_on_node(int busno, struct pci_ops *ops,
 					    int node);
 extern struct pci_bus *pci_scan_bus_with_sysdata(int busno);
 
+void pcibios_root_rescan(void);
+
 #ifdef CONFIG_PCI
 
 #ifdef CONFIG_PCI_DOMAINS
diff --git a/arch/x86/pci/legacy.c b/arch/x86/pci/legacy.c
index aab0e41..251f838 100644
--- a/arch/x86/pci/legacy.c
+++ b/arch/x86/pci/legacy.c
@@ -79,3 +79,28 @@ int __init pci_subsys_init(void)
 	return 0;
 }
 subsys_initcall(pci_subsys_init);
+
+void __ref pcibios_root_rescan(void)
+{
+	int busn;
+	struct pci_bus *bus;
+
+	if (pcibios_last_bus <= 0 || pcibios_last_bus > 0xff)
+		return;
+
+	for (busn = 0; busn <= pcibios_last_bus; busn++) {
+		bus = pci_find_bus(0, busn);
+
+		if (bus)
+			continue;
+
+		bus = __pcibios_scan_specific_bus(busn, false);
+
+		if (!bus)
+			continue;
+
+		pci_assign_unassigned_bus_resources(bus);
+
+		pci_bus_add_devices(bus);
+	}
+}
-- 
1.7.7

Re: [PATCH v2 33/37] x86, PCI: Add pcibios_root_rescan()

[Bjorn Helgaas]

On Sat, Mar 10, 2012 at 12:00 AM, Yinghai Lu <yinghai@kernel.org> wrote:
> Need use it to rescan root bus that was not added via acpi probe.

Why would we ever need to rescan for this?  If we're doing host
bridge hotplug on x86, it's via ACPI, so we'll always have an ACPI
host bridge object.

There's no way to even express the idea of "adding or removing buses
we found by blindly probing," so there's no need to do any kind of
rescanning to find them again.  We find them once at boot-time, and
that's more than enough.

> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> ---
>  arch/x86/include/asm/pci.h |    2 ++
>  arch/x86/pci/legacy.c      |   25 +++++++++++++++++++++++++
>  2 files changed, 27 insertions(+), 0 deletions(-)
>
> diff --git a/arch/x86/include/asm/pci.h b/arch/x86/include/asm/pci.h
> index bb015c7..332c191 100644
> --- a/arch/x86/include/asm/pci.h
> +++ b/arch/x86/include/asm/pci.h
> @@ -30,6 +30,8 @@ extern struct pci_bus *pci_scan_bus_on_node(int busno, struct pci_ops *ops,
>                                            int node);
>  extern struct pci_bus *pci_scan_bus_with_sysdata(int busno);
>
> +void pcibios_root_rescan(void);
> +
>  #ifdef CONFIG_PCI
>
>  #ifdef CONFIG_PCI_DOMAINS
> diff --git a/arch/x86/pci/legacy.c b/arch/x86/pci/legacy.c
> index aab0e41..251f838 100644
> --- a/arch/x86/pci/legacy.c
> +++ b/arch/x86/pci/legacy.c
> @@ -79,3 +79,28 @@ int __init pci_subsys_init(void)
>        return 0;
>  }
>  subsys_initcall(pci_subsys_init);
> +
> +void __ref pcibios_root_rescan(void)
> +{
> +       int busn;
> +       struct pci_bus *bus;
> +
> +       if (pcibios_last_bus <= 0 || pcibios_last_bus > 0xff)
> +               return;
> +
> +       for (busn = 0; busn <= pcibios_last_bus; busn++) {
> +               bus = pci_find_bus(0, busn);
> +
> +               if (bus)
> +                       continue;
> +
> +               bus = __pcibios_scan_specific_bus(busn, false);
> +
> +               if (!bus)
> +                       continue;
> +
> +               pci_assign_unassigned_bus_resources(bus);
> +
> +               pci_bus_add_devices(bus);
> +       }
> +}
> --
> 1.7.7
>

[PATCH v2 34/37] x86, PCI: Add arch version pci_root_rescan()

[Yinghai Lu]

It will call acpi version pci_root_rescan and legacy pci_root_rescan.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
 arch/x86/pci/common.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/arch/x86/pci/common.c b/arch/x86/pci/common.c
index 2ebdc231..0ec860f 100644
--- a/arch/x86/pci/common.c
+++ b/arch/x86/pci/common.c
@@ -10,6 +10,7 @@
 #include <linux/init.h>
 #include <linux/dmi.h>
 #include <linux/slab.h>
+#include <linux/pci-acpi.h>
 
 #include <asm/acpi.h>
 #include <asm/segment.h>
@@ -661,6 +662,12 @@ struct pci_bus * __devinit pci_scan_bus_with_sysdata(int busno)
 	return pci_scan_bus_on_node(busno, &pci_root_ops, -1);
 }
 
+void arch_pci_root_rescan(void)
+{
+	acpi_pci_root_rescan();
+	pcibios_root_rescan();
+}
+
 /*
  * NUMA info for PCI busses
  *
-- 
1.7.7

Re: [PATCH v2 34/37] x86, PCI: Add arch version pci_root_rescan()

[Bjorn Helgaas]

On Sat, Mar 10, 2012 at 12:00 AM, Yinghai Lu <yinghai@kernel.org> wrote:
> It will call acpi version pci_root_rescan and legacy pci_root_rescan.
>
> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> ---
>  arch/x86/pci/common.c |    7 +++++++
>  1 files changed, 7 insertions(+), 0 deletions(-)
>
> diff --git a/arch/x86/pci/common.c b/arch/x86/pci/common.c
> index 2ebdc231..0ec860f 100644
> --- a/arch/x86/pci/common.c
> +++ b/arch/x86/pci/common.c
> @@ -10,6 +10,7 @@
>  #include <linux/init.h>
>  #include <linux/dmi.h>
>  #include <linux/slab.h>
> +#include <linux/pci-acpi.h>
>
>  #include <asm/acpi.h>
>  #include <asm/segment.h>
> @@ -661,6 +662,12 @@ struct pci_bus * __devinit pci_scan_bus_with_sysdata(int busno)
>        return pci_scan_bus_on_node(busno, &pci_root_ops, -1);
>  }
>
> +void arch_pci_root_rescan(void)
> +{
> +       acpi_pci_root_rescan();
> +       pcibios_root_rescan();

I don't think we need this either.

We don't need pcibios_root_rescan() because there's no way to add or
remove buses except via the normal ACPI hotplug mechanisms.  (Well,
you did add sysfs ways to do it, but I don't like those either, and
they certainly don't work for any kind of normal hotplug users might
do.)

We don't need acpi_pci_root_rescan(), because for actual host bridge
add/remove, we will get ACPI notify events, and we can use the normal
ways of handling those events.  For debug purposes, we can have a
sysfs interface that kicks things off the same way a notify event
would.

> +}
> +
>  /*
>  * NUMA info for PCI busses
>  *
> --
> 1.7.7
>

[PATCH v2 35/37] PCI: Make /sys/bus/pci/rescan rescan root

[Yinghai Lu]

It will rediscover removed pci root buses.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
 drivers/pci/pci-sysfs.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index 4d122cb..fc0a7de 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -284,6 +284,7 @@ msi_bus_store(struct device *dev, struct device_attribute *attr,
 
 #ifdef CONFIG_HOTPLUG
 static DEFINE_MUTEX(pci_remove_rescan_mutex);
+void __weak arch_pci_root_rescan(void) { }
 static ssize_t bus_rescan_store(struct bus_type *bus, const char *buf,
 				size_t count)
 {
@@ -295,6 +296,7 @@ static ssize_t bus_rescan_store(struct bus_type *bus, const char *buf,
 
 	if (val) {
 		mutex_lock(&pci_remove_rescan_mutex);
+		arch_pci_root_rescan();
 		while ((b = pci_find_next_bus(b)) != NULL)
 			pci_rescan_bus(b);
 		mutex_unlock(&pci_remove_rescan_mutex);
-- 
1.7.7

Re: [PATCH v2 35/37] PCI: Make /sys/bus/pci/rescan rescan root

[Bjorn Helgaas]

On Sat, Mar 10, 2012 at 12:00 AM, Yinghai Lu <yinghai@kernel.org> wrote:
> It will rediscover removed pci root buses.
>
> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> ---
>  drivers/pci/pci-sysfs.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
> index 4d122cb..fc0a7de 100644
> --- a/drivers/pci/pci-sysfs.c
> +++ b/drivers/pci/pci-sysfs.c
> @@ -284,6 +284,7 @@ msi_bus_store(struct device *dev, struct device_attribute *attr,
>
>  #ifdef CONFIG_HOTPLUG
>  static DEFINE_MUTEX(pci_remove_rescan_mutex);
> +void __weak arch_pci_root_rescan(void) { }
>  static ssize_t bus_rescan_store(struct bus_type *bus, const char *buf,
>                                size_t count)
>  {
> @@ -295,6 +296,7 @@ static ssize_t bus_rescan_store(struct bus_type *bus, const char *buf,
>
>        if (val) {
>                mutex_lock(&pci_remove_rescan_mutex);
> +               arch_pci_root_rescan();
>                while ((b = pci_find_next_bus(b)) != NULL)
>                        pci_rescan_bus(b);
>                mutex_unlock(&pci_remove_rescan_mutex);

This is gross.  We used to rescan every previously-discovered PCI root bus.

Now, we do the same, plus:
1) Scan the ACPI namespace for PCI host bridges, adding and scanning
any we haven't seen before, and
2) scan domain 0, buses 0 - pcibios_last_bus, looking for any we
haven't seen before.

These just don't fit with the previous functionality.  The scan should
be done on the *ACPI namespace*, which is where any new bridges would
live.  It shouldn't be done in the PCI namespace.  The new bridges
aren't part of PCI.

Re: [PATCH v2 35/37] PCI: Make /sys/bus/pci/rescan rescan root

[Yinghai Lu]

On Mon, Mar 12, 2012 at 8:44 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> On Sat, Mar 10, 2012 at 12:00 AM, Yinghai Lu <yinghai@kernel.org> wrote:
>> It will rediscover removed pci root buses.
>>
>> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
>> ---
>>  drivers/pci/pci-sysfs.c |    2 ++
>>  1 files changed, 2 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
>> index 4d122cb..fc0a7de 100644
>> --- a/drivers/pci/pci-sysfs.c
>> +++ b/drivers/pci/pci-sysfs.c
>> @@ -284,6 +284,7 @@ msi_bus_store(struct device *dev, struct device_attribute *attr,
>>
>>  #ifdef CONFIG_HOTPLUG
>>  static DEFINE_MUTEX(pci_remove_rescan_mutex);
>> +void __weak arch_pci_root_rescan(void) { }
>>  static ssize_t bus_rescan_store(struct bus_type *bus, const char *buf,
>>                                size_t count)
>>  {
>> @@ -295,6 +296,7 @@ static ssize_t bus_rescan_store(struct bus_type *bus, const char *buf,
>>
>>        if (val) {
>>                mutex_lock(&pci_remove_rescan_mutex);
>> +               arch_pci_root_rescan();
>>                while ((b = pci_find_next_bus(b)) != NULL)
>>                        pci_rescan_bus(b);
>>                mutex_unlock(&pci_remove_rescan_mutex);
>
> This is gross.  We used to rescan every previously-discovered PCI root bus.
>
> Now, we do the same, plus:
> 1) Scan the ACPI namespace for PCI host bridges, adding and scanning
> any we haven't seen before, and
> 2) scan domain 0, buses 0 - pcibios_last_bus, looking for any we
> haven't seen before.

to have /sys/bus/pci/scan_root again?

Yinghai

Re: [PATCH v2 35/37] PCI: Make /sys/bus/pci/rescan rescan root

[Bjorn Helgaas]

On Tue, Mar 13, 2012 at 12:26 AM, Yinghai Lu <yinghai@kernel.org> wrote:
> On Mon, Mar 12, 2012 at 8:44 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
>> On Sat, Mar 10, 2012 at 12:00 AM, Yinghai Lu <yinghai@kernel.org> wrote:
>>> It will rediscover removed pci root buses.
>>>
>>> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
>>> ---
>>>  drivers/pci/pci-sysfs.c |    2 ++
>>>  1 files changed, 2 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
>>> index 4d122cb..fc0a7de 100644
>>> --- a/drivers/pci/pci-sysfs.c
>>> +++ b/drivers/pci/pci-sysfs.c
>>> @@ -284,6 +284,7 @@ msi_bus_store(struct device *dev, struct device_attribute *attr,
>>>
>>>  #ifdef CONFIG_HOTPLUG
>>>  static DEFINE_MUTEX(pci_remove_rescan_mutex);
>>> +void __weak arch_pci_root_rescan(void) { }
>>>  static ssize_t bus_rescan_store(struct bus_type *bus, const char *buf,
>>>                                size_t count)
>>>  {
>>> @@ -295,6 +296,7 @@ static ssize_t bus_rescan_store(struct bus_type *bus, const char *buf,
>>>
>>>        if (val) {
>>>                mutex_lock(&pci_remove_rescan_mutex);
>>> +               arch_pci_root_rescan();
>>>                while ((b = pci_find_next_bus(b)) != NULL)
>>>                        pci_rescan_bus(b);
>>>                mutex_unlock(&pci_remove_rescan_mutex);
>>
>> This is gross.  We used to rescan every previously-discovered PCI root bus.
>>
>> Now, we do the same, plus:
>> 1) Scan the ACPI namespace for PCI host bridges, adding and scanning
>> any we haven't seen before, and
>> 2) scan domain 0, buses 0 - pcibios_last_bus, looking for any we
>> haven't seen before.
>
> to have /sys/bus/pci/scan_root again?

Host bridges are not PCI devices, so scanning for them is not a PCI
operation.  It should not be in the /sys/bus/pci hierarchy.

[PATCH v2 36/37] ACPI: Enable SCI_EMULATE to manually simulate physical hotplug testing.

[Yinghai Lu]

From: Ashok Raj <ashok.raj@intel.com>

Emulate an ACPI SCI interrupt to emulate a hot-plug event. Useful
for testing ACPI based hot-plug on systems that don't have the
necessary firmware support.

Enable CONFIG_ACPI_SCI_EMULATE on kernel compile.

Now you will notice /proc/acpi/sci/notify when new kernel is booted.

echo "\_SB.CPU4 1" > /proc/acpi/sci/notify to trigger a hot-add of CPU4.
You will now notice an entry /sys/firmware/acpi/namespace/ACPI/_SB/CPU4
if the namespace had an entry CPU4 under _SB scope. If the entry had a
_EJ0 method, you will also notice a file "eject" under the CPU4 directory.

-v2: Update to current upstream, and remove not related stuff.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
Cc: Len Brown <lenb@kernel.org>
Cc: linux-acpi@vger.kernel.org

===================================================================
---
 drivers/acpi/Kconfig    |   10 +++
 drivers/acpi/Makefile   |    1 +
 drivers/acpi/bus.c      |    2 +
 drivers/acpi/internal.h |    6 ++
 drivers/acpi/sci_emu.c  |  141 +++++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 160 insertions(+), 0 deletions(-)
 create mode 100644 drivers/acpi/sci_emu.c

diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index 7556913..b7b8541 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -272,6 +272,16 @@ config ACPI_BLACKLIST_YEAR
 	  Enter 0 to disable this mechanism and allow ACPI to
 	  run by default no matter what the year.  (default)
 
+config ACPI_SCI_EMULATE
+	bool "ACPI SCI Event Emulation Support"
+	depends on ACPI
+	default n
+	help
+	  This will enable your system to emulate sci hotplug event
+	  notification through proc file system. For example user needs to
+	  echo "XXX 0" > /proc/acpi/sci/notify (where, XXX is a target ACPI
+	  device object name present under \_SB scope).
+
 config ACPI_DEBUG
 	bool "Debug Statements"
 	default n
diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
index bc6e53f..3580f04 100644
--- a/drivers/acpi/Makefile
+++ b/drivers/acpi/Makefile
@@ -31,6 +31,7 @@ acpi-$(CONFIG_ACPI_SLEEP)	+= proc.o
 # ACPI Bus and Device Drivers
 #
 acpi-y				+= bus.o glue.o
+acpi-$(CONFIG_ACPI_SCI_EMULATE)	+= sci_emu.o
 acpi-y				+= scan.o
 acpi-y				+= processor_core.o
 acpi-y				+= ec.o
diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
index 9ecec98..512235e 100644
--- a/drivers/acpi/bus.c
+++ b/drivers/acpi/bus.c
@@ -1001,6 +1001,8 @@ static int __init acpi_bus_init(void)
 	 */
 	acpi_root_dir = proc_mkdir(ACPI_BUS_FILE_ROOT, NULL);
 
+	acpi_init_sci_emulate();
+
 	return 0;
 
 	/* Mimic structured exception handling */
diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h
index ca75b9c..5b22cd2 100644
--- a/drivers/acpi/internal.h
+++ b/drivers/acpi/internal.h
@@ -34,6 +34,12 @@ int acpi_debugfs_init(void);
 static inline void acpi_debugfs_init(void) { return; }
 #endif
 
+#ifdef CONFIG_ACPI_SCI_EMULATE
+int acpi_init_sci_emulate(void);
+#else
+static inline int acpi_init_sci_emulate(void) { return 0; }
+#endif
+
 /* --------------------------------------------------------------------------
                                   Power Resource
    -------------------------------------------------------------------------- */
diff --git a/drivers/acpi/sci_emu.c b/drivers/acpi/sci_emu.c
new file mode 100644
index 0000000..d972436
--- /dev/null
+++ b/drivers/acpi/sci_emu.c
@@ -0,0 +1,141 @@
+/*
+ *  Code to emulate SCI interrupt for Hotplug node insertion/removal
+ */
+#include <linux/kernel.h>
+#include <linux/proc_fs.h>
+#include <linux/acpi.h>
+
+#include "internal.h"
+
+#include "acpica/accommon.h"
+#include "acpica/acnamesp.h"
+#include "acpica/acevents.h"
+
+#define _COMPONENT		ACPI_SYSTEM_COMPONENT
+ACPI_MODULE_NAME("sci_emu");
+
+static void acpi_sci_notify_client(char *acpi_name, u32 event);
+static int acpi_sci_notify_write_proc(struct file *file, const char *buffer, \
+	unsigned long count, void *data);
+struct proc_dir_entry *acpi_sci_dir;
+
+static int acpi_sci_notify_write_proc(struct file *file, const char *buffer,
+				      unsigned long count, void *data)
+{
+	u32 event;
+	char *name1 = NULL;
+	char *name2 = NULL;
+	char *end_name = NULL;
+	const char *delim = " ";
+	char *temp_buf = NULL;
+	char *temp_buf_addr = NULL;
+
+	temp_buf = kmalloc(count+1, GFP_ATOMIC);
+	if (!temp_buf) {
+		printk(KERN_WARNING PREFIX
+		 "acpi_sci_notify_wire_proc: Memory allocation failed\n");
+		return count;
+	}
+	temp_buf[count] = '\0';
+	temp_buf_addr = temp_buf;
+	memcpy(temp_buf, buffer, count);
+	name1 = strsep(&temp_buf, delim);
+	name2 = strsep(&temp_buf, delim);
+
+	if (name1 && name2)
+		event = simple_strtoul(name2, &end_name, 10);
+	else {
+		printk(KERN_WARNING PREFIX "unknown device\n");
+		kfree(temp_buf_addr);
+		return count;
+	}
+
+	printk(KERN_INFO PREFIX
+		"ACPI device name is <%s>, event code is <%d>\n",
+		name1, event);
+
+	acpi_sci_notify_client(name1, event);
+
+	kfree(temp_buf_addr);
+
+	return count;
+}
+
+static void acpi_sci_notify_client(char *acpi_name, u32 event)
+{
+	struct acpi_namespace_node *node;
+	acpi_status status, status1;
+	acpi_handle hlsb, hsb;
+	union acpi_operand_object *obj_desc;
+
+	status = acpi_get_handle(NULL, "\\_SB", &hsb);
+	status1 = acpi_get_handle(hsb, acpi_name, &hlsb);
+	if (ACPI_FAILURE(status) || ACPI_FAILURE(status1)) {
+		printk(KERN_ERR PREFIX
+	"acpi getting handle to <\\_SB.%s> failed inside notify_client\n",
+			acpi_name);
+		return;
+	}
+
+	status = acpi_ut_acquire_mutex(ACPI_MTX_NAMESPACE);
+	if (ACPI_FAILURE(status)) {
+		printk(KERN_ERR PREFIX "Acquiring acpi namespace mutext failed\n");
+		return;
+	}
+
+	node = acpi_ns_validate_handle(hlsb);
+	if (!node) {
+		acpi_ut_release_mutex(ACPI_MTX_NAMESPACE);
+		printk(KERN_ERR PREFIX "Mapping handle to node failed\n");
+		return;
+	}
+
+	/*
+	 * Check for internal object and make sure there is a handler
+	 * registered for this object
+	 */
+	obj_desc = acpi_ns_get_attached_object(node);
+	if (obj_desc) {
+		if (obj_desc->common_notify.system_notify) {
+			/*
+			 * Release the lock and queue the item for later
+			 * exectuion
+			 */
+			acpi_ut_release_mutex(ACPI_MTX_NAMESPACE);
+			status = acpi_ev_queue_notify_request(node, event);
+			if (ACPI_FAILURE(status))
+				printk(KERN_ERR PREFIX "acpi_ev_queue_notify_request failed\n");
+			else
+				printk(KERN_INFO PREFIX "Notify event is queued\n");
+			return;
+		}
+	} else {
+		printk(KERN_INFO PREFIX "Notify handler not registered for this device\n");
+	}
+
+	acpi_ut_release_mutex(ACPI_MTX_NAMESPACE);
+	return;
+}
+
+int __init acpi_init_sci_emulate(void)
+{
+	struct proc_dir_entry   *notify_entry = NULL;
+
+	ACPI_FUNCTION_TRACE("acpi_init_sci_emulate");
+
+	acpi_sci_dir = proc_mkdir("sci", acpi_root_dir);
+	if (!acpi_sci_dir)
+		return_VALUE(-ENODEV);
+
+	notify_entry = create_proc_entry("notify", \
+		S_IWUGO|S_IRUGO, acpi_sci_dir);
+	if (!notify_entry) {
+		ACPI_DEBUG_PRINT((ACPI_DB_INIT,
+			"Unable to create '%s' fs entry\n", "notify"));
+	} else {
+		notify_entry->write_proc = acpi_sci_notify_write_proc;
+		notify_entry->data = NULL;
+	}
+
+	return_VALUE(0);
+}
-- 
1.7.7

[PATCH v2 37/37] PCI, sysfs: Prepare to kill pci device rescan

[Yinghai Lu]

Hope we can kill it after one year.

Print warning to catch real users for that feature.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
 Documentation/feature-removal-schedule.txt |    9 +++++++++
 drivers/pci/pci-sysfs.c                    |    3 +++
 2 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/Documentation/feature-removal-schedule.txt b/Documentation/feature-removal-schedule.txt
index a0ffac0..1b984a1 100644
--- a/Documentation/feature-removal-schedule.txt
+++ b/Documentation/feature-removal-schedule.txt
@@ -299,6 +299,15 @@ Why:	In 2.6.27, the semantics of /sys/bus/pci/slots was redefined to
 	fakephp interface.
 Who:	Alex Chiang <achiang@hp.com>
 
+-----------------------------
+
+What:	/sys/bus/pci/devices/.../rescan
+When:	April 2013
+Why:	That rescan will rescan pci parent's bus. It is confusing.
+	Now we have bridge rescan_bridge and bus rescan, and they could
+	be used on exact device and bus that need to be rescaned.
+Who:	Yinghai Lu <yinghai@kernel.org>
+
 ---------------------------
 
 What:	CONFIG_RFKILL_INPUT
diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index fc0a7de..c180455 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -320,6 +320,9 @@ dev_rescan_store(struct device *dev, struct device_attribute *attr,
 		return -EINVAL;
 
 	if (val) {
+		printk(KERN_WARNING "rescan with pci device will be removed "
+			 "shortly, please use bridge rescan_bridge\n"
+			 "or bus/rescan instead\n");
 		mutex_lock(&pci_remove_rescan_mutex);
 		pci_rescan_bus(pdev->bus);
 		mutex_unlock(&pci_remove_rescan_mutex);
-- 
1.7.7