[PATCH 1/8] platform: x86: acer-wmi: no need to check return value of debugfs_create functions

[PATCH 1/8] platform: x86: acer-wmi: 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.

Also, because there is no need to save the file dentry, remove the
variable that was saving it and just recursively delete the whole
directory.

Cc: "Lee, Chun-Yi" <jlee@suse.com>
Cc: Darren Hart <dvhart@infradead.org>
Cc: Andy Shevchenko <andy@infradead.org>
Cc: platform-driver-x86@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/platform/x86/acer-wmi.c | 29 +++++------------------------
 1 file changed, 5 insertions(+), 24 deletions(-)

diff --git a/drivers/platform/x86/acer-wmi.c b/drivers/platform/x86/acer-wmi.c
index 521b526cd467..f8f0e98b1f0b 100644
--- a/drivers/platform/x86/acer-wmi.c
+++ b/drivers/platform/x86/acer-wmi.c
@@ -259,7 +259,6 @@ struct acer_data {
 
 struct acer_debug {
 	struct dentry *root;
-	struct dentry *devices;
 	u32 wmid_devices;
 };
 
@@ -2148,29 +2147,15 @@ static struct platform_device *acer_platform_device;
 
 static void remove_debugfs(void)
 {
-	debugfs_remove(interface->debug.devices);
-	debugfs_remove(interface->debug.root);
+	debugfs_remove_recursive(interface->debug.root);
 }
 
-static int __init create_debugfs(void)
+static void __init create_debugfs(void)
 {
 	interface->debug.root = debugfs_create_dir("acer-wmi", NULL);
-	if (!interface->debug.root) {
-		pr_err("Failed to create debugfs directory");
-		return -ENOMEM;
-	}
 
-	interface->debug.devices = debugfs_create_u32("devices", S_IRUGO,
-					interface->debug.root,
-					&interface->debug.wmid_devices);
-	if (!interface->debug.devices)
-		goto error_debugfs;
-
-	return 0;
-
-error_debugfs:
-	remove_debugfs();
-	return -ENOMEM;
+	debugfs_create_u32("devices", S_IRUGO, interface->debug.root,
+			   &interface->debug.wmid_devices);
 }
 
 static int __init acer_wmi_init(void)
@@ -2300,9 +2285,7 @@ static int __init acer_wmi_init(void)
 
 	if (wmi_has_guid(WMID_GUID2)) {
 		interface->debug.wmid_devices = get_wmid_devices();
-		err = create_debugfs();
-		if (err)
-			goto error_create_debugfs;
+		create_debugfs();
 	}
 
 	/* Override any initial settings with values from the commandline */
@@ -2310,8 +2293,6 @@ static int __init acer_wmi_init(void)
 
 	return 0;
 
-error_create_debugfs:
-	platform_device_del(acer_platform_device);
 error_device_add:
 	platform_device_put(acer_platform_device);
 error_device_alloc:
-- 
2.22.0

[PATCH 2/8] platform: x86: asus-wmi: 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: Corentin Chary <corentin.chary@gmail.com>
Cc: Darren Hart <dvhart@infradead.org>
Cc: Andy Shevchenko <andy@infradead.org>
Cc: acpi4asus-user@lists.sourceforge.net
Cc: platform-driver-x86@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/platform/x86/asus-wmi.c | 47 ++++++++-------------------------
 1 file changed, 11 insertions(+), 36 deletions(-)

diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
index 3e4336025e8f..4e3786eae349 100644
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -2005,50 +2005,29 @@ static void asus_wmi_debugfs_exit(struct asus_wmi *asus)
 	debugfs_remove_recursive(asus->debug.root);
 }
 
-static int asus_wmi_debugfs_init(struct asus_wmi *asus)
+static void asus_wmi_debugfs_init(struct asus_wmi *asus)
 {
-	struct dentry *dent;
 	int i;
 
 	asus->debug.root = debugfs_create_dir(asus->driver->name, NULL);
-	if (!asus->debug.root) {
-		pr_err("failed to create debugfs directory\n");
-		goto error_debugfs;
-	}
 
-	dent = debugfs_create_x32("method_id", S_IRUGO | S_IWUSR,
-				  asus->debug.root, &asus->debug.method_id);
-	if (!dent)
-		goto error_debugfs;
+	debugfs_create_x32("method_id", S_IRUGO | S_IWUSR, asus->debug.root,
+			   &asus->debug.method_id);
 
-	dent = debugfs_create_x32("dev_id", S_IRUGO | S_IWUSR,
-				  asus->debug.root, &asus->debug.dev_id);
-	if (!dent)
-		goto error_debugfs;
+	debugfs_create_x32("dev_id", S_IRUGO | S_IWUSR, asus->debug.root,
+			   &asus->debug.dev_id);
 
-	dent = debugfs_create_x32("ctrl_param", S_IRUGO | S_IWUSR,
-				  asus->debug.root, &asus->debug.ctrl_param);
-	if (!dent)
-		goto error_debugfs;
+	debugfs_create_x32("ctrl_param", S_IRUGO | S_IWUSR, asus->debug.root,
+			   &asus->debug.ctrl_param);
 
 	for (i = 0; i < ARRAY_SIZE(asus_wmi_debug_files); i++) {
 		struct asus_wmi_debugfs_node *node = &asus_wmi_debug_files[i];
 
 		node->asus = asus;
-		dent = debugfs_create_file(node->name, S_IFREG | S_IRUGO,
-					   asus->debug.root, node,
-					   &asus_wmi_debugfs_io_ops);
-		if (!dent) {
-			pr_err("failed to create debug file: %s\n", node->name);
-			goto error_debugfs;
-		}
+		debugfs_create_file(node->name, S_IFREG | S_IRUGO,
+				    asus->debug.root, node,
+				    &asus_wmi_debugfs_io_ops);
 	}
-
-	return 0;
-
-error_debugfs:
-	asus_wmi_debugfs_exit(asus);
-	return -ENOMEM;
 }
 
 static int asus_wmi_fan_init(struct asus_wmi *asus)
