[PATCH] regmap: no need to check return value of debugfs_create functions

[PATCH] regmap: no need to check return value of debugfs_create functions

[Greg Kroah-Hartman]

When calling debugfs functions, there is no need to ever check the
return value.  The function can work or not, but the code logic should
never do something different based on this.

Cc: Mark Brown <broonie@kernel.org>
Cc: "Rafael J. Wysocki" <rafael@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/base/regmap/regmap-debugfs.c | 12 ------------
 1 file changed, 12 deletions(-)

diff --git a/drivers/base/regmap/regmap-debugfs.c b/drivers/base/regmap/regmap-debugfs.c
index 19eb454f26c3..ce05ae1de630 100644
--- a/drivers/base/regmap/regmap-debugfs.c
+++ b/drivers/base/regmap/regmap-debugfs.c
@@ -572,14 +572,6 @@ void regmap_debugfs_init(struct regmap *map, const char *name)
 	}
 
 	map->debugfs = debugfs_create_dir(name, regmap_debugfs_root);
-	if (!map->debugfs) {
-		dev_warn(map->dev,
-			 "Failed to create %s debugfs directory\n", name);
-
-		kfree(map->debugfs_name);
-		map->debugfs_name = NULL;
-		return;
-	}
 
 	debugfs_create_file("name", 0400, map->debugfs,
 			    map, &regmap_name_fops);
@@ -656,10 +648,6 @@ void regmap_debugfs_initcall(void)
 	struct regmap_debugfs_node *node, *tmp;
 
 	regmap_debugfs_root = debugfs_create_dir("regmap", NULL);
-	if (!regmap_debugfs_root) {
-		pr_warn("regmap: Failed to create debugfs root\n");
-		return;
-	}
 
 	mutex_lock(&regmap_debugfs_early_lock);
 	list_for_each_entry_safe(node, tmp, &regmap_debugfs_early_list, link) {
-- 
2.20.1

Re: [PATCH] regmap: no need to check return value of debugfs_create functions

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 457 bytes --]

On Tue, Jan 22, 2019 at 04:21:06PM +0100, Greg Kroah-Hartman wrote:
> When calling debugfs functions, there is no need to ever check the
> return value.  The function can work or not, but the code logic should
> never do something different based on this.

Given that all the rest of the function is doing is further debugfs
operations and when it fails people trying to use the debugfs do welcome
some diagnostics I'm not sure that's particularly helpful.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] regmap: no need to check return value of debugfs_create functions

[Greg Kroah-Hartman]

On Tue, Jan 22, 2019 at 03:56:06PM +0000, Mark Brown wrote:
> On Tue, Jan 22, 2019 at 04:21:06PM +0100, Greg Kroah-Hartman wrote:
> > When calling debugfs functions, there is no need to ever check the
> > return value.  The function can work or not, but the code logic should
> > never do something different based on this.
> 
> Given that all the rest of the function is doing is further debugfs
> operations and when it fails people trying to use the debugfs do welcome
> some diagnostics I'm not sure that's particularly helpful.

The only way it will fail is if we are out of memory.  And you are in a
bigger mess then, no one cares about debugfs calls, just make them and
move on, you should never care about the result of such a call.

thanks,

greg k-h

Re: [PATCH] regmap: no need to check return value of debugfs_create functions

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 802 bytes --]

On Tue, Jan 22, 2019 at 05:08:57PM +0100, Greg Kroah-Hartman wrote:
> On Tue, Jan 22, 2019 at 03:56:06PM +0000, Mark Brown wrote:

> > Given that all the rest of the function is doing is further debugfs
> > operations and when it fails people trying to use the debugfs do welcome
> > some diagnostics I'm not sure that's particularly helpful.

> The only way it will fail is if we are out of memory.  And you are in a
> bigger mess then, no one cares about debugfs calls, just make them and
> move on, you should never care about the result of such a call.

No, it also fails if there's already something with the same name in
debugfs which can happen as as a result of configuration.  This gets
confusing for users, they see the debugfs files they're expecting but
the contents don't match up at all.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] regmap: no need to check return value of debugfs_create functions

[Greg Kroah-Hartman]

On Tue, Jan 22, 2019 at 04:38:26PM +0000, Mark Brown wrote:
> On Tue, Jan 22, 2019 at 05:08:57PM +0100, Greg Kroah-Hartman wrote:
> > On Tue, Jan 22, 2019 at 03:56:06PM +0000, Mark Brown wrote:
> 
> > > Given that all the rest of the function is doing is further debugfs
> > > operations and when it fails people trying to use the debugfs do welcome
> > > some diagnostics I'm not sure that's particularly helpful.
> 
> > The only way it will fail is if we are out of memory.  And you are in a
> > bigger mess then, no one cares about debugfs calls, just make them and
> > move on, you should never care about the result of such a call.
> 
> No, it also fails if there's already something with the same name in
> debugfs which can happen as as a result of configuration.  This gets
> confusing for users, they see the debugfs files they're expecting but
> the contents don't match up at all.

How can you allow a duplicate name for the other regmap stuff?  Will
that not also cause a collision somewhere else?

Anyway, if this is that big of a problem, ok, but then your code will
run differently if debugfs is enabled or not, which isn't ok.  Don't
rely on debugfs to do your name filtering for you :)

thanks,

greg k-h

Re: [PATCH] regmap: no need to check return value of debugfs_create functions

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1064 bytes --]

On Tue, Jan 22, 2019 at 05:43:52PM +0100, Greg Kroah-Hartman wrote:
> On Tue, Jan 22, 2019 at 04:38:26PM +0000, Mark Brown wrote:

> > No, it also fails if there's already something with the same name in
> > debugfs which can happen as as a result of configuration.  This gets
> > confusing for users, they see the debugfs files they're expecting but
> > the contents don't match up at all.

> How can you allow a duplicate name for the other regmap stuff?  Will
> that not also cause a collision somewhere else?

No, the only place we actually use the names for anything is when
creating the debugfs.  Otherwise the driver using the regmap deals with
a pointer to the regmap which doesn't have any collision risk.

> Anyway, if this is that big of a problem, ok, but then your code will
> run differently if debugfs is enabled or not, which isn't ok.  Don't
> rely on debugfs to do your name filtering for you :)

I'd rather hope that the debugfs creation code runs differently
depending on if debugfs is enabled, perhaps that's just a strange foible
of mine. :)

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]