All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 00/11] firewire: cdev: proposed ABI extensions
@ 2009-01-04 15:23 Stefan Richter
  2009-01-04 15:24 ` [PATCH 01/11] firewire: cdev: reference-count client instances Stefan Richter
                   ` (11 more replies)
  0 siblings, 12 replies; 17+ messages in thread
From: Stefan Richter @ 2009-01-04 15:23 UTC (permalink / raw)
  To: linux1394-devel; +Cc: linux-kernel

Hi all,

here come updated implementations (based on Jay Fenlason's patches) or
first implementations of the pending <linux/firewire-cdev.h> ABI
extensions. Still not done though:
  - Add ioctl to deallocate an isochronous context.
  - Allow more than one isochronous context per fd.
  - Document David Moore's OHCI timestamp change,
    increment ABI version number accordingly.

Also, while Jay Fenlason already tested his implementations, I did not
yet do any runtime test at all of any of the new ioctls as implemented
in the following updates.  Nor do I have matching libraw1394 code and
libdc1394 code yet.  I thought I post this stuff nevertheless in order
to gather your criticism...

Therefore I feel that this is all too late to be pushed into mainline
before 2.6.29(-rc1).  I'd like to target 2.6.30(-rc1) now instead.  I
hope those who have been waiting for all of this for months don't murder
me for the repeated delays to get this released.

At least I runtime-tested the base changes related to reference counting
and spinlocks though.

These patches apply after several as yet uncommitted firewire patches,
most notably "firewire: cdev: use an idr rather than a linked list for
resources" as necessary infrastructure, and "firewire: standardize a
variable name" and "firewire: remove line breaks before function names"
as ones which presumable create conflicts if not applied before.

Coming as follow-ups:

[PATCH 01/11] firewire: cdev: reference-count client instances
[PATCH 02/11] firewire: cdev: unify names of struct types and of their instances
[PATCH 03/11] firewire: cdev: sort includes
[PATCH 04/11] firewire: core: topology header fix
[PATCH 05/11] firewire: cdev: add ioctls for isochronous resource management
[PATCH 06/11] firewire: cdev: add ioctls for manual iso resource management
[PATCH 07/11] firewire: cdev: add ioctl to query maximum transmission speed
[PATCH 08/11] firewire: cdev: add ioctl for broadcast write requests
[PATCH 09/11] firewire: cdev: restrict broadcast write requests to Units Space
[PATCH 10/11] firewire: cdev: extend transaction payload size check
[PATCH 11/11] firewire: cdev: replace some spin_lock_irqsave by spin_lock_irq

 drivers/firewire/fw-cdev.c        |  749 +++++++++++++++++++++---------
 drivers/firewire/fw-iso.c         |  176 ++++++-
 drivers/firewire/fw-topology.h    |    6 
 drivers/firewire/fw-transaction.h |    4 
 include/linux/firewire-cdev.h     |  133 ++++-
 5 files changed, 836 insertions(+), 232 deletions(-)

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


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

* [PATCH 01/11] firewire: cdev: reference-count client instances
  2009-01-04 15:23 [PATCH 00/11] firewire: cdev: proposed ABI extensions Stefan Richter
@ 2009-01-04 15:24 ` Stefan Richter
  2009-01-11 20:31   ` David Moore
  2009-01-04 15:25 ` [PATCH 02/11] firewire: cdev: unify names of struct types and of their instances Stefan Richter
                   ` (10 subsequent siblings)
  11 siblings, 1 reply; 17+ messages in thread
From: Stefan Richter @ 2009-01-04 15:24 UTC (permalink / raw)
  To: linux1394-devel; +Cc: linux-kernel

The lifetime of struct client instances must be longer than the lifetime
of any client resource.

This fixes a possible race between fw_device_op_release and transaction
completions.  It also prepares for new ioctls for isochronous resource
management which will involve delayed processing of client resources.

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

Index: linux/drivers/firewire/fw-cdev.c
===================================================================
--- linux.orig/drivers/firewire/fw-cdev.c
+++ linux/drivers/firewire/fw-cdev.c
@@ -20,6 +20,7 @@
 
 #include <linux/module.h>
 #include <linux/kernel.h>
+#include <linux/kref.h>
 #include <linux/wait.h>
 #include <linux/errno.h>
 #include <linux/device.h>
@@ -94,8 +95,27 @@ struct client {
 	unsigned long vm_start;
 
 	struct list_head link;
+	struct kref kref;
 };
 
+static inline void client_get(struct client *client)
+{
+	kref_get(&client->kref);
+}
+
+static void client_release(struct kref *kref)
+{
+	struct client *client = container_of(kref, struct client, kref);
+
+	fw_device_put(client->device);
+	kfree(client);
+}
+
+static void client_put(struct client *client)
+{
+	kref_put(&client->kref, client_release);
+}
+
 static inline void __user *u64_to_uptr(__u64 value)
 {
 	return (void __user *)(unsigned long)value;
@@ -131,6 +151,7 @@ static int fw_device_op_open(struct inod
 	idr_init(&client->resource_idr);
 	INIT_LIST_HEAD(&client->event_list);
 	init_waitqueue_head(&client->wait);
+	kref_init(&client->kref);
 
 	file->private_data = client;
 
@@ -326,6 +347,8 @@ static int add_client_resource(struct cl
 	else
 		ret = idr_get_new(&client->resource_idr, resource,
 				  &resource->handle);
+	if (ret >= 0)
+		client_get(client);
 	spin_unlock_irqrestore(&client->lock, flags);
 
 	if (ret == -EAGAIN)
@@ -358,6 +381,8 @@ static int release_client_resource(struc
 	else
 		r->release(client, r);
 
+	client_put(client);
+
 	return 0;
 }
 
@@ -385,11 +410,21 @@ static void complete_transaction(struct 
 
 	spin_lock_irqsave(&client->lock, flags);
 	/*
-	 * If called while in shutdown, the idr tree must be left untouched.
-	 * The idr handle will be removed later.
+	 * 1. If called while in shutdown, the idr tree must be left untouched.
+	 *    The idr handle will be removed and the client reference will be
+	 *    dropped later.
+	 * 2. If the call chain was release_client_resource ->
+	 *    release_transaction -> complete_transaction (instead of a normal
+	 *    conclusion of the transaction), i.e. if this resource was already
+	 *    unregistered from the idr, the client reference will be dropped
+	 *    by release_client_resource and we must not drop it here.
 	 */
-	if (!client->in_shutdown)
+	if (!client->in_shutdown &&
+	    idr_find(&client->resource_idr, response->resource.handle)) {
 		idr_remove(&client->resource_idr, response->resource.handle);
+		/* Drop the idr's reference */
+		client_put(client);
+	}
 	spin_unlock_irqrestore(&client->lock, flags);
 
 	r->type   = FW_CDEV_EVENT_RESPONSE;
@@ -408,6 +443,9 @@ static void complete_transaction(struct 
 	else
 		queue_event(client, &response->event, r, sizeof(*r) + r->length,
 			    NULL, 0);
+
+	/* Drop the transaction callback's reference */
+	client_put(client);
 }
 
 static int ioctl_send_request(struct client *client, void *buffer)
@@ -459,6 +497,9 @@ static int ioctl_send_request(struct cli
 	if (ret < 0)
 		goto failed;
 
+	/* Get a reference for the transaction callback */
+	client_get(client);
+
 	fw_send_request(device->card, &response->transaction,
 			request->tcode & 0x1f,
 			device->node->node_id,
@@ -1044,6 +1085,7 @@ static int shutdown_resource(int id, voi
 	struct client *client = data;
 
 	r->release(client, r);
+	client_put(client);
 
 	return 0;
 }
@@ -1076,12 +1118,7 @@ static int fw_device_op_release(struct i
 	list_for_each_entry_safe(e, next_e, &client->event_list, link)
 		kfree(e);
 
-	/*
-	 * FIXME: client should be reference-counted.  It's extremely unlikely
-	 * but there may still be transactions being completed at this point.
-	 */
-	fw_device_put(client->device);
-	kfree(client);
+	client_put(client);
 
 	return 0;
 }

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


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

* [PATCH 02/11] firewire: cdev: unify names of struct types and of their instances
  2009-01-04 15:23 [PATCH 00/11] firewire: cdev: proposed ABI extensions Stefan Richter
  2009-01-04 15:24 ` [PATCH 01/11] firewire: cdev: reference-count client instances Stefan Richter
@ 2009-01-04 15:25 ` Stefan Richter
  2009-01-04 15:25 ` [PATCH 03/11] firewire: cdev: sort includes Stefan Richter
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 17+ messages in thread
From: Stefan Richter @ 2009-01-04 15:25 UTC (permalink / raw)
  To: linux1394-devel; +Cc: linux-kernel

to indicate that they are specializations of struct event or of struct
client_resource, respectively.

struct response was both an event and a client_resource; it is now split
into struct outbound_transaction_resource and ~_event in order to
document more explicitly which types of client resources exist.

struct request and struct_request_event are renamed to struct
inbound_transaction_resource and ~_event because requests and responses
occur in outbound and in inbound transactions.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
---
 drivers/firewire/fw-cdev.c |  337 ++++++++++++++++++-------------------
 1 file changed, 169 insertions(+), 168 deletions(-)

Index: linux/drivers/firewire/fw-cdev.c
===================================================================
--- linux.orig/drivers/firewire/fw-cdev.c
+++ linux/drivers/firewire/fw-cdev.c
@@ -41,43 +41,6 @@
 #include "fw-topology.h"
 #include "fw-device.h"
 
-struct client;
-struct client_resource;
-typedef void (*client_resource_release_fn_t)(struct client *,
-					     struct client_resource *);
-struct client_resource {
-	client_resource_release_fn_t release;
-	int handle;
-};
-
-/*
- * dequeue_event() just kfree()'s the event, so the event has to be
- * the first field in the struct.
- */
-
-struct event {
-	struct { void *data; size_t size; } v[2];
-	struct list_head link;
-};
-
-struct bus_reset {
-	struct event event;
-	struct fw_cdev_event_bus_reset reset;
-};
-
-struct response {
-	struct event event;
-	struct fw_transaction transaction;
-	struct client *client;
-	struct client_resource resource;
-	struct fw_cdev_event_response response;
-};
-
-struct iso_interrupt {
-	struct event event;
-	struct fw_cdev_event_iso_interrupt interrupt;
-};
-
 struct client {
 	u32 version;
 	struct fw_device *device;
@@ -116,6 +79,70 @@ static void client_put(struct client *cl
 	kref_put(&client->kref, client_release);
 }
 
+struct client_resource;
+typedef void (*client_resource_release_fn_t)(struct client *,
+					     struct client_resource *);
+struct client_resource {
+	client_resource_release_fn_t release;
+	int handle;
+};
+
+struct address_handler_resource {
+	struct client_resource resource;
+	struct fw_address_handler handler;
+	__u64 closure;
+	struct client *client;
+};
+
+struct outbound_transaction_resource {
+	struct client_resource resource;
+	struct fw_transaction transaction;
+};
+
+struct inbound_transaction_resource {
+	struct client_resource resource;
+	struct fw_request *request;
+	void *data;
+	size_t length;
+};
+
+struct descriptor_resource {
+	struct client_resource resource;
+	struct fw_descriptor descriptor;
+	u32 data[0];
+};
+
+/*
+ * dequeue_event() just kfree()'s the event, so the event has to be
+ * the first field in a struct XYZ_event.
+ */
+struct event {
+	struct { void *data; size_t size; } v[2];
+	struct list_head link;
+};
+
+struct bus_reset_event {
+	struct event event;
+	struct fw_cdev_event_bus_reset reset;
+};
+
+struct outbound_transaction_event {
+	struct event event;
+	struct client *client;
+	struct outbound_transaction_resource r;
+	struct fw_cdev_event_response response;
+};
+
+struct inbound_transaction_event {
+	struct event event;
+	struct fw_cdev_event_request request;
+};
+
+struct iso_interrupt_event {
+	struct event event;
+	struct fw_cdev_event_iso_interrupt interrupt;
+};
+
 static inline void __user *u64_to_uptr(__u64 value)
 {
 	return (void __user *)(unsigned long)value;
@@ -263,18 +290,18 @@ static void for_each_client(struct fw_de
 
 static void queue_bus_reset_event(struct client *client)
 {
-	struct bus_reset *bus_reset;
+	struct bus_reset_event *e;
 
-	bus_reset = kzalloc(sizeof(*bus_reset), GFP_KERNEL);
-	if (bus_reset == NULL) {
+	e = kzalloc(sizeof(*e), GFP_KERNEL);
+	if (e == NULL) {
 		fw_notify("Out of memory when allocating bus reset event\n");
 		return;
 	}
 
-	fill_bus_reset_event(&bus_reset->reset, client);
+	fill_bus_reset_event(&e->reset, client);
 
-	queue_event(client, &bus_reset->event,
-		    &bus_reset->reset, sizeof(bus_reset->reset), NULL, 0);
+	queue_event(client, &e->event,
+		    &e->reset, sizeof(e->reset), NULL, 0);
 }
 
 void fw_device_cdev_update(struct fw_device *device)
@@ -389,24 +416,24 @@ static int release_client_resource(struc
 static void release_transaction(struct client *client,
 				struct client_resource *resource)
 {
-	struct response *response =
-		container_of(resource, struct response, resource);
+	struct outbound_transaction_resource *r = container_of(resource,
+			struct outbound_transaction_resource, resource);
 
-	fw_cancel_transaction(client->device->card, &response->transaction);
+	fw_cancel_transaction(client->device->card, &r->transaction);
 }
 
 static void complete_transaction(struct fw_card *card, int rcode,
 				 void *payload, size_t length, void *data)
 {
-	struct response *response = data;
-	struct client *client = response->client;
+	struct outbound_transaction_event *e = data;
+	struct fw_cdev_event_response *rsp = &e->response;
+	struct client *client = e->client;
 	unsigned long flags;
-	struct fw_cdev_event_response *r = &response->response;
 
-	if (length < r->length)
-		r->length = length;
+	if (length < rsp->length)
+		rsp->length = length;
 	if (rcode == RCODE_COMPLETE)
-		memcpy(r->data, payload, r->length);
+		memcpy(rsp->data, payload, rsp->length);
 
 	spin_lock_irqsave(&client->lock, flags);
 	/*
@@ -420,28 +447,28 @@ static void complete_transaction(struct 
 	 *    by release_client_resource and we must not drop it here.
 	 */
 	if (!client->in_shutdown &&
-	    idr_find(&client->resource_idr, response->resource.handle)) {
-		idr_remove(&client->resource_idr, response->resource.handle);
+	    idr_find(&client->resource_idr, e->r.resource.handle)) {
+		idr_remove(&client->resource_idr, e->r.resource.handle);
 		/* Drop the idr's reference */
 		client_put(client);
 	}
 	spin_unlock_irqrestore(&client->lock, flags);
 
-	r->type   = FW_CDEV_EVENT_RESPONSE;
-	r->rcode  = rcode;
+	rsp->type = FW_CDEV_EVENT_RESPONSE;
+	rsp->rcode = rcode;
 
 	/*
-	 * In the case that sizeof(*r) doesn't align with the position of the
+	 * In the case that sizeof(*rsp) 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);
+	if (rsp->length <= sizeof(*rsp) - offsetof(typeof(*rsp), data))
+		queue_event(client, &e->event, rsp, sizeof(*rsp),
+			    rsp->data, rsp->length);
 	else
-		queue_event(client, &response->event, r, sizeof(*r) + r->length,
+		queue_event(client, &e->event, rsp, sizeof(*rsp) + rsp->length,
 			    NULL, 0);
 
 	/* Drop the transaction callback's reference */
@@ -452,23 +479,23 @@ static int ioctl_send_request(struct cli
 {
 	struct fw_device *device = client->device;
 	struct fw_cdev_send_request *request = buffer;
-	struct response *response;
+	struct outbound_transaction_event *e;
 	int ret;
 
 	/* What is the biggest size we'll accept, really? */
 	if (request->length > 4096)
 		return -EINVAL;
 
-	response = kmalloc(sizeof(*response) + request->length, GFP_KERNEL);
-	if (response == NULL)
+	e = kmalloc(sizeof(*e) + request->length, GFP_KERNEL);
+	if (e == NULL)
 		return -ENOMEM;
 
-	response->client = client;
-	response->response.length = request->length;
-	response->response.closure = request->closure;
+	e->client = client;
+	e->response.length = request->length;
+	e->response.closure = request->closure;
 
 	if (request->data &&
-	    copy_from_user(response->response.data,
+	    copy_from_user(e->response.data,
 			   u64_to_uptr(request->data), request->length)) {
 		ret = -EFAULT;
 		goto failed;
@@ -492,86 +519,66 @@ static int ioctl_send_request(struct cli
 		goto failed;
 	}
 
-	response->resource.release = release_transaction;
-	ret = add_client_resource(client, &response->resource, GFP_KERNEL);
+	e->r.resource.release = release_transaction;
+	ret = add_client_resource(client, &e->r.resource, GFP_KERNEL);
 	if (ret < 0)
 		goto failed;
 
 	/* Get a reference for the transaction callback */
 	client_get(client);
 
-	fw_send_request(device->card, &response->transaction,
+	fw_send_request(device->card, &e->r.transaction,
 			request->tcode & 0x1f,
 			device->node->node_id,
 			request->generation,
 			device->max_speed,
 			request->offset,
-			response->response.data, request->length,
-			complete_transaction, response);
+			e->response.data, request->length,
+			complete_transaction, e);
 
 	if (request->data)
 		return sizeof(request) + request->length;
 	else
 		return sizeof(request);
  failed:
-	kfree(response);
+	kfree(e);
 
 	return ret;
 }
 
-struct address_handler {
-	struct fw_address_handler handler;
-	__u64 closure;
-	struct client *client;
-	struct client_resource resource;
-};
-
-struct request {
-	struct fw_request *request;
-	void *data;
-	size_t length;
-	struct client_resource resource;
-};
-
-struct request_event {
-	struct event event;
-	struct fw_cdev_event_request request;
-};
-
 static void release_request(struct client *client,
 			    struct client_resource *resource)
 {
-	struct request *request =
-		container_of(resource, struct request, resource);
+	struct inbound_transaction_resource *r = container_of(resource,
+			struct inbound_transaction_resource, resource);
 
-	fw_send_response(client->device->card, request->request,
+	fw_send_response(client->device->card, r->request,
 			 RCODE_CONFLICT_ERROR);
-	kfree(request);
+	kfree(r);
 }
 
-static void handle_request(struct fw_card *card, struct fw_request *r,
+static void handle_request(struct fw_card *card, struct fw_request *request,
 			   int tcode, int destination, int source,
 			   int generation, int speed,
 			   unsigned long long offset,
 			   void *payload, size_t length, void *callback_data)
 {
-	struct address_handler *handler = callback_data;
-	struct request *request;
-	struct request_event *e;
-	struct client *client = handler->client;
+	struct address_handler_resource *handler = callback_data;
+	struct inbound_transaction_resource *r;
+	struct inbound_transaction_event *e;
 	int ret;
 
-	request = kmalloc(sizeof(*request), GFP_ATOMIC);
+	r = kmalloc(sizeof(*r), GFP_ATOMIC);
 	e = kmalloc(sizeof(*e), GFP_ATOMIC);
-	if (request == NULL || e == NULL)
+	if (r == NULL || e == NULL)
 		goto failed;
 
-	request->request = r;
-	request->data    = payload;
-	request->length  = length;
+	r->request = request;
+	r->data    = payload;
+	r->length  = length;
 
-	request->resource.release = release_request;
-	ret = add_client_resource(client, &request->resource, GFP_ATOMIC);
+	r->resource.release = release_request;
+	ret = add_client_resource(handler->client, &r->resource, GFP_ATOMIC);
 	if (ret < 0)
 		goto failed;
 
@@ -579,61 +586,61 @@ static void handle_request(struct fw_car
 	e->request.tcode   = tcode;
 	e->request.offset  = offset;
 	e->request.length  = length;
-	e->request.handle  = request->resource.handle;
+	e->request.handle  = r->resource.handle;
 	e->request.closure = handler->closure;
 
-	queue_event(client, &e->event,
+	queue_event(handler->client, &e->event,
 		    &e->request, sizeof(e->request), payload, length);
 	return;
 
  failed:
-	kfree(request);
+	kfree(r);
 	kfree(e);
-	fw_send_response(card, r, RCODE_CONFLICT_ERROR);
+	fw_send_response(card, request, RCODE_CONFLICT_ERROR);
 }
 
 static void release_address_handler(struct client *client,
 				    struct client_resource *resource)
 {
-	struct address_handler *handler =
-		container_of(resource, struct address_handler, resource);
+	struct address_handler_resource *r =
+	    container_of(resource, struct address_handler_resource, resource);
 
-	fw_core_remove_address_handler(&handler->handler);
-	kfree(handler);
+	fw_core_remove_address_handler(&r->handler);
+	kfree(r);
 }
 
 static int ioctl_allocate(struct client *client, void *buffer)
 {
 	struct fw_cdev_allocate *request = buffer;
-	struct address_handler *handler;
+	struct address_handler_resource *r;
 	struct fw_address_region region;
 	int ret;
 
-	handler = kmalloc(sizeof(*handler), GFP_KERNEL);
-	if (handler == NULL)
+	r = kmalloc(sizeof(*r), GFP_KERNEL);
+	if (r == NULL)
 		return -ENOMEM;
 
 	region.start = request->offset;
 	region.end = request->offset + request->length;
-	handler->handler.length = request->length;
-	handler->handler.address_callback = handle_request;
-	handler->handler.callback_data = handler;
-	handler->closure = request->closure;
-	handler->client = client;
+	r->handler.length = request->length;
+	r->handler.address_callback = handle_request;
+	r->handler.callback_data = r;
+	r->closure = request->closure;
+	r->client = client;
 
-	ret = fw_core_add_address_handler(&handler->handler, &region);
+	ret = fw_core_add_address_handler(&r->handler, &region);
 	if (ret < 0) {
-		kfree(handler);
+		kfree(r);
 		return ret;
 	}
 
-	handler->resource.release = release_address_handler;
-	ret = add_client_resource(client, &handler->resource, GFP_KERNEL);
+	r->resource.release = release_address_handler;
+	ret = add_client_resource(client, &r->resource, GFP_KERNEL);
 	if (ret < 0) {
-		release_address_handler(client, &handler->resource);
+		release_address_handler(client, &r->resource);
 		return ret;
 	}
-	request->handle = handler->resource.handle;
+	request->handle = r->resource.handle;
 
 	return 0;
 }
@@ -650,13 +657,14 @@ static int ioctl_send_response(struct cl
 {
 	struct fw_cdev_send_response *request = buffer;
 	struct client_resource *resource;
-	struct request *r;
+	struct inbound_transaction_resource *r;
 
 	if (release_client_resource(client, request->handle,
 				    release_request, &resource) < 0)
 		return -EINVAL;
 
-	r = container_of(resource, struct request, resource);
+	r = container_of(resource, struct inbound_transaction_resource,
+			 resource);
 	if (request->length < r->length)
 		r->length = request->length;
 	if (copy_from_user(r->data, u64_to_uptr(request->data), r->length))
@@ -678,62 +686,55 @@ static int ioctl_initiate_bus_reset(stru
 	return fw_core_initiate_bus_reset(client->device->card, short_reset);
 }
 
-struct descriptor {
-	struct fw_descriptor d;
-	struct client_resource resource;
-	u32 data[0];
-};
-
 static void release_descriptor(struct client *client,
 			       struct client_resource *resource)
 {
-	struct descriptor *descriptor =
-		container_of(resource, struct descriptor, resource);
+	struct descriptor_resource *r =
+		container_of(resource, struct descriptor_resource, resource);
 
-	fw_core_remove_descriptor(&descriptor->d);
-	kfree(descriptor);
+	fw_core_remove_descriptor(&r->descriptor);
+	kfree(r);
 }
 
 static int ioctl_add_descriptor(struct client *client, void *buffer)
 {
 	struct fw_cdev_add_descriptor *request = buffer;
-	struct descriptor *descriptor;
+	struct descriptor_resource *r;
 	int ret;
 
 	if (request->length > 256)
 		return -EINVAL;
 
-	descriptor =
-		kmalloc(sizeof(*descriptor) + request->length * 4, GFP_KERNEL);
-	if (descriptor == NULL)
+	r = kmalloc(sizeof(*r) + request->length * 4, GFP_KERNEL);
+	if (r == NULL)
 		return -ENOMEM;
 
-	if (copy_from_user(descriptor->data,
+	if (copy_from_user(r->data,
 			   u64_to_uptr(request->data), request->length * 4)) {
 		ret = -EFAULT;
 		goto failed;
 	}
 
-	descriptor->d.length = request->length;
-	descriptor->d.immediate = request->immediate;
-	descriptor->d.key = request->key;
-	descriptor->d.data = descriptor->data;
+	r->descriptor.length    = request->length;
+	r->descriptor.immediate = request->immediate;
+	r->descriptor.key       = request->key;
+	r->descriptor.data      = r->data;
 
-	ret = fw_core_add_descriptor(&descriptor->d);
+	ret = fw_core_add_descriptor(&r->descriptor);
 	if (ret < 0)
 		goto failed;
 
-	descriptor->resource.release = release_descriptor;
-	ret = add_client_resource(client, &descriptor->resource, GFP_KERNEL);
+	r->resource.release = release_descriptor;
+	ret = add_client_resource(client, &r->resource, GFP_KERNEL);
 	if (ret < 0) {
-		fw_core_remove_descriptor(&descriptor->d);
+		fw_core_remove_descriptor(&r->descriptor);
 		goto failed;
 	}
-	request->handle = descriptor->resource.handle;
+	request->handle = r->resource.handle;
 
 	return 0;
  failed:
-	kfree(descriptor);
+	kfree(r);
 
 	return ret;
 }
@@ -750,19 +751,19 @@ static void iso_callback(struct fw_iso_c
 			 size_t header_length, void *header, void *data)
 {
 	struct client *client = data;
-	struct iso_interrupt *irq;
+	struct iso_interrupt_event *e;
 
-	irq = kzalloc(sizeof(*irq) + header_length, GFP_ATOMIC);
-	if (irq == NULL)
+	e = kzalloc(sizeof(*e) + header_length, GFP_ATOMIC);
+	if (e == NULL)
 		return;
 
-	irq->interrupt.type      = FW_CDEV_EVENT_ISO_INTERRUPT;
-	irq->interrupt.closure   = client->iso_closure;
-	irq->interrupt.cycle     = cycle;
-	irq->interrupt.header_length = header_length;
-	memcpy(irq->interrupt.header, header, header_length);
-	queue_event(client, &irq->event, &irq->interrupt,
-		    sizeof(irq->interrupt) + header_length, NULL, 0);
+	e->interrupt.type      = FW_CDEV_EVENT_ISO_INTERRUPT;
+	e->interrupt.closure   = client->iso_closure;
+	e->interrupt.cycle     = cycle;
+	e->interrupt.header_length = header_length;
+	memcpy(e->interrupt.header, header, header_length);
+	queue_event(client, &e->event, &e->interrupt,
+		    sizeof(e->interrupt) + header_length, NULL, 0);
 }
 
 static int ioctl_create_iso_context(struct client *client, void *buffer)

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


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

* [PATCH 03/11] firewire: cdev: sort includes
  2009-01-04 15:23 [PATCH 00/11] firewire: cdev: proposed ABI extensions Stefan Richter
  2009-01-04 15:24 ` [PATCH 01/11] firewire: cdev: reference-count client instances Stefan Richter
  2009-01-04 15:25 ` [PATCH 02/11] firewire: cdev: unify names of struct types and of their instances Stefan Richter
@ 2009-01-04 15:25 ` Stefan Richter
  2009-01-04 15:26 ` [PATCH 04/11] firewire: core: topology header fix Stefan Richter
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 17+ messages in thread
From: Stefan Richter @ 2009-01-04 15:25 UTC (permalink / raw)
  To: linux1394-devel; +Cc: linux-kernel

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

Index: linux/drivers/firewire/fw-cdev.c
===================================================================
--- linux.orig/drivers/firewire/fw-cdev.c
+++ linux/drivers/firewire/fw-cdev.c
@@ -18,28 +18,30 @@
  * Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
  */
 
-#include <linux/module.h>
+#include <linux/compat.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/errno.h>
+#include <linux/firewire-cdev.h>
+#include <linux/idr.h>
 #include <linux/kernel.h>
 #include <linux/kref.h>
-#include <linux/wait.h>
-#include <linux/errno.h>
-#include <linux/device.h>
-#include <linux/vmalloc.h>
+#include <linux/mm.h>
+#include <linux/module.h>
 #include <linux/mutex.h>
 #include <linux/poll.h>
 #include <linux/preempt.h>
-#include <linux/time.h>
 #include <linux/spinlock.h>
-#include <linux/delay.h>
-#include <linux/mm.h>
-#include <linux/idr.h>
-#include <linux/compat.h>
-#include <linux/firewire-cdev.h>
+#include <linux/time.h>
+#include <linux/vmalloc.h>
+#include <linux/wait.h>
+
 #include <asm/system.h>
 #include <asm/uaccess.h>
-#include "fw-transaction.h"
-#include "fw-topology.h"
+
 #include "fw-device.h"
+#include "fw-topology.h"
+#include "fw-transaction.h"
 
 struct client {
 	u32 version;

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


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

* [PATCH 04/11] firewire: core: topology header fix
  2009-01-04 15:23 [PATCH 00/11] firewire: cdev: proposed ABI extensions Stefan Richter
                   ` (2 preceding siblings ...)
  2009-01-04 15:25 ` [PATCH 03/11] firewire: cdev: sort includes Stefan Richter
@ 2009-01-04 15:26 ` Stefan Richter
  2009-01-04 15:26 ` [PATCH 05/11] firewire: cdev: add ioctls for isochronous resource management Stefan Richter
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 17+ messages in thread
From: Stefan Richter @ 2009-01-04 15:26 UTC (permalink / raw)
  To: linux1394-devel; +Cc: linux-kernel

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

Index: linux/drivers/firewire/fw-topology.h
===================================================================
--- linux.orig/drivers/firewire/fw-topology.h
+++ linux/drivers/firewire/fw-topology.h
@@ -19,6 +19,11 @@
 #ifndef __fw_topology_h
 #define __fw_topology_h
 
+#include <linux/list.h>
+#include <linux/slab.h>
+
+#include <asm/atomic.h>
+
 enum {
 	FW_NODE_CREATED,
 	FW_NODE_UPDATED,
@@ -64,6 +69,7 @@ static inline void fw_node_put(struct fw
 		kfree(node);
 }
 
+struct fw_card;
 void fw_destroy_nodes(struct fw_card *card);
 
 int fw_compute_block_crc(u32 *block);

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


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

* [PATCH 05/11] firewire: cdev: add ioctls for isochronous resource management
  2009-01-04 15:23 [PATCH 00/11] firewire: cdev: proposed ABI extensions Stefan Richter
                   ` (3 preceding siblings ...)
  2009-01-04 15:26 ` [PATCH 04/11] firewire: core: topology header fix Stefan Richter
@ 2009-01-04 15:26 ` Stefan Richter
       [not found]   ` <1231404355.18613.68.camel@localhost.localdomain>
  2009-01-11 20:32   ` [PATCH 05/11] firewire: cdev: add ioctls for isochronous resource management David Moore
  2009-01-04 15:27 ` [PATCH 06/11] firewire: cdev: add ioctls for manual iso " Stefan Richter
                   ` (6 subsequent siblings)
  11 siblings, 2 replies; 17+ messages in thread
From: Stefan Richter @ 2009-01-04 15:26 UTC (permalink / raw)
  To: linux1394-devel; +Cc: linux-kernel

Based on
    Date: Tue, 18 Nov 2008 11:41:27 -0500
    From: Jay Fenlason <fenlason@redhat.com>
    Subject: [Patch V4] Add ISO resource management support
with several changes to the ABI and implementation.  Only the part of
the ABI which enables auto-reallocation and auto-deallocation is
included here.

This implements ioctls for kernel-assisted allocation of isochronous
channels and isochronous bandwidth.  The benefits are:
  - The client does not have to have write access to the /dev/fw* device
    corresponding to the IRM.
  - The client does not have to perform reallocation after bus resets.
  - Channel and bandwidth are deallocated by the kernel if the file is
    closed before the client deallocated the resources.  Thus resources
    are released even if the client crashes.

It is anticipated that future in-kernel code (firewire-core IRM code;
the firewire port of firedtv), will use the fw-iso.c portions of this
code too.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
---
 drivers/firewire/fw-cdev.c        |  215 +++++++++++++++++++++++++++++-
 drivers/firewire/fw-iso.c         |  176 +++++++++++++++++++++++-
 drivers/firewire/fw-transaction.h |    4 
 include/linux/firewire-cdev.h     |  100 ++++++++++++-
 4 files changed, 475 insertions(+), 20 deletions(-)

Index: linux/include/linux/firewire-cdev.h
===================================================================
--- linux.orig/include/linux/firewire-cdev.h
+++ linux/include/linux/firewire-cdev.h
@@ -25,10 +25,12 @@
 #include <linux/types.h>
 #include <linux/firewire-constants.h>
 
-#define FW_CDEV_EVENT_BUS_RESET		0x00
-#define FW_CDEV_EVENT_RESPONSE		0x01
-#define FW_CDEV_EVENT_REQUEST		0x02
-#define FW_CDEV_EVENT_ISO_INTERRUPT	0x03
+#define FW_CDEV_EVENT_BUS_RESET			0x00
+#define FW_CDEV_EVENT_RESPONSE			0x01
+#define FW_CDEV_EVENT_REQUEST			0x02
+#define FW_CDEV_EVENT_ISO_INTERRUPT		0x03
+#define FW_CDEV_EVENT_ISO_RESOURCE_ALLOCATED	0x04
+#define FW_CDEV_EVENT_ISO_RESOURCE_DEALLOCATED	0x05
 
 /**
  * struct fw_cdev_event_common - Common part of all fw_cdev_event_ types
@@ -147,12 +149,46 @@ struct fw_cdev_event_iso_interrupt {
 };
 
 /**
+ * struct fw_cdev_event_iso_resource - Iso resources were allocated or freed
+ * @closure:	See &fw_cdev_event_common;
+ *		set by %FW_CDEV_IOC_ALLOCATE_ISO_RESOURCE ioctl
+ * @type:	%FW_CDEV_EVENT_ISO_RESOURCE_ALLOCATED or
+ *		%FW_CDEV_EVENT_ISO_RESOURCE_DEALLOCATED
+ * @handle:	Reference by which an allocated resource can be deallocated
+ * @channel:	Isochronous channel which was (de)allocated, if any
+ * @bandwidth:	Bandwidth allocation units which were (de)allocated, if any
+ * @channels_available:  Last known availability of channels
+ * @bandwidth_available: Last known availability of bandwidth
+ *
+ * An %FW_CDEV_EVENT_ISO_RESOURCE_ALLOCATED event is sent after an isochronous
+ * resource was allocated at the IRM.  The client has to check @channel and
+ * @bandwidth for whether the allocation actually succeeded.
+ *
+ * @channel is <0 if no channel was allocated.
+ * @bandwidth is 0 if no bandwidth was allocated.
+ *
+ * An %FW_CDEV_EVENT_ISO_RESOURCE_DEALLOCATED event is sent after an isochronous
+ * resource was deallocated at the IRM.  It is also sent when automatic
+ * reallocation after a bus reset failed.
+ */
+struct fw_cdev_event_iso_resource {
+	__u64 closure;
+	__u32 type;
+	__u32 handle;
+	__s32 channel;
+	__s32 bandwidth;
+};
+
+/**
  * union fw_cdev_event - Convenience union of fw_cdev_event_ types
  * @common:        Valid for all types
  * @bus_reset:     Valid if @common.type == %FW_CDEV_EVENT_BUS_RESET
  * @response:      Valid if @common.type == %FW_CDEV_EVENT_RESPONSE
  * @request:       Valid if @common.type == %FW_CDEV_EVENT_REQUEST
  * @iso_interrupt: Valid if @common.type == %FW_CDEV_EVENT_ISO_INTERRUPT
+ * @iso_resource:  Valid if @common.type ==
+ *				%FW_CDEV_EVENT_ISO_RESOURCE_ALLOCATED or
+ *				%FW_CDEV_EVENT_ISO_RESOURCE_DEALLOCATED
  *
  * Convenience union for userspace use.  Events could be read(2) into an
  * appropriately aligned char buffer and then cast to this union for further
@@ -163,13 +199,15 @@ struct fw_cdev_event_iso_interrupt {
  * not fit will be discarded so that the next read(2) will return a new event.
  */
 union fw_cdev_event {
-	struct fw_cdev_event_common common;
-	struct fw_cdev_event_bus_reset bus_reset;
-	struct fw_cdev_event_response response;
-	struct fw_cdev_event_request request;
-	struct fw_cdev_event_iso_interrupt iso_interrupt;
+	struct fw_cdev_event_common		common;
+	struct fw_cdev_event_bus_reset		bus_reset;
+	struct fw_cdev_event_response		response;
+	struct fw_cdev_event_request		request;
+	struct fw_cdev_event_iso_interrupt	iso_interrupt;
+	struct fw_cdev_event_iso_resource	iso_resource;
 };
 
+/* available since kernel version 2.6.22 */
 #define FW_CDEV_IOC_GET_INFO		_IOWR('#', 0x00, struct fw_cdev_get_info)
 #define FW_CDEV_IOC_SEND_REQUEST	_IOW('#', 0x01, struct fw_cdev_send_request)
 #define FW_CDEV_IOC_ALLOCATE		_IOWR('#', 0x02, struct fw_cdev_allocate)
@@ -178,13 +216,18 @@ union fw_cdev_event {
 #define FW_CDEV_IOC_INITIATE_BUS_RESET	_IOW('#', 0x05, struct fw_cdev_initiate_bus_reset)
 #define FW_CDEV_IOC_ADD_DESCRIPTOR	_IOWR('#', 0x06, struct fw_cdev_add_descriptor)
 #define FW_CDEV_IOC_REMOVE_DESCRIPTOR	_IOW('#', 0x07, struct fw_cdev_remove_descriptor)
-
 #define FW_CDEV_IOC_CREATE_ISO_CONTEXT	_IOWR('#', 0x08, struct fw_cdev_create_iso_context)
 #define FW_CDEV_IOC_QUEUE_ISO		_IOWR('#', 0x09, struct fw_cdev_queue_iso)
 #define FW_CDEV_IOC_START_ISO		_IOW('#', 0x0a, struct fw_cdev_start_iso)
 #define FW_CDEV_IOC_STOP_ISO		_IOW('#', 0x0b, struct fw_cdev_stop_iso)
+
+/* available since kernel version 2.6.24 */
 #define FW_CDEV_IOC_GET_CYCLE_TIMER	_IOR('#', 0x0c, struct fw_cdev_get_cycle_timer)
 
+/* available since kernel version 2.6.30 */
+#define FW_CDEV_IOC_ALLOCATE_ISO_RESOURCE   _IOWR('#', 0x0d, struct fw_cdev_allocate_iso_resource)
+#define FW_CDEV_IOC_DEALLOCATE_ISO_RESOURCE _IOW('#', 0x0e, struct fw_cdev_deallocate)
+
 /* FW_CDEV_VERSION History
  *
  * 1	Feb 18, 2007:  Initial version.
@@ -284,9 +327,9 @@ struct fw_cdev_allocate {
 };
 
 /**
- * struct fw_cdev_deallocate - Free an address range allocation
- * @handle:	Handle to the address range, as returned by the kernel when the
- *		range was allocated
+ * struct fw_cdev_deallocate - Free a CSR address range or isochronous resource
+ * @handle:	Handle to the address range or iso resource, as returned by the
+ *		kernel when the range or resource was allocated
  */
 struct fw_cdev_deallocate {
 	__u32 handle;
@@ -479,4 +522,35 @@ struct fw_cdev_get_cycle_timer {
 	__u32 cycle_timer;
 };
 
+/**
+ * struct fw_cdev_allocate_iso_resource - Allocate a channel or bandwidth
+ * @closure:	Passed back to userspace in correponding iso resource events
+ * @channels:	Isochronous channels of which one is to be allocated
+ * @bandwidth:	Isochronous bandwidth units to be allocated
+ * @handle:	Handle to the allocation, written by the kernel
+ *
+ * The %FW_CDEV_IOC_ALLOCATE_ISO_RESOURCE ioctl initiates allocation of an
+ * isochronous channel and/or of isochronous bandwidth at the isochronous
+ * resource manager (IRM).  Only one of the channels specified in @channels is
+ * allocated.  An %FW_CDEV_EVENT_ISO_RESOURCE_ALLOCATED is sent after
+ * communication with the IRM, indicating success or failure in the event data.
+ * The kernel will automatically reallocate the resources after bus resets.
+ * Should a reallocation fail, an %FW_CDEV_EVENT_ISO_RESOURCE_DEALLOCATED event
+ * will be sent.  The kernel will also automatically deallocate the resources
+ * when the file descriptor is closed.
+ *
+ * @channels is a host-endian bitfield with the most significant bit
+ * representing channel 0 and the least significant bit representing channel 63:
+ * 1ULL << (63 - c)
+ *
+ * @bandwidth is expressed in bandwidth allocation units, i.e. the time to send
+ * one quadlet of data (payload or header data) at speed S1600.
+ */
+struct fw_cdev_allocate_iso_resource {
+	__u64 closure;
+	__u64 channels;
+	__u32 bandwidth;
+	__u32 handle;
+};
+
 #endif /* _LINUX_FIREWIRE_CDEV_H */
Index: linux/drivers/firewire/fw-cdev.c
===================================================================
--- linux.orig/drivers/firewire/fw-cdev.c
+++ linux/drivers/firewire/fw-cdev.c
@@ -24,6 +24,7 @@
 #include <linux/errno.h>
 #include <linux/firewire-cdev.h>
 #include <linux/idr.h>
+#include <linux/jiffies.h>
 #include <linux/kernel.h>
 #include <linux/kref.h>
 #include <linux/mm.h>
@@ -35,6 +36,7 @@
 #include <linux/time.h>
 #include <linux/vmalloc.h>
 #include <linux/wait.h>
+#include <linux/workqueue.h>
 
 #include <asm/system.h>
 #include <asm/uaccess.h>
@@ -114,6 +116,21 @@ struct descriptor_resource {
 	u32 data[0];
 };
 
+struct iso_resource {
+	struct client_resource resource;
+	struct client *client;
+	/* Schedule work and access todo only with client->lock held. */
+	struct delayed_work work;
+	enum {ISO_RES_ALLOC, ISO_RES_REALLOC, ISO_RES_DEALLOC,} todo;
+	int generation;
+	u64 channels;
+	s32 bandwidth;
+	struct iso_resource_event *e_alloc, *e_dealloc;
+};
+
+static void schedule_iso_resource(struct iso_resource *);
+static void release_iso_resource(struct client *, struct client_resource *);
+
 /*
  * dequeue_event() just kfree()'s the event, so the event has to be
  * the first field in a struct XYZ_event.
@@ -145,6 +162,11 @@ struct iso_interrupt_event {
 	struct fw_cdev_event_iso_interrupt interrupt;
 };
 
+struct iso_resource_event {
+	struct event event;
+	struct fw_cdev_event_iso_resource resource;
+};
+
 static inline void __user *u64_to_uptr(__u64 value)
 {
 	return (void __user *)(unsigned long)value;
@@ -290,6 +312,16 @@ static void for_each_client(struct fw_de
 	mutex_unlock(&device->client_list_mutex);
 }
 
+static int schedule_reallocations(int id, void *p, void *data)
+{
+	struct client_resource *r = p;
+
+	if (r->release == release_iso_resource)
+		schedule_iso_resource(container_of(r,
+					struct iso_resource, resource));
+	return 0;
+}
+
 static void queue_bus_reset_event(struct client *client)
 {
 	struct bus_reset_event *e;
@@ -304,6 +336,10 @@ static void queue_bus_reset_event(struct
 
 	queue_event(client, &e->event,
 		    &e->reset, sizeof(e->reset), NULL, 0);
+
+	spin_lock_irq(&client->lock);
+	idr_for_each(&client->resource_idr, schedule_reallocations, client);
+	spin_unlock_irq(&client->lock);
 }
 
 void fw_device_cdev_update(struct fw_device *device)
@@ -376,8 +412,12 @@ static int add_client_resource(struct cl
 	else
 		ret = idr_get_new(&client->resource_idr, resource,
 				  &resource->handle);
-	if (ret >= 0)
+	if (ret >= 0) {
 		client_get(client);
+		if (resource->release == release_iso_resource)
+			schedule_iso_resource(container_of(resource,
+						struct iso_resource, resource));
+	}
 	spin_unlock_irqrestore(&client->lock, flags);
 
 	if (ret == -EAGAIN)
@@ -970,6 +1010,177 @@ static int ioctl_get_cycle_timer(struct 
 	return 0;
 }
 
+static void iso_resource_work(struct work_struct *work)
+{
+	struct iso_resource_event *e;
+	struct iso_resource *r =
+			container_of(work, struct iso_resource, work.work);
+	struct client *client = r->client;
+	int generation, channel, bandwidth, todo;
+	bool skip, free, success;
+
+	spin_lock_irq(&client->lock);
+	generation = client->device->generation;
+	todo = r->todo;
+	/* Allow 1000ms grace period for other reallocations. */
+	if (todo == ISO_RES_ALLOC &&
+	    time_is_after_jiffies(client->device->card->reset_jiffies + HZ)) {
+		if (schedule_delayed_work(&r->work, DIV_ROUND_UP(HZ, 3)))
+			client_get(client);
+		skip = true;
+	} else {
+		/* We could be called twice within the same generation. */
+		skip = todo == ISO_RES_REALLOC &&
+		       r->generation == generation;
+	}
+	free = todo == ISO_RES_DEALLOC;
+	r->generation = generation;
+	spin_unlock_irq(&client->lock);
+
+	if (skip)
+		goto out;
+
+	bandwidth = r->bandwidth;
+
+	fw_iso_resource_manage(client->device->card, generation,
+			r->channels, &channel, &bandwidth,
+			todo == ISO_RES_ALLOC || todo == ISO_RES_REALLOC);
+	/*
+	 * Is this generation outdated already?  As long as this resource sticks
+	 * in the idr, it will be scheduled again for a newer generation or at
+	 * shutdown.
+	 */
+	if (channel == -EAGAIN &&
+	    (todo == ISO_RES_ALLOC || todo == ISO_RES_REALLOC))
+		goto out;
+
+	success = channel >= 0 || bandwidth > 0;
+
+	spin_lock_irq(&client->lock);
+	/*
+	 * Transit from allocation to reallocation, except if the client
+	 * requested deallocation in the meantime.
+	 */
+	if (r->todo == ISO_RES_ALLOC)
+		r->todo = ISO_RES_REALLOC;
+	/*
+	 * Allocation or reallocation failure?  Pull this resource out of the
+	 * idr and prepare for deletion, unless the client is shutting down.
+	 */
+	if (r->todo == ISO_RES_REALLOC && !success &&
+	    !client->in_shutdown &&
+	    idr_find(&client->resource_idr, r->resource.handle)) {
+		idr_remove(&client->resource_idr, r->resource.handle);
+		client_put(client);
+		free = true;
+	}
+	spin_unlock_irq(&client->lock);
+
+	if (todo == ISO_RES_ALLOC && channel >= 0)
+		r->channels = 1ULL << (63 - channel);
+
+	if (todo == ISO_RES_REALLOC && success)
+		goto out;
+
+	if (todo == ISO_RES_ALLOC) {
+		e = r->e_alloc;
+		r->e_alloc = NULL;
+	} else {
+		e = r->e_dealloc;
+		r->e_dealloc = NULL;
+	}
+	e->resource.handle	= r->resource.handle;
+	e->resource.channel	= channel;
+	e->resource.bandwidth	= bandwidth;
+
+	queue_event(client, &e->event,
+		    &e->resource, sizeof(e->resource), NULL, 0);
+
+	if (free) {
+		cancel_delayed_work(&r->work);
+		kfree(r->e_alloc);
+		kfree(r->e_dealloc);
+		kfree(r);
+	}
+ out:
+	client_put(client);
+}
+
+static void schedule_iso_resource(struct iso_resource *r)
+{
+	if (schedule_delayed_work(&r->work, 0))
+		client_get(r->client);
+}
+
+static void release_iso_resource(struct client *client,
+				 struct client_resource *resource)
+{
+	struct iso_resource *r =
+		container_of(resource, struct iso_resource, resource);
+
+	spin_lock_irq(&client->lock);
+	r->todo = ISO_RES_DEALLOC;
+	schedule_iso_resource(r);
+	spin_unlock_irq(&client->lock);
+}
+
+static int ioctl_allocate_iso_resource(struct client *client, void *buffer)
+{
+	struct fw_cdev_allocate_iso_resource *request = buffer;
+	struct iso_resource_event *e1, *e2;
+	struct iso_resource *r;
+	int ret;
+
+	if ((request->channels == 0 && request->bandwidth == 0) ||
+	    request->bandwidth > BANDWIDTH_AVAILABLE_INITIAL ||
+	    request->bandwidth < 0)
+		return -EINVAL;
+
+	r  = kmalloc(sizeof(*r), GFP_KERNEL);
+	e1 = kmalloc(sizeof(*e1), GFP_KERNEL);
+	e2 = kmalloc(sizeof(*e2), GFP_KERNEL);
+	if (r == NULL || e1 == NULL || e2 == NULL) {
+		ret = -ENOMEM;
+		goto fail;
+	}
+
+	INIT_DELAYED_WORK(&r->work, iso_resource_work);
+	r->client	= client;
+	r->todo		= ISO_RES_ALLOC;
+	r->generation	= -1;
+	r->channels	= request->channels;
+	r->bandwidth	= request->bandwidth;
+	r->e_alloc	= e1;
+	r->e_dealloc	= e2;
+
+	e1->resource.closure	= request->closure;
+	e1->resource.type	= FW_CDEV_EVENT_ISO_RESOURCE_ALLOCATED;
+	e2->resource.closure	= request->closure;
+	e2->resource.type	= FW_CDEV_EVENT_ISO_RESOURCE_DEALLOCATED;
+
+	r->resource.release = release_iso_resource;
+	ret = add_client_resource(client, &r->resource, GFP_KERNEL);
+	if (ret < 0)
+		goto fail;
+	request->handle = r->resource.handle;
+
+	return 0;
+ fail:
+	kfree(r);
+	kfree(e1);
+	kfree(e2);
+
+	return ret;
+}
+
+static int ioctl_deallocate_iso_resource(struct client *client, void *buffer)
+{
+	struct fw_cdev_deallocate *request = buffer;
+
+	return release_client_resource(client, request->handle,
+				       release_iso_resource, NULL);
+}
+
 static int (* const ioctl_handlers[])(struct client *client, void *buffer) = {
 	ioctl_get_info,
 	ioctl_send_request,
@@ -984,6 +1195,8 @@ static int (* const ioctl_handlers[])(st
 	ioctl_start_iso,
 	ioctl_stop_iso,
 	ioctl_get_cycle_timer,
+	ioctl_allocate_iso_resource,
+	ioctl_deallocate_iso_resource,
 };
 
 static int dispatch_ioctl(struct client *client,
Index: linux/drivers/firewire/fw-iso.c
===================================================================
--- linux.orig/drivers/firewire/fw-iso.c
+++ linux/drivers/firewire/fw-iso.c
@@ -1,5 +1,7 @@
 /*
- * Isochronous IO functionality
+ * Isochronous I/O functionality:
+ *   - Isochronous DMA context management
+ *   - Isochronous bus resource management (channels, bandwidth), client side
  *
  * Copyright (C) 2006 Kristian Hoegsberg <krh@bitplanet.net>
  *
@@ -18,15 +20,20 @@
  * Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
  */
 
-#include <linux/kernel.h>
-#include <linux/module.h>
 #include <linux/dma-mapping.h>
-#include <linux/vmalloc.h>
+#include <linux/errno.h>
+#include <linux/firewire-constants.h>
+#include <linux/kernel.h>
 #include <linux/mm.h>
+#include <linux/spinlock.h>
+#include <linux/vmalloc.h>
 
-#include "fw-transaction.h"
 #include "fw-topology.h"
-#include "fw-device.h"
+#include "fw-transaction.h"
+
+/*
+ * Isochronous DMA context management
+ */
 
 int fw_iso_buffer_init(struct fw_iso_buffer *buffer, struct fw_card *card,
 		       int page_count, enum dma_data_direction direction)
@@ -153,3 +160,160 @@ int fw_iso_context_stop(struct fw_iso_co
 {
 	return ctx->card->driver->stop_iso(ctx);
 }
+
+/*
+ * Isochronous bus resource management (channels, bandwidth), client side
+ */
+
+static int manage_bandwidth(struct fw_card *card, int irm_id, int generation,
+			    int bandwidth, bool allocate)
+{
+	__be32 data[2];
+	int try, new, old = allocate ? BANDWIDTH_AVAILABLE_INITIAL : 0;
+
+	/*
+	 * On a 1394a IRM with low contention, try < 1 is enough.
+	 * On a 1394-1995 IRM, we need at least try < 2.
+	 * Let's just do try < 5.
+	 */
+	for (try = 0; try < 5; try++) {
+		new = allocate ? old - bandwidth : old + bandwidth;
+		if (new < 0 || new > BANDWIDTH_AVAILABLE_INITIAL)
+			break;
+
+		data[0] = cpu_to_be32(old);
+		data[1] = cpu_to_be32(new);
+		switch (fw_run_transaction(card, TCODE_LOCK_COMPARE_SWAP,
+				irm_id, generation, SCODE_100,
+				CSR_REGISTER_BASE + CSR_BANDWIDTH_AVAILABLE,
+				data, sizeof(data))) {
+		case RCODE_GENERATION:
+			/* A generation change frees all bandwidth. */
+			return allocate ? -EAGAIN : bandwidth;
+
+		case RCODE_COMPLETE:
+			if (be32_to_cpup(data) == old)
+				return bandwidth;
+
+			old = be32_to_cpup(data);
+			/* Fall through. */
+		}
+	}
+
+	return -EIO;
+}
+
+static int manage_channel(struct fw_card *card, int irm_id, int generation,
+			  __be32 channels_mask, u64 offset, bool allocate)
+{
+	__be32 data[2], c, old = allocate ? cpu_to_be32(~0) : 0;
+	int i, retry = 5;
+
+	for (i = 0; i < 32; i++) {
+		c = cpu_to_be32(1 << (31 - i));
+		if (!(channels_mask & c))
+			continue;
+
+		if (allocate == !(old & c))
+			continue;
+
+		data[0] = old;
+		data[1] = old ^ c;
+		switch (fw_run_transaction(card, TCODE_LOCK_COMPARE_SWAP,
+					   irm_id, generation, SCODE_100,
+					   offset, data, sizeof(data))) {
+		case RCODE_GENERATION:
+			/* A generation change frees all channels. */
+			return allocate ? -EAGAIN : i;
+
+		case RCODE_COMPLETE:
+			if (data[0] == old)
+				return i;
+
+			old = data[0];
+
+			/* Is the IRM 1394a-2000 compliant? */
+			if ((data[0] & c) != (data[1] & c))
+				continue;
+
+			/* 1394-1995 IRM, fall through to retry. */
+		default:
+			if (retry--)
+				i--;
+		}
+	}
+
+	return -EIO;
+}
+
+static void deallocate_channel(struct fw_card *card, int irm_id,
+			       int generation, int channel)
+{
+	__be32 mask;
+	u64 offset;
+
+	mask = channel < 32 ? cpu_to_be32(1 << (31 - channel)) :
+			      cpu_to_be32(1 << (63 - channel));
+	offset = channel < 32 ? CSR_REGISTER_BASE + CSR_CHANNELS_AVAILABLE_HI :
+				CSR_REGISTER_BASE + CSR_CHANNELS_AVAILABLE_LO;
+
+	manage_channel(card, irm_id, generation, mask, offset, false);
+}
+
+/**
+ * fw_iso_resource_manage - Allocate or deallocate a channel and/or bandwidth
+ *
+ * In parameters: card, generation, channels_mask, bandwidth, allocate
+ * Out parameters: channel, bandwidth
+ * This function blocks (sleeps) during communication with the IRM.
+ * Allocates or deallocates at most one channel out of channels_mask.
+ *
+ * Returns channel < 0 if no channel was allocated or deallocated.
+ * Returns bandwidth = 0 if no bandwidth was allocated or deallocated.
+ *
+ * If generation is stale, deallocations succeed but allocations fail with
+ * channel = -EAGAIN.
+ *
+ * If channel (de)allocation fails, bandwidth (de)allocation fails too.
+ * If bandwidth allocation fails, no channel will be allocated either.
+ * If bandwidth deallocation fails, channel deallocation may still have been
+ * successful.
+ */
+void fw_iso_resource_manage(struct fw_card *card, int generation,
+			    u64 channels_mask, int *channel, int *bandwidth,
+			    bool allocate)
+{
+	__be32 channels_hi = cpu_to_be32(channels_mask >> 32);
+	__be32 channels_lo = cpu_to_be32(channels_mask);
+	int irm_id, ret, c = -EINVAL;
+
+	spin_lock_irq(&card->lock);
+	irm_id = card->irm_node->node_id;
+	spin_unlock_irq(&card->lock);
+
+	if (channels_hi)
+		c = manage_channel(card, irm_id, generation, channels_hi,
+		    CSR_REGISTER_BASE + CSR_CHANNELS_AVAILABLE_HI, allocate);
+	if (channels_lo && c < 0) {
+		c = manage_channel(card, irm_id, generation, channels_lo,
+		    CSR_REGISTER_BASE + CSR_CHANNELS_AVAILABLE_LO, allocate);
+		if (c >= 0)
+			c += 32;
+	}
+	*channel = c;
+
+	if (channels_mask != 0 && c < 0)
+		*bandwidth = 0;
+
+	if (*bandwidth == 0)
+		return;
+
+	ret = manage_bandwidth(card, irm_id, generation, *bandwidth, allocate);
+	if (ret < 0)
+		*bandwidth = 0;
+
+	if (ret < 0 && c >= 0 && allocate) {
+		deallocate_channel(card, irm_id, generation, c);
+		*channel = ret;
+	}
+}
Index: linux/drivers/firewire/fw-transaction.h
===================================================================
--- linux.orig/drivers/firewire/fw-transaction.h
+++ linux/drivers/firewire/fw-transaction.h
@@ -82,6 +82,7 @@
 #define CSR_SPEED_MAP			0x2000
 #define CSR_SPEED_MAP_END		0x3000
 
+#define BANDWIDTH_AVAILABLE_INITIAL	4915
 #define BROADCAST_CHANNEL_INITIAL	(1 << 31 | 31)
 #define BROADCAST_CHANNEL_VALID		(1 << 30)
 
@@ -343,6 +344,9 @@ int fw_iso_context_start(struct fw_iso_c
 int fw_iso_context_stop(struct fw_iso_context *ctx);
 void fw_iso_context_destroy(struct fw_iso_context *ctx);
 
+void fw_iso_resource_manage(struct fw_card *card, int generation,
+		u64 channels_mask, int *channel, int *bandwidth, bool allocate);
+
 struct fw_card_driver {
 	/*
 	 * Enable the given card with the given initial config rom.

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


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

* [PATCH 06/11] firewire: cdev: add ioctls for manual iso resource management
  2009-01-04 15:23 [PATCH 00/11] firewire: cdev: proposed ABI extensions Stefan Richter
                   ` (4 preceding siblings ...)
  2009-01-04 15:26 ` [PATCH 05/11] firewire: cdev: add ioctls for isochronous resource management Stefan Richter
@ 2009-01-04 15:27 ` Stefan Richter
       [not found]   ` <1231643968.3538.59.camel@localhost.localdomain>
  2009-01-04 15:28 ` [PATCH 07/11] firewire: cdev: add ioctl to query maximum transmission speed Stefan Richter
                   ` (5 subsequent siblings)
  11 siblings, 1 reply; 17+ messages in thread
From: Stefan Richter @ 2009-01-04 15:27 UTC (permalink / raw)
  To: linux1394-devel; +Cc: linux-kernel

This adds ioctls for allocation and deallocation of a channel or/and
bandwidth without auto-reallocation and without auto-deallocation.

The benefit of these ioctls is that libraw1394-style isochronous
resource management can be implemented without write access to the IRM's
character device file.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
---
 drivers/firewire/fw-cdev.c    |   67 +++++++++++++++++++++++++++-------
 include/linux/firewire-cdev.h |   38 +++++++++++++++----
 2 files changed, 84 insertions(+), 21 deletions(-)

Index: linux/include/linux/firewire-cdev.h
===================================================================
--- linux.orig/include/linux/firewire-cdev.h
+++ linux/include/linux/firewire-cdev.h
@@ -151,7 +151,7 @@ struct fw_cdev_event_iso_interrupt {
 /**
  * struct fw_cdev_event_iso_resource - Iso resources were allocated or freed
  * @closure:	See &fw_cdev_event_common;
- *		set by %FW_CDEV_IOC_ALLOCATE_ISO_RESOURCE ioctl
+ *		set by %FW_CDEV_IOC_(DE)ALLOCATE_ISO_RESOURCE(_ONCE) ioctl
  * @type:	%FW_CDEV_EVENT_ISO_RESOURCE_ALLOCATED or
  *		%FW_CDEV_EVENT_ISO_RESOURCE_DEALLOCATED
  * @handle:	Reference by which an allocated resource can be deallocated
@@ -164,12 +164,12 @@ struct fw_cdev_event_iso_interrupt {
  * resource was allocated at the IRM.  The client has to check @channel and
  * @bandwidth for whether the allocation actually succeeded.
  *
- * @channel is <0 if no channel was allocated.
- * @bandwidth is 0 if no bandwidth was allocated.
- *
  * An %FW_CDEV_EVENT_ISO_RESOURCE_DEALLOCATED event is sent after an isochronous
  * resource was deallocated at the IRM.  It is also sent when automatic
  * reallocation after a bus reset failed.
+ *
+ * @channel is <0 if no channel was (de)allocated or if reallocation failed.
+ * @bandwidth is 0 if no bandwidth was (de)allocated or if reallocation failed.
  */
 struct fw_cdev_event_iso_resource {
 	__u64 closure;
@@ -227,6 +227,8 @@ union fw_cdev_event {
 /* available since kernel version 2.6.30 */
 #define FW_CDEV_IOC_ALLOCATE_ISO_RESOURCE   _IOWR('#', 0x0d, struct fw_cdev_allocate_iso_resource)
 #define FW_CDEV_IOC_DEALLOCATE_ISO_RESOURCE _IOW('#', 0x0e, struct fw_cdev_deallocate)
+#define FW_CDEV_IOC_ALLOCATE_ISO_RESOURCE_ONCE _IOW('#', 0x0f, struct fw_cdev_allocate_iso_resource)
+#define FW_CDEV_IOC_DEALLOCATE_ISO_RESOURCE_ONCE _IOW('#', 0x10, struct fw_cdev_allocate_iso_resource)
 
 /* FW_CDEV_VERSION History
  *
@@ -523,11 +525,12 @@ struct fw_cdev_get_cycle_timer {
 };
 
 /**
- * struct fw_cdev_allocate_iso_resource - Allocate a channel or bandwidth
+ * struct fw_cdev_allocate_iso_resource - (De)allocate a channel or bandwidth
  * @closure:	Passed back to userspace in correponding iso resource events
- * @channels:	Isochronous channels of which one is to be allocated
- * @bandwidth:	Isochronous bandwidth units to be allocated
- * @handle:	Handle to the allocation, written by the kernel
+ * @channels:	Isochronous channels of which one is to be (de)allocated
+ * @bandwidth:	Isochronous bandwidth units to be (de)allocated
+ * @handle:	Handle to the allocation, written by the kernel (only valid in
+ *		case of %FW_CDEV_IOC_ALLOCATE_ISO_RESOURCE ioctls)
  *
  * The %FW_CDEV_IOC_ALLOCATE_ISO_RESOURCE ioctl initiates allocation of an
  * isochronous channel and/or of isochronous bandwidth at the isochronous
@@ -539,6 +542,25 @@ struct fw_cdev_get_cycle_timer {
  * will be sent.  The kernel will also automatically deallocate the resources
  * when the file descriptor is closed.
  *
+ * The %FW_CDEV_IOC_DEALLOCATE_ISO_RESOURCE ioctl can be used to initiate
+ * deallocation of resources which were allocated as described above.
+ * An %FW_CDEV_EVENT_ISO_RESOURCE_DEALLOCATED event concludes this operation.
+ *
+ * The %FW_CDEV_IOC_ALLOCATE_ISO_RESOURCE_ONCE ioctl is a variant of allocation
+ * without automatic re- or deallocation.
+ * An %FW_CDEV_EVENT_ISO_RESOURCE_ALLOCATED event concludes this operation,
+ * indicating success or failure in its data.
+ *
+ * The %FW_CDEV_IOC_DEALLOCATE_ISO_RESOURCE_ONCE ioctl works like
+ * %FW_CDEV_IOC_ALLOCATE_ISO_RESOURCE_ONCE except that resources are freed
+ * instead of allocated.  At most one channel may be specified in this ioctl.
+ * An %FW_CDEV_EVENT_ISO_RESOURCE_DEALLOCATED event concludes this operation.
+ *
+ * To summarize, %FW_CDEV_IOC_DEALLOCATE_ISO_RESOURCE allocates iso resources
+ * for the lifetime of the fd or handle.
+ * In contrast, %FW_CDEV_IOC_ALLOCATE_ISO_RESOURCE_ONCE allocates iso resources
+ * for the duration of a bus generation.
+ *
  * @channels is a host-endian bitfield with the most significant bit
  * representing channel 0 and the least significant bit representing channel 63:
  * 1ULL << (63 - c)
Index: linux/drivers/firewire/fw-cdev.c
===================================================================
--- linux.orig/drivers/firewire/fw-cdev.c
+++ linux/drivers/firewire/fw-cdev.c
@@ -121,14 +121,15 @@ struct iso_resource {
 	struct client *client;
 	/* Schedule work and access todo only with client->lock held. */
 	struct delayed_work work;
-	enum {ISO_RES_ALLOC, ISO_RES_REALLOC, ISO_RES_DEALLOC,} todo;
+	enum {ISO_RES_ALLOC, ISO_RES_REALLOC, ISO_RES_DEALLOC,
+	      ISO_RES_ALLOC_ONCE, ISO_RES_DEALLOC_ONCE,} todo;
 	int generation;
 	u64 channels;
 	s32 bandwidth;
 	struct iso_resource_event *e_alloc, *e_dealloc;
 };
 
-static void schedule_iso_resource(struct iso_resource *);
+static int schedule_iso_resource(struct iso_resource *);
 static void release_iso_resource(struct client *, struct client_resource *);
 
 /*
@@ -1033,7 +1034,9 @@ static void iso_resource_work(struct wor
 		skip = todo == ISO_RES_REALLOC &&
 		       r->generation == generation;
 	}
-	free = todo == ISO_RES_DEALLOC;
+	free = todo == ISO_RES_DEALLOC ||
+	       todo == ISO_RES_ALLOC_ONCE ||
+	       todo == ISO_RES_DEALLOC_ONCE;
 	r->generation = generation;
 	spin_unlock_irq(&client->lock);
 
@@ -1044,7 +1047,9 @@ static void iso_resource_work(struct wor
 
 	fw_iso_resource_manage(client->device->card, generation,
 			r->channels, &channel, &bandwidth,
-			todo == ISO_RES_ALLOC || todo == ISO_RES_REALLOC);
+			todo == ISO_RES_ALLOC ||
+			todo == ISO_RES_REALLOC ||
+			todo == ISO_RES_ALLOC_ONCE);
 	/*
 	 * Is this generation outdated already?  As long as this resource sticks
 	 * in the idr, it will be scheduled again for a newer generation or at
@@ -1082,7 +1087,7 @@ static void iso_resource_work(struct wor
 	if (todo == ISO_RES_REALLOC && success)
 		goto out;
 
-	if (todo == ISO_RES_ALLOC) {
+	if (todo == ISO_RES_ALLOC || todo == ISO_RES_ALLOC_ONCE) {
 		e = r->e_alloc;
 		r->e_alloc = NULL;
 	} else {
@@ -1106,10 +1111,17 @@ static void iso_resource_work(struct wor
 	client_put(client);
 }
 
-static void schedule_iso_resource(struct iso_resource *r)
+static int schedule_iso_resource(struct iso_resource *r)
 {
-	if (schedule_delayed_work(&r->work, 0))
-		client_get(r->client);
+	int scheduled;
+
+	client_get(r->client);
+
+	scheduled = schedule_delayed_work(&r->work, 0);
+	if (!scheduled)
+		client_put(r->client);
+
+	return scheduled;
 }
 
 static void release_iso_resource(struct client *client,
@@ -1124,9 +1136,9 @@ static void release_iso_resource(struct 
 	spin_unlock_irq(&client->lock);
 }
 
-static int ioctl_allocate_iso_resource(struct client *client, void *buffer)
+static int init_iso_resource(struct client *client,
+		struct fw_cdev_allocate_iso_resource *request, int todo)
 {
-	struct fw_cdev_allocate_iso_resource *request = buffer;
 	struct iso_resource_event *e1, *e2;
 	struct iso_resource *r;
 	int ret;
@@ -1146,7 +1158,7 @@ static int ioctl_allocate_iso_resource(s
 
 	INIT_DELAYED_WORK(&r->work, iso_resource_work);
 	r->client	= client;
-	r->todo		= ISO_RES_ALLOC;
+	r->todo		= todo;
 	r->generation	= -1;
 	r->channels	= request->channels;
 	r->bandwidth	= request->bandwidth;
@@ -1158,8 +1170,14 @@ static int ioctl_allocate_iso_resource(s
 	e2->resource.closure	= request->closure;
 	e2->resource.type	= FW_CDEV_EVENT_ISO_RESOURCE_DEALLOCATED;
 
-	r->resource.release = release_iso_resource;
-	ret = add_client_resource(client, &r->resource, GFP_KERNEL);
+	if (todo == ISO_RES_ALLOC) {
+		r->resource.release = release_iso_resource;
+		ret = add_client_resource(client, &r->resource, GFP_KERNEL);
+	} else {
+		r->resource.release = NULL;
+		r->resource.handle = -1;
+		ret = schedule_iso_resource(r) ? 0 : -ENOMEM;
+	}
 	if (ret < 0)
 		goto fail;
 	request->handle = r->resource.handle;
@@ -1173,6 +1191,13 @@ static int ioctl_allocate_iso_resource(s
 	return ret;
 }
 
+static int ioctl_allocate_iso_resource(struct client *client, void *buffer)
+{
+	struct fw_cdev_allocate_iso_resource *request = buffer;
+
+	return init_iso_resource(client, request, ISO_RES_ALLOC);
+}
+
 static int ioctl_deallocate_iso_resource(struct client *client, void *buffer)
 {
 	struct fw_cdev_deallocate *request = buffer;
@@ -1181,6 +1206,20 @@ static int ioctl_deallocate_iso_resource
 				       release_iso_resource, NULL);
 }
 
+static int ioctl_allocate_iso_resource_once(struct client *client, void *buffer)
+{
+	struct fw_cdev_allocate_iso_resource *request = buffer;
+
+	return init_iso_resource(client, request, ISO_RES_ALLOC_ONCE);
+}
+
+static int ioctl_deallocate_iso_resource_once(struct client *client, void *buffer)
+{
+	struct fw_cdev_allocate_iso_resource *request = buffer;
+
+	return init_iso_resource(client, request, ISO_RES_DEALLOC_ONCE);
+}
+
 static int (* const ioctl_handlers[])(struct client *client, void *buffer) = {
 	ioctl_get_info,
 	ioctl_send_request,
@@ -1197,6 +1236,8 @@ static int (* const ioctl_handlers[])(st
 	ioctl_get_cycle_timer,
 	ioctl_allocate_iso_resource,
 	ioctl_deallocate_iso_resource,
+	ioctl_allocate_iso_resource_once,
+	ioctl_deallocate_iso_resource_once,
 };
 
 static int dispatch_ioctl(struct client *client,

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


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

* [PATCH 07/11] firewire: cdev: add ioctl to query maximum transmission speed
  2009-01-04 15:23 [PATCH 00/11] firewire: cdev: proposed ABI extensions Stefan Richter
                   ` (5 preceding siblings ...)
  2009-01-04 15:27 ` [PATCH 06/11] firewire: cdev: add ioctls for manual iso " Stefan Richter
@ 2009-01-04 15:28 ` Stefan Richter
  2009-01-04 15:29 ` [PATCH 08/11] firewire: cdev: add ioctl for broadcast write requests Stefan Richter
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 17+ messages in thread
From: Stefan Richter @ 2009-01-04 15:28 UTC (permalink / raw)
  To: linux1394-devel; +Cc: linux-kernel

While the speed of asynchronous transactions is automatically chosen by
the kernel, the speed of isochronous streams has to be chosen by the
initiating client.

In case of 1394a bus topologies, the maximum possible speed could be
figured out with some effort by evaluation of the remote node's link
speed field in the config ROM, the local node's link speed field, and
the PHY speeds and topologic information in the local node's or IRM's
topology map CSR.  However, this does not work in case of 1394b buses.

Hence add an ioctl to export the maximum speed which the kernel already
determined.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
---
 drivers/firewire/fw-cdev.c    |   10 ++++++++++
 include/linux/firewire-cdev.h |   10 ++++++++++
 2 files changed, 20 insertions(+)

Index: linux/include/linux/firewire-cdev.h
===================================================================
--- linux.orig/include/linux/firewire-cdev.h
+++ linux/include/linux/firewire-cdev.h
@@ -229,6 +229,7 @@ union fw_cdev_event {
 #define FW_CDEV_IOC_DEALLOCATE_ISO_RESOURCE      _IOW('#', 0x0e, struct fw_cdev_deallocate)
 #define FW_CDEV_IOC_ALLOCATE_ISO_RESOURCE_ONCE   _IOW('#', 0x0f, struct fw_cdev_allocate_iso_resource)
 #define FW_CDEV_IOC_DEALLOCATE_ISO_RESOURCE_ONCE _IOW('#', 0x10, struct fw_cdev_allocate_iso_resource)
+#define FW_CDEV_IOC_GET_SPEED                    _IOR('#', 0x11, struct fw_cdev_get_speed)
 
 /* FW_CDEV_VERSION History
  *
@@ -575,4 +576,13 @@ struct fw_cdev_allocate_iso_resource {
 	__u32 handle;
 };
 
+/**
+ * struct fw_cdev_get_speed - Query maximum speed to or from this device
+ * @max_speed:	Speed code; minimum of the device's link speed, the local node's
+ *		link speed, and all PHY port speeds between the two links
+ */
+struct fw_cdev_get_speed {
+	__u32 max_speed;
+};
+
 #endif /* _LINUX_FIREWIRE_CDEV_H */
Index: linux/drivers/firewire/fw-cdev.c
===================================================================
--- linux.orig/drivers/firewire/fw-cdev.c
+++ linux/drivers/firewire/fw-cdev.c
@@ -1220,6 +1220,15 @@ static int ioctl_deallocate_iso_resource
 	return init_iso_resource(client, request, ISO_RES_DEALLOC_ONCE);
 }
 
+static int ioctl_get_speed(struct client *client, void *buffer)
+{
+	struct fw_cdev_get_speed *request = buffer;
+
+	request->max_speed = client->device->max_speed;
+
+	return 0;
+}
+
 static int (* const ioctl_handlers[])(struct client *client, void *buffer) = {
 	ioctl_get_info,
 	ioctl_send_request,
@@ -1238,6 +1247,7 @@ static int (* const ioctl_handlers[])(st
 	ioctl_deallocate_iso_resource,
 	ioctl_allocate_iso_resource_once,
 	ioctl_deallocate_iso_resource_once,
+	ioctl_get_speed,
 };
 
 static int dispatch_ioctl(struct client *client,

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


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

* [PATCH 08/11] firewire: cdev: add ioctl for broadcast write requests
  2009-01-04 15:23 [PATCH 00/11] firewire: cdev: proposed ABI extensions Stefan Richter
                   ` (6 preceding siblings ...)
  2009-01-04 15:28 ` [PATCH 07/11] firewire: cdev: add ioctl to query maximum transmission speed Stefan Richter
@ 2009-01-04 15:29 ` Stefan Richter
  2009-01-04 15:30 ` [PATCH 09/11] firewire: cdev: restrict broadcast write requests to Units Space Stefan Richter
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 17+ messages in thread
From: Stefan Richter @ 2009-01-04 15:29 UTC (permalink / raw)
  To: linux1394-devel; +Cc: linux-kernel

Write transactions to the broadcast node ID are a convenient way to
trigger functions of multiple nodes at once.  IIDC is a protocol which
can make use of this if multiple cameras with same command_regs_base are
connected at the same bus.

Based on
    Date: Wed, 10 Sep 2008 11:32:16 -0400
    From: Jay Fenlason <fenlason@redhat.com>
    Subject: [patch] SEND_BROADCAST_REQUEST
Changes:  ioctl_send_request() and ioctl_send_broadcast_request() now
share code.  Broadcast speed corrected to S100.  Check for proper tcode.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
---
 drivers/firewire/fw-cdev.c    |   74 +++++++++++++++++++++-------------
 include/linux/firewire-cdev.h |    1 
 2 files changed, 48 insertions(+), 27 deletions(-)

Index: linux/include/linux/firewire-cdev.h
===================================================================
--- linux.orig/include/linux/firewire-cdev.h
+++ linux/include/linux/firewire-cdev.h
@@ -230,6 +230,7 @@ union fw_cdev_event {
 #define FW_CDEV_IOC_ALLOCATE_ISO_RESOURCE_ONCE   _IOW('#', 0x0f, struct fw_cdev_allocate_iso_resource)
 #define FW_CDEV_IOC_DEALLOCATE_ISO_RESOURCE_ONCE _IOW('#', 0x10, struct fw_cdev_allocate_iso_resource)
 #define FW_CDEV_IOC_GET_SPEED                    _IOR('#', 0x11, struct fw_cdev_get_speed)
+#define FW_CDEV_IOC_SEND_BROADCAST_REQUEST       _IOW('#', 0x12, struct fw_cdev_send_request)
 
 /* FW_CDEV_VERSION History
  *
Index: linux/drivers/firewire/fw-cdev.c
===================================================================
--- linux.orig/drivers/firewire/fw-cdev.c
+++ linux/drivers/firewire/fw-cdev.c
@@ -518,10 +518,10 @@ static void complete_transaction(struct 
 	client_put(client);
 }
 
-static int ioctl_send_request(struct client *client, void *buffer)
+static int init_request(struct client *client,
+			struct fw_cdev_send_request *request,
+			int destination_id, int speed)
 {
-	struct fw_device *device = client->device;
-	struct fw_cdev_send_request *request = buffer;
 	struct outbound_transaction_event *e;
 	int ret;
 
@@ -544,24 +544,6 @@ static int ioctl_send_request(struct cli
 		goto failed;
 	}
 
-	switch (request->tcode) {
-	case TCODE_WRITE_QUADLET_REQUEST:
-	case TCODE_WRITE_BLOCK_REQUEST:
-	case TCODE_READ_QUADLET_REQUEST:
-	case TCODE_READ_BLOCK_REQUEST:
-	case TCODE_LOCK_MASK_SWAP:
-	case TCODE_LOCK_COMPARE_SWAP:
-	case TCODE_LOCK_FETCH_ADD:
-	case TCODE_LOCK_LITTLE_ADD:
-	case TCODE_LOCK_BOUNDED_ADD:
-	case TCODE_LOCK_WRAP_ADD:
-	case TCODE_LOCK_VENDOR_DEPENDENT:
-		break;
-	default:
-		ret = -EINVAL;
-		goto failed;
-	}
-
 	e->r.resource.release = release_transaction;
 	ret = add_client_resource(client, &e->r.resource, GFP_KERNEL);
 	if (ret < 0)
@@ -570,12 +552,9 @@ static int ioctl_send_request(struct cli
 	/* Get a reference for the transaction callback */
 	client_get(client);
 
-	fw_send_request(device->card, &e->r.transaction,
-			request->tcode & 0x1f,
-			device->node->node_id,
-			request->generation,
-			device->max_speed,
-			request->offset,
+	fw_send_request(client->device->card, &e->r.transaction,
+			request->tcode & 0x1f, destination_id,
+			request->generation, speed, request->offset,
 			e->response.data, request->length,
 			complete_transaction, e);
 
@@ -589,6 +568,31 @@ static int ioctl_send_request(struct cli
 	return ret;
 }
 
+static int ioctl_send_request(struct client *client, void *buffer)
+{
+	struct fw_cdev_send_request *request = buffer;
+
+	switch (request->tcode) {
+	case TCODE_WRITE_QUADLET_REQUEST:
+	case TCODE_WRITE_BLOCK_REQUEST:
+	case TCODE_READ_QUADLET_REQUEST:
+	case TCODE_READ_BLOCK_REQUEST:
+	case TCODE_LOCK_MASK_SWAP:
+	case TCODE_LOCK_COMPARE_SWAP:
+	case TCODE_LOCK_FETCH_ADD:
+	case TCODE_LOCK_LITTLE_ADD:
+	case TCODE_LOCK_BOUNDED_ADD:
+	case TCODE_LOCK_WRAP_ADD:
+	case TCODE_LOCK_VENDOR_DEPENDENT:
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return init_request(client, request, client->device->node->node_id,
+			    client->device->max_speed);
+}
+
 static void release_request(struct client *client,
 			    struct client_resource *resource)
 {
@@ -1229,6 +1233,21 @@ static int ioctl_get_speed(struct client
 	return 0;
 }
 
+static int ioctl_send_broadcast_request(struct client *client, void *buffer)
+{
+	struct fw_cdev_send_request *request = buffer;
+
+	switch (request->tcode) {
+	case TCODE_WRITE_QUADLET_REQUEST:
+	case TCODE_WRITE_BLOCK_REQUEST:
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return init_request(client, request, LOCAL_BUS | 0x3f, SCODE_100);
+}
+
 static int (* const ioctl_handlers[])(struct client *client, void *buffer) = {
 	ioctl_get_info,
 	ioctl_send_request,
@@ -1248,6 +1267,7 @@ static int (* const ioctl_handlers[])(st
 	ioctl_allocate_iso_resource_once,
 	ioctl_deallocate_iso_resource_once,
 	ioctl_get_speed,
+	ioctl_send_broadcast_request,
 };
 
 static int dispatch_ioctl(struct client *client,

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


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

* [PATCH 09/11] firewire: cdev: restrict broadcast write requests to Units Space
  2009-01-04 15:23 [PATCH 00/11] firewire: cdev: proposed ABI extensions Stefan Richter
                   ` (7 preceding siblings ...)
  2009-01-04 15:29 ` [PATCH 08/11] firewire: cdev: add ioctl for broadcast write requests Stefan Richter
@ 2009-01-04 15:30 ` Stefan Richter
  2009-01-04 15:30 ` [PATCH 10/11] firewire: cdev: extend transaction payload size check Stefan Richter
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 17+ messages in thread
From: Stefan Richter @ 2009-01-04 15:30 UTC (permalink / raw)
  To: linux1394-devel; +Cc: linux-kernel

We don't want random users write to Memory Space (e.g. PCs with physical
DMA filters down) or to core CSRs like Reset_Start.

This does not protect SBP-2 target CSRs.  But properly behaving SBP-2
targets ignore broadcast write requests to these registers, and the
maximum damage which can happen with laxer targets is DOS.  But there
are ways to create DOS situations anyway if there are devices with weak
device file permissions (like audio/video devices) present at the same
bus as an SBP-2 target.

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

Index: linux/drivers/firewire/fw-cdev.c
===================================================================
--- linux.orig/drivers/firewire/fw-cdev.c
+++ linux/drivers/firewire/fw-cdev.c
@@ -1245,6 +1245,10 @@ static int ioctl_send_broadcast_request(
 		return -EINVAL;
 	}
 
+	/* Security policy: Only allow accesses to Units Space. */
+	if (request->offset < CSR_REGISTER_BASE + CSR_CONFIG_ROM_END)
+		return -EACCES;
+
 	return init_request(client, request, LOCAL_BUS | 0x3f, SCODE_100);
 }
 

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


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

* [PATCH 10/11] firewire: cdev: extend transaction payload size check
  2009-01-04 15:23 [PATCH 00/11] firewire: cdev: proposed ABI extensions Stefan Richter
                   ` (8 preceding siblings ...)
  2009-01-04 15:30 ` [PATCH 09/11] firewire: cdev: restrict broadcast write requests to Units Space Stefan Richter
@ 2009-01-04 15:30 ` Stefan Richter
  2009-01-04 15:31 ` [PATCH 11/11] firewire: cdev: replace some spin_lock_irqsave by spin_lock_irq Stefan Richter
  2009-01-04 23:28 ` [PATCH 00/11] firewire: cdev: proposed ABI extensions Stefan Richter
  11 siblings, 0 replies; 17+ messages in thread
From: Stefan Richter @ 2009-01-04 15:30 UTC (permalink / raw)
  To: linux1394-devel; +Cc: linux-kernel

Make the size check of ioctl_send_request and
ioctl_send_broadcast_request speed dependent.  Also change the error
return code from -EINVAL to -EIO to distinguish this from other errors
concerning the ioctl parameters.

Another payload size limit for which we don't check here though is the
remote node's Bus_Info_Block.max_rec.

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

Index: linux/drivers/firewire/fw-cdev.c
===================================================================
--- linux.orig/drivers/firewire/fw-cdev.c
+++ linux/drivers/firewire/fw-cdev.c
@@ -525,9 +525,8 @@ static int init_request(struct client *c
 	struct outbound_transaction_event *e;
 	int ret;
 
-	/* What is the biggest size we'll accept, really? */
-	if (request->length > 4096)
-		return -EINVAL;
+	if (request->length > 4096 || request->length > 512 << speed)
+		return -EIO;
 
 	e = kmalloc(sizeof(*e) + request->length, GFP_KERNEL);
 	if (e == NULL)

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


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

* [PATCH 11/11] firewire: cdev: replace some spin_lock_irqsave by spin_lock_irq
  2009-01-04 15:23 [PATCH 00/11] firewire: cdev: proposed ABI extensions Stefan Richter
                   ` (9 preceding siblings ...)
  2009-01-04 15:30 ` [PATCH 10/11] firewire: cdev: extend transaction payload size check Stefan Richter
@ 2009-01-04 15:31 ` Stefan Richter
  2009-01-04 23:28 ` [PATCH 00/11] firewire: cdev: proposed ABI extensions Stefan Richter
  11 siblings, 0 replies; 17+ messages in thread
From: Stefan Richter @ 2009-01-04 15:31 UTC (permalink / raw)
  To: linux1394-devel; +Cc: linux-kernel

All of these functions are entered with IRQs enabled.
Hence the unconditional spin_unlock_irq can be used.

Function:                  Caller context:
    dequeue_event()            client process, via read(2)
    fill_bus_reset_event()     fw-device.c update worqueue job
    release_client_resource()  client process, via ioctl(2)
    fw_device_op_release()     client process, via close(2)

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

Index: linux/drivers/firewire/fw-cdev.c
===================================================================
--- linux.orig/drivers/firewire/fw-cdev.c
+++ linux/drivers/firewire/fw-cdev.c
@@ -237,7 +237,6 @@ static void queue_event(struct client *c
 static int dequeue_event(struct client *client,
 			 char __user *buffer, size_t count)
 {
-	unsigned long flags;
 	struct event *event;
 	size_t size, total;
 	int i, ret;
@@ -252,10 +251,10 @@ static int dequeue_event(struct client *
 		       fw_device_is_shutdown(client->device))
 		return -ENODEV;
 
-	spin_lock_irqsave(&client->lock, flags);
+	spin_lock_irq(&client->lock);
 	event = list_first_entry(&client->event_list, struct event, link);
 	list_del(&event->link);
-	spin_unlock_irqrestore(&client->lock, flags);
+	spin_unlock_irq(&client->lock);
 
 	total = 0;
 	for (i = 0; i < ARRAY_SIZE(event->v) && total < count; i++) {
@@ -286,9 +285,8 @@ static void fill_bus_reset_event(struct 
 				 struct client *client)
 {
 	struct fw_card *card = client->device->card;
-	unsigned long flags;
 
-	spin_lock_irqsave(&card->lock, flags);
+	spin_lock_irq(&card->lock);
 
 	event->closure	     = client->bus_reset_closure;
 	event->type          = FW_CDEV_EVENT_BUS_RESET;
@@ -299,7 +297,7 @@ static void fill_bus_reset_event(struct 
 	event->irm_node_id   = card->irm_node->node_id;
 	event->root_node_id  = card->root_node->node_id;
 
-	spin_unlock_irqrestore(&card->lock, flags);
+	spin_unlock_irq(&card->lock);
 }
 
 static void for_each_client(struct fw_device *device,
@@ -432,16 +430,15 @@ static int release_client_resource(struc
 				   struct client_resource **resource)
 {
 	struct client_resource *r;
-	unsigned long flags;
 
-	spin_lock_irqsave(&client->lock, flags);
+	spin_lock_irq(&client->lock);
 	if (client->in_shutdown)
 		r = NULL;
 	else
 		r = idr_find(&client->resource_idr, handle);
 	if (r && r->release == release)
 		idr_remove(&client->resource_idr, handle);
-	spin_unlock_irqrestore(&client->lock, flags);
+	spin_unlock_irq(&client->lock);
 
 	if (!(r && r->release == release))
 		return -EINVAL;
@@ -1384,7 +1381,6 @@ static int fw_device_op_release(struct i
 {
 	struct client *client = file->private_data;
 	struct event *e, *next_e;
-	unsigned long flags;
 
 	mutex_lock(&client->device->client_list_mutex);
 	list_del(&client->link);
@@ -1397,9 +1393,9 @@ static int fw_device_op_release(struct i
 		fw_iso_context_destroy(client->iso_context);
 
 	/* Freeze client->resource_idr and client->event_list */
-	spin_lock_irqsave(&client->lock, flags);
+	spin_lock_irq(&client->lock);
 	client->in_shutdown = true;
-	spin_unlock_irqrestore(&client->lock, flags);
+	spin_unlock_irq(&client->lock);
 
 	idr_for_each(&client->resource_idr, shutdown_resource, client);
 	idr_remove_all(&client->resource_idr);

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


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

* Re: [PATCH 00/11] firewire: cdev: proposed ABI extensions
  2009-01-04 15:23 [PATCH 00/11] firewire: cdev: proposed ABI extensions Stefan Richter
                   ` (10 preceding siblings ...)
  2009-01-04 15:31 ` [PATCH 11/11] firewire: cdev: replace some spin_lock_irqsave by spin_lock_irq Stefan Richter
@ 2009-01-04 23:28 ` Stefan Richter
  11 siblings, 0 replies; 17+ messages in thread
From: Stefan Richter @ 2009-01-04 23:28 UTC (permalink / raw)
  To: linux1394-devel; +Cc: linux-kernel

I wrote:
> These patches apply after several as yet uncommitted firewire patches,

I committed the prerequisites to linux1394-2.6.git now, branch: cdev.
I also committed all the new patches to branch: test, for those who'd
rather look through git or gitweb at the patches.

The linux1394-2.6.git branches misc, cdev, topology, for-next, and
master have been rebased onto v2.6.28.
-- 
Stefan Richter
-=====-==--= ---= --=-=
http://arcgraph.de/sr/

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

* [PATCH] firewire: cdev: add ioctls for iso resource management, amendment
       [not found]       ` <496648EC.3060806@s5r6.in-berlin.de>
@ 2009-01-08 22:07         ` Stefan Richter
  0 siblings, 0 replies; 17+ messages in thread
From: Stefan Richter @ 2009-01-08 22:07 UTC (permalink / raw)
  To: linux1394-devel; +Cc: linux-kernel, David Moore

Some fixes:
  - Remove stale documentation.
  - Fix a != vs. == thinko that got in the way of channel management.
  - Try bandwidth deallocation even if channel deallocation failed.

A simplification:
  - fw_cdev_allocate_iso_resource.channels is now ordered like
    libdc1394's dc1394_iso_allocate_channel() channels_allowed
    argument.

By the way, I looked closer at cards from NEC, TI, and VIA, and noticed
that they all don't implement IEEE 1394a behaviour which is meant to
deviate from IEEE 1212's notion of lock compare-swap.  This means that
we have to do two lock transactions instead of one in many cases where
one transaction would already succeed on a fully 1394a compliant IRM.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
---
 drivers/firewire/fw-cdev.c    |    2 -
 drivers/firewire/fw-iso.c     |   38 +++++++++++++++++++---------------
 include/linux/firewire-cdev.h |   10 +++-----
 3 files changed, 27 insertions(+), 23 deletions(-)

Index: linux/include/linux/firewire-cdev.h
===================================================================
--- linux.orig/include/linux/firewire-cdev.h
+++ linux/include/linux/firewire-cdev.h
@@ -174,8 +174,6 @@ struct fw_cdev_event_iso_interrupt {
  * @handle:	Reference by which an allocated resource can be deallocated
  * @channel:	Isochronous channel which was (de)allocated, if any
  * @bandwidth:	Bandwidth allocation units which were (de)allocated, if any
- * @channels_available:  Last known availability of channels
- * @bandwidth_available: Last known availability of bandwidth
  *
  * An %FW_CDEV_EVENT_ISO_RESOURCE_ALLOCATED event is sent after an isochronous
  * resource was allocated at the IRM.  The client has to check @channel and
@@ -580,7 +578,7 @@ struct fw_cdev_get_cycle_timer {
  *
  * The %FW_CDEV_IOC_DEALLOCATE_ISO_RESOURCE_ONCE ioctl works like
  * %FW_CDEV_IOC_ALLOCATE_ISO_RESOURCE_ONCE except that resources are freed
- * instead of allocated.  At most one channel may be specified in this ioctl.
+ * instead of allocated.
  * An %FW_CDEV_EVENT_ISO_RESOURCE_DEALLOCATED event concludes this operation.
  *
  * To summarize, %FW_CDEV_IOC_DEALLOCATE_ISO_RESOURCE allocates iso resources
@@ -588,9 +586,9 @@ struct fw_cdev_get_cycle_timer {
  * In contrast, %FW_CDEV_IOC_ALLOCATE_ISO_RESOURCE_ONCE allocates iso resources
  * for the duration of a bus generation.
  *
- * @channels is a host-endian bitfield with the most significant bit
- * representing channel 0 and the least significant bit representing channel 63:
- * 1ULL << (63 - c)
+ * @channels is a host-endian bitfield with the least significant bit
+ * representing channel 0 and the most significant bit representing channel 63:
+ * 1ULL << c for each channel c that is a candidate for (de)allocation.
  *
  * @bandwidth is expressed in bandwidth allocation units, i.e. the time to send
  * one quadlet of data (payload or header data) at speed S1600.
Index: linux/drivers/firewire/fw-iso.c
===================================================================
--- linux.orig/drivers/firewire/fw-iso.c
+++ linux/drivers/firewire/fw-iso.c
@@ -204,17 +204,19 @@ static int manage_bandwidth(struct fw_ca
 }
 
 static int manage_channel(struct fw_card *card, int irm_id, int generation,
-			  __be32 channels_mask, u64 offset, bool allocate)
+			  u32 channels_mask, u64 offset, bool allocate)
 {
-	__be32 data[2], c, old = allocate ? cpu_to_be32(~0) : 0;
+	__be32 data[2], c, all, old;
 	int i, retry = 5;
 
+	old = all = allocate ? cpu_to_be32(~0) : 0;
+
 	for (i = 0; i < 32; i++) {
-		c = cpu_to_be32(1 << (31 - i));
-		if (!(channels_mask & c))
+		if (!(channels_mask & 1 << i))
 			continue;
 
-		if (allocate == !(old & c))
+		c = cpu_to_be32(1 << (31 - i));
+		if ((old & c) != (all & c))
 			continue;
 
 		data[0] = old;
@@ -233,7 +235,7 @@ static int manage_channel(struct fw_card
 			old = data[0];
 
 			/* Is the IRM 1394a-2000 compliant? */
-			if ((data[0] & c) != (data[1] & c))
+			if ((data[0] & c) == (data[1] & c))
 				continue;
 
 			/* 1394-1995 IRM, fall through to retry. */
@@ -249,11 +251,10 @@ static int manage_channel(struct fw_card
 static void deallocate_channel(struct fw_card *card, int irm_id,
 			       int generation, int channel)
 {
-	__be32 mask;
+	u32 mask;
 	u64 offset;
 
-	mask = channel < 32 ? cpu_to_be32(1 << (31 - channel)) :
-			      cpu_to_be32(1 << (63 - channel));
+	mask = channel < 32 ? 1 << channel : 1 << (channel - 32);
 	offset = channel < 32 ? CSR_REGISTER_BASE + CSR_CHANNELS_AVAILABLE_HI :
 				CSR_REGISTER_BASE + CSR_CHANNELS_AVAILABLE_LO;
 
@@ -266,7 +267,12 @@ static void deallocate_channel(struct fw
  * In parameters: card, generation, channels_mask, bandwidth, allocate
  * Out parameters: channel, bandwidth
  * This function blocks (sleeps) during communication with the IRM.
+ *
  * Allocates or deallocates at most one channel out of channels_mask.
+ * channels_mask is a bitfield with MSB for channel 63 and LSB for channel 0.
+ * (Note, the IRM's CHANNELS_AVAILABLE is a big-endian bitfield with MSB for
+ * channel 0 and LSB for channel 63.)
+ * Allocates or deallocates as many bandwidth allocation units as specified.
  *
  * Returns channel < 0 if no channel was allocated or deallocated.
  * Returns bandwidth = 0 if no bandwidth was allocated or deallocated.
@@ -274,17 +280,17 @@ static void deallocate_channel(struct fw
  * If generation is stale, deallocations succeed but allocations fail with
  * channel = -EAGAIN.
  *
- * If channel (de)allocation fails, bandwidth (de)allocation fails too.
+ * If channel allocation fails, no bandwidth will be allocated either.
  * If bandwidth allocation fails, no channel will be allocated either.
- * If bandwidth deallocation fails, channel deallocation may still have been
- * successful.
+ * But deallocations of channel and bandwidth are tried independently
+ * of each other's success.
  */
 void fw_iso_resource_manage(struct fw_card *card, int generation,
 			    u64 channels_mask, int *channel, int *bandwidth,
 			    bool allocate)
 {
-	__be32 channels_hi = cpu_to_be32(channels_mask >> 32);
-	__be32 channels_lo = cpu_to_be32(channels_mask);
+	u32 channels_hi = channels_mask;	/* channels 31...0 */
+	u32 channels_lo = channels_mask >> 32;	/* channels 63...32 */
 	int irm_id, ret, c = -EINVAL;
 
 	spin_lock_irq(&card->lock);
@@ -302,7 +308,7 @@ void fw_iso_resource_manage(struct fw_ca
 	}
 	*channel = c;
 
-	if (channels_mask != 0 && c < 0)
+	if (allocate && channels_mask != 0 && c < 0)
 		*bandwidth = 0;
 
 	if (*bandwidth == 0)
@@ -312,7 +318,7 @@ void fw_iso_resource_manage(struct fw_ca
 	if (ret < 0)
 		*bandwidth = 0;
 
-	if (ret < 0 && c >= 0 && allocate) {
+	if (allocate && ret < 0 && c >= 0) {
 		deallocate_channel(card, irm_id, generation, c);
 		*channel = ret;
 	}
Index: linux/drivers/firewire/fw-cdev.c
===================================================================
--- linux.orig/drivers/firewire/fw-cdev.c
+++ linux/drivers/firewire/fw-cdev.c
@@ -1082,7 +1082,7 @@ static void iso_resource_work(struct wor
 	spin_unlock_irq(&client->lock);
 
 	if (todo == ISO_RES_ALLOC && channel >= 0)
-		r->channels = 1ULL << (63 - channel);
+		r->channels = 1ULL << channel;
 
 	if (todo == ISO_RES_REALLOC && success)
 		goto out;

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


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

* [PATCH] firewire: cdev: simplify a schedule_delayed_work wrapper
       [not found]       ` <4969CE1A.8010800@s5r6.in-berlin.de>
@ 2009-01-11 12:44         ` Stefan Richter
  0 siblings, 0 replies; 17+ messages in thread
From: Stefan Richter @ 2009-01-11 12:44 UTC (permalink / raw)
  To: linux1394-devel; +Cc: linux-kernel, David Moore

The kernel API documentation says that queue_delayed_work() returns 0
(only) if the work was already queued.  The return codes of
schedule_delayed_work() are not documented but the same.

In init_iso_resource(), the work has never been queued yet, hence we
can assume schedule_delayed_work() to be a guaranteed success there.

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

Index: linux/drivers/firewire/fw-cdev.c
===================================================================
--- linux.orig/drivers/firewire/fw-cdev.c
+++ linux/drivers/firewire/fw-cdev.c
@@ -129,7 +129,7 @@ struct iso_resource {
 	struct iso_resource_event *e_alloc, *e_dealloc;
 };
 
-static int schedule_iso_resource(struct iso_resource *);
+static void schedule_iso_resource(struct iso_resource *);
 static void release_iso_resource(struct client *, struct client_resource *);
 
 /*
@@ -1111,17 +1111,11 @@ static void iso_resource_work(struct wor
 	client_put(client);
 }
 
-static int schedule_iso_resource(struct iso_resource *r)
+static void schedule_iso_resource(struct iso_resource *r)
 {
-	int scheduled;
-
 	client_get(r->client);
-
-	scheduled = schedule_delayed_work(&r->work, 0);
-	if (!scheduled)
+	if (!schedule_delayed_work(&r->work, 0))
 		client_put(r->client);
-
-	return scheduled;
 }
 
 static void release_iso_resource(struct client *client,
@@ -1173,13 +1167,13 @@ static int init_iso_resource(struct clie
 	if (todo == ISO_RES_ALLOC) {
 		r->resource.release = release_iso_resource;
 		ret = add_client_resource(client, &r->resource, GFP_KERNEL);
+		if (ret < 0)
+			goto fail;
 	} else {
 		r->resource.release = NULL;
 		r->resource.handle = -1;
-		ret = schedule_iso_resource(r) ? 0 : -ENOMEM;
+		schedule_iso_resource(r);
 	}
-	if (ret < 0)
-		goto fail;
 	request->handle = r->resource.handle;
 
 	return 0;

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


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

* Re: [PATCH 01/11] firewire: cdev: reference-count client instances
  2009-01-04 15:24 ` [PATCH 01/11] firewire: cdev: reference-count client instances Stefan Richter
@ 2009-01-11 20:31   ` David Moore
  0 siblings, 0 replies; 17+ messages in thread
From: David Moore @ 2009-01-11 20:31 UTC (permalink / raw)
  To: Stefan Richter; +Cc: linux1394-devel, linux-kernel

On Sun, 2009-01-04 at 16:24 +0100, Stefan Richter wrote:
> The lifetime of struct client instances must be longer than the lifetime
> of any client resource.
> 
> This fixes a possible race between fw_device_op_release and transaction
> completions.  It also prepares for new ioctls for isochronous resource
> management which will involve delayed processing of client resources.
> 
> Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>

Reviewed-by: David Moore <dcm@acm.org>


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

* Re: [PATCH 05/11] firewire: cdev: add ioctls for isochronous resource management
  2009-01-04 15:26 ` [PATCH 05/11] firewire: cdev: add ioctls for isochronous resource management Stefan Richter
       [not found]   ` <1231404355.18613.68.camel@localhost.localdomain>
@ 2009-01-11 20:32   ` David Moore
  1 sibling, 0 replies; 17+ messages in thread
From: David Moore @ 2009-01-11 20:32 UTC (permalink / raw)
  To: Stefan Richter; +Cc: linux1394-devel, linux-kernel

On Sun, 2009-01-04 at 16:26 +0100, Stefan Richter wrote:
> Based on
>     Date: Tue, 18 Nov 2008 11:41:27 -0500
>     From: Jay Fenlason <fenlason@redhat.com>
>     Subject: [Patch V4] Add ISO resource management support
> with several changes to the ABI and implementation.  Only the part of
> the ABI which enables auto-reallocation and auto-deallocation is
> included here.
> 
> This implements ioctls for kernel-assisted allocation of isochronous
> channels and isochronous bandwidth.  The benefits are:
>   - The client does not have to have write access to the /dev/fw* device
>     corresponding to the IRM.
>   - The client does not have to perform reallocation after bus resets.
>   - Channel and bandwidth are deallocated by the kernel if the file is
>     closed before the client deallocated the resources.  Thus resources
>     are released even if the client crashes.
> 
> It is anticipated that future in-kernel code (firewire-core IRM code;
> the firewire port of firedtv), will use the fw-iso.c portions of this
> code too.
> 
> Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>

Tested-by: David Moore <dcm@acm.org>


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

end of thread, other threads:[~2009-01-11 20:32 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-01-04 15:23 [PATCH 00/11] firewire: cdev: proposed ABI extensions Stefan Richter
2009-01-04 15:24 ` [PATCH 01/11] firewire: cdev: reference-count client instances Stefan Richter
2009-01-11 20:31   ` David Moore
2009-01-04 15:25 ` [PATCH 02/11] firewire: cdev: unify names of struct types and of their instances Stefan Richter
2009-01-04 15:25 ` [PATCH 03/11] firewire: cdev: sort includes Stefan Richter
2009-01-04 15:26 ` [PATCH 04/11] firewire: core: topology header fix Stefan Richter
2009-01-04 15:26 ` [PATCH 05/11] firewire: cdev: add ioctls for isochronous resource management Stefan Richter
     [not found]   ` <1231404355.18613.68.camel@localhost.localdomain>
     [not found]     ` <4965D58E.1050606@s5r6.in-berlin.de>
     [not found]       ` <496648EC.3060806@s5r6.in-berlin.de>
2009-01-08 22:07         ` [PATCH] firewire: cdev: add ioctls for iso resource management, amendment Stefan Richter
2009-01-11 20:32   ` [PATCH 05/11] firewire: cdev: add ioctls for isochronous resource management David Moore
2009-01-04 15:27 ` [PATCH 06/11] firewire: cdev: add ioctls for manual iso " Stefan Richter
     [not found]   ` <1231643968.3538.59.camel@localhost.localdomain>
     [not found]     ` <1231656885.3538.67.camel@localhost.localdomain>
     [not found]       ` <4969CE1A.8010800@s5r6.in-berlin.de>
2009-01-11 12:44         ` [PATCH] firewire: cdev: simplify a schedule_delayed_work wrapper Stefan Richter
2009-01-04 15:28 ` [PATCH 07/11] firewire: cdev: add ioctl to query maximum transmission speed Stefan Richter
2009-01-04 15:29 ` [PATCH 08/11] firewire: cdev: add ioctl for broadcast write requests Stefan Richter
2009-01-04 15:30 ` [PATCH 09/11] firewire: cdev: restrict broadcast write requests to Units Space Stefan Richter
2009-01-04 15:30 ` [PATCH 10/11] firewire: cdev: extend transaction payload size check Stefan Richter
2009-01-04 15:31 ` [PATCH 11/11] firewire: cdev: replace some spin_lock_irqsave by spin_lock_irq Stefan Richter
2009-01-04 23:28 ` [PATCH 00/11] firewire: cdev: proposed ABI extensions Stefan Richter

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.