@@ -2162,14 +2141,10 @@ static int asus_wmi_add(struct platform_device *pdev)
 		goto fail_wmi_handler;
 	}
 
-	err = asus_wmi_debugfs_init(asus);
-	if (err)
-		goto fail_debugfs;
+	asus_wmi_debugfs_init(asus);
 
 	return 0;
 
-fail_debugfs:
-	wmi_remove_notify_handler(asus->driver->event_guid);
 fail_wmi_handler:
 	asus_wmi_backlight_exit(asus);
 fail_backlight:
-- 
2.22.0

[PATCH 3/8] platform: x86: dell-laptop: 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: Matthew Garrett <mjg59@srcf.ucam.org>
Cc: "Pali Rohár" <pali.rohar@gmail.com>
Cc: Darren Hart <dvhart@infradead.org>
Cc: Andy Shevchenko <andy@infradead.org>
Cc: platform-driver-x86@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/platform/x86/dell-laptop.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/platform/x86/dell-laptop.c b/drivers/platform/x86/dell-laptop.c
index a561f653cf13..94a2f259031c 100644
--- a/drivers/platform/x86/dell-laptop.c
+++ b/drivers/platform/x86/dell-laptop.c
@@ -2176,9 +2176,8 @@ static int __init dell_init(void)
 	kbd_led_init(&platform_device->dev);
 
 	dell_laptop_dir = debugfs_create_dir("dell_laptop", NULL);
-	if (dell_laptop_dir != NULL)
-		debugfs_create_file("rfkill", 0444, dell_laptop_dir, NULL,
-				    &dell_debugfs_fops);
+	debugfs_create_file("rfkill", 0444, dell_laptop_dir, NULL,
+			    &dell_debugfs_fops);
 
 	dell_laptop_register_notifier(&dell_laptop_notifier);
 
-- 
2.22.0

[PATCH 4/8] platform: x86: ideapad-laptop: 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: Ike Panhc <ike.pan@canonical.com>
Cc: Darren Hart <dvhart@infradead.org>
Cc: Andy Shevchenko <andy@infradead.org>
Cc: platform-driver-x86@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/platform/x86/ideapad-laptop.c | 36 ++++++---------------------
 1 file changed, 7 insertions(+), 29 deletions(-)

diff --git a/drivers/platform/x86/ideapad-laptop.c b/drivers/platform/x86/ideapad-laptop.c
index 5fb9bfdf1019..7598cd46cf60 100644
--- a/drivers/platform/x86/ideapad-laptop.c
+++ b/drivers/platform/x86/ideapad-laptop.c
@@ -316,34 +316,15 @@ static int debugfs_cfg_show(struct seq_file *s, void *data)
 }
 DEFINE_SHOW_ATTRIBUTE(debugfs_cfg);
 
-static int ideapad_debugfs_init(struct ideapad_private *priv)
+static void ideapad_debugfs_init(struct ideapad_private *priv)
 {
-	struct dentry *node;
+	struct dentry *dir;
 
-	priv->debug = debugfs_create_dir("ideapad", NULL);
-	if (priv->debug == NULL) {
-		pr_err("failed to create debugfs directory");
-		goto errout;
-	}
-
-	node = debugfs_create_file("cfg", S_IRUGO, priv->debug, priv,
-				   &debugfs_cfg_fops);
-	if (!node) {
-		pr_err("failed to create cfg in debugfs");
-		goto errout;
-	}
-
-	node = debugfs_create_file("status", S_IRUGO, priv->debug, priv,
-				   &debugfs_status_fops);
-	if (!node) {
-		pr_err("failed to create status in debugfs");
-		goto errout;
-	}
-
-	return 0;
+	dir = debugfs_create_dir("ideapad", NULL);
+	priv->debug = dir;
 
-errout:
-	return -ENOMEM;
+	debugfs_create_file("cfg", S_IRUGO, dir, priv, &debugfs_cfg_fops);
+	debugfs_create_file("status", S_IRUGO, dir, priv, &debugfs_status_fops);
 }
 
 static void ideapad_debugfs_exit(struct ideapad_private *priv)
@@ -1012,9 +993,7 @@ static int ideapad_acpi_add(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	ret = ideapad_debugfs_init(priv);
-	if (ret)
-		goto debugfs_failed;
+	ideapad_debugfs_init(priv);
 
 	ret = ideapad_input_init(priv);
 	if (ret)
@@ -1071,7 +1050,6 @@ static int ideapad_acpi_add(struct platform_device *pdev)
 	ideapad_input_exit(priv);
 input_failed:
 	ideapad_debugfs_exit(priv);
-debugfs_failed:
 	ideapad_sysfs_exit(priv);
 	return ret;
 }
