linux1394-devel

Re: [PATCH] firewire: Replace ENOSYS with appropriate error codes

[Takashi S. <o-t...@sa...>]

Hi,

On Mon, Nov 17, 2025 at 04:39:01PM +0530, Nirbhay Sharma wrote:
> ENOSYS is reserved for "invalid syscall number" and should not be used
> for other error conditions. Replace incorrect usages with more
> appropriate error codes:
 
Yes. The newly-written code should not use ENOSYS for cadual use, indeed.

> - In sbp2.c: Use -EOPNOTSUPP for unsupported operation (re-adding
>   logical units via SCSI stack).
> 
> - In ohci.c: Use -EINVAL for invalid ISO context types in switch
>   statements, and -EOPNOTSUPP for unsupported Pinnacle MovieBoard
>   hardware.
> 
> - In core-cdev.c: Use -EACCES for access policy violations when
>   operations are restricted to local nodes' device files.
>
> Signed-off-by: Nirbhay Sharma <nir...@gm...>
> ---
>  drivers/firewire/core-cdev.c | 6 +++---
>  drivers/firewire/ohci.c      | 8 ++++----
>  drivers/firewire/sbp2.c      | 2 +-
>  3 files changed, 8 insertions(+), 8 deletions(-)
 
There is a rest to discuss when changing existing code in respect to
this topic, since it brings loss of backward-compatibility to userspace
software. In this reason, I've left them as is.

If there are any strong and specific reasons to correct them, let us
change them. Do you have such reasons? For example, Linux kernel
developer have shared the consensus and decision to ostracize such codes?


Thanks

Takashi Sakamoto

[GIT PULL] firewire fixes for v6.18-rc6

[Takashi S. <o-t...@sa...>]

Hi Linus,

Please apply the fixes for FireWire subsystem.

The following changes since commit e9a6fb0bcdd7609be6969112f3fbfcce3b1d4a7c:

  Linux 6.18-rc5 (2025-11-09 15:10:19 -0800)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/ieee1394/linux1394.git tags/firewire-fixes-6.18-rc6

for you to fetch changes up to 1107aac1ad7f445a83604b14af7be47f1a795c66:

  firewire: core: fix to update generation field in topology map (2025-11-16 21:30:26 +0900)

----------------------------------------------------------------
firewire fixes for 6.18-rc6

This includes some fixes for bugs to generate topology map, newly
introduced in v6.18 kernel.

----------------------------------------------------------------
Takashi Sakamoto (1):
      firewire: core: fix to update generation field in topology map

Ville Syrjälä (1):
      firewire: core: Initialize topology_map.lock

 drivers/firewire/core-card.c     | 2 ++
 drivers/firewire/core-topology.c | 3 ++-
 2 files changed, 4 insertions(+), 1 deletion(-)


Regards

Takashi Sakamoto

Re: [PATCH] firewire: core: fix to update generation field in topology map

[Takashi S. <o-t...@sa...>]

On Fri, Nov 14, 2025 at 11:44:21PM +0900, Takashi Sakamoto wrote:
> The generation field of topology map is updated after initialized by zero.
> The updated value of generation field is always zero, and is against
> specification.
> 
> This commit fixes the bug.
> 
> Fixes: 7d138cb269db ("firewire: core: use spin lock specific to topology map")
> Signed-off-by: Takashi Sakamoto <o-t...@sa...>
> ---
>  drivers/firewire/core-topology.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

Applied to for-linus branch.


Regards

Takashi Sakamoto

[PATCH] firewire: core: fix to update generation field in topology map

[Takashi S. <o-t...@sa...>]

The generation field of topology map is updated after initialized by zero.
The updated value of generation field is always zero, and is against
specification.

This commit fixes the bug.

Fixes: 7d138cb269db ("firewire: core: use spin lock specific to topology map")
Signed-off-by: Takashi Sakamoto <o-t...@sa...>
---
 drivers/firewire/core-topology.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/firewire/core-topology.c b/drivers/firewire/core-topology.c
