* [PATCH 01/17] wimax: no need to check return value of debugfs_create functions
2019-08-06 16:11 [PATCH 00/17] Networking driver debugfs cleanups Greg Kroah-Hartman
@ 2019-08-06 16:11 ` Greg Kroah-Hartman
2019-08-06 16:11 ` [PATCH 02/17] bonding: no need to print a message if debugfs_create_dir() fails Greg Kroah-Hartman
` (16 subsequent siblings)
17 siblings, 0 replies; 32+ messages in thread
From: Greg Kroah-Hartman @ 2019-08-06 16:11 UTC (permalink / raw)
To: netdev; +Cc: Greg Kroah-Hartman, Inaky Perez-Gonzalez, linux-wimax
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.
This cleans up a lot of unneeded code and logic around the debugfs wimax
files, making all of this much simpler and easier to understand.
Cc: Inaky Perez-Gonzalez <inaky.perez-gonzalez@intel.com>
Cc: linux-wimax@intel.com
Cc: netdev@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
drivers/net/wimax/i2400m/debugfs.c | 149 +++++------------------------
drivers/net/wimax/i2400m/driver.c | 7 +-
drivers/net/wimax/i2400m/i2400m.h | 7 +-
drivers/net/wimax/i2400m/usb.c | 61 +++---------
include/linux/wimax/debug.h | 20 +---
net/wimax/debugfs.c | 42 ++------
net/wimax/stack.c | 11 +--
net/wimax/wimax-internal.h | 7 +-
8 files changed, 51 insertions(+), 253 deletions(-)
diff --git a/drivers/net/wimax/i2400m/debugfs.c b/drivers/net/wimax/i2400m/debugfs.c
index 6544ac9df047..01492c1fa70b 100644
--- a/drivers/net/wimax/i2400m/debugfs.c
+++ b/drivers/net/wimax/i2400m/debugfs.c
@@ -30,15 +30,6 @@ DEFINE_SIMPLE_ATTRIBUTE(fops_netdev_queue_stopped,
debugfs_netdev_queue_stopped_get,
NULL, "%llu\n");
-
-static
-struct dentry *debugfs_create_netdev_queue_stopped(
- const char *name, struct dentry *parent, struct i2400m *i2400m)
-{
- return debugfs_create_file(name, 0400, parent, i2400m,
- &fops_netdev_queue_stopped);
-}
-
/*
* We don't allow partial reads of this file, as then the reader would
* get weirdly confused data as it is updated.
@@ -167,15 +158,6 @@ DEFINE_SIMPLE_ATTRIBUTE(fops_i2400m_suspend,
NULL, debugfs_i2400m_suspend_set,
"%llu\n");
-static
-struct dentry *debugfs_create_i2400m_suspend(
- const char *name, struct dentry *parent, struct i2400m *i2400m)
-{
- return debugfs_create_file(name, 0200, parent, i2400m,
- &fops_i2400m_suspend);
-}
-
-
/*
* Reset the device
*
@@ -205,73 +187,26 @@ DEFINE_SIMPLE_ATTRIBUTE(fops_i2400m_reset,
NULL, debugfs_i2400m_reset_set,
"%llu\n");
-static
-struct dentry *debugfs_create_i2400m_reset(
- const char *name, struct dentry *parent, struct i2400m *i2400m)
+void i2400m_debugfs_add(struct i2400m *i2400m)
{
- return debugfs_create_file(name, 0200, parent, i2400m,
- &fops_i2400m_reset);
-}
-
-
-#define __debugfs_register(prefix, name, parent) \
-do { \
- result = d_level_register_debugfs(prefix, name, parent); \
- if (result < 0) \
- goto error; \
-} while (0)
-
-
-int i2400m_debugfs_add(struct i2400m *i2400m)
-{
- int result;
struct device *dev = i2400m_dev(i2400m);
struct dentry *dentry = i2400m->wimax_dev.debugfs_dentry;
- struct dentry *fd;
dentry = debugfs_create_dir("i2400m", dentry);
- result = PTR_ERR(dentry);
- if (IS_ERR(dentry)) {
- if (result == -ENODEV)
- result = 0; /* No debugfs support */
- goto error;
- }
i2400m->debugfs_dentry = dentry;
- __debugfs_register("dl_", control, dentry);
- __debugfs_register("dl_", driver, dentry);
- __debugfs_register("dl_", debugfs, dentry);
- __debugfs_register("dl_", fw, dentry);
- __debugfs_register("dl_", netdev, dentry);
- __debugfs_register("dl_", rfkill, dentry);
- __debugfs_register("dl_", rx, dentry);
- __debugfs_register("dl_", tx, dentry);
-
- fd = debugfs_create_size_t("tx_in", 0400, dentry,
- &i2400m->tx_in);
- result = PTR_ERR(fd);
- if (IS_ERR(fd) && result != -ENODEV) {
- dev_err(dev, "Can't create debugfs entry "
- "tx_in: %d\n", result);
- goto error;
- }
- fd = debugfs_create_size_t("tx_out", 0400, dentry,
- &i2400m->tx_out);
- result = PTR_ERR(fd);
- if (IS_ERR(fd) && result != -ENODEV) {
- dev_err(dev, "Can't create debugfs entry "
- "tx_out: %d\n", result);
- goto error;
- }
+ d_level_register_debugfs("dl_", control, dentry);
+ d_level_register_debugfs("dl_", driver, dentry);
+ d_level_register_debugfs("dl_", debugfs, dentry);
+ d_level_register_debugfs("dl_", fw, dentry);
+ d_level_register_debugfs("dl_", netdev, dentry);
+ d_level_register_debugfs("dl_", rfkill, dentry);
+ d_level_register_debugfs("dl_", rx, dentry);
+ d_level_register_debugfs("dl_", tx, dentry);
- fd = debugfs_create_u32("state", 0600, dentry,
- &i2400m->state);
- result = PTR_ERR(fd);
- if (IS_ERR(fd) && result != -ENODEV) {
- dev_err(dev, "Can't create debugfs entry "
- "state: %d\n", result);
- goto error;
- }
+ debugfs_create_size_t("tx_in", 0400, dentry, &i2400m->tx_in);
+ debugfs_create_size_t("tx_out", 0400, dentry, &i2400m->tx_out);
+ debugfs_create_u32("state", 0600, dentry, &i2400m->state);
/*
* Trace received messages from user space
@@ -295,60 +230,22 @@ int i2400m_debugfs_add(struct i2400m *i2400m)
* It is not really very atomic, but it is also not too
* critical.
*/
- fd = debugfs_create_u8("trace_msg_from_user", 0600, dentry,
- &i2400m->trace_msg_from_user);
- result = PTR_ERR(fd);
- if (IS_ERR(fd) && result != -ENODEV) {
- dev_err(dev, "Can't create debugfs entry "
- "trace_msg_from_user: %d\n", result);
- goto error;
- }
+ debugfs_create_u8("trace_msg_from_user", 0600, dentry,
+ &i2400m->trace_msg_from_user);
- fd = debugfs_create_netdev_queue_stopped("netdev_queue_stopped",
- dentry, i2400m);
- result = PTR_ERR(fd);
- if (IS_ERR(fd) && result != -ENODEV) {
- dev_err(dev, "Can't create debugfs entry "
- "netdev_queue_stopped: %d\n", result);
- goto error;
- }
+ debugfs_create_file("netdev_queue_stopped", 0400, dentry, i2400m,
+ &fops_netdev_queue_stopped);
- fd = debugfs_create_file("rx_stats", 0600, dentry, i2400m,
- &i2400m_rx_stats_fops);
- result = PTR_ERR(fd);
- if (IS_ERR(fd) && result != -ENODEV) {
- dev_err(dev, "Can't create debugfs entry "
- "rx_stats: %d\n", result);
- goto error;
- }
+ debugfs_create_file("rx_stats", 0600, dentry, i2400m,
+ &i2400m_rx_stats_fops);
- fd = debugfs_create_file("tx_stats", 0600, dentry, i2400m,
- &i2400m_tx_stats_fops);
- result = PTR_ERR(fd);
- if (IS_ERR(fd) && result != -ENODEV) {
- dev_err(dev, "Can't create debugfs entry "
- "tx_stats: %d\n", result);
- goto error;
- }
+ debugfs_create_file("tx_stats", 0600, dentry, i2400m,
+ &i2400m_tx_stats_fops);
- fd = debugfs_create_i2400m_suspend("suspend", dentry, i2400m);
- result = PTR_ERR(fd);
- if (IS_ERR(fd) && result != -ENODEV) {
- dev_err(dev, "Can't create debugfs entry suspend: %d\n",
- result);
- goto error;
- }
+ debugfs_create_file("suspend", 0200, dentry, i2400m,
+ &fops_i2400m_suspend);
- fd = debugfs_create_i2400m_reset("reset", dentry, i2400m);
- result = PTR_ERR(fd);
- if (IS_ERR(fd) && result != -ENODEV) {
- dev_err(dev, "Can't create debugfs entry reset: %d\n", result);
- goto error;
- }
-
- result = 0;
-error:
- return result;
+ debugfs_create_file("reset", 0200, dentry, i2400m, &fops_i2400m_reset);
}
void i2400m_debugfs_rm(struct i2400m *i2400m)
diff --git a/drivers/net/wimax/i2400m/driver.c b/drivers/net/wimax/i2400m/driver.c
index 0a29222a1bf9..f66c0f8f6f4a 100644
--- a/drivers/net/wimax/i2400m/driver.c
+++ b/drivers/net/wimax/i2400m/driver.c
@@ -905,11 +905,7 @@ int i2400m_setup(struct i2400m *i2400m, enum i2400m_bri bm_flags)
goto error_sysfs_setup;
}
- result = i2400m_debugfs_add(i2400m);
- if (result < 0) {
- dev_err(dev, "cannot setup i2400m's debugfs: %d\n", result);
- goto error_debugfs_setup;
- }
+ i2400m_debugfs_add(i2400m);
result = i2400m_dev_start(i2400m, bm_flags);
if (result < 0)
@@ -919,7 +915,6 @@ int i2400m_setup(struct i2400m *i2400m, enum i2400m_bri bm_flags)
error_dev_start:
i2400m_debugfs_rm(i2400m);
-error_debugfs_setup:
sysfs_remove_group(&i2400m->wimax_dev.net_dev->dev.kobj,
&i2400m_dev_attr_group);
error_sysfs_setup:
diff --git a/drivers/net/wimax/i2400m/i2400m.h b/drivers/net/wimax/i2400m/i2400m.h
index 5a34e72bab9a..a3733a6d14f5 100644
--- a/drivers/net/wimax/i2400m/i2400m.h
+++ b/drivers/net/wimax/i2400m/i2400m.h
@@ -812,13 +812,10 @@ enum i2400m_pt;
int i2400m_tx(struct i2400m *, const void *, size_t, enum i2400m_pt);
#ifdef CONFIG_DEBUG_FS
-int i2400m_debugfs_add(struct i2400m *);
+void i2400m_debugfs_add(struct i2400m *);
void i2400m_debugfs_rm(struct i2400m *);
#else
-static inline int i2400m_debugfs_add(struct i2400m *i2400m)
-{
- return 0;
-}
+static inline void i2400m_debugfs_add(struct i2400m *i2400m) {}
static inline void i2400m_debugfs_rm(struct i2400m *i2400m) {}
#endif
diff --git a/drivers/net/wimax/i2400m/usb.c b/drivers/net/wimax/i2400m/usb.c
index 2075e7b1fff6..5bc48100b52e 100644
--- a/drivers/net/wimax/i2400m/usb.c
+++ b/drivers/net/wimax/i2400m/usb.c
@@ -366,17 +366,8 @@ struct d_level D_LEVEL[] = {
};
size_t D_LEVEL_SIZE = ARRAY_SIZE(D_LEVEL);
-
-#define __debugfs_register(prefix, name, parent) \
-do { \
- result = d_level_register_debugfs(prefix, name, parent); \
- if (result < 0) \
- goto error; \
-} while (0)
-
-
static
-int i2400mu_debugfs_add(struct i2400mu *i2400mu)
+void i2400mu_debugfs_add(struct i2400mu *i2400mu)
{
int result;
struct device *dev = &i2400mu->usb_iface->dev;
@@ -384,43 +375,19 @@ int i2400mu_debugfs_add(struct i2400mu *i2400mu)
struct dentry *fd;
dentry = debugfs_create_dir("i2400m-usb", dentry);
- result = PTR_ERR(dentry);
- if (IS_ERR(dentry)) {
- if (result == -ENODEV)
- result = 0; /* No debugfs support */
- goto error;
- }
i2400mu->debugfs_dentry = dentry;
- __debugfs_register("dl_", usb, dentry);
- __debugfs_register("dl_", fw, dentry);
- __debugfs_register("dl_", notif, dentry);
- __debugfs_register("dl_", rx, dentry);
- __debugfs_register("dl_", tx, dentry);
- /* Don't touch these if you don't know what you are doing */
- fd = debugfs_create_u8("rx_size_auto_shrink", 0600, dentry,
- &i2400mu->rx_size_auto_shrink);
- result = PTR_ERR(fd);
- if (IS_ERR(fd) && result != -ENODEV) {
- dev_err(dev, "Can't create debugfs entry "
- "rx_size_auto_shrink: %d\n", result);
- goto error;
- }
-
- fd = debugfs_create_size_t("rx_size", 0600, dentry,
- &i2400mu->rx_size);
- result = PTR_ERR(fd);
- if (IS_ERR(fd) && result != -ENODEV) {
- dev_err(dev, "Can't create debugfs entry "
- "rx_size: %d\n", result);
- goto error;
- }
+ d_level_register_debugfs("dl_", usb, dentry);
+ d_level_register_debugfs("dl_", fw, dentry);
+ d_level_register_debugfs("dl_", notif, dentry);
+ d_level_register_debugfs("dl_", rx, dentry);
+ d_level_register_debugfs("dl_", tx, dentry);
- return 0;
+ /* Don't touch these if you don't know what you are doing */
+ debugfs_create_u8("rx_size_auto_shrink", 0600, dentry,
+ &i2400mu->rx_size_auto_shrink);
-error:
- debugfs_remove_recursive(i2400mu->debugfs_dentry);
- return result;
+ debugfs_create_size_t("rx_size", 0600, dentry, &i2400mu->rx_size);
}
@@ -534,15 +501,9 @@ int i2400mu_probe(struct usb_interface *iface,
dev_err(dev, "cannot setup device: %d\n", result);
goto error_setup;
}
- result = i2400mu_debugfs_add(i2400mu);
- if (result < 0) {
- dev_err(dev, "Can't register i2400mu's debugfs: %d\n", result);
- goto error_debugfs_add;
- }
+ i2400mu_debugfs_add(i2400mu);
return 0;
-error_debugfs_add:
- i2400m_release(i2400m);
error_setup:
usb_set_intfdata(iface, NULL);
usb_put_dev(i2400mu->usb_dev);
diff --git a/include/linux/wimax/debug.h b/include/linux/wimax/debug.h
index 7cb63e4ec0ae..4dd2c1cea6a9 100644
--- a/include/linux/wimax/debug.h
+++ b/include/linux/wimax/debug.h
@@ -98,9 +98,7 @@
* To manipulate from user space the levels, create a debugfs dentry
* and then register each submodule with:
*
- * result = d_level_register_debugfs("PREFIX_", submodule_X, parent);
- * if (result < 0)
- * goto error;
+ * d_level_register_debugfs("PREFIX_", submodule_X, parent);
*
* Where PREFIX_ is a name of your chosing. This will create debugfs
* file with a single numeric value that can be use to tweak it. To
@@ -408,25 +406,13 @@ do { \
* @submodule: name of submodule (not a string, just the name)
* @dentry: debugfs parent dentry
*
- * Returns: 0 if ok, < 0 errno on error.
- *
* For removing, just use debugfs_remove_recursive() on the parent.
*/
#define d_level_register_debugfs(prefix, name, parent) \
({ \
- int rc; \
- struct dentry *fd; \
- struct dentry *verify_parent_type = parent; \
- fd = debugfs_create_u8( \
- prefix #name, 0600, verify_parent_type, \
+ debugfs_create_u8( \
+ prefix #name, 0600, parent, \
&(D_LEVEL[__D_SUBMODULE_ ## name].level)); \
- rc = PTR_ERR(fd); \
- if (IS_ERR(fd) && rc != -ENODEV) \
- printk(KERN_ERR "%s: Can't create debugfs entry %s: " \
- "%d\n", __func__, prefix #name, rc); \
- else \
- rc = 0; \
- rc; \
})
diff --git a/net/wimax/debugfs.c b/net/wimax/debugfs.c
index 1af56df30276..3c54bb6b925a 100644
--- a/net/wimax/debugfs.c
+++ b/net/wimax/debugfs.c
@@ -13,49 +13,23 @@
#define D_SUBMODULE debugfs
#include "debug-levels.h"
-
-#define __debugfs_register(prefix, name, parent) \
-do { \
- result = d_level_register_debugfs(prefix, name, parent); \
- if (result < 0) \
- goto error; \
-} while (0)
-
-
-int wimax_debugfs_add(struct wimax_dev *wimax_dev)
+void wimax_debugfs_add(struct wimax_dev *wimax_dev)
{
- int result;
struct net_device *net_dev = wimax_dev->net_dev;
- struct device *dev = net_dev->dev.parent;
struct dentry *dentry;
char buf[128];
snprintf(buf, sizeof(buf), "wimax:%s", net_dev->name);
dentry = debugfs_create_dir(buf, NULL);
- result = PTR_ERR(dentry);
- if (IS_ERR(dentry)) {
- if (result == -ENODEV)
- result = 0; /* No debugfs support */
- else
- dev_err(dev, "Can't create debugfs dentry: %d\n",
- result);
- goto out;
- }
wimax_dev->debugfs_dentry = dentry;
- __debugfs_register("wimax_dl_", debugfs, dentry);
- __debugfs_register("wimax_dl_", id_table, dentry);
- __debugfs_register("wimax_dl_", op_msg, dentry);
- __debugfs_register("wimax_dl_", op_reset, dentry);
- __debugfs_register("wimax_dl_", op_rfkill, dentry);
- __debugfs_register("wimax_dl_", op_state_get, dentry);
- __debugfs_register("wimax_dl_", stack, dentry);
- result = 0;
-out:
- return result;
-error:
- debugfs_remove_recursive(wimax_dev->debugfs_dentry);
- return result;
+ d_level_register_debugfs("wimax_dl_", debugfs, dentry);
+ d_level_register_debugfs("wimax_dl_", id_table, dentry);
+ d_level_register_debugfs("wimax_dl_", op_msg, dentry);
+ d_level_register_debugfs("wimax_dl_", op_reset, dentry);
+ d_level_register_debugfs("wimax_dl_", op_rfkill, dentry);
+ d_level_register_debugfs("wimax_dl_", op_state_get, dentry);
+ d_level_register_debugfs("wimax_dl_", stack, dentry);
}
void wimax_debugfs_rm(struct wimax_dev *wimax_dev)
diff --git a/net/wimax/stack.c b/net/wimax/stack.c
index 1ba99d65feca..4b9b1c5e8f3a 100644
--- a/net/wimax/stack.c
+++ b/net/wimax/stack.c
@@ -481,12 +481,7 @@ int wimax_dev_add(struct wimax_dev *wimax_dev, struct net_device *net_dev)
/* Set up user-space interaction */
mutex_lock(&wimax_dev->mutex);
wimax_id_table_add(wimax_dev);
- result = wimax_debugfs_add(wimax_dev);
- if (result < 0) {
- dev_err(dev, "cannot initialize debugfs: %d\n",
- result);
- goto error_debugfs_add;
- }
+ wimax_debugfs_add(wimax_dev);
__wimax_state_set(wimax_dev, WIMAX_ST_DOWN);
mutex_unlock(&wimax_dev->mutex);
@@ -498,10 +493,6 @@ int wimax_dev_add(struct wimax_dev *wimax_dev, struct net_device *net_dev)
d_fnend(3, dev, "(wimax_dev %p net_dev %p) = 0\n", wimax_dev, net_dev);
return 0;
-error_debugfs_add:
- wimax_id_table_rm(wimax_dev);
- mutex_unlock(&wimax_dev->mutex);
- wimax_rfkill_rm(wimax_dev);
error_rfkill_add:
d_fnend(3, dev, "(wimax_dev %p net_dev %p) = %d\n",
wimax_dev, net_dev, result);
diff --git a/net/wimax/wimax-internal.h b/net/wimax/wimax-internal.h
index e819a09337ee..40751207296c 100644
--- a/net/wimax/wimax-internal.h
+++ b/net/wimax/wimax-internal.h
@@ -57,13 +57,10 @@ void __wimax_state_set(struct wimax_dev *wimax_dev, enum wimax_st state)
void __wimax_state_change(struct wimax_dev *, enum wimax_st);
#ifdef CONFIG_DEBUG_FS
-int wimax_debugfs_add(struct wimax_dev *);
+void wimax_debugfs_add(struct wimax_dev *);
void wimax_debugfs_rm(struct wimax_dev *);
#else
-static inline int wimax_debugfs_add(struct wimax_dev *wimax_dev)
-{
- return 0;
-}
+static inline void wimax_debugfs_add(struct wimax_dev *wimax_dev) {}
static inline void wimax_debugfs_rm(struct wimax_dev *wimax_dev) {}
#endif
--
2.22.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH 02/17] bonding: no need to print a message if debugfs_create_dir() fails
2019-08-06 16:11 [PATCH 00/17] Networking driver debugfs cleanups Greg Kroah-Hartman
2019-08-06 16:11 ` [PATCH 01/17] wimax: no need to check return value of debugfs_create functions Greg Kroah-Hartman
@ 2019-08-06 16:11 ` Greg Kroah-Hartman
2019-08-06 16:11 ` [PATCH 03/17] mlx5: no need to check return value of debugfs_create functions Greg Kroah-Hartman
` (15 subsequent siblings)
17 siblings, 0 replies; 32+ messages in thread
From: Greg Kroah-Hartman @ 2019-08-06 16:11 UTC (permalink / raw)
To: netdev
Cc: Greg Kroah-Hartman, Jay Vosburgh, Veaceslav Falico,
Andy Gospodarek, David S. Miller
The debugfs core now will print a message if this function fails, so
don't duplicate that logic. Also, no need to change the code logic if
the call fails either, as no debugfs calls should interrupt normal
kernel code for any reason.
Cc: Jay Vosburgh <j.vosburgh@gmail.com>
Cc: Veaceslav Falico <vfalico@gmail.com>
Cc: Andy Gospodarek <andy@greyhouse.net>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: netdev@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
drivers/net/bonding/bond_debugfs.c | 5 -----
1 file changed, 5 deletions(-)
diff --git a/drivers/net/bonding/bond_debugfs.c b/drivers/net/bonding/bond_debugfs.c
index 1360f1ffe070..f3f86ef68ae0 100644
--- a/drivers/net/bonding/bond_debugfs.c
+++ b/drivers/net/bonding/bond_debugfs.c
@@ -55,11 +55,6 @@ void bond_debug_register(struct bonding *bond)
bond->debug_dir =
debugfs_create_dir(bond->dev->name, bonding_debug_root);
- if (!bond->debug_dir) {
- netdev_warn(bond->dev, "failed to register to debugfs\n");
- return;
- }
-
debugfs_create_file("rlb_hash_table", 0400, bond->debug_dir,
bond, &bond_debug_rlb_hash_fops);
}
--
2.22.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH 03/17] mlx5: no need to check return value of debugfs_create functions
2019-08-06 16:11 [PATCH 00/17] Networking driver debugfs cleanups Greg Kroah-Hartman
2019-08-06 16:11 ` [PATCH 01/17] wimax: no need to check return value of debugfs_create functions Greg Kroah-Hartman
2019-08-06 16:11 ` [PATCH 02/17] bonding: no need to print a message if debugfs_create_dir() fails Greg Kroah-Hartman
@ 2019-08-06 16:11 ` Greg Kroah-Hartman
2019-08-06 19:41 ` Saeed Mahameed
2019-08-06 20:37 ` Saeed Mahameed
2019-08-06 16:11 ` [PATCH 04/17] xgbe: " Greg Kroah-Hartman
` (14 subsequent siblings)
17 siblings, 2 replies; 32+ messages in thread
From: Greg Kroah-Hartman @ 2019-08-06 16:11 UTC (permalink / raw)
To: netdev; +Cc: Greg Kroah-Hartman, Saeed Mahameed, Leon Romanovsky
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.
This cleans up a lot of unneeded code and logic around the debugfs
files, making all of this much simpler and easier to understand as we
don't need to keep the dentries saved anymore.
Cc: Saeed Mahameed <saeedm@mellanox.com>
Cc: Leon Romanovsky <leon@kernel.org>
Cc: netdev@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
drivers/net/ethernet/mellanox/mlx5/core/cmd.c | 51 ++-------
.../net/ethernet/mellanox/mlx5/core/debugfs.c | 102 ++----------------
drivers/net/ethernet/mellanox/mlx5/core/eq.c | 11 +-
.../net/ethernet/mellanox/mlx5/core/lib/eq.h | 2 +-
.../net/ethernet/mellanox/mlx5/core/main.c | 7 +-
.../ethernet/mellanox/mlx5/core/mlx5_core.h | 2 +-
include/linux/mlx5/driver.h | 12 +--
7 files changed, 24 insertions(+), 163 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/cmd.c b/drivers/net/ethernet/mellanox/mlx5/core/cmd.c
index 8cdd7e66f8df..973f90888b1f 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/cmd.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/cmd.c
@@ -1368,49 +1368,19 @@ static void clean_debug_files(struct mlx5_core_dev *dev)
debugfs_remove_recursive(dbg->dbg_root);
}
-static int create_debugfs_files(struct mlx5_core_dev *dev)
+static void create_debugfs_files(struct mlx5_core_dev *dev)
{
struct mlx5_cmd_debug *dbg = &dev->cmd.dbg;
- int err = -ENOMEM;
-
- if (!mlx5_debugfs_root)
- return 0;
dbg->dbg_root = debugfs_create_dir("cmd", dev->priv.dbg_root);
- if (!dbg->dbg_root)
- return err;
-
- dbg->dbg_in = debugfs_create_file("in", 0400, dbg->dbg_root,
- dev, &dfops);
- if (!dbg->dbg_in)
- goto err_dbg;
- dbg->dbg_out = debugfs_create_file("out", 0200, dbg->dbg_root,
- dev, &dfops);
- if (!dbg->dbg_out)
- goto err_dbg;
-
- dbg->dbg_outlen = debugfs_create_file("out_len", 0600, dbg->dbg_root,
- dev, &olfops);
- if (!dbg->dbg_outlen)
- goto err_dbg;
-
- dbg->dbg_status = debugfs_create_u8("status", 0600, dbg->dbg_root,
- &dbg->status);
- if (!dbg->dbg_status)
- goto err_dbg;
-
- dbg->dbg_run = debugfs_create_file("run", 0200, dbg->dbg_root, dev, &fops);
- if (!dbg->dbg_run)
- goto err_dbg;
+ debugfs_create_file("in", 0400, dbg->dbg_root, dev, &dfops);
+ debugfs_create_file("out", 0200, dbg->dbg_root, dev, &dfops);
+ debugfs_create_file("out_len", 0600, dbg->dbg_root, dev, &olfops);
+ debugfs_create_u8("status", 0600, dbg->dbg_root, &dbg->status);
+ debugfs_create_file("run", 0200, dbg->dbg_root, dev, &fops);
mlx5_cmdif_debugfs_init(dev);
-
- return 0;
-
-err_dbg:
- clean_debug_files(dev);
- return err;
}
static void mlx5_cmd_change_mod(struct mlx5_core_dev *dev, int mode)
@@ -2007,17 +1977,10 @@ int mlx5_cmd_init(struct mlx5_core_dev *dev)
goto err_cache;
}
- err = create_debugfs_files(dev);
- if (err) {
- err = -ENOMEM;
- goto err_wq;
- }
+ create_debugfs_files(dev);
return 0;
-err_wq:
- destroy_workqueue(cmd->wq);
-
err_cache:
destroy_msg_cache(dev);
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/debugfs.c b/drivers/net/ethernet/mellanox/mlx5/core/debugfs.c
index a11e22d0b0cc..04854e5fbcd7 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/debugfs.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/debugfs.c
@@ -92,8 +92,6 @@ EXPORT_SYMBOL(mlx5_debugfs_root);
void mlx5_register_debugfs(void)
{
mlx5_debugfs_root = debugfs_create_dir("mlx5", NULL);
- if (IS_ERR_OR_NULL(mlx5_debugfs_root))
- mlx5_debugfs_root = NULL;
}
void mlx5_unregister_debugfs(void)
@@ -101,45 +99,25 @@ void mlx5_unregister_debugfs(void)
debugfs_remove(mlx5_debugfs_root);
}
-int mlx5_qp_debugfs_init(struct mlx5_core_dev *dev)
+void mlx5_qp_debugfs_init(struct mlx5_core_dev *dev)
{
- if (!mlx5_debugfs_root)
- return 0;
-
atomic_set(&dev->num_qps, 0);
dev->priv.qp_debugfs = debugfs_create_dir("QPs", dev->priv.dbg_root);
- if (!dev->priv.qp_debugfs)
- return -ENOMEM;
-
- return 0;
}
void mlx5_qp_debugfs_cleanup(struct mlx5_core_dev *dev)
{
- if (!mlx5_debugfs_root)
- return;
-
debugfs_remove_recursive(dev->priv.qp_debugfs);
}
-int mlx5_eq_debugfs_init(struct mlx5_core_dev *dev)
+void mlx5_eq_debugfs_init(struct mlx5_core_dev *dev)
{
- if (!mlx5_debugfs_root)
- return 0;
-
dev->priv.eq_debugfs = debugfs_create_dir("EQs", dev->priv.dbg_root);
- if (!dev->priv.eq_debugfs)
- return -ENOMEM;
-
- return 0;
}
void mlx5_eq_debugfs_cleanup(struct mlx5_core_dev *dev)
{
- if (!mlx5_debugfs_root)
- return;
-
debugfs_remove_recursive(dev->priv.eq_debugfs);
}
@@ -183,85 +161,41 @@ static const struct file_operations stats_fops = {
.write = average_write,
};
-int mlx5_cmdif_debugfs_init(struct mlx5_core_dev *dev)
+void mlx5_cmdif_debugfs_init(struct mlx5_core_dev *dev)
{
struct mlx5_cmd_stats *stats;
struct dentry **cmd;
const char *namep;
- int err;
int i;
- if (!mlx5_debugfs_root)
- return 0;
-
cmd = &dev->priv.cmdif_debugfs;
*cmd = debugfs_create_dir("commands", dev->priv.dbg_root);
- if (!*cmd)
- return -ENOMEM;
for (i = 0; i < ARRAY_SIZE(dev->cmd.stats); i++) {
stats = &dev->cmd.stats[i];
namep = mlx5_command_str(i);
if (strcmp(namep, "unknown command opcode")) {
stats->root = debugfs_create_dir(namep, *cmd);
- if (!stats->root) {
- mlx5_core_warn(dev, "failed adding command %d\n",
- i);
- err = -ENOMEM;
- goto out;
- }
-
- stats->avg = debugfs_create_file("average", 0400,
- stats->root, stats,
- &stats_fops);
- if (!stats->avg) {
- mlx5_core_warn(dev, "failed creating debugfs file\n");
- err = -ENOMEM;
- goto out;
- }
-
- stats->count = debugfs_create_u64("n", 0400,
- stats->root,
- &stats->n);
- if (!stats->count) {
- mlx5_core_warn(dev, "failed creating debugfs file\n");
- err = -ENOMEM;
- goto out;
- }
+
+ debugfs_create_file("average", 0400, stats->root, stats,
+ &stats_fops);
+ debugfs_create_u64("n", 0400, stats->root, &stats->n);
}
}
-
- return 0;
-out:
- debugfs_remove_recursive(dev->priv.cmdif_debugfs);
- return err;
}
void mlx5_cmdif_debugfs_cleanup(struct mlx5_core_dev *dev)
{
- if (!mlx5_debugfs_root)
- return;
-
debugfs_remove_recursive(dev->priv.cmdif_debugfs);
}
-int mlx5_cq_debugfs_init(struct mlx5_core_dev *dev)
+void mlx5_cq_debugfs_init(struct mlx5_core_dev *dev)
{
- if (!mlx5_debugfs_root)
- return 0;
-
dev->priv.cq_debugfs = debugfs_create_dir("CQs", dev->priv.dbg_root);
- if (!dev->priv.cq_debugfs)
- return -ENOMEM;
-
- return 0;
}
void mlx5_cq_debugfs_cleanup(struct mlx5_core_dev *dev)
{
- if (!mlx5_debugfs_root)
- return;
-
debugfs_remove_recursive(dev->priv.cq_debugfs);
}
@@ -484,7 +418,6 @@ static int add_res_tree(struct mlx5_core_dev *dev, enum dbg_rsc_type type,
{
struct mlx5_rsc_debug *d;
char resn[32];
- int err;
int i;
d = kzalloc(struct_size(d, fields, nfile), GFP_KERNEL);
@@ -496,30 +429,15 @@ static int add_res_tree(struct mlx5_core_dev *dev, enum dbg_rsc_type type,
d->type = type;
sprintf(resn, "0x%x", rsn);
d->root = debugfs_create_dir(resn, root);
- if (!d->root) {
- err = -ENOMEM;
- goto out_free;
- }
for (i = 0; i < nfile; i++) {
d->fields[i].i = i;
- d->fields[i].dent = debugfs_create_file(field[i], 0400,
- d->root, &d->fields[i],
- &fops);
- if (!d->fields[i].dent) {
- err = -ENOMEM;
- goto out_rem;
- }
+ debugfs_create_file(field[i], 0400, d->root, &d->fields[i],
+ &fops);
}
*dbg = d;
return 0;
-out_rem:
- debugfs_remove_recursive(d->root);
-
-out_free:
- kfree(d);
- return err;
}
static void rem_res_tree(struct mlx5_rsc_debug *d)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eq.c b/drivers/net/ethernet/mellanox/mlx5/core/eq.c
index 2df9aaa421c6..09d4c64b6e73 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/eq.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/eq.c
@@ -411,7 +411,7 @@ void mlx5_eq_del_cq(struct mlx5_eq *eq, struct mlx5_core_cq *cq)
int mlx5_eq_table_init(struct mlx5_core_dev *dev)
{
struct mlx5_eq_table *eq_table;
- int i, err;
+ int i;
eq_table = kvzalloc(sizeof(*eq_table), GFP_KERNEL);
if (!eq_table)
@@ -419,9 +419,7 @@ int mlx5_eq_table_init(struct mlx5_core_dev *dev)
dev->priv.eq_table = eq_table;
- err = mlx5_eq_debugfs_init(dev);
- if (err)
- goto kvfree_eq_table;
+ mlx5_eq_debugfs_init(dev);
mutex_init(&eq_table->lock);
for (i = 0; i < MLX5_EVENT_TYPE_MAX; i++)
@@ -429,11 +427,6 @@ int mlx5_eq_table_init(struct mlx5_core_dev *dev)
eq_table->irq_table = dev->priv.irq_table;
return 0;
-
-kvfree_eq_table:
- kvfree(eq_table);
- dev->priv.eq_table = NULL;
- return err;
}
void mlx5_eq_table_cleanup(struct mlx5_core_dev *dev)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/lib/eq.h b/drivers/net/ethernet/mellanox/mlx5/core/lib/eq.h
index 3dfab91ae5f2..4be4d2d36218 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/lib/eq.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/lib/eq.h
@@ -87,7 +87,7 @@ void mlx5_eq_synchronize_cmd_irq(struct mlx5_core_dev *dev);
int mlx5_debug_eq_add(struct mlx5_core_dev *dev, struct mlx5_eq *eq);
void mlx5_debug_eq_remove(struct mlx5_core_dev *dev, struct mlx5_eq *eq);
-int mlx5_eq_debugfs_init(struct mlx5_core_dev *dev);
+void mlx5_eq_debugfs_init(struct mlx5_core_dev *dev);
void mlx5_eq_debugfs_cleanup(struct mlx5_core_dev *dev);
/* This function should only be called after mlx5_cmd_force_teardown_hca */
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/main.c b/drivers/net/ethernet/mellanox/mlx5/core/main.c
index fa0e991f1983..0b70b1d6338d 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/main.c
@@ -826,11 +826,7 @@ static int mlx5_init_once(struct mlx5_core_dev *dev)
goto err_eq_cleanup;
}
- err = mlx5_cq_debugfs_init(dev);
- if (err) {
- mlx5_core_err(dev, "failed to initialize cq debugfs\n");
- goto err_events_cleanup;
- }
+ mlx5_cq_debugfs_init(dev);
mlx5_init_qp_table(dev);
@@ -891,7 +887,6 @@ static int mlx5_init_once(struct mlx5_core_dev *dev)
mlx5_cleanup_mkey_table(dev);
mlx5_cleanup_qp_table(dev);
mlx5_cq_debugfs_cleanup(dev);
-err_events_cleanup:
mlx5_events_cleanup(dev);
err_eq_cleanup:
mlx5_eq_table_cleanup(dev);
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/mlx5_core.h b/drivers/net/ethernet/mellanox/mlx5/core/mlx5_core.h
index 471bbc48bc1f..87b75b2207c4 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/mlx5_core.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/mlx5_core.h
@@ -146,7 +146,7 @@ u64 mlx5_read_internal_timer(struct mlx5_core_dev *dev,
void mlx5_cmd_trigger_completions(struct mlx5_core_dev *dev);
void mlx5_cmd_flush(struct mlx5_core_dev *dev);
-int mlx5_cq_debugfs_init(struct mlx5_core_dev *dev);
+void mlx5_cq_debugfs_init(struct mlx5_core_dev *dev);
void mlx5_cq_debugfs_cleanup(struct mlx5_core_dev *dev);
int mlx5_query_pcam_reg(struct mlx5_core_dev *dev, u32 *pcam, u8 feature_group,
diff --git a/include/linux/mlx5/driver.h b/include/linux/mlx5/driver.h
index d8f348ef9c33..df23f17eed64 100644
--- a/include/linux/mlx5/driver.h
+++ b/include/linux/mlx5/driver.h
@@ -189,7 +189,6 @@ enum mlx5_coredev_type {
};
struct mlx5_field_desc {
- struct dentry *dent;
int i;
};
@@ -242,11 +241,6 @@ struct mlx5_cmd_msg {
struct mlx5_cmd_debug {
struct dentry *dbg_root;
- struct dentry *dbg_in;
- struct dentry *dbg_out;
- struct dentry *dbg_outlen;
- struct dentry *dbg_status;
- struct dentry *dbg_run;
void *in_msg;
void *out_msg;
u8 status;
@@ -271,8 +265,6 @@ struct mlx5_cmd_stats {
u64 sum;
u64 n;
struct dentry *root;
- struct dentry *avg;
- struct dentry *count;
/* protect command average calculations */
spinlock_t lock;
};
@@ -972,7 +964,7 @@ int mlx5_vector2eqn(struct mlx5_core_dev *dev, int vector, int *eqn,
int mlx5_core_attach_mcg(struct mlx5_core_dev *dev, union ib_gid *mgid, u32 qpn);
int mlx5_core_detach_mcg(struct mlx5_core_dev *dev, union ib_gid *mgid, u32 qpn);
-int mlx5_qp_debugfs_init(struct mlx5_core_dev *dev);
+void mlx5_qp_debugfs_init(struct mlx5_core_dev *dev);
void mlx5_qp_debugfs_cleanup(struct mlx5_core_dev *dev);
int mlx5_core_access_reg(struct mlx5_core_dev *dev, void *data_in,
int size_in, void *data_out, int size_out,
@@ -984,7 +976,7 @@ int mlx5_db_alloc_node(struct mlx5_core_dev *dev, struct mlx5_db *db,
void mlx5_db_free(struct mlx5_core_dev *dev, struct mlx5_db *db);
const char *mlx5_command_str(int command);
-int mlx5_cmdif_debugfs_init(struct mlx5_core_dev *dev);
+void mlx5_cmdif_debugfs_init(struct mlx5_core_dev *dev);
void mlx5_cmdif_debugfs_cleanup(struct mlx5_core_dev *dev);
int mlx5_core_create_psv(struct mlx5_core_dev *dev, u32 pdn,
int npsvs, u32 *sig_index);
--
2.22.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH 03/17] mlx5: no need to check return value of debugfs_create functions
2019-08-06 16:11 ` [PATCH 03/17] mlx5: no need to check return value of debugfs_create functions Greg Kroah-Hartman
@ 2019-08-06 19:41 ` Saeed Mahameed
2019-08-06 19:47 ` gregkh
2019-08-06 20:37 ` Saeed Mahameed
1 sibling, 1 reply; 32+ messages in thread
From: Saeed Mahameed @ 2019-08-06 19:41 UTC (permalink / raw)
To: gregkh, netdev; +Cc: leon
On Tue, 2019-08-06 at 18:11 +0200, 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.
>
> This cleans up a lot of unneeded code and logic around the debugfs
> files, making all of this much simpler and easier to understand as we
> don't need to keep the dentries saved anymore.
>
Hi Greg,
Basically i am ok with this patch and i like it very much.., but i am
concerned about some of the driver internal flows that are dependent on
these debug fs entries being valid.
for example mlx5_debug_eq_add if failed, it will fail the whole flow. I
know it is wrong even before your patch.. but maybe we should deal with
it now ? or let me know if you want me to follow up with my own patch.
All we need to improve in this patch is to void out add_res_tree()
implemented in
drivers/net/ethernet/mellanox/mlx5/core/debugfs.c
as i will comment below.
> Cc: Saeed Mahameed <saeedm@mellanox.com>
> Cc: Leon Romanovsky <leon@kernel.org>
> Cc: netdev@vger.kernel.org
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
> drivers/net/ethernet/mellanox/mlx5/core/cmd.c | 51 ++-------
> .../net/ethernet/mellanox/mlx5/core/debugfs.c | 102 ++------------
> ----
> drivers/net/ethernet/mellanox/mlx5/core/eq.c | 11 +-
> .../net/ethernet/mellanox/mlx5/core/lib/eq.h | 2 +-
> .../net/ethernet/mellanox/mlx5/core/main.c | 7 +-
> .../ethernet/mellanox/mlx5/core/mlx5_core.h | 2 +-
> include/linux/mlx5/driver.h | 12 +--
> 7 files changed, 24 insertions(+), 163 deletions(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/cmd.c
> b/drivers/net/ethernet/mellanox/mlx5/core/cmd.c
> index 8cdd7e66f8df..973f90888b1f 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/cmd.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/cmd.c
> @@ -1368,49 +1368,19 @@ static void clean_debug_files(struct
> mlx5_core_dev *dev)
> debugfs_remove_recursive(dbg->dbg_root);
> }
>
[...]
> void mlx5_cq_debugfs_cleanup(struct mlx5_core_dev *dev)
> {
> - if (!mlx5_debugfs_root)
> - return;
> -
> debugfs_remove_recursive(dev->priv.cq_debugfs);
> }
>
> @@ -484,7 +418,6 @@ static int add_res_tree(struct mlx5_core_dev
Basically this function is a debugfs wrapper that should behave the
same as debug_fs_*, we should fix it to return void as well, and
improve all the its callers to ignore the return value.
callers are:
mlx5_debug_qp_add()
mlx5_debug_eq_add()
mlx5_debug_cq_add()
> *dev, enum dbg_rsc_type type,
> {
> struct mlx5_rsc_debug *d;
> char resn[32];
> - int err;
> int i;
>
> d = kzalloc(struct_size(d, fields, nfile), GFP_KERNEL);
> @@ -496,30 +429,15 @@ static int add_res_tree(struct mlx5_core_dev
> *dev, enum dbg_rsc_type type,
> d->type = type;
> sprintf(resn, "0x%x", rsn);
> d->root = debugfs_create_dir(resn, root);
> - if (!d->root) {
> - err = -ENOMEM;
> - goto out_free;
> - }
>
> for (i = 0; i < nfile; i++) {
> d->fields[i].i = i;
> - d->fields[i].dent = debugfs_create_file(field[i], 0400,
> - d->root, &d-
> >fields[i],
> - &fops);
> - if (!d->fields[i].dent) {
> - err = -ENOMEM;
> - goto out_rem;
> - }
> + debugfs_create_file(field[i], 0400, d->root, &d-
> >fields[i],
> + &fops);
> }
> *dbg = d;
>
> return 0;
> -out_rem:
> - debugfs_remove_recursive(d->root);
> -
> -out_free:
> - kfree(d);
> - return err;
> }
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH 03/17] mlx5: no need to check return value of debugfs_create functions
2019-08-06 19:41 ` Saeed Mahameed
@ 2019-08-06 19:47 ` gregkh
0 siblings, 0 replies; 32+ messages in thread
From: gregkh @ 2019-08-06 19:47 UTC (permalink / raw)
To: Saeed Mahameed; +Cc: netdev, leon
On Tue, Aug 06, 2019 at 07:41:57PM +0000, Saeed Mahameed wrote:
> On Tue, 2019-08-06 at 18:11 +0200, 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.
> >
> > This cleans up a lot of unneeded code and logic around the debugfs
> > files, making all of this much simpler and easier to understand as we
> > don't need to keep the dentries saved anymore.
> >
>
> Hi Greg,
>
> Basically i am ok with this patch and i like it very much.., but i am
> concerned about some of the driver internal flows that are dependent on
> these debug fs entries being valid.
That's never good, that's a bug in the driver :)
> for example mlx5_debug_eq_add if failed, it will fail the whole flow. I
> know it is wrong even before your patch.. but maybe we should deal with
> it now ? or let me know if you want me to follow up with my own patch.
Your own patch would be good as I do not know this part of the codebase
at all, thanks.
> All we need to improve in this patch is to void out add_res_tree()
> implemented in
> drivers/net/ethernet/mellanox/mlx5/core/debugfs.c
> as i will comment below.
>
>
> > Cc: Saeed Mahameed <saeedm@mellanox.com>
> > Cc: Leon Romanovsky <leon@kernel.org>
> > Cc: netdev@vger.kernel.org
> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > ---
> > drivers/net/ethernet/mellanox/mlx5/core/cmd.c | 51 ++-------
> > .../net/ethernet/mellanox/mlx5/core/debugfs.c | 102 ++------------
> > ----
> > drivers/net/ethernet/mellanox/mlx5/core/eq.c | 11 +-
> > .../net/ethernet/mellanox/mlx5/core/lib/eq.h | 2 +-
> > .../net/ethernet/mellanox/mlx5/core/main.c | 7 +-
> > .../ethernet/mellanox/mlx5/core/mlx5_core.h | 2 +-
> > include/linux/mlx5/driver.h | 12 +--
> > 7 files changed, 24 insertions(+), 163 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/mellanox/mlx5/core/cmd.c
> > b/drivers/net/ethernet/mellanox/mlx5/core/cmd.c
> > index 8cdd7e66f8df..973f90888b1f 100644
> > --- a/drivers/net/ethernet/mellanox/mlx5/core/cmd.c
> > +++ b/drivers/net/ethernet/mellanox/mlx5/core/cmd.c
> > @@ -1368,49 +1368,19 @@ static void clean_debug_files(struct
> > mlx5_core_dev *dev)
> > debugfs_remove_recursive(dbg->dbg_root);
> > }
> >
>
> [...]
>
> > void mlx5_cq_debugfs_cleanup(struct mlx5_core_dev *dev)
> > {
> > - if (!mlx5_debugfs_root)
> > - return;
> > -
> > debugfs_remove_recursive(dev->priv.cq_debugfs);
> > }
> >
> > @@ -484,7 +418,6 @@ static int add_res_tree(struct mlx5_core_dev
>
> Basically this function is a debugfs wrapper that should behave the
> same as debug_fs_*, we should fix it to return void as well, and
> improve all the its callers to ignore the return value.
But mlx5_cq_debugfs_cleanup() is a void function.
> callers are:
> mlx5_debug_qp_add()
> mlx5_debug_eq_add()
> mlx5_debug_cq_add()
Ah, you mean add_res_tree(). Yes, make that void as well.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 03/17] mlx5: no need to check return value of debugfs_create functions
2019-08-06 16:11 ` [PATCH 03/17] mlx5: no need to check return value of debugfs_create functions Greg Kroah-Hartman
2019-08-06 19:41 ` Saeed Mahameed
@ 2019-08-06 20:37 ` Saeed Mahameed
1 sibling, 0 replies; 32+ messages in thread
From: Saeed Mahameed @ 2019-08-06 20:37 UTC (permalink / raw)
To: gregkh, netdev; +Cc: leon
On Tue, 2019-08-06 at 18:11 +0200, 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.
>
> This cleans up a lot of unneeded code and logic around the debugfs
> files, making all of this much simpler and easier to understand as we
> don't need to keep the dentries saved anymore.
>
> Cc: Saeed Mahameed <saeedm@mellanox.com>
> Cc: Leon Romanovsky <leon@kernel.org>
> Cc: netdev@vger.kernel.org
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Acked-by: Saeed Mahameed <saeedm@mellanox.com>
Some unexpected/undefined driver behavior might occur if some of the
debug_fs_* calls should fail in this driver, I will follow up with a
patch to address this.
Thanks,
Saeed.
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH 04/17] xgbe: no need to check return value of debugfs_create functions
2019-08-06 16:11 [PATCH 00/17] Networking driver debugfs cleanups Greg Kroah-Hartman
` (2 preceding siblings ...)
2019-08-06 16:11 ` [PATCH 03/17] mlx5: no need to check return value of debugfs_create functions Greg Kroah-Hartman
@ 2019-08-06 16:11 ` Greg Kroah-Hartman
2019-08-06 16:28 ` Lendacky, Thomas
2019-08-06 16:11 ` [PATCH 05/17] bnxt: " Greg Kroah-Hartman
` (13 subsequent siblings)
17 siblings, 1 reply; 32+ messages in thread
From: Greg Kroah-Hartman @ 2019-08-06 16:11 UTC (permalink / raw)
To: netdev; +Cc: Greg Kroah-Hartman, Tom Lendacky, David S. Miller
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.
This cleans up a lot of unneeded code and logic around the debugfs
files, making all of this much simpler and easier to understand.
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: netdev@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
drivers/net/ethernet/amd/xgbe/xgbe-debugfs.c | 107 ++++++-------------
1 file changed, 31 insertions(+), 76 deletions(-)
diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-debugfs.c b/drivers/net/ethernet/amd/xgbe/xgbe-debugfs.c
index b91143947ed2..b0a6c96b6ef4 100644
--- a/drivers/net/ethernet/amd/xgbe/xgbe-debugfs.c
+++ b/drivers/net/ethernet/amd/xgbe/xgbe-debugfs.c
@@ -438,7 +438,6 @@ static const struct file_operations xi2c_reg_value_fops = {
void xgbe_debugfs_init(struct xgbe_prv_data *pdata)
{
- struct dentry *pfile;
char *buf;
/* Set defaults */
@@ -451,88 +450,48 @@ void xgbe_debugfs_init(struct xgbe_prv_data *pdata)
return;
pdata->xgbe_debugfs = debugfs_create_dir(buf, NULL);
- if (!pdata->xgbe_debugfs) {
- netdev_err(pdata->netdev, "debugfs_create_dir failed\n");
- kfree(buf);
- return;
- }
- pfile = debugfs_create_file("xgmac_register", 0600,
- pdata->xgbe_debugfs, pdata,
- &xgmac_reg_addr_fops);
- if (!pfile)
- netdev_err(pdata->netdev, "debugfs_create_file failed\n");
+ debugfs_create_file("xgmac_register", 0600, pdata->xgbe_debugfs, pdata,
+ &xgmac_reg_addr_fops);
- pfile = debugfs_create_file("xgmac_register_value", 0600,
- pdata->xgbe_debugfs, pdata,
- &xgmac_reg_value_fops);
- if (!pfile)
- netdev_err(pdata->netdev, "debugfs_create_file failed\n");
+ debugfs_create_file("xgmac_register_value", 0600, pdata->xgbe_debugfs,
+ pdata, &xgmac_reg_value_fops);
- pfile = debugfs_create_file("xpcs_mmd", 0600,
- pdata->xgbe_debugfs, pdata,
- &xpcs_mmd_fops);
- if (!pfile)
- netdev_err(pdata->netdev, "debugfs_create_file failed\n");
+ debugfs_create_file("xpcs_mmd", 0600, pdata->xgbe_debugfs, pdata,
+ &xpcs_mmd_fops);
- pfile = debugfs_create_file("xpcs_register", 0600,
- pdata->xgbe_debugfs, pdata,
- &xpcs_reg_addr_fops);
- if (!pfile)
- netdev_err(pdata->netdev, "debugfs_create_file failed\n");
+ debugfs_create_file("xpcs_register", 0600, pdata->xgbe_debugfs, pdata,
+ &xpcs_reg_addr_fops);
- pfile = debugfs_create_file("xpcs_register_value", 0600,
- pdata->xgbe_debugfs, pdata,
- &xpcs_reg_value_fops);
- if (!pfile)
- netdev_err(pdata->netdev, "debugfs_create_file failed\n");
+ debugfs_create_file("xpcs_register_value", 0600, pdata->xgbe_debugfs,
+ pdata, &xpcs_reg_value_fops);
if (pdata->xprop_regs) {
- pfile = debugfs_create_file("xprop_register", 0600,
- pdata->xgbe_debugfs, pdata,
- &xprop_reg_addr_fops);
- if (!pfile)
- netdev_err(pdata->netdev,
- "debugfs_create_file failed\n");
-
- pfile = debugfs_create_file("xprop_register_value", 0600,
- pdata->xgbe_debugfs, pdata,
- &xprop_reg_value_fops);
- if (!pfile)
- netdev_err(pdata->netdev,
- "debugfs_create_file failed\n");
+ debugfs_create_file("xprop_register", 0600, pdata->xgbe_debugfs,
+ pdata, &xprop_reg_addr_fops);
+
+ debugfs_create_file("xprop_register_value", 0600,
+ pdata->xgbe_debugfs, pdata,
+ &xprop_reg_value_fops);
}
if (pdata->xi2c_regs) {
- pfile = debugfs_create_file("xi2c_register", 0600,
- pdata->xgbe_debugfs, pdata,
- &xi2c_reg_addr_fops);
- if (!pfile)
- netdev_err(pdata->netdev,
- "debugfs_create_file failed\n");
-
- pfile = debugfs_create_file("xi2c_register_value", 0600,
- pdata->xgbe_debugfs, pdata,
- &xi2c_reg_value_fops);
- if (!pfile)
- netdev_err(pdata->netdev,
- "debugfs_create_file failed\n");
+ debugfs_create_file("xi2c_register", 0600, pdata->xgbe_debugfs,
+ pdata, &xi2c_reg_addr_fops);
+
+ debugfs_create_file("xi2c_register_value", 0600,
+ pdata->xgbe_debugfs, pdata,
+ &xi2c_reg_value_fops);
}
if (pdata->vdata->an_cdr_workaround) {
- pfile = debugfs_create_bool("an_cdr_workaround", 0600,
- pdata->xgbe_debugfs,
- &pdata->debugfs_an_cdr_workaround);
- if (!pfile)
- netdev_err(pdata->netdev,
- "debugfs_create_bool failed\n");
-
- pfile = debugfs_create_bool("an_cdr_track_early", 0600,
- pdata->xgbe_debugfs,
- &pdata->debugfs_an_cdr_track_early);
- if (!pfile)
- netdev_err(pdata->netdev,
- "debugfs_create_bool failed\n");
+ debugfs_create_bool("an_cdr_workaround", 0600,
+ pdata->xgbe_debugfs,
+ &pdata->debugfs_an_cdr_workaround);
+
+ debugfs_create_bool("an_cdr_track_early", 0600,
+ pdata->xgbe_debugfs,
+ &pdata->debugfs_an_cdr_track_early);
}
kfree(buf);
@@ -546,7 +505,6 @@ void xgbe_debugfs_exit(struct xgbe_prv_data *pdata)
void xgbe_debugfs_rename(struct xgbe_prv_data *pdata)
{
- struct dentry *pfile;
char *buf;
if (!pdata->xgbe_debugfs)
@@ -559,11 +517,8 @@ void xgbe_debugfs_rename(struct xgbe_prv_data *pdata)
if (!strcmp(pdata->xgbe_debugfs->d_name.name, buf))
goto out;
- pfile = debugfs_rename(pdata->xgbe_debugfs->d_parent,
- pdata->xgbe_debugfs,
- pdata->xgbe_debugfs->d_parent, buf);
- if (!pfile)
- netdev_err(pdata->netdev, "debugfs_rename failed\n");
+ debugfs_rename(pdata->xgbe_debugfs->d_parent, pdata->xgbe_debugfs,
+ pdata->xgbe_debugfs->d_parent, buf);
out:
kfree(buf);
--
2.22.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH 04/17] xgbe: no need to check return value of debugfs_create functions
2019-08-06 16:11 ` [PATCH 04/17] xgbe: " Greg Kroah-Hartman
@ 2019-08-06 16:28 ` Lendacky, Thomas
0 siblings, 0 replies; 32+ messages in thread
From: Lendacky, Thomas @ 2019-08-06 16:28 UTC (permalink / raw)
To: Greg Kroah-Hartman, netdev; +Cc: David S. Miller
On 8/6/19 11:11 AM, 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.
>
> This cleans up a lot of unneeded code and logic around the debugfs
> files, making all of this much simpler and easier to understand.
>
> Cc: Tom Lendacky <thomas.lendacky@amd.com>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: netdev@vger.kernel.org
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Acked-by: Tom Lendacky <thomas.lendacky@amd.com>
> ---
> drivers/net/ethernet/amd/xgbe/xgbe-debugfs.c | 107 ++++++-------------
> 1 file changed, 31 insertions(+), 76 deletions(-)
>
> diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-debugfs.c b/drivers/net/ethernet/amd/xgbe/xgbe-debugfs.c
> index b91143947ed2..b0a6c96b6ef4 100644
> --- a/drivers/net/ethernet/amd/xgbe/xgbe-debugfs.c
> +++ b/drivers/net/ethernet/amd/xgbe/xgbe-debugfs.c
> @@ -438,7 +438,6 @@ static const struct file_operations xi2c_reg_value_fops = {
>
> void xgbe_debugfs_init(struct xgbe_prv_data *pdata)
> {
> - struct dentry *pfile;
> char *buf;
>
> /* Set defaults */
> @@ -451,88 +450,48 @@ void xgbe_debugfs_init(struct xgbe_prv_data *pdata)
> return;
>
> pdata->xgbe_debugfs = debugfs_create_dir(buf, NULL);
> - if (!pdata->xgbe_debugfs) {
> - netdev_err(pdata->netdev, "debugfs_create_dir failed\n");
> - kfree(buf);
> - return;
> - }
>
> - pfile = debugfs_create_file("xgmac_register", 0600,
> - pdata->xgbe_debugfs, pdata,
> - &xgmac_reg_addr_fops);
> - if (!pfile)
> - netdev_err(pdata->netdev, "debugfs_create_file failed\n");
> + debugfs_create_file("xgmac_register", 0600, pdata->xgbe_debugfs, pdata,
> + &xgmac_reg_addr_fops);
>
> - pfile = debugfs_create_file("xgmac_register_value", 0600,
> - pdata->xgbe_debugfs, pdata,
> - &xgmac_reg_value_fops);
> - if (!pfile)
> - netdev_err(pdata->netdev, "debugfs_create_file failed\n");
> + debugfs_create_file("xgmac_register_value", 0600, pdata->xgbe_debugfs,
> + pdata, &xgmac_reg_value_fops);
>
> - pfile = debugfs_create_file("xpcs_mmd", 0600,
> - pdata->xgbe_debugfs, pdata,
> - &xpcs_mmd_fops);
> - if (!pfile)
> - netdev_err(pdata->netdev, "debugfs_create_file failed\n");
> + debugfs_create_file("xpcs_mmd", 0600, pdata->xgbe_debugfs, pdata,
> + &xpcs_mmd_fops);
>
> - pfile = debugfs_create_file("xpcs_register", 0600,
> - pdata->xgbe_debugfs, pdata,
> - &xpcs_reg_addr_fops);
> - if (!pfile)
> - netdev_err(pdata->netdev, "debugfs_create_file failed\n");
> + debugfs_create_file("xpcs_register", 0600, pdata->xgbe_debugfs, pdata,
> + &xpcs_reg_addr_fops);
>
> - pfile = debugfs_create_file("xpcs_register_value", 0600,
> - pdata->xgbe_debugfs, pdata,
> - &xpcs_reg_value_fops);
> - if (!pfile)
> - netdev_err(pdata->netdev, "debugfs_create_file failed\n");
> + debugfs_create_file("xpcs_register_value", 0600, pdata->xgbe_debugfs,
> + pdata, &xpcs_reg_value_fops);
>
> if (pdata->xprop_regs) {
> - pfile = debugfs_create_file("xprop_register", 0600,
> - pdata->xgbe_debugfs, pdata,
> - &xprop_reg_addr_fops);
> - if (!pfile)
> - netdev_err(pdata->netdev,
> - "debugfs_create_file failed\n");
> -
> - pfile = debugfs_create_file("xprop_register_value", 0600,
> - pdata->xgbe_debugfs, pdata,
> - &xprop_reg_value_fops);
> - if (!pfile)
> - netdev_err(pdata->netdev,
> - "debugfs_create_file failed\n");
> + debugfs_create_file("xprop_register", 0600, pdata->xgbe_debugfs,
> + pdata, &xprop_reg_addr_fops);
> +
> + debugfs_create_file("xprop_register_value", 0600,
> + pdata->xgbe_debugfs, pdata,
> + &xprop_reg_value_fops);
> }
>
> if (pdata->xi2c_regs) {
> - pfile = debugfs_create_file("xi2c_register", 0600,
> - pdata->xgbe_debugfs, pdata,
> - &xi2c_reg_addr_fops);
> - if (!pfile)
> - netdev_err(pdata->netdev,
> - "debugfs_create_file failed\n");
> -
> - pfile = debugfs_create_file("xi2c_register_value", 0600,
> - pdata->xgbe_debugfs, pdata,
> - &xi2c_reg_value_fops);
> - if (!pfile)
> - netdev_err(pdata->netdev,
> - "debugfs_create_file failed\n");
> + debugfs_create_file("xi2c_register", 0600, pdata->xgbe_debugfs,
> + pdata, &xi2c_reg_addr_fops);
> +
> + debugfs_create_file("xi2c_register_value", 0600,
> + pdata->xgbe_debugfs, pdata,
> + &xi2c_reg_value_fops);
> }
>
> if (pdata->vdata->an_cdr_workaround) {
> - pfile = debugfs_create_bool("an_cdr_workaround", 0600,
> - pdata->xgbe_debugfs,
> - &pdata->debugfs_an_cdr_workaround);
> - if (!pfile)
> - netdev_err(pdata->netdev,
> - "debugfs_create_bool failed\n");
> -
> - pfile = debugfs_create_bool("an_cdr_track_early", 0600,
> - pdata->xgbe_debugfs,
> - &pdata->debugfs_an_cdr_track_early);
> - if (!pfile)
> - netdev_err(pdata->netdev,
> - "debugfs_create_bool failed\n");
> + debugfs_create_bool("an_cdr_workaround", 0600,
> + pdata->xgbe_debugfs,
> + &pdata->debugfs_an_cdr_workaround);
> +
> + debugfs_create_bool("an_cdr_track_early", 0600,
> + pdata->xgbe_debugfs,
> + &pdata->debugfs_an_cdr_track_early);
> }
>
> kfree(buf);
> @@ -546,7 +505,6 @@ void xgbe_debugfs_exit(struct xgbe_prv_data *pdata)
>
> void xgbe_debugfs_rename(struct xgbe_prv_data *pdata)
> {
> - struct dentry *pfile;
> char *buf;
>
> if (!pdata->xgbe_debugfs)
> @@ -559,11 +517,8 @@ void xgbe_debugfs_rename(struct xgbe_prv_data *pdata)
> if (!strcmp(pdata->xgbe_debugfs->d_name.name, buf))
> goto out;
>
> - pfile = debugfs_rename(pdata->xgbe_debugfs->d_parent,
> - pdata->xgbe_debugfs,
> - pdata->xgbe_debugfs->d_parent, buf);
> - if (!pfile)
> - netdev_err(pdata->netdev, "debugfs_rename failed\n");
> + debugfs_rename(pdata->xgbe_debugfs->d_parent, pdata->xgbe_debugfs,
> + pdata->xgbe_debugfs->d_parent, buf);
>
> out:
> kfree(buf);
>
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH 05/17] bnxt: no need to check return value of debugfs_create functions
2019-08-06 16:11 [PATCH 00/17] Networking driver debugfs cleanups Greg Kroah-Hartman
` (3 preceding siblings ...)
2019-08-06 16:11 ` [PATCH 04/17] xgbe: " Greg Kroah-Hartman
@ 2019-08-06 16:11 ` Greg Kroah-Hartman
2019-08-06 16:11 ` [PATCH 06/17] cxgb4: " Greg Kroah-Hartman
` (12 subsequent siblings)
17 siblings, 0 replies; 32+ messages in thread
From: Greg Kroah-Hartman @ 2019-08-06 16:11 UTC (permalink / raw)
To: netdev; +Cc: Greg Kroah-Hartman, Michael Chan, David S. Miller
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.
This cleans up a lot of unneeded code and logic around the debugfs
files, making all of this much simpler and easier to understand.
Cc: Michael Chan <michael.chan@broadcom.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: netdev@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
drivers/net/ethernet/broadcom/bnxt/bnxt.h | 1 -
.../net/ethernet/broadcom/bnxt/bnxt_debugfs.c | 39 ++++++-------------
2 files changed, 11 insertions(+), 29 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.h b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
index e3262089b751..1b1610d5b573 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
@@ -1724,7 +1724,6 @@ struct bnxt {
u8 switch_id[8];
struct bnxt_tc_info *tc_info;
struct dentry *debugfs_pdev;
- struct dentry *debugfs_dim;
struct device *hwmon_dev;
};
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_debugfs.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_debugfs.c
index 61393f351a77..156c2404854f 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_debugfs.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_debugfs.c
@@ -61,45 +61,30 @@ static const struct file_operations debugfs_dim_fops = {
.read = debugfs_dim_read,
};
-static struct dentry *debugfs_dim_ring_init(struct dim *dim, int ring_idx,
- struct dentry *dd)
+static void debugfs_dim_ring_init(struct dim *dim, int ring_idx,
+ struct dentry *dd)
{
static char qname[16];
snprintf(qname, 10, "%d", ring_idx);
- return debugfs_create_file(qname, 0600, dd,
- dim, &debugfs_dim_fops);
+ debugfs_create_file(qname, 0600, dd, dim, &debugfs_dim_fops);
}
void bnxt_debug_dev_init(struct bnxt *bp)
{
const char *pname = pci_name(bp->pdev);
- struct dentry *pdevf;
+ struct dentry *dir;
int i;
bp->debugfs_pdev = debugfs_create_dir(pname, bnxt_debug_mnt);
- if (bp->debugfs_pdev) {
- pdevf = debugfs_create_dir("dim", bp->debugfs_pdev);
- if (!pdevf) {
- pr_err("failed to create debugfs entry %s/dim\n",
- pname);
- return;
- }
- bp->debugfs_dim = pdevf;
- /* create files for each rx ring */
- for (i = 0; i < bp->cp_nr_rings; i++) {
- struct bnxt_cp_ring_info *cpr = &bp->bnapi[i]->cp_ring;
+ dir = debugfs_create_dir("dim", bp->debugfs_pdev);
- if (cpr && bp->bnapi[i]->rx_ring) {
- pdevf = debugfs_dim_ring_init(&cpr->dim, i,
- bp->debugfs_dim);
- if (!pdevf)
- pr_err("failed to create debugfs entry %s/dim/%d\n",
- pname, i);
- }
- }
- } else {
- pr_err("failed to create debugfs entry %s\n", pname);
+ /* create files for each rx ring */
+ for (i = 0; i < bp->cp_nr_rings; i++) {
+ struct bnxt_cp_ring_info *cpr = &bp->bnapi[i]->cp_ring;
+
+ if (cpr && bp->bnapi[i]->rx_ring)
+ debugfs_dim_ring_init(&cpr->dim, i, dir);
}
}
@@ -114,8 +99,6 @@ void bnxt_debug_dev_exit(struct bnxt *bp)
void bnxt_debug_init(void)
{
bnxt_debug_mnt = debugfs_create_dir("bnxt_en", NULL);
- if (!bnxt_debug_mnt)
- pr_err("failed to init bnxt_en debugfs\n");
}
void bnxt_debug_exit(void)
--
2.22.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH 06/17] cxgb4: no need to check return value of debugfs_create functions
2019-08-06 16:11 [PATCH 00/17] Networking driver debugfs cleanups Greg Kroah-Hartman
` (4 preceding siblings ...)
2019-08-06 16:11 ` [PATCH 05/17] bnxt: " Greg Kroah-Hartman
@ 2019-08-06 16:11 ` Greg Kroah-Hartman
2019-08-06 16:11 ` [PATCH 07/17] hns3: " Greg Kroah-Hartman
` (11 subsequent siblings)
17 siblings, 0 replies; 32+ messages in thread
From: Greg Kroah-Hartman @ 2019-08-06 16:11 UTC (permalink / raw)
To: netdev; +Cc: Greg Kroah-Hartman, Vishal Kulkarni, David S. Miller, Casey Leedom
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.
If a debugfs call fails, it will properly warn in the syslog, there's no
need for all individual drivers to also print a message, so that is one
more reason to not care about checking the return values.
Cc: Vishal Kulkarni <vishal@chelsio.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Casey Leedom <leedom@chelsio.com>
Cc: netdev@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
.../ethernet/chelsio/cxgb4/cxgb4_debugfs.c | 5 ++---
.../net/ethernet/chelsio/cxgb4/cxgb4_main.c | 3 ---
.../ethernet/chelsio/cxgb4vf/cxgb4vf_main.c | 21 +++++++------------
3 files changed, 9 insertions(+), 20 deletions(-)
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_debugfs.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_debugfs.c
index 02959035ed3f..dd99c55d9a88 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_debugfs.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_debugfs.c
@@ -3529,7 +3529,6 @@ int t4_setup_debugfs(struct adapter *adap)
{
int i;
u32 size = 0;
- struct dentry *de;
static struct t4_debugfs_entry t4_debugfs_files[] = {
{ "cim_la", &cim_la_fops, 0400, 0 },
@@ -3640,8 +3639,8 @@ int t4_setup_debugfs(struct adapter *adap)
}
}
- de = debugfs_create_file_size("flash", 0400, adap->debugfs_root, adap,
- &flash_debugfs_fops, adap->params.sf_size);
+ debugfs_create_file_size("flash", 0400, adap->debugfs_root, adap,
+ &flash_debugfs_fops, adap->params.sf_size);
debugfs_create_bool("use_backdoor", 0600,
adap->debugfs_root, &adap->use_bd);
debugfs_create_bool("trace_rss", 0600,
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
index 4311ad9c84b2..71854a19cebe 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
@@ -6269,10 +6269,7 @@ static int __init cxgb4_init_module(void)
{
int ret;
- /* Debugfs support is optional, just warn if this fails */
cxgb4_debugfs_root = debugfs_create_dir(KBUILD_MODNAME, NULL);
- if (!cxgb4_debugfs_root)
- pr_warn("could not create debugfs entry, continuing\n");
ret = pci_register_driver(&cxgb4_driver);
if (ret < 0)
diff --git a/drivers/net/ethernet/chelsio/cxgb4vf/cxgb4vf_main.c b/drivers/net/ethernet/chelsio/cxgb4vf/cxgb4vf_main.c
index 6d4cf3d0b2f0..f6fc0875d5b0 100644
--- a/drivers/net/ethernet/chelsio/cxgb4vf/cxgb4vf_main.c
+++ b/drivers/net/ethernet/chelsio/cxgb4vf/cxgb4vf_main.c
@@ -2478,11 +2478,10 @@ static int setup_debugfs(struct adapter *adapter)
* Debugfs support is best effort.
*/
for (i = 0; i < ARRAY_SIZE(debugfs_files); i++)
- (void)debugfs_create_file(debugfs_files[i].name,
- debugfs_files[i].mode,
- adapter->debugfs_root,
- (void *)adapter,
- debugfs_files[i].fops);
+ debugfs_create_file(debugfs_files[i].name,
+ debugfs_files[i].mode,
+ adapter->debugfs_root, (void *)adapter,
+ debugfs_files[i].fops);
return 0;
}
@@ -3257,11 +3256,7 @@ static int cxgb4vf_pci_probe(struct pci_dev *pdev,
adapter->debugfs_root =
debugfs_create_dir(pci_name(pdev),
cxgb4vf_debugfs_root);
- if (IS_ERR_OR_NULL(adapter->debugfs_root))
- dev_warn(&pdev->dev, "could not create debugfs"
- " directory");
- else
- setup_debugfs(adapter);
+ setup_debugfs(adapter);
}
/*
@@ -3486,13 +3481,11 @@ static int __init cxgb4vf_module_init(void)
return -EINVAL;
}
- /* Debugfs support is optional, just warn if this fails */
+ /* Debugfs support is optional, debugfs will warn if this fails */
cxgb4vf_debugfs_root = debugfs_create_dir(KBUILD_MODNAME, NULL);
- if (IS_ERR_OR_NULL(cxgb4vf_debugfs_root))
- pr_warn("could not create debugfs entry, continuing\n");
ret = pci_register_driver(&cxgb4vf_driver);
- if (ret < 0 && !IS_ERR_OR_NULL(cxgb4vf_debugfs_root))
+ if (ret < 0)
debugfs_remove(cxgb4vf_debugfs_root);
return ret;
}
--
2.22.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH 07/17] hns3: no need to check return value of debugfs_create functions
2019-08-06 16:11 [PATCH 00/17] Networking driver debugfs cleanups Greg Kroah-Hartman
` (5 preceding siblings ...)
2019-08-06 16:11 ` [PATCH 06/17] cxgb4: " Greg Kroah-Hartman
@ 2019-08-06 16:11 ` Greg Kroah-Hartman
2019-08-06 16:11 ` [PATCH 08/17] nfp: " Greg Kroah-Hartman
` (10 subsequent siblings)
17 siblings, 0 replies; 32+ messages in thread
From: Greg Kroah-Hartman @ 2019-08-06 16:11 UTC (permalink / raw)
To: netdev; +Cc: Greg Kroah-Hartman, Yisen Zhuang, Salil Mehta, David S. Miller
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: Yisen Zhuang <yisen.zhuang@huawei.com>
Cc: Salil Mehta <salil.mehta@huawei.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: netdev@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
.../net/ethernet/hisilicon/hns3/hns3_debugfs.c | 17 ++---------------
1 file changed, 2 insertions(+), 15 deletions(-)
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_debugfs.c b/drivers/net/ethernet/hisilicon/hns3/hns3_debugfs.c
index a4b937286f55..2e79380502e9 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3_debugfs.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3_debugfs.c
@@ -372,20 +372,11 @@ static const struct file_operations hns3_dbg_cmd_fops = {
void hns3_dbg_init(struct hnae3_handle *handle)
{
const char *name = pci_name(handle->pdev);
- struct dentry *pfile;
handle->hnae3_dbgfs = debugfs_create_dir(name, hns3_dbgfs_root);
- if (!handle->hnae3_dbgfs)
- return;
- pfile = debugfs_create_file("cmd", 0600, handle->hnae3_dbgfs, handle,
- &hns3_dbg_cmd_fops);
- if (!pfile) {
- debugfs_remove_recursive(handle->hnae3_dbgfs);
- handle->hnae3_dbgfs = NULL;
- dev_warn(&handle->pdev->dev, "create file for %s fail\n",
- name);
- }
+ debugfs_create_file("cmd", 0600, handle->hnae3_dbgfs, handle,
+ &hns3_dbg_cmd_fops);
}
void hns3_dbg_uninit(struct hnae3_handle *handle)
@@ -397,10 +388,6 @@ void hns3_dbg_uninit(struct hnae3_handle *handle)
void hns3_dbg_register_debugfs(const char *debugfs_dir_name)
{
hns3_dbgfs_root = debugfs_create_dir(debugfs_dir_name, NULL);
- if (!hns3_dbgfs_root) {
- pr_warn("Register debugfs for %s fail\n", debugfs_dir_name);
- return;
- }
}
void hns3_dbg_unregister_debugfs(void)
--
2.22.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH 08/17] nfp: no need to check return value of debugfs_create functions
2019-08-06 16:11 [PATCH 00/17] Networking driver debugfs cleanups Greg Kroah-Hartman
` (6 preceding siblings ...)
2019-08-06 16:11 ` [PATCH 07/17] hns3: " Greg Kroah-Hartman
@ 2019-08-06 16:11 ` Greg Kroah-Hartman
2019-08-06 16:50 ` Jakub Kicinski
2019-08-06 16:11 ` [PATCH 09/17] stmmac: " Greg Kroah-Hartman
` (9 subsequent siblings)
17 siblings, 1 reply; 32+ messages in thread
From: Greg Kroah-Hartman @ 2019-08-06 16:11 UTC (permalink / raw)
To: netdev
Cc: Greg Kroah-Hartman, Jakub Kicinski, David S. Miller,
Alexei Starovoitov, Daniel Borkmann, Jesper Dangaard Brouer,
John Fastabend, Edwin Peer, Yangtao Li, Simon Horman,
oss-drivers
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: Jakub Kicinski <jakub.kicinski@netronome.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: Jesper Dangaard Brouer <hawk@kernel.org>
Cc: John Fastabend <john.fastabend@gmail.com>
Cc: Edwin Peer <edwin.peer@netronome.com>
Cc: Yangtao Li <tiny.windzz@gmail.com>
Cc: Simon Horman <simon.horman@netronome.com>
Cc: oss-drivers@netronome.com
Cc: netdev@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
.../ethernet/netronome/nfp/nfp_net_debugfs.c | 17 +----------------
1 file changed, 1 insertion(+), 16 deletions(-)
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_debugfs.c b/drivers/net/ethernet/netronome/nfp/nfp_net_debugfs.c
index ab7f2498e1c4..553c708694e8 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_debugfs.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_debugfs.c
@@ -159,19 +159,13 @@ void nfp_net_debugfs_vnic_add(struct nfp_net *nn, struct dentry *ddir)
else
strcpy(name, "ctrl-vnic");
nn->debugfs_dir = debugfs_create_dir(name, ddir);
- if (IS_ERR_OR_NULL(nn->debugfs_dir))
- return;
/* Create queue debugging sub-tree */
queues = debugfs_create_dir("queue", nn->debugfs_dir);
- if (IS_ERR_OR_NULL(queues))
- return;
rx = debugfs_create_dir("rx", queues);
tx = debugfs_create_dir("tx", queues);
xdp = debugfs_create_dir("xdp", queues);
- if (IS_ERR_OR_NULL(rx) || IS_ERR_OR_NULL(tx) || IS_ERR_OR_NULL(xdp))
- return;
for (i = 0; i < min(nn->max_rx_rings, nn->max_r_vecs); i++) {
sprintf(name, "%d", i);
@@ -190,16 +184,7 @@ void nfp_net_debugfs_vnic_add(struct nfp_net *nn, struct dentry *ddir)
struct dentry *nfp_net_debugfs_device_add(struct pci_dev *pdev)
{
- struct dentry *dev_dir;
-
- if (IS_ERR_OR_NULL(nfp_dir))
- return NULL;
-
- dev_dir = debugfs_create_dir(pci_name(pdev), nfp_dir);
- if (IS_ERR_OR_NULL(dev_dir))
- return NULL;
-
- return dev_dir;
+ return debugfs_create_dir(pci_name(pdev), nfp_dir);
}
void nfp_net_debugfs_dir_clean(struct dentry **dir)
--
2.22.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH 08/17] nfp: no need to check return value of debugfs_create functions
2019-08-06 16:11 ` [PATCH 08/17] nfp: " Greg Kroah-Hartman
@ 2019-08-06 16:50 ` Jakub Kicinski
2019-08-06 17:00 ` Greg Kroah-Hartman
0 siblings, 1 reply; 32+ messages in thread
From: Jakub Kicinski @ 2019-08-06 16:50 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: netdev, David S. Miller, Alexei Starovoitov, Daniel Borkmann,
Jesper Dangaard Brouer, John Fastabend, Edwin Peer, Yangtao Li,
Simon Horman, oss-drivers
On Tue, 6 Aug 2019 18:11:19 +0200, 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: Jakub Kicinski <jakub.kicinski@netronome.com>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Alexei Starovoitov <ast@kernel.org>
> Cc: Daniel Borkmann <daniel@iogearbox.net>
> Cc: Jesper Dangaard Brouer <hawk@kernel.org>
> Cc: John Fastabend <john.fastabend@gmail.com>
> Cc: Edwin Peer <edwin.peer@netronome.com>
> Cc: Yangtao Li <tiny.windzz@gmail.com>
> Cc: Simon Horman <simon.horman@netronome.com>
> Cc: oss-drivers@netronome.com
> Cc: netdev@vger.kernel.org
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Acked-by: Jakub Kicinski <jakub.kicinski@netronome.com>
I take it this is the case since commit ff9fb72bc077 ("debugfs: return
error values, not NULL")? I.e. v5.0? It'd be useful to know for backport
purposes.
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH 08/17] nfp: no need to check return value of debugfs_create functions
2019-08-06 16:50 ` Jakub Kicinski
@ 2019-08-06 17:00 ` Greg Kroah-Hartman
2019-08-06 17:30 ` Jakub Kicinski
0 siblings, 1 reply; 32+ messages in thread
From: Greg Kroah-Hartman @ 2019-08-06 17:00 UTC (permalink / raw)
To: Jakub Kicinski
Cc: netdev, David S. Miller, Alexei Starovoitov, Daniel Borkmann,
Jesper Dangaard Brouer, John Fastabend, Edwin Peer, Yangtao Li,
Simon Horman, oss-drivers
On Tue, Aug 06, 2019 at 09:50:08AM -0700, Jakub Kicinski wrote:
> On Tue, 6 Aug 2019 18:11:19 +0200, 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: Jakub Kicinski <jakub.kicinski@netronome.com>
> > Cc: "David S. Miller" <davem@davemloft.net>
> > Cc: Alexei Starovoitov <ast@kernel.org>
> > Cc: Daniel Borkmann <daniel@iogearbox.net>
> > Cc: Jesper Dangaard Brouer <hawk@kernel.org>
> > Cc: John Fastabend <john.fastabend@gmail.com>
> > Cc: Edwin Peer <edwin.peer@netronome.com>
> > Cc: Yangtao Li <tiny.windzz@gmail.com>
> > Cc: Simon Horman <simon.horman@netronome.com>
> > Cc: oss-drivers@netronome.com
> > Cc: netdev@vger.kernel.org
> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>
> Acked-by: Jakub Kicinski <jakub.kicinski@netronome.com>
>
> I take it this is the case since commit ff9fb72bc077 ("debugfs: return
> error values, not NULL")? I.e. v5.0? It'd be useful to know for backport
> purposes.
You were always safe to ignore debugfs calls before that, but in 5.0 and
then 5.2 we got a bit more "robust" with some internal debugfs logic to
make it even easier. These can be backported to 2.6.11+ if you really
want to, no functionality should change.
But why would you want to backport them? This really isn't a "bugfix"
for a stable kernel. No one should ever noticed the difference except
for less memory being used.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH 08/17] nfp: no need to check return value of debugfs_create functions
2019-08-06 17:00 ` Greg Kroah-Hartman
@ 2019-08-06 17:30 ` Jakub Kicinski
0 siblings, 0 replies; 32+ messages in thread
From: Jakub Kicinski @ 2019-08-06 17:30 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: netdev, David S. Miller, Alexei Starovoitov, Daniel Borkmann,
Jesper Dangaard Brouer, John Fastabend, Edwin Peer, Yangtao Li,
Simon Horman, oss-drivers
On Tue, 6 Aug 2019 19:00:49 +0200, Greg Kroah-Hartman wrote:
> On Tue, Aug 06, 2019 at 09:50:08AM -0700, Jakub Kicinski wrote:
> > On Tue, 6 Aug 2019 18:11:19 +0200, 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: Jakub Kicinski <jakub.kicinski@netronome.com>
> > > Cc: "David S. Miller" <davem@davemloft.net>
> > > Cc: Alexei Starovoitov <ast@kernel.org>
> > > Cc: Daniel Borkmann <daniel@iogearbox.net>
> > > Cc: Jesper Dangaard Brouer <hawk@kernel.org>
> > > Cc: John Fastabend <john.fastabend@gmail.com>
> > > Cc: Edwin Peer <edwin.peer@netronome.com>
> > > Cc: Yangtao Li <tiny.windzz@gmail.com>
> > > Cc: Simon Horman <simon.horman@netronome.com>
> > > Cc: oss-drivers@netronome.com
> > > Cc: netdev@vger.kernel.org
> > > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> >
> > Acked-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> >
> > I take it this is the case since commit ff9fb72bc077 ("debugfs: return
> > error values, not NULL")? I.e. v5.0? It'd be useful to know for backport
> > purposes.
>
> You were always safe to ignore debugfs calls before that, but in 5.0 and
> then 5.2 we got a bit more "robust" with some internal debugfs logic to
> make it even easier. These can be backported to 2.6.11+ if you really
> want to, no functionality should change.
Oh sorry! I meant vendor out-of-tree driver backport. We all maintain
a tarball version of the drivers that compile on old kernels, I was
mostly wondering from that perspective.
> But why would you want to backport them? This really isn't a "bugfix"
> for a stable kernel. No one should ever noticed the difference except
> for less memory being used.
Right, it wouldn't really help to do an upstream backport.
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH 09/17] stmmac: no need to check return value of debugfs_create functions
2019-08-06 16:11 [PATCH 00/17] Networking driver debugfs cleanups Greg Kroah-Hartman
` (7 preceding siblings ...)
2019-08-06 16:11 ` [PATCH 08/17] nfp: " Greg Kroah-Hartman
@ 2019-08-06 16:11 ` Greg Kroah-Hartman
2019-08-06 16:11 ` [PATCH 10/17] dpaa2: " Greg Kroah-Hartman
` (8 subsequent siblings)
17 siblings, 0 replies; 32+ messages in thread
From: Greg Kroah-Hartman @ 2019-08-06 16:11 UTC (permalink / raw)
To: netdev
Cc: Greg Kroah-Hartman, Giuseppe Cavallaro, Alexandre Torgue,
Jose Abreu, David S. Miller, Maxime Coquelin, linux-stm32
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.
Because we don't care about the individual files, we can remove the
stored dentry for the files, as they are not needed to be kept track of
at all.
Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Cc: Alexandre Torgue <alexandre.torgue@st.com>
Cc: Jose Abreu <joabreu@synopsys.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com>
Cc: netdev@vger.kernel.org
Cc: linux-stm32@st-md-mailman.stormreply.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
drivers/net/ethernet/stmicro/stmmac/stmmac.h | 2 -
.../net/ethernet/stmicro/stmmac/stmmac_main.c | 52 +++----------------
2 files changed, 8 insertions(+), 46 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac.h b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
index 5cd966c154f3..fcc68782f8f8 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac.h
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
@@ -188,8 +188,6 @@ struct stmmac_priv {
#ifdef CONFIG_DEBUG_FS
struct dentry *dbgfs_dir;
- struct dentry *dbgfs_rings_status;
- struct dentry *dbgfs_dma_cap;
#endif
unsigned long state;
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index c7c9e5f162e6..f8a8e88ab05b 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -105,7 +105,7 @@ MODULE_PARM_DESC(chain_mode, "To use chain instead of ring mode");
static irqreturn_t stmmac_interrupt(int irq, void *dev_id);
#ifdef CONFIG_DEBUG_FS
-static int stmmac_init_fs(struct net_device *dev);
+static void stmmac_init_fs(struct net_device *dev);
static void stmmac_exit_fs(struct net_device *dev);
#endif
@@ -3961,45 +3961,20 @@ static int stmmac_dma_cap_show(struct seq_file *seq, void *v)
}
DEFINE_SHOW_ATTRIBUTE(stmmac_dma_cap);
-static int stmmac_init_fs(struct net_device *dev)
+static void stmmac_init_fs(struct net_device *dev)
{
struct stmmac_priv *priv = netdev_priv(dev);
/* Create per netdev entries */
priv->dbgfs_dir = debugfs_create_dir(dev->name, stmmac_fs_dir);
- if (!priv->dbgfs_dir || IS_ERR(priv->dbgfs_dir)) {
- netdev_err(priv->dev, "ERROR failed to create debugfs directory\n");
-
- return -ENOMEM;
- }
-
/* Entry to report DMA RX/TX rings */
- priv->dbgfs_rings_status =
- debugfs_create_file("descriptors_status", 0444,
- priv->dbgfs_dir, dev,
- &stmmac_rings_status_fops);
-
- if (!priv->dbgfs_rings_status || IS_ERR(priv->dbgfs_rings_status)) {
- netdev_err(priv->dev, "ERROR creating stmmac ring debugfs file\n");
- debugfs_remove_recursive(priv->dbgfs_dir);
-
- return -ENOMEM;
- }
+ debugfs_create_file("descriptors_status", 0444, priv->dbgfs_dir, dev,
+ &stmmac_rings_status_fops);
/* Entry to report the DMA HW features */
- priv->dbgfs_dma_cap = debugfs_create_file("dma_cap", 0444,
- priv->dbgfs_dir,
- dev, &stmmac_dma_cap_fops);
-
- if (!priv->dbgfs_dma_cap || IS_ERR(priv->dbgfs_dma_cap)) {
- netdev_err(priv->dev, "ERROR creating stmmac MMC debugfs file\n");
- debugfs_remove_recursive(priv->dbgfs_dir);
-
- return -ENOMEM;
- }
-
- return 0;
+ debugfs_create_file("dma_cap", 0444, priv->dbgfs_dir, dev,
+ &stmmac_dma_cap_fops);
}
static void stmmac_exit_fs(struct net_device *dev)
@@ -4366,10 +4341,7 @@ int stmmac_dvr_probe(struct device *device,
}
#ifdef CONFIG_DEBUG_FS
- ret = stmmac_init_fs(ndev);
- if (ret < 0)
- netdev_warn(priv->dev, "%s: failed debugFS registration\n",
- __func__);
+ stmmac_init_fs(ndev);
#endif
return ret;
@@ -4615,16 +4587,8 @@ static int __init stmmac_init(void)
{
#ifdef CONFIG_DEBUG_FS
/* Create debugfs main directory if it doesn't exist yet */
- if (!stmmac_fs_dir) {
+ if (!stmmac_fs_dir)
stmmac_fs_dir = debugfs_create_dir(STMMAC_RESOURCE_NAME, NULL);
-
- if (!stmmac_fs_dir || IS_ERR(stmmac_fs_dir)) {
- pr_err("ERROR %s, debugfs create directory failed\n",
- STMMAC_RESOURCE_NAME);
-
- return -ENOMEM;
- }
- }
#endif
return 0;
--
2.22.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH 10/17] dpaa2: no need to check return value of debugfs_create functions
2019-08-06 16:11 [PATCH 00/17] Networking driver debugfs cleanups Greg Kroah-Hartman
` (8 preceding siblings ...)
2019-08-06 16:11 ` [PATCH 09/17] stmmac: " Greg Kroah-Hartman
@ 2019-08-06 16:11 ` Greg Kroah-Hartman
2019-08-06 16:11 ` [PATCH 11/17] qca: " Greg Kroah-Hartman
` (7 subsequent siblings)
17 siblings, 0 replies; 32+ messages in thread
From: Greg Kroah-Hartman @ 2019-08-06 16:11 UTC (permalink / raw)
To: netdev; +Cc: Greg Kroah-Hartman, Ioana Radulescu, David S. Miller
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.
Because we don't care about the individual files, we can remove the
stored dentry for the files, as they are not needed to be kept track of
at all.
Cc: Ioana Radulescu <ruxandra.radulescu@nxp.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: netdev@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
.../freescale/dpaa2/dpaa2-eth-debugfs.c | 54 +++----------------
.../freescale/dpaa2/dpaa2-eth-debugfs.h | 3 --
2 files changed, 7 insertions(+), 50 deletions(-)
diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth-debugfs.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth-debugfs.c
index a027f4a9d0cc..a9afe46b837f 100644
--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth-debugfs.c
+++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth-debugfs.c
@@ -164,70 +164,30 @@ static const struct file_operations dpaa2_dbg_ch_ops = {
void dpaa2_dbg_add(struct dpaa2_eth_priv *priv)
{
- if (!dpaa2_dbg_root)
- return;
+ struct dentry *dir;
/* Create a directory for the interface */
- priv->dbg.dir = debugfs_create_dir(priv->net_dev->name,
- dpaa2_dbg_root);
- if (!priv->dbg.dir) {
- netdev_err(priv->net_dev, "debugfs_create_dir() failed\n");
- return;
- }
+ dir = debugfs_create_dir(priv->net_dev->name, dpaa2_dbg_root);
+ priv->dbg.dir = dir;
/* per-cpu stats file */
- priv->dbg.cpu_stats = debugfs_create_file("cpu_stats", 0444,
- priv->dbg.dir, priv,
- &dpaa2_dbg_cpu_ops);
- if (!priv->dbg.cpu_stats) {
- netdev_err(priv->net_dev, "debugfs_create_file() failed\n");
- goto err_cpu_stats;
- }
+ debugfs_create_file("cpu_stats", 0444, dir, priv, &dpaa2_dbg_cpu_ops);
/* per-fq stats file */
- priv->dbg.fq_stats = debugfs_create_file("fq_stats", 0444,
- priv->dbg.dir, priv,
- &dpaa2_dbg_fq_ops);
- if (!priv->dbg.fq_stats) {
- netdev_err(priv->net_dev, "debugfs_create_file() failed\n");
- goto err_fq_stats;
- }
+ debugfs_create_file("fq_stats", 0444, dir, priv, &dpaa2_dbg_fq_ops);
/* per-fq stats file */
- priv->dbg.ch_stats = debugfs_create_file("ch_stats", 0444,
- priv->dbg.dir, priv,
- &dpaa2_dbg_ch_ops);
- if (!priv->dbg.fq_stats) {
- netdev_err(priv->net_dev, "debugfs_create_file() failed\n");
- goto err_ch_stats;
- }
-
- return;
-
-err_ch_stats:
- debugfs_remove(priv->dbg.fq_stats);
-err_fq_stats:
- debugfs_remove(priv->dbg.cpu_stats);
-err_cpu_stats:
- debugfs_remove(priv->dbg.dir);
+ debugfs_create_file("ch_stats", 0444, dir, priv, &dpaa2_dbg_ch_ops);
}
void dpaa2_dbg_remove(struct dpaa2_eth_priv *priv)
{
- debugfs_remove(priv->dbg.fq_stats);
- debugfs_remove(priv->dbg.ch_stats);
- debugfs_remove(priv->dbg.cpu_stats);
- debugfs_remove(priv->dbg.dir);
+ debugfs_remove_recursive(priv->dbg.dir);
}
void dpaa2_eth_dbg_init(void)
{
dpaa2_dbg_root = debugfs_create_dir(DPAA2_ETH_DBG_ROOT, NULL);
- if (!dpaa2_dbg_root) {
- pr_err("DPAA2-ETH: debugfs create failed\n");
- return;
- }
-
pr_debug("DPAA2-ETH: debugfs created\n");
}
diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth-debugfs.h b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth-debugfs.h
index 4f63de997a26..15598b28f03b 100644
--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth-debugfs.h
+++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth-debugfs.h
@@ -11,9 +11,6 @@ struct dpaa2_eth_priv;
struct dpaa2_debugfs {
struct dentry *dir;
- struct dentry *fq_stats;
- struct dentry *ch_stats;
- struct dentry *cpu_stats;
};
#ifdef CONFIG_DEBUG_FS
--
2.22.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH 11/17] qca: no need to check return value of debugfs_create functions
2019-08-06 16:11 [PATCH 00/17] Networking driver debugfs cleanups Greg Kroah-Hartman
` (9 preceding siblings ...)
2019-08-06 16:11 ` [PATCH 10/17] dpaa2: " Greg Kroah-Hartman
@ 2019-08-06 16:11 ` Greg Kroah-Hartman
2019-08-08 6:30 ` Michael Heimpold
2019-08-06 16:11 ` [PATCH 12/17] skge: " Greg Kroah-Hartman
` (6 subsequent siblings)
17 siblings, 1 reply; 32+ messages in thread
From: Greg Kroah-Hartman @ 2019-08-06 16:11 UTC (permalink / raw)
To: netdev
Cc: Greg Kroah-Hartman, David S. Miller, Stefan Wahren,
Michael Heimpold, Yangtao Li
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: "David S. Miller" <davem@davemloft.net>
Cc: Stefan Wahren <stefan.wahren@i2se.com>
Cc: Michael Heimpold <michael.heimpold@i2se.com>
Cc: Yangtao Li <tiny.windzz@gmail.com>
Cc: netdev@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
drivers/net/ethernet/qualcomm/qca_debug.c | 13 +++----------
1 file changed, 3 insertions(+), 10 deletions(-)
diff --git a/drivers/net/ethernet/qualcomm/qca_debug.c b/drivers/net/ethernet/qualcomm/qca_debug.c
index bcb890b18a94..702aa217a27a 100644
--- a/drivers/net/ethernet/qualcomm/qca_debug.c
+++ b/drivers/net/ethernet/qualcomm/qca_debug.c
@@ -131,17 +131,10 @@ DEFINE_SHOW_ATTRIBUTE(qcaspi_info);
void
qcaspi_init_device_debugfs(struct qcaspi *qca)
{
- struct dentry *device_root;
+ qca->device_root = debugfs_create_dir(dev_name(&qca->net_dev->dev),
+ NULL);
- device_root = debugfs_create_dir(dev_name(&qca->net_dev->dev), NULL);
- qca->device_root = device_root;
-
- if (IS_ERR(device_root) || !device_root) {
- pr_warn("failed to create debugfs directory for %s\n",
- dev_name(&qca->net_dev->dev));
- return;
- }
- debugfs_create_file("info", S_IFREG | 0444, device_root, qca,
+ debugfs_create_file("info", S_IFREG | 0444, qca->device_root, qca,
&qcaspi_info_fops);
}
--
2.22.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH 11/17] qca: no need to check return value of debugfs_create functions
2019-08-06 16:11 ` [PATCH 11/17] qca: " Greg Kroah-Hartman
@ 2019-08-08 6:30 ` Michael Heimpold
0 siblings, 0 replies; 32+ messages in thread
From: Michael Heimpold @ 2019-08-08 6:30 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: netdev, David S. Miller, Stefan Wahren, Yangtao Li
Am Dienstag, 6. August 2019, 18:11:22 CEST schrieb 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: "David S. Miller" <davem@davemloft.net>
> Cc: Stefan Wahren <stefan.wahren@i2se.com>
> Cc: Michael Heimpold <michael.heimpold@i2se.com>
> Cc: Yangtao Li <tiny.windzz@gmail.com>
> Cc: netdev@vger.kernel.org
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
> drivers/net/ethernet/qualcomm/qca_debug.c | 13 +++----------
> 1 file changed, 3 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/net/ethernet/qualcomm/qca_debug.c
> b/drivers/net/ethernet/qualcomm/qca_debug.c index
> bcb890b18a94..702aa217a27a 100644
> --- a/drivers/net/ethernet/qualcomm/qca_debug.c
> +++ b/drivers/net/ethernet/qualcomm/qca_debug.c
> @@ -131,17 +131,10 @@ DEFINE_SHOW_ATTRIBUTE(qcaspi_info);
> void
> qcaspi_init_device_debugfs(struct qcaspi *qca)
> {
> - struct dentry *device_root;
> + qca->device_root = debugfs_create_dir(dev_name(&qca->net_dev->dev),
> + NULL);
>
> - device_root = debugfs_create_dir(dev_name(&qca->net_dev->dev), NULL);
> - qca->device_root = device_root;
> -
> - if (IS_ERR(device_root) || !device_root) {
> - pr_warn("failed to create debugfs directory for %s\n",
> - dev_name(&qca->net_dev->dev));
> - return;
> - }
> - debugfs_create_file("info", S_IFREG | 0444, device_root, qca,
> + debugfs_create_file("info", S_IFREG | 0444, qca->device_root, qca,
> &qcaspi_info_fops);
> }
Acked-by: Michael Heimpold <michael.heimpold@i2se.com>
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH 12/17] skge: no need to check return value of debugfs_create functions
2019-08-06 16:11 [PATCH 00/17] Networking driver debugfs cleanups Greg Kroah-Hartman
` (10 preceding siblings ...)
2019-08-06 16:11 ` [PATCH 11/17] qca: " Greg Kroah-Hartman
@ 2019-08-06 16:11 ` Greg Kroah-Hartman
2019-08-06 17:44 ` Stephen Hemminger
2019-08-06 16:11 ` [PATCH 13/17] mvpp2: " Greg Kroah-Hartman
` (5 subsequent siblings)
17 siblings, 1 reply; 32+ messages in thread
From: Greg Kroah-Hartman @ 2019-08-06 16:11 UTC (permalink / raw)
To: netdev
Cc: Greg Kroah-Hartman, Mirko Lindner, Stephen Hemminger, David S. Miller
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: Mirko Lindner <mlindner@marvell.com>
Cc: Stephen Hemminger <stephen@networkplumber.org>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: netdev@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
drivers/net/ethernet/marvell/skge.c | 39 +++++++----------------------
1 file changed, 9 insertions(+), 30 deletions(-)
diff --git a/drivers/net/ethernet/marvell/skge.c b/drivers/net/ethernet/marvell/skge.c
index 06dffee81e02..0a2ec387a482 100644
--- a/drivers/net/ethernet/marvell/skge.c
+++ b/drivers/net/ethernet/marvell/skge.c
@@ -3731,7 +3731,6 @@ static int skge_device_event(struct notifier_block *unused,
{
struct net_device *dev = netdev_notifier_info_to_dev(ptr);
struct skge_port *skge;
- struct dentry *d;
if (dev->netdev_ops->ndo_open != &skge_up || !skge_debug)
goto done;
@@ -3739,33 +3738,20 @@ static int skge_device_event(struct notifier_block *unused,
skge = netdev_priv(dev);
switch (event) {
case NETDEV_CHANGENAME:
- if (skge->debugfs) {
- d = debugfs_rename(skge_debug, skge->debugfs,
- skge_debug, dev->name);
- if (d)
- skge->debugfs = d;
- else {
- netdev_info(dev, "rename failed\n");
- debugfs_remove(skge->debugfs);
- }
- }
+ if (skge->debugfs)
+ skge->debugfs = debugfs_rename(skge_debug,
+ skge->debugfs,
+ skge_debug, dev->name);
break;
case NETDEV_GOING_DOWN:
- if (skge->debugfs) {
- debugfs_remove(skge->debugfs);
- skge->debugfs = NULL;
- }
+ debugfs_remove(skge->debugfs);
+ skge->debugfs = NULL;
break;
case NETDEV_UP:
- d = debugfs_create_file(dev->name, 0444,
- skge_debug, dev,
- &skge_debug_fops);
- if (!d || IS_ERR(d))
- netdev_info(dev, "debugfs create failed\n");
- else
- skge->debugfs = d;
+ skge->debugfs = debugfs_create_file(dev->name, 0444, skge_debug,
+ dev, &skge_debug_fops);
break;
}
@@ -3780,15 +3766,8 @@ static struct notifier_block skge_notifier = {
static __init void skge_debug_init(void)
{
- struct dentry *ent;
-
- ent = debugfs_create_dir("skge", NULL);
- if (!ent || IS_ERR(ent)) {
- pr_info("debugfs create directory failed\n");
- return;
- }
+ skge_debug = debugfs_create_dir("skge", NULL);
- skge_debug = ent;
register_netdevice_notifier(&skge_notifier);
}
--
2.22.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH 12/17] skge: no need to check return value of debugfs_create functions
2019-08-06 16:11 ` [PATCH 12/17] skge: " Greg Kroah-Hartman
@ 2019-08-06 17:44 ` Stephen Hemminger
0 siblings, 0 replies; 32+ messages in thread
From: Stephen Hemminger @ 2019-08-06 17:44 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: netdev, Mirko Lindner, David S. Miller
On Tue, 6 Aug 2019 18:11:23 +0200
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.
>
> Cc: Mirko Lindner <mlindner@marvell.com>
> Cc: Stephen Hemminger <stephen@networkplumber.org>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: netdev@vger.kernel.org
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Acked-by: Stephen Hemminger <stephen@networkplumber.org>
Did not pull card out of dust bin to test it though
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH 13/17] mvpp2: no need to check return value of debugfs_create functions
2019-08-06 16:11 [PATCH 00/17] Networking driver debugfs cleanups Greg Kroah-Hartman
` (11 preceding siblings ...)
2019-08-06 16:11 ` [PATCH 12/17] skge: " Greg Kroah-Hartman
@ 2019-08-06 16:11 ` Greg Kroah-Hartman
2019-08-06 16:11 ` [PATCH 14/17] fm10k: " Greg Kroah-Hartman
` (4 subsequent siblings)
17 siblings, 0 replies; 32+ messages in thread
From: Greg Kroah-Hartman @ 2019-08-06 16:11 UTC (permalink / raw)
To: netdev
Cc: Greg Kroah-Hartman, David S. Miller, Maxime Chevallier,
Nick Desaulniers, Nathan Huckleberry
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: "David S. Miller" <davem@davemloft.net>
Cc: Maxime Chevallier <maxime.chevallier@bootlin.com>
Cc: Nick Desaulniers <ndesaulniers@google.com>
Cc: Nathan Huckleberry <nhuck@google.com>
Cc: netdev@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
.../ethernet/marvell/mvpp2/mvpp2_debugfs.c | 19 +------------------
1 file changed, 1 insertion(+), 18 deletions(-)
diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_debugfs.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_debugfs.c
index 274fb07362cb..4a3baa7e0142 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_debugfs.c
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_debugfs.c
@@ -452,8 +452,6 @@ static int mvpp2_dbgfs_flow_port_init(struct dentry *parent,
struct dentry *port_dir;
port_dir = debugfs_create_dir(port->dev->name, parent);
- if (IS_ERR(port_dir))
- return PTR_ERR(port_dir);
port_entry = &port->priv->dbgfs_entries->port_flow_entries[port->id];
@@ -480,8 +478,6 @@ static int mvpp2_dbgfs_flow_entry_init(struct dentry *parent,
sprintf(flow_entry_name, "%02d", flow);
flow_entry_dir = debugfs_create_dir(flow_entry_name, parent);
- if (!flow_entry_dir)
- return -ENOMEM;
entry = &priv->dbgfs_entries->flow_entries[flow];
@@ -514,8 +510,6 @@ static int mvpp2_dbgfs_flow_init(struct dentry *parent, struct mvpp2 *priv)
int i, ret;
flow_dir = debugfs_create_dir("flows", parent);
- if (!flow_dir)
- return -ENOMEM;
for (i = 0; i < MVPP2_N_PRS_FLOWS; i++) {
ret = mvpp2_dbgfs_flow_entry_init(flow_dir, priv, i);
@@ -539,8 +533,6 @@ static int mvpp2_dbgfs_prs_entry_init(struct dentry *parent,
sprintf(prs_entry_name, "%03d", tid);
prs_entry_dir = debugfs_create_dir(prs_entry_name, parent);
- if (!prs_entry_dir)
- return -ENOMEM;
entry = &priv->dbgfs_entries->prs_entries[tid];
@@ -578,8 +570,6 @@ static int mvpp2_dbgfs_prs_init(struct dentry *parent, struct mvpp2 *priv)
int i, ret;
prs_dir = debugfs_create_dir("parser", parent);
- if (!prs_dir)
- return -ENOMEM;
for (i = 0; i < MVPP2_PRS_TCAM_SRAM_SIZE; i++) {
ret = mvpp2_dbgfs_prs_entry_init(prs_dir, priv, i);
@@ -688,8 +678,6 @@ static int mvpp2_dbgfs_port_init(struct dentry *parent,
struct dentry *port_dir;
port_dir = debugfs_create_dir(port->dev->name, parent);
- if (IS_ERR(port_dir))
- return PTR_ERR(port_dir);
debugfs_create_file("parser_entries", 0444, port_dir, port,
&mvpp2_dbgfs_port_parser_fops);
@@ -716,15 +704,10 @@ void mvpp2_dbgfs_init(struct mvpp2 *priv, const char *name)
int ret, i;
mvpp2_root = debugfs_lookup(MVPP2_DRIVER_NAME, NULL);
- if (!mvpp2_root) {
+ if (!mvpp2_root)
mvpp2_root = debugfs_create_dir(MVPP2_DRIVER_NAME, NULL);
- if (IS_ERR(mvpp2_root))
- return;
- }
mvpp2_dir = debugfs_create_dir(name, mvpp2_root);
- if (IS_ERR(mvpp2_dir))
- return;
priv->dbgfs_dir = mvpp2_dir;
priv->dbgfs_entries = kzalloc(sizeof(*priv->dbgfs_entries), GFP_KERNEL);
--
2.22.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH 14/17] fm10k: no need to check return value of debugfs_create functions
2019-08-06 16:11 [PATCH 00/17] Networking driver debugfs cleanups Greg Kroah-Hartman
` (12 preceding siblings ...)
2019-08-06 16:11 ` [PATCH 13/17] mvpp2: " Greg Kroah-Hartman
@ 2019-08-06 16:11 ` Greg Kroah-Hartman
2019-08-06 16:11 ` [PATCH 15/17] i40e: " Greg Kroah-Hartman
` (3 subsequent siblings)
17 siblings, 0 replies; 32+ messages in thread
From: Greg Kroah-Hartman @ 2019-08-06 16:11 UTC (permalink / raw)
To: netdev; +Cc: Greg Kroah-Hartman, Jeff Kirsher, David S. Miller, intel-wired-lan
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: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: intel-wired-lan@lists.osuosl.org
Cc: netdev@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
drivers/net/ethernet/intel/fm10k/fm10k_debugfs.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_debugfs.c b/drivers/net/ethernet/intel/fm10k/fm10k_debugfs.c
index dca104121c05..1d27b2fb23af 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k_debugfs.c
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_debugfs.c
@@ -160,8 +160,6 @@ void fm10k_dbg_q_vector_init(struct fm10k_q_vector *q_vector)
snprintf(name, sizeof(name), "q_vector.%03d", q_vector->v_idx);
q_vector->dbg_q_vector = debugfs_create_dir(name, interface->dbg_intfc);
- if (!q_vector->dbg_q_vector)
- return;
/* Generate a file for each rx ring in the q_vector */
for (i = 0; i < q_vector->tx.count; i++) {
--
2.22.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH 15/17] i40e: no need to check return value of debugfs_create functions
2019-08-06 16:11 [PATCH 00/17] Networking driver debugfs cleanups Greg Kroah-Hartman
` (13 preceding siblings ...)
2019-08-06 16:11 ` [PATCH 14/17] fm10k: " Greg Kroah-Hartman
@ 2019-08-06 16:11 ` Greg Kroah-Hartman
2019-08-06 16:11 ` [PATCH 16/17] ixgbe: " Greg Kroah-Hartman
` (2 subsequent siblings)
17 siblings, 0 replies; 32+ messages in thread
From: Greg Kroah-Hartman @ 2019-08-06 16:11 UTC (permalink / raw)
To: netdev; +Cc: Greg Kroah-Hartman, Jeff Kirsher, David S. Miller, intel-wired-lan
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: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: intel-wired-lan@lists.osuosl.org
Cc: netdev@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
.../net/ethernet/intel/i40e/i40e_debugfs.c | 21 ++++---------------
1 file changed, 4 insertions(+), 17 deletions(-)
diff --git a/drivers/net/ethernet/intel/i40e/i40e_debugfs.c b/drivers/net/ethernet/intel/i40e/i40e_debugfs.c
index 55d20acfcf70..0df9454b3315 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_debugfs.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_debugfs.c
@@ -1732,29 +1732,16 @@ static const struct file_operations i40e_dbg_netdev_ops_fops = {
**/
void i40e_dbg_pf_init(struct i40e_pf *pf)
{
- struct dentry *pfile;
const char *name = pci_name(pf->pdev);
const struct device *dev = &pf->pdev->dev;
pf->i40e_dbg_pf = debugfs_create_dir(name, i40e_dbg_root);
- if (!pf->i40e_dbg_pf)
- return;
-
- pfile = debugfs_create_file("command", 0600, pf->i40e_dbg_pf, pf,
- &i40e_dbg_command_fops);
- if (!pfile)
- goto create_failed;
- pfile = debugfs_create_file("netdev_ops", 0600, pf->i40e_dbg_pf, pf,
- &i40e_dbg_netdev_ops_fops);
- if (!pfile)
- goto create_failed;
+ debugfs_create_file("command", 0600, pf->i40e_dbg_pf, pf,
+ &i40e_dbg_command_fops);
- return;
-
-create_failed:
- dev_info(dev, "debugfs dir/file for %s failed\n", name);
- debugfs_remove_recursive(pf->i40e_dbg_pf);
+ debugfs_create_file("netdev_ops", 0600, pf->i40e_dbg_pf, pf,
+ &i40e_dbg_netdev_ops_fops);
}
/**
--
2.22.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH 16/17] ixgbe: no need to check return value of debugfs_create functions
2019-08-06 16:11 [PATCH 00/17] Networking driver debugfs cleanups Greg Kroah-Hartman
` (14 preceding siblings ...)
2019-08-06 16:11 ` [PATCH 15/17] i40e: " Greg Kroah-Hartman
@ 2019-08-06 16:11 ` Greg Kroah-Hartman
2019-08-06 16:11 ` [PATCH 17/17] ieee802154: " Greg Kroah-Hartman
2019-08-09 1:37 ` [PATCH 00/17] Networking driver debugfs cleanups David Miller
17 siblings, 0 replies; 32+ messages in thread
From: Greg Kroah-Hartman @ 2019-08-06 16:11 UTC (permalink / raw)
To: netdev; +Cc: Greg Kroah-Hartman, Jeff Kirsher, David S. Miller, intel-wired-lan
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: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: intel-wired-lan@lists.osuosl.org
Cc: netdev@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
.../net/ethernet/intel/ixgbe/ixgbe_debugfs.c | 22 +++++--------------
1 file changed, 5 insertions(+), 17 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_debugfs.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_debugfs.c
index 50dfb02fa34c..171cdc552961 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_debugfs.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_debugfs.c
@@ -190,22 +190,12 @@ static const struct file_operations ixgbe_dbg_netdev_ops_fops = {
void ixgbe_dbg_adapter_init(struct ixgbe_adapter *adapter)
{
const char *name = pci_name(adapter->pdev);
- struct dentry *pfile;
+
adapter->ixgbe_dbg_adapter = debugfs_create_dir(name, ixgbe_dbg_root);
- if (adapter->ixgbe_dbg_adapter) {
- pfile = debugfs_create_file("reg_ops", 0600,
- adapter->ixgbe_dbg_adapter, adapter,
- &ixgbe_dbg_reg_ops_fops);
- if (!pfile)
- e_dev_err("debugfs reg_ops for %s failed\n", name);
- pfile = debugfs_create_file("netdev_ops", 0600,
- adapter->ixgbe_dbg_adapter, adapter,
- &ixgbe_dbg_netdev_ops_fops);
- if (!pfile)
- e_dev_err("debugfs netdev_ops for %s failed\n", name);
- } else {
- e_dev_err("debugfs entry for %s failed\n", name);
- }
+ debugfs_create_file("reg_ops", 0600, adapter->ixgbe_dbg_adapter,
+ adapter, &ixgbe_dbg_reg_ops_fops);
+ debugfs_create_file("netdev_ops", 0600, adapter->ixgbe_dbg_adapter,
+ adapter, &ixgbe_dbg_netdev_ops_fops);
}
/**
@@ -224,8 +214,6 @@ void ixgbe_dbg_adapter_exit(struct ixgbe_adapter *adapter)
void ixgbe_dbg_init(void)
{
ixgbe_dbg_root = debugfs_create_dir(ixgbe_driver_name, NULL);
- if (ixgbe_dbg_root == NULL)
- pr_err("init of debugfs failed\n");
}
/**
--
2.22.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH 17/17] ieee802154: no need to check return value of debugfs_create functions
2019-08-06 16:11 [PATCH 00/17] Networking driver debugfs cleanups Greg Kroah-Hartman
` (15 preceding siblings ...)
2019-08-06 16:11 ` [PATCH 16/17] ixgbe: " Greg Kroah-Hartman
@ 2019-08-06 16:11 ` Greg Kroah-Hartman
2019-08-06 19:22 ` Stefan Schmidt
2019-08-09 1:37 ` [PATCH 00/17] Networking driver debugfs cleanups David Miller
17 siblings, 1 reply; 32+ messages in thread
From: Greg Kroah-Hartman @ 2019-08-06 16:11 UTC (permalink / raw)
To: netdev
Cc: Greg Kroah-Hartman, Michael Hennerich, Alexander Aring,
Stefan Schmidt, David S. Miller, Harry Morris, linux-wpan
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: Michael Hennerich <michael.hennerich@analog.com>
Cc: Alexander Aring <alex.aring@gmail.com>
Cc: Stefan Schmidt <stefan@datenfreihafen.org>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Harry Morris <h.morris@cascoda.com>
Cc: linux-wpan@vger.kernel.org
Cc: netdev@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
drivers/net/ieee802154/adf7242.c | 12 +++---------
drivers/net/ieee802154/at86rf230.c | 20 +++++---------------
drivers/net/ieee802154/ca8210.c | 9 +--------
3 files changed, 9 insertions(+), 32 deletions(-)
diff --git a/drivers/net/ieee802154/adf7242.c b/drivers/net/ieee802154/adf7242.c
index c9392d70e639..7b95add2235a 100644
--- a/drivers/net/ieee802154/adf7242.c
+++ b/drivers/net/ieee802154/adf7242.c
@@ -1158,7 +1158,7 @@ static int adf7242_stats_show(struct seq_file *file, void *offset)
return 0;
}
-static int adf7242_debugfs_init(struct adf7242_local *lp)
+static void adf7242_debugfs_init(struct adf7242_local *lp)
{
char debugfs_dir_name[DNAME_INLINE_LEN + 1] = "adf7242-";
struct dentry *stats;
@@ -1166,15 +1166,9 @@ static int adf7242_debugfs_init(struct adf7242_local *lp)
strncat(debugfs_dir_name, dev_name(&lp->spi->dev), DNAME_INLINE_LEN);
lp->debugfs_root = debugfs_create_dir(debugfs_dir_name, NULL);
- if (IS_ERR_OR_NULL(lp->debugfs_root))
- return PTR_ERR_OR_ZERO(lp->debugfs_root);
- stats = debugfs_create_devm_seqfile(&lp->spi->dev, "status",
- lp->debugfs_root,
- adf7242_stats_show);
- return PTR_ERR_OR_ZERO(stats);
-
- return 0;
+ debugfs_create_devm_seqfile(&lp->spi->dev, "status", lp->debugfs_root,
+ adf7242_stats_show);
}
static const s32 adf7242_powers[] = {
diff --git a/drivers/net/ieee802154/at86rf230.c b/drivers/net/ieee802154/at86rf230.c
index 595cf7e2a651..7d67f41387f5 100644
--- a/drivers/net/ieee802154/at86rf230.c
+++ b/drivers/net/ieee802154/at86rf230.c
@@ -1626,24 +1626,16 @@ static int at86rf230_stats_show(struct seq_file *file, void *offset)
}
DEFINE_SHOW_ATTRIBUTE(at86rf230_stats);
-static int at86rf230_debugfs_init(struct at86rf230_local *lp)
+static void at86rf230_debugfs_init(struct at86rf230_local *lp)
{
char debugfs_dir_name[DNAME_INLINE_LEN + 1] = "at86rf230-";
- struct dentry *stats;
strncat(debugfs_dir_name, dev_name(&lp->spi->dev), DNAME_INLINE_LEN);
at86rf230_debugfs_root = debugfs_create_dir(debugfs_dir_name, NULL);
- if (!at86rf230_debugfs_root)
- return -ENOMEM;
-
- stats = debugfs_create_file("trac_stats", 0444,
- at86rf230_debugfs_root, lp,
- &at86rf230_stats_fops);
- if (!stats)
- return -ENOMEM;
- return 0;
+ debugfs_create_file("trac_stats", 0444, at86rf230_debugfs_root, lp,
+ &at86rf230_stats_fops);
}
static void at86rf230_debugfs_remove(void)
@@ -1651,7 +1643,7 @@ static void at86rf230_debugfs_remove(void)
debugfs_remove_recursive(at86rf230_debugfs_root);
}
#else
-static int at86rf230_debugfs_init(struct at86rf230_local *lp) { return 0; }
+static void at86rf230_debugfs_init(struct at86rf230_local *lp) { }
static void at86rf230_debugfs_remove(void) { }
#endif
@@ -1751,9 +1743,7 @@ static int at86rf230_probe(struct spi_device *spi)
/* going into sleep by default */
at86rf230_sleep(lp);
- rc = at86rf230_debugfs_init(lp);
- if (rc)
- goto free_dev;
+ at86rf230_debugfs_init(lp);
rc = ieee802154_register_hw(lp->hw);
if (rc)
diff --git a/drivers/net/ieee802154/ca8210.c b/drivers/net/ieee802154/ca8210.c
index b188fce3f641..11402dc347db 100644
--- a/drivers/net/ieee802154/ca8210.c
+++ b/drivers/net/ieee802154/ca8210.c
@@ -3019,14 +3019,7 @@ static int ca8210_test_interface_init(struct ca8210_priv *priv)
priv,
&test_int_fops
);
- if (IS_ERR(test->ca8210_dfs_spi_int)) {
- dev_err(
- &priv->spi->dev,
- "Error %ld when creating debugfs node\n",
- PTR_ERR(test->ca8210_dfs_spi_int)
- );
- return PTR_ERR(test->ca8210_dfs_spi_int);
- }
+
debugfs_create_symlink("ca8210", NULL, node_name);
init_waitqueue_head(&test->readq);
return kfifo_alloc(
--
2.22.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH 17/17] ieee802154: no need to check return value of debugfs_create functions
2019-08-06 16:11 ` [PATCH 17/17] ieee802154: " Greg Kroah-Hartman
@ 2019-08-06 19:22 ` Stefan Schmidt
2019-08-06 19:48 ` Greg Kroah-Hartman
0 siblings, 1 reply; 32+ messages in thread
From: Stefan Schmidt @ 2019-08-06 19:22 UTC (permalink / raw)
To: Greg Kroah-Hartman, netdev
Cc: Michael Hennerich, Alexander Aring, David S. Miller,
Harry Morris, linux-wpan
Hello.
On 06.08.19 18:11, 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: Michael Hennerich <michael.hennerich@analog.com>
> Cc: Alexander Aring <alex.aring@gmail.com>
> Cc: Stefan Schmidt <stefan@datenfreihafen.org>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Harry Morris <h.morris@cascoda.com>
> Cc: linux-wpan@vger.kernel.org
> Cc: netdev@vger.kernel.org
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
> drivers/net/ieee802154/adf7242.c | 12 +++---------
> drivers/net/ieee802154/at86rf230.c | 20 +++++---------------
> drivers/net/ieee802154/ca8210.c | 9 +--------
> 3 files changed, 9 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/net/ieee802154/adf7242.c b/drivers/net/ieee802154/adf7242.c
> index c9392d70e639..7b95add2235a 100644
> --- a/drivers/net/ieee802154/adf7242.c
> +++ b/drivers/net/ieee802154/adf7242.c
> @@ -1158,7 +1158,7 @@ static int adf7242_stats_show(struct seq_file *file, void *offset)
> return 0;
> }
>
> -static int adf7242_debugfs_init(struct adf7242_local *lp)
> +static void adf7242_debugfs_init(struct adf7242_local *lp)
> {
> char debugfs_dir_name[DNAME_INLINE_LEN + 1] = "adf7242-";
> struct dentry *stats;
A quick look over the code indicates that the stats variable can go as
well now as it is only used in the now removed code.
> @@ -1166,15 +1166,9 @@ static int adf7242_debugfs_init(struct adf7242_local *lp)
> strncat(debugfs_dir_name, dev_name(&lp->spi->dev), DNAME_INLINE_LEN);
>
> lp->debugfs_root = debugfs_create_dir(debugfs_dir_name, NULL);
> - if (IS_ERR_OR_NULL(lp->debugfs_root))
> - return PTR_ERR_OR_ZERO(lp->debugfs_root);
>
> - stats = debugfs_create_devm_seqfile(&lp->spi->dev, "status",
> - lp->debugfs_root,
> - adf7242_stats_show);
> - return PTR_ERR_OR_ZERO(stats);
> -
> - return 0;
> + debugfs_create_devm_seqfile(&lp->spi->dev, "status", lp->debugfs_root,
> + adf7242_stats_show);
> }
>
> static const s32 adf7242_powers[] = {
> diff --git a/drivers/net/ieee802154/at86rf230.c b/drivers/net/ieee802154/at86rf230.c
> index 595cf7e2a651..7d67f41387f5 100644
> --- a/drivers/net/ieee802154/at86rf230.c
> +++ b/drivers/net/ieee802154/at86rf230.c
> @@ -1626,24 +1626,16 @@ static int at86rf230_stats_show(struct seq_file *file, void *offset)
> }
> DEFINE_SHOW_ATTRIBUTE(at86rf230_stats);
>
> -static int at86rf230_debugfs_init(struct at86rf230_local *lp)
> +static void at86rf230_debugfs_init(struct at86rf230_local *lp)
> {
> char debugfs_dir_name[DNAME_INLINE_LEN + 1] = "at86rf230-";
> - struct dentry *stats;
>
> strncat(debugfs_dir_name, dev_name(&lp->spi->dev), DNAME_INLINE_LEN);
>
> at86rf230_debugfs_root = debugfs_create_dir(debugfs_dir_name, NULL);
> - if (!at86rf230_debugfs_root)
> - return -ENOMEM;
> -
> - stats = debugfs_create_file("trac_stats", 0444,
> - at86rf230_debugfs_root, lp,
> - &at86rf230_stats_fops);
> - if (!stats)
> - return -ENOMEM;
>
> - return 0;
> + debugfs_create_file("trac_stats", 0444, at86rf230_debugfs_root, lp,
> + &at86rf230_stats_fops);
> }
>
> static void at86rf230_debugfs_remove(void)
> @@ -1651,7 +1643,7 @@ static void at86rf230_debugfs_remove(void)
> debugfs_remove_recursive(at86rf230_debugfs_root);
> }
> #else
> -static int at86rf230_debugfs_init(struct at86rf230_local *lp) { return 0; }
> +static void at86rf230_debugfs_init(struct at86rf230_local *lp) { }
> static void at86rf230_debugfs_remove(void) { }
> #endif
>
> @@ -1751,9 +1743,7 @@ static int at86rf230_probe(struct spi_device *spi)
> /* going into sleep by default */
> at86rf230_sleep(lp);
>
> - rc = at86rf230_debugfs_init(lp);
> - if (rc)
> - goto free_dev;
> + at86rf230_debugfs_init(lp);
>
> rc = ieee802154_register_hw(lp->hw);
> if (rc)
> diff --git a/drivers/net/ieee802154/ca8210.c b/drivers/net/ieee802154/ca8210.c
> index b188fce3f641..11402dc347db 100644
> --- a/drivers/net/ieee802154/ca8210.c
> +++ b/drivers/net/ieee802154/ca8210.c
> @@ -3019,14 +3019,7 @@ static int ca8210_test_interface_init(struct ca8210_priv *priv)
> priv,
> &test_int_fops
> );
> - if (IS_ERR(test->ca8210_dfs_spi_int)) {
> - dev_err(
> - &priv->spi->dev,
> - "Error %ld when creating debugfs node\n",
> - PTR_ERR(test->ca8210_dfs_spi_int)
> - );
> - return PTR_ERR(test->ca8210_dfs_spi_int);
> - }
> +
> debugfs_create_symlink("ca8210", NULL, node_name);
> init_waitqueue_head(&test->readq);
> return kfifo_alloc(
>
With a fix for the above included you can have my
Acked-by: Stefan Schmidt <stefan@datenfreihafen.org>
for version 2.
regards
Stefan Schmidt
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH 17/17] ieee802154: no need to check return value of debugfs_create functions
2019-08-06 19:22 ` Stefan Schmidt
@ 2019-08-06 19:48 ` Greg Kroah-Hartman
0 siblings, 0 replies; 32+ messages in thread
From: Greg Kroah-Hartman @ 2019-08-06 19:48 UTC (permalink / raw)
To: Stefan Schmidt
Cc: netdev, Michael Hennerich, Alexander Aring, David S. Miller,
Harry Morris, linux-wpan
On Tue, Aug 06, 2019 at 09:22:43PM +0200, Stefan Schmidt wrote:
> Hello.
>
> On 06.08.19 18:11, 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: Michael Hennerich <michael.hennerich@analog.com>
> > Cc: Alexander Aring <alex.aring@gmail.com>
> > Cc: Stefan Schmidt <stefan@datenfreihafen.org>
> > Cc: "David S. Miller" <davem@davemloft.net>
> > Cc: Harry Morris <h.morris@cascoda.com>
> > Cc: linux-wpan@vger.kernel.org
> > Cc: netdev@vger.kernel.org
> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > ---
> > drivers/net/ieee802154/adf7242.c | 12 +++---------
> > drivers/net/ieee802154/at86rf230.c | 20 +++++---------------
> > drivers/net/ieee802154/ca8210.c | 9 +--------
> > 3 files changed, 9 insertions(+), 32 deletions(-)
> >
> > diff --git a/drivers/net/ieee802154/adf7242.c b/drivers/net/ieee802154/adf7242.c
> > index c9392d70e639..7b95add2235a 100644
> > --- a/drivers/net/ieee802154/adf7242.c
> > +++ b/drivers/net/ieee802154/adf7242.c
> > @@ -1158,7 +1158,7 @@ static int adf7242_stats_show(struct seq_file *file, void *offset)
> > return 0;
> > }
> >
> > -static int adf7242_debugfs_init(struct adf7242_local *lp)
> > +static void adf7242_debugfs_init(struct adf7242_local *lp)
> > {
> > char debugfs_dir_name[DNAME_INLINE_LEN + 1] = "adf7242-";
> > struct dentry *stats;
>
> A quick look over the code indicates that the stats variable can go as
> well now as it is only used in the now removed code.
Odd 0-day never gave me a build warning for it, sorry about that.
>
> > @@ -1166,15 +1166,9 @@ static int adf7242_debugfs_init(struct adf7242_local *lp)
> > strncat(debugfs_dir_name, dev_name(&lp->spi->dev), DNAME_INLINE_LEN);
> >
> > lp->debugfs_root = debugfs_create_dir(debugfs_dir_name, NULL);
> > - if (IS_ERR_OR_NULL(lp->debugfs_root))
> > - return PTR_ERR_OR_ZERO(lp->debugfs_root);
> >
> > - stats = debugfs_create_devm_seqfile(&lp->spi->dev, "status",
> > - lp->debugfs_root,
> > - adf7242_stats_show);
> > - return PTR_ERR_OR_ZERO(stats);
> > -
> > - return 0;
> > + debugfs_create_devm_seqfile(&lp->spi->dev, "status", lp->debugfs_root,
> > + adf7242_stats_show);
> > }
> >
> > static const s32 adf7242_powers[] = {
> > diff --git a/drivers/net/ieee802154/at86rf230.c b/drivers/net/ieee802154/at86rf230.c
> > index 595cf7e2a651..7d67f41387f5 100644
> > --- a/drivers/net/ieee802154/at86rf230.c
> > +++ b/drivers/net/ieee802154/at86rf230.c
> > @@ -1626,24 +1626,16 @@ static int at86rf230_stats_show(struct seq_file *file, void *offset)
> > }
> > DEFINE_SHOW_ATTRIBUTE(at86rf230_stats);
> >
> > -static int at86rf230_debugfs_init(struct at86rf230_local *lp)
> > +static void at86rf230_debugfs_init(struct at86rf230_local *lp)
> > {
> > char debugfs_dir_name[DNAME_INLINE_LEN + 1] = "at86rf230-";
> > - struct dentry *stats;
> >
> > strncat(debugfs_dir_name, dev_name(&lp->spi->dev), DNAME_INLINE_LEN);
> >
> > at86rf230_debugfs_root = debugfs_create_dir(debugfs_dir_name, NULL);
> > - if (!at86rf230_debugfs_root)
> > - return -ENOMEM;
> > -
> > - stats = debugfs_create_file("trac_stats", 0444,
> > - at86rf230_debugfs_root, lp,
> > - &at86rf230_stats_fops);
> > - if (!stats)
> > - return -ENOMEM;
> >
> > - return 0;
> > + debugfs_create_file("trac_stats", 0444, at86rf230_debugfs_root, lp,
> > + &at86rf230_stats_fops);
> > }
> >
> > static void at86rf230_debugfs_remove(void)
> > @@ -1651,7 +1643,7 @@ static void at86rf230_debugfs_remove(void)
> > debugfs_remove_recursive(at86rf230_debugfs_root);
> > }
> > #else
> > -static int at86rf230_debugfs_init(struct at86rf230_local *lp) { return 0; }
> > +static void at86rf230_debugfs_init(struct at86rf230_local *lp) { }
> > static void at86rf230_debugfs_remove(void) { }
> > #endif
> >
> > @@ -1751,9 +1743,7 @@ static int at86rf230_probe(struct spi_device *spi)
> > /* going into sleep by default */
> > at86rf230_sleep(lp);
> >
> > - rc = at86rf230_debugfs_init(lp);
> > - if (rc)
> > - goto free_dev;
> > + at86rf230_debugfs_init(lp);
> >
> > rc = ieee802154_register_hw(lp->hw);
> > if (rc)
> > diff --git a/drivers/net/ieee802154/ca8210.c b/drivers/net/ieee802154/ca8210.c
> > index b188fce3f641..11402dc347db 100644
> > --- a/drivers/net/ieee802154/ca8210.c
> > +++ b/drivers/net/ieee802154/ca8210.c
> > @@ -3019,14 +3019,7 @@ static int ca8210_test_interface_init(struct ca8210_priv *priv)
> > priv,
> > &test_int_fops
> > );
> > - if (IS_ERR(test->ca8210_dfs_spi_int)) {
> > - dev_err(
> > - &priv->spi->dev,
> > - "Error %ld when creating debugfs node\n",
> > - PTR_ERR(test->ca8210_dfs_spi_int)
> > - );
> > - return PTR_ERR(test->ca8210_dfs_spi_int);
> > - }
> > +
> > debugfs_create_symlink("ca8210", NULL, node_name);
> > init_waitqueue_head(&test->readq);
> > return kfifo_alloc(
> >
>
> With a fix for the above included you can have my
>
>
> Acked-by: Stefan Schmidt <stefan@datenfreihafen.org>
>
> for version 2.
I'll fix it up and resend tomorrow, thanks!
greg k-h
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 00/17] Networking driver debugfs cleanups
2019-08-06 16:11 [PATCH 00/17] Networking driver debugfs cleanups Greg Kroah-Hartman
` (16 preceding siblings ...)
2019-08-06 16:11 ` [PATCH 17/17] ieee802154: " Greg Kroah-Hartman
@ 2019-08-09 1:37 ` David Miller
2019-08-09 1:42 ` David Miller
17 siblings, 1 reply; 32+ messages in thread
From: David Miller @ 2019-08-09 1:37 UTC (permalink / raw)
To: gregkh; +Cc: netdev
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Date: Tue, 6 Aug 2019 18:11:11 +0200
> There is no need to test the result of any debugfs call anymore. The
> debugfs core warns the user if something fails, and the return value of
> a debugfs call can always be fed back into another debugfs call with no
> problems.
>
> Also, debugfs is for debugging, so if there are problems with debugfs
> (i.e. the system is out of memory) the rest of the kernel should not
> change behavior, so testing for debugfs calls is pointless and not the
> goal of debugfs at all.
>
> This series cleans up a lot of networking drivers and some wimax code
> that was calling debugfs and trying to do something with the return
> value that it didn't need to. Removing this logic makes the code
> smaller, easier to understand, and use less run-time memory in some
> cases, all good things.
>
> The series is against net-next, and have no dependancies between any of
> them if they want to go through any random tree/order. Or, if wanted,
> I can take them through my driver-core tree where other debugfs cleanups
> are being slowly fed during major merge windows.
I applied this without patch #17 which you said you would respin in order
to get rid of the now unused local variable.
Thanks.
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH 00/17] Networking driver debugfs cleanups
2019-08-09 1:37 ` [PATCH 00/17] Networking driver debugfs cleanups David Miller
@ 2019-08-09 1:42 ` David Miller
2019-08-09 12:30 ` Greg KH
0 siblings, 1 reply; 32+ messages in thread
From: David Miller @ 2019-08-09 1:42 UTC (permalink / raw)
To: gregkh; +Cc: netdev
From: David Miller <davem@davemloft.net>
Date: Thu, 08 Aug 2019 18:37:56 -0700 (PDT)
> I applied this without patch #17 which you said you would respin in order
> to get rid of the now unused local variable.
Actually, there is a bunch of fallout still:
drivers/net/wimax/i2400m/debugfs.c: In function ‘i2400m_debugfs_add’:
drivers/net/wimax/i2400m/debugfs.c:192:17: warning: unused variable ‘dev’ [-Wunused-variable]
struct device *dev = i2400m_dev(i2400m);
^~~
drivers/net/wimax/i2400m/usb.c: In function ‘i2400mu_debugfs_add’:
drivers/net/wimax/i2400m/usb.c:375:17: warning: unused variable ‘fd’ [-Wunused-variable]
struct dentry *fd;
^~
drivers/net/wimax/i2400m/usb.c:373:17: warning: unused variable ‘dev’ [-Wunused-variable]
struct device *dev = &i2400mu->usb_iface->dev;
^~~
drivers/net/wimax/i2400m/usb.c:372:6: warning: unused variable ‘result’ [-Wunused-variable]
int result;
^~~~~~
drivers/net/ethernet/intel/i40e/i40e_debugfs.c: In function ‘i40e_dbg_pf_init’:
drivers/net/ethernet/intel/i40e/i40e_debugfs.c:1736:23: warning: unused variable ‘dev’ [-Wunused-variable]
const struct device *dev = &pf->pdev->dev;
^~~
This is with:
[davem@localhost net-next]$ gcc --version
gcc (GCC) 8.3.1 20190223 (Red Hat 8.3.1-2)
Copyright (C) 2018 Free Software Foundation, Inc.
This is free software; see the source for copying conditions. There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
[davem@localhost net-next]$
So I'm reverting.
Please respin the series with this stuff fixed, thanks Greg.
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH 00/17] Networking driver debugfs cleanups
2019-08-09 1:42 ` David Miller
@ 2019-08-09 12:30 ` Greg KH
0 siblings, 0 replies; 32+ messages in thread
From: Greg KH @ 2019-08-09 12:30 UTC (permalink / raw)
To: David Miller; +Cc: netdev
On Thu, Aug 08, 2019 at 06:42:37PM -0700, David Miller wrote:
> From: David Miller <davem@davemloft.net>
> Date: Thu, 08 Aug 2019 18:37:56 -0700 (PDT)
>
> > I applied this without patch #17 which you said you would respin in order
> > to get rid of the now unused local variable.
>
> Actually, there is a bunch of fallout still:
>
> drivers/net/wimax/i2400m/debugfs.c: In function ‘i2400m_debugfs_add’:
> drivers/net/wimax/i2400m/debugfs.c:192:17: warning: unused variable ‘dev’ [-Wunused-variable]
> struct device *dev = i2400m_dev(i2400m);
> ^~~
> drivers/net/wimax/i2400m/usb.c: In function ‘i2400mu_debugfs_add’:
> drivers/net/wimax/i2400m/usb.c:375:17: warning: unused variable ‘fd’ [-Wunused-variable]
> struct dentry *fd;
> ^~
> drivers/net/wimax/i2400m/usb.c:373:17: warning: unused variable ‘dev’ [-Wunused-variable]
> struct device *dev = &i2400mu->usb_iface->dev;
> ^~~
> drivers/net/wimax/i2400m/usb.c:372:6: warning: unused variable ‘result’ [-Wunused-variable]
> int result;
> ^~~~~~
> drivers/net/ethernet/intel/i40e/i40e_debugfs.c: In function ‘i40e_dbg_pf_init’:
> drivers/net/ethernet/intel/i40e/i40e_debugfs.c:1736:23: warning: unused variable ‘dev’ [-Wunused-variable]
> const struct device *dev = &pf->pdev->dev;
> ^~~
>
> This is with:
>
> [davem@localhost net-next]$ gcc --version
> gcc (GCC) 8.3.1 20190223 (Red Hat 8.3.1-2)
> Copyright (C) 2018 Free Software Foundation, Inc.
> This is free software; see the source for copying conditions. There is NO
> warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
>
> [davem@localhost net-next]$
>
> So I'm reverting.
>
> Please respin the series with this stuff fixed, thanks Greg.
Ugh, that sucks, it looks like I never even built this series. My
apologies for wasting people's time.
Let me fix this all up, properly build it, and resend.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 32+ messages in thread