on patch "DVB: add firesat driver" in staging.git

Mauro Carvalho Chehab mchehab at infradead.org
Tue Jun 17 20:01:07 UTC 2008


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



More information about the devel mailing list