From 68e2aa793ed55b0c1461b4ab92554bb5ad152724 Mon Sep 17 00:00:00 2001 From: Julia Lawall Date: Sat, 2 Aug 2008 17:13:09 +0200 Subject: ieee1394: Use DIV_ROUND_UP Signed-off-by: Julia Lawall Signed-off-by: Stefan Richter --- drivers/ieee1394/csr1212.c | 2 +- drivers/ieee1394/eth1394.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/ieee1394/csr1212.c b/drivers/ieee1394/csr1212.c index 9f95337139e..5e38a68b8af 100644 --- a/drivers/ieee1394/csr1212.c +++ b/drivers/ieee1394/csr1212.c @@ -84,7 +84,7 @@ static const u8 csr1212_key_id_type_map[0x30] = { #define quads_to_bytes(_q) ((_q) * sizeof(u32)) -#define bytes_to_quads(_b) (((_b) + sizeof(u32) - 1) / sizeof(u32)) +#define bytes_to_quads(_b) DIV_ROUND_UP(_b, sizeof(u32)) static void free_keyval(struct csr1212_keyval *kv) { diff --git a/drivers/ieee1394/eth1394.c b/drivers/ieee1394/eth1394.c index b166b3575fa..20128692b33 100644 --- a/drivers/ieee1394/eth1394.c +++ b/drivers/ieee1394/eth1394.c @@ -1361,7 +1361,7 @@ static unsigned int ether1394_encapsulate_prep(unsigned int max_payload, hdr->ff.dgl = dgl; adj_max_payload = max_payload - hdr_type_len[ETH1394_HDR_LF_FF]; } - return (dg_size + adj_max_payload - 1) / adj_max_payload; + return DIV_ROUND_UP(dg_size, adj_max_payload); } static unsigned int ether1394_encapsulate(struct sk_buff *skb, -- cgit v1.2.3 From 0a77b17c855c4ec1c87ed80e0f280095a4ee1f4f Mon Sep 17 00:00:00 2001 From: Stefan Richter Date: Sat, 9 Aug 2008 20:13:00 +0200 Subject: ieee1394: sbp2: stricter dma_sync Two dma_sync_single_for_cpu() were called in the wrong place. Luckily they were merely for DMA_TO_DEVICE, hence nobody noticed. Also reorder the matching dma_sync_single_for_device() a little bit so that they reside in the same functions as their counterparts. This also avoids syncing the s/g table for requests which don't use it. Signed-off-by: Stefan Richter --- drivers/ieee1394/sbp2.c | 50 ++++++++++++++++--------------------------------- 1 file changed, 16 insertions(+), 34 deletions(-) diff --git a/drivers/ieee1394/sbp2.c b/drivers/ieee1394/sbp2.c index 1d6ad343553..7a8119e0c91 100644 --- a/drivers/ieee1394/sbp2.c +++ b/drivers/ieee1394/sbp2.c @@ -1533,6 +1533,10 @@ static void sbp2_prep_command_orb_sg(struct sbp2_command_orb *orb, orb->misc |= ORB_SET_PAGE_TABLE_PRESENT(0x1); orb->data_descriptor_lo = cmd->sge_dma; + dma_sync_single_for_cpu(hi->host->device.parent, cmd->sge_dma, + sizeof(cmd->scatter_gather_element), + DMA_TO_DEVICE); + /* loop through and fill out our SBP-2 page tables * (and split up anything too large) */ for (i = 0, sg_count = 0; i < count; i++, sg = sg_next(sg)) { @@ -1559,6 +1563,11 @@ static void sbp2_prep_command_orb_sg(struct sbp2_command_orb *orb, sbp2util_cpu_to_be32_buffer(sg_element, (sizeof(struct sbp2_unrestricted_page_table)) * sg_count); + + dma_sync_single_for_device(hi->host->device.parent, + cmd->sge_dma, + sizeof(cmd->scatter_gather_element), + DMA_TO_DEVICE); } } @@ -1566,12 +1575,14 @@ static void sbp2_create_command_orb(struct sbp2_lu *lu, struct sbp2_command_info *cmd, struct scsi_cmnd *SCpnt) { - struct sbp2_fwhost_info *hi = lu->hi; + struct device *dmadev = lu->hi->host->device.parent; struct sbp2_command_orb *orb = &cmd->command_orb; u32 orb_direction; unsigned int scsi_request_bufflen = scsi_bufflen(SCpnt); enum dma_data_direction dma_dir = SCpnt->sc_data_direction; + dma_sync_single_for_cpu(dmadev, cmd->command_orb_dma, + sizeof(struct sbp2_command_orb), DMA_TO_DEVICE); /* * Set-up our command ORB. * @@ -1603,7 +1614,7 @@ static void sbp2_create_command_orb(struct sbp2_lu *lu, orb->data_descriptor_lo = 0x0; orb->misc |= ORB_SET_DIRECTION(1); } else - sbp2_prep_command_orb_sg(orb, hi, cmd, scsi_sg_count(SCpnt), + sbp2_prep_command_orb_sg(orb, lu->hi, cmd, scsi_sg_count(SCpnt), scsi_sglist(SCpnt), orb_direction, dma_dir); @@ -1611,6 +1622,9 @@ static void sbp2_create_command_orb(struct sbp2_lu *lu, memset(orb->cdb, 0, sizeof(orb->cdb)); memcpy(orb->cdb, SCpnt->cmnd, SCpnt->cmd_len); + + dma_sync_single_for_device(dmadev, cmd->command_orb_dma, + sizeof(struct sbp2_command_orb), DMA_TO_DEVICE); } static void sbp2_link_orb_command(struct sbp2_lu *lu, @@ -1624,14 +1638,6 @@ static void sbp2_link_orb_command(struct sbp2_lu *lu, size_t length; unsigned long flags; - dma_sync_single_for_device(hi->host->device.parent, - cmd->command_orb_dma, - sizeof(struct sbp2_command_orb), - DMA_TO_DEVICE); - dma_sync_single_for_device(hi->host->device.parent, cmd->sge_dma, - sizeof(cmd->scatter_gather_element), - DMA_TO_DEVICE); - /* check to see if there are any previous orbs to use */ spin_lock_irqsave(&lu->cmd_orb_lock, flags); last_orb = lu->last_orb; @@ -1789,13 +1795,6 @@ static int sbp2_handle_status_write(struct hpsb_host *host, int nodeid, else cmd = sbp2util_find_command_for_orb(lu, sb->ORB_offset_lo); if (cmd) { - dma_sync_single_for_cpu(hi->host->device.parent, - cmd->command_orb_dma, - sizeof(struct sbp2_command_orb), - DMA_TO_DEVICE); - dma_sync_single_for_cpu(hi->host->device.parent, cmd->sge_dma, - sizeof(cmd->scatter_gather_element), - DMA_TO_DEVICE); /* Grab SCSI command pointers and check status. */ /* * FIXME: If the src field in the status is 1, the ORB DMA must @@ -1912,7 +1911,6 @@ done: static void sbp2scsi_complete_all_commands(struct sbp2_lu *lu, u32 status) { - struct sbp2_fwhost_info *hi = lu->hi; struct list_head *lh; struct sbp2_command_info *cmd; unsigned long flags; @@ -1921,13 +1919,6 @@ static void sbp2scsi_complete_all_commands(struct sbp2_lu *lu, u32 status) while (!list_empty(&lu->cmd_orb_inuse)) { lh = lu->cmd_orb_inuse.next; cmd = list_entry(lh, struct sbp2_command_info, list); - dma_sync_single_for_cpu(hi->host->device.parent, - cmd->command_orb_dma, - sizeof(struct sbp2_command_orb), - DMA_TO_DEVICE); - dma_sync_single_for_cpu(hi->host->device.parent, cmd->sge_dma, - sizeof(cmd->scatter_gather_element), - DMA_TO_DEVICE); sbp2util_mark_command_completed(lu, cmd); if (cmd->Current_SCpnt) { cmd->Current_SCpnt->result = status << 16; @@ -2049,7 +2040,6 @@ static void sbp2scsi_slave_destroy(struct scsi_device *sdev) static int sbp2scsi_abort(struct scsi_cmnd *SCpnt) { struct sbp2_lu *lu = (struct sbp2_lu *)SCpnt->device->host->hostdata[0]; - struct sbp2_fwhost_info *hi = lu->hi; struct sbp2_command_info *cmd; unsigned long flags; @@ -2063,14 +2053,6 @@ static int sbp2scsi_abort(struct scsi_cmnd *SCpnt) spin_lock_irqsave(&lu->cmd_orb_lock, flags); cmd = sbp2util_find_command_for_SCpnt(lu, SCpnt); if (cmd) { - dma_sync_single_for_cpu(hi->host->device.parent, - cmd->command_orb_dma, - sizeof(struct sbp2_command_orb), - DMA_TO_DEVICE); - dma_sync_single_for_cpu(hi->host->device.parent, - cmd->sge_dma, - sizeof(cmd->scatter_gather_element), - DMA_TO_DEVICE); sbp2util_mark_command_completed(lu, cmd); if (cmd->Current_SCpnt) { cmd->Current_SCpnt->result = DID_ABORT << 16; -- cgit v1.2.3 From cd8c79f17a878b01f3e83a8efd89da18ccdc7ef3 Mon Sep 17 00:00:00 2001 From: Stefan Richter Date: Sat, 9 Aug 2008 20:16:24 +0200 Subject: ieee1394: sbp2: check for DMA mapping failures Signed-off-by: Stefan Richter --- drivers/ieee1394/sbp2.c | 94 +++++++++++++++++++++++++++++++------------------ 1 file changed, 59 insertions(+), 35 deletions(-) diff --git a/drivers/ieee1394/sbp2.c b/drivers/ieee1394/sbp2.c index 7a8119e0c91..0037305f599 100644 --- a/drivers/ieee1394/sbp2.c +++ b/drivers/ieee1394/sbp2.c @@ -526,26 +526,41 @@ static void sbp2util_write_doorbell(struct work_struct *work) static int sbp2util_create_command_orb_pool(struct sbp2_lu *lu) { - struct sbp2_fwhost_info *hi = lu->hi; struct sbp2_command_info *cmd; + struct device *dmadev = lu->hi->host->device.parent; int i, orbs = sbp2_serialize_io ? 2 : SBP2_MAX_CMDS; for (i = 0; i < orbs; i++) { cmd = kzalloc(sizeof(*cmd), GFP_KERNEL); if (!cmd) - return -ENOMEM; - cmd->command_orb_dma = dma_map_single(hi->host->device.parent, - &cmd->command_orb, - sizeof(struct sbp2_command_orb), - DMA_TO_DEVICE); - cmd->sge_dma = dma_map_single(hi->host->device.parent, - &cmd->scatter_gather_element, - sizeof(cmd->scatter_gather_element), - DMA_TO_DEVICE); + goto failed_alloc; + + cmd->command_orb_dma = + dma_map_single(dmadev, &cmd->command_orb, + sizeof(struct sbp2_command_orb), + DMA_TO_DEVICE); + if (dma_mapping_error(dmadev, cmd->command_orb_dma)) + goto failed_orb; + + cmd->sge_dma = + dma_map_single(dmadev, &cmd->scatter_gather_element, + sizeof(cmd->scatter_gather_element), + DMA_TO_DEVICE); + if (dma_mapping_error(dmadev, cmd->sge_dma)) + goto failed_sge; + INIT_LIST_HEAD(&cmd->list); list_add_tail(&cmd->list, &lu->cmd_orb_completed); } return 0; + +failed_sge: + dma_unmap_single(dmadev, cmd->command_orb_dma, + sizeof(struct sbp2_command_orb), DMA_TO_DEVICE); +failed_orb: + kfree(cmd); +failed_alloc: + return -ENOMEM; } static void sbp2util_remove_command_orb_pool(struct sbp2_lu *lu, @@ -1494,14 +1509,16 @@ static int sbp2_agent_reset(struct sbp2_lu *lu, int wait) return 0; } -static void sbp2_prep_command_orb_sg(struct sbp2_command_orb *orb, - struct sbp2_fwhost_info *hi, - struct sbp2_command_info *cmd, - unsigned int scsi_use_sg, - struct scatterlist *sg, - u32 orb_direction, - enum dma_data_direction dma_dir) +static int sbp2_prep_command_orb_sg(struct sbp2_command_orb *orb, + struct sbp2_fwhost_info *hi, + struct sbp2_command_info *cmd, + unsigned int scsi_use_sg, + struct scatterlist *sg, + u32 orb_direction, + enum dma_data_direction dma_dir) { + struct device *dmadev = hi->host->device.parent; + cmd->dma_dir = dma_dir; orb->data_descriptor_hi = ORB_SET_NODE_ID(hi->host->node_id); orb->misc |= ORB_SET_DIRECTION(orb_direction); @@ -1511,9 +1528,12 @@ static void sbp2_prep_command_orb_sg(struct sbp2_command_orb *orb, 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, + cmd->cmd_dma = dma_map_page(dmadev, sg_page(sg), sg->offset, cmd->dma_size, cmd->dma_dir); + if (dma_mapping_error(dmadev, cmd->cmd_dma)) { + cmd->cmd_dma = 0; + return -ENOMEM; + } orb->data_descriptor_lo = cmd->cmd_dma; orb->misc |= ORB_SET_DATA_SIZE(cmd->dma_size); @@ -1523,8 +1543,7 @@ static void sbp2_prep_command_orb_sg(struct sbp2_command_orb *orb, &cmd->scatter_gather_element[0]; u32 sg_count, sg_len; dma_addr_t sg_addr; - int i, count = dma_map_sg(hi->host->device.parent, sg, - scsi_use_sg, dma_dir); + int i, count = dma_map_sg(dmadev, sg, scsi_use_sg, dma_dir); cmd->dma_size = scsi_use_sg; cmd->sge_buffer = sg; @@ -1533,7 +1552,7 @@ static void sbp2_prep_command_orb_sg(struct sbp2_command_orb *orb, orb->misc |= ORB_SET_PAGE_TABLE_PRESENT(0x1); orb->data_descriptor_lo = cmd->sge_dma; - dma_sync_single_for_cpu(hi->host->device.parent, cmd->sge_dma, + dma_sync_single_for_cpu(dmadev, cmd->sge_dma, sizeof(cmd->scatter_gather_element), DMA_TO_DEVICE); @@ -1564,22 +1583,23 @@ static void sbp2_prep_command_orb_sg(struct sbp2_command_orb *orb, (sizeof(struct sbp2_unrestricted_page_table)) * sg_count); - dma_sync_single_for_device(hi->host->device.parent, - cmd->sge_dma, + dma_sync_single_for_device(dmadev, cmd->sge_dma, sizeof(cmd->scatter_gather_element), DMA_TO_DEVICE); } + return 0; } -static void sbp2_create_command_orb(struct sbp2_lu *lu, - struct sbp2_command_info *cmd, - struct scsi_cmnd *SCpnt) +static int sbp2_create_command_orb(struct sbp2_lu *lu, + struct sbp2_command_info *cmd, + struct scsi_cmnd *SCpnt) { struct device *dmadev = lu->hi->host->device.parent; struct sbp2_command_orb *orb = &cmd->command_orb; - u32 orb_direction; unsigned int scsi_request_bufflen = scsi_bufflen(SCpnt); enum dma_data_direction dma_dir = SCpnt->sc_data_direction; + u32 orb_direction; + int ret; dma_sync_single_for_cpu(dmadev, cmd->command_orb_dma, sizeof(struct sbp2_command_orb), DMA_TO_DEVICE); @@ -1613,11 +1633,13 @@ static void sbp2_create_command_orb(struct sbp2_lu *lu, orb->data_descriptor_hi = 0x0; orb->data_descriptor_lo = 0x0; orb->misc |= ORB_SET_DIRECTION(1); - } else - sbp2_prep_command_orb_sg(orb, lu->hi, cmd, scsi_sg_count(SCpnt), - scsi_sglist(SCpnt), - orb_direction, dma_dir); - + ret = 0; + } else { + ret = sbp2_prep_command_orb_sg(orb, lu->hi, cmd, + scsi_sg_count(SCpnt), + scsi_sglist(SCpnt), + orb_direction, dma_dir); + } sbp2util_cpu_to_be32_buffer(orb, sizeof(*orb)); memset(orb->cdb, 0, sizeof(orb->cdb)); @@ -1625,6 +1647,7 @@ static void sbp2_create_command_orb(struct sbp2_lu *lu, dma_sync_single_for_device(dmadev, cmd->command_orb_dma, sizeof(struct sbp2_command_orb), DMA_TO_DEVICE); + return ret; } static void sbp2_link_orb_command(struct sbp2_lu *lu, @@ -1705,9 +1728,10 @@ static int sbp2_send_command(struct sbp2_lu *lu, struct scsi_cmnd *SCpnt, if (!cmd) return -EIO; - sbp2_create_command_orb(lu, cmd, SCpnt); - sbp2_link_orb_command(lu, cmd); + if (sbp2_create_command_orb(lu, cmd, SCpnt)) + return -ENOMEM; + sbp2_link_orb_command(lu, cmd); return 0; } -- cgit v1.2.3 From ed6ffd08084c68e9c3911e27706dec9d4c9a4175 Mon Sep 17 00:00:00 2001 From: Stefan Richter Date: Thu, 14 Aug 2008 09:28:14 +0200 Subject: ieee1394: sbp2: enforce s/g segment size limit 1. We don't need to round the SBP-2 segment size limit down to a multiple of 4 kB (0xffff -> 0xf000). It is only necessary to ensure quadlet alignment (0xffff -> 0xfffc). 2. Use dma_set_max_seg_size() to tell the DMA mapping infrastructure and the block IO layer about the restriction. This way we can remove the size checks and segment splitting in the queuecommand path. This assumes that no other code in the ieee1394 stack uses dma_map_sg() with conflicting requirements. It furthermore assumes that the controller device's platform actually allows us to set the segment size to our liking. Assert the latter with a BUG_ON(). 3. Also use blk_queue_max_segment_size() to tell the block IO layer about it. It cannot know it because our scsi_add_host() does not point to the FireWire controller's device. We can also uniformly use dma_map_sg() for the single segment case just like for the multi segment case, to further simplify the code. Also clean up how the page table is converted to big endian. Thanks to Grant Grundler and FUJITA Tomonori for advice. Signed-off-by: Stefan Richter --- drivers/ieee1394/sbp2.c | 98 ++++++++++++++----------------------------------- drivers/ieee1394/sbp2.h | 33 ++++++----------- 2 files changed, 39 insertions(+), 92 deletions(-) diff --git a/drivers/ieee1394/sbp2.c b/drivers/ieee1394/sbp2.c index 0037305f599..c52f6e6e8af 100644 --- a/drivers/ieee1394/sbp2.c +++ b/drivers/ieee1394/sbp2.c @@ -656,24 +656,11 @@ static struct sbp2_command_info *sbp2util_allocate_command_orb( static void sbp2util_mark_command_completed(struct sbp2_lu *lu, struct sbp2_command_info *cmd) { - struct hpsb_host *host = lu->ud->ne->host; - - if (cmd->cmd_dma) { - if (cmd->dma_type == CMD_DMA_SINGLE) - dma_unmap_single(host->device.parent, cmd->cmd_dma, - cmd->dma_size, cmd->dma_dir); - else if (cmd->dma_type == CMD_DMA_PAGE) - dma_unmap_page(host->device.parent, cmd->cmd_dma, - cmd->dma_size, cmd->dma_dir); - /* XXX: Check for CMD_DMA_NONE bug */ - cmd->dma_type = CMD_DMA_NONE; - cmd->cmd_dma = 0; - } - if (cmd->sge_buffer) { - dma_unmap_sg(host->device.parent, cmd->sge_buffer, - cmd->dma_size, cmd->dma_dir); - cmd->sge_buffer = NULL; - } + if (scsi_sg_count(cmd->Current_SCpnt)) + dma_unmap_sg(lu->ud->ne->host->device.parent, + scsi_sglist(cmd->Current_SCpnt), + scsi_sg_count(cmd->Current_SCpnt), + cmd->Current_SCpnt->sc_data_direction); list_move_tail(&cmd->list, &lu->cmd_orb_completed); } @@ -838,6 +825,10 @@ static struct sbp2_lu *sbp2_alloc_device(struct unit_directory *ud) #endif } + if (dma_get_max_seg_size(hi->host->device.parent) > SBP2_MAX_SEG_SIZE) + BUG_ON(dma_set_max_seg_size(hi->host->device.parent, + SBP2_MAX_SEG_SIZE)); + /* Prevent unloading of the 1394 host */ if (!try_module_get(hi->host->driver->owner)) { SBP2_ERR("failed to get a reference on 1394 host driver"); @@ -1512,76 +1503,41 @@ static int sbp2_agent_reset(struct sbp2_lu *lu, int wait) static int sbp2_prep_command_orb_sg(struct sbp2_command_orb *orb, struct sbp2_fwhost_info *hi, struct sbp2_command_info *cmd, - unsigned int scsi_use_sg, + unsigned int sg_count, struct scatterlist *sg, u32 orb_direction, enum dma_data_direction dma_dir) { struct device *dmadev = hi->host->device.parent; + struct sbp2_unrestricted_page_table *pt; + int i, n; + + n = dma_map_sg(dmadev, sg, sg_count, dma_dir); + if (n == 0) + return -ENOMEM; - cmd->dma_dir = dma_dir; orb->data_descriptor_hi = ORB_SET_NODE_ID(hi->host->node_id); 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->length <= SBP2_MAX_SG_ELEMENT_LENGTH) { - - cmd->dma_size = sg->length; - cmd->dma_type = CMD_DMA_PAGE; - cmd->cmd_dma = dma_map_page(dmadev, sg_page(sg), sg->offset, - cmd->dma_size, cmd->dma_dir); - if (dma_mapping_error(dmadev, cmd->cmd_dma)) { - cmd->cmd_dma = 0; - return -ENOMEM; - } - - orb->data_descriptor_lo = cmd->cmd_dma; - orb->misc |= ORB_SET_DATA_SIZE(cmd->dma_size); - + if (n == 1) { + orb->misc |= ORB_SET_DATA_SIZE(sg_dma_len(sg)); + orb->data_descriptor_lo = sg_dma_address(sg); } else { - struct sbp2_unrestricted_page_table *sg_element = - &cmd->scatter_gather_element[0]; - u32 sg_count, sg_len; - dma_addr_t sg_addr; - int i, count = dma_map_sg(dmadev, sg, scsi_use_sg, dma_dir); - - cmd->dma_size = scsi_use_sg; - cmd->sge_buffer = sg; - - /* use page tables (s/g) */ - orb->misc |= ORB_SET_PAGE_TABLE_PRESENT(0x1); - orb->data_descriptor_lo = cmd->sge_dma; + pt = &cmd->scatter_gather_element[0]; dma_sync_single_for_cpu(dmadev, cmd->sge_dma, sizeof(cmd->scatter_gather_element), DMA_TO_DEVICE); - /* loop through and fill out our SBP-2 page tables - * (and split up anything too large) */ - for (i = 0, sg_count = 0; i < count; i++, sg = sg_next(sg)) { - sg_len = sg_dma_len(sg); - sg_addr = sg_dma_address(sg); - while (sg_len) { - sg_element[sg_count].segment_base_lo = sg_addr; - if (sg_len > SBP2_MAX_SG_ELEMENT_LENGTH) { - sg_element[sg_count].length_segment_base_hi = - PAGE_TABLE_SET_SEGMENT_LENGTH(SBP2_MAX_SG_ELEMENT_LENGTH); - sg_addr += SBP2_MAX_SG_ELEMENT_LENGTH; - sg_len -= SBP2_MAX_SG_ELEMENT_LENGTH; - } else { - sg_element[sg_count].length_segment_base_hi = - PAGE_TABLE_SET_SEGMENT_LENGTH(sg_len); - sg_len = 0; - } - sg_count++; - } + for_each_sg(sg, sg, n, i) { + pt[i].high = cpu_to_be32(sg_dma_len(sg) << 16); + pt[i].low = cpu_to_be32(sg_dma_address(sg)); } - orb->misc |= ORB_SET_DATA_SIZE(sg_count); - - sbp2util_cpu_to_be32_buffer(sg_element, - (sizeof(struct sbp2_unrestricted_page_table)) * - sg_count); + orb->misc |= ORB_SET_PAGE_TABLE_PRESENT(0x1) | + ORB_SET_DATA_SIZE(n); + orb->data_descriptor_lo = cmd->sge_dma; dma_sync_single_for_device(dmadev, cmd->sge_dma, sizeof(cmd->scatter_gather_element), @@ -2048,6 +2004,8 @@ static int sbp2scsi_slave_configure(struct scsi_device *sdev) sdev->start_stop_pwr_cond = 1; if (lu->workarounds & SBP2_WORKAROUND_128K_MAX_TRANS) blk_queue_max_sectors(sdev->request_queue, 128 * 1024 / 512); + + blk_queue_max_segment_size(sdev->request_queue, SBP2_MAX_SEG_SIZE); return 0; } diff --git a/drivers/ieee1394/sbp2.h b/drivers/ieee1394/sbp2.h index 875428bc8d2..c5036f1cc5b 100644 --- a/drivers/ieee1394/sbp2.h +++ b/drivers/ieee1394/sbp2.h @@ -139,13 +139,10 @@ struct sbp2_logout_orb { u32 status_fifo_lo; } __attribute__((packed)); -#define PAGE_TABLE_SET_SEGMENT_BASE_HI(v) ((v) & 0xffff) -#define PAGE_TABLE_SET_SEGMENT_LENGTH(v) (((v) & 0xffff) << 16) - struct sbp2_unrestricted_page_table { - u32 length_segment_base_hi; - u32 segment_base_lo; -} __attribute__((packed)); + __be32 high; + __be32 low; +}; #define RESP_STATUS_REQUEST_COMPLETE 0x0 #define RESP_STATUS_TRANSPORT_FAILURE 0x1 @@ -216,15 +213,18 @@ struct sbp2_status_block { #define SBP2_UNIT_SPEC_ID_ENTRY 0x0000609e #define SBP2_SW_VERSION_ENTRY 0x00010483 - /* - * SCSI specific definitions + * The default maximum s/g segment size of a FireWire controller is + * usually 0x10000, but SBP-2 only allows 0xffff. Since buffers have to + * be quadlet-aligned, we set the length limit to 0xffff & ~3. */ +#define SBP2_MAX_SEG_SIZE 0xfffc -#define SBP2_MAX_SG_ELEMENT_LENGTH 0xf000 -/* There is no real limitation of the queue depth (i.e. length of the linked +/* + * There is no real limitation of the queue depth (i.e. length of the linked * list of command ORBs) at the target. The chosen depth is merely an - * implementation detail of the sbp2 driver. */ + * implementation detail of the sbp2 driver. + */ #define SBP2_MAX_CMDS 8 #define SBP2_SCSI_STATUS_GOOD 0x0 @@ -240,12 +240,6 @@ struct sbp2_status_block { * Representations of commands and devices */ -enum sbp2_dma_types { - CMD_DMA_NONE, - CMD_DMA_PAGE, - CMD_DMA_SINGLE -}; - /* Per SCSI command */ struct sbp2_command_info { struct list_head list; @@ -258,11 +252,6 @@ struct sbp2_command_info { struct sbp2_unrestricted_page_table scatter_gather_element[SG_ALL] __attribute__((aligned(8))); dma_addr_t sge_dma; - void *sge_buffer; - dma_addr_t cmd_dma; - enum sbp2_dma_types dma_type; - unsigned long dma_size; - enum dma_data_direction dma_dir; }; /* Per FireWire host */ -- cgit v1.2.3 From 10963ea1bd966ba46a46178c4d6abcdf3c23538d Mon Sep 17 00:00:00 2001 From: Stefan Richter Date: Sat, 16 Aug 2008 00:11:48 +0200 Subject: ieee1394: raw1394: replace BKL by local mutex, make ioctl() and mmap() thread-safe This removes the last usage of the Big Kernel Lock from the ieee1394 stack, i.e. from raw1394's (unlocked_)ioctl and compat_ioctl. The ioctl()s don't need to take the BKL, but they need to be serialized per struct file *. In particular, accesses to ->iso_state need to be serial. We simply use a blocking mutex for this purpose because libraw1394 does not use O_NONBLOCK. In practice, there is no lock contention anyway because most if not all libraw1394 clients use a libraw1394 handle only in a single thread. mmap() also accesses ->iso_state. Until now this was unprotected against concurrent changes by ioctls. Fix this bug while we are at it. Signed-off-by: Stefan Richter --- drivers/ieee1394/raw1394-private.h | 1 + drivers/ieee1394/raw1394.c | 23 +++++++++++++++++------ 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/drivers/ieee1394/raw1394-private.h b/drivers/ieee1394/raw1394-private.h index a06aaad5b44..7a225a40598 100644 --- a/drivers/ieee1394/raw1394-private.h +++ b/drivers/ieee1394/raw1394-private.h @@ -22,6 +22,7 @@ enum raw1394_iso_state { RAW1394_ISO_INACTIVE = 0, struct file_info { struct list_head list; + struct mutex state_mutex; enum { opened, initialized, connected } state; unsigned int protocol_version; diff --git a/drivers/ieee1394/raw1394.c b/drivers/ieee1394/raw1394.c index 6fa9e4a2184..975ed7062aa 100644 --- a/drivers/ieee1394/raw1394.c +++ b/drivers/ieee1394/raw1394.c @@ -34,6 +34,7 @@ #include #include #include +#include #include #include #include @@ -2541,11 +2542,18 @@ static int raw1394_read_cycle_timer(struct file_info *fi, void __user * uaddr) static int raw1394_mmap(struct file *file, struct vm_area_struct *vma) { struct file_info *fi = file->private_data; + int ret; + + mutex_lock(&fi->state_mutex); if (fi->iso_state == RAW1394_ISO_INACTIVE) - return -EINVAL; + ret = -EINVAL; + else + ret = dma_region_mmap(&fi->iso_handle->data_buf, file, vma); + + mutex_unlock(&fi->state_mutex); - return dma_region_mmap(&fi->iso_handle->data_buf, file, vma); + return ret; } /* ioctl is only used for rawiso operations */ @@ -2659,10 +2667,12 @@ static long do_raw1394_ioctl(struct file *file, unsigned int cmd, static long raw1394_ioctl(struct file *file, unsigned int cmd, unsigned long arg) { + struct file_info *fi = file->private_data; long ret; - lock_kernel(); + + mutex_lock(&fi->state_mutex); ret = do_raw1394_ioctl(file, cmd, arg); - unlock_kernel(); + mutex_unlock(&fi->state_mutex); return ret; } @@ -2724,7 +2734,7 @@ static long raw1394_compat_ioctl(struct file *file, void __user *argp = (void __user *)arg; long err; - lock_kernel(); + mutex_lock(&fi->state_mutex); switch (cmd) { /* These requests have same format as long as 'int' has same size. */ case RAW1394_IOC_ISO_RECV_INIT: @@ -2757,7 +2767,7 @@ static long raw1394_compat_ioctl(struct file *file, err = -EINVAL; break; } - unlock_kernel(); + mutex_unlock(&fi->state_mutex); return err; } @@ -2791,6 +2801,7 @@ static int raw1394_open(struct inode *inode, struct file *file) fi->notification = (u8) RAW1394_NOTIFY_ON; /* busreset notification */ INIT_LIST_HEAD(&fi->list); + mutex_init(&fi->state_mutex); fi->state = opened; INIT_LIST_HEAD(&fi->req_pending); INIT_LIST_HEAD(&fi->req_complete); -- cgit v1.2.3 From ddfb908d3f905dbb5964d6fbf783e69c417eb13e Mon Sep 17 00:00:00 2001 From: Stefan Richter Date: Sat, 16 Aug 2008 17:52:04 +0200 Subject: ieee1394: raw1394: narrow down the state_mutex protected region Refactor the ioctl dispatcher in order to move a fraction of it out of the section which is serialized by fi->state_mutex. This is not so much about performance but more about self-documentation: The mutex_lock()/ mutex_unlock() calls are now closer to the data accesses which the mutex protects, i.e. to the iso_state switch. Signed-off-by: Stefan Richter --- drivers/ieee1394/raw1394.c | 211 +++++++++++++++++++++++---------------------- 1 file changed, 110 insertions(+), 101 deletions(-) diff --git a/drivers/ieee1394/raw1394.c b/drivers/ieee1394/raw1394.c index 975ed7062aa..d1594427601 100644 --- a/drivers/ieee1394/raw1394.c +++ b/drivers/ieee1394/raw1394.c @@ -2556,102 +2556,106 @@ static int raw1394_mmap(struct file *file, struct vm_area_struct *vma) return ret; } -/* ioctl is only used for rawiso operations */ -static long do_raw1394_ioctl(struct file *file, unsigned int cmd, - unsigned long arg) +static long raw1394_ioctl_inactive(struct file_info *fi, unsigned int cmd, + void __user *argp) +{ + switch (cmd) { + case RAW1394_IOC_ISO_XMIT_INIT: + return raw1394_iso_xmit_init(fi, argp); + case RAW1394_IOC_ISO_RECV_INIT: + return raw1394_iso_recv_init(fi, argp); + default: + return -EINVAL; + } +} + +static long raw1394_ioctl_recv(struct file_info *fi, unsigned int cmd, + unsigned long arg) { - struct file_info *fi = file->private_data; void __user *argp = (void __user *)arg; - switch (fi->iso_state) { - case RAW1394_ISO_INACTIVE: - switch (cmd) { - case RAW1394_IOC_ISO_XMIT_INIT: - return raw1394_iso_xmit_init(fi, argp); - case RAW1394_IOC_ISO_RECV_INIT: - return raw1394_iso_recv_init(fi, argp); - default: - break; + switch (cmd) { + case RAW1394_IOC_ISO_RECV_START:{ + int args[3]; + + if (copy_from_user(&args[0], argp, sizeof(args))) + return -EFAULT; + return hpsb_iso_recv_start(fi->iso_handle, + args[0], args[1], args[2]); } - break; - case RAW1394_ISO_RECV: - switch (cmd) { - case RAW1394_IOC_ISO_RECV_START:{ - /* copy args from user-space */ - int args[3]; - if (copy_from_user - (&args[0], argp, sizeof(args))) - return -EFAULT; - return hpsb_iso_recv_start(fi->iso_handle, - args[0], args[1], - args[2]); - } - case RAW1394_IOC_ISO_XMIT_RECV_STOP: - hpsb_iso_stop(fi->iso_handle); - return 0; - case RAW1394_IOC_ISO_RECV_LISTEN_CHANNEL: - return hpsb_iso_recv_listen_channel(fi->iso_handle, - arg); - case RAW1394_IOC_ISO_RECV_UNLISTEN_CHANNEL: - return hpsb_iso_recv_unlisten_channel(fi->iso_handle, - arg); - case RAW1394_IOC_ISO_RECV_SET_CHANNEL_MASK:{ - /* copy the u64 from user-space */ - u64 mask; - if (copy_from_user(&mask, argp, sizeof(mask))) - return -EFAULT; - return hpsb_iso_recv_set_channel_mask(fi-> - iso_handle, - mask); - } - case RAW1394_IOC_ISO_GET_STATUS: - return raw1394_iso_get_status(fi, argp); - case RAW1394_IOC_ISO_RECV_PACKETS: - return raw1394_iso_recv_packets(fi, argp); - case RAW1394_IOC_ISO_RECV_RELEASE_PACKETS: - return hpsb_iso_recv_release_packets(fi->iso_handle, - arg); - case RAW1394_IOC_ISO_RECV_FLUSH: - return hpsb_iso_recv_flush(fi->iso_handle); - case RAW1394_IOC_ISO_SHUTDOWN: - raw1394_iso_shutdown(fi); - return 0; - case RAW1394_IOC_ISO_QUEUE_ACTIVITY: - queue_rawiso_event(fi); - return 0; + case RAW1394_IOC_ISO_XMIT_RECV_STOP: + hpsb_iso_stop(fi->iso_handle); + return 0; + case RAW1394_IOC_ISO_RECV_LISTEN_CHANNEL: + return hpsb_iso_recv_listen_channel(fi->iso_handle, arg); + case RAW1394_IOC_ISO_RECV_UNLISTEN_CHANNEL: + return hpsb_iso_recv_unlisten_channel(fi->iso_handle, arg); + case RAW1394_IOC_ISO_RECV_SET_CHANNEL_MASK:{ + u64 mask; + + if (copy_from_user(&mask, argp, sizeof(mask))) + return -EFAULT; + return hpsb_iso_recv_set_channel_mask(fi->iso_handle, + mask); } - break; - case RAW1394_ISO_XMIT: - switch (cmd) { - case RAW1394_IOC_ISO_XMIT_START:{ - /* copy two ints from user-space */ - int args[2]; - if (copy_from_user - (&args[0], argp, sizeof(args))) - return -EFAULT; - return hpsb_iso_xmit_start(fi->iso_handle, - args[0], args[1]); - } - case RAW1394_IOC_ISO_XMIT_SYNC: - return hpsb_iso_xmit_sync(fi->iso_handle); - case RAW1394_IOC_ISO_XMIT_RECV_STOP: - hpsb_iso_stop(fi->iso_handle); - return 0; - case RAW1394_IOC_ISO_GET_STATUS: - return raw1394_iso_get_status(fi, argp); - case RAW1394_IOC_ISO_XMIT_PACKETS: - return raw1394_iso_send_packets(fi, argp); - case RAW1394_IOC_ISO_SHUTDOWN: - raw1394_iso_shutdown(fi); - return 0; - case RAW1394_IOC_ISO_QUEUE_ACTIVITY: - queue_rawiso_event(fi); - return 0; + case RAW1394_IOC_ISO_GET_STATUS: + return raw1394_iso_get_status(fi, argp); + case RAW1394_IOC_ISO_RECV_PACKETS: + return raw1394_iso_recv_packets(fi, argp); + case RAW1394_IOC_ISO_RECV_RELEASE_PACKETS: + return hpsb_iso_recv_release_packets(fi->iso_handle, arg); + case RAW1394_IOC_ISO_RECV_FLUSH: + return hpsb_iso_recv_flush(fi->iso_handle); + case RAW1394_IOC_ISO_SHUTDOWN: + raw1394_iso_shutdown(fi); + return 0; + case RAW1394_IOC_ISO_QUEUE_ACTIVITY: + queue_rawiso_event(fi); + return 0; + default: + return -EINVAL; + } +} + +static long raw1394_ioctl_xmit(struct file_info *fi, unsigned int cmd, + void __user *argp) +{ + switch (cmd) { + case RAW1394_IOC_ISO_XMIT_START:{ + int args[2]; + + if (copy_from_user(&args[0], argp, sizeof(args))) + return -EFAULT; + return hpsb_iso_xmit_start(fi->iso_handle, + args[0], args[1]); } - break; + case RAW1394_IOC_ISO_XMIT_SYNC: + return hpsb_iso_xmit_sync(fi->iso_handle); + case RAW1394_IOC_ISO_XMIT_RECV_STOP: + hpsb_iso_stop(fi->iso_handle); + return 0; + case RAW1394_IOC_ISO_GET_STATUS: + return raw1394_iso_get_status(fi, argp); + case RAW1394_IOC_ISO_XMIT_PACKETS: + return raw1394_iso_send_packets(fi, argp); + case RAW1394_IOC_ISO_SHUTDOWN: + raw1394_iso_shutdown(fi); + return 0; + case RAW1394_IOC_ISO_QUEUE_ACTIVITY: + queue_rawiso_event(fi); + return 0; default: - break; + return -EINVAL; } +} + +/* ioctl is only used for rawiso operations */ +static long raw1394_ioctl(struct file *file, unsigned int cmd, + unsigned long arg) +{ + struct file_info *fi = file->private_data; + void __user *argp = (void __user *)arg; + long ret; /* state-independent commands */ switch(cmd) { @@ -2661,18 +2665,25 @@ static long do_raw1394_ioctl(struct file *file, unsigned int cmd, break; } - return -EINVAL; -} + mutex_lock(&fi->state_mutex); -static long raw1394_ioctl(struct file *file, unsigned int cmd, - unsigned long arg) -{ - struct file_info *fi = file->private_data; - long ret; + switch (fi->iso_state) { + case RAW1394_ISO_INACTIVE: + ret = raw1394_ioctl_inactive(fi, cmd, argp); + break; + case RAW1394_ISO_RECV: + ret = raw1394_ioctl_recv(fi, cmd, arg); + break; + case RAW1394_ISO_XMIT: + ret = raw1394_ioctl_xmit(fi, cmd, argp); + break; + default: + ret = -EINVAL; + break; + } - mutex_lock(&fi->state_mutex); - ret = do_raw1394_ioctl(file, cmd, arg); mutex_unlock(&fi->state_mutex); + return ret; } @@ -2710,7 +2721,7 @@ static long raw1394_iso_xmit_recv_packets32(struct file *file, unsigned int cmd, !copy_from_user(&infos32, &arg->infos, sizeof infos32)) { infos = compat_ptr(infos32); if (!copy_to_user(&dst->infos, &infos, sizeof infos)) - err = do_raw1394_ioctl(file, cmd, (unsigned long)dst); + err = raw1394_ioctl(file, cmd, (unsigned long)dst); } return err; } @@ -2734,7 +2745,6 @@ static long raw1394_compat_ioctl(struct file *file, void __user *argp = (void __user *)arg; long err; - mutex_lock(&fi->state_mutex); switch (cmd) { /* These requests have same format as long as 'int' has same size. */ case RAW1394_IOC_ISO_RECV_INIT: @@ -2751,7 +2761,7 @@ static long raw1394_compat_ioctl(struct file *file, case RAW1394_IOC_ISO_GET_STATUS: case RAW1394_IOC_ISO_SHUTDOWN: case RAW1394_IOC_ISO_QUEUE_ACTIVITY: - err = do_raw1394_ioctl(file, cmd, arg); + err = raw1394_ioctl(file, cmd, arg); break; /* These request have different format. */ case RAW1394_IOC_ISO_RECV_PACKETS32: @@ -2767,7 +2777,6 @@ static long raw1394_compat_ioctl(struct file *file, err = -EINVAL; break; } - mutex_unlock(&fi->state_mutex); return err; } -- cgit v1.2.3 From f22e52b89e036fd12b9374212da8b5d4a447bd1e Mon Sep 17 00:00:00 2001 From: Stefan Richter Date: Sat, 16 Aug 2008 00:15:16 +0200 Subject: ieee1394: raw1394: make write() thread-safe Application programs should use a libraw1394 handle only in a single thread. The raw1394 driver was apparently relying on this, because it did nothing to protect its fi->state variable from corruption due to concurrent accesses. We now serialize the fi->state accesses. This affects the write() path. We re-use the state_mutex which was introduced to protect fi->iso_state accesses in the ioctl() path. These paths and accesses are independent of each other, hence separate mutexes could be used. But I don't see much benefit in that. Signed-off-by: Stefan Richter --- drivers/ieee1394/raw1394.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/drivers/ieee1394/raw1394.c b/drivers/ieee1394/raw1394.c index d1594427601..2cf4ae75bec 100644 --- a/drivers/ieee1394/raw1394.c +++ b/drivers/ieee1394/raw1394.c @@ -2268,6 +2268,8 @@ static ssize_t raw1394_write(struct file *file, const char __user * buffer, return -EFAULT; } + mutex_lock(&fi->state_mutex); + switch (fi->state) { case opened: retval = state_opened(fi, req); @@ -2282,6 +2284,8 @@ static ssize_t raw1394_write(struct file *file, const char __user * buffer, break; } + mutex_unlock(&fi->state_mutex); + if (retval < 0) { free_pending_request(req); } else { -- cgit v1.2.3 From d98562d12f71284593b3a5db020d6f2655061efe Mon Sep 17 00:00:00 2001 From: Stefan Richter Date: Thu, 11 Sep 2008 19:22:53 +0200 Subject: ieee1394: dv1394, video1394: remove unnecessary expressions init->channel and v.buffer are unsigned and tests for < 0 therefore always false. gcc knows this and eliminates the code, but anyway... Reported by Roel Kluin. Signed-off-by: Stefan Richter --- drivers/ieee1394/dv1394.c | 2 +- drivers/ieee1394/video1394.c | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/ieee1394/dv1394.c b/drivers/ieee1394/dv1394.c index b6eb2cf2591..df70f51279d 100644 --- a/drivers/ieee1394/dv1394.c +++ b/drivers/ieee1394/dv1394.c @@ -918,7 +918,7 @@ static int do_dv1394_init(struct video_card *video, struct dv1394_init *init) /* default SYT offset is 3 cycles */ init->syt_offset = 3; - if ( (init->channel > 63) || (init->channel < 0) ) + if (init->channel > 63) init->channel = 63; chan_mask = (u64)1 << init->channel; diff --git a/drivers/ieee1394/video1394.c b/drivers/ieee1394/video1394.c index 25db6e67fa4..fa9e7d8b51f 100644 --- a/drivers/ieee1394/video1394.c +++ b/drivers/ieee1394/video1394.c @@ -893,7 +893,7 @@ static long video1394_ioctl(struct file *file, if (unlikely(d == NULL)) return -EFAULT; - if (unlikely((v.buffer<0) || (v.buffer>=d->num_desc - 1))) { + if (unlikely(v.buffer >= d->num_desc - 1)) { PRINT(KERN_ERR, ohci->host->id, "Buffer %d out of range",v.buffer); return -EINVAL; @@ -959,7 +959,7 @@ static long video1394_ioctl(struct file *file, if (unlikely(d == NULL)) return -EFAULT; - if (unlikely((v.buffer<0) || (v.buffer>d->num_desc - 1))) { + if (unlikely(v.buffer > d->num_desc - 1)) { PRINT(KERN_ERR, ohci->host->id, "Buffer %d out of range",v.buffer); return -EINVAL; @@ -1030,7 +1030,7 @@ static long video1394_ioctl(struct file *file, d = find_ctx(&ctx->context_list, OHCI_ISO_TRANSMIT, v.channel); if (d == NULL) return -EFAULT; - if ((v.buffer<0) || (v.buffer>=d->num_desc - 1)) { + if (v.buffer >= d->num_desc - 1) { PRINT(KERN_ERR, ohci->host->id, "Buffer %d out of range",v.buffer); return -EINVAL; @@ -1137,7 +1137,7 @@ static long video1394_ioctl(struct file *file, d = find_ctx(&ctx->context_list, OHCI_ISO_TRANSMIT, v.channel); if (d == NULL) return -EFAULT; - if ((v.buffer<0) || (v.buffer>=d->num_desc-1)) { + if (v.buffer >= d->num_desc - 1) { PRINT(KERN_ERR, ohci->host->id, "Buffer %d out of range",v.buffer); return -EINVAL; -- cgit v1.2.3 From 11305c3eda233d3aff52d755a2d6c1706c509962 Mon Sep 17 00:00:00 2001 From: Stefan Richter Date: Tue, 19 Aug 2008 21:29:23 +0200 Subject: ieee1394: nodemgr clean up class iterators Remove useless pointer type casts. Remove unnecessary hi->host indirection where only host is used. Remove an unnecessary WARN_ON. Change a few names. Signed-off-by: Stefan Richter --- drivers/ieee1394/nodemgr.c | 178 +++++++++++++++++++++------------------------ 1 file changed, 81 insertions(+), 97 deletions(-) diff --git a/drivers/ieee1394/nodemgr.c b/drivers/ieee1394/nodemgr.c index 16240a78965..b9d3f46c2b0 100644 --- a/drivers/ieee1394/nodemgr.c +++ b/drivers/ieee1394/nodemgr.c @@ -154,7 +154,7 @@ struct host_info { static int nodemgr_bus_match(struct device * dev, struct device_driver * drv); static int nodemgr_uevent(struct device *dev, struct kobj_uevent_env *env); -static void nodemgr_resume_ne(struct node_entry *ne); +static void nodemgr_reactivate_ne(struct node_entry *ne); static void nodemgr_remove_ne(struct node_entry *ne); static struct node_entry *find_entry_by_guid(u64 guid); @@ -734,10 +734,10 @@ static int nodemgr_bus_match(struct device * dev, struct device_driver * drv) static DEFINE_MUTEX(nodemgr_serialize_remove_uds); -static int __match_ne(struct device *dev, void *data) +static int match_ne(struct device *dev, void *data) { struct unit_directory *ud; - struct node_entry *ne = (struct node_entry *)data; + struct node_entry *ne = data; ud = container_of(dev, struct unit_directory, unit_dev); return ud->ne == ne; @@ -754,8 +754,7 @@ static void nodemgr_remove_uds(struct node_entry *ne) */ mutex_lock(&nodemgr_serialize_remove_uds); for (;;) { - dev = class_find_device(&nodemgr_ud_class, NULL, ne, - __match_ne); + dev = class_find_device(&nodemgr_ud_class, NULL, ne, match_ne); if (!dev) break; ud = container_of(dev, struct unit_directory, unit_dev); @@ -785,7 +784,7 @@ static void nodemgr_remove_ne(struct node_entry *ne) put_device(dev); } -static int __nodemgr_remove_host_dev(struct device *dev, void *data) +static int remove_host_dev(struct device *dev, void *data) { if (dev->bus == &ieee1394_bus_type) nodemgr_remove_ne(container_of(dev, struct node_entry, @@ -795,7 +794,7 @@ static int __nodemgr_remove_host_dev(struct device *dev, void *data) static void nodemgr_remove_host_dev(struct device *dev) { - WARN_ON(device_for_each_child(dev, NULL, __nodemgr_remove_host_dev)); + device_for_each_child(dev, NULL, remove_host_dev); sysfs_remove_link(&dev->kobj, "irm_id"); sysfs_remove_link(&dev->kobj, "busmgr_id"); sysfs_remove_link(&dev->kobj, "host_id"); @@ -830,11 +829,10 @@ static void nodemgr_update_bus_options(struct node_entry *ne) } -static struct node_entry *nodemgr_create_node(octlet_t guid, struct csr1212_csr *csr, - struct host_info *hi, nodeid_t nodeid, - unsigned int generation) +static struct node_entry *nodemgr_create_node(octlet_t guid, + struct csr1212_csr *csr, struct hpsb_host *host, + nodeid_t nodeid, unsigned int generation) { - struct hpsb_host *host = hi->host; struct node_entry *ne; ne = kzalloc(sizeof(*ne), GFP_KERNEL); @@ -888,10 +886,10 @@ fail_alloc: return NULL; } -static int __match_ne_guid(struct device *dev, void *data) +static int match_ne_guid(struct device *dev, void *data) { struct node_entry *ne; - u64 *guid = (u64 *)data; + u64 *guid = data; ne = container_of(dev, struct node_entry, node_dev); return ne->guid == *guid; @@ -902,8 +900,7 @@ static struct node_entry *find_entry_by_guid(u64 guid) struct device *dev; struct node_entry *ne; - dev = class_find_device(&nodemgr_ne_class, NULL, &guid, - __match_ne_guid); + dev = class_find_device(&nodemgr_ne_class, NULL, &guid, match_ne_guid); if (!dev) return NULL; ne = container_of(dev, struct node_entry, node_dev); @@ -912,21 +909,21 @@ static struct node_entry *find_entry_by_guid(u64 guid) return ne; } -struct match_nodeid_param { +struct match_nodeid_parameter { struct hpsb_host *host; nodeid_t nodeid; }; -static int __match_ne_nodeid(struct device *dev, void *data) +static int match_ne_nodeid(struct device *dev, void *data) { int found = 0; struct node_entry *ne; - struct match_nodeid_param *param = (struct match_nodeid_param *)data; + struct match_nodeid_parameter *p = data; if (!dev) goto ret; ne = container_of(dev, struct node_entry, node_dev); - if (ne->host == param->host && ne->nodeid == param->nodeid) + if (ne->host == p->host && ne->nodeid == p->nodeid) found = 1; ret: return found; @@ -937,13 +934,12 @@ static struct node_entry *find_entry_by_nodeid(struct hpsb_host *host, { struct device *dev; struct node_entry *ne; - struct match_nodeid_param param; + struct match_nodeid_parameter p; - param.host = host; - param.nodeid = nodeid; + p.host = host; + p.nodeid = nodeid; - dev = class_find_device(&nodemgr_ne_class, NULL, ¶m, - __match_ne_nodeid); + dev = class_find_device(&nodemgr_ne_class, NULL, &p, match_ne_nodeid); if (!dev) return NULL; ne = container_of(dev, struct node_entry, node_dev); @@ -990,7 +986,7 @@ fail_devreg: * immediate unit directories looking for software_id and * software_version entries, in order to get driver autoloading working. */ static struct unit_directory *nodemgr_process_unit_directory - (struct host_info *hi, struct node_entry *ne, struct csr1212_keyval *ud_kv, + (struct node_entry *ne, struct csr1212_keyval *ud_kv, unsigned int *id, struct unit_directory *parent) { struct unit_directory *ud; @@ -1083,7 +1079,7 @@ static struct unit_directory *nodemgr_process_unit_directory nodemgr_register_device(ne, ud, &ne->device); /* process the child unit */ - ud_child = nodemgr_process_unit_directory(hi, ne, kv, id, ud); + ud_child = nodemgr_process_unit_directory(ne, kv, id, ud); if (ud_child == NULL) break; @@ -1137,7 +1133,7 @@ unit_directory_error: } -static void nodemgr_process_root_directory(struct host_info *hi, struct node_entry *ne) +static void nodemgr_process_root_directory(struct node_entry *ne) { unsigned int ud_id = 0; struct csr1212_dentry *dentry; @@ -1157,7 +1153,7 @@ static void nodemgr_process_root_directory(struct host_info *hi, struct node_ent break; case CSR1212_KV_ID_UNIT: - nodemgr_process_unit_directory(hi, ne, kv, &ud_id, NULL); + nodemgr_process_unit_directory(ne, kv, &ud_id, NULL); break; case CSR1212_KV_ID_DESCRIPTOR: @@ -1273,8 +1269,7 @@ void hpsb_unregister_protocol(struct hpsb_protocol_driver *driver) * the to take whatever actions required. */ static void nodemgr_update_node(struct node_entry *ne, struct csr1212_csr *csr, - struct host_info *hi, nodeid_t nodeid, - unsigned int generation) + nodeid_t nodeid, unsigned int generation) { if (ne->nodeid != nodeid) { HPSB_DEBUG("Node changed: " NODE_BUS_FMT " -> " NODE_BUS_FMT, @@ -1306,7 +1301,7 @@ static void nodemgr_update_node(struct node_entry *ne, struct csr1212_csr *csr, } if (ne->in_limbo) - nodemgr_resume_ne(ne); + nodemgr_reactivate_ne(ne); /* Mark the node current */ ne->generation = generation; @@ -1314,10 +1309,9 @@ static void nodemgr_update_node(struct node_entry *ne, struct csr1212_csr *csr, -static void nodemgr_node_scan_one(struct host_info *hi, +static void nodemgr_node_scan_one(struct hpsb_host *host, nodeid_t nodeid, int generation) { - struct hpsb_host *host = hi->host; struct node_entry *ne; octlet_t guid; struct csr1212_csr *csr; @@ -1373,16 +1367,15 @@ static void nodemgr_node_scan_one(struct host_info *hi, } if (!ne) - nodemgr_create_node(guid, csr, hi, nodeid, generation); + nodemgr_create_node(guid, csr, host, nodeid, generation); else - nodemgr_update_node(ne, csr, hi, nodeid, generation); + nodemgr_update_node(ne, csr, nodeid, generation); } -static void nodemgr_node_scan(struct host_info *hi, int generation) +static void nodemgr_node_scan(struct hpsb_host *host, int generation) { int count; - struct hpsb_host *host = hi->host; struct selfid *sid = (struct selfid *)host->topology_map; nodeid_t nodeid = LOCAL_BUS; @@ -1395,15 +1388,15 @@ static void nodemgr_node_scan(struct host_info *hi, int generation) nodeid++; continue; } - nodemgr_node_scan_one(hi, nodeid++, generation); + nodemgr_node_scan_one(host, nodeid++, generation); } } -static int __nodemgr_driver_suspend(struct device *dev, void *data) +static int pause_ne(struct device *dev, void *data) { struct unit_directory *ud; struct device_driver *drv; - struct node_entry *ne = (struct node_entry *)data; + struct node_entry *ne = data; int error; ud = container_of(dev, struct unit_directory, unit_dev); @@ -1425,11 +1418,23 @@ static int __nodemgr_driver_suspend(struct device *dev, void *data) return 0; } -static int __nodemgr_driver_resume(struct device *dev, void *data) +static void nodemgr_pause_ne(struct node_entry *ne) +{ + HPSB_DEBUG("Node suspended: ID:BUS[" NODE_BUS_FMT "] GUID[%016Lx]", + NODE_BUS_ARGS(ne->host, ne->nodeid), + (unsigned long long)ne->guid); + + ne->in_limbo = 1; + WARN_ON(device_create_file(&ne->device, &dev_attr_ne_in_limbo)); + + class_for_each_device(&nodemgr_ud_class, NULL, ne, pause_ne); +} + +static int reactivate_ne(struct device *dev, void *data) { struct unit_directory *ud; struct device_driver *drv; - struct node_entry *ne = (struct node_entry *)data; + struct node_entry *ne = data; ud = container_of(dev, struct unit_directory, unit_dev); if (ud->ne == ne) { @@ -1447,37 +1452,23 @@ static int __nodemgr_driver_resume(struct device *dev, void *data) return 0; } -static void nodemgr_suspend_ne(struct node_entry *ne) -{ - HPSB_DEBUG("Node suspended: ID:BUS[" NODE_BUS_FMT "] GUID[%016Lx]", - NODE_BUS_ARGS(ne->host, ne->nodeid), - (unsigned long long)ne->guid); - - ne->in_limbo = 1; - WARN_ON(device_create_file(&ne->device, &dev_attr_ne_in_limbo)); - - class_for_each_device(&nodemgr_ud_class, NULL, ne, - __nodemgr_driver_suspend); -} - - -static void nodemgr_resume_ne(struct node_entry *ne) +static void nodemgr_reactivate_ne(struct node_entry *ne) { ne->in_limbo = 0; device_remove_file(&ne->device, &dev_attr_ne_in_limbo); - class_for_each_device(&nodemgr_ud_class, NULL, ne, - __nodemgr_driver_resume); + class_for_each_device(&nodemgr_ud_class, NULL, ne, reactivate_ne); HPSB_DEBUG("Node resumed: ID:BUS[" NODE_BUS_FMT "] GUID[%016Lx]", - NODE_BUS_ARGS(ne->host, ne->nodeid), (unsigned long long)ne->guid); + NODE_BUS_ARGS(ne->host, ne->nodeid), + (unsigned long long)ne->guid); } -static int __nodemgr_update_pdrv(struct device *dev, void *data) +static int update_pdrv(struct device *dev, void *data) { struct unit_directory *ud; struct device_driver *drv; struct hpsb_protocol_driver *pdrv; - struct node_entry *ne = (struct node_entry *)data; + struct node_entry *ne = data; int error; ud = container_of(dev, struct unit_directory, unit_dev); @@ -1503,8 +1494,7 @@ static int __nodemgr_update_pdrv(struct device *dev, void *data) static void nodemgr_update_pdrv(struct node_entry *ne) { - class_for_each_device(&nodemgr_ud_class, NULL, ne, - __nodemgr_update_pdrv); + class_for_each_device(&nodemgr_ud_class, NULL, ne, update_pdrv); } @@ -1535,11 +1525,12 @@ static void nodemgr_irm_write_bc(struct node_entry *ne, int generation) } -static void nodemgr_probe_ne(struct host_info *hi, struct node_entry *ne, int generation) +static void nodemgr_probe_ne(struct hpsb_host *host, struct node_entry *ne, + int generation) { struct device *dev; - if (ne->host != hi->host || ne->in_limbo) + if (ne->host != host || ne->in_limbo) return; dev = get_device(&ne->device); @@ -1554,40 +1545,40 @@ static void nodemgr_probe_ne(struct host_info *hi, struct node_entry *ne, int ge * down to the drivers. Otherwise, this is a dead node and we * suspend it. */ if (ne->needs_probe) - nodemgr_process_root_directory(hi, ne); + nodemgr_process_root_directory(ne); else if (ne->generation == generation) nodemgr_update_pdrv(ne); else - nodemgr_suspend_ne(ne); + nodemgr_pause_ne(ne); put_device(dev); } -struct probe_param { - struct host_info *hi; +struct node_probe_parameter { + struct hpsb_host *host; int generation; bool probe_now; }; static int node_probe(struct device *dev, void *data) { - struct probe_param *p = data; + struct node_probe_parameter *p = data; struct node_entry *ne; - if (p->generation != get_hpsb_generation(p->hi->host)) + if (p->generation != get_hpsb_generation(p->host)) return -EAGAIN; ne = container_of(dev, struct node_entry, node_dev); if (ne->needs_probe == p->probe_now) - nodemgr_probe_ne(p->hi, ne, p->generation); + nodemgr_probe_ne(p->host, ne, p->generation); return 0; } -static void nodemgr_node_probe(struct host_info *hi, int generation) +static void nodemgr_node_probe(struct hpsb_host *host, int generation) { - struct probe_param p; + struct node_probe_parameter p; - p.hi = hi; + p.host = host; p.generation = generation; /* * Do some processing of the nodes we've probed. This pulls them @@ -1730,10 +1721,9 @@ static int nodemgr_check_irm_capability(struct hpsb_host *host, int cycles) return 1; } -static int nodemgr_host_thread(void *__hi) +static int nodemgr_host_thread(void *data) { - struct host_info *hi = (struct host_info *)__hi; - struct hpsb_host *host = hi->host; + struct hpsb_host *host = data; unsigned int g, generation = 0; int i, reset_cycles = 0; @@ -1787,11 +1777,11 @@ static int nodemgr_host_thread(void *__hi) * entries. This does not do the sysfs stuff, since that * would trigger uevents and such, which is a bad idea at * this point. */ - nodemgr_node_scan(hi, generation); + nodemgr_node_scan(host, generation); /* This actually does the full probe, with sysfs * registration. */ - nodemgr_node_probe(hi, generation); + nodemgr_node_probe(host, generation); /* Update some of our sysfs symlinks */ nodemgr_update_host_dev_links(host); @@ -1801,22 +1791,20 @@ exit: return 0; } -struct host_iter_param { +struct per_host_parameter { void *data; int (*cb)(struct hpsb_host *, void *); }; -static int __nodemgr_for_each_host(struct device *dev, void *data) +static int per_host(struct device *dev, void *data) { struct hpsb_host *host; - struct host_iter_param *hip = (struct host_iter_param *)data; - int error = 0; + struct per_host_parameter *p = data; host = container_of(dev, struct hpsb_host, host_dev); - error = hip->cb(host, hip->data); - - return error; + return p->cb(host, p->data); } + /** * nodemgr_for_each_host - call a function for each IEEE 1394 host * @data: an address to supply to the callback @@ -1831,15 +1819,11 @@ static int __nodemgr_for_each_host(struct device *dev, void *data) */ int nodemgr_for_each_host(void *data, int (*cb)(struct hpsb_host *, void *)) { - struct host_iter_param hip; - int error; + struct per_host_parameter p; - hip.cb = cb; - hip.data = data; - error = class_for_each_device(&hpsb_host_class, NULL, &hip, - __nodemgr_for_each_host); - - return error; + p.cb = cb; + p.data = data; + return class_for_each_device(&hpsb_host_class, NULL, &p, per_host); } /* The following two convenience functions use a struct node_entry @@ -1893,7 +1877,7 @@ static void nodemgr_add_host(struct hpsb_host *host) return; } hi->host = host; - hi->thread = kthread_run(nodemgr_host_thread, hi, "knodemgrd_%d", + hi->thread = kthread_run(nodemgr_host_thread, host, "knodemgrd_%d", host->id); if (IS_ERR(hi->thread)) { HPSB_ERR("NodeMgr: cannot start thread for host %d", host->id); -- cgit v1.2.3 From fc392fe83176cefbab99f9d12e6e27395aa2b5d0 Mon Sep 17 00:00:00 2001 From: Stefan Richter Date: Tue, 19 Aug 2008 21:30:17 +0200 Subject: ieee1394: survive a few seconds connection loss There are situations when nodes vanish from the bus and come back in quickly thereafter: - When certain bus-powered hubs are plugged in, - when certain disk enclosures are switched from self-power to bus power or vice versa and break the daisy chain during the transition, - when the user plugs a cable out and quickly plugs it back in, e.g. to reorder a daisy chain (works on Mac OS X if done quickly enough), - when certain hubs temporarily malfunction during high bus traffic. The ieee1394 driver's nodemgr already contained a function to set vanished nodes aside into "limbo"; i.e. they wouldn't actually be deleted right away. (In fact, only unloading the driver or writing into an obscure sysfs attribute would delete them eventually.) If nodes reappeared later, they would be resurrected out of limbo. Moving nodes into and out of limbo was accompanied with calling the .suspend() and .resume() driver methods of the drivers which were bound to a respective node's unit directories. Not only is this somewhat strange due to the intended use of these driver methods for power management, also the sbp2 driver in particular does not implement .suspend() and .resume(). Hence sbp2 would be disconnected from devices in situations as listed above. We now: - leave drivers bound when nodes go into limbo, - call the drivers' .update() when nodes come out of limbo, - automatically delete in-limbo nodes 3 seconds after the last bus reset and bus rescan. - Because of the automatic removal, the now obsolete bus attribute /sys/bus/ieee1394/destroy_node is removed. This especially lets sbp2 survive brief disconnections. You can for example yank a disk's cable and plug it back in while reading the respective disk with dd, but dd will happily continue as if nothing happened. Signed-off-by: Stefan Richter --- drivers/ieee1394/nodemgr.c | 147 +++++++++++++++------------------------------ drivers/ieee1394/nodemgr.h | 2 +- 2 files changed, 51 insertions(+), 98 deletions(-) diff --git a/drivers/ieee1394/nodemgr.c b/drivers/ieee1394/nodemgr.c index b9d3f46c2b0..2376b729e87 100644 --- a/drivers/ieee1394/nodemgr.c +++ b/drivers/ieee1394/nodemgr.c @@ -154,9 +154,6 @@ struct host_info { static int nodemgr_bus_match(struct device * dev, struct device_driver * drv); static int nodemgr_uevent(struct device *dev, struct kobj_uevent_env *env); -static void nodemgr_reactivate_ne(struct node_entry *ne); -static void nodemgr_remove_ne(struct node_entry *ne); -static struct node_entry *find_entry_by_guid(u64 guid); struct bus_type ieee1394_bus_type = { .name = "ieee1394", @@ -385,27 +382,6 @@ static ssize_t fw_get_ignore_driver(struct device *dev, struct device_attribute static DEVICE_ATTR(ignore_driver, S_IWUSR | S_IRUGO, fw_get_ignore_driver, fw_set_ignore_driver); -static ssize_t fw_set_destroy_node(struct bus_type *bus, const char *buf, size_t count) -{ - struct node_entry *ne; - u64 guid = (u64)simple_strtoull(buf, NULL, 16); - - ne = find_entry_by_guid(guid); - - if (ne == NULL || !ne->in_limbo) - return -EINVAL; - - nodemgr_remove_ne(ne); - - return count; -} -static ssize_t fw_get_destroy_node(struct bus_type *bus, char *buf) -{ - return sprintf(buf, "You can destroy in_limbo nodes by writing their GUID to this file\n"); -} -static BUS_ATTR(destroy_node, S_IWUSR | S_IRUGO, fw_get_destroy_node, fw_set_destroy_node); - - static ssize_t fw_set_rescan(struct bus_type *bus, const char *buf, size_t count) { @@ -442,7 +418,6 @@ static BUS_ATTR(ignore_drivers, S_IWUSR | S_IRUGO, fw_get_ignore_drivers, fw_set struct bus_attribute *const fw_bus_attrs[] = { - &bus_attr_destroy_node, &bus_attr_rescan, &bus_attr_ignore_drivers, NULL @@ -1300,14 +1275,19 @@ static void nodemgr_update_node(struct node_entry *ne, struct csr1212_csr *csr, csr1212_destroy_csr(csr); } - if (ne->in_limbo) - nodemgr_reactivate_ne(ne); - /* Mark the node current */ ne->generation = generation; -} + if (ne->in_limbo) { + device_remove_file(&ne->device, &dev_attr_ne_in_limbo); + ne->in_limbo = false; + HPSB_DEBUG("Node reactivated: " + "ID:BUS[" NODE_BUS_FMT "] GUID[%016Lx]", + NODE_BUS_ARGS(ne->host, ne->nodeid), + (unsigned long long)ne->guid); + } +} static void nodemgr_node_scan_one(struct hpsb_host *host, nodeid_t nodeid, int generation) @@ -1392,75 +1372,14 @@ static void nodemgr_node_scan(struct hpsb_host *host, int generation) } } -static int pause_ne(struct device *dev, void *data) -{ - struct unit_directory *ud; - struct device_driver *drv; - struct node_entry *ne = data; - int error; - - ud = container_of(dev, struct unit_directory, unit_dev); - if (ud->ne == ne) { - drv = get_driver(ud->device.driver); - if (drv) { - error = 1; /* release if suspend is not implemented */ - if (drv->suspend) { - down(&ud->device.sem); - error = drv->suspend(&ud->device, PMSG_SUSPEND); - up(&ud->device.sem); - } - if (error) - device_release_driver(&ud->device); - put_driver(drv); - } - } - - return 0; -} - static void nodemgr_pause_ne(struct node_entry *ne) { - HPSB_DEBUG("Node suspended: ID:BUS[" NODE_BUS_FMT "] GUID[%016Lx]", + HPSB_DEBUG("Node paused: ID:BUS[" NODE_BUS_FMT "] GUID[%016Lx]", NODE_BUS_ARGS(ne->host, ne->nodeid), (unsigned long long)ne->guid); - ne->in_limbo = 1; + ne->in_limbo = true; WARN_ON(device_create_file(&ne->device, &dev_attr_ne_in_limbo)); - - class_for_each_device(&nodemgr_ud_class, NULL, ne, pause_ne); -} - -static int reactivate_ne(struct device *dev, void *data) -{ - struct unit_directory *ud; - struct device_driver *drv; - struct node_entry *ne = data; - - ud = container_of(dev, struct unit_directory, unit_dev); - if (ud->ne == ne) { - drv = get_driver(ud->device.driver); - if (drv) { - if (drv->resume) { - down(&ud->device.sem); - drv->resume(&ud->device); - up(&ud->device.sem); - } - put_driver(drv); - } - } - - return 0; -} - -static void nodemgr_reactivate_ne(struct node_entry *ne) -{ - ne->in_limbo = 0; - device_remove_file(&ne->device, &dev_attr_ne_in_limbo); - - class_for_each_device(&nodemgr_ud_class, NULL, ne, reactivate_ne); - HPSB_DEBUG("Node resumed: ID:BUS[" NODE_BUS_FMT "] GUID[%016Lx]", - NODE_BUS_ARGS(ne->host, ne->nodeid), - (unsigned long long)ne->guid); } static int update_pdrv(struct device *dev, void *data) @@ -1497,7 +1416,6 @@ static void nodemgr_update_pdrv(struct node_entry *ne) class_for_each_device(&nodemgr_ud_class, NULL, ne, update_pdrv); } - /* Write the BROADCAST_CHANNEL as per IEEE1394a 8.3.2.3.11 and 8.4.2.3. This * seems like an optional service but in the end it is practically mandatory * as a consequence of these clauses. @@ -1574,7 +1492,7 @@ static int node_probe(struct device *dev, void *data) return 0; } -static void nodemgr_node_probe(struct hpsb_host *host, int generation) +static int nodemgr_node_probe(struct hpsb_host *host, int generation) { struct node_probe_parameter p; @@ -1595,11 +1513,11 @@ static void nodemgr_node_probe(struct hpsb_host *host, int generation) */ p.probe_now = false; if (class_for_each_device(&nodemgr_ne_class, NULL, &p, node_probe) != 0) - return; + return 0; p.probe_now = true; if (class_for_each_device(&nodemgr_ne_class, NULL, &p, node_probe) != 0) - return; + return 0; /* * Now let's tell the bus to rescan our devices. This may seem * like overhead, but the driver-model core will only scan a @@ -1611,6 +1529,27 @@ static void nodemgr_node_probe(struct hpsb_host *host, int generation) */ if (bus_rescan_devices(&ieee1394_bus_type) != 0) HPSB_DEBUG("bus_rescan_devices had an error"); + + return 1; +} + +static int remove_nodes_in_limbo(struct device *dev, void *data) +{ + struct node_entry *ne; + + if (dev->bus != &ieee1394_bus_type) + return 0; + + ne = container_of(dev, struct node_entry, device); + if (ne->in_limbo) + nodemgr_remove_ne(ne); + + return 0; +} + +static void nodemgr_remove_nodes_in_limbo(struct hpsb_host *host) +{ + device_for_each_child(&host->device, NULL, remove_nodes_in_limbo); } static int nodemgr_send_resume_packet(struct hpsb_host *host) @@ -1781,10 +1720,24 @@ static int nodemgr_host_thread(void *data) /* This actually does the full probe, with sysfs * registration. */ - nodemgr_node_probe(host, generation); + if (!nodemgr_node_probe(host, generation)) + continue; /* Update some of our sysfs symlinks */ nodemgr_update_host_dev_links(host); + + /* Sleep 3 seconds */ + for (i = 3000/200; i; i--) { + msleep_interruptible(200); + if (kthread_should_stop()) + goto exit; + + if (generation != get_hpsb_generation(host)) + break; + } + /* Remove nodes which are gone, unless a bus reset happened */ + if (!i) + nodemgr_remove_nodes_in_limbo(host); } exit: HPSB_VERBOSE("NodeMgr: Exiting thread"); diff --git a/drivers/ieee1394/nodemgr.h b/drivers/ieee1394/nodemgr.h index 6eb26465a84..4f287a3561b 100644 --- a/drivers/ieee1394/nodemgr.h +++ b/drivers/ieee1394/nodemgr.h @@ -110,7 +110,7 @@ struct node_entry { struct device node_dev; /* Means this node is not attached anymore */ - int in_limbo; + bool in_limbo; struct csr1212_csr *csr; }; -- cgit v1.2.3 From 1e119fa9950dfe0e6d97470098db776110ca47a9 Mon Sep 17 00:00:00 2001 From: Jay Fenlason Date: Sun, 20 Jul 2008 14:20:53 +0200 Subject: firewire: fw_send_request_sync() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Share code between fw_send_request + wait_for_completion callers. Signed-off-by: Jay Fenlason Addendum: Removes an unnecessary struct and an ununsed retry loop. Calls it fw_run_transaction() instead of fw_send_request_sync(). Signed-off-by: Stefan Richter Acked-by: Kristian Høgsberg --- drivers/firewire/fw-card.c | 56 ++++++++++----------------------------- drivers/firewire/fw-device.c | 37 +++++--------------------- drivers/firewire/fw-sbp2.c | 46 +++++++++----------------------- drivers/firewire/fw-transaction.c | 48 ++++++++++++++++++++++++++++++--- drivers/firewire/fw-transaction.h | 9 ++++--- 5 files changed, 82 insertions(+), 114 deletions(-) diff --git a/drivers/firewire/fw-card.c b/drivers/firewire/fw-card.c index bbd73a406e5..418c18f07e9 100644 --- a/drivers/firewire/fw-card.c +++ b/drivers/firewire/fw-card.c @@ -189,39 +189,16 @@ static const char gap_count_table[] = { 63, 5, 7, 8, 10, 13, 16, 18, 21, 24, 26, 29, 32, 35, 37, 40 }; -struct bm_data { - struct fw_transaction t; - struct { - __be32 arg; - __be32 data; - } lock; - u32 old; - int rcode; - struct completion done; -}; - -static void -complete_bm_lock(struct fw_card *card, int rcode, - void *payload, size_t length, void *data) -{ - struct bm_data *bmd = data; - - if (rcode == RCODE_COMPLETE) - bmd->old = be32_to_cpu(*(__be32 *) payload); - bmd->rcode = rcode; - complete(&bmd->done); -} - 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_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 root_id, new_root_id, irm_id, gap_count, generation, grace, rcode; bool do_reset = false; + __be32 lock_data[2]; spin_lock_irqsave(&card->lock, flags); local_node = card->local_node; @@ -263,33 +240,28 @@ fw_card_bm_work(struct work_struct *work) goto pick_me; } - bmd.lock.arg = cpu_to_be32(0x3f); - bmd.lock.data = cpu_to_be32(local_node->node_id); + lock_data[0] = cpu_to_be32(0x3f); + lock_data[1] = cpu_to_be32(local_node->node_id); spin_unlock_irqrestore(&card->lock, flags); - init_completion(&bmd.done); - fw_send_request(card, &bmd.t, TCODE_LOCK_COMPARE_SWAP, - irm_id, generation, - SCODE_100, CSR_REGISTER_BASE + CSR_BUS_MANAGER_ID, - &bmd.lock, sizeof(bmd.lock), - complete_bm_lock, &bmd); - wait_for_completion(&bmd.done); + rcode = fw_run_transaction(card, TCODE_LOCK_COMPARE_SWAP, + irm_id, generation, SCODE_100, + CSR_REGISTER_BASE + CSR_BUS_MANAGER_ID, + lock_data, sizeof(lock_data)); - if (bmd.rcode == RCODE_GENERATION) { - /* - * Another bus reset happened. Just return, - * the BM work has been rescheduled. - */ + if (rcode == RCODE_GENERATION) + /* Another bus reset, BM work has been rescheduled. */ goto out; - } - if (bmd.rcode == RCODE_COMPLETE && bmd.old != 0x3f) + if (rcode == RCODE_COMPLETE && + lock_data[0] != cpu_to_be32(0x3f)) /* Somebody else is BM, let them do the work. */ goto out; spin_lock_irqsave(&card->lock, flags); - if (bmd.rcode != RCODE_COMPLETE) { + + if (rcode != RCODE_COMPLETE) { /* * The lock request failed, maybe the IRM * isn't really IRM capable after all. Let's diff --git a/drivers/firewire/fw-device.c b/drivers/firewire/fw-device.c index 0855fb5568e..3fccdd48410 100644 --- a/drivers/firewire/fw-device.c +++ b/drivers/firewire/fw-device.c @@ -381,46 +381,21 @@ static struct device_attribute fw_device_attributes[] = { __ATTR_NULL, }; -struct read_quadlet_callback_data { - struct completion done; - int rcode; - u32 data; -}; - -static void -complete_transaction(struct fw_card *card, int rcode, - void *payload, size_t length, void *data) -{ - struct read_quadlet_callback_data *callback_data = data; - - if (rcode == RCODE_COMPLETE) - callback_data->data = be32_to_cpu(*(__be32 *)payload); - callback_data->rcode = rcode; - complete(&callback_data->done); -} - 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 rcode; /* device->node_id, accessed below, must not be older than generation */ smp_rmb(); - init_completion(&callback_data.done); - - offset = (CSR_REGISTER_BASE | CSR_CONFIG_ROM) + index * 4; - fw_send_request(device->card, &t, TCODE_READ_QUADLET_REQUEST, + rcode = fw_run_transaction(device->card, TCODE_READ_QUADLET_REQUEST, device->node_id, generation, device->max_speed, - offset, NULL, 4, complete_transaction, &callback_data); - - wait_for_completion(&callback_data.done); - - *data = callback_data.data; + (CSR_REGISTER_BASE | CSR_CONFIG_ROM) + index * 4, + data, 4); + be32_to_cpus(data); - return callback_data.rcode; + return rcode; } #define READ_BIB_ROM_SIZE 256 diff --git a/drivers/firewire/fw-sbp2.c b/drivers/firewire/fw-sbp2.c index aaff50ebba1..05997cee4f3 100644 --- a/drivers/firewire/fw-sbp2.c +++ b/drivers/firewire/fw-sbp2.c @@ -621,25 +621,15 @@ sbp2_send_management_orb(struct sbp2_logical_unit *lu, int node_id, return retval; } -static void -complete_agent_reset_write(struct fw_card *card, int rcode, - void *payload, size_t length, void *done) -{ - 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; + __be32 d = 0; - 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); + fw_run_transaction(device->card, TCODE_WRITE_QUADLET_REQUEST, + lu->tgt->node_id, lu->generation, device->max_speed, + lu->command_block_agent_address + SBP2_AGENT_RESET, + &d, sizeof(d)); } static void @@ -653,7 +643,7 @@ 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 z; + static __be32 d; t = kmalloc(sizeof(*t), GFP_ATOMIC); if (t == NULL) @@ -662,7 +652,7 @@ static void sbp2_agent_reset_no_wait(struct sbp2_logical_unit *lu) 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_no_wait, t); + &d, sizeof(d), complete_agent_reset_write_no_wait, t); } static void sbp2_set_generation(struct sbp2_logical_unit *lu, int generation) @@ -823,13 +813,6 @@ 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); -} - /* * Write retransmit retry values into the BUSY_TIMEOUT register. * - The single-phase retry protocol is supported by all SBP-2 devices, but the @@ -849,17 +832,12 @@ complete_set_busy_timeout(struct fw_card *card, int rcode, 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; - - busy_timeout = cpu_to_be32(SBP2_CYCLE_LIMIT | SBP2_RETRY_LIMIT); + __be32 d = cpu_to_be32(SBP2_CYCLE_LIMIT | 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); + fw_run_transaction(device->card, TCODE_WRITE_QUADLET_REQUEST, + lu->tgt->node_id, lu->generation, device->max_speed, + CSR_REGISTER_BASE + CSR_BUSY_TIMEOUT, + &d, sizeof(d)); } static void sbp2_reconnect(struct work_struct *work); diff --git a/drivers/firewire/fw-transaction.c b/drivers/firewire/fw-transaction.c index e5d1a0b64fc..022ac4fabb6 100644 --- a/drivers/firewire/fw-transaction.c +++ b/drivers/firewire/fw-transaction.c @@ -247,7 +247,7 @@ fw_fill_request(struct fw_packet *packet, int tcode, int tlabel, */ void fw_send_request(struct fw_card *card, struct fw_transaction *t, - int tcode, int node_id, int generation, int speed, + int tcode, int destination_id, int generation, int speed, unsigned long long offset, void *payload, size_t length, fw_transaction_callback_t callback, void *callback_data) @@ -279,13 +279,14 @@ fw_send_request(struct fw_card *card, struct fw_transaction *t, card->current_tlabel = (card->current_tlabel + 1) & 0x1f; card->tlabel_mask |= (1 << tlabel); - t->node_id = node_id; + t->node_id = destination_id; t->tlabel = tlabel; t->callback = callback; t->callback_data = callback_data; - fw_fill_request(&t->packet, tcode, t->tlabel, node_id, card->node_id, - generation, speed, offset, payload, length); + fw_fill_request(&t->packet, tcode, t->tlabel, + destination_id, card->node_id, generation, + speed, offset, payload, length); t->packet.callback = transmit_complete_callback; list_add_tail(&t->link, &card->transaction_list); @@ -296,6 +297,45 @@ fw_send_request(struct fw_card *card, struct fw_transaction *t, } EXPORT_SYMBOL(fw_send_request); +struct transaction_callback_data { + struct completion done; + void *payload; + int rcode; +}; + +static void transaction_callback(struct fw_card *card, int rcode, + void *payload, size_t length, void *data) +{ + struct transaction_callback_data *d = data; + + if (rcode == RCODE_COMPLETE) + memcpy(d->payload, payload, length); + d->rcode = rcode; + complete(&d->done); +} + +/** + * fw_run_transaction - send request and sleep until transaction is completed + * + * Returns the RCODE. + */ +int fw_run_transaction(struct fw_card *card, int tcode, int destination_id, + int generation, int speed, unsigned long long offset, + void *data, size_t length) +{ + struct transaction_callback_data d; + struct fw_transaction t; + + init_completion(&d.done); + d.payload = data; + fw_send_request(card, &t, tcode, destination_id, generation, speed, + offset, data, length, transaction_callback, &d); + wait_for_completion(&d.done); + + return d.rcode; +} +EXPORT_SYMBOL(fw_run_transaction); + static DEFINE_MUTEX(phy_config_mutex); static DECLARE_COMPLETION(phy_config_done); diff --git a/drivers/firewire/fw-transaction.h b/drivers/firewire/fw-transaction.h index 2ae1b0d6cb7..027f58ce81a 100644 --- a/drivers/firewire/fw-transaction.h +++ b/drivers/firewire/fw-transaction.h @@ -426,11 +426,14 @@ fw_core_initiate_bus_reset(struct fw_card *card, int short_reset); void fw_send_request(struct fw_card *card, struct fw_transaction *t, - int tcode, int node_id, int generation, int speed, - unsigned long long offset, - void *data, size_t length, + int tcode, int destination_id, int generation, int speed, + unsigned long long offset, void *data, size_t length, fw_transaction_callback_t callback, void *callback_data); +int fw_run_transaction(struct fw_card *card, int tcode, int destination_id, + int generation, int speed, unsigned long long offset, + void *data, size_t length); + int fw_cancel_transaction(struct fw_card *card, struct fw_transaction *transaction); -- cgit v1.2.3 From 09b12dd4e3caff165a0f17a2f3ebd2bbc8544cc6 Mon Sep 17 00:00:00 2001 From: Stefan Richter Date: Thu, 14 Aug 2008 21:47:21 +0200 Subject: firewire: fw-sbp2: enforce s/g segment size limit 1. We don't need to round the SBP-2 segment size limit down to a multiple of 4 kB (0xffff -> 0xf000). It is only necessary to ensure quadlet alignment (0xffff -> 0xfffc). 2. Use dma_set_max_seg_size() to tell the DMA mapping infrastructure and the block IO layer about the restriction. This way we can remove the size checks and segment splitting in the queuecommand path. This assumes that no other code in the firewire stack uses dma_map_sg() with conflicting requirements. It furthermore assumes that the controller device's platform actually allows us to set the segment size to our liking. Assert the latter with a BUG_ON(). 3. Also use blk_queue_max_segment_size() to tell the block IO layer about it. It cannot know it because our scsi_add_host() does not point to the FireWire controller's device. Thanks to Grant Grundler and FUJITA Tomonori for advice. Signed-off-by: Stefan Richter --- drivers/firewire/fw-sbp2.c | 63 ++++++++++++++++++++-------------------------- 1 file changed, 27 insertions(+), 36 deletions(-) diff --git a/drivers/firewire/fw-sbp2.c b/drivers/firewire/fw-sbp2.c index 05997cee4f3..5d8411afced 100644 --- a/drivers/firewire/fw-sbp2.c +++ b/drivers/firewire/fw-sbp2.c @@ -29,6 +29,7 @@ */ #include +#include #include #include #include @@ -181,10 +182,16 @@ struct sbp2_target { #define SBP2_MAX_LOGIN_ORB_TIMEOUT 40000U /* Timeout in ms */ #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_CYCLE_LIMIT (0xc8 << 12) /* 200 125us cycles */ +/* + * The default maximum s/g segment size of a FireWire controller is + * usually 0x10000, but SBP-2 only allows 0xffff. Since buffers have to + * be quadlet-aligned, we set the length limit to 0xffff & ~3. + */ +#define SBP2_MAX_SEG_SIZE 0xfffc + /* Unit directory keys */ #define SBP2_CSR_UNIT_CHARACTERISTICS 0x3a #define SBP2_CSR_FIRMWARE_REVISION 0x3c @@ -1099,6 +1106,10 @@ static int sbp2_probe(struct device *dev) struct Scsi_Host *shost; u32 model, firmware_revision; + if (dma_get_max_seg_size(device->card->device) > SBP2_MAX_SEG_SIZE) + BUG_ON(dma_set_max_seg_size(device->card->device, + SBP2_MAX_SEG_SIZE)); + shost = scsi_host_alloc(&scsi_driver_template, sizeof(*tgt)); if (shost == NULL) return -ENOMEM; @@ -1347,14 +1358,12 @@ static int sbp2_map_scatterlist(struct sbp2_command_orb *orb, struct fw_device *device, struct sbp2_logical_unit *lu) { - struct scatterlist *sg; - int sg_len, l, i, j, count; - dma_addr_t sg_addr; - - sg = scsi_sglist(orb->cmd); - count = dma_map_sg(device->card->device, sg, scsi_sg_count(orb->cmd), - orb->cmd->sc_data_direction); - if (count == 0) + struct scatterlist *sg = scsi_sglist(orb->cmd); + int i, n; + + n = dma_map_sg(device->card->device, sg, scsi_sg_count(orb->cmd), + orb->cmd->sc_data_direction); + if (n == 0) goto fail; /* @@ -1364,7 +1373,7 @@ sbp2_map_scatterlist(struct sbp2_command_orb *orb, struct fw_device *device, * as the second generation iPod which doesn't support page * tables. */ - if (count == 1 && sg_dma_len(sg) < SBP2_MAX_SG_ELEMENT_LENGTH) { + if (n == 1) { orb->request.data_descriptor.high = cpu_to_be32(lu->tgt->address_high); orb->request.data_descriptor.low = @@ -1374,29 +1383,9 @@ sbp2_map_scatterlist(struct sbp2_command_orb *orb, struct fw_device *device, return 0; } - /* - * Convert the scatterlist to an sbp2 page table. If any - * scatterlist entries are too big for sbp2, we split them as we - * go. Even if we ask the block I/O layer to not give us sg - * elements larger than 65535 bytes, some IOMMUs may merge sg elements - * during DMA mapping, and Linux currently doesn't prevent this. - */ - for (i = 0, j = 0; i < count; i++, sg = sg_next(sg)) { - sg_len = sg_dma_len(sg); - sg_addr = sg_dma_address(sg); - while (sg_len) { - /* FIXME: This won't get us out of the pinch. */ - if (unlikely(j >= ARRAY_SIZE(orb->page_table))) { - fw_error("page table overflow\n"); - goto fail_page_table; - } - l = min(sg_len, SBP2_MAX_SG_ELEMENT_LENGTH); - orb->page_table[j].low = cpu_to_be32(sg_addr); - orb->page_table[j].high = cpu_to_be32(l << 16); - sg_addr += l; - sg_len -= l; - j++; - } + for_each_sg(sg, sg, n, i) { + orb->page_table[i].high = cpu_to_be32(sg_dma_len(sg) << 16); + orb->page_table[i].low = cpu_to_be32(sg_dma_address(sg)); } orb->page_table_bus = @@ -1415,13 +1404,13 @@ sbp2_map_scatterlist(struct sbp2_command_orb *orb, struct fw_device *device, orb->request.data_descriptor.high = cpu_to_be32(lu->tgt->address_high); orb->request.data_descriptor.low = cpu_to_be32(orb->page_table_bus); orb->request.misc |= cpu_to_be32(COMMAND_ORB_PAGE_TABLE_PRESENT | - COMMAND_ORB_DATA_SIZE(j)); + COMMAND_ORB_DATA_SIZE(n)); return 0; fail_page_table: - dma_unmap_sg(device->card->device, sg, scsi_sg_count(orb->cmd), - orb->cmd->sc_data_direction); + dma_unmap_sg(device->card->device, scsi_sglist(orb->cmd), + scsi_sg_count(orb->cmd), orb->cmd->sc_data_direction); fail: return -ENOMEM; } @@ -1542,6 +1531,8 @@ static int sbp2_scsi_slave_configure(struct scsi_device *sdev) if (lu->tgt->workarounds & SBP2_WORKAROUND_128K_MAX_TRANS) blk_queue_max_sectors(sdev->request_queue, 128 * 1024 / 512); + blk_queue_max_segment_size(sdev->request_queue, SBP2_MAX_SEG_SIZE); + return 0; } -- cgit v1.2.3 From 4bbc1bdd010cbfcb749e4f947161ec3ab3337893 Mon Sep 17 00:00:00 2001 From: Stefan Richter Date: Sat, 9 Aug 2008 20:22:17 +0200 Subject: firewire: fw-sbp2: fix another small generation access bug queuecommand() looked at the remote and local node IDs before it read the bus generation. The corresponding race with sbp2_reconnect updating these data was probably impossible to happen though because the current code blocks the SCSI layer during reconnection. However, better safe than sorry, especially if someone later improves the code to not block the SCSI layer. Signed-off-by: Stefan Richter --- drivers/firewire/fw-sbp2.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/drivers/firewire/fw-sbp2.c b/drivers/firewire/fw-sbp2.c index 5d8411afced..ef0b9b419c2 100644 --- a/drivers/firewire/fw-sbp2.c +++ b/drivers/firewire/fw-sbp2.c @@ -1423,7 +1423,7 @@ static int sbp2_scsi_queuecommand(struct scsi_cmnd *cmd, scsi_done_fn_t done) struct fw_device *device = fw_device(lu->tgt->unit->device.parent); struct sbp2_command_orb *orb; unsigned int max_payload; - int retval = SCSI_MLQUEUE_HOST_BUSY; + int generation, retval = SCSI_MLQUEUE_HOST_BUSY; /* * Bidirectional commands are not yet implemented, and unknown @@ -1467,6 +1467,9 @@ static int sbp2_scsi_queuecommand(struct scsi_cmnd *cmd, scsi_done_fn_t done) if (cmd->sc_data_direction == DMA_FROM_DEVICE) orb->request.misc |= cpu_to_be32(COMMAND_ORB_DIRECTION); + generation = device->generation; + smp_rmb(); /* sbp2_map_scatterlist looks at tgt->address_high */ + if (scsi_sg_count(cmd) && sbp2_map_scatterlist(orb, device, lu) < 0) goto out; @@ -1479,7 +1482,7 @@ static int sbp2_scsi_queuecommand(struct scsi_cmnd *cmd, scsi_done_fn_t done) if (dma_mapping_error(device->card->device, orb->base.request_bus)) goto out; - sbp2_send_orb(&orb->base, lu, lu->tgt->node_id, lu->generation, + sbp2_send_orb(&orb->base, lu, lu->tgt->node_id, generation, lu->command_block_agent_address + SBP2_ORB_POINTER); retval = 0; out: -- cgit v1.2.3 From 7a1003449c693f0d57443c8786bbf19717921ae0 Mon Sep 17 00:00:00 2001 From: Stefan Richter Date: Fri, 12 Sep 2008 18:09:55 +0200 Subject: firewire: fix setting tag and sy in iso transmission Reported by Jay Fenlason: The iso packet control accessors in fw-cdev.c had bogus masks. Signed-off-by: Stefan Richter --- drivers/firewire/fw-cdev.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/firewire/fw-cdev.c b/drivers/firewire/fw-cdev.c index 2e6d5848d21..cfceb2cba4e 100644 --- a/drivers/firewire/fw-cdev.c +++ b/drivers/firewire/fw-cdev.c @@ -720,8 +720,8 @@ static int ioctl_create_iso_context(struct client *client, void *buffer) #define GET_PAYLOAD_LENGTH(v) ((v) & 0xffff) #define GET_INTERRUPT(v) (((v) >> 16) & 0x01) #define GET_SKIP(v) (((v) >> 17) & 0x01) -#define GET_TAG(v) (((v) >> 18) & 0x02) -#define GET_SY(v) (((v) >> 20) & 0x04) +#define GET_TAG(v) (((v) >> 18) & 0x03) +#define GET_SY(v) (((v) >> 20) & 0x0f) #define GET_HEADER_LENGTH(v) (((v) >> 24) & 0xff) static int ioctl_queue_iso(struct client *client, void *buffer) -- cgit v1.2.3 From 99692f71ee04c6f249d0bf6a581359f32f409a38 Mon Sep 17 00:00:00 2001 From: Stefan Richter Date: Fri, 12 Sep 2008 18:20:16 +0200 Subject: firewire: fix ioctl() return code Reported by Jay Fenlason: ioctl() did not return as intended - the size of data read into ioctl_send_request, - the number of datagrams enqueued by ioctl_queue_iso. Signed-off-by: Stefan Richter --- drivers/firewire/fw-cdev.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/firewire/fw-cdev.c b/drivers/firewire/fw-cdev.c index cfceb2cba4e..ed03234cbea 100644 --- a/drivers/firewire/fw-cdev.c +++ b/drivers/firewire/fw-cdev.c @@ -913,7 +913,7 @@ dispatch_ioctl(struct client *client, unsigned int cmd, void __user *arg) return -EFAULT; } - return 0; + return retval; } static long -- cgit v1.2.3 From be585c07dd577faac26014db4246e6d7c7a131e7 Mon Sep 17 00:00:00 2001 From: Jay Fenlason Date: Wed, 1 Oct 2008 18:13:20 -0400 Subject: firewire: Add more documentation to firewire-cdev.h Signed-off-by: Jay Fenlason Signed-off-by: Stefan Richter --- include/linux/firewire-cdev.h | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/include/linux/firewire-cdev.h b/include/linux/firewire-cdev.h index 0f0e271f97f..4d078e99c01 100644 --- a/include/linux/firewire-cdev.h +++ b/include/linux/firewire-cdev.h @@ -154,8 +154,13 @@ struct fw_cdev_event_iso_interrupt { * @request: Valid if @common.type == %FW_CDEV_EVENT_REQUEST * @iso_interrupt: Valid if @common.type == %FW_CDEV_EVENT_ISO_INTERRUPT * - * Convenience union for userspace use. Events could be read(2) into a char - * buffer and then cast to this union for further processing. + * Convenience union for userspace use. Events could be read(2) into an + * appropriately aligned char buffer and then cast to this union for further + * processing. Note that for a request, response or iso_interrupt event, + * the data[] or header[] may make the size of the full event larger than + * sizeof(union fw_cdev_event). Also note that if you attempt to read(2) + * an event into a buffer that is not large enough for it, the data that does + * not fit will be discarded so that the next read(2) will return a new event. */ union fw_cdev_event { struct fw_cdev_event_common common; -- cgit v1.2.3