2010-07-16 20:23:42

by Stefan Richter

[permalink] [raw]
Subject: [PATCH 0/4] firewire: 2 simple updates + 2 ABI additions: PHY packet tx/rx

Coming as replies:

[PATCH 1/4] firewire: cdev: some clarifications to the API documentation
[PATCH 2/4] firewire: core: use C99 initializer in array of ioctl handlers
[PATCH 3/4 RFC] firewire: cdev: add PHY packet transmission
[PATCH 4/4 RFC] firewire: cdev: add PHY packet reception

drivers/firewire/core-card.c | 1
drivers/firewire/core-cdev.c | 179 ++++++++++++++++++++++++++++++------
drivers/firewire/core-transaction.c | 5 +
drivers/firewire/core.h | 2
drivers/firewire/ohci.c | 3
include/linux/firewire-cdev.h | 98 ++++++++++++++++++-
include/linux/firewire.h | 3
7 files changed, 254 insertions(+), 37 deletions(-)

A libraw1394 backend counterpart to patch 3/4 will be posted to
linux1394-devel. It can be tested with firecontrol.

A simple test tool for patch 4/4 will be posted to linux1394-devel too.
--
Stefan Richter
-=====-==-=- -=== =----
http://arcgraph.de/sr/


2010-07-16 20:24:16

by Stefan Richter

[permalink] [raw]
Subject: [PATCH 1/4] firewire: cdev: some clarifications to the API documentation

Response events:
- are generated on more occasions than their documentation claimed.

Bus resets:
- Note that FW_CDEV_IOC_INITIATE_BUS_RESET is nonblocking and that the
client is not required to observe a grace period since kernels
2.6.36+ will enforce it now (commit 02d37bed).

- The possible values of fw_cdev_initiate_bus_reset.type are listed in
the kerneldoc comment already.

- Clarify that an application that uses FW_CDEV_IOC_ADD_DESCRIPTOR and
FW_CDEV_IOC_REMOVE_DESCRIPTOR does not have to issue a bus reset.

Isochronous I/O contexts:
- At most one can be created per open file descriptor.

Signed-off-by: Stefan Richter <[email protected]>
---
include/linux/firewire-cdev.h | 29 ++++++++++++++++++++++-------
1 file changed, 22 insertions(+), 7 deletions(-)