-- 
2.22.0

[PATCH 5/8] platform: x86: samsung-laptop: 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: Corentin Chary <corentin.chary@gmail.com>
Cc: Darren Hart <dvhart@infradead.org>
Cc: Andy Shevchenko <andy@infradead.org>
Cc: platform-driver-x86@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/platform/x86/samsung-laptop.c | 89 +++++++--------------------
 1 file changed, 23 insertions(+), 66 deletions(-)

diff --git a/drivers/platform/x86/samsung-laptop.c b/drivers/platform/x86/samsung-laptop.c
index 7b160ee98115..e84f11398c1b 100644
--- a/drivers/platform/x86/samsung-laptop.c
+++ b/drivers/platform/x86/samsung-laptop.c
@@ -1280,15 +1280,12 @@ static void samsung_debugfs_exit(struct samsung_laptop *samsung)
 	debugfs_remove_recursive(samsung->debug.root);
 }
 
-static int samsung_debugfs_init(struct samsung_laptop *samsung)
+static void samsung_debugfs_init(struct samsung_laptop *samsung)
 {
-	struct dentry *dent;
+	struct dentry *root;
 
-	samsung->debug.root = debugfs_create_dir("samsung-laptop", NULL);
-	if (!samsung->debug.root) {
-		pr_err("failed to create debugfs directory");
-		goto error_debugfs;
-	}
+	root = debugfs_create_dir("samsung-laptop", NULL);
+	samsung->debug.root = root;
 
 	samsung->debug.f0000_wrapper.data = samsung->f0000_segment;
 	samsung->debug.f0000_wrapper.size = 0xffff;
@@ -1299,60 +1296,24 @@ static int samsung_debugfs_init(struct samsung_laptop *samsung)
 	samsung->debug.sdiag_wrapper.data = samsung->sdiag;
 	samsung->debug.sdiag_wrapper.size = strlen(samsung->sdiag);
 
-	dent = debugfs_create_u16("command", S_IRUGO | S_IWUSR,
-				  samsung->debug.root, &samsung->debug.command);
-	if (!dent)
-		goto error_debugfs;
-
-	dent = debugfs_create_u32("d0", S_IRUGO | S_IWUSR, samsung->debug.root,
-				  &samsung->debug.data.d0);
-	if (!dent)
-		goto error_debugfs;
-
-	dent = debugfs_create_u32("d1", S_IRUGO | S_IWUSR, samsung->debug.root,
-				  &samsung->debug.data.d1);
-	if (!dent)
-		goto error_debugfs;
-
-	dent = debugfs_create_u16("d2", S_IRUGO | S_IWUSR, samsung->debug.root,
-				  &samsung->debug.data.d2);
-	if (!dent)
-		goto error_debugfs;
-
-	dent = debugfs_create_u8("d3", S_IRUGO | S_IWUSR, samsung->debug.root,
-				 &samsung->debug.data.d3);
-	if (!dent)
-		goto error_debugfs;
-
-	dent = debugfs_create_blob("data", S_IRUGO | S_IWUSR,
-				   samsung->debug.root,
-				   &samsung->debug.data_wrapper);
-	if (!dent)
-		goto error_debugfs;
-
-	dent = debugfs_create_blob("f0000_segment", S_IRUSR | S_IWUSR,
-				   samsung->debug.root,
-				   &samsung->debug.f0000_wrapper);
-	if (!dent)
-		goto error_debugfs;
-
-	dent = debugfs_create_file("call", S_IFREG | S_IRUGO,
-				   samsung->debug.root, samsung,
-				   &samsung_laptop_call_fops);
-	if (!dent)
-		goto error_debugfs;
-
-	dent = debugfs_create_blob("sdiag", S_IRUGO | S_IWUSR,
-				   samsung->debug.root,
-				   &samsung->debug.sdiag_wrapper);
-	if (!dent)
-		goto error_debugfs;
-
-	return 0;
-
-error_debugfs:
-	samsung_debugfs_exit(samsung);
-	return -ENOMEM;
+	debugfs_create_u16("command", S_IRUGO | S_IWUSR, root,
+			   &samsung->debug.command);
+	debugfs_create_u32("d0", S_IRUGO | S_IWUSR, root,
+			   &samsung->debug.data.d0);
+	debugfs_create_u32("d1", S_IRUGO | S_IWUSR, root,
+			   &samsung->debug.data.d1);
+	debugfs_create_u16("d2", S_IRUGO | S_IWUSR, root,
+			   &samsung->debug.data.d2);
+	debugfs_create_u8("d3", S_IRUGO | S_IWUSR, root,
+			  &samsung->debug.data.d3);
+	debugfs_create_blob("data", S_IRUGO | S_IWUSR, root,
+			    &samsung->debug.data_wrapper);
+	debugfs_create_blob("f0000_segment", S_IRUSR | S_IWUSR, root,
+			    &samsung->debug.f0000_wrapper);
+	debugfs_create_file("call", S_IFREG | S_IRUGO, root, samsung,
+			    &samsung_laptop_call_fops);
+	debugfs_create_blob("sdiag", S_IRUGO | S_IWUSR, root,
+			    &samsung->debug.sdiag_wrapper);
 }
 
 static void samsung_sabi_exit(struct samsung_laptop *samsung)
