Re: [RFC][PATCH 5/8] ACPI / hotplug / PCI: Unified notify handler for hotplug events

2013-07-09 Thread Rafael J. Wysocki
On Tuesday, July 09, 2013 12:30:45 PM Mika Westerberg wrote:
> On Tue, Jul 09, 2013 at 02:19:04AM +0200, Rafael J. Wysocki wrote:
> > Index: linux-pm/drivers/pci/hotplug/acpiphp.h
> > ===
> > --- linux-pm.orig/drivers/pci/hotplug/acpiphp.h
> > +++ linux-pm/drivers/pci/hotplug/acpiphp.h
> > @@ -137,6 +137,7 @@ struct acpiphp_context {
> > acpi_handle handle;
> > struct acpiphp_func *func;
> > struct acpiphp_bridge *bridge;
> > +   bool handler_for_func:1;
> 
> Hmm, should it be just plain:
> 
>   bool handler_for_func;
> 
> ? What's the reason using bitfields for bool?

If there are more of them, they can be stored together in one int (they are
unsigned int under the hood).

I this particular case it doesn't matter and one of subsequent patches will
remove that field anyway. :-)

> >  };
-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RFC][PATCH 5/8] ACPI / hotplug / PCI: Unified notify handler for hotplug events

2013-07-09 Thread Mika Westerberg
On Tue, Jul 09, 2013 at 02:19:04AM +0200, Rafael J. Wysocki wrote:
> Index: linux-pm/drivers/pci/hotplug/acpiphp.h
> ===
> --- linux-pm.orig/drivers/pci/hotplug/acpiphp.h
> +++ linux-pm/drivers/pci/hotplug/acpiphp.h
> @@ -137,6 +137,7 @@ struct acpiphp_context {
>   acpi_handle handle;
>   struct acpiphp_func *func;
>   struct acpiphp_bridge *bridge;
> + bool handler_for_func:1;

Hmm, should it be just plain:

bool handler_for_func;

? What's the reason using bitfields for bool?

>  };
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[RFC][PATCH 5/8] ACPI / hotplug / PCI: Unified notify handler for hotplug events

2013-07-08 Thread Rafael J. Wysocki
From: Rafael J. Wysocki 

Using the hotplug context objects introduced previously rework the
ACPI-based PCI hotplug (acpiphp) core code so that all notifications
for ACPI device objects corresponding to the hotplug PCI devices are
handled by one function, handle_hotplug_event(), which recognizes
whether it has to handle a bridge or a function.

In addition to code size reduction it allows some ugly pieces of code
where notify handlers have to be uninstalled and installed again to
go away.

Signed-off-by: Rafael J. Wysocki 
---
 drivers/pci/hotplug/acpiphp.h  |1 
 drivers/pci/hotplug/acpiphp_glue.c |  127 +
 2 files changed, 48 insertions(+), 80 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
@@ -58,11 +58,10 @@ static DEFINE_MUTEX(bridge_mutex);
 
 #define MY_NAME "acpiphp_glue"
 
-static void handle_hotplug_event_bridge (acpi_handle, u32, void *);
+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_func(acpi_handle handle, u32 type, void *context);
-static void handle_hotplug_event_func(acpi_handle handle, u32 type, void 
*context);
 static void free_bridge(struct kref *kref);
 
 /* callback routine to check for the existence of a pci dock device */
@@ -163,13 +162,13 @@ static void free_bridge(struct kref *kre
kfree(slot);
}
 
+   context = bridge->context;
/* Release reference acquired by acpiphp_enumerate_slots(). */
-   if ((bridge->flags & BRIDGE_HAS_EJ0) && bridge->func)
+   if (context->handler_for_func)
put_bridge(bridge->func->slot->bridge);
 
put_device(&bridge->pci_bus->dev);
pci_dev_put(bridge->pci_dev);
-   context = bridge->context;
context->bridge = NULL;
kfree(bridge);
acpiphp_put_context(context);
@@ -406,12 +405,12 @@ 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_func,
-newfunc);
-
-   if (ACPI_FAILURE(status))
+   status = acpi_install_notify_handler(handle, ACPI_SYSTEM_NOTIFY,
+handle_hotplug_event,
+context);
+   if (ACPI_SUCCESS(status))
+   context->handler_for_func = true;
+   else
err("failed to register interrupt notify handler\n");
}
 
@@ -465,23 +464,13 @@ static void cleanup_bridge(struct acpiph
acpi_status status;
acpi_handle handle = bridge->handle;
 
-   if (!pci_is_root_bus(bridge->pci_bus)) {
-   status = acpi_remove_notify_handler(handle,
-   ACPI_SYSTEM_NOTIFY,
-   handle_hotplug_event_bridge);
+   if (!bridge->context->handler_for_func) {
+   status = acpi_remove_notify_handler(handle, ACPI_SYSTEM_NOTIFY,
+   handle_hotplug_event);
if (ACPI_FAILURE(status))
err("failed to remove notify handler\n");
}
 
-   if ((bridge->flags & BRIDGE_HAS_EJ0) && bridge->func) {
-   status = acpi_install_notify_handler(bridge->func->handle,
-   ACPI_SYSTEM_NOTIFY,
-   handle_hotplug_event_func,
-   bridge->func);
-   if (ACPI_FAILURE(status))
-   err("failed to install interrupt notify handler\n");
-   }
-
list_for_each_entry(slot, &bridge->slots, node) {
list_for_each_entry(func, &slot->funcs, sibling) {
if (is_dock_device(func->handle)) {
@@ -489,9 +478,10 @@ static void cleanup_bridge(struct acpiph
unregister_dock_notifier(&func->nb);
}
if (!(func->flags & FUNC_HAS_DCK)) {
+   func->context->handler_for_func = false;
status = 
acpi_remove_notify_handler(func->handle,
-   ACPI_SYSTEM_NOTIFY,
-   handle_hotplug_event_func);
+   ACPI_SYSTEM_NOTIFY,
+