From 5a43dc9f4ee0a3624d0598ee14e8ef8468914525 Mon Sep 17 00:00:00 2001 From: Takashi Sakamoto Date: Mon, 13 Oct 2025 23:03:10 +0900 Subject: [PATCH 01/12] firewire: core: detect device quirk when reading configuration ROM Every time the bus manager runs, the cached configuration ROM content of the IRM device is investigated to detect device-specific quirks. This detection can be performed in advance when reading the configuration ROM. This commit adds device quirk flags to the fw_device structure, and initializes them after reading the bus information block of the configuration ROM. The quirk flags are immutable once the configuration ROM has been read. Although they are likely accessed concurrently only by the bus manager, this commit ensures safe access by preventing torn writes and reads using the WRITE_ONCE()/READ_ONCE() macros. Link: https://lore.kernel.org/r/20251013140311.97159-2-o-takashi@sakamocchi.jp Signed-off-by: Takashi Sakamoto --- drivers/firewire/core-card.c | 21 +++++++-------------- drivers/firewire/core-device.c | 25 +++++++++++++++++++++++-- include/linux/firewire.h | 11 +++++++++++ 3 files changed, 41 insertions(+), 16 deletions(-) diff --git a/drivers/firewire/core-card.c b/drivers/firewire/core-card.c index e5e0174a0335..6979d6a88ae2 100644 --- a/drivers/firewire/core-card.c +++ b/drivers/firewire/core-card.c @@ -86,8 +86,6 @@ static size_t config_rom_length = 1 + 4 + 1 + 1; */ #define DEFAULT_SPLIT_TIMEOUT (2 * 8000) -#define CANON_OUI 0x000085 - static void generate_config_rom(struct fw_card *card, __be32 *config_rom) { struct fw_descriptor *desc; @@ -308,11 +306,9 @@ __must_hold(&card->lock) cpu_to_be32(local_id), }; bool grace = time_is_before_jiffies64(card->reset_jiffies + msecs_to_jiffies(125)); - bool irm_is_1394_1995_only = false; - bool keep_this_irm = false; struct fw_node *irm_node; struct fw_device *irm_device; - int irm_node_id; + int irm_node_id, irm_device_quirks = 0; int rcode; lockdep_assert_held(&card->lock); @@ -328,15 +324,12 @@ __must_hold(&card->lock) return BM_CONTENTION_OUTCOME_IRM_HAS_LINK_OFF; } + // NOTE: It is likely that the quirk detection for IRM device has not done yet. irm_device = fw_node_get_device(irm_node); - if (irm_device && irm_device->config_rom) { - irm_is_1394_1995_only = (irm_device->config_rom[2] & 0x000000f0) == 0; - - // Canon MV5i works unreliably if it is not root node. - keep_this_irm = irm_device->config_rom[3] >> 8 == CANON_OUI; - } - - if (irm_is_1394_1995_only && !keep_this_irm) { + if (irm_device) + irm_device_quirks = READ_ONCE(irm_device->quirks); + if ((irm_device_quirks & FW_DEVICE_QUIRK_IRM_IS_1394_1995_ONLY) && + !(irm_device_quirks & FW_DEVICE_QUIRK_IRM_IGNORES_BUS_MANAGER)) { fw_notice(card, "IRM is not 1394a compliant, making local node (%02x) root\n", local_id); return BM_CONTENTION_OUTCOME_IRM_COMPLIES_1394_1995_ONLY; @@ -373,7 +366,7 @@ __must_hold(&card->lock) return BM_CONTENTION_OUTCOME_IRM_HOLDS_LOCAL_NODE_AS_BM; } default: - if (!keep_this_irm) { + if (!(irm_device_quirks & FW_DEVICE_QUIRK_IRM_IGNORES_BUS_MANAGER)) { fw_notice(card, "BM lock failed (%s), making local node (%02x) root\n", fw_rcode_string(rcode), local_id); return BM_CONTENTION_OUTCOME_IRM_COMPLIES_1394_1995_ONLY; diff --git a/drivers/firewire/core-device.c b/drivers/firewire/core-device.c index 457a0da024a7..9bab2d594b89 100644 --- a/drivers/firewire/core-device.c +++ b/drivers/firewire/core-device.c @@ -542,6 +542,21 @@ static struct device_attribute fw_device_attributes[] = { __ATTR_NULL, }; +#define CANON_OUI 0x000085 + +static int detect_quirks_by_bus_information_block(const u32 *bus_information_block) +{ + int quirks = 0; + + if ((bus_information_block[2] & 0x000000f0) == 0) + quirks |= FW_DEVICE_QUIRK_IRM_IS_1394_1995_ONLY; + + if ((bus_information_block[3] >> 8) == CANON_OUI) + quirks |= FW_DEVICE_QUIRK_IRM_IGNORES_BUS_MANAGER; + + return quirks; +} + static int read_rom(struct fw_device *device, int generation, int index, u32 *data) { @@ -582,6 +597,7 @@ static int read_config_rom(struct fw_device *device, int generation) u32 *rom, *stack; u32 sp, key; int i, end, length, ret; + int quirks; rom = kmalloc(sizeof(*rom) * MAX_CONFIG_ROM_SIZE + sizeof(*stack) * MAX_CONFIG_ROM_SIZE, GFP_KERNEL); @@ -612,6 +628,11 @@ static int read_config_rom(struct fw_device *device, int generation) } } + quirks = detect_quirks_by_bus_information_block(rom); + + // Just prevent from torn writing/reading. + WRITE_ONCE(device->quirks, quirks); + device->max_speed = device->node->max_speed; /* @@ -1122,10 +1143,10 @@ static void fw_device_init(struct work_struct *work) device->workfn = fw_device_shutdown; fw_schedule_device_work(device, SHUTDOWN_DELAY); } else { - fw_notice(card, "created device %s: GUID %08x%08x, S%d00\n", + fw_notice(card, "created device %s: GUID %08x%08x, S%d00, quirks %08x\n", dev_name(&device->device), device->config_rom[3], device->config_rom[4], - 1 << device->max_speed); + 1 << device->max_speed, device->quirks); device->config_rom_retries = 0; set_broadcast_channel(device, device->generation); diff --git a/include/linux/firewire.h b/include/linux/firewire.h index 6d208769d456..161829cfcc00 100644 --- a/include/linux/firewire.h +++ b/include/linux/firewire.h @@ -170,6 +170,14 @@ struct fw_attribute_group { struct attribute *attrs[13]; }; +enum fw_device_quirk { + // See afa1282a35d3 ("firewire: core: check for 1394a compliant IRM, fix inaccessibility of Sony camcorder"). + FW_DEVICE_QUIRK_IRM_IS_1394_1995_ONLY = BIT(0), + + // See a509e43ff338 ("firewire: core: fix unstable I/O with Canon camcorder"). + FW_DEVICE_QUIRK_IRM_IGNORES_BUS_MANAGER = BIT(1), +}; + enum fw_device_state { FW_DEVICE_INITIALIZING, FW_DEVICE_RUNNING, @@ -203,6 +211,9 @@ struct fw_device { struct fw_card *card; struct device device; + // A set of enum fw_device_quirk. + int quirks; + struct mutex client_list_mutex; struct list_head client_list; From 15f9610fc96ac6fd2844e63f7bf5a0b08e1c31c8 Mon Sep 17 00:00:00 2001 From: Takashi Sakamoto Date: Mon, 13 Oct 2025 23:03:11 +0900 Subject: [PATCH 02/12] firewire: core: handle device quirk of MOTU Audio Express A commit 3a93d082bacf ("ALSA: firewire-motu: add support for MOTU Audio Express") describes a quirk of MOTU Audio Express. The device returns acknowledge packet with 0x10 as the pending state of any types of asynchronous request transaction. It is completely out of specification. This commit implements handling for that device-specific quirk. The quirk is detected after reading the root directory of configuration ROM. When processing the acknowledge code in 1394 OHCI AT context event handler, firewire-ohci module seeks the device instance of destination node by traversing device hierarchy. If the device has the quirk, the acknowledge code is replaced with the standard code. The 1394 OHCI AT context events occur for outgoing asynchronous request packets. The device traversal is safe since no new request initiators exist after the fw_card_instance has been invalidated. Link: https://lore.kernel.org/r/20251013140311.97159-3-o-takashi@sakamocchi.jp Signed-off-by: Takashi Sakamoto --- drivers/firewire/core-device.c | 53 ++++++++++++++++++++++++++++++++++ drivers/firewire/ohci.c | 29 +++++++++++++++++++ include/linux/firewire.h | 3 ++ 3 files changed, 85 insertions(+) diff --git a/drivers/firewire/core-device.c b/drivers/firewire/core-device.c index 9bab2d594b89..33ce4cd357ed 100644 --- a/drivers/firewire/core-device.c +++ b/drivers/firewire/core-device.c @@ -557,6 +557,54 @@ static int detect_quirks_by_bus_information_block(const u32 *bus_information_blo return quirks; } +struct entry_match { + unsigned int index; + u32 value; +}; + +static const struct entry_match motu_audio_express_matches[] = { + { 1, 0x030001f2 }, + { 3, 0xd1000002 }, + { 4, 0x8d000005 }, + { 6, 0x120001f2 }, + { 7, 0x13000033 }, + { 8, 0x17104800 }, +}; + +static int detect_quirks_by_root_directory(const u32 *root_directory, unsigned int length) +{ + static const struct { + enum fw_device_quirk quirk; + const struct entry_match *matches; + unsigned int match_count; + } *entry, entries[] = { + { + .quirk = FW_DEVICE_QUIRK_ACK_PACKET_WITH_INVALID_PENDING_CODE, + .matches = motu_audio_express_matches, + .match_count = ARRAY_SIZE(motu_audio_express_matches), + }, + }; + int quirks = 0; + int i; + + for (i = 0; i < ARRAY_SIZE(entries); ++i) { + int j; + + entry = entries + i; + for (j = 0; j < entry->match_count; ++j) { + unsigned int index = entry->matches[j].index; + unsigned int value = entry->matches[j].value; + + if ((length < index) || (root_directory[index] != value)) + break; + } + if (j == entry->match_count) + quirks |= entry->quirk; + } + + return quirks; +} + static int read_rom(struct fw_device *device, int generation, int index, u32 *data) { @@ -737,6 +785,11 @@ static int read_config_rom(struct fw_device *device, int generation) length = i; } + quirks |= detect_quirks_by_root_directory(rom + ROOT_DIR_OFFSET, length - ROOT_DIR_OFFSET); + + // Just prevent from torn writing/reading. + WRITE_ONCE(device->quirks, quirks); + old_rom = device->config_rom; new_rom = kmemdup(rom, length * 4, GFP_KERNEL); if (new_rom == NULL) { diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c index 030aed5453a1..757dd9c64b1c 100644 --- a/drivers/firewire/ohci.c +++ b/drivers/firewire/ohci.c @@ -1319,6 +1319,14 @@ static void at_context_flush(struct at_context *ctx) enable_work(&ctx->work); } +static int find_fw_device(struct device *dev, const void *data) +{ + struct fw_device *device = fw_device(dev); + const u32 *params = data; + + return (device->generation == params[0]) && (device->node_id == params[1]); +} + static int handle_at_packet(struct context *context, struct descriptor *d, struct descriptor *last) @@ -1390,6 +1398,27 @@ static int handle_at_packet(struct context *context, fallthrough; default: + if (unlikely(evt == 0x10)) { + u32 params[2] = { + packet->generation, + async_header_get_destination(packet->header), + }; + struct device *dev; + + fw_card_get(&ohci->card); + dev = device_find_child(ohci->card.device, (const void *)params, find_fw_device); + fw_card_put(&ohci->card); + if (dev) { + struct fw_device *device = fw_device(dev); + int quirks = READ_ONCE(device->quirks); + + put_device(dev); + if (quirks & FW_DEVICE_QUIRK_ACK_PACKET_WITH_INVALID_PENDING_CODE) { + packet->ack = ACK_PENDING; + break; + } + } + } packet->ack = RCODE_SEND_ERROR; break; } diff --git a/include/linux/firewire.h b/include/linux/firewire.h index 161829cfcc00..f1d8734c0ec6 100644 --- a/include/linux/firewire.h +++ b/include/linux/firewire.h @@ -176,6 +176,9 @@ enum fw_device_quirk { // See a509e43ff338 ("firewire: core: fix unstable I/O with Canon camcorder"). FW_DEVICE_QUIRK_IRM_IGNORES_BUS_MANAGER = BIT(1), + + // MOTU Audio Express transfers acknowledge packet with 0x10 for pending state. + FW_DEVICE_QUIRK_ACK_PACKET_WITH_INVALID_PENDING_CODE = BIT(2), }; enum fw_device_state { From 665ad59b891a98e7bbd50720cc30289c5c253272 Mon Sep 17 00:00:00 2001 From: Takashi Sakamoto Date: Sat, 18 Oct 2025 12:55:29 +0900 Subject: [PATCH 03/12] firewire: core: code refactoring to compute transaction speed This commit refactors the helper function to read the content of configuration ROM with the passed speed. Link: https://lore.kernel.org/r/20251018035532.287124-2-o-takashi@sakamocchi.jp Signed-off-by: Takashi Sakamoto --- drivers/firewire/core-device.c | 35 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 18 deletions(-) diff --git a/drivers/firewire/core-device.c b/drivers/firewire/core-device.c index 33ce4cd357ed..c698d4ced7d7 100644 --- a/drivers/firewire/core-device.c +++ b/drivers/firewire/core-device.c @@ -605,8 +605,7 @@ static int detect_quirks_by_root_directory(const u32 *root_directory, unsigned i return quirks; } -static int read_rom(struct fw_device *device, - int generation, int index, u32 *data) +static int read_rom(struct fw_device *device, int generation, int speed, int index, u32 *data) { u64 offset = (CSR_REGISTER_BASE | CSR_CONFIG_ROM) + index * 4; int i, rcode; @@ -617,7 +616,7 @@ static int read_rom(struct fw_device *device, for (i = 10; i < 100; i += 10) { rcode = fw_run_transaction(device->card, TCODE_READ_QUADLET_REQUEST, device->node_id, - generation, device->max_speed, offset, data, 4); + generation, speed, offset, data, 4); if (rcode != RCODE_BUSY) break; msleep(i); @@ -644,7 +643,7 @@ static int read_config_rom(struct fw_device *device, int generation) const u32 *old_rom, *new_rom; u32 *rom, *stack; u32 sp, key; - int i, end, length, ret; + int i, end, length, ret, speed; int quirks; rom = kmalloc(sizeof(*rom) * MAX_CONFIG_ROM_SIZE + @@ -655,11 +654,11 @@ static int read_config_rom(struct fw_device *device, int generation) stack = &rom[MAX_CONFIG_ROM_SIZE]; memset(rom, 0, sizeof(*rom) * MAX_CONFIG_ROM_SIZE); - device->max_speed = SCODE_100; + speed = SCODE_100; /* First read the bus info block. */ for (i = 0; i < 5; i++) { - ret = read_rom(device, generation, i, &rom[i]); + ret = read_rom(device, generation, speed, i, &rom[i]); if (ret != RCODE_COMPLETE) goto out; /* @@ -681,7 +680,7 @@ static int read_config_rom(struct fw_device *device, int generation) // Just prevent from torn writing/reading. WRITE_ONCE(device->quirks, quirks); - device->max_speed = device->node->max_speed; + speed = device->node->max_speed; /* * Determine the speed of @@ -692,20 +691,18 @@ static int read_config_rom(struct fw_device *device, int generation) * because some buggy firmwares set it lower than necessary and because * 1394-1995 nodes do not have the field. */ - if ((rom[2] & 0x7) < device->max_speed || - device->max_speed == SCODE_BETA || - card->beta_repeaters_present) { + if ((rom[2] & 0x7) < speed || speed == SCODE_BETA || card->beta_repeaters_present) { u32 dummy; /* for S1600 and S3200 */ - if (device->max_speed == SCODE_BETA) - device->max_speed = card->link_speed; + if (speed == SCODE_BETA) + speed = card->link_speed; - while (device->max_speed > SCODE_100) { - if (read_rom(device, generation, 0, &dummy) == + while (speed > SCODE_100) { + if (read_rom(device, generation, speed, 0, &dummy) == RCODE_COMPLETE) break; - device->max_speed--; + --speed; } } @@ -734,7 +731,7 @@ static int read_config_rom(struct fw_device *device, int generation) } /* Read header quadlet for the block to get the length. */ - ret = read_rom(device, generation, i, &rom[i]); + ret = read_rom(device, generation, speed, i, &rom[i]); if (ret != RCODE_COMPLETE) goto out; end = i + (rom[i] >> 16) + 1; @@ -758,7 +755,7 @@ static int read_config_rom(struct fw_device *device, int generation) * it references another block, and push it in that case. */ for (; i < end; i++) { - ret = read_rom(device, generation, i, &rom[i]); + ret = read_rom(device, generation, speed, i, &rom[i]); if (ret != RCODE_COMPLETE) goto out; @@ -785,6 +782,8 @@ static int read_config_rom(struct fw_device *device, int generation) length = i; } + device->max_speed = speed; + quirks |= detect_quirks_by_root_directory(rom + ROOT_DIR_OFFSET, length - ROOT_DIR_OFFSET); // Just prevent from torn writing/reading. @@ -1234,7 +1233,7 @@ static int reread_config_rom(struct fw_device *device, int generation, int i, rcode; for (i = 0; i < 6; i++) { - rcode = read_rom(device, generation, i, &q); + rcode = read_rom(device, generation, device->max_speed, i, &q); if (rcode != RCODE_COMPLETE) return rcode; From 55b4e903a156bc81e15fbe3af9be0664f7a3b3ca Mon Sep 17 00:00:00 2001 From: Takashi Sakamoto Date: Sat, 18 Oct 2025 12:55:30 +0900 Subject: [PATCH 04/12] firewire: core: determine transaction speed after detecting quirks Current implementation determines the maximum transaction speed supported by the target device after reading bus information block of configuration ROM. The read operations for root directory block are then performed at the determined speed. However, some devices have quirks that cause issues when transactions are performed at the determined speed. In the first place, all devices are required to support the lowest speed (S100) and must respond successfully to any read request within the configuration ROM space. Therefore it is safe to postpone speed determination until the entire configuration ROM has been read. This commit moves the speed determination after reading root directory. Link: https://lore.kernel.org/r/20251018035532.287124-3-o-takashi@sakamocchi.jp Signed-off-by: Takashi Sakamoto --- drivers/firewire/core-device.c | 53 ++++++++++++++++------------------ 1 file changed, 25 insertions(+), 28 deletions(-) diff --git a/drivers/firewire/core-device.c b/drivers/firewire/core-device.c index c698d4ced7d7..6a5740ed4934 100644 --- a/drivers/firewire/core-device.c +++ b/drivers/firewire/core-device.c @@ -680,32 +680,6 @@ static int read_config_rom(struct fw_device *device, int generation) // Just prevent from torn writing/reading. WRITE_ONCE(device->quirks, quirks); - speed = device->node->max_speed; - - /* - * Determine the speed of - * - devices with link speed less than PHY speed, - * - devices with 1394b PHY (unless only connected to 1394a PHYs), - * - all devices if there are 1394b repeaters. - * Note, we cannot use the bus info block's link_spd as starting point - * because some buggy firmwares set it lower than necessary and because - * 1394-1995 nodes do not have the field. - */ - if ((rom[2] & 0x7) < speed || speed == SCODE_BETA || card->beta_repeaters_present) { - u32 dummy; - - /* for S1600 and S3200 */ - if (speed == SCODE_BETA) - speed = card->link_speed; - - while (speed > SCODE_100) { - if (read_rom(device, generation, speed, 0, &dummy) == - RCODE_COMPLETE) - break; - --speed; - } - } - /* * Now parse the config rom. The config rom is a recursive * directory structure so we parse it using a stack of @@ -782,13 +756,36 @@ static int read_config_rom(struct fw_device *device, int generation) length = i; } - device->max_speed = speed; - quirks |= detect_quirks_by_root_directory(rom + ROOT_DIR_OFFSET, length - ROOT_DIR_OFFSET); // Just prevent from torn writing/reading. WRITE_ONCE(device->quirks, quirks); + speed = device->node->max_speed; + + // Determine the speed of + // - devices with link speed less than PHY speed, + // - devices with 1394b PHY (unless only connected to 1394a PHYs), + // - all devices if there are 1394b repeaters. + // Note, we cannot use the bus info block's link_spd as starting point because some buggy + // firmwares set it lower than necessary and because 1394-1995 nodes do not have the field. + if ((rom[2] & 0x7) < speed || speed == SCODE_BETA || card->beta_repeaters_present) { + u32 dummy; + + // for S1600 and S3200. + if (speed == SCODE_BETA) + speed = card->link_speed; + + while (speed > SCODE_100) { + if (read_rom(device, generation, speed, 0, &dummy) == + RCODE_COMPLETE) + break; + --speed; + } + } + + device->max_speed = speed; + old_rom = device->config_rom; new_rom = kmemdup(rom, length * 4, GFP_KERNEL); if (new_rom == NULL) { From d52bb3daad3f28403676dff31fa0577bdaf8e7c6 Mon Sep 17 00:00:00 2001 From: Takashi Sakamoto Date: Sat, 18 Oct 2025 12:55:31 +0900 Subject: [PATCH 05/12] firewire: core: handle device quirk of TASCAM FW-1884/FW-1804/FW-1082 TASCAM FW-1884/FW-1804/FW-1082 is too lazy to repspond to asynchronous request at S400. The asynchronous transaction often results in timeout. This is a problematic quirk. This commit adds support for the quirk. When identifying the new quirk flag, then the transaction speed is configured at S200. Link: https://lore.kernel.org/r/20251018035532.287124-4-o-takashi@sakamocchi.jp Signed-off-by: Takashi Sakamoto --- drivers/firewire/core-device.c | 18 +++++++++++++++++- include/linux/firewire.h | 3 +++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/drivers/firewire/core-device.c b/drivers/firewire/core-device.c index 6a5740ed4934..1674de477852 100644 --- a/drivers/firewire/core-device.c +++ b/drivers/firewire/core-device.c @@ -571,6 +571,14 @@ static const struct entry_match motu_audio_express_matches[] = { { 8, 0x17104800 }, }; +static const struct entry_match tascam_fw_series_matches[] = { + { 1, 0x0300022e }, + { 3, 0x8d000006 }, + { 4, 0xd1000001 }, + { 6, 0x1200022e }, + { 8, 0xd4000004 }, +}; + static int detect_quirks_by_root_directory(const u32 *root_directory, unsigned int length) { static const struct { @@ -583,6 +591,11 @@ static int detect_quirks_by_root_directory(const u32 *root_directory, unsigned i .matches = motu_audio_express_matches, .match_count = ARRAY_SIZE(motu_audio_express_matches), }, + { + .quirk = FW_DEVICE_QUIRK_UNSTABLE_AT_S400, + .matches = tascam_fw_series_matches, + .match_count = ARRAY_SIZE(tascam_fw_series_matches), + }, }; int quirks = 0; int i; @@ -761,7 +774,10 @@ static int read_config_rom(struct fw_device *device, int generation) // Just prevent from torn writing/reading. WRITE_ONCE(device->quirks, quirks); - speed = device->node->max_speed; + if (unlikely(quirks & FW_DEVICE_QUIRK_UNSTABLE_AT_S400)) + speed = SCODE_200; + else + speed = device->node->max_speed; // Determine the speed of // - devices with link speed less than PHY speed, diff --git a/include/linux/firewire.h b/include/linux/firewire.h index f1d8734c0ec6..6143b7d28eac 100644 --- a/include/linux/firewire.h +++ b/include/linux/firewire.h @@ -179,6 +179,9 @@ enum fw_device_quirk { // MOTU Audio Express transfers acknowledge packet with 0x10 for pending state. FW_DEVICE_QUIRK_ACK_PACKET_WITH_INVALID_PENDING_CODE = BIT(2), + + // TASCAM FW-1082/FW-1804/FW-1884 often freezes when receiving S400 packets. + FW_DEVICE_QUIRK_UNSTABLE_AT_S400 = BIT(3), }; enum fw_device_state { From dbd0cf204fe6ba7ba226153d1d90369019b90164 Mon Sep 17 00:00:00 2001 From: Takashi Sakamoto Date: Sat, 18 Oct 2025 12:55:32 +0900 Subject: [PATCH 06/12] ALSA: firewire-tascam: reserve resources for transferred isochronous packets at S400 TASCAM FW-1884/FW-1804/FW-1082 have a quirk that they often freeze when receiving isochronous packets at S400. This behaviour is suppressed by a new quirk flag added in Linux FireWire core to restrict maximum speed. Consequently both of the asynchronous transactions and isochronous transmissions are done at S200. However, the device still transfers isochronous packet at S400, and the way to indicate the transmission speed is not cleared yet. This commit correctly reserves isochronous resources for the transferred packet stream at S400. As a beneficial side effect, the pair of isochronous transmissions for FW-1884 fits within the bandwidth capacity of the bus. Link: https://lore.kernel.org/r/20251018035532.287124-5-o-takashi@sakamocchi.jp Signed-off-by: Takashi Sakamoto --- sound/firewire/tascam/tascam-stream.c | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/sound/firewire/tascam/tascam-stream.c b/sound/firewire/tascam/tascam-stream.c index 9c8fddd7dee1..4ecd151a46c1 100644 --- a/sound/firewire/tascam/tascam-stream.c +++ b/sound/firewire/tascam/tascam-stream.c @@ -282,20 +282,22 @@ static int keep_resources(struct snd_tscm *tscm, unsigned int rate, struct amdtp_stream *stream) { struct fw_iso_resources *resources; + int speed; int err; - if (stream == &tscm->tx_stream) + if (stream == &tscm->tx_stream) { resources = &tscm->tx_resources; - else + speed = fw_parent_device(tscm->unit)->max_speed; + } else { resources = &tscm->rx_resources; + speed = SCODE_400; + } err = amdtp_tscm_set_parameters(stream, rate); if (err < 0) return err; - return fw_iso_resources_allocate(resources, - amdtp_stream_get_max_payload(stream), - fw_parent_device(tscm->unit)->max_speed); + return fw_iso_resources_allocate(resources, amdtp_stream_get_max_payload(stream), speed); } static int init_stream(struct snd_tscm *tscm, struct amdtp_stream *s) @@ -455,7 +457,6 @@ int snd_tscm_stream_start_duplex(struct snd_tscm *tscm, unsigned int rate) } if (!amdtp_stream_running(&tscm->rx_stream)) { - int spd = fw_parent_device(tscm->unit)->max_speed; unsigned int tx_init_skip_cycles; err = set_stream_formats(tscm, rate); @@ -466,13 +467,13 @@ int snd_tscm_stream_start_duplex(struct snd_tscm *tscm, unsigned int rate) if (err < 0) goto error; - err = amdtp_domain_add_stream(&tscm->domain, &tscm->rx_stream, - tscm->rx_resources.channel, spd); + err = amdtp_domain_add_stream(&tscm->domain, &tscm->rx_stream, tscm->rx_resources.channel, + fw_parent_device(tscm->unit)->max_speed); if (err < 0) goto error; - err = amdtp_domain_add_stream(&tscm->domain, &tscm->tx_stream, - tscm->tx_resources.channel, spd); + err = amdtp_domain_add_stream(&tscm->domain, &tscm->tx_stream, tscm->tx_resources.channel, + SCODE_400); if (err < 0) goto error; From b330f98ff238ad9446574965d09cab33736519d5 Mon Sep 17 00:00:00 2001 From: Takashi Sakamoto Date: Mon, 20 Oct 2025 20:58:10 +0900 Subject: [PATCH 07/12] firewire: core: use cleanup function to release cached configuration ROM When returning from read_config_rom() function, the allocated buffer and the previous buffer for configuration ROM should be released. The cleanup function is useful in the case. This commit uses the cleanup function to remove goto statements. Link: https://lore.kernel.org/r/20251020115810.92839-1-o-takashi@sakamocchi.jp Signed-off-by: Takashi Sakamoto --- drivers/firewire/core-device.c | 34 ++++++++++++---------------------- 1 file changed, 12 insertions(+), 22 deletions(-) diff --git a/drivers/firewire/core-device.c b/drivers/firewire/core-device.c index 1674de477852..9b0080397154 100644 --- a/drivers/firewire/core-device.c +++ b/drivers/firewire/core-device.c @@ -653,8 +653,8 @@ static int read_rom(struct fw_device *device, int generation, int speed, int ind static int read_config_rom(struct fw_device *device, int generation) { struct fw_card *card = device->card; - const u32 *old_rom, *new_rom; - u32 *rom, *stack; + const u32 *new_rom, *old_rom __free(kfree) = NULL; + u32 *stack, *rom __free(kfree) = NULL; u32 sp, key; int i, end, length, ret, speed; int quirks; @@ -673,7 +673,7 @@ static int read_config_rom(struct fw_device *device, int generation) for (i = 0; i < 5; i++) { ret = read_rom(device, generation, speed, i, &rom[i]); if (ret != RCODE_COMPLETE) - goto out; + return ret; /* * As per IEEE1212 7.2, during initialization, devices can * reply with a 0 for the first quadlet of the config @@ -682,10 +682,8 @@ static int read_config_rom(struct fw_device *device, int generation) * harddisk). In that case we just fail, and the * retry mechanism will try again later. */ - if (i == 0 && rom[i] == 0) { - ret = RCODE_BUSY; - goto out; - } + if (i == 0 && rom[i] == 0) + return RCODE_BUSY; } quirks = detect_quirks_by_bus_information_block(rom); @@ -712,15 +710,13 @@ static int read_config_rom(struct fw_device *device, int generation) */ key = stack[--sp]; i = key & 0xffffff; - if (WARN_ON(i >= MAX_CONFIG_ROM_SIZE)) { - ret = -ENXIO; - goto out; - } + if (WARN_ON(i >= MAX_CONFIG_ROM_SIZE)) + return -ENXIO; /* Read header quadlet for the block to get the length. */ ret = read_rom(device, generation, speed, i, &rom[i]); if (ret != RCODE_COMPLETE) - goto out; + return ret; end = i + (rom[i] >> 16) + 1; if (end > MAX_CONFIG_ROM_SIZE) { /* @@ -744,7 +740,7 @@ static int read_config_rom(struct fw_device *device, int generation) for (; i < end; i++) { ret = read_rom(device, generation, speed, i, &rom[i]); if (ret != RCODE_COMPLETE) - goto out; + return ret; if ((key >> 30) != 3 || (rom[i] >> 30) < 2) continue; @@ -804,25 +800,19 @@ static int read_config_rom(struct fw_device *device, int generation) old_rom = device->config_rom; new_rom = kmemdup(rom, length * 4, GFP_KERNEL); - if (new_rom == NULL) { - ret = -ENOMEM; - goto out; - } + if (new_rom == NULL) + return -ENOMEM; scoped_guard(rwsem_write, &fw_device_rwsem) { device->config_rom = new_rom; device->config_rom_length = length; } - kfree(old_rom); - ret = RCODE_COMPLETE; device->max_rec = rom[2] >> 12 & 0xf; device->cmc = rom[2] >> 30 & 1; device->irmc = rom[2] >> 31 & 1; - out: - kfree(rom); - return ret; + return RCODE_COMPLETE; } static void fw_unit_release(struct device *dev) From ddc021b58b52f22216b24ac0900e9d07eefa5f0f Mon Sep 17 00:00:00 2001 From: Takashi Sakamoto Date: Sat, 1 Nov 2025 19:21:30 +0900 Subject: [PATCH 08/12] firewire: core: code refactoring to remove transaction entry The list operation to remove transaction entry appears several times in transaction implementation and can be replaced with a helper function. Link: https://lore.kernel.org/r/20251101102131.925071-2-o-takashi@sakamocchi.jp Signed-off-by: Takashi Sakamoto --- drivers/firewire/core-transaction.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/drivers/firewire/core-transaction.c b/drivers/firewire/core-transaction.c index dd3656a0c1ff..8bd79c3f97f2 100644 --- a/drivers/firewire/core-transaction.c +++ b/drivers/firewire/core-transaction.c @@ -44,6 +44,13 @@ static int try_cancel_split_timeout(struct fw_transaction *t) return 1; } +// card->transactions.lock must be acquired in advance. +static void remove_transaction_entry(struct fw_card *card, struct fw_transaction *entry) +{ + list_del_init(&entry->link); + card->transactions.tlabel_mask &= ~(1ULL << entry->tlabel); +} + static int close_transaction(struct fw_transaction *transaction, struct fw_card *card, int rcode, u32 response_tstamp) { @@ -55,8 +62,7 @@ static int close_transaction(struct fw_transaction *transaction, struct fw_card list_for_each_entry(iter, &card->transactions.list, link) { if (iter == transaction) { if (try_cancel_split_timeout(iter)) { - list_del_init(&iter->link); - card->transactions.tlabel_mask &= ~(1ULL << iter->tlabel); + remove_transaction_entry(card, iter); t = iter; } break; @@ -122,8 +128,7 @@ static void split_transaction_timeout_callback(struct timer_list *timer) scoped_guard(spinlock_irqsave, &card->transactions.lock) { if (list_empty(&t->link)) return; - list_del(&t->link); - card->transactions.tlabel_mask &= ~(1ULL << t->tlabel); + remove_transaction_entry(card, t); } if (!t->with_tstamp) { @@ -1142,8 +1147,7 @@ void fw_core_handle_response(struct fw_card *card, struct fw_packet *p) list_for_each_entry(iter, &card->transactions.list, link) { if (iter->node_id == source && iter->tlabel == tlabel) { if (try_cancel_split_timeout(iter)) { - list_del_init(&iter->link); - card->transactions.tlabel_mask &= ~(1ULL << iter->tlabel); + remove_transaction_entry(card, iter); t = iter; } break; From fa2dc27100768a8a994c377051fa17a47cc66c76 Mon Sep 17 00:00:00 2001 From: Takashi Sakamoto Date: Sat, 1 Nov 2025 19:21:31 +0900 Subject: [PATCH 09/12] firewire: core: code refactoring to find and pop transaction entry The list operation to find and pop transaction entry appears several times in transaction implementation, and can be replaced with a helper functional macro. Link: https://lore.kernel.org/r/20251101102131.925071-3-o-takashi@sakamocchi.jp Signed-off-by: Takashi Sakamoto --- drivers/firewire/core-transaction.c | 45 ++++++++++++++--------------- 1 file changed, 22 insertions(+), 23 deletions(-) diff --git a/drivers/firewire/core-transaction.c b/drivers/firewire/core-transaction.c index 8bd79c3f97f2..e80791d6d46b 100644 --- a/drivers/firewire/core-transaction.c +++ b/drivers/firewire/core-transaction.c @@ -51,28 +51,34 @@ static void remove_transaction_entry(struct fw_card *card, struct fw_transaction card->transactions.tlabel_mask &= ~(1ULL << entry->tlabel); } +// card->transactions.lock must be acquired in advance. +#define find_and_pop_transaction_entry(card, condition) \ +({ \ + struct fw_transaction *iter, *t = NULL; \ + list_for_each_entry(iter, &card->transactions.list, link) { \ + if (condition) { \ + t = iter; \ + break; \ + } \ + } \ + if (t && try_cancel_split_timeout(t)) \ + remove_transaction_entry(card, t); \ + t; \ +}) + static int close_transaction(struct fw_transaction *transaction, struct fw_card *card, int rcode, u32 response_tstamp) { - struct fw_transaction *t = NULL, *iter; + struct fw_transaction *t; // NOTE: This can be without irqsave when we can guarantee that __fw_send_request() for // local destination never runs in any type of IRQ context. scoped_guard(spinlock_irqsave, &card->transactions.lock) { - list_for_each_entry(iter, &card->transactions.list, link) { - if (iter == transaction) { - if (try_cancel_split_timeout(iter)) { - remove_transaction_entry(card, iter); - t = iter; - } - break; - } - } + t = find_and_pop_transaction_entry(card, iter == transaction); + if (!t) + return -ENOENT; } - if (!t) - return -ENOENT; - if (!t->with_tstamp) { t->callback.without_tstamp(card, rcode, NULL, 0, t->callback_data); } else { @@ -1102,7 +1108,7 @@ EXPORT_SYMBOL(fw_core_handle_request); void fw_core_handle_response(struct fw_card *card, struct fw_packet *p) { - struct fw_transaction *t = NULL, *iter; + struct fw_transaction *t = NULL; u32 *data; size_t data_length; int tcode, tlabel, source, rcode; @@ -1144,15 +1150,8 @@ void fw_core_handle_response(struct fw_card *card, struct fw_packet *p) // NOTE: This can be without irqsave when we can guarantee that __fw_send_request() for // local destination never runs in any type of IRQ context. scoped_guard(spinlock_irqsave, &card->transactions.lock) { - list_for_each_entry(iter, &card->transactions.list, link) { - if (iter->node_id == source && iter->tlabel == tlabel) { - if (try_cancel_split_timeout(iter)) { - remove_transaction_entry(card, iter); - t = iter; - } - break; - } - } + t = find_and_pop_transaction_entry(card, + iter->node_id == source && iter->tlabel == tlabel); } trace_async_response_inbound((uintptr_t)t, card->index, p->generation, p->speed, p->ack, From 594a6a27fb17b967fcdf32166845efdf3e1ecfbf Mon Sep 17 00:00:00 2001 From: Takashi Sakamoto Date: Sun, 9 Nov 2025 15:55:25 +0900 Subject: [PATCH 10/12] firewire: core: clear sources of hardware interrupt at card removal Due to the factors external to the system, hardware events may still be handled while a card instance is being removed. The sources of hardware IRQs should be cleared during card removal so that workqueues can be safely destroyed. This commit adds a disable callback to the underlying driver operations. After this callback returns, the underlying driver guarantees that it will no longer handle hardware events. Link: https://lore.kernel.org/r/20251109065525.163464-1-o-takashi@sakamocchi.jp Signed-off-by: Takashi Sakamoto --- drivers/firewire/core-card.c | 3 +++ drivers/firewire/core.h | 3 +++ drivers/firewire/ohci.c | 44 +++++++++++++++++++++++++++++------- 3 files changed, 42 insertions(+), 8 deletions(-) diff --git a/drivers/firewire/core-card.c b/drivers/firewire/core-card.c index 6979d6a88ae2..65bd9db996c0 100644 --- a/drivers/firewire/core-card.c +++ b/drivers/firewire/core-card.c @@ -784,9 +784,12 @@ void fw_core_remove_card(struct fw_card *card) /* Switch off most of the card driver interface. */ dummy_driver.free_iso_context = card->driver->free_iso_context; dummy_driver.stop_iso = card->driver->stop_iso; + dummy_driver.disable = card->driver->disable; card->driver = &dummy_driver; + drain_workqueue(card->isoc_wq); drain_workqueue(card->async_wq); + card->driver->disable(card); scoped_guard(spinlock_irqsave, &card->lock) fw_destroy_nodes(card); diff --git a/drivers/firewire/core.h b/drivers/firewire/core.h index e67395ce26b5..903812b6bb3f 100644 --- a/drivers/firewire/core.h +++ b/drivers/firewire/core.h @@ -65,6 +65,9 @@ struct fw_card_driver { int (*enable)(struct fw_card *card, const __be32 *config_rom, size_t length); + // After returning the call, any function is no longer triggered to handle hardware event. + void (*disable)(struct fw_card *card); + int (*read_phy_reg)(struct fw_card *card, int address); int (*update_phy_reg)(struct fw_card *card, int address, int clear_bits, int set_bits); diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c index 757dd9c64b1c..0625d11dbd74 100644 --- a/drivers/firewire/ohci.c +++ b/drivers/firewire/ohci.c @@ -2408,6 +2408,41 @@ static int ohci_enable(struct fw_card *card, return 0; } +static void ohci_disable(struct fw_card *card) +{ + struct pci_dev *pdev = to_pci_dev(card->device); + struct fw_ohci *ohci = pci_get_drvdata(pdev); + int i, irq = pci_irq_vector(pdev, 0); + + // If the removal is happening from the suspend state, LPS won't be enabled and host + // registers (eg., IntMaskClear) won't be accessible. + if (!(reg_read(ohci, OHCI1394_HCControlSet) & OHCI1394_HCControl_LPS)) + return; + + reg_write(ohci, OHCI1394_IntMaskClear, ~0); + flush_writes(ohci); + + if (irq >= 0) + synchronize_irq(irq); + + flush_work(&ohci->ar_request_ctx.work); + flush_work(&ohci->ar_response_ctx.work); + flush_work(&ohci->at_request_ctx.work); + flush_work(&ohci->at_response_ctx.work); + + for (i = 0; i < ohci->n_ir; ++i) { + if (!(ohci->ir_context_mask & BIT(i))) + flush_work(&ohci->ir_context_list[i].base.work); + } + for (i = 0; i < ohci->n_it; ++i) { + if (!(ohci->it_context_mask & BIT(i))) + flush_work(&ohci->it_context_list[i].base.work); + } + + at_context_flush(&ohci->at_request_ctx); + at_context_flush(&ohci->at_response_ctx); +} + static int ohci_set_config_rom(struct fw_card *card, const __be32 *config_rom, size_t length) { @@ -3442,6 +3477,7 @@ static int ohci_flush_iso_completions(struct fw_iso_context *base) static const struct fw_card_driver ohci_driver = { .enable = ohci_enable, + .disable = ohci_disable, .read_phy_reg = ohci_read_phy_reg, .update_phy_reg = ohci_update_phy_reg, .set_config_rom = ohci_set_config_rom, @@ -3681,14 +3717,6 @@ static void pci_remove(struct pci_dev *dev) struct fw_ohci *ohci = pci_get_drvdata(dev); int irq; - /* - * If the removal is happening from the suspend state, LPS won't be - * enabled and host registers (eg., IntMaskClear) won't be accessible. - */ - if (reg_read(ohci, OHCI1394_HCControlSet) & OHCI1394_HCControl_LPS) { - reg_write(ohci, OHCI1394_IntMaskClear, ~0); - flush_writes(ohci); - } fw_core_remove_card(&ohci->card); /* From ae1ef2fbb8c9091e0ea62734a4a232ad9928701b Mon Sep 17 00:00:00 2001 From: Marco Crivellari Date: Wed, 12 Nov 2025 13:01:25 +0100 Subject: [PATCH 11/12] firewire: core: add WQ_UNBOUND to alloc_workqueue users MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Currently if a user enqueues a work item using schedule_delayed_work() the used wq is "system_wq" (per-cpu wq) while queue_delayed_work() use WORK_CPU_UNBOUND (used when a cpu is not specified). The same applies to schedule_work() that is using system_wq and queue_work(), that makes use again of WORK_CPU_UNBOUND. This lack of consistency cannot be addressed without refactoring the API. alloc_workqueue() treats all queues as per-CPU by default, while unbound workqueues must opt-in via WQ_UNBOUND. This default is suboptimal: most workloads benefit from unbound queues, allowing the scheduler to place worker threads where they’re needed and reducing noise when CPUs are isolated. This continues the effort to refactor workqueue APIs, which began with the introduction of new workqueues and a new alloc_workqueue flag in: commit 128ea9f6ccfb ("workqueue: Add system_percpu_wq and system_dfl_wq") commit 930c2ea566af ("workqueue: Add new WQ_PERCPU flag") This change adds the WQ_UNBOUND flag to explicitly request alloc_workqueue() to be unbound, because this specific workload has no benefit being per-cpu. With the introduction of the WQ_PERCPU flag (equivalent to !WQ_UNBOUND), any alloc_workqueue() caller that doesn’t explicitly specify WQ_UNBOUND must now use WQ_PERCPU. Once migration is complete, WQ_UNBOUND can be removed and unbound will become the implicit default. Suggested-by: Tejun Heo Signed-off-by: Marco Crivellari Link: https://lore.kernel.org/r/20251112120125.124578-1-marco.crivellari@suse.com Signed-off-by: Takashi Sakamoto --- drivers/firewire/core-transaction.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/firewire/core-transaction.c b/drivers/firewire/core-transaction.c index e80791d6d46b..642387f0d7ca 100644 --- a/drivers/firewire/core-transaction.c +++ b/drivers/firewire/core-transaction.c @@ -1440,7 +1440,8 @@ static int __init fw_core_init(void) { int ret; - fw_workqueue = alloc_workqueue("firewire", WQ_MEM_RECLAIM, 0); + fw_workqueue = alloc_workqueue("firewire", WQ_MEM_RECLAIM | WQ_UNBOUND, + 0); if (!fw_workqueue) return -ENOMEM; From 036176d9dba74e23e3ef358e171a77b75837fee0 Mon Sep 17 00:00:00 2001 From: Takashi Sakamoto Date: Wed, 12 Nov 2025 07:38:34 +0900 Subject: [PATCH 12/12] firewire: core: abort pending transactions at card removal IEEE 1394 defines the split, concatenated, and unified transaction. To support the split transaction, core function uses linked list to maintain the transactions waiting for acknowledge packet. After clearing sources of hardware interrupts, the acknowledge packet is no longer handled, therefore it is required to abort the pending transactions. This commit executes callback with RCODE_CANCELLED for the pending transactions at card removal. Link: https://lore.kernel.org/r/20251111223834.311287-1-o-takashi@sakamocchi.jp Signed-off-by: Takashi Sakamoto --- drivers/firewire/core-card.c | 1 + drivers/firewire/core-transaction.c | 28 ++++++++++++++++++++++++++++ drivers/firewire/core.h | 2 ++ drivers/firewire/ohci.c | 5 ----- 4 files changed, 31 insertions(+), 5 deletions(-) diff --git a/drivers/firewire/core-card.c b/drivers/firewire/core-card.c index 65bd9db996c0..9869ea3fd9fc 100644 --- a/drivers/firewire/core-card.c +++ b/drivers/firewire/core-card.c @@ -790,6 +790,7 @@ void fw_core_remove_card(struct fw_card *card) drain_workqueue(card->isoc_wq); drain_workqueue(card->async_wq); card->driver->disable(card); + fw_cancel_pending_transactions(card); scoped_guard(spinlock_irqsave, &card->lock) fw_destroy_nodes(card); diff --git a/drivers/firewire/core-transaction.c b/drivers/firewire/core-transaction.c index 642387f0d7ca..7be4ae3b36b5 100644 --- a/drivers/firewire/core-transaction.c +++ b/drivers/firewire/core-transaction.c @@ -51,6 +51,34 @@ static void remove_transaction_entry(struct fw_card *card, struct fw_transaction card->transactions.tlabel_mask &= ~(1ULL << entry->tlabel); } +// Must be called without holding card->transactions.lock. +void fw_cancel_pending_transactions(struct fw_card *card) +{ + struct fw_transaction *t, *tmp; + LIST_HEAD(pending_list); + + // NOTE: This can be without irqsave when we can guarantee that __fw_send_request() for + // local destination never runs in any type of IRQ context. + scoped_guard(spinlock_irqsave, &card->transactions.lock) { + list_for_each_entry_safe(t, tmp, &card->transactions.list, link) { + if (try_cancel_split_timeout(t)) + list_move(&t->link, &pending_list); + } + } + + list_for_each_entry_safe(t, tmp, &pending_list, link) { + list_del(&t->link); + + if (!t->with_tstamp) { + t->callback.without_tstamp(card, RCODE_CANCELLED, NULL, 0, + t->callback_data); + } else { + t->callback.with_tstamp(card, RCODE_CANCELLED, t->packet.timestamp, 0, + NULL, 0, t->callback_data); + } + } +} + // card->transactions.lock must be acquired in advance. #define find_and_pop_transaction_entry(card, condition) \ ({ \ diff --git a/drivers/firewire/core.h b/drivers/firewire/core.h index 903812b6bb3f..41fb39d9a4e6 100644 --- a/drivers/firewire/core.h +++ b/drivers/firewire/core.h @@ -287,6 +287,8 @@ void fw_fill_response(struct fw_packet *response, u32 *request_header, void fw_request_get(struct fw_request *request); void fw_request_put(struct fw_request *request); +void fw_cancel_pending_transactions(struct fw_card *card); + // Convert the value of IEEE 1394 CYCLE_TIME register to the format of timeStamp field in // descriptors of 1394 OHCI. static inline u32 cycle_time_to_ohci_tstamp(u32 tstamp) diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c index 0625d11dbd74..e3e78dc42530 100644 --- a/drivers/firewire/ohci.c +++ b/drivers/firewire/ohci.c @@ -3719,11 +3719,6 @@ static void pci_remove(struct pci_dev *dev) fw_core_remove_card(&ohci->card); - /* - * FIXME: Fail all pending packets here, now that the upper - * layers can't queue any more. - */ - software_reset(ohci); irq = pci_irq_vector(dev, 0);