[PATCH v2 4/4] rtl8192u: fix printk calls in r8192U_core.c

[Xenia Ragiadakou]

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