@@ -1745,9 +1706,7 @@ static int __init samsung_init(void)
 	if (ret)
 		goto error_lid_handling;
 
-	ret = samsung_debugfs_init(samsung);
-	if (ret)
-		goto error_debugfs;
+	samsung_debugfs_init(samsung);
 
 	samsung->pm_nb.notifier_call = samsung_pm_notification;
 	register_pm_notifier(&samsung->pm_nb);
@@ -1755,8 +1714,6 @@ static int __init samsung_init(void)
 	samsung_platform_device = samsung->platform_device;
 	return ret;
 
-error_debugfs:
-	samsung_lid_handling_exit(samsung);
 error_lid_handling:
 	samsung_leds_exit(samsung);
 error_leds:
-- 
2.22.0

[PATCH 6/8] platform: x86: pmc_atom: 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: Darren Hart <dvhart@infradead.org>
Cc: Andy Shevchenko <andy@infradead.org>
Cc: platform-driver-x86@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/platform/x86/pmc_atom.c | 43 ++++++++-------------------------
 1 file changed, 10 insertions(+), 33 deletions(-)

diff --git a/drivers/platform/x86/pmc_atom.c b/drivers/platform/x86/pmc_atom.c
index b1d804376237..2104e1ad38d9 100644
--- a/drivers/platform/x86/pmc_atom.c
+++ b/drivers/platform/x86/pmc_atom.c
@@ -350,45 +350,24 @@ static int pmc_sleep_tmr_show(struct seq_file *s, void *unused)
 
 DEFINE_SHOW_ATTRIBUTE(pmc_sleep_tmr);
 
-static void pmc_dbgfs_unregister(struct pmc_dev *pmc)
+static void pmc_dbgfs_register(struct pmc_dev *pmc)
 {
-	debugfs_remove_recursive(pmc->dbgfs_dir);
-}
-
-static int pmc_dbgfs_register(struct pmc_dev *pmc)
-{
-	struct dentry *dir, *f;
+	struct dentry *dir;
 
 	dir = debugfs_create_dir("pmc_atom", NULL);
-	if (!dir)
-		return -ENOMEM;
 
 	pmc->dbgfs_dir = dir;
 
-	f = debugfs_create_file("dev_state", S_IFREG | S_IRUGO,
-				dir, pmc, &pmc_dev_state_fops);
-	if (!f)
-		goto err;
-
-	f = debugfs_create_file("pss_state", S_IFREG | S_IRUGO,
-				dir, pmc, &pmc_pss_state_fops);
-	if (!f)
-		goto err;
-
-	f = debugfs_create_file("sleep_state", S_IFREG | S_IRUGO,
-				dir, pmc, &pmc_sleep_tmr_fops);
-	if (!f)
-		goto err;
-
-	return 0;
-err:
-	pmc_dbgfs_unregister(pmc);
-	return -ENODEV;
+	debugfs_create_file("dev_state", S_IFREG | S_IRUGO, dir, pmc,
+			    &pmc_dev_state_fops);
+	debugfs_create_file("pss_state", S_IFREG | S_IRUGO, dir, pmc,
+			    &pmc_pss_state_fops);
+	debugfs_create_file("sleep_state", S_IFREG | S_IRUGO, dir, pmc,
+			    &pmc_sleep_tmr_fops);
 }
 #else
-static int pmc_dbgfs_register(struct pmc_dev *pmc)
+static void pmc_dbgfs_register(struct pmc_dev *pmc)
 {
-	return 0;
 }
 #endif /* CONFIG_DEBUG_FS */
 
@@ -500,9 +479,7 @@ static int pmc_setup_dev(struct pci_dev *pdev, const struct pci_device_id *ent)
 	/* PMC hardware registers setup */
 	pmc_hw_reg_setup(pmc);
 
-	ret = pmc_dbgfs_register(pmc);
-	if (ret)
-		dev_warn(&pdev->dev, "debugfs register failed\n");
+	pmc_dbgfs_register(pmc);
 
 	/* Register platform clocks - PMC_PLT_CLK [0..5] */
 	ret = pmc_setup_clks(pdev, pmc->regmap, data);
-- 
2.22.0

[PATCH 7/8] platform: x86: intel_pmc: 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: Rajneesh Bhardwaj <rajneesh.bhardwaj@intel.com>
Cc: Vishwanath Somayaji <vishwanath.somayaji@intel.com>
Cc: Darren Hart <dvhart@infradead.org>
Cc: Andy Shevchenko <andy@infradead.org>
Cc: platform-driver-x86@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/platform/x86/intel_pmc_core.c | 18 +++---------------
 1 file changed, 3 insertions(+), 15 deletions(-)

diff --git a/drivers/platform/x86/intel_pmc_core.c b/drivers/platform/x86/intel_pmc_core.c
index 1d902230ba61..27d6470e43ec 100644
--- a/drivers/platform/x86/intel_pmc_core.c
+++ b/drivers/platform/x86/intel_pmc_core.c
@@ -753,14 +753,11 @@ static void pmc_core_dbgfs_unregister(struct pmc_dev *pmcdev)
 	debugfs_remove_recursive(pmcdev->dbgfs_dir);
 }
 
