From df7ce66363bf66b2e8ef6245f5f42c2f6fb0db52 Mon Sep 17 00:00:00 2001 From: Paul Bolle Date: Sat, 16 Mar 2013 14:09:07 +0100 Subject: firewire: Remove two unneeded checks for macros The old IEEE 1394 driver stack was removed in v2.6.37. That made the checks for two Kconfig (module) macros unneeded, since they will now always evaluate to true. Remove these two checks. Signed-off-by: Paul Bolle Signed-off-by: Stefan Richter --- drivers/firewire/ohci.c | 2 -- drivers/firewire/sbp2.c | 2 -- 2 files changed, 4 deletions(-) (limited to 'drivers/firewire') diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c index 45912e6e0ac..4a55b519b77 100644 --- a/drivers/firewire/ohci.c +++ b/drivers/firewire/ohci.c @@ -3837,6 +3837,4 @@ MODULE_DESCRIPTION("Driver for PCI OHCI IEEE1394 controllers"); MODULE_LICENSE("GPL"); /* Provide a module alias so root-on-sbp2 initrds don't break. */ -#ifndef CONFIG_IEEE1394_OHCI1394_MODULE MODULE_ALIAS("ohci1394"); -#endif diff --git a/drivers/firewire/sbp2.c b/drivers/firewire/sbp2.c index 1162d6b3bf8..12ec0e6fd09 100644 --- a/drivers/firewire/sbp2.c +++ b/drivers/firewire/sbp2.c @@ -1636,9 +1636,7 @@ MODULE_LICENSE("GPL"); MODULE_DEVICE_TABLE(ieee1394, sbp2_id_table); /* Provide a module alias so root-on-sbp2 initrds don't break. */ -#ifndef CONFIG_IEEE1394_SBP2_MODULE MODULE_ALIAS("sbp2"); -#endif static int __init sbp2_init(void) { -- cgit v1.2.3-18-g5258 From bdabfa54635e5d95a5aa2087794f203cc1915f7a Mon Sep 17 00:00:00 2001 From: Stefan Richter Date: Sun, 24 Mar 2013 17:31:09 +0100 Subject: firewire: core: remove an always false test struct fw_cdev_allocate_iso_resource.bandwidth is unsigned. Signed-off-by: Stefan Richter --- drivers/firewire/core-cdev.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'drivers/firewire') diff --git a/drivers/firewire/core-cdev.c b/drivers/firewire/core-cdev.c index 27ac423ab25..13573e8f518 100644 --- a/drivers/firewire/core-cdev.c +++ b/drivers/firewire/core-cdev.c @@ -1366,8 +1366,7 @@ static int init_iso_resource(struct client *client, int ret; if ((request->channels == 0 && request->bandwidth == 0) || - request->bandwidth > BANDWIDTH_AVAILABLE_INITIAL || - request->bandwidth < 0) + request->bandwidth > BANDWIDTH_AVAILABLE_INITIAL) return -EINVAL; r = kmalloc(sizeof(*r), GFP_KERNEL); -- cgit v1.2.3-18-g5258 From d6c8cefc69a0b1e75b369613f902141f2c621914 Mon Sep 17 00:00:00 2001 From: Stefan Richter Date: Sun, 24 Mar 2013 17:31:38 +0100 Subject: firewire: sbp2: replace BUG_ON by WARN_ON No need to crash and burn if S/G element sizes cannot be set to our liking; just leave a message in the log. Signed-off-by: Stefan Richter --- drivers/firewire/sbp2.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers/firewire') diff --git a/drivers/firewire/sbp2.c b/drivers/firewire/sbp2.c index 12ec0e6fd09..cbf969abe31 100644 --- a/drivers/firewire/sbp2.c +++ b/drivers/firewire/sbp2.c @@ -1144,8 +1144,8 @@ static int sbp2_probe(struct device *dev) return -ENODEV; if (dma_get_max_seg_size(device->card->device) > SBP2_MAX_SEG_SIZE) - BUG_ON(dma_set_max_seg_size(device->card->device, - SBP2_MAX_SEG_SIZE)); + WARN_ON(dma_set_max_seg_size(device->card->device, + SBP2_MAX_SEG_SIZE)); shost = scsi_host_alloc(&scsi_driver_template, sizeof(*tgt)); if (shost == NULL) -- cgit v1.2.3-18-g5258 From cfb0c9d1ffbf930a4a852f178b161c522b21b0ab Mon Sep 17 00:00:00 2001 From: Stefan Richter Date: Sun, 24 Mar 2013 17:32:00 +0100 Subject: firewire: remove unnecessary alloc/OOM messages These are redundant to log messages from the mm core. Signed-off-by: Stefan Richter --- drivers/firewire/core-cdev.c | 24 +++++++++--------------- drivers/firewire/core-device.c | 4 +--- drivers/firewire/net.c | 7 +------ drivers/firewire/ohci.c | 3 --- drivers/firewire/sbp2.c | 4 +--- 5 files changed, 12 insertions(+), 30 deletions(-) (limited to 'drivers/firewire') diff --git a/drivers/firewire/core-cdev.c b/drivers/firewire/core-cdev.c index 13573e8f518..7ef316fdc4d 100644 --- a/drivers/firewire/core-cdev.c +++ b/drivers/firewire/core-cdev.c @@ -389,10 +389,8 @@ static void queue_bus_reset_event(struct client *client) struct bus_reset_event *e; e = kzalloc(sizeof(*e), GFP_KERNEL); - if (e == NULL) { - fw_notice(client->device->card, "out of memory when allocating event\n"); + if (e == NULL) return; - } fill_bus_reset_event(&e->reset, client); @@ -693,10 +691,9 @@ static void handle_request(struct fw_card *card, struct fw_request *request, r = kmalloc(sizeof(*r), GFP_ATOMIC); e = kmalloc(sizeof(*e), GFP_ATOMIC); - if (r == NULL || e == NULL) { - fw_notice(card, "out of memory when allocating event\n"); + if (r == NULL || e == NULL) goto failed; - } + r->card = card; r->request = request; r->data = payload; @@ -930,10 +927,9 @@ static void iso_callback(struct fw_iso_context *context, u32 cycle, struct iso_interrupt_event *e; e = kmalloc(sizeof(*e) + header_length, GFP_ATOMIC); - if (e == NULL) { - fw_notice(context->card, "out of memory when allocating event\n"); + if (e == NULL) return; - } + e->interrupt.type = FW_CDEV_EVENT_ISO_INTERRUPT; e->interrupt.closure = client->iso_closure; e->interrupt.cycle = cycle; @@ -950,10 +946,9 @@ static void iso_mc_callback(struct fw_iso_context *context, struct iso_interrupt_mc_event *e; e = kmalloc(sizeof(*e), GFP_ATOMIC); - if (e == NULL) { - fw_notice(context->card, "out of memory when allocating event\n"); + if (e == NULL) return; - } + e->interrupt.type = FW_CDEV_EVENT_ISO_INTERRUPT_MULTICHANNEL; e->interrupt.closure = client->iso_closure; e->interrupt.completed = fw_iso_buffer_lookup(&client->buffer, @@ -1581,10 +1576,9 @@ void fw_cdev_handle_phy_packet(struct fw_card *card, struct fw_packet *p) list_for_each_entry(client, &card->phy_receiver_list, phy_receiver_link) { e = kmalloc(sizeof(*e) + 8, GFP_ATOMIC); - if (e == NULL) { - fw_notice(card, "out of memory when allocating event\n"); + if (e == NULL) break; - } + e->phy_packet.closure = client->phy_receiver_closure; e->phy_packet.type = FW_CDEV_EVENT_PHY_PACKET_RECEIVED; e->phy_packet.rcode = RCODE_COMPLETE; diff --git a/drivers/firewire/core-device.c b/drivers/firewire/core-device.c index 03ce7d980c6..664a6ff0a82 100644 --- a/drivers/firewire/core-device.c +++ b/drivers/firewire/core-device.c @@ -692,10 +692,8 @@ static void create_units(struct fw_device *device) * match the drivers id_tables against it. */ unit = kzalloc(sizeof(*unit), GFP_KERNEL); - if (unit == NULL) { - fw_err(device->card, "out of memory for unit\n"); + if (unit == NULL) continue; - } unit->directory = ci.p + value - 1; unit->device.bus = &fw_bus_type; diff --git a/drivers/firewire/net.c b/drivers/firewire/net.c index 2b27bff2591..75c1133e437 100644 --- a/drivers/firewire/net.c +++ b/drivers/firewire/net.c @@ -368,10 +368,8 @@ static struct fwnet_fragment_info *fwnet_frag_new( } new = kmalloc(sizeof(*new), GFP_ATOMIC); - if (!new) { - dev_err(&pd->skb->dev->dev, "out of memory\n"); + if (!new) return NULL; - } new->offset = offset; new->len = len; @@ -414,8 +412,6 @@ fail_w_fi: fail_w_new: kfree(new); fail: - dev_err(&net->dev, "out of memory\n"); - return NULL; } @@ -692,7 +688,6 @@ static int fwnet_incoming_packet(struct fwnet_device *dev, __be32 *buf, int len, skb = dev_alloc_skb(len + LL_RESERVED_SPACE(net)); if (unlikely(!skb)) { - dev_err(&net->dev, "out of memory\n"); net->stats.rx_dropped++; return -ENOMEM; diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c index 4a55b519b77..d9be53c1d80 100644 --- a/drivers/firewire/ohci.c +++ b/drivers/firewire/ohci.c @@ -3711,9 +3711,6 @@ static int pci_probe(struct pci_dev *dev, kfree(ohci); pmac_ohci_off(dev); fail: - if (err == -ENOMEM) - dev_err(&dev->dev, "out of memory\n"); - return err; } diff --git a/drivers/firewire/sbp2.c b/drivers/firewire/sbp2.c index cbf969abe31..47674b91384 100644 --- a/drivers/firewire/sbp2.c +++ b/drivers/firewire/sbp2.c @@ -1475,10 +1475,8 @@ static int sbp2_scsi_queuecommand(struct Scsi_Host *shost, } orb = kzalloc(sizeof(*orb), GFP_ATOMIC); - if (orb == NULL) { - dev_notice(lu_dev(lu), "failed to alloc ORB\n"); + if (orb == NULL) return SCSI_MLQUEUE_HOST_BUSY; - } /* Initialize rcode to something not RCODE_COMPLETE. */ orb->base.rcode = -1; -- cgit v1.2.3-18-g5258 From 247fd50b5953d2b07832a07bd1d1c3b8e221fe9e Mon Sep 17 00:00:00 2001 From: Peter Hurley Date: Wed, 27 Mar 2013 06:59:58 -0400 Subject: firewire: ohci: Fix double free_irq() A pci device can be removed while in its suspended state. Because the ohci driver freed the irq to suspend, free_irq() is called twice; once from pci_remove() and again from pci_suspend(), which issues the warning below [1]. Rather than allocate the irq in the .enable() path, move the allocation to .probe(). Consequently, the irq is not reallocated upon pci_resume() and thus is not freed upon pci_suspend(). [1] Warning reported by Mark Einon when suspending an MSI MS-1727 GT740 laptop on Ubuntu 3.5.0-22-generic WARNING: at ./kernel/irq/manage.c:1198 __free_irq+0xa3/0x1e0() Hardware name: MS-1727 Trying to free already-free IRQ 16 Modules linked in: ip6table_filter ip6_tables ebtable_nat ebtables <...snip...> Pid: 4, comm: kworker/0:0 Tainted: P O 3.5.0-22-generic #34-Ubuntu Call Trace: [] warn_slowpath_common+0x7f/0xc0 [] warn_slowpath_fmt+0x46/0x50 [] ? default_spin_lock_flags+0x9/0x10 [] __free_irq+0xa3/0x1e0 [] free_irq+0x54/0xc0 [] pci_remove+0x6e/0x210 [firewire_ohci] [] pci_device_remove+0x3f/0x110 [] __device_release_driver+0x7c/0xe0 [] device_release_driver+0x2c/0x40 [] bus_remove_device+0xe1/0x120 [] device_del+0x12a/0x1c0 [] device_unregister+0x16/0x30 [] pci_stop_bus_device+0x94/0xa0 [] acpiphp_disable_slot+0xb7/0x1a0 [acpiphp] [] ? get_slot_status+0x46/0xc0 [acpiphp] [] acpiphp_check_bridge.isra.15+0x2d/0xf0 [acpiphp] [] _handle_hotplug_event_bridge+0x372/0x4d0 [acpiphp] [] ? acpi_os_execute_deferred+0x2f/0x34 [] ? kfree+0xed/0x110 [] process_one_work+0x12a/0x420 [] ? _handle_hotplug_event_func+0x1d0/0x1d0 [acpiphp] [] worker_thread+0x12e/0x2f0 [] ? manage_workers.isra.26+0x200/0x200 [] kthread+0x93/0xa0 [] kernel_thread_helper+0x4/0x10 [] ? kthread_freezable_should_stop+0x70/0x70 [] ? gs_change+0x13/0x13 Reported-by: Mark Einon Signed-off-by: Peter Hurley Signed-off-by: Stefan Richter --- drivers/firewire/ohci.c | 38 ++++++++++++++++---------------------- 1 file changed, 16 insertions(+), 22 deletions(-) (limited to 'drivers/firewire') diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c index d9be53c1d80..48889353723 100644 --- a/drivers/firewire/ohci.c +++ b/drivers/firewire/ohci.c @@ -2246,7 +2246,6 @@ static int ohci_enable(struct fw_card *card, const __be32 *config_rom, size_t length) { struct fw_ohci *ohci = fw_ohci(card); - struct pci_dev *dev = to_pci_dev(card->device); u32 lps, version, irqs; int i, ret; @@ -2382,24 +2381,6 @@ static int ohci_enable(struct fw_card *card, reg_write(ohci, OHCI1394_AsReqFilterHiSet, 0x80000000); - if (!(ohci->quirks & QUIRK_NO_MSI)) - pci_enable_msi(dev); - if (request_irq(dev->irq, irq_handler, - pci_dev_msi_enabled(dev) ? 0 : IRQF_SHARED, - ohci_driver_name, ohci)) { - dev_err(card->device, "failed to allocate interrupt %d\n", - dev->irq); - pci_disable_msi(dev); - - if (config_rom) { - dma_free_coherent(ohci->card.device, CONFIG_ROM_SIZE, - ohci->next_config_rom, - ohci->next_config_rom_bus); - ohci->next_config_rom = NULL; - } - return -EIO; - } - irqs = OHCI1394_reqTxComplete | OHCI1394_respTxComplete | OHCI1394_RQPkt | OHCI1394_RSPkt | OHCI1394_isochTx | OHCI1394_isochRx | @@ -3675,9 +3656,20 @@ static int pci_probe(struct pci_dev *dev, guid = ((u64) reg_read(ohci, OHCI1394_GUIDHi) << 32) | reg_read(ohci, OHCI1394_GUIDLo); + if (!(ohci->quirks & QUIRK_NO_MSI)) + pci_enable_msi(dev); + if (request_irq(dev->irq, irq_handler, + pci_dev_msi_enabled(dev) ? 0 : IRQF_SHARED, + ohci_driver_name, ohci)) { + dev_err(&dev->dev, "failed to allocate interrupt %d\n", + dev->irq); + err = -EIO; + goto fail_msi; + } + err = fw_card_add(&ohci->card, max_receive, link_speed, guid); if (err) - goto fail_contexts; + goto fail_irq; version = reg_read(ohci, OHCI1394_Version) & 0x00ff00ff; dev_notice(&dev->dev, @@ -3688,6 +3680,10 @@ static int pci_probe(struct pci_dev *dev, return 0; + fail_irq: + free_irq(dev->irq, ohci); + fail_msi: + pci_disable_msi(dev); fail_contexts: kfree(ohci->ir_context_list); kfree(ohci->it_context_list); @@ -3763,8 +3759,6 @@ static int pci_suspend(struct pci_dev *dev, pm_message_t state) int err; software_reset(ohci); - free_irq(dev->irq, ohci); - pci_disable_msi(dev); err = pci_save_state(dev); if (err) { dev_err(&dev->dev, "pci_save_state failed\n"); -- cgit v1.2.3-18-g5258 From 8db491490b88c8b016b41ad56ac980c4cbb06d7a Mon Sep 17 00:00:00 2001 From: Peter Hurley Date: Wed, 27 Mar 2013 06:59:59 -0400 Subject: firewire: ohci: Check LPS before register access on pci removal A pci device can be removed while in its suspended state. If the ohci host controller is suspended, the PHY is also in low-power mode and LPS is disabled. If LPS is disabled, most of the host registers aren't accessible, including IntMaskClear. Furthermore, access to these registers when LPS is disabled can cause hard lockups on some hardware. Since interrupts are already disabled in this mode, further action is unnecessary. Test LPS before attempting to write IntMaskClear to disable interrupts. [Stefan R: whitespace changes] Signed-off-by: Peter Hurley Signed-off-by: Stefan Richter --- drivers/firewire/ohci.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) (limited to 'drivers/firewire') diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c index 48889353723..673c8970749 100644 --- a/drivers/firewire/ohci.c +++ b/drivers/firewire/ohci.c @@ -3712,11 +3712,16 @@ static int pci_probe(struct pci_dev *dev, static void pci_remove(struct pci_dev *dev) { - struct fw_ohci *ohci; + struct fw_ohci *ohci = pci_get_drvdata(dev); - ohci = pci_get_drvdata(dev); - reg_write(ohci, OHCI1394_IntMaskClear, ~0); - flush_writes(ohci); + /* + * If the removal is happening from the suspend state, LPS won't be + * enabled and host registers (eg., IntMaskClear) won't be accessible. + */ + if (reg_read(ohci, OHCI1394_HCControlSet) & OHCI1394_HCControl_LPS) { + reg_write(ohci, OHCI1394_IntMaskClear, ~0); + flush_writes(ohci); + } cancel_work_sync(&ohci->bus_reset_work); fw_core_remove_card(&ohci->card); -- cgit v1.2.3-18-g5258 From be8dcab942e1c0ec2aa13eb2af2a79ab51b46293 Mon Sep 17 00:00:00 2001 From: Andy Leiserson Date: Wed, 24 Apr 2013 09:10:32 -0700 Subject: firewire: ohci: fix VIA VT6306 video reception Add quirk for VT6306 wake bit behavior. VT6306 seems to reread the wrong descriptor when the wake bit is written. work around by putting a copy of the branch address in the first descriptor of the block. [Stefan R: This fixes the known broken video reception via gstreamer on VIA VT6306. 100% repeatable testcase: $ gst-launch-0.10 dv1394src \! dvdemux \! dvdec \! xvimagesink with a camcorder or other DV source connected. Likewise for MPEG2-TS reception via gstreamer, e.g. from TV settop boxes. Perhaps this also fixes dv4l on VT6306, but this is as yet untested. Kino, dvgrab or FFADO had not been affected by this chip quirk. Additional comments from Andy:] I've looked into some problems with the wake bit on a vt6306 family chip (1106:3044, rev 46). I used this firewire card in a mythtv setup (ISO receive MPEG2 stream) with Debian 2.6.32 kernels for ~2 years without problems. Since upgrading to 3.2, I've been having problems with the input stream freezing -- input data stops until I restart mythtv (I expect closing and reopening the device would be sufficient). This happens infrequently, maybe one out of 20 recordings. I eventually determined that the problem is more likely to occur if the system is loaded. I isolated the kernel version as the triggering SW factor and then specifically the change from dualbuffer back to packet-per-buffer DMA mode. The possibility that the controller does not properly respond to the wake bit was suggested in https://bugzilla.redhat.com/show_bug.cgi?id=415841, but not proven. Based on the fact that dualbuffer mode worked while packet-per-buffer has trouble, I guessed that upon seeing the wake bit written, the vt6306 controller only checks the branch address in the first descriptor of the block, even if that is not the correct place to look (because the block has multiple descriptors). This theory seems to be correct. When the ISO reception is hung, I am able to resume it by manually writing the branch address to the first descriptor in the block, and then writing the wake bit. I've had luck so far with the attached patch, so I'm including it. It's probably not a complete solution -- I haven't tested transmit modes to see whether they have a similar issue. I doubt that the quirk test is any cheaper than just writing the extra branch address in all cases, but it does reduce the risk of breaking other hardware. [Stefan R: omitted QUIRK_NO_MSI from VT6306 quirks table entry, changed whitespace] Signed-off-by: Andy Leiserson Signed-off-by: Stefan Richter --- drivers/firewire/ohci.c | 38 +++++++++++++++++++++++++++++++++++--- 1 file changed, 35 insertions(+), 3 deletions(-) (limited to 'drivers/firewire') diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c index 673c8970749..a309d89f4df 100644 --- a/drivers/firewire/ohci.c +++ b/drivers/firewire/ohci.c @@ -68,6 +68,8 @@ #define DESCRIPTOR_BRANCH_ALWAYS (3 << 2) #define DESCRIPTOR_WAIT (3 << 0) +#define DESCRIPTOR_CMD (0xf << 12) + struct descriptor { __le16 req_count; __le16 control; @@ -149,10 +151,11 @@ struct context { struct descriptor *last; /* - * The last descriptor in the DMA program. It contains the branch + * The last descriptor block in the DMA program. It contains the branch * address that must be updated upon appending a new descriptor. */ struct descriptor *prev; + int prev_z; descriptor_callback_t callback; @@ -270,7 +273,9 @@ static char ohci_driver_name[] = KBUILD_MODNAME; #define PCI_DEVICE_ID_TI_TSB12LV22 0x8009 #define PCI_DEVICE_ID_TI_TSB12LV26 0x8020 #define PCI_DEVICE_ID_TI_TSB82AA2 0x8025 +#define PCI_DEVICE_ID_VIA_VT630X 0x3044 #define PCI_VENDOR_ID_PINNACLE_SYSTEMS 0x11bd +#define PCI_REV_ID_VIA_VT6306 0x46 #define QUIRK_CYCLE_TIMER 1 #define QUIRK_RESET_PACKET 2 @@ -278,6 +283,7 @@ static char ohci_driver_name[] = KBUILD_MODNAME; #define QUIRK_NO_1394A 8 #define QUIRK_NO_MSI 16 #define QUIRK_TI_SLLZ059 32 +#define QUIRK_IR_WAKE 64 /* In case of multiple matches in ohci_quirks[], only the first one is used. */ static const struct { @@ -319,6 +325,9 @@ static const struct { {PCI_VENDOR_ID_TI, PCI_ANY_ID, PCI_ANY_ID, QUIRK_RESET_PACKET}, + {PCI_VENDOR_ID_VIA, PCI_DEVICE_ID_VIA_VT630X, PCI_REV_ID_VIA_VT6306, + QUIRK_CYCLE_TIMER | QUIRK_IR_WAKE}, + {PCI_VENDOR_ID_VIA, PCI_ANY_ID, PCI_ANY_ID, QUIRK_CYCLE_TIMER | QUIRK_NO_MSI}, }; @@ -333,6 +342,7 @@ MODULE_PARM_DESC(quirks, "Chip quirks (default = 0" ", no 1394a enhancements = " __stringify(QUIRK_NO_1394A) ", disable MSI = " __stringify(QUIRK_NO_MSI) ", TI SLLZ059 erratum = " __stringify(QUIRK_TI_SLLZ059) + ", IR wake unreliable = " __stringify(QUIRK_IR_WAKE) ")"); #define OHCI_PARAM_DEBUG_AT_AR 1 @@ -1157,6 +1167,7 @@ static int context_init(struct context *ctx, struct fw_ohci *ohci, ctx->buffer_tail->used += sizeof(*ctx->buffer_tail->buffer); ctx->last = ctx->buffer_tail->buffer; ctx->prev = ctx->buffer_tail->buffer; + ctx->prev_z = 1; return 0; } @@ -1221,14 +1232,35 @@ static void context_append(struct context *ctx, { dma_addr_t d_bus; struct descriptor_buffer *desc = ctx->buffer_tail; + struct descriptor *d_branch; d_bus = desc->buffer_bus + (d - desc->buffer) * sizeof(*d); desc->used += (z + extra) * sizeof(*d); wmb(); /* finish init of new descriptors before branch_address update */ - ctx->prev->branch_address = cpu_to_le32(d_bus | z); - ctx->prev = find_branch_descriptor(d, z); + + d_branch = find_branch_descriptor(ctx->prev, ctx->prev_z); + d_branch->branch_address = cpu_to_le32(d_bus | z); + + /* + * VT6306 incorrectly checks only the single descriptor at the + * CommandPtr when the wake bit is written, so if it's a + * multi-descriptor block starting with an INPUT_MORE, put a copy of + * the branch address in the first descriptor. + * + * Not doing this for transmit contexts since not sure how it interacts + * with skip addresses. + */ + if (unlikely(ctx->ohci->quirks & QUIRK_IR_WAKE) && + d_branch != ctx->prev && + (ctx->prev->control & cpu_to_le16(DESCRIPTOR_CMD)) == + cpu_to_le16(DESCRIPTOR_INPUT_MORE)) { + ctx->prev->branch_address = cpu_to_le32(d_bus | z); + } + + ctx->prev = d; + ctx->prev_z = z; } static void context_stop(struct context *ctx) -- cgit v1.2.3-18-g5258 From bd972688eb2404239a8f1255db26b0bb6b604686 Mon Sep 17 00:00:00 2001 From: Peter Hurley Date: Sun, 28 Apr 2013 23:24:08 +0200 Subject: firewire: ohci: Fix 'failed to read phy reg' on FW643 rev8 With the LSI FW643 rev 8 [1], the first commanded bus reset at the conclusion of ohci_enable() has been observed to fail with the following messages: [ 4.884015] firewire_ohci 0000:01:00.0: failed to read phy reg .... [ 5.684012] firewire_ohci 0000:01:00.0: failed to read phy reg With drivers/firewire/ohci.c instrumented, the error condition [2] indicates the PHY arbitration state machine has timed out prior to enabling PHY LCtrl. Furthermore, instrumenting ohci_enable() shows that LPS has been enabled within 1 ms. Test LPS latching every 1 ms rather than every 50ms. [1] lspci -v 01:00.0 FireWire (IEEE 1394): LSI Corporation FW643 [TrueFire] PCIe 1394b Controller (rev 08) (prog-if 10 [OHCI]) Subsystem: LSI Corporation FW643 [TrueFire] PCIe 1394b Controller Flags: bus master, fast devsel, latency 0, IRQ 92 Memory at fbeff000 (64-bit, non-prefetchable) [size=4K] Capabilities: [44] Power Management version 3 Capabilities: [4c] MSI: Enable+ Count=1/1 Maskable- 64bit+ Capabilities: [60] Express Endpoint, MSI 00 Capabilities: [100] Advanced Error Reporting Capabilities: [140] Virtual Channel Capabilities: [170] Device Serial Number 08-14-43-82-00-00-41-fc Kernel driver in use: firewire_ohci Kernel modules: firewire-ohci [2] instrumented WARNING in read_phy_reg() [ 4.576010] ------------[ cut here ]------------ [ 4.576035] WARNING: at ./drivers/firewire/ohci.c:570 read_phy_reg+0x93/0xe0 [firewire_ohci]() [ 4.576050] Hardware name: Precision WorkStation T5400 [ 4.576058] failed to read phy reg:1 (phy(5) @ config enhance:19) [ 4.576068] Modules linked in: hid_logitech_dj hid_generic(+) usbhid <...snip...> [ 4.576140] Pid: 61, comm: kworker/2:1 Not tainted 3.8.0-2+fwtest-xeon #2+fwtest [ 4.576149] Call Trace: [ 4.576160] [] warn_slowpath_common+0x7f/0xc0 [ 4.576168] [] warn_slowpath_fmt+0x46/0x50 [ 4.576178] [] read_phy_reg+0x93/0xe0 [firewire_ohci] [ 4.576188] [] ohci_read_phy_reg+0x39/0x60 [firewire_ohci] [ 4.576203] [] fw_send_phy_config+0xbf/0xe0 [firewire_core] [ 4.576214] [] br_work+0x46/0xb0 [firewire_core] [ 4.576225] [] process_one_work+0x13c/0x500 [ 4.576238] [] ? fw_card_initialize+0x180/0x180 [firewire_core] [ 4.576248] [] worker_thread+0x16d/0x470 [ 4.576257] [] ? busy_worker_rebind_fn+0x100/0x100 [ 4.576266] [] kthread+0xc0/0xd0 [ 4.576275] [] ? pcpu_dump_alloc_info+0x1cb/0x2c4 [ 4.576284] [] ? kthread_create_on_node+0x130/0x130 [ 4.576297] [] ret_from_fork+0x7c/0xb0 [ 4.576305] [] ? kthread_create_on_node+0x130/0x130 [ 4.576313] ---[ end trace cbc940994b300302 ]--- [Stefan R: Peter also reports a change of behavior with LSI FW323. Before the patch, there would often occur a lock transaction failure during firewire-core startup: [ 6.056022] firewire_core 0000:07:06.0: BM lock failed (timeout), making local node (ffc0) root This failure no longer happens after the patch, without an obvious reason for the failure or the fix.] [Stefan R: Added quirk flag, quirk table entry, and comment.] Reported-by: Tim Jordan Signed-off-by: Peter Hurley Signed-off-by: Stefan Richter --- drivers/firewire/ohci.c | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) (limited to 'drivers/firewire') diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c index a309d89f4df..77a64bf1802 100644 --- a/drivers/firewire/ohci.c +++ b/drivers/firewire/ohci.c @@ -284,6 +284,7 @@ static char ohci_driver_name[] = KBUILD_MODNAME; #define QUIRK_NO_MSI 16 #define QUIRK_TI_SLLZ059 32 #define QUIRK_IR_WAKE 64 +#define QUIRK_PHY_LCTRL_TIMEOUT 128 /* In case of multiple matches in ohci_quirks[], only the first one is used. */ static const struct { @@ -296,7 +297,10 @@ static const struct { QUIRK_BE_HEADERS}, {PCI_VENDOR_ID_ATT, PCI_DEVICE_ID_AGERE_FW643, 6, - QUIRK_NO_MSI}, + QUIRK_PHY_LCTRL_TIMEOUT | QUIRK_NO_MSI}, + + {PCI_VENDOR_ID_ATT, PCI_ANY_ID, PCI_ANY_ID, + QUIRK_PHY_LCTRL_TIMEOUT}, {PCI_VENDOR_ID_CREATIVE, PCI_DEVICE_ID_CREATIVE_SB1394, PCI_ANY_ID, QUIRK_RESET_PACKET}, @@ -343,6 +347,7 @@ MODULE_PARM_DESC(quirks, "Chip quirks (default = 0" ", disable MSI = " __stringify(QUIRK_NO_MSI) ", TI SLLZ059 erratum = " __stringify(QUIRK_TI_SLLZ059) ", IR wake unreliable = " __stringify(QUIRK_IR_WAKE) + ", phy LCtrl timeout = " __stringify(QUIRK_PHY_LCTRL_TIMEOUT) ")"); #define OHCI_PARAM_DEBUG_AT_AR 1 @@ -2293,14 +2298,25 @@ static int ohci_enable(struct fw_card *card, * will lock up the machine. Wait 50msec to make sure we have * full link enabled. However, with some cards (well, at least * a JMicron PCIe card), we have to try again sometimes. + * + * TI TSB82AA2 + TSB81BA3(A) cards signal LPS enabled early but + * cannot actually use the phy at that time. These need tens of + * millisecods pause between LPS write and first phy access too. + * + * But do not wait for 50msec on Agere/LSI cards. Their phy + * arbitration state machine may time out during such a long wait. */ + reg_write(ohci, OHCI1394_HCControlSet, OHCI1394_HCControl_LPS | OHCI1394_HCControl_postedWriteEnable); flush_writes(ohci); - for (lps = 0, i = 0; !lps && i < 3; i++) { + if (!(ohci->quirks & QUIRK_PHY_LCTRL_TIMEOUT)) msleep(50); + + for (lps = 0, i = 0; !lps && i < 150; i++) { + msleep(1); lps = reg_read(ohci, OHCI1394_HCControlSet) & OHCI1394_HCControl_LPS; } -- cgit v1.2.3-18-g5258 From de97cb64a959fe5ccf828b02d3a78de9f9defb67 Mon Sep 17 00:00:00 2001 From: Peter Hurley Date: Tue, 26 Mar 2013 11:54:06 -0400 Subject: firewire: ohci: Alias dev_* log functions Convert dev_xxxx(ohci->card.device, ...) log functions to ohci_xxxx(ohci, ...). [Stefan R: Peter argues that this increases readability of the code.] [Stefan R: changed whitespace] Signed-off-by: Peter Hurley Signed-off-by: Stefan Richter --- drivers/firewire/ohci.c | 143 ++++++++++++++++++++++-------------------------- 1 file changed, 65 insertions(+), 78 deletions(-) (limited to 'drivers/firewire') diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c index 77a64bf1802..925a2a6fe68 100644 --- a/drivers/firewire/ohci.c +++ b/drivers/firewire/ohci.c @@ -54,6 +54,10 @@ #include "core.h" #include "ohci.h" +#define ohci_info(ohci, f, args...) dev_info(ohci->card.device, f, ##args) +#define ohci_notice(ohci, f, args...) dev_notice(ohci->card.device, f, ##args) +#define ohci_err(ohci, f, args...) dev_err(ohci->card.device, f, ##args) + #define DESCRIPTOR_OUTPUT_MORE 0 #define DESCRIPTOR_OUTPUT_LAST (1 << 12) #define DESCRIPTOR_INPUT_MORE (2 << 12) @@ -374,8 +378,7 @@ static void log_irqs(struct fw_ohci *ohci, u32 evt) !(evt & OHCI1394_busReset)) return; - dev_notice(ohci->card.device, - "IRQ %08x%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s\n", evt, + ohci_notice(ohci, "IRQ %08x%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s\n", evt, evt & OHCI1394_selfIDComplete ? " selfID" : "", evt & OHCI1394_RQPkt ? " AR_req" : "", evt & OHCI1394_RSPkt ? " AR_resp" : "", @@ -421,21 +424,19 @@ static void log_selfids(struct fw_ohci *ohci, int generation, int self_id_count) if (likely(!(param_debug & OHCI_PARAM_DEBUG_SELFIDS))) return; - dev_notice(ohci->card.device, - "%d selfIDs, generation %d, local node ID %04x\n", - self_id_count, generation, ohci->node_id); + ohci_notice(ohci, "%d selfIDs, generation %d, local node ID %04x\n", + self_id_count, generation, ohci->node_id); for (s = ohci->self_id_buffer; self_id_count--; ++s) if ((*s & 1 << 23) == 0) - dev_notice(ohci->card.device, - "selfID 0: %08x, phy %d [%c%c%c] " - "%s gc=%d %s %s%s%s\n", + ohci_notice(ohci, + "selfID 0: %08x, phy %d [%c%c%c] %s gc=%d %s %s%s%s\n", *s, *s >> 24 & 63, _p(s, 6), _p(s, 4), _p(s, 2), speed[*s >> 14 & 3], *s >> 16 & 63, power[*s >> 8 & 7], *s >> 22 & 1 ? "L" : "", *s >> 11 & 1 ? "c" : "", *s & 2 ? "i" : ""); else - dev_notice(ohci->card.device, + ohci_notice(ohci, "selfID n: %08x, phy %d [%c%c%c%c%c%c%c%c]\n", *s, *s >> 24 & 63, _p(s, 16), _p(s, 14), _p(s, 12), _p(s, 10), @@ -485,9 +486,8 @@ static void log_ar_at_event(struct fw_ohci *ohci, evt = 0x1f; if (evt == OHCI1394_evt_bus_reset) { - dev_notice(ohci->card.device, - "A%c evt_bus_reset, generation %d\n", - dir, (header[2] >> 16) & 0xff); + ohci_notice(ohci, "A%c evt_bus_reset, generation %d\n", + dir, (header[2] >> 16) & 0xff); return; } @@ -506,32 +506,26 @@ static void log_ar_at_event(struct fw_ohci *ohci, switch (tcode) { case 0xa: - dev_notice(ohci->card.device, - "A%c %s, %s\n", - dir, evts[evt], tcodes[tcode]); + ohci_notice(ohci, "A%c %s, %s\n", + dir, evts[evt], tcodes[tcode]); break; case 0xe: - dev_notice(ohci->card.device, - "A%c %s, PHY %08x %08x\n", - dir, evts[evt], header[1], header[2]); + ohci_notice(ohci, "A%c %s, PHY %08x %08x\n", + dir, evts[evt], header[1], header[2]); break; case 0x0: case 0x1: case 0x4: case 0x5: case 0x9: - dev_notice(ohci->card.device, - "A%c spd %x tl %02x, " - "%04x -> %04x, %s, " - "%s, %04x%08x%s\n", - dir, speed, header[0] >> 10 & 0x3f, - header[1] >> 16, header[0] >> 16, evts[evt], - tcodes[tcode], header[1] & 0xffff, header[2], specific); + ohci_notice(ohci, + "A%c spd %x tl %02x, %04x -> %04x, %s, %s, %04x%08x%s\n", + dir, speed, header[0] >> 10 & 0x3f, + header[1] >> 16, header[0] >> 16, evts[evt], + tcodes[tcode], header[1] & 0xffff, header[2], specific); break; default: - dev_notice(ohci->card.device, - "A%c spd %x tl %02x, " - "%04x -> %04x, %s, " - "%s%s\n", - dir, speed, header[0] >> 10 & 0x3f, - header[1] >> 16, header[0] >> 16, evts[evt], - tcodes[tcode], specific); + ohci_notice(ohci, + "A%c spd %x tl %02x, %04x -> %04x, %s, %s%s\n", + dir, speed, header[0] >> 10 & 0x3f, + header[1] >> 16, header[0] >> 16, evts[evt], + tcodes[tcode], specific); } } @@ -578,7 +572,7 @@ static int read_phy_reg(struct fw_ohci *ohci, int addr) if (i >= 3) msleep(1); } - dev_err(ohci->card.device, "failed to read phy reg\n"); + ohci_err(ohci, "failed to read phy reg\n"); return -EBUSY; } @@ -600,7 +594,7 @@ static int write_phy_reg(const struct fw_ohci *ohci, int addr, u32 val) if (i >= 3) msleep(1); } - dev_err(ohci->card.device, "failed to write phy reg\n"); + ohci_err(ohci, "failed to write phy reg\n"); return -EBUSY; } @@ -705,8 +699,7 @@ static void ar_context_abort(struct ar_context *ctx, const char *error_msg) reg_write(ohci, CONTROL_CLEAR(ctx->regs), CONTEXT_RUN); flush_writes(ohci); - dev_err(ohci->card.device, "AR error: %s; DMA stopped\n", - error_msg); + ohci_err(ohci, "AR error: %s; DMA stopped\n", error_msg); } /* FIXME: restart? */ } @@ -1285,7 +1278,7 @@ static void context_stop(struct context *ctx) if (i) udelay(10); } - dev_err(ohci->card.device, "DMA context still active (0x%08x)\n", reg); + ohci_err(ohci, "DMA context still active (0x%08x)\n", reg); } struct driver_data { @@ -1594,7 +1587,7 @@ static void handle_local_lock(struct fw_ohci *ohci, goto out; } - dev_err(ohci->card.device, "swap not done (CSR lock timeout)\n"); + ohci_err(ohci, "swap not done (CSR lock timeout)\n"); fw_fill_response(&response, packet->header, RCODE_BUSY, NULL, 0); out: @@ -1669,8 +1662,7 @@ static void detect_dead_context(struct fw_ohci *ohci, ctl = reg_read(ohci, CONTROL_SET(regs)); if (ctl & CONTEXT_DEAD) - dev_err(ohci->card.device, - "DMA context %s has stopped, error code: %s\n", + ohci_err(ohci, "DMA context %s has stopped, error code: %s\n", name, evts[ctl & 0x1f]); } @@ -1852,8 +1844,8 @@ static int find_and_insert_self_id(struct fw_ohci *ohci, int self_id_count) reg = reg_read(ohci, OHCI1394_NodeID); if (!(reg & OHCI1394_NodeID_idValid)) { - dev_notice(ohci->card.device, - "node ID not valid, new bus reset in progress\n"); + ohci_notice(ohci, + "node ID not valid, new bus reset in progress\n"); return -EBUSY; } self_id |= ((reg & 0x3f) << 24); /* phy ID */ @@ -1900,12 +1892,12 @@ static void bus_reset_work(struct work_struct *work) reg = reg_read(ohci, OHCI1394_NodeID); if (!(reg & OHCI1394_NodeID_idValid)) { - dev_notice(ohci->card.device, - "node ID not valid, new bus reset in progress\n"); + ohci_notice(ohci, + "node ID not valid, new bus reset in progress\n"); return; } if ((reg & OHCI1394_NodeID_nodeNumber) == 63) { - dev_notice(ohci->card.device, "malconfigured bus\n"); + ohci_notice(ohci, "malconfigured bus\n"); return; } ohci->node_id = reg & (OHCI1394_NodeID_busNumber | @@ -1919,7 +1911,7 @@ static void bus_reset_work(struct work_struct *work) reg = reg_read(ohci, OHCI1394_SelfIDCount); if (reg & OHCI1394_SelfIDCount_selfIDError) { - dev_notice(ohci->card.device, "inconsistent self IDs\n"); + ohci_notice(ohci, "inconsistent self IDs\n"); return; } /* @@ -1931,7 +1923,7 @@ static void bus_reset_work(struct work_struct *work) self_id_count = (reg >> 3) & 0xff; if (self_id_count > 252) { - dev_notice(ohci->card.device, "inconsistent self IDs\n"); + ohci_notice(ohci, "inconsistent self IDs\n"); return; } @@ -1949,13 +1941,12 @@ static void bus_reset_work(struct work_struct *work) */ if (cond_le32_to_cpu(ohci->self_id_cpu[i]) == 0xffff008f) { - dev_notice(ohci->card.device, - "ignoring spurious self IDs\n"); + ohci_notice(ohci, + "ignoring spurious self IDs\n"); self_id_count = j; break; } else { - dev_notice(ohci->card.device, - "inconsistent self IDs\n"); + ohci_notice(ohci, "inconsistent self IDs\n"); return; } } @@ -1966,14 +1957,14 @@ static void bus_reset_work(struct work_struct *work) if (ohci->quirks & QUIRK_TI_SLLZ059) { self_id_count = find_and_insert_self_id(ohci, self_id_count); if (self_id_count < 0) { - dev_notice(ohci->card.device, - "could not construct local self ID\n"); + ohci_notice(ohci, + "could not construct local self ID\n"); return; } } if (self_id_count == 0) { - dev_notice(ohci->card.device, "inconsistent self IDs\n"); + ohci_notice(ohci, "inconsistent self IDs\n"); return; } rmb(); @@ -1994,8 +1985,7 @@ static void bus_reset_work(struct work_struct *work) new_generation = (reg_read(ohci, OHCI1394_SelfIDCount) >> 16) & 0xff; if (new_generation != generation) { - dev_notice(ohci->card.device, - "new bus reset, discarding self ids\n"); + ohci_notice(ohci, "new bus reset, discarding self ids\n"); return; } @@ -2133,7 +2123,7 @@ static irqreturn_t irq_handler(int irq, void *data) } if (unlikely(event & OHCI1394_regAccessFail)) - dev_err(ohci->card.device, "register access failure\n"); + ohci_err(ohci, "register access failure\n"); if (unlikely(event & OHCI1394_postedWriteErr)) { reg_read(ohci, OHCI1394_PostedWriteAddressHi); @@ -2141,13 +2131,12 @@ static irqreturn_t irq_handler(int irq, void *data) reg_write(ohci, OHCI1394_IntEventClear, OHCI1394_postedWriteErr); if (printk_ratelimit()) - dev_err(ohci->card.device, "PCI posted write error\n"); + ohci_err(ohci, "PCI posted write error\n"); } if (unlikely(event & OHCI1394_cycleTooLong)) { if (printk_ratelimit()) - dev_notice(ohci->card.device, - "isochronous cycle too long\n"); + ohci_notice(ohci, "isochronous cycle too long\n"); reg_write(ohci, OHCI1394_LinkControlSet, OHCI1394_LinkControl_cycleMaster); } @@ -2160,8 +2149,7 @@ static irqreturn_t irq_handler(int irq, void *data) * them at least two cycles later. (FIXME?) */ if (printk_ratelimit()) - dev_notice(ohci->card.device, - "isochronous cycle inconsistent\n"); + ohci_notice(ohci, "isochronous cycle inconsistent\n"); } if (unlikely(event & OHCI1394_unrecoverableError)) @@ -2287,7 +2275,7 @@ static int ohci_enable(struct fw_card *card, int i, ret; if (software_reset(ohci)) { - dev_err(card->device, "failed to reset ohci card\n"); + ohci_err(ohci, "failed to reset ohci card\n"); return -EBUSY; } @@ -2322,7 +2310,7 @@ static int ohci_enable(struct fw_card *card, } if (!lps) { - dev_err(card->device, "failed to set Link Power Status\n"); + ohci_err(ohci, "failed to set Link Power Status\n"); return -EIO; } @@ -2331,7 +2319,7 @@ static int ohci_enable(struct fw_card *card, if (ret < 0) return ret; if (ret) - dev_notice(card->device, "local TSB41BA3D phy\n"); + ohci_notice(ohci, "local TSB41BA3D phy\n"); else ohci->quirks &= ~QUIRK_TI_SLLZ059; } @@ -3607,20 +3595,20 @@ static int pci_probe(struct pci_dev *dev, if (!(pci_resource_flags(dev, 0) & IORESOURCE_MEM) || pci_resource_len(dev, 0) < OHCI1394_REGISTER_SIZE) { - dev_err(&dev->dev, "invalid MMIO resource\n"); + ohci_err(ohci, "invalid MMIO resource\n"); err = -ENXIO; goto fail_disable; } err = pci_request_region(dev, 0, ohci_driver_name); if (err) { - dev_err(&dev->dev, "MMIO resource unavailable\n"); + ohci_err(ohci, "MMIO resource unavailable\n"); goto fail_disable; } ohci->registers = pci_iomap(dev, 0, OHCI1394_REGISTER_SIZE); if (ohci->registers == NULL) { - dev_err(&dev->dev, "failed to remap registers\n"); + ohci_err(ohci, "failed to remap registers\n"); err = -ENXIO; goto fail_iomem; } @@ -3709,8 +3697,7 @@ static int pci_probe(struct pci_dev *dev, if (request_irq(dev->irq, irq_handler, pci_dev_msi_enabled(dev) ? 0 : IRQF_SHARED, ohci_driver_name, ohci)) { - dev_err(&dev->dev, "failed to allocate interrupt %d\n", - dev->irq); + ohci_err(ohci, "failed to allocate interrupt %d\n", dev->irq); err = -EIO; goto fail_msi; } @@ -3720,11 +3707,11 @@ static int pci_probe(struct pci_dev *dev, goto fail_irq; version = reg_read(ohci, OHCI1394_Version) & 0x00ff00ff; - dev_notice(&dev->dev, - "added OHCI v%x.%x device as card %d, " - "%d IR + %d IT contexts, quirks 0x%x\n", - version >> 16, version & 0xff, ohci->card.index, - ohci->n_ir, ohci->n_it, ohci->quirks); + ohci_notice(ohci, + "added OHCI v%x.%x device as card %d, " + "%d IR + %d IT contexts, quirks 0x%x\n", + version >> 16, version & 0xff, ohci->card.index, + ohci->n_ir, ohci->n_it, ohci->quirks); return 0; @@ -3814,12 +3801,12 @@ static int pci_suspend(struct pci_dev *dev, pm_message_t state) software_reset(ohci); err = pci_save_state(dev); if (err) { - dev_err(&dev->dev, "pci_save_state failed\n"); + ohci_err(ohci, "pci_save_state failed\n"); return err; } err = pci_set_power_state(dev, pci_choose_state(dev, state)); if (err) - dev_err(&dev->dev, "pci_set_power_state failed with %d\n", err); + ohci_err(ohci, "pci_set_power_state failed with %d\n", err); pmac_ohci_off(dev); return 0; @@ -3835,7 +3822,7 @@ static int pci_resume(struct pci_dev *dev) pci_restore_state(dev); err = pci_enable_device(dev); if (err) { - dev_err(&dev->dev, "pci_enable_device failed\n"); + ohci_err(ohci, "pci_enable_device failed\n"); return err; } -- cgit v1.2.3-18-g5258 From 67672134aaafd520a61dda448a662336e8fde236 Mon Sep 17 00:00:00 2001 From: Peter Hurley Date: Wed, 27 Mar 2013 06:56:01 -0400 Subject: firewire: ohci: Improve bus reset error messages Many of the error messages possible from bus_reset_work() do not contain enough information to distinguish which error condition occurred nor enough information to evaluate the error afterwards. Differentiate all error conditions in bus_reset_work(); add additional information to make error diagnosis possible. [Stefan R: fixed self-ID endian conversion] Signed-off-by: Peter Hurley Signed-off-by: Stefan Richter --- drivers/firewire/ohci.c | 27 ++++++++++++++------------- 1 file changed, 14 insertions(+), 13 deletions(-) (limited to 'drivers/firewire') diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c index 925a2a6fe68..c64b4801ada 100644 --- a/drivers/firewire/ohci.c +++ b/drivers/firewire/ohci.c @@ -1911,7 +1911,7 @@ static void bus_reset_work(struct work_struct *work) reg = reg_read(ohci, OHCI1394_SelfIDCount); if (reg & OHCI1394_SelfIDCount_selfIDError) { - ohci_notice(ohci, "inconsistent self IDs\n"); + ohci_notice(ohci, "self ID receive error\n"); return; } /* @@ -1923,7 +1923,7 @@ static void bus_reset_work(struct work_struct *work) self_id_count = (reg >> 3) & 0xff; if (self_id_count > 252) { - ohci_notice(ohci, "inconsistent self IDs\n"); + ohci_notice(ohci, "bad selfIDSize (%08x)\n", reg); return; } @@ -1931,7 +1931,10 @@ static void bus_reset_work(struct work_struct *work) rmb(); for (i = 1, j = 0; j < self_id_count; i += 2, j++) { - if (ohci->self_id_cpu[i] != ~ohci->self_id_cpu[i + 1]) { + u32 id = cond_le32_to_cpu(ohci->self_id_cpu[i]); + u32 id2 = cond_le32_to_cpu(ohci->self_id_cpu[i + 1]); + + if (id != ~id2) { /* * If the invalid data looks like a cycle start packet, * it's likely to be the result of the cycle master @@ -1939,19 +1942,17 @@ static void bus_reset_work(struct work_struct *work) * so far are valid and should be processed so that the * bus manager can then correct the gap count. */ - if (cond_le32_to_cpu(ohci->self_id_cpu[i]) - == 0xffff008f) { - ohci_notice(ohci, - "ignoring spurious self IDs\n"); + if (id == 0xffff008f) { + ohci_notice(ohci, "ignoring spurious self IDs\n"); self_id_count = j; break; - } else { - ohci_notice(ohci, "inconsistent self IDs\n"); - return; } + + ohci_notice(ohci, "bad self ID %d/%d (%08x != ~%08x)\n", + j, self_id_count, id, id2); + return; } - ohci->self_id_buffer[j] = - cond_le32_to_cpu(ohci->self_id_cpu[i]); + ohci->self_id_buffer[j] = id; } if (ohci->quirks & QUIRK_TI_SLLZ059) { @@ -1964,7 +1965,7 @@ static void bus_reset_work(struct work_struct *work) } if (self_id_count == 0) { - ohci_notice(ohci, "inconsistent self IDs\n"); + ohci_notice(ohci, "no self IDs\n"); return; } rmb(); -- cgit v1.2.3-18-g5258 From 6fe9efb9c9fbce690018c31e652924ae28019868 Mon Sep 17 00:00:00 2001 From: Peter Hurley Date: Wed, 27 Mar 2013 06:57:40 -0400 Subject: firewire: ohci: dump_stack() for PHY regs read/write failures A stack trace is an invaluable tool in determining the basis and cause of PHY regs read/write failures. Include PHY reg addr (and value for writes) in the diagnostic. [Stefan R: changed whitespace] Signed-off-by: Peter Hurley Signed-off-by: Stefan Richter --- drivers/firewire/ohci.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'drivers/firewire') diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c index c64b4801ada..9e1db6490b9 100644 --- a/drivers/firewire/ohci.c +++ b/drivers/firewire/ohci.c @@ -572,7 +572,8 @@ static int read_phy_reg(struct fw_ohci *ohci, int addr) if (i >= 3) msleep(1); } - ohci_err(ohci, "failed to read phy reg\n"); + ohci_err(ohci, "failed to read phy reg %d\n", addr); + dump_stack(); return -EBUSY; } @@ -594,7 +595,8 @@ static int write_phy_reg(const struct fw_ohci *ohci, int addr, u32 val) if (i >= 3) msleep(1); } - ohci_err(ohci, "failed to write phy reg\n"); + ohci_err(ohci, "failed to write phy reg %d, val %u\n", addr, val); + dump_stack(); return -EBUSY; } -- cgit v1.2.3-18-g5258