> > Hi Mister X! if you want to maintain the ncr53c8xx driver by yourself,
> > let me know. I do consider that sending directly patches to official
> > guys without having submitted the change to the maintainer (at least
> > for his information) to be act of rudeness.
>
> The mdelay patch touched over 50 files, so looking up and e-mailing 50
> different people was not really an option. No rudeness was intended
> or implied. (The patch was also sent to linux-kernel@vger however)
I donnot trust vger since most of patches that are incorporated into their
repository are not made available to the corresponding vger list, unless
my ISP does lose most of the mails I receive. On the other hand, vger
is not the official reference for Linux kernel, AFAIK.
If they want a kernel for their own, they just have to write their own
kernel.
> > BTW, I do consider using several DIVISIONS when 1 COMPARISON is
> > enough to be BAD PROGRAMMING, expecially when the code is intended
> > to deal with a fiew MICRO-SECONDS.
>
> If you are concerned with the accuracy of the smaller delays, then I
> submit the following patch to you for your consideration.
I am concerned not to have to deal with messy development.
> With this patch, the smaller delays get translated directly into
> "call __const_udelay", without any extra overhead of a "call DELAY"
> function that in turn calls __udelay.
If you want to change code, you first have to proove the change is needed.
I dont care of any change that is not needed. IMO, it is such a bogus
approach that broke to much 2.0.30 and current 2.1.X kernel series.
> For sub-millisecond delays, [e.g. DELAY(100)] the executed code is:
>
> Your Original This Patch
> ------------- ----------
> pushl $100 pushl $429400
> call DELAY call __const_udelay
> pushl %ebx
> movl 8(%esp),%ebx
> cmpl $1000,%ebx
> jle (taken)
> testl %ebx,%ebx
> je (not taken)
> pushl %ebx
> call __udelay
> addl $4,%esp
> popl %ebx
> ret
You write a code that is intended to perform useless arithmetic
operations and you trust the compiler to understand you are stupid
and generate good code. NO and NO and NO. That's loose.
> where the code assoicated with the DELAY function is indented. The
> above comparison also desn't include the fact that __udelay contains
> about six movl+leal pairs that __const_udelay does not have.
>
> So, if getting small delays without extra overhead is that important, then
> this patch is even more efficient than the original (103 and earlier)
> behaviour, and I trust you will agree after comparing the resulting
> assembly.
>
> Paul.
>
> --- linux-21104/drivers/scsi/ncr53c8xx.c Sat Jun 6 18:12:52 1998
> +++ linux/drivers/scsi/ncr53c8xx.c Mon Jun 8 15:36:17 1998
> @@ -367,11 +367,13 @@
> ** Insert a delay in micro-seconds.
> */
>
> -static void DELAY(long us)
> -{
> - if (us/1000) mdelay(us/1000);
> - if (us%1000) udelay(us%1000);
> -}
> +#define DELAY(n) (\
> + (__builtin_constant_p(n) && (n)<=(MAX_UDELAY_MS*1000)) ? \
> + udelay(n) : \
> + ({unsigned long us=(n); \
> + for (; us>MAX_UDELAY_MS*1000; us-=MAX_UDELAY_MS*1000) \
> + mdelay(MAX_UDELAY_MS); \
> + if (us) udelay(us);}))
If such a macro is defined in a kernel header, it is good, but if
it is defined in any C code or driver header this is utter SHIT.
On the other hand, it seems that the mdelay() function is not defined in
2.0 kernels. Given the time needed to have a new stable kernel, drivers
have to be often updated in stable and so must be source compatible with
stable kernels.
Gerard.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu