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.
Link: https://lore.kernel.org/r/20240912133038.238786-6-o-takashi@sakamocchi.jp
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
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.
Link: https://lore.kernel.org/r/20240912133038.238786-4-o-takashi@sakamocchi.jp
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
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.
Link: https://lore.kernel.org/r/20240912133038.238786-3-o-takashi@sakamocchi.jp
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
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.
Link: https://lore.kernel.org/r/20240909140018.65289-3-o-takashi@sakamocchi.jp
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
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.
Link: https://lore.kernel.org/r/20240909140018.65289-2-o-takashi@sakamocchi.jp
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
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().
Link: https://lore.kernel.org/r/20240908040549.75304-2-o-takashi@sakamocchi.jp
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
This commit queues work item for IT/IR events at hardIRQ handler to operate
the corresponding isochronous context. The work item is queued to any of
worker-pools.
The callback for either the implementation of unit protocol and user space
clients is executed in sleepable work process context. The change could
results in any errors of concurrent processing as well as sleep at atomic
context. These errors are fixed by the following commits.
Tested-by: Edmund Raile <edmund.raile@protonmail.com>
Link: https://lore.kernel.org/r/20240904125155.461886-4-o-takashi@sakamocchi.jp
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
This commit adds a workqueue dedicated for isochronous context processing.
The workqueue is allocated per instance of fw_card structure to satisfy the
following characteristics descending from 1394 OHCI specification:
In 1394 OHCI specification, memory pages are reserved to each isochronous
context dedicated to DMA transmission. It allows to operate these
per-context pages concurrently. Software can schedule hardware interrupt
for several isochronous context to the same cycle, thus WQ_UNBOUND is
specified. Additionally, it is sleepable to operate the content of pages,
thus WQ_BH is not used.
The isochronous context delivers the packets with time stamp, thus
WQ_HIGHPRI is specified for semi real-time data such as IEC 61883-1/6
protocol implemented by ALSA firewire stack. The isochronous context is not
used by the implementation of SCSI over IEEE1394 protocol (sbp2), thus
WQ_MEM_RECLAIM is not specified.
It is useful for users to adjust cpu affinity of the workqueue depending
on their work loads, thus WQ_SYS is specified to expose the attributes to
user space.
Tested-by: Edmund Raile <edmund.raile@protonmail.com>
Link: https://lore.kernel.org/r/20240904125155.461886-2-o-takashi@sakamocchi.jp
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
Many tracepoints events have been added to 6.10 and 6.11 kernels. They are
available as an alternative of debug parameter in firewire-ohci module.
The logging messages enabled by the parameter require some cumbersomes in
a point of maintenance; e.g. the code to decode transaction frame.
This commit adds deprecation text to conduct users to them..
Link: https://lore.kernel.org/r/20240903101455.317067-1-o-takashi@sakamocchi.jp
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
A commit 404957c1e207 ("firewire: ohci: use guard macro to serialize
accesses to phy registers") refactored initiated_reset() helper function,
while the error path was changed wrongly.
This commit fixes the bug.
Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
Fixes: 80f3401dfeb2 ("firewire: ohci: use guard macro to serialize accesses to phy registers")
Link: https://lore.kernel.org/r/20240817091128.180303-1-o-takashi@sakamocchi.jp
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
The size of space for configuration ROM is defined by IEEE 1212. The start
and end offsets are available as some macros in UAPI header.
This commit uses these macros to compute the size.
Link: https://lore.kernel.org/r/20240814131222.69949-4-o-takashi@sakamocchi.jp
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
The 1394 OHCI driver maintains bus time to respond to querying request.
The concurrent access to the bus time is protected by spinlock.
This commit uses guard macro to maintain the spinlock.
Link: https://lore.kernel.org/r/20240805085408.251763-16-o-takashi@sakamocchi.jp
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
The 1394 OHCI driver protects concurrent accesses to phy registers by
mutex object in fw_ohci structure.
This commit uses guard macro to maintain the mutex.
Link: https://lore.kernel.org/r/20240805085408.251763-5-o-takashi@sakamocchi.jp
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
The string table for tcode is just used by log_ar_at_event(). In the case,
it is suitable to move the table inner the function definition.
This commit is for the purpose. Additionally, the hard-coded value for
tcode is replaced with defined macros as many as possible.
Link: https://lore.kernel.org/r/20240729134631.127189-3-o-takashi@sakamocchi.jp
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
In IEEE 1394 specification, 0x0e in tcode field is reserved for internal
purpose depending on link layer. In 1394 OHCI specification, it is used to
express phy packet in AT/AR contexts.
Current implementation of 1394 OHCI driver has several macros for the code.
They can be simply replaced with a macro in core code.
This commit obsoletes the macros.
Link: https://lore.kernel.org/r/20240729134631.127189-2-o-takashi@sakamocchi.jp
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
In 1394 OHCI, the SelfIDComplete event occurs when the hardware has
finished transmitting all of the self ID packets received during the bus
initialization process to the host memory by DMA.
This commit adds a tracepoints event for this event to trace the timing
and packet data of Self-ID DMA. It is the part of following tracepoints
events helpful to debug some events at bus reset; e.g. the issue addressed
at a commit d0b06dc48f ("firewire: core: use long bus reset on gap count
error")[1]:
* firewire_ohci:irqs
* firewire_ohci:self_id_complete
* firewire:bus_reset_handle
* firewire:self_id_sequence
They would be also helpful in the problem about invocation timing of
hardIRQ and process (workqueue) contexts. We can often see this kind of
problem with -rt kernel[2].
[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=d0b06dc48fb1
[2] https://lore.kernel.org/linux-rt-users/YAwPoaUZ1gTD5y+k@hmbx/
Link: https://lore.kernel.org/r/20240702222034.1378764-6-o-takashi@sakamocchi.jp
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
The code of 1394 OHCI driver includes hard-coded magic number to operate
data of Self-ID DMA.
This commit replaces them with the inline functions added/tested in the
former commit.
Link: https://lore.kernel.org/r/20240702222034.1378764-5-o-takashi@sakamocchi.jp
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
It is preferable to use static function instead of functional macro in
some points. It checks type of argument, but would be optimized to
embedded code instead of function calls.
This commit obsoletes the functional macro with the static function.
Additionally this commit refactors quirk detection to ease the later work.
Link: https://lore.kernel.org/r/20240702222034.1378764-3-o-takashi@sakamocchi.jp
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
The 1394 OHCI driver configures the hardware to transfer the data quadlets
of packet via DMA after converting it to little endian, therefore the data
is typed as __le32. Nevertheless some actual hardware ignores the
configuration. In the case, the data in DMA buffer is aligned to big endian
(__be32).
For the case in big-endian machine, the driver includes the following
interpretation from __le32 to u32 (host-endian = __be32):
* (__force __u32)(v)
In include/linux/byteorder/generic.h, be32_to_cpu() is available. It is
expanded to the following expression in
'include/uapi/linux/byteorder/big_endian.h':
* (__force __u32)(__be32)(x)
This commit replace the ad-hoc endian interpretation with the above.
Link: https://lore.kernel.org/r/20240702222034.1378764-2-o-takashi@sakamocchi.jp
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
1394 OHCI hardware triggers PCI interrupts to notify any events to
software. Current driver for the hardware is programmed by the typical
way to utilize top- and bottom- halves, thus it has a timing gap to handle
the notification in softIRQ (tasklet).
This commit adds a tracepoint event for the hardIRQ event. The comparison
of the tracepoint event to tracepoints events in firewire subsystem is
helpful to diagnose the timing gap.
Link: https://lore.kernel.org/r/20240625031806.956650-3-o-takashi@sakamocchi.jp
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
The Linux Kernel Tracepoints framework is enough useful to trace the
interaction between 1394 OHCI hardware and its driver.
This commit adds firewire_ohci subsystem to use the framework. It is
defined as the different subsystem from the existing firewire subsystem.
The definition file for the existing subsystem is slightly changed so that
both subsystems are available in 1394 OHCI driver.
Link: https://lore.kernel.org/r/20240625031806.956650-2-o-takashi@sakamocchi.jp
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
It is helpful to trace completion of packets in isochronous context when
the core function is requested them by both in-kernel units driver and
userspace applications.
This commit adds some tracepoints events for the aim.
Link: https://lore.kernel.org/r/20240623220859.851685-8-o-takashi@sakamocchi.jp
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
It is helpful to trace the flushing completions of isochronous context when
the core function is requested them by both in-kernel unit drivers and
userspace applications.
This commit adds some tracepoints events for the aim.
Link: https://lore.kernel.org/r/20240623220859.851685-6-o-takashi@sakamocchi.jp
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
Current implementation to log self ID sequence has the rest to be
refactored; e.g. moving translation-unit level variables to the
dependent block.
This commit is for the purpose.
Link: https://lore.kernel.org/r/20240605235155.116468-5-o-takashi@sakamocchi.jp
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
The OHCI_PARAM_DEBUG_BUSRESETS bit of debug module parameter was added at
a commit a007bb857e ("firewire: fw-ohci: conditionally log busReset
interrupts").
At the former commit, the bit becomes less meaningful, just to skip
logging.
This commit obsoletes it.
Link: https://lore.kernel.org/r/20240501073238.72769-3-o-takashi@sakamocchi.jp
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
In the former commits, the spurious interrupt events are suppressed as
possible, by unset bus-reset event from the set of handled irq. The change
was written with the less-intrusive style, thus it firstly works at the
second time to handle the event. But it is slightly inconvenient.
This commit adds the event for the initial set of irq to handle. As a
result, the event can be handled even if it is the first time. The change
has a benefit that the OHCI_PARAM_DEBUG_BUSRESETS bit in debug module
parameter is always effective.
Tested-by: Adam Goldman <adamg@pobox.com>
Link: https://lore.kernel.org/r/20240501073238.72769-2-o-takashi@sakamocchi.jp
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
In the helper function for logging in 1394 ohci driver includes the
hard-coded variables for transaction code. They can be replaced with
the enumerations in UAPI header.
Link: https://lore.kernel.org/r/20240428071347.409202-6-o-takashi@sakamocchi.jp
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
The usage of the pair of pci_enable_msi() and pci_disable_msi() is
deprecated.
This commit uses the preferred pair of API for the purpose. The call of
pci_alloc_irq_vectors() can have a subeffect to change the return value
of pci_dev_msi_enabled().
Link: https://lore.kernel.org/r/20240331135037.191479-4-o-takashi@sakamocchi.jp
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
Nowadays request_irq() is a wrapper of request_threaded_irq(). The IRQ
handler of 1394 ohci driver has never been optimized yet, while it is
a good preparation for the future work to replace the latter.
This commit replaces the former.
Link: https://lore.kernel.org/r/20240331135037.191479-3-o-takashi@sakamocchi.jp
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
1394 OHCI driver generates packet data for the response subaction to the
request subaction to some local registers. In the case, the driver should
assign timestamp to them by itself.
This commit fulfills the timestamp for the subaction.
Cc: stable@vger.kernel.org
Fixes: dcadfd7f7c ("firewire: core: use union for callback of transaction completion")
Link: https://lore.kernel.org/r/20240429084709.707473-1-o-takashi@sakamocchi.jp
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
In the FireWire OHCI interrupt handler, if a bus reset interrupt has
occurred, mask bus reset interrupts until bus_reset_work has serviced and
cleared the interrupt.
Normally, we always leave bus reset interrupts masked. We infer the bus
reset from the self-ID interrupt that happens shortly thereafter. A
scenario where we unmask bus reset interrupts was introduced in 2008 in
a007bb857e0b26f5d8b73c2ff90782d9c0972620: If
OHCI_PARAM_DEBUG_BUSRESETS (8) is set in the debug parameter bitmask, we
will unmask bus reset interrupts so we can log them.
irq_handler logs the bus reset interrupt. However, we can't clear the bus
reset event flag in irq_handler, because we won't service the event until
later. irq_handler exits with the event flag still set. If the
corresponding interrupt is still unmasked, the first bus reset will
usually freeze the system due to irq_handler being called again each
time it exits. This freeze can be reproduced by loading firewire_ohci
with "modprobe firewire_ohci debug=-1" (to enable all debugging output).
Apparently there are also some cases where bus_reset_work will get called
soon enough to clear the event, and operation will continue normally.
This freeze was first reported a few months after a007bb85 was committed,
but until now it was never fixed. The debug level could safely be set
to -1 through sysfs after the module was loaded, but this would be
ineffectual in logging bus reset interrupts since they were only
unmasked during initialization.
irq_handler will now leave the event flag set but mask bus reset
interrupts, so irq_handler won't be called again and there will be no
freeze. If OHCI_PARAM_DEBUG_BUSRESETS is enabled, bus_reset_work will
unmask the interrupt after servicing the event, so future interrupts
will be caught as desired.
As a side effect to this change, OHCI_PARAM_DEBUG_BUSRESETS can now be
enabled through sysfs in addition to during initial module loading.
However, when enabled through sysfs, logging of bus reset interrupts will
be effective only starting with the second bus reset, after
bus_reset_work has executed.
Signed-off-by: Adam Goldman <adamg@pobox.com>
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
Commit 5a95f1ded2 ("firewire: ohci: use devres for requested IRQ")
also removed the call to free_irq() in pci_remove(), leading to a
leftover irq of devm_request_irq() at pci_disable_msi() in pci_remove()
when unbinding the driver from the device
remove_proc_entry: removing non-empty directory 'irq/136', leaking at
least 'firewire_ohci'
Call Trace:
? remove_proc_entry+0x19c/0x1c0
? __warn+0x81/0x130
? remove_proc_entry+0x19c/0x1c0
? report_bug+0x171/0x1a0
? console_unlock+0x78/0x120
? handle_bug+0x3c/0x80
? exc_invalid_op+0x17/0x70
? asm_exc_invalid_op+0x1a/0x20
? remove_proc_entry+0x19c/0x1c0
unregister_irq_proc+0xf4/0x120
free_desc+0x3d/0xe0
? kfree+0x29f/0x2f0
irq_free_descs+0x47/0x70
msi_domain_free_locked.part.0+0x19d/0x1d0
msi_domain_free_irqs_all_locked+0x81/0xc0
pci_free_msi_irqs+0x12/0x40
pci_disable_msi+0x4c/0x60
pci_remove+0x9d/0xc0 [firewire_ohci
01b483699bebf9cb07a3d69df0aa2bee71db1b26]
pci_device_remove+0x37/0xa0
device_release_driver_internal+0x19f/0x200
unbind_store+0xa1/0xb0
remove irq with devm_free_irq() before pci_disable_msi()
also remove it in fail_msi: of pci_probe() as this would lead to
an identical leak
Cc: stable@vger.kernel.org
Fixes: 5a95f1ded2 ("firewire: ohci: use devres for requested IRQ")
Signed-off-by: Edmund Raile <edmund.raile@proton.me>
Link: https://lore.kernel.org/r/20240229144723.13047-2-edmund.raile@proton.me
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
VIA VT6306/6307/6308 provides PCI interface compliant to 1394 OHCI. When
the hardware is combined with Asmedia ASM1083/1085 PCIe-to-PCI bus bridge,
it appears that accesses to its 'Isochronous Cycle Timer' register (offset
0xf0 on PCI memory space) often causes unexpected system reboot in any
type of AMD Ryzen machine (both 0x17 and 0x19 families). It does not
appears in the other type of machine (AMD pre-Ryzen machine, Intel
machine, at least), or in the other OHCI 1394 hardware (e.g. Texas
Instruments).
The issue explicitly appears at a commit dcadfd7f7c ("firewire: core:
use union for callback of transaction completion") added to v6.5 kernel.
It changed 1394 OHCI driver to access to the register every time to
dispatch local asynchronous transaction. However, the issue exists in
older version of kernel as long as it runs in AMD Ryzen machine, since
the access to the register is required to maintain bus time. It is not
hard to imagine that users experience the unexpected system reboot when
generating bus reset by plugging any devices in, or reading the register
by time-aware application programs; e.g. audio sample processing.
This commit suppresses the unexpected system reboot in the combination of
hardware. It avoids the access itself. As a result, the software stack can
not provide the hardware time anymore to unit drivers, userspace
applications, and nodes in the same IEEE 1394 bus. It brings apparent
disadvantage since time-aware application programs require it, while
time-unaware applications are available again; e.g. sbp2.
Cc: stable@vger.kernel.org
Reported-by: Jiri Slaby <jirislaby@kernel.org>
Closes: https://bugzilla.suse.com/show_bug.cgi?id=1215436
Reported-by: Mario Limonciello <mario.limonciello@amd.com>
Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217994
Reported-by: Tobias Gruetzmacher <tobias-lists@23.gs>
Closes: https://sourceforge.net/p/linux1394/mailman/message/58711901/
Closes: https://bugzilla.redhat.com/show_bug.cgi?id=2240973
Closes: https://bugs.launchpad.net/linux/+bug/2043905
Link: https://lore.kernel.org/r/20240102110150.244475-1-o-takashi@sakamocchi.jp
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
The 1394 OHCI driver allocates several non-coherent DMA buffers for AR
request and response contexts. The buffers are mapped to kernel virtual
address (VMA) so that the first page locates after the last page. Even
when large payload of packet is handled crossing the boundary of buffers,
the driver operates continuously on VMA.
No kernel API is provided for this kind of mapping, while it is possible
to release the buffer when PCI device is going to be released.
This commit moves the call of release helper function to the callback
function of release resources.
Link: https://lore.kernel.org/r/20230604054451.161076-10-o-takashi@sakamocchi.jp
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
Takashi Sakamoto
Adam Goldman
Edmund Raile