[PATCH 0/8] firewire: misc updates

[PATCH 0/8] firewire: misc updates

[Stefan Richter]

Coming up as replies:  Several cleanups, one fix for a bug that went
unnoticed for an astonishingly long time, and a feature that two or so
people had asked for a while ago (a parameter for firewire-ohci similar
to firewire-sbp2.workarounds).

[PATCH 1/8] firewire: core: change type of a data buffer
[PATCH 2/8] firewire: core: combine a bit of repeated code
[PATCH 3/8] firewire: ohci: remove unused dualbuffer IR code
[PATCH 4/8] firewire: ohci: reorder struct fw_ohci for better cache efficiency
[PATCH 5/8] firewire: ohci: use an ID table for quirks detection
[PATCH 6/8] firewire: ohci: add module parameter to activate quirk fixes
[PATCH 7/8] firewire: ohci: fix IR/IT context mask mixup
[PATCH 8/8] firewire: ohci: extend initialization log message

 drivers/firewire/core-cdev.c |  342 ++++++++++++++++-------------------
 drivers/firewire/ohci.c      |  289 ++++++-----------------------
 include/linux/pci_ids.h      |    1 
 3 files changed, 223 insertions(+), 409 deletions(-)
-- 
Stefan Richter
-=====-==-=- --=- =-=-=
http://arcgraph.de/sr/

[PATCH 1/8] firewire: core: change type of a data buffer

[Stefan Richter]

from array of char to union of structs.  I already used a union to size
the buffer which holds ioctl arguments; more consequent is to define it
as an instance of this union in the first place.

Also rename several local variables from "request" to "a"(rgument) since
the term request can be mistaken to mean a transaction subaction, e.g.
an instance of struct fw_request.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
---
 drivers/firewire/core-cdev.c |  325 ++++++++++++++++-------------------
 1 file changed, 152 insertions(+), 173 deletions(-)

Index: b/drivers/firewire/core-cdev.c
===================================================================
--- a/drivers/firewire/core-cdev.c
+++ b/drivers/firewire/core-cdev.c
@@ -368,39 +368,56 @@ void fw_device_cdev_remove(struct fw_dev
 	for_each_client(device, wake_up_client);
 }
 