index 2f73bcd5696f..ed3ae8cdb0cd 100644
--- a/drivers/firewire/core-topology.c
+++ b/drivers/firewire/core-topology.c
@@ -441,12 +441,13 @@ static void update_topology_map(__be32 *buffer, size_t buffer_size, int root_nod
 				const u32 *self_ids, int self_id_count)
 {
 	__be32 *map = buffer;
+	u32 next_generation = be32_to_cpu(buffer[1]) + 1;
 	int node_count = (root_node_id & 0x3f) + 1;
 
 	memset(map, 0, buffer_size);
 
 	*map++ = cpu_to_be32((self_id_count + 2) << 16);
-	*map++ = cpu_to_be32(be32_to_cpu(buffer[1]) + 1);
+	*map++ = cpu_to_be32(next_generation);
 	*map++ = cpu_to_be32((node_count << 16) | self_id_count);
 
 	while (self_id_count--)
-- 
2.51.0

Re: [PATCH] firewire: core: Initialize topology_map.lock

[Takashi S. <o-t...@sa...>]

Hi,

Thanks for your sending the patch. I completely overlooked the lack of
initialization... Your patch fixes the same issue reported by Erhard
Furtner[1].

On Fri, Nov 14, 2025 at 12:25:31AM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <vil...@li...>
> 
> Lockdep barfs on the new uninitialized spinlock.
> Initialize it.
> 
> protip: enable lockdep (CONFIG_PROVE_LOCKING=y) when
>         doing locking changes
> 
> firewire_ohci 0000:02:01.1: added OHCI v1.10 device as card 0, 4 IR + 4 IT contexts, quirks 0x11
> INFO: trying to register non-static key.
> The code is fine but needs lockdep annotation, or maybe
> you didn't initialize this object before use?
> turning off the locking correctness validator.
> CPU: 0 UID: 0 PID: 1042 Comm: irq/17-firewire Not tainted 6.17.0-rc2-cl-bisect2-00026-g7d138cb269db #136 PREEMPT
> Hardware name: Dell Inc. Latitude E5400                  /0D695C, BIOS A19 06/13/2013
> Call Trace:
>  <TASK>
>  dump_stack_lvl+0x6d/0xa0
>  register_lock_class+0x783/0x790
>  ? find_held_lock+0x2b/0x80
>  ? __mod_timer+0x110/0x320
>  ? __mod_timer+0x110/0x320
>  __lock_acquire+0x405/0x2600
>  lock_acquire+0xca/0x2e0
>  ? fw_core_handle_bus_reset+0x888/0xca0 [firewire_core]
>  ? fw_core_handle_bus_reset+0x878/0xca0 [firewire_core]
>  ? fw_core_handle_bus_reset+0x878/0xca0 [firewire_core]
>  _raw_spin_lock+0x2e/0x40
>  ? fw_core_handle_bus_reset+0x888/0xca0 [firewire_core]
>  fw_core_handle_bus_reset+0x888/0xca0 [firewire_core]
>  handle_selfid_complete_event+0x35c/0x7a0 [firewire_ohci]
>  ? irq_thread+0x8d/0x280
>  irq_thread_fn+0x18/0x50
>  irq_thread+0x15a/0x280
>  ? irq_check_status_bit+0x100/0x100
>  ? lockdep_hardirqs_on+0x78/0x100
>  ? irq_finalize_oneshot.part.0+0xc0/0xc0
>  ? irq_forced_thread_fn+0x60/0x60
>  kthread+0x114/0x200
>  ? kthreads_online_cpu+0x110/0x110
>  ret_from_fork+0x158/0x1e0
>  ? kthreads_online_cpu+0x110/0x110
>  ret_from_fork_asm+0x11/0x20
>  </TASK>
> 
> Cc: lin...@li...
> Cc: Takashi Sakamoto <o-t...@sa...>
> Fixes: 7d138cb269db ("firewire: core: use spin lock specific to topology map")
> Signed-off-by: Ville Syrjälä <vil...@li...>
> ---
>  drivers/firewire/core-card.c | 2 ++
>  1 file changed, 2 insertions(+)

Applied to for-linus branch. I will send it to upstream as a part of
fixes for v6.18-rc6 kernel.

[1] https://lore.kernel.org/lkml/992...@ma.../


Thanks

Takashi Sakamoto

Re: [PATCH] firewire: core: abort pending transactions at card removal

[Takashi S. <o-t...@sa...>]

On Wed, Nov 12, 2025 at 07:38:34AM +0900, Takashi Sakamoto wrote:
> IEEE 1394 defines the split, concatenated, and unified transaction.
> To support the split transaction, core function uses linked list to
> maintain the transactions waiting for acknowledge packet. After clearing
> sources of hardware interrupts, the acknowledge packet is no longer
> handled, therefore it is required to abort the pending transactions.
> 
> This commit executes callback with RCODE_CANCELLED for the pending
> transactions at card removal.
> 
> Signed-off-by: Takashi Sakamoto <o-t...@sa...>
> ---
>  drivers/firewire/core-card.c        |  1 +
>  drivers/firewire/core-transaction.c | 28 ++++++++++++++++++++++++++++
>  drivers/firewire/core.h             |  2 ++
>  drivers/firewire/ohci.c             |  5 -----
>  4 files changed, 31 insertions(+), 5 deletions(-)

Applied to for-next branch.


Regards

Takashi Sakamoto

Re: [PATCH v2] firewire: core: add WQ_UNBOUND to alloc_workqueue users

[Takashi S. <o-t...@sa...>]

Hi,

On Wed, Nov 12, 2025 at 01:01:25PM +0100, Marco Crivellari wrote:
> Currently if a user enqueues a work item using schedule_delayed_work() the
> used wq is "system_wq" (per-cpu wq) while queue_delayed_work() use
> WORK_CPU_UNBOUND (used when a cpu is not specified). The same applies to
> schedule_work() that is using system_wq and queue_work(), that makes use
> again of WORK_CPU_UNBOUND.
> This lack of consistency cannot be addressed without refactoring the API.
> 
> alloc_workqueue() treats all queues as per-CPU by default, while unbound
> workqueues must opt-in via WQ_UNBOUND.
> 
> This default is suboptimal: most workloads benefit from unbound queues,
> allowing the scheduler to place worker threads where they’re needed and
> reducing noise when CPUs are isolated.
> 
> This continues the effort to refactor workqueue APIs, which began with
> the introduction of new workqueues and a new alloc_workqueue flag in:
> 
> commit 128ea9f6ccfb ("workqueue: Add system_percpu_wq and system_dfl_wq")
> commit 930c2ea566af ("workqueue: Add new WQ_PERCPU flag")
> 
> This change adds the WQ_UNBOUND flag to explicitly request
> alloc_workqueue() to be unbound, because this specific workload has no
> benefit being per-cpu.
> 
> With the introduction of the WQ_PERCPU flag (equivalent to !WQ_UNBOUND),
> any alloc_workqueue() caller that doesn’t explicitly specify WQ_UNBOUND
> must now use WQ_PERCPU.
> 
> Once migration is complete, WQ_UNBOUND can be removed and unbound will
> become the implicit default.
> 
> Suggested-by: Tejun Heo <tj...@ke...>
> Signed-off-by: Marco Crivellari <mar...@su...>
> ---
> Changes in v2:
> - This workload benefit from an unbound workqueue. So change WQ_PERCPU
>   with WQ_UNBOUND_WQ.
> 
> - Rebased on 6.18-rc5
> ---
>  drivers/firewire/core-transaction.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

Applied to for-next branch.

I'm sorry not to reply to your previous post since it did not reach my
mail box... Your v2 patch is a good reminder ;)