-static int pmc_core_dbgfs_register(struct pmc_dev *pmcdev)
+static void pmc_core_dbgfs_register(struct pmc_dev *pmcdev)
 {
 	struct dentry *dir;
 
 	dir = debugfs_create_dir("pmc_core", NULL);
-	if (!dir)
-		return -ENOMEM;
-
 	pmcdev->dbgfs_dir = dir;
 
 	debugfs_create_file("slp_s0_residency_usec", 0444, dir, pmcdev,
@@ -794,13 +791,10 @@ static int pmc_core_dbgfs_register(struct pmc_dev *pmcdev)
 		debugfs_create_bool("slp_s0_dbg_latch", 0644,
 				    dir, &slps0_dbg_latch);
 	}
-
-	return 0;
 }
 #else
-static inline int pmc_core_dbgfs_register(struct pmc_dev *pmcdev)
+static inline void pmc_core_dbgfs_register(struct pmc_dev *pmcdev)
 {
-	return 0;
 }
 
 static inline void pmc_core_dbgfs_unregister(struct pmc_dev *pmcdev)
@@ -862,7 +856,6 @@ static int pmc_core_probe(struct platform_device *pdev)
 	struct pmc_dev *pmcdev = &pmc;
 	const struct x86_cpu_id *cpu_id;
 	u64 slp_s0_addr;
-	int err;
 
 	if (device_initialized)
 		return -ENODEV;
@@ -896,12 +889,7 @@ static int pmc_core_probe(struct platform_device *pdev)
 	pmcdev->pmc_xram_read_bit = pmc_core_check_read_lock_bit();
 	dmi_check_system(pmc_core_dmi_table);
 
-	err = pmc_core_dbgfs_register(pmcdev);
-	if (err < 0) {
-		dev_warn(&pdev->dev, "debugfs register failed.\n");
-		iounmap(pmcdev->regbase);
-		return err;
-	}
+	pmc_core_dbgfs_register(pmcdev);
 
 	device_initialized = true;
 	dev_info(&pdev->dev, " initialized\n");
-- 
2.22.0

[PATCH 8/8] platform: x86: intel_telemetry: 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: Rajneesh Bhardwaj <rajneesh.bhardwaj@linux.intel.com>
Cc: "David E. Box" <david.e.box@linux.intel.com>
Cc: Darren Hart <dvhart@infradead.org>
Cc: Andy Shevchenko <andy@infradead.org>
Cc: platform-driver-x86@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 .../platform/x86/intel_telemetry_debugfs.c    | 78 ++++---------------
 1 file changed, 16 insertions(+), 62 deletions(-)

diff --git a/drivers/platform/x86/intel_telemetry_debugfs.c b/drivers/platform/x86/intel_telemetry_debugfs.c
index 98ba9185a27b..e84d3e983e0c 100644
--- a/drivers/platform/x86/intel_telemetry_debugfs.c
+++ b/drivers/platform/x86/intel_telemetry_debugfs.c
@@ -900,7 +900,7 @@ static int __init telemetry_debugfs_init(void)
 {
 	const struct x86_cpu_id *id;
 	int err;
-	struct dentry *f;
+	struct dentry *dir;
 
 	/* Only APL supported for now */
 	id = x86_match_cpu(telemetry_debugfs_cpu_ids);
@@ -923,68 +923,22 @@ static int __init telemetry_debugfs_init(void)
 
 	register_pm_notifier(&pm_notifier);
 
-	err = -ENOMEM;
-	debugfs_conf->telemetry_dbg_dir = debugfs_create_dir("telemetry", NULL);
-	if (!debugfs_conf->telemetry_dbg_dir)
-		goto out_pm;
-
-	f = debugfs_create_file("pss_info", S_IFREG | S_IRUGO,
-				debugfs_conf->telemetry_dbg_dir, NULL,
-				&telem_pss_states_fops);
-	if (!f) {
-		pr_err("pss_sample_info debugfs register failed\n");
-		goto out;
-	}
-
-	f = debugfs_create_file("ioss_info", S_IFREG | S_IRUGO,
-				debugfs_conf->telemetry_dbg_dir, NULL,
-				&telem_ioss_states_fops);
-	if (!f) {
-		pr_err("ioss_sample_info debugfs register failed\n");
-		goto out;
-	}
-
-	f = debugfs_create_file("soc_states", S_IFREG | S_IRUGO,
-				debugfs_conf->telemetry_dbg_dir,
-				NULL, &telem_soc_states_fops);
-	if (!f) {
-		pr_err("ioss_sample_info debugfs register failed\n");
-		goto out;
-	}
-
-	f = debugfs_create_file("s0ix_residency_usec", S_IFREG | S_IRUGO,
-				debugfs_conf->telemetry_dbg_dir,
-				NULL, &telem_s0ix_fops);
-	if (!f) {
-		pr_err("s0ix_residency_usec debugfs register failed\n");
-		goto out;
-	}
-
-	f = debugfs_create_file("pss_trace_verbosity", S_IFREG | S_IRUGO,
-				debugfs_conf->telemetry_dbg_dir, NULL,
-				&telem_pss_trc_verb_ops);
-	if (!f) {
-		pr_err("pss_trace_verbosity debugfs register failed\n");
-		goto out;
-	}
-
-	f = debugfs_create_file("ioss_trace_verbosity", S_IFREG | S_IRUGO,
-				debugfs_conf->telemetry_dbg_dir, NULL,
-				&telem_ioss_trc_verb_ops);
-	if (!f) {
-		pr_err("ioss_trace_verbosity debugfs register failed\n");
-		goto out;
-	}
-
+	dir = debugfs_create_dir("telemetry", NULL);
+	debugfs_conf->telemetry_dbg_dir = dir;
+
+	debugfs_create_file("pss_info", S_IFREG | S_IRUGO, dir, NULL,
+			    &telem_pss_states_fops);
+	debugfs_create_file("ioss_info", S_IFREG | S_IRUGO, dir, NULL,
+			    &telem_ioss_states_fops);
+	debugfs_create_file("soc_states", S_IFREG | S_IRUGO, dir, NULL,
+			    &telem_soc_states_fops);
+	debugfs_create_file("s0ix_residency_usec", S_IFREG | S_IRUGO, dir, NULL,
+			    &telem_s0ix_fops);
+	debugfs_create_file("pss_trace_verbosity", S_IFREG | S_IRUGO, dir, NULL,
+			    &telem_pss_trc_verb_ops);
+	debugfs_create_file("ioss_trace_verbosity", S_IFREG | S_IRUGO, dir,
+			    NULL, &telem_ioss_trc_verb_ops);
 	return 0;
-
-out:
-	debugfs_remove_recursive(debugfs_conf->telemetry_dbg_dir);
-	debugfs_conf->telemetry_dbg_dir = NULL;
-out_pm:
-	unregister_pm_notifier(&pm_notifier);
-
-	return err;
 }
 
 static void __exit telemetry_debugfs_exit(void)
