From 7a481436787cbc932af6c407b317ac603969a242 Mon Sep 17 00:00:00 2001 From: Stefan Richter Date: Tue, 26 Jan 2010 21:39:07 +0100 Subject: firewire: ohci: fix crashes with TSB43AB23 on 64bit systems Unsurprisingly, Texas Instruments TSB43AB23 exhibits the same behaviour as TSB43AB22/A in dual buffer IR DMA mode: If descriptors are located at physical addresses above the 31 bit address range (2 GB), the controller will overwrite random memory. With luck, this merely prevents video reception. With only a little less luck, the machine crashes. We use the same workaround here as with TSB43AB22/A: Switch off the dual buffer capability flag and use packet-per-buffer IR DMA instead. Another possible workaround would be to limit the coherent DMA mask to 31 bits. In Linux 2.6.33, this change serves effectively only as documentation since dual buffer mode is not used for any controller anymore. But somebody might want to re-enable it in the future to make use of features of dual buffer DMA that are not available in packet-per-buffer mode. In Linux 2.6.32 and older, this update is vital for anyone with this controller, more than 2 GB RAM, a 64 bit kernel, and FireWire video or audio applications. We have at least four reports: http://bugzilla.kernel.org/show_bug.cgi?id=13808 http://marc.info/?l=linux1394-user&m=126154279004083 https://bugzilla.redhat.com/show_bug.cgi?id=552142 http://marc.info/?l=linux1394-user&m=126432246128386 Reported-by: Paul Johnson Reported-by: Ronneil Camara Reported-by: G Zornetzer Reported-by: Mark Thompson Cc: stable@kernel.org Signed-off-by: Stefan Richter --- drivers/firewire/ohci.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'drivers/firewire/ohci.c') diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c index a61571c63c5..2345d4103fe 100644 --- a/drivers/firewire/ohci.c +++ b/drivers/firewire/ohci.c @@ -2420,6 +2420,7 @@ static void ohci_pmac_off(struct pci_dev *dev) #define PCI_VENDOR_ID_AGERE PCI_VENDOR_ID_ATT #define PCI_DEVICE_ID_AGERE_FW643 0x5901 +#define PCI_DEVICE_ID_TI_TSB43AB23 0x8024 static int __devinit pci_probe(struct pci_dev *dev, const struct pci_device_id *ent) @@ -2488,7 +2489,8 @@ static int __devinit pci_probe(struct pci_dev *dev, #if !defined(CONFIG_X86_32) /* dual-buffer mode is broken with descriptor addresses above 2G */ if (dev->vendor == PCI_VENDOR_ID_TI && - dev->device == PCI_DEVICE_ID_TI_TSB43AB22) + (dev->device == PCI_DEVICE_ID_TI_TSB43AB22 || + dev->device == PCI_DEVICE_ID_TI_TSB43AB23)) ohci->use_dualbuffer = false; #endif -- cgit v1.2.3 From 7f51a100bba517196ac4bdf29408d20ee1c771e8 Mon Sep 17 00:00:00 2001 From: Clemens Ladisch Date: Mon, 8 Feb 2010 08:30:03 +0100 Subject: firewire: ohci: retransmit isochronous transmit packets on cycle loss In isochronous transmit DMA descriptors, link the skip address pointer back to the descriptor itself. When a cycle is lost, the controller will send the packet in the next cycle, instead of terminating the entire DMA program. There are two reasons for this: * This behaviour is compatible with the old IEEE1394 stack. Old applications would not expect the DMA program to stop in this case. * Since the OHCI driver does not report any uncompleted packets, the context would stop silently; clients would not have any chance to detect and handle this error without a watchdog timer. Signed-off-by: Clemens Ladisch Pieter Palmers notes: "The reason I added this retry behavior to the old stack is because some cards now and then fail to send a packet (e.g. the o2micro card in my dell laptop). I couldn't figure out why exactly this happens, my best guess is that the card cannot fetch the payload data on time. This happens much more frequently when sending large packets, which leads me to suspect that there are some contention issues with the DMA that fills the transmit FIFO. In the old stack it was a pretty critical issue as it resulted in a freeze of the userspace application. The omission of a packet doesn't necessarily have to be an issue. E.g. in IEC61883 streams the DBC field can be used to detect discontinuities in the stream. So as long as the other side doesn't bail when no [packet] is present in a cycle, there is not really a problem. I'm not convinced though that retrying is the proper solution, but it is simple and effective for what it had to do. And I think there are no reasons not to do it this way. Userspace can still detect this by checking the cycle the descriptor was sent in." Signed-off-by: Stefan Richter (changelog, comment) --- drivers/firewire/ohci.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) (limited to 'drivers/firewire/ohci.c') diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c index 2345d4103fe..43ebf337b13 100644 --- a/drivers/firewire/ohci.c +++ b/drivers/firewire/ohci.c @@ -2101,11 +2101,6 @@ static int ohci_queue_iso_transmit(struct fw_iso_context *base, u32 payload_index, payload_end_index, next_page_index; int page, end_page, i, length, offset; - /* - * FIXME: Cycle lost behavior should be configurable: lose - * packet, retransmit or terminate.. - */ - p = packet; payload_index = payload; @@ -2135,6 +2130,14 @@ static int ohci_queue_iso_transmit(struct fw_iso_context *base, if (!p->skip) { d[0].control = cpu_to_le16(DESCRIPTOR_KEY_IMMEDIATE); d[0].req_count = cpu_to_le16(8); + /* + * Link the skip address to this descriptor itself. This causes + * a context to skip a cycle whenever lost cycles or FIFO + * overruns occur, without dropping the data. The application + * should then decide whether this is an error condition or not. + * FIXME: Make the context's cycle-lost behaviour configurable? + */ + d[0].branch_address = cpu_to_le32(d_bus | z); header = (__le32 *) &d[1]; header[0] = cpu_to_le32(IT_HEADER_SY(p->sy) | -- cgit v1.2.3