[PATCH] 1.1.4-2313 aacraid driver for LiNUX 2.4.25-pre4

[PATCH] 1.1.4-2313 aacraid driver for LiNUX 2.4.25-pre4

[Xose Vazquez Perez]

[-- Attachment #1: Type: text/plain, Size: 120 bytes --]

hi,

here goes latest aacraid driver, it was sent for 2.4 kernel
inclusion. CHANGELOG has an extensive list of changes.

[-- Attachment #2: aacraid.diff.gz --]
[-- Type: application/x-gzip, Size: 42196 bytes --]

Re: [PATCH] 1.1.4-2313 aacraid driver for LiNUX 2.4.25-pre4

[Arjan van de Ven]

[-- Attachment #1: Type: text/plain, Size: 2295 bytes --]

On Wed, Jan 07, 2004 at 07:28:01PM +0100, Xose Vazquez Perez wrote:
> hi,
> 
> here goes latest aacraid driver, it was sent for 2.4 kernel
> inclusion. CHANGELOG has an extensive list of changes.

+#define aac_spin_lock_irqsave(host_lock, cpu_flags)            spin_lock_irqsave(&io_request_lock, cpu_flags)
+#define aac_spin_lock_irq(host_lock)                           spin_lock_irq(&io_request_lock)
+#define aac_spin_unlock_irqrestore(host_lock, cpu_flags)       spin_unlock_irqrestore(&io_request_lock, cpu_flags)
+#define aac_spin_unlock_irq(host_lock)                         spin_unlock_irq(&io_request_lock)
                                                                                                                        
ewwww


+#ifdef AAC_DETAILED_STATUS_INFO
 static char *aac_get_status_string(u32 status);
+#endif

there's no need for ifdefs around prototypes, they just clutter the code

+        *      Only enable DAC mode if the dma_addr_t is larger than 32
+        * bit addressing, and we have more than 32 bit addressing worth of
+        * memory and if the controller supports 64 bit scatter gather elements.
+        */
+       if( (sizeof(dma_addr_t) > 4) && (num_physpages > (0xFFFFFFFFULL >> PAGE_SHIFT)) && (dev->adapter_info.options &
+               dev->dac_support = 1;
        }

that looks broken, at least drivers should never need to check num_physpages etc etc


+static void get_sd_devname(int disknum, char *buffer)
+{
+.....


why on earth is that in a driver?????

+# define strlcpy(s1,s2,n) strncpy(s1,s2,n);s1[n-1]='\0'


this is very broken; consider

if (foo)
    bar(foo);
else
    strlcpy(foo,bar,3);


+# ifndef min
+#  define min(a,b) (((a)<(b))?(a):(b))
+# endif

that is quite broken..... arguments get evaluated multiple times



 struct fib_ioctl
 {
-       char    *fibctx;
-       int     wait;
-       char    *fib;
+       u32     fibctx;
+       s32     wait;
+#if (defined(__x86_64__))
+       u64     fib;
+#else
+       u32     fib;
+#endif
 };


that looks really fishy; __x86_64__ most certainly is not the only 64 bit architecture out there



this driver "update" appears to intruduce quite a few nastys... doesn't look like a good idea to me

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] 1.1.4-2313 aacraid driver for LiNUX 2.4.25-pre4

[Christoph Hellwig]

On Wed, Jan 07, 2004 at 07:12:25PM +0100, Xose Vazquez Perez wrote:
> hi,
> 
> here goes latest aacraid driver, it was sent for 2.4 kernel
> inclusion. CHANGELOG has an extensive list of changes.

That's a huge mess adding lots of broken crap and hooks for binary only modules
through ioctls (urgg..).

If you want to help getting this driver merged please split out reviewable
pieces and send them to linux-scsi.

_______________________________________________
Linux-aacraid-devel mailing list
Linux-aacraid-devel-8PEkshWhKlo@public.gmane.org
http://lists.us.dell.com/mailman/listinfo/linux-aacraid-devel
Please read the FAQ at http://lists.us.dell.com/faq or search the list archives at http://lists.us.dell.com/htdig/

Re: [PATCH] 1.1.4-2313 aacraid driver for LiNUX 2.4.25-pre4

[Xose Vazquez Perez]

Christoph Hellwig wrote:

> That's a huge mess adding lots of broken crap and hooks for binary only modules
> through ioctls (urgg..).

yes, but the driver was sent to Marcelo without to pass linux-scsi revision, like
others drivers. And I think that is very important a NET review before the inclusion
in kernel.org tree.

> If you want to help getting this driver merged please split out reviewable
> pieces and send them to linux-scsi.

Mark Salyzyn(adaptec) and Mark Haverkamp(osdl) already are working on it.

_______________________________________________
Linux-aacraid-devel mailing list
Linux-aacraid-devel-8PEkshWhKlo@public.gmane.org
http://lists.us.dell.com/mailman/listinfo/linux-aacraid-devel
Please read the FAQ at http://lists.us.dell.com/faq or search the list archives at http://lists.us.dell.com/htdig/

Re: [PATCH] 1.1.4-2313 aacraid driver for LiNUX 2.4.25-pre4

[Christoph Hellwig]

On Thu, Jan 08, 2004 at 04:58:50PM +0100, Xose Vazquez Perez wrote:
> Christoph Hellwig wrote:
> 
> > That's a huge mess adding lots of broken crap and hooks for binary only modules
> > through ioctls (urgg..).
> 
> yes, but the driver was sent to Marcelo without to pass linux-scsi revision, like
> others drivers. And I think that is very important a NET review before the inclusion
> in kernel.org tree.

Well, the review is pretty short.  The driver is an unreadable POS that
introduces bugs and backdoors for binary modules.

If marcelo is going to merge it anyway I'll urge him to back it out.

And who the heck thinks it's a good idea to submit scsi driver directly
instead of through the proper lists?

_______________________________________________
Linux-aacraid-devel mailing list
Linux-aacraid-devel-8PEkshWhKlo@public.gmane.org
http://lists.us.dell.com/mailman/listinfo/linux-aacraid-devel
Please read the FAQ at http://lists.us.dell.com/faq or search the list archives at http://lists.us.dell.com/htdig/

scsi drivers was Re: [PATCH] 1.1.4-2313 aacraid driver for LiNUX 2.4.25-pre4

[Xose Vazquez Perez]

Christoph Hellwig wrote:

> And who the heck thinks it's a good idea to submit scsi driver directly
> instead of through the proper lists?

A lot of vendor's drivers maintainers, do that. And the driver is
automagically rejected ;-). This is the reason why LiNUX SCSI drivers are
obsolete, and some of them have important bugs.

_______________________________________________
Linux-aacraid-devel mailing list
Linux-aacraid-devel-8PEkshWhKlo@public.gmane.org
http://lists.us.dell.com/mailman/listinfo/linux-aacraid-devel
Please read the FAQ at http://lists.us.dell.com/faq or search the list archives at http://lists.us.dell.com/htdig/