| Message ID | 1331720516-11801-1-git-send-email-u.kleine-koenig@pengutronix.de |
|---|---|
| State | New |
| Headers | show |
On Wednesday 14 March 2012, Uwe Kleine-König wrote: > This is a needed followup for > > ARM: protect usage of cr_alignment by #ifdef CONFIG_CPU_CP15 > > otherwise no newline is printed for !CONFIG_CPU_CP15 > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > --- > Should this be folded into the > > ARM: protect usage of cr_alignment by #ifdef CONFIG_CPU_CP15 > > patch? Or should it come before it? > I think this patch needs a better explanation. It looks to me that it's simply a bug fix, because every printk that is not a continuation of a previous printk needs a KERN_* line. In that case, it should just get applied as a bug fix independent of the other patches. However, if you sometimes don't have a newline after the previous printk, this patch is actually wrong and should not get applied at all. Arnd
On Wed, 14 Mar 2012, Uwe Kleine-König wrote: > This is a needed followup for > > ARM: protect usage of cr_alignment by #ifdef CONFIG_CPU_CP15 > > otherwise no newline is printed for !CONFIG_CPU_CP15 > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > --- > Should this be folded into the > > ARM: protect usage of cr_alignment by #ifdef CONFIG_CPU_CP15 > > patch? Or should it come before it? Given my comments on that other patch, this one could be dropped altogether. Nicolas
On Wed, Mar 14, 2012 at 01:12:51PM +0000, Arnd Bergmann wrote: > On Wednesday 14 March 2012, Uwe Kleine-König wrote: > > This is a needed followup for > > > > ARM: protect usage of cr_alignment by #ifdef CONFIG_CPU_CP15 > > > > otherwise no newline is printed for !CONFIG_CPU_CP15 > > > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > > --- > > Should this be folded into the > > > > ARM: protect usage of cr_alignment by #ifdef CONFIG_CPU_CP15 > > > > patch? Or should it come before it? > > > > I think this patch needs a better explanation. It looks to me that > it's simply a bug fix, because every printk that is not a continuation > of a previous printk needs a KERN_* line. In that case, it should > just get applied as a bug fix independent of the other patches. > > However, if you sometimes don't have a newline after the previous > printk, this patch is actually wrong and should not get applied at > all. I don't get your reasoning here. An (independant) printk (that is expected to start at a new line) must start with KERN_SOMETHING iff the previous printk always ends in \n? Probably not. Independently of the correctness of the first patch that splits the printk I'm just taking it as an example: In this case the two printks in question are: printk("CPU: %s [%08x] revision %d (ARMv%s)", ...); #ifdef CONFIG_CPU_CP15 printk(KERN_CONT ", cr=%08lx\n", cr_alignment); #endif and printk("CPU: %s data cache, %s instruction cache\n", ...) In this case it's correct to add KERN_INFO to the last printk, isn't it? When would you consider it to be wrong? If you want to have the first group of printks to always end in \n, I'd have to do: printk("CPU: %s [%08x] revision %d (ARMv%s)", ...); #ifdef CONFIG_CPU_CP15 printk(KERN_CONT ", cr=%08lx", cr_alignment); #endif printk(KERN_CONT "\n"); but ISTR Linus (Torvalds) telling me that KERN_CONT "\n" is wrong and should just be skipped. Best regards Uwe
On Wednesday 14 March 2012, Uwe Kleine-König wrote: > I don't get your reasoning here. > > An (independant) printk (that is expected to start at a new line) must > start with KERN_SOMETHING iff the previous printk always ends in \n? > Probably not. > > Independently of the correctness of the first patch that splits the > printk I'm just taking it as an example: > > In this case the two printks in question are: > > printk("CPU: %s [%08x] revision %d (ARMv%s)", ...); > #ifdef CONFIG_CPU_CP15 > printk(KERN_CONT ", cr=%08lx\n", cr_alignment); > #endif > > and > > printk("CPU: %s data cache, %s instruction cache\n", ...) > > In this case it's correct to add KERN_INFO to the last printk, isn't it? > When would you consider it to be wrong? I forgot about KERN_CONT, which did not exist until a few years ago and is still not all that common. Initially, KERN_CONT was just an empty string that was used to make it clear when a printk was intentionally used as a continuation of the previous line, rather than having no KERN_* at all. In the example above, the first and the second line should get a KERN_INFO or one of the others. > If you want to have the first group of printks to always end in \n, I'd > have to do: > > printk("CPU: %s [%08x] revision %d (ARMv%s)", ...); > #ifdef CONFIG_CPU_CP15 > printk(KERN_CONT ", cr=%08lx", cr_alignment); > #endif > printk(KERN_CONT "\n"); > > but ISTR Linus (Torvalds) telling me that KERN_CONT "\n" is wrong and > should just be skipped. Ok, didn't know that. Arnd
diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c index 50d3df8..23893b1 100644 --- a/arch/arm/kernel/setup.c +++ b/arch/arm/kernel/setup.c @@ -325,7 +325,7 @@ static void __init cacheid_init(void) cacheid = CACHEID_VIVT; } - printk("CPU: %s data cache, %s instruction cache\n", + printk(KERN_INFO "CPU: %s data cache, %s instruction cache\n", cache_is_vivt() ? "VIVT" : cache_is_vipt_aliasing() ? "VIPT aliasing" : cache_is_vipt_nonaliasing() ? "PIPT / VIPT nonaliasing" : "unknown",
This is a needed followup for ARM: protect usage of cr_alignment by #ifdef CONFIG_CPU_CP15 otherwise no newline is printed for !CONFIG_CPU_CP15 Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> --- Should this be folded into the ARM: protect usage of cr_alignment by #ifdef CONFIG_CPU_CP15 patch? Or should it come before it? arch/arm/kernel/setup.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)