On 05/23/2013 03:06 PM, Dan Carpenter wrote:
Please don't drop the list from the CC. I have added it again.
Sorry, i forgot to add the list. Thank you for adding it.
On Thu, May 23, 2013 at 01:41:14PM +0300, Xenia Ragiadakou wrote:
Thank you for your comments. I was counting to fix the identation
issues and other coding style issues in future patches.
I try to make one type of change per patch so that it can be easily
reviewed by maintainers and i think i will stick on that.
I review staging patches so I am one of the people you are refering
to. :P The corrections I suggested were closely related to your
patch so it still falls under the one thing per patch rule.
Maybe for other maintainers it is not easy to review multiple type
of changes per line. Even for me that i review my own patches
it is not easy. And as i said the corrections you suggested will be
made in following patches, definitely :)
Also, I am not sure that i understood the reason that you gave
for replacing pr_devel with pr_debug (i could not find pr_dbg())
They are similar except from the fact that pr_devel does not support
dynamic debug (and for this reason i think too that pr_debug should
be preferred and i will change it) by looking in their definitions.
Can you please explain me why pr_devel makes you more nervous than pr_debug?
Because there is no point to pr_devel() unless you are the
maintainer and are doing it as part of a larger plan. What you are
doing just changes the code into dead code which someone else has to
delete.
Actually, i did not know that the rtl8192u is not under development.
I thought that since it is under staging, it is currently under development
(it includes all these TODO, FIXME, unimplemented function bodies etc)
I will wait for the opinion of Greg on this and maybe before writing again
the patch for the printk replacement, i will first cleanup completely
the code.
Also for KERN_CONT, in include/linux/kernel_levels.h, it says:
Only to be used by core/arch core during early bootup (a continued
line is not SMP_safe
otherwise). So is it safe to use it in this driver as you suggested?
To be honest, the places where I suggested use pr_cont() in this
driver are already broken() but your change makes it even more
broken because they change "\n" to "<7> prefix:\n". The correct
thing is to rewrite this so it prints a line at a time like this:
"<7>prefix: my message.\n"
Instead of how you have changed it:
"<7>prefix: my<7>prefix: message<7>prefix:\n".
Create a hello.ko module and play with this a bit to see what I
mean.
You are right. I will do that.
Sorry, i am completely new in linux kernel code and my intuition
does not help me.
No worries. We all started as newbies.
regards,
dan carpenter
Thank you again, your advices are valuable to me.
regards,
Ksenia
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
