[permalink] [raw]

Subject: Re: [PATCH 0/5][RFT] ACPI / hotplug / PCI: Consolidation of ACPIPHP with ACPI core device hotplug

On Tuesday, January 28, 2014 11:10:30 PM Rafael J. Wysocki wrote:
> Hi All,
>
> It looks like there's time for more adventurous stuff. :-)
>
> The following series is on top of the one I sent on Sunday:
>
> https://lkml.org/lkml/2014/1/26/191
>
> The final outcome of the patches below is that all ACPI hotplug notifications
> for PCI devices and for core system things like CPU, memory, PCI roots etc.,
> will be dispatched from acpi_bus_notify() and it is not necessary to install a
> separate hotplug notify handler for each device any more.
>
> [1/5] Attach ACPIPHP hotplug contexts to struct acpi_device objects.
> [2/5] Introduce wrappers for installing and removing hotplug notify handlers
> (those wrappers go away later on, but they are useful for separating
> changes).
> [3/5] Consolidate ACPI hotplug signaling for PCI and ACPI core.
> [4/5] Simplify notify handle registration wrapper.
> [5/5] Dispatch ACPI hotplug notifications for "core" devices and PCI from acpi_bus_notify().

Unfortunately, I realized that patches [3-5/5] were buggy. The bugs were
kind of subtle and might not be easy to reproduce, but they were bugs anyway. :-)

A respin of the whole series follows.

Thanks!

--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

2014-01-28 23:50:32

by Rafael J. Wysocki

[permalink] [raw]

Subject: [Resend][PATCH 1/5][RFT] ACPI / hotplug / PCI: Attach hotplug contexts to struct acpi_device

2014-01-28 23:51:07

by Rafael J. Wysocki

[permalink] [raw]

Subject: [Update][PATCH 3/5][RFT] ACPI / hotplug / PCI: Consolidate ACPIPHP with ACPI core hotplug

From: Rafael J. Wysocki <[email protected]>

Use the same ACPI notify handler, acpi_hotplug_notify_cb() for both
ACPI-based PCI hotplug (ACPIPHP) and the generic ACPI-based hotplug
of devices. For PCI devices use the .hp.event() callback from
their ACPI companions that points to acpiphp_hotplug_event().
For other devices (CPU, memory, containers, PCI host bridges) the
generic ACPI-based device hotplug code is used.

This allows code duplication between ACPIPHP and the ACPI core to be
reduced significantly and makes further ACPI-based device hotplug
consolidation possible.

Signed-off-by: Rafael J. Wysocki <[email protected]>
---
drivers/acpi/scan.c | 79 +++++++++++++++++---------
drivers/pci/hotplug/acpiphp_glue.c | 110 +++----------------------------------
include/acpi/acpi_bus.h | 1
3 files changed, 63 insertions(+), 127 deletions(-)

