firewire: Implement proper transaction cancelation.

Drivers such as fw-sbp2 had no way to properly cancel in-progress
transactions, which could leave a pending transaction or an unset
packet in the low-level queues after kfree'ing the containing
structure. fw_cancel_transaction() lets drivers cancel a submitted
transaction.

Signed-off-by: Kristian Høgsberg <krh@redhat.com>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>

diff --git a/drivers/firewire/fw-card.c b/drivers/firewire/fw-card.c
index 7f5dc43ec131e658bf41e0d7fc033631d261d967..f785b1005284e00de87aa94bc8ef829d1a73507c 100644 (file)
--- a/drivers/firewire/fw-card.c
+++ b/drivers/firewire/fw-card.c
@@ -474,6 +474,12 @@ dummy_send_response(struct fw_card *card, struct fw_packet *packet)
        packet->callback(packet, card, -ENODEV);
 }
 
+static int
+dummy_cancel_packet(struct fw_card *card, struct fw_packet *packet)
+{
+       return -ENOENT;
+}
+
 static int
 dummy_enable_phys_dma(struct fw_card *card,
                      int node_id, int generation)
@@ -487,6 +493,7 @@ static struct fw_card_driver dummy_driver = {
        .update_phy_reg  = dummy_update_phy_reg,
        .set_config_rom  = dummy_set_config_rom,
        .send_request    = dummy_send_request,
+       .cancel_packet   = dummy_cancel_packet,
        .send_response   = dummy_send_response,
        .enable_phys_dma = dummy_enable_phys_dma,
 };

diff --git a/drivers/firewire/fw-ohci.c b/drivers/firewire/fw-ohci.c
index 02b2b69c87410793b454e8e8cbe16baed15c5486..e6fa3496183e7c5e96eb24a7f7d2deb41d68892d 100644 (file)
--- a/drivers/firewire/fw-ohci.c
+++ b/drivers/firewire/fw-ohci.c
@@ -79,6 +79,7 @@ struct at_context {
        struct fw_ohci *ohci;
        dma_addr_t descriptor_bus;
        dma_addr_t buffer_bus;
+       struct fw_packet *current_packet;
 
        struct list_head list;
 
@@ -489,6 +490,7 @@ at_context_setup_packet(struct at_context *ctx, struct list_head *list)
                          ctx->descriptor_bus | z);
                reg_write(ctx->ohci, control_set(ctx->regs),
                          CONTEXT_RUN | CONTEXT_WAKE);
+               ctx->current_packet = packet;
        } else {
                /* We dont return error codes from this function; all
                 * transmission errors are reported through the
@@ -524,6 +526,12 @@ static void at_context_tasklet(unsigned long data)
 
        at_context_stop(ctx);
 
+       /* If the head of the list isn't the packet that just got
+        * transmitted, the packet got cancelled before we finished
+        * transmitting it. */
+       if (ctx->current_packet != packet)
+               goto skip_to_next;
+
        if (packet->payload_length > 0) {
                dma_unmap_single(ohci->card.device, packet->payload_bus,
                                 packet->payload_length, DMA_TO_DEVICE);
@@ -564,6 +572,7 @@ static void at_context_tasklet(unsigned long data)
        } else
                complete_transmission(packet, evt - 16, &list);
 
+ skip_to_next:
        /* If more packets are queued, set up the next one. */
        if (!list_empty(&ctx->list))
                at_context_setup_packet(ctx, &list);
@@ -1012,6 +1021,29 @@ static void ohci_send_response(struct fw_card *card, struct fw_packet *packet)
        at_context_transmit(&ohci->at_response_ctx, packet);
 }
 