Thanks

Takashi Sakamoto

[PATCH] firewire: core: abort pending transactions at card removal

[Takashi S. <o-t...@sa...>]

IEEE 1394 defines the split, concatenated, and unified transaction.
To support the split transaction, core function uses linked list to
maintain the transactions waiting for acknowledge packet. After clearing
sources of hardware interrupts, the acknowledge packet is no longer
handled, therefore it is required to abort the pending transactions.

This commit executes callback with RCODE_CANCELLED for the pending
transactions at card removal.

Signed-off-by: Takashi Sakamoto <o-t...@sa...>
---
 drivers/firewire/core-card.c        |  1 +
 drivers/firewire/core-transaction.c | 28 ++++++++++++++++++++++++++++
 drivers/firewire/core.h             |  2 ++
 drivers/firewire/ohci.c             |  5 -----
 4 files changed, 31 insertions(+), 5 deletions(-)

diff --git a/drivers/firewire/core-card.c b/drivers/firewire/core-card.c
index 65bd9db996c0..9869ea3fd9fc 100644
--- a/drivers/firewire/core-card.c
+++ b/drivers/firewire/core-card.c
@@ -790,6 +790,7 @@ void fw_core_remove_card(struct fw_card *card)
 	drain_workqueue(card->isoc_wq);
 	drain_workqueue(card->async_wq);
 	card->driver->disable(card);
+	fw_cancel_pending_transactions(card);
 
 	scoped_guard(spinlock_irqsave, &card->lock)
 		fw_destroy_nodes(card);
diff --git a/drivers/firewire/core-transaction.c b/drivers/firewire/core-transaction.c
index e80791d6d46b..fe96429ba395 100644
--- a/drivers/firewire/core-transaction.c
+++ b/drivers/firewire/core-transaction.c
@@ -51,6 +51,34 @@ static void remove_transaction_entry(struct fw_card *card, struct fw_transaction
 	card->transactions.tlabel_mask &= ~(1ULL << entry->tlabel);
 }
 