-- 
2.22.0

Re: [PATCH 3/8] platform: x86: dell-laptop: no need to check return value of debugfs_create functions

[Pali Rohár]

On Wednesday 12 June 2019 14:12:53 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.
> 
> Cc: Matthew Garrett <mjg59@srcf.ucam.org>
> Cc: "Pali Rohár" <pali.rohar@gmail.com>
> Cc: Darren Hart <dvhart@infradead.org>
> Cc: Andy Shevchenko <andy@infradead.org>
> Cc: platform-driver-x86@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
>  drivers/platform/x86/dell-laptop.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/platform/x86/dell-laptop.c b/drivers/platform/x86/dell-laptop.c
> index a561f653cf13..94a2f259031c 100644
> --- a/drivers/platform/x86/dell-laptop.c
> +++ b/drivers/platform/x86/dell-laptop.c
> @@ -2176,9 +2176,8 @@ static int __init dell_init(void)
>  	kbd_led_init(&platform_device->dev);
>  
>  	dell_laptop_dir = debugfs_create_dir("dell_laptop", NULL);
> -	if (dell_laptop_dir != NULL)
> -		debugfs_create_file("rfkill", 0444, dell_laptop_dir, NULL,
> -				    &dell_debugfs_fops);
> +	debugfs_create_file("rfkill", 0444, dell_laptop_dir, NULL,
> +			    &dell_debugfs_fops);

Hi!

So... debugfs_create_dir() can return NULL, right?

And it is then OK to call
debugfs_create_file("rfkill", 0444, dell_laptop_dir, ...) with
dell_laptop_dir = NULL?

Where would be that "rfkill" file created?

>  
>  	dell_laptop_register_notifier(&dell_laptop_notifier);
>  

-- 
Pali Rohár
pali.rohar@gmail.com

Re: [PATCH 3/8] platform: x86: dell-laptop: no need to check return value of debugfs_create functions

[Greg Kroah-Hartman]

On Wed, Jun 12, 2019 at 02:21:05PM +0200, Pali Rohár wrote:
> On Wednesday 12 June 2019 14:12:53 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.
> > 
> > Cc: Matthew Garrett <mjg59@srcf.ucam.org>
> > Cc: "Pali Rohár" <pali.rohar@gmail.com>
> > Cc: Darren Hart <dvhart@infradead.org>
> > Cc: Andy Shevchenko <andy@infradead.org>
> > Cc: platform-driver-x86@vger.kernel.org
> > Cc: linux-kernel@vger.kernel.org
> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > ---
> >  drivers/platform/x86/dell-laptop.c | 5 ++---
> >  1 file changed, 2 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/platform/x86/dell-laptop.c b/drivers/platform/x86/dell-laptop.c
> > index a561f653cf13..94a2f259031c 100644
> > --- a/drivers/platform/x86/dell-laptop.c
> > +++ b/drivers/platform/x86/dell-laptop.c
> > @@ -2176,9 +2176,8 @@ static int __init dell_init(void)
> >  	kbd_led_init(&platform_device->dev);
> >  
> >  	dell_laptop_dir = debugfs_create_dir("dell_laptop", NULL);
> > -	if (dell_laptop_dir != NULL)
> > -		debugfs_create_file("rfkill", 0444, dell_laptop_dir, NULL,
> > -				    &dell_debugfs_fops);
> > +	debugfs_create_file("rfkill", 0444, dell_laptop_dir, NULL,
> > +			    &dell_debugfs_fops);
> 
> Hi!
> 
> So... debugfs_create_dir() can return NULL, right?

Nope.

> And it is then OK to call
> debugfs_create_file("rfkill", 0444, dell_laptop_dir, ...) with
> dell_laptop_dir = NULL?

Yes.

> Where would be that "rfkill" file created?

The root of debugfs.

