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

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

[Maria Lisina]

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.

While at it, this patch also clean ups legacy and deprecated usage
of S_IRUGO macro and debugfs_remove_recursive() function.

Signed-off-by: Maria Lisina <sekoohaka.sarisan@gmail.com>
---
Changes in v5:
- Changed the goal of the patch to DebugFS modernization.
- Refactored intel_lpss_debugfs_add() into a void function.
- Removed deprecated macros and functions.
- Removed dmesg message completely.
- Link to v4: https://lore.kernel.org/r/20260914-intel-lpss-debugfs-v4-1-0eb82ea7c1e3@gmail.com

Changes in v4:
- Keep the dmesg messages, instead check whether DebugFS is initialized
- Link to v3: https://lore.kernel.org/r/20260913-intel-lpss-debugfs-v3-1-1b8e2b1f0992@gmail.com

Changes in v3:
- Fixed commit name and description.
- Link to v2: https://lore.kernel.org/r/20260913-intel-lpss-debugfs-v2-1-a1a4faf5cc63@gmail.com

Changes in v2:
- Fixed function name in comment section.
- Link to v1: https://lore.kernel.org/r/20260913-intel-lpss-debugfs-v1-1-833cbb6fffc9@gmail.com
---
 drivers/mfd/intel-lpss.c | 28 +++++++++++-----------------
 1 file changed, 11 insertions(+), 17 deletions(-)

diff --git a/drivers/mfd/intel-lpss.c b/drivers/mfd/intel-lpss.c
index 63d6694f71457b2e09d238af0a9bfb897a169a64..1a61ef1a62b3e8a2777f357d5c4605d5d57d9532 100644
--- a/drivers/mfd/intel-lpss.c
+++ b/drivers/mfd/intel-lpss.c
@@ -142,28 +142,25 @@ static void intel_lpss_cache_ltr(struct intel_lpss *lpss)
 	lpss->idle_ltr = readl(lpss->priv + LPSS_PRIV_IDLELTR);
 }
 
-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);
-	if (IS_ERR(dir))
-		return PTR_ERR(dir);
+	lpss->debugfs =
+		debugfs_create_dir(dev_name(lpss->dev), intel_lpss_debugfs);
 
 	/* Cache the values into lpss structure */
 	intel_lpss_cache_ltr(lpss);
 
-	debugfs_create_x32("capabilities", S_IRUGO, dir, &lpss->caps);
-	debugfs_create_x32("active_ltr", S_IRUGO, dir, &lpss->active_ltr);
-	debugfs_create_x32("idle_ltr", S_IRUGO, dir, &lpss->idle_ltr);
-
-	lpss->debugfs = dir;
-	return 0;
+	debugfs_create_x32("capabilities", 0444, lpss->debugfs,
+					   &lpss->caps);
+	debugfs_create_x32("active_ltr", 0444, lpss->debugfs,
+					   &lpss->active_ltr);
+	debugfs_create_x32("idle_ltr", 0444, lpss->debugfs,
+					   &lpss->idle_ltr);
 }
 
 static void intel_lpss_debugfs_remove(struct intel_lpss *lpss)
 {
-	debugfs_remove_recursive(lpss->debugfs);
+	debugfs_remove(lpss->debugfs);
 }
 
 static void intel_lpss_ltr_set(struct device *dev, s32 val)
