| Message ID | 1340812911-6793-4-git-send-email-shangw@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Benjamin Herrenschmidt |
| Headers | show |
On Thu, 2012-06-28 at 00:01 +0800, Gavin Shan wrote: > The patch adds more logs to EEH initialization functions for > debugging purpose. Also, the machine type ("pSeries") is checked > in the platform initialization to assure it's the correct platform > to invoke it. Hi Gavin, Our boot logs are full enough. pr_info() is not right for this sort of stuff. For debug use: * pr_debug() - which can be enabled dynamically. * pr_devel() - which needs to be built with #define DEBUG * printk(KERN_DEBUG) - for things you always want printed, but needn't go to the console by default. > diff --git a/arch/powerpc/platforms/pseries/eeh_dev.c b/arch/powerpc/platforms/pseries/eeh_dev.c > index 8e3443b..a0cee3a 100644 > --- a/arch/powerpc/platforms/pseries/eeh_dev.c > +++ b/arch/powerpc/platforms/pseries/eeh_dev.c > @@ -100,6 +100,8 @@ static int __init eeh_dev_phb_init(void) > list_for_each_entry_safe(phb, tmp, &hose_list, list_node) > eeh_dev_phb_init_dynamic(phb); > > + pr_info("EEH: devices created\n"); That's not actually very informative. > diff --git a/arch/powerpc/platforms/pseries/eeh_pseries.c b/arch/powerpc/platforms/pseries/eeh_pseries.c > index bcf0bb8..bb2bd90 100644 > --- a/arch/powerpc/platforms/pseries/eeh_pseries.c > +++ b/arch/powerpc/platforms/pseries/eeh_pseries.c > @@ -561,7 +561,18 @@ static struct eeh_ops pseries_eeh_ops = { > */ > static int __init eeh_pseries_init(void) > { > - return eeh_ops_register(&pseries_eeh_ops); > + int ret = -EINVAL; > + > + if (!machine_is(pseries)) > + return ret; > + > + ret = eeh_ops_register(&pseries_eeh_ops); > + if (!ret) > + pr_info("EEH: pSeries platform initialized\n"); > + else > + pr_info("EEH: pSeries platform initialization failure\n"); > + > + return ret; > } > > early_initcall(eeh_pseries_init); You can achieve the same with initcall_debug. But if you want to keep it at least print the return code, that's the first thing you will want to know if it fails. cheers
>On Thu, 2012-06-28 at 00:01 +0800, Gavin Shan wrote: >> The patch adds more logs to EEH initialization functions for >> debugging purpose. Also, the machine type ("pSeries") is checked >> in the platform initialization to assure it's the correct platform >> to invoke it. > >Hi Gavin, > >Our boot logs are full enough. pr_info() is not right for this sort of >stuff. > >For debug use: > * pr_debug() - which can be enabled dynamically. > * pr_devel() - which needs to be built with #define DEBUG > * printk(KERN_DEBUG) - for things you always want printed, but > needn't go to the console by default. > Yes, Michael. I'll replace "pr_info" with "pr_debug" in next revision :-) >> diff --git a/arch/powerpc/platforms/pseries/eeh_dev.c b/arch/powerpc/platforms/pseries/eeh_dev.c >> index 8e3443b..a0cee3a 100644 >> --- a/arch/powerpc/platforms/pseries/eeh_dev.c >> +++ b/arch/powerpc/platforms/pseries/eeh_dev.c >> @@ -100,6 +100,8 @@ static int __init eeh_dev_phb_init(void) >> list_for_each_entry_safe(phb, tmp, &hose_list, list_node) >> eeh_dev_phb_init_dynamic(phb); >> >> + pr_info("EEH: devices created\n"); > >That's not actually very informative. > Yep. Let me make it more informative in next revision. >> diff --git a/arch/powerpc/platforms/pseries/eeh_pseries.c b/arch/powerpc/platforms/pseries/eeh_pseries.c >> index bcf0bb8..bb2bd90 100644 >> --- a/arch/powerpc/platforms/pseries/eeh_pseries.c >> +++ b/arch/powerpc/platforms/pseries/eeh_pseries.c >> @@ -561,7 +561,18 @@ static struct eeh_ops pseries_eeh_ops = { >> */ >> static int __init eeh_pseries_init(void) >> { >> - return eeh_ops_register(&pseries_eeh_ops); >> + int ret = -EINVAL; >> + >> + if (!machine_is(pseries)) >> + return ret; >> + >> + ret = eeh_ops_register(&pseries_eeh_ops); >> + if (!ret) >> + pr_info("EEH: pSeries platform initialized\n"); >> + else >> + pr_info("EEH: pSeries platform initialization failure\n"); >> + >> + return ret; >> } >> >> early_initcall(eeh_pseries_init); > >You can achieve the same with initcall_debug. > >But if you want to keep it at least print the return code, that's the >first thing you will want to know if it fails. > Thanks, Michael. Let me print "ret" for the failure case in next revision. Also, I will replace "pr_info" with "pr_debug" as you suggested :-) Thanks, Gavin >cheers > > > >_______________________________________________ >Linuxppc-dev mailing list >Linuxppc-dev@lists.ozlabs.org >https://lists.ozlabs.org/listinfo/linuxppc-dev >
diff --git a/arch/powerpc/platforms/pseries/eeh_dev.c b/arch/powerpc/platforms/pseries/eeh_dev.c index 8e3443b..a0cee3a 100644 --- a/arch/powerpc/platforms/pseries/eeh_dev.c +++ b/arch/powerpc/platforms/pseries/eeh_dev.c @@ -100,6 +100,8 @@ static int __init eeh_dev_phb_init(void) list_for_each_entry_safe(phb, tmp, &hose_list, list_node) eeh_dev_phb_init_dynamic(phb); + pr_info("EEH: devices created\n"); + return 0; } diff --git a/arch/powerpc/platforms/pseries/eeh_pseries.c b/arch/powerpc/platforms/pseries/eeh_pseries.c index bcf0bb8..bb2bd90 100644 --- a/arch/powerpc/platforms/pseries/eeh_pseries.c +++ b/arch/powerpc/platforms/pseries/eeh_pseries.c @@ -561,7 +561,18 @@ static struct eeh_ops pseries_eeh_ops = { */ static int __init eeh_pseries_init(void) { - return eeh_ops_register(&pseries_eeh_ops); + int ret = -EINVAL; + + if (!machine_is(pseries)) + return ret; + + ret = eeh_ops_register(&pseries_eeh_ops); + if (!ret) + pr_info("EEH: pSeries platform initialized\n"); + else + pr_info("EEH: pSeries platform initialization failure\n"); + + return ret; } early_initcall(eeh_pseries_init);
The patch adds more logs to EEH initialization functions for debugging purpose. Also, the machine type ("pSeries") is checked in the platform initialization to assure it's the correct platform to invoke it. Signed-off-by: Gavin Shan <shangw@linux.vnet.ibm.com> --- arch/powerpc/platforms/pseries/eeh_dev.c | 2 ++ arch/powerpc/platforms/pseries/eeh_pseries.c | 13 ++++++++++++- 2 files changed, 14 insertions(+), 1 deletion(-)