[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

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

[Lee Jones]

On Wed, 16 Sep 2026, Andy Shevchenko wrote:

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

If you engage your Embedded mind, you will reel at the wasted cycles!

-- 
Lee Jones

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

[Andy Shevchenko]

On Wed, Sep 16, 2026 at 02:42:47PM +0100, Lee Jones wrote:
> On Wed, 16 Sep 2026, Andy Shevchenko wrote:
> > On Wed, Sep 16, 2026 at 11:27:32AM +0100, Lee Jones wrote:
> > > On Wed, 16 Sep 2026, Maria Lisina wrote:

...

> > > Why continue if this returns an error?
> > 
> > Because we don't care.
> 
> If you engage your Embedded mind, you will reel at the wasted cycles!

True, but it not the hot path. Anyway, Greg KH is keen to see debugfs properly
used, id est without checking. I am fine with that.

-- 
With Best Regards,
Andy Shevchenko