From: Ching <[email protected]>
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 <[email protected]>
From: Ching <[email protected]>
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 <[email protected]>
On Mon, 2013-08-26 at 12:14 +0800, 黃清隆 wrote:
> From: Ching <[email protected]>
>
> 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 <[email protected]>
Checkpatch really doesn't like this:
total: 1 errors, 173 warnings, 7131 lines checked
I fixed up the error and the few relevant warnings. I don't really care
about the lines over 80 characters. However, if you're going to have
long lines, don't split strings (another checkpatch complaint). I'd
also like to see the spurious braces removed in a later patch, please.
The prinks without a KERN_ also need fixing.
James
On 08/26/2013 06:14 AM, ???M?? wrote:
> From: Ching <[email protected]>
>
> 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 <[email protected]>
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