Re: Could this be part of the Boomerang problem?

Nathan Bryant (
Sun, 15 Jun 1997 15:07:34 -0400 (EDT)

On Sat, 14 Jun 1997, Michael H. Warfield wrote:

> I recently tried to compile in the 0.40 driver into a 2.1.42
> kernel on a recent RedHat distribution. This is with gcc version
> Before, it had been with earlier kernels, distributions, and compilers.
> This time I got the following errors:
> gcc -D__KERNEL__ -I/usr/src/linux-2.1.42/include -Wall -Wstrict-prototypes -O2 -fomit-frame-pointer -pipe -fno-strength-reduce -m486 -DCPU=486 -c -o 3c59x.o 3c59x.c
> 3c59x.c: In function `vortex_start_xmit':
> 3c59x.c:1054: void value not ignored as it ought to be
> 3c59x.c: In function `boomerang_start_xmit':
> 3c59x.c:1135: void value not ignored as it ought to be

> These two lines are "if( set_bit( .... ) )".
> It seems that the "set_bit" function is (or is now) a void function.
> Since this is right at the heart of some of the busy/time-out logic, the
> immediate question becomes "Is this something new or is this something
> that has been an influence in the driver screw-ups?"

No, nothing that complex. In 2.0 kernels set_bit() was a function used to
atomically test and set bits. In 2.1 kernels set_bit() only sets bits, and
doesn't test. There is a new function, test_and_set_bit(), for this
purpose. Just change those two lines to test_and_set_bit() and the driver
works with 2.1.42.

> Next question, and one that I have not delved into the code deep
> enought to answer yet (maybe someone else can answer) is "what is the
> correct function at this point". The two obvious alternatives are to
> use "test_bit( ... )" if all we are doing is testing the state of the
> bit, or "set_and_test_bit( ... )" if we are attempting to do an atomic
> test and set operation. I think the later is the correct answer, but I'm
> not sure and would appreciate comments from others.

The code is doing an atomic test and set operation. This is what set_bit()
did in 2.0 kernels, but the semantics have changed in 2.1.

> If this is reaching Don Becker, we could probably use the time-out
> patches and the correct fix for the set_bit problem above in the sources.

All that would be needed is an #ifdef to check the kernel version and use
either set_bit() or test_and_set_bit()...