[PATCH v2 0/2] firewire: core: invoke packet callback from workqueue when removing card instance

[PATCH v2 0/2] firewire: core: invoke packet callback from workqueue when removing card instance

[Takashi Sakamoto]

Hi,

This series is take 2 of the previous series[1].

This series updates the dummy driver used during card removal.

The first patch switches to a designated initializer for the dummy
driver, which contains mutable variables.

The second patch ensures that packet callbacks are invoked from a
workqueue in process context during card removal, maintaining a
consistent execution context for callbacks.

[1] https://lore.kernel.org/lkml/20261016135948.85343-1-o-takashi@sakamocchi.jp/

Takashi Sakamoto (2):
  firewire: core: use designated initializer for dummy driver on card
    removal
  firewire: core: invoke packet callback from workqueue when removing
    card instance

 drivers/firewire/core-card.c | 88 +++++++++++++++++++++++++-----------
 1 file changed, 61 insertions(+), 27 deletions(-)


base-commit: ae20f20e962cffe9a3baf189b12eb176f4058c63
-- 
2.53.0

[PATCH v2 1/2] firewire: core: use designated initializer for dummy driver on card removal

[Takashi Sakamoto]

The dummy driver used during card removal contains mutable variables.
Use a designated initializer instead of a static constant assignment.

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

diff --git a/drivers/firewire/core-card.c b/drivers/firewire/core-card.c
index a43fc3a278f2..83be100a1f4b 100644
--- a/drivers/firewire/core-card.c
+++ b/drivers/firewire/core-card.c
@@ -763,23 +763,6 @@ static int dummy_flush_iso_completions(struct fw_iso_context *ctx)
 	return -ENODEV;
 }
 