Index: linux-pm/include/acpi/acpi_bus.h
===================================================================
--- linux-pm.orig/include/acpi/acpi_bus.h
+++ linux-pm/include/acpi/acpi_bus.h
@@ -145,6 +145,7 @@ struct acpi_scan_handler {

struct acpi_hotplug_context {
struct acpi_device *self;
+ int (*event)(struct acpi_device *, u32);
void (*release)(struct acpi_hotplug_context *);
};

Index: linux-pm/drivers/acpi/scan.c
===================================================================
--- linux-pm.orig/drivers/acpi/scan.c
+++ linux-pm/drivers/acpi/scan.c
@@ -439,38 +439,42 @@ static int acpi_scan_bus_check(struct ac
return 0;
}

+static int acpi_generic_hotplug_event(struct acpi_device *adev, u32 type)
+{
+ switch (type) {
+ case ACPI_NOTIFY_BUS_CHECK:
+ return acpi_scan_bus_check(adev);
+ case ACPI_NOTIFY_DEVICE_CHECK:
+ return acpi_scan_device_check(adev);
+ case ACPI_NOTIFY_EJECT_REQUEST:
+ case ACPI_OST_EC_OSPM_EJECT:
+ return acpi_scan_hot_remove(adev);
+ }
+ return -EINVAL;
+}
+
static void acpi_device_hotplug(void *data, u32 src)
{
u32 ost_code = ACPI_OST_SC_NON_SPECIFIC_FAILURE;
struct acpi_device *adev = data;
- int error;
+ int error = -ENODEV;

lock_device_hotplug();
mutex_lock(&acpi_scan_lock);

/*
* The device object's ACPI handle cannot become invalid as long as we
- * are holding acpi_scan_lock, but it may have become invalid before
+ * are holding acpi_scan_lock, but it might have become invalid before
* that lock was acquired.
*/
if (adev->handle == INVALID_ACPI_HANDLE)
goto out;

- switch (src) {
- case ACPI_NOTIFY_BUS_CHECK:
- error = acpi_scan_bus_check(adev);
- break;
- case ACPI_NOTIFY_DEVICE_CHECK:
- error = acpi_scan_device_check(adev);
- break;
- case ACPI_NOTIFY_EJECT_REQUEST:
- case ACPI_OST_EC_OSPM_EJECT:
- error = acpi_scan_hot_remove(adev);
- break;
- default:
- error = -EINVAL;
- break;
- }
+ if (adev->handler)
+ error = acpi_generic_hotplug_event(adev, src);
+ else if (adev->hp && adev->hp->event)
+ error = adev->hp->event(adev, src);
+
if (!error)
ost_code = ACPI_OST_SC_SUCCESS;

@@ -483,35 +487,58 @@ static void acpi_device_hotplug(void *da

static void acpi_hotplug_notify_cb(acpi_handle handle, u32 type, void *data)
{
- u32 ost_code = ACPI_OST_SC_NON_SPECIFIC_FAILURE;
struct acpi_scan_handler *handler = data;
+ u32 ost_code = ACPI_OST_SC_SUCCESS;
struct acpi_device *adev;
acpi_status status;

- if (acpi_bus_get_device(handle, &adev))
- goto err_out;
-
switch (type) {
case ACPI_NOTIFY_BUS_CHECK:
acpi_handle_debug(handle, "ACPI_NOTIFY_BUS_CHECK event\n");
break;
+
case ACPI_NOTIFY_DEVICE_CHECK:
acpi_handle_debug(handle, "ACPI_NOTIFY_DEVICE_CHECK event\n");
break;
+
case ACPI_NOTIFY_EJECT_REQUEST:
acpi_handle_debug(handle, "ACPI_NOTIFY_EJECT_REQUEST event\n");
- if (!handler->hotplug.enabled) {
+ if (handler && !handler->hotplug.enabled) {
acpi_handle_err(handle, "Eject disabled\n");
ost_code = ACPI_OST_SC_EJECT_NOT_SUPPORTED;
- goto err_out;
+ goto out;
}
acpi_evaluate_hotplug_ost(handle, ACPI_NOTIFY_EJECT_REQUEST,
ACPI_OST_SC_EJECT_IN_PROGRESS, NULL);
break;
- default:
- /* non-hotplug event; possibly handled by other handler */
+
+ case ACPI_NOTIFY_DEVICE_WAKE:
return;
+
+ case ACPI_NOTIFY_FREQUENCY_MISMATCH:
+ acpi_handle_err(handle, "Device cannot be configured due "
+ "to a frequency mismatch\n");
+ goto out;
+
+ case ACPI_NOTIFY_BUS_MODE_MISMATCH:
+ acpi_handle_err(handle, "Device cannot be configured due "
+ "to a bus mode mismatch\n");
+ goto out;
+
+ case ACPI_NOTIFY_POWER_FAULT:
+ acpi_handle_err(handle, "Device has suffered a power fault\n");
+ goto out;
+
+ default:
+ acpi_handle_warn(handle, "Unsupported event type 0x%x\n", type);
+ ost_code = ACPI_OST_SC_UNRECOGNIZED_NOTIFY;
+ goto out;
}
+
+ ost_code = ACPI_OST_SC_NON_SPECIFIC_FAILURE;
+ if (acpi_bus_get_device(handle, &adev))
+ goto out;
+
get_device(&adev->dev);
status = acpi_hotplug_execute(acpi_device_hotplug, adev, type);
if (ACPI_SUCCESS(status))
@@ -519,7 +546,7 @@ static void acpi_hotplug_notify_cb(acpi_

put_device(&adev->dev);

- err_out:
+ out:
acpi_evaluate_hotplug_ost(handle, type, ost_code, NULL);
}

Index: linux-pm/drivers/pci/hotplug/acpiphp_glue.c
===================================================================
--- linux-pm.orig/drivers/pci/hotplug/acpiphp_glue.c
+++ linux-pm/drivers/pci/hotplug/acpiphp_glue.c
@@ -53,18 +53,14 @@
#include <linux/slab.h>
#include <linux/acpi.h>

-#include <asm/pgtable.h>
-
#include "../pci.h"
#include "acpiphp.h"

-#define INVALID_ACPI_HANDLE ((acpi_handle)empty_zero_page)
-
static LIST_HEAD(bridge_list);
static DEFINE_MUTEX(bridge_mutex);
static DEFINE_MUTEX(acpiphp_context_lock);

-static void handle_hotplug_event(acpi_handle handle, u32 type, void *data);
+static int acpiphp_hotplug_event(struct acpi_device *adev, u32 type);
static void acpiphp_sanitize_bus(struct pci_bus *bus);
static void acpiphp_set_hpp_values(struct pci_bus *bus);
static void hotplug_event(u32 type, void *data);
@@ -91,6 +87,7 @@ static struct acpiphp_context *acpiphp_i

context->hp.self = adev;
context->hp.release = acpiphp_free_context;
+ context->hp.event = acpiphp_hotplug_event;
context->refcount = 1;
adev->hp = &context->hp;
return context;
@@ -373,14 +370,8 @@ static acpi_status register_slot(acpi_ha
}

/* install notify handler */
- if (!(newfunc->flags & FUNC_HAS_DCK)) {
- status = acpi_install_notify_handler(handle, ACPI_SYSTEM_NOTIFY,
- handle_hotplug_event,
- context);
- if (ACPI_FAILURE(status))
- acpi_handle_err(handle,
- "failed to install notify handler\n");
- }
+ if (!(newfunc->flags & FUNC_HAS_DCK))
+ acpi_install_hotplug_notify_handler(handle, NULL);

return AE_OK;
}
@@ -411,7 +402,6 @@ static void cleanup_bridge(struct acpiph
{
struct acpiphp_slot *slot;
struct acpiphp_func *func;
- acpi_status status;

list_for_each_entry(slot, &bridge->slots, node) {
list_for_each_entry(func, &slot->funcs, sibling) {
@@ -420,13 +410,8 @@ static void cleanup_bridge(struct acpiph
if (is_dock_device(handle))
unregister_hotplug_dock_device(handle);

- if (!(func->flags & FUNC_HAS_DCK)) {
- status = acpi_remove_notify_handler(handle,
- ACPI_SYSTEM_NOTIFY,
- handle_hotplug_event);
- if (ACPI_FAILURE(status))
- pr_err("failed to remove notify handler\n");
- }
+ if (!(func->flags & FUNC_HAS_DCK))
+ acpi_remove_hotplug_notify_handler(handle);
}
slot->flags |= SLOT_IS_GOING_AWAY;
if (slot->slot)
@@ -838,26 +823,15 @@ static void hotplug_event(u32 type, void
put_bridge(bridge);
}

-static void hotplug_event_work(void *data, u32 type)
+static int acpiphp_hotplug_event(struct acpi_device *adev, u32 type)
{
- struct acpi_device *adev = data;
struct acpiphp_context *context;
- u32 ost_code = ACPI_OST_SC_NON_SPECIFIC_FAILURE;
-
- acpi_scan_lock_acquire();
- /*
- * The device object's ACPI handle cannot become invalid as long as we
- * are holding acpi_scan_lock, but it might have become invalid before
- * that lock was acquired.
- */
- if (adev->handle == INVALID_ACPI_HANDLE)
- goto out;

mutex_lock(&acpiphp_context_lock);
context = acpiphp_get_context(adev);
if (!context) {
mutex_unlock(&acpiphp_context_lock);
- goto out;
+ return -ENODATA;
}
get_bridge(context->func.parent);
acpiphp_put_context(context);
@@ -867,73 +841,7 @@ static void hotplug_event_work(void *dat
hotplug_event(type, context);
pci_unlock_rescan_remove();
put_bridge(context->func.parent);
- ost_code = ACPI_OST_SC_SUCCESS;
-
- out:
- acpi_evaluate_hotplug_ost(adev->handle, type, ost_code, NULL);
- put_device(&adev->dev);
- acpi_scan_lock_release();
-}
-
-/**
- * handle_hotplug_event - handle ACPI hotplug event
- * @handle: Notify()'ed acpi_handle
- * @type: Notify code
- * @data: pointer to acpiphp_context structure
- *
- * Handles ACPI event notification on slots.
- */
-static void handle_hotplug_event(acpi_handle handle, u32 type, void *data)
-{
- struct acpi_device *adev;
- acpi_status status;
- u32 ost_code = ACPI_OST_SC_SUCCESS;
-
- switch (type) {
- case ACPI_NOTIFY_BUS_CHECK:
- case ACPI_NOTIFY_DEVICE_CHECK:
- break;
- case ACPI_NOTIFY_EJECT_REQUEST:
- ost_code = ACPI_OST_SC_EJECT_IN_PROGRESS;
- acpi_evaluate_hotplug_ost(handle, type, ost_code, NULL);
- break;
-
- case ACPI_NOTIFY_DEVICE_WAKE:
- return;
-
- case ACPI_NOTIFY_FREQUENCY_MISMATCH:
- acpi_handle_err(handle, "Device cannot be configured due "
- "to a frequency mismatch\n");
- goto out;
-
- case ACPI_NOTIFY_BUS_MODE_MISMATCH:
- acpi_handle_err(handle, "Device cannot be configured due "
- "to a bus mode mismatch\n");
- goto out;
-
- case ACPI_NOTIFY_POWER_FAULT:
- acpi_handle_err(handle, "Device has suffered a power fault\n");
- goto out;
-
- default:
- acpi_handle_warn(handle, "Unsupported event type 0x%x\n", type);
- ost_code = ACPI_OST_SC_UNRECOGNIZED_NOTIFY;
- goto out;
- }
-
- ost_code = ACPI_OST_SC_NON_SPECIFIC_FAILURE;
- if (acpi_bus_get_device(handle, &adev))
- goto out;
-
- get_device(&adev->dev);
- status = acpi_hotplug_execute(hotplug_event_work, adev, type);
- if (ACPI_SUCCESS(status))
- return;
-
- put_device(&adev->dev);
-
- out:
- acpi_evaluate_hotplug_ost(handle, type, ost_code, NULL);
+ return 0;
}

/**

2014-01-28 23:51:05

by Rafael J. Wysocki

[permalink] [raw]

2014-02-02 00:20:16

by Rafael J. Wysocki

[permalink] [raw]

Subject: [PATCH v2 6/13] ACPI / hotplug / PCI: Drop acpiphp_bus_trim()

From: Rafael J. Wysocki <[email protected]>

If trim_stale_devices() calls acpi_bus_trim() directly, we can
save a potentially costly acpi_bus_get_device() invocation. After
making that change acpiphp_bus_trim() would only be called from one
place, so move the code from it to that place and drop it.

Signed-off-by: Rafael J. Wysocki <[email protected]>
---
drivers/pci/hotplug/acpiphp_glue.c | 30 +++++++++++-------------------
1 file changed, 11 insertions(+), 19 deletions(-)

Index: linux-pm/drivers/pci/hotplug/acpiphp_glue.c
===================================================================
--- linux-pm.orig/drivers/pci/hotplug/acpiphp_glue.c
+++ linux-pm/drivers/pci/hotplug/acpiphp_glue.c
@@ -468,19 +468,6 @@ static unsigned char acpiphp_max_busnr(s
}

/**
- * acpiphp_bus_trim - Trim device objects in an ACPI namespace subtree.
- * @handle: ACPI device object handle to start from.
- */
-static void acpiphp_bus_trim(acpi_handle handle)
-{
- struct acpi_device *adev = NULL;
-
- acpi_bus_get_device(handle, &adev);
- if (adev)
- acpi_bus_trim(adev);
-}
-
-/**
* acpiphp_bus_add - Scan ACPI namespace subtree.
* @handle: ACPI object handle to start the scan from.
*/
@@ -620,8 +607,12 @@ static void disable_slot(struct acpiphp_
if (PCI_SLOT(dev->devfn) == slot->device)
pci_stop_and_remove_bus_device(dev);

- list_for_each_entry(func, &slot->funcs, sibling)
- acpiphp_bus_trim(func_to_handle(func));
+ list_for_each_entry(func, &slot->funcs, sibling) {
+ struct acpi_device *adev;
+
+ if (!acpi_bus_get_device(func_to_handle(func), &adev))
+ acpi_bus_trim(adev);
+ }

slot->flags &= (~SLOT_ENABLED);
}
@@ -693,11 +684,12 @@ static unsigned int get_slot_status(stru
*/
static void trim_stale_devices(struct pci_dev *dev)
{
- acpi_handle handle = ACPI_HANDLE(&dev->dev);
+ struct acpi_device *adev = ACPI_COMPANION(&dev->dev);
struct pci_bus *bus = dev->subordinate;
bool alive = false;

- if (handle) {
+ if (adev) {
+ acpi_handle handle = adev->handle;
acpi_status status;
unsigned long long sta;

@@ -713,8 +705,8 @@ static void trim_stale_devices(struct pc
}
if (!alive) {
pci_stop_and_remove_bus_device(dev);
- if (handle)
- acpiphp_bus_trim(handle);
+ if (adev)
+ acpi_bus_trim(adev);
} else if (bus) {
struct pci_dev *child, *tmp;

2014-02-02 00:20:56

by Rafael J. Wysocki

[permalink] [raw]

Subject: [PATCH v2 8/13] ACPI / hotplug / PCI: Store acpi_device pointer in acpiphp_context

From: Rafael J. Wysocki <[email protected]>

After recent modifications of the ACPI core making it create a struct
acpi_device object for every namespace node representing a device
regardless of the current status of that device the ACPIPHP code
can store a struct acpi_device pointer instead of an ACPI handle
in struct acpiphp_context. This immediately makes it possible to
avoid making potentially costly calls to acpi_bus_get_device() in
two places and allows some more simplifications to be made going
forward.

The reason why that is correct is because ACPIPHP only installs
hotify handlers for namespace nodes that exist when
acpiphp_enumerate_slots() is called for their parent bridge.
That only happens if the parent bridge has an ACPI companion
associated with it, which means that the ACPI namespace scope
in question has been scanned already at that point. That, in
turn, means that struct acpi_device objects have been created
for all namespace nodes in that scope and pointers to those
objects can be stored directly instead of their ACPI handles.

Signed-off-by: Rafael J. Wysocki <[email protected]>
---
drivers/pci/hotplug/acpiphp.h | 9 +++++--
drivers/pci/hotplug/acpiphp_glue.c | 44 +++++++++++++++++--------------------
2 files changed, 28 insertions(+), 25 deletions(-)

Index: linux-pm/drivers/pci/hotplug/acpiphp.h
===================================================================
--- linux-pm.orig/drivers/pci/hotplug/acpiphp.h
+++ linux-pm/drivers/pci/hotplug/acpiphp.h
@@ -117,8 +117,8 @@ struct acpiphp_func {
};

struct acpiphp_context {
- acpi_handle handle;
struct acpiphp_func func;
+ struct acpi_device *adev;
struct acpiphp_bridge *bridge;
unsigned int refcount;
};
@@ -128,9 +128,14 @@ static inline struct acpiphp_context *fu
return container_of(func, struct acpiphp_context, func);
}

+static inline struct acpi_device *func_to_acpi_device(struct acpiphp_func *func)
+{
+ return func_to_context(func)->adev;
+}
+
static inline acpi_handle func_to_handle(struct acpiphp_func *func)
{
- return func_to_context(func)->handle;
+ return func_to_acpi_device(func)->handle;
}

/*
Index: linux-pm/drivers/pci/hotplug/acpiphp_glue.c
===================================================================
--- linux-pm.orig/drivers/pci/hotplug/acpiphp_glue.c
+++ linux-pm/drivers/pci/hotplug/acpiphp_glue.c
@@ -73,11 +73,11 @@ static void acpiphp_context_handler(acpi

/**
* acpiphp_init_context - Create hotplug context and grab a reference to it.
- * @handle: ACPI object handle to create the context for.
+ * @adev: ACPI device object to create the context for.
*
* Call under acpiphp_context_lock.
*/
-static struct acpiphp_context *acpiphp_init_context(acpi_handle handle)
+static struct acpiphp_context *acpiphp_init_context(struct acpi_device *adev)
{
struct acpiphp_context *context;
acpi_status status;
@@ -86,9 +86,9 @@ static struct acpiphp_context *acpiphp_i
if (!context)
return NULL;

- context->handle = handle;
+ context->adev = adev;
context->refcount = 1;
- status = acpi_attach_data(handle, acpiphp_context_handler, context);
+ status = acpi_attach_data(adev->handle, acpiphp_context_handler, context);
if (ACPI_FAILURE(status)) {
kfree(context);
return NULL;
@@ -118,7 +118,7 @@ static struct acpiphp_context *acpiphp_g

/**
* acpiphp_put_context - Drop a reference to ACPI hotplug context.
- * @handle: ACPI object handle to put the context for.
+ * @context: ACPI hotplug context to drop a reference to.
*
* The context object is removed if there are no more references to it.
*
@@ -130,7 +130,7 @@ static void acpiphp_put_context(struct a
return;

WARN_ON(context->bridge);
- acpi_detach_data(context->handle, acpiphp_context_handler);
+ acpi_detach_data(context->adev->handle, acpiphp_context_handler);
kfree(context);
}

@@ -265,6 +265,7 @@ static acpi_status register_slot(acpi_ha
{
struct acpiphp_bridge *bridge = data;
struct acpiphp_context *context;
+ struct acpi_device *adev;
struct acpiphp_slot *slot;
struct acpiphp_func *newfunc;
acpi_status status = AE_OK;
@@ -284,12 +285,14 @@ static acpi_status register_slot(acpi_ha
"can't evaluate _ADR (%#x)\n", status);
return AE_OK;
}
+ if (acpi_bus_get_device(handle, &adev))
+ return AE_OK;

device = (adr >> 16) & 0xffff;
function = adr & 0xffff;

mutex_lock(&acpiphp_context_lock);
- context = acpiphp_init_context(handle);
+ context = acpiphp_init_context(adev);
if (!context) {
mutex_unlock(&acpiphp_context_lock);
acpi_handle_err(handle, "No hotplug context\n");
@@ -607,12 +610,8 @@ static void disable_slot(struct acpiphp_
if (PCI_SLOT(dev->devfn) == slot->device)
pci_stop_and_remove_bus_device(dev);

- list_for_each_entry(func, &slot->funcs, sibling) {
- struct acpi_device *adev;
-
- if (!acpi_bus_get_device(func_to_handle(func), &adev))
- acpi_bus_trim(adev);
- }
+ list_for_each_entry(func, &slot->funcs, sibling)
+ acpi_bus_trim(func_to_acpi_device(func));

slot->flags &= (~SLOT_ENABLED);
}
@@ -626,13 +625,10 @@ static bool slot_no_hotplug(struct acpip
{
struct acpiphp_func *func;

- list_for_each_entry(func, &slot->funcs, sibling) {
- struct acpi_device *adev = NULL;
-
- acpi_bus_get_device(func_to_handle(func), &adev);
- if (acpiphp_no_hotplug(adev))
+ list_for_each_entry(func, &slot->funcs, sibling)
+ if (acpiphp_no_hotplug(func_to_acpi_device(func)))
return true;
- }
+
return false;
}

@@ -883,7 +879,7 @@ static void hotplug_event(acpi_handle ha
static void hotplug_event_work(void *data, u32 type)
{
struct acpiphp_context *context = data;
- acpi_handle handle = context->handle;
+ acpi_handle handle = context->adev->handle;

acpi_scan_lock_acquire();

@@ -941,7 +937,7 @@ static void handle_hotplug_event(acpi_ha

mutex_lock(&acpiphp_context_lock);
context = acpiphp_get_context(handle);
- if (context && !WARN_ON(context->handle != handle)) {
+ if (context && !WARN_ON(context->adev->handle != handle)) {
get_bridge(context->func.parent);
acpiphp_put_context(context);
acpi_hotplug_execute(hotplug_event_work, context, type);
@@ -965,16 +961,18 @@ static void handle_hotplug_event(acpi_ha
void acpiphp_enumerate_slots(struct pci_bus *bus)
{
struct acpiphp_bridge *bridge;
+ struct acpi_device *adev;
acpi_handle handle;
acpi_status status;

if (acpiphp_disabled)
return;

- handle = ACPI_HANDLE(bus->bridge);
- if (!handle)
+ adev = ACPI_COMPANION(bus->bridge);
+ if (!adev)
return;

+ handle = adev->handle;
bridge = kzalloc(sizeof(struct acpiphp_bridge), GFP_KERNEL);
if (!bridge) {
acpi_handle_err(handle, "No memory for bridge object\n");

2014-02-02 00:20:55

by Rafael J. Wysocki

[permalink] [raw]

Subject: [PATCH v2 7/13] ACPI / hotplug / PCI: Rework acpiphp_no_hotplug()

From: Rafael J. Wysocki <[email protected]>

If a struct acpi_device pointer is passed to acpiphp_no_hotplug()
instead of an ACPI handle, the function won't need to call
acpi_bus_get_device(), which may be costly, any more. Then,
trim_stale_devices() can call acpiphp_no_hotplug() passing
the struct acpi_device object it already has directly to that
function.

Make those changes and update slot_no_hotplug() accordingly.

Signed-off-by: Rafael J. Wysocki <[email protected]>
---
drivers/pci/hotplug/acpiphp_glue.c | 19 +++++++++----------
1 file changed, 9 insertions(+), 10 deletions(-)

Index: linux-pm/drivers/pci/hotplug/acpiphp_glue.c
===================================================================
--- linux-pm.orig/drivers/pci/hotplug/acpiphp_glue.c
+++ linux-pm/drivers/pci/hotplug/acpiphp_glue.c
@@ -617,11 +617,8 @@ static void disable_slot(struct acpiphp_
slot->flags &= (~SLOT_ENABLED);
}

-static bool acpiphp_no_hotplug(acpi_handle handle)
+static bool acpiphp_no_hotplug(struct acpi_device *adev)
{
- struct acpi_device *adev = NULL;
-
- acpi_bus_get_device(handle, &adev);
return adev && adev->flags.no_hotplug;
}

@@ -629,10 +626,13 @@ static bool slot_no_hotplug(struct acpip
{
struct acpiphp_func *func;

- list_for_each_entry(func, &slot->funcs, sibling)
- if (acpiphp_no_hotplug(func_to_handle(func)))
- return true;
+ list_for_each_entry(func, &slot->funcs, sibling) {
+ struct acpi_device *adev = NULL;

+ acpi_bus_get_device(func_to_handle(func), &adev);
+ if (acpiphp_no_hotplug(adev))
+ return true;
+ }
return false;
}

@@ -689,13 +689,12 @@ static void trim_stale_devices(struct pc
bool alive = false;

if (adev) {
- acpi_handle handle = adev->handle;
acpi_status status;
unsigned long long sta;

- status = acpi_evaluate_integer(handle, "_STA", NULL, &sta);
+ status = acpi_evaluate_integer(adev->handle, "_STA", NULL, &sta);
alive = (ACPI_SUCCESS(status) && sta == ACPI_STA_ALL)
- || acpiphp_no_hotplug(handle);
+ || acpiphp_no_hotplug(adev);
}
if (!alive) {
u32 v;

2014-02-02 00:18:19

by Rafael J. Wysocki

[permalink] [raw]

Subject: [PATCH v2 13/13] ACPI / hotplug: Do not pass ACPI handles to ACPI dock operations

From: Rafael J. Wysocki <[email protected]>

None of the existing users of struct acpi_dock_ops actually needs the
first argument of its member functions, so redefine those functions
to take only two arguments, the event type and data pointer, and
update the users of struct acpi_dock_ops accordingly.

Signed-off-by: Rafael J. Wysocki <[email protected]>
---
drivers/acpi/dock.c | 4 ++--
drivers/ata/libata-acpi.c | 8 ++++----
drivers/pci/hotplug/acpiphp_glue.c | 13 +++++++------
include/acpi/acpi_drivers.h | 6 +++---
4 files changed, 16 insertions(+), 15 deletions(-)

Index: linux-pm/include/acpi/acpi_drivers.h
===================================================================
--- linux-pm.orig/include/acpi/acpi_drivers.h
+++ linux-pm/include/acpi/acpi_drivers.h
@@ -110,9 +110,9 @@ void pci_acpi_crs_quirks(void);
Dock Station
-------------------------------------------------------------------------- */
struct acpi_dock_ops {
- acpi_notify_handler fixup;
- acpi_notify_handler handler;
- acpi_notify_handler uevent;
+ void (*handler)(u32 event_type, void *data);
+ void (*fixup)(u32 event_type, void *data);
+ void (*uevent)(u32 event_type, void *data);
};

#ifdef CONFIG_ACPI_DOCK
Index: linux-pm/drivers/pci/hotplug/acpiphp_glue.c
===================================================================
--- linux-pm.orig/drivers/pci/hotplug/acpiphp_glue.c
+++ linux-pm/drivers/pci/hotplug/acpiphp_glue.c
@@ -63,7 +63,7 @@ static DEFINE_MUTEX(acpiphp_context_lock
static void handle_hotplug_event(acpi_handle handle, u32 type, void *data);
static void acpiphp_sanitize_bus(struct pci_bus *bus);
static void acpiphp_set_hpp_values(struct pci_bus *bus);
-static void hotplug_event(acpi_handle handle, u32 type, void *data);
+static void hotplug_event(u32 type, void *data);
static void free_bridge(struct kref *kref);

static void acpiphp_context_handler(acpi_handle handle, void *context)
@@ -185,7 +185,7 @@ static void free_bridge(struct kref *kre
* TBD - figure out a way to only call fixups for
* systems that require them.
*/
-static void post_dock_fixups(acpi_handle not_used, u32 event, void *data)
+static void post_dock_fixups(u32 event, void *data)
{
struct acpiphp_context *context = data;
struct pci_bus *bus = context->func.slot->bus;
@@ -788,11 +788,12 @@ void acpiphp_check_host_bridge(acpi_hand

static int acpiphp_disable_and_eject_slot(struct acpiphp_slot *slot);

-static void hotplug_event(acpi_handle handle, u32 type, void *data)
+static void hotplug_event(u32 type, void *data)
{
struct acpiphp_context *context = data;
struct acpiphp_func *func = &context->func;
struct acpiphp_slot *slot = func->slot;
+ acpi_handle handle = context->adev->handle;
struct acpiphp_bridge *bridge;

mutex_lock(&acpiphp_context_lock);
@@ -845,14 +846,14 @@ static void hotplug_event(acpi_handle ha
static void hotplug_event_work(void *data, u32 type)
{
struct acpiphp_context *context = data;
- acpi_handle handle = context->adev->handle;

acpi_scan_lock_acquire();

- hotplug_event(handle, type, context);
+ hotplug_event(type, context);

acpi_scan_lock_release();
- acpi_evaluate_hotplug_ost(handle, type, ACPI_OST_SC_SUCCESS, NULL);
+ acpi_evaluate_hotplug_ost(context->adev->handle, type,
+ ACPI_OST_SC_SUCCESS, NULL);
put_bridge(context->func.parent);
}

Index: linux-pm/drivers/acpi/dock.c
===================================================================
--- linux-pm.orig/drivers/acpi/dock.c
+++ linux-pm/drivers/acpi/dock.c
@@ -185,7 +185,7 @@ static void dock_release_hotplug(struct
static void dock_hotplug_event(struct dock_dependent_device *dd, u32 event,
enum dock_callback_type cb_type)
{
- acpi_notify_handler cb = NULL;
+ void (*cb)(u32, void *) = NULL;
bool run = false;

mutex_lock(&hotplug_lock);
@@ -213,7 +213,7 @@ static void dock_hotplug_event(struct do
return;

if (cb)
- cb(dd->handle, event, dd->hp_context);
+ cb(event, dd->hp_context);

dock_release_hotplug(dd);
}
Index: linux-pm/drivers/ata/libata-acpi.c
===================================================================
--- linux-pm.orig/drivers/ata/libata-acpi.c
+++ linux-pm/drivers/ata/libata-acpi.c
@@ -121,14 +121,14 @@ static void ata_acpi_handle_hotplug(stru
ata_port_wait_eh(ap);
}

-static void ata_acpi_dev_notify_dock(acpi_handle handle, u32 event, void *data)
+static void ata_acpi_dev_notify_dock(u32 event, void *data)
{
struct ata_device *dev = data;

ata_acpi_handle_hotplug(dev->link->ap, dev, event);
}

-static void ata_acpi_ap_notify_dock(acpi_handle handle, u32 event, void *data)
+static void ata_acpi_ap_notify_dock(u32 event, void *data)
{
struct ata_port *ap = data;

@@ -154,12 +154,12 @@ static void ata_acpi_uevent(struct ata_p
}
}

-static void ata_acpi_ap_uevent(acpi_handle handle, u32 event, void *data)
+static void ata_acpi_ap_uevent(u32 event, void *data)
{
ata_acpi_uevent(data, NULL, event);
}

-static void ata_acpi_dev_uevent(acpi_handle handle, u32 event, void *data)
+static void ata_acpi_dev_uevent(u32 event, void *data)
{
struct ata_device *dev = data;
ata_acpi_uevent(dev->link->ap, dev, event);

2014-02-02 00:21:38

by Rafael J. Wysocki

[permalink] [raw]

On Mon, Jan 27, 2014 at 01:37:17AM +0100, Rafael J. Wysocki wrote:
> Hi All,
>
> ACPIPHP can be simplified a bit on top of some PCI and ACPI changes merged
> recently and the following series of patches implements those simplifications:
>
> [1/11] Fix up two kerneldoc comments in acpiphp_glue.c.
> [2/11] Get rid of an unnecessary label in register_slot().
> [3/11] Drop acpiphp_bus_trim() and use acpi_bus_trim() instead of it directly.
> [4/11] Move the acpi_bus_get_device() call out of acpiphp_no_hotplug().
> [5/11] Store struct acpi_device pointers instead of ACPI handles in struct acpiphp_context.
> [6/11] Drop acpiphp_bus_add() (which has only one user).
> [7/11] Drop crit_sect mutexes (that are redundant).
> [8/11] Clean up the usage of the slot variable in hotplug_event().
> [9/11] Drop dev_in_slot() and rework disable_slot() to walk bus->devices directly.
> [10/11] Use acpi_handle_debug() in hotplug_event() instead of open-coded stuff.
> [11/11] Drop handle argument from the member functions of struct acpi_dock_ops.

I assume you'll handle all these through your tree, Rafael. Let me know if
you need me to do anything.

Bjorn

> All of that is relateively straightforward, but I have some more intrusive changes
> on top of it in the works. They will be posted separately later this week.
>
> Thanks!
>
> --
> I speak only for myself.
> Rafael J. Wysocki, Intel Open Source Technology Center.

2014-02-11 12:39:24

by Rafael J. Wysocki

[permalink] [raw]

Subject: Re: [PATCH 0/11] ACPI / hotplug / PCI: Updates on top of changes merged recently

On Monday, February 10, 2014 05:19:58 PM Bjorn Helgaas wrote:
> On Mon, Jan 27, 2014 at 01:37:17AM +0100, Rafael J. Wysocki wrote:
> > Hi All,
> >
> > ACPIPHP can be simplified a bit on top of some PCI and ACPI changes merged
> > recently and the following series of patches implements those simplifications:
> >
> > [1/11] Fix up two kerneldoc comments in acpiphp_glue.c.
> > [2/11] Get rid of an unnecessary label in register_slot().
> > [3/11] Drop acpiphp_bus_trim() and use acpi_bus_trim() instead of it directly.
> > [4/11] Move the acpi_bus_get_device() call out of acpiphp_no_hotplug().
> > [5/11] Store struct acpi_device pointers instead of ACPI handles in struct acpiphp_context.
> > [6/11] Drop acpiphp_bus_add() (which has only one user).
> > [7/11] Drop crit_sect mutexes (that are redundant).
> > [8/11] Clean up the usage of the slot variable in hotplug_event().
> > [9/11] Drop dev_in_slot() and rework disable_slot() to walk bus->devices directly.
> > [10/11] Use acpi_handle_debug() in hotplug_event() instead of open-coded stuff.
> > [11/11] Drop handle argument from the member functions of struct acpi_dock_ops.
>
> I assume you'll handle all these through your tree, Rafael. Let me know if
> you need me to do anything.

Yes, I'll handle these, thanks!

Rafael