| Message ID | 20211105102636.1016378-5-clg@kaod.org (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | powerpc/xive: Improve diagnostic and activate StoreEOI on P10 PowerNV | expand |
Cédric Le Goater <clg@kaod.org> writes: > and fix some compile issues when !CONFIG_DEBUG_FS. > > Signed-off-by: Cédric Le Goater <clg@kaod.org> > --- > arch/powerpc/sysdev/xive/common.c | 17 ++++++++++++++--- > 1 file changed, 14 insertions(+), 3 deletions(-) > > diff --git a/arch/powerpc/sysdev/xive/common.c b/arch/powerpc/sysdev/xive/common.c > index 3d558cad1f19..b71cc1020296 100644 > --- a/arch/powerpc/sysdev/xive/common.c > +++ b/arch/powerpc/sysdev/xive/common.c ... > @@ -1779,10 +1782,18 @@ static int xive_core_debug_show(struct seq_file *m, void *private) > } > DEFINE_SHOW_ATTRIBUTE(xive_core_debug); > > +static void xive_core_debugfs_create(void) > +{ > + debugfs_create_file("xive", 0400, arch_debugfs_dir, > + NULL, &xive_core_debug_fops); > +} > + > +#endif /* CONFIG_DEBUG_FS */ > + > int xive_core_debug_init(void) > { > - if (xive_enabled()) > - debugfs_create_file("xive", 0400, arch_debugfs_dir, > - NULL, &xive_core_debug_fops); > + if (xive_enabled() && IS_ENABLED(CONFIG_DEBUG_FS)) > + xive_core_debugfs_create(); > + > return 0; > } For skiroot_defconfig this gives me: arch/powerpc/sysdev/xive/common.c: In function ‘xive_core_init’: arch/powerpc/sysdev/xive/common.c:1676:2: error: implicit declaration of function ‘xive_core_debugfs_create’; did you mean ‘xive_core_debug_init’? [-Werror=implicit-function-declaration] 1676 | xive_core_debugfs_create(); | ^~~~~~~~~~~~~~~~~~~~~~~~ | xive_core_debug_init cc1: all warnings being treated as errors We need an empty inline stub of xive_core_debugfs_create() for the CONFIG_DEBUG_FS=n case. I'm wondering though why do we have xive_core_debug_init() at all, why don't we just initialise the debugfs files in xive_core_init()? cheers
On 11/18/21 10:21, Michael Ellerman wrote: > Cédric Le Goater <clg@kaod.org> writes: > >> and fix some compile issues when !CONFIG_DEBUG_FS. >> >> Signed-off-by: Cédric Le Goater <clg@kaod.org> >> --- >> arch/powerpc/sysdev/xive/common.c | 17 ++++++++++++++--- >> 1 file changed, 14 insertions(+), 3 deletions(-) >> >> diff --git a/arch/powerpc/sysdev/xive/common.c b/arch/powerpc/sysdev/xive/common.c >> index 3d558cad1f19..b71cc1020296 100644 >> --- a/arch/powerpc/sysdev/xive/common.c >> +++ b/arch/powerpc/sysdev/xive/common.c > ... >> @@ -1779,10 +1782,18 @@ static int xive_core_debug_show(struct seq_file *m, void *private) >> } >> DEFINE_SHOW_ATTRIBUTE(xive_core_debug); >> >> +static void xive_core_debugfs_create(void) >> +{ >> + debugfs_create_file("xive", 0400, arch_debugfs_dir, >> + NULL, &xive_core_debug_fops); >> +} >> + >> +#endif /* CONFIG_DEBUG_FS */ >> + >> int xive_core_debug_init(void) >> { >> - if (xive_enabled()) >> - debugfs_create_file("xive", 0400, arch_debugfs_dir, >> - NULL, &xive_core_debug_fops); >> + if (xive_enabled() && IS_ENABLED(CONFIG_DEBUG_FS)) >> + xive_core_debugfs_create(); >> + >> return 0; >> } > > For skiroot_defconfig this gives me: > > arch/powerpc/sysdev/xive/common.c: In function ‘xive_core_init’: > arch/powerpc/sysdev/xive/common.c:1676:2: error: implicit declaration of function ‘xive_core_debugfs_create’; did you mean ‘xive_core_debug_init’? [-Werror=implicit-function-declaration] > 1676 | xive_core_debugfs_create(); > | ^~~~~~~~~~~~~~~~~~~~~~~~ > | xive_core_debug_init > cc1: all warnings being treated as errors > > > We need an empty inline stub of xive_core_debugfs_create() for the > CONFIG_DEBUG_FS=n case. I thought IS_ENABLED(CONFIG_DEBUG_FS)) was protecting compile from that issue. Do you want a v2 for the whole ? Or, I can send a replacement for patch 4 only. > I'm wondering though why do we have xive_core_debug_init() at all, why > don't we just initialise the debugfs files in xive_core_init()? I think I tried that and there was an ordering issue. Thanks, C.
diff --git a/arch/powerpc/sysdev/xive/common.c b/arch/powerpc/sysdev/xive/common.c index 3d558cad1f19..b71cc1020296 100644 --- a/arch/powerpc/sysdev/xive/common.c +++ b/arch/powerpc/sysdev/xive/common.c @@ -227,6 +227,7 @@ static void xive_esb_write(struct xive_irq_data *xd, u32 offset, u64 data) out_be64(xd->eoi_mmio + offset, data); } +#if defined(CONFIG_XMON) || defined(CONFIG_DEBUG_FS) static void xive_irq_data_dump(struct xive_irq_data *xd, char *buffer, size_t size) { u64 val = xive_esb_read(xd, XIVE_ESB_GET); @@ -239,6 +240,7 @@ static void xive_irq_data_dump(struct xive_irq_data *xd, char *buffer, size_t si val & XIVE_ESB_VAL_Q ? 'Q' : '-', xd->trig_page, xd->eoi_page); } +#endif #ifdef CONFIG_XMON static notrace void xive_dump_eq(const char *name, struct xive_q *q) @@ -1701,6 +1703,7 @@ static int __init xive_off(char *arg) } __setup("xive=off", xive_off); +#ifdef CONFIG_DEBUG_FS static void xive_debug_show_cpu(struct seq_file *m, int cpu) { struct xive_cpu *xc = per_cpu(xive_cpu, cpu); @@ -1779,10 +1782,18 @@ static int xive_core_debug_show(struct seq_file *m, void *private) } DEFINE_SHOW_ATTRIBUTE(xive_core_debug); +static void xive_core_debugfs_create(void) +{ + debugfs_create_file("xive", 0400, arch_debugfs_dir, + NULL, &xive_core_debug_fops); +} + +#endif /* CONFIG_DEBUG_FS */ + int xive_core_debug_init(void) { - if (xive_enabled()) - debugfs_create_file("xive", 0400, arch_debugfs_dir, - NULL, &xive_core_debug_fops); + if (xive_enabled() && IS_ENABLED(CONFIG_DEBUG_FS)) + xive_core_debugfs_create(); + return 0; }
and fix some compile issues when !CONFIG_DEBUG_FS. Signed-off-by: Cédric Le Goater <clg@kaod.org> --- arch/powerpc/sysdev/xive/common.c | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-)