-static int ioctl_get_info(struct client *client, void *buffer)
+union ioctl_arg {
+	struct fw_cdev_get_info			get_info;
+	struct fw_cdev_send_request		send_request;
+	struct fw_cdev_allocate			allocate;
+	struct fw_cdev_deallocate		deallocate;
+	struct fw_cdev_send_response		send_response;
+	struct fw_cdev_initiate_bus_reset	initiate_bus_reset;
+	struct fw_cdev_add_descriptor		add_descriptor;
+	struct fw_cdev_remove_descriptor	remove_descriptor;
+	struct fw_cdev_create_iso_context	create_iso_context;
+	struct fw_cdev_queue_iso		queue_iso;
+	struct fw_cdev_start_iso		start_iso;
+	struct fw_cdev_stop_iso			stop_iso;
+	struct fw_cdev_get_cycle_timer		get_cycle_timer;
+	struct fw_cdev_allocate_iso_resource	allocate_iso_resource;
+	struct fw_cdev_send_stream_packet	send_stream_packet;
+	struct fw_cdev_get_cycle_timer2		get_cycle_timer2;
+};
+
+static int ioctl_get_info(struct client *client, union ioctl_arg *arg)
 {
-	struct fw_cdev_get_info *get_info = buffer;
+	struct fw_cdev_get_info *a = &arg->get_info;
 	struct fw_cdev_event_bus_reset bus_reset;
 	unsigned long ret = 0;
 
-	client->version = get_info->version;
-	get_info->version = FW_CDEV_VERSION;
-	get_info->card = client->device->card->index;
+	client->version = a->version;
+	a->version = FW_CDEV_VERSION;
+	a->card = client->device->card->index;
 
 	down_read(&fw_device_rwsem);
 
-	if (get_info->rom != 0) {
-		void __user *uptr = u64_to_uptr(get_info->rom);
-		size_t want = get_info->rom_length;
+	if (a->rom != 0) {
+		size_t want = a->rom_length;
 		size_t have = client->device->config_rom_length * 4;
 
-		ret = copy_to_user(uptr, client->device->config_rom,
-				   min(want, have));
+		ret = copy_to_user(u64_to_uptr(a->rom),
+				   client->device->config_rom, min(want, have));
 	}
-	get_info->rom_length = client->device->config_rom_length * 4;
+	a->rom_length = client->device->config_rom_length * 4;
 
 	up_read(&fw_device_rwsem);
 
 	if (ret != 0)
 		return -EFAULT;
 
-	client->bus_reset_closure = get_info->bus_reset_closure;
-	if (get_info->bus_reset != 0) {
-		void __user *uptr = u64_to_uptr(get_info->bus_reset);
-
+	client->bus_reset_closure = a->bus_reset_closure;
+	if (a->bus_reset != 0) {
 		fill_bus_reset_event(&bus_reset, client);
-		if (copy_to_user(uptr, &bus_reset, sizeof(bus_reset)))
+		if (copy_to_user(u64_to_uptr(a->bus_reset),
+				 &bus_reset, sizeof(bus_reset)))
 			return -EFAULT;
 	}
 
@@ -571,11 +588,9 @@ static int init_request(struct client *c
 	return ret;
 }
 
-static int ioctl_send_request(struct client *client, void *buffer)
+static int ioctl_send_request(struct client *client, union ioctl_arg *arg)
 {
-	struct fw_cdev_send_request *request = buffer;
-
-	switch (request->tcode) {
+	switch (arg->send_request.tcode) {
 	case TCODE_WRITE_QUADLET_REQUEST:
 	case TCODE_WRITE_BLOCK_REQUEST:
 	case TCODE_READ_QUADLET_REQUEST:
@@ -592,7 +607,7 @@ static int ioctl_send_request(struct cli
 		return -EINVAL;
 	}
 
-	return init_request(client, request, client->device->node_id,
+	return init_request(client, &arg->send_request, client->device->node_id,
 			    client->device->max_speed);
 }
 
@@ -683,9 +698,9 @@ static void release_address_handler(stru
 	kfree(r);
 }
 
-static int ioctl_allocate(struct client *client, void *buffer)
+static int ioctl_allocate(struct client *client, union ioctl_arg *arg)
 {
-	struct fw_cdev_allocate *request = buffer;
+	struct fw_cdev_allocate *a = &arg->allocate;
 	struct address_handler_resource *r;
 	struct fw_address_region region;
 	int ret;
@@ -694,13 +709,13 @@ static int ioctl_allocate(struct client 
 	if (r == NULL)
 		return -ENOMEM;
 
-	region.start = request->offset;
-	region.end = request->offset + request->length;
-	r->handler.length = request->length;
+	region.start = a->offset;
+	region.end   = a->offset + a->length;
+	r->handler.length           = a->length;
 	r->handler.address_callback = handle_request;
-	r->handler.callback_data = r;
-	r->closure = request->closure;
-	r->client = client;
+	r->handler.callback_data    = r;
+	r->closure   = a->closure;
+	r->client    = client;
 
 	ret = fw_core_add_address_handler(&r->handler, &region);
 	if (ret < 0) {
@@ -714,27 +729,25 @@ static int ioctl_allocate(struct client 
 		release_address_handler(client, &r->resource);
 		return ret;
 	}
-	request->handle = r->resource.handle;
+	a->handle = r->resource.handle;
 
 	return 0;
 }
 
-static int ioctl_deallocate(struct client *client, void *buffer)
+static int ioctl_deallocate(struct client *client, union ioctl_arg *arg)
 {
-	struct fw_cdev_deallocate *request = buffer;
-
-	return release_client_resource(client, request->handle,
+	return release_client_resource(client, arg->deallocate.handle,
 				       release_address_handler, NULL);
 }
 
-static int ioctl_send_response(struct client *client, void *buffer)
+static int ioctl_send_response(struct client *client, union ioctl_arg *arg)
 {
-	struct fw_cdev_send_response *request = buffer;
+	struct fw_cdev_send_response *a = &arg->send_response;
 	struct client_resource *resource;
 	struct inbound_transaction_resource *r;
 	int ret = 0;
 
-	if (release_client_resource(client, request->handle,
+	if (release_client_resource(client, a->handle,
 				    release_request, &resource) < 0)
 		return -EINVAL;
 
@@ -743,28 +756,24 @@ static int ioctl_send_response(struct cl
 	if (is_fcp_request(r->request))
 		goto out;
 
-	if (request->length < r->length)
-		r->length = request->length;
-	if (copy_from_user(r->data, u64_to_uptr(request->data), r->length)) {
+	if (a->length < r->length)
+		r->length = a->length;
+	if (copy_from_user(r->data, u64_to_uptr(a->data), r->length)) {
 		ret = -EFAULT;
 		kfree(r->request);
 		goto out;
 	}
-	fw_send_response(client->device->card, r->request, request->rcode);
+	fw_send_response(client->device->card, r->request, a->rcode);
  out:
 	kfree(r);
 
 	return ret;
 }
 
-static int ioctl_initiate_bus_reset(struct client *client, void *buffer)
+static int ioctl_initiate_bus_reset(struct client *client, union ioctl_arg *arg)
 {
-	struct fw_cdev_initiate_bus_reset *request = buffer;
-	int short_reset;
-
-	short_reset = (request->type == FW_CDEV_SHORT_RESET);
-
-	return fw_core_initiate_bus_reset(client->device->card, short_reset);
+	return fw_core_initiate_bus_reset(client->device->card,
+			arg->initiate_bus_reset.type == FW_CDEV_SHORT_RESET);
 }
 
 static void release_descriptor(struct client *client,
@@ -777,9 +786,9 @@ static void release_descriptor(struct cl
 	kfree(r);
 }
 
-static int ioctl_add_descriptor(struct client *client, void *buffer)
+static int ioctl_add_descriptor(struct client *client, union ioctl_arg *arg)
 {
-	struct fw_cdev_add_descriptor *request = buffer;
+	struct fw_cdev_add_descriptor *a = &arg->add_descriptor;
 	struct descriptor_resource *r;
 	int ret;
 
@@ -787,22 +796,21 @@ static int ioctl_add_descriptor(struct c
 	if (!client->device->is_local)
 		return -ENOSYS;
 
-	if (request->length > 256)
+	if (a->length > 256)
 		return -EINVAL;
 
-	r = kmalloc(sizeof(*r) + request->length * 4, GFP_KERNEL);
+	r = kmalloc(sizeof(*r) + a->length * 4, GFP_KERNEL);
 	if (r == NULL)
 		return -ENOMEM;
 
-	if (copy_from_user(r->data,
-			   u64_to_uptr(request->data), request->length * 4)) {
+	if (copy_from_user(r->data, u64_to_uptr(a->data), a->length * 4)) {
 		ret = -EFAULT;
 		goto failed;
 	}
 
-	r->descriptor.length    = request->length;
-	r->descriptor.immediate = request->immediate;
-	r->descriptor.key       = request->key;
+	r->descriptor.length    = a->length;
+	r->descriptor.immediate = a->immediate;
+	r->descriptor.key       = a->key;
 	r->descriptor.data      = r->data;
 
 	ret = fw_core_add_descriptor(&r->descriptor);
@@ -815,7 +823,7 @@ static int ioctl_add_descriptor(struct c
 		fw_core_remove_descriptor(&r->descriptor);
 		goto failed;
 	}
-	request->handle = r->resource.handle;
+	a->handle = r->resource.handle;
 
 	return 0;
  failed:
@@ -824,11 +832,9 @@ static int ioctl_add_descriptor(struct c
 	return ret;
 }
 
-static int ioctl_remove_descriptor(struct client *client, void *buffer)
+static int ioctl_remove_descriptor(struct client *client, union ioctl_arg *arg)
 {
-	struct fw_cdev_remove_descriptor *request = buffer;
-
-	return release_client_resource(client, request->handle,
+	return release_client_resource(client, arg->remove_descriptor.handle,
 				       release_descriptor, NULL);
 }
 
@@ -851,49 +857,44 @@ static void iso_callback(struct fw_iso_c
 		    sizeof(e->interrupt) + header_length, NULL, 0);
 }
 
-static int ioctl_create_iso_context(struct client *client, void *buffer)
+static int ioctl_create_iso_context(struct client *client, union ioctl_arg *arg)
 {
-	struct fw_cdev_create_iso_context *request = buffer;
+	struct fw_cdev_create_iso_context *a = &arg->create_iso_context;
 	struct fw_iso_context *context;
 
 	/* We only support one context at this time. */
 	if (client->iso_context != NULL)
 		return -EBUSY;
 
-	if (request->channel > 63)
+	if (a->channel > 63)
 		return -EINVAL;
 
-	switch (request->type) {
+	switch (a->type) {
 	case FW_ISO_CONTEXT_RECEIVE:
-		if (request->header_size < 4 || (request->header_size & 3))
+		if (a->header_size < 4 || (a->header_size & 3))
 			return -EINVAL;
-
 		break;
 
 	case FW_ISO_CONTEXT_TRANSMIT:
-		if (request->speed > SCODE_3200)
+		if (a->speed > SCODE_3200)
 			return -EINVAL;
-
 		break;
 
 	default:
 		return -EINVAL;
 	}
 
-	context =  fw_iso_context_create(client->device->card,
-					 request->type,
-					 request->channel,
-					 request->speed,
-					 request->header_size,
-					 iso_callback, client);
+	context = fw_iso_context_create(client->device->card, a->type,
+					a->channel, a->speed, a->header_size,
+					iso_callback, client);
 	if (IS_ERR(context))
 		return PTR_ERR(context);
 
-	client->iso_closure = request->closure;
+	client->iso_closure = a->closure;
 	client->iso_context = context;
 
 	/* We only support one context at this time. */
-	request->handle = 0;
+	a->handle = 0;
 
 	return 0;
 }
@@ -906,9 +907,9 @@ static int ioctl_create_iso_context(stru
 #define GET_SY(v)		(((v) >> 20) & 0x0f)
 #define GET_HEADER_LENGTH(v)	(((v) >> 24) & 0xff)
 
-static int ioctl_queue_iso(struct client *client, void *buffer)
+static int ioctl_queue_iso(struct client *client, union ioctl_arg *arg)
 {
-	struct fw_cdev_queue_iso *request = buffer;
+	struct fw_cdev_queue_iso *a = &arg->queue_iso;
 	struct fw_cdev_iso_packet __user *p, *end, *next;
 	struct fw_iso_context *ctx = client->iso_context;
 	unsigned long payload, buffer_end, header_length;
@@ -919,7 +920,7 @@ static int ioctl_queue_iso(struct client
 		u8 header[256];
 	} u;
 
-	if (ctx == NULL || request->handle != 0)
+	if (ctx == NULL || a->handle != 0)
 		return -EINVAL;
 
 	/*
@@ -929,23 +930,23 @@ static int ioctl_queue_iso(struct client
 	 * set them both to 0, which will still let packets with
 	 * payload_length == 0 through.  In other words, if no packets
 	 * use the indirect payload, the iso buffer need not be mapped
-	 * and the request->data pointer is ignored.
+	 * and the a->data pointer is ignored.
 	 */
 
-	payload = (unsigned long)request->data - client->vm_start;
+	payload = (unsigned long)a->data - client->vm_start;
 	buffer_end = client->buffer.page_count << PAGE_SHIFT;
-	if (request->data == 0 || client->buffer.pages == NULL ||
+	if (a->data == 0 || client->buffer.pages == NULL ||
 	    payload >= buffer_end) {
 		payload = 0;
 		buffer_end = 0;
 	}
 
-	p = (struct fw_cdev_iso_packet __user *)u64_to_uptr(request->packets);
+	p = (struct fw_cdev_iso_packet __user *)u64_to_uptr(a->packets);
 
-	if (!access_ok(VERIFY_READ, p, request->size))
+	if (!access_ok(VERIFY_READ, p, a->size))
 		return -EFAULT;
 
-	end = (void __user *)p + request->size;
+	end = (void __user *)p + a->size;
 	count = 0;
 	while (p < end) {
 		if (get_user(control, &p->control))
@@ -995,45 +996,41 @@ static int ioctl_queue_iso(struct client
 		count++;
 	}
 
-	request->size    -= uptr_to_u64(p) - request->packets;
-	request->packets  = uptr_to_u64(p);
-	request->data     = client->vm_start + payload;
+	a->size    -= uptr_to_u64(p) - a->packets;
+	a->packets  = uptr_to_u64(p);
+	a->data     = client->vm_start + payload;
 
 	return count;
 }
 
-static int ioctl_start_iso(struct client *client, void *buffer)
+static int ioctl_start_iso(struct client *client, union ioctl_arg *arg)
 {
-	struct fw_cdev_start_iso *request = buffer;
+	struct fw_cdev_start_iso *a = &arg->start_iso;
 
-	if (client->iso_context == NULL || request->handle != 0)
+	if (client->iso_context == NULL || a->handle != 0)
 		return -EINVAL;
 
-	if (client->iso_context->type == FW_ISO_CONTEXT_RECEIVE) {
-		if (request->tags == 0 || request->tags > 15)
-			return -EINVAL;
-
-		if (request->sync > 15)
-			return -EINVAL;
-	}
+	if (client->iso_context->type == FW_ISO_CONTEXT_RECEIVE &&
+	    (a->tags == 0 || a->tags > 15 || a->sync > 15))
+		return -EINVAL;
 
-	return fw_iso_context_start(client->iso_context, request->cycle,
-				    request->sync, request->tags);
+	return fw_iso_context_start(client->iso_context,
+				    a->cycle, a->sync, a->tags);
 }
 
-static int ioctl_stop_iso(struct client *client, void *buffer)
+static int ioctl_stop_iso(struct client *client, union ioctl_arg *arg)
 {
-	struct fw_cdev_stop_iso *request = buffer;
+	struct fw_cdev_stop_iso *a = &arg->stop_iso;
 
-	if (client->iso_context == NULL || request->handle != 0)
+	if (client->iso_context == NULL || a->handle != 0)
 		return -EINVAL;
 
 	return fw_iso_context_stop(client->iso_context);
 }
 
-static int ioctl_get_cycle_timer2(struct client *client, void *buffer)
+static int ioctl_get_cycle_timer2(struct client *client, union ioctl_arg *arg)
 {
-	struct fw_cdev_get_cycle_timer2 *request = buffer;
+	struct fw_cdev_get_cycle_timer2 *a = &arg->get_cycle_timer2;
 	struct fw_card *card = client->device->card;
 	struct timespec ts = {0, 0};
 	u32 cycle_time;
@@ -1043,7 +1040,7 @@ static int ioctl_get_cycle_timer2(struct
 
 	cycle_time = card->driver->get_cycle_time(card);
 
-	switch (request->clk_id) {
+	switch (a->clk_id) {
 	case CLOCK_REALTIME:      getnstimeofday(&ts);                   break;
 	case CLOCK_MONOTONIC:     do_posix_clock_monotonic_gettime(&ts); break;
 	case CLOCK_MONOTONIC_RAW: getrawmonotonic(&ts);                  break;
@@ -1053,24 +1050,23 @@ static int ioctl_get_cycle_timer2(struct
 
 	local_irq_enable();
 
-	request->tv_sec		= ts.tv_sec;
-	request->tv_nsec	= ts.tv_nsec;
-	request->cycle_timer	= cycle_time;
+	a->tv_sec      = ts.tv_sec;
+	a->tv_nsec     = ts.tv_nsec;
+	a->cycle_timer = cycle_time;
 
 	return ret;
 }
 
-static int ioctl_get_cycle_timer(struct client *client, void *buffer)
+static int ioctl_get_cycle_timer(struct client *client, union ioctl_arg *arg)
 {
-	struct fw_cdev_get_cycle_timer *request = buffer;
+	struct fw_cdev_get_cycle_timer *a = &arg->get_cycle_timer;
 	struct fw_cdev_get_cycle_timer2 ct2;
 
 	ct2.clk_id = CLOCK_REALTIME;
-	ioctl_get_cycle_timer2(client, &ct2);
+	ioctl_get_cycle_timer2(client, (union ioctl_arg *)&ct2);
 
-	request->local_time	= ct2.tv_sec * USEC_PER_SEC +
-				  ct2.tv_nsec / NSEC_PER_USEC;
-	request->cycle_timer	= ct2.cycle_timer;
+	a->local_time = ct2.tv_sec * USEC_PER_SEC + ct2.tv_nsec / NSEC_PER_USEC;
+	a->cycle_timer = ct2.cycle_timer;
 
 	return 0;
 }
@@ -1242,33 +1238,32 @@ static int init_iso_resource(struct clie
 	return ret;
 }
 
-static int ioctl_allocate_iso_resource(struct client *client, void *buffer)
+static int ioctl_allocate_iso_resource(struct client *client,
+				       union ioctl_arg *arg)
 {
-	struct fw_cdev_allocate_iso_resource *request = buffer;
-
-	return init_iso_resource(client, request, ISO_RES_ALLOC);
+	return init_iso_resource(client,
+			&arg->allocate_iso_resource, ISO_RES_ALLOC);
 }
 
-static int ioctl_deallocate_iso_resource(struct client *client, void *buffer)
+static int ioctl_deallocate_iso_resource(struct client *client,
+					 union ioctl_arg *arg)
 {
-	struct fw_cdev_deallocate *request = buffer;
-
-	return release_client_resource(client, request->handle,
-				       release_iso_resource, NULL);
+	return release_client_resource(client,
+			arg->deallocate.handle, release_iso_resource, NULL);
 }
 
-static int ioctl_allocate_iso_resource_once(struct client *client, void *buffer)
+static int ioctl_allocate_iso_resource_once(struct client *client,
+					    union ioctl_arg *arg)
 {
-	struct fw_cdev_allocate_iso_resource *request = buffer;
-
-	return init_iso_resource(client, request, ISO_RES_ALLOC_ONCE);
+	return init_iso_resource(client,
+			&arg->allocate_iso_resource, ISO_RES_ALLOC_ONCE);
 }
 
-static int ioctl_deallocate_iso_resource_once(struct client *client, void *buffer)
+static int ioctl_deallocate_iso_resource_once(struct client *client,
+					      union ioctl_arg *arg)
 {
-	struct fw_cdev_allocate_iso_resource *request = buffer;
-
-	return init_iso_resource(client, request, ISO_RES_DEALLOC_ONCE);
+	return init_iso_resource(client,
+			&arg->allocate_iso_resource, ISO_RES_DEALLOC_ONCE);
 }
 
 /*
@@ -1276,16 +1271,17 @@ static int ioctl_deallocate_iso_resource
  * limited by the device's link speed, the local node's link speed,
  * and all PHY port speeds between the two links.
  */
-static int ioctl_get_speed(struct client *client, void *buffer)
+static int ioctl_get_speed(struct client *client, union ioctl_arg *arg)
 {
 	return client->device->max_speed;
 }
 
-static int ioctl_send_broadcast_request(struct client *client, void *buffer)
+static int ioctl_send_broadcast_request(struct client *client,
+					union ioctl_arg *arg)
 {
-	struct fw_cdev_send_request *request = buffer;
+	struct fw_cdev_send_request *a = &arg->send_request;
 
-	switch (request->tcode) {
+	switch (a->tcode) {
 	case TCODE_WRITE_QUADLET_REQUEST:
 	case TCODE_WRITE_BLOCK_REQUEST:
 		break;
@@ -1294,36 +1290,36 @@ static int ioctl_send_broadcast_request(
 	}
 
 	/* Security policy: Only allow accesses to Units Space. */
-	if (request->offset < CSR_REGISTER_BASE + CSR_CONFIG_ROM_END)
+	if (a->offset < CSR_REGISTER_BASE + CSR_CONFIG_ROM_END)
 		return -EACCES;
 
-	return init_request(client, request, LOCAL_BUS | 0x3f, SCODE_100);
+	return init_request(client, a, LOCAL_BUS | 0x3f, SCODE_100);
 }
 
-static int ioctl_send_stream_packet(struct client *client, void *buffer)
+static int ioctl_send_stream_packet(struct client *client, union ioctl_arg *arg)
 {
-	struct fw_cdev_send_stream_packet *p = buffer;
+	struct fw_cdev_send_stream_packet *a = &arg->send_stream_packet;
 	struct fw_cdev_send_request request;
 	int dest;
 
-	if (p->speed > client->device->card->link_speed ||
-	    p->length > 1024 << p->speed)
+	if (a->speed > client->device->card->link_speed ||
+	    a->length > 1024 << a->speed)
 		return -EIO;
 
-	if (p->tag > 3 || p->channel > 63 || p->sy > 15)
+	if (a->tag > 3 || a->channel > 63 || a->sy > 15)
 		return -EINVAL;
 
-	dest = fw_stream_packet_destination_id(p->tag, p->channel, p->sy);
+	dest = fw_stream_packet_destination_id(a->tag, a->channel, a->sy);
 	request.tcode		= TCODE_STREAM_DATA;
-	request.length		= p->length;
-	request.closure		= p->closure;
-	request.data		= p->data;
-	request.generation	= p->generation;
+	request.length		= a->length;
+	request.closure		= a->closure;
+	request.data		= a->data;
+	request.generation	= a->generation;
 
-	return init_request(client, &request, dest, p->speed);
+	return init_request(client, &request, dest, a->speed);
 }
 
-static int (* const ioctl_handlers[])(struct client *client, void *buffer) = {
+static int (* const ioctl_handlers[])(struct client *, union ioctl_arg *) = {
 	ioctl_get_info,
 	ioctl_send_request,
 	ioctl_allocate,
@@ -1350,24 +1346,7 @@ static int (* const ioctl_handlers[])(st
 static int dispatch_ioctl(struct client *client,
 			  unsigned int cmd, void __user *arg)
 {
-	char buffer[sizeof(union {
-		struct fw_cdev_get_info			_00;
-		struct fw_cdev_send_request		_01;
-		struct fw_cdev_allocate			_02;
-		struct fw_cdev_deallocate		_03;
-		struct fw_cdev_send_response		_04;
-		struct fw_cdev_initiate_bus_reset	_05;
-		struct fw_cdev_add_descriptor		_06;
-		struct fw_cdev_remove_descriptor	_07;
-		struct fw_cdev_create_iso_context	_08;
-		struct fw_cdev_queue_iso		_09;
-		struct fw_cdev_start_iso		_0a;
-		struct fw_cdev_stop_iso			_0b;
-		struct fw_cdev_get_cycle_timer		_0c;
-		struct fw_cdev_allocate_iso_resource	_0d;
-		struct fw_cdev_send_stream_packet	_13;
-		struct fw_cdev_get_cycle_timer2		_14;
-	})];
+	union ioctl_arg buffer;
 	int ret;
 
 	if (_IOC_TYPE(cmd) != '#' ||
@@ -1376,17 +1355,17 @@ static int dispatch_ioctl(struct client 
 
 	if (_IOC_DIR(cmd) & _IOC_WRITE) {
 		if (_IOC_SIZE(cmd) > sizeof(buffer) ||
-		    copy_from_user(buffer, arg, _IOC_SIZE(cmd)))
+		    copy_from_user(&buffer, arg, _IOC_SIZE(cmd)))
 			return -EFAULT;
 	}
 
-	ret = ioctl_handlers[_IOC_NR(cmd)](client, buffer);
+	ret = ioctl_handlers[_IOC_NR(cmd)](client, &buffer);
 	if (ret < 0)
 		return ret;
 
 	if (_IOC_DIR(cmd) & _IOC_READ) {
 		if (_IOC_SIZE(cmd) > sizeof(buffer) ||
-		    copy_to_user(arg, buffer, _IOC_SIZE(cmd)))
+		    copy_to_user(arg, &buffer, _IOC_SIZE(cmd)))
 			return -EFAULT;
 	}
 

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

[PATCH 2/8] firewire: core: combine a bit of repeated code

[Stefan Richter]

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
---
 drivers/firewire/core-cdev.c |   17 +++++------------
 1 file changed, 5 insertions(+), 12 deletions(-)

Index: b/drivers/firewire/core-cdev.c
===================================================================
--- a/drivers/firewire/core-cdev.c
+++ b/drivers/firewire/core-cdev.c
@@ -1349,6 +1349,9 @@ static int dispatch_ioctl(struct client 
 	union ioctl_arg buffer;
 	int ret;
 
+	if (fw_device_is_shutdown(client->device))
+		return -ENODEV;
+
 	if (_IOC_TYPE(cmd) != '#' ||
 	    _IOC_NR(cmd) >= ARRAY_SIZE(ioctl_handlers))
 		return -EINVAL;
@@ -1375,24 +1378,14 @@ static int dispatch_ioctl(struct client 
 static long fw_device_op_ioctl(struct file *file,
 			       unsigned int cmd, unsigned long arg)
 {
-	struct client *client = file->private_data;
-
-	if (fw_device_is_shutdown(client->device))
-		return -ENODEV;
-
-	return dispatch_ioctl(client, cmd, (void __user *) arg);
+	return dispatch_ioctl(file->private_data, cmd, (void __user *)arg);
 }
 
 #ifdef CONFIG_COMPAT
 static long fw_device_op_compat_ioctl(struct file *file,
 				      unsigned int cmd, unsigned long arg)
 {
-	struct client *client = file->private_data;
-
-	if (fw_device_is_shutdown(client->device))
-		return -ENODEV;
-
-	return dispatch_ioctl(client, cmd, compat_ptr(arg));
+	return dispatch_ioctl(file->private_data, cmd, compat_ptr(arg));
 }
 #endif
 

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

[PATCH 3/8] firewire: ohci: remove unused dualbuffer IR code

[Stefan Richter]

This code was no longer used since 2.6.33, "firewire: ohci: always use
packet-per-buffer mode for isochronous reception" commit 090699c0.  If
anybody needs this code in the future for special purposes, it can be
brought back in.  But it must not be re-enabled by default; drivers
(kernelspace or userspace drivers) should only get this mode if they
explicitly request it.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
---
 drivers/firewire/ohci.c |  184 ----------------------------------------
 include/linux/pci_ids.h |    1 -
 2 files changed, 1 insertion(+), 184 deletions(-)

Index: b/drivers/firewire/ohci.c
===================================================================
--- a/drivers/firewire/ohci.c
+++ b/drivers/firewire/ohci.c
@@ -72,20 +72,6 @@ struct descriptor {
 	__le16 transfer_status;
 } __attribute__((aligned(16)));
 
-struct db_descriptor {
-	__le16 first_size;
-	__le16 control;
-	__le16 second_req_count;
-	__le16 first_req_count;
-	__le32 branch_address;
-	__le16 second_res_count;
-	__le16 first_res_count;
-	__le32 reserved0;
-	__le32 first_buffer;
-	__le32 second_buffer;
-	__le32 reserved1;
-} __attribute__((aligned(16)));
-
 #define CONTROL_SET(regs)	(regs)
 #define CONTROL_CLEAR(regs)	((regs) + 4)
 #define COMMAND_PTR(regs)	((regs) + 12)
@@ -187,7 +173,6 @@ struct fw_ohci {
 	int generation;
 	int request_generation;	/* for timestamping incoming requests */
 
-	bool use_dualbuffer;
 	bool old_uninorth;
 	bool bus_reset_packet_quirk;
 	bool iso_cycle_timer_quirk;
@@ -1863,52 +1848,6 @@ static void copy_iso_headers(struct iso_
 	ctx->header_length += ctx->base.header_size;
 }
 
-static int handle_ir_dualbuffer_packet(struct context *context,
-				       struct descriptor *d,
-				       struct descriptor *last)
-{
-	struct iso_context *ctx =
-		container_of(context, struct iso_context, context);
-	struct db_descriptor *db = (struct db_descriptor *) d;
-	__le32 *ir_header;
-	size_t header_length;
-	void *p, *end;
-
-	if (db->first_res_count != 0 && db->second_res_count != 0) {
-		if (ctx->excess_bytes <= le16_to_cpu(db->second_req_count)) {
-			/* This descriptor isn't done yet, stop iteration. */
-			return 0;
-		}
-		ctx->excess_bytes -= le16_to_cpu(db->second_req_count);
-	}
-
-	header_length = le16_to_cpu(db->first_req_count) -
-		le16_to_cpu(db->first_res_count);
-
-	p = db + 1;
-	end = p + header_length;
-	while (p < end) {
-		copy_iso_headers(ctx, p);
-		ctx->excess_bytes +=
-			(le32_to_cpu(*(__le32 *)(p + 4)) >> 16) & 0xffff;
-		p += max(ctx->base.header_size, (size_t)8);
-	}
-
-	ctx->excess_bytes -= le16_to_cpu(db->second_req_count) -
-		le16_to_cpu(db->second_res_count);
-
-	if (le16_to_cpu(db->control) & DESCRIPTOR_IRQ_ALWAYS) {
-		ir_header = (__le32 *) (db + 1);
-		ctx->base.callback(&ctx->base,
-				   le32_to_cpu(ir_header[0]) & 0xffff,
-				   ctx->header_length, ctx->header,
-				   ctx->base.callback_data);
-		ctx->header_length = 0;
-	}
-
-	return 1;
-}
-
 static int handle_ir_packet_per_buffer(struct context *context,
 				       struct descriptor *d,
 				       struct descriptor *last)
@@ -1995,10 +1934,7 @@ static struct fw_iso_context *ohci_alloc
 		channels = &ohci->ir_context_channels;
 		mask = &ohci->ir_context_mask;
 		list = ohci->ir_context_list;
-		if (ohci->use_dualbuffer)
-			callback = handle_ir_dualbuffer_packet;
-		else
-			callback = handle_ir_packet_per_buffer;
+		callback = handle_ir_packet_per_buffer;
 	}
 
 	spin_lock_irqsave(&ohci->lock, flags);
@@ -2061,8 +1997,6 @@ static int ohci_start_iso(struct fw_iso_
 	} else {
 		index = ctx - ohci->ir_context_list;
 		control = IR_CONTEXT_ISOCH_HEADER;
-		if (ohci->use_dualbuffer)
-			control |= IR_CONTEXT_DUAL_BUFFER_MODE;
 		match = (tags << 28) | (sync << 8) | ctx->base.channel;
 		if (cycle >= 0) {
 			match |= (cycle & 0x07fff) << 12;
@@ -2223,92 +2157,6 @@ static int ohci_queue_iso_transmit(struc
 	return 0;
 }
 
-static int ohci_queue_iso_receive_dualbuffer(struct fw_iso_context *base,
-					     struct fw_iso_packet *packet,
-					     struct fw_iso_buffer *buffer,
-					     unsigned long payload)
-{
-	struct iso_context *ctx = container_of(base, struct iso_context, base);
-	struct db_descriptor *db = NULL;
-	struct descriptor *d;
-	struct fw_iso_packet *p;
-	dma_addr_t d_bus, page_bus;
-	u32 z, header_z, length, rest;
-	int page, offset, packet_count, header_size;
-
-	/*
-	 * FIXME: Cycle lost behavior should be configurable: lose
-	 * packet, retransmit or terminate..
-	 */
-
-	p = packet;
-	z = 2;
-
-	/*
-	 * The OHCI controller puts the isochronous header and trailer in the
-	 * buffer, so we need at least 8 bytes.
-	 */
-	packet_count = p->header_length / ctx->base.header_size;
-	header_size = packet_count * max(ctx->base.header_size, (size_t)8);
-
-	/* Get header size in number of descriptors. */
-	header_z = DIV_ROUND_UP(header_size, sizeof(*d));
-	page     = payload >> PAGE_SHIFT;
-	offset   = payload & ~PAGE_MASK;
-	rest     = p->payload_length;
-	/*
-	 * The controllers I've tested have not worked correctly when
-	 * second_req_count is zero.  Rather than do something we know won't
-	 * work, return an error
-	 */
-	if (rest == 0)
-		return -EINVAL;
-
-	while (rest > 0) {
-		d = context_get_descriptors(&ctx->context,
-					    z + header_z, &d_bus);
-		if (d == NULL)
-			return -ENOMEM;
-
-		db = (struct db_descriptor *) d;
-		db->control = cpu_to_le16(DESCRIPTOR_STATUS |
-					  DESCRIPTOR_BRANCH_ALWAYS);
-		db->first_size =
-		    cpu_to_le16(max(ctx->base.header_size, (size_t)8));
-		if (p->skip && rest == p->payload_length) {
-			db->control |= cpu_to_le16(DESCRIPTOR_WAIT);
-			db->first_req_count = db->first_size;
-		} else {
-			db->first_req_count = cpu_to_le16(header_size);
-		}
-		db->first_res_count = db->first_req_count;
-		db->first_buffer = cpu_to_le32(d_bus + sizeof(*db));
-
-		if (p->skip && rest == p->payload_length)
-			length = 4;
-		else if (offset + rest < PAGE_SIZE)
-			length = rest;
-		else
-			length = PAGE_SIZE - offset;
-
-		db->second_req_count = cpu_to_le16(length);
-		db->second_res_count = db->second_req_count;
-		page_bus = page_private(buffer->pages[page]);
-		db->second_buffer = cpu_to_le32(page_bus + offset);
-
-		if (p->interrupt && length == rest)
-			db->control |= cpu_to_le16(DESCRIPTOR_IRQ_ALWAYS);
-
-		context_append(&ctx->context, d, z, header_z);
-		offset = (offset + length) & ~PAGE_MASK;
-		rest -= length;
-		if (offset == 0)
-			page++;
-	}
-
-	return 0;
-}
-
 static int ohci_queue_iso_receive_packet_per_buffer(struct fw_iso_context *base,
 					struct fw_iso_packet *packet,
 					struct fw_iso_buffer *buffer,
@@ -2399,9 +2247,6 @@ static int ohci_queue_iso(struct fw_iso_
 	spin_lock_irqsave(&ctx->context.ohci->lock, flags);
 	if (base->type == FW_ISO_CONTEXT_TRANSMIT)
 		ret = ohci_queue_iso_transmit(base, packet, buffer, payload);
-	else if (ctx->context.ohci->use_dualbuffer)
-		ret = ohci_queue_iso_receive_dualbuffer(base, packet,
-							buffer, payload);
 	else
 		ret = ohci_queue_iso_receive_packet_per_buffer(base, packet,
 							buffer, payload);
@@ -2456,10 +2301,6 @@ static void ohci_pmac_off(struct pci_dev
 #define ohci_pmac_off(dev)
 #endif /* CONFIG_PPC_PMAC */
 
-#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)
 {
@@ -2508,29 +2349,6 @@ static int __devinit pci_probe(struct pc
 	}
 
 	version = reg_read(ohci, OHCI1394_Version) & 0x00ff00ff;
-#if 0
-	/* FIXME: make it a context option or remove dual-buffer mode */
-	ohci->use_dualbuffer = version >= OHCI_VERSION_1_1;
-#endif
-
-	/* dual-buffer mode is broken if more than one IR context is active */
-	if (dev->vendor == PCI_VENDOR_ID_AGERE &&
-	    dev->device == PCI_DEVICE_ID_AGERE_FW643)
-		ohci->use_dualbuffer = false;
-
-	/* dual-buffer mode is broken */
-	if (dev->vendor == PCI_VENDOR_ID_RICOH &&
-	    dev->device == PCI_DEVICE_ID_RICOH_R5C832)
-		ohci->use_dualbuffer = false;
-
-/* x86-32 currently doesn't use highmem for dma_alloc_coherent */
-#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_TSB43AB23))
-		ohci->use_dualbuffer = false;
-#endif
 
 #if defined(CONFIG_PPC_PMAC) && defined(CONFIG_PPC32)
 	ohci->old_uninorth = dev->vendor == PCI_VENDOR_ID_APPLE &&
Index: b/include/linux/pci_ids.h
===================================================================
--- a/include/linux/pci_ids.h
+++ b/include/linux/pci_ids.h
@@ -770,7 +770,6 @@
 #define PCI_VENDOR_ID_TI		0x104c
 #define PCI_DEVICE_ID_TI_TVP4020	0x3d07
 #define PCI_DEVICE_ID_TI_4450		0x8011
-#define PCI_DEVICE_ID_TI_TSB43AB22	0x8023
 #define PCI_DEVICE_ID_TI_XX21_XX11	0x8031
 #define PCI_DEVICE_ID_TI_XX21_XX11_FM	0x8033
 #define PCI_DEVICE_ID_TI_XX21_XX11_SD	0x8034

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

[PATCH 4/8] firewire: ohci: reorder struct fw_ohci for better cache efficiency

[Stefan Richter]

The config_rom struct members are only accessed during relatively
infrequent self-ID-complete interrupts and only if the local config ROM
was changed, while the ar_, at_, ir_, it_ members are used very
frequently during I/O.  Hence move the config_rom members further down.

More importantly, make the huge self_id_buffer member the last one; this
is only accessed in self-ID-complete interrupts.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
---
 drivers/firewire/ohci.c |   23 ++++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

Index: b/drivers/firewire/ohci.c
===================================================================
--- a/drivers/firewire/ohci.c
+++ b/drivers/firewire/ohci.c
@@ -166,9 +166,6 @@ struct fw_ohci {
 	struct fw_card card;
 
 	__iomem char *registers;
-	dma_addr_t self_id_bus;
-	__le32 *self_id_cpu;
-	struct tasklet_struct bus_reset_tasklet;
 	int node_id;
 	int generation;
 	int request_generation;	/* for timestamping incoming requests */
@@ -182,14 +179,6 @@ struct fw_ohci {
 	 * this driver with this lock held.
 	 */
 	spinlock_t lock;
-	u32 self_id_buffer[512];
-
-	/* Config rom buffers */
-	__be32 *config_rom;
-	dma_addr_t config_rom_bus;
-	__be32 *next_config_rom;
-	dma_addr_t next_config_rom_bus;
-	__be32 next_header;
 
 	struct ar_context ar_request_ctx;
 	struct ar_context ar_response_ctx;
@@ -201,6 +190,18 @@ struct fw_ohci {
 	u64 ir_context_channels;
 	u32 ir_context_mask;
 	struct iso_context *ir_context_list;
+
+	__be32    *config_rom;
+	dma_addr_t config_rom_bus;
+	__be32    *next_config_rom;
+	dma_addr_t next_config_rom_bus;
+	__be32     next_header;
+
+	__le32    *self_id_cpu;
+	dma_addr_t self_id_bus;
+	struct tasklet_struct bus_reset_tasklet;
+
+	u32 self_id_buffer[512];
 };
 
 static inline struct fw_ohci *fw_ohci(struct fw_card *card)

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

[PATCH 5/8] firewire: ohci: use an ID table for quirks detection

[Stefan Richter]

We don't have a lot of quirks to take into account (especially since
dual-buffer IR is out of the picture), but still, a table-based approach
is more organized than a series of if () clauses.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
---
 drivers/firewire/ohci.c |   46 ++++++++++++++++++++++++----------------
 1 file changed, 28 insertions(+), 18 deletions(-)

Index: b/drivers/firewire/ohci.c
===================================================================
--- a/drivers/firewire/ohci.c
+++ b/drivers/firewire/ohci.c
@@ -169,10 +169,7 @@ struct fw_ohci {
 	int node_id;
 	int generation;
 	int request_generation;	/* for timestamping incoming requests */
-
-	bool old_uninorth;
-	bool bus_reset_packet_quirk;
-	bool iso_cycle_timer_quirk;
+	unsigned quirks;
 
 	/*
 	 * Spinlock for accessing fw_ohci data.  Never call out of
@@ -234,6 +231,21 @@ static inline struct fw_ohci *fw_ohci(st
 
 static char ohci_driver_name[] = KBUILD_MODNAME;
 
+#define QUIRK_CYCLE_TIMER		1
+#define QUIRK_RESET_PACKET		2
+#define QUIRK_BE_HEADERS		4
+
+/* In case of multiple matches in ohci_quirks[], only the first one is used. */
+static const struct {
+	unsigned short vendor, device, flags;
+} ohci_quirks[] = {
+	{PCI_VENDOR_ID_TI,	PCI_ANY_ID,	QUIRK_RESET_PACKET},
+	{PCI_VENDOR_ID_AL,	PCI_ANY_ID,	QUIRK_CYCLE_TIMER},
+	{PCI_VENDOR_ID_NEC,	PCI_ANY_ID,	QUIRK_CYCLE_TIMER},
+	{PCI_VENDOR_ID_VIA,	PCI_ANY_ID,	QUIRK_CYCLE_TIMER},
+	{PCI_VENDOR_ID_APPLE,	PCI_DEVICE_ID_APPLE_UNI_N_FW, QUIRK_BE_HEADERS},
+};
+
 #ifdef CONFIG_FIREWIRE_OHCI_DEBUG
 
 #define OHCI_PARAM_DEBUG_AT_AR		1
@@ -507,7 +519,7 @@ static void ar_context_release(struct ar
 
 #if defined(CONFIG_PPC_PMAC) && defined(CONFIG_PPC32)
 #define cond_le32_to_cpu(v) \
-	(ohci->old_uninorth ? (__force __u32)(v) : le32_to_cpu(v))
+	(ohci->quirks & QUIRK_BE_HEADERS ? (__force __u32)(v) : le32_to_cpu(v))
 #else
 #define cond_le32_to_cpu(v) le32_to_cpu(v)
 #endif
@@ -588,7 +600,7 @@ static __le32 *handle_ar_packet(struct a
 	 * at a slightly incorrect time (in bus_reset_tasklet).
 	 */
 	if (evt == OHCI1394_evt_bus_reset) {
-		if (!ohci->bus_reset_packet_quirk)
+		if (!(ohci->quirks & QUIRK_RESET_PACKET))
 			ohci->request_generation = (p.header[2] >> 16) & 0xff;
 	} else if (ctx == &ohci->ar_request_ctx) {
 		fw_core_handle_request(&ohci->card, &p);
@@ -1312,7 +1324,7 @@ static void bus_reset_tasklet(unsigned l
 	context_stop(&ohci->at_response_ctx);
 	reg_write(ohci, OHCI1394_IntEventClear, OHCI1394_busReset);
 
-	if (ohci->bus_reset_packet_quirk)
+	if (ohci->quirks & QUIRK_RESET_PACKET)
 		ohci->request_generation = generation;
 
 	/*
@@ -1806,7 +1818,7 @@ static u32 ohci_get_cycle_time(struct fw
 
 	c2 = reg_read(ohci, OHCI1394_IsochronousCycleTimer);
 
-	if (ohci->iso_cycle_timer_quirk) {
+	if (ohci->quirks & QUIRK_CYCLE_TIMER) {
 		i = 0;
 		c1 = c2;
 		c2 = reg_read(ohci, OHCI1394_IsochronousCycleTimer);
@@ -2308,7 +2320,7 @@ static int __devinit pci_probe(struct pc
 	struct fw_ohci *ohci;
 	u32 bus_options, max_receive, link_speed, version;
 	u64 guid;
-	int err;
+	int i, err;
 	size_t size;
 
 	ohci = kzalloc(sizeof(*ohci), GFP_KERNEL);
@@ -2351,15 +2363,13 @@ static int __devinit pci_probe(struct pc
 
 	version = reg_read(ohci, OHCI1394_Version) & 0x00ff00ff;
 
-#if defined(CONFIG_PPC_PMAC) && defined(CONFIG_PPC32)
-	ohci->old_uninorth = dev->vendor == PCI_VENDOR_ID_APPLE &&
-			     dev->device == PCI_DEVICE_ID_APPLE_UNI_N_FW;
-#endif
-	ohci->bus_reset_packet_quirk = dev->vendor == PCI_VENDOR_ID_TI;
-
-	ohci->iso_cycle_timer_quirk = dev->vendor == PCI_VENDOR_ID_AL	||
-				      dev->vendor == PCI_VENDOR_ID_NEC	||
-				      dev->vendor == PCI_VENDOR_ID_VIA;
+	for (i = 0; i < ARRAY_SIZE(ohci_quirks); i++)
+		if (ohci_quirks[i].vendor == dev->vendor &&
+		    (ohci_quirks[i].device == dev->device ||
+		     ohci_quirks[i].device == (unsigned short)PCI_ANY_ID)) {
+			ohci->quirks = ohci_quirks[i].flags;
+			break;
+		}
 
 	ar_context_init(&ohci->ar_request_ctx, ohci,
 			OHCI1394_AsReqRcvContextControlSet);

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

[PATCH 6/8] firewire: ohci: add module parameter to activate quirk fixes

[Stefan Richter]

This way, we can advise users of precompiled kernel packages to test
existing quirk fixes on chips which have not been listed yet, without
them having to build a kernel from source.

Note, to use this feature on a machine with more than one controller,
steps like these are necessary:
# lspci | grep 1394
# ls /sys/bus/pci/drivers/firewire_ohci/
# echo -n "0000:03:02.0" > /sys/bus/pci/drivers/firewire_ohci/unbind
# echo 2 > /sys/module/firewire_ohci/parameters/quirks
# echo -n "0000:03:02.0" > /sys/bus/pci/drivers/firewire_ohci/bind
# echo 0 > /sys/module/firewire_ohci/parameters/quirks

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
---
 drivers/firewire/ohci.c |   11 +++++++++++
 1 file changed, 11 insertions(+)

Index: b/drivers/firewire/ohci.c
===================================================================
--- a/drivers/firewire/ohci.c
+++ b/drivers/firewire/ohci.c
@@ -246,6 +246,15 @@ static const struct {
 	{PCI_VENDOR_ID_APPLE,	PCI_DEVICE_ID_APPLE_UNI_N_FW, QUIRK_BE_HEADERS},
 };
 
+/* This overrides anything that was found in ohci_quirks[]. */
+static int param_quirks;
+module_param_named(quirks, param_quirks, int, 0644);
+MODULE_PARM_DESC(quirks, "Chip quirks (default = 0"
+	", nonatomic cycle timer = "	__stringify(QUIRK_CYCLE_TIMER)
+	", reset packet generation = "	__stringify(QUIRK_RESET_PACKET)
+	", AR/selfID endianess = "	__stringify(QUIRK_BE_HEADERS)
+	")");
+
 #ifdef CONFIG_FIREWIRE_OHCI_DEBUG
 
 #define OHCI_PARAM_DEBUG_AT_AR		1
@@ -2370,6 +2379,8 @@ static int __devinit pci_probe(struct pc
 			ohci->quirks = ohci_quirks[i].flags;
 			break;
 		}
+	if (param_quirks)
+		ohci->quirks = param_quirks;
 
 	ar_context_init(&ohci->ar_request_ctx, ohci,
 			OHCI1394_AsReqRcvContextControlSet);

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

[PATCH 7/8] firewire: ohci: fix IR/IT context mask mixup

[Stefan Richter]

This bug was present in firewire-ohci since day one:  The number of
available isochronous receive DMA contexts was mixed up with that of
available isochronous transmit DMA contexts.

This is harmless on a few chips which offer the same number of contexts
in both directions, but most chips nowadays implement only the standard
minimum of 4 IR contexts, but 8 IT contexts.  If a user attempted to run
a lot of IR contexts at once, results with more than four were therefore
unpredictable.  I suppose the controller would simply refuse to start
DMA of any unimplemented context.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
---
 drivers/firewire/ohci.c |   14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

Index: b/drivers/firewire/ohci.c
===================================================================
--- a/drivers/firewire/ohci.c
+++ b/drivers/firewire/ohci.c
@@ -2395,17 +2395,17 @@ static int __devinit pci_probe(struct pc
 		     OHCI1394_AsRspTrContextControlSet, handle_at_packet);
 
 	reg_write(ohci, OHCI1394_IsoRecvIntMaskSet, ~0);
-	ohci->it_context_mask = reg_read(ohci, OHCI1394_IsoRecvIntMaskSet);
+	ohci->ir_context_channels = ~0ULL;
+	ohci->ir_context_mask = reg_read(ohci, OHCI1394_IsoRecvIntMaskSet);
 	reg_write(ohci, OHCI1394_IsoRecvIntMaskClear, ~0);
-	size = sizeof(struct iso_context) * hweight32(ohci->it_context_mask);
-	ohci->it_context_list = kzalloc(size, GFP_KERNEL);
+	size = sizeof(struct iso_context) * hweight32(ohci->ir_context_mask);
+	ohci->ir_context_list = kzalloc(size, GFP_KERNEL);
 
 	reg_write(ohci, OHCI1394_IsoXmitIntMaskSet, ~0);
-	ohci->ir_context_channels = ~0ULL;
-	ohci->ir_context_mask = reg_read(ohci, OHCI1394_IsoXmitIntMaskSet);
+	ohci->it_context_mask = reg_read(ohci, OHCI1394_IsoXmitIntMaskSet);
 	reg_write(ohci, OHCI1394_IsoXmitIntMaskClear, ~0);
-	size = sizeof(struct iso_context) * hweight32(ohci->ir_context_mask);
-	ohci->ir_context_list = kzalloc(size, GFP_KERNEL);
+	size = sizeof(struct iso_context) * hweight32(ohci->it_context_mask);
+	ohci->it_context_list = kzalloc(size, GFP_KERNEL);
 
 	if (ohci->it_context_list == NULL || ohci->ir_context_list == NULL) {
 		err = -ENOMEM;

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

[PATCH 8/8] firewire: ohci: extend initialization log message

[Stefan Richter]

by the number of available isochronous DMA contexts and active quirks
which is occasionally useful information.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
---
 drivers/firewire/ohci.c |   17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

Index: b/drivers/firewire/ohci.c
===================================================================
--- a/drivers/firewire/ohci.c
+++ b/drivers/firewire/ohci.c
@@ -2329,7 +2329,7 @@ static int __devinit pci_probe(struct pc
 	struct fw_ohci *ohci;
 	u32 bus_options, max_receive, link_speed, version;
 	u64 guid;
-	int i, err;
+	int i, err, n_ir, n_it;
 	size_t size;
 
 	ohci = kzalloc(sizeof(*ohci), GFP_KERNEL);
@@ -2370,8 +2370,6 @@ static int __devinit pci_probe(struct pc
 		goto fail_iomem;
 	}
 
-	version = reg_read(ohci, OHCI1394_Version) & 0x00ff00ff;
-
 	for (i = 0; i < ARRAY_SIZE(ohci_quirks); i++)
 		if (ohci_quirks[i].vendor == dev->vendor &&
 		    (ohci_quirks[i].device == dev->device ||
@@ -2398,13 +2396,15 @@ static int __devinit pci_probe(struct pc
 	ohci->ir_context_channels = ~0ULL;
 	ohci->ir_context_mask = reg_read(ohci, OHCI1394_IsoRecvIntMaskSet);
 	reg_write(ohci, OHCI1394_IsoRecvIntMaskClear, ~0);
-	size = sizeof(struct iso_context) * hweight32(ohci->ir_context_mask);
+	n_ir = hweight32(ohci->ir_context_mask);
+	size = sizeof(struct iso_context) * n_ir;
 	ohci->ir_context_list = kzalloc(size, GFP_KERNEL);
 
 	reg_write(ohci, OHCI1394_IsoXmitIntMaskSet, ~0);
 	ohci->it_context_mask = reg_read(ohci, OHCI1394_IsoXmitIntMaskSet);
 	reg_write(ohci, OHCI1394_IsoXmitIntMaskClear, ~0);
-	size = sizeof(struct iso_context) * hweight32(ohci->it_context_mask);
+	n_it = hweight32(ohci->it_context_mask);
+	size = sizeof(struct iso_context) * n_it;
 	ohci->it_context_list = kzalloc(size, GFP_KERNEL);
 
 	if (ohci->it_context_list == NULL || ohci->ir_context_list == NULL) {
@@ -2432,8 +2432,11 @@ static int __devinit pci_probe(struct pc
 	if (err)
 		goto fail_self_id;
 
-	fw_notify("Added fw-ohci device %s, OHCI version %x.%x\n",
-		  dev_name(&dev->dev), version >> 16, version & 0xff);
+	version = reg_read(ohci, OHCI1394_Version) & 0x00ff00ff;
+	fw_notify("Added fw-ohci device %s, OHCI v%x.%x, "
+		  "%d IR + %d IT contexts, quirks 0x%x\n",
+		  dev_name(&dev->dev), version >> 16, version & 0xff,
+		  n_ir, n_it, ohci->quirks);
 
 	return 0;
 

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

Re: [PATCH 6/8] firewire: ohci: add module parameter to activate quirk fixes

[Stefan Richter]

I wrote:
> This way, we can advise users of precompiled kernel packages to test
> existing quirk fixes on chips which have not been listed yet, without
> them having to build a kernel from source.

PS: Jay Fenlason posted a very similar patch already in November.  In
the meantime, the forceful disappearance of dual-buffer issues has made
such a facility less relevant, but the addition of the cycle timer quirk
fix made it a useful idea again.

> Note, to use this feature on a machine with more than one controller,
> steps like these are necessary:
> # lspci | grep 1394
> # ls /sys/bus/pci/drivers/firewire_ohci/
> # echo -n "0000:03:02.0" > /sys/bus/pci/drivers/firewire_ohci/unbind
> # echo 2 > /sys/module/firewire_ohci/parameters/quirks
> # echo -n "0000:03:02.0" > /sys/bus/pci/drivers/firewire_ohci/bind
> # echo 0 > /sys/module/firewire_ohci/parameters/quirks
[...]
>  
> +/* This overrides anything that was found in ohci_quirks[]. */
> +static int param_quirks;

PS2: The parameter can also be used to switch off quirk flags that were
hardwired into firewire-ohci's quirks table.  Simply specify a non-zero
quirks value but without any known flags; e.g. 0x100.
-- 
Stefan Richter
-=====-==-=- --=- =-=-=
http://arcgraph.de/sr/