But, if debugfs_create_dir() return an error, and you pass that value
into debugfs_create_file() it will happily just return an error back
again, and move on.

So it is always safe to pass the return value of one debugfs call into
another, no need to check anything.  If the system is so messed up that
debugfs_create_dir() fails (i.e. you are out of memory), failing to
create a debugfs file is the least of your worries :)

And even then, no need to change your code logic, the functionality of
your code should never depend on if debugfs is working properly at the
moment or not.

thanks,

greg k-h

Re: [PATCH 3/8] platform: x86: dell-laptop: no need to check return value of debugfs_create functions

[Pali Rohár]

On Wednesday 12 June 2019 14:36:04 Greg Kroah-Hartman wrote:
> On Wed, Jun 12, 2019 at 02:21:05PM +0200, Pali Rohár wrote:
> > On Wednesday 12 June 2019 14:12:53 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.
> > > 
> > > Cc: Matthew Garrett <mjg59@srcf.ucam.org>
> > > Cc: "Pali Rohár" <pali.rohar@gmail.com>
> > > Cc: Darren Hart <dvhart@infradead.org>
> > > Cc: Andy Shevchenko <andy@infradead.org>
> > > Cc: platform-driver-x86@vger.kernel.org
> > > Cc: linux-kernel@vger.kernel.org
> > > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > > ---
> > >  drivers/platform/x86/dell-laptop.c | 5 ++---
> > >  1 file changed, 2 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/drivers/platform/x86/dell-laptop.c b/drivers/platform/x86/dell-laptop.c
> > > index a561f653cf13..94a2f259031c 100644
> > > --- a/drivers/platform/x86/dell-laptop.c
> > > +++ b/drivers/platform/x86/dell-laptop.c
> > > @@ -2176,9 +2176,8 @@ static int __init dell_init(void)
> > >  	kbd_led_init(&platform_device->dev);
> > >  
> > >  	dell_laptop_dir = debugfs_create_dir("dell_laptop", NULL);
> > > -	if (dell_laptop_dir != NULL)
> > > -		debugfs_create_file("rfkill", 0444, dell_laptop_dir, NULL,
> > > -				    &dell_debugfs_fops);
> > > +	debugfs_create_file("rfkill", 0444, dell_laptop_dir, NULL,
> > > +			    &dell_debugfs_fops);
> > 
> > Hi!
> > 
> > So... debugfs_create_dir() can return NULL, right?
> 
> Nope.

Yea, now I see implementation. It does not return NULL on error, but
rather ERR_PTR.

So dell_laptop_dir is always not-NULL. And that check was wrong.

You can add my
Reviewed-by: Pali Rohár <pali.rohar@gmail.com>

> > And it is then OK to call
> > debugfs_create_file("rfkill", 0444, dell_laptop_dir, ...) with
> > dell_laptop_dir = NULL?
> 
> Yes.
> 
> > Where would be that "rfkill" file created?
> 
> The root of debugfs.
> 
> But, if debugfs_create_dir() return an error, and you pass that value
> into debugfs_create_file() it will happily just return an error back
> again, and move on.
> 
> So it is always safe to pass the return value of one debugfs call into
> another, no need to check anything.  If the system is so messed up that
> debugfs_create_dir() fails (i.e. you are out of memory), failing to
> create a debugfs file is the least of your worries :)
> 
> And even then, no need to change your code logic, the functionality of
> your code should never depend on if debugfs is working properly at the
> moment or not.
> 
> thanks,
> 
> greg k-h

-- 
Pali Rohár
pali.rohar@gmail.com

Re: [PATCH 3/8] platform: x86: dell-laptop: no need to check return value of debugfs_create functions

[Greg Kroah-Hartman]

On Wed, Jun 12, 2019 at 02:44:11PM +0200, Pali Rohár wrote:
> On Wednesday 12 June 2019 14:36:04 Greg Kroah-Hartman wrote:
> > On Wed, Jun 12, 2019 at 02:21:05PM +0200, Pali Rohár wrote:
> > > On Wednesday 12 June 2019 14:12:53 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.
> > > > 
> > > > Cc: Matthew Garrett <mjg59@srcf.ucam.org>
> > > > Cc: "Pali Rohár" <pali.rohar@gmail.com>
> > > > Cc: Darren Hart <dvhart@infradead.org>
> > > > Cc: Andy Shevchenko <andy@infradead.org>
> > > > Cc: platform-driver-x86@vger.kernel.org
> > > > Cc: linux-kernel@vger.kernel.org
> > > > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > > > ---
> > > >  drivers/platform/x86/dell-laptop.c | 5 ++---
> > > >  1 file changed, 2 insertions(+), 3 deletions(-)
> > > > 
> > > > diff --git a/drivers/platform/x86/dell-laptop.c b/drivers/platform/x86/dell-laptop.c
> > > > index a561f653cf13..94a2f259031c 100644
> > > > --- a/drivers/platform/x86/dell-laptop.c
> > > > +++ b/drivers/platform/x86/dell-laptop.c
> > > > @@ -2176,9 +2176,8 @@ static int __init dell_init(void)
> > > >  	kbd_led_init(&platform_device->dev);
> > > >  
> > > >  	dell_laptop_dir = debugfs_create_dir("dell_laptop", NULL);
> > > > -	if (dell_laptop_dir != NULL)
> > > > -		debugfs_create_file("rfkill", 0444, dell_laptop_dir, NULL,
> > > > -				    &dell_debugfs_fops);
> > > > +	debugfs_create_file("rfkill", 0444, dell_laptop_dir, NULL,
> > > > +			    &dell_debugfs_fops);
> > > 
> > > Hi!
> > > 
> > > So... debugfs_create_dir() can return NULL, right?
> > 
> > Nope.
> 
> Yea, now I see implementation. It does not return NULL on error, but
> rather ERR_PTR.
> 
> So dell_laptop_dir is always not-NULL. And that check was wrong.
> 
> You can add my
> Reviewed-by: Pali Rohár <pali.rohar@gmail.com>

