public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [git pull] FireWire regression fix
@ 2011-05-04 11:45 Stefan Richter
  0 siblings, 0 replies; 4+ messages in thread
From: Stefan Richter @ 2011-05-04 11:45 UTC (permalink / raw)
  To: Linus Torvalds, Andrew Morton; +Cc: linux-kernel, linux1394-devel

Linus, please pull from the fixes branch at

    git://git.kernel.org/pub/scm/linux/kernel/git/ieee1394/linux1394-2.6.git fixes

to receive the following update for the IEEE 1394 (FireWire) subsystem.
Thanks.

B.J. Buchalter (1):
      firewire: Fix for broken configrom updates in quick succession

 drivers/firewire/ohci.c |   39 +++++++++++++++++++++++++--------------
 1 files changed, 25 insertions(+), 14 deletions(-)


commit 2e053a27d9d5ad5e0831e002cbf8043836fb2060
Author: B.J. Buchalter <bj@mhlabs.com>
Date:   Mon May 2 13:33:42 2011 -0400

    firewire: Fix for broken configrom updates in quick succession
    
    Current implementation of ohci_set_config_rom() uses a deferred
    bus reset via fw_schedule_bus_reset(). If clients add multiple
    unit descriptors to the config_rom in quick succession, the
    deferred bus reset may not have fired before succeeding update
    requests have come in. This can lead to an incorrect partial
    update of the config_rom for both addition and removal of
    config_rom descriptors, as the ohci_set_config_rom() routine
    will return -EBUSY if a previous pending update has not been
    completed yet; the requested update just gets dropped on the floor.
    
    This patch recognizes that the "in-flight" update can be modified
    until it has been processed by the bus-reset, and the locking
    in the bus_reset_tasklet ensures that the update is done atomically
    with respect to modifications made by ohci_set_config_rom(). The
    -EBUSY error case is simply removed.
    
    [Stefan R:  The bug always existed at least theoretically.  But it
    became easy to trigger since 2.6.36 commit 02d37bed188c "firewire: core:
    integrate software-forced bus resets with bus management" which
    introduced long mandatory delays between janitorial bus resets.]
    
    Signed-off-by: Benjamin Buchalter <bj@mhlabs.com>
    Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de> (trivial style changes)
    Cc: <stable@kernel.org> # 2.6.36.y and newer

diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c
index f903d7b6..23d1468 100644
--- a/drivers/firewire/ohci.c
+++ b/drivers/firewire/ohci.c
@@ -2199,7 +2199,6 @@ static int ohci_set_config_rom(struct fw_card *card,
 {
 	struct fw_ohci *ohci;
 	unsigned long flags;
-	int ret = -EBUSY;
 	__be32 *next_config_rom;
 	dma_addr_t uninitialized_var(next_config_rom_bus);
 
@@ -2240,22 +2239,37 @@ static int ohci_set_config_rom(struct fw_card *card,
 
 	spin_lock_irqsave(&ohci->lock, flags);
 
+	/*
+	 * If there is not an already pending config_rom update,
+	 * push our new allocation into the ohci->next_config_rom
+	 * and then mark the local variable as null so that we
+	 * won't deallocate the new buffer.
+	 *
+	 * OTOH, if there is a pending config_rom update, just
+	 * use that buffer with the new config_rom data, and
+	 * let this routine free the unused DMA allocation.
+	 */
+
 	if (ohci->next_config_rom == NULL) {
 		ohci->next_config_rom = next_config_rom;
 		ohci->next_config_rom_bus = next_config_rom_bus;
+		next_config_rom = NULL;
+	}
 
-		copy_config_rom(ohci->next_config_rom, config_rom, length);
+	copy_config_rom(ohci->next_config_rom, config_rom, length);
 
-		ohci->next_header = config_rom[0];
-		ohci->next_config_rom[0] = 0;
+	ohci->next_header = config_rom[0];
+	ohci->next_config_rom[0] = 0;
 
-		reg_write(ohci, OHCI1394_ConfigROMmap,
-			  ohci->next_config_rom_bus);
-		ret = 0;
-	}
+	reg_write(ohci, OHCI1394_ConfigROMmap, ohci->next_config_rom_bus);
 
 	spin_unlock_irqrestore(&ohci->lock, flags);
 
+	/* If we didn't use the DMA allocation, delete it. */
+	if (next_config_rom != NULL)
+		dma_free_coherent(ohci->card.device, CONFIG_ROM_SIZE,
+				  next_config_rom, next_config_rom_bus);
+
 	/*
 	 * Now initiate a bus reset to have the changes take
 	 * effect. We clean up the old config rom memory and DMA
@@ -2263,13 +2277,10 @@ static int ohci_set_config_rom(struct fw_card *card,
 	 * controller could need to access it before the bus reset
 	 * takes effect.
 	 */
-	if (ret == 0)
-		fw_schedule_bus_reset(&ohci->card, true, true);
-	else
-		dma_free_coherent(ohci->card.device, CONFIG_ROM_SIZE,
-				  next_config_rom, next_config_rom_bus);
 
-	return ret;
+	fw_schedule_bus_reset(&ohci->card, true, true);
+
+	return 0;
 }
 
 static void ohci_send_request(struct fw_card *card, struct fw_packet *packet)

-- 
Stefan Richter
-=====-==-== -=-= --=--
http://arcgraph.de/sr/

^ permalink raw reply related	[flat|nested] 4+ messages in thread
* [git pull] FireWire regression fix
@ 2014-07-26  8:48 Stefan Richter
  0 siblings, 0 replies; 4+ messages in thread
From: Stefan Richter @ 2014-07-26  8:48 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: linux-kernel, linux1394-devel

Linus,

please pull from the tag "firewire-fix-vt6315" at

    git://git.kernel.org/pub/scm/linux/kernel/git/ieee1394/linux1394.git firewire-fix-vt6315

to receive an IEEE 1394 (FireWire) subsystem fix:  MSI don't work on
VIA PCIe controllers with some isochronous workloads (regression since
v3.16-rc1).

Stefan Richter (1):
      firewire: ohci: disable MSI for VIA VT6315 again

 drivers/firewire/ohci.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Thank you,
-- 
Stefan Richter
-=====-====- -=== ==-=-
http://arcgraph.de/sr/

^ permalink raw reply	[flat|nested] 4+ messages in thread
* [git pull] FireWire regression fix
@ 2013-07-29 20:33 Stefan Richter
  0 siblings, 0 replies; 4+ messages in thread
From: Stefan Richter @ 2013-07-29 20:33 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: linux-kernel, linux1394-devel


Linus,

please pull from the tag "firewire-fix" at

    git://git.kernel.org/pub/scm/linux/kernel/git/ieee1394/linux1394.git firewire-fix

This fixes corrupted video capture, seen with IIDC/DCAM video and certain
buffer settings.  (Regression since v3.4 inclusive.)
Thanks.

Clemens Ladisch (1):
      firewire: fix libdc1394/FlyCap2 iso event regression

 drivers/firewire/core-cdev.c       |  3 +++
 drivers/firewire/ohci.c            | 10 ++++++++--
 include/linux/firewire.h           |  1 +
 include/uapi/linux/firewire-cdev.h |  4 ++--
 4 files changed, 14 insertions(+), 4 deletions(-)


commit 0699a73af3811b66b1ab5650575acee5eea841ab
Author: Clemens Ladisch <clemens@ladisch.de>
Date:   Mon Jul 22 21:32:09 2013 +0200

    firewire: fix libdc1394/FlyCap2 iso event regression
    
    Commit 18d627113b83 (firewire: prevent dropping of completed iso packet
    header data) was intended to be an obvious bug fix, but libdc1394 and
    FlyCap2 depend on the old behaviour by ignoring all returned information
    and thus not noticing that not all packets have been received yet.  The
    result was that the video frame buffers would be saved before they
    contained the correct data.
    
    Reintroduce the old behaviour for old clients.
    
    Tested-by: Stepan Salenikovich <stepan.salenikovich@gmail.com>
    Tested-by: Josep Bosch <jep250@gmail.com>
    Cc: <stable@vger.kernel.org> # 3.4+
    Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
    Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>

diff --git a/drivers/firewire/core-cdev.c b/drivers/firewire/core-cdev.c
index 7ef316f..ac1b43a 100644
--- a/drivers/firewire/core-cdev.c
+++ b/drivers/firewire/core-cdev.c
@@ -54,6 +54,7 @@
 #define FW_CDEV_KERNEL_VERSION			5
 #define FW_CDEV_VERSION_EVENT_REQUEST2		4
 #define FW_CDEV_VERSION_ALLOCATE_REGION_END	4
+#define FW_CDEV_VERSION_AUTO_FLUSH_ISO_OVERFLOW	5
 
 struct client {
 	u32 version;
@@ -1005,6 +1006,8 @@ static int ioctl_create_iso_context(struct client *client, union ioctl_arg *arg)
 			a->channel, a->speed, a->header_size, cb, client);
 	if (IS_ERR(context))
 		return PTR_ERR(context);
+	if (client->version < FW_CDEV_VERSION_AUTO_FLUSH_ISO_OVERFLOW)
+		context->drop_overflow_headers = true;
 
 	/* We only support one context at this time. */
 	spin_lock_irq(&client->lock);
diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c
index 9e1db64..afb701e 100644
--- a/drivers/firewire/ohci.c
+++ b/drivers/firewire/ohci.c
@@ -2749,8 +2749,11 @@ static void copy_iso_headers(struct iso_context *ctx, const u32 *dma_hdr)
 {
 	u32 *ctx_hdr;
 
-	if (ctx->header_length + ctx->base.header_size > PAGE_SIZE)
+	if (ctx->header_length + ctx->base.header_size > PAGE_SIZE) {
+		if (ctx->base.drop_overflow_headers)
+			return;
 		flush_iso_completions(ctx);
+	}
 
 	ctx_hdr = ctx->header + ctx->header_length;
 	ctx->last_timestamp = (u16)le32_to_cpu((__force __le32)dma_hdr[0]);
@@ -2910,8 +2913,11 @@ static int handle_it_packet(struct context *context,
 
 	sync_it_packet_for_cpu(context, d);
 
-	if (ctx->header_length + 4 > PAGE_SIZE)
+	if (ctx->header_length + 4 > PAGE_SIZE) {
+		if (ctx->base.drop_overflow_headers)
+			return 1;
 		flush_iso_completions(ctx);
+	}
 
 	ctx_hdr = ctx->header + ctx->header_length;
 	ctx->last_timestamp = le16_to_cpu(last->res_count);
diff --git a/include/linux/firewire.h b/include/linux/firewire.h
index 3b0e820..5d7782e 100644
--- a/include/linux/firewire.h
+++ b/include/linux/firewire.h
@@ -436,6 +436,7 @@ struct fw_iso_context {
 	int type;
 	int channel;
 	int speed;
+	bool drop_overflow_headers;
 	size_t header_size;
 	union {
 		fw_iso_callback_t sc;
diff --git a/include/uapi/linux/firewire-cdev.h b/include/uapi/linux/firewire-cdev.h
index d500369..1db453e 100644
--- a/include/uapi/linux/firewire-cdev.h
+++ b/include/uapi/linux/firewire-cdev.h
@@ -215,8 +215,8 @@ struct fw_cdev_event_request2 {
  * with the %FW_CDEV_ISO_INTERRUPT bit set, when explicitly requested with
  * %FW_CDEV_IOC_FLUSH_ISO, or when there have been so many completed packets
  * without the interrupt bit set that the kernel's internal buffer for @header
- * is about to overflow.  (In the last case, kernels with ABI version < 5 drop
- * header data up to the next interrupt packet.)
+ * is about to overflow.  (In the last case, ABI versions < 5 drop header data
+ * up to the next interrupt packet.)
  *
  * Isochronous transmit events (context type %FW_CDEV_ISO_CONTEXT_TRANSMIT):
  *

-- 
Stefan Richter
-=====-===-= -=== ===-=
http://arcgraph.de/sr/

^ permalink raw reply related	[flat|nested] 4+ messages in thread
* [git pull] FireWire regression fix
@ 2008-08-06 18:47 Stefan Richter
  0 siblings, 0 replies; 4+ messages in thread
From: Stefan Richter @ 2008-08-06 18:47 UTC (permalink / raw)
  To: Linus Torvalds, Andrew Morton; +Cc: linux-kernel, linux1394-devel

Linus, please pull from the for-linus branch at

    git://git.kernel.org/pub/scm/linux/kernel/git/ieee1394/linux1394-2.6.git for-linus

to receive a FireWire subsystem fix-for-a-fix.

David Moore (1):
      firewire: Preserve response data alignment bug when it is harmless

 drivers/firewire/fw-cdev.c |   29 ++++++++++++++++++++---------
 1 files changed, 20 insertions(+), 9 deletions(-)



commit 8401d92ba46a1e859464cbd9c9ee304f6e361da3
Author: David Moore <dcm@acm.org>
Date:   Tue Jul 29 23:46:25 2008 -0700

    firewire: Preserve response data alignment bug when it is harmless
    
    Recently, a bug having to do with the alignment of transaction response
    data was fixed.  However, some apps such as libdc1394 relied on the
    presence of that bug in order to function correctly.  In order to stay
    compatible with old versions of those apps, this patch preserves the bug
    in cases where it is harmless to normal operation (such as the single
    quadlet read) due to a simple duplication of data.  This guarantees
    maximum compatability for those users who are using the old app with the
    fixed kernel.
    
    Signed-off-by: David Moore <dcm@acm.org>
    Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>

diff --git a/drivers/firewire/fw-cdev.c b/drivers/firewire/fw-cdev.c
index bc81d6f..2e6d584 100644
--- a/drivers/firewire/fw-cdev.c
+++ b/drivers/firewire/fw-cdev.c
@@ -369,22 +369,33 @@ complete_transaction(struct fw_card *card, int rcode,
 	struct response *response = data;
 	struct client *client = response->client;
 	unsigned long flags;
+	struct fw_cdev_event_response *r = &response->response;
 
-	if (length < response->response.length)
-		response->response.length = length;
+	if (length < r->length)
+		r->length = length;
 	if (rcode == RCODE_COMPLETE)
-		memcpy(response->response.data, payload,
-		       response->response.length);
+		memcpy(r->data, payload, r->length);
 
 	spin_lock_irqsave(&client->lock, flags);
 	list_del(&response->resource.link);
 	spin_unlock_irqrestore(&client->lock, flags);
 
-	response->response.type   = FW_CDEV_EVENT_RESPONSE;
-	response->response.rcode  = rcode;
-	queue_event(client, &response->event, &response->response,
-		    sizeof(response->response) + response->response.length,
-		    NULL, 0);
+	r->type   = FW_CDEV_EVENT_RESPONSE;
+	r->rcode  = rcode;
+
+	/*
+	 * In the case that sizeof(*r) doesn't align with the position of the
+	 * data, and the read is short, preserve an extra copy of the data
+	 * to stay compatible with a pre-2.6.27 bug.  Since the bug is harmless
+	 * for short reads and some apps depended on it, this is both safe
+	 * and prudent for compatibility.
+	 */
+	if (r->length <= sizeof(*r) - offsetof(typeof(*r), data))
+		queue_event(client, &response->event, r, sizeof(*r),
+			    r->data, r->length);
+	else
+		queue_event(client, &response->event, r, sizeof(*r) + r->length,
+			    NULL, 0);
 }
 
 static int ioctl_send_request(struct client *client, void *buffer)


Thanks,
-- 
Stefan Richter
-=====-==--- =--- --==-
http://arcgraph.de/sr/


^ permalink raw reply related	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2014-07-26  8:48 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-05-04 11:45 [git pull] FireWire regression fix Stefan Richter
  -- strict thread matches above, loose matches on Subject: below --
2014-07-26  8:48 Stefan Richter
2013-07-29 20:33 Stefan Richter
2008-08-06 18:47 Stefan Richter

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox