Re: [PATCH v5] mfd: intel-lpss: Clean up and modernize DebugFS usage

[Andy Shevchenko]

On Wed, Sep 16, 2026 at 10:23:18AM +0300, Andy Shevchenko wrote:
> On Wed, Sep 16, 2026 at 03:13:35AM +0500, Maria Lisina wrote:
> 
> Thank you for the update, my comments below.
> 
> > The DebugFS API is designed to handle errors gracefully.
> > Any explicit checking on return values is considered an anti-pattern.
> > 
> > This patch removes unnecessary error checking and converts
> > intel_lpss_debugfs_add() into a void function.
> 
> Yep, these two paragraphs are good!
> 
> > While at it, this patch also clean ups legacy and deprecated usage
> > of S_IRUGO macro and debugfs_remove_recursive() function.
> 
> No, this should be a separate patch.

...

> > -static int intel_lpss_debugfs_add(struct intel_lpss *lpss)
> > +static void intel_lpss_debugfs_add(struct intel_lpss *lpss)
> >  {
> 
> > -	struct dentry *dir;
> > -
> > -	dir = debugfs_create_dir(dev_name(lpss->dev), intel_lpss_debugfs);
> 
> > +	lpss->debugfs =
> > +		debugfs_create_dir(dev_name(lpss->dev), intel_lpss_debugfs);
> 
> > -	lpss->debugfs = dir;
> 
> This is unneeded churn. Yes, I understand that dir won't be used, but if you
> really want to refactor do it in a separate change.
> 
> >  }

...

> >  static void intel_lpss_debugfs_remove(struct intel_lpss *lpss)
> >  {
> > -	debugfs_remove_recursive(lpss->debugfs);
> > +	debugfs_remove(lpss->debugfs);
> 
> Hmm... Why?

Answering this will give you...

> >  }

...

> So, if you really want a refactoring, split this to up to three patches:
> 1) drop debugfs checks along with the warn message (as your initial goal);
> 2) replace macros by plain octal values for the permissions (optional);
> 3) assign debugfs directory directly (optional).

4) replace debugfs_remove_recursive() with debugfs_remove() (optional).

-- 
With Best Regards,
Andy Shevchenko