2014-01-18 02:47:47

by Jiang Liu

Subject: [Patch v1 1/3] ACPI: add callback prepare() into acpi_hotplug_handler

Add callback prepare() into acpi_hotplug_handler, which will get called
at the very beginning of ACPI hotplug event handler. The ACPI core will
ignore the event if prepare() returns NOTIFY_STOP.

Signed-off-by: Jiang Liu <[email protected]>
---
drivers/acpi/scan.c | 4 ++++
include/acpi/acpi_bus.h | 1 +
2 files changed, 5 insertions(+)

diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index fd39459..6b0f419 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -392,6 +392,10 @@ static void acpi_hotplug_notify_cb(acpi_handle handle, u32 type, void *data)
struct acpi_device *adev;
acpi_status status;

+ if (handler->prepare &&
+ handler->prepare(handle, type, data) == NOTIFY_STOP)
+ return;
+
if (!handler->hotplug.enabled)
return acpi_hotplug_unsupported(handle, type);

diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index ddabed1..09a73bd 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -113,6 +113,7 @@ static inline struct acpi_hotplug_profile *to_acpi_hotplug_profile(
struct acpi_scan_handler {
const struct acpi_device_id *ids;
struct list_head list_node;
+ int (*prepare)(acpi_handle handle, u32 type, void *context);
int (*attach)(struct acpi_device *dev, const struct acpi_device_id *id);
void (*detach)(struct acpi_device *dev);
struct acpi_hotplug_profile hotplug;
--
1.7.10.4

2014-01-18 02:48:21

by Jiang Liu

Subject: [Patch v1 2/3] ACPI, PCI: reuse ACPI hotplug framework to support PCI host bridge hotplug

Reuse ACPI hotplug framework to support PCI host bridge hotplug, this
makes PCI host bridge hotplug implementation simpler and more clear.

It also fixes a bug in support of PCI host bridge hot-addition.
Currently pci_root driver fails to install notification handler for
PCI host bridge absent at boot time because acpi_is_root_bridge()
returns false if no ACPI device created for handle. So PCI host
bridge hot-addition event will just be ignored by system.

Signed-off-by: Jiang Liu <[email protected]>
---
drivers/acpi/internal.h | 1 -
drivers/acpi/pci_root.c | 117 +++++++----------------------------------------
drivers/acpi/scan.c | 2 -
3 files changed, 17 insertions(+), 103 deletions(-)

diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h
index a29739c..03efa56 100644
--- a/drivers/acpi/internal.h
+++ b/drivers/acpi/internal.h
@@ -28,7 +28,6 @@ int init_acpi_device_notify(void);
int acpi_scan_init(void);
void acpi_pci_root_init(void);
void acpi_pci_link_init(void);
-void acpi_pci_root_hp_init(void);
void acpi_processor_init(void);
void acpi_platform_init(void);
int acpi_sysfs_init(void);
diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
index 20360e4..6f6e6c1 100644
--- a/drivers/acpi/pci_root.c
+++ b/drivers/acpi/pci_root.c
@@ -50,6 +50,7 @@ ACPI_MODULE_NAME("pci_root");
static int acpi_pci_root_add(struct acpi_device *device,
const struct acpi_device_id *not_used);
static void acpi_pci_root_remove(struct acpi_device *device);
+static int handle_hotplug_event_root(acpi_handle handle, u32 type, void *ctx);

#define ACPI_PCIE_REQ_SUPPORT (OSC_PCI_EXT_CONFIG_SUPPORT \
| OSC_PCI_ASPM_SUPPORT \
@@ -61,12 +62,14 @@ static const struct acpi_device_id root_device_ids[] = {
{"", 0},
};

+
static struct acpi_scan_handler pci_root_handler = {
.ids = root_device_ids,
+ .prepare = handle_hotplug_event_root,
.attach = acpi_pci_root_add,
.detach = acpi_pci_root_remove,
.hotplug = {
- .ignore = true,
+ .enabled = true,
},
};

@@ -627,113 +630,27 @@ void __init acpi_pci_root_init(void)

if (!acpi_pci_disabled) {
pci_acpi_crs_quirks();
- acpi_scan_add_handler(&pci_root_handler);
- }
-}
-/* Support root bridge hotplug */
-
-static void handle_root_bridge_insertion(acpi_handle handle)
-{
- struct acpi_device *device;
-
- if (!acpi_bus_get_device(handle, &device)) {
- dev_printk(KERN_DEBUG, &device->dev,
- "acpi device already exists; ignoring notify\n");
- return;
+ acpi_scan_add_handler_with_hotplug(&pci_root_handler,
+ "pci_hostbridge");
}
-
- if (acpi_bus_scan(handle))
- acpi_handle_err(handle, "cannot add bridge to acpi list\n");
}

-static void hotplug_event_root(void *data, u32 type)
+static int handle_hotplug_event_root(acpi_handle handle, u32 type, void *ctx)
{
- acpi_handle handle = data;
+ int ret = NOTIFY_OK;
struct acpi_pci_root *root;

- acpi_scan_lock_acquire();
-
- root = acpi_pci_find_root(handle);
-
- switch (type) {
- case ACPI_NOTIFY_BUS_CHECK:
- /* bus enumerate */
- acpi_handle_printk(KERN_DEBUG, handle,
- "Bus check notify on %s\n", __func__);
- if (root)
+ if (type == ACPI_NOTIFY_BUS_CHECK) {
+ acpi_scan_lock_acquire();
+ root = acpi_pci_find_root(handle);
+ if (root) {
+ acpi_handle_printk(KERN_DEBUG, handle,
+ "Bus check notify on %s\n", __func__);
acpiphp_check_host_bridge(handle);
- else
- handle_root_bridge_insertion(handle);
-
- break;
-
- case ACPI_NOTIFY_DEVICE_CHECK:
- /* device check */
- acpi_handle_printk(KERN_DEBUG, handle,
- "Device check notify on %s\n", __func__);
- if (!root)
- handle_root_bridge_insertion(handle);
- break;
-
- case ACPI_NOTIFY_EJECT_REQUEST:
- /* request device eject */
- acpi_handle_printk(KERN_DEBUG, handle,
- "Device eject notify on %s\n", __func__);
- if (!root)
- break;
-
- get_device(&root->device->dev);
-
+ ret = NOTIFY_STOP;
+ }
acpi_scan_lock_release();
-
- acpi_bus_device_eject(root->device, ACPI_NOTIFY_EJECT_REQUEST);
- return;
- default:
- acpi_handle_warn(handle,
- "notify_handler: unknown event type 0x%x\n",
- type);
- break;
}

- acpi_scan_lock_release();
-}
-
-static void handle_hotplug_event_root(acpi_handle handle, u32 type,
- void *context)
-{
- acpi_hotplug_execute(hotplug_event_root, handle, type);
-}
-
-static acpi_status __init
-find_root_bridges(acpi_handle handle, u32 lvl, void *context, void **rv)
-{
- acpi_status status;
- int *count = (int *)context;
-
- if (!acpi_is_root_bridge(handle))
- return AE_OK;
-
- (*count)++;
-
- status = acpi_install_notify_handler(handle, ACPI_SYSTEM_NOTIFY,
- handle_hotplug_event_root, NULL);
- if (ACPI_FAILURE(status))
- acpi_handle_printk(KERN_DEBUG, handle,
- "notify handler is not installed, exit status: %u\n",
- (unsigned int)status);
- else
- acpi_handle_printk(KERN_DEBUG, handle,
- "notify handler is installed\n");
-
- return AE_OK;
-}
-
-void __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 ret;
}
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 6b0f419..d83e0ff 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -2057,8 +2057,6 @@ int __init acpi_scan_init(void)

acpi_update_all_gpes();

- acpi_pci_root_hp_init();
-
out:
mutex_unlock(&acpi_scan_lock);
return result;
--
1.7.10.4

2014-01-18 02:53:08

by Jiang Liu

Subject: [Patch v1 3/3] ACPI: kill field 'ignore' in acpi_hotplug_profile

Field 'ignore' in acpi_hotplug_profile is introduced by "ca499fc87ed945
ACPI / hotplug: Fix conflicted PCI bridge notify handlers" to support
PCI host bridge hotplug. Now it's useless, so kill it.

Signed-off-by: Jiang Liu <[email protected]>
---
drivers/acpi/scan.c | 2 +-
include/acpi/acpi_bus.h | 1 -
2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index d83e0ff..b932ae6 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -1776,7 +1776,7 @@ static void acpi_scan_init_hotplug(acpi_handle handle, int type)
*/
list_for_each_entry(hwid, &pnp.ids, list) {
handler = acpi_scan_match_handler(hwid->id, NULL);
- if (handler && !handler->hotplug.ignore) {
+ if (handler) {
acpi_install_notify_handler(handle, ACPI_SYSTEM_NOTIFY,
acpi_hotplug_notify_cb, handler);
break;
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index 09a73bd..d1f6ebd 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -100,7 +100,6 @@ enum acpi_hotplug_mode {
struct acpi_hotplug_profile {
struct kobject kobj;
bool enabled:1;
- bool ignore:1;
enum acpi_hotplug_mode mode;
};

--
1.7.10.4

2014-01-18 03:23:38

by Yinghai Lu

Subject: Re: [Patch v1 2/3] ACPI, PCI: reuse ACPI hotplug framework to support PCI host bridge hotplug

On Fri, Jan 17, 2014 at 6:48 PM, Jiang Liu <[email protected]> wrote:
> Reuse ACPI hotplug framework to support PCI host bridge hotplug, this
> makes PCI host bridge hotplug implementation simpler and more clear.
>
> It also fixes a bug in support of PCI host bridge hot-addition.
> Currently pci_root driver fails to install notification handler for
> PCI host bridge absent at boot time because acpi_is_root_bridge()
> returns false if no ACPI device created for handle. So PCI host
> bridge hot-addition event will just be ignored by system.
>
> Signed-off-by: Jiang Liu <[email protected]>

is the same as

commit 3338db0057ed9f554050bd06863731c515d79672
Author: Rafael J. Wysocki <[email protected]>
Date: Fri Nov 22 21:55:20 2013 +0100

ACPI / hotplug: Make ACPI PCI root hotplug use common hotplug code

in rafael tree pm/linux-next for 3.14?

> ---
> drivers/acpi/internal.h | 1 -
> drivers/acpi/pci_root.c | 117 +++++++----------------------------------------
> drivers/acpi/scan.c | 2 -
> 3 files changed, 17 insertions(+), 103 deletions(-)
>
> diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h
> index a29739c..03efa56 100644
> --- a/drivers/acpi/internal.h
> +++ b/drivers/acpi/internal.h
> @@ -28,7 +28,6 @@ int init_acpi_device_notify(void);
> int acpi_scan_init(void);
> void acpi_pci_root_init(void);
> void acpi_pci_link_init(void);
> -void acpi_pci_root_hp_init(void);
> void acpi_processor_init(void);
> void acpi_platform_init(void);
> int acpi_sysfs_init(void);
> diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
> index 20360e4..6f6e6c1 100644
> --- a/drivers/acpi/pci_root.c
> +++ b/drivers/acpi/pci_root.c
> @@ -50,6 +50,7 @@ ACPI_MODULE_NAME("pci_root");
> static int acpi_pci_root_add(struct acpi_device *device,
> const struct acpi_device_id *not_used);
> static void acpi_pci_root_remove(struct acpi_device *device);
> +static int handle_hotplug_event_root(acpi_handle handle, u32 type, void *ctx);
>
> #define ACPI_PCIE_REQ_SUPPORT (OSC_PCI_EXT_CONFIG_SUPPORT \
> | OSC_PCI_ASPM_SUPPORT \
> @@ -61,12 +62,14 @@ static const struct acpi_device_id root_device_ids[] = {
> {"", 0},
> };
>
> +
> static struct acpi_scan_handler pci_root_handler = {
> .ids = root_device_ids,
> + .prepare = handle_hotplug_event_root,
> .attach = acpi_pci_root_add,
> .detach = acpi_pci_root_remove,
> .hotplug = {
> - .ignore = true,
> + .enabled = true,
> },
> };
>
> @@ -627,113 +630,27 @@ void __init acpi_pci_root_init(void)
>
> if (!acpi_pci_disabled) {
> pci_acpi_crs_quirks();
> - acpi_scan_add_handler(&pci_root_handler);
> - }
> -}
> -/* Support root bridge hotplug */
> -
> -static void handle_root_bridge_insertion(acpi_handle handle)
> -{
> - struct acpi_device *device;
> -
> - if (!acpi_bus_get_device(handle, &device)) {
> - dev_printk(KERN_DEBUG, &device->dev,
> - "acpi device already exists; ignoring notify\n");
> - return;
> + acpi_scan_add_handler_with_hotplug(&pci_root_handler,
> + "pci_hostbridge");
> }
> -
> - if (acpi_bus_scan(handle))
> - acpi_handle_err(handle, "cannot add bridge to acpi list\n");
> }
>
> -static void hotplug_event_root(void *data, u32 type)
> +static int handle_hotplug_event_root(acpi_handle handle, u32 type, void *ctx)
> {
> - acpi_handle handle = data;
> + int ret = NOTIFY_OK;
> struct acpi_pci_root *root;
>
> - acpi_scan_lock_acquire();
> -
> - root = acpi_pci_find_root(handle);
> -
> - switch (type) {
> - case ACPI_NOTIFY_BUS_CHECK:
> - /* bus enumerate */
> - acpi_handle_printk(KERN_DEBUG, handle,
> - "Bus check notify on %s\n", __func__);
> - if (root)
> + if (type == ACPI_NOTIFY_BUS_CHECK) {
> + acpi_scan_lock_acquire();
> + root = acpi_pci_find_root(handle);
> + if (root) {
> + acpi_handle_printk(KERN_DEBUG, handle,
> + "Bus check notify on %s\n", __func__);
> acpiphp_check_host_bridge(handle);
> - else
> - handle_root_bridge_insertion(handle);
> -
> - break;
> -
> - case ACPI_NOTIFY_DEVICE_CHECK:
> - /* device check */
> - acpi_handle_printk(KERN_DEBUG, handle,
> - "Device check notify on %s\n", __func__);
> - if (!root)
> - handle_root_bridge_insertion(handle);
> - break;
> -
> - case ACPI_NOTIFY_EJECT_REQUEST:
> - /* request device eject */
> - acpi_handle_printk(KERN_DEBUG, handle,
> - "Device eject notify on %s\n", __func__);
> - if (!root)
> - break;
> -
> - get_device(&root->device->dev);
> -
> + ret = NOTIFY_STOP;
> + }
> acpi_scan_lock_release();
> -
> - acpi_bus_device_eject(root->device, ACPI_NOTIFY_EJECT_REQUEST);
> - return;
> - default:
> - acpi_handle_warn(handle,
> - "notify_handler: unknown event type 0x%x\n",
> - type);
> - break;
> }
>
> - acpi_scan_lock_release();
> -}
> -
> -static void handle_hotplug_event_root(acpi_handle handle, u32 type,
> - void *context)
> -{
> - acpi_hotplug_execute(hotplug_event_root, handle, type);
> -}
> -
> -static acpi_status __init
> -find_root_bridges(acpi_handle handle, u32 lvl, void *context, void **rv)
> -{
> - acpi_status status;
> - int *count = (int *)context;
> -
> - if (!acpi_is_root_bridge(handle))
> - return AE_OK;
> -
> - (*count)++;
> -
> - status = acpi_install_notify_handler(handle, ACPI_SYSTEM_NOTIFY,
> - handle_hotplug_event_root, NULL);
> - if (ACPI_FAILURE(status))
> - acpi_handle_printk(KERN_DEBUG, handle,
> - "notify handler is not installed, exit status: %u\n",
> - (unsigned int)status);
> - else
> - acpi_handle_printk(KERN_DEBUG, handle,
> - "notify handler is installed\n");
> -
> - return AE_OK;
> -}
> -
> -void __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 ret;
> }
> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> index 6b0f419..d83e0ff 100644
> --- a/drivers/acpi/scan.c
> +++ b/drivers/acpi/scan.c
> @@ -2057,8 +2057,6 @@ int __init acpi_scan_init(void)
>
> acpi_update_all_gpes();
>
> - acpi_pci_root_hp_init();
> -
> out:
> mutex_unlock(&acpi_scan_lock);
> return result;
> --
> 1.7.10.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

2014-01-18 03:48:32

by Jiang Liu

Subject: Re: [Patch v1 2/3] ACPI, PCI: reuse ACPI hotplug framework to support PCI host bridge hotplug

Hi yinghai,
Sorry for the noise. I didn't noticed Rafael's work,
so I generated this patchset when encountered this issue
during testing PCI host bridge hotplug. It should achieve
the same goal.
Thanks!
Gerry

On 2014/1/18 11:23, Yinghai Lu wrote:
> ACPI / hotplug: Make ACPI PCI root hotplug use common hotplug code

2014-01-21 21:21:08

by Toshi Kani

Subject: Re: [Patch v1 1/3] ACPI: add callback prepare() into acpi_hotplug_handler

On Sat, 2014-01-18 at 10:48 +0800, Jiang Liu wrote:
> Add callback prepare() into acpi_hotplug_handler, which will get called
> at the very beginning of ACPI hotplug event handler. The ACPI core will
> ignore the event if prepare() returns NOTIFY_STOP.
>
> Signed-off-by: Jiang Liu <[email protected]>
> ---
> drivers/acpi/scan.c | 4 ++++
> include/acpi/acpi_bus.h | 1 +
> 2 files changed, 5 insertions(+)
>
> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> index fd39459..6b0f419 100644
> --- a/drivers/acpi/scan.c
> +++ b/drivers/acpi/scan.c
> @@ -392,6 +392,10 @@ static void acpi_hotplug_notify_cb(acpi_handle handle, u32 type, void *data)
> struct acpi_device *adev;
> acpi_status status;
>
> + if (handler->prepare &&
> + handler->prepare(handle, type, data) == NOTIFY_STOP)
> + return;

The OS is responsible for calling _OST when it is implemented. So you
cannot just return here. See acpi_hotplug_unsupported(handle, type)
next line. Also, please describe why prepare() needs to be added.

Thanks,
-Toshi



> +
> if (!handler->hotplug.enabled)
> return acpi_hotplug_unsupported(handle, type);
>
> diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
> index ddabed1..09a73bd 100644
> --- a/include/acpi/acpi_bus.h
> +++ b/include/acpi/acpi_bus.h
> @@ -113,6 +113,7 @@ static inline struct acpi_hotplug_profile *to_acpi_hotplug_profile(
> struct acpi_scan_handler {
> const struct acpi_device_id *ids;
> struct list_head list_node;
> + int (*prepare)(acpi_handle handle, u32 type, void *context);
> int (*attach)(struct acpi_device *dev, const struct acpi_device_id *id);
> void (*detach)(struct acpi_device *dev);
> struct acpi_hotplug_profile hotplug;

2014-01-21 23:03:11

by Rafael J. Wysocki

Subject: Re: [Patch v1 1/3] ACPI: add callback prepare() into acpi_hotplug_handler

On Tuesday, January 21, 2014 02:14:57 PM Toshi Kani wrote:
> On Sat, 2014-01-18 at 10:48 +0800, Jiang Liu wrote:
> > Add callback prepare() into acpi_hotplug_handler, which will get called
> > at the very beginning of ACPI hotplug event handler. The ACPI core will
> > ignore the event if prepare() returns NOTIFY_STOP.
> >
> > Signed-off-by: Jiang Liu <[email protected]>
> > ---
> > drivers/acpi/scan.c | 4 ++++
> > include/acpi/acpi_bus.h | 1 +
> > 2 files changed, 5 insertions(+)
> >
> > diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> > index fd39459..6b0f419 100644
> > --- a/drivers/acpi/scan.c
> > +++ b/drivers/acpi/scan.c
> > @@ -392,6 +392,10 @@ static void acpi_hotplug_notify_cb(acpi_handle handle, u32 type, void *data)
> > struct acpi_device *adev;
> > acpi_status status;
> >
> > + if (handler->prepare &&
> > + handler->prepare(handle, type, data) == NOTIFY_STOP)
> > + return;
>
> The OS is responsible for calling _OST when it is implemented. So you
> cannot just return here. See acpi_hotplug_unsupported(handle, type)
> next line. Also, please describe why prepare() needs to be added.

I don't think it's needed any more, please see:

http://marc.info/?l=linux-acpi&m=139001691317575&w=2

Thanks,
Rafael

2014-01-21 23:12:47

by Toshi Kani

Subject: Re: [Patch v1 1/3] ACPI: add callback prepare() into acpi_hotplug_handler

On Wed, 2014-01-22 at 00:17 +0100, Rafael J. Wysocki wrote:
> On Tuesday, January 21, 2014 02:14:57 PM Toshi Kani wrote:
> > On Sat, 2014-01-18 at 10:48 +0800, Jiang Liu wrote:
> > > Add callback prepare() into acpi_hotplug_handler, which will get called
> > > at the very beginning of ACPI hotplug event handler. The ACPI core will
> > > ignore the event if prepare() returns NOTIFY_STOP.
> > >
> > > Signed-off-by: Jiang Liu <[email protected]>
> > > ---
> > > drivers/acpi/scan.c | 4 ++++
> > > include/acpi/acpi_bus.h | 1 +
> > > 2 files changed, 5 insertions(+)
> > >
> > > diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> > > index fd39459..6b0f419 100644
> > > --- a/drivers/acpi/scan.c
> > > +++ b/drivers/acpi/scan.c
> > > @@ -392,6 +392,10 @@ static void acpi_hotplug_notify_cb(acpi_handle handle, u32 type, void *data)
> > > struct acpi_device *adev;
> > > acpi_status status;
> > >
> > > + if (handler->prepare &&
> > > + handler->prepare(handle, type, data) == NOTIFY_STOP)
> > > + return;
> >
> > The OS is responsible for calling _OST when it is implemented. So you
> > cannot just return here. See acpi_hotplug_unsupported(handle, type)
> > next line. Also, please describe why prepare() needs to be added.
>
> I don't think it's needed any more, please see:
>
> http://marc.info/?l=linux-acpi&m=139001691317575&w=2

Oh, I see. Thanks!
-Toshi