LKML Archive on lore.kernel.org help / color / mirror / Atom feed
* [GIT PULL] FireWire updates post 2.6.24 @ 2008-01-30 22:53 Stefan Richter 2008-01-30 22:55 ` Stefan Richter 2008-02-02 13:05 ` [GIT PULL] IEEE 1394 regression fix Stefan Richter 0 siblings, 2 replies; 18+ messages in thread From: Stefan Richter @ 2008-01-30 22:53 UTC (permalink / raw) To: Linus Torvalds; +Cc: Andrew Morton, linux-kernel, linux1394-devel Linus, please pull from the for-linus branch at git://git.kernel.org/pub/scm/linux/kernel/git/ieee1394/linux1394-2.6.git for-linus to receive the following IEEE 1394/ FireWire subsystem updates. Once again it is almost all about bug fixes. Most notable are David Moore's fixes for isochronous reception which greatly enhance the new firewire stack's usability in the IIDC domain (uncompressed video from industrial cameras and webcams). The storage driver firewire-sbp2 received some attention too, prompted by user reports, but that's an ongoing story. I let self-discipline slide this time and added 9 patches which have not yet been in -mm and were not listed in the "What's in linux1394-2.6.git?" report from January 17. Half of those are bug fixes from November which have been laying around waiting for last touch-up until we noticed in January that they indeed address a good number of bug reports. The other new additions are bug fixes too. Everything has been intensively tested with affected and unaffected hardware. Updated status of the new (still EXPERIMENTAL) drivers/firewire subsystem: - integration with userspace libraries still with rough edges, - DV reception broken on OHCI 1.0 variants of VIA VT630x, - IP over 1394 still not implemented/ ported, - SBP-2 (storage) fragile during hotplugging on some setups. Before distributors of kernel packages consider to enable the new firewire drivers in parallel or instead of the classic ieee1394 drivers, we ask that they first read up about all the caveats of the new drivers at wiki.linux1394.org which we regularly update. I will re-post the newly added patches in a reply. Stat and shortlog of the entire queue: MAINTAINERS | 4 +- drivers/firewire/fw-cdev.c | 3 +- drivers/firewire/fw-device.c | 38 ++- drivers/firewire/fw-device.h | 12 + drivers/firewire/fw-ohci.c | 390 +++++++++++++--------- drivers/firewire/fw-sbp2.c | 127 +++++--- drivers/firewire/fw-topology.c | 6 + drivers/firewire/fw-transaction.c | 4 +- drivers/ieee1394/dma.c | 39 +-- drivers/ieee1394/ieee1394_transactions.c | 68 ---- drivers/ieee1394/ohci1394.c | 12 +- drivers/ieee1394/raw1394.c | 4 +- drivers/ieee1394/sbp2.c | 52 ++-- drivers/ieee1394/sbp2.h | 1 - 14 files changed, 424 insertions(+), 336 deletions(-) David Moore (3): firewire: fw-ohci: Fix for dualbuffer three-or-more buffers firewire: fw-ohci: Bug fixes for packet-per-buffer support firewire: fw-ohci: Dynamically allocate buffers for DMA descriptors Jarod Wilson (3): firewire: replace subtraction with bitwise and firewire: fw-sbp2: increase login orb reply timeout, fix "failed to login" firewire: fw-sbp2: Use sbp2 device-provided mgt orb timeout for logins Joe Perches (1): ieee1394: Add missing "space" Nick Piggin (1): ieee1394: nopage Rabin Vincent (1): firewire: Fix extraction of source node id Stefan Richter (17): ieee1394: sbp2: prepare for s/g chaining ieee1394: sbp2: s/g list access cosmetics ieee1394: small cleanup after "nopage" ieee1394: remove unused code ieee1394: sbp2: raise default transfer size limit ieee1394: ohci1394: don't schedule IT tasklets on IR events firewire: fw-sbp2: refactor workq and kref handling firewire: fw-sbp2: prepare for s/g chaining firewire: fw-sbp2: remove unused misleading macro firewire: fw-ohci: CycleTooLong interrupt management firewire vs. ieee1394: clarify MAINTAINERS firewire: fw-sbp2: skip unnecessary logout firewire: fw-sbp2: try to increase reconnect_hold (speed up reconnection) firewire: fw-sbp2: use device generation, not card generation firewire: fw-cdev: use device generation, not card generation firewire: enforce access order between generation and node ID, fix "giving up on config rom" firewire: fw-core: react on bus resets while the config ROM is being fetched -- Stefan Richter -=====-==--- ---= ====- http://arcgraph.de/sr/ ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [GIT PULL] FireWire updates post 2.6.24 2008-01-30 22:53 [GIT PULL] FireWire updates post 2.6.24 Stefan Richter @ 2008-01-30 22:55 ` Stefan Richter 2008-02-02 13:05 ` [GIT PULL] IEEE 1394 regression fix Stefan Richter 1 sibling, 0 replies; 18+ messages in thread From: Stefan Richter @ 2008-01-30 22:55 UTC (permalink / raw) To: Linus Torvalds; +Cc: Andrew Morton, linux-kernel, linux1394-devel Patches recently added to linux1394-2.6.git/for-linus: Jarod Wilson (3): firewire: replace subtraction with bitwise and firewire: fw-sbp2: increase login orb reply timeout, fix "failed to login" firewire: fw-sbp2: Use sbp2 device-provided mgt orb timeout for logins Stefan Richter (6): firewire: fw-sbp2: skip unnecessary logout firewire: fw-sbp2: try to increase reconnect_hold (speed up reconnection) firewire: fw-sbp2: use device generation, not card generation firewire: fw-cdev: use device generation, not card generation firewire: enforce access order between generation and node ID, fix "giving up on config rom" firewire: fw-core: react on bus resets while the config ROM is being fetched drivers/firewire/fw-cdev.c | 3 +- drivers/firewire/fw-device.c | 38 +++++++++++++---- drivers/firewire/fw-device.h | 12 +++++ drivers/firewire/fw-sbp2.c | 63 +++++++++++++++++++++++------ drivers/firewire/fw-topology.c | 6 +++ drivers/firewire/fw-transaction.c | 2 +- 6 files changed, 100 insertions(+), 24 deletions(-) commit 384170da9384b7bb3650c0c9b9d17ba0f7bde4ff Author: Jarod Wilson <jwilson@redhat.com> Date: Fri Jan 25 23:31:12 2008 -0500 firewire: fw-sbp2: Use sbp2 device-provided mgt orb timeout for logins To be more compliant with section 7.4.8 of the SBP-2 specification, use the mgt_ORB_timeout specified in the SBP-2 device's config rom for login ORB attempts (though with some sanity checks). A happy side-effect is that certain device and controller combinations that sometimes take more than 20 seconds to get synced up (like my laptop with just about any SBP-2 device) now function more reliably. Signed-off-by: Jarod Wilson <jwilson@redhat.com> Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de> (silenced sparse) diff --git a/drivers/firewire/fw-sbp2.c b/drivers/firewire/fw-sbp2.c index 794bade..19ece9b 100644 --- a/drivers/firewire/fw-sbp2.c +++ b/drivers/firewire/fw-sbp2.c @@ -149,15 +149,17 @@ struct sbp2_target { unsigned workarounds; struct list_head lu_list; + + unsigned int mgt_orb_timeout; }; /* * Per section 7.4.8 of the SBP-2 spec, a mgt_ORB_timeout value can be - * provided in the config rom. A high timeout value really only matters - * on initial login, where we'll just use 20s rather than hassling with - * reading the config rom, since it really wouldn't buy us much. + * provided in the config rom. Most devices do provide a value, which + * we'll use for login management orbs, but with some sane limits. */ -#define SBP2_LOGIN_ORB_TIMEOUT 20000 /* Timeout in ms */ +#define SBP2_MIN_LOGIN_ORB_TIMEOUT 5000U /* Timeout in ms */ +#define SBP2_MAX_LOGIN_ORB_TIMEOUT 40000U /* Timeout in ms */ #define SBP2_ORB_TIMEOUT 2000 /* Timeout in ms */ #define SBP2_ORB_NULL 0x80000000 #define SBP2_MAX_SG_ELEMENT_LENGTH 0xf000 @@ -166,6 +168,7 @@ struct sbp2_target { #define SBP2_DIRECTION_FROM_MEDIA 0x1 /* Unit directory keys */ +#define SBP2_CSR_UNIT_CHARACTERISTICS 0x3a #define SBP2_CSR_FIRMWARE_REVISION 0x3c #define SBP2_CSR_LOGICAL_UNIT_NUMBER 0x14 #define SBP2_CSR_LOGICAL_UNIT_DIRECTORY 0xd4 @@ -527,7 +530,7 @@ sbp2_send_management_orb(struct sbp2_logical_unit *lu, int node_id, orb->request.misc |= MANAGEMENT_ORB_RECONNECT(2) | MANAGEMENT_ORB_EXCLUSIVE(sbp2_param_exclusive_login); - timeout = SBP2_LOGIN_ORB_TIMEOUT; + timeout = lu->tgt->mgt_orb_timeout; } else { timeout = SBP2_ORB_TIMEOUT; } @@ -777,6 +780,7 @@ static int sbp2_scan_unit_dir(struct sbp2_target *tgt, u32 *directory, { struct fw_csr_iterator ci; int key, value; + unsigned int timeout; fw_csr_iterator_init(&ci, directory); while (fw_csr_iterator_next(&ci, &key, &value)) { @@ -799,6 +803,21 @@ static int sbp2_scan_unit_dir(struct sbp2_target *tgt, u32 *directory, *firmware_revision = value; break; + case SBP2_CSR_UNIT_CHARACTERISTICS: + /* the timeout value is stored in 500ms units */ + timeout = ((unsigned int) value >> 8 & 0xff) * 500; + timeout = max(timeout, SBP2_MIN_LOGIN_ORB_TIMEOUT); + tgt->mgt_orb_timeout = + min(timeout, SBP2_MAX_LOGIN_ORB_TIMEOUT); + + if (timeout > tgt->mgt_orb_timeout) + fw_notify("%s: config rom contains %ds " + "management ORB timeout, limiting " + "to %ds\n", tgt->unit->device.bus_id, + timeout / 1000, + tgt->mgt_orb_timeout / 1000); + break; + case SBP2_CSR_LOGICAL_UNIT_NUMBER: if (sbp2_add_logical_unit(tgt, value) < 0) return -ENOMEM; commit a4c379c1979fbc417099cd22ba16735bc3625bbf Author: Jarod Wilson <jwilson@redhat.com> Date: Sat Jan 19 13:15:05 2008 +0100 firewire: fw-sbp2: increase login orb reply timeout, fix "failed to login" Increase (and rename) the login orb reply timeout value to 20s to match that of the old firewire stack. 2s simply didn't give many devices enough time to spin up and reply. Fixes inability to recognize some devices. Failure mode was "orb reply timed out"/"failed to login". Signed-off-by: Jarod Wilson <jwilson@redhat.com> Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de> (style, comments, changelog) diff --git a/drivers/firewire/fw-sbp2.c b/drivers/firewire/fw-sbp2.c index 705a20c..794bade 100644 --- a/drivers/firewire/fw-sbp2.c +++ b/drivers/firewire/fw-sbp2.c @@ -151,9 +151,16 @@ struct sbp2_target { struct list_head lu_list; }; -#define SBP2_MAX_SG_ELEMENT_LENGTH 0xf000 +/* + * Per section 7.4.8 of the SBP-2 spec, a mgt_ORB_timeout value can be + * provided in the config rom. A high timeout value really only matters + * on initial login, where we'll just use 20s rather than hassling with + * reading the config rom, since it really wouldn't buy us much. + */ +#define SBP2_LOGIN_ORB_TIMEOUT 20000 /* Timeout in ms */ #define SBP2_ORB_TIMEOUT 2000 /* Timeout in ms */ #define SBP2_ORB_NULL 0x80000000 +#define SBP2_MAX_SG_ELEMENT_LENGTH 0xf000 #define SBP2_DIRECTION_TO_MEDIA 0x0 #define SBP2_DIRECTION_FROM_MEDIA 0x1 @@ -488,6 +495,7 @@ sbp2_send_management_orb(struct sbp2_logical_unit *lu, int node_id, { struct fw_device *device = fw_device(lu->tgt->unit->device.parent); struct sbp2_management_orb *orb; + unsigned int timeout; int retval = -ENOMEM; orb = kzalloc(sizeof(*orb), GFP_ATOMIC); @@ -519,6 +527,9 @@ sbp2_send_management_orb(struct sbp2_logical_unit *lu, int node_id, orb->request.misc |= MANAGEMENT_ORB_RECONNECT(2) | MANAGEMENT_ORB_EXCLUSIVE(sbp2_param_exclusive_login); + timeout = SBP2_LOGIN_ORB_TIMEOUT; + } else { + timeout = SBP2_ORB_TIMEOUT; } fw_memcpy_to_be32(&orb->request, &orb->request, sizeof(orb->request)); @@ -535,8 +546,7 @@ sbp2_send_management_orb(struct sbp2_logical_unit *lu, int node_id, sbp2_send_orb(&orb->base, lu, node_id, generation, lu->tgt->management_agent_address); - wait_for_completion_timeout(&orb->done, - msecs_to_jiffies(SBP2_ORB_TIMEOUT)); + wait_for_completion_timeout(&orb->done, msecs_to_jiffies(timeout)); retval = -EIO; if (sbp2_cancel_orbs(lu) == 0) { commit 8f9f963e5d9853dbc5fa5091f15ae64f423d3d89 Author: Jarod Wilson <jwilson@redhat.com> Date: Wed Jan 23 16:05:45 2008 -0500 firewire: replace subtraction with bitwise and Replace an unnecessary subtraction with a bitwise AND when determining the value of ext_tcode in fw_fill_transaction() to save a cpu cycle or two in a somewhat critical path. Signed-off-by: Jarod Wilson <jwilson@redhat.com> Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de> diff --git a/drivers/firewire/fw-transaction.c b/drivers/firewire/fw-transaction.c index 8018c3b..7fcc59d 100644 --- a/drivers/firewire/fw-transaction.c +++ b/drivers/firewire/fw-transaction.c @@ -153,7 +153,7 @@ fw_fill_request(struct fw_packet *packet, int tcode, int tlabel, int ext_tcode; if (tcode > 0x10) { - ext_tcode = tcode - 0x10; + ext_tcode = tcode & ~0x10; tcode = TCODE_LOCK_REQUEST; } else ext_tcode = 0; commit f8d2dc39389d6ccc0def290dc4b7eb71d68645a2 Author: Stefan Richter <stefanr@s5r6.in-berlin.de> Date: Fri Jan 25 17:53:49 2008 +0100 firewire: fw-core: react on bus resets while the config ROM is being fetched read_rom() obtained a fresh new fw_device.generation for each read transaction. Hence it was able to continue reading in the middle of the ROM even if a bus reset happened. However the device may have modified the ROM during the reset. We would end up with a corrupt fetched ROM image then. Although all of this is quite unlikely, it is not impossible. Therefore we now restart reading the ROM if the bus generation changed. Note, the memory barrier in read_rom() is still necessary according to tests by Jarod Wilson, despite of the ->generation access being moved up in the call chain. Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de> This is essentially what I've been beating on locally, and I've yet to hit another config rom read failure with it. Signed-off-by: Jarod Wilson <jwilson@redhat.com> diff --git a/drivers/firewire/fw-device.c b/drivers/firewire/fw-device.c index 872df22..de9066e 100644 --- a/drivers/firewire/fw-device.c +++ b/drivers/firewire/fw-device.c @@ -390,12 +390,12 @@ complete_transaction(struct fw_card *card, int rcode, complete(&callback_data->done); } -static int read_rom(struct fw_device *device, int index, u32 * data) +static int +read_rom(struct fw_device *device, int generation, int index, u32 *data) { struct read_quadlet_callback_data callback_data; struct fw_transaction t; u64 offset; - int generation = device->generation; /* device->node_id, accessed below, must not be older than generation */ smp_rmb(); @@ -414,7 +414,14 @@ static int read_rom(struct fw_device *device, int index, u32 * data) return callback_data.rcode; } -static int read_bus_info_block(struct fw_device *device) +/* + * Read the bus info block, perform a speed probe, and read all of the rest of + * the config ROM. We do all this with a cached bus generation. If the bus + * generation changes under us, read_bus_info_block will fail and get retried. + * It's better to start all over in this case because the node from which we + * are reading the ROM may have changed the ROM during the reset. + */ +static int read_bus_info_block(struct fw_device *device, int generation) { static u32 rom[256]; u32 stack[16], sp, key; @@ -424,7 +431,7 @@ static int read_bus_info_block(struct fw_device *device) /* First read the bus info block. */ for (i = 0; i < 5; i++) { - if (read_rom(device, i, &rom[i]) != RCODE_COMPLETE) + if (read_rom(device, generation, i, &rom[i]) != RCODE_COMPLETE) return -1; /* * As per IEEE1212 7.2, during power-up, devices can @@ -459,7 +466,8 @@ static int read_bus_info_block(struct fw_device *device) device->max_speed = device->card->link_speed; while (device->max_speed > SCODE_100) { - if (read_rom(device, 0, &dummy) == RCODE_COMPLETE) + if (read_rom(device, generation, 0, &dummy) == + RCODE_COMPLETE) break; device->max_speed--; } @@ -492,7 +500,7 @@ static int read_bus_info_block(struct fw_device *device) return -1; /* Read header quadlet for the block to get the length. */ - if (read_rom(device, i, &rom[i]) != RCODE_COMPLETE) + if (read_rom(device, generation, i, &rom[i]) != RCODE_COMPLETE) return -1; end = i + (rom[i] >> 16) + 1; i++; @@ -511,7 +519,8 @@ static int read_bus_info_block(struct fw_device *device) * it references another block, and push it in that case. */ while (i < end) { - if (read_rom(device, i, &rom[i]) != RCODE_COMPLETE) + if (read_rom(device, generation, i, &rom[i]) != + RCODE_COMPLETE) return -1; if ((key >> 30) == 3 && (rom[i] >> 30) > 1 && sp < ARRAY_SIZE(stack)) @@ -658,7 +667,7 @@ static void fw_device_init(struct work_struct *work) * device. */ - if (read_bus_info_block(device) < 0) { + if (read_bus_info_block(device, device->generation) < 0) { if (device->config_rom_retries < MAX_RETRIES) { device->config_rom_retries++; schedule_delayed_work(&device->work, RETRY_DELAY); commit b5d2a5e04e6a26cb3f77af8cbc31e74c361d706c Author: Stefan Richter <stefanr@s5r6.in-berlin.de> Date: Fri Jan 25 18:57:41 2008 +0100 firewire: enforce access order between generation and node ID, fix "giving up on config rom" fw_device.node_id and fw_device.generation are accessed without mutexes. We have to ensure that all readers will get to see node_id updates before generation updates. Fixes an inability to recognize devices after "giving up on config rom", https://bugzilla.redhat.com/show_bug.cgi?id=429950 Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de> Reviewed by Nick Piggin <nickpiggin@yahoo.com.au>. Verified to fix 'giving up on config rom' issues on multiple system and drive combinations that were previously affected. Signed-off-by: Jarod Wilson <jwilson@redhat.com> Signed-off-by: Kristian Høgsberg <krh@redhat.com> diff --git a/drivers/firewire/fw-cdev.c b/drivers/firewire/fw-cdev.c index cea8a79..7e73cba 100644 --- a/drivers/firewire/fw-cdev.c +++ b/drivers/firewire/fw-cdev.c @@ -207,6 +207,7 @@ fill_bus_reset_event(struct fw_cdev_event_bus_reset *event, event->closure = client->bus_reset_closure; event->type = FW_CDEV_EVENT_BUS_RESET; event->generation = client->device->generation; + smp_rmb(); /* node_id must not be older than generation */ event->node_id = client->device->node_id; event->local_node_id = card->local_node->node_id; event->bm_node_id = 0; /* FIXME: We don't track the BM. */ diff --git a/drivers/firewire/fw-device.c b/drivers/firewire/fw-device.c index 56681b3..872df22 100644 --- a/drivers/firewire/fw-device.c +++ b/drivers/firewire/fw-device.c @@ -27,6 +27,7 @@ #include <linux/idr.h> #include <linux/rwsem.h> #include <asm/semaphore.h> +#include <asm/system.h> #include <linux/ctype.h> #include "fw-transaction.h" #include "fw-topology.h" @@ -182,9 +183,14 @@ static void fw_device_release(struct device *dev) int fw_device_enable_phys_dma(struct fw_device *device) { + int generation = device->generation; + + /* device->node_id, accessed below, must not be older than generation */ + smp_rmb(); + return device->card->driver->enable_phys_dma(device->card, device->node_id, - device->generation); + generation); } EXPORT_SYMBOL(fw_device_enable_phys_dma); @@ -389,12 +395,16 @@ static int read_rom(struct fw_device *device, int index, u32 * data) struct read_quadlet_callback_data callback_data; struct fw_transaction t; u64 offset; + int generation = device->generation; + + /* device->node_id, accessed below, must not be older than generation */ + smp_rmb(); init_completion(&callback_data.done); offset = 0xfffff0000400ULL + index * 4; fw_send_request(device->card, &t, TCODE_READ_QUADLET_REQUEST, - device->node_id, device->generation, device->max_speed, + device->node_id, generation, device->max_speed, offset, NULL, 4, complete_transaction, &callback_data); wait_for_completion(&callback_data.done); @@ -801,6 +811,7 @@ void fw_node_event(struct fw_card *card, struct fw_node *node, int event) device = node->data; device->node_id = node->node_id; + smp_wmb(); /* update node_id before generation */ device->generation = card->generation; if (atomic_read(&device->state) == FW_DEVICE_RUNNING) { PREPARE_DELAYED_WORK(&device->work, fw_device_update); diff --git a/drivers/firewire/fw-device.h b/drivers/firewire/fw-device.h index 894d4a9..0854fe2 100644 --- a/drivers/firewire/fw-device.h +++ b/drivers/firewire/fw-device.h @@ -35,6 +35,18 @@ struct fw_attribute_group { struct attribute *attrs[11]; }; +/* + * Note, fw_device.generation always has to be read before fw_device.node_id. + * Use SMP memory barriers to ensure this. Otherwise requests will be sent + * to an outdated node_id if the generation was updated in the meantime due + * to a bus reset. + * + * Likewise, fw-core will take care to update .node_id before .generation so + * that whenever fw_device.generation is current WRT the actual bus generation, + * fw_device.node_id is guaranteed to be current too. + * + * The same applies to fw_device.card->node_id vs. fw_device.generation. + */ struct fw_device { atomic_t state; struct fw_node *node; diff --git a/drivers/firewire/fw-sbp2.c b/drivers/firewire/fw-sbp2.c index d406c34..705a20c 100644 --- a/drivers/firewire/fw-sbp2.c +++ b/drivers/firewire/fw-sbp2.c @@ -40,6 +40,7 @@ #include <linux/stringify.h> #include <linux/timer.h> #include <linux/workqueue.h> +#include <asm/system.h> #include <scsi/scsi.h> #include <scsi/scsi_cmnd.h> @@ -662,6 +663,7 @@ static void sbp2_login(struct work_struct *work) int generation, node_id, local_node_id; generation = device->generation; + smp_rmb(); /* node_id must not be older than generation */ node_id = device->node_id; local_node_id = device->card->node_id; @@ -912,6 +914,7 @@ static void sbp2_reconnect(struct work_struct *work) int generation, node_id, local_node_id; generation = device->generation; + smp_rmb(); /* node_id must not be older than generation */ node_id = device->node_id; local_node_id = device->card->node_id; diff --git a/drivers/firewire/fw-topology.c b/drivers/firewire/fw-topology.c index 0fc9b00..172c186 100644 --- a/drivers/firewire/fw-topology.c +++ b/drivers/firewire/fw-topology.c @@ -21,6 +21,7 @@ #include <linux/module.h> #include <linux/wait.h> #include <linux/errno.h> +#include <asm/system.h> #include "fw-transaction.h" #include "fw-topology.h" @@ -518,6 +519,11 @@ fw_core_handle_bus_reset(struct fw_card *card, card->bm_retries = 0; card->node_id = node_id; + /* + * Update node_id before generation to prevent anybody from using + * a stale node_id together with a current generation. + */ + smp_wmb(); card->generation = generation; card->reset_jiffies = jiffies; schedule_delayed_work(&card->work, 0); commit cf5a56ac8083dd04ffe8b9b2ec7895e9bcff44bc Author: Stefan Richter <stefanr@s5r6.in-berlin.de> Date: Thu Jan 24 01:53:51 2008 +0100 firewire: fw-cdev: use device generation, not card generation We have to use the fw_device.generation here, not the fw_card.generation, because the generation must never be newer than the node ID when we emit a transaction. This cannot be guaranteed with fw_card.generation. Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de> Verified in concert with subsequent memory barriers patch to fix 'giving up on config rom' issues on multiple system and drive combinations that were previously affected. Signed-off-by: Jarod Wilson <jwilson@redhat.com> diff --git a/drivers/firewire/fw-cdev.c b/drivers/firewire/fw-cdev.c index 60f1a89..cea8a79 100644 --- a/drivers/firewire/fw-cdev.c +++ b/drivers/firewire/fw-cdev.c @@ -206,12 +206,12 @@ fill_bus_reset_event(struct fw_cdev_event_bus_reset *event, event->closure = client->bus_reset_closure; event->type = FW_CDEV_EVENT_BUS_RESET; + event->generation = client->device->generation; event->node_id = client->device->node_id; event->local_node_id = card->local_node->node_id; event->bm_node_id = 0; /* FIXME: We don't track the BM. */ event->irm_node_id = card->irm_node->node_id; event->root_node_id = card->root_node->node_id; - event->generation = card->generation; } static void commit 5a8a1bcd15dfb9f177f3605fe6b9ba2bef2bf55a Author: Stefan Richter <stefanr@s5r6.in-berlin.de> Date: Thu Jan 24 01:53:19 2008 +0100 firewire: fw-sbp2: use device generation, not card generation There was a small window where a login or reconnect job could use an already updated card generation with an outdated node ID. We have to use the fw_device.generation here, not the fw_card.generation, because the generation must never be newer than the node ID when we emit a transaction. This cannot be guaranteed with fw_card.generation. Furthermore, the target's and initiator's node IDs can be obtained from fw_device and fw_card. Dereferencing their underlying topology objects is not necessary. Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de> Verified in concert with subsequent memory barriers patch to fix 'giving up on config rom' issues on multiple system and drive combinations that were previously affected. Signed-off-by: Jarod Wilson <jwilson@redhat.com> diff --git a/drivers/firewire/fw-sbp2.c b/drivers/firewire/fw-sbp2.c index d2fbfc6..d406c34 100644 --- a/drivers/firewire/fw-sbp2.c +++ b/drivers/firewire/fw-sbp2.c @@ -661,9 +661,9 @@ static void sbp2_login(struct work_struct *work) struct sbp2_login_response response; int generation, node_id, local_node_id; - generation = device->card->generation; - node_id = device->node->node_id; - local_node_id = device->card->local_node->node_id; + generation = device->generation; + node_id = device->node_id; + local_node_id = device->card->node_id; if (sbp2_send_management_orb(lu, node_id, generation, SBP2_LOGIN_REQUEST, lu->lun, &response) < 0) { @@ -911,9 +911,9 @@ static void sbp2_reconnect(struct work_struct *work) struct fw_device *device = fw_device(unit->device.parent); int generation, node_id, local_node_id; - generation = device->card->generation; - node_id = device->node->node_id; - local_node_id = device->card->local_node->node_id; + generation = device->generation; + node_id = device->node_id; + local_node_id = device->card->node_id; if (sbp2_send_management_orb(lu, node_id, generation, SBP2_RECONNECT_REQUEST, commit 14dc992aa782f8759c6d117d4322db62f62600ce Author: Stefan Richter <stefanr@s5r6.in-berlin.de> Date: Sun Jan 20 01:25:31 2008 +0100 firewire: fw-sbp2: try to increase reconnect_hold (speed up reconnection) Ask the target to grant 4 seconds instead of the standard and minimum of 1 second window after bus reset for reconnection. This accelerates reconnection if there are more than one targets on the bus: If a login and inquiry to one target blocks the fw-sbp2 workqueue for more than 1s after bus reset, we now still can reconnect to the other target. Before that, fw-sbp2's reconnect attempts would be rejected with "error status: 0:9" (function rejected), and fw-sbp2 would finally re-login. All those futile reconnect attemps cost extra time until the target which needs re-login is ready for I/O again. The reconnect timeout field in the login ORB doesn't have to be honored by the target though. I found that we could get up to - allegedly 32768s from an old OXFW911 firmware - 256s from LSI bridges - 4s from OXUF922 and OXFW912 bridges, - 2s from TI bridges, - only the standard 1s from Initio and Prolific bridges and from Apple OpenFirmware in target mode. We just try to get 4 seconds which already covers the case of a few HDDs on the same bus quite nicely. A minor drawback occurs in the following (rare and impractical) border case: - two initiators are there, initiator 1 holds an exclusive login to a target, - initiator 1 goes off the bus, - target refuses login attempts from initiator 2 until reconnect_hold seconds after bus reset. An alternative approach to the issue at hand would be to parallelize fw-sbp2's reconnect and login work. Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de> Acked-by: Jarod Wilson <jwilson@redhat.com> diff --git a/drivers/firewire/fw-sbp2.c b/drivers/firewire/fw-sbp2.c index 661a5b6..d2fbfc6 100644 --- a/drivers/firewire/fw-sbp2.c +++ b/drivers/firewire/fw-sbp2.c @@ -514,9 +514,10 @@ sbp2_send_management_orb(struct sbp2_logical_unit *lu, int node_id, orb->request.status_fifo.low = lu->address_handler.offset; if (function == SBP2_LOGIN_REQUEST) { + /* Ask for 2^2 == 4 seconds reconnect grace period */ orb->request.misc |= - MANAGEMENT_ORB_EXCLUSIVE(sbp2_param_exclusive_login) | - MANAGEMENT_ORB_RECONNECT(0); + MANAGEMENT_ORB_RECONNECT(2) | + MANAGEMENT_ORB_EXCLUSIVE(sbp2_param_exclusive_login); } fw_memcpy_to_be32(&orb->request, &orb->request, sizeof(orb->request)); commit 4dccd020d7ca5e673d7804cc4ff80fbf58d8a37e Author: Stefan Richter <stefanr@s5r6.in-berlin.de> Date: Sun Jan 20 01:24:26 2008 +0100 firewire: fw-sbp2: skip unnecessary logout Don't attempt to send a logout ORB if the target was already unplugged or had its link switched off. If two targets are attached, this enhances the chance to quickly reconnect to the remaining target when one target is plugged out. Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de> Acked-by: Jarod Wilson <jwilson@redhat.com> diff --git a/drivers/firewire/fw-sbp2.c b/drivers/firewire/fw-sbp2.c index e5a2571..661a5b6 100644 --- a/drivers/firewire/fw-sbp2.c +++ b/drivers/firewire/fw-sbp2.c @@ -606,13 +606,17 @@ static void sbp2_release_target(struct kref *kref) struct sbp2_logical_unit *lu, *next; struct Scsi_Host *shost = container_of((void *)tgt, struct Scsi_Host, hostdata[0]); + struct fw_device *device = fw_device(tgt->unit->device.parent); list_for_each_entry_safe(lu, next, &tgt->lu_list, link) { if (lu->sdev) scsi_remove_device(lu->sdev); - sbp2_send_management_orb(lu, tgt->node_id, lu->generation, - SBP2_LOGOUT_REQUEST, lu->login_id, NULL); + if (!fw_device_is_shutdown(device)) + sbp2_send_management_orb(lu, tgt->node_id, + lu->generation, SBP2_LOGOUT_REQUEST, + lu->login_id, NULL); + fw_core_remove_address_handler(&lu->address_handler); list_del(&lu->link); kfree(lu); -- Stefan Richter -=====-==--- ---= ====- http://arcgraph.de/sr/ ^ permalink raw reply [flat|nested] 18+ messages in thread
* [GIT PULL] IEEE 1394 regression fix 2008-01-30 22:53 [GIT PULL] FireWire updates post 2.6.24 Stefan Richter 2008-01-30 22:55 ` Stefan Richter @ 2008-02-02 13:05 ` Stefan Richter 2008-02-25 17:58 ` [GIT PULL] FireWire updates Stefan Richter 1 sibling, 1 reply; 18+ messages in thread From: Stefan Richter @ 2008-02-02 13:05 UTC (permalink / raw) To: Linus Torvalds; +Cc: Andrew Morton, linux-kernel, linux1394-devel Linus, please pull from the for-linus branch at git://git.kernel.org/pub/scm/linux/kernel/git/ieee1394/linux1394-2.6.git for-linus to receive the following IEEE 1394/ FireWire subsystem update. It's telling. I didn't run-time test the ieee1394 part of the post 2.6.24 patch queue on anything but i386... Stefan Richter (1): ieee1394: sbp2: fix bogus s/g access change drivers/ieee1394/sbp2.c | 5 ++--- 1 files changed, 2 insertions(+), 3 deletions(-) commit 5fcf500058d5f06720302c5ce138c7bca93f7655 Author: Stefan Richter <stefanr@s5r6.in-berlin.de> Date: Fri Feb 1 22:31:10 2008 +0100 ieee1394: sbp2: fix bogus s/g access change sg_dma_len(sg) is invalid before the s/g list is DMA-mapped. This fixes a post 2.6.24 regression which prevents access to SBP-2 devices on several architectures, introduced by "ieee1394: sbp2: s/g list access cosmetics", commit 825f1df545ab0289185373b0eaf06fb0b3487422. Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de> diff --git a/drivers/ieee1394/sbp2.c b/drivers/ieee1394/sbp2.c index 2b889d9..28e155a 100644 --- a/drivers/ieee1394/sbp2.c +++ b/drivers/ieee1394/sbp2.c @@ -1465,10 +1465,9 @@ static void sbp2_prep_command_orb_sg(struct sbp2_command_orb *orb, orb->misc |= ORB_SET_DIRECTION(orb_direction); /* special case if only one element (and less than 64KB in size) */ - if ((scsi_use_sg == 1) && - (sg_dma_len(sg) <= SBP2_MAX_SG_ELEMENT_LENGTH)) { + if (scsi_use_sg == 1 && sg->length <= SBP2_MAX_SG_ELEMENT_LENGTH) { - cmd->dma_size = sg_dma_len(sg); + cmd->dma_size = sg->length; cmd->dma_type = CMD_DMA_PAGE; cmd->cmd_dma = dma_map_page(hi->host->device.parent, sg_page(sg), sg->offset, -- Stefan Richter -=====-==--- --=- ---=- http://arcgraph.de/sr/ ^ permalink raw reply [flat|nested] 18+ messages in thread
* [GIT PULL] FireWire updates 2008-02-02 13:05 ` [GIT PULL] IEEE 1394 regression fix Stefan Richter @ 2008-02-25 17:58 ` Stefan Richter 2008-02-25 18:00 ` Stefan Richter 2008-03-02 12:47 ` Stefan Richter 0 siblings, 2 replies; 18+ messages in thread From: Stefan Richter @ 2008-02-25 17:58 UTC (permalink / raw) To: Linus Torvalds, Andrew Morton; +Cc: linux-kernel, linux1394-devel Linus, please pull from the for-linus branch at git://git.kernel.org/pub/scm/linux/kernel/git/ieee1394/linux1394-2.6.git for-linus to receive the following updates for the firewire and the ieee1394 subsystems. This queue consists of fixes developed between end of January and February 20. Therefore several of these updates were not yet visible in -mm. But all of them showed up in -next (behind planned post 2.6.25 stuff), most of them AFAIK also in Fedora test kernels. The queue improves the SBP-2 driver's reliability during device recognition and during bus topology changes. All patches have been seen on LKML and linux1394-devel but I will follow up with the combined full log and diff. Diffstat and shortlog: Documentation/debugging-via-ohci1394.txt | 17 +- drivers/firewire/fw-cdev.c | 17 +- drivers/firewire/fw-device.c | 48 ++- drivers/firewire/fw-device.h | 2 +- drivers/firewire/fw-sbp2.c | 358 +++++++++++++++++----- drivers/ieee1394/sbp2.c | 15 + drivers/ieee1394/sbp2.h | 2 + 7 files changed, 350 insertions(+), 109 deletions(-) Stefan Richter (19): firewire: fw-sbp2: unsigned int vs. unsigned firewire: fw-sbp2: fix logout before login retry firewire: fix "kobject_add failed for fw* with -EEXIST" firewire: fw-sbp2: don't retry login or reconnect after unplug firewire: log GUID of new devices firewire: fw-sbp2: add INQUIRY delay workaround ieee1394: sbp2: add INQUIRY delay workaround firewire: fw-sbp2: wait for completion of fetch agent reset firewire: fw-sbp2: log bus_id at management request failures firewire: fw-sbp2: don't add scsi_device twice firewire: fw-sbp2: logout and login after failed reconnect firewire: fw-sbp2: sort includes firewire: fw-sbp2: enforce a retry of __scsi_add_device if bus generation changed firewire: fw-sbp2: (try to) avoid I/O errors during reconnect firewire: fw-sbp2: fix NULL pointer deref. in slave_alloc firewire: fw-sbp2: fix NULL pointer deref. in scsi_remove_device ieee1394: sbp2: fix rescan-scsi-bus Documentation: correction to debugging-via-ohci1394 firewire: fix NULL pointer deref. and resource leak Thanks, -- Stefan Richter -=====-==--- --=- ==--= http://arcgraph.de/sr/ ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [GIT PULL] FireWire updates 2008-02-25 17:58 ` [GIT PULL] FireWire updates Stefan Richter @ 2008-02-25 18:00 ` Stefan Richter 2008-03-02 12:47 ` Stefan Richter 1 sibling, 0 replies; 18+ messages in thread From: Stefan Richter @ 2008-02-25 18:00 UTC (permalink / raw) To: Linus Torvalds; +Cc: Andrew Morton, linux-kernel, linux1394-devel I wrote: > Linus, please pull from the for-linus branch at > > git://git.kernel.org/pub/scm/linux/kernel/git/ieee1394/linux1394-2.6.git for-linus > > to receive the following updates for the firewire and the ieee1394 > subsystems. ... > Documentation/debugging-via-ohci1394.txt | 17 +- > drivers/firewire/fw-cdev.c | 17 +- > drivers/firewire/fw-device.c | 48 ++- > drivers/firewire/fw-device.h | 2 +- > drivers/firewire/fw-sbp2.c | 358 +++++++++++++++++----- > drivers/ieee1394/sbp2.c | 15 + > drivers/ieee1394/sbp2.h | 2 + > 7 files changed, 350 insertions(+), 109 deletions(-) > > Stefan Richter (19): > firewire: fw-sbp2: unsigned int vs. unsigned > firewire: fw-sbp2: fix logout before login retry > firewire: fix "kobject_add failed for fw* with -EEXIST" > firewire: fw-sbp2: don't retry login or reconnect after unplug > firewire: log GUID of new devices > firewire: fw-sbp2: add INQUIRY delay workaround > ieee1394: sbp2: add INQUIRY delay workaround > firewire: fw-sbp2: wait for completion of fetch agent reset > firewire: fw-sbp2: log bus_id at management request failures > firewire: fw-sbp2: don't add scsi_device twice > firewire: fw-sbp2: logout and login after failed reconnect > firewire: fw-sbp2: sort includes > firewire: fw-sbp2: enforce a retry of __scsi_add_device if bus generation changed > firewire: fw-sbp2: (try to) avoid I/O errors during reconnect > firewire: fw-sbp2: fix NULL pointer deref. in slave_alloc > firewire: fw-sbp2: fix NULL pointer deref. in scsi_remove_device > ieee1394: sbp2: fix rescan-scsi-bus > Documentation: correction to debugging-via-ohci1394 > firewire: fix NULL pointer deref. and resource leak commit fae603121428ba83b7343c88e68a7144525ab3eb Author: Stefan Richter <stefanr@s5r6.in-berlin.de> Date: Wed Feb 20 21:10:06 2008 +0100 firewire: fix NULL pointer deref. and resource leak By supplying ioctl()s in the wrong order, a userspace client was able to trigger NULL pointer dereferences. Furthermore, by calling ioctl_create_iso_context more than once, new contexts could be created without ever freeing the previously created contexts. Thanks to Anders Blomdell for the report. Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de> diff --git a/drivers/firewire/fw-cdev.c b/drivers/firewire/fw-cdev.c index 44ccee2..46bc197 100644 --- a/drivers/firewire/fw-cdev.c +++ b/drivers/firewire/fw-cdev.c @@ -646,6 +646,10 @@ static int ioctl_create_iso_context(struct client *client, void *buffer) struct fw_cdev_create_iso_context *request = buffer; struct fw_iso_context *context; + /* We only support one context at this time. */ + if (client->iso_context != NULL) + return -EBUSY; + if (request->channel > 63) return -EINVAL; @@ -792,8 +796,9 @@ static int ioctl_start_iso(struct client *client, void *buffer) { struct fw_cdev_start_iso *request = buffer; - if (request->handle != 0) + if (client->iso_context == NULL || request->handle != 0) return -EINVAL; + if (client->iso_context->type == FW_ISO_CONTEXT_RECEIVE) { if (request->tags == 0 || request->tags > 15) return -EINVAL; @@ -810,7 +815,7 @@ static int ioctl_stop_iso(struct client *client, void *buffer) { struct fw_cdev_stop_iso *request = buffer; - if (request->handle != 0) + if (client->iso_context == NULL || request->handle != 0) return -EINVAL; return fw_iso_context_stop(client->iso_context); commit 09d7328e62e3b4cefe4bf3eeeeacb54f62a7ae5c Author: Stefan Richter <stefanr@s5r6.in-berlin.de> Date: Mon Feb 18 21:38:35 2008 +0100 Documentation: correction to debugging-via-ohci1394 Rectify a factoid about firewire-ohci. Acked-by: Ingo Molnar <mingo@elte.hu> Also fix a typo spotted by Bernhard Kaindl. Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de> diff --git a/Documentation/debugging-via-ohci1394.txt b/Documentation/debugging-via-ohci1394.txt index de4804e..c360d4e 100644 --- a/Documentation/debugging-via-ohci1394.txt +++ b/Documentation/debugging-via-ohci1394.txt @@ -36,14 +36,15 @@ available (notebooks) or too slow for extensive debug information (like ACPI). Drivers ------- -The OHCI-1394 drivers in drivers/firewire and drivers/ieee1394 initialize -the OHCI-1394 controllers to a working state and can be used to enable -physical DMA. By default you only have to load the driver, and physical -DMA access will be granted to all remote nodes, but it can be turned off -when using the ohci1394 driver. - -Because these drivers depend on the PCI enumeration to be completed, an -initialization routine which can runs pretty early (long before console_init(), +The ohci1394 driver in drivers/ieee1394 initializes the OHCI-1394 controllers +to a working state and enables physical DMA by default for all remote nodes. +This can be turned off by ohci1394's module parameter phys_dma=0. + +The alternative firewire-ohci driver in drivers/firewire uses filtered physical +DMA, hence is not yet suitable for remote debugging. + +Because ohci1394 depends on the PCI enumeration to be completed, an +initialization routine which runs pretty early (long before console_init() which makes the printk buffer appear on the console can be called) was written. To activate it, enable CONFIG_PROVIDE_OHCI1394_DMA_INIT (Kernel hacking menu: commit ef774c16a744f130f27c654bf9c4806e767fc773 Author: Stefan Richter <stefanr@s5r6.in-berlin.de> Date: Sun Feb 17 14:57:10 2008 +0100 ieee1394: sbp2: fix rescan-scsi-bus rescan-scsi-bus used to add SBP-2 targets which weren't there. Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de> diff --git a/drivers/ieee1394/sbp2.c b/drivers/ieee1394/sbp2.c index accb2ad..9e2b196 100644 --- a/drivers/ieee1394/sbp2.c +++ b/drivers/ieee1394/sbp2.c @@ -1974,6 +1974,9 @@ static int sbp2scsi_slave_alloc(struct scsi_device *sdev) { struct sbp2_lu *lu = (struct sbp2_lu *)sdev->host->hostdata[0]; + if (sdev->lun != 0 || sdev->id != lu->ud->id || sdev->channel != 0) + return -ENODEV; + lu->sdev = sdev; sdev->allow_restart = 1; commit 33f1c6c3529f5f279e2e98e5cca0c5bac152153b Author: Stefan Richter <stefanr@s5r6.in-berlin.de> Date: Tue Feb 19 09:05:49 2008 +0100 firewire: fw-sbp2: fix NULL pointer deref. in scsi_remove_device Fix a kernel bug when unplugging an SBP-2 device after having its scsi_device already removed via the "delete" sysfs attribute. Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de> diff --git a/drivers/firewire/fw-sbp2.c b/drivers/firewire/fw-sbp2.c index 60ebcb5..5259491 100644 --- a/drivers/firewire/fw-sbp2.c +++ b/drivers/firewire/fw-sbp2.c @@ -762,9 +762,10 @@ static void sbp2_release_target(struct kref *kref) sbp2_unblock(tgt); list_for_each_entry_safe(lu, next, &tgt->lu_list, link) { - if (lu->sdev) + if (lu->sdev) { scsi_remove_device(lu->sdev); - + scsi_device_put(lu->sdev); + } sbp2_send_management_orb(lu, tgt->node_id, lu->generation, SBP2_LOGOUT_REQUEST, lu->login_id, NULL); @@ -886,12 +887,11 @@ static void sbp2_login(struct work_struct *work) if (IS_ERR(sdev)) goto out_logout_login; - scsi_device_put(sdev); - /* Unreported error during __scsi_add_device() */ smp_rmb(); /* get current card generation */ if (generation != device->card->generation) { scsi_remove_device(sdev); + scsi_device_put(sdev); goto out_logout_login; } commit 5513c5f6f9bd8c8ad3727130910fa288c62526a7 Author: Stefan Richter <stefanr@s5r6.in-berlin.de> Date: Sun Feb 17 14:56:19 2008 +0100 firewire: fw-sbp2: fix NULL pointer deref. in slave_alloc Fix a kernel bug when running rescan-scsi-bus while a FireWire disk is connected: http://bugzilla.kernel.org/show_bug.cgi?id=10008 Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de> diff --git a/drivers/firewire/fw-sbp2.c b/drivers/firewire/fw-sbp2.c index ea4811c..60ebcb5 100644 --- a/drivers/firewire/fw-sbp2.c +++ b/drivers/firewire/fw-sbp2.c @@ -1473,6 +1473,10 @@ static int sbp2_scsi_slave_alloc(struct scsi_device *sdev) { struct sbp2_logical_unit *lu = sdev->hostdata; + /* (Re-)Adding logical units via the SCSI stack is not supported. */ + if (!lu) + return -ENOSYS; + sdev->allow_restart = 1; /* commit 2e2705bdcb959372d54bf7f79dd9a555ec2adfb4 Author: Stefan Richter <stefanr@s5r6.in-berlin.de> Date: Sat Feb 16 16:37:28 2008 +0100 firewire: fw-sbp2: (try to) avoid I/O errors during reconnect While fw-sbp2 takes the necessary time to reconnect to a logical unit after bus reset, the SCSI core keeps sending new commands. They are all immediately completed with host busy status, and application clients or filesystems will break quickly. The SCSI device might even be taken offline: http://bugzilla.kernel.org/show_bug.cgi?id=9734 The only remedy seems to be to block the SCSI device until reconnect. Alas the SCSI core has no useful API to block only one logical unit i.e. the scsi_device, therefore we block the entire Scsi_Host. This currently corresponds to an SBP-2 target. In case of targets with multiple logical units, we need to satisfy the dependencies between logical units by carefully tracking the blocking state of the target and its units. We block all logical units of a target as soon as one of them needs to be blocked, and keep them blocked until all of them are ready to be unblocked. Furthermore, as the history of the old sbp2 driver has shown, the scsi_block_requests() API is a minefield with high potential of deadlocks. We therefore take extra measures to keep logical units unblocked during __scsi_add_device() and during shutdown. This avoids I/O errors during reconnect in many but alas not in all cases. There may still be errors after a re-login had to be performed. Also, some bridges have been seen to cease fetching management ORBs if I/O went on up until a bus reset. In these cases, all management ORBs time out after mgt_orb_timeout. The old sbp2 driver is less vulnerable or maybe not vulnerable to this, for as yet unknown reasons. Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de> diff --git a/drivers/firewire/fw-sbp2.c b/drivers/firewire/fw-sbp2.c index 6d10934..ea4811c 100644 --- a/drivers/firewire/fw-sbp2.c +++ b/drivers/firewire/fw-sbp2.c @@ -139,6 +139,7 @@ struct sbp2_logical_unit { int generation; int retries; struct delayed_work work; + bool blocked; }; /* @@ -157,6 +158,9 @@ struct sbp2_target { int address_high; unsigned int workarounds; unsigned int mgt_orb_timeout; + + int dont_block; /* counter for each logical unit */ + int blocked; /* ditto */ }; /* @@ -646,6 +650,107 @@ static void sbp2_agent_reset_no_wait(struct sbp2_logical_unit *lu) &z, sizeof(z), complete_agent_reset_write_no_wait, t); } +static void sbp2_set_generation(struct sbp2_logical_unit *lu, int generation) +{ + struct fw_card *card = fw_device(lu->tgt->unit->device.parent)->card; + unsigned long flags; + + /* serialize with comparisons of lu->generation and card->generation */ + spin_lock_irqsave(&card->lock, flags); + lu->generation = generation; + spin_unlock_irqrestore(&card->lock, flags); +} + +static inline void sbp2_allow_block(struct sbp2_logical_unit *lu) +{ + /* + * We may access dont_block without taking card->lock here: + * All callers of sbp2_allow_block() and all callers of sbp2_unblock() + * are currently serialized against each other. + * And a wrong result in sbp2_conditionally_block()'s access of + * dont_block is rather harmless, it simply misses its first chance. + */ + --lu->tgt->dont_block; +} + +/* + * Blocks lu->tgt if all of the following conditions are met: + * - Login, INQUIRY, and high-level SCSI setup of all of the target's + * logical units have been finished (indicated by dont_block == 0). + * - lu->generation is stale. + * + * Note, scsi_block_requests() must be called while holding card->lock, + * otherwise it might foil sbp2_[conditionally_]unblock()'s attempt to + * unblock the target. + */ +static void sbp2_conditionally_block(struct sbp2_logical_unit *lu) +{ + struct sbp2_target *tgt = lu->tgt; + struct fw_card *card = fw_device(tgt->unit->device.parent)->card; + struct Scsi_Host *shost = + container_of((void *)tgt, struct Scsi_Host, hostdata[0]); + unsigned long flags; + + spin_lock_irqsave(&card->lock, flags); + if (!tgt->dont_block && !lu->blocked && + lu->generation != card->generation) { + lu->blocked = true; + if (++tgt->blocked == 1) { + scsi_block_requests(shost); + fw_notify("blocked %s\n", lu->tgt->bus_id); + } + } + spin_unlock_irqrestore(&card->lock, flags); +} + +/* + * Unblocks lu->tgt as soon as all its logical units can be unblocked. + * Note, it is harmless to run scsi_unblock_requests() outside the + * card->lock protected section. On the other hand, running it inside + * the section might clash with shost->host_lock. + */ +static void sbp2_conditionally_unblock(struct sbp2_logical_unit *lu) +{ + struct sbp2_target *tgt = lu->tgt; + struct fw_card *card = fw_device(tgt->unit->device.parent)->card; + struct Scsi_Host *shost = + container_of((void *)tgt, struct Scsi_Host, hostdata[0]); + unsigned long flags; + bool unblock = false; + + spin_lock_irqsave(&card->lock, flags); + if (lu->blocked && lu->generation == card->generation) { + lu->blocked = false; + unblock = --tgt->blocked == 0; + } + spin_unlock_irqrestore(&card->lock, flags); + + if (unblock) { + scsi_unblock_requests(shost); + fw_notify("unblocked %s\n", lu->tgt->bus_id); + } +} + +/* + * Prevents future blocking of tgt and unblocks it. + * Note, it is harmless to run scsi_unblock_requests() outside the + * card->lock protected section. On the other hand, running it inside + * the section might clash with shost->host_lock. + */ +static void sbp2_unblock(struct sbp2_target *tgt) +{ + struct fw_card *card = fw_device(tgt->unit->device.parent)->card; + struct Scsi_Host *shost = + container_of((void *)tgt, struct Scsi_Host, hostdata[0]); + unsigned long flags; + + spin_lock_irqsave(&card->lock, flags); + ++tgt->dont_block; + spin_unlock_irqrestore(&card->lock, flags); + + scsi_unblock_requests(shost); +} + static void sbp2_release_target(struct kref *kref) { struct sbp2_target *tgt = container_of(kref, struct sbp2_target, kref); @@ -653,6 +758,9 @@ static void sbp2_release_target(struct kref *kref) struct Scsi_Host *shost = container_of((void *)tgt, struct Scsi_Host, hostdata[0]); + /* prevent deadlocks */ + sbp2_unblock(tgt); + list_for_each_entry_safe(lu, next, &tgt->lu_list, link) { if (lu->sdev) scsi_remove_device(lu->sdev); @@ -717,17 +825,20 @@ static void sbp2_login(struct work_struct *work) if (sbp2_send_management_orb(lu, node_id, generation, SBP2_LOGIN_REQUEST, lu->lun, &response) < 0) { - if (lu->retries++ < 5) + if (lu->retries++ < 5) { sbp2_queue_work(lu, DIV_ROUND_UP(HZ, 5)); - else + } else { fw_error("%s: failed to login to LUN %04x\n", tgt->bus_id, lu->lun); + /* Let any waiting I/O fail from now on. */ + sbp2_unblock(lu->tgt); + } goto out; } - lu->generation = generation; tgt->node_id = node_id; tgt->address_high = local_node_id << 16; + sbp2_set_generation(lu, generation); /* Get command block agent offset and login id. */ lu->command_block_agent_address = @@ -749,6 +860,7 @@ static void sbp2_login(struct work_struct *work) /* This was a re-login. */ if (lu->sdev) { sbp2_cancel_orbs(lu); + sbp2_conditionally_unblock(lu); goto out; } @@ -785,6 +897,7 @@ static void sbp2_login(struct work_struct *work) /* No error during __scsi_add_device() */ lu->sdev = sdev; + sbp2_allow_block(lu); goto out; out_logout_login: @@ -825,6 +938,8 @@ static int sbp2_add_logical_unit(struct sbp2_target *tgt, int lun_entry) lu->sdev = NULL; lu->lun = lun_entry & 0xffff; lu->retries = 0; + lu->blocked = false; + ++tgt->dont_block; INIT_LIST_HEAD(&lu->orb_list); INIT_DELAYED_WORK(&lu->work, sbp2_login); @@ -1041,15 +1156,16 @@ static void sbp2_reconnect(struct work_struct *work) goto out; } - lu->generation = generation; tgt->node_id = node_id; tgt->address_high = local_node_id << 16; + sbp2_set_generation(lu, generation); fw_notify("%s: reconnected to LUN %04x (%d retries)\n", tgt->bus_id, lu->lun, lu->retries); sbp2_agent_reset(lu); sbp2_cancel_orbs(lu); + sbp2_conditionally_unblock(lu); out: sbp2_target_put(tgt); } @@ -1066,6 +1182,7 @@ static void sbp2_update(struct fw_unit *unit) * Iteration over tgt->lu_list is therefore safe here. */ list_for_each_entry(lu, &tgt->lu_list, link) { + sbp2_conditionally_block(lu); lu->retries = 0; sbp2_queue_work(lu, 0); } @@ -1169,6 +1286,7 @@ complete_command_orb(struct sbp2_orb *base_orb, struct sbp2_status *status) * or when sending the write (less likely). */ result = DID_BUS_BUSY << 16; + sbp2_conditionally_block(orb->lu); } dma_unmap_single(device->card->device, orb->base.request_bus, commit e80de3704ac30ddb7f9a12447a2ecee32ccd7880 Author: Stefan Richter <stefanr@s5r6.in-berlin.de> Date: Fri Feb 15 21:29:02 2008 +0100 firewire: fw-sbp2: enforce a retry of __scsi_add_device if bus generation changed fw-sbp2 is unable to reconnect while performing __scsi_add_device because there is only a single workqueue thread context available for both at the moment. This should be fixed eventually. An actual failure of __scsi_add_device is easy to handle, but an incomplete execution of __scsi_add_device with an sdev returned would remain undetected and leave the SBP-2 target unusable. Therefore we use a workaround: If there was a bus reset during __scsi_add_device (i.e. during the SCSI probe), we remove the new sdev immediately, log out, and attempt login and SCSI probe again. Tested-by: Jarod Wilson <jwilson@redhat.com> (earlier version) Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de> diff --git a/drivers/firewire/fw-sbp2.c b/drivers/firewire/fw-sbp2.c index 323b03b..6d10934 100644 --- a/drivers/firewire/fw-sbp2.c +++ b/drivers/firewire/fw-sbp2.c @@ -762,22 +762,43 @@ static void sbp2_login(struct work_struct *work) sdev = __scsi_add_device(shost, 0, 0, scsilun_to_int(&eight_bytes_lun), lu); - if (IS_ERR(sdev)) { - smp_rmb(); /* generation may have changed */ - generation = device->generation; - smp_rmb(); /* node_id must not be older than generation */ + /* + * FIXME: We are unable to perform reconnects while in sbp2_login(). + * Therefore __scsi_add_device() will get into trouble if a bus reset + * happens in parallel. It will either fail or leave us with an + * unusable sdev. As a workaround we check for this and retry the + * whole login and SCSI probing. + */ - sbp2_send_management_orb(lu, device->node_id, generation, - SBP2_LOGOUT_REQUEST, lu->login_id, NULL); - /* - * Set this back to sbp2_login so we fall back and - * retry login on bus reset. - */ - PREPARE_DELAYED_WORK(&lu->work, sbp2_login); - } else { - lu->sdev = sdev; - scsi_device_put(sdev); + /* Reported error during __scsi_add_device() */ + if (IS_ERR(sdev)) + goto out_logout_login; + + scsi_device_put(sdev); + + /* Unreported error during __scsi_add_device() */ + smp_rmb(); /* get current card generation */ + if (generation != device->card->generation) { + scsi_remove_device(sdev); + goto out_logout_login; } + + /* No error during __scsi_add_device() */ + lu->sdev = sdev; + goto out; + + out_logout_login: + smp_rmb(); /* generation may have changed */ + generation = device->generation; + smp_rmb(); /* node_id must not be older than generation */ + + sbp2_send_management_orb(lu, device->node_id, generation, + SBP2_LOGOUT_REQUEST, lu->login_id, NULL); + /* + * If a bus reset happened, sbp2_update will have requeued + * lu->work already. Reset the work from reconnect to login. + */ + PREPARE_DELAYED_WORK(&lu->work, sbp2_login); out: sbp2_target_put(tgt); } commit 7bb6bf7c8ba0b4ccfecaa00d6faea51b0bd42c8c Author: Stefan Richter <stefanr@s5r6.in-berlin.de> Date: Sun Feb 3 23:12:17 2008 +0100 firewire: fw-sbp2: sort includes Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de> diff --git a/drivers/firewire/fw-sbp2.c b/drivers/firewire/fw-sbp2.c index 80ab651..323b03b 100644 --- a/drivers/firewire/fw-sbp2.c +++ b/drivers/firewire/fw-sbp2.c @@ -28,15 +28,15 @@ * and many others. */ +#include <linux/blkdev.h> +#include <linux/delay.h> +#include <linux/device.h> +#include <linux/dma-mapping.h> #include <linux/kernel.h> +#include <linux/mod_devicetable.h> #include <linux/module.h> #include <linux/moduleparam.h> -#include <linux/mod_devicetable.h> -#include <linux/delay.h> -#include <linux/device.h> #include <linux/scatterlist.h> -#include <linux/dma-mapping.h> -#include <linux/blkdev.h> #include <linux/string.h> #include <linux/stringify.h> #include <linux/timer.h> @@ -48,9 +48,9 @@ #include <scsi/scsi_device.h> #include <scsi/scsi_host.h> -#include "fw-transaction.h" -#include "fw-topology.h" #include "fw-device.h" +#include "fw-topology.h" +#include "fw-transaction.h" /* * So far only bridges from Oxford Semiconductor are known to support commit ce896d95cc7886ae05859c5b409a7b2f3b606ec1 Author: Stefan Richter <stefanr@s5r6.in-berlin.de> Date: Sun Feb 3 23:11:39 2008 +0100 firewire: fw-sbp2: logout and login after failed reconnect If fw-sbp2 was too late with requesting the reconnect, the target would reject this. In this case, log out before attempting the reconnect. Else several firmwares will deny the re-login because they somehow didn't invalidate the old login. Also, don't retry reconnects in this situation. The retries won't succeed either. These changes improve chances for successful re-login and shorten the period during which the logical unit is inaccessible. Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de> Signed-off-by: Jarod Wilson <jwilson@redhat.com> diff --git a/drivers/firewire/fw-sbp2.c b/drivers/firewire/fw-sbp2.c index 914170b..80ab651 100644 --- a/drivers/firewire/fw-sbp2.c +++ b/drivers/firewire/fw-sbp2.c @@ -710,6 +710,11 @@ static void sbp2_login(struct work_struct *work) node_id = device->node_id; local_node_id = device->card->node_id; + /* If this is a re-login attempt, log out, or we might be rejected. */ + if (lu->sdev) + sbp2_send_management_orb(lu, device->node_id, generation, + SBP2_LOGOUT_REQUEST, lu->login_id, NULL); + if (sbp2_send_management_orb(lu, node_id, generation, SBP2_LOGIN_REQUEST, lu->lun, &response) < 0) { if (lu->retries++ < 5) @@ -997,9 +1002,17 @@ static void sbp2_reconnect(struct work_struct *work) if (sbp2_send_management_orb(lu, node_id, generation, SBP2_RECONNECT_REQUEST, lu->login_id, NULL) < 0) { - if (lu->retries++ >= 5) { + /* + * If reconnect was impossible even though we are in the + * current generation, fall back and try to log in again. + * + * We could check for "Function rejected" status, but + * looking at the bus generation as simpler and more general. + */ + smp_rmb(); /* get current card generation */ + if (generation == device->card->generation || + lu->retries++ >= 5) { fw_error("%s: failed to reconnect\n", tgt->bus_id); - /* Fall back and try to log in again. */ lu->retries = 0; PREPARE_DELAYED_WORK(&lu->work, sbp2_login); } commit 0fa6dfdb0a2768541e998a5dab10b368de56c60a Author: Stefan Richter <stefanr@s5r6.in-berlin.de> Date: Sun Feb 3 23:10:47 2008 +0100 firewire: fw-sbp2: don't add scsi_device twice When a reconnect failed but re-login succeeded, __scsi_add_device was called again. In those cases, __scsi_add_device succeeded and returned the pointer to the existing scsi_device. fw-sbp2 then continued orderly, except that it missed to call sbp2_cancel_orbs. SCSI core would call fw-sbp2's eh_abort_handler eventually if there had been an outstanding command. This patch avoids the needless lookups and temporary allocations in SCSI core and I/O stall and timeout until eh_abort_handler hits. Also, __scsi_add_device tolerating calls for devices which already exist is undocumented behavior on which we shouldn't rely. Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de> Signed-off-by: Jarod Wilson <jwilson@redhat.com> diff --git a/drivers/firewire/fw-sbp2.c b/drivers/firewire/fw-sbp2.c index 077f1c0..914170b 100644 --- a/drivers/firewire/fw-sbp2.c +++ b/drivers/firewire/fw-sbp2.c @@ -741,6 +741,12 @@ static void sbp2_login(struct work_struct *work) PREPARE_DELAYED_WORK(&lu->work, sbp2_reconnect); sbp2_agent_reset(lu); + /* This was a re-login. */ + if (lu->sdev) { + sbp2_cancel_orbs(lu); + goto out; + } + if (lu->tgt->workarounds & SBP2_WORKAROUND_DELAY_INQUIRY) ssleep(SBP2_INQUIRY_DELAY); commit 48f18c761c001a66ef1928b42799c717368b1d64 Author: Stefan Richter <stefanr@s5r6.in-berlin.de> Date: Sun Feb 3 23:09:50 2008 +0100 firewire: fw-sbp2: log bus_id at management request failures for easier readable logs if more than one SBP-2 device is present. Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de> Signed-off-by: Jarod Wilson <jwilson@redhat.com> diff --git a/drivers/firewire/fw-sbp2.c b/drivers/firewire/fw-sbp2.c index 32b50f1..077f1c0 100644 --- a/drivers/firewire/fw-sbp2.c +++ b/drivers/firewire/fw-sbp2.c @@ -148,6 +148,7 @@ struct sbp2_logical_unit { struct sbp2_target { struct kref kref; struct fw_unit *unit; + const char *bus_id; struct list_head lu_list; u64 management_agent_address; @@ -566,20 +567,20 @@ sbp2_send_management_orb(struct sbp2_logical_unit *lu, int node_id, retval = -EIO; if (sbp2_cancel_orbs(lu) == 0) { - fw_error("orb reply timed out, rcode=0x%02x\n", - orb->base.rcode); + fw_error("%s: orb reply timed out, rcode=0x%02x\n", + lu->tgt->bus_id, orb->base.rcode); goto out; } if (orb->base.rcode != RCODE_COMPLETE) { - fw_error("management write failed, rcode 0x%02x\n", - orb->base.rcode); + fw_error("%s: management write failed, rcode 0x%02x\n", + lu->tgt->bus_id, orb->base.rcode); goto out; } if (STATUS_GET_RESPONSE(orb->status) != 0 || STATUS_GET_SBP_STATUS(orb->status) != 0) { - fw_error("error status: %d:%d\n", + fw_error("%s: error status: %d:%d\n", lu->tgt->bus_id, STATUS_GET_RESPONSE(orb->status), STATUS_GET_SBP_STATUS(orb->status)); goto out; @@ -664,7 +665,7 @@ static void sbp2_release_target(struct kref *kref) kfree(lu); } scsi_remove_host(shost); - fw_notify("released %s\n", tgt->unit->device.bus_id); + fw_notify("released %s\n", tgt->bus_id); put_device(&tgt->unit->device); scsi_host_put(shost); @@ -693,12 +694,11 @@ static void sbp2_login(struct work_struct *work) { struct sbp2_logical_unit *lu = container_of(work, struct sbp2_logical_unit, work.work); - struct Scsi_Host *shost = - container_of((void *)lu->tgt, struct Scsi_Host, hostdata[0]); + struct sbp2_target *tgt = lu->tgt; + struct fw_device *device = fw_device(tgt->unit->device.parent); + struct Scsi_Host *shost; struct scsi_device *sdev; struct scsi_lun eight_bytes_lun; - struct fw_unit *unit = lu->tgt->unit; - struct fw_device *device = fw_device(unit->device.parent); struct sbp2_login_response response; int generation, node_id, local_node_id; @@ -715,14 +715,14 @@ static void sbp2_login(struct work_struct *work) if (lu->retries++ < 5) sbp2_queue_work(lu, DIV_ROUND_UP(HZ, 5)); else - fw_error("failed to login to %s LUN %04x\n", - unit->device.bus_id, lu->lun); + fw_error("%s: failed to login to LUN %04x\n", + tgt->bus_id, lu->lun); goto out; } - lu->generation = generation; - lu->tgt->node_id = node_id; - lu->tgt->address_high = local_node_id << 16; + lu->generation = generation; + tgt->node_id = node_id; + tgt->address_high = local_node_id << 16; /* Get command block agent offset and login id. */ lu->command_block_agent_address = @@ -730,8 +730,8 @@ static void sbp2_login(struct work_struct *work) response.command_block_agent.low; lu->login_id = LOGIN_RESPONSE_GET_LOGIN_ID(response); - fw_notify("logged in to %s LUN %04x (%d retries)\n", - unit->device.bus_id, lu->lun, lu->retries); + fw_notify("%s: logged in to LUN %04x (%d retries)\n", + tgt->bus_id, lu->lun, lu->retries); #if 0 /* FIXME: The linux1394 sbp2 does this last step. */ @@ -747,6 +747,7 @@ static void sbp2_login(struct work_struct *work) memset(&eight_bytes_lun, 0, sizeof(eight_bytes_lun)); eight_bytes_lun.scsi_lun[0] = (lu->lun >> 8) & 0xff; eight_bytes_lun.scsi_lun[1] = lu->lun & 0xff; + shost = container_of((void *)tgt, struct Scsi_Host, hostdata[0]); sdev = __scsi_add_device(shost, 0, 0, scsilun_to_int(&eight_bytes_lun), lu); @@ -767,7 +768,7 @@ static void sbp2_login(struct work_struct *work) scsi_device_put(sdev); } out: - sbp2_target_put(lu->tgt); + sbp2_target_put(tgt); } static int sbp2_add_logical_unit(struct sbp2_target *tgt, int lun_entry) @@ -850,7 +851,7 @@ static int sbp2_scan_unit_dir(struct sbp2_target *tgt, u32 *directory, if (timeout > tgt->mgt_orb_timeout) fw_notify("%s: config rom contains %ds " "management ORB timeout, limiting " - "to %ds\n", tgt->unit->device.bus_id, + "to %ds\n", tgt->bus_id, timeout / 1000, tgt->mgt_orb_timeout / 1000); break; @@ -878,7 +879,7 @@ static void sbp2_init_workarounds(struct sbp2_target *tgt, u32 model, if (w) fw_notify("Please notify linux1394-devel@lists.sourceforge.net " "if you need the workarounds parameter for %s\n", - tgt->unit->device.bus_id); + tgt->bus_id); if (w & SBP2_WORKAROUND_OVERRIDE) goto out; @@ -900,8 +901,7 @@ static void sbp2_init_workarounds(struct sbp2_target *tgt, u32 model, if (w) fw_notify("Workarounds for %s: 0x%x " "(firmware_revision 0x%06x, model_id 0x%06x)\n", - tgt->unit->device.bus_id, - w, firmware_revision, model); + tgt->bus_id, w, firmware_revision, model); tgt->workarounds = w; } @@ -925,6 +925,7 @@ static int sbp2_probe(struct device *dev) tgt->unit = unit; kref_init(&tgt->kref); INIT_LIST_HEAD(&tgt->lu_list); + tgt->bus_id = unit->device.bus_id; if (fw_device_enable_phys_dma(device) < 0) goto fail_shost_put; @@ -975,8 +976,8 @@ static void sbp2_reconnect(struct work_struct *work) { struct sbp2_logical_unit *lu = container_of(work, struct sbp2_logical_unit, work.work); - struct fw_unit *unit = lu->tgt->unit; - struct fw_device *device = fw_device(unit->device.parent); + struct sbp2_target *tgt = lu->tgt; + struct fw_device *device = fw_device(tgt->unit->device.parent); int generation, node_id, local_node_id; if (fw_device_is_shutdown(device)) @@ -991,8 +992,7 @@ static void sbp2_reconnect(struct work_struct *work) SBP2_RECONNECT_REQUEST, lu->login_id, NULL) < 0) { if (lu->retries++ >= 5) { - fw_error("failed to reconnect to %s\n", - unit->device.bus_id); + fw_error("%s: failed to reconnect\n", tgt->bus_id); /* Fall back and try to log in again. */ lu->retries = 0; PREPARE_DELAYED_WORK(&lu->work, sbp2_login); @@ -1001,17 +1001,17 @@ static void sbp2_reconnect(struct work_struct *work) goto out; } - lu->generation = generation; - lu->tgt->node_id = node_id; - lu->tgt->address_high = local_node_id << 16; + lu->generation = generation; + tgt->node_id = node_id; + tgt->address_high = local_node_id << 16; - fw_notify("reconnected to %s LUN %04x (%d retries)\n", - unit->device.bus_id, lu->lun, lu->retries); + fw_notify("%s: reconnected to LUN %04x (%d retries)\n", + tgt->bus_id, lu->lun, lu->retries); sbp2_agent_reset(lu); sbp2_cancel_orbs(lu); out: - sbp2_target_put(lu->tgt); + sbp2_target_put(tgt); } static void sbp2_update(struct fw_unit *unit) @@ -1359,7 +1359,7 @@ static int sbp2_scsi_abort(struct scsi_cmnd *cmd) { struct sbp2_logical_unit *lu = cmd->device->hostdata; - fw_notify("sbp2_scsi_abort\n"); + fw_notify("%s: sbp2_scsi_abort\n", lu->tgt->bus_id); sbp2_agent_reset(lu); sbp2_cancel_orbs(lu); commit e0e60215552d4d40caf581a8d3247203fe948fe7 Author: Stefan Richter <stefanr@s5r6.in-berlin.de> Date: Sun Feb 3 23:08:58 2008 +0100 firewire: fw-sbp2: wait for completion of fetch agent reset Like the old sbp2 driver, wait for the write transaction to the AGENT_RESET to complete before proceeding (after login, after reconnect, or in SCSI error handling). There is one occasion where AGENT_RESET is written to from atomic context when getting DEAD status for a command ORB. There we still continue without waiting for the transaction to complete because this is more difficult to fix... Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de> diff --git a/drivers/firewire/fw-sbp2.c b/drivers/firewire/fw-sbp2.c index 4a118fb..32b50f1 100644 --- a/drivers/firewire/fw-sbp2.c +++ b/drivers/firewire/fw-sbp2.c @@ -603,29 +603,46 @@ sbp2_send_management_orb(struct sbp2_logical_unit *lu, int node_id, static void complete_agent_reset_write(struct fw_card *card, int rcode, - void *payload, size_t length, void *data) + void *payload, size_t length, void *done) { - struct fw_transaction *t = data; + complete(done); +} + +static void sbp2_agent_reset(struct sbp2_logical_unit *lu) +{ + struct fw_device *device = fw_device(lu->tgt->unit->device.parent); + DECLARE_COMPLETION_ONSTACK(done); + struct fw_transaction t; + static u32 z; - kfree(t); + fw_send_request(device->card, &t, TCODE_WRITE_QUADLET_REQUEST, + lu->tgt->node_id, lu->generation, device->max_speed, + lu->command_block_agent_address + SBP2_AGENT_RESET, + &z, sizeof(z), complete_agent_reset_write, &done); + wait_for_completion(&done); } -static int sbp2_agent_reset(struct sbp2_logical_unit *lu) +static void +complete_agent_reset_write_no_wait(struct fw_card *card, int rcode, + void *payload, size_t length, void *data) +{ + kfree(data); +} + +static void sbp2_agent_reset_no_wait(struct sbp2_logical_unit *lu) { struct fw_device *device = fw_device(lu->tgt->unit->device.parent); struct fw_transaction *t; - static u32 zero; + static u32 z; - t = kzalloc(sizeof(*t), GFP_ATOMIC); + t = kmalloc(sizeof(*t), GFP_ATOMIC); if (t == NULL) - return -ENOMEM; + return; fw_send_request(device->card, t, TCODE_WRITE_QUADLET_REQUEST, lu->tgt->node_id, lu->generation, device->max_speed, lu->command_block_agent_address + SBP2_AGENT_RESET, - &zero, sizeof(zero), complete_agent_reset_write, t); - - return 0; + &z, sizeof(z), complete_agent_reset_write_no_wait, t); } static void sbp2_release_target(struct kref *kref) @@ -1086,7 +1103,7 @@ complete_command_orb(struct sbp2_orb *base_orb, struct sbp2_status *status) if (status != NULL) { if (STATUS_GET_DEAD(*status)) - sbp2_agent_reset(orb->lu); + sbp2_agent_reset_no_wait(orb->lu); switch (STATUS_GET_RESPONSE(*status)) { case SBP2_STATUS_REQUEST_COMPLETE: commit d94a983526cb868658c958ab689410dc1c6a31f3 Author: Stefan Richter <stefanr@s5r6.in-berlin.de> Date: Sun Feb 3 23:07:44 2008 +0100 ieee1394: sbp2: add INQUIRY delay workaround Add the same workaround as found in fw-sbp2 for feature parity and compatibility of the workarounds module parameter. Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de> Signed-off-by: Jarod Wilson <jwilson@redhat.com> diff --git a/drivers/ieee1394/sbp2.c b/drivers/ieee1394/sbp2.c index 28e155a..accb2ad 100644 --- a/drivers/ieee1394/sbp2.c +++ b/drivers/ieee1394/sbp2.c @@ -183,6 +183,9 @@ MODULE_PARM_DESC(exclusive_login, "Exclusive login to sbp2 device " * Avoids access beyond actual disk limits on devices with an off-by-one bug. * Don't use this with devices which don't have this bug. * + * - delay inquiry + * Wait extra SBP2_INQUIRY_DELAY seconds after login before SCSI inquiry. + * * - override internal blacklist * Instead of adding to the built-in blacklist, use only the workarounds * specified in the module load parameter. @@ -195,6 +198,7 @@ MODULE_PARM_DESC(workarounds, "Work around device bugs (default = 0" ", 36 byte inquiry = " __stringify(SBP2_WORKAROUND_INQUIRY_36) ", skip mode page 8 = " __stringify(SBP2_WORKAROUND_MODE_SENSE_8) ", fix capacity = " __stringify(SBP2_WORKAROUND_FIX_CAPACITY) + ", delay inquiry = " __stringify(SBP2_WORKAROUND_DELAY_INQUIRY) ", override internal blacklist = " __stringify(SBP2_WORKAROUND_OVERRIDE) ", or a combination)"); @@ -357,6 +361,11 @@ static const struct { .workarounds = SBP2_WORKAROUND_INQUIRY_36 | SBP2_WORKAROUND_MODE_SENSE_8, }, + /* DViCO Momobay FX-3A with TSB42AA9A bridge */ { + .firmware_revision = 0x002800, + .model_id = 0x000000, + .workarounds = SBP2_WORKAROUND_DELAY_INQUIRY, + }, /* Initio bridges, actually only needed for some older ones */ { .firmware_revision = 0x000200, .model_id = SBP2_ROM_VALUE_WILDCARD, @@ -914,6 +923,9 @@ static int sbp2_start_device(struct sbp2_lu *lu) sbp2_agent_reset(lu, 1); sbp2_max_speed_and_size(lu); + if (lu->workarounds & SBP2_WORKAROUND_DELAY_INQUIRY) + ssleep(SBP2_INQUIRY_DELAY); + error = scsi_add_device(lu->shost, 0, lu->ud->id, 0); if (error) { SBP2_ERR("scsi_add_device failed"); diff --git a/drivers/ieee1394/sbp2.h b/drivers/ieee1394/sbp2.h index d2ecb0d..80d8e09 100644 --- a/drivers/ieee1394/sbp2.h +++ b/drivers/ieee1394/sbp2.h @@ -343,6 +343,8 @@ enum sbp2lu_state_types { #define SBP2_WORKAROUND_INQUIRY_36 0x2 #define SBP2_WORKAROUND_MODE_SENSE_8 0x4 #define SBP2_WORKAROUND_FIX_CAPACITY 0x8 +#define SBP2_WORKAROUND_DELAY_INQUIRY 0x10 +#define SBP2_INQUIRY_DELAY 12 #define SBP2_WORKAROUND_OVERRIDE 0x100 #endif /* SBP2_H */ commit 9220f1946209a5b3335ea2d28f8462695885791b Author: Stefan Richter <stefanr@s5r6.in-berlin.de> Date: Sun Feb 3 23:04:38 2008 +0100 firewire: fw-sbp2: add INQUIRY delay workaround Several different SBP-2 bridges accept a login early while the IDE device is still powering up. They are therefore unable to respond to SCSI INQUIRY immediately, and the SCSI core has to retry the INQUIRY. One of these retries is typically successful, and all is well. But in case of Momobay FX-3A, the INQUIRY retries tend to fail entirely. This can usually be avoided by waiting a little while after login before letting the SCSI core send the INQUIRY. The old sbp2 driver handles this more gracefully for as yet unknown reasons (perhaps because it waits for fetch agent resets to complete, unlike fw-sbp2 which quickly proceeds after requesting the agent reset). Therefore the workaround is not as much necessary for sbp2. Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de> Signed-off-by: Jarod Wilson <jwilson@redhat.com> diff --git a/drivers/firewire/fw-sbp2.c b/drivers/firewire/fw-sbp2.c index 72fddf5..4a118fb 100644 --- a/drivers/firewire/fw-sbp2.c +++ b/drivers/firewire/fw-sbp2.c @@ -32,6 +32,7 @@ #include <linux/module.h> #include <linux/moduleparam.h> #include <linux/mod_devicetable.h> +#include <linux/delay.h> #include <linux/device.h> #include <linux/scatterlist.h> #include <linux/dma-mapping.h> @@ -82,6 +83,9 @@ MODULE_PARM_DESC(exclusive_login, "Exclusive login to sbp2 device " * Avoids access beyond actual disk limits on devices with an off-by-one bug. * Don't use this with devices which don't have this bug. * + * - delay inquiry + * Wait extra SBP2_INQUIRY_DELAY seconds after login before SCSI inquiry. + * * - override internal blacklist * Instead of adding to the built-in blacklist, use only the workarounds * specified in the module load parameter. @@ -91,6 +95,8 @@ MODULE_PARM_DESC(exclusive_login, "Exclusive login to sbp2 device " #define SBP2_WORKAROUND_INQUIRY_36 0x2 #define SBP2_WORKAROUND_MODE_SENSE_8 0x4 #define SBP2_WORKAROUND_FIX_CAPACITY 0x8 +#define SBP2_WORKAROUND_DELAY_INQUIRY 0x10 +#define SBP2_INQUIRY_DELAY 12 #define SBP2_WORKAROUND_OVERRIDE 0x100 static int sbp2_param_workarounds; @@ -100,6 +106,7 @@ MODULE_PARM_DESC(workarounds, "Work around device bugs (default = 0" ", 36 byte inquiry = " __stringify(SBP2_WORKAROUND_INQUIRY_36) ", skip mode page 8 = " __stringify(SBP2_WORKAROUND_MODE_SENSE_8) ", fix capacity = " __stringify(SBP2_WORKAROUND_FIX_CAPACITY) + ", delay inquiry = " __stringify(SBP2_WORKAROUND_DELAY_INQUIRY) ", override internal blacklist = " __stringify(SBP2_WORKAROUND_OVERRIDE) ", or a combination)"); @@ -303,6 +310,11 @@ static const struct { .workarounds = SBP2_WORKAROUND_INQUIRY_36 | SBP2_WORKAROUND_MODE_SENSE_8, }, + /* DViCO Momobay FX-3A with TSB42AA9A bridge */ { + .firmware_revision = 0x002800, + .model = 0x000000, + .workarounds = SBP2_WORKAROUND_DELAY_INQUIRY, + }, /* Initio bridges, actually only needed for some older ones */ { .firmware_revision = 0x000200, .model = ~0, @@ -712,6 +724,9 @@ static void sbp2_login(struct work_struct *work) PREPARE_DELAYED_WORK(&lu->work, sbp2_reconnect); sbp2_agent_reset(lu); + if (lu->tgt->workarounds & SBP2_WORKAROUND_DELAY_INQUIRY) + ssleep(SBP2_INQUIRY_DELAY); + memset(&eight_bytes_lun, 0, sizeof(eight_bytes_lun)); eight_bytes_lun.scsi_lun[0] = (lu->lun >> 8) & 0xff; eight_bytes_lun.scsi_lun[1] = lu->lun & 0xff; commit fa6e697b85d705d37b3b03829095c22bcbe95ab6 Author: Stefan Richter <stefanr@s5r6.in-berlin.de> Date: Sun Feb 3 23:03:00 2008 +0100 firewire: log GUID of new devices This should help to interpret user reports. E.g. one can look up the vendor OUI (first three bytes of the GUID) and thus tell what is what. Also simplifies the math in the GUID sysfs attribute. Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de> Signed-off-by: Jarod Wilson <jwilson@redhat.com> diff --git a/drivers/firewire/fw-device.c b/drivers/firewire/fw-device.c index c04c288..2ab13e0 100644 --- a/drivers/firewire/fw-device.c +++ b/drivers/firewire/fw-device.c @@ -358,12 +358,9 @@ static ssize_t guid_show(struct device *dev, struct device_attribute *attr, char *buf) { struct fw_device *device = fw_device(dev); - u64 guid; - guid = ((u64)device->config_rom[3] << 32) | device->config_rom[4]; - - return snprintf(buf, PAGE_SIZE, "0x%016llx\n", - (unsigned long long)guid); + return snprintf(buf, PAGE_SIZE, "0x%08x%08x\n", + device->config_rom[3], device->config_rom[4]); } static struct device_attribute fw_device_attributes[] = { @@ -723,13 +720,22 @@ static void fw_device_init(struct work_struct *work) */ if (atomic_cmpxchg(&device->state, FW_DEVICE_INITIALIZING, - FW_DEVICE_RUNNING) == FW_DEVICE_SHUTDOWN) + FW_DEVICE_RUNNING) == FW_DEVICE_SHUTDOWN) { fw_device_shutdown(&device->work.work); - else - fw_notify("created new fw device %s " - "(%d config rom retries, S%d00)\n", - device->device.bus_id, device->config_rom_retries, - 1 << device->max_speed); + } else { + if (device->config_rom_retries) + fw_notify("created device %s: GUID %08x%08x, S%d00, " + "%d config ROM retries\n", + device->device.bus_id, + device->config_rom[3], device->config_rom[4], + 1 << device->max_speed, + device->config_rom_retries); + else + fw_notify("created device %s: GUID %08x%08x, S%d00\n", + device->device.bus_id, + device->config_rom[3], device->config_rom[4], + 1 << device->max_speed); + } /* * Reschedule the IRM work if we just finished reading the commit be6f48b0174584c9c415012ca14803c7e941e27e Author: Stefan Richter <stefanr@s5r6.in-berlin.de> Date: Sun Jan 27 19:14:44 2008 +0100 firewire: fw-sbp2: don't retry login or reconnect after unplug If a device is being unplugged while fw-sbp2 had a login or reconnect on schedule, it would take about half a minute to shut the fw_unit down: Jan 27 18:34:54 stein firewire_sbp2: logged in to fw2.0 LUN 0000 (0 retries) <unplug> Jan 27 18:34:59 stein firewire_sbp2: sbp2_scsi_abort Jan 27 18:34:59 stein scsi 25:0:0:0: Device offlined - not ready after error recovery Jan 27 18:35:01 stein firewire_sbp2: orb reply timed out, rcode=0x11 Jan 27 18:35:06 stein firewire_sbp2: orb reply timed out, rcode=0x11 Jan 27 18:35:12 stein firewire_sbp2: orb reply timed out, rcode=0x11 Jan 27 18:35:17 stein firewire_sbp2: orb reply timed out, rcode=0x11 Jan 27 18:35:22 stein firewire_sbp2: orb reply timed out, rcode=0x11 Jan 27 18:35:27 stein firewire_sbp2: orb reply timed out, rcode=0x11 Jan 27 18:35:32 stein firewire_sbp2: orb reply timed out, rcode=0x11 Jan 27 18:35:32 stein firewire_sbp2: failed to login to fw2.0 LUN 0000 Jan 27 18:35:32 stein firewire_sbp2: released fw2.0 After this patch, typically only a few seconds spent in __scsi_add_device remain: Jan 27 19:05:50 stein firewire_sbp2: logged in to fw2.0 LUN 0000 (0 retries) <unplug> Jan 27 19:05:56 stein firewire_sbp2: sbp2_scsi_abort Jan 27 19:05:56 stein scsi 33:0:0:0: Device offlined - not ready after error recovery Jan 27 19:05:56 stein firewire_sbp2: released fw2.0 The benefit of this is less noise in the syslog. It furthermore avoids a few wasted CPU cycles and needlessly prolonged lifetime of a few driver objects. Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de> Signed-off-by: Jarod Wilson <jwilson@redhat.com> diff --git a/drivers/firewire/fw-sbp2.c b/drivers/firewire/fw-sbp2.c index a15e3c7..72fddf5 100644 --- a/drivers/firewire/fw-sbp2.c +++ b/drivers/firewire/fw-sbp2.c @@ -499,6 +499,9 @@ sbp2_send_management_orb(struct sbp2_logical_unit *lu, int node_id, unsigned int timeout; int retval = -ENOMEM; + if (function == SBP2_LOGOUT_REQUEST && fw_device_is_shutdown(device)) + return 0; + orb = kzalloc(sizeof(*orb), GFP_ATOMIC); if (orb == NULL) return -ENOMEM; @@ -619,16 +622,13 @@ static void sbp2_release_target(struct kref *kref) struct sbp2_logical_unit *lu, *next; struct Scsi_Host *shost = container_of((void *)tgt, struct Scsi_Host, hostdata[0]); - struct fw_device *device = fw_device(tgt->unit->device.parent); list_for_each_entry_safe(lu, next, &tgt->lu_list, link) { if (lu->sdev) scsi_remove_device(lu->sdev); - if (!fw_device_is_shutdown(device)) - sbp2_send_management_orb(lu, tgt->node_id, - lu->generation, SBP2_LOGOUT_REQUEST, - lu->login_id, NULL); + sbp2_send_management_orb(lu, tgt->node_id, lu->generation, + SBP2_LOGOUT_REQUEST, lu->login_id, NULL); fw_core_remove_address_handler(&lu->address_handler); list_del(&lu->link); @@ -673,6 +673,9 @@ static void sbp2_login(struct work_struct *work) struct sbp2_login_response response; int generation, node_id, local_node_id; + if (fw_device_is_shutdown(device)) + goto out; + generation = device->generation; smp_rmb(); /* node_id must not be older than generation */ node_id = device->node_id; @@ -944,6 +947,9 @@ static void sbp2_reconnect(struct work_struct *work) struct fw_device *device = fw_device(unit->device.parent); int generation, node_id, local_node_id; + if (fw_device_is_shutdown(device)) + goto out; + generation = device->generation; smp_rmb(); /* node_id must not be older than generation */ node_id = device->node_id; commit 96b19062e741b715cf399312c30e0672d8889569 Author: Stefan Richter <stefanr@s5r6.in-berlin.de> Date: Sat Feb 2 15:01:09 2008 +0100 firewire: fix "kobject_add failed for fw* with -EEXIST" There is a race between shutdown and creation of devices: fw-core may attempt to add a device with the same name of an already existing device. http://bugzilla.kernel.org/show_bug.cgi?id=9828 Impact of the bug: Happens rarely (when shutdown of a device coincides with creation of another), forces the user to unplug and replug the new device to get it working. The fix is obvious: Free the minor number *after* instead of *before* device_unregister(). This requires to take an additional reference of the fw_device as long as the IDR tree points to it. And while we are at it, we fix an additional race condition: fw_device_op_open() took its reference of the fw_device a little bit too late, hence was in danger to access an already invalid fw_device. Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de> diff --git a/drivers/firewire/fw-cdev.c b/drivers/firewire/fw-cdev.c index 7e73cba..44ccee2 100644 --- a/drivers/firewire/fw-cdev.c +++ b/drivers/firewire/fw-cdev.c @@ -109,15 +109,17 @@ static int fw_device_op_open(struct inode *inode, struct file *file) struct client *client; unsigned long flags; - device = fw_device_from_devt(inode->i_rdev); + device = fw_device_get_by_devt(inode->i_rdev); if (device == NULL) return -ENODEV; client = kzalloc(sizeof(*client), GFP_KERNEL); - if (client == NULL) + if (client == NULL) { + fw_device_put(device); return -ENOMEM; + } - client->device = fw_device_get(device); + client->device = device; INIT_LIST_HEAD(&client->event_list); INIT_LIST_HEAD(&client->resource_list); spin_lock_init(&client->lock); diff --git a/drivers/firewire/fw-device.c b/drivers/firewire/fw-device.c index de9066e..c04c288 100644 --- a/drivers/firewire/fw-device.c +++ b/drivers/firewire/fw-device.c @@ -610,12 +610,14 @@ static DECLARE_RWSEM(idr_rwsem); static DEFINE_IDR(fw_device_idr); int fw_cdev_major; -struct fw_device *fw_device_from_devt(dev_t devt) +struct fw_device *fw_device_get_by_devt(dev_t devt) { struct fw_device *device; down_read(&idr_rwsem); device = idr_find(&fw_device_idr, MINOR(devt)); + if (device) + fw_device_get(device); up_read(&idr_rwsem); return device; @@ -627,13 +629,14 @@ static void fw_device_shutdown(struct work_struct *work) container_of(work, struct fw_device, work.work); int minor = MINOR(device->device.devt); - down_write(&idr_rwsem); - idr_remove(&fw_device_idr, minor); - up_write(&idr_rwsem); - fw_device_cdev_remove(device); device_for_each_child(&device->device, NULL, shutdown_unit); device_unregister(&device->device); + + down_write(&idr_rwsem); + idr_remove(&fw_device_idr, minor); + up_write(&idr_rwsem); + fw_device_put(device); } static struct device_type fw_device_type = { @@ -682,10 +685,13 @@ static void fw_device_init(struct work_struct *work) } err = -ENOMEM; + + fw_device_get(device); down_write(&idr_rwsem); if (idr_pre_get(&fw_device_idr, GFP_KERNEL)) err = idr_get_new(&fw_device_idr, device, &minor); up_write(&idr_rwsem); + if (err < 0) goto error; @@ -741,7 +747,9 @@ static void fw_device_init(struct work_struct *work) idr_remove(&fw_device_idr, minor); up_write(&idr_rwsem); error: - put_device(&device->device); + fw_device_put(device); /* fw_device_idr's reference */ + + put_device(&device->device); /* our reference */ } static int update_unit(struct device *dev, void *data) diff --git a/drivers/firewire/fw-device.h b/drivers/firewire/fw-device.h index 0854fe2..43808c0 100644 --- a/drivers/firewire/fw-device.h +++ b/drivers/firewire/fw-device.h @@ -77,13 +77,13 @@ fw_device_is_shutdown(struct fw_device *device) } struct fw_device *fw_device_get(struct fw_device *device); +struct fw_device *fw_device_get_by_devt(dev_t devt); void fw_device_put(struct fw_device *device); int fw_device_enable_phys_dma(struct fw_device *device); void fw_device_cdev_update(struct fw_device *device); void fw_device_cdev_remove(struct fw_device *device); -struct fw_device *fw_device_from_devt(dev_t devt); extern int fw_cdev_major; struct fw_unit { commit 1b9c12ba2fdf802a23630f70eddb0e821296634e Author: Stefan Richter <stefanr@s5r6.in-berlin.de> Date: Sat Jan 26 17:43:23 2008 +0100 firewire: fw-sbp2: fix logout before login retry This fixes a "can't recognize device" kind of bug. If the SCSI INQUIRY failed and hence __scsi_add_device failed due to a bus reset, we tried a logout and then waited for the already scheduled login work to happen. So far so good, but the generation used for the logout was outdated, hence the logout never reached the target. The target might therefore deny the subsequent relogin attempt, which would also leave the target inaccessible. Therefore fetch a fresh device->generation for the logout. Use memory barriers to prevent our plan being foiled by compiler or hardware optimizations. Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de> diff --git a/drivers/firewire/fw-sbp2.c b/drivers/firewire/fw-sbp2.c index f2a9a33..a15e3c7 100644 --- a/drivers/firewire/fw-sbp2.c +++ b/drivers/firewire/fw-sbp2.c @@ -716,7 +716,11 @@ static void sbp2_login(struct work_struct *work) sdev = __scsi_add_device(shost, 0, 0, scsilun_to_int(&eight_bytes_lun), lu); if (IS_ERR(sdev)) { - sbp2_send_management_orb(lu, node_id, generation, + smp_rmb(); /* generation may have changed */ + generation = device->generation; + smp_rmb(); /* node_id must not be older than generation */ + + sbp2_send_management_orb(lu, device->node_id, generation, SBP2_LOGOUT_REQUEST, lu->login_id, NULL); /* * Set this back to sbp2_login so we fall back and commit 05cca7381429e12d66c5b5c8b5c5848055b88bf7 Author: Stefan Richter <stefanr@s5r6.in-berlin.de> Date: Sat Jan 26 17:42:45 2008 +0100 firewire: fw-sbp2: unsigned int vs. unsigned Standardize on "unsigned int" style. Sort some struct members thematically. Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de> diff --git a/drivers/firewire/fw-sbp2.c b/drivers/firewire/fw-sbp2.c index 19ece9b..f2a9a33 100644 --- a/drivers/firewire/fw-sbp2.c +++ b/drivers/firewire/fw-sbp2.c @@ -141,15 +141,13 @@ struct sbp2_logical_unit { struct sbp2_target { struct kref kref; struct fw_unit *unit; + struct list_head lu_list; u64 management_agent_address; int directory_id; int node_id; int address_high; - - unsigned workarounds; - struct list_head lu_list; - + unsigned int workarounds; unsigned int mgt_orb_timeout; }; @@ -160,7 +158,7 @@ struct sbp2_target { */ #define SBP2_MIN_LOGIN_ORB_TIMEOUT 5000U /* Timeout in ms */ #define SBP2_MAX_LOGIN_ORB_TIMEOUT 40000U /* Timeout in ms */ -#define SBP2_ORB_TIMEOUT 2000 /* Timeout in ms */ +#define SBP2_ORB_TIMEOUT 2000U /* Timeout in ms */ #define SBP2_ORB_NULL 0x80000000 #define SBP2_MAX_SG_ELEMENT_LENGTH 0xf000 @@ -297,7 +295,7 @@ struct sbp2_command_orb { static const struct { u32 firmware_revision; u32 model; - unsigned workarounds; + unsigned int workarounds; } sbp2_workarounds_table[] = { /* DViCO Momobay CX-1 with TSB42AA9 bridge */ { .firmware_revision = 0x002800, @@ -836,7 +834,7 @@ static void sbp2_init_workarounds(struct sbp2_target *tgt, u32 model, u32 firmware_revision) { int i; - unsigned w = sbp2_param_workarounds; + unsigned int w = sbp2_param_workarounds; if (w) fw_notify("Please notify linux1394-devel@lists.sourceforge.net " @@ -1197,7 +1195,7 @@ static int sbp2_scsi_queuecommand(struct scsi_cmnd *cmd, scsi_done_fn_t done) struct sbp2_logical_unit *lu = cmd->device->hostdata; struct fw_device *device = fw_device(lu->tgt->unit->device.parent); struct sbp2_command_orb *orb; - unsigned max_payload; + unsigned int max_payload; int retval = SCSI_MLQUEUE_HOST_BUSY; /* -- Stefan Richter -=====-==--- --=- ==--= http://arcgraph.de/sr/ ^ permalink raw reply [flat|nested] 18+ messages in thread
* [GIT PULL] FireWire updates 2008-02-25 17:58 ` [GIT PULL] FireWire updates Stefan Richter 2008-02-25 18:00 ` Stefan Richter @ 2008-03-02 12:47 ` Stefan Richter 2008-03-02 12:49 ` Stefan Richter 2008-03-14 18:07 ` Stefan Richter 1 sibling, 2 replies; 18+ messages in thread From: Stefan Richter @ 2008-03-02 12:47 UTC (permalink / raw) To: Linus Torvalds, Andrew Morton; +Cc: linux-kernel, linux1394-devel Linus, please pull from the for-linus branch at git://git.kernel.org/pub/scm/linux/kernel/git/ieee1394/linux1394-2.6.git for-linus to receive the following updates for the firewire subsystem. They fix module unloading. I will follow up with the combined full log and diff. Diffstat and shortlog: drivers/firewire/fw-card.c | 61 ++++++++++++++++++++-------- drivers/firewire/fw-device.c | 21 +++------- drivers/firewire/fw-device.h | 16 +++++++- drivers/firewire/fw-sbp2.c | 50 +++++++++++++++--------- drivers/firewire/fw-topology.c | 1 + drivers/firewire/fw-transaction.h | 2 + 6 files changed, 97 insertions(+), 54 deletions(-) Stefan Richter (3): firewire: fw-sbp2: better fix for NULL pointer dereference in scsi_remove_device firewire: potentially invalid pointers used in fw_card_bm_work firewire: fix crash in automatic module unloading Thanks, -- Stefan Richter -=====-==--- --== ---=- http://arcgraph.de/sr/ ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [GIT PULL] FireWire updates 2008-03-02 12:47 ` Stefan Richter @ 2008-03-02 12:49 ` Stefan Richter 2008-03-14 18:07 ` Stefan Richter 1 sibling, 0 replies; 18+ messages in thread From: Stefan Richter @ 2008-03-02 12:49 UTC (permalink / raw) To: Linus Torvalds; +Cc: Andrew Morton, linux-kernel, linux1394-devel I wrote: > Linus, please pull from the for-linus branch at > > git://git.kernel.org/pub/scm/linux/kernel/git/ieee1394/linux1394-2.6.git for-linus > > to receive the following updates for the firewire subsystem. They fix > module unloading. I will follow up with the combined full log and diff. > Diffstat and shortlog: > > drivers/firewire/fw-card.c | 61 ++++++++++++++++++++-------- > drivers/firewire/fw-device.c | 21 +++------- > drivers/firewire/fw-device.h | 16 +++++++- > drivers/firewire/fw-sbp2.c | 50 +++++++++++++++--------- > drivers/firewire/fw-topology.c | 1 + > drivers/firewire/fw-transaction.h | 2 + > 6 files changed, 97 insertions(+), 54 deletions(-) > > Stefan Richter (3): > firewire: fw-sbp2: better fix for NULL pointer dereference in scsi_remove_device > firewire: potentially invalid pointers used in fw_card_bm_work > firewire: fix crash in automatic module unloading commit 855c603d61ede7e2810217f15f0d574b4f29c891 Author: Stefan Richter <stefanr@s5r6.in-berlin.de> Date: Wed Feb 27 22:14:27 2008 +0100 firewire: fix crash in automatic module unloading "modprobe firewire-ohci; sleep .1; modprobe -r firewire-ohci" used to result in crashes like this: BUG: unable to handle kernel paging request at ffffffff8807b455 IP: [<ffffffff8807b455>] PGD 203067 PUD 207063 PMD 7c170067 PTE 0 Oops: 0010 [1] PREEMPT SMP CPU 0 Modules linked in: i915 drm cpufreq_ondemand acpi_cpufreq freq_table applesmc input_polldev led_class coretemp hwmon eeprom snd_seq_oss snd_seq_midi_event snd_seq snd_seq_device snd_pcm_oss snd_mixer_oss button thermal processor sg snd_hda_intel snd_pcm snd_timer snd snd_page_alloc sky2 i2c_i801 rtc [last unloaded: crc_itu_t] Pid: 9, comm: events/0 Not tainted 2.6.25-rc2 #3 RIP: 0010:[<ffffffff8807b455>] [<ffffffff8807b455>] RSP: 0018:ffff81007dcdde88 EFLAGS: 00010246 RAX: ffff81007dc95040 RBX: ffff81007dee5390 RCX: 0000000000005e13 RDX: 0000000000008c8b RSI: 0000000000000001 RDI: ffff81007dee5388 RBP: ffff81007dc5eb40 R08: 0000000000000002 R09: ffffffff8022d05c R10: ffffffff8023b34c R11: ffffffff8041a353 R12: ffff81007dee5388 R13: ffffffff8807b455 R14: ffffffff80593bc0 R15: 0000000000000000 FS: 0000000000000000(0000) GS:ffffffff8055a000(0000) knlGS:0000000000000000 CS: 0010 DS: 0018 ES: 0018 CR0: 000000008005003b CR2: ffffffff8807b455 CR3: 0000000000201000 CR4: 00000000000006e0 DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400 Process events/0 (pid: 9, threadinfo ffff81007dcdc000, task ffff81007dc95040) Stack: ffffffff8023b396 ffffffff88082524 0000000000000000 ffffffff8807d9ae ffff81007dc5eb40 ffff81007dc9dce0 ffff81007dc5eb40 ffff81007dc5eb80 ffff81007dc9dce0 ffffffffffffffff ffffffff8023be87 0000000000000000 Call Trace: [<ffffffff8023b396>] ? run_workqueue+0xdf/0x1df [<ffffffff8023be87>] ? worker_thread+0xd8/0xe3 [<ffffffff8023e917>] ? autoremove_wake_function+0x0/0x2e [<ffffffff8023bdaf>] ? worker_thread+0x0/0xe3 [<ffffffff8023e813>] ? kthread+0x47/0x74 [<ffffffff804198e0>] ? trace_hardirqs_on_thunk+0x35/0x3a [<ffffffff8020c008>] ? child_rip+0xa/0x12 [<ffffffff8020b6e3>] ? restore_args+0x0/0x3d [<ffffffff8023e68a>] ? kthreadd+0x14c/0x171 [<ffffffff8023e68a>] ? kthreadd+0x14c/0x171 [<ffffffff8023e7cc>] ? kthread+0x0/0x74 [<ffffffff8020bffe>] ? child_rip+0x0/0x12 Code: Bad RIP value. RIP [<ffffffff8807b455>] RSP <ffff81007dcdde88> CR2: ffffffff8807b455 ---[ end trace c7366c6657fe5bed ]--- Note that this crash happened _after_ firewire-core was unloaded. The shared workqueue tried to run firewire-core's device initialization jobs or similar jobs. The fix makes sure that firewire-ohci and hence firewire-core is not unloaded before all device shutdown jobs have been completed. This is determined by the count of device initializations minus device releases. Also skip useless retries in the node initialization job if the node is to be shut down. Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de> Signed-off-by: Jarod Wilson <jwilson@redhat.com> diff --git a/drivers/firewire/fw-card.c b/drivers/firewire/fw-card.c index e6395b2..a034627 100644 --- a/drivers/firewire/fw-card.c +++ b/drivers/firewire/fw-card.c @@ -18,6 +18,7 @@ #include <linux/module.h> #include <linux/errno.h> +#include <linux/delay.h> #include <linux/device.h> #include <linux/mutex.h> #include <linux/crc-itu-t.h> @@ -398,6 +399,7 @@ fw_card_initialize(struct fw_card *card, const struct fw_card_driver *driver, static atomic_t index = ATOMIC_INIT(-1); kref_init(&card->kref); + atomic_set(&card->device_count, 0); card->index = atomic_inc_return(&index); card->driver = driver; card->device = device; @@ -528,8 +530,14 @@ fw_core_remove_card(struct fw_card *card) card->driver = &dummy_driver; fw_destroy_nodes(card); - flush_scheduled_work(); + /* + * Wait for all device workqueue jobs to finish. Otherwise the + * firewire-core module could be unloaded before the jobs ran. + */ + while (atomic_read(&card->device_count) > 0) + msleep(100); + cancel_delayed_work_sync(&card->work); fw_flush_transactions(card); del_timer_sync(&card->flush_timer); diff --git a/drivers/firewire/fw-device.c b/drivers/firewire/fw-device.c index 2ab13e0..870125a 100644 --- a/drivers/firewire/fw-device.c +++ b/drivers/firewire/fw-device.c @@ -150,21 +150,10 @@ struct bus_type fw_bus_type = { }; EXPORT_SYMBOL(fw_bus_type); -struct fw_device *fw_device_get(struct fw_device *device) -{ - get_device(&device->device); - - return device; -} - -void fw_device_put(struct fw_device *device) -{ - put_device(&device->device); -} - static void fw_device_release(struct device *dev) { struct fw_device *device = fw_device(dev); + struct fw_card *card = device->card; unsigned long flags; /* @@ -176,9 +165,9 @@ static void fw_device_release(struct device *dev) spin_unlock_irqrestore(&device->card->lock, flags); fw_node_put(device->node); - fw_card_put(device->card); kfree(device->config_rom); kfree(device); + atomic_dec(&card->device_count); } int fw_device_enable_phys_dma(struct fw_device *device) @@ -668,7 +657,8 @@ static void fw_device_init(struct work_struct *work) */ if (read_bus_info_block(device, device->generation) < 0) { - if (device->config_rom_retries < MAX_RETRIES) { + if (device->config_rom_retries < MAX_RETRIES && + atomic_read(&device->state) == FW_DEVICE_INITIALIZING) { device->config_rom_retries++; schedule_delayed_work(&device->work, RETRY_DELAY); } else { @@ -805,7 +795,8 @@ void fw_node_event(struct fw_card *card, struct fw_node *node, int event) */ device_initialize(&device->device); atomic_set(&device->state, FW_DEVICE_INITIALIZING); - device->card = fw_card_get(card); + atomic_inc(&card->device_count); + device->card = card; device->node = fw_node_get(node); device->node_id = node->node_id; device->generation = card->generation; diff --git a/drivers/firewire/fw-device.h b/drivers/firewire/fw-device.h index 43808c0..78ecd39 100644 --- a/drivers/firewire/fw-device.h +++ b/drivers/firewire/fw-device.h @@ -76,9 +76,21 @@ fw_device_is_shutdown(struct fw_device *device) return atomic_read(&device->state) == FW_DEVICE_SHUTDOWN; } -struct fw_device *fw_device_get(struct fw_device *device); +static inline struct fw_device * +fw_device_get(struct fw_device *device) +{ + get_device(&device->device); + + return device; +} + +static inline void +fw_device_put(struct fw_device *device) +{ + put_device(&device->device); +} + struct fw_device *fw_device_get_by_devt(dev_t devt); -void fw_device_put(struct fw_device *device); int fw_device_enable_phys_dma(struct fw_device *device); void fw_device_cdev_update(struct fw_device *device); diff --git a/drivers/firewire/fw-sbp2.c b/drivers/firewire/fw-sbp2.c index a093ac3..03069a4 100644 --- a/drivers/firewire/fw-sbp2.c +++ b/drivers/firewire/fw-sbp2.c @@ -769,6 +769,7 @@ static void sbp2_release_target(struct kref *kref) struct Scsi_Host *shost = container_of((void *)tgt, struct Scsi_Host, hostdata[0]); struct scsi_device *sdev; + struct fw_device *device = fw_device(tgt->unit->device.parent); /* prevent deadlocks */ sbp2_unblock(tgt); @@ -791,6 +792,7 @@ static void sbp2_release_target(struct kref *kref) put_device(&tgt->unit->device); scsi_host_put(shost); + fw_device_put(device); } static struct workqueue_struct *sbp2_wq; @@ -1088,6 +1090,8 @@ static int sbp2_probe(struct device *dev) if (scsi_add_host(shost, &unit->device) < 0) goto fail_shost_put; + fw_device_get(device); + /* Initialize to values that won't match anything in our table. */ firmware_revision = 0xff000000; model = 0xff000000; diff --git a/drivers/firewire/fw-transaction.h b/drivers/firewire/fw-transaction.h index fa7967b..09cb728 100644 --- a/drivers/firewire/fw-transaction.h +++ b/drivers/firewire/fw-transaction.h @@ -26,6 +26,7 @@ #include <linux/fs.h> #include <linux/dma-mapping.h> #include <linux/firewire-constants.h> +#include <asm/atomic.h> #define TCODE_IS_READ_REQUEST(tcode) (((tcode) & ~1) == 4) #define TCODE_IS_BLOCK_PACKET(tcode) (((tcode) & 1) != 0) @@ -219,6 +220,7 @@ extern struct bus_type fw_bus_type; struct fw_card { const struct fw_card_driver *driver; struct device *device; + atomic_t device_count; struct kref kref; int node_id; commit 15803478fdea964e5f76079851fcd13068208d5d Author: Stefan Richter <stefanr@s5r6.in-berlin.de> Date: Sun Feb 24 18:57:23 2008 +0100 firewire: potentially invalid pointers used in fw_card_bm_work The bus management workqueue job was in danger to dereference NULL pointers. Also, after having temporarily lifted card->lock, a few node pointers and a device pointer may have become invalid. Add NULL pointer checks and get the necessary references. Also, move card->local_node out of fw_card_bm_work's sight during shutdown of the card. Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de> Signed-off-by: Jarod Wilson <jwilson@redhat.com> diff --git a/drivers/firewire/fw-card.c b/drivers/firewire/fw-card.c index 3e97199..e6395b2 100644 --- a/drivers/firewire/fw-card.c +++ b/drivers/firewire/fw-card.c @@ -214,17 +214,29 @@ static void fw_card_bm_work(struct work_struct *work) { struct fw_card *card = container_of(work, struct fw_card, work.work); - struct fw_device *root; + struct fw_device *root_device; + struct fw_node *root_node, *local_node; struct bm_data bmd; unsigned long flags; int root_id, new_root_id, irm_id, gap_count, generation, grace; int do_reset = 0; spin_lock_irqsave(&card->lock, flags); + local_node = card->local_node; + root_node = card->root_node; + + if (local_node == NULL) { + spin_unlock_irqrestore(&card->lock, flags); + return; + } + fw_node_get(local_node); + fw_node_get(root_node); generation = card->generation; - root = card->root_node->data; - root_id = card->root_node->node_id; + root_device = root_node->data; + if (root_device) + fw_device_get(root_device); + root_id = root_node->node_id; grace = time_after(jiffies, card->reset_jiffies + DIV_ROUND_UP(HZ, 10)); if (card->bm_generation + 1 == generation || @@ -243,14 +255,14 @@ fw_card_bm_work(struct work_struct *work) irm_id = card->irm_node->node_id; if (!card->irm_node->link_on) { - new_root_id = card->local_node->node_id; + new_root_id = local_node->node_id; fw_notify("IRM has link off, making local node (%02x) root.\n", new_root_id); goto pick_me; } bmd.lock.arg = cpu_to_be32(0x3f); - bmd.lock.data = cpu_to_be32(card->local_node->node_id); + bmd.lock.data = cpu_to_be32(local_node->node_id); spin_unlock_irqrestore(&card->lock, flags); @@ -267,12 +279,12 @@ fw_card_bm_work(struct work_struct *work) * Another bus reset happened. Just return, * the BM work has been rescheduled. */ - return; + goto out; } if (bmd.rcode == RCODE_COMPLETE && bmd.old != 0x3f) /* Somebody else is BM, let them do the work. */ - return; + goto out; spin_lock_irqsave(&card->lock, flags); if (bmd.rcode != RCODE_COMPLETE) { @@ -282,7 +294,7 @@ fw_card_bm_work(struct work_struct *work) * do a bus reset and pick the local node as * root, and thus, IRM. */ - new_root_id = card->local_node->node_id; + new_root_id = local_node->node_id; fw_notify("BM lock failed, making local node (%02x) root.\n", new_root_id); goto pick_me; @@ -295,7 +307,7 @@ fw_card_bm_work(struct work_struct *work) */ spin_unlock_irqrestore(&card->lock, flags); schedule_delayed_work(&card->work, DIV_ROUND_UP(HZ, 10)); - return; + goto out; } /* @@ -305,20 +317,20 @@ fw_card_bm_work(struct work_struct *work) */ card->bm_generation = generation; - if (root == NULL) { + if (root_device == NULL) { /* * Either link_on is false, or we failed to read the * config rom. In either case, pick another root. */ - new_root_id = card->local_node->node_id; - } else if (atomic_read(&root->state) != FW_DEVICE_RUNNING) { + new_root_id = local_node->node_id; + } else if (atomic_read(&root_device->state) != FW_DEVICE_RUNNING) { /* * If we haven't probed this device yet, bail out now * and let's try again once that's done. */ spin_unlock_irqrestore(&card->lock, flags); - return; - } else if (root->config_rom[2] & BIB_CMC) { + goto out; + } else if (root_device->config_rom[2] & BIB_CMC) { /* * FIXME: I suppose we should set the cmstr bit in the * STATE_CLEAR register of this node, as described in @@ -332,7 +344,7 @@ fw_card_bm_work(struct work_struct *work) * successfully read the config rom, but it's not * cycle master capable. */ - new_root_id = card->local_node->node_id; + new_root_id = local_node->node_id; } pick_me: @@ -341,8 +353,8 @@ fw_card_bm_work(struct work_struct *work) * the typically much larger 1394b beta repeater delays though. */ if (!card->beta_repeaters_present && - card->root_node->max_hops < ARRAY_SIZE(gap_count_table)) - gap_count = gap_count_table[card->root_node->max_hops]; + root_node->max_hops < ARRAY_SIZE(gap_count_table)) + gap_count = gap_count_table[root_node->max_hops]; else gap_count = 63; @@ -364,6 +376,11 @@ fw_card_bm_work(struct work_struct *work) fw_send_phy_config(card, new_root_id, generation, gap_count); fw_core_initiate_bus_reset(card, 1); } + out: + if (root_device) + fw_device_put(root_device); + fw_node_put(root_node); + fw_node_put(local_node); } static void diff --git a/drivers/firewire/fw-topology.c b/drivers/firewire/fw-topology.c index 172c186..e47bb04 100644 --- a/drivers/firewire/fw-topology.c +++ b/drivers/firewire/fw-topology.c @@ -383,6 +383,7 @@ void fw_destroy_nodes(struct fw_card *card) card->color++; if (card->local_node != NULL) for_each_fw_node(card, card->local_node, report_lost_node); + card->local_node = NULL; spin_unlock_irqrestore(&card->lock, flags); } commit f8436158b1d76e6842856048f287799468b56eb2 Author: Stefan Richter <stefanr@s5r6.in-berlin.de> Date: Tue Feb 26 23:30:02 2008 +0100 firewire: fw-sbp2: better fix for NULL pointer dereference in scsi_remove_device Patch "firewire: fw-sbp2: fix NULL pointer deref. in scsi_remove_device" had the unintended effect that firewire-sbp2 could not be unloaded anymore until all SBP-2 devices were unplugged. We now fix the NULL pointer bug by reacquiring a reference to the sdev instead of holding a reference to the sdev (and to the module) all the time. Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de> Tested-by: Jarod Wilson <jwilson@redhat.com> diff --git a/drivers/firewire/fw-sbp2.c b/drivers/firewire/fw-sbp2.c index 5259491..a093ac3 100644 --- a/drivers/firewire/fw-sbp2.c +++ b/drivers/firewire/fw-sbp2.c @@ -122,7 +122,6 @@ static const char sbp2_driver_name[] = "sbp2"; struct sbp2_logical_unit { struct sbp2_target *tgt; struct list_head link; - struct scsi_device *sdev; struct fw_address_handler address_handler; struct list_head orb_list; @@ -139,6 +138,7 @@ struct sbp2_logical_unit { int generation; int retries; struct delayed_work work; + bool has_sdev; bool blocked; }; @@ -751,20 +751,33 @@ static void sbp2_unblock(struct sbp2_target *tgt) scsi_unblock_requests(shost); } +static int sbp2_lun2int(u16 lun) +{ + struct scsi_lun eight_bytes_lun; + + memset(&eight_bytes_lun, 0, sizeof(eight_bytes_lun)); + eight_bytes_lun.scsi_lun[0] = (lun >> 8) & 0xff; + eight_bytes_lun.scsi_lun[1] = lun & 0xff; + + return scsilun_to_int(&eight_bytes_lun); +} + static void sbp2_release_target(struct kref *kref) { struct sbp2_target *tgt = container_of(kref, struct sbp2_target, kref); struct sbp2_logical_unit *lu, *next; struct Scsi_Host *shost = container_of((void *)tgt, struct Scsi_Host, hostdata[0]); + struct scsi_device *sdev; /* prevent deadlocks */ sbp2_unblock(tgt); list_for_each_entry_safe(lu, next, &tgt->lu_list, link) { - if (lu->sdev) { - scsi_remove_device(lu->sdev); - scsi_device_put(lu->sdev); + sdev = scsi_device_lookup(shost, 0, 0, sbp2_lun2int(lu->lun)); + if (sdev) { + scsi_remove_device(sdev); + scsi_device_put(sdev); } sbp2_send_management_orb(lu, tgt->node_id, lu->generation, SBP2_LOGOUT_REQUEST, lu->login_id, NULL); @@ -807,7 +820,6 @@ static void sbp2_login(struct work_struct *work) struct fw_device *device = fw_device(tgt->unit->device.parent); struct Scsi_Host *shost; struct scsi_device *sdev; - struct scsi_lun eight_bytes_lun; struct sbp2_login_response response; int generation, node_id, local_node_id; @@ -820,7 +832,7 @@ static void sbp2_login(struct work_struct *work) local_node_id = device->card->node_id; /* If this is a re-login attempt, log out, or we might be rejected. */ - if (lu->sdev) + if (lu->has_sdev) sbp2_send_management_orb(lu, device->node_id, generation, SBP2_LOGOUT_REQUEST, lu->login_id, NULL); @@ -859,7 +871,7 @@ static void sbp2_login(struct work_struct *work) sbp2_agent_reset(lu); /* This was a re-login. */ - if (lu->sdev) { + if (lu->has_sdev) { sbp2_cancel_orbs(lu); sbp2_conditionally_unblock(lu); goto out; @@ -868,13 +880,8 @@ static void sbp2_login(struct work_struct *work) if (lu->tgt->workarounds & SBP2_WORKAROUND_DELAY_INQUIRY) ssleep(SBP2_INQUIRY_DELAY); - memset(&eight_bytes_lun, 0, sizeof(eight_bytes_lun)); - eight_bytes_lun.scsi_lun[0] = (lu->lun >> 8) & 0xff; - eight_bytes_lun.scsi_lun[1] = lu->lun & 0xff; shost = container_of((void *)tgt, struct Scsi_Host, hostdata[0]); - - sdev = __scsi_add_device(shost, 0, 0, - scsilun_to_int(&eight_bytes_lun), lu); + sdev = __scsi_add_device(shost, 0, 0, sbp2_lun2int(lu->lun), lu); /* * FIXME: We are unable to perform reconnects while in sbp2_login(). * Therefore __scsi_add_device() will get into trouble if a bus reset @@ -896,7 +903,8 @@ static void sbp2_login(struct work_struct *work) } /* No error during __scsi_add_device() */ - lu->sdev = sdev; + lu->has_sdev = true; + scsi_device_put(sdev); sbp2_allow_block(lu); goto out; @@ -934,11 +942,11 @@ static int sbp2_add_logical_unit(struct sbp2_target *tgt, int lun_entry) return -ENOMEM; } - lu->tgt = tgt; - lu->sdev = NULL; - lu->lun = lun_entry & 0xffff; - lu->retries = 0; - lu->blocked = false; + lu->tgt = tgt; + lu->lun = lun_entry & 0xffff; + lu->retries = 0; + lu->has_sdev = false; + lu->blocked = false; ++tgt->dont_block; INIT_LIST_HEAD(&lu->orb_list); INIT_DELAYED_WORK(&lu->work, sbp2_login); -- Stefan Richter -=====-==--- --== ---=- http://arcgraph.de/sr/ ^ permalink raw reply [flat|nested] 18+ messages in thread
* [GIT PULL] FireWire updates 2008-03-02 12:47 ` Stefan Richter 2008-03-02 12:49 ` Stefan Richter @ 2008-03-14 18:07 ` Stefan Richter 2008-03-14 18:08 ` Stefan Richter 2008-03-20 17:28 ` [GIT PULL] FireWire update Stefan Richter 1 sibling, 2 replies; 18+ messages in thread From: Stefan Richter @ 2008-03-14 18:07 UTC (permalink / raw) To: Linus Torvalds, Andrew Morton; +Cc: linux-kernel, linux1394-devel Linus, please pull from the for-linus branch at git://git.kernel.org/pub/scm/linux/kernel/git/ieee1394/linux1394-2.6.git for-linus to receive the following updates for the firewire subsystem and one regression fix for the ieee1394 subsystem. They fix - I/O errors with firewire-sbp2 if the PCI bus became slightly busy, - I/O errors with some LSIL (ex Symbios) based targets (2.6.25-rc1 regression in case of sbp2), - asynchronous reception on systems with memory outside cache- coherent range, - machine check exception when resuming or reloading firewire-ohci on PowerMacs, PowerBooks, iBooks, - split transactions with big endian CPUs. There are also a small documentation update and sparse annotations in there. I consider this batch OK in this late -rc phase because the most intrusive change was easy to test on a range of machines, and the other changes are very local and were therefore also rather easy to verify and test. The PPC PMac parts are already known to work from ohci1394. I will follow up with a combined full log and diff. Diffstat and shortlog: drivers/firewire/Kconfig | 50 +++++-------- drivers/firewire/fw-ohci.c | 108 +++++++++++++++++++++------- drivers/firewire/fw-sbp2.c | 36 +++++++++- drivers/firewire/fw-topology.c | 3 +- drivers/firewire/fw-transaction.c | 2 +- drivers/firewire/fw-transaction.h | 6 +- drivers/ieee1394/sbp2.c | 5 ++ 7 files changed, 143 insertions(+), 67 deletions(-) Jarod Wilson (2): firewire: fw-sbp2: set single-phase retry_limit firewire: fw-ohci: use dma_alloc_coherent for ar_buffer Stefan Richter (9): firewire: endianess fix firewire: endianess annotations firewire: fw-ohci: PPC PMac platform code firewire: fw-ohci: Apple UniNorth 1st generation support firewire: warn on fatal condition in topology code firewire: update Kconfig help text firewire: fw-sbp2: fix for SYM13FW500 bridge (Datafab disk) ieee1394: sbp2: fix for SYM13FW500 bridge (Datafab disk) firewire: fw-ohci: shut up false compiler warning on PPC32 Thanks, -- Stefan Richter -=====-==--- --== -===- http://arcgraph.de/sr/ ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [GIT PULL] FireWire updates 2008-03-14 18:07 ` Stefan Richter @ 2008-03-14 18:08 ` Stefan Richter 2008-03-20 17:28 ` [GIT PULL] FireWire update Stefan Richter 1 sibling, 0 replies; 18+ messages in thread From: Stefan Richter @ 2008-03-14 18:08 UTC (permalink / raw) To: Linus Torvalds; +Cc: Andrew Morton, linux-kernel, linux1394-devel I wrote: > Linus, please pull from the for-linus branch at > > git://git.kernel.org/pub/scm/linux/kernel/git/ieee1394/linux1394-2.6.git for-linus > > to receive the following updates for the firewire subsystem and one > regression fix for the ieee1394 subsystem. ... > drivers/firewire/Kconfig | 50 +++++-------- > drivers/firewire/fw-ohci.c | 108 +++++++++++++++++++++------- > drivers/firewire/fw-sbp2.c | 36 +++++++++- > drivers/firewire/fw-topology.c | 3 +- > drivers/firewire/fw-transaction.c | 2 +- > drivers/firewire/fw-transaction.h | 6 +- > drivers/ieee1394/sbp2.c | 5 ++ > 7 files changed, 143 insertions(+), 67 deletions(-) > > Jarod Wilson (2): > firewire: fw-sbp2: set single-phase retry_limit > firewire: fw-ohci: use dma_alloc_coherent for ar_buffer > > Stefan Richter (9): > firewire: endianess fix > firewire: endianess annotations > firewire: fw-ohci: PPC PMac platform code > firewire: fw-ohci: Apple UniNorth 1st generation support > firewire: warn on fatal condition in topology code > firewire: update Kconfig help text > firewire: fw-sbp2: fix for SYM13FW500 bridge (Datafab disk) > ieee1394: sbp2: fix for SYM13FW500 bridge (Datafab disk) > firewire: fw-ohci: shut up false compiler warning on PPC32 commit f5101d58afc528c1d0c863fe03cd2d607766c4a1 Author: Stefan Richter <stefanr@s5r6.in-berlin.de> Date: Fri Mar 14 00:27:49 2008 +0100 firewire: fw-ohci: shut up false compiler warning on PPC32 Shut up "may be used uninitialised in this function" warnings due to PPC32's implementation of dma_alloc_coherent(). Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de> diff --git a/drivers/firewire/fw-ohci.c b/drivers/firewire/fw-ohci.c index fcf59fc..996d61f 100644 --- a/drivers/firewire/fw-ohci.c +++ b/drivers/firewire/fw-ohci.c @@ -281,7 +281,7 @@ static int ar_context_add_page(struct ar_context *ctx) { struct device *dev = ctx->ohci->card.device; struct ar_buffer *ab; - dma_addr_t ab_bus; + dma_addr_t uninitialized_var(ab_bus); size_t offset; ab = dma_alloc_coherent(dev, PAGE_SIZE, &ab_bus, GFP_ATOMIC); @@ -536,7 +536,7 @@ static int context_add_buffer(struct context *ctx) { struct descriptor_buffer *desc; - dma_addr_t bus_addr; + dma_addr_t uninitialized_var(bus_addr); int offset; /* @@ -1321,7 +1321,7 @@ ohci_set_config_rom(struct fw_card *card, u32 *config_rom, size_t length) unsigned long flags; int retval = -EBUSY; __be32 *next_config_rom; - dma_addr_t next_config_rom_bus; + dma_addr_t uninitialized_var(next_config_rom_bus); ohci = fw_ohci(card); commit bde1709aaa98f5004ab1580842c422be18eb4bc3 Author: Jarod Wilson <jwilson@redhat.com> Date: Wed Mar 12 17:43:26 2008 -0400 firewire: fw-ohci: use dma_alloc_coherent for ar_buffer Currently, we do nothing to guarantee we have a consistent DMA buffer for asynchronous receive packets. Rather than doing several sync's following a dma_map_single() to get consistent buffers, just switch to using dma_alloc_coherent(). Resolves constant buffer failures on my own x86_64 laptop w/4GB of RAM and likely to fix a number of other failures witnessed on x86_64 systems with 4GB of RAM or more. Signed-off-by: Jarod Wilson <jwilson@redhat.com> Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de> diff --git a/drivers/firewire/fw-ohci.c b/drivers/firewire/fw-ohci.c index eaa213e..fcf59fc 100644 --- a/drivers/firewire/fw-ohci.c +++ b/drivers/firewire/fw-ohci.c @@ -284,16 +284,10 @@ static int ar_context_add_page(struct ar_context *ctx) dma_addr_t ab_bus; size_t offset; - ab = (struct ar_buffer *) __get_free_page(GFP_ATOMIC); + ab = dma_alloc_coherent(dev, PAGE_SIZE, &ab_bus, GFP_ATOMIC); if (ab == NULL) return -ENOMEM; - ab_bus = dma_map_single(dev, ab, PAGE_SIZE, DMA_BIDIRECTIONAL); - if (dma_mapping_error(ab_bus)) { - free_page((unsigned long) ab); - return -ENOMEM; - } - memset(&ab->descriptor, 0, sizeof(ab->descriptor)); ab->descriptor.control = cpu_to_le16(DESCRIPTOR_INPUT_MORE | DESCRIPTOR_STATUS | @@ -304,8 +298,6 @@ static int ar_context_add_page(struct ar_context *ctx) ab->descriptor.res_count = cpu_to_le16(PAGE_SIZE - offset); ab->descriptor.branch_address = 0; - dma_sync_single_for_device(dev, ab_bus, PAGE_SIZE, DMA_BIDIRECTIONAL); - ctx->last_buffer->descriptor.branch_address = cpu_to_le32(ab_bus | 1); ctx->last_buffer->next = ab; ctx->last_buffer = ab; @@ -409,6 +401,7 @@ static void ar_context_tasklet(unsigned long data) if (d->res_count == 0) { size_t size, rest, offset; + dma_addr_t buffer_bus; /* * This descriptor is finished and we may have a @@ -417,9 +410,7 @@ static void ar_context_tasklet(unsigned long data) */ offset = offsetof(struct ar_buffer, data); - dma_unmap_single(ohci->card.device, - le32_to_cpu(ab->descriptor.data_address) - offset, - PAGE_SIZE, DMA_BIDIRECTIONAL); + buffer_bus = le32_to_cpu(ab->descriptor.data_address) - offset; buffer = ab; ab = ab->next; @@ -435,7 +426,8 @@ static void ar_context_tasklet(unsigned long data) while (buffer < end) buffer = handle_ar_packet(ctx, buffer); - free_page((unsigned long)buffer); + dma_free_coherent(ohci->card.device, PAGE_SIZE, + buffer, buffer_bus); ar_context_add_page(ctx); } else { buffer = ctx->pointer; commit 6e45ef4c7aeefbf97df748866cd1b24f73b86160 Author: Stefan Richter <stefanr@s5r6.in-berlin.de> Date: Tue Mar 11 22:32:52 2008 +0100 ieee1394: sbp2: fix for SYM13FW500 bridge (Datafab disk) Fix I/O errors due to SYM13FW500's inability to handle larger request sizes. Reported by Piergiorgio Sartor <piergiorgio.sartor@nexgo.de> for firewire-sbp2 in https://bugzilla.redhat.com/show_bug.cgi?id=436879 This fix is necessary because sbp2's default request size limit has been lifted since 2.6.25-rc1. Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de> Signed-off-by: Jarod Wilson <jwilson@redhat.com> diff --git a/drivers/ieee1394/sbp2.c b/drivers/ieee1394/sbp2.c index 9e2b196..f53f72d 100644 --- a/drivers/ieee1394/sbp2.c +++ b/drivers/ieee1394/sbp2.c @@ -376,6 +376,11 @@ static const struct { .model_id = SBP2_ROM_VALUE_WILDCARD, .workarounds = SBP2_WORKAROUND_128K_MAX_TRANS, }, + /* Datafab MD2-FW2 with Symbios/LSILogic SYM13FW500 bridge */ { + .firmware_revision = 0x002600, + .model_id = SBP2_ROM_VALUE_WILDCARD, + .workarounds = SBP2_WORKAROUND_128K_MAX_TRANS, + }, /* iPod 4th generation */ { .firmware_revision = 0x0a2700, .model_id = 0x000021, commit 2aa9ff7fc5bc41d4b77c2da02086259a86f3d472 Author: Stefan Richter <stefanr@s5r6.in-berlin.de> Date: Tue Mar 11 22:32:03 2008 +0100 firewire: fw-sbp2: fix for SYM13FW500 bridge (Datafab disk) Fix I/O errors due to SYM13FW500's inability to handle larger request sizes. Reported by Piergiorgio Sartor <piergiorgio.sartor@nexgo.de> in https://bugzilla.redhat.com/show_bug.cgi?id=436879 Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de> Signed-off-by: Jarod Wilson <jwilson@redhat.com> diff --git a/drivers/firewire/fw-sbp2.c b/drivers/firewire/fw-sbp2.c index 8bce569..62b4e47 100644 --- a/drivers/firewire/fw-sbp2.c +++ b/drivers/firewire/fw-sbp2.c @@ -331,6 +331,11 @@ static const struct { .model = ~0, .workarounds = SBP2_WORKAROUND_128K_MAX_TRANS, }, + /* Datafab MD2-FW2 with Symbios/LSILogic SYM13FW500 bridge */ { + .firmware_revision = 0x002600, + .model = ~0, + .workarounds = SBP2_WORKAROUND_128K_MAX_TRANS, + }, /* * There are iPods (2nd gen, 3rd gen) with model_id == 0, but commit 0a8da30dc7bd6828f42d9f0585367731f634a0c8 Author: Stefan Richter <stefanr@s5r6.in-berlin.de> Date: Sun Mar 9 00:27:20 2008 +0100 firewire: update Kconfig help text Remove some less necessary information, point out that video1394 and dv1394 should be blacklisted along with ohci1394. Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de> diff --git a/drivers/firewire/Kconfig b/drivers/firewire/Kconfig index fe9e768..25bdc2d 100644 --- a/drivers/firewire/Kconfig +++ b/drivers/firewire/Kconfig @@ -1,5 +1,3 @@ -# -*- shell-script -*- - comment "An alternative FireWire stack is available with EXPERIMENTAL=y" depends on EXPERIMENTAL=n @@ -21,27 +19,7 @@ config FIREWIRE NOTE: You should only build ONE of the stacks, unless you REALLY know what - you are doing. If you install both, you should configure them only as - modules rather than link them statically, and you should blacklist one - of the concurrent low-level drivers in /etc/modprobe.conf. Add either - - blacklist firewire-ohci - or - blacklist ohci1394 - - there depending on which driver you DON'T want to have auto-loaded. - You can optionally do the same with the other IEEE 1394/ FireWire - drivers. - - If you have an old modprobe which doesn't implement the blacklist - directive, use either - - install firewire-ohci /bin/true - or - install ohci1394 /bin/true - - and so on, depending on which modules you DON't want to have - auto-loaded. + you are doing. config FIREWIRE_OHCI tristate "Support for OHCI FireWire host controllers" @@ -57,8 +35,24 @@ config FIREWIRE_OHCI NOTE: - If you also build ohci1394 of the classic stack, blacklist either - ohci1394 or firewire-ohci to let hotplug load only the desired driver. + You should only build ohci1394 or firewire-ohci, but not both. + If you nevertheless want to install both, you should configure them + only as modules and blacklist the driver(s) which you don't want to + have auto-loaded. Add either + + blacklist firewire-ohci + or + blacklist ohci1394 + blacklist video1394 + blacklist dv1394 + + to /etc/modprobe.conf or /etc/modprobe.d/* and update modprobe.conf + depending on your distribution. The latter two modules should be + blacklisted together with ohci1394 because they depend on ohci1394. + + If you have an old modprobe which doesn't implement the blacklist + directive, use "install modulename /bin/true" for the modules to be + blacklisted. config FIREWIRE_SBP2 tristate "Support for storage devices (SBP-2 protocol driver)" @@ -75,9 +69,3 @@ config FIREWIRE_SBP2 You should also enable support for disks, CD-ROMs, etc. in the SCSI configuration section. - - NOTE: - - If you also build sbp2 of the classic stack, blacklist either sbp2 - or firewire-sbp2 to let hotplug load only the desired driver. - commit a2cdebe33f4c40a1bc7f66522303df89d5026cb4 Author: Stefan Richter <stefanr@s5r6.in-berlin.de> Date: Sat Mar 8 22:38:16 2008 +0100 firewire: warn on fatal condition in topology code If this ever happens to anybody, we want to have it in his log. Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de> diff --git a/drivers/firewire/fw-topology.c b/drivers/firewire/fw-topology.c index e47bb04..d2c7a3d 100644 --- a/drivers/firewire/fw-topology.c +++ b/drivers/firewire/fw-topology.c @@ -21,6 +21,7 @@ #include <linux/module.h> #include <linux/wait.h> #include <linux/errno.h> +#include <asm/bug.h> #include <asm/system.h> #include "fw-transaction.h" #include "fw-topology.h" @@ -424,8 +425,8 @@ update_tree(struct fw_card *card, struct fw_node *root) node1 = fw_node(list1.next); while (&node0->link != &list0) { + WARN_ON(node0->port_count != node1->port_count); - /* assert(node0->port_count == node1->port_count); */ if (node0->link_on && !node1->link_on) event = FW_NODE_LINK_OFF; else if (!node0->link_on && node1->link_on) commit 51f9dbef5be41f3ff6000c874741a3a357f9bad7 Author: Jarod Wilson <jwilson@redhat.com> Date: Fri Mar 7 01:43:01 2008 -0500 firewire: fw-sbp2: set single-phase retry_limit Per the SBP-2 specification, all SBP-2 target devices must have a BUSY_TIMEOUT register. Per the 1394-1995 specification, the retry_limt portion of the register should be set to 0x0 initially, and set on the target by a logged in initiator (i.e., a Linux host w/firewire controller(s)). Well, as it turns out, lots of devices these days have actually moved on to starting to implement SBP-3 compliance, which says that retry_limit should default to 0xf instead (yes, SBP-3 stomps directly on 1394-1995, oops). Prior to this change, the firewire driver stack didn't touch retry_limit, and any SBP-3 compliant device worked fine, while SBP-2 compliant ones were unable to retransmit when the host returned an ack_busy_X, which resulted in stalled out I/O, eventually causing the SCSI layer to give up and offline the device. The simple fix is for us to set retry_limit to 0xf in the register for all devices (which actually matches what the old ieee1394 stack did). Prior to this change, a hard disk behind an SBP-2 Prolific PL-3507 bridge chip would routinely encounter buffer I/O errors and wind up offlined by the SCSI layer. With this change, I've encountered zero I/O failures moving tens of GB of data around. Signed-off-by: Jarod Wilson <jwilson@redhat.com> Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de> diff --git a/drivers/firewire/fw-sbp2.c b/drivers/firewire/fw-sbp2.c index 03069a4..8bce569 100644 --- a/drivers/firewire/fw-sbp2.c +++ b/drivers/firewire/fw-sbp2.c @@ -173,6 +173,7 @@ struct sbp2_target { #define SBP2_ORB_TIMEOUT 2000U /* Timeout in ms */ #define SBP2_ORB_NULL 0x80000000 #define SBP2_MAX_SG_ELEMENT_LENGTH 0xf000 +#define SBP2_RETRY_LIMIT 0xf /* 15 retries */ #define SBP2_DIRECTION_TO_MEDIA 0x0 #define SBP2_DIRECTION_FROM_MEDIA 0x1 @@ -812,6 +813,30 @@ static void sbp2_target_put(struct sbp2_target *tgt) kref_put(&tgt->kref, sbp2_release_target); } +static void +complete_set_busy_timeout(struct fw_card *card, int rcode, + void *payload, size_t length, void *done) +{ + complete(done); +} + +static void sbp2_set_busy_timeout(struct sbp2_logical_unit *lu) +{ + struct fw_device *device = fw_device(lu->tgt->unit->device.parent); + DECLARE_COMPLETION_ONSTACK(done); + struct fw_transaction t; + static __be32 busy_timeout; + + /* FIXME: we should try to set dual-phase cycle_limit too */ + busy_timeout = cpu_to_be32(SBP2_RETRY_LIMIT); + + fw_send_request(device->card, &t, TCODE_WRITE_QUADLET_REQUEST, + lu->tgt->node_id, lu->generation, device->max_speed, + CSR_REGISTER_BASE + CSR_BUSY_TIMEOUT, &busy_timeout, + sizeof(busy_timeout), complete_set_busy_timeout, &done); + wait_for_completion(&done); +} + static void sbp2_reconnect(struct work_struct *work); static void sbp2_login(struct work_struct *work) @@ -864,10 +889,8 @@ static void sbp2_login(struct work_struct *work) fw_notify("%s: logged in to LUN %04x (%d retries)\n", tgt->bus_id, lu->lun, lu->retries); -#if 0 - /* FIXME: The linux1394 sbp2 does this last step. */ - sbp2_set_busy_timeout(scsi_id); -#endif + /* set appropriate retry limit(s) in BUSY_TIMEOUT register */ + sbp2_set_busy_timeout(lu); PREPARE_DELAYED_WORK(&lu->work, sbp2_reconnect); sbp2_agent_reset(lu); commit 11bf20ad028880a56689f086bfbabfd88b2af38b Author: Stefan Richter <stefanr@s5r6.in-berlin.de> Date: Sat Mar 1 02:47:15 2008 +0100 firewire: fw-ohci: Apple UniNorth 1st generation support Mostly copied from ohci1394.c. Necessary for some older Macs, e.g. PowerBook G3 Pismo and early PowerBook G4 Titanium. Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de> diff --git a/drivers/firewire/fw-ohci.c b/drivers/firewire/fw-ohci.c index 182be86..eaa213e 100644 --- a/drivers/firewire/fw-ohci.c +++ b/drivers/firewire/fw-ohci.c @@ -179,6 +179,7 @@ struct fw_ohci { int generation; int request_generation; u32 bus_seconds; + bool old_uninorth; /* * Spinlock for accessing fw_ohci data. Never call out of @@ -315,15 +316,22 @@ static int ar_context_add_page(struct ar_context *ctx) return 0; } +#if defined(CONFIG_PPC_PMAC) && defined(CONFIG_PPC32) +#define cond_le32_to_cpu(v) \ + (ohci->old_uninorth ? (__force __u32)(v) : le32_to_cpu(v)) +#else +#define cond_le32_to_cpu(v) le32_to_cpu(v) +#endif + static __le32 *handle_ar_packet(struct ar_context *ctx, __le32 *buffer) { struct fw_ohci *ohci = ctx->ohci; struct fw_packet p; u32 status, length, tcode; - p.header[0] = le32_to_cpu(buffer[0]); - p.header[1] = le32_to_cpu(buffer[1]); - p.header[2] = le32_to_cpu(buffer[2]); + p.header[0] = cond_le32_to_cpu(buffer[0]); + p.header[1] = cond_le32_to_cpu(buffer[1]); + p.header[2] = cond_le32_to_cpu(buffer[2]); tcode = (p.header[0] >> 4) & 0x0f; switch (tcode) { @@ -335,7 +343,7 @@ static __le32 *handle_ar_packet(struct ar_context *ctx, __le32 *buffer) break; case TCODE_READ_BLOCK_REQUEST : - p.header[3] = le32_to_cpu(buffer[3]); + p.header[3] = cond_le32_to_cpu(buffer[3]); p.header_length = 16; p.payload_length = 0; break; @@ -344,7 +352,7 @@ static __le32 *handle_ar_packet(struct ar_context *ctx, __le32 *buffer) case TCODE_READ_BLOCK_RESPONSE: case TCODE_LOCK_REQUEST: case TCODE_LOCK_RESPONSE: - p.header[3] = le32_to_cpu(buffer[3]); + p.header[3] = cond_le32_to_cpu(buffer[3]); p.header_length = 16; p.payload_length = p.header[3] >> 16; break; @@ -361,7 +369,7 @@ static __le32 *handle_ar_packet(struct ar_context *ctx, __le32 *buffer) /* FIXME: What to do about evt_* errors? */ length = (p.header_length + p.payload_length + 3) / 4; - status = le32_to_cpu(buffer[length]); + status = cond_le32_to_cpu(buffer[length]); p.ack = ((status >> 16) & 0x1f) - 16; p.speed = (status >> 21) & 0x7; @@ -1026,13 +1034,14 @@ static void bus_reset_tasklet(unsigned long data) */ self_id_count = (reg_read(ohci, OHCI1394_SelfIDCount) >> 3) & 0x3ff; - generation = (le32_to_cpu(ohci->self_id_cpu[0]) >> 16) & 0xff; + generation = (cond_le32_to_cpu(ohci->self_id_cpu[0]) >> 16) & 0xff; rmb(); for (i = 1, j = 0; j < self_id_count; i += 2, j++) { if (ohci->self_id_cpu[i] != ~ohci->self_id_cpu[i + 1]) fw_error("inconsistent self IDs\n"); - ohci->self_id_buffer[j] = le32_to_cpu(ohci->self_id_cpu[i]); + ohci->self_id_buffer[j] = + cond_le32_to_cpu(ohci->self_id_cpu[i]); } rmb(); @@ -2082,6 +2091,10 @@ pci_probe(struct pci_dev *dev, const struct pci_device_id *ent) pci_write_config_dword(dev, OHCI1394_PCI_HCI_Control, 0); pci_set_drvdata(dev, ohci); +#if defined(CONFIG_PPC_PMAC) && defined(CONFIG_PPC32) + ohci->old_uninorth = dev->vendor == PCI_VENDOR_ID_APPLE && + dev->device == PCI_DEVICE_ID_APPLE_UNI_N_FW; +#endif spin_lock_init(&ohci->lock); tasklet_init(&ohci->bus_reset_tasklet, commit ea8d006b91ac58ec5a0862d09e0b629db399517f Author: Stefan Richter <stefanr@s5r6.in-berlin.de> Date: Sat Mar 1 02:42:56 2008 +0100 firewire: fw-ohci: PPC PMac platform code Copied from ohci1394.c. This code is necessary to prevent machine check exceptions when reloading or resuming the driver. Tested on a 1st generation PowerBook G4 Titanium, which also needs the pci_probe() hunk. Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de> I was able to reproduce the system exception on resume with a 3rd-gen Titanium PowerBook G4 667, and this patch does let the system resume successfully now. Not quite clear if there was possibly an updated version coming using pci_enable_device() instead of the pair of pmac_call_feature() calls, but either way, this is a definite must-have, at least for older ppc macs -- my Aluminum PowerBook G4/1.67 suspends and resumes without this patch just fine. Signed-off-by: Jarod Wilson <jwilson@redhat.com> Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> diff --git a/drivers/firewire/fw-ohci.c b/drivers/firewire/fw-ohci.c index f9440a7..182be86 100644 --- a/drivers/firewire/fw-ohci.c +++ b/drivers/firewire/fw-ohci.c @@ -33,6 +33,10 @@ #include <asm/page.h> #include <asm/system.h> +#ifdef CONFIG_PPC_PMAC +#include <asm/pmac_feature.h> +#endif + #include "fw-ohci.h" #include "fw-transaction.h" @@ -2048,6 +2052,18 @@ pci_probe(struct pci_dev *dev, const struct pci_device_id *ent) int err; size_t size; +#ifdef CONFIG_PPC_PMAC + /* Necessary on some machines if fw-ohci was loaded/ unloaded before */ + if (machine_is(powermac)) { + struct device_node *ofn = pci_device_to_OF_node(dev); + + if (ofn) { + pmac_call_feature(PMAC_FTR_1394_CABLE_POWER, ofn, 0, 1); + pmac_call_feature(PMAC_FTR_1394_ENABLE, ofn, 0, 1); + } + } +#endif /* CONFIG_PPC_PMAC */ + ohci = kzalloc(sizeof(*ohci), GFP_KERNEL); if (ohci == NULL) { fw_error("Could not malloc fw_ohci data.\n"); @@ -2182,6 +2198,19 @@ static void pci_remove(struct pci_dev *dev) pci_disable_device(dev); fw_card_put(&ohci->card); +#ifdef CONFIG_PPC_PMAC + /* On UniNorth, power down the cable and turn off the chip clock + * to save power on laptops */ + if (machine_is(powermac)) { + struct device_node *ofn = pci_device_to_OF_node(dev); + + if (ofn) { + pmac_call_feature(PMAC_FTR_1394_ENABLE, ofn, 0, 0); + pmac_call_feature(PMAC_FTR_1394_CABLE_POWER, ofn, 0, 0); + } + } +#endif /* CONFIG_PPC_PMAC */ + fw_notify("Removed fw-ohci device.\n"); } @@ -2202,6 +2231,16 @@ static int pci_suspend(struct pci_dev *pdev, pm_message_t state) if (err) fw_error("pci_set_power_state failed with %d\n", err); +/* PowerMac suspend code comes last */ +#ifdef CONFIG_PPC_PMAC + if (machine_is(powermac)) { + struct device_node *ofn = pci_device_to_OF_node(pdev); + + if (ofn) + pmac_call_feature(PMAC_FTR_1394_ENABLE, ofn, 0, 0); + } +#endif /* CONFIG_PPC_PMAC */ + return 0; } @@ -2210,6 +2249,16 @@ static int pci_resume(struct pci_dev *pdev) struct fw_ohci *ohci = pci_get_drvdata(pdev); int err; +/* PowerMac resume code comes first */ +#ifdef CONFIG_PPC_PMAC + if (machine_is(powermac)) { + struct device_node *ofn = pci_device_to_OF_node(pdev); + + if (ofn) + pmac_call_feature(PMAC_FTR_1394_ENABLE, ofn, 0, 1); + } +#endif /* CONFIG_PPC_PMAC */ + pci_set_power_state(pdev, PCI_D0); pci_restore_state(pdev); err = pci_enable_device(pdev); commit efbf390a2d940315efff174455243e61f23c03b9 Author: Stefan Richter <stefanr@s5r6.in-berlin.de> Date: Sat Feb 23 12:24:57 2008 +0100 firewire: endianess annotations Kills warnings from 'make C=1 CHECKFLAGS="-D__CHECK_ENDIAN__" modules': drivers/firewire/fw-transaction.c:771:10: warning: incorrect type in assignment (different base types) drivers/firewire/fw-transaction.c:771:10: expected unsigned int [unsigned] [usertype] <noident> drivers/firewire/fw-transaction.c:771:10: got restricted unsigned int [usertype] <noident> drivers/firewire/fw-transaction.h:93:10: warning: incorrect type in assignment (different base types) drivers/firewire/fw-transaction.h:93:10: expected unsigned int [unsigned] [usertype] <noident> drivers/firewire/fw-transaction.h:93:10: got restricted unsigned int [usertype] <noident> drivers/firewire/fw-ohci.c:1490:8: warning: restricted degrades to integer drivers/firewire/fw-ohci.c:1490:35: warning: restricted degrades to integer drivers/firewire/fw-ohci.c:1516:5: warning: cast to restricted type Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de> Signed-off-by: Jarod Wilson <jwilson@redhat.com> diff --git a/drivers/firewire/fw-ohci.c b/drivers/firewire/fw-ohci.c index ed4e357..f9440a7 100644 --- a/drivers/firewire/fw-ohci.c +++ b/drivers/firewire/fw-ohci.c @@ -1487,7 +1487,7 @@ static int handle_ir_dualbuffer_packet(struct context *context, void *p, *end; int i; - if (db->first_res_count > 0 && db->second_res_count > 0) { + if (db->first_res_count != 0 && db->second_res_count != 0) { if (ctx->excess_bytes <= le16_to_cpu(db->second_req_count)) { /* This descriptor isn't done yet, stop iteration. */ return 0; @@ -1513,7 +1513,7 @@ static int handle_ir_dualbuffer_packet(struct context *context, memcpy(ctx->header + i + 4, p + 8, ctx->base.header_size - 4); i += ctx->base.header_size; ctx->excess_bytes += - (le32_to_cpu(*(u32 *)(p + 4)) >> 16) & 0xffff; + (le32_to_cpu(*(__le32 *)(p + 4)) >> 16) & 0xffff; p += ctx->base.header_size + 4; } ctx->header_length = i; diff --git a/drivers/firewire/fw-transaction.c b/drivers/firewire/fw-transaction.c index 7fcc59d..99529e5 100644 --- a/drivers/firewire/fw-transaction.c +++ b/drivers/firewire/fw-transaction.c @@ -751,7 +751,7 @@ handle_topology_map(struct fw_card *card, struct fw_request *request, void *payload, size_t length, void *callback_data) { int i, start, end; - u32 *map; + __be32 *map; if (!TCODE_IS_READ_REQUEST(tcode)) { fw_send_response(card, request, RCODE_TYPE_ERROR); diff --git a/drivers/firewire/fw-transaction.h b/drivers/firewire/fw-transaction.h index 09cb728..a43bb22 100644 --- a/drivers/firewire/fw-transaction.h +++ b/drivers/firewire/fw-transaction.h @@ -86,12 +86,12 @@ static inline void fw_memcpy_from_be32(void *_dst, void *_src, size_t size) { - u32 *dst = _dst; - u32 *src = _src; + u32 *dst = _dst; + __be32 *src = _src; int i; for (i = 0; i < size / 4; i++) - dst[i] = cpu_to_be32(src[i]); + dst[i] = be32_to_cpu(src[i]); } static inline void commit 25df287dc7434edf8dda10ce85e43f88e834a494 Author: Stefan Richter <stefanr@s5r6.in-berlin.de> Date: Sat Feb 23 12:24:17 2008 +0100 firewire: endianess fix The generation of incoming requests was filled in in wrong byte order on machines with big endian CPU. Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de> Signed-off-by: Jarod Wilson <jwilson@redhat.com> diff --git a/drivers/firewire/fw-ohci.c b/drivers/firewire/fw-ohci.c index 7ebad3c..ed4e357 100644 --- a/drivers/firewire/fw-ohci.c +++ b/drivers/firewire/fw-ohci.c @@ -375,7 +375,7 @@ static __le32 *handle_ar_packet(struct ar_context *ctx, __le32 *buffer) */ if (p.ack + 16 == 0x09) - ohci->request_generation = (buffer[2] >> 16) & 0xff; + ohci->request_generation = (p.header[2] >> 16) & 0xff; else if (ctx == &ohci->ar_request_ctx) fw_core_handle_request(&ohci->card, &p); else -- Stefan Richter -=====-==--- --== -===- http://arcgraph.de/sr/ ^ permalink raw reply [flat|nested] 18+ messages in thread
* [GIT PULL] FireWire update 2008-03-14 18:07 ` Stefan Richter 2008-03-14 18:08 ` Stefan Richter @ 2008-03-20 17:28 ` Stefan Richter 2008-03-27 20:37 ` Stefan Richter 1 sibling, 1 reply; 18+ messages in thread From: Stefan Richter @ 2008-03-20 17:28 UTC (permalink / raw) To: Linus Torvalds, Andrew Morton; +Cc: linux-kernel, linux1394-devel Linus, please pull from the for-linus branch at git://git.kernel.org/pub/scm/linux/kernel/git/ieee1394/linux1394-2.6.git for-linus to receive the following update for the firewire subsystem. drivers/firewire/fw-transaction.c | 6 ++++++ 1 files changed, 6 insertions(+), 0 deletions(-) Stefan Richter (1): firewire: fix panic in handle_at_packet commit 10a4c735515a5afc317abe4d697a4c95f6d9d764 Author: Stefan Richter <stefanr@s5r6.in-berlin.de> Date: Sun Mar 16 00:56:41 2008 +0100 firewire: fix panic in handle_at_packet This fixes a use-after-free bug in the handling of split transactions. The AT DMA handler of the request was occasionally executed after the AR DMA handler of the response. The AT DMA handler then accessed an already freed packet. Reported by Johannes Berg. http://bugzilla.kernel.org/show_bug.cgi?id=9617 Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de> Tested-by: Johannes Berg <johannes@sipsolutions.net> Signed-off-by: Jarod Wilson <jwilson@redhat.com> diff --git a/drivers/firewire/fw-transaction.c b/drivers/firewire/fw-transaction.c index 99529e5..e6f1bda 100644 --- a/drivers/firewire/fw-transaction.c +++ b/drivers/firewire/fw-transaction.c @@ -736,6 +736,12 @@ fw_core_handle_response(struct fw_card *card, struct fw_packet *p) break; } + /* + * The response handler may be executed while the request handler + * is still pending. Cancel the request handler. + */ + card->driver->cancel_packet(card, &t->packet); + t->callback(card, rcode, data, data_length, t->callback_data); } EXPORT_SYMBOL(fw_core_handle_response); Thanks, -- Stefan Richter -=====-==--- --== =-=-- http://arcgraph.de/sr/ ^ permalink raw reply [flat|nested] 18+ messages in thread
* [GIT PULL] FireWire update 2008-03-20 17:28 ` [GIT PULL] FireWire update Stefan Richter @ 2008-03-27 20:37 ` Stefan Richter 2008-03-31 8:46 ` Stefan Richter 0 siblings, 1 reply; 18+ messages in thread From: Stefan Richter @ 2008-03-27 20:37 UTC (permalink / raw) To: Linus Torvalds, Andrew Morton; +Cc: linux-kernel, linux1394-devel Linus, please pull from the for-linus branch at git://git.kernel.org/pub/scm/linux/kernel/git/ieee1394/linux1394-2.6.git for-linus to receive the following update for the firewire subsystem. drivers/firewire/fw-ohci.c | 9 +++++---- 1 files changed, 5 insertions(+), 4 deletions(-) Jarod Wilson (1): firewire: fw-ohci: plug dma memory leak in AR handler commit 6b84236d37ef602d1e4f52b27162c20394e83359 Author: Jarod Wilson <jwilson@redhat.com> Date: Tue Mar 25 16:47:16 2008 -0400 firewire: fw-ohci: plug dma memory leak in AR handler There's an ugly little memory leak in firewire-ohci's ar_context_tasklet(), where we're not freeing up some of the memory we use for each ar_buffer, due to a moving pointer. The problem has been there for a while, but didn't get noticed until after converting the AR routines over to use coherent DMA and I started running into I/O stall- outs with the following message output repeatedly to the console: PCI-DMA: Out of IOMMU space for 53248 bytes at device 0000:04:09.0 Plugging this leak is definitely necessary, but unfortunately, isn't the entire answer to my problem, it only increases the amount of I/O that I can do before hitting the problem. Still working on tracking down the root cause.. Signed-off-by: Jarod Wilson <jwilson@redhat.com> Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de> diff --git a/drivers/firewire/fw-ohci.c b/drivers/firewire/fw-ohci.c index 996d61f..ca6d51e 100644 --- a/drivers/firewire/fw-ohci.c +++ b/drivers/firewire/fw-ohci.c @@ -401,7 +401,8 @@ static void ar_context_tasklet(unsigned long data) if (d->res_count == 0) { size_t size, rest, offset; - dma_addr_t buffer_bus; + dma_addr_t start_bus; + void *start; /* * This descriptor is finished and we may have a @@ -410,9 +411,9 @@ static void ar_context_tasklet(unsigned long data) */ offset = offsetof(struct ar_buffer, data); - buffer_bus = le32_to_cpu(ab->descriptor.data_address) - offset; + start = buffer = ab; + start_bus = le32_to_cpu(ab->descriptor.data_address) - offset; - buffer = ab; ab = ab->next; d = &ab->descriptor; size = buffer + PAGE_SIZE - ctx->pointer; @@ -427,7 +428,7 @@ static void ar_context_tasklet(unsigned long data) buffer = handle_ar_packet(ctx, buffer); dma_free_coherent(ohci->card.device, PAGE_SIZE, - buffer, buffer_bus); + start, start_bus); ar_context_add_page(ctx); } else { buffer = ctx->pointer; Thanks, -- Stefan Richter -=====-==--- --== ==-== http://arcgraph.de/sr/ ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [GIT PULL] FireWire update 2008-03-27 20:37 ` Stefan Richter @ 2008-03-31 8:46 ` Stefan Richter 0 siblings, 0 replies; 18+ messages in thread From: Stefan Richter @ 2008-03-31 8:46 UTC (permalink / raw) To: Linus Torvalds; +Cc: Andrew Morton, linux-kernel, linux1394-devel I wrote: > Linus, please pull from the for-linus branch at > > git://git.kernel.org/pub/scm/linux/kernel/git/ieee1394/linux1394-2.6.git for-linus > > to receive the following update for the firewire subsystem. > > drivers/firewire/fw-ohci.c | 9 +++++---- > 1 files changed, 5 insertions(+), 4 deletions(-) > > Jarod Wilson (1): > firewire: fw-ohci: plug dma memory leak in AR handler Ping. This stops firewire-ohci from feeding random arguments to dma_free_coherent(). -- Stefan Richter -=====-==--- --== ===== http://arcgraph.de/sr/ ^ permalink raw reply [flat|nested] 18+ messages in thread
* [git pull] FireWire update @ 2011-07-11 13:42 Stefan Richter 0 siblings, 0 replies; 18+ messages in thread From: Stefan Richter @ 2011-07-11 13:42 UTC (permalink / raw) To: Linus Torvalds, Andrew Morton; +Cc: linux-kernel, linux1394-devel Linus, please pull from the movieboard branch at git://git.kernel.org/pub/scm/linux/kernel/git/ieee1394/linux1394-2.6.git movieboard to receive the following quirks workaround for the IEEE 1394 (FireWire) subsystem. This is a hopefully temporary bandaid for a comparably rare piece of hardware. Thanks. Stefan Richter (1): firewire: ohci: do not bind to Pinnacle cards, avert panic drivers/firewire/ohci.c | 6 ++++++ 1 files changed, 6 insertions(+), 0 deletions(-) commit 7f7e37115a8b6724f26d0637a04e1d35e3c59717 Author: Stefan Richter <stefanr@s5r6.in-berlin.de> Date: Sun Jul 10 00:23:03 2011 +0200 firewire: ohci: do not bind to Pinnacle cards, avert panic When firewire-ohci is bound to a Pinnacle MovieBoard, eventually a "Register access failure" is logged and an interrupt storm or a kernel panic happens. https://bugzilla.kernel.org/show_bug.cgi?id=36622 Until this is sorted out (if that is going to succeed at all), let's just prevent firewire-ohci from touching these devices. Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de> Cc: <stable@kernel.org> diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c index 438e6c8..ebb8973 100644 --- a/drivers/firewire/ohci.c +++ b/drivers/firewire/ohci.c @@ -264,6 +264,7 @@ static char ohci_driver_name[] = KBUILD_MODNAME; #define PCI_DEVICE_ID_AGERE_FW643 0x5901 #define PCI_DEVICE_ID_JMICRON_JMB38X_FW 0x2380 #define PCI_DEVICE_ID_TI_TSB12LV22 0x8009 +#define PCI_VENDOR_ID_PINNACLE_SYSTEMS 0x11bd #define QUIRK_CYCLE_TIMER 1 #define QUIRK_RESET_PACKET 2 @@ -3190,6 +3191,11 @@ static int __devinit pci_probe(struct pci_dev *dev, int i, err; size_t size; + if (dev->vendor == PCI_VENDOR_ID_PINNACLE_SYSTEMS) { + dev_err(&dev->dev, "Pinnacle MovieBoard is not yet supported\n"); + return -ENOSYS; + } + ohci = kzalloc(sizeof(*ohci), GFP_KERNEL); if (ohci == NULL) { err = -ENOMEM; -- Stefan Richter -=====-==-== -=== -=-== http://arcgraph.de/sr/ ^ permalink raw reply [flat|nested] 18+ messages in thread
* [GIT PULL] firewire update @ 2008-06-27 19:05 Stefan Richter 0 siblings, 0 replies; 18+ messages in thread From: Stefan Richter @ 2008-06-27 19:05 UTC (permalink / raw) To: Linus Torvalds, Andrew Morton; +Cc: linux-kernel, linux1394-devel Linus, please pull from the for-linus branch at git://git.kernel.org/pub/scm/linux/kernel/git/ieee1394/linux1394-2.6.git for-linus to receive the following one-liner. Richard Sharpe (1): firewire: fw-sbp2: fix parsing of logical unit directories drivers/firewire/fw-sbp2.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) commit 0e3e2eabf4fbc0162e1f1eb4fd90cb3e9513a554 Author: Richard Sharpe <realrichardsharpe@gmail.com> Date: Tue Jun 24 19:11:13 2008 -0700 firewire: fw-sbp2: fix parsing of logical unit directories There is a small off-by-one bug in firewire-sbp2. This causes problems when a device exports multiple LUN Directories. I found it when trying to talk to a SONY DVD Jukebox. Signed-off-by: Richard Sharpe <realrichardsharpe@gmail.com> Acked-by: Kristian Høgsberg <krh@redhat.com> Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de> (op. order, changelog) diff --git a/drivers/firewire/fw-sbp2.c b/drivers/firewire/fw-sbp2.c index b2458bb..227d2e0 100644 --- a/drivers/firewire/fw-sbp2.c +++ b/drivers/firewire/fw-sbp2.c @@ -1051,7 +1051,8 @@ static int sbp2_scan_unit_dir(struct sbp2_target *tgt, u32 *directory, break; case SBP2_CSR_LOGICAL_UNIT_DIRECTORY: - if (sbp2_scan_logical_unit_dir(tgt, ci.p + value) < 0) + /* Adjust for the increment in the iterator */ + if (sbp2_scan_logical_unit_dir(tgt, ci.p - 1 + value) < 0) return -ENOMEM; break; } -- Stefan Richter -=====-==--- -==- ==-== http://arcgraph.de/sr/ ^ permalink raw reply [flat|nested] 18+ messages in thread
* [GIT PULL] FireWire update @ 2007-12-10 21:25 Stefan Richter [not found] ` <59ad55d30712110818u2717b329j80778ec7cc290988@mail.gmail.com> 0 siblings, 1 reply; 18+ messages in thread From: Stefan Richter @ 2007-12-10 21:25 UTC (permalink / raw) To: Linus Torvalds; +Cc: Andrew Morton, linux-kernel, linux1394-devel Linus, please pull from the for-linus branch at git://git.kernel.org/pub/scm/linux/kernel/git/ieee1394/linux1394-2.6.git for-linus to receive the following FireWire subsystem update. This considerably enhances compatibility of the new firewire-ohci driver with a number of controllers. It shrinks the list of chips with trouble with isochronous reception to VIA VT6306 and some variants of VT6307. The patch is somewhat big for this late -rc phase, and it has so far only surfaced in 2.6.24-rc4-mm1 and in recent Fedora test kernels. But the author and I did a lot of tests with as much previously working chips as we could get our hands on (some more than listed below) to make sure that there is no regression. drivers/firewire/fw-ohci.c | 175 +++++++++++++++++++++++++++++++++++++++----- 1 files changed, 155 insertions(+), 20 deletions(-) Jarod Wilson (1): firewire: OHCI 1.0 Isochronous Receive support Full log and diff: commit a186b4a6b22fdc96a1ed63da483d267b5d00839e Author: Jarod Wilson <jwilson@redhat.com> Date: Mon Dec 3 13:43:12 2007 -0500 firewire: OHCI 1.0 Isochronous Receive support Third rendition of FireWire OHCI 1.0 Isochronous Receive support, using a zer-copy method similar to OHCI 1.1 which puts the IR data payload directly into the userspace buffer. The zero-copy implementation eliminates the video artifacts, audio popping, and buffer underrun problems seen with version 1 of this patch, as well as fixing a regression in OHCI 1.1 support introduced by version 2 of this patch. Successfully tested in OHCI 1.1 mode on the following chipsets: - NEC uPD72847 (rev 01), OHCI 1.1 (PCI) - Ti XIO2200(A) (rev 01), OHCI 1.1 (PCIe) - Ti TSB41AB2 (rev 01), OHCI 1.1 (PCI on SB Audigy) - Apple UniNorth 2 (rev 81), OHCI 1.1 (PowerBook G4 onboard) Successfully tested in OHCI 1.0 mode on the following chipsets: - Agere FW323 (rev 06), OHCI 1.0 (Mac Mini onboard) - Agere FW323 (rev 06), OHCI 1.0 (PCI) - Via VT6306 (rev 46), OHCI 1.0 (PCI) - NEC OrangeLink (rev 01), OHCI 1.0 (PCI) - NEC uPD72847 (rev 01), OHCI 1.1 (PCI) - Ti XIO2200(A) (rev 01), OHCI 1.1 (PCIe) The bulk of testing was done in an x86_64 system, but was also successfully sanity-tested on other systems, including a PPC(32) PowerBook G4 and an i686 EPIA M10k. Crude benchmarking (watching top during capture) puts the cpu utilization during capture on the EPIA's 1GHz Via C3 processor around 13%, which is down from 30% with the v1 code. Some implementation details: To maintain the same userspace API as dual-buffer mode, we set up two descriptors for every incoming packet. The first is an INPUT_MORE descriptor, pointing to a buffer large enough to hold just the packet's iso headers, immediately followed by an INPUT_LAST descriptor, pointing to a chunk of the userspace buffer big enough for the packet's data payload. With this setup, each incoming packet fills in these two descriptors in a manner that very closely emulates dual-buffer receive, to the point where the bulk of the handle_ir_* code is now identical between the two (and probably primed for some restructuring to share code between them). The only caveat I have at the moment is that neither of my OHCI 1.0 Via VT6307-based FireWire controllers work particularly well with this code for reasons I have yet to figure out. Signed-off-by: Jarod Wilson <jwilson@redhat.com> Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de> diff --git a/drivers/firewire/fw-ohci.c b/drivers/firewire/fw-ohci.c index c9b9081..436a855 100644 --- a/drivers/firewire/fw-ohci.c +++ b/drivers/firewire/fw-ohci.c @@ -437,6 +437,21 @@ static void ar_context_run(struct ar_context *ctx) flush_writes(ctx->ohci); } +static struct descriptor * +find_branch_descriptor(struct descriptor *d, int z) +{ + int b, key; + + b = (le16_to_cpu(d->control) & DESCRIPTOR_BRANCH_ALWAYS) >> 2; + key = (le16_to_cpu(d->control) & DESCRIPTOR_KEY_IMMEDIATE) >> 8; + + /* figure out which descriptor the branch address goes in */ + if (z == 2 && (b == 3 || key == 2)) + return d; + else + return d + z - 1; +} + static void context_tasklet(unsigned long data) { struct context *ctx = (struct context *) data; @@ -455,7 +470,7 @@ static void context_tasklet(unsigned long data) address = le32_to_cpu(last->branch_address); z = address & 0xf; d = ctx->buffer + (address - ctx->buffer_bus) / sizeof(*d); - last = (z == 2) ? d : d + z - 1; + last = find_branch_descriptor(d, z); if (!ctx->callback(ctx, d, last)) break; @@ -566,7 +581,7 @@ static void context_append(struct context *ctx, ctx->head_descriptor = d + z + extra; ctx->prev_descriptor->branch_address = cpu_to_le32(d_bus | z); - ctx->prev_descriptor = z == 2 ? d : d + z - 1; + ctx->prev_descriptor = find_branch_descriptor(d, z); dma_sync_single_for_device(ctx->ohci->card.device, ctx->buffer_bus, ctx->buffer_size, DMA_TO_DEVICE); @@ -655,7 +670,7 @@ at_context_queue_packet(struct context *ctx, struct fw_packet *packet) driver_data = (struct driver_data *) &d[3]; driver_data->packet = packet; packet->driver_data = driver_data; - + if (packet->payload_length > 0) { payload_bus = dma_map_single(ohci->card.device, packet->payload, @@ -903,7 +918,7 @@ at_context_transmit(struct context *ctx, struct fw_packet *packet) if (retval < 0) packet->callback(packet, &ctx->ohci->card, packet->ack); - + } static void bus_reset_tasklet(unsigned long data) @@ -1431,6 +1446,57 @@ static int handle_ir_dualbuffer_packet(struct context *context, return 1; } +static int handle_ir_packet_per_buffer(struct context *context, + struct descriptor *d, + struct descriptor *last) +{ + struct iso_context *ctx = + container_of(context, struct iso_context, context); + struct descriptor *pd = d + 1; + __le32 *ir_header; + size_t header_length; + void *p, *end; + int i, z; + + if (pd->res_count == pd->req_count) + /* Descriptor(s) not done yet, stop iteration */ + return 0; + + header_length = le16_to_cpu(d->req_count); + + i = ctx->header_length; + z = le32_to_cpu(pd->branch_address) & 0xf; + p = d + z; + end = p + header_length; + + while (p < end && i + ctx->base.header_size <= PAGE_SIZE) { + /* + * The iso header is byteswapped to little endian by + * the controller, but the remaining header quadlets + * are big endian. We want to present all the headers + * as big endian, so we have to swap the first quadlet. + */ + *(u32 *) (ctx->header + i) = __swab32(*(u32 *) (p + 4)); + memcpy(ctx->header + i + 4, p + 8, ctx->base.header_size - 4); + i += ctx->base.header_size; + p += ctx->base.header_size + 4; + } + + ctx->header_length = i; + + if (le16_to_cpu(pd->control) & DESCRIPTOR_IRQ_ALWAYS) { + ir_header = (__le32 *) (d + z); + ctx->base.callback(&ctx->base, + le32_to_cpu(ir_header[0]) & 0xffff, + ctx->header_length, ctx->header, + ctx->base.callback_data); + ctx->header_length = 0; + } + + + return 1; +} + static int handle_it_packet(struct context *context, struct descriptor *d, struct descriptor *last) @@ -1466,14 +1532,12 @@ ohci_allocate_iso_context(struct fw_card *card, int type, size_t header_size) } else { mask = &ohci->ir_context_mask; list = ohci->ir_context_list; - callback = handle_ir_dualbuffer_packet; + if (ohci->version >= OHCI_VERSION_1_1) + callback = handle_ir_dualbuffer_packet; + else + callback = handle_ir_packet_per_buffer; } - /* FIXME: We need a fallback for pre 1.1 OHCI. */ - if (callback == handle_ir_dualbuffer_packet && - ohci->version < OHCI_VERSION_1_1) - return ERR_PTR(-ENOSYS); - spin_lock_irqsave(&ohci->lock, flags); index = ffs(*mask) - 1; if (index >= 0) @@ -1532,7 +1596,9 @@ static int ohci_start_iso(struct fw_iso_context *base, context_run(&ctx->context, match); } else { index = ctx - ohci->ir_context_list; - control = IR_CONTEXT_DUAL_BUFFER_MODE | IR_CONTEXT_ISOCH_HEADER; + control = IR_CONTEXT_ISOCH_HEADER; + if (ohci->version >= OHCI_VERSION_1_1) + control |= IR_CONTEXT_DUAL_BUFFER_MODE; match = (tags << 28) | (sync << 8) | ctx->base.channel; if (cycle >= 0) { match |= (cycle & 0x07fff) << 12; @@ -1738,7 +1804,6 @@ ohci_queue_iso_receive_dualbuffer(struct fw_iso_context *base, offset = payload & ~PAGE_MASK; rest = p->payload_length; - /* FIXME: OHCI 1.0 doesn't support dual buffer receive */ /* FIXME: make packet-per-buffer/dual-buffer a context option */ while (rest > 0) { d = context_get_descriptors(&ctx->context, @@ -1777,6 +1842,81 @@ ohci_queue_iso_receive_dualbuffer(struct fw_iso_context *base, } static int +ohci_queue_iso_receive_packet_per_buffer(struct fw_iso_context *base, + struct fw_iso_packet *packet, + struct fw_iso_buffer *buffer, + unsigned long payload) +{ + struct iso_context *ctx = container_of(base, struct iso_context, base); + struct descriptor *d = NULL, *pd = NULL; + struct fw_iso_packet *p; + dma_addr_t d_bus, page_bus; + u32 z, header_z, rest; + int i, page, offset, packet_count, header_size; + + if (packet->skip) { + d = context_get_descriptors(&ctx->context, 1, &d_bus); + if (d == NULL) + return -ENOMEM; + + d->control = cpu_to_le16(DESCRIPTOR_STATUS | + DESCRIPTOR_INPUT_LAST | + DESCRIPTOR_BRANCH_ALWAYS | + DESCRIPTOR_WAIT); + context_append(&ctx->context, d, 1, 0); + } + + /* one descriptor for header, one for payload */ + /* FIXME: handle cases where we need multiple desc. for payload */ + z = 2; + p = packet; + + /* + * The OHCI controller puts the status word in the + * buffer too, so we need 4 extra bytes per packet. + */ + packet_count = p->header_length / ctx->base.header_size; + header_size = packet_count * (ctx->base.header_size + 4); + + /* Get header size in number of descriptors. */ + header_z = DIV_ROUND_UP(header_size, sizeof(*d)); + page = payload >> PAGE_SHIFT; + offset = payload & ~PAGE_MASK; + rest = p->payload_length; + + for (i = 0; i < packet_count; i++) { + /* d points to the header descriptor */ + d = context_get_descriptors(&ctx->context, + z + header_z, &d_bus); + if (d == NULL) + return -ENOMEM; + + d->control = cpu_to_le16(DESCRIPTOR_INPUT_MORE); + d->req_count = cpu_to_le16(header_size); + d->res_count = d->req_count; + d->data_address = cpu_to_le32(d_bus + (z * sizeof(*d))); + + /* pd points to the payload descriptor */ + pd = d + 1; + pd->control = cpu_to_le16(DESCRIPTOR_STATUS | + DESCRIPTOR_INPUT_LAST | + DESCRIPTOR_BRANCH_ALWAYS); + if (p->interrupt) + pd->control |= cpu_to_le16(DESCRIPTOR_IRQ_ALWAYS); + + pd->req_count = cpu_to_le16(rest); + pd->res_count = pd->req_count; + + page_bus = page_private(buffer->pages[page]); + pd->data_address = cpu_to_le32(page_bus + offset); + + context_append(&ctx->context, d, z, header_z); + } + + return 0; +} + +static int ohci_queue_iso(struct fw_iso_context *base, struct fw_iso_packet *packet, struct fw_iso_buffer *buffer, @@ -1790,8 +1930,9 @@ ohci_queue_iso(struct fw_iso_context *base, return ohci_queue_iso_receive_dualbuffer(base, packet, buffer, payload); else - /* FIXME: Implement fallback for OHCI 1.0 controllers. */ - return -ENOSYS; + return ohci_queue_iso_receive_packet_per_buffer(base, packet, + buffer, + payload); } static const struct fw_card_driver ohci_driver = { @@ -1911,12 +2052,6 @@ pci_probe(struct pci_dev *dev, const struct pci_device_id *ent) ohci->version = reg_read(ohci, OHCI1394_Version) & 0x00ff00ff; fw_notify("Added fw-ohci device %s, OHCI version %x.%x\n", dev->dev.bus_id, ohci->version >> 16, ohci->version & 0xff); - if (ohci->version < OHCI_VERSION_1_1) { - fw_notify(" Isochronous I/O is not yet implemented for " - "OHCI 1.0 chips.\n"); - fw_notify(" Cameras, audio devices etc. won't work on " - "this controller with this driver version.\n"); - } return 0; fail_self_id: -- Stefan Richter -=====-=-=== ==-- -=-=- http://arcgraph.de/sr/ ^ permalink raw reply [flat|nested] 18+ messages in thread
[parent not found: <59ad55d30712110818u2717b329j80778ec7cc290988@mail.gmail.com>]
* Re: [GIT PULL] FireWire update [not found] ` <59ad55d30712110818u2717b329j80778ec7cc290988@mail.gmail.com> @ 2007-12-11 17:40 ` Stefan Richter 0 siblings, 0 replies; 18+ messages in thread From: Stefan Richter @ 2007-12-11 17:40 UTC (permalink / raw) To: Kristian Høgsberg Cc: Linus Torvalds, Andrew Morton, linux1394-devel, linux-kernel Kristian Høgsberg wrote: > On Dec 10, 2007 4:25 PM, Stefan Richter <stefanr@s5r6.in-berlin.de> wrote: >> This considerably enhances compatibility of the new firewire-ohci driver ... > And I worked with Jarod on this so I'll add a > > Signed-off-by: Kristian Høgsberg <krh@redhat.com> The commit already went public; nevertheless I much appreciate the sign-off and your work on the OHCI 1.0 issue. Thanks. -- Stefan Richter -=====-=-=== ==-- -=-== http://arcgraph.de/sr/ ^ permalink raw reply [flat|nested] 18+ messages in thread
* [GIT PULL] FireWire update @ 2007-11-07 1:20 Stefan Richter 0 siblings, 0 replies; 18+ messages in thread From: Stefan Richter @ 2007-11-07 1:20 UTC (permalink / raw) To: Linus Torvalds; +Cc: Andrew Morton, linux-kernel, linux1394-devel Linus, please pull from the for-linus branch at git://git.kernel.org/pub/scm/linux/kernel/git/ieee1394/linux1394-2.6.git for-linus to receive the following fix for a regression since 2.6.24-rc1. (Or apply from this e-mail.) drivers/firewire/fw-sbp2.c | 11 +++++++---- 1 files changed, 7 insertions(+), 4 deletions(-) Stefan Richter (1): firewire: fw-sbp2: fix refcounting Full log and diff: commit 7c45d1913f0a1d597eb4bc3b2c962bc2967da9ea Author: Stefan Richter <stefanr@s5r6.in-berlin.de> Date: Wed Nov 7 01:11:56 2007 +0100 firewire: fw-sbp2: fix refcounting Since patch "fw-sbp2: use an own workqueue (fix system responsiveness)" increased parallelism between fw-sbp2 and fw-core, it was possible that fw-sbp2 didn't release the SCSI device when the FireWire device was disconnected. This happened if sbp2_update() ran during sbp2_login(), because a bus reset occurred during sbp2_login(). The sbp2_login() work would [try to] reschedule itself because it failed due to the bus reset, and it would _not_ drop its reference on the target. However, sbp2_update() would schedule sbp2_login() too before sbp2_login() rescheduled itself and hence sbp2_update() would take an additional reference. And then we would have one reference too many. The fix is to _always_ drop the reference when leaving the sbp2_login() work. If the sbp2_login() work reschedules itself, it takes a reference, but only if it wasn't already rescheduled by sbp2_update(). Ditto in the sbp2_reconnect() work. The resulting code is actually simpler than before: We _always_ take a reference when successfully scheduling work. And we _always_ drop a reference when leaving a workqueue job. No exceptions. Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de> diff --git a/drivers/firewire/fw-sbp2.c b/drivers/firewire/fw-sbp2.c index 5596df6..624ff3e 100644 --- a/drivers/firewire/fw-sbp2.c +++ b/drivers/firewire/fw-sbp2.c @@ -650,13 +650,14 @@ static void sbp2_login(struct work_struct *work) if (sbp2_send_management_orb(lu, node_id, generation, SBP2_LOGIN_REQUEST, lu->lun, &response) < 0) { if (lu->retries++ < 5) { - queue_delayed_work(sbp2_wq, &lu->work, - DIV_ROUND_UP(HZ, 5)); + if (queue_delayed_work(sbp2_wq, &lu->work, + DIV_ROUND_UP(HZ, 5))) + kref_get(&lu->tgt->kref); } else { fw_error("failed to login to %s LUN %04x\n", unit->device.bus_id, lu->lun); - kref_put(&lu->tgt->kref, sbp2_release_target); } + kref_put(&lu->tgt->kref, sbp2_release_target); return; } @@ -914,7 +915,9 @@ static void sbp2_reconnect(struct work_struct *work) lu->retries = 0; PREPARE_DELAYED_WORK(&lu->work, sbp2_login); } - queue_delayed_work(sbp2_wq, &lu->work, DIV_ROUND_UP(HZ, 5)); + if (queue_delayed_work(sbp2_wq, &lu->work, DIV_ROUND_UP(HZ, 5))) + kref_get(&lu->tgt->kref); + kref_put(&lu->tgt->kref, sbp2_release_target); return; } -- Stefan Richter -=====-=-=== =-== --=== http://arcgraph.de/sr/ ^ permalink raw reply [flat|nested] 18+ messages in thread
* [GIT PULL] FireWire update @ 2007-11-04 13:41 Stefan Richter 0 siblings, 0 replies; 18+ messages in thread From: Stefan Richter @ 2007-11-04 13:41 UTC (permalink / raw) To: Linus Torvalds; +Cc: Andrew Morton, linux-kernel, linux1394-devel Linus, please pull from the for-linus branch at git://git.kernel.org/pub/scm/linux/kernel/git/ieee1394/linux1394-2.6.git for-linus to receive the following IEEE 1394 subsystem update. This fixes "2.6.24-rc1-54866.. fails to boot: kernel BUG at include/linux/scatterlist.h:49!", bug# 9296. drivers/ieee1394/dma.c | 3 +-- 1 files changed, 1 insertions(+), 2 deletions(-) Jens Axboe (1): ieee1394: iso and async streams: s/g list fix Full log and diff: commit 9e66269d40229cd9823024120910a43af57a9d72 Author: Jens Axboe <jens.axboe@oracle.com> Date: Sun Nov 4 09:44:56 2007 +0100 ieee1394: iso and async streams: s/g list fix Torsten Kaiser wrote: > Looking that calltrace upwards, it seems replacing the > memset(dma->sglist,...) with sg_init_table(...) would fix the BUG_ON() > as that inits the SG_MAGIC. Tested-by: Torsten Kaiser <just.for.lkml@googlemail.com> Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de> diff --git a/drivers/ieee1394/dma.c b/drivers/ieee1394/dma.c index f5f4983..7c4eb39 100644 --- a/drivers/ieee1394/dma.c +++ b/drivers/ieee1394/dma.c @@ -103,8 +103,7 @@ int dma_region_alloc(struct dma_region *dma, unsigned long n_bytes, goto err; } - /* just to be safe - this will become unnecessary once sglist->address goes away */ - memset(dma->sglist, 0, dma->n_pages * sizeof(*dma->sglist)); + sg_init_table(dma->sglist, dma->n_pages); /* fill scatter/gather list with pages */ for (i = 0; i < dma->n_pages; i++) { -- Stefan Richter -=====-=-=== =-== --=-- http://arcgraph.de/sr/ ^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2011-07-11 13:43 UTC | newest] Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2008-01-30 22:53 [GIT PULL] FireWire updates post 2.6.24 Stefan Richter 2008-01-30 22:55 ` Stefan Richter 2008-02-02 13:05 ` [GIT PULL] IEEE 1394 regression fix Stefan Richter 2008-02-25 17:58 ` [GIT PULL] FireWire updates Stefan Richter 2008-02-25 18:00 ` Stefan Richter 2008-03-02 12:47 ` Stefan Richter 2008-03-02 12:49 ` Stefan Richter 2008-03-14 18:07 ` Stefan Richter 2008-03-14 18:08 ` Stefan Richter 2008-03-20 17:28 ` [GIT PULL] FireWire update Stefan Richter 2008-03-27 20:37 ` Stefan Richter 2008-03-31 8:46 ` Stefan Richter -- strict thread matches above, loose matches on Subject: below -- 2011-07-11 13:42 [git pull] " Stefan Richter 2008-06-27 19:05 [GIT PULL] firewire update Stefan Richter 2007-12-10 21:25 [GIT PULL] FireWire update Stefan Richter [not found] ` <59ad55d30712110818u2717b329j80778ec7cc290988@mail.gmail.com> 2007-12-11 17:40 ` Stefan Richter 2007-11-07 1:20 Stefan Richter 2007-11-04 13:41 Stefan Richter
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).