: version 2.00.3 megaraid driver for 2.4.x and 2.5.67 kernels

: version 2.00.3 megaraid driver for 2.4.x and 2.5.67 kernels

Post by Mukker, Atu » Sun, 27 Apr 2003 01:10:08



I have posted the 2.00.5 driver on ftp.lsil.com/pub/linux-megaraid I have
tried to address most of your concerns in this driver except for the
following

>     b) please don't use such global arrays but always use the
>     ->hostdata field

The driver needs a global access to all adapters since it implements a
private ioctl node for applications . The applications use a single node for
all controllers

The patch for 2.5.6x is not available yet but I will announce that pretty
soon.

Thanks
-Atul Mukker <at...@lsil.com>

> -----Original Message-----
> From: Christoph Hellwig [mailto:h...@infradead.org]
> Sent: Thursday, April 17, 2003 8:38 AM
> To: Mukker, Atul
> Cc: 'a...@redhat.com'; 'James.Bottom...@steeleye.com';
> 'linux-ker...@vger.kernel.org'; 'linux-s...@vger.kernel.org';
> 'linux-megaraid-de...@dell.com'; 'linux-megaraid-annou...@dell.com'
> Subject: Re: [ANNOUNCE]: version 2.00.3 megaraid driver for 2.4.x and
> 2.5.67 kernels

> On Wed, Apr 16, 2003 at 04:34:22PM -0400, Mukker, Atul wrote:
> > New megaraid driver 2.00.3 is now available at
> > ftp://ftp.lsil.com/pub/linux-megaraid For this driver, a
> patch is also
> > available for 2.5.67 kernel.

> Hmm, why are you patching and announcing 2.00.3 here if 2.00.4beta is
> already out?

> Anyway, here's some comments for the driver:

> static int hba_count;
> static adapter_t *hba_soft_state[MAX_CONTROLLERS];

>     a) please avoid typedefs for structs.  Also adapter_t is a bit
>     too generic.  What about struct mega_adapter instead?
>     b) please don't use such global arrays but always use the
>     ->hostdata field.

> static struct proc_dir_entry *mega_proc_dir_entry;

> static struct notifier_block mega_notifier = {
>    .notifier_call = megaraid_reboot_notify
> };

>     Do you really need this?  Why can you use
>     struct device_driver->shutdown?

> /*
>  * megaraid_validate_parms()
>  *
>  * Validate that any module parms passed in
>  * have proper values.
>  */
> static void
> megaraid_validate_parms(void)
> {
>    if( (max_cmd_per_lun <= 0) || (max_cmd_per_lun >
> MAX_CMD_PER_LUN) )
>            max_cmd_per_lun = MAX_CMD_PER_LUN;
>    if( max_mbox_busy_wait > MBOX_BUSY_WAIT )
>            max_mbox_busy_wait = MBOX_BUSY_WAIT;
> }

>     Please run the whole driver through scripts/Lindent

> /**
>  * megaraid_detect()
>  * @host_template - Our soft state maintained by mid-layer
>  *
>  * the detect entry point for the mid-layer.
>  * We scan the PCI bus for our controllers and start them.
>  *
>  * Note: PCI_DEVICE_ID_PERC4_DI below represents the PERC4/Di class of
>  * products. All of them share the same vendor id, device id,
> and subsystem
>  * vendor id but different subsystem ids. As of now, driver
> does not use the
>  * subsystem id.
>  */
> static int
> megaraid_detect(Scsi_Host_Template *host_template)

>     Please get rid of ->detect and use the new-style pci API +
>     scsi_add_host instead.

>    host_template->proc_name = "megaraid";

>     CAn be initialized at compile-time.

>    printk(KERN_NOTICE "megaraid: " MEGARAID_VERSION);

>    megaraid_validate_parms();

>    memset(mega_hbas, 0, sizeof (mega_hbas));

>    hba_count = 0;

>     These are in .bss

>    }

>    return;

> }
>     This is superflous.

> /*
>  * megaraid_proc_info()
>  *
>  * Returns data to be displayed in /proc/scsi/megaraid/X
>  */
> static int
> megaraid_proc_info(char *buffer, char **start, off_t offset,
> int length,
>            int host_no, int inout)
> {
>    *start = buffer;
>    return 0;
> }

>     Just don't implement this method instead?

> volatile static int internal_done_flag = 0;
> volatile static int internal_done_errcode = 0;

> static DECLARE_WAIT_QUEUE_HEAD (internal_wait);

> static void internal_done (Scsi_Cmnd *cmd)
> {
>    internal_done_errcode = cmd->result;
>    internal_done_flag++;
>    wake_up (&internal_wait);
> }

> /* shouldn't be used, but included for completeness */

