From: Jeff Garzik <jgarzik@pobox.com>
To: "Bagalkote, Sreenivas" <sreenib@lsil.com>
Cc: "'linux-kernel@vger.kernel.org'" <linux-kernel@vger.kernel.org>,
"'linux-scsi@vger.kernel.org'" <linux-scsi@vger.kernel.org>
Subject: Re: [PATCH][RELEASE] megaraid 2.10.2 Driver
Date: Mon, 22 Mar 2004 18:07:55 -0500 [thread overview]
Message-ID: <405F71CB.7000902@pobox.com> (raw)
In-Reply-To: <0E3FA95632D6D047BA649F95DAB60E570230C77A@exa-atlanta.se.lsil.com>
Bagalkote, Sreenivas wrote:
> Hello,
> @@ -45,6 +46,10 @@
>
> #include "megaraid2.h"
>
> +#ifdef LSI_CONFIG_COMPAT
> +#include <asm/ioctl32.h>
> +#endif
> +
For upstream, this should just be CONFIG_COMPAT I presume.
> MODULE_AUTHOR ("LSI Logic Corporation");
> MODULE_DESCRIPTION ("LSI Logic MegaRAID driver");
> MODULE_LICENSE ("GPL");
> @@ -206,6 +211,10 @@
> */
> major = register_chrdev(0, "megadev", &megadev_fops);
>
> + if (!major) {
> + printk(KERN_WARNING
> + "megaraid: failed to register char
> device.\n");
> + }
> /*
> * Register the Shutdown Notification hook in kernel
> */
> @@ -214,6 +223,13 @@
> "MegaRAID Shutdown routine not
> registered!!\n");
> }
>
> +#ifdef LSI_CONFIG_COMPAT
> + /*
> + * Register the 32-bit ioctl conversion
> + */
> + register_ioctl32_conversion(MEGAIOCCMD,
> megadev_compat_ioctl);
> +#endif
> +
ditto
> @@ -620,12 +638,15 @@
>
> /* Set the Mode of addressing to 64 bit if we can */
> if((adapter->flag & BOARD_64BIT)&&(sizeof(dma_addr_t) == 8))
> {
> - pci_set_dma_mask(pdev, 0xffffffffffffffffULL);
> - adapter->has_64bit_addr = 1;
> + if (pci_set_dma_mask(pdev, 0xffffffffffffffffULL) ==
> 0)
> + adapter->has_64bit_addr = 1;
> }
> - else {
> - pci_set_dma_mask(pdev, 0xffffffff);
> - adapter->has_64bit_addr = 0;
> + if (!adapter->has_64bit_addr) {
> + if (pci_set_dma_mask(pdev, 0xffffffff) != 0) {
> + printk("megaraid%d: DMA not available.\n",
> + host->host_no);
> + goto fail_attach;
> + }
Bug -- always set dma mask. Do not conditionally _not_ call
pci_set_dma_mask(), for the 64-bit case.
Minor: add ULL to the constant.
> @@ -2549,7 +2575,9 @@
> /*
> * Unregister the character device interface to the driver.
> */
> - unregister_chrdev(major, "megadev");
> + if (major) {
> + unregister_chrdev(major, "megadev");
> + }
register_chrdev() returns a negative errno value on error, such as -EBUSY.
> @@ -4434,8 +4332,9 @@
> /*
> * Get the user data
> */
> - if( copy_from_user(data, (char *)uxferaddr,
> - pthru->dataxferlen)
> ) {
> + if( copy_from_user(data,
> + (char *)((ulong)uxferaddr),
> + pthru->dataxferlen) ) {
ummmm what??? uxferaddr is u32. why are you casting it to a pointer?
> @@ -4460,8 +4359,8 @@
> * Is data going up-stream
> */
> if( pthru->dataxferlen && (uioc.flags & UIOC_RD) ) {
> - if( copy_to_user((char *)uxferaddr, data,
> - pthru->dataxferlen)
> ) {
> + if( copy_to_user((char *)((ulong)uxferaddr),
> + data, pthru->dataxferlen) )
> {
ditto
> diff -Naur old/drivers/scsi/megaraid2.h new/drivers/scsi/megaraid2.h
> --- old/drivers/scsi/megaraid2.h 2004-03-22 17:28:38.000000000 -0500
> +++ new/drivers/scsi/megaraid2.h 2004-03-22 13:10:48.000000000 -0500
> #ifndef PCI_VENDOR_ID_LSI_LOGIC
> #define PCI_VENDOR_ID_LSI_LOGIC 0x1000
> #endif
this can be removed.
> +#if defined (CONFIG_COMPAT) || defined ( __x86_64__)
> +#define LSI_CONFIG_COMPAT
> +#endif
> +#ifdef LSI_CONFIG_COMPAT
> +static int megadev_compat_ioctl(unsigned int, unsigned int, unsigned long,
> + struct file *);
> +#endif
I don't see how this construct will work in all cases. Hence my
CONFIG_COMPAT command above.
Jeff
next prev parent reply other threads:[~2004-03-22 23:08 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2004-03-22 22:56 [PATCH][RELEASE] megaraid 2.10.2 Driver Bagalkote, Sreenivas
2004-03-22 23:07 ` Jeff Garzik [this message]
2004-03-23 8:12 ` Arjan van de Ven
-- strict thread matches above, loose matches on Subject: below --
2004-03-23 0:02 Bagalkote, Sreenivas
2004-03-23 0:45 ` Matthew Wilcox
2004-03-23 6:27 ` Christoph Hellwig
2004-03-23 13:30 ` Matthew Wilcox
2004-03-23 13:36 ` Christoph Hellwig
[not found] ` <20040324061553.GA13681@logos.cnet>
2004-03-24 20:39 ` Marcelo Tosatti
2004-04-17 2:15 ` Jeff Garzik
2004-03-23 16:00 Bagalkote, Sreenivas
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=405F71CB.7000902@pobox.com \
--to=jgarzik@pobox.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-scsi@vger.kernel.org \
--cc=sreenib@lsil.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).