Picking nits in patch-2.2.0-pre2

Horst von Brand (vonbrand@sleipnir.valparaiso.cl)
Thu, 31 Dec 1998 22:35:34 -0400


Some funny things I saw reading the patch:

diff -u --recursive --new-file
v2.2.0-pre1/linux/Documentation/kernel-docs.txt
linux/Documentation/kernel-docs.txt
--- v2.2.0-pre1/linux/Documentation/kernel-docs.txt Wed Dec 31 16:00:00
1969+++ linux/Documentation/kernel-docs.txt Tue Dec 29 15:47:32 1998

+ corrections, ideas or comments are also wellcomed.
^^^^^^^^^

diff -u --recursive --new-file v2.2.0-pre1/linux/drivers/block/ide-tape.c
linux/drivers/block/ide-tape.c
--- v2.2.0-pre1/linux/drivers/block/ide-tape.c Fri Oct 9 13:27:07 1998
+++ linux/drivers/block/ide-tape.c Wed Dec 30 14:51:31 1998
@@ -212,6 +212,8 @@
* number of tape blocks.
* Add support for INTERRUPT DRQ devices.
* Ver 1.13 Jan 2 98 Add "speed == 0" work-around for HP COLORADO 5GB
+ * Ver 1.14 Dec 30 99 Partial fixes for the Sony/AIWA tape drives.
+ * Replace cli()/sti() with hwgroup spinlocks.
^^

A bit premature, that patch ;-)

diff -u --recursive --new-file v2.2.0-pre1/linux/drivers/block/pdc4030.c
linux/drivers/block/pdc4030.c
--- v2.2.0-pre1/linux/drivers/block/pdc4030.c Thu May 7 22:51:48 1998
+++ linux/drivers/block/pdc4030.c Tue Dec 29 11:24:57 1998

+ while (time_after(timer, jiffies));
^
This is too easy to overlook. Better put the ';' on a line of its own

diff -u --recursive --new-file v2.2.0-pre1/linux/drivers/char/console.c
linux/drivers/char/console.c
--- v2.2.0-pre1/linux/drivers/char/console.c Sun Nov 8 14:02:53 1998
+++ linux/drivers/char/console.c Tue Dec 29 14:28:37 1998
@@ -1029,7 +1029,7 @@
*/
translate = set_translate(charset == 0
? G0_charset
- : G1_charset);
+ : G1_charset,currcons);

Better put the currcons on a separate line?

There are more calls to set_translate, where ? : are nicely separated, but
not currcons from the first arg

diff -u --recursive --new-file v2.2.0-pre1/linux/drivers/scsi/seagate.c
linux/drivers/scsi/seagate.c
--- v2.2.0-pre1/linux/drivers/scsi/seagate.c Sat Sep 5 16:46:41 1998
+++ linux/drivers/scsi/seagate.c Tue Dec 29 11:40:35 1998
@@ -379,8 +379,8 @@
{
register int count = 0, start = jiffies + 1, stop = start + 25;

- while (jiffies < start) ;
- for (; jiffies < stop; ++count) ;
+ while (time_before(jiffies, start)) ;
+ for (; time_before(jiffies, stop); ++count) ;

Very confusing... put the ';' on separate lines

@@ -903,9 +903,9 @@

while (((STATUS | STATUS | STATUS) &
(STAT_BSY | STAT_SEL)) &&
- (!st0x_aborted) && (jiffies < clock));
+ (!st0x_aborted) && time_before(jiffies, clock));

Even worse!

diff -u --recursive --new-file v2.2.0-pre1/linux/include/asm-alpha/delay.h
linux/include/asm-alpha/delay.h
--- v2.2.0-pre1/linux/include/asm-alpha/delay.h Fri May 8 23:14:54 1998
+++ linux/include/asm-alpha/delay.h Tue Dec 29 13:56:15 1998

+ * Optimize small constants further by exposing the second multiplication
+ * to the compiler. In addition, mulq is 2 cycles faster than umulh.

It then goes ahead and uses umulq in the following asm()s in __udelay(). Or
am I missing something here?

Happy hacking in 1999!

-- 
Horst von Brand                             vonbrand@sleipnir.valparaiso.cl
Casilla 9G, Viņa del Mar, Chile                               +56 32 672616

- 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.tux.org/lkml/