On 08/26/2013 06:14 AM, 黃清隆 wrote: > From: Ching <ching2048@xxxxxxxxxxxx> > > Support Areca new SATA Raid adapter ARC1214/1224/1264/1284. > Modify maximum outstanding command number, notify command complete with auto > request sense > Signed-off-by: Ching <ching2048@xxxxxxxxxxxx> Hi Ching, +static bool +arcmsr_hbaD_get_config(struct AdapterControlBlock *acb) +{ ... + dma_coherent = dma_alloc_coherent(&pdev->dev, acb->uncache_size, + &dma_coherent_handle, GFP_KERNEL); + if (!dma_coherent) { + pr_notice("DMA allocation failed...\n"); + return -ENOMEM; You've declared the return value as a bool so the function should return false or true. --------------------------------------- In arcmsr_alloc_ccb_pool - roundup_ccbsize = roundup(sizeof(struct CommandControlBlock) + (max_sg_entrys - 1) * sizeof(struct SG64ENTRY), 32); - acb->uncache_size = roundup_ccbsize * ARCMSR_MAX_FREECCB_NUM; - dma_coherent = dma_alloc_coherent(&pdev->dev, acb->uncache_size, &dma_coherent_handle, GFP_KERNEL); - if(!dma_coherent){ - printk(KERN_NOTICE "arcmsr%d: dma_alloc_coherent got error\n", acb->host->host_no); - return -ENOMEM; + switch (acb->adapter_type) { + case ACB_ADAPTER_TYPE_A: { + roundup_ccbsize = + roundup(sizeof(struct CommandControlBlock) + + max_sg_entrys * sizeof(struct SG64ENTRY), 32); + acb->uncache_size = roundup_ccbsize * + ARCMSR_MAX_FREECCB_NUM; + dma_coherent = dma_alloc_coherent(&pdev->dev, + acb->uncache_size, &dma_coherent_handle, + GFP_KERNEL); ... + case ACB_ADAPTER_TYPE_B: { + roundup_ccbsize = roundup(sizeof(struct CommandControlBlock) + + (max_sg_entrys - 1) * sizeof(struct SG64ENTRY), 32); + acb->uncache_size = roundup_ccbsize * + ARCMSR_MAX_FREECCB_NUM; + dma_coherent = dma_alloc_coherent(&pdev->dev, + acb->uncache_size, &dma_coherent_handle, + GFP_KERNEL); You've added a switch with four almost identical cases, what is the point of splitting the code this way? ----------------------------------------- struct AdapterControlBlock + void *dma_coherent; + dma_addr_t dma_coherent_handle; + dma_addr_t dma_coherent_handle2; + void *dma_coherent2; why do you need these pairs? Is there an adapter which uses both for example dma_coherent and dma_coherent2 at the same time? Please include the patch into the message body next time, it makes the review easier. Thanks, Tomas -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html