Re: [PATCH] ppc4xx: ocm: fix errnous "failed to create file" errors

[Christian Lamparter <chunkeey@gmail.com>]

On Thursday, November 1, 2018 12:45:00 PM CET Michael Ellerman wrote:
> Hi Christian,
> 
> Christian Lamparter <chunkeey@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 <chunkeey@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.