Hi,

At Thu, 26 Jan 2006 20:14:25 -0800,
Greg KH <[EMAIL PROTECTED]> wrote:
> 
> On Fri, Jan 20, 2006 at 07:08:30PM +0900, MUNEDA Takahiro wrote:
> > Hi,
> > 
> > This series of patches contains slots management fix.
> > Here are summary of changes:
> > 
> > o [PATCH 1/7] - removes unnecesary struct members
> > 
> > o [PATCH 2/7] - removes init_slots(), because acpiphp doesn't
> >                 register slots when driver is loaded.
> > 
> > o [PATCH 3/7] - changes register_slot() to register hotplug
> >                 slots when they are found
> > 
> > o [PATCH 4/7] - changes init_bridge_misc() to register hotplug
> >                 slots when they are found
> > 
> > o [PATCH 5/7] - removes cleanup_slots(), because acpiphp
> >                 doesn't unregister slots when driver is going
> >                 to be un-loaded.
> > 
> > o [PATCH 6/7] - changes cleanup_brige() to unregister hotplug
> >                 slots when acpiphp cleanup the bridge.
> > 
> > o [PATCH 7/7] - adds spin_lock/_unlock to protect slot_list
> > 
> > 
> > For more specifc information, please see the header of each patch.
> > These patches are against 2.6.16-rc1. I tested them on Tiger4.
> 
> This patch series does not even compile when applied:
> 
>   CC [M]  drivers/pci/hotplug/acpiphp_core.o
> drivers/pci/hotplug/acpiphp_core.c: In function 
> `acpiphp_register_hotplug_slot':
> drivers/pci/hotplug/acpiphp_core.c:355: error: `hotplug_slot' undeclared 
> (first use in this function)
> drivers/pci/hotplug/acpiphp_core.c:355: error: (Each undeclared identifier is 
> reported only once
> drivers/pci/hotplug/acpiphp_core.c:355: error: for each function it appears 
> in.)
> drivers/pci/hotplug/acpiphp_core.c:359: error: `hotplug_slot_info' undeclared 
> (first use in this function)
> make[2]: *** [drivers/pci/hotplug/acpiphp_core.o] Error 1
> 
> Care to redo them?


I'm sorry for my late replying.
And thanks for your taking care of my patch.

At first, please let me introduce the base idea of this patch.
I'm trying to support p2p bridge hotplug with acpiphp.

 Current acpiphp manages hotplug slot by ID. The ID is incremented
 when acpiphp founds hotpluggable slots.
 If the bridges(with hotpluggable slots) are hotplugged many times,
 the hotpluggable slots are added many times also. The ID might be
 overflowed. So this patch removes IDs to manage slots.

 And, this patch changes the slot register/unregister timing.
 Current acpiphp registers the slots in the init_slots()
 called from acpiphp_init(), and unregisters the slots in the
 cleanup_slots() called from acpiphp_exit().
 acpiphp doesn't assume the increase and decrease of the
 hotpluggable slots.
 This patch removes the slot register/unregister processes from the
 init/exit phases. Instead, adds the these processes in the bridge
 add/cleanup phases.

Currently, this change doesn't have any meanings. But these changes
are needed to support p2p bridge(with hotplug slot) hotplug.

Here is an updated patch. Old patches were not need to be separated,
so I merge them into below patch.

This patch is against 2.6.16-rc1-mm3. I tested them on Tiger4 and
it works fine.

Thanks,
MUNE

o This patch removes IDs to manage slots.
o This patch removes the slot register/unregister processes from the
  init/exit phases. Instead, adds these processes in the bridge 
  add/cleanup phases.

Signed-off-by: MUNEDA Takahiro <[EMAIL PROTECTED]>

 drivers/pci/hotplug/acpiphp.h      |    5 -
 drivers/pci/hotplug/acpiphp_core.c |  118 ++++++++++++++++++-------------------
 drivers/pci/hotplug/acpiphp_glue.c |   48 +++++++--------
 3 files changed, 85 insertions(+), 86 deletions(-)

Index: linux-2.6.16-rc1-mm3/drivers/pci/hotplug/acpiphp.h
===================================================================
--- linux-2.6.16-rc1-mm3.orig/drivers/pci/hotplug/acpiphp.h
+++ linux-2.6.16-rc1-mm3/drivers/pci/hotplug/acpiphp.h
@@ -60,7 +60,6 @@ struct acpiphp_slot;
  * struct slot - slot information for each *physical* slot
  */
 struct slot {
-       u8 number;
        struct hotplug_slot     *hotplug_slot;
        struct list_head        slot_list;
 
@@ -121,7 +120,6 @@ struct acpiphp_slot {
                                           objects (i.e. for each function) */
        struct mutex crit_sect;
 
-       u32             id;             /* slot id (serial #) for hotplug core 
*/
        u8              device;         /* pci device# */
 
        u32             sun;            /* ACPI _SUN (slot unique number) */
@@ -206,12 +204,13 @@ struct acpiphp_attention_info
 /* acpiphp_core.c */
 extern int acpiphp_register_attention(struct acpiphp_attention_info*info);
 extern int acpiphp_unregister_attention(struct acpiphp_attention_info *info);
+extern int acpiphp_register_hotplug_slot(struct acpiphp_slot *slot);
+extern void acpiphp_unregister_hotplug_slot(struct acpiphp_slot *slot);
 
 /* acpiphp_glue.c */
 extern int acpiphp_glue_init (void);
 extern void acpiphp_glue_exit (void);
 extern int acpiphp_get_num_slots (void);
-extern struct acpiphp_slot *get_slot_from_id (int id);
 typedef int (*acpiphp_callback)(struct acpiphp_slot *slot, void *data);
 
 extern int acpiphp_enable_slot (struct acpiphp_slot *slot);
Index: linux-2.6.16-rc1-mm3/drivers/pci/hotplug/acpiphp_core.c
===================================================================
--- linux-2.6.16-rc1-mm3.orig/drivers/pci/hotplug/acpiphp_core.c
+++ linux-2.6.16-rc1-mm3/drivers/pci/hotplug/acpiphp_core.c
@@ -45,6 +45,7 @@
 #include "acpiphp.h"
 
 static LIST_HEAD(slot_list);
+static DEFINE_SPINLOCK(list_lock);
 
 #define MY_NAME        "acpiphp"
 
@@ -341,62 +342,56 @@ static void release_slot(struct hotplug_
        kfree(slot);
 }
 
-/**
- * init_slots - initialize 'struct slot' structures for each slot
- *
- */
-static int __init init_slots(void)
+/* callback routine to initialize 'struct slot' for each slot */
+int acpiphp_register_hotplug_slot(struct acpiphp_slot *acpiphp_slot)
 {
        struct slot *slot;
+       struct hotplug_slot *hotplug_slot;
+       struct hotplug_slot_info *hotplug_slot_info;
        int retval = -ENOMEM;
-       int i;
 
-       for (i = 0; i < num_slots; ++i) {
-               slot = kmalloc(sizeof(struct slot), GFP_KERNEL);
-               if (!slot)
-                       goto error;
-               memset(slot, 0, sizeof(struct slot));
-
-               slot->hotplug_slot = kmalloc(sizeof(struct hotplug_slot), 
GFP_KERNEL);
-               if (!slot->hotplug_slot)
-                       goto error_slot;
-               memset(slot->hotplug_slot, 0, sizeof(struct hotplug_slot));
-
-               slot->hotplug_slot->info = kmalloc(sizeof(struct 
hotplug_slot_info), GFP_KERNEL);
-               if (!slot->hotplug_slot->info)
-                       goto error_hpslot;
-               memset(slot->hotplug_slot->info, 0, sizeof(struct 
hotplug_slot_info));
-
-               slot->hotplug_slot->name = kmalloc(SLOT_NAME_SIZE, GFP_KERNEL);
-               if (!slot->hotplug_slot->name)
-                       goto error_info;
-
-               slot->number = i;
-
-               slot->hotplug_slot->private = slot;
-               slot->hotplug_slot->release = &release_slot;
-               slot->hotplug_slot->ops = &acpi_hotplug_slot_ops;
-
-               slot->acpi_slot = get_slot_from_id(i);
-               slot->hotplug_slot->info->power_status = 
acpiphp_get_power_status(slot->acpi_slot);
-               slot->hotplug_slot->info->attention_status = 0;
-               slot->hotplug_slot->info->latch_status = 
acpiphp_get_latch_status(slot->acpi_slot);
-               slot->hotplug_slot->info->adapter_status = 
acpiphp_get_adapter_status(slot->acpi_slot);
-               slot->hotplug_slot->info->max_bus_speed = PCI_SPEED_UNKNOWN;
-               slot->hotplug_slot->info->cur_bus_speed = PCI_SPEED_UNKNOWN;
-
-               make_slot_name(slot);
-
-               retval = pci_hp_register(slot->hotplug_slot);
-               if (retval) {
-                       err("pci_hp_register failed with error %d\n", retval);
-                       goto error_name;
-               }
-
-               /* add slot to our internal list */
-               list_add(&slot->slot_list, &slot_list);
-               info("Slot [%s] registered\n", slot->hotplug_slot->name);
-       }
+       slot = kzalloc(sizeof(*slot), GFP_KERNEL);
+       if (!slot)
+               goto error;
+
+       slot->hotplug_slot = kzalloc(sizeof(*hotplug_slot), GFP_KERNEL);
+       if (!slot->hotplug_slot)
+               goto error_slot;
+
+       slot->hotplug_slot->info = kzalloc(sizeof(*hotplug_slot_info),
+                                          GFP_KERNEL);
+       if (!slot->hotplug_slot->info)
+               goto error_hpslot;
+
+       slot->hotplug_slot->name = kzalloc(SLOT_NAME_SIZE, GFP_KERNEL);
+       if (!slot->hotplug_slot->name)
+               goto error_info;
+
+       slot->hotplug_slot->private = slot;
+       slot->hotplug_slot->release = &release_slot;
+       slot->hotplug_slot->ops = &acpi_hotplug_slot_ops;
+
+       slot->acpi_slot = acpiphp_slot;
+       slot->hotplug_slot->info->power_status = 
acpiphp_get_power_status(slot->acpi_slot);
+       slot->hotplug_slot->info->attention_status = 0;
+       slot->hotplug_slot->info->latch_status = 
acpiphp_get_latch_status(slot->acpi_slot);
+       slot->hotplug_slot->info->adapter_status = 
acpiphp_get_adapter_status(slot->acpi_slot);
+       slot->hotplug_slot->info->max_bus_speed = PCI_SPEED_UNKNOWN;
+       slot->hotplug_slot->info->cur_bus_speed = PCI_SPEED_UNKNOWN;
+
+       make_slot_name(slot);
+
+       retval = pci_hp_register(slot->hotplug_slot);
+       if (retval) {
+               err("pci_hp_register failed with error %d\n", retval);
+               goto error_name;
+       }
+
+       /* add slot to our internal list */
+       spin_lock(&list_lock);
+       list_add(&slot->slot_list, &slot_list);
+       spin_unlock(&list_lock);
+       info("Slot [%s] registered\n", slot->hotplug_slot->name);
 
        return 0;
 error_name:
@@ -412,16 +407,26 @@ error:
 }
 
 
-static void __exit cleanup_slots (void)
+void acpiphp_unregister_hotplug_slot(struct acpiphp_slot *acpiphp_slot)
 {
        struct list_head *tmp, *n;
        struct slot *slot;
+       int retval = 0;
 
        list_for_each_safe (tmp, n, &slot_list) {
                /* memory will be freed in release_slot callback */
                slot = list_entry(tmp, struct slot, slot_list);
+               if (slot->acpi_slot->sun != acpiphp_slot->sun)
+                       continue;
+
+               spin_lock(&list_lock);
                list_del(&slot->slot_list);
-               pci_hp_deregister(slot->hotplug_slot);
+               spin_unlock(&list_lock);
+               info ("Slot [%s] unregistered\n", slot->hotplug_slot->name);
+
+               retval = pci_hp_deregister(slot->hotplug_slot);
+               if (retval)
+                       err("pci_hp_deregister failed with error %d\n", retval);
        }
 }
 
@@ -436,16 +441,13 @@ static int __init acpiphp_init(void)
 
        /* read all the ACPI info from the system */
        retval = init_acpi();
-       if (retval)
-               return retval;
 
-       return init_slots();
+       return retval;
 }
 
 
 static void __exit acpiphp_exit(void)
 {
-       cleanup_slots();
        /* deallocate internal data structures etc. */
        acpiphp_glue_exit();
 }
Index: linux-2.6.16-rc1-mm3/drivers/pci/hotplug/acpiphp_glue.c
===================================================================
--- linux-2.6.16-rc1-mm3.orig/drivers/pci/hotplug/acpiphp_glue.c
+++ linux-2.6.16-rc1-mm3/drivers/pci/hotplug/acpiphp_glue.c
@@ -154,8 +154,7 @@ register_slot(acpi_handle handle, u32 lv
        acpi_handle tmp;
        acpi_status status = AE_OK;
        unsigned long adr, sun;
-       int device, function;
-       static int num_slots = 0;       /* XXX if we support I/O node 
hotplug... */
+       int device, function, retval;
 
        status = acpi_evaluate_integer(handle, "_ADR", NULL, &adr);
 
@@ -210,7 +209,6 @@ register_slot(acpi_handle handle, u32 lv
 
                memset(slot, 0, sizeof(struct acpiphp_slot));
                slot->bridge = bridge;
-               slot->id = num_slots++;
                slot->device = device;
                slot->sun = sun;
                INIT_LIST_HEAD(&slot->funcs);
@@ -224,6 +222,11 @@ register_slot(acpi_handle handle, u32 lv
                dbg("found ACPI PCI Hotplug slot %d at PCI %04x:%02x:%02x\n",
                                slot->sun, pci_domain_nr(bridge->pci_bus),
                                bridge->pci_bus->number, slot->device);
+               retval = acpiphp_register_hotplug_slot(slot);
+               if (retval) {
+                       warn("acpiphp_register_hotplug_slot failed(err code = 
0x%x)\n", retval);
+                       goto err_exit;
+               }
        }
 
        newfunc->slot = slot;
@@ -254,6 +257,14 @@ register_slot(acpi_handle handle, u32 lv
        }
 
        return AE_OK;
+
+ err_exit:
+       bridge->nr_slots--;
+       bridge->slots = slot->next;
+       kfree(slot);
+       kfree(newfunc);
+
+       return AE_OK;
 }
 
 
@@ -336,9 +347,16 @@ static void init_bridge_misc(struct acpi
        /* decode ACPI 2.0 _HPP (hot plug parameters) */
        decode_hpp(bridge);
 
+       /* must be added to the list prior to calling register_slot */
+       list_add(&bridge->list, &bridge_list);
+
        /* register all slot objects under this bridge */
        status = acpi_walk_namespace(ACPI_TYPE_DEVICE, bridge->handle, (u32)1,
                                     register_slot, bridge, NULL);
+       if (ACPI_FAILURE(status)) {
+               list_del(&bridge->list);
+               return;
+       }
 
        /* install notify handler */
        if (bridge->type != BRIDGE_TYPE_HOST) {
@@ -351,8 +369,6 @@ static void init_bridge_misc(struct acpi
                        err("failed to register interrupt notify handler\n");
                }
        }
-
-       list_add(&bridge->list, &bridge_list);
 }
 
 
@@ -553,6 +569,8 @@ static void cleanup_bridge(struct acpiph
                        list_del(list);
                        kfree(func);
                }
+               acpiphp_unregister_hotplug_slot(slot);
+               list_del(&slot->funcs);
                kfree(slot);
                slot = next;
        }
@@ -1570,26 +1588,6 @@ static int acpiphp_for_each_slot(acpiphp
 }
 #endif
 
-/* search matching slot from id  */
-struct acpiphp_slot *get_slot_from_id(int id)
-{
-       struct list_head *node;
-       struct acpiphp_bridge *bridge;
-       struct acpiphp_slot *slot;
-
-       list_for_each (node, &bridge_list) {
-               bridge = (struct acpiphp_bridge *)node;
-               for (slot = bridge->slots; slot; slot = slot->next)
-                       if (slot->id == id)
-                               return slot;
-       }
-
-       /* should never happen! */
-       err("%s: no object for id %d\n", __FUNCTION__, id);
-       WARN_ON(1);
-       return NULL;
-}
-
 
 /**
  * acpiphp_enable_slot - power on slot

-
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

Reply via email to