On Sat, 14 Jun 2008 23:55:19 +0200 Stefan Richter <stefanr at s5r6.in-berlin.de> wrote: > Hi list, > > since the mentioned driver interfaces with the drivers/ieee1394 > subsystem, I had a brief look at it today. There is a number of > stylistic issues and kernel API issues to work on, like > - use of a semaphore, > - struct types with bitfields for what appears to be on-the-wire data, > - camel case names, > - "#define BYTE unsigned char" and friends, > - stale duplicated code like "BUG_ON(in_interrupt())" or all > references to ohci1394 which seem unnecessary, > - homebrewed down_timeout, > - comment style not as in linux kernel. I would add other things, like: - usage of HZ, instead of msecs_to_jiffies(); - its own implementation of wait_event_timeout(); - abuse of typedef's; - some structures are defined differently, depending on endiannes, at avc_api.h. Cheers, Mauro
