* [PATCH v5] mfd: intel-lpss: Clean up and modernize DebugFS usage
@ 2026-09-15 22:13 Maria Lisina
2026-09-16 7:23 ` Andy Shevchenko
2026-09-16 10:27 ` Lee Jones
0 siblings, 2 replies; 6+ messages in thread
From: Maria Lisina @ 2026-09-15 22:13 UTC (permalink / raw)
To: Andy Shevchenko, Lee Jones; +Cc: mfd, linux-kernel, 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>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v5] mfd: intel-lpss: Clean up and modernize DebugFS usage
2026-09-15 22:13 [PATCH v5] mfd: intel-lpss: Clean up and modernize DebugFS usage Maria Lisina
@ 2026-09-16 7:23 ` Andy Shevchenko
2026-09-16 7:25 ` Andy Shevchenko
2026-09-16 10:27 ` Lee Jones
1 sibling, 1 reply; 6+ messages in thread
From: Andy Shevchenko @ 2026-09-16 7:23 UTC (permalink / raw)
To: Maria Lisina; +Cc: Lee Jones, mfd, linux-kernel
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v5] mfd: intel-lpss: Clean up and modernize DebugFS usage
2026-09-16 7:23 ` Andy Shevchenko
@ 2026-09-16 7:25 ` Andy Shevchenko
0 siblings, 0 replies; 6+ messages in thread
From: Andy Shevchenko @ 2026-09-16 7:25 UTC (permalink / raw)
To: Maria Lisina; +Cc: Lee Jones, mfd, linux-kernel
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v5] mfd: intel-lpss: Clean up and modernize DebugFS usage
2026-09-15 22:13 [PATCH v5] mfd: intel-lpss: Clean up and modernize DebugFS usage Maria Lisina
2026-09-16 7:23 ` Andy Shevchenko
@ 2026-09-16 10:27 ` Lee Jones
2026-09-16 11:01 ` Andy Shevchenko
1 sibling, 1 reply; 6+ messages in thread
From: Lee Jones @ 2026-09-16 10:27 UTC (permalink / raw)
To: Maria Lisina; +Cc: Andy Shevchenko, mfd, linux-kernel
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v5] mfd: intel-lpss: Clean up and modernize DebugFS usage
2026-09-16 10:27 ` Lee Jones
@ 2026-09-16 11:01 ` Andy Shevchenko
2026-09-16 13:42 ` Lee Jones
0 siblings, 1 reply; 6+ messages in thread
From: Andy Shevchenko @ 2026-09-16 11:01 UTC (permalink / raw)
To: Lee Jones; +Cc: Maria Lisina, mfd, linux-kernel
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v5] mfd: intel-lpss: Clean up and modernize DebugFS usage
2026-09-16 11:01 ` Andy Shevchenko
@ 2026-09-16 13:42 ` Lee Jones
0 siblings, 0 replies; 6+ messages in thread
From: Lee Jones @ 2026-09-16 13:42 UTC (permalink / raw)
To: Andy Shevchenko; +Cc: Maria Lisina, mfd, linux-kernel
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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-09-16 13:42 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-15 22:13 [PATCH v5] mfd: intel-lpss: Clean up and modernize DebugFS usage Maria Lisina
2026-09-16 7:23 ` Andy Shevchenko
2026-09-16 7:25 ` Andy Shevchenko
2026-09-16 10:27 ` Lee Jones
2026-09-16 11:01 ` Andy Shevchenko
2026-09-16 13:42 ` Lee Jones
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®