linux1394-devel

[GIT PULL] firewire fixes for 6.12-rc5

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

Hi Linus,

Please accept this PR from firewire subsystem to fix a regression.


The following changes since commit 42f7652d3eb527d03665b09edac47f85fb600924:

  Linux 6.12-rc4 (2024-10-20 15:19:38 -0700)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/ieee1394/linux1394.git tags/firewire-fixes-6.12-rc5

for you to fetch changes up to f6a6780e0b9bbcf311a727afed06fee533a5e957:

  firewire: core: fix invalid port index for parent device (2024-10-27 11:14:35 +0900)

----------------------------------------------------------------
firewire fixes for 6.12-rc5

This update includes a single commit to resolve a regression existing in
v6.11 or later.

The change in 1394 OHCI driver in v6.11 kernel could cause general
protection faults when rediscovering nodes in IEEE 1394 bus while holding
a spin lock. Consequently, watchdog checks can report a hard lockup.

Currently, this issue is observed primarily during the system resume phase
when using an extra node with three ports or more is used. However, it
could potentially occur in the other cases as well.

----------------------------------------------------------------
Takashi Sakamoto (1):
      firewire: core: fix invalid port index for parent device

 drivers/firewire/core-topology.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


Regards

Takashi Sakamoto

Re: [PATCH] firewire: core: fix invalid port index for parent device

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