Thanks!

greg k-h

Re: [PATCH 1/8] platform: x86: acer-wmi: no need to check return value of debugfs_create functions

[Andy Shevchenko]

On Wed, Jun 12, 2019 at 3:13 PM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> 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.
>
> Also, because there is no need to save the file dentry, remove the
> variable that was saving it and just recursively delete the whole
> directory.
>

Through which tree you want to proceed this?

> Cc: "Lee, Chun-Yi" <jlee@suse.com>
> Cc: Darren Hart <dvhart@infradead.org>
> Cc: Andy Shevchenko <andy@infradead.org>
> Cc: platform-driver-x86@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
>  drivers/platform/x86/acer-wmi.c | 29 +++++------------------------
>  1 file changed, 5 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/platform/x86/acer-wmi.c b/drivers/platform/x86/acer-wmi.c
> index 521b526cd467..f8f0e98b1f0b 100644
> --- a/drivers/platform/x86/acer-wmi.c
> +++ b/drivers/platform/x86/acer-wmi.c
> @@ -259,7 +259,6 @@ struct acer_data {
>
>  struct acer_debug {
>         struct dentry *root;
> -       struct dentry *devices;
>         u32 wmid_devices;
>  };
>
> @@ -2148,29 +2147,15 @@ static struct platform_device *acer_platform_device;
>
>  static void remove_debugfs(void)
>  {
> -       debugfs_remove(interface->debug.devices);
> -       debugfs_remove(interface->debug.root);
> +       debugfs_remove_recursive(interface->debug.root);
>  }
>
> -static int __init create_debugfs(void)
> +static void __init create_debugfs(void)
>  {
>         interface->debug.root = debugfs_create_dir("acer-wmi", NULL);
> -       if (!interface->debug.root) {
> -               pr_err("Failed to create debugfs directory");
> -               return -ENOMEM;
> -       }
>
> -       interface->debug.devices = debugfs_create_u32("devices", S_IRUGO,
> -                                       interface->debug.root,
> -                                       &interface->debug.wmid_devices);
> -       if (!interface->debug.devices)
> -               goto error_debugfs;
> -
> -       return 0;
> -
> -error_debugfs:
> -       remove_debugfs();
> -       return -ENOMEM;
> +       debugfs_create_u32("devices", S_IRUGO, interface->debug.root,
> +                          &interface->debug.wmid_devices);
>  }
>
>  static int __init acer_wmi_init(void)
> @@ -2300,9 +2285,7 @@ static int __init acer_wmi_init(void)
>
>         if (wmi_has_guid(WMID_GUID2)) {
>                 interface->debug.wmid_devices = get_wmid_devices();
> -               err = create_debugfs();
> -               if (err)
> -                       goto error_create_debugfs;
> +               create_debugfs();
>         }
>
>         /* Override any initial settings with values from the commandline */
> @@ -2310,8 +2293,6 @@ static int __init acer_wmi_init(void)
>
>         return 0;
>
> -error_create_debugfs:
> -       platform_device_del(acer_platform_device);
>  error_device_add:
>         platform_device_put(acer_platform_device);
>  error_device_alloc:
> --
> 2.22.0
>


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 1/8] platform: x86: acer-wmi: no need to check return value of debugfs_create functions

[Greg Kroah-Hartman]

On Fri, Jun 14, 2019 at 09:48:04AM +0300, Andy Shevchenko wrote:
> On Wed, Jun 12, 2019 at 3:13 PM Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> 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.
> >
> > Also, because there is no need to save the file dentry, remove the
> > variable that was saving it and just recursively delete the whole
> > directory.
> >
> 
> Through which tree you want to proceed this?

What ever is easier for you, I can take it through mine, as I have a lot
of other patches like this queued up already, or it can go through
yours.

thanks,

greg k-h

Re: [PATCH 1/8] platform: x86: acer-wmi: no need to check return value of debugfs_create functions

[Andy Shevchenko]

On Fri, Jun 14, 2019 at 9:53 AM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Fri, Jun 14, 2019 at 09:48:04AM +0300, Andy Shevchenko wrote:
> > On Wed, Jun 12, 2019 at 3:13 PM Greg Kroah-Hartman
> > <gregkh@linuxfoundation.org> 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.
> > >
> > > Also, because there is no need to save the file dentry, remove the
> > > variable that was saving it and just recursively delete the whole
> > > directory.
> > >
> >
> > Through which tree you want to proceed this?
>
> What ever is easier for you, I can take it through mine, as I have a lot
> of other patches like this queued up already, or it can go through
> yours.

All 8 pushed to my review and testing queue, thanks!

-- 
With Best Regards,
Andy Shevchenko