Message ID | 3922453a668745466cb291f889d2c067932e4894.1232001793.git.michael@ellerman.id.au (mailing list archive) |
---|---|
State | Changes Requested, archived |
Delegated to: | Benjamin Herrenschmidt |
Headers | show |
On Wed, Jan 14, 2009 at 11:43 PM, Michael Ellerman <michael@ellerman.id.au> wrote: > There's quite a lot of code that does: > > if (ppc_md.progress) > ppc_md.progress(...) > > So move that idiom into a wrapper. Having a wrapper also allows us > to have a fallback to printk if no progress routine is specified. > > Signed-off-by: Michael Ellerman <michael@ellerman.id.au> Good idea. Acked-by: Grant Likely <grant.likely@secretlab.ca> > --- > arch/powerpc/include/asm/machdep.h | 2 ++ > arch/powerpc/kernel/setup-common.c | 8 ++++++++ > 2 files changed, 10 insertions(+), 0 deletions(-) > > diff --git a/arch/powerpc/include/asm/machdep.h b/arch/powerpc/include/asm/machdep.h > index 6c34a0d..9e4ab07 100644 > --- a/arch/powerpc/include/asm/machdep.h > +++ b/arch/powerpc/include/asm/machdep.h > @@ -328,6 +328,8 @@ extern void __devinit smp_generic_take_timebase(void); > /* Print a boot progress message. */ > void ppc64_boot_msg(unsigned int src, const char *msg); > > +extern void ppc_progress(char *msg, unsigned short code); > + > static inline void log_error(char *buf, unsigned int err_type, int fatal) > { > if (ppc_md.log_error) > diff --git a/arch/powerpc/kernel/setup-common.c b/arch/powerpc/kernel/setup-common.c > index 705fc4b..cc4997b 100644 > --- a/arch/powerpc/kernel/setup-common.c > +++ b/arch/powerpc/kernel/setup-common.c > @@ -669,3 +669,11 @@ static int powerpc_debugfs_init(void) > } > arch_initcall(powerpc_debugfs_init); > #endif > + > +void ppc_progress(char *msg, unsigned short code) > +{ > + if (ppc_md.progress) > + ppc_md.progress(msg, code); > + else > + printk(KERN_DEBUG "*** %04x : %s\n", code, msg ? msg : ""); > +} > -- > 1.5.5 > > _______________________________________________ > Linuxppc-dev mailing list > Linuxppc-dev@ozlabs.org > https://ozlabs.org/mailman/listinfo/linuxppc-dev >
Michael Ellerman writes: > There's quite a lot of code that does: > > if (ppc_md.progress) > ppc_md.progress(...) > > So move that idiom into a wrapper. Having a wrapper also allows us > to have a fallback to printk if no progress routine is specified. It certainly used to be the case on ppc32 (and may still be) that ppc_md.progress was called very very early, in some cases before the MMU was set up, so we don't want to call printk from it. Falling back to udbg_printf would be more appropriate. The idea of ppc_md.progress was that if you need to debug very early boot and you have something that you can poke with absolutely minimal setup and observe externally, you can hook that up to ppc_md.progress and get an idea where the system is dying. Once you get to the point where printk works then you can just use printk. Paul.
> It certainly used to be the case on ppc32 (and may still be) that > ppc_md.progress was called very very early, in some cases before the > MMU was set up, so we don't want to call printk from it. Falling back > to udbg_printf would be more appropriate. > > The idea of ppc_md.progress was that if you need to debug very early > boot and you have something that you can poke with absolutely minimal > setup and observe externally, you can hook that up to ppc_md.progress > and get an idea where the system is dying. Once you get to the point > where printk works then you can just use printk. Right, printk shouldn't be called before lockdep is initialized, though I don't see a call to lockdep_init on ppc32 .. Appart from that, printk is pretty safe as long as we have some kind of translation from PAGE_OFFSET that covers the kernel text, data and bss, which we do have fairly early. I think the reasons we did that back then is that printk output will end up nowhere. Nowadays however, we have udbg console which can possibly be initialized very early too. I'd say let's keep that patch on the back burner until we get a chance to play a bit more with that stuff. Among other things, I still want to change the early initialization order of ppc32 to look more like ppc64 which would help in that are as well so ... Cheers, Ben.
On Thu, 2009-01-15 at 22:34 +1100, Paul Mackerras wrote: > Michael Ellerman writes: > > > There's quite a lot of code that does: > > > > if (ppc_md.progress) > > ppc_md.progress(...) > > > > So move that idiom into a wrapper. Having a wrapper also allows us > > to have a fallback to printk if no progress routine is specified. > > It certainly used to be the case on ppc32 (and may still be) that > ppc_md.progress was called very very early, in some cases before the > MMU was set up, so we don't want to call printk from it. Falling back > to udbg_printf would be more appropriate. Except on some platforms udbg_printf() doesn't work. And all the platforms that care still can and do use udbg_progress(). This just saves us having several implementations of progress that just call printk(). cheers
diff --git a/arch/powerpc/include/asm/machdep.h b/arch/powerpc/include/asm/machdep.h index 6c34a0d..9e4ab07 100644 --- a/arch/powerpc/include/asm/machdep.h +++ b/arch/powerpc/include/asm/machdep.h @@ -328,6 +328,8 @@ extern void __devinit smp_generic_take_timebase(void); /* Print a boot progress message. */ void ppc64_boot_msg(unsigned int src, const char *msg); +extern void ppc_progress(char *msg, unsigned short code); + static inline void log_error(char *buf, unsigned int err_type, int fatal) { if (ppc_md.log_error) diff --git a/arch/powerpc/kernel/setup-common.c b/arch/powerpc/kernel/setup-common.c index 705fc4b..cc4997b 100644 --- a/arch/powerpc/kernel/setup-common.c +++ b/arch/powerpc/kernel/setup-common.c @@ -669,3 +669,11 @@ static int powerpc_debugfs_init(void) } arch_initcall(powerpc_debugfs_init); #endif + +void ppc_progress(char *msg, unsigned short code) +{ + if (ppc_md.progress) + ppc_md.progress(msg, code); + else + printk(KERN_DEBUG "*** %04x : %s\n", code, msg ? msg : ""); +}
There's quite a lot of code that does: if (ppc_md.progress) ppc_md.progress(...) So move that idiom into a wrapper. Having a wrapper also allows us to have a fallback to printk if no progress routine is specified. Signed-off-by: Michael Ellerman <michael@ellerman.id.au> --- arch/powerpc/include/asm/machdep.h | 2 ++ arch/powerpc/kernel/setup-common.c | 8 ++++++++ 2 files changed, 10 insertions(+), 0 deletions(-)