From fe2af11c220c7bb3a67f7aec0594811e5c59e019 Mon Sep 17 00:00:00 2001 From: Axel Lin Date: Tue, 3 Apr 2012 10:07:01 +0800 Subject: firewire: use module_pci_driver This patch converts the drivers in drivers/firewire/* to use module_pci_driver() macro which makes the code smaller and a bit simpler. Signed-off-by: Axel Lin Signed-off-by: Stefan Richter diff --git a/drivers/firewire/nosy.c b/drivers/firewire/nosy.c index a7c4422..4ebfb22 100644 --- a/drivers/firewire/nosy.c +++ b/drivers/firewire/nosy.c @@ -693,6 +693,8 @@ static struct pci_device_id pci_table[] __devinitdata = { { } /* Terminating entry */ }; +MODULE_DEVICE_TABLE(pci, pci_table); + static struct pci_driver lynx_pci_driver = { .name = driver_name, .id_table = pci_table, @@ -700,22 +702,8 @@ static struct pci_driver lynx_pci_driver = { .remove = remove_card, }; +module_pci_driver(lynx_pci_driver); + MODULE_AUTHOR("Kristian Hoegsberg"); MODULE_DESCRIPTION("Snoop mode driver for TI pcilynx 1394 controllers"); MODULE_LICENSE("GPL"); -MODULE_DEVICE_TABLE(pci, pci_table); - -static int __init nosy_init(void) -{ - return pci_register_driver(&lynx_pci_driver); -} - -static void __exit nosy_cleanup(void) -{ - pci_unregister_driver(&lynx_pci_driver); - - pr_info("Unloaded %s\n", driver_name); -} - -module_init(nosy_init); -module_exit(nosy_cleanup); diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c index 2b54600..67c8d27 100644 --- a/drivers/firewire/ohci.c +++ b/drivers/firewire/ohci.c @@ -3789,6 +3789,8 @@ static struct pci_driver fw_ohci_pci_driver = { #endif }; +module_pci_driver(fw_ohci_pci_driver); + MODULE_AUTHOR("Kristian Hoegsberg "); MODULE_DESCRIPTION("Driver for PCI OHCI IEEE1394 controllers"); MODULE_LICENSE("GPL"); @@ -3797,16 +3799,3 @@ MODULE_LICENSE("GPL"); #ifndef CONFIG_IEEE1394_OHCI1394_MODULE MODULE_ALIAS("ohci1394"); #endif - -static int __init fw_ohci_init(void) -{ - return pci_register_driver(&fw_ohci_pci_driver); -} - -static void __exit fw_ohci_cleanup(void) -{ - pci_unregister_driver(&fw_ohci_pci_driver); -} - -module_init(fw_ohci_init); -module_exit(fw_ohci_cleanup); -- cgit v0.10.2 From 0b6c4857f7684f6d3f59e0506f62953575346978 Mon Sep 17 00:00:00 2001 From: Stefan Richter Date: Mon, 9 Apr 2012 20:51:18 +0200 Subject: firewire: core: fix DMA mapping direction Seen with recent libdc1394: If a client mmap()s the buffer of an isochronous reception buffer with PROT_READ|PROT_WRITE instead of just PROT_READ, firewire-core sets the wrong DMA mapping direction during buffer initialization. The fix is to split fw_iso_buffer_init() into allocation and DMA mapping and to perform the latter after both buffer and DMA context were allocated. Buffer allocation and context allocation may happen in any order, but we need the context type (reception or transmission) in order to set the DMA direction of the buffer. Signed-off-by: Stefan Richter diff --git a/drivers/firewire/core-cdev.c b/drivers/firewire/core-cdev.c index 2e6b245..2783f69 100644 --- a/drivers/firewire/core-cdev.c +++ b/drivers/firewire/core-cdev.c @@ -22,6 +22,7 @@ #include #include #include +#include #include #include #include @@ -70,6 +71,7 @@ struct client { u64 iso_closure; struct fw_iso_buffer buffer; unsigned long vm_start; + bool buffer_is_mapped; struct list_head phy_receiver_link; u64 phy_receiver_closure; @@ -959,11 +961,20 @@ static void iso_mc_callback(struct fw_iso_context *context, sizeof(e->interrupt), NULL, 0); } +static enum dma_data_direction iso_dma_direction(struct fw_iso_context *context) +{ + if (context->type == FW_ISO_CONTEXT_TRANSMIT) + return DMA_TO_DEVICE; + else + return DMA_FROM_DEVICE; +} + static int ioctl_create_iso_context(struct client *client, union ioctl_arg *arg) { struct fw_cdev_create_iso_context *a = &arg->create_iso_context; struct fw_iso_context *context; fw_iso_callback_t cb; + int ret; BUILD_BUG_ON(FW_CDEV_ISO_CONTEXT_TRANSMIT != FW_ISO_CONTEXT_TRANSMIT || FW_CDEV_ISO_CONTEXT_RECEIVE != FW_ISO_CONTEXT_RECEIVE || @@ -1004,8 +1015,21 @@ static int ioctl_create_iso_context(struct client *client, union ioctl_arg *arg) if (client->iso_context != NULL) { spin_unlock_irq(&client->lock); fw_iso_context_destroy(context); + return -EBUSY; } + if (!client->buffer_is_mapped) { + ret = fw_iso_buffer_map_dma(&client->buffer, + client->device->card, + iso_dma_direction(context)); + if (ret < 0) { + spin_unlock_irq(&client->lock); + fw_iso_context_destroy(context); + + return ret; + } + client->buffer_is_mapped = true; + } client->iso_closure = a->closure; client->iso_context = context; spin_unlock_irq(&client->lock); @@ -1651,7 +1675,6 @@ static long fw_device_op_compat_ioctl(struct file *file, static int fw_device_op_mmap(struct file *file, struct vm_area_struct *vma) { struct client *client = file->private_data; - enum dma_data_direction direction; unsigned long size; int page_count, ret; @@ -1674,20 +1697,28 @@ static int fw_device_op_mmap(struct file *file, struct vm_area_struct *vma) if (size & ~PAGE_MASK) return -EINVAL; - if (vma->vm_flags & VM_WRITE) - direction = DMA_TO_DEVICE; - else - direction = DMA_FROM_DEVICE; - - ret = fw_iso_buffer_init(&client->buffer, client->device->card, - page_count, direction); + ret = fw_iso_buffer_alloc(&client->buffer, page_count); if (ret < 0) return ret; - ret = fw_iso_buffer_map(&client->buffer, vma); + spin_lock_irq(&client->lock); + if (client->iso_context) { + ret = fw_iso_buffer_map_dma(&client->buffer, + client->device->card, + iso_dma_direction(client->iso_context)); + client->buffer_is_mapped = (ret == 0); + } + spin_unlock_irq(&client->lock); if (ret < 0) - fw_iso_buffer_destroy(&client->buffer, client->device->card); + goto fail; + ret = fw_iso_buffer_map_vma(&client->buffer, vma); + if (ret < 0) + goto fail; + + return 0; + fail: + fw_iso_buffer_destroy(&client->buffer, client->device->card); return ret; } diff --git a/drivers/firewire/core-iso.c b/drivers/firewire/core-iso.c index d156582..8382e27 100644 --- a/drivers/firewire/core-iso.c +++ b/drivers/firewire/core-iso.c @@ -39,52 +39,73 @@ * Isochronous DMA context management */ -int fw_iso_buffer_init(struct fw_iso_buffer *buffer, struct fw_card *card, - int page_count, enum dma_data_direction direction) +int fw_iso_buffer_alloc(struct fw_iso_buffer *buffer, int page_count) { - int i, j; - dma_addr_t address; - - buffer->page_count = page_count; - buffer->direction = direction; + int i; + buffer->page_count = 0; + buffer->page_count_mapped = 0; buffer->pages = kmalloc(page_count * sizeof(buffer->pages[0]), GFP_KERNEL); if (buffer->pages == NULL) - goto out; + return -ENOMEM; - for (i = 0; i < buffer->page_count; i++) { + for (i = 0; i < page_count; i++) { buffer->pages[i] = alloc_page(GFP_KERNEL | GFP_DMA32 | __GFP_ZERO); if (buffer->pages[i] == NULL) - goto out_pages; + break; + } + buffer->page_count = i; + if (i < page_count) { + fw_iso_buffer_destroy(buffer, NULL); + return -ENOMEM; + } + return 0; +} + +int fw_iso_buffer_map_dma(struct fw_iso_buffer *buffer, struct fw_card *card, + enum dma_data_direction direction) +{ + dma_addr_t address; + int i; + + buffer->direction = direction; + + for (i = 0; i < buffer->page_count; i++) { address = dma_map_page(card->device, buffer->pages[i], 0, PAGE_SIZE, direction); - if (dma_mapping_error(card->device, address)) { - __free_page(buffer->pages[i]); - goto out_pages; - } + if (dma_mapping_error(card->device, address)) + break; + set_page_private(buffer->pages[i], address); } + buffer->page_count_mapped = i; + if (i < buffer->page_count) + return -ENOMEM; return 0; +} - out_pages: - for (j = 0; j < i; j++) { - address = page_private(buffer->pages[j]); - dma_unmap_page(card->device, address, - PAGE_SIZE, direction); - __free_page(buffer->pages[j]); - } - kfree(buffer->pages); - out: - buffer->pages = NULL; +int fw_iso_buffer_init(struct fw_iso_buffer *buffer, struct fw_card *card, + int page_count, enum dma_data_direction direction) +{ + int ret; + + ret = fw_iso_buffer_alloc(buffer, page_count); + if (ret < 0) + return ret; + + ret = fw_iso_buffer_map_dma(buffer, card, direction); + if (ret < 0) + fw_iso_buffer_destroy(buffer, card); - return -ENOMEM; + return ret; } EXPORT_SYMBOL(fw_iso_buffer_init); -int fw_iso_buffer_map(struct fw_iso_buffer *buffer, struct vm_area_struct *vma) +int fw_iso_buffer_map_vma(struct fw_iso_buffer *buffer, + struct vm_area_struct *vma) { unsigned long uaddr; int i, err; @@ -107,15 +128,18 @@ void fw_iso_buffer_destroy(struct fw_iso_buffer *buffer, int i; dma_addr_t address; - for (i = 0; i < buffer->page_count; i++) { + for (i = 0; i < buffer->page_count_mapped; i++) { address = page_private(buffer->pages[i]); dma_unmap_page(card->device, address, PAGE_SIZE, buffer->direction); - __free_page(buffer->pages[i]); } + for (i = 0; i < buffer->page_count; i++) + __free_page(buffer->pages[i]); kfree(buffer->pages); buffer->pages = NULL; + buffer->page_count = 0; + buffer->page_count_mapped = 0; } EXPORT_SYMBOL(fw_iso_buffer_destroy); diff --git a/drivers/firewire/core.h b/drivers/firewire/core.h index 9047f55..94257ae 100644 --- a/drivers/firewire/core.h +++ b/drivers/firewire/core.h @@ -3,6 +3,7 @@ #include #include +#include #include #include #include @@ -169,7 +170,11 @@ void fw_node_event(struct fw_card *card, struct fw_node *node, int event); /* -iso */ -int fw_iso_buffer_map(struct fw_iso_buffer *buffer, struct vm_area_struct *vma); +int fw_iso_buffer_alloc(struct fw_iso_buffer *buffer, int page_count); +int fw_iso_buffer_map_dma(struct fw_iso_buffer *buffer, struct fw_card *card, + enum dma_data_direction direction); +int fw_iso_buffer_map_vma(struct fw_iso_buffer *buffer, + struct vm_area_struct *vma); /* -topology */ diff --git a/include/linux/firewire.h b/include/linux/firewire.h index cdc9b71..0a19057 100644 --- a/include/linux/firewire.h +++ b/include/linux/firewire.h @@ -391,6 +391,7 @@ struct fw_iso_buffer { enum dma_data_direction direction; struct page **pages; int page_count; + int page_count_mapped; }; int fw_iso_buffer_init(struct fw_iso_buffer *buffer, struct fw_card *card, -- cgit v0.10.2 From d713dfa708e14352cfb71342cf985d08fe14be95 Mon Sep 17 00:00:00 2001 From: Stefan Richter Date: Mon, 9 Apr 2012 21:39:53 +0200 Subject: firewire: ohci: correct signedness of a local variable bus_reset_work's reg is a bitfield. Signed-off-by: Stefan Richter diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c index 67c8d27..b66112d 100644 --- a/drivers/firewire/ohci.c +++ b/drivers/firewire/ohci.c @@ -1821,8 +1821,8 @@ static void bus_reset_work(struct work_struct *work) { struct fw_ohci *ohci = container_of(work, struct fw_ohci, bus_reset_work); - int self_id_count, i, j, reg; - int generation, new_generation; + int self_id_count, generation, new_generation, i, j; + u32 reg; unsigned long flags; void *free_rom = NULL; dma_addr_t free_rom_bus = 0; -- cgit v0.10.2 From 8a8c47364eef8595e05b5bf53352aa6f16784356 Mon Sep 17 00:00:00 2001 From: Stefan Richter Date: Mon, 9 Apr 2012 21:40:33 +0200 Subject: firewire: ohci: omit spinlock IRQ flags where possible bus_reset_work() is only called from workqueue thread context. ohci_set_config_rom() and ohci_allocate_iso_context() perform GFP_KERNEL memory allocations, therefore they must be called with interrupts enabled. Hence these functions may disable and enable local IRQs without having to track IRQ state. Signed-off-by: Stefan Richter diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c index b66112d..c1af05e 100644 --- a/drivers/firewire/ohci.c +++ b/drivers/firewire/ohci.c @@ -1823,7 +1823,6 @@ static void bus_reset_work(struct work_struct *work) container_of(work, struct fw_ohci, bus_reset_work); int self_id_count, generation, new_generation, i, j; u32 reg; - unsigned long flags; void *free_rom = NULL; dma_addr_t free_rom_bus = 0; bool is_new_root; @@ -1930,13 +1929,13 @@ static void bus_reset_work(struct work_struct *work) } /* FIXME: Document how the locking works. */ - spin_lock_irqsave(&ohci->lock, flags); + spin_lock_irq(&ohci->lock); ohci->generation = -1; /* prevent AT packet queueing */ context_stop(&ohci->at_request_ctx); context_stop(&ohci->at_response_ctx); - spin_unlock_irqrestore(&ohci->lock, flags); + spin_unlock_irq(&ohci->lock); /* * Per OHCI 1.2 draft, clause 7.2.3.3, hardware may leave unsent @@ -1946,7 +1945,7 @@ static void bus_reset_work(struct work_struct *work) at_context_flush(&ohci->at_request_ctx); at_context_flush(&ohci->at_response_ctx); - spin_lock_irqsave(&ohci->lock, flags); + spin_lock_irq(&ohci->lock); ohci->generation = generation; reg_write(ohci, OHCI1394_IntEventClear, OHCI1394_busReset); @@ -1990,7 +1989,7 @@ static void bus_reset_work(struct work_struct *work) reg_write(ohci, OHCI1394_PhyReqFilterLoSet, ~0); #endif - spin_unlock_irqrestore(&ohci->lock, flags); + spin_unlock_irq(&ohci->lock); if (free_rom) dma_free_coherent(ohci->card.device, CONFIG_ROM_SIZE, @@ -2402,7 +2401,6 @@ static int ohci_set_config_rom(struct fw_card *card, const __be32 *config_rom, size_t length) { struct fw_ohci *ohci; - unsigned long flags; __be32 *next_config_rom; dma_addr_t uninitialized_var(next_config_rom_bus); @@ -2441,7 +2439,7 @@ static int ohci_set_config_rom(struct fw_card *card, if (next_config_rom == NULL) return -ENOMEM; - spin_lock_irqsave(&ohci->lock, flags); + spin_lock_irq(&ohci->lock); /* * If there is not an already pending config_rom update, @@ -2467,7 +2465,7 @@ static int ohci_set_config_rom(struct fw_card *card, reg_write(ohci, OHCI1394_ConfigROMmap, ohci->next_config_rom_bus); - spin_unlock_irqrestore(&ohci->lock, flags); + spin_unlock_irq(&ohci->lock); /* If we didn't use the DMA allocation, delete it. */ if (next_config_rom != NULL) @@ -2891,10 +2889,9 @@ static struct fw_iso_context *ohci_allocate_iso_context(struct fw_card *card, descriptor_callback_t uninitialized_var(callback); u64 *uninitialized_var(channels); u32 *uninitialized_var(mask), uninitialized_var(regs); - unsigned long flags; int index, ret = -EBUSY; - spin_lock_irqsave(&ohci->lock, flags); + spin_lock_irq(&ohci->lock); switch (type) { case FW_ISO_CONTEXT_TRANSMIT: @@ -2938,7 +2935,7 @@ static struct fw_iso_context *ohci_allocate_iso_context(struct fw_card *card, ret = -ENOSYS; } - spin_unlock_irqrestore(&ohci->lock, flags); + spin_unlock_irq(&ohci->lock); if (index < 0) return ERR_PTR(ret); @@ -2964,7 +2961,7 @@ static struct fw_iso_context *ohci_allocate_iso_context(struct fw_card *card, out_with_header: free_page((unsigned long)ctx->header); out: - spin_lock_irqsave(&ohci->lock, flags); + spin_lock_irq(&ohci->lock); switch (type) { case FW_ISO_CONTEXT_RECEIVE: @@ -2977,7 +2974,7 @@ static struct fw_iso_context *ohci_allocate_iso_context(struct fw_card *card, } *mask |= 1 << index; - spin_unlock_irqrestore(&ohci->lock, flags); + spin_unlock_irq(&ohci->lock); return ERR_PTR(ret); } -- cgit v0.10.2 From d33ec3b55e91f1c285099fee731b79a722d10fe6 Mon Sep 17 00:00:00 2001 From: Clemens Ladisch Date: Wed, 11 Apr 2012 17:36:39 +0200 Subject: firewire: core: wait for inaccessible devices after bus reset When reread_config_rom() encountered a config rom that was marked as not yet accessible, that device would be treated as "gone". This would mean that that device would effectively vanish until the next bus reset. The correct way to handle this situation is the same as in read_config_rom(), to treat this like other errors and to retry the read later, when the (possibly changed) config rom is available. The device is marked "gone" only if it continues to return zero values after these retries. Signed-off-by: Clemens Ladisch Signed-off-by: Stefan Richter diff --git a/drivers/firewire/core-device.c b/drivers/firewire/core-device.c index 68109e9..8038311 100644 --- a/drivers/firewire/core-device.c +++ b/drivers/firewire/core-device.c @@ -505,7 +505,7 @@ static int read_config_rom(struct fw_device *device, int generation) if (read_rom(device, generation, i, &rom[i]) != RCODE_COMPLETE) goto out; /* - * As per IEEE1212 7.2, during power-up, devices can + * As per IEEE1212 7.2, during initialization, devices can * reply with a 0 for the first quadlet of the config * rom to indicate that they are booting (for example, * if the firmware is on the disk of a external @@ -1071,7 +1071,6 @@ static void fw_device_init(struct work_struct *work) enum { REREAD_BIB_ERROR, - REREAD_BIB_GONE, REREAD_BIB_UNCHANGED, REREAD_BIB_CHANGED, }; @@ -1087,7 +1086,8 @@ static int reread_config_rom(struct fw_device *device, int generation) return REREAD_BIB_ERROR; if (i == 0 && q == 0) - return REREAD_BIB_GONE; + /* inaccessible (see read_config_rom); retry later */ + return REREAD_BIB_ERROR; if (q != device->config_rom[i]) return REREAD_BIB_CHANGED; @@ -1114,9 +1114,6 @@ static void fw_device_refresh(struct work_struct *work) } goto give_up; - case REREAD_BIB_GONE: - goto gone; - case REREAD_BIB_UNCHANGED: if (atomic_cmpxchg(&device->state, FW_DEVICE_INITIALIZING, -- cgit v0.10.2 From db7494e2ce616f2e39e877cf9143b7d873701ec6 Mon Sep 17 00:00:00 2001 From: Clemens Ladisch Date: Wed, 11 Apr 2012 17:37:36 +0200 Subject: firewire: core: improve reread_config_rom() interface The return value of reread_config_rom() was a mixture of two pieces of information: whether the function succeeded, and whether the config rom had changed. To clarify the semantics, and to allow returning the actual error code, split the second information into a new output parameter. Signed-off-by: Clemens Ladisch Signed-off-by: Stefan Richter diff --git a/drivers/firewire/core-device.c b/drivers/firewire/core-device.c index 8038311..f9f782a 100644 --- a/drivers/firewire/core-device.c +++ b/drivers/firewire/core-device.c @@ -1069,31 +1069,30 @@ static void fw_device_init(struct work_struct *work) put_device(&device->device); /* our reference */ } -enum { - REREAD_BIB_ERROR, - REREAD_BIB_UNCHANGED, - REREAD_BIB_CHANGED, -}; - /* Reread and compare bus info block and header of root directory */ -static int reread_config_rom(struct fw_device *device, int generation) +static int reread_config_rom(struct fw_device *device, int generation, + bool *changed) { u32 q; - int i; + int i, rcode; for (i = 0; i < 6; i++) { - if (read_rom(device, generation, i, &q) != RCODE_COMPLETE) - return REREAD_BIB_ERROR; + rcode = read_rom(device, generation, i, &q); + if (rcode != RCODE_COMPLETE) + return rcode; if (i == 0 && q == 0) /* inaccessible (see read_config_rom); retry later */ - return REREAD_BIB_ERROR; + return RCODE_BUSY; - if (q != device->config_rom[i]) - return REREAD_BIB_CHANGED; + if (q != device->config_rom[i]) { + *changed = true; + return RCODE_COMPLETE; + } } - return REREAD_BIB_UNCHANGED; + *changed = false; + return RCODE_COMPLETE; } static void fw_device_refresh(struct work_struct *work) @@ -1101,10 +1100,11 @@ static void fw_device_refresh(struct work_struct *work) struct fw_device *device = container_of(work, struct fw_device, work.work); struct fw_card *card = device->card; - int node_id = device->node_id; + int ret, node_id = device->node_id; + bool changed; - switch (reread_config_rom(device, device->generation)) { - case REREAD_BIB_ERROR: + ret = reread_config_rom(device, device->generation, &changed); + if (ret != RCODE_COMPLETE) { if (device->config_rom_retries < MAX_RETRIES / 2 && atomic_read(&device->state) == FW_DEVICE_INITIALIZING) { device->config_rom_retries++; @@ -1113,8 +1113,9 @@ static void fw_device_refresh(struct work_struct *work) return; } goto give_up; + } - case REREAD_BIB_UNCHANGED: + if (!changed) { if (atomic_cmpxchg(&device->state, FW_DEVICE_INITIALIZING, FW_DEVICE_RUNNING) == FW_DEVICE_GONE) @@ -1123,9 +1124,6 @@ static void fw_device_refresh(struct work_struct *work) fw_device_update(work); device->config_rom_retries = 0; goto out; - - case REREAD_BIB_CHANGED: - break; } /* -- cgit v0.10.2 From 7bdbff6762a573b911e4ee5715779d8ee6a62631 Mon Sep 17 00:00:00 2001 From: Clemens Ladisch Date: Wed, 11 Apr 2012 17:38:10 +0200 Subject: firewire: move rcode_string() to core There is nothing audio-specific about the rcode_string() helper, so move it from snd-firewire-lib into firewire-core to allow other code to use it. Signed-off-by: Clemens Ladisch Signed-off-by: Stefan Richter (fixed sound/firewire/cmp.c) diff --git a/drivers/firewire/core-transaction.c b/drivers/firewire/core-transaction.c index dea2dcc..1c4980c 100644 --- a/drivers/firewire/core-transaction.c +++ b/drivers/firewire/core-transaction.c @@ -994,6 +994,32 @@ void fw_core_handle_response(struct fw_card *card, struct fw_packet *p) } EXPORT_SYMBOL(fw_core_handle_response); +/** + * fw_rcode_string - convert a firewire result code to an error description + * @rcode: the result code + */ +const char *fw_rcode_string(int rcode) +{ + static const char *const names[] = { + [RCODE_COMPLETE] = "no error", + [RCODE_CONFLICT_ERROR] = "conflict error", + [RCODE_DATA_ERROR] = "data error", + [RCODE_TYPE_ERROR] = "type error", + [RCODE_ADDRESS_ERROR] = "address error", + [RCODE_SEND_ERROR] = "send error", + [RCODE_CANCELLED] = "timeout", + [RCODE_BUSY] = "busy", + [RCODE_GENERATION] = "bus reset", + [RCODE_NO_ACK] = "no ack", + }; + + if ((unsigned int)rcode < ARRAY_SIZE(names) && names[rcode]) + return names[rcode]; + else + return "unknown"; +} +EXPORT_SYMBOL(fw_rcode_string); + static const struct fw_address_region topology_map_region = { .start = CSR_REGISTER_BASE | CSR_TOPOLOGY_MAP, .end = CSR_REGISTER_BASE | CSR_TOPOLOGY_MAP_END, }; diff --git a/include/linux/firewire.h b/include/linux/firewire.h index 0a19057..584826b 100644 --- a/include/linux/firewire.h +++ b/include/linux/firewire.h @@ -334,6 +334,7 @@ int fw_cancel_transaction(struct fw_card *card, int fw_run_transaction(struct fw_card *card, int tcode, int destination_id, int generation, int speed, unsigned long long offset, void *payload, size_t length); +const char *fw_rcode_string(int rcode); static inline int fw_stream_packet_destination_id(int tag, int channel, int sy) { diff --git a/sound/firewire/cmp.c b/sound/firewire/cmp.c index 76294f2..645cb0b 100644 --- a/sound/firewire/cmp.c +++ b/sound/firewire/cmp.c @@ -84,7 +84,7 @@ static int pcr_modify(struct cmp_connection *c, return 0; io_error: - cmp_error(c, "transaction failed: %s\n", rcode_string(rcode)); + cmp_error(c, "transaction failed: %s\n", fw_rcode_string(rcode)); return -EIO; bus_reset: diff --git a/sound/firewire/lib.c b/sound/firewire/lib.c index 4750cea..14eb414 100644 --- a/sound/firewire/lib.c +++ b/sound/firewire/lib.c @@ -14,32 +14,6 @@ #define ERROR_RETRY_DELAY_MS 5 /** - * rcode_string - convert a firewire result code to a string - * @rcode: the result - */ -const char *rcode_string(unsigned int rcode) -{ - static const char *const names[] = { - [RCODE_COMPLETE] = "complete", - [RCODE_CONFLICT_ERROR] = "conflict error", - [RCODE_DATA_ERROR] = "data error", - [RCODE_TYPE_ERROR] = "type error", - [RCODE_ADDRESS_ERROR] = "address error", - [RCODE_SEND_ERROR] = "send error", - [RCODE_CANCELLED] = "cancelled", - [RCODE_BUSY] = "busy", - [RCODE_GENERATION] = "generation", - [RCODE_NO_ACK] = "no ack", - }; - - if (rcode < ARRAY_SIZE(names) && names[rcode]) - return names[rcode]; - else - return "unknown"; -} -EXPORT_SYMBOL(rcode_string); - -/** * snd_fw_transaction - send a request and wait for its completion * @unit: the driver's unit on the target device * @tcode: the transaction code @@ -71,7 +45,7 @@ int snd_fw_transaction(struct fw_unit *unit, int tcode, if (rcode_is_permanent_error(rcode) || ++tries >= 3) { dev_err(&unit->device, "transaction failed: %s\n", - rcode_string(rcode)); + fw_rcode_string(rcode)); return -EIO; } diff --git a/sound/firewire/lib.h b/sound/firewire/lib.h index 064f3fd..aef3014 100644 --- a/sound/firewire/lib.h +++ b/sound/firewire/lib.h @@ -8,7 +8,6 @@ struct fw_unit; int snd_fw_transaction(struct fw_unit *unit, int tcode, u64 offset, void *buffer, size_t length); -const char *rcode_string(unsigned int rcode); /* returns true if retrying the transaction would not make sense */ static inline bool rcode_is_permanent_error(int rcode) -- cgit v0.10.2 From 3b00b008888a851499bc039e70d12002af00ec9c Mon Sep 17 00:00:00 2001 From: Clemens Ladisch Date: Wed, 11 Apr 2012 17:38:47 +0200 Subject: firewire: core: log error in case of failed bus manager lock If the lock access to the bus manager register fails, also log the actual error that caused it to fail. Signed-off-by: Clemens Ladisch Signed-off-by: Stefan Richter diff --git a/drivers/firewire/core-card.c b/drivers/firewire/core-card.c index cc595eb..2a03bb9 100644 --- a/drivers/firewire/core-card.c +++ b/drivers/firewire/core-card.c @@ -421,8 +421,8 @@ static void bm_work(struct work_struct *work) * root, and thus, IRM. */ new_root_id = local_id; - fw_notice(card, "%s, making local node (%02x) root\n", - "BM lock failed", new_root_id); + fw_notice(card, "BM lock failed (%s), making local node (%02x) root\n", + fw_rcode_string(rcode), new_root_id); goto pick_me; } } else if (card->bm_generation != generation) { -- cgit v0.10.2 From 94fba9fbeac44462c498e848496ba088198d78d1 Mon Sep 17 00:00:00 2001 From: Clemens Ladisch Date: Wed, 11 Apr 2012 17:39:19 +0200 Subject: firewire: core: log config rom reading errors If reading or refreshing a config rom fails, also log the actual error that caused it to fail. Signed-off-by: Clemens Ladisch Signed-off-by: Stefan Richter diff --git a/drivers/firewire/core-device.c b/drivers/firewire/core-device.c index f9f782a..a3f486f 100644 --- a/drivers/firewire/core-device.c +++ b/drivers/firewire/core-device.c @@ -481,6 +481,7 @@ static int read_rom(struct fw_device *device, * generation changes under us, read_config_rom will fail and get retried. * It's better to start all over in this case because the node from which we * are reading the ROM may have changed the ROM during the reset. + * Returns either a result code or a negative error code. */ static int read_config_rom(struct fw_device *device, int generation) { @@ -488,7 +489,7 @@ static int read_config_rom(struct fw_device *device, int generation) const u32 *old_rom, *new_rom; u32 *rom, *stack; u32 sp, key; - int i, end, length, ret = -1; + int i, end, length, ret; rom = kmalloc(sizeof(*rom) * MAX_CONFIG_ROM_SIZE + sizeof(*stack) * MAX_CONFIG_ROM_SIZE, GFP_KERNEL); @@ -502,7 +503,8 @@ static int read_config_rom(struct fw_device *device, int generation) /* First read the bus info block. */ for (i = 0; i < 5; i++) { - if (read_rom(device, generation, i, &rom[i]) != RCODE_COMPLETE) + ret = read_rom(device, generation, i, &rom[i]); + if (ret != RCODE_COMPLETE) goto out; /* * As per IEEE1212 7.2, during initialization, devices can @@ -512,8 +514,10 @@ static int read_config_rom(struct fw_device *device, int generation) * harddisk). In that case we just fail, and the * retry mechanism will try again later. */ - if (i == 0 && rom[i] == 0) + if (i == 0 && rom[i] == 0) { + ret = RCODE_BUSY; goto out; + } } device->max_speed = device->node->max_speed; @@ -563,11 +567,14 @@ static int read_config_rom(struct fw_device *device, int generation) */ key = stack[--sp]; i = key & 0xffffff; - if (WARN_ON(i >= MAX_CONFIG_ROM_SIZE)) + if (WARN_ON(i >= MAX_CONFIG_ROM_SIZE)) { + ret = -ENXIO; goto out; + } /* Read header quadlet for the block to get the length. */ - if (read_rom(device, generation, i, &rom[i]) != RCODE_COMPLETE) + ret = read_rom(device, generation, i, &rom[i]); + if (ret != RCODE_COMPLETE) goto out; end = i + (rom[i] >> 16) + 1; if (end > MAX_CONFIG_ROM_SIZE) { @@ -590,8 +597,8 @@ static int read_config_rom(struct fw_device *device, int generation) * it references another block, and push it in that case. */ for (; i < end; i++) { - if (read_rom(device, generation, i, &rom[i]) != - RCODE_COMPLETE) + ret = read_rom(device, generation, i, &rom[i]); + if (ret != RCODE_COMPLETE) goto out; if ((key >> 30) != 3 || (rom[i] >> 30) < 2) @@ -619,8 +626,10 @@ static int read_config_rom(struct fw_device *device, int generation) old_rom = device->config_rom; new_rom = kmemdup(rom, length * 4, GFP_KERNEL); - if (new_rom == NULL) + if (new_rom == NULL) { + ret = -ENOMEM; goto out; + } down_write(&fw_device_rwsem); device->config_rom = new_rom; @@ -628,7 +637,7 @@ static int read_config_rom(struct fw_device *device, int generation) up_write(&fw_device_rwsem); kfree(old_rom); - ret = 0; + ret = RCODE_COMPLETE; device->max_rec = rom[2] >> 12 & 0xf; device->cmc = rom[2] >> 30 & 1; device->irmc = rom[2] >> 31 & 1; @@ -967,15 +976,17 @@ static void fw_device_init(struct work_struct *work) * device. */ - if (read_config_rom(device, device->generation) < 0) { + ret = read_config_rom(device, device->generation); + if (ret != RCODE_COMPLETE) { if (device->config_rom_retries < MAX_RETRIES && atomic_read(&device->state) == FW_DEVICE_INITIALIZING) { device->config_rom_retries++; fw_schedule_device_work(device, RETRY_DELAY); } else { if (device->node->link_on) - fw_notice(card, "giving up on Config ROM for node id %x\n", - device->node_id); + fw_notice(card, "giving up on node %x: reading config rom failed: %s\n", + device->node_id, + fw_rcode_string(ret)); if (device->node == card->root_node) fw_schedule_bm_work(card, 0); fw_device_release(&device->device); @@ -1132,7 +1143,8 @@ static void fw_device_refresh(struct work_struct *work) */ device_for_each_child(&device->device, NULL, shutdown_unit); - if (read_config_rom(device, device->generation) < 0) { + ret = read_config_rom(device, device->generation); + if (ret != RCODE_COMPLETE) { if (device->config_rom_retries < MAX_RETRIES && atomic_read(&device->state) == FW_DEVICE_INITIALIZING) { device->config_rom_retries++; @@ -1159,8 +1171,8 @@ static void fw_device_refresh(struct work_struct *work) goto out; give_up: - fw_notice(card, "giving up on refresh of device %s\n", - dev_name(&device->device)); + fw_notice(card, "giving up on refresh of device %s: %s\n", + dev_name(&device->device), fw_rcode_string(ret)); gone: atomic_set(&device->state, FW_DEVICE_GONE); PREPARE_DELAYED_WORK(&device->work, fw_device_shutdown); -- cgit v0.10.2 From 8527f8e2934683e53405fbe876a4e6f4a0c46eb8 Mon Sep 17 00:00:00 2001 From: Clemens Ladisch Date: Wed, 11 Apr 2012 17:39:59 +0200 Subject: firewire: core: fw_device_refresh(): clean up error handling In fw_device_init() and fw_device_refresh(), if a call to read_cofig_rom() fails, the operation is retried a few times, with these retries being controlled by the MAX_RETRIES and RETRY_DELAY symbols. fw_device_refresh() also reads part of the config rom by calling reread_config_rom(). Any errors from this call resulted in retries with MAX_RETRIES/2 and RETRY_DELAY/2. There is no reason to require that a device that has initiated a bus reset must react faster to read requests than a device that has just been plugged in. Furthermore, if the config rom has changed, any errors from the following read_config_rom() call are then handled with the normal retry count and delay. Remove this inconsistency by always using the normal retry count and delay. (This also makes the two error handlers identical and allows merging them.) Signed-off-by: Clemens Ladisch Signed-off-by: Stefan Richter diff --git a/drivers/firewire/core-device.c b/drivers/firewire/core-device.c index a3f486f..4d460ef 100644 --- a/drivers/firewire/core-device.c +++ b/drivers/firewire/core-device.c @@ -1115,16 +1115,8 @@ static void fw_device_refresh(struct work_struct *work) bool changed; ret = reread_config_rom(device, device->generation, &changed); - if (ret != RCODE_COMPLETE) { - if (device->config_rom_retries < MAX_RETRIES / 2 && - atomic_read(&device->state) == FW_DEVICE_INITIALIZING) { - device->config_rom_retries++; - fw_schedule_device_work(device, RETRY_DELAY / 2); - - return; - } - goto give_up; - } + if (ret != RCODE_COMPLETE) + goto failed_config_rom; if (!changed) { if (atomic_cmpxchg(&device->state, @@ -1144,16 +1136,8 @@ static void fw_device_refresh(struct work_struct *work) device_for_each_child(&device->device, NULL, shutdown_unit); ret = read_config_rom(device, device->generation); - if (ret != RCODE_COMPLETE) { - if (device->config_rom_retries < MAX_RETRIES && - atomic_read(&device->state) == FW_DEVICE_INITIALIZING) { - device->config_rom_retries++; - fw_schedule_device_work(device, RETRY_DELAY); - - return; - } - goto give_up; - } + if (ret != RCODE_COMPLETE) + goto failed_config_rom; fw_device_cdev_update(device); create_units(device); @@ -1170,7 +1154,14 @@ static void fw_device_refresh(struct work_struct *work) device->config_rom_retries = 0; goto out; - give_up: + failed_config_rom: + if (device->config_rom_retries < MAX_RETRIES && + atomic_read(&device->state) == FW_DEVICE_INITIALIZING) { + device->config_rom_retries++; + fw_schedule_device_work(device, RETRY_DELAY); + return; + } + fw_notice(card, "giving up on refresh of device %s: %s\n", dev_name(&device->device), fw_rcode_string(ret)); gone: -- cgit v0.10.2 From 473ffe6560fd5fa5fd5a488e8948899231972bd5 Mon Sep 17 00:00:00 2001 From: Clemens Ladisch Date: Fri, 18 May 2012 18:39:39 +0200 Subject: firewire: sbp2: give correct DMA device to scsi framework The sbp2 driver does DMA not on the unit but on the card device. The driver worked even with the wrong device because at the moment, it happens to reimplement the DMA functions of the SCSI framework. Signed-off-by: Clemens Ladisch Signed-off-by: Stefan Richter diff --git a/drivers/firewire/sbp2.c b/drivers/firewire/sbp2.c index b7e65d7..2e202d3 100644 --- a/drivers/firewire/sbp2.c +++ b/drivers/firewire/sbp2.c @@ -1163,7 +1163,8 @@ static int sbp2_probe(struct device *dev) shost->max_cmd_len = SBP2_MAX_CDB_SIZE; - if (scsi_add_host(shost, &unit->device) < 0) + if (scsi_add_host_with_dma(shost, &unit->device, + device->card->device) < 0) goto fail_shost_put; /* implicit directory ID */ -- cgit v0.10.2 From f203022353eb3e0b059a72a43762e240e9682c91 Mon Sep 17 00:00:00 2001 From: Clemens Ladisch Date: Fri, 18 May 2012 18:40:19 +0200 Subject: firewire: sbp2: use scsi_dma_(un)map Use the scsi_dma_map/scsi_dma_unmap helper to simplify the code a little. Signed-off-by: Clemens Ladisch Signed-off-by: Stefan Richter diff --git a/drivers/firewire/sbp2.c b/drivers/firewire/sbp2.c index 2e202d3..6ffaef3 100644 --- a/drivers/firewire/sbp2.c +++ b/drivers/firewire/sbp2.c @@ -1296,10 +1296,7 @@ static struct fw_driver sbp2_driver = { static void sbp2_unmap_scatterlist(struct device *card_device, struct sbp2_command_orb *orb) { - if (scsi_sg_count(orb->cmd)) - dma_unmap_sg(card_device, scsi_sglist(orb->cmd), - scsi_sg_count(orb->cmd), - orb->cmd->sc_data_direction); + scsi_dma_unmap(orb->cmd); if (orb->request.misc & cpu_to_be32(COMMAND_ORB_PAGE_TABLE_PRESENT)) dma_unmap_single(card_device, orb->page_table_bus, @@ -1405,9 +1402,8 @@ static int sbp2_map_scatterlist(struct sbp2_command_orb *orb, struct scatterlist *sg = scsi_sglist(orb->cmd); int i, n; - n = dma_map_sg(device->card->device, sg, scsi_sg_count(orb->cmd), - orb->cmd->sc_data_direction); - if (n == 0) + n = scsi_dma_map(orb->cmd); + if (n <= 0) goto fail; /* @@ -1453,8 +1449,7 @@ static int sbp2_map_scatterlist(struct sbp2_command_orb *orb, return 0; fail_page_table: - dma_unmap_sg(device->card->device, scsi_sglist(orb->cmd), - scsi_sg_count(orb->cmd), orb->cmd->sc_data_direction); + scsi_dma_unmap(orb->cmd); fail: return -ENOMEM; } -- cgit v0.10.2 From 935f672e02c8172a23bd3e54feafffcfcce39f0d Mon Sep 17 00:00:00 2001 From: Clemens Ladisch Date: Fri, 18 May 2012 18:41:28 +0200 Subject: firewire: sbp2: remove superfluous blk_queue_max_segment_size() call The SCSI framework automatically initializes the block queue's segment size with the DMA device's segment size. Signed-off-by: Clemens Ladisch Signed-off-by: Stefan Richter diff --git a/drivers/firewire/sbp2.c b/drivers/firewire/sbp2.c index 6ffaef3..1f888f3 100644 --- a/drivers/firewire/sbp2.c +++ b/drivers/firewire/sbp2.c @@ -1564,8 +1564,6 @@ static int sbp2_scsi_slave_configure(struct scsi_device *sdev) if (lu->tgt->workarounds & SBP2_WORKAROUND_128K_MAX_TRANS) blk_queue_max_hw_sectors(sdev->request_queue, 128 * 1024 / 512); - blk_queue_max_segment_size(sdev->request_queue, SBP2_MAX_SEG_SIZE); - return 0; } -- cgit v0.10.2 From 26c72e22c94fbc28604c94e3a96fdae9c6fd0a42 Mon Sep 17 00:00:00 2001 From: Clemens Ladisch Date: Fri, 18 May 2012 22:26:21 +0200 Subject: firewire: sbp2: document the absence of alignment requirements The SBP-2/3 specifications do not require any alignment of data buffers; only their own data structures need to be quadlet-aligned [SR: or octlet-aligned]. Fix the comments to reflect this, but leave the actual alignment at 32 bits to avoid theoretical problems with target implementations that might handle this incorrectly. Signed-off-by: Clemens Ladisch Signed-off-by: Stefan Richter diff --git a/drivers/firewire/sbp2.c b/drivers/firewire/sbp2.c index 1f888f3..1162d6b 100644 --- a/drivers/firewire/sbp2.c +++ b/drivers/firewire/sbp2.c @@ -207,9 +207,8 @@ static const struct device *lu_dev(const struct sbp2_logical_unit *lu) #define SBP2_MAX_CDB_SIZE 16 /* - * The default maximum s/g segment size of a FireWire controller is - * usually 0x10000, but SBP-2 only allows 0xffff. Since buffers have to - * be quadlet-aligned, we set the length limit to 0xffff & ~3. + * The maximum SBP-2 data buffer size is 0xffff. We quadlet-align this + * for compatibility with earlier versions of this driver. */ #define SBP2_MAX_SEG_SIZE 0xfffc @@ -1530,7 +1529,10 @@ static int sbp2_scsi_slave_alloc(struct scsi_device *sdev) sdev->allow_restart = 1; - /* SBP-2 requires quadlet alignment of the data buffers. */ + /* + * SBP-2 does not require any alignment, but we set it anyway + * for compatibility with earlier versions of this driver. + */ blk_queue_update_dma_alignment(sdev->request_queue, 4 - 1); if (lu->tgt->workarounds & SBP2_WORKAROUND_INQUIRY_36) -- cgit v0.10.2