diff mbox series

regulator: core: Streamline debugfs operations

Message ID 587f259fb8634daa268595f60fa52c3c4d9bbec2.1684854090.git.geert+renesas@glider.be
State New
Headers
Series regulator: core: Streamline debugfs operations |

Commit Message

Geert Uytterhoeven May 23, 2023, 3:03 p.m. UTC 
  If CONFIG_DEBUG_FS is not set:

    regulator: Failed to create debugfs directory
    ...
    regulator-dummy: Failed to create debugfs directory

As per the comments for debugfs_create_dir(), errors returned by this
function should be expected, and ignored:

 * If debugfs is not enabled in the kernel, the value -%ENODEV will be
 * returned.
 *
 * NOTE: it's expected that most callers should _ignore_ the errors returned
 * by this function. Other debugfs functions handle the fact that the "dentry"
 * passed to them could be an error and they don't crash in that case.
 * Drivers should generally work fine even if debugfs fails to init anyway.

Afhere to the debugfs spirit, and streamline all operations by:
  1. Demoting the importance of the printed error messages to debug
     level, like is already done in create_regulator(),
  2. Further ignoring any returned errors.

Fix the error check on the return value of debugfs_create_dir() in
create_regulator(), which was missed before.

Fixes: 2bf1c45be3b8f3a3 ("regulator: Fix error checking for debugfs_create_dir")
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/regulator/core.c | 30 +++++++++++++-----------------
 1 file changed, 13 insertions(+), 17 deletions(-)

Comments

Osama Muhammad May 23, 2023, 4:48 p.m. UTC | #1 
Hi,

The patch to Fix error checking for debugfs_create_dir has already
been submitted.
Here is the link to the patch :-
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git/commit/?id=2bf1c45be3b8f3a3f898d0756c1282f09719debd


On Tue, 23 May 2023 at 20:17, Mark Brown <broonie@kernel.org> wrote:
>
> On Tue, May 23, 2023 at 05:03:58PM +0200, Geert Uytterhoeven wrote:
>
> > Fix the error check on the return value of debugfs_create_dir() in
> > create_regulator(), which was missed before.
> >
> > Fixes: 2bf1c45be3b8f3a3 ("regulator: Fix error checking for debugfs_create_dir")
>
> Also: this should be a separate patch.
Geert Uytterhoeven May 24, 2023, 6:40 a.m. UTC | #2 
Hi Mark,

On Tue, May 23, 2023 at 5:16 PM Mark Brown <broonie@kernel.org> wrote:
> On Tue, May 23, 2023 at 05:03:58PM +0200, Geert Uytterhoeven wrote:
>
> > -     if (!regulator->debugfs) {
> > +     if (IS_ERR(regulator->debugfs))
> >               rdev_dbg(rdev, "Failed to create debugfs directory\n");
> > -     } else {
> > -             debugfs_create_u32("uA_load", 0444, regulator->debugfs,
> > -                                &regulator->uA_load);
> > -     }
> > +
> > +     debugfs_create_u32("uA_load", 0444, regulator->debugfs,
> > +                        &regulator->uA_load);
>
> No, it's actually useful to not just dump these files in the root
> directory if we fail to create the per regulator directory.

If regulator->debugfs is an error, no files are dumped in the root
directory.

By design, all debugfs functions are no-ops when passed an error,
cfr. the comment quoted above:

    Other debugfs functions handle the fact that the "dentry"
    passed to them could be an error and they don't crash in that case.

Gr{oetje,eeting}s,

                        Geert
Geert Uytterhoeven May 24, 2023, 6:40 a.m. UTC | #3 
Hi Mark,

On Tue, May 23, 2023 at 5:17 PM Mark Brown <broonie@kernel.org> wrote:
> On Tue, May 23, 2023 at 05:03:58PM +0200, Geert Uytterhoeven wrote:
> > Fix the error check on the return value of debugfs_create_dir() in
> > create_regulator(), which was missed before.
> >
> > Fixes: 2bf1c45be3b8f3a3 ("regulator: Fix error checking for debugfs_create_dir")
>
> Also: this should be a separate patch.

