ChangeSet 1.1267.48.2, 2004/03/31 15:29:24-08:00, t-kochi@bq.jp.nec.com [PATCH] PCI Hotplug: acpiphp unable to power off slots Hi Greg, Here's acpiphp updates for 2.4 (cleanup patch + changes from 2.6). From: Takayoshi Kochi Subject: Re: [Pcihpd-discuss] acpiphp unable to power off slots Date: Fri, 05 Mar 2004 19:37:37 +0900 (JST) > Hi Gary, > > Thanks for looking at the problem. I greatly appreciate that. > Your analysis and fix looks correct to me. > > I had a luck to test on the real machine (ia64) today and > tested the patch with your previous I/O space patch. > I confirmed it works for an e100 card. > > Attached patch includes the I/O space fix and applies to 2.6.3. > This should also solve the problem Maeda-san reported in January > (sorry for replying so late!) > > Here are changes in the patch: > > - fix the acpiphp driver not powering down a PCI card (from Gary Hade) > - fix I/O space size calculation and ISA aliasing (from Gary Hade) > - fix some debug messages > - only execute ACPI methods on the first existing function > > Greg, could you pick this up? > Then I'll rework the pending 2.4 patch. > > From: Gary Hade > Subject: [Pcihpd-discuss] acpiphp unable to power off slots > Date: Thu, 4 Mar 2004 18:09:09 -0800 > > > Kochi-san, > > While verifying that the PCI hotplug support in SLES9 Preview 2 > > for ia64 (based on 2.6.2 kernel) was working correctly I discovered > > that it was impossible to power off a PCI adapter containing slots > > by echoing 0 to the power file. While the slot power off problem > > that I previously reported only appeared to happen only with cards > > containing a PCI-to-PCI bridge, the current problem happens with > > _any_ PCI adapter. > > > > After some investigation I believe I found a flaw in the fix you > > provided for the previously reported slow power off problem. Within > > power_off_slot() the following expressions will always be false > > because func->pci_dev will always be set to NULL during an earlier > > call to disable_device(). > > (func->pci_dev && (func->flags & FUNC_HAS_PS3) > > (func->pci_dev && (func->flags & FUNC_HAS_EJ0) > > As a result, acpi_evaluate_object() is never called to power > > down the slot. > > > > Below is a possible fix for the problem. Please review as soon > > as possible. drivers/hotplug/acpiphp.h | 5 +- drivers/hotplug/acpiphp_core.c | 30 ++++++++++++++-- drivers/hotplug/acpiphp_glue.c | 56 +++++++++++++++++------------- drivers/hotplug/acpiphp_pci.c | 18 ++++----- drivers/hotplug/acpiphp_res.c | 5 +- drivers/hotplug/pci_hotplug.h | 6 +++ drivers/hotplug/pci_hotplug_core.c | 69 +++++++++++++++++++++++++++++++++++++ 7 files changed, 148 insertions(+), 41 deletions(-) diff -Nru a/drivers/hotplug/acpiphp.h b/drivers/hotplug/acpiphp.h --- a/drivers/hotplug/acpiphp.h Wed Apr 28 15:20:52 2004 +++ b/drivers/hotplug/acpiphp.h Wed Apr 28 15:20:52 2004 @@ -202,7 +202,7 @@ #define SLOT_POWEREDON (0x00000001) #define SLOT_ENABLED (0x00000002) -#define SLOT_MULTIFUNCTION (x000000004) +#define SLOT_MULTIFUNCTION (0x00000004) /* function flags */ @@ -213,8 +213,6 @@ #define FUNC_HAS_PS2 (0x00000040) #define FUNC_HAS_PS3 (0x00000080) -#define FUNC_EXISTS (0x10000000) /* to make sure we call _EJ0 only for existing funcs */ - /* function prototypes */ /* acpiphp_glue.c */ @@ -232,6 +230,7 @@ extern u8 acpiphp_get_attention_status (struct acpiphp_slot *slot); extern u8 acpiphp_get_latch_status (struct acpiphp_slot *slot); extern u8 acpiphp_get_adapter_status (struct acpiphp_slot *slot); +extern u32 acpiphp_get_address (struct acpiphp_slot *slot); /* acpiphp_pci.c */ extern struct pci_dev *acpiphp_allocate_pcidev (struct pci_bus *pbus, int dev, int fn); diff -Nru a/drivers/hotplug/acpiphp_core.c b/drivers/hotplug/acpiphp_core.c --- a/drivers/hotplug/acpiphp_core.c Wed Apr 28 15:20:52 2004 +++ b/drivers/hotplug/acpiphp_core.c Wed Apr 28 15:20:52 2004 @@ -30,14 +30,14 @@ * */ -#include -#include +#include #include + +#include #include #include #include #include -#include #include "pci_hotplug.h" #include "acpiphp.h" @@ -73,6 +73,7 @@ static int get_attention_status (struct hotplug_slot *slot, u8 *value); static int get_latch_status (struct hotplug_slot *slot, u8 *value); static int get_adapter_status (struct hotplug_slot *slot, u8 *value); +static int get_address (struct hotplug_slot *slot, u32 *value); static int get_max_bus_speed (struct hotplug_slot *hotplug_slot, enum pci_bus_speed *value); static int get_cur_bus_speed (struct hotplug_slot *hotplug_slot, enum pci_bus_speed *value); @@ -86,6 +87,7 @@ .get_attention_status = get_attention_status, .get_latch_status = get_latch_status, .get_adapter_status = get_adapter_status, + .get_address = get_address, .get_max_bus_speed = get_max_bus_speed, .get_cur_bus_speed = get_cur_bus_speed, }; @@ -317,6 +319,28 @@ dbg("%s - physical_slot = %s\n", __FUNCTION__, hotplug_slot->name); *value = acpiphp_get_adapter_status(slot->acpi_slot); + + return retval; +} + + +/** + * get_address - get pci address of a slot + * @hotplug_slot: slot to get status + * @busdev: pointer to struct pci_busdev (seg, bus, dev) + * + */ +static int get_address (struct hotplug_slot *hotplug_slot, u32 *value) +{ + struct slot *slot = get_slot(hotplug_slot, __FUNCTION__); + int retval = 0; + + if (slot == NULL) + return -ENODEV; + + dbg("%s - physical_slot = %s\n", __FUNCTION__, hotplug_slot->name); + + *value = acpiphp_get_address(slot->acpi_slot); return retval; } diff -Nru a/drivers/hotplug/acpiphp_glue.c b/drivers/hotplug/acpiphp_glue.c --- a/drivers/hotplug/acpiphp_glue.c Wed Apr 28 15:20:52 2004 +++ b/drivers/hotplug/acpiphp_glue.c Wed Apr 28 15:20:52 2004 @@ -26,12 +26,12 @@ * */ -#include -#include +#include #include + +#include #include #include -#include #include #include "pci_hotplug.h" @@ -783,7 +783,7 @@ struct list_head *l; int retval = 0; - /* is this already enabled? */ + /* if already enabled, just skip */ if (slot->flags & SLOT_POWEREDON) goto err_exit; @@ -791,14 +791,14 @@ func = list_entry(l, struct acpiphp_func, sibling); if (func->flags & FUNC_HAS_PS0) { - dbg("%s: executing _PS0 on %s\n", __FUNCTION__, - func->pci_dev->slot_name); + dbg("%s: executing _PS0\n", __FUNCTION__); status = acpi_evaluate_object(func->handle, "_PS0", NULL, NULL); if (ACPI_FAILURE(status)) { warn("%s: _PS0 failed\n", __FUNCTION__); retval = -1; goto err_exit; - } + } else + break; } } @@ -821,20 +821,21 @@ int retval = 0; - /* is this already enabled? */ + /* if already disabled, just skip */ if ((slot->flags & SLOT_POWEREDON) == 0) goto err_exit; list_for_each (l, &slot->funcs) { func = list_entry(l, struct acpiphp_func, sibling); - if (func->flags & (FUNC_HAS_PS3 | FUNC_EXISTS)) { + if (func->pci_dev && (func->flags & FUNC_HAS_PS3)) { status = acpi_evaluate_object(func->handle, "_PS3", NULL, NULL); if (ACPI_FAILURE(status)) { warn("%s: _PS3 failed\n", __FUNCTION__); retval = -1; goto err_exit; - } + } else + break; } } @@ -842,20 +843,19 @@ func = list_entry(l, struct acpiphp_func, sibling); /* We don't want to call _EJ0 on non-existing functions. */ - if (func->flags & (FUNC_HAS_EJ0 | FUNC_EXISTS)) { + if (func->pci_dev && (func->flags & FUNC_HAS_EJ0)) { /* _EJ0 method take one argument */ arg_list.count = 1; arg_list.pointer = &arg; arg.type = ACPI_TYPE_INTEGER; arg.integer.value = 1; - status = acpi_evaluate_object(func->handle, "_EJ0", &arg_list, NULL); if (ACPI_FAILURE(status)) { warn("%s: _EJ0 failed\n", __FUNCTION__); retval = -1; goto err_exit; - } - func->flags &= (~FUNC_EXISTS); + } else + break; } } @@ -937,8 +937,6 @@ retval = acpiphp_configure_function(func); if (retval) goto err_exit; - - func->flags |= FUNC_EXISTS; } slot->flags |= SLOT_ENABLED; @@ -967,15 +965,12 @@ list_for_each (l, &slot->funcs) { func = list_entry(l, struct acpiphp_func, sibling); - if (func->pci_dev) { - if (acpiphp_unconfigure_function(func) == 0) { - func->pci_dev = NULL; - } else { + if (func->pci_dev) + if (acpiphp_unconfigure_function(func)) { err("failed to unconfigure device\n"); retval = -1; goto err_exit; } - } } slot->flags &= (~SLOT_ENABLED); @@ -1355,7 +1350,7 @@ up(&slot->crit_sect); goto err_exit; } - enabled++; + disabled++; } } else { /* if disabled but present, enable */ @@ -1366,7 +1361,7 @@ up(&slot->crit_sect); goto err_exit; } - disabled++; + enabled++; } } } @@ -1431,4 +1426,19 @@ sta = get_slot_status(slot); return (sta == 0) ? 0 : 1; +} + + +/* + * pci address (seg/bus/dev) + */ +u32 acpiphp_get_address (struct acpiphp_slot *slot) +{ + u32 address; + + address = ((slot->bridge->seg) << 16) | + ((slot->bridge->bus) << 8) | + slot->device; + + return address; } diff -Nru a/drivers/hotplug/acpiphp_pci.c b/drivers/hotplug/acpiphp_pci.c --- a/drivers/hotplug/acpiphp_pci.c Wed Apr 28 15:20:52 2004 +++ b/drivers/hotplug/acpiphp_pci.c Wed Apr 28 15:20:52 2004 @@ -29,11 +29,11 @@ * */ -#include -#include +#include #include + +#include #include -#include #include "pci_hotplug.h" #include "acpiphp.h" @@ -78,8 +78,8 @@ if (bar & PCI_BASE_ADDRESS_SPACE_IO) { /* This is IO */ - len = bar & 0xFFFFFFFC; - len = ~len + 1; + len = bar & (PCI_BASE_ADDRESS_IO_MASK & 0xFFFF); + len = len & ~(len - 1); dbg("len in IO %x, BAR %d\n", len, count); @@ -340,8 +340,8 @@ if (len & PCI_BASE_ADDRESS_SPACE_IO) { /* This is IO */ base = bar & 0xFFFFFFFC; - len &= 0xFFFFFFFC; - len = ~len + 1; + len = len & (PCI_BASE_ADDRESS_IO_MASK & 0xFFFF); + len = len & ~(len - 1); dbg("BAR[%d] %08x - %08x (IO)\n", count, (u32)base, (u32)base + len - 1); @@ -465,8 +465,8 @@ if (len & PCI_BASE_ADDRESS_SPACE_IO) { /* This is IO */ base = bar & 0xFFFFFFFC; - len &= 0xFFFFFFFC; - len = ~len + 1; + len = len & (PCI_BASE_ADDRESS_IO_MASK & 0xFFFF); + len = len & ~(len - 1); dbg("BAR[%d] %08x - %08x (IO)\n", count, (u32)base, (u32)base + len - 1); diff -Nru a/drivers/hotplug/acpiphp_res.c b/drivers/hotplug/acpiphp_res.c --- a/drivers/hotplug/acpiphp_res.c Wed Apr 28 15:20:52 2004 +++ b/drivers/hotplug/acpiphp_res.c Wed Apr 28 15:20:52 2004 @@ -29,7 +29,7 @@ * */ -#include +#include #include #include @@ -39,7 +39,6 @@ #include #include #include -#include #include #include @@ -225,7 +224,7 @@ } /* End of too big on top end */ /* For IO make sure it's not in the ISA aliasing space */ - if (node->base & 0x300L) + if ((node->base & 0x300L) && !(node->base & 0xfffff000)) continue; /* If we got here, then it is the right size diff -Nru a/drivers/hotplug/pci_hotplug.h b/drivers/hotplug/pci_hotplug.h --- a/drivers/hotplug/pci_hotplug.h Wed Apr 28 15:20:52 2004 +++ b/drivers/hotplug/pci_hotplug.h Wed Apr 28 15:20:52 2004 @@ -69,6 +69,9 @@ * @get_adapter_status: Called to get see if an adapter is present in the slot or not. * If this field is NULL, the value passed in the struct hotplug_slot_info * will be used when this value is requested by a user. + * @get_address: Called to get pci address of a slot. + * If this field is NULL, the value passed in the struct hotplug_slot_info + * will be used when this value is requested by a user. * @get_max_bus_speed: Called to get the max bus speed for a slot. * If this field is NULL, the value passed in the struct hotplug_slot_info * will be used when this value is requested by a user. @@ -91,6 +94,7 @@ int (*get_attention_status) (struct hotplug_slot *slot, u8 *value); int (*get_latch_status) (struct hotplug_slot *slot, u8 *value); int (*get_adapter_status) (struct hotplug_slot *slot, u8 *value); + int (*get_address) (struct hotplug_slot *slot, u32 *value); int (*get_max_bus_speed) (struct hotplug_slot *slot, enum pci_bus_speed *value); int (*get_cur_bus_speed) (struct hotplug_slot *slot, enum pci_bus_speed *value); }; @@ -101,6 +105,7 @@ * @attention_status: if the attention light is enabled or not (1/0) * @latch_status: if the latch (if any) is open or closed (1/0) * @adapter_present: if there is a pci board present in the slot or not (1/0) + * @address: (domain << 16 | bus << 8 | dev) * * Used to notify the hotplug pci core of the status of a specific slot. */ @@ -109,6 +114,7 @@ u8 attention_status; u8 latch_status; u8 adapter_status; + u32 address; enum pci_bus_speed max_bus_speed; enum pci_bus_speed cur_bus_speed; }; diff -Nru a/drivers/hotplug/pci_hotplug_core.c b/drivers/hotplug/pci_hotplug_core.c --- a/drivers/hotplug/pci_hotplug_core.c Wed Apr 28 15:20:52 2004 +++ b/drivers/hotplug/pci_hotplug_core.c Wed Apr 28 15:20:52 2004 @@ -74,6 +74,7 @@ struct dentry *attention_dentry; struct dentry *latch_dentry; struct dentry *adapter_dentry; + struct dentry *address_dentry; struct dentry *test_dentry; struct dentry *max_bus_speed_dentry; struct dentry *cur_bus_speed_dentry; @@ -312,6 +313,15 @@ llseek: default_file_lseek, }; +/* file ops for the "address" files */ +static ssize_t address_read_file (struct file *file, char *buf, size_t count, loff_t *offset); +static struct file_operations address_file_operations = { + read: address_read_file, + write: default_write_file, + open: default_open, + llseek: default_file_lseek, +}; + /* file ops for the "max bus speed" files */ static ssize_t max_bus_speed_read_file (struct file *file, char *buf, size_t count, loff_t *offset); static struct file_operations max_bus_speed_file_operations = { @@ -566,6 +576,7 @@ GET_STATUS(attention_status, u8) GET_STATUS(latch_status, u8) GET_STATUS(adapter_status, u8) +GET_STATUS(address, u32) GET_STATUS(max_bus_speed, enum pci_bus_speed) GET_STATUS(cur_bus_speed, enum pci_bus_speed) @@ -859,6 +870,52 @@ return retval; } +static ssize_t address_read_file (struct file *file, char *buf, size_t count, loff_t *offset) +{ + struct hotplug_slot *slot = file->private_data; + unsigned char *page; + int retval; + int len; + u32 address; + + dbg("count = %d, offset = %lld\n", count, *offset); + + if (*offset < 0) + return -EINVAL; + if (count <= 0) + return 0; + if (*offset != 0) + return 0; + + if (slot == NULL) { + dbg("slot == NULL???\n"); + return -ENODEV; + } + + page = (unsigned char *)__get_free_page(GFP_KERNEL); + if (!page) + return -ENOMEM; + + retval = get_address (slot, &address); + if (retval) + goto exit; + len = sprintf (page, "%04x:%02x:%02x\n", + (address >> 16) & 0xffff, + (address >> 8) & 0xff, + address & 0xff); + + if (copy_to_user (buf, page, len)) { + retval = -EFAULT; + goto exit; + } + *offset += len; + retval = len; + +exit: + free_page((unsigned long)page); + return retval; +} + static char *unknown_speed = "Unknown bus speed"; static ssize_t max_bus_speed_read_file (struct file *file, char *buf, size_t count, loff_t *offset) @@ -1055,6 +1112,13 @@ core->dir_dentry, slot, &presence_file_operations); + if (slot->ops->get_address) + core->address_dentry = + fs_create_file ("address", + S_IFREG | S_IRUGO, + core->dir_dentry, slot, + &address_file_operations); + if (slot->ops->get_max_bus_speed) core->max_bus_speed_dentry = fs_create_file ("max_bus_speed", @@ -1092,6 +1156,8 @@ fs_remove_file (core->latch_dentry); if (core->adapter_dentry) fs_remove_file (core->adapter_dentry); + if (core->address_dentry) + fs_remove_file (core->address_dentry); if (core->max_bus_speed_dentry) fs_remove_file (core->max_bus_speed_dentry); if (core->cur_bus_speed_dentry) @@ -1243,6 +1309,9 @@ if ((core->adapter_dentry) && (temp->info->adapter_status != info->adapter_status)) update_dentry_inode_time (core->adapter_dentry); + if ((core->address_dentry) && + (temp->info->address != info->address)) + update_dentry_inode_time (core->address_dentry); if ((core->cur_bus_speed_dentry) && (temp->info->cur_bus_speed != info->cur_bus_speed)) update_dentry_inode_time (core->cur_bus_speed_dentry);