On Fri, Oct 25, 2024 at 12:41:37PM +0900, Takashi Sakamoto wrote:
> In a commit 24b7f8e5cd65 ("firewire: core: use helper functions for self
> ID sequence"), the enumeration over self ID sequence was refactored with
> some helper functions with KUnit tests. These helper functions are
> guaranteed to work expectedly by the KUnit tests, however their application
> includes a mistake to assign invalid value to the index of port connected
> to parent device.
> 
> This bug affects the case that any extra node devices which has three or
> more ports are connected to 1394 OHCI controller. In the case, the path
> to update the tree cache could hits WARN_ON(), and gets general protection
> fault due to the access to invalid address computed by the invalid value.
> 
> This commit fixes the bug to assign correct port index.
> 
> Cc: st...@vg...
> Reported-by: Edmund Raile <edm...@pr...>
> Closes: https://lore.kernel.org/lkml/8a9...@pr.../
> Fixes: 24b7f8e5cd65 ("firewire: core: use helper functions for self ID sequence")
> Signed-off-by: Takashi Sakamoto <o-t...@sa...>
> ---
>  drivers/firewire/core-topology.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Applied to for-linus branch.


Regards

Takashi Sakamoto

Re: firewire-ohci: device rediscovery hardlock regression

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

Hi,

Thanks for the bug report.

Coincidentally, I face the same problem with my TC Electronic Desktop
Konnekt 6, which reports one available port and two invalidated ports, and
investigate its cause. I think the problem occurs just for the devices
which have three or more ports.

I sent a fix[1] just now by referring to your suggestions. Would you please
evaluate the fix with your device?

I'm sorry for your inconvenience.


[1] [PATCH] firewire: core: fix invalid port index for parent device
https://lore.kernel.org/lkml/202...@sa.../


Thanks

Takashi Sakamoto

On Thu, Oct 24, 2024 at 01:56:31PM +0000, Edmund Raile wrote:
> Hello,
> 
> I'd like to report a regression in firewire-ohci that results
> in the kernel hardlocking when re-discovering a FireWire device.
> 
> TI XIO2213B
> RME FireFace 800
> 
> It will occur under three conditions:
>  * power-cycling the FireWire device
>  * un- and re-plugging the FireWire device
>  * suspending and then waking the PC
> 
> Often it would also occur directly on boot in QEMU but I have not
> yet observed this specific behavior on bare metal.
> 
> Here is an excerpt from the stack trace (don't know whether it is
> acceptable to send in full):
> 
> kernel: ------------[ cut here ]------------
> kernel: refcount_t: addition on 0; use-after-free.
> kernel: WARNING: CPU: 3 PID: 116 at lib/refcount.c:25
> refcount_warn_saturate (/build/linux/lib/refcount.c:25 (discriminator
> 1)) 
> kernel: Workqueue: firewire_ohci bus_reset_work
> kernel: RIP: 0010:refcount_warn_saturate
> (/build/linux/lib/refcount.c:25 (discriminator 1)) 
> kernel: Call Trace:
> kernel:  <TASK>
> kernel: ? refcount_warn_saturate (/build/linux/lib/refcount.c:25
> (discriminator 1)) 
> kernel: ? __warn.cold (/build/linux/kernel/panic.c:693) 
> kernel: ? refcount_warn_saturate (/build/linux/lib/refcount.c:25
> (discriminator 1)) 
> kernel: ? report_bug (/build/linux/lib/bug.c:180
> /build/linux/lib/bug.c:219) 
> kernel: ? handle_bug (/build/linux/arch/x86/kernel/traps.c:218) 
> kernel: ? exc_invalid_op (/build/linux/arch/x86/kernel/traps.c:260
> (discriminator 1)) 
> kernel: ? asm_exc_invalid_op
> (/build/linux/./arch/x86/include/asm/idtentry.h:621) 
> kernel: ? refcount_warn_saturate (/build/linux/lib/refcount.c:25
> (discriminator 1)) 
> kernel: for_each_fw_node (/build/linux/./include/linux/refcount.h:190
> /build/linux/./include/linux/refcount.h:241
> /build/linux/./include/linux/refcount.h:258
> /build/linux/drivers/firewire/core.h:199
> /build/linux/drivers/firewire/core-topology.c:275) 
> kernel: ? __pfx_report_found_node (/build/linux/drivers/firewire/core-
> topology.c:312) 
> kernel: fw_core_handle_bus_reset (/build/linux/drivers/firewire/core-
> topology.c:399 (discriminator 1) /build/linux/drivers/firewire/core-
> topology.c:504 (discriminator 1)) 
> kernel: bus_reset_work (/build/linux/drivers/firewire/ohci.c:2121) 
> kernel: process_one_work
> (/build/linux/./arch/x86/include/asm/jump_label.h:27
> /build/linux/./include/linux/jump_label.h:207
> /build/linux/./include/trace/events/workqueue.h:110
> /build/linux/kernel/workqueue.c:3236) 
> kernel: worker_thread (/build/linux/kernel/workqueue.c:3306
> (discriminator 2) /build/linux/kernel/workqueue.c:3393 (discriminator
> 2)) 
> kernel: ? __pfx_worker_thread (/build/linux/kernel/workqueue.c:3339) 
> kernel: kthread (/build/linux/kernel/kthread.c:389) 
> kernel: ? __pfx_kthread (/build/linux/kernel/kthread.c:342) 
> kernel: ret_from_fork (/build/linux/arch/x86/kernel/process.c:153) 
> kernel: ? __pfx_kthread (/build/linux/kernel/kthread.c:342) 
> kernel: ret_from_fork_asm (/build/linux/arch/x86/entry/entry_64.S:254) 
> kernel:  </TASK>
> 
> I have identified the commit via bisection:
> 24b7f8e5cd656196a13077e160aec45ad89b58d9
> firewire: core: use helper functions for self ID sequence
> 
> It was part of the following patch series:
> firewire: add tracepoints events for self ID sequence
> https://lore.kernel.org/all/202...@sa.../
> 
> #regzbot introduced: 24b7f8e5cd65
> 
> Since this was before v6.10-rc5 and stable 6.10.14 is EOL,
> stable v6.11.5 and mainline are affected.
> 
> Reversion appears to be non-trivial as it is part of a patch
> series, other files have been altered as well and other commits
> build on top of it.
> 
> Call chain:
> core-topology.c fw_core_handle_bus_reset()
> -> core-topology.c   for_each_fw_node(card, local_node,
> report_found_node)
> -> core.h            fw_node_get(root)
> -> refcount.h        __refcount_inc(&node)
> -> refcount.h        __refcount_add(1, r, oldp);
> -> refcount.h        refcount_warn_saturate(r, REFCOUNT_ADD_UAF);
> -> refcount.h        REFCOUNT_WARN("addition on 0; use-after-free")
> 
> Since local_node of fw_core_handle_bus_reset() is retrieved by
> 	local_node = build_tree(card, self_ids, self_id_count);
> build_tree() needs to be looked at, it was indeed altered by
> 24b7f8e5cd65.
> 
> After a hard 3 hour look traversing all used functions and comparing
> against the original function (as of e404cacfc5ed), this caught my eye:
>        for (port_index = 0; port_index < total_port_count;
> ++port_index) {
>                switch (port_status) {
>                case PHY_PACKET_SELF_ID_PORT_STATUS_PARENT:
>                        node->color = i;
> 
> In both for loops, "port_index" was replaced by "i"
> "i" remains in use above:
>        for (i = 0, h = &stack; i < child_port_count; i++)
>                h = h->prev;
> 
> While the original also used the less descriptive i in the loop
>        for (i = 0; i < port_count; i++) {
>                switch (get_port_type(sid, i)) {
>                case SELFID_PORT_PARENT:
>                         node->color = i;
> but reset it to 0 at the beginning of the loop.
> 
> So the stray "i" in the for loop should be replaced with the loop
> iterator "port_index" as it is meant to be synchronous with the
> loop iterator (i.e. the port_index), no?
> 
> diff --git a/drivers/firewire/core-topology.c b/drivers/firewire/core-
> topology.c
> index 8c10f47cc8fc..7fd91ba9c9c4 100644
> --- a/drivers/firewire/core-topology.c
> +++ b/drivers/firewire/core-topology.c
> @@ -207,7 +207,7 @@ static struct fw_node *build_tree(struct fw_card
> *card, const u32 *sid, int self
>                                 // the node->ports array where the
> parent node should be.  Later,
>                                 // when we handle the parent node, we
> fix up the reference.
>                                 ++parent_count;
> -                               node->color = i;
> +                               node->color = port_index;
>                                 break;
> 
> What threw me off was discaridng node->color as it would be replaced
> later anyways (can't be important!), or so I thought.
> 
> Please tell me, is this line of reasoning correct or am I missing
> something?
> 
> Compiling 24b7f8e5cd65 and later mainline with the patch above
> resulted in a kernel that didn't crash!
> 
> In case my solution should turn out to be correct, I will gladly
> submit the patch.
> 
> Kind regards,
> Edmund Raile.

[PATCH] firewire: core: fix invalid port index for parent device

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

In a commit 24b7f8e5cd65 ("firewire: core: use helper functions for self
ID sequence"), the enumeration over self ID sequence was refactored with
some helper functions with KUnit tests. These helper functions are
guaranteed to work expectedly by the KUnit tests, however their application
includes a mistake to assign invalid value to the index of port connected
to parent device.

This bug affects the case that any extra node devices which has three or
more ports are connected to 1394 OHCI controller. In the case, the path
to update the tree cache could hits WARN_ON(), and gets general protection
fault due to the access to invalid address computed by the invalid value.

This commit fixes the bug to assign correct port index.

Cc: st...@vg...
Reported-by: Edmund Raile <edm...@pr...>
Closes: https://lore.kernel.org/lkml/8a9...@pr.../
Fixes: 24b7f8e5cd65 ("firewire: core: use helper functions for self ID sequence")
Signed-off-by: Takashi Sakamoto <o-t...@sa...>
---
 drivers/firewire/core-topology.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/firewire/core-topology.c b/drivers/firewire/core-topology.c
index 6adadb11962e..892b94cfd626 100644
--- a/drivers/firewire/core-topology.c
+++ b/drivers/firewire/core-topology.c
@@ -204,7 +204,7 @@ static struct fw_node *build_tree(struct fw_card *card, const u32 *sid, int self
 				// the node->ports array where the parent node should be.  Later,
 				// when we handle the parent node, we fix up the reference.
 				++parent_count;
-				node->color = i;
+				node->color = port_index;
 				break;
 
 			case PHY_PACKET_SELF_ID_PORT_STATUS_CHILD:
-- 
2.45.2

Re: [PATCH v1] firewire: Correct some typos

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

Hi,

On Mon, Sep 30, 2024 at 10:33:44AM +0800, Shen Lichuan wrote:
> Fixed some confusing typos that were currently identified with codespell,
> the details are as follows:
> 
> drivers/firewire/core-topology.c:59: containted ==> contained
> drivers/firewire/core.h:83: reenable ==> re-enable
> drivers/firewire/ohci.c:1387: apppends ==> appends
> drivers/firewire/ohci.c:2216: superceded ==> superseded
> drivers/firewire/ohci.c:2617: litte ==> little
> 
> Signed-off-by: Shen Lichuan <she...@vi...>
> ---
>  drivers/firewire/core-topology.c | 2 +-
>  drivers/firewire/core.h          | 2 +-
>  drivers/firewire/ohci.c          | 6 +++---
>  3 files changed, 5 insertions(+), 5 deletions(-)

Applied to for-next branch.


Thanks

Takashi Sakamoto

[GIT PULL] firewire updates for v6.12

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

Hi Linus,

Please pull FireWire subsystem updates for v6.11.

This updates includes a significant change to the processing context for
isochronous packets. Hopefully, many developers will test it before the
official release.

I hope you have a pleasant trip ;)


The following changes since commit de9c2c66ad8e787abec7c9d7eff4f8c3cdd28aed:

  Linux 6.11-rc2 (2024-08-04 13:50:53 -0700)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/ieee1394/linux1394.git tags/firewire-updates-6.12

for you to fetch changes up to f1cba5212e252243a539e079813bc96fbf53e241:

  firewire: core: rename cause flag of tracepoints event (2024-09-12 22:30:38 +0900)

----------------------------------------------------------------
firewire updates for v6.12

The batch of changes includes the followwing:

- Replacing tasklet with usual workqueue for isochronous context
- Replacing IDR with XArray
- Utilizing guard macro where possible
- Printing deprecation warning when enabling debug parameter of
  firewire-ohci module

Additionally, it includes a single patch for sound subsystem which the
subsystem maintainer acked:

- Switching to nonatomic PCM operation

In FireWire subsystem, tasklet has been used as the bottom half of 1394
OHCi hardIRQ so long. In the recent kernel updates, BH workqueue has
been available, and some developers have proposed replacing tasklet with
BH workqueue. While it is fortunate that developers are still considering
the legacy subsystem, a simple replacement is not necessarily suitable.

As a first step towards dropping tasklet, I've investigated the
feasibility for 1394 OHCI isochronous context, and concluded that usual
workqueue is available. In the context, the batch of packets is processed
in the specific queue, thus the timing jitter caused by task scheduling is
not so critical. Additionally, DMA transmission can be scheduled
per-packet basis, therefore the context can be sleep between the operation
of transmissions. Furthermore, in-kernel protocol implementation involves
some CPU-bound tasks, which can sometimes consumes CPU time so long. These
characteristics suggest that usual workqueue is suitable, through BH
workqueues are not.

The replacement with usual workqueue allows unit drivers to process the
content of packets in non-atomic context. It brings some reliefs to some
drivers in sound subsystem that spin-lock is not mandatory anymore during
isochronous packet processing.

----------------------------------------------------------------
Takashi Sakamoto (55):
      firewire: ohci: use TCODE_LINK_INTERNAL consistently
      firewire: ohci: minor code refactoring to localize text table
      firewire: core: use common helper function to serialize phy configuration packet
      firewire: core: utilize kref to maintain fw_node with reference counting
      firewire: ohci: add static inline functions to serialize/deserialize data of AT DMA
      firewire: ohci: use static inline functions to serialize data of AT DMA
      firewire: ohci: add static inline functions to serialize/deserialize data of IT DMA
      firewire: ohci: use static inline functions to serialize data of IT DMA
      firewire: core: use guard macro to maintain static packet data for phy configuration
      firewire: core: use guard macro to maintain the list of card
      firewire: core: use guard macro to maintain the list of cdev clients
      firewire: ohci: use guard macro to serialize accesses to phy registers
      firewire: core: use guard macro to maintain RCU scope for transaction address handler
      firewire: core: use guard macro to access to IDR for fw_device
      firewire: core: use guard macro to maintain the list of address handler for transaction
      firewire: core: use guard macro to disable local IRQ
      firewire: core: use guard macro to maintain list of events for userspace clients
      firewire: core: use guard macro to maintain IDR of isochronous resources for userspace clients
      firewire: core: use guard macro to maintain isochronous context for userspace client
      firewire: core: use guard macro to maintain list of receivers for phy configuration packets
      firewire: core: use guard macro to maintain list of asynchronous transaction
      firewire: core: use guard macro to maintain properties of fw_card
      firewire: ohci: use guard macro to maintain bus time
      firewire: ohci: use guard macro to maintain image of configuration ROM
      firewire: ohci: use guard macro to serialize operations for isochronous contexts
      firewire: core: correct range of block for case of switch statement
      firewire: core: replace IDR with XArray to maintain fw_device
      firewire: core: use lock in Xarray instead of local R/W semaphore
      firewire: core: minor code refactoring to release client resource
      firewire: core: add helper functions to convert to parent resource structure
      firewire: core: add helper function to detect data of iso resource structure
      firewire: core: code refactoring to use idr_for_each_entry() macro instead of idr_for_each() function
      firewire: core: use xarray instead of idr to maintain client resource
      firewire: ohci: use helper macro for compiler aligned attribute
      firewire: ohci: remove unused wrapper macro for dev_info()
      firewire: core/ohci: minor refactoring for computation of configuration ROM size
      firewire: ohci: fix error path to detect initiated reset in TI TSB41BA3D phy
      firewire: core: update fw_device outside of device_find_child()
      firewire: ohci: deprecate debug parameter
      firewire: ohci: obsolete direct usage of printk_ratelimit()
      firewire: core: allocate workqueue to handle isochronous contexts in card
      firewire: core: add local API to queue work item to workqueue specific to isochronous contexts
      firewire: ohci: operate IT/IR events in sleepable work process instead of tasklet softIRQ
      firewire: core: non-atomic memory allocation for isochronous event to user client
      ALSA: firewire: use nonatomic PCM operation
      firewire: core: use WARN_ON_ONCE() to avoid superfluous dumps
      firewire: core: expose kernel API to schedule work item to process isochronous context
      firewire: core: fulfill documentation of fw_iso_context_flush_completions()
      firewire: core: move workqueue handler from 1394 OHCI driver to core function
      firewire: core: use mutex to coordinate concurrent calls to flush completions
      Revert "firewire: core: use mutex to coordinate concurrent calls to flush completions"
      Revert "firewire: core: move workqueue handler from 1394 OHCI driver to core function"
      firewire: core: add helper function to retire descriptors
      firewire: core: update documentation of kernel APIs for flushing completions
      firewire: core: rename cause flag of tracepoints event

 Documentation/driver-api/firewire.rst    |   2 +
 drivers/firewire/core-card.c             |  91 ++++++++++++++++++-------------
 drivers/firewire/core-cdev.c             | 400 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---------------------------------------------------------------------
 drivers/firewire/core-device.c           | 202 +++++++++++++++++++++++++++++++-------------------------------------
 drivers/firewire/core-iso.c              |  49 +++++++++++++++--
 drivers/firewire/core-topology.c         |   7 +--
 drivers/firewire/core-transaction.c      | 151 +++++++++++++++++++++------------------------------
 drivers/firewire/core.h                  |  28 +++++++---
 drivers/firewire/ohci-serdes-test.c      |  66 +++++++++++++++++++++++
 drivers/firewire/ohci.c                  | 574 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-----------------------------------------------------------------------------------------------------
 drivers/firewire/ohci.h                  | 200 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 include/linux/firewire.h                 |  22 ++++++++
 include/trace/events/firewire.h          |   4 +-
 sound/firewire/amdtp-stream.c            |  34 +++++++++---
 sound/firewire/bebob/bebob_pcm.c         |   1 +
 sound/firewire/dice/dice-pcm.c           |   1 +
 sound/firewire/digi00x/digi00x-pcm.c     |   1 +
 sound/firewire/fireface/ff-pcm.c         |   1 +
 sound/firewire/fireworks/fireworks_pcm.c |   1 +
 sound/firewire/isight.c                  |   1 +
 sound/firewire/motu/motu-pcm.c           |   1 +
 sound/firewire/oxfw/oxfw-pcm.c           |   1 +
 sound/firewire/tascam/tascam-pcm.c       |   1 +
 23 files changed, 1065 insertions(+), 774 deletions(-)


Regards

Takashi Sakamoto

Re: [PATCH 0/5] firewire: core: revert changes to keep interface compatibility

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

On Thu, Sep 12, 2024 at 10:30:33PM +0900, Takashi Sakamoto wrote:
> Hi,
> 
> It appears that the changes in my previous series[1] bring the behaviour
> change of core function in the view of user space applications. It is
> not preferable at all without any preparation.
> 
> This series of changes includes two reverts commits as well as some
> minor code refactoring.
> 
> [1] https://lore.kernel.org/lkml/20240911151253.GA167609@workstation.local/
> 
> 
> Takashi Sakamoto (5):
>   Revert "firewire: core: use mutex to coordinate concurrent calls to
>     flush completions"
>   Revert "firewire: core: move workqueue handler from 1394 OHCI driver
>     to core function"
>   firewire: core: add helper function to retire descriptors
>   firewire: core: update documentation of kernel APIs for flushing
>     completions
>   firewire: core: rename cause flag of tracepoints event
> 
>  drivers/firewire/core-iso.c     | 40 +++++++++++----------
>  drivers/firewire/core.h         |  5 +++
>  drivers/firewire/ohci.c         | 61 +++++++++++++++++++++++----------
>  include/linux/firewire.h        |  9 ++---
>  include/trace/events/firewire.h |  4 +--
>  5 files changed, 75 insertions(+), 44 deletions(-)

Applied to for-next branch.


Regards

Takashi Sakamoto

[PATCH 5/5] firewire: core: rename cause flag of tracepoints event

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

The flag of FW_ISO_CONTEXT_COMPLETIONS_CAUSE_IRQ directly causes hardIRQ
request by 1394 OHCI hardware when the corresponding isochronous packet is
transferred, however it is not so directly associated to hardIRQ
processing itself.

This commit renames the flag so that it relates to interrupt parameter of
internal packet data.

Signed-off-by: Takashi Sakamoto <o-t...@sa...>
---
 drivers/firewire/ohci.c         | 6 +++---
 include/trace/events/firewire.h | 4 ++--
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c
index 4af4c9af4fe4..7ee55c2804de 100644
--- a/drivers/firewire/ohci.c
+++ b/drivers/firewire/ohci.c
@@ -2927,7 +2927,7 @@ static int handle_ir_packet_per_buffer(struct context *context,
 	copy_iso_headers(ctx, (u32 *) (last + 1));
 
 	if (last->control & cpu_to_le16(DESCRIPTOR_IRQ_ALWAYS))
-		flush_iso_completions(ctx, FW_ISO_CONTEXT_COMPLETIONS_CAUSE_IRQ);
+		flush_iso_completions(ctx, FW_ISO_CONTEXT_COMPLETIONS_CAUSE_INTERRUPT);
 
 	return 1;
 }
@@ -2963,7 +2963,7 @@ static int handle_ir_buffer_fill(struct context *context,
 
 	if (last->control & cpu_to_le16(DESCRIPTOR_IRQ_ALWAYS)) {
 		trace_isoc_inbound_multiple_completions(&ctx->base, completed,
-							FW_ISO_CONTEXT_COMPLETIONS_CAUSE_IRQ);
+							FW_ISO_CONTEXT_COMPLETIONS_CAUSE_INTERRUPT);
 
 		ctx->base.callback.mc(&ctx->base,
 				      buffer_dma + completed,
@@ -3059,7 +3059,7 @@ static int handle_it_packet(struct context *context,
 	ctx->header_length += 4;
 
 	if (last->control & cpu_to_le16(DESCRIPTOR_IRQ_ALWAYS))
-		flush_iso_completions(ctx, FW_ISO_CONTEXT_COMPLETIONS_CAUSE_IRQ);
+		flush_iso_completions(ctx, FW_ISO_CONTEXT_COMPLETIONS_CAUSE_INTERRUPT);
 
 	return 1;
 }
diff --git a/include/trace/events/firewire.h b/include/trace/events/firewire.h
index b108176deb22..ad0e0cf82b9c 100644
--- a/include/trace/events/firewire.h
+++ b/include/trace/events/firewire.h
@@ -830,13 +830,13 @@ TRACE_EVENT_CONDITION(isoc_inbound_multiple_queue,
 #ifndef show_cause
 enum fw_iso_context_completions_cause {
 	FW_ISO_CONTEXT_COMPLETIONS_CAUSE_FLUSH = 0,
-	FW_ISO_CONTEXT_COMPLETIONS_CAUSE_IRQ,
+	FW_ISO_CONTEXT_COMPLETIONS_CAUSE_INTERRUPT,
 	FW_ISO_CONTEXT_COMPLETIONS_CAUSE_HEADER_OVERFLOW,
 };
 #define show_cause(cause) 								\
 	__print_symbolic(cause,								\
 		{ FW_ISO_CONTEXT_COMPLETIONS_CAUSE_FLUSH, "FLUSH" },			\
-		{ FW_ISO_CONTEXT_COMPLETIONS_CAUSE_IRQ, "IRQ" },			\
+		{ FW_ISO_CONTEXT_COMPLETIONS_CAUSE_INTERRUPT, "INTERRUPT" },		\
 		{ FW_ISO_CONTEXT_COMPLETIONS_CAUSE_HEADER_OVERFLOW, "HEADER_OVERFLOW" }	\
 	)
 #endif
-- 
2.43.0

[PATCH 4/5] firewire: core: update documentation of kernel APIs for flushing completions

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

There is a slight difference between fw_iso_context_flush_completions() and
fw_iso_context_schedule_flush_completions().

This commit updates the documentations for them.

Signed-off-by: Takashi Sakamoto <o-t...@sa...>
---
 drivers/firewire/core-iso.c | 9 ++++++---
 include/linux/firewire.h    | 8 +++++---
 2 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/drivers/firewire/core-iso.c b/drivers/firewire/core-iso.c
index f2394f3ed194..a67493862c85 100644
--- a/drivers/firewire/core-iso.c
+++ b/drivers/firewire/core-iso.c
@@ -214,9 +214,12 @@ EXPORT_SYMBOL(fw_iso_context_queue_flush);
  * @ctx: the isochronous context
  *
  * Process the isochronous context in the current process context. The registered callback function
- * is called if some packets have been already transferred since the last time. If it is required
- * to process the context asynchronously, fw_iso_context_schedule_flush_completions() is available
- * instead.
+ * is called when a queued packet buffer with the interrupt flag is completed, either after
+ * transmission in the IT context or after being filled in the IR context. Additionally, the
+ * callback function is also called for the packet buffer completed at last. Furthermore, the
+ * callback function is called as well when the header buffer in the context becomes full. If it is
+ * required to process the context asynchronously, fw_iso_context_schedule_flush_completions() is
+ * available instead.
  *
  * Context: Process context. May sleep due to disable_work_sync().
  */
diff --git a/include/linux/firewire.h b/include/linux/firewire.h
index f815d12deda0..b632eec3ab52 100644
--- a/include/linux/firewire.h
+++ b/include/linux/firewire.h
@@ -537,9 +537,11 @@ int fw_iso_context_flush_completions(struct fw_iso_context *ctx);
  * @ctx: the isochronous context
  *
  * Schedule a work item on workqueue to process the isochronous context. The registered callback
- * function is called in the worker if some packets have been already transferred since the last
- * time. If it is required to process the context in the current context,
- * fw_iso_context_flush_completions() is available instead.
+ * function is called by the worker when a queued packet buffer with the interrupt flag is
+ * completed, either after transmission in the IT context or after being filled in the IR context.
+ * The callback function is also called when the header buffer in the context becomes full, If it
+ * is required to process the context in the current context, fw_iso_context_flush_completions() is
+ * available instead.
  *
  * Context: Any context.
  */
-- 
2.43.0

[PATCH 1/5] Revert "firewire: core: use mutex to coordinate concurrent calls to flush completions"

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

This reverts commit d9605d67562505e27dcc0f71af418118d3db91e5, since this
commit is on the following reverted changes.

Signed-off-by: Takashi Sakamoto <o-t...@sa...>
---
 drivers/firewire/core-iso.c | 11 ++---------
 drivers/firewire/ohci.c     | 37 +++++++++++++++++++++++--------------
 include/linux/firewire.h    |  1 -
 3 files changed, 25 insertions(+), 24 deletions(-)

diff --git a/drivers/firewire/core-iso.c b/drivers/firewire/core-iso.c
index 1405d2e9cb2c..9f41c78878ad 100644
--- a/drivers/firewire/core-iso.c
+++ b/drivers/firewire/core-iso.c
@@ -157,7 +157,6 @@ struct fw_iso_context *fw_iso_context_create(struct fw_card *card,
 	ctx->callback.sc = callback;
 	ctx->callback_data = callback_data;
 	INIT_WORK(&ctx->work, flush_completions_work);
-	mutex_init(&ctx->flushing_completions_mutex);
 
 	trace_isoc_outbound_allocate(ctx, channel, speed);
 	trace_isoc_inbound_single_allocate(ctx, channel, header_size);
@@ -174,8 +173,6 @@ void fw_iso_context_destroy(struct fw_iso_context *ctx)
 	trace_isoc_inbound_multiple_destroy(ctx);
 
 	ctx->card->driver->free_iso_context(ctx);
-
-	mutex_destroy(&ctx->flushing_completions_mutex);
 }
 EXPORT_SYMBOL(fw_iso_context_destroy);
 
@@ -229,7 +226,7 @@ EXPORT_SYMBOL(fw_iso_context_queue_flush);
  * to process the context asynchronously, fw_iso_context_schedule_flush_completions() is available
  * instead.
  *
- * Context: Process context due to mutex_trylock().
+ * Context: Process context.
  */
 int fw_iso_context_flush_completions(struct fw_iso_context *ctx)
 {
@@ -237,11 +234,7 @@ int fw_iso_context_flush_completions(struct fw_iso_context *ctx)
 	trace_isoc_inbound_single_flush_completions(ctx);
 	trace_isoc_inbound_multiple_flush_completions(ctx);
 
-	scoped_cond_guard(mutex_try, /* nothing to do */, &ctx->flushing_completions_mutex) {
-		return ctx->card->driver->flush_iso_completions(ctx);
-	}
-
-	return 0;
+	return ctx->card->driver->flush_iso_completions(ctx);
 }
 EXPORT_SYMBOL(fw_iso_context_flush_completions);
 
diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c
index b182998a77f4..02ff0363d3ad 100644
--- a/drivers/firewire/ohci.c
+++ b/drivers/firewire/ohci.c
@@ -166,6 +166,7 @@ struct iso_context {
 	struct context context;
 	void *header;
 	size_t header_length;
+	unsigned long flushing_completions;
 	u32 mc_buffer_bus;
 	u16 mc_completed;
 	u16 last_timestamp;
@@ -3578,23 +3579,31 @@ static void ohci_flush_queue_iso(struct fw_iso_context *base)
 static int ohci_flush_iso_completions(struct fw_iso_context *base)
 {
 	struct iso_context *ctx = container_of(base, struct iso_context, base);
+	int ret = 0;
 
-	// Note that tasklet softIRQ is not used to process isochronous context anymore.
-	context_tasklet((unsigned long)&ctx->context);
+	if (!test_and_set_bit_lock(0, &ctx->flushing_completions)) {
+		// Note that tasklet softIRQ is not used to process isochronous context anymore.
+		context_tasklet((unsigned long)&ctx->context);
 
-	switch (base->type) {
-	case FW_ISO_CONTEXT_TRANSMIT:
-	case FW_ISO_CONTEXT_RECEIVE:
-		if (ctx->header_length != 0)
-			flush_iso_completions(ctx, FW_ISO_CONTEXT_COMPLETIONS_CAUSE_FLUSH);
-		return 0;
-	case FW_ISO_CONTEXT_RECEIVE_MULTICHANNEL:
-		if (ctx->mc_completed != 0)
-			flush_ir_buffer_fill(ctx);
-		return 0;
-	default:
-		return -ENOSYS;
+		switch (base->type) {
+		case FW_ISO_CONTEXT_TRANSMIT:
+		case FW_ISO_CONTEXT_RECEIVE:
+			if (ctx->header_length != 0)
+				flush_iso_completions(ctx, FW_ISO_CONTEXT_COMPLETIONS_CAUSE_FLUSH);
+			break;
+		case FW_ISO_CONTEXT_RECEIVE_MULTICHANNEL:
+			if (ctx->mc_completed != 0)
+				flush_ir_buffer_fill(ctx);
+			break;
+		default:
+			ret = -ENOSYS;
+		}
+
+		clear_bit_unlock(0, &ctx->flushing_completions);
+		smp_mb__after_atomic();
 	}
+
+	return ret;
 }
 
 static const struct fw_card_driver ohci_driver = {
diff --git a/include/linux/firewire.h b/include/linux/firewire.h
index 19e8c5f9537c..f815d12deda0 100644
--- a/include/linux/firewire.h
+++ b/include/linux/firewire.h
@@ -512,7 +512,6 @@ union fw_iso_callback {
 struct fw_iso_context {
 	struct fw_card *card;
 	struct work_struct work;
-	struct mutex flushing_completions_mutex;
 	int type;
 	int channel;
 	int speed;
-- 
2.43.0

[PATCH 3/5] firewire: core: add helper function to retire descriptors

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

Both IR/IT contexts use the same code to retire completed descriptors
as AT context uses.

This commit adds a helper function to reduce the duplicated codes.

Signed-off-by: Takashi Sakamoto <o-t...@sa...>
---
 drivers/firewire/ohci.c | 45 +++++++++--------------------------------
 1 file changed, 9 insertions(+), 36 deletions(-)

diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c
index 3a911cfb5ff3..4af4c9af4fe4 100644
--- a/drivers/firewire/ohci.c
+++ b/drivers/firewire/ohci.c
@@ -1141,9 +1141,8 @@ static struct descriptor *find_branch_descriptor(struct descriptor *d, int z)
 		return d + z - 1;
 }
 
-static void context_tasklet(unsigned long data)
+static void context_retire_descriptors(struct context *ctx)
 {
-	struct context *ctx = (struct context *) data;
 	struct descriptor *d, *last;
 	u32 address;
 	int z;
@@ -1182,45 +1181,19 @@ static void context_tasklet(unsigned long data)
 	}
 }
 
+static void context_tasklet(unsigned long data)
+{
+	struct context *ctx = (struct context *) data;
+
+	context_retire_descriptors(ctx);
+}
+
 static void ohci_isoc_context_work(struct work_struct *work)
 {
 	struct fw_iso_context *base = container_of(work, struct fw_iso_context, work);
 	struct iso_context *isoc_ctx = container_of(base, struct iso_context, base);
-	struct context *ctx = &isoc_ctx->context;
-	struct descriptor *d, *last;
-	u32 address;
-	int z;
-	struct descriptor_buffer *desc;
-
-	desc = list_entry(ctx->buffer_list.next, struct descriptor_buffer, list);
-	last = ctx->last;
-	while (last->branch_address != 0) {
-		struct descriptor_buffer *old_desc = desc;
-
-		address = le32_to_cpu(last->branch_address);
-		z = address & 0xf;
-		address &= ~0xf;
-		ctx->current_bus = address;
-
-		// If the branch address points to a buffer outside of the current buffer, advance
-		// to the next buffer.
-		if (address < desc->buffer_bus || address >= desc->buffer_bus + desc->used)
-			desc = list_entry(desc->list.next, struct descriptor_buffer, list);
-		d = desc->buffer + (address - desc->buffer_bus) / sizeof(*d);
-		last = find_branch_descriptor(d, z);
-
-		if (!ctx->callback(ctx, d, last))
-			break;
 
-		if (old_desc != desc) {
-			// If we've advanced to the next buffer, move the previous buffer to the
-			// free list.
-			old_desc->used = 0;
-			guard(spinlock_irqsave)(&ctx->ohci->lock);
-			list_move_tail(&old_desc->list, &ctx->buffer_list);
-		}
-		ctx->last = last;
-	}
+	context_retire_descriptors(&isoc_ctx->context);
 }
 
 /*
-- 
2.43.0

[PATCH 0/5] firewire: core: revert changes to keep interface compatibility

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

Hi,

It appears that the changes in my previous series[1] bring the behaviour
change of core function in the view of user space applications. It is
not preferable at all without any preparation.

This series of changes includes two reverts commits as well as some
minor code refactoring.

[1] https://lore.kernel.org/lkml/20240911151253.GA167609@workstation.local/


Takashi Sakamoto (5):
  Revert "firewire: core: use mutex to coordinate concurrent calls to
    flush completions"
  Revert "firewire: core: move workqueue handler from 1394 OHCI driver
    to core function"
  firewire: core: add helper function to retire descriptors
  firewire: core: update documentation of kernel APIs for flushing
    completions
  firewire: core: rename cause flag of tracepoints event

 drivers/firewire/core-iso.c     | 40 +++++++++++----------
 drivers/firewire/core.h         |  5 +++
 drivers/firewire/ohci.c         | 61 +++++++++++++++++++++++----------
 include/linux/firewire.h        |  9 ++---
 include/trace/events/firewire.h |  4 +--
 5 files changed, 75 insertions(+), 44 deletions(-)

-- 
2.43.0

[PATCH 2/5] Revert "firewire: core: move workqueue handler from 1394 OHCI driver to core function"

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

This reverts commit 767bfb9ef27ebf760290d9f8bc303828b018c312. It appears
that the call of ohci_flush_iso_completions() in the work item scheduled
by hardIRQ of 1394 OHCI for any isochronous context changes the timing to
queue events in the view of user space application.

Signed-off-by: Takashi Sakamoto <o-t...@sa...>
---
 drivers/firewire/core-iso.c | 26 ++++++++++++---------
 drivers/firewire/core.h     |  5 +++++
 drivers/firewire/ohci.c     | 45 +++++++++++++++++++++++++++++++++++--
 3 files changed, 64 insertions(+), 12 deletions(-)

diff --git a/drivers/firewire/core-iso.c b/drivers/firewire/core-iso.c
index 9f41c78878ad..f2394f3ed194 100644
--- a/drivers/firewire/core-iso.c
+++ b/drivers/firewire/core-iso.c
@@ -131,13 +131,6 @@ size_t fw_iso_buffer_lookup(struct fw_iso_buffer *buffer, dma_addr_t completed)
 	return 0;
 }
 
-static void flush_completions_work(struct work_struct *work)
-{
-	struct fw_iso_context *ctx = container_of(work, struct fw_iso_context, work);
-
-	fw_iso_context_flush_completions(ctx);
-}
-
 struct fw_iso_context *fw_iso_context_create(struct fw_card *card,
 		int type, int channel, int speed, size_t header_size,
 		fw_iso_callback_t callback, void *callback_data)
@@ -156,7 +149,6 @@ struct fw_iso_context *fw_iso_context_create(struct fw_card *card,
 	ctx->header_size = header_size;
 	ctx->callback.sc = callback;
 	ctx->callback_data = callback_data;
-	INIT_WORK(&ctx->work, flush_completions_work);
 
 	trace_isoc_outbound_allocate(ctx, channel, speed);
 	trace_isoc_inbound_single_allocate(ctx, channel, header_size);
@@ -226,15 +218,29 @@ EXPORT_SYMBOL(fw_iso_context_queue_flush);
  * to process the context asynchronously, fw_iso_context_schedule_flush_completions() is available
  * instead.
  *
- * Context: Process context.
+ * Context: Process context. May sleep due to disable_work_sync().
  */
 int fw_iso_context_flush_completions(struct fw_iso_context *ctx)
 {
+	int err;
+
 	trace_isoc_outbound_flush_completions(ctx);
 	trace_isoc_inbound_single_flush_completions(ctx);
 	trace_isoc_inbound_multiple_flush_completions(ctx);
 
-	return ctx->card->driver->flush_iso_completions(ctx);
+	might_sleep();
+
+	// Avoid dead lock due to programming mistake.
+	if (WARN_ON_ONCE(current_work() == &ctx->work))
+		return 0;
+
+	disable_work_sync(&ctx->work);
+
+	err = ctx->card->driver->flush_iso_completions(ctx);
+
+	enable_work(&ctx->work);
+
+	return err;
 }
 EXPORT_SYMBOL(fw_iso_context_flush_completions);
 
diff --git a/drivers/firewire/core.h b/drivers/firewire/core.h
index 96ae366889e0..0ae2c84ecafe 100644
--- a/drivers/firewire/core.h
+++ b/drivers/firewire/core.h
@@ -159,6 +159,11 @@ int fw_iso_buffer_alloc(struct fw_iso_buffer *buffer, int page_count);
 int fw_iso_buffer_map_dma(struct fw_iso_buffer *buffer, struct fw_card *card,
 			  enum dma_data_direction direction);
 
+static inline void fw_iso_context_init_work(struct fw_iso_context *ctx, work_func_t func)
+{
+	INIT_WORK(&ctx->work, func);
+}
+
 
 /* -topology */
 
diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c
index 02ff0363d3ad..3a911cfb5ff3 100644
--- a/drivers/firewire/ohci.c
+++ b/drivers/firewire/ohci.c
@@ -1182,6 +1182,47 @@ static void context_tasklet(unsigned long data)
 	}
 }
 
+static void ohci_isoc_context_work(struct work_struct *work)
+{
+	struct fw_iso_context *base = container_of(work, struct fw_iso_context, work);
+	struct iso_context *isoc_ctx = container_of(base, struct iso_context, base);
+	struct context *ctx = &isoc_ctx->context;
+	struct descriptor *d, *last;
+	u32 address;
+	int z;
+	struct descriptor_buffer *desc;
+
+	desc = list_entry(ctx->buffer_list.next, struct descriptor_buffer, list);
+	last = ctx->last;
+	while (last->branch_address != 0) {
+		struct descriptor_buffer *old_desc = desc;
+
+		address = le32_to_cpu(last->branch_address);
+		z = address & 0xf;
+		address &= ~0xf;
+		ctx->current_bus = address;
+
+		// If the branch address points to a buffer outside of the current buffer, advance
+		// to the next buffer.
+		if (address < desc->buffer_bus || address >= desc->buffer_bus + desc->used)
+			desc = list_entry(desc->list.next, struct descriptor_buffer, list);
+		d = desc->buffer + (address - desc->buffer_bus) / sizeof(*d);
+		last = find_branch_descriptor(d, z);
+
+		if (!ctx->callback(ctx, d, last))
+			break;
+
+		if (old_desc != desc) {
+			// If we've advanced to the next buffer, move the previous buffer to the
+			// free list.
+			old_desc->used = 0;
+			guard(spinlock_irqsave)(&ctx->ohci->lock);
+			list_move_tail(&old_desc->list, &ctx->buffer_list);
+		}
+		ctx->last = last;
+	}
+}
+
 /*
  * Allocate a new buffer and add it to the list of free buffers for this
  * context.  Must be called with ohci->lock held.
@@ -3128,6 +3169,7 @@ static struct fw_iso_context *ohci_allocate_iso_context(struct fw_card *card,
 	ret = context_init(&ctx->context, ohci, regs, callback);
 	if (ret < 0)
 		goto out_with_header;
+	fw_iso_context_init_work(&ctx->base, ohci_isoc_context_work);
 
 	if (type == FW_ISO_CONTEXT_RECEIVE_MULTICHANNEL) {
 		set_multichannel_mask(ohci, 0);
@@ -3582,8 +3624,7 @@ static int ohci_flush_iso_completions(struct fw_iso_context *base)
 	int ret = 0;
 
 	if (!test_and_set_bit_lock(0, &ctx->flushing_completions)) {
-		// Note that tasklet softIRQ is not used to process isochronous context anymore.
-		context_tasklet((unsigned long)&ctx->context);
+		ohci_isoc_context_work(&base->work);
 
 		switch (base->type) {
 		case FW_ISO_CONTEXT_TRANSMIT:
-- 
2.43.0

Re: [PATCH 0/2] firewire: core: optimize for concurrent calls of fw_iso_context_flush_completions()

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

Hi,

On Mon, Sep 09, 2024 at 11:00:16PM +0900, Takashi Sakamoto wrote:
> Hi,
> 
> It seems to be the last week for v6.12 development. I realize it
> unpreferable to propose intrusive changes, however I also realized that
> there is a room to refactor core functions in respect to handler of work
> item for isochronous context for the next merge window...
> 
> This series of changes refactors the core function to call
> fw_iso_context_flush_completions() from the work item. It optimizes some
> event waiting and mediation of concurrent calls as well.
> 
> Takashi Sakamoto (2):
>   firewire: core: move workqueue handler from 1394 OHCI driver to core
>     function
>   firewire: core: use mutex to coordinate concurrent calls to flush
>     completions
> 
>  drivers/firewire/core-iso.c | 31 ++++++++-------
>  drivers/firewire/core.h     |  5 ---
>  drivers/firewire/ohci.c     | 78 +++++++------------------------------
>  include/linux/firewire.h    |  1 +
>  4 files changed, 31 insertions(+), 84 deletions(-)

I realized that the above changes have unpreferable effects to the behaviour
for user space interface. The changes allow to call the handler of
isochronous context again to drain the rest of packet buffer after calling
the handler at first due to processing the interrupt flag of 1394 OHCI IT/IR
descriptor. As a result, it is possible to enqueue two iso_interrupt events
for user space applications in the bottom half of hardIRQ. However, this is
against the description in UAPI header:

```
$ cat include/uapi/linux/firewire-cdev.h
...
 * struct fw_cdev_event_iso_interrupt - Sent when an iso packet was completed
...
 * This event is sent when the controller has completed an &fw_cdev_iso_packet
 * with the %FW_CDEV_ISO_INTERRUPT bit set, when explicitly requested with
 * %FW_CDEV_IOC_FLUSH_ISO, or when there have been so many completed packets
 * without the interrupt bit set that the kernel's internal buffer for @header
 * is about to overflow.  (In the last case, ABI versions < 5 drop header data
 * up to the next interrupt packet.)
```

As a bottom half of hardIRQ, the work item should enqueue a single event
associated to the interrupt event. The rest of packet buffer should be
handled in the bottom half of next hardIRQ unless in the path of
FW_CDEV_ISO_INTERRUPT.

Let me revert these changes later.


Regards

Takashi Sakamoto

Re: [PATCH 0/2] firewire: core: optimize for concurrent calls of fw_iso_context_flush_completions()

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

On Mon, Sep 09, 2024 at 11:00:16PM +0900, Takashi Sakamoto wrote:
> Hi,
> 
> It seems to be the last week for v6.12 development. I realize it
> unpreferable to propose intrusive changes, however I also realized that
> there is a room to refactor core functions in respect to handler of work
> item for isochronous context for the next merge window...
> 
> This series of changes refactors the core function to call
> fw_iso_context_flush_completions() from the work item. It optimizes some
> event waiting and mediation of concurrent calls as well.
> 
> Takashi Sakamoto (2):
>   firewire: core: move workqueue handler from 1394 OHCI driver to core
>     function
>   firewire: core: use mutex to coordinate concurrent calls to flush
>     completions
> 
>  drivers/firewire/core-iso.c | 31 ++++++++-------
>  drivers/firewire/core.h     |  5 ---
>  drivers/firewire/ohci.c     | 78 +++++++------------------------------
>  include/linux/firewire.h    |  1 +
>  4 files changed, 31 insertions(+), 84 deletions(-)

Applied to for-next branch.


Regards

Takashi Sakamoto

[PATCH 2/2] firewire: core: use mutex to coordinate concurrent calls to flush completions

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

In current implementation, test_and_set_bit_lock() is used to mediate
concurrent calls of ohci_flush_iso_completions(). However, the ad-hoc
usage of atomic operations is not preferable.

This commit uses mutex_trylock() as the similar operations. The core
function is responsible for the mediation, instead of 1394 OHCI driver.

Signed-off-by: Takashi Sakamoto <o-t...@sa...>
---
 drivers/firewire/core-iso.c | 11 +++++++++--
 drivers/firewire/ohci.c     | 37 ++++++++++++++-----------------------
 include/linux/firewire.h    |  1 +
 3 files changed, 24 insertions(+), 25 deletions(-)

diff --git a/drivers/firewire/core-iso.c b/drivers/firewire/core-iso.c
index 9f41c78878ad..a5c5ef3c725d 100644
--- a/drivers/firewire/core-iso.c
+++ b/drivers/firewire/core-iso.c
@@ -157,6 +157,7 @@ struct fw_iso_context *fw_iso_context_create(struct fw_card *card,
 	ctx->callback.sc = callback;
 	ctx->callback_data = callback_data;
 	INIT_WORK(&ctx->work, flush_completions_work);
+	mutex_init(&ctx->flushing_completions_mutex);
 
 	trace_isoc_outbound_allocate(ctx, channel, speed);
 	trace_isoc_inbound_single_allocate(ctx, channel, header_size);
@@ -173,6 +174,8 @@ void fw_iso_context_destroy(struct fw_iso_context *ctx)
 	trace_isoc_inbound_multiple_destroy(ctx);
 
 	ctx->card->driver->free_iso_context(ctx);
+
+	mutex_destroy(&ctx->flushing_completions_mutex);
 }
 EXPORT_SYMBOL(fw_iso_context_destroy);
 
@@ -226,7 +229,7 @@ EXPORT_SYMBOL(fw_iso_context_queue_flush);
  * to process the context asynchronously, fw_iso_context_schedule_flush_completions() is available
  * instead.
  *
- * Context: Process context.
+ * Context: Process context due to mutex_trylock().
  */
 int fw_iso_context_flush_completions(struct fw_iso_context *ctx)
 {
@@ -234,7 +237,11 @@ int fw_iso_context_flush_completions(struct fw_iso_context *ctx)
 	trace_isoc_inbound_single_flush_completions(ctx);
 	trace_isoc_inbound_multiple_flush_completions(ctx);
 
-	return ctx->card->driver->flush_iso_completions(ctx);
+	scoped_cond_guard(mutex_try, /* nothing to do */, &ctx->flushing_completions_mutex) {
+		return ctx->card->driver->flush_iso_completions(ctx);
+	}
+
+	return 0;
 }
 EXPORT_SYMBOL(fw_iso_context_flush_completions);
 
diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c
index 02ff0363d3ad..b182998a77f4 100644
--- a/drivers/firewire/ohci.c
+++ b/drivers/firewire/ohci.c
@@ -166,7 +166,6 @@ struct iso_context {
 	struct context context;
 	void *header;
 	size_t header_length;
-	unsigned long flushing_completions;
 	u32 mc_buffer_bus;
 	u16 mc_completed;
 	u16 last_timestamp;
@@ -3579,31 +3578,23 @@ static void ohci_flush_queue_iso(struct fw_iso_context *base)
 static int ohci_flush_iso_completions(struct fw_iso_context *base)
 {
 	struct iso_context *ctx = container_of(base, struct iso_context, base);
-	int ret = 0;
 
-	if (!test_and_set_bit_lock(0, &ctx->flushing_completions)) {
-		// Note that tasklet softIRQ is not used to process isochronous context anymore.
-		context_tasklet((unsigned long)&ctx->context);
+	// Note that tasklet softIRQ is not used to process isochronous context anymore.
+	context_tasklet((unsigned long)&ctx->context);
 
-		switch (base->type) {
-		case FW_ISO_CONTEXT_TRANSMIT:
-		case FW_ISO_CONTEXT_RECEIVE:
-			if (ctx->header_length != 0)
-				flush_iso_completions(ctx, FW_ISO_CONTEXT_COMPLETIONS_CAUSE_FLUSH);
-			break;
-		case FW_ISO_CONTEXT_RECEIVE_MULTICHANNEL:
-			if (ctx->mc_completed != 0)
-				flush_ir_buffer_fill(ctx);
-			break;
-		default:
-			ret = -ENOSYS;
-		}
-
-		clear_bit_unlock(0, &ctx->flushing_completions);
-		smp_mb__after_atomic();
+	switch (base->type) {
+	case FW_ISO_CONTEXT_TRANSMIT:
+	case FW_ISO_CONTEXT_RECEIVE:
+		if (ctx->header_length != 0)
+			flush_iso_completions(ctx, FW_ISO_CONTEXT_COMPLETIONS_CAUSE_FLUSH);
+		return 0;
+	case FW_ISO_CONTEXT_RECEIVE_MULTICHANNEL:
+		if (ctx->mc_completed != 0)
+			flush_ir_buffer_fill(ctx);
+		return 0;
+	default:
+		return -ENOSYS;
 	}
-
-	return ret;
 }
 
 static const struct fw_card_driver ohci_driver = {
diff --git a/include/linux/firewire.h b/include/linux/firewire.h
index f815d12deda0..19e8c5f9537c 100644
--- a/include/linux/firewire.h
+++ b/include/linux/firewire.h
@@ -512,6 +512,7 @@ union fw_iso_callback {
 struct fw_iso_context {
 	struct fw_card *card;
 	struct work_struct work;
+	struct mutex flushing_completions_mutex;
 	int type;
 	int channel;
 	int speed;
-- 
2.43.0

[PATCH 1/2] firewire: core: move workqueue handler from 1394 OHCI driver to core function

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

In current implementation, the work item for isochronous context executes
the same procedure of fw_iso_context_flush_completions() internally. There
is a space to refactor the implementation.

This commit calls fw_iso_context_flush_completions() in the work item. It
obsoletes fw_iso_context_init_work(). It also obsoletes a pair of
disable_work_sync() and enable_work() since the usage of
test_and_set_bit_lock() mediates concurrent call already.

Signed-off-by: Takashi Sakamoto <o-t...@sa...>
---
 drivers/firewire/core-iso.c | 26 +++++++++------------
 drivers/firewire/core.h     |  5 -----
 drivers/firewire/ohci.c     | 45 ++-----------------------------------
 3 files changed, 12 insertions(+), 64 deletions(-)

diff --git a/drivers/firewire/core-iso.c b/drivers/firewire/core-iso.c
index f2394f3ed194..9f41c78878ad 100644
--- a/drivers/firewire/core-iso.c
+++ b/drivers/firewire/core-iso.c
@@ -131,6 +131,13 @@ size_t fw_iso_buffer_lookup(struct fw_iso_buffer *buffer, dma_addr_t completed)
 	return 0;
 }
 
+static void flush_completions_work(struct work_struct *work)
+{
+	struct fw_iso_context *ctx = container_of(work, struct fw_iso_context, work);
+
+	fw_iso_context_flush_completions(ctx);
+}
+
 struct fw_iso_context *fw_iso_context_create(struct fw_card *card,
 		int type, int channel, int speed, size_t header_size,
 		fw_iso_callback_t callback, void *callback_data)
@@ -149,6 +156,7 @@ struct fw_iso_context *fw_iso_context_create(struct fw_card *card,
 	ctx->header_size = header_size;
 	ctx->callback.sc = callback;
 	ctx->callback_data = callback_data;
+	INIT_WORK(&ctx->work, flush_completions_work);
 
 	trace_isoc_outbound_allocate(ctx, channel, speed);
 	trace_isoc_inbound_single_allocate(ctx, channel, header_size);
@@ -218,29 +226,15 @@ EXPORT_SYMBOL(fw_iso_context_queue_flush);
  * to process the context asynchronously, fw_iso_context_schedule_flush_completions() is available
  * instead.
  *
- * Context: Process context. May sleep due to disable_work_sync().
+ * Context: Process context.
  */
 int fw_iso_context_flush_completions(struct fw_iso_context *ctx)
 {
-	int err;
-
 	trace_isoc_outbound_flush_completions(ctx);
 	trace_isoc_inbound_single_flush_completions(ctx);
 	trace_isoc_inbound_multiple_flush_completions(ctx);
 
-	might_sleep();
-
-	// Avoid dead lock due to programming mistake.
-	if (WARN_ON_ONCE(current_work() == &ctx->work))
-		return 0;
-
-	disable_work_sync(&ctx->work);
-
-	err = ctx->card->driver->flush_iso_completions(ctx);
-
-	enable_work(&ctx->work);
-
-	return err;
+	return ctx->card->driver->flush_iso_completions(ctx);
 }
 EXPORT_SYMBOL(fw_iso_context_flush_completions);
 
diff --git a/drivers/firewire/core.h b/drivers/firewire/core.h
index 0ae2c84ecafe..96ae366889e0 100644
--- a/drivers/firewire/core.h
+++ b/drivers/firewire/core.h
@@ -159,11 +159,6 @@ int fw_iso_buffer_alloc(struct fw_iso_buffer *buffer, int page_count);
 int fw_iso_buffer_map_dma(struct fw_iso_buffer *buffer, struct fw_card *card,
 			  enum dma_data_direction direction);
 
-static inline void fw_iso_context_init_work(struct fw_iso_context *ctx, work_func_t func)
-{
-	INIT_WORK(&ctx->work, func);
-}
-
 
 /* -topology */
 
diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c
index 3a911cfb5ff3..02ff0363d3ad 100644
--- a/drivers/firewire/ohci.c
+++ b/drivers/firewire/ohci.c
@@ -1182,47 +1182,6 @@ static void context_tasklet(unsigned long data)
 	}
 }
 
-static void ohci_isoc_context_work(struct work_struct *work)
-{
-	struct fw_iso_context *base = container_of(work, struct fw_iso_context, work);
-	struct iso_context *isoc_ctx = container_of(base, struct iso_context, base);
-	struct context *ctx = &isoc_ctx->context;
-	struct descriptor *d, *last;
-	u32 address;
-	int z;
-	struct descriptor_buffer *desc;
-
-	desc = list_entry(ctx->buffer_list.next, struct descriptor_buffer, list);
-	last = ctx->last;
-	while (last->branch_address != 0) {
-		struct descriptor_buffer *old_desc = desc;
-
-		address = le32_to_cpu(last->branch_address);
-		z = address & 0xf;
-		address &= ~0xf;
-		ctx->current_bus = address;
-
-		// If the branch address points to a buffer outside of the current buffer, advance
-		// to the next buffer.
-		if (address < desc->buffer_bus || address >= desc->buffer_bus + desc->used)
-			desc = list_entry(desc->list.next, struct descriptor_buffer, list);
-		d = desc->buffer + (address - desc->buffer_bus) / sizeof(*d);
-		last = find_branch_descriptor(d, z);
-
-		if (!ctx->callback(ctx, d, last))
-			break;
-
-		if (old_desc != desc) {
-			// If we've advanced to the next buffer, move the previous buffer to the
-			// free list.
-			old_desc->used = 0;
-			guard(spinlock_irqsave)(&ctx->ohci->lock);
-			list_move_tail(&old_desc->list, &ctx->buffer_list);
-		}
-		ctx->last = last;
-	}
-}
-
 /*
  * Allocate a new buffer and add it to the list of free buffers for this
  * context.  Must be called with ohci->lock held.
@@ -3169,7 +3128,6 @@ static struct fw_iso_context *ohci_allocate_iso_context(struct fw_card *card,
 	ret = context_init(&ctx->context, ohci, regs, callback);
 	if (ret < 0)
 		goto out_with_header;
-	fw_iso_context_init_work(&ctx->base, ohci_isoc_context_work);
 
 	if (type == FW_ISO_CONTEXT_RECEIVE_MULTICHANNEL) {
 		set_multichannel_mask(ohci, 0);
@@ -3624,7 +3582,8 @@ static int ohci_flush_iso_completions(struct fw_iso_context *base)
 	int ret = 0;
 
 	if (!test_and_set_bit_lock(0, &ctx->flushing_completions)) {
-		ohci_isoc_context_work(&base->work);
+		// Note that tasklet softIRQ is not used to process isochronous context anymore.
+		context_tasklet((unsigned long)&ctx->context);
 
 		switch (base->type) {
 		case FW_ISO_CONTEXT_TRANSMIT:
-- 
2.43.0

[PATCH 0/2] firewire: core: optimize for concurrent calls of fw_iso_context_flush_completions()

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

Hi,

It seems to be the last week for v6.12 development. I realize it
unpreferable to propose intrusive changes, however I also realized that
there is a room to refactor core functions in respect to handler of work
item for isochronous context for the next merge window...

This series of changes refactors the core function to call
fw_iso_context_flush_completions() from the work item. It optimizes some
event waiting and mediation of concurrent calls as well.

Takashi Sakamoto (2):
  firewire: core: move workqueue handler from 1394 OHCI driver to core
    function
  firewire: core: use mutex to coordinate concurrent calls to flush
    completions

 drivers/firewire/core-iso.c | 31 ++++++++-------
 drivers/firewire/core.h     |  5 ---
 drivers/firewire/ohci.c     | 78 +++++++------------------------------
 include/linux/firewire.h    |  1 +
 4 files changed, 31 insertions(+), 84 deletions(-)

-- 
2.43.0

Re: [PATCH 0/2] firewire: core: allow unit drivers to schedule work item to process isochronous context

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

On Sun, Sep 08, 2024 at 01:05:47PM +0900, Takashi Sakamoto wrote:
> Hi,
> 
> ALSA firewire stack uses packet-per-buffer mode for both isochronous
> receive (IR) and transmit (IT) contexts of 1394 OHCI. To process time
> stamp per packet (or per sample in some cases) steadily for media clock
> recovery against unexpected transmission skips in the IR context, it uses
> an IT context to operate all of isochronous contexts by calls of
> fw_iso_context_flush_completions() in the bottom-half of hardIRQ for the
> IT context.
> 
> Although it looks well to handle all of isochronous contexts in a single
> bottom-half context, it relatively takes longer time to finish. In the
> future code integration (not yet), it is possible to apply parallelism
> method to process these context. In the case, it is useful to allow unit
> drivers to schedule work items to process these isochronous contexts.
> 
> As a preparation, in this series of changes,
> fw_iso_context_schedule_flush_completions() is exposed as a kernel API
> available by unit drivers. It is a counter part of
> fw_iso_context_flush_completions(). This series of changes also includes
> documentation about these kernel APIs.
> 
> Takashi Sakamoto (2):
>   firewire: core: expose kernel API to schedule work item to process
>     isochronous context
>   firewire: core: fulfill documentation of
>     fw_iso_context_flush_completions()
> 
>  Documentation/driver-api/firewire.rst |  2 ++
>  drivers/firewire/core-iso.c           | 11 +++++++++++
>  drivers/firewire/core.h               |  5 -----
>  drivers/firewire/ohci.c               |  4 ++--
>  include/linux/firewire.h              | 17 +++++++++++++++++
>  5 files changed, 32 insertions(+), 7 deletions(-)

Applied to for-next branch.


Regards

Takashi Sakamoto

Re: Bug report: Mac Target Disk Mode not working in some cases

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

On Mon, Sep 02, 2024 at 05:07:40PM +0100, Andrew Ferguson wrote:
> Adam, would you be willing to send me the serial number of the PowerBook G4
> you used? That lets me see all of the specs and whether or not it is
> similar to mine. Separately (and very much a n00b question!), what's the
> best / easiest way to get kernel version 6.8-rc1? is there a specific
> distro that ships with development / rc builds of the kernel, or is it
> better to install something like Debian Stable and then compile / install a
> development build? Ideally I'd replicate your setup to confirm it isn't an
> issue on my end.

Hi Andrew,

Sorry for my late reply.

My PowerBook G4's serial number is QT23727VLP4.

This is my recipe for building kernels from kernel.org tarballs on 
Debian Bookworm:

sudo apt-get install build-essential
sudo apt-get build-dep linux
tar xzf linux-6.11-rc1.tar.gz
cd linux-source-6.11-rc1
cp /boot/config-6.1.0-15-amd64 .config
scripts/config --enable DEBUG_INFO_NONE
scripts/config --disable DEBUG_INFO_DWARF_TOOLCHAIN_DEFAULT
scripts/config --disable CONFIG_DEBUG_INFO_BTF
scripts/config --disable SECURITY_LOCKDOWN_LSM
scripts/config --disable MODULE_SIG
make olddefconfig
make clean
make bindeb-pkg

Note that the upstream kernel source contains a build script for Debian 
packages.

However, Debian has their own, different procedure for building 
development kernels:

https://kernel-team.pages.debian.net/kernel-handbook/ch-common-tasks.html#s-common-official-vcs

I've also tested with 6.11-rc1 now. Again, the Linux box was able to 
access the PowerBook's drive over Firewire. I was able to mount sdb10 as 
an hfsplus filesystem and see the contents.

-- Adam

[PATCH 2/2] firewire: core: fulfill documentation of fw_iso_context_flush_completions()

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

The fw_iso_context_flush_completions() is the counterpart of
fw_iso_context_schedule_work() to process isochronous context in current
process context.

This commit fulfills its documentation.

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

diff --git a/drivers/firewire/core-iso.c b/drivers/firewire/core-iso.c
index a249974a0f87..f2394f3ed194 100644
--- a/drivers/firewire/core-iso.c
+++ b/drivers/firewire/core-iso.c
@@ -209,6 +209,17 @@ void fw_iso_context_queue_flush(struct fw_iso_context *ctx)
 }
 EXPORT_SYMBOL(fw_iso_context_queue_flush);
 
+/**
+ * fw_iso_context_flush_completions() - process isochronous context in current process context.
+ * @ctx: the isochronous context
+ *
+ * Process the isochronous context in the current process context. The registered callback function
+ * is called if some packets have been already transferred since the last time. If it is required
+ * to process the context asynchronously, fw_iso_context_schedule_flush_completions() is available
+ * instead.
+ *
+ * Context: Process context. May sleep due to disable_work_sync().
+ */
 int fw_iso_context_flush_completions(struct fw_iso_context *ctx)
 {
 	int err;
-- 
2.43.0

[PATCH 1/2] firewire: core: expose kernel API to schedule work item to process isochronous context

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

In packet-per-buffer mode for isochronous context of 1394 OHCI, software
can schedule hardIRQ to the buffer in which the content of isochronous
packet is processed. The actual behaviour is different between isochronous
receive (IR) and transmit (IT) contexts in respect to isochronous cycle in
which the hardIRQ occurs.

In IR context, the hardIRQ occurs when the buffer is filled actually by
the content of received packet. If there are any isochronous cycles in
which the packet transmission is skipped, it is postponed to generate
the hardIRQ in respect to the isochronous cycle. In IT context, software
can schedule the content of packet every isochronous cycle including
skipping, therefore the hardIRQ occurs in the isochronous cycle to which
the software scheduled.

ALSA firewire stack uses the packet-per-buffer mode for both IR/IT
contexts. To process time stamp per packet (or per sample in some cases)
steadily for media clock recovery against unexpected transmission skips,
it uses an IT context to operate all of isochronous contexts by calls of
fw_iso_context_flush_completions() in the bottom-half of hardIRQ for the
IT context.

Although it looks well to handle all of isochronous contexts in a single
bottom-half context, it relatively takes longer time. In the future code
integration (not yet), it is possible to apply parallelism method to
process these context. In the case, it is useful to allow unit drivers to
schedule work items to process these isochronous contexts.

As a preparation, this commit exposes
fw_iso_context_schedule_flush_completions() as a kernel API available by
unit drivers. It is renamed from fw_iso_context_queue_work() since it is
a counter part of fw_iso_context_flush_completions().

Signed-off-by: Takashi Sakamoto <o-t...@sa...>
---
 Documentation/driver-api/firewire.rst |  2 ++
 drivers/firewire/core.h               |  5 -----
 drivers/firewire/ohci.c               |  4 ++--
 include/linux/firewire.h              | 17 +++++++++++++++++
 4 files changed, 21 insertions(+), 7 deletions(-)

diff --git a/Documentation/driver-api/firewire.rst b/Documentation/driver-api/firewire.rst
index d3cfa73cbb2b..28a32410f7d2 100644
--- a/Documentation/driver-api/firewire.rst
+++ b/Documentation/driver-api/firewire.rst
@@ -43,6 +43,8 @@ Firewire core transaction interfaces
 Firewire Isochronous I/O interfaces
 ===================================
 
+.. kernel-doc:: include/linux/firewire.h
+   :functions: fw_iso_context_schedule_flush_completions
 .. kernel-doc:: drivers/firewire/core-iso.c
    :export:
 
diff --git a/drivers/firewire/core.h b/drivers/firewire/core.h
index 2874f316156a..0ae2c84ecafe 100644
--- a/drivers/firewire/core.h
+++ b/drivers/firewire/core.h
@@ -164,11 +164,6 @@ static inline void fw_iso_context_init_work(struct fw_iso_context *ctx, work_fun
 	INIT_WORK(&ctx->work, func);
 }
 
-static inline void fw_iso_context_queue_work(struct fw_iso_context *ctx)
-{
-	queue_work(ctx->card->isoc_wq, &ctx->work);
-}
-
 
 /* -topology */
 
diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c
index d0b1fccc450f..3a911cfb5ff3 100644
--- a/drivers/firewire/ohci.c
+++ b/drivers/firewire/ohci.c
@@ -2283,7 +2283,7 @@ static irqreturn_t irq_handler(int irq, void *data)
 
 		while (iso_event) {
 			i = ffs(iso_event) - 1;
-			fw_iso_context_queue_work(&ohci->ir_context_list[i].base);
+			fw_iso_context_schedule_flush_completions(&ohci->ir_context_list[i].base);
 			iso_event &= ~(1 << i);
 		}
 	}
@@ -2294,7 +2294,7 @@ static irqreturn_t irq_handler(int irq, void *data)
 
 		while (iso_event) {
 			i = ffs(iso_event) - 1;
-			fw_iso_context_queue_work(&ohci->it_context_list[i].base);
+			fw_iso_context_schedule_flush_completions(&ohci->it_context_list[i].base);
 			iso_event &= ~(1 << i);
 		}
 	}
diff --git a/include/linux/firewire.h b/include/linux/firewire.h
index 72f497b61739..f815d12deda0 100644
--- a/include/linux/firewire.h
+++ b/include/linux/firewire.h
@@ -531,6 +531,23 @@ int fw_iso_context_queue(struct fw_iso_context *ctx,
 			 unsigned long payload);
 void fw_iso_context_queue_flush(struct fw_iso_context *ctx);
 int fw_iso_context_flush_completions(struct fw_iso_context *ctx);
+
+/**
+ * fw_iso_context_schedule_flush_completions() - schedule work item to process isochronous context.
+ * @ctx: the isochronous context
+ *
+ * Schedule a work item on workqueue to process the isochronous context. The registered callback
+ * function is called in the worker if some packets have been already transferred since the last
+ * time. If it is required to process the context in the current context,
+ * fw_iso_context_flush_completions() is available instead.
+ *
+ * Context: Any context.
+ */
+static inline void fw_iso_context_schedule_flush_completions(struct fw_iso_context *ctx)
+{
+	queue_work(ctx->card->isoc_wq, &ctx->work);
+}
+
 int fw_iso_context_start(struct fw_iso_context *ctx,
 			 int cycle, int sync, int tags);
 int fw_iso_context_stop(struct fw_iso_context *ctx);
-- 
2.43.0

[PATCH 0/2] firewire: core: allow unit drivers to schedule work item to process isochronous context

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

Hi,

ALSA firewire stack uses packet-per-buffer mode for both isochronous
receive (IR) and transmit (IT) contexts of 1394 OHCI. To process time
stamp per packet (or per sample in some cases) steadily for media clock
recovery against unexpected transmission skips in the IR context, it uses
an IT context to operate all of isochronous contexts by calls of
fw_iso_context_flush_completions() in the bottom-half of hardIRQ for the
IT context.

Although it looks well to handle all of isochronous contexts in a single
bottom-half context, it relatively takes longer time to finish. In the
future code integration (not yet), it is possible to apply parallelism
method to process these context. In the case, it is useful to allow unit
drivers to schedule work items to process these isochronous contexts.

As a preparation, in this series of changes,
fw_iso_context_schedule_flush_completions() is exposed as a kernel API
available by unit drivers. It is a counter part of
fw_iso_context_flush_completions(). This series of changes also includes
documentation about these kernel APIs.

Takashi Sakamoto (2):
  firewire: core: expose kernel API to schedule work item to process
    isochronous context
  firewire: core: fulfill documentation of
    fw_iso_context_flush_completions()

 Documentation/driver-api/firewire.rst |  2 ++
 drivers/firewire/core-iso.c           | 11 +++++++++++
 drivers/firewire/core.h               |  5 -----
 drivers/firewire/ohci.c               |  4 ++--
 include/linux/firewire.h              | 17 +++++++++++++++++
 5 files changed, 32 insertions(+), 7 deletions(-)

-- 
2.43.0

Re: [PATCH] firewire: core: use WARN_ON_ONCE() to avoid superfluous dumps

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

On Thu, Sep 05, 2024 at 10:10:29PM +0900, Takashi Sakamoto wrote:
> It is enough to notify programming mistakes to programmers just once.
> 
> Suggested-by: Takashi Iwai <ti...@su...>
> Signed-off-by: Takashi Sakamoto <o-t...@sa...>
> ---
>  drivers/firewire/core-iso.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/firewire/core-iso.c b/drivers/firewire/core-iso.c
> index af76fa1823f1..a249974a0f87 100644
> --- a/drivers/firewire/core-iso.c
> +++ b/drivers/firewire/core-iso.c
> @@ -220,7 +220,7 @@ int fw_iso_context_flush_completions(struct fw_iso_context *ctx)
>  	might_sleep();
>  
>  	// Avoid dead lock due to programming mistake.
> -	if (WARN_ON(current_work() == &ctx->work))
> +	if (WARN_ON_ONCE(current_work() == &ctx->work))
>  		return 0;
>  
>  	disable_work_sync(&ctx->work);
> @@ -244,7 +244,7 @@ int fw_iso_context_stop(struct fw_iso_context *ctx)
>  	might_sleep();
>  
>  	// Avoid dead lock due to programming mistake.
> -	if (WARN_ON(current_work() == &ctx->work))
> +	if (WARN_ON_ONCE(current_work() == &ctx->work))
>  		return 0;
>  
>  	err = ctx->card->driver->stop_iso(ctx);

Applied to for-next branch.


Regards

Takashi Sakamoto

Re: [PATCH 0/5] firewire: use sleepable workqueue to handle 1394 OHCI IT/IR context events

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

On Wed, Sep 04, 2024 at 09:58:19PM +0900, Takashi Sakamoto wrote:
> Hi,
> 
> (resend to linux1394-devel since the original post has so many
> receivers. Please refer to it for any reply:
> https://lore.kernel.org/lkml/202...@sa.../)
> 
> This series of changes updates my previous RFT[1] to apply for v6.12
> kernel. For the detail, please refer to the previous one.
> 
> To Iwai-san, this series includes the change for sound subsystem as
> well. All of changes are specific to ALSA firewire stack, so I would
> like to send it to Linus as the part of firewire subsystem updates if
> you do not mind it.
> 
> Changes from the RFT:
> * WQ_FREEZABLE is newly supported in the workqueue
> * Improve code comment in IEC 61883-1/6 packet streaming engine
> * Avoid dead lock in the calls of workqueue sync API
> 
> [1] https://lore.kernel.org/lkml/202...@sa.../
> 
> 
> Regards
> 
> Takashi Sakamoto (5):
>   firewire: core: allocate workqueue to handle isochronous contexts in
>     card
>   firewire: core: add local API to queue work item to workqueue specific
>     to isochronous contexts
>   firewire: ohci: operate IT/IR events in sleepable work process instead
>     of tasklet softIRQ
>   firewire: core: non-atomic memory allocation for isochronous event to
>     user client
>   ALSA: firewire: use nonatomic PCM operation
> 
>  drivers/firewire/core-card.c             | 33 ++++++++++++--
>  drivers/firewire/core-cdev.c             |  4 +-
>  drivers/firewire/core-iso.c              | 30 ++++++++++++-
>  drivers/firewire/core.h                  | 14 +++++-
>  drivers/firewire/ohci.c                  | 57 +++++++++++++++++++-----
>  include/linux/firewire.h                 |  3 ++
>  sound/firewire/amdtp-stream.c            | 34 +++++++++++---
>  sound/firewire/bebob/bebob_pcm.c         |  1 +
>  sound/firewire/dice/dice-pcm.c           |  1 +
>  sound/firewire/digi00x/digi00x-pcm.c     |  1 +
>  sound/firewire/fireface/ff-pcm.c         |  1 +
>  sound/firewire/fireworks/fireworks_pcm.c |  1 +
>  sound/firewire/isight.c                  |  1 +
>  sound/firewire/motu/motu-pcm.c           |  1 +
>  sound/firewire/oxfw/oxfw-pcm.c           |  1 +
>  sound/firewire/tascam/tascam-pcm.c       |  1 +
>  16 files changed, 157 insertions(+), 27 deletions(-)

I applied all of them to for-next branch.


Regards

Takashi Sakamoto