I can  do that, sure (patch stats++ ;-)

Gr{oetje,eeting}s,

                        Geert
Geert Uytterhoeven May 24, 2023, 6:41 a.m. UTC | #4 
Hi Osama,

On Tue, May 23, 2023 at 6:48 PM Osama Muhammad <osmtendev@gmail.com> wrote:
> The patch to Fix error checking for debugfs_create_dir has already
> been submitted.
> Here is the link to the patch :-
> https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git/commit/?id=2bf1c45be3b8f3a3f898d0756c1282f09719debd

Your patch fixed two cases, but missed the third.

> On Tue, 23 May 2023 at 20:17, Mark Brown <broonie@kernel.org> wrote:
> >
> > On Tue, May 23, 2023 at 05:03:58PM +0200, Geert Uytterhoeven wrote:
> >
> > > Fix the error check on the return value of debugfs_create_dir() in
> > > create_regulator(), which was missed before.
> > >
> > > Fixes: 2bf1c45be3b8f3a3 ("regulator: Fix error checking for debugfs_create_dir")
> >
> > Also: this should be a separate patch.

Gr{oetje,eeting}s,

                        Geert
diff mbox series

Patch

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 698ab7f5004bf6b7..d8e1caaf207e108f 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -1911,19 +1911,17 @@  static struct regulator *create_regulator(struct regulator_dev *rdev,
 
 	if (err != -EEXIST)
 		regulator->debugfs = debugfs_create_dir(supply_name, rdev->debugfs);
-	if (!regulator->debugfs) {
+	if (IS_ERR(regulator->debugfs))
 		rdev_dbg(rdev, "Failed to create debugfs directory\n");
-	} else {
-		debugfs_create_u32("uA_load", 0444, regulator->debugfs,
-				   &regulator->uA_load);
-		debugfs_create_u32("min_uV", 0444, regulator->debugfs,
-				   &regulator->voltage[PM_SUSPEND_ON].min_uV);
-		debugfs_create_u32("max_uV", 0444, regulator->debugfs,
-				   &regulator->voltage[PM_SUSPEND_ON].max_uV);
-		debugfs_create_file("constraint_flags", 0444,
-				    regulator->debugfs, regulator,
-				    &constraint_flags_fops);
-	}
+
+	debugfs_create_u32("uA_load", 0444, regulator->debugfs,
+			   &regulator->uA_load);
+	debugfs_create_u32("min_uV", 0444, regulator->debugfs,
+			   &regulator->voltage[PM_SUSPEND_ON].min_uV);
+	debugfs_create_u32("max_uV", 0444, regulator->debugfs,
+			   &regulator->voltage[PM_SUSPEND_ON].max_uV);
+	debugfs_create_file("constraint_flags", 0444, regulator->debugfs,
+			    regulator, &constraint_flags_fops);
 
 	/*
 	 * Check now if the regulator is an always on regulator - if
@@ -5256,10 +5254,8 @@  static void rdev_init_debugfs(struct regulator_dev *rdev)
 	}
 
 	rdev->debugfs = debugfs_create_dir(rname, debugfs_root);
-	if (IS_ERR(rdev->debugfs)) {
-		rdev_warn(rdev, "Failed to create debugfs directory\n");
-		return;
-	}
+	if (IS_ERR(rdev->debugfs))
+		rdev_dbg(rdev, "Failed to create debugfs directory\n");
 
 	debugfs_create_u32("use_count", 0444, rdev->debugfs,
 			   &rdev->use_count);
@@ -6179,7 +6175,7 @@  static int __init regulator_init(void)
 
 	debugfs_root = debugfs_create_dir("regulator", NULL);
 	if (IS_ERR(debugfs_root))
-		pr_warn("regulator: Failed to create debugfs directory\n");
+		pr_debug("regulator: Failed to create debugfs directory\n");
 
 #ifdef CONFIG_DEBUG_FS
 	debugfs_create_file("supply_map", 0444, debugfs_root, NULL,