> this patch will add support for the MegaRAID 428 and 438 controllers, and
> most likely
> will work with others but has not yet been tested. If anyone out there has
> any
> MegaRAID controller and wants to test this driver, please send me feedback
> on bugs,
> performance, suggestions, etc.
I don't have any MegaRAIDs here to test the driver, but I've read the driver
and there are several comments:
> +#include <linux/init.h>
Do you use the __init macros anywhere?
> +#if LINUX_VERSION_CODE <= 0x20034
> +#include <linux/bios32.h>
> +#endif
It should be #if LINUX_VERSION_CODE < 0x20100.
> +#define ENQUEUE(obj,type,list,next) \
> +{ type **node; long cpuflag; \
> + save_flags(cpuflag); cli(); \
> + for(node=&(list); *node; node=(type **)&(*node)->##next); \
> + (*node) = obj; \
> + (*node)->##next = NULL; \
> + restore_flags(cpuflag); \
> +};
> +
> +#define DEQUEUE(obj,type,list,next) \
> +{ long cpuflag; \
> + save_flags(cpuflag); cli(); \
> + if ((obj=list) != NULL) {\
> + list = (type *)(list)->##next; \
> + } \
> + restore_flags(cpuflag); \
> +};
Do you really need a global cli() here? IMHO a IRQ-safe spinlock would be
sufficient and it would give better performance on SMP systems.
> +#define SERDEBUG 0
> +#ifdef SERDEBUG
?? #if SERDEBUG instead?
> +static void freeSCB(mega_scb *scb)
> +{
> + long flags;
> +
> + save_flags(flags);
> + cli();
> + scb->flag = 0;
> + scb->idx = -1;
> + scb->next = NULL;
> + scb->SCpnt = NULL;
> + restore_flags(flags);
> +}
Maybe the atomic read/write operations in <asm/atomic.h> would be strong
enough and again give better performance.
> + save_flags(flags);
> + cli();
Global cli() in interrupt handler is going to deadlock SMP systems
and it's probably not needed anyway.
> + /* Read the base port and IRQ from PCI */
> + pcibios_read_config_dword(pciBus, pciDevFun,
> + PCI_BASE_ADDRESS_0,
> + (u_int *)&megaBase);
> + pcibios_read_config_byte(pciBus, pciDevFun,
>
> + PCI_INTERRUPT_LINE,
> + &megaIrq);
On 2.1, you have to fetch the base address and IRQ from the pci_dev structure
instead of reading it from the configuration space since it doesn't fit there
on several architectures and the kernel does its own mapping of both addresses
and IRQ's.
> + else {
> + megaBase += 0xF;
> + }
megaBase &= PCI_BASE_ADDRESS_MEM_MASK is the correct way to do this for memory
regions, &= PCI_BASE_ADDRESS_IO_MASK for I/O regions.
> + count += findCard(pHostTmpl, 0x101E, 0x9010, 0);
> + count += findCard(pHostTmpl, 0x101E, 0x9060, 0);
> + count += findCard(pHostTmpl, 0x8086, 0x1960, BOARD_QUARTZ);
Please send these device IDs to Jens Maurer <linux-pcisupport@cck.uni-kl.de>
who maintains the Linux PCI ID list, so that pciutils will be able to recognize
these cards.
> + for (i=0;i<0xfffff;i++);
udelay() should be used instead.
> + printk(""); /* coax kernel into running tasks... whole
> */
Call schedule() instead.
> + cylinders = disk->capacity / (heads * sectors);
I guess it should be rounded up, not down.
> + cylinders = disk->capacity / (heads * sectors);
Again.
> +#define MEGARAID \
> + { NULL, /* Next */\
> + NULL, /* Module */\
In 2.1 kernels, SCSI drivers use GCC extensions for naming structure fields
during initialization, so that the structure can be modified without touching
the drivers (and it's of course much more readable):
proc_info: megaraid_info,
detect: megaraid_detect,
... and so on
> +#pragma pack(1)
This pragma is not supported by GCC. If your structures don't have natural
alignment (which doesn't seem to be your case), add __attribute__((packed))
to the declaration, but then you of course cannot access the structure members
directly since most CPUs don't allow unaligned accesses.
Have a nice fortnight
-- Martin `MJ' Mares <mj@ucw.cz> http://atrey.karlin.mff.cuni.cz/~mj/ Faculty of Math and Physics, Charles University, Prague, Czech Rep., Earth "Light-year? One-third less calories than a regular year."- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.rutgers.edu Please read the FAQ at http://www.altern.org/andrebalsa/doc/lkml-faq.html