Re: cvs commit: src/sys/dev/iwi if_iwi.c if_iwivar.h
Luigi Rizzo wrote:
> + add debugging code IWI_LOCK_ASSERT() to detect missing locks.
> These only do a printf, and should go away once we figure out why
> the driver sometimes freezes the system due to a (yet unidentified)
> race condition.
LOCK_ASSERT should do a mtx_assert and not printf. If you want to
diverge from existing convention in all other drivers please use a
different name; e.g. IWI_LOCK_CHECK.
> + add a device_printf() in iwi_ioctl() in certain conditions
> (see comment in the code). This helps preventing the race condition
> mentioned above, and makes the system survive. This printf will
> also go away once fixing this bug is completed.
Please don't commit debug stuff like this; we've discussed this
privately. This driver has been in use for many months w/o complaints
of the sort you are seeing.
> + change iwi_getfw() to return 0 on success, 1 on error, consistently
> with other functions.
> + fix the argument of a sizeof() in iwi_get_firmware()
> + use le32toh() to access little-endian fields
> + simplify error handling in iwi_load_firmware() and iwi_init_locked()
> The bugs fixed by this commit (the freezing one especially) are serious
> enough to call for a quick MFC
See above. Don't pollute stable w/ debug stuff. Fix the problem first
Received on Tue Feb 20 2007 - 17:06:09 UTC