lkml.org 
[lkml]   [2004]   [Mar]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: cpqarray patches for 2.6 [4 of 5]
mikem@beardog.cca.cpqcorp.net wrote:
> This is patch 4 of 5 for cpqarray. Please apply in order.
>
> Thanks,
> mikem
> -------------------------------------------------------------------------------
> - Change to use pci APIs (change from 2.4.18 to 2.4.19)
> PS: This also includes eisa detection fix during initialization
> which was missing from 2.4.19 but fixed in 2.4.25


While I like this patch a lot, there is one tiny bug I spotted, and one
question:


> + /* sendcmd will turn off interrupt, and send the flush...
> + * To write all data in the battery backed cache to disks
> + * no data returned, but don't want to send NULL to sendcmd */
> + if( sendcmd(FLUSH_CACHE, i, buff, 4, 0, 0, 0))
> + {
> + printk(KERN_WARNING "Unable to flush cache on controller %d\n",
> + i);
> + }
> + free_irq(hba[i]->intr, hba[i]);
> + iounmap(hba[i]->vaddr);
> + unregister_blkdev(COMPAQ_SMART2_MAJOR+i, hba[i]->devname);
> + del_timer(&hba[i]->timer);

should this be del_timer_sync()?


> + remove_proc_entry(hba[i]->devname, proc_array);
> + pci_free_consistent(hba[i]->pci_dev,
> + NR_CMDS * sizeof(cmdlist_t), (hba[i]->cmd_pool),
> hba[i]->cmd_pool_dhandle);
> - kfree(hba[i]->cmd_pool_bits);
> + kfree(hba[i]->cmd_pool_bits);
> + for(j = 0; j < NWD; j++) {
> + if (ida_gendisk[i][j]->flags & GENHD_FL_UP)
> + del_gendisk(ida_gendisk[i][j]);
> + devfs_remove("ida/c%dd%d",i,j);
> + put_disk(ida_gendisk[i][j]);
> + }
> + blk_cleanup_queue(hba[i]->queue);
> + release_io_mem(hba[i]);
> + free_hba(i);
> +}
>

> + hba[i]->access.set_intr_mask(hba[i], 0);
> + if (request_irq(hba[i]->intr, do_ida_intr,
> + SA_INTERRUPT|SA_SHIRQ, hba[i]->devname, hba[i]))
> + {
> + printk(KERN_ERR "cpqarray: Unable to get irq %d for %s\n",
> hba[i]->intr, hba[i]->devname);

Even though this line of code existed in the driver prior to your patch,
this highlights a bug: SA_SHIRQ should really only be passed to
request_irq() when the device is a PCI device. If it's an EISA device,
that really shouldn't be present.

I am also curious why SA_INTERRUPT is needed. EISA requirement?

This patch gets my ACK otherwise... I'm glad somebody did this.

Jeff



-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2005-03-22 14:01    [W:0.040 / U:0.060 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site