Index: b/include/linux/firewire-cdev.h
===================================================================
--- a/include/linux/firewire-cdev.h
+++ b/include/linux/firewire-cdev.h
@@ -89,8 +89,9 @@ struct fw_cdev_event_bus_reset {

/**
* struct fw_cdev_event_response - Sent when a response packet was received
- * @closure: See &fw_cdev_event_common;
- * set by %FW_CDEV_IOC_SEND_REQUEST ioctl
+ * @closure: See &fw_cdev_event_common; set by %FW_CDEV_IOC_SEND_REQUEST
+ * or %FW_CDEV_IOC_SEND_BROADCAST_REQUEST
+ * or %FW_CDEV_IOC_SEND_STREAM_PACKET ioctl
* @type: See &fw_cdev_event_common; always %FW_CDEV_EVENT_RESPONSE
* @rcode: Response code returned by the remote node
* @length: Data length, i.e. the response's payload size in bytes
@@ -100,6 +101,11 @@ struct fw_cdev_event_bus_reset {
* sent by %FW_CDEV_IOC_SEND_REQUEST ioctl. The payload data for responses
* carrying data (read and lock responses) follows immediately and can be
* accessed through the @data field.
+ *
+ * The event is also generated after conclusions of transactions that do not
+ * involve response packets. This includes unified write transactions,
+ * broadcast write transactions, and transmission of asynchronous stream
+ * packets. @rcode indicates success or failure of such transmissions.
*/
struct fw_cdev_event_response {
__u64 closure;
@@ -480,9 +486,14 @@ struct fw_cdev_deallocate {
* Initiate a bus reset for the bus this device is on. The bus reset can be
* either the original (long) bus reset or the arbitrated (short) bus reset
* introduced in 1394a-2000.
+ *
+ * The ioctl returns immediately. A subsequent &fw_cdev_event_bus_reset
+ * indicates when the reset actually happened. Since ABI v4, this may be
+ * considerably later than the ioctl because the kernel ensures a grace period
+ * between subsequent bus resets as per IEEE 1394 bus management specification.
*/
struct fw_cdev_initiate_bus_reset {
- __u32 type; /* FW_CDEV_SHORT_RESET or FW_CDEV_LONG_RESET */
+ __u32 type;
};

/**
@@ -506,9 +517,10 @@ struct fw_cdev_initiate_bus_reset {
*
* @immediate, @key, and @data array elements are CPU-endian quadlets.
*
- * If successful, the kernel adds the descriptor and writes back a handle to the
- * kernel-side object to be used for later removal of the descriptor block and
- * immediate key.
+ * If successful, the kernel adds the descriptor and writes back a @handle to
+ * the kernel-side object to be used for later removal of the descriptor block
+ * and immediate key. The kernel will also generate a bus reset to signal the
+ * change of the configuration ROM to other nodes.
*
* This ioctl affects the configuration ROMs of all local nodes.
* The ioctl only succeeds on device files which represent a local node.
@@ -527,7 +539,8 @@ struct fw_cdev_add_descriptor {
* descriptor was added
*
* Remove a descriptor block and accompanying immediate key from the local
- * nodes' configuration ROMs.
+ * nodes' configuration ROMs. The kernel will also generate a bus reset to
+ * signal the change of the configuration ROM to other nodes.
*/
struct fw_cdev_remove_descriptor {
__u32 handle;
@@ -559,6 +572,8 @@ struct fw_cdev_remove_descriptor {
*
* Note that the effect of a @header_size > 4 depends on
* &fw_cdev_get_info.version, as documented at &fw_cdev_event_iso_interrupt.
+ *
+ * No more than one iso context can be created per fd.
*/
struct fw_cdev_create_iso_context {
__u32 type;

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

2010-07-16 20:24:43

by Stefan Richter

[permalink] [raw]
Subject: [PATCH 2/4] firewire: core: use C99 initializer in array of ioctl handlers

to make the correspondence of ioctl numbers and handlers more obvious.

Signed-off-by: Stefan Richter <[email protected]>
---
drivers/firewire/core-cdev.c | 42 +++++++++++++++++-----------------
1 file changed, 21 insertions(+), 21 deletions(-)

Index: b/drivers/firewire/core-cdev.c
===================================================================
--- a/drivers/firewire/core-cdev.c
+++ b/drivers/firewire/core-cdev.c
@@ -1386,27 +1386,27 @@ static int ioctl_send_stream_packet(stru
}

static int (* const ioctl_handlers[])(struct client *, union ioctl_arg *) = {
- ioctl_get_info,
- ioctl_send_request,
- ioctl_allocate,
- ioctl_deallocate,
- ioctl_send_response,
- ioctl_initiate_bus_reset,
- ioctl_add_descriptor,
- ioctl_remove_descriptor,
- ioctl_create_iso_context,
- ioctl_queue_iso,
- ioctl_start_iso,
- ioctl_stop_iso,
- ioctl_get_cycle_timer,
- ioctl_allocate_iso_resource,
- ioctl_deallocate_iso_resource,
- ioctl_allocate_iso_resource_once,
- ioctl_deallocate_iso_resource_once,
- ioctl_get_speed,
- ioctl_send_broadcast_request,
- ioctl_send_stream_packet,
- ioctl_get_cycle_timer2,
+ [0x00] = ioctl_get_info,
+ [0x01] = ioctl_send_request,
+ [0x02] = ioctl_allocate,
+ [0x03] = ioctl_deallocate,
+ [0x04] = ioctl_send_response,
+ [0x05] = ioctl_initiate_bus_reset,
+ [0x06] = ioctl_add_descriptor,
+ [0x07] = ioctl_remove_descriptor,
+ [0x08] = ioctl_create_iso_context,
+ [0x09] = ioctl_queue_iso,
+ [0x0a] = ioctl_start_iso,
+ [0x0b] = ioctl_stop_iso,
+ [0x0c] = ioctl_get_cycle_timer,
+ [0x0d] = ioctl_allocate_iso_resource,
+ [0x0e] = ioctl_deallocate_iso_resource,
+ [0x0f] = ioctl_allocate_iso_resource_once,
+ [0x10] = ioctl_deallocate_iso_resource_once,
+ [0x11] = ioctl_get_speed,
+ [0x12] = ioctl_send_broadcast_request,
+ [0x13] = ioctl_send_stream_packet,
+ [0x14] = ioctl_get_cycle_timer2,
};

static int dispatch_ioctl(struct client *client,

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

2010-07-16 20:25:27

by Stefan Richter

[permalink] [raw]
Subject: [PATCH 3/4 RFC] firewire: cdev: add PHY packet transmission

Add an FW_CDEV_IOC_SEND_PHY_PACKET ioctl() for /dev/fw* which can be
used to implement bus management related functionality in userspace.

This is also half of the functionality (the transmit part) that is
needed to support a userspace implementation of a VersaPHY transaction
layer.

Safety considerations:

- PHY packets are generally broadcasts and may have interesting
effects on PHYs and the bus, e.g. make asynchronous arbitration
impossible due to too low gap count. Hence some kind of elevated
privileges should be required of a process to be able to send
PHY packets. This implementation assumes that a process that is
allowed to open the /dev/fw* of a local node does have this
privilege.

There was an inconclusive discussion about introducing POSIX
capabilities as a means to check for user privileges for these
kinds of operations.

- The kernel does not check integrity of the supplied packet data.
That would be far too much code, considering the many kinds of
PHY packets. A process which got the privilege to send these
packets is trusted to do it correctly.

Just like with the other "send packet" ioctls, a non-blocking API is
chosen; i.e. the ioctl may return even before AT DMA started. After
transmission, an event for poll()/read() is enqueued. Most users are
going to need a blocking API, but a blocking userspace wrapper is easy
to implement, and the second of the two existing libraw1394 calls
raw1394_phy_packet_write() and raw1394_start_phy_packet_write() can be
better supported that way.

Signed-off-by: Stefan Richter <[email protected]>
---
drivers/firewire/core-cdev.c | 64 ++++++++++++++++++++++++++++++++++
include/linux/firewire-cdev.h | 44 +++++++++++++++++++++++
2 files changed, 107 insertions(+), 1 deletion(-)

Index: b/drivers/firewire/core-cdev.c
===================================================================
--- a/drivers/firewire/core-cdev.c
+++ b/drivers/firewire/core-cdev.c
@@ -195,6 +195,13 @@ struct iso_resource_event {
struct fw_cdev_event_iso_resource iso_resource;
};

+struct outbound_phy_packet_event {
+ struct event event;
+ struct client *client;
+ struct fw_packet p;
+ struct fw_cdev_event_phy_packet phy_packet;
+};
+
static inline void __user *u64_to_uptr(__u64 value)
{
return (void __user *)(unsigned long)value;
@@ -397,6 +404,7 @@ union ioctl_arg {
struct fw_cdev_allocate_iso_resource allocate_iso_resource;
struct fw_cdev_send_stream_packet send_stream_packet;
struct fw_cdev_get_cycle_timer2 get_cycle_timer2;
+ struct fw_cdev_send_phy_packet send_phy_packet;
};

static int ioctl_get_info(struct client *client, union ioctl_arg *arg)
@@ -1385,6 +1393,61 @@ static int ioctl_send_stream_packet(stru
return init_request(client, &request, dest, a->speed);
}

+static void outbound_phy_packet_callback(struct fw_packet *packet,
+ struct fw_card *card, int status)
+{
+ struct outbound_phy_packet_event *e =
+ container_of(packet, struct outbound_phy_packet_event, p);
+
+ switch (status) {
+ /* expected: */
+ case ACK_COMPLETE: e->phy_packet.rcode = RCODE_COMPLETE; break;
+ /* should never happen with PHY packets: */
+ case ACK_PENDING: e->phy_packet.rcode = RCODE_COMPLETE; break;
+ case ACK_BUSY_X:
+ case ACK_BUSY_A:
+ case ACK_BUSY_B: e->phy_packet.rcode = RCODE_BUSY; break;
+ case ACK_DATA_ERROR: e->phy_packet.rcode = RCODE_DATA_ERROR; break;
+ case ACK_TYPE_ERROR: e->phy_packet.rcode = RCODE_TYPE_ERROR; break;
+ /* stale generation; cancelled; on certain controllers: no ack */
+ default: e->phy_packet.rcode = status; break;
+ }
+
+ queue_event(e->client, &e->event,
+ &e->phy_packet, sizeof(e->phy_packet), NULL, 0);
+ client_put(e->client);
+}
+
+static int ioctl_send_phy_packet(struct client *client, union ioctl_arg *arg)
+{
+ struct fw_cdev_send_phy_packet *a = &arg->send_phy_packet;
+ struct fw_card *card = client->device->card;
+ struct outbound_phy_packet_event *e;
+
+ /* Access policy: Allow this ioctl only on local nodes' device files. */
+ if (!client->device->is_local)
+ return -ENOSYS;
+
+ e = kzalloc(sizeof(*e), GFP_KERNEL);
+ if (e == NULL)
+ return -ENOMEM;
+
+ client_get(client);
+ e->client = client;
+ e->p.speed = SCODE_100;
+ e->p.generation = a->generation;
+ e->p.header[0] = a->data[0];
+ e->p.header[1] = a->data[1];
+ e->p.header_length = 8;
+ e->p.callback = outbound_phy_packet_callback;
+ e->phy_packet.closure = a->closure;
+ e->phy_packet.type = FW_CDEV_EVENT_PHY_PACKET_SENT;
+
+ card->driver->send_request(card, &e->p);
+
+ return 0;
+}
+
static int (* const ioctl_handlers[])(struct client *, union ioctl_arg *) = {
[0x00] = ioctl_get_info,
[0x01] = ioctl_send_request,
@@ -1407,6 +1470,7 @@ static int (* const ioctl_handlers[])(st
[0x12] = ioctl_send_broadcast_request,
[0x13] = ioctl_send_stream_packet,
[0x14] = ioctl_get_cycle_timer2,
+ [0x15] = ioctl_send_phy_packet,
};

static int dispatch_ioctl(struct client *client,
Index: b/include/linux/firewire-cdev.h
===================================================================
--- a/include/linux/firewire-cdev.h
+++ b/include/linux/firewire-cdev.h
@@ -39,6 +39,7 @@

/* available since kernel version 2.6.36 */
#define FW_CDEV_EVENT_REQUEST2 0x06
+#define FW_CDEV_EVENT_PHY_PACKET_SENT 0x07

/**
* struct fw_cdev_event_common - Common part of all fw_cdev_event_ types
@@ -289,6 +290,19 @@ struct fw_cdev_event_iso_resource {
};

/**
+ * struct fw_cdev_event_phy_packet - A PHY packet was transmitted
+ * @closure: See &fw_cdev_event_common;
+ * set by %FW_CDEV_IOC_SEND_PHY_PACKET ioctl
+ * @type: %FW_CDEV_EVENT_PHY_PACKET_SENT
+ * @rcode: %RCODE_..., indicates success or failure of transmission
+ */
+struct fw_cdev_event_phy_packet {
+ __u64 closure;
+ __u32 type;
+ __u32 rcode;
+};
+
+/**
* 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
@@ -299,6 +313,7 @@ struct fw_cdev_event_iso_resource {
* @iso_resource: Valid if @common.type ==
* %FW_CDEV_EVENT_ISO_RESOURCE_ALLOCATED or
* %FW_CDEV_EVENT_ISO_RESOURCE_DEALLOCATED
+ * @phy_packet: Valid if @common.type == %FW_CDEV_EVENT_PHY_PACKET_SENT
*
* Convenience union for userspace use. Events could be read(2) into an
* appropriately aligned char buffer and then cast to this union for further
@@ -316,6 +331,7 @@ union fw_cdev_event {
struct fw_cdev_event_request2 request2; /* added in 2.6.36 */
struct fw_cdev_event_iso_interrupt iso_interrupt;
struct fw_cdev_event_iso_resource iso_resource; /* added in 2.6.30 */
+ struct fw_cdev_event_phy_packet phy_packet; /* added in 2.6.36 */
};

/* available since kernel version 2.6.22 */
@@ -347,6 +363,9 @@ union fw_cdev_event {
/* available since kernel version 2.6.34 */
#define FW_CDEV_IOC_GET_CYCLE_TIMER2 _IOWR('#', 0x14, struct fw_cdev_get_cycle_timer2)

+/* available since kernel version 2.6.36 */
+#define FW_CDEV_IOC_SEND_PHY_PACKET _IOWR('#', 0x15, struct fw_cdev_send_phy_packet)
+
/*
* ABI version history
* 1 (2.6.22) - initial version
@@ -362,8 +381,9 @@ union fw_cdev_event {
* - shared use and auto-response for FCP registers
* 3 (2.6.34) - made &fw_cdev_get_cycle_timer reliable
* - added %FW_CDEV_IOC_GET_CYCLE_TIMER2
- * 4 (2.6.36) - added %FW_CDEV_EVENT_REQUEST2
+ * 4 (2.6.36) - added %FW_CDEV_EVENT_REQUEST2, %FW_CDEV_EVENT_PHY_PACKET_SENT
* - implemented &fw_cdev_event_bus_reset.bm_node_id
+ * - added %FW_CDEV_IOC_SEND_PHY_PACKET
*/
#define FW_CDEV_VERSION 3 /* Meaningless; don't use this macro. */

@@ -811,4 +831,26 @@ struct fw_cdev_send_stream_packet {
__u32 speed;
};

+/**
+ * struct fw_cdev_send_phy_packet - send a PHY packet
+ * @closure: Passed back to userspace in the PHY-packet-sent event
+ * @data: First and second quadlet of the PHY packet
+ * @generation: The bus generation where packet is valid
+ *
+ * The %FW_CDEV_IOC_SEND_PHY_PACKET ioctl sends a PHY packet to all nodes
+ * on the same card as this device. After transmission, an
+ * %FW_CDEV_EVENT_PHY_PACKET_SENT event is generated.
+ *
+ * The payload @data[] shall be specified in host byte order. Usually,
+ * @data[1] needs to be the bitwise inverse of @data[0]. VersaPHY packets
+ * are an exception to this rule.
+ *
+ * The ioctl is only permitted on device files which represent a local node.
+ */
+struct fw_cdev_send_phy_packet {
+ __u64 closure;
+ __u32 data[2];
+ __u32 generation;
+};
+
#endif /* _LINUX_FIREWIRE_CDEV_H */

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

2010-07-16 20:26:06

by Stefan Richter

[permalink] [raw]
Subject: [PATCH 4/4 RFC] firewire: cdev: add PHY packet reception

Add an FW_CDEV_IOC_RECEIVE_PHY_PACKETS ioctl() and
FW_CDEV_EVENT_PHY_PACKET_RECEIVED poll()/read() event for /dev/fw*.
This can be used to get information from remote PHYs by remote access
PHY packets.

This is also the 2nd half of the functionality (the receive part) to
support a userspace implementation of a VersaPHY transaction layer.

Safety considerations:

- PHY packets are generally broadcasts, hence some kind of elevated
privileges should be required of a process to be able to listen in
on PHY packets. This implementation assumes that a process that is
allowed to open the /dev/fw* of a local node does have this
privilege.

There was an inconclusive discussion about introducing POSIX
capabilities as a means to check for user privileges for these
kinds of operations.

Other limitations:

- PHY packet reception may be switched on by ioctl() but cannot be
switched off again. It would be trivial to provide an off switch,
but this is not worth the code. The client should simply close()
the fd then, or just ignore further events.

- For sake of simplicity of API and kernel-side implementation, no
filter per packet content is provided.

Signed-off-by: Stefan Richter <[email protected]>
---
drivers/firewire/core-card.c | 1 +
drivers/firewire/core-cdev.c | 73 ++++++++++++++++++++++++++--
drivers/firewire/core-transaction.c | 5 ++
drivers/firewire/core.h | 2 +
drivers/firewire/ohci.c | 3 -
include/linux/firewire-cdev.h | 39 ++++++++++++---
include/linux/firewire.h | 3 +
7 files changed, 111 insertions(+), 15 deletions(-)

Index: b/drivers/firewire/core-card.c
===================================================================
--- a/drivers/firewire/core-card.c
+++ b/drivers/firewire/core-card.c
@@ -514,6 +514,7 @@ void fw_card_initialize(struct fw_card *
kref_init(&card->kref);
init_completion(&card->done);
INIT_LIST_HEAD(&card->transaction_list);
+ INIT_LIST_HEAD(&card->phy_receiver_list);
spin_lock_init(&card->lock);

card->local_node = NULL;
Index: b/drivers/firewire/core-cdev.c
===================================================================
--- a/drivers/firewire/core-cdev.c
+++ b/drivers/firewire/core-cdev.c
@@ -70,6 +70,9 @@ struct client {
struct fw_iso_buffer buffer;
unsigned long vm_start;

+ struct list_head phy_receiver_link;
+ u64 phy_receiver_closure;
+
struct list_head link;
struct kref kref;
};
@@ -202,6 +205,11 @@ struct outbound_phy_packet_event {
struct fw_cdev_event_phy_packet phy_packet;
};

+struct inbound_phy_packet_event {
+ struct event event;
+ struct fw_cdev_event_phy_packet phy_packet;
+};
+
static inline void __user *u64_to_uptr(__u64 value)
{
return (void __user *)(unsigned long)value;
@@ -237,6 +245,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);
+ INIT_LIST_HEAD(&client->phy_receiver_link);
kref_init(&client->kref);

file->private_data = client;
@@ -358,7 +367,7 @@ static void queue_bus_reset_event(struct

e = kzalloc(sizeof(*e), GFP_KERNEL);
if (e == NULL) {
- fw_notify("Out of memory when allocating bus reset event\n");
+ fw_notify("Out of memory when allocating event\n");
return;
}

@@ -405,6 +414,7 @@ union ioctl_arg {
struct fw_cdev_send_stream_packet send_stream_packet;
struct fw_cdev_get_cycle_timer2 get_cycle_timer2;
struct fw_cdev_send_phy_packet send_phy_packet;
+ struct fw_cdev_receive_phy_packets receive_phy_packets;
};

static int ioctl_get_info(struct client *client, union ioctl_arg *arg)
@@ -672,9 +682,10 @@ static void handle_request(struct fw_car

r = kmalloc(sizeof(*r), GFP_ATOMIC);
e = kmalloc(sizeof(*e), GFP_ATOMIC);
- if (r == NULL || e == NULL)
+ if (r == NULL || e == NULL) {
+ fw_notify("Out of memory when allocating event\n");
goto failed;
-
+ }
r->card = card;
r->request = request;
r->data = payload;
@@ -903,9 +914,10 @@ static void iso_callback(struct fw_iso_c
struct iso_interrupt_event *e;

e = kmalloc(sizeof(*e) + header_length, GFP_ATOMIC);
- if (e == NULL)
+ if (e == NULL) {
+ fw_notify("Out of memory when allocating event\n");
return;
-
+ }
e->interrupt.type = FW_CDEV_EVENT_ISO_INTERRUPT;
e->interrupt.closure = client->iso_closure;
e->interrupt.cycle = cycle;
@@ -1448,6 +1460,52 @@ static int ioctl_send_phy_packet(struct
return 0;
}

+static int ioctl_receive_phy_packets(struct client *client, union ioctl_arg *arg)
+{
+ struct fw_cdev_receive_phy_packets *a = &arg->receive_phy_packets;
+ struct fw_card *card = client->device->card;
+
+ /* Access policy: Allow this ioctl only on local nodes' device files. */
+ if (!client->device->is_local)
+ return -ENOSYS;
+
+ spin_lock_irq(&card->lock);
+
+ list_move_tail(&client->phy_receiver_link, &card->phy_receiver_list);
+ client->phy_receiver_closure = a->closure;
+
+ spin_unlock_irq(&card->lock);
+
+ return 0;
+}
+
+void fw_cdev_handle_phy_packet(struct fw_card *card, struct fw_packet *p)
+{
+ struct client *client;
+ struct inbound_phy_packet_event *e;
+ unsigned long flags;
+
+ spin_lock_irqsave(&card->lock, flags);
+
+ list_for_each_entry(client, &card->phy_receiver_list, phy_receiver_link) {
+ e = kmalloc(sizeof(*e) + 8, GFP_ATOMIC);
+ if (e == NULL) {
+ fw_notify("Out of memory when allocating event\n");
+ break;
+ }
+ e->phy_packet.closure = client->phy_receiver_closure;
+ e->phy_packet.type = FW_CDEV_EVENT_PHY_PACKET_RECEIVED;
+ e->phy_packet.rcode = RCODE_COMPLETE;
+ e->phy_packet.length = 8;
+ e->phy_packet.data[0] = p->header[1];
+ e->phy_packet.data[1] = p->header[2];
+ queue_event(client, &e->event,
+ &e->phy_packet, sizeof(e->phy_packet) + 8, NULL, 0);
+ }
+
+ spin_unlock_irqrestore(&card->lock, flags);
+}
+
static int (* const ioctl_handlers[])(struct client *, union ioctl_arg *) = {
[0x00] = ioctl_get_info,
[0x01] = ioctl_send_request,
@@ -1471,6 +1529,7 @@ static int (* const ioctl_handlers[])(st
[0x13] = ioctl_send_stream_packet,
[0x14] = ioctl_get_cycle_timer2,
[0x15] = ioctl_send_phy_packet,
+ [0x16] = ioctl_receive_phy_packets,
};

static int dispatch_ioctl(struct client *client,
@@ -1578,6 +1637,10 @@ static int fw_device_op_release(struct i
struct client *client = file->private_data;
struct event *event, *next_event;

+ spin_lock_irq(&client->device->card->lock);
+ list_del(&client->phy_receiver_link);
+ spin_unlock_irq(&client->device->card->lock);
+
mutex_lock(&client->device->client_list_mutex);
list_del(&client->link);
mutex_unlock(&client->device->client_list_mutex);
Index: b/drivers/firewire/core-transaction.c
===================================================================
--- a/drivers/firewire/core-transaction.c
+++ b/drivers/firewire/core-transaction.c
@@ -880,6 +880,11 @@ void fw_core_handle_request(struct fw_ca
if (p->ack != ACK_PENDING && p->ack != ACK_COMPLETE)
return;

+ if (TCODE_IS_LINK_INTERNAL(HEADER_GET_TCODE(p->header[0]))) {
+ fw_cdev_handle_phy_packet(card, p);
+ return;
+ }
+
request = allocate_request(card, p);
if (request == NULL) {
/* FIXME: send statically allocated busy packet. */
Index: b/drivers/firewire/core.h
===================================================================
--- a/drivers/firewire/core.h
+++ b/drivers/firewire/core.h
@@ -128,6 +128,7 @@ extern const struct file_operations fw_d

void fw_device_cdev_update(struct fw_device *device);
void fw_device_cdev_remove(struct fw_device *device);
+void fw_cdev_handle_phy_packet(struct fw_card *card, struct fw_packet *p);


/* -device */
@@ -214,6 +215,7 @@ static inline bool is_next_generation(in

#define TCODE_IS_READ_REQUEST(tcode) (((tcode) & ~1) == 4)
#define TCODE_IS_BLOCK_PACKET(tcode) (((tcode) & 1) != 0)
+#define TCODE_IS_LINK_INTERNAL(tcode) ((tcode) == 0xe)
#define TCODE_IS_REQUEST(tcode) (((tcode) & 2) == 0)
#define TCODE_IS_RESPONSE(tcode) (((tcode) & 2) != 0)
#define TCODE_HAS_REQUEST_DATA(tcode) (((tcode) & 12) != 4)
Index: b/drivers/firewire/ohci.c
===================================================================
--- a/drivers/firewire/ohci.c
+++ b/drivers/firewire/ohci.c
@@ -1764,10 +1764,9 @@ static int ohci_enable(struct fw_card *c
OHCI1394_HCControl_noByteSwapData);

reg_write(ohci, OHCI1394_SelfIDBuffer, ohci->self_id_bus);
- reg_write(ohci, OHCI1394_LinkControlClear,
- OHCI1394_LinkControl_rcvPhyPkt);
reg_write(ohci, OHCI1394_LinkControlSet,
OHCI1394_LinkControl_rcvSelfID |
+ OHCI1394_LinkControl_rcvPhyPkt |
OHCI1394_LinkControl_cycleTimerEnable |
OHCI1394_LinkControl_cycleMaster);

Index: b/include/linux/firewire-cdev.h
===================================================================
--- a/include/linux/firewire-cdev.h
+++ b/include/linux/firewire-cdev.h
@@ -40,6 +40,7 @@
/* available since kernel version 2.6.36 */
#define FW_CDEV_EVENT_REQUEST2 0x06
#define FW_CDEV_EVENT_PHY_PACKET_SENT 0x07
+#define FW_CDEV_EVENT_PHY_PACKET_RECEIVED 0x08

/**
* struct fw_cdev_event_common - Common part of all fw_cdev_event_ types
@@ -290,16 +291,24 @@ struct fw_cdev_event_iso_resource {
};

/**
- * struct fw_cdev_event_phy_packet - A PHY packet was transmitted
- * @closure: See &fw_cdev_event_common;
- * set by %FW_CDEV_IOC_SEND_PHY_PACKET ioctl
- * @type: %FW_CDEV_EVENT_PHY_PACKET_SENT
+ * struct fw_cdev_event_phy_packet - A PHY packet was transmitted or received
+ * @closure: See &fw_cdev_event_common; set by %FW_CDEV_IOC_SEND_PHY_PACKET
+ * or %FW_CDEV_IOC_RECEIVE_PHY_PACKETS ioctl
+ * @type: %FW_CDEV_EVENT_PHY_PACKET_SENT or %..._RECEIVED
* @rcode: %RCODE_..., indicates success or failure of transmission
+ * @length: Data length in bytes
+ * @data: Incoming data
+ *
+ * If @type is %FW_CDEV_EVENT_PHY_PACKET_SENT, @length is 0 and @data empty.
+ * If @type is %FW_CDEV_EVENT_PHY_PACKET_RECEIVED, @length is 8 and @data
+ * consists of the two PHY packet quadlets, in host byte order.
*/
struct fw_cdev_event_phy_packet {
__u64 closure;
__u32 type;
__u32 rcode;
+ __u32 length;
+ __u32 data[0];
};

/**
@@ -313,7 +322,9 @@ struct fw_cdev_event_phy_packet {
* @iso_resource: Valid if @common.type ==
* %FW_CDEV_EVENT_ISO_RESOURCE_ALLOCATED or
* %FW_CDEV_EVENT_ISO_RESOURCE_DEALLOCATED
- * @phy_packet: Valid if @common.type == %FW_CDEV_EVENT_PHY_PACKET_SENT
+ * @phy_packet: Valid if @common.type ==
+ * %FW_CDEV_EVENT_PHY_PACKET_SENT or
+ * %FW_CDEV_EVENT_PHY_PACKET_RECEIVED
*
* Convenience union for userspace use. Events could be read(2) into an
* appropriately aligned char buffer and then cast to this union for further
@@ -365,6 +376,7 @@ union fw_cdev_event {

/* available since kernel version 2.6.36 */
#define FW_CDEV_IOC_SEND_PHY_PACKET _IOWR('#', 0x15, struct fw_cdev_send_phy_packet)
+#define FW_CDEV_IOC_RECEIVE_PHY_PACKETS _IOW('#', 0x16, struct fw_cdev_receive_phy_packets)

/*
* ABI version history
@@ -381,9 +393,9 @@ union fw_cdev_event {
* - shared use and auto-response for FCP registers
* 3 (2.6.34) - made &fw_cdev_get_cycle_timer reliable
* - added %FW_CDEV_IOC_GET_CYCLE_TIMER2
- * 4 (2.6.36) - added %FW_CDEV_EVENT_REQUEST2, %FW_CDEV_EVENT_PHY_PACKET_SENT
+ * 4 (2.6.36) - added %FW_CDEV_EVENT_REQUEST2, %FW_CDEV_EVENT_PHY_PACKET_*
* - implemented &fw_cdev_event_bus_reset.bm_node_id
- * - added %FW_CDEV_IOC_SEND_PHY_PACKET
+ * - added %FW_CDEV_IOC_SEND_PHY_PACKET, _RECEIVE_PHY_PACKETS
*/
#define FW_CDEV_VERSION 3 /* Meaningless; don't use this macro. */

@@ -853,4 +865,17 @@ struct fw_cdev_send_phy_packet {
__u32 generation;
};

+/**
+ * struct fw_cdev_receive_phy_packets - start reception of PHY packets
+ * @closure: Passed back to userspace in phy packet events
+ *
+ * This ioctl activates issuing of %FW_CDEV_EVENT_PHY_PACKET_RECEIVED due to
+ * incoming PHY packets from any node on the same bus as the device.
+ *
+ * The ioctl is only permitted on device files which represent a local node.
+ */
+struct fw_cdev_receive_phy_packets {
+ __u64 closure;
+};
+
#endif /* _LINUX_FIREWIRE_CDEV_H */
Index: b/include/linux/firewire.h
===================================================================
--- a/include/linux/firewire.h
+++ b/include/linux/firewire.h
@@ -111,9 +111,10 @@ struct fw_card {
bool beta_repeaters_present;

int index;
-
struct list_head link;

+ struct list_head phy_receiver_list;
+
struct delayed_work br_work; /* bus reset job */
bool br_short;


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

2010-07-16 20:29:07

by Stefan Richter

[permalink] [raw]
Subject: [PATCH libraw1394] Implement raw1394_(start_)phy_packet_write() on new firewire backend

Requires kernel 2.6.36 or newer at runtime and linux-headers 2.6.36 or newer
at build time.

Signed-off-by: Stefan Richter <[email protected]>
---
src/dispatch.c | 2 +-
src/fw.c | 91 +++++++++++++++++++++++++++++++++++++++++++++++--------
src/fw.h | 6 ++--
3 files changed, 81 insertions(+), 18 deletions(-)

diff --git a/src/dispatch.c b/src/dispatch.c
index c6ff332..04fbad6 100644
--- a/src/dispatch.c
+++ b/src/dispatch.c
@@ -287,7 +287,7 @@ int raw1394_phy_packet_write (raw1394handle_t handle, quadlet_t data)
return -1;
}
if (handle->is_fw)
- return fw_phy_packet_write(handle->mode.fw, data);
+ return fw_phy_packet_write(handle, data);
else
return ieee1394_phy_packet_write(handle, data);
}
diff --git a/src/fw.c b/src/fw.c
index b1f12c9..bfe1e5e 100644
--- a/src/fw.c
+++ b/src/fw.c
@@ -340,6 +340,16 @@ handle_device_event(raw1394handle_t handle,
sizeof u->iso_resource);
return 0;
#endif
+
+#ifdef FW_CDEV_EVENT_PHY_PACKET_SENT /* added in kernel 2.6.36 */
+ case FW_CDEV_EVENT_PHY_PACKET_SENT:
+ rc = u64_to_ptr(u->phy_packet.closure);
+ errcode = fw_to_raw1394_errcode(u->phy_packet.rcode);
+ tag = rc->tag;
+ free(rc);
+
+ return fwhandle->tag_handler(handle, tag, errcode);
+#endif
default:
case FW_CDEV_EVENT_ISO_INTERRUPT:
/* Never happens. */
@@ -985,20 +995,6 @@ int fw_wake_up(fw_handle_t handle)
return fw_echo_request(handle, 0);
}

-int fw_phy_packet_write (fw_handle_t handle, quadlet_t data)
-{
- errno = ENOSYS;
- return -1;
-}
-
-int
-fw_start_phy_packet_write(fw_handle_t handle,
- quadlet_t data, unsigned long tag)
-{
- errno = ENOSYS;
- return -1;
-}
-
static int
send_request(fw_handle_t handle, int tcode,
nodeid_t node, nodeaddr_t addr,
@@ -1220,6 +1216,47 @@ fw_start_async_stream(fw_handle_t handle, unsigned int channel,
0, addr, length, data, 0, NULL, rawtag);
}

+int
+fw_start_phy_packet_write(fw_handle_t handle, quadlet_t data, unsigned long tag)
+{
+#ifdef FW_CDEV_IOC_SEND_PHY_PACKET /* added in kernel 2.6.36 */
+ struct fw_cdev_send_phy_packet send_phy_packet;
+ struct request_closure *closure;
+ int retval;
+
+ if (handle->local_device == NULL) {
+ handle->err = -EPERM;
+ errno = EPERM;
+ return -1;
+ }
+
+ closure = malloc(sizeof *closure);
+ if (closure == NULL) {
+ handle->err = -RCODE_SEND_ERROR;
+ errno = fw_errcode_to_errno(handle->err);
+ return -1;
+ }
+
+ closure->data = NULL;
+ closure->length = 0;
+ closure->tag = tag;
+
+ send_phy_packet.closure = ptr_to_u64(closure);
+ send_phy_packet.data[0] = be32_to_cpu(data);
+ send_phy_packet.data[1] = ~be32_to_cpu(data);
+ send_phy_packet.generation = handle->local_device->generation;
+ retval = ioctl(handle->local_device->fd, FW_CDEV_IOC_SEND_PHY_PACKET,
+ &send_phy_packet);
+ if (retval < 0)
+ free(closure);
+
+ return retval;
+#else
+ errno = ENOSYS;
+ handle->err = -errno;
+ return -1;
+#endif
+}

int
fw_start_async_send(fw_handle_t handle,
@@ -1356,6 +1393,32 @@ fw_async_stream(raw1394handle_t handle, unsigned int channel,
}

int
+fw_phy_packet_write(raw1394handle_t handle, quadlet_t data)
+{
+ fw_handle_t fwhandle = handle->mode.fw;
+ struct raw1394_reqhandle reqhandle;
+ struct sync_data sd = { 0, 0 };
+ int err;
+
+ reqhandle.callback = sync_callback;
+ reqhandle.data = &sd;
+
+ err = fw_start_phy_packet_write(fwhandle, data,
+ (unsigned long) &reqhandle);
+
+ while (!sd.done) {
+ if (err < 0)
+ return err;
+ err = fw_loop_iterate(handle);
+ }
+
+ fwhandle->err = sd.err;
+ errno = fw_errcode_to_errno(sd.err);
+
+ return (errno ? -1 : 0);
+}
+
+int
fw_async_send(fw_handle_t handle,
size_t length, size_t header_length,
unsigned int expect_response,
diff --git a/src/fw.h b/src/fw.h
index b0ff169..bc374d5 100644
--- a/src/fw.h
+++ b/src/fw.h
@@ -159,9 +159,6 @@ int fw_arm_get_buf(fw_handle_t handle, nodeaddr_t start,
size_t length, void *buf);
int fw_echo_request(fw_handle_t handle, quadlet_t data);
int fw_wake_up(fw_handle_t handle);
-int fw_phy_packet_write (fw_handle_t handle, quadlet_t data);
-int fw_start_phy_packet_write(fw_handle_t handle,
- quadlet_t data, unsigned long tag);
int fw_start_read(fw_handle_t handle, nodeid_t node, nodeaddr_t addr,
size_t length, quadlet_t *buffer, unsigned long tag);
int fw_start_write(fw_handle_t handle, nodeid_t node, nodeaddr_t addr,
@@ -176,6 +173,8 @@ int fw_start_async_stream(fw_handle_t handle, unsigned int channel,
unsigned int tag, unsigned int sy,
unsigned int speed, size_t length, quadlet_t *data,
unsigned long rawtag);
+int fw_start_phy_packet_write(fw_handle_t handle,
+ quadlet_t data, unsigned long tag);
int fw_start_async_send(fw_handle_t handle,
size_t length, size_t header_length,
unsigned int expect_response,
@@ -193,6 +192,7 @@ int fw_lock64(raw1394handle_t handle, nodeid_t node, nodeaddr_t addr,
int fw_async_stream(raw1394handle_t handle, unsigned int channel,
unsigned int tag, unsigned int sy, unsigned int speed,
size_t length, quadlet_t *data);
+int fw_phy_packet_write(raw1394handle_t handle, quadlet_t data);
int fw_async_send(fw_handle_t handle,
size_t length, size_t header_length,
unsigned int expect_response,
--
1.7.1


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

2010-07-17 17:20:09

by Stefan Richter

[permalink] [raw]
Subject: [PATCH 5/4 RFC] firewire: cdev: add PHY pinging

This extends the FW_CDEV_IOC_SEND_PHY_PACKET ioctl() for /dev/fw* to be
useful for ping time measurements. One application for it would be gap
count optimization in userspace that is based on ping times rather than
hop count. (The latter is implemented in firewire-core itself but is
not applicable to beta PHYs that act as repeater.)

Signed-off-by: Stefan Richter <[email protected]>
---
A simple test tool will be posted at linux1394-devel.

drivers/firewire/core-cdev.c | 9 ++++++---
drivers/firewire/core.h | 5 +++++
drivers/firewire/ohci.c | 4 ++++
include/linux/firewire-cdev.h | 5 ++++-
4 files changed, 19 insertions(+), 4 deletions(-)

Index: b/drivers/firewire/core-cdev.c
===================================================================
--- a/drivers/firewire/core-cdev.c
+++ b/drivers/firewire/core-cdev.c
@@ -1424,9 +1424,10 @@ static void outbound_phy_packet_callback
/* stale generation; cancelled; on certain controllers: no ack */
default: e->phy_packet.rcode = status; break;
}
+ e->phy_packet.data[0] = packet->timestamp;

- queue_event(e->client, &e->event,
- &e->phy_packet, sizeof(e->phy_packet), NULL, 0);
+ queue_event(e->client, &e->event, &e->phy_packet,
+ sizeof(e->phy_packet) + e->phy_packet.length, NULL, 0);
client_put(e->client);
}

@@ -1440,7 +1441,7 @@ static int ioctl_send_phy_packet(struct
if (!client->device->is_local)
return -ENOSYS;

- e = kzalloc(sizeof(*e), GFP_KERNEL);
+ e = kzalloc(sizeof(*e) + 4, GFP_KERNEL);
if (e == NULL)
return -ENOMEM;

@@ -1454,6 +1455,8 @@ static int ioctl_send_phy_packet(struct
e->p.callback = outbound_phy_packet_callback;
e->phy_packet.closure = a->closure;
e->phy_packet.type = FW_CDEV_EVENT_PHY_PACKET_SENT;
+ if (is_ping_packet(a->data))
+ e->phy_packet.length = 4;

card->driver->send_request(card, &e->p);

Index: b/drivers/firewire/core.h
===================================================================
--- a/drivers/firewire/core.h
+++ b/drivers/firewire/core.h
@@ -234,4 +234,9 @@ void fw_fill_response(struct fw_packet *
void fw_send_phy_config(struct fw_card *card,
int node_id, int generation, int gap_count);

+static inline bool is_ping_packet(u32 *data)
+{
+ return (data[0] & 0xc0ffffff) == 0 && ~data[0] == data[1];
+}
+
#endif /* _FIREWIRE_CORE_H */
Index: b/drivers/firewire/ohci.c
===================================================================
--- a/drivers/firewire/ohci.c
+++ b/drivers/firewire/ohci.c
@@ -1105,6 +1105,10 @@ static int at_context_queue_packet(struc
} else {
last = &d[0];
z = 2;
+
+ if (packet->header_length == 8 &&
+ is_ping_packet(packet->header))
+ last->control |= cpu_to_le16(DESCRIPTOR_PING);
}

last->control |= cpu_to_le16(DESCRIPTOR_OUTPUT_LAST |
Index: b/include/linux/firewire-cdev.h
===================================================================
--- a/include/linux/firewire-cdev.h
+++ b/include/linux/firewire-cdev.h
@@ -299,7 +299,10 @@ struct fw_cdev_event_iso_resource {
* @length: Data length in bytes
* @data: Incoming data
*
- * If @type is %FW_CDEV_EVENT_PHY_PACKET_SENT, @length is 0 and @data empty.
+ * If @type is %FW_CDEV_EVENT_PHY_PACKET_SENT, @length is 0 and @data empty,
+ * except in case of a ping packet: Then, @length is 4, and @data[0] is the
+ * ping time in 49.152MHz clocks if @rcode is %RCODE_COMPLETE.
+ *
* If @type is %FW_CDEV_EVENT_PHY_PACKET_RECEIVED, @length is 8 and @data
* consists of the two PHY packet quadlets, in host byte order.
*/

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

2010-07-17 17:21:05

by Stefan Richter

[permalink] [raw]
Subject: PHY pinging test tool

/*
* Ping packet transmission test
*
* Usage:
* - Compile with "gcc test-phy-pinging.c".
* - Run with "sudo ./a.out /dev/fw*".
* - It will print the ping time to all PHYs and exit.
*/

#include <fcntl.h>
#include <linux/firewire-cdev.h>
#include <poll.h>
#include <stdio.h>
#include <string.h>
#include <unistd.h>
#include <sys/ioctl.h>
#include <sys/stat.h>
#include <sys/types.h>

static const char *rcode_to_str(int rcode)
{
switch (rcode) {
case RCODE_COMPLETE: return "complete";
case RCODE_CONFLICT_ERROR: return "conflict error";
case RCODE_DATA_ERROR: return "data error";
case RCODE_TYPE_ERROR: return "type error";
case RCODE_ADDRESS_ERROR: return "address error";
case RCODE_SEND_ERROR: return "send error";
case RCODE_CANCELLED: return "cancelled";
case RCODE_BUSY: return "busy";
case RCODE_GENERATION: return "generation";
case RCODE_NO_ACK: return "no ack";
}
return "unknown";
}

static void read_events(int fd)
{
char buffer[sizeof(struct fw_cdev_event_phy_packet) + 4];
struct fw_cdev_event_common *ec = (void *)buffer;
struct fw_cdev_event_phy_packet *epp = (void *)buffer;
ssize_t s = sizeof buffer;
struct pollfd pfd = {
.fd = fd,
.events = POLLIN | POLLHUP,
};

while (poll(&pfd, 1, 10) > 0) {
memset(buffer, 0, s);
s = read(fd, buffer, sizeof buffer);
if (s < 0) {
fprintf(stderr, "Failed read: %m\n");
return;
}

switch (ec->type) {
case FW_CDEV_EVENT_PHY_PACKET_SENT:
if (epp->rcode == RCODE_COMPLETE)
printf("PING - phy_id = %d, data = %08x (%.2f us)\n",
(int)epp->closure,
epp->data[0], epp->data[0] / 49.152);
else
printf("PING - phy_id = %d, rcode = %s\n",
(int)epp->closure,
rcode_to_str(epp->rcode));
break;
}
}
}

static void cooked_ioctl(int fd, int req, void *arg)
{
if (ioctl(fd, req, arg) < 0)
fprintf(stderr, "Failed ioctl '%c' 0x%02x: %m\n",
_IOC_TYPE(req), _IOC_NR(req));
}

void test_phy_pinging(const char *filename)
{
__u32 rom[5];
struct fw_cdev_event_bus_reset reset;
struct fw_cdev_get_info info = {
.version = 4,
.rom = (unsigned long)rom,
.rom_length = sizeof(rom),
.bus_reset = (unsigned long)&reset,
};
struct fw_cdev_send_phy_packet p;
int fd, ln, nn, i;

fd = open(filename, O_RDWR);
if (fd < 0) {
fprintf(stderr, "Failed to open %s: %m\n", filename);
goto out;
}

cooked_ioctl(fd, FW_CDEV_IOC_GET_INFO, &info);
if (reset.node_id != reset.local_node_id) {
fprintf(stderr, "%s: Not a local node\n", filename);
goto out_fd;
}

p.generation = reset.generation;
ln = reset.local_node_id & 63;
nn = (reset.root_node_id & 63) + 1;

printf("%s: card %d, generation %d, %d nodes, "
"local phy_id %d, EUI-64 %08x%08x\n",
filename, info.card, reset.generation,
nn, ln, rom[3], rom[4]);

for (i = 0; i < nn; i++) {
p.closure = i;
p.data[0] = i << 24;
p.data[1] = ~p.data[0];
cooked_ioctl(fd, FW_CDEV_IOC_SEND_PHY_PACKET, &p);
}

read_events(fd);
out_fd:
close(fd);
out:
printf("\n");
}

int main(int argc, char **argv)
{
int i;

if (argc < 2) {
fprintf(stderr, "Usage: %s /dev/fwX [/dev/fwY ...]\n", argv[0]);
return 1;
}

for (i = 1; i < argc; i++)
test_phy_pinging(argv[i]);

return 0;
}

2010-07-17 19:36:15

by Stefan Richter

[permalink] [raw]
Subject: [PATCH 1/4 update] firewire: cdev: some clarifications to the API documentation

Response events:
- are generated on more occasions than their documentation claimed.

CSR allocation:
- An already occupied CSR can be determined from errno==EBUSY.

Bus resets:
- Note that FW_CDEV_IOC_INITIATE_BUS_RESET is nonblocking and that the
client is not required to observe a grace period since kernels
2.6.36+ will enforce it now (commit 02d37bed).

- The possible values of fw_cdev_initiate_bus_reset.type are listed in
the kerneldoc comment already.

- Clarify that an application that uses FW_CDEV_IOC_ADD_DESCRIPTOR and
FW_CDEV_IOC_REMOVE_DESCRIPTOR does not have to issue a bus reset.

Isochronous I/O contexts:
- At most one can be created per open file descriptor.

Signed-off-by: Stefan Richter <[email protected]>
---

Update: Added the occupied CSR == EBUSY sentence.

include/linux/firewire-cdev.h | 33 +++++++++++++++++++++++++--------
1 file changed, 25 insertions(+), 8 deletions(-)

Index: b/include/linux/firewire-cdev.h
===================================================================
--- a/include/linux/firewire-cdev.h
+++ b/include/linux/firewire-cdev.h
@@ -89,8 +89,9 @@ struct fw_cdev_event_bus_reset {

/**
* struct fw_cdev_event_response - Sent when a response packet was received
- * @closure: See &fw_cdev_event_common;
- * set by %FW_CDEV_IOC_SEND_REQUEST ioctl
+ * @closure: See &fw_cdev_event_common; set by %FW_CDEV_IOC_SEND_REQUEST
+ * or %FW_CDEV_IOC_SEND_BROADCAST_REQUEST
+ * or %FW_CDEV_IOC_SEND_STREAM_PACKET ioctl
* @type: See &fw_cdev_event_common; always %FW_CDEV_EVENT_RESPONSE
* @rcode: Response code returned by the remote node
* @length: Data length, i.e. the response's payload size in bytes
@@ -100,6 +101,11 @@ struct fw_cdev_event_bus_reset {
* sent by %FW_CDEV_IOC_SEND_REQUEST ioctl. The payload data for responses
* carrying data (read and lock responses) follows immediately and can be
* accessed through the @data field.
+ *
+ * The event is also generated after conclusions of transactions that do not
+ * involve response packets. This includes unified write transactions,
+ * broadcast write transactions, and transmission of asynchronous stream
+ * packets. @rcode indicates success or failure of such transmissions.
*/
struct fw_cdev_event_response {
__u64 closure;
@@ -452,7 +458,9 @@ struct fw_cdev_send_response {
* range to be used for later deallocation of the range.
*
* The address range is allocated on all local nodes. The address allocation
- * is exclusive except for the FCP command and response registers.
+ * is exclusive except for the FCP command and response registers. If an
+ * exclusive address region is already in use, the ioctl fails with errno set
+ * to %EBUSY.
*/
struct fw_cdev_allocate {
__u64 offset;
@@ -480,9 +488,14 @@ struct fw_cdev_deallocate {
* Initiate a bus reset for the bus this device is on. The bus reset can be
* either the original (long) bus reset or the arbitrated (short) bus reset
* introduced in 1394a-2000.
+ *
+ * The ioctl returns immediately. A subsequent &fw_cdev_event_bus_reset
+ * indicates when the reset actually happened. Since ABI v4, this may be
+ * considerably later than the ioctl because the kernel ensures a grace period
+ * between subsequent bus resets as per IEEE 1394 bus management specification.
*/
struct fw_cdev_initiate_bus_reset {
- __u32 type; /* FW_CDEV_SHORT_RESET or FW_CDEV_LONG_RESET */
+ __u32 type;
};

/**
@@ -506,9 +519,10 @@ struct fw_cdev_initiate_bus_reset {
*
* @immediate, @key, and @data array elements are CPU-endian quadlets.
*
- * If successful, the kernel adds the descriptor and writes back a handle to the
- * kernel-side object to be used for later removal of the descriptor block and
- * immediate key.
+ * If successful, the kernel adds the descriptor and writes back a @handle to
+ * the kernel-side object to be used for later removal of the descriptor block
+ * and immediate key. The kernel will also generate a bus reset to signal the
+ * change of the configuration ROM to other nodes.
*
* This ioctl affects the configuration ROMs of all local nodes.
* The ioctl only succeeds on device files which represent a local node.
@@ -527,7 +541,8 @@ struct fw_cdev_add_descriptor {
* descriptor was added
*
* Remove a descriptor block and accompanying immediate key from the local
- * nodes' configuration ROMs.
+ * nodes' configuration ROMs. The kernel will also generate a bus reset to
+ * signal the change of the configuration ROM to other nodes.
*/
struct fw_cdev_remove_descriptor {
__u32 handle;
@@ -559,6 +574,8 @@ struct fw_cdev_remove_descriptor {
*
* Note that the effect of a @header_size > 4 depends on
* &fw_cdev_get_info.version, as documented at &fw_cdev_event_iso_interrupt.
+ *
+ * No more than one iso context can be created per fd.
*/
struct fw_cdev_create_iso_context {
__u32 type;

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

2010-07-18 11:01:04

by Stefan Richter

[permalink] [raw]
Subject: Re: [PATCH 5/4 RFC] firewire: cdev: add PHY pinging

On 17 Jul, Stefan Richter wrote:
> --- a/drivers/firewire/ohci.c
> +++ b/drivers/firewire/ohci.c
> @@ -1105,6 +1105,10 @@ static int at_context_queue_packet(struc
> } else {
> last = &d[0];
> z = 2;
> +
> + if (packet->header_length == 8 &&
> + is_ping_packet(packet->header))
> + last->control |= cpu_to_le16(DESCRIPTOR_PING);
> }
>
> last->control |= cpu_to_le16(DESCRIPTOR_OUTPUT_LAST |

Here is a better implementation of the card driver's part. It moves the
ping packet test out of an else branch that is taken for all no-payload
packets into a switch block that is only reached in case of PHY packets.

--- a/drivers/firewire/ohci.c
+++ b/drivers/firewire/ohci.c
@@ -1068,6 +1068,9 @@ static int at_context_queue_packet(struc
header[1] = cpu_to_le32(packet->header[0]);
header[2] = cpu_to_le32(packet->header[1]);
d[0].req_count = cpu_to_le16(12);
+
+ if (is_ping_packet(packet->header))
+ d[0].control |= cpu_to_le16(DESCRIPTOR_PING);
break;

case 4:

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