On Thursday, November 1, 2018 12:45:00 PM CET Michael Ellerman wrote:
> Hi Christian,
>
> Christian Lamparter <chunk...@gmail.com> writes:
> > Previously, the kernel would complain:
> >
> > | debugfs ppc4xx ocm: failed to create file
> >
> > But the "info" file was still created and working. This
> > is because debugfs_create_file() returns a pointer to a
> > struct *dentry on success or -ENODEV when debugfs isn't
> > available. This patch fixes both the debugfs_create_dir()
> > and debugfs_create_file() check, so this will work as
> > expected.
> >
> > Signed-off-by: Christian Lamparter <chunk...@gmail.com>
> >
> > diff --git a/arch/powerpc/platforms/4xx/ocm.c
> > b/arch/powerpc/platforms/4xx/ocm.c
> > index 69d9f60d9fe5..e616c8636303 100644
> > --- a/arch/powerpc/platforms/4xx/ocm.c
> > +++ b/arch/powerpc/platforms/4xx/ocm.c
> > @@ -293,13 +293,14 @@ static int ocm_debugfs_init(void)
> > {
> > struct dentry *junk;
> >
> > - junk = debugfs_create_dir("ppc4xx_ocm", 0);
> > - if (!junk) {
> > + junk = debugfs_create_dir("ppc4xx_ocm", NULL);
> > + if (IS_ERR_OR_NULL(junk)) {
> > printk(KERN_ALERT "debugfs ppc4xx ocm: failed to create dir\n");
> > return -1;
> > }
> >
> > - if (debugfs_create_file("info", 0644, junk, NULL, &ocm_debugfs_fops)) {
> > + if (IS_ERR_OR_NULL(debugfs_create_file("info", 0644, junk, NULL,
> > + &ocm_debugfs_fops))) {
> > printk(KERN_ALERT "debugfs ppc4xx ocm: failed to create
> > file\n");
> > return -1;
> > }
>
> Typically we would just make the whole function not compile when DEBUG_FS
> is disabled, eg:
> diff --git a/arch/powerpc/platforms/4xx/ocm.c
> b/arch/powerpc/platforms/4xx/ocm.c
> index f5bbd4563342..c5293a28545c 100644
> --- a/arch/powerpc/platforms/4xx/ocm.c
> +++ b/arch/powerpc/platforms/4xx/ocm.c
> @@ -286,6 +286,7 @@ static const struct file_operations ocm_debugfs_fops = {
> .release = single_release,
> };
>
> +#ifdef CONFIG_DEBUG_FS
> static int ocm_debugfs_init(void)
> {
> struct dentry *junk;
> @@ -303,6 +304,9 @@ static int ocm_debugfs_init(void)
>
> return 0;
> }
> +#else
> +static int ocm_debugfs_init(void) { return 0; }
> +#endif
>
> void *ppc4xx_ocm_alloc(phys_addr_t *phys, int size, int align,
> int flags, const char *owner)
>
>
> I don't really mind, but I think the #ifdef approach is simpler to
> reason about.
>
> cheers
Thank you. I'll post a v2 right away.
