[PATCH 0/2] firewire: core: code refactoring for transaction layer

[PATCH 0/2] firewire: core: code refactoring for transaction layer

[Takashi Sakamoto]

Hi,

The current implementation of transaction layer includes some duplicated
code for managing transaction list. This patchset adds some helper
functions and macros to eliminate the duplication.

Takashi Sakamoto (2):
  firewire: core: code refactoring to remove transaction entry
  firewire: core: code refactoring to find and pop transaction entry

 drivers/firewire/core-transaction.c | 57 +++++++++++++++--------------
 1 file changed, 30 insertions(+), 27 deletions(-)


base-commit: b330f98ff238ad9446574965d09cab33736519d5
-- 
2.51.0

[PATCH 1/2] firewire: core: code refactoring to remove transaction entry

[Takashi Sakamoto]

The list operation to remove transaction entry appears several times in
transaction implementation and can be replaced with a helper function.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 drivers/firewire/core-transaction.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/firewire/core-transaction.c b/drivers/firewire/core-transaction.c
index dd3656a0c1ff..8bd79c3f97f2 100644
--- a/drivers/firewire/core-transaction.c
+++ b/drivers/firewire/core-transaction.c
@@ -44,6 +44,13 @@ static int try_cancel_split_timeout(struct fw_transaction *t)
 		return 1;
 }
 
+// card->transactions.lock must be acquired in advance.
+static void remove_transaction_entry(struct fw_card *card, struct fw_transaction *entry)
+{
+	list_del_init(&entry->link);
+	card->transactions.tlabel_mask &= ~(1ULL << entry->tlabel);
+}
+
 static int close_transaction(struct fw_transaction *transaction, struct fw_card *card, int rcode,
 			     u32 response_tstamp)
 {
@@ -55,8 +62,7 @@ static int close_transaction(struct fw_transaction *transaction, struct fw_card
 		list_for_each_entry(iter, &card->transactions.list, link) {
 			if (iter == transaction) {
 				if (try_cancel_split_timeout(iter)) {
-					list_del_init(&iter->link);
-					card->transactions.tlabel_mask &= ~(1ULL << iter->tlabel);
+					remove_transaction_entry(card, iter);
 					t = iter;
 				}
 				break;
@@ -122,8 +128,7 @@ static void split_transaction_timeout_callback(struct timer_list *timer)
 	scoped_guard(spinlock_irqsave, &card->transactions.lock) {
 		if (list_empty(&t->link))
 			return;
-		list_del(&t->link);
-		card->transactions.tlabel_mask &= ~(1ULL << t->tlabel);
+		remove_transaction_entry(card, t);
 	}
 
 	if (!t->with_tstamp) {
@@ -1142,8 +1147,7 @@ void fw_core_handle_response(struct fw_card *card, struct fw_packet *p)
 		list_for_each_entry(iter, &card->transactions.list, link) {
 			if (iter->node_id == source && iter->tlabel == tlabel) {
 				if (try_cancel_split_timeout(iter)) {
-					list_del_init(&iter->link);
-					card->transactions.tlabel_mask &= ~(1ULL << iter->tlabel);
+					remove_transaction_entry(card, iter);
 					t = iter;
 				}
 				break;
-- 
2.51.0

[PATCH 2/2] firewire: core: code refactoring to find and pop transaction entry

[Takashi Sakamoto]

The list operation to find and pop transaction entry appears several
times in transaction implementation, and can be replaced with a helper
functional macro.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 drivers/firewire/core-transaction.c | 45 ++++++++++++++---------------
 1 file changed, 22 insertions(+), 23 deletions(-)

diff --git a/drivers/firewire/core-transaction.c b/drivers/firewire/core-transaction.c
index 8bd79c3f97f2..e80791d6d46b 100644
--- a/drivers/firewire/core-transaction.c
+++ b/drivers/firewire/core-transaction.c
@@ -51,28 +51,34 @@ static void remove_transaction_entry(struct fw_card *card, struct fw_transaction
 	card->transactions.tlabel_mask &= ~(1ULL << entry->tlabel);
 }
 
+// card->transactions.lock must be acquired in advance.
+#define find_and_pop_transaction_entry(card, condition)			\
+({									\
+	struct fw_transaction *iter, *t = NULL;				\
+	list_for_each_entry(iter, &card->transactions.list, link) {	\
+		if (condition) {					\
+			t = iter;					\
+			break;						\
+		}							\
+	}								\
+	if (t && try_cancel_split_timeout(t))				\
+		remove_transaction_entry(card, t);			\
+	t;								\
+})
+
 static int close_transaction(struct fw_transaction *transaction, struct fw_card *card, int rcode,
 			     u32 response_tstamp)
 {
-	struct fw_transaction *t = NULL, *iter;
+	struct fw_transaction *t;
 
 	// NOTE: This can be without irqsave when we can guarantee that __fw_send_request() for
 	// local destination never runs in any type of IRQ context.
 	scoped_guard(spinlock_irqsave, &card->transactions.lock) {
-		list_for_each_entry(iter, &card->transactions.list, link) {
-			if (iter == transaction) {
-				if (try_cancel_split_timeout(iter)) {
-					remove_transaction_entry(card, iter);
-					t = iter;
-				}
-				break;
-			}
-		}
+		t = find_and_pop_transaction_entry(card, iter == transaction);
+		if (!t)
+			return -ENOENT;
 	}
 
-	if (!t)
-		return -ENOENT;
-
 	if (!t->with_tstamp) {
 		t->callback.without_tstamp(card, rcode, NULL, 0, t->callback_data);
 	} else {
@@ -1102,7 +1108,7 @@ EXPORT_SYMBOL(fw_core_handle_request);
 
 void fw_core_handle_response(struct fw_card *card, struct fw_packet *p)
 {
-	struct fw_transaction *t = NULL, *iter;
+	struct fw_transaction *t = NULL;
 	u32 *data;
 	size_t data_length;
 	int tcode, tlabel, source, rcode;
@@ -1144,15 +1150,8 @@ void fw_core_handle_response(struct fw_card *card, struct fw_packet *p)
 	// NOTE: This can be without irqsave when we can guarantee that __fw_send_request() for
 	// local destination never runs in any type of IRQ context.
 	scoped_guard(spinlock_irqsave, &card->transactions.lock) {
-		list_for_each_entry(iter, &card->transactions.list, link) {
-			if (iter->node_id == source && iter->tlabel == tlabel) {
-				if (try_cancel_split_timeout(iter)) {
-					remove_transaction_entry(card, iter);
-					t = iter;
-				}
-				break;
-			}
-		}
+		t = find_and_pop_transaction_entry(card,
+				iter->node_id == source && iter->tlabel == tlabel);
 	}
 
 	trace_async_response_inbound((uintptr_t)t, card->index, p->generation, p->speed, p->ack,
-- 
2.51.0

Re: [PATCH 0/2] firewire: core: code refactoring for transaction layer

[Takashi Sakamoto]

On Sat, Nov 01, 2025 at 07:21:29PM +0900, Takashi Sakamoto wrote:
> Hi,
> 
> The current implementation of transaction layer includes some duplicated
> code for managing transaction list. This patchset adds some helper
> functions and macros to eliminate the duplication.
> 
> Takashi Sakamoto (2):
>   firewire: core: code refactoring to remove transaction entry
>   firewire: core: code refactoring to find and pop transaction entry
> 
>  drivers/firewire/core-transaction.c | 57 +++++++++++++++--------------
>  1 file changed, 30 insertions(+), 27 deletions(-)

Applied to for-next branch.


Regards

Takashi Sakamoto