-static const struct fw_card_driver dummy_driver_template = {
-	.read_phy_reg		= dummy_read_phy_reg,
-	.update_phy_reg		= dummy_update_phy_reg,
-	.send_request		= dummy_send_request,
-	.send_response		= dummy_send_response,
-	.cancel_packet		= dummy_cancel_packet,
-	.enable_phys_dma	= dummy_enable_phys_dma,
-	.read_csr		= dummy_read_csr,
-	.write_csr		= dummy_write_csr,
-	.allocate_iso_context	= dummy_allocate_iso_context,
-	.start_iso		= dummy_start_iso,
-	.set_iso_channels	= dummy_set_iso_channels,
-	.queue_iso		= dummy_queue_iso,
-	.flush_queue_iso	= dummy_flush_queue_iso,
-	.flush_iso_completions	= dummy_flush_iso_completions,
-};
-
 void fw_card_release(struct kref *kref)
 {
 	struct fw_card *card = container_of(kref, struct fw_card, kref);
@@ -790,7 +773,28 @@ EXPORT_SYMBOL_GPL(fw_card_release);
 
 void fw_core_remove_card(struct fw_card *card)
 {
-	struct fw_card_driver dummy_driver = dummy_driver_template;
+	// Switch off most of the card driver interface.
+	const struct fw_card_driver dummy_driver = {
+		.enable			= NULL,
+		.disable		= card->driver->disable,
+		.read_phy_reg		= dummy_read_phy_reg,
+		.update_phy_reg		= dummy_update_phy_reg,
+		.set_config_rom		= NULL,
+		.send_request		= dummy_send_request,
+		.send_response		= dummy_send_response,
+		.cancel_packet		= dummy_cancel_packet,
+		.enable_phys_dma	= dummy_enable_phys_dma,
+		.read_csr		= dummy_read_csr,
+		.write_csr		= dummy_write_csr,
+		.allocate_iso_context	= dummy_allocate_iso_context,
+		.free_iso_context	= card->driver->free_iso_context,
+		.start_iso		= dummy_start_iso,
+		.set_iso_channels	= dummy_set_iso_channels,
+		.queue_iso		= dummy_queue_iso,
+		.flush_queue_iso	= dummy_flush_queue_iso,
+		.flush_iso_completions	= dummy_flush_iso_completions,
+		.stop_iso		= card->driver->stop_iso,
+	};
 
 	might_sleep();
 
@@ -801,10 +805,6 @@ void fw_core_remove_card(struct fw_card *card)
 	scoped_guard(mutex, &card_mutex)
 		list_del_init(&card->link);
 
-	/* Switch off most of the card driver interface. */
-	dummy_driver.free_iso_context	= card->driver->free_iso_context;
-	dummy_driver.stop_iso		= card->driver->stop_iso;
-	dummy_driver.disable		= card->driver->disable;
 	// Ensure the entire contents of the driver are visible to other CPUs.
 	smp_store_release(&card->driver, &dummy_driver);
 
-- 
2.53.0

[PATCH v2 2/2] firewire: core: invoke packet callback from workqueue when removing card instance

[Takashi Sakamoto]

Current implementation switches the underlying driver of a card instance
to the dummy driver in the kernel stack when removing the card. The dummy
driver simply invokes callbacks when sending request and response
packets.

Recent changes for the 1394 OHCI driver and core transaction layer ensure
that callbacks are invoked in process context from a workqueue. This
guarantees a consistent execution context for callbacks. However, card
removel is currently an exception.

Invoke packet callbacks from a workqueue during card removal.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 drivers/firewire/core-card.c | 86 +++++++++++++++++++++++++-----------
 1 file changed, 60 insertions(+), 26 deletions(-)

diff --git a/drivers/firewire/core-card.c b/drivers/firewire/core-card.c
index 83be100a1f4b..6dd88001f751 100644
--- a/drivers/firewire/core-card.c
+++ b/drivers/firewire/core-card.c
@@ -689,6 +689,15 @@ EXPORT_SYMBOL(fw_card_add);
  * .set_config_rom() is never called since the card is taken out of card_list
  * before switching to the dummy driver.
  */
+struct fw_card_driver_closer {
+	const struct fw_card_driver driver;
+
+	// For shutdown operations.
+	struct fw_card *card;
+	struct list_head packet_list;
+	spinlock_t packet_list_lock;
+	struct work_struct work;
+};
 
 static int dummy_read_phy_reg(struct fw_card *card, int address)
 {
@@ -701,14 +710,31 @@ static int dummy_update_phy_reg(struct fw_card *card, int address,
 	return -ENODEV;
 }
 
-static void dummy_send_request(struct fw_card *card, struct fw_packet *packet)
+static void dummy_packet_callback_work(struct work_struct *work)
 {
-	packet->callback(packet, card, RCODE_CANCELLED);
+	struct fw_card_driver_closer *closer = container_of(work, typeof(*closer), work);
+	unsigned long flags;
+	struct fw_packet *p;
+
+	spin_lock_irqsave(&closer->packet_list_lock, flags);
+	while ((p = list_first_entry_or_null(&closer->packet_list, typeof(*p), link_for_local))) {
+		list_del(&p->link_for_local);
+		spin_unlock_irqrestore(&closer->packet_list_lock, flags);
+
+		p->callback(p, closer->card, RCODE_CANCELLED);
+
+		spin_lock_irqsave(&closer->packet_list_lock, flags);
+	}
+	spin_unlock_irqrestore(&closer->packet_list_lock, flags);
 }
 
-static void dummy_send_response(struct fw_card *card, struct fw_packet *packet)
+static void dummy_schedule_packet_callback(struct fw_card *card, struct fw_packet *packet)
 {
-	packet->callback(packet, card, RCODE_CANCELLED);
+	struct fw_card_driver_closer *closer = container_of(card->driver, typeof(*closer), driver);
+
+	scoped_guard(spinlock_irqsave, &closer->packet_list_lock)
+		list_add_tail(&packet->link_for_local, &closer->packet_list);
+	queue_work(card->async_wq, &closer->work);
 }
 
 static int dummy_cancel_packet(struct fw_card *card, struct fw_packet *packet)
@@ -773,27 +799,30 @@ EXPORT_SYMBOL_GPL(fw_card_release);
 
 void fw_core_remove_card(struct fw_card *card)
 {
-	// Switch off most of the card driver interface.
-	const struct fw_card_driver dummy_driver = {
-		.enable			= NULL,
-		.disable		= card->driver->disable,
-		.read_phy_reg		= dummy_read_phy_reg,
-		.update_phy_reg		= dummy_update_phy_reg,
-		.set_config_rom		= NULL,
-		.send_request		= dummy_send_request,
-		.send_response		= dummy_send_response,
-		.cancel_packet		= dummy_cancel_packet,
-		.enable_phys_dma	= dummy_enable_phys_dma,
-		.read_csr		= dummy_read_csr,
-		.write_csr		= dummy_write_csr,
-		.allocate_iso_context	= dummy_allocate_iso_context,
-		.free_iso_context	= card->driver->free_iso_context,
-		.start_iso		= dummy_start_iso,
-		.set_iso_channels	= dummy_set_iso_channels,
-		.queue_iso		= dummy_queue_iso,
-		.flush_queue_iso	= dummy_flush_queue_iso,
-		.flush_iso_completions	= dummy_flush_iso_completions,
-		.stop_iso		= card->driver->stop_iso,
+	struct fw_card_driver_closer closer = {
+		// Switch off most of the card driver interface.
+		.driver = {
+			.enable			= NULL,
+			.disable		= card->driver->disable,
+			.read_phy_reg		= dummy_read_phy_reg,
+			.update_phy_reg		= dummy_update_phy_reg,
+			.set_config_rom		= NULL,
+			.send_request		= dummy_schedule_packet_callback,
+			.send_response		= dummy_schedule_packet_callback,
+			.cancel_packet		= dummy_cancel_packet,
+			.enable_phys_dma	= dummy_enable_phys_dma,
+			.read_csr		= dummy_read_csr,
+			.write_csr		= dummy_write_csr,
+			.allocate_iso_context	= dummy_allocate_iso_context,
+			.free_iso_context	= card->driver->free_iso_context,
+			.start_iso		= dummy_start_iso,
+			.set_iso_channels	= dummy_set_iso_channels,
+			.queue_iso		= dummy_queue_iso,
+			.flush_queue_iso	= dummy_flush_queue_iso,
+			.flush_iso_completions	= dummy_flush_iso_completions,
+			.stop_iso		= card->driver->stop_iso,
+		},
+		.card = card,
 	};
 
 	might_sleep();
@@ -805,8 +834,12 @@ void fw_core_remove_card(struct fw_card *card)
 	scoped_guard(mutex, &card_mutex)
 		list_del_init(&card->link);
 
+	INIT_LIST_HEAD(&closer.packet_list);
+	spin_lock_init(&closer.packet_list_lock);
+	INIT_WORK(&closer.work, dummy_packet_callback_work);
+
 	// Ensure the entire contents of the driver are visible to other CPUs.
-	smp_store_release(&card->driver, &dummy_driver);
+	smp_store_release(&card->driver, &closer.driver);
 
 	drain_workqueue(card->isoc_wq);
 	drain_workqueue(card->async_wq);
@@ -825,6 +858,7 @@ void fw_core_remove_card(struct fw_card *card)
 	destroy_workqueue(card->isoc_wq);
 	destroy_workqueue(card->async_wq);
 
+	WARN_ON(!list_empty(&closer.packet_list));
 	WARN_ON(!list_empty(&card->transactions.list));
 }
 EXPORT_SYMBOL(fw_core_remove_card);
-- 
2.53.0

Re: [PATCH v2 0/2] firewire: core: invoke packet callback from workqueue when removing card instance

[Takashi Sakamoto]

On Sun, Oct 18, 2026 at 10:29:53PM +0900, Takashi Sakamoto wrote:
> Hi,
> 
> This series is take 2 of the previous series[1].
> 
> This series updates the dummy driver used during card removal.
> 
> The first patch switches to a designated initializer for the dummy
> driver, which contains mutable variables.
> 
> The second patch ensures that packet callbacks are invoked from a
> workqueue in process context during card removal, maintaining a
> consistent execution context for callbacks.
> 
> [1] https://lore.kernel.org/lkml/20261016135948.85343-1-o-takashi@sakamocchi.jp/
> 
> Takashi Sakamoto (2):
>   firewire: core: use designated initializer for dummy driver on card
>     removal
>   firewire: core: invoke packet callback from workqueue when removing
>     card instance
> 
>  drivers/firewire/core-card.c | 88 +++++++++++++++++++++++++-----------
>  1 file changed, 61 insertions(+), 27 deletions(-)

Applied to for-next branch with a small change that both
disable_work_sync() and destroy_work_on_stack() are called for closer.work
in the final part of fw_core_remove_card().


Regards

Takashi Sakamoto