On Friday, January 21, 2011, Jeff Chua wrote: > 2011/1/21 Rafael J. Wysocki <rjw@xxxxxxx>: > > Thanks, but unfortunately this wasn't conclusive. Please apply the patch below > > instead of the previous one (on top of [1/11] - [11/11]) and collect dmesg > > output containing a (failing) suspend attempt. > > > > Please attach full dmesg, so that I can see the size and address of your > > system's NVS region. > > Attached. So, below is a replacement for [11/11]. Please test it on top of [1/11] - [10/11] (the current Linus' tree already contains [1/11] and [2/11]) and let me know if it works for you (in either case, please attach dmesg output containing a suspend attempt). If it works, I'll remove the diagnostic messages and submit along with the rest of the patchset. Thanks, Rafael --- Subject: ACPI / PM: Make NVS save/restore code use slightly less memory (test) Remove the unnecessary field phys_start from struct nvs_page and modify the code in nvs.c to use an array of struct nvs_page objects instead of a list which makes it a bit more straightforward (in addition to saving a little memory). Not-signed-off-by: Rafael J. Wysocki <rjw@xxxxxxx> --- drivers/acpi/nvs.c | 125 +++++++++++++++++++++++++++++++++-------------------- 1 file changed, 79 insertions(+), 46 deletions(-) Index: linux-2.6/drivers/acpi/nvs.c =================================================================== --- linux-2.6.orig/drivers/acpi/nvs.c +++ linux-2.6/drivers/acpi/nvs.c @@ -22,15 +22,15 @@ */ struct nvs_page { - unsigned long phys_start; unsigned int size; - void *kaddr; + void __iomem *kaddr; void *data; bool unmap; - struct list_head node; }; -static LIST_HEAD(nvs_list); +static struct nvs_page *nvs; +static unsigned long nvs_start; +static unsigned int nvs_span; /** * suspend_nvs_register - register platform NVS memory region to save @@ -43,31 +43,40 @@ static LIST_HEAD(nvs_list); */ int suspend_nvs_register(unsigned long start, unsigned long size) { - struct nvs_page *entry, *next; + struct nvs_page *p, *p_end; + unsigned int head, tail; - while (size > 0) { - unsigned int nr_bytes; + pr_info("PM: Registering ACPI NVS region at %lx (%ld bytes)\n", + start, size); - entry = kzalloc(sizeof(struct nvs_page), GFP_KERNEL); - if (!entry) - goto Error; - - list_add_tail(&entry->node, &nvs_list); - entry->phys_start = start; - nr_bytes = PAGE_SIZE - (start & ~PAGE_MASK); - entry->size = (size < nr_bytes) ? size : nr_bytes; - - start += entry->size; - size -= entry->size; + nvs_start = start; + head = PAGE_SIZE - (start & ~PAGE_MASK); + if (head) { + nvs_span = 1; + size -= size > head ? head : size; + } else { + nvs_span = 0; + head = PAGE_SIZE; } - return 0; + nvs_span += size >> PAGE_SHIFT; + tail = size & ~PAGE_MASK; + if (tail) + nvs_span++; + else + tail = PAGE_SIZE; + + nvs = kzalloc(sizeof(*nvs) * nvs_span, GFP_KERNEL); + if (!nvs) + return -ENOMEM; + + p = nvs; + p->size = head; + for (p++, p_end = nvs + nvs_span - 1; p < p_end; p++) + p->size = PAGE_SIZE; + if (p_end > nvs) + p_end->size = tail; - Error: - list_for_each_entry_safe(entry, next, &nvs_list, node) { - list_del(&entry->node); - kfree(entry); - } - return -ENOMEM; + return 0; } /** @@ -75,17 +84,23 @@ int suspend_nvs_register(unsigned long s */ void suspend_nvs_free(void) { - struct nvs_page *entry; + struct nvs_page *entry, *end; - list_for_each_entry(entry, &nvs_list, node) + for (entry = nvs, end = nvs + nvs_span; entry < end; entry++) if (entry->data) { free_page((unsigned long)entry->data); entry->data = NULL; if (entry->kaddr) { if (entry->unmap) { + pr_info("%s: Unmapping %p\n", __func__, + entry->kaddr); + iounmap(entry->kaddr); entry->unmap = false; } else { + pr_info("%s: Releasing %p\n", __func__, + entry->kaddr); + acpi_os_unmap_memory(entry->kaddr, entry->size); } @@ -99,9 +114,9 @@ void suspend_nvs_free(void) */ int suspend_nvs_alloc(void) { - struct nvs_page *entry; + struct nvs_page *entry, *end; - list_for_each_entry(entry, &nvs_list, node) { + for (entry = nvs, end = nvs + nvs_span; entry < end; entry++) { entry->data = (void *)__get_free_page(GFP_KERNEL); if (!entry->data) { suspend_nvs_free(); @@ -116,26 +131,44 @@ int suspend_nvs_alloc(void) */ int suspend_nvs_save(void) { - struct nvs_page *entry; + struct nvs_page *entry, *end; + unsigned long phys = nvs_start; printk(KERN_INFO "PM: Saving platform NVS memory\n"); - list_for_each_entry(entry, &nvs_list, node) - if (entry->data) { - unsigned long phys = entry->phys_start; - unsigned int size = entry->size; + for (entry = nvs, end = nvs + nvs_span; entry < end; entry++) { + unsigned int size = entry->size; + void __iomem *kaddr; - entry->kaddr = acpi_os_get_iomem(phys, size); - if (!entry->kaddr) { - entry->kaddr = acpi_os_ioremap(phys, size); - entry->unmap = true; - } - if (!entry->kaddr) { - suspend_nvs_free(); - return -ENOMEM; - } - memcpy(entry->data, entry->kaddr, entry->size); + if (!entry->data) { + suspend_nvs_free(); + return -ENOMEM; + } + + pr_info("%s: Trying to map %lx, %d\n", __func__, phys, size); + + kaddr = acpi_os_get_iomem(phys, size); + if (!kaddr) { + kaddr = acpi_os_ioremap(phys, size); + entry->unmap = !!kaddr; + } else { + pr_info("%s: Got address %p\n", __func__, kaddr); } + entry->kaddr = kaddr; + + if (!entry->kaddr) { + pr_info("%s: Mapping failed\n", __func__); + + suspend_nvs_free(); + return -EIO; + } else { + pr_info("%s: Using address %p\n", __func__, entry->kaddr); + } + + memcpy(entry->data, entry->kaddr, size); + + phys += size; + } return 0; } @@ -148,11 +181,11 @@ int suspend_nvs_save(void) */ void suspend_nvs_restore(void) { - struct nvs_page *entry; + struct nvs_page *entry, *end; printk(KERN_INFO "PM: Restoring platform NVS memory\n"); - list_for_each_entry(entry, &nvs_list, node) + for (entry = nvs, end = nvs + nvs_span; entry < end; entry++) if (entry->data) memcpy(entry->kaddr, entry->data, entry->size); } -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html