Re: Fw: ACARD SCSI driver update for Linux kernel v2.6

Matthew Wilcox
2005-01-14T17:45:10Z

On Fri, Jan 14, 2005 at 06:03:02PM +0800, jameshsu wrote:
> We, Acard, total understand what you suggest. (including advise from
> Christoph, James B. and Alan.)
> Therefore, we modify the internal code to create this patch file from kernel
> v2.6.10.
> However, as you see this is the smallest patch files which could be
> generated. (Sorry, unable to meet your expetation)
> The reason is : this driver could supports ATP870 as well as mutiples of
> Acard's chipsets. (this includes ATP880(AEC67160) & ATP885(AEC67162)). This
> is why the patch files is huge than what you expected.
> We did not make any change for ATP870(AEC6712) on this rev.. Besides, we
> reverse following codes according to all of your suggestions. Please review
> and let me know if this is feasible. Welcome to any of question you may
> have.
> Thanks for your help!

The change to pci.ids needs to be handled through pciids.sourceforge.net

----

I really don't like the:
+ atp_dev.ioport[0] = base_io + 0x80;
+ atp_dev.ioport[1] = base_io + 0xc0;

Just record base_io in your atp_dev. I don't like the way this patch
turns everything into a 2-d array ... surely there has to be a better
way to do it than this?

----

+ if (dev->dev_id == ATP885_DEVID) {
+ tmpcip += 2;
+ outb(0x06, tmpcip);
+ tmpcip -= 2;

would be much better written as:

if (dev->dev_id == ATP885_DEVID)
outb(0x06, tmpcip+2);

(hmm, this seems to be common coding style throughout the driver ...)

----

- if (pci_set_dma_mask(dev, 0xFFFFFFFFUL)) {
- printk(KERN_ERR "atp870u: 32bit DMA mask required but not availa
ble.\n");
- return -EIO;
- }
+ if (!pci_set_dma_mask(pdev, 0xFFFFFFUL)) {
+ printk(KERN_INFO "atp870u: use 32bit DMA mask.\n");
+ } else {
+ printk(KERN_ERR "atp870u: DMA mask required but not available.\
n");
+ return -EIO;
+ }

You've actually set a 24-bit DMA mask there. Should use the symbolic
constant DMA_32BIT_MASK anyway.

----

+ { PCI_DEVICE(PCI_VENDOR_ID_ARTOP, ATP885_DEVID) },

Please use the PCI_DEVICE_ID_ name instead

----

--
"Next the statesmen will invent cheap lies, putting the blame upon
the nation that is attacked, and every man will be glad of those
conscience-soothing falsities, and will diligently study them, and refuse
to examine any refutations of them; and thus he will by and by convince
himself that the war is just, and will thank God for the better sleep
he enjoys after this process of grotesque self-deception." -- Mark Twain