@@ -432,10 +429,7 @@ int intel_lpss_probe(struct device *dev,
 		goto err_clk_register;
 
 	intel_lpss_ltr_expose(lpss);
-
-	ret = intel_lpss_debugfs_add(lpss);
-	if (ret)
-		dev_warn(dev, "Failed to create debugfs entries\n");
+	intel_lpss_debugfs_add(lpss);
 
 	if (intel_lpss_has_idma(lpss)) {
 		ret = mfd_add_devices(dev, lpss->devid, &intel_lpss_idma64_cell,

---
base-commit: 2f0c1cf72f4682178506f513bbf015e591b1aa4a
change-id: 20260913-intel-lpss-debugfs-eff3580d1295

Best regards,
-- 
Maria Lisina <sekoohaka.sarisan@gmail.com>

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

[Andy Shevchenko]

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?

>  }

...

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).

-- 
With Best Regards,
Andy Shevchenko

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

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

[Lee Jones]

On Wed, 16 Sep 2026, Maria Lisina wrote:

> 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.
> 
> While at it, this patch also clean ups legacy and deprecated usage
> of S_IRUGO macro and debugfs_remove_recursive() function.
> 
> Signed-off-by: Maria Lisina <sekoohaka.sarisan@gmail.com>
> ---
> Changes in v5:
> - Changed the goal of the patch to DebugFS modernization.
> - Refactored intel_lpss_debugfs_add() into a void function.
> - Removed deprecated macros and functions.
> - Removed dmesg message completely.
> - Link to v4: https://lore.kernel.org/r/20260914-intel-lpss-debugfs-v4-1-0eb82ea7c1e3@gmail.com
> 
> Changes in v4:
> - Keep the dmesg messages, instead check whether DebugFS is initialized
> - Link to v3: https://lore.kernel.org/r/20260913-intel-lpss-debugfs-v3-1-1b8e2b1f0992@gmail.com
> 
> Changes in v3:
> - Fixed commit name and description.
> - Link to v2: https://lore.kernel.org/r/20260913-intel-lpss-debugfs-v2-1-a1a4faf5cc63@gmail.com
> 
> Changes in v2:
> - Fixed function name in comment section.
> - Link to v1: https://lore.kernel.org/r/20260913-intel-lpss-debugfs-v1-1-833cbb6fffc9@gmail.com
> ---
>  drivers/mfd/intel-lpss.c | 28 +++++++++++-----------------
>  1 file changed, 11 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/mfd/intel-lpss.c b/drivers/mfd/intel-lpss.c
> index 63d6694f71457b2e09d238af0a9bfb897a169a64..1a61ef1a62b3e8a2777f357d5c4605d5d57d9532 100644
> --- a/drivers/mfd/intel-lpss.c
> +++ b/drivers/mfd/intel-lpss.c
> @@ -142,28 +142,25 @@ static void intel_lpss_cache_ltr(struct intel_lpss *lpss)
>  	lpss->idle_ltr = readl(lpss->priv + LPSS_PRIV_IDLELTR);
>  }
>  
> -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);
> -	if (IS_ERR(dir))
> -		return PTR_ERR(dir);
> +	lpss->debugfs =
> +		debugfs_create_dir(dev_name(lpss->dev), intel_lpss_debugfs);

No need to line-wrap for this.  Use up to 100-chars to prevent this craziness.

Why continue if this returns an error?

>  
>  	/* Cache the values into lpss structure */
>  	intel_lpss_cache_ltr(lpss);
>  
> -	debugfs_create_x32("capabilities", S_IRUGO, dir, &lpss->caps);
> -	debugfs_create_x32("active_ltr", S_IRUGO, dir, &lpss->active_ltr);
> -	debugfs_create_x32("idle_ltr", S_IRUGO, dir, &lpss->idle_ltr);
> -
> -	lpss->debugfs = dir;
> -	return 0;
> +	debugfs_create_x32("capabilities", 0444, lpss->debugfs,

Does this accept a NULL pointer or an error in parameter 3?

> +					   &lpss->caps);

You should always align to the '(' but in this case, don't wrap at all.

> +	debugfs_create_x32("active_ltr", 0444, lpss->debugfs,
> +					   &lpss->active_ltr);
> +	debugfs_create_x32("idle_ltr", 0444, lpss->debugfs,
> +					   &lpss->idle_ltr);
>  }
>  
>  static void intel_lpss_debugfs_remove(struct intel_lpss *lpss)
>  {
> -	debugfs_remove_recursive(lpss->debugfs);
> +	debugfs_remove(lpss->debugfs);

Please explain why this is required?

>  }
>  
>  static void intel_lpss_ltr_set(struct device *dev, s32 val)
> @@ -432,10 +429,7 @@ int intel_lpss_probe(struct device *dev,
>  		goto err_clk_register;
>  
>  	intel_lpss_ltr_expose(lpss);
> -
> -	ret = intel_lpss_debugfs_add(lpss);
> -	if (ret)
> -		dev_warn(dev, "Failed to create debugfs entries\n");
> +	intel_lpss_debugfs_add(lpss);
>  
>  	if (intel_lpss_has_idma(lpss)) {
>  		ret = mfd_add_devices(dev, lpss->devid, &intel_lpss_idma64_cell,
> 
> ---
> base-commit: 2f0c1cf72f4682178506f513bbf015e591b1aa4a
> change-id: 20260913-intel-lpss-debugfs-eff3580d1295
> 
> Best regards,
> -- 
> Maria Lisina <sekoohaka.sarisan@gmail.com>
> 

-- 
Lee Jones

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

[Andy Shevchenko]

On Wed, Sep 16, 2026 at 11:27:32AM +0100, Lee Jones wrote:
> On Wed, 16 Sep 2026, Maria Lisina wrote:
> 
> > 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.
> > 
> > While at it, this patch also clean ups legacy and deprecated usage
> > of S_IRUGO macro and debugfs_remove_recursive() function.

...

> Why continue if this returns an error?

Because we don't care.

> > +	debugfs_create_x32("capabilities", 0444, lpss->debugfs,
> 
> Does this accept a NULL pointer or an error in parameter 3?

Yes, debugfs is designed to be used without checks.

> > +					   &lpss->caps);

-- 
With Best Regards,
Andy Shevchenko