USB: typec: fsusb302: no need to check return value of debugfs_create_dir()

Quoted 60 lines · Guenter Roeck

Quoted 59 lines · 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.

Clean up the fsusb302 driver to not care about the dentry being created, or if the root directory was created, as the code should work properly either way. We do not need to save the dentry anymore as things are properly cleaned up when we remove the root directory.

Note, this driver seems to only work for one device per system, otherwise the debugfs directories are going to get confused when a device is removed.

Cc Heikki Krogerus <[email protected]>

Cc Guenter Roeck <[email protected]>

Cc Hans de Goede <[email protected]>

Cc Andy Shevchenko <[email protected]>

Cc Adam Thomson <[email protected]>

Signed-off-by Greg Kroah-Hartman <[email protected]>

---
 drivers/usb/typec/fusb302/fusb302.c | 24 +++++++-----------------
 1 file changed, 7 insertions(+), 17 deletions(-)

drivers/usb/typec/fusb302/fusb302.c +4 -11

index 9c1eba9ea004..07b07ddf6af0 100644
@@ -117,7 +117,6 @@ struct fusb302_chip {
 	u32 snk_pdo[PDO_MAX_OBJECTS];
 
 #ifdef CONFIG_DEBUG_FS
-	struct dentry *dentry;
 	/* lock for log buffer access */
 	struct mutex logbuffer_lock;
 	int logbuffer_head;

@@ -215,33 +214,26 @@ DEFINE_SHOW_ATTRIBUTE(fusb302_debug);
 
 static struct dentry *rootdir;
 
-static int fusb302_debugfs_init(struct fusb302_chip *chip)
+static void fusb302_debugfs_init(struct fusb302_chip *chip)
 {
 	mutex_init(&chip->logbuffer_lock);
-	if (!rootdir) {
+	if (!rootdir)
 		rootdir = debugfs_create_dir("fusb302", NULL);
-		if (!rootdir)
-			return -ENOMEM;
-	}
-
-	chip->dentry = debugfs_create_file(dev_name(chip->dev),
-					   S_IFREG | 0444, rootdir,
-					   chip, &fusb302_debug_fops);
 
-	return 0;
+	debugfs_create_file(dev_name(chip->dev), S_IFREG | 0444, rootdir, chip,
+			    &fusb302_debug_fops);
 }
 

Quoted 12 lines

drivers/usb/typec/fusb302/fusb302.c DEFINE_SHOW_ATTRIBUTE(fusb302_debug); · line 229

Quoted 5 lines

drivers/usb/typec/fusb302/fusb302.c +1 -2

 static void fusb302_debugfs_exit(struct fusb302_chip *chip)
 {
-	debugfs_remove(chip->dentry);
-	debugfs_remove(rootdir);
+	debugfs_remove_recursive(rootdir);

The idea was that debugfs_remove() would only remove the root directory if nothing else was in it. This was the reason for the preceding debugfs_remove(chip->dentry) followed by debugfs_remove(). Your patch changes this behavior. "... this driver seems to only work for one device per system" is introduced by your patch and was not previously the case.

Ah, that makes more sense. Nice hack :)

Why not just have a new directory per device like "most" drivers have? That would make this a bit simpler to manage. Mind of I change the code that way? Hm, you still end up having to save off a dentry that way, ah, it's the same either way...