+// Must be called without holding card->transactions.lock.
+void fw_cancel_pending_transactions(struct fw_card *card)
+{
+	struct fw_transaction *t, *tmp;
+	LIST_HEAD(pending_list);
+
+	// 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_safe(t, tmp, &card->transactions.list, link) {
+			if (try_cancel_split_timeout(t))
+				list_move(&t->link, &pending_list);
+		}
+	}
+
+	list_for_each_entry_safe(t, tmp, &pending_list, link) {
+		list_del(&t->link);
+
+		if (!t->with_tstamp) {
+			t->callback.without_tstamp(card, RCODE_CANCELLED, NULL, 0,
+						   t->callback_data);
+		} else {
+			t->callback.with_tstamp(card, RCODE_CANCELLED, t->packet.timestamp, 0,
+						NULL, 0, t->callback_data);
+		}
+	}
+}
+
 // card->transactions.lock must be acquired in advance.
 #define find_and_pop_transaction_entry(card, condition)			\
 ({									\
diff --git a/drivers/firewire/core.h b/drivers/firewire/core.h
index 903812b6bb3f..41fb39d9a4e6 100644
--- a/drivers/firewire/core.h
+++ b/drivers/firewire/core.h
@@ -287,6 +287,8 @@ void fw_fill_response(struct fw_packet *response, u32 *request_header,
 void fw_request_get(struct fw_request *request);
 void fw_request_put(struct fw_request *request);
 
+void fw_cancel_pending_transactions(struct fw_card *card);
+
 // Convert the value of IEEE 1394 CYCLE_TIME register to the format of timeStamp field in
 // descriptors of 1394 OHCI.
 static inline u32 cycle_time_to_ohci_tstamp(u32 tstamp)
diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c
index 0625d11dbd74..e3e78dc42530 100644
--- a/drivers/firewire/ohci.c
+++ b/drivers/firewire/ohci.c
@@ -3719,11 +3719,6 @@ static void pci_remove(struct pci_dev *dev)
 
 	fw_core_remove_card(&ohci->card);
 
-	/*
-	 * FIXME: Fail all pending packets here, now that the upper
-	 * layers can't queue any more.
-	 */
-
 	software_reset(ohci);
 
 	irq = pci_irq_vector(dev, 0);
-- 
2.51.0

Re: BUG: spinlock bad magic on CPU#1, irq/39-firewire/245 (v6.18-rc4, ppc64)

[Takashi S. <o-t...@sa...>]

Hi,

Thanks for the report, and sorry for your inconvenience.

On Tue, Nov 11, 2025 at 01:41:21PM +0100, Erhard Furtner wrote:
> On 11/9/25 15:17, Erhard Furtner wrote:
> > [...]
> > firewire_ohci 0001:03:0e.0: added OHCI v1.0 device as card 0, 8 IR + 8
> > IT contexts, quirks 0x0
> > BUG: spinlock bad magic on CPU#1, irq/39-firewire/245
> >   lock: 0xc00000001f672618, .magic: 00000000, .owner: irq/39-
> > firewire/245, .owner_cpu: 1
> > CPU: 1 UID: 0 PID: 245 Comm: irq/39-firewire Tainted: G N  6.18.0-rc4-
> > PMacG5 #1 PREEMPTLAZY
> > Tainted: [N]=TEST
> > Hardware name: PowerMac11,2 PPC970MP 0x440101 PowerMac
> > Call Trace:
> > [c000000005dafb20] [c000000000bc054c] __dump_stack+0x30/0x54 (unreliable)
> > [c000000005dafb50] [c000000000bc04e4] dump_stack_lvl+0x98/0xd0
> > [c000000005dafb90] [c0000000000f22a8] spin_dump+0x88/0xb4
> > [c000000005dafc10] [c0000000000f1d4c] do_raw_spin_unlock+0xdc/0x164
> > [c000000005dafc50] [c000000000bf65d0] _raw_spin_unlock+0x18/0x68
> > [c000000005dafc70] [c0003d0013ce1d5c]
> > fw_core_handle_bus_reset+0xa98/0xb64 [firewire_core]
> > [c000000005dafdc0] [c0003d0013d19aec]
> > handle_selfid_complete_event+0x610/0x764 [firewire_ohci]
> > [c000000005dafe80] [c000000000106050] irq_thread_fn+0x40/0x9c
> > [c000000005dafec0] [c000000000105ecc] irq_thread+0x1c0/0x298
> > [c000000005daff60] [c0000000000b5e54] kthread+0x250/0x280
> > [c000000005daffe0] [c00000000000bd30] start_kernel_thread+0x14/0x18
> I bisected the issue. First bad commit is:
> 
>  # git bisect good
> 7d138cb269dbd2fa9b0da89a9c10503d1cf269d5 is the first bad commit
> commit 7d138cb269dbd2fa9b0da89a9c10503d1cf269d5
> Author: Takashi Sakamoto <o-t...@sa...>
> Date:   Tue Sep 16 08:47:44 2025 +0900
> 
>     firewire: core: use spin lock specific to topology map
> 
>     At present, the operation for read transaction to topology map register
> is
>     not protected by any kind of lock primitives. This causes a potential
>     problem to result in the mixed content of topology map.
> 
>     This commit adds and uses spin lock specific to topology map.
> 
>     Link:
> https://lore.kernel.org/r/202...@sa...
>     Signed-off-by: Takashi Sakamoto <o-t...@sa...>
> 
>  drivers/firewire/core-topology.c    | 22 ++++++++++++++--------
>  drivers/firewire/core-transaction.c |  6 +++++-
>  include/linux/firewire.h            |  6 +++++-
>  3 files changed, 24 insertions(+), 10 deletions(-)
> 
> 
> Bisect.log attached.

At present, I suspect the buffer overflow over 'struct
fw_card.topology_map.buffer[256]' and the cause would be unexpected value
of 'self_id_count' variable provided to 'fw_core_handle_bus_reset()'. It
means that in your machine the 1394 OHCI PCI driver computes the
unexpected value.

Would I ask you to retrieve verbose data by the following steps?

Step 1. Applying the following patch to avoid the suspicious buffer
overflow by limiting the pointer cursor within the buffer.

```
diff --git a/drivers/firewire/core-topology.c b/drivers/firewire/core-topology.c
index 2f73bcd5696f..5e66428ec4b0 100644
--- a/drivers/firewire/core-topology.c
+++ b/drivers/firewire/core-topology.c
@@ -442,6 +442,7 @@ static void update_topology_map(__be32 *buffer, size_t buffer_size, int root_nod
 {
        __be32 *map = buffer;
        int node_count = (root_node_id & 0x3f) + 1;
+       size_t count;

        memset(map, 0, buffer_size);

@@ -449,7 +450,10 @@ static void update_topology_map(__be32 *buffer, size_t buffer_size, int root_nod
        *map++ = cpu_to_be32(be32_to_cpu(buffer[1]) + 1);
        *map++ = cpu_to_be32((node_count << 16) | self_id_count);

-       while (self_id_count--)
+       count = buffer_size / sizeof(*buffer) - 3;
+       if (self_id_count > 0 && count > self_id_count)
+               count = self_id_count;
+       while (count--)
                *map++ = cpu_to_be32p(self_ids++);

        fw_compute_block_crc(buffer);

```

Step 2. The value of self_id_count can be retrieved as the part of
'firewire:bus_reset_handle' tracepoint event. Please work with Linux
tracepoints framework[1] and store the event log. I think unbind/bind
operation to firewire-ohci driver is useful[2].

[1] https://docs.kernel.org/trace/events.html
[2] https://lwn.net/Articles/143397/


Thanks

Takashi Sakamoto

Re: [PATCH] firewire: core: clear sources of hardware interrupt at card removal

[Takashi S. <o-t...@sa...>]

On Sun, Nov 09, 2025 at 03:55:25PM +0900, Takashi Sakamoto wrote:
> Due to the factors external to the system, hardware events may still be
> handled while a card instance is being removed. The sources of hardware
> IRQs should be cleared during card removal so that workqueues can be safely
> destroyed.
> 
> This commit adds a disable callback to the underlying driver operations.
> After this callback returns, the underlying driver guarantees that it
> will no longer handle hardware events.
> 
> Signed-off-by: Takashi Sakamoto <o-t...@sa...>
> ---
>  drivers/firewire/core-card.c |  3 +++
>  drivers/firewire/core.h      |  3 +++
>  drivers/firewire/ohci.c      | 44 +++++++++++++++++++++++++++++-------
>  3 files changed, 42 insertions(+), 8 deletions(-)

Applied to for-next branch.


Regards

Takashi Sakamoto

[PATCH] firewire: core: clear sources of hardware interrupt at card removal

[Takashi S. <o-t...@sa...>]

Due to the factors external to the system, hardware events may still be
handled while a card instance is being removed. The sources of hardware
IRQs should be cleared during card removal so that workqueues can be safely
destroyed.

This commit adds a disable callback to the underlying driver operations.
After this callback returns, the underlying driver guarantees that it
will no longer handle hardware events.

Signed-off-by: Takashi Sakamoto <o-t...@sa...>
---
 drivers/firewire/core-card.c |  3 +++
 drivers/firewire/core.h      |  3 +++
 drivers/firewire/ohci.c      | 44 +++++++++++++++++++++++++++++-------
 3 files changed, 42 insertions(+), 8 deletions(-)

diff --git a/drivers/firewire/core-card.c b/drivers/firewire/core-card.c
index 6979d6a88ae2..65bd9db996c0 100644
--- a/drivers/firewire/core-card.c
+++ b/drivers/firewire/core-card.c
@@ -784,9 +784,12 @@ void fw_core_remove_card(struct fw_card *card)
 	/* 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;
 	card->driver = &dummy_driver;
+
 	drain_workqueue(card->isoc_wq);
 	drain_workqueue(card->async_wq);
+	card->driver->disable(card);
 
 	scoped_guard(spinlock_irqsave, &card->lock)
 		fw_destroy_nodes(card);
diff --git a/drivers/firewire/core.h b/drivers/firewire/core.h
index e67395ce26b5..903812b6bb3f 100644
--- a/drivers/firewire/core.h
+++ b/drivers/firewire/core.h
@@ -65,6 +65,9 @@ struct fw_card_driver {
 	int (*enable)(struct fw_card *card,
 		      const __be32 *config_rom, size_t length);
 
+	// After returning the call, any function is no longer triggered to handle hardware event.
+	void (*disable)(struct fw_card *card);
+
 	int (*read_phy_reg)(struct fw_card *card, int address);
 	int (*update_phy_reg)(struct fw_card *card, int address,
 			      int clear_bits, int set_bits);
diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c
index 757dd9c64b1c..0625d11dbd74 100644
--- a/drivers/firewire/ohci.c
+++ b/drivers/firewire/ohci.c
@@ -2408,6 +2408,41 @@ static int ohci_enable(struct fw_card *card,
 	return 0;
 }
 
+static void ohci_disable(struct fw_card *card)
+{
+	struct pci_dev *pdev = to_pci_dev(card->device);
+	struct fw_ohci *ohci = pci_get_drvdata(pdev);
+	int i, irq = pci_irq_vector(pdev, 0);
+
+	// If the removal is happening from the suspend state, LPS won't be enabled and host
+	// registers (eg., IntMaskClear) won't be accessible.
+	if (!(reg_read(ohci, OHCI1394_HCControlSet) & OHCI1394_HCControl_LPS))
+		return;
+
+	reg_write(ohci, OHCI1394_IntMaskClear, ~0);
+	flush_writes(ohci);
+
+	if (irq >= 0)
+		synchronize_irq(irq);
+
+	flush_work(&ohci->ar_request_ctx.work);
+	flush_work(&ohci->ar_response_ctx.work);
+	flush_work(&ohci->at_request_ctx.work);
+	flush_work(&ohci->at_response_ctx.work);
+
+	for (i = 0; i < ohci->n_ir; ++i) {
+		if (!(ohci->ir_context_mask & BIT(i)))
+			flush_work(&ohci->ir_context_list[i].base.work);
+	}
+	for (i = 0; i < ohci->n_it; ++i) {
+		if (!(ohci->it_context_mask & BIT(i)))
+			flush_work(&ohci->it_context_list[i].base.work);
+	}
+
+	at_context_flush(&ohci->at_request_ctx);
+	at_context_flush(&ohci->at_response_ctx);
+}
+
 static int ohci_set_config_rom(struct fw_card *card,
 			       const __be32 *config_rom, size_t length)
 {
@@ -3442,6 +3477,7 @@ static int ohci_flush_iso_completions(struct fw_iso_context *base)
 
 static const struct fw_card_driver ohci_driver = {
 	.enable			= ohci_enable,
+	.disable		= ohci_disable,
 	.read_phy_reg		= ohci_read_phy_reg,
 	.update_phy_reg		= ohci_update_phy_reg,
 	.set_config_rom		= ohci_set_config_rom,
@@ -3681,14 +3717,6 @@ static void pci_remove(struct pci_dev *dev)
 	struct fw_ohci *ohci = pci_get_drvdata(dev);
 	int irq;
 
-	/*
-	 * If the removal is happening from the suspend state, LPS won't be
-	 * enabled and host registers (eg., IntMaskClear) won't be accessible.
-	 */
-	if (reg_read(ohci, OHCI1394_HCControlSet) & OHCI1394_HCControl_LPS) {
-		reg_write(ohci, OHCI1394_IntMaskClear, ~0);
-		flush_writes(ohci);
-	}
 	fw_core_remove_card(&ohci->card);
 
 	/*

base-commit: fa2dc27100768a8a994c377051fa17a47cc66c76
-- 
2.51.0

Re: [PATCH] firewire: core: add WQ_PERCPU to alloc_workqueue users

[Takashi S. <o-t...@sa...>]

Hi,

Thanks for the patch.

On Fri, Nov 07, 2025 at 12:26:19PM +0100, Marco Crivellari wrote:
> Currently if a user enqueues a work item using schedule_delayed_work() the
> used wq is "system_wq" (per-cpu wq) while queue_delayed_work() use
> WORK_CPU_UNBOUND (used when a cpu is not specified). The same applies to
> schedule_work() that is using system_wq and queue_work(), that makes use
> again of WORK_CPU_UNBOUND.
> This lack of consistency cannot be addressed without refactoring the API.
> 
> alloc_workqueue() treats all queues as per-CPU by default, while unbound
> workqueues must opt-in via WQ_UNBOUND.
> 
> This default is suboptimal: most workloads benefit from unbound queues,
> allowing the scheduler to place worker threads where they’re needed and
> reducing noise when CPUs are isolated.
> 
> This continues the effort to refactor workqueue APIs, which began with
> the introduction of new workqueues and a new alloc_workqueue flag in:
> 
> commit 128ea9f6ccfb ("workqueue: Add system_percpu_wq and system_dfl_wq")
> commit 930c2ea566af ("workqueue: Add new WQ_PERCPU flag")
> 
> This change adds a new WQ_PERCPU flag to explicitly request alloc_workqueue()
> to be per-cpu when WQ_UNBOUND has not been specified.
> 
> With the introduction of the WQ_PERCPU flag (equivalent to !WQ_UNBOUND),
> any alloc_workqueue() caller that doesn’t explicitly specify WQ_UNBOUND
> must now use WQ_PERCPU.
> 
> Once migration is complete, WQ_UNBOUND can be removed and unbound will
> become the implicit default.
> 
> Suggested-by: Tejun Heo <tj...@ke...>
> Signed-off-by: Marco Crivellari <mar...@su...>
> ---
>  drivers/firewire/core-transaction.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/firewire/core-transaction.c b/drivers/firewire/core-transaction.c
> index c65f491c54d0..c15dbe882cbe 100644
> --- a/drivers/firewire/core-transaction.c
> +++ b/drivers/firewire/core-transaction.c
> @@ -1437,7 +1437,8 @@ static int __init fw_core_init(void)
>  {
>  	int ret;
>  
> -	fw_workqueue = alloc_workqueue("firewire", WQ_MEM_RECLAIM, 0);
> +	fw_workqueue = alloc_workqueue("firewire", WQ_MEM_RECLAIM | WQ_PERCPU,
> +				       0);
>  	if (!fw_workqueue)
>  		return -ENOMEM;

As far as I know, there is no specific reason to use per-cpu workqueue
for this purpose in this subsystem. I believe that using unbound workqueue
would be more beneficial, since the workqueue users just operate chained
1394 OHCI DMA descriptor over system memory for asynchronous
communication.

Would it be acceptable for you to add WQ_UNBOUND flag to the workqueue?
If so, I can prepare a patch for the next merge window.


Thanks

Takashi Sakamoto

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

[Takashi S. <o-t...@sa...>]

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

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

[Takashi S. <o-t...@sa...>]

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-t...@sa...>
---
 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 S. <o-t...@sa...>]

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-t...@sa...>
---
 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

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

[Takashi S. <o-t...@sa...>]

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

Re: [PATCH 0/4] firewire: ohci: remove obsolete module-level debug parameter

[Adam G. <ad...@po...>]

On Sat, Oct 25, 2025 at 09:46:29AM +0900, Takashi Sakamoto wrote:
> In my opinion, using tracepoints means to leave from the message buffer
> for printk once. There are several ways to retrieve the content of ring
> buffer for tracepoints events in userspace, and we have some userspace
> applications to utilize them.
> 
> 1. By debugfs
> 2. By file descriptor returned from perf_event_open(2) system call
> 3. By tracefs
> 4. By printk message buffer
> 5. By BPF
>
> [...]

Hi Takashi,

Thank you for your guidance. It is very helpful.

-- Adam

[GIT PULL] firewire fixes for v6.18-rc3

[Takashi S. <o-t...@sa...>]

Hi Linus,

Please apply the following fixes to your tree for FireWire subsystem.


The following changes since commit 211ddde0823f1442e4ad052a2f30f050145ccada:

  Linux 6.18-rc2 (2025-10-19 15:19:16 -1000)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/ieee1394/linux1394.git tags/firewire-fixes-6.18-rc3

for you to fetch changes up to 73ba88fb04081372a69f0395958ac6b65d53d134:

  firewire: init_ohci1394_dma: add missing function parameter documentation (2025-10-25 08:29:56 +0900)

----------------------------------------------------------------
firewire fixes for 6.18-rc3

A small collection of FireWire fixes. This includes collections to sparse
and API documentation.

----------------------------------------------------------------
Nirbhay Sharma (1):
      firewire: init_ohci1394_dma: add missing function parameter documentation

Takashi Sakamoto (1):
      firewire: core: fix __must_hold() annotation

 drivers/firewire/core-transaction.c  |  2 +-
 drivers/firewire/init_ohci1394_dma.c | 10 ++++++++++
 2 files changed, 11 insertions(+), 1 deletion(-)


Regards

Takashi Sakamoto

Re: [PATCH 0/4] firewire: ohci: remove obsolete module-level debug parameter

[Takashi S. <o-t...@sa...>]

Hi Adam,

On Tue, Oct 21, 2025 at 11:36:42PM -0700, Adam Goldman wrote:
> On Thu, Aug 21, 2025 at 09:30:13AM +0900, Takashi Sakamoto wrote:
> > The "firewire-ohci" module has long provided a "debug" parameter that
> > enabled debug logging by calling printk() from hardIRQ context.
> > 
> > Between v6.11 and v6.12, a series of tracepoints events have been added as
> > a more suitable alternative. Since v6.12, a commit cd7023729877
> > ("firewire: ohci: deprecate debug parameter") has already marked the
> > parameter as deprecated.
> > 
> > This series removes the parameter, as its functionality is now fully
> > covered by tracepoints.
> 
> Hi Takashi,
> 
> Now that the "debug" parameter has been removed, can you provide 
> instructions for using tracepoints? For example, what is the new 
> procedure instead of adding "debug=7" to the module command line? What 
> is the equivalent to 
> "echo -1 > /sys/module/firewire_ohci/parameters/debug"?
> 
> -- Adam

In my opinion, using tracepoints means to leave from the message buffer
for printk once. There are several ways to retrieve the content of ring
buffer for tracepoints events in userspace, and we have some userspace
applications to utilize them.

1. By debugfs
2. By file descriptor returned from perf_event_open(2) system call
3. By tracefs
4. By printk message buffer
5. By BPF

For the 1st option, it is required to mount 'debugfs' into anywhere in
your root file system. In my environment:

```
$ mount | grep debugfs
debugfs on /sys/kernel/debug type debugfs (rw,nosuid,nodev,noexec,relatime)
```

Then you can see many directories for events under
'/sys/kernel/debug/tracing/events/'. For the events specific to this
subsystem:

* /sys/kernel/debug/tracing/events/firewire_ohci
  * irq
  * self_id_complete
* /sys/kernel/debug/tracing/events/firewire
  * bus_reset_handle
  * self_id_sequence
  * bus_reset_schedule
  * bus_reset_postpone
  * bus_reset_initiate
  * async_phy_inbound
  * async_phy_outbound_initiate
  * async_phy_outbound_complete
  * etc...

Each of the above directory includes 'enable' file. By writing 1 to the
file, the corresponding event is enabled.

```
$ echo 1 > /sys/kernel/debug/tracing/events/firewire/self_id_sequence/enable
```

The read operation to '/sys/kernel/debug/tracing/trace' retrieves the event
content from the ring buffer.

```
$ cat /sys/kernel/debug/tracing/trace
 irq/121-firewir-73902   [000] ...1. 114132.060856: self_id_sequence: card_index=0 generation=4 phy_id=0x00 link_active=true gap_count=63 scode=3 contender=true power_class=4 initiated_reset=true port_status={0x1,0x1,0x1} self_id_sequence={0x807fcc56}
```

I think this is the most-straightforward way to use the tracepoints
framework. Using cat and shell-builtin echo commands satisfies our aim.


For the 2nd option, perf(1) command would be a good fontend application.
For tracepoints events, 'list', 'record', and 'script' subcommands are
available to enumerate events, record events, and report. In
detail, see https://perfwiki.github.io/main/.


For the 3rd option, trace-cmd(1) would be a good frontend application.
For tracepoints events, 'list', 'record', and 'report' subcommands are
available. In detail, see https://www.trace-cmd.org/.


For the 4th option, we need to use either some kernel command-line options
or corresponding sysctl configurations:
* tp_printk
* trace_event

This way has an advantage at boot time analysis. In detail, see:
* https://docs.kernel.org/admin-guide/kernel-parameters.html


I have never used the 5th option, since it is relatively new,


Regards

Takashi Sakamoto

Re: [PATCH] firewire: init_ohci1394_dma: add missing function parameter documentation

[Takashi S. <o-t...@sa...>]

On Sat, Oct 25, 2025 at 02:02:19AM +0530, Nirbhay Sharma wrote:
> Add missing kernel-doc parameter descriptions for five functions
> in init_ohci1394_dma.c to fix documentation warnings when building
> with W=1.
> 
> This patch addresses the following warnings:
> - init_ohci1394_wait_for_busresets: missing @ohci description
> - init_ohci1394_enable_physical_dma: missing @ohci description
> - init_ohci1394_reset_and_init_dma: missing @ohci description
> - init_ohci1394_controller: missing @num, @slot, @func descriptions
> - setup_ohci1394_dma: missing @opt description
> 
> Tested with GCC 13.2.0 and W=1 flag. All documentation warnings
> for these functions have been resolved.
> 
> Signed-off-by: Nirbhay Sharma <nir...@gm...>
> ---
>  drivers/firewire/init_ohci1394_dma.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)

Applied to for-linus branch.


Thanks

Takashi Sakamoto

Re: [PATCH] firewire: core: fix __must_hold() annotation

[Takashi S. <o-t...@sa...>]

On Thu, Oct 23, 2025 at 07:43:49PM +0900, Takashi Sakamoto wrote:
> The variable name passed to __must_hold() annotation is invalid.
> 
> This commit fixes it.
> 
> Fixes: 420bd7068cbf ("firewire: core: use spin lock specific to transaction")
> Signed-off-by: Takashi Sakamoto <o-t...@sa...>
> ---
>  drivers/firewire/core-transaction.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/firewire/core-transaction.c b/drivers/firewire/core-transaction.c
> index dd3656a0c1ff..c65f491c54d0 100644
> --- a/drivers/firewire/core-transaction.c
> +++ b/drivers/firewire/core-transaction.c
> @@ -269,7 +269,7 @@ static void fw_fill_request(struct fw_packet *packet, int tcode, int tlabel,
>  }
>  
>  static int allocate_tlabel(struct fw_card *card)
> -__must_hold(&card->transactions_lock)
> +__must_hold(&card->transactions.lock)
>  {
>  	int tlabel;

Applied to for-linus branch.


Regards

Takashi Sakamoto

[PATCH] firewire: core: fix __must_hold() annotation

[Takashi S. <o-t...@sa...>]

The variable name passed to __must_hold() annotation is invalid.

This commit fixes it.

Fixes: 420bd7068cbf ("firewire: core: use spin lock specific to transaction")
Signed-off-by: Takashi Sakamoto <o-t...@sa...>
---
 drivers/firewire/core-transaction.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/firewire/core-transaction.c b/drivers/firewire/core-transaction.c
index dd3656a0c1ff..c65f491c54d0 100644
--- a/drivers/firewire/core-transaction.c
+++ b/drivers/firewire/core-transaction.c
@@ -269,7 +269,7 @@ static void fw_fill_request(struct fw_packet *packet, int tcode, int tlabel,
 }
 
 static int allocate_tlabel(struct fw_card *card)
-__must_hold(&card->transactions_lock)
+__must_hold(&card->transactions.lock)
 {
 	int tlabel;
 

base-commit: 211ddde0823f1442e4ad052a2f30f050145ccada
-- 
2.51.0

Re: [PATCH 0/4] firewire: ohci: remove obsolete module-level debug parameter

[Adam G. <ad...@po...>]

On Thu, Aug 21, 2025 at 09:30:13AM +0900, Takashi Sakamoto wrote:
> The "firewire-ohci" module has long provided a "debug" parameter that
> enabled debug logging by calling printk() from hardIRQ context.
> 
> Between v6.11 and v6.12, a series of tracepoints events have been added as
> a more suitable alternative. Since v6.12, a commit cd7023729877
> ("firewire: ohci: deprecate debug parameter") has already marked the
> parameter as deprecated.
> 
> This series removes the parameter, as its functionality is now fully
> covered by tracepoints.

Hi Takashi,

Now that the "debug" parameter has been removed, can you provide 
instructions for using tracepoints? For example, what is the new 
procedure instead of adding "debug=7" to the module command line? What 
is the equivalent to 
"echo -1 > /sys/module/firewire_ohci/parameters/debug"?

-- Adam

Re: [PATCH] firewire: core: use cleanup function to release cached configuration ROM

[Takashi S. <o-t...@sa...>]

On Mon, Oct 20, 2025 at 08:58:10PM +0900, Takashi Sakamoto wrote:
> When returning from read_config_rom() function, the allocated buffer and
> the previous buffer for configuration ROM should be released. The cleanup
> function is useful in the case.
> 
> This commit uses the cleanup function to remove goto statements.
> 
> Signed-off-by: Takashi Sakamoto <o-t...@sa...>
> ---
>  drivers/firewire/core-device.c | 34 ++++++++++++----------------------
>  1 file changed, 12 insertions(+), 22 deletions(-)

Applied to for-next branch.


Regards

Takashi Sakamoto

[PATCH] firewire: core: use cleanup function to release cached configuration ROM

[Takashi S. <o-t...@sa...>]

When returning from read_config_rom() function, the allocated buffer and
the previous buffer for configuration ROM should be released. The cleanup
function is useful in the case.

This commit uses the cleanup function to remove goto statements.

Signed-off-by: Takashi Sakamoto <o-t...@sa...>
---
 drivers/firewire/core-device.c | 34 ++++++++++++----------------------
 1 file changed, 12 insertions(+), 22 deletions(-)

diff --git a/drivers/firewire/core-device.c b/drivers/firewire/core-device.c
index 1674de477852..9b0080397154 100644
--- a/drivers/firewire/core-device.c
+++ b/drivers/firewire/core-device.c
@@ -653,8 +653,8 @@ static int read_rom(struct fw_device *device, int generation, int speed, int ind
 static int read_config_rom(struct fw_device *device, int generation)
 {
 	struct fw_card *card = device->card;
-	const u32 *old_rom, *new_rom;
-	u32 *rom, *stack;
+	const u32 *new_rom, *old_rom __free(kfree) = NULL;
+	u32 *stack, *rom __free(kfree) = NULL;
 	u32 sp, key;
 	int i, end, length, ret, speed;
 	int quirks;
@@ -673,7 +673,7 @@ static int read_config_rom(struct fw_device *device, int generation)
 	for (i = 0; i < 5; i++) {
 		ret = read_rom(device, generation, speed, i, &rom[i]);
 		if (ret != RCODE_COMPLETE)
-			goto out;
+			return ret;
 		/*
 		 * As per IEEE1212 7.2, during initialization, devices can
 		 * reply with a 0 for the first quadlet of the config
@@ -682,10 +682,8 @@ static int read_config_rom(struct fw_device *device, int generation)
 		 * harddisk).  In that case we just fail, and the
 		 * retry mechanism will try again later.
 		 */
-		if (i == 0 && rom[i] == 0) {
-			ret = RCODE_BUSY;
-			goto out;
-		}
+		if (i == 0 && rom[i] == 0)
+			return RCODE_BUSY;
 	}
 
 	quirks = detect_quirks_by_bus_information_block(rom);
@@ -712,15 +710,13 @@ static int read_config_rom(struct fw_device *device, int generation)
 		 */
 		key = stack[--sp];
 		i = key & 0xffffff;
-		if (WARN_ON(i >= MAX_CONFIG_ROM_SIZE)) {
-			ret = -ENXIO;
-			goto out;
-		}
+		if (WARN_ON(i >= MAX_CONFIG_ROM_SIZE))
+			return -ENXIO;
 
 		/* Read header quadlet for the block to get the length. */
 		ret = read_rom(device, generation, speed, i, &rom[i]);
 		if (ret != RCODE_COMPLETE)
-			goto out;
+			return ret;
 		end = i + (rom[i] >> 16) + 1;
 		if (end > MAX_CONFIG_ROM_SIZE) {
 			/*
@@ -744,7 +740,7 @@ static int read_config_rom(struct fw_device *device, int generation)
 		for (; i < end; i++) {
 			ret = read_rom(device, generation, speed, i, &rom[i]);
 			if (ret != RCODE_COMPLETE)
-				goto out;
+				return ret;
 
 			if ((key >> 30) != 3 || (rom[i] >> 30) < 2)
 				continue;
@@ -804,25 +800,19 @@ static int read_config_rom(struct fw_device *device, int generation)
 
 	old_rom = device->config_rom;
 	new_rom = kmemdup(rom, length * 4, GFP_KERNEL);
-	if (new_rom == NULL) {
-		ret = -ENOMEM;
-		goto out;
-	}
+	if (new_rom == NULL)
+		return -ENOMEM;
 
 	scoped_guard(rwsem_write, &fw_device_rwsem) {
 		device->config_rom = new_rom;
 		device->config_rom_length = length;
 	}
 
-	kfree(old_rom);
-	ret = RCODE_COMPLETE;
 	device->max_rec	= rom[2] >> 12 & 0xf;
 	device->cmc	= rom[2] >> 30 & 1;
 	device->irmc	= rom[2] >> 31 & 1;
- out:
-	kfree(rom);
 
-	return ret;
+	return RCODE_COMPLETE;
 }
 
 static void fw_unit_release(struct device *dev)

base-commit: dbd0cf204fe6ba7ba226153d1d90369019b90164
-- 
2.51.0