> static int
> megaraid_command (Scsi_Cmnd *cmd)
> {
>    internal_done_flag = 0;

>    /* Queue command, and wait until it has completed */
>    megaraid_queue (cmd, internal_done);

>    while (!internal_done_flag)
>            interruptible_sleep_on (&internal_wait);

>    return internal_done_errcode;
> }

>     This looks horribly racy.

>                            return TRUE;
>                    }
>            }
>    }

>    return FALSE;
> }

>     Please don't use TRUE/FALSE but 1/0 directly.

> static int
> megadev_open (struct inode *inode, struct file *filep)
> {
>    /*
>     * Only allow superuser to access private ioctl interface
>     */
>    if( !capable(CAP_SYS_ADMIN) ) return -EACCES;

>    if (!try_module_get(THIS_MODULE)) {
>            return -ENXIO;
>    }

>    This is broken as hell (and I fixed it in the old megraid driver
>    ages ago!)  Just set ->owner in the file_operation.

>    /*
>     * Wait till all the issued commands are complete and
> there are no
>     * commands in the pending queue
>     */
>    while( atomic_read(&adapter->pend_cmds) > 0 ||
>                    !list_empty(&adapter->pending_list) ) {

>            sleep_on_timeout( &wq, 1*HZ );      /* sleep for 1s */
>    }

>   Again racy.  You need to opencode this (similar to wait_event, maybe
>   just add wait_even_timeout).

>    if (adapter->read_ldidmap) {
>            struct list_head *pos;
>            list_for_each(pos, &adapter->pending_list) {
>                    scb = list_entry(pos, scb_t, list);

>   list_for_each_entry()?

> /**
>  * mega_allocate_inquiry()
>  * @dma_handle - handle returned for dma address
>  * @pdev - handle to pci device
>  *
>  * allocates memory for inquiry structure
>  */
> static inline caddr_t
> mega_allocate_inquiry(dma_addr_t *dma_handle, struct pci_dev *pdev)
> {
>    return pci_alloc_consistent(pdev,
> sizeof(mega_inquiry3), dma_handle);
> }

> static inline void
> mega_free_inquiry(caddr_t inquiry, dma_addr_t dma_handle,
> struct pci_dev *pdev)
> {
>    pci_free_consistent(pdev, sizeof(mega_inquiry3),
> inquiry, dma_handle);
> }

> Do you really need a wrapper for those?

>    /*
>     * For all internal commands, the buffer must be
> allocated in <4GB
>     * address range
>     */
>    if( make_local_pdev(adapter, &pdev) != 0 ) return -1;

>     So use dma_alloc_coherent with a GFP_KERNEL arg instead
> of such ugly hacks.

>    /*
>     * Wait till this command finishes. Do not use
>     * wait_event_interruptible(). It causes panic if
> CTRL-C is hit when
>     * dumping e.g., physical disk information through
> /proc interface.
>     */
> #if 0
>    wait_event_interruptible(adapter->int_waitq, scmd->state);
> #endif
>    wait_event(adapter->int_waitq, scmd->state);

>     Maybe you need to actually look at wait_event_interruptible and
>     handle it's return value? :)

> static Scsi_Host_Template driver_template = MEGARAID;

>     Get rid of the ugly macro and add the members right here.

> While we're at it:  Please also get rid of all those prototypes in
> megaraid.h
> -
> To unsubscribe from this list: send the line "unsubscribe
> linux-scsi" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

: version 2.00.3 megaraid driver for 2.4.x and 2.5.67 kernels

Post by Mukker, Atu » Sun, 27 Apr 2003 21:50:08


Quote:> The patch for 2.5.6x is not available yet but I will announce
> that pretty soon.

The patch for kernel 2.5.6[78] for driver 2.00.5 is now available at

ftp://ftp.lsil.com/pub/linux-megaraid/drivers/version-2.00.5/

Regards

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in

More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

 
 
 

1. : version 2.00.3 megaraid driver for 2.4.x and 2.5.67 kernels

New megaraid driver 2.00.3 is now available at
ftp://ftp.lsil.com/pub/linux-megaraid For this driver, a patch is also
available for 2.5.67 kernel.

Regards
Atul Mukker
Storage Systems
LSI Logic Corporation
U.S.A.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in

More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

2. Q: Redhat reinstallation.

3. [2.5.67] kexec for 2.5.67 available

4. Linksys LNE100TX v2.0 and RedHat7.0

5. SOLVED : LILO gives me L 00 00 00 00 00 00 00

6. Slackware

7. i2c driver changes for 2.5.67 - take 2

8. Another reason why MS Windows dominanceis bad (was Re: Teamers See the Future 3

9. - 2.5.67 - sysfs (modifiable size) interface for ramdisk driver

10. Help LILO gives me L 00 00 00 00 00 00 00

11. 2.5.67: ppa driver & preempt == oops

12. i2c driver changes for 2.5.67

13. - 2.5.67 - proc interface to ramdisk driver.