Hi,
> First of all, please use real tabs characters. It looks to me like you
> use a broken editor with tab width of 4 characters; this is absolutely
> wrong. Look at Documentation/CodingStyle.txt as a ultimate reference.
>
>
ok, i will fix that.
> Next observation is that you must remove explicitly calling
> mq1100_fb_init() since this function is automatically called by kernel
> at module init time (no matter if it's statically or dynamically
> linked). This is simply wrong, since it's marked as __init and goes away
> as soon as the module gets unloaded. Also it removes the need for the
> initialization lock.
>
>
you are right, that was my first try, and since fbmem uses exactly the
same mechanism for other fbs, i thought it would be consistent to use it
for the mq fb also, i will remove it since it is unnecessary with the
notification mechanism
> Moving debug_func() to the beginning of function is also wrong, as
> debug_func() is basically a wrapper around printk(), and calling
> printk before
> declaring variables is a syntax error in C.
>
> Further, the driver_register variable and associated logic should be
> removed.
> The mq1100fb_init function should be called only by the kernel and never
> automatically, and kernel tracks itself this issue.
>
> Please fix these issues and re-submit the patch. The core logic of it
> looks
> okay to me.
>
>
I will fix the issues and resubmit the patch, thanks for looking over it
Andrew!
BTW, following your suggestion, i submitted the patch to the fbdev
development team, and it seems they might use it as a start for further
event notifications, so maybe it will come back from the mainstream
kernel sometime...
Cheers, Patty
>
>
Received on Wed Mar 03 2004 - 06:00:00 EST
This archive was generated by hypermail 2.2.0 : Mon Jul 25 2005 - 17:19:25 EDT