+static int ohci_cancel_packet(struct fw_card *card, struct fw_packet *packet)
+{
+       struct fw_ohci *ohci = fw_ohci(card);
+       LIST_HEAD(list);
+       unsigned long flags;
+
+       spin_lock_irqsave(&ohci->lock, flags);
+
+       if (packet->ack == 0) {
+               fw_notify("cancelling packet %p (header[0]=%08x)\n",
+                         packet, packet->header[0]);
+
+               complete_transmission(packet, RCODE_CANCELLED, &list);
+       }
+
+       spin_unlock_irqrestore(&ohci->lock, flags);
+
+       do_packet_callbacks(ohci, &list);
+
+       /* Return success if we actually cancelled something. */
+       return list_empty(&list) ? -ENOENT : 0;
+}
+
 static int
 ohci_enable_phys_dma(struct fw_card *card, int node_id, int generation)
 {
@@ -1339,6 +1371,7 @@ static const struct fw_card_driver ohci_driver = {
        .set_config_rom         = ohci_set_config_rom,
        .send_request           = ohci_send_request,
        .send_response          = ohci_send_response,
+       .cancel_packet          = ohci_cancel_packet,
        .enable_phys_dma        = ohci_enable_phys_dma,
 
        .allocate_iso_context   = ohci_allocate_iso_context,

diff --git a/drivers/firewire/fw-sbp2.c b/drivers/firewire/fw-sbp2.c
index 54cad3a5dfb8bc620191e300247155f1da8d6a36..bb133398feecf3ab7e77b0ded07feef5cc489e2b 100644 (file)
--- a/drivers/firewire/fw-sbp2.c
+++ b/drivers/firewire/fw-sbp2.c
@@ -348,6 +348,9 @@ static void sbp2_cancel_orbs(struct fw_unit *unit)
        spin_unlock_irqrestore(&device->card->lock, flags);
 
        list_for_each_entry_safe(orb, next, &list, link) {
+               if (fw_cancel_transaction(device->card, &orb->t) == 0)
+                       continue;
+
                orb->rcode = RCODE_CANCELLED;
                orb->callback(orb, NULL);
        }

diff --git a/drivers/firewire/fw-transaction.c b/drivers/firewire/fw-transaction.c
index fb3b77e1bb2dc1e9cf61085403274c7dd06e6b11..5394569a1c8934a9763d4955447cf05176254925 100644 (file)
--- a/drivers/firewire/fw-transaction.c
+++ b/drivers/firewire/fw-transaction.c
@@ -59,20 +59,52 @@
 #define phy_config_root_id(node_id)    ((((node_id) & 0x3f) << 24) | (1 << 23))
 #define phy_identifier(id)             ((id) << 30)
 
-static void
-close_transaction(struct fw_transaction *t, struct fw_card *card, int rcode,
-                 u32 * payload, size_t length)
+static int
+close_transaction(struct fw_transaction *transaction,
+                 struct fw_card *card, int rcode,
+                 u32 *payload, size_t length)
 {
+       struct fw_transaction *t;
        unsigned long flags;
 
        spin_lock_irqsave(&card->lock, flags);
-       card->tlabel_mask &= ~(1 << t->tlabel);
-       list_del(&t->link);
+       list_for_each_entry(t, &card->transaction_list, link) {
+               if (t == transaction) {
+                       list_del(&t->link);
+                       card->tlabel_mask &= ~(1 << t->tlabel);
+                       break;
+               }
+       }
        spin_unlock_irqrestore(&card->lock, flags);
 
-       t->callback(card, rcode, payload, length, t->callback_data);
+       if (&t->link != &card->transaction_list) {
+               t->callback(card, rcode, payload, length, t->callback_data);
+               return 0;
+       }
+
+       return -ENOENT;
 }
 
+/* Only valid for transactions that are potentially pending (ie have
+ * been sent). */
+int
+fw_cancel_transaction(struct fw_card *card,
+                     struct fw_transaction *transaction)
+{
+       /* Cancel the packet transmission if it's still queued.  That
+        * will call the packet transmission callback which cancels
+        * the transaction. */
+
+       if (card->driver->cancel_packet(card, &transaction->packet) == 0)
+               return 0;
+
+       /* If the request packet has already been sent, we need to see
+        * if the transaction is still pending and remove it in that case. */
+
+       return close_transaction(transaction, card, RCODE_CANCELLED, NULL, 0);
+}
+EXPORT_SYMBOL(fw_cancel_transaction);
+
 static void
 transmit_complete_callback(struct fw_packet *packet,
                           struct fw_card *card, int status)
@@ -162,6 +194,7 @@ fw_fill_request(struct fw_packet *packet, int tcode, int tlabel,
 
        packet->speed = speed;
        packet->generation = generation;
+       packet->ack = 0;
 }
 
 /**
@@ -298,8 +331,14 @@ void fw_flush_transactions(struct fw_card *card)
        card->tlabel_mask = 0;
        spin_unlock_irqrestore(&card->lock, flags);
 
-       list_for_each_entry_safe(t, next, &list, link)
+       list_for_each_entry_safe(t, next, &list, link) {
+               card->driver->cancel_packet(card, &t->packet);
+
+               /* At this point cancel_packet will never call the
+                * transaction callback, since we just took all the
+                * transactions out of the list.  So do it here.*/
                t->callback(card, RCODE_CANCELLED, NULL, 0, t->callback_data);
+       }
 }
 
 static struct fw_address_handler *
@@ -531,6 +570,7 @@ allocate_request(struct fw_packet *p)
        request->response.speed = p->speed;
        request->response.timestamp = t;
        request->response.generation = p->generation;
+       request->response.ack = 0;
        request->response.callback = free_response_callback;
        request->ack = p->ack;
        request->length = length;

diff --git a/drivers/firewire/fw-transaction.h b/drivers/firewire/fw-transaction.h
index 50ec2c683a318165af0d85505982f37d8b79c643..8f0283cf1a7a191f169aec7edb88ad24ec90f05b 100644 (file)
--- a/drivers/firewire/fw-transaction.h
+++ b/drivers/firewire/fw-transaction.h
@@ -391,6 +391,8 @@ struct fw_card_driver {
 
        void (*send_request) (struct fw_card *card, struct fw_packet *packet);
        void (*send_response) (struct fw_card *card, struct fw_packet *packet);
+       /* Calling cancel is valid once a packet has been submitted. */
+       int (*cancel_packet) (struct fw_card *card, struct fw_packet *packet);
 
        /* Allow the specified node ID to do direct DMA out and in of
         * host memory.  The card will disable this for all node when
@@ -421,6 +423,9 @@ fw_send_request(struct fw_card *card, struct fw_transaction *t,
                void *data, size_t length,
                fw_transaction_callback_t callback, void *callback_data);
 
+int fw_cancel_transaction(struct fw_card *card,
+                         struct fw_transaction *transaction);
+
 void fw_flush_transactions(struct fw_card *card);
 
 void fw_send_phy_config(struct fw_card *card,