[PATCH 1/5] clk: no need to check return value of debugfs_create functions

[PATCH 1/5] clk: no need to check return value of debugfs_create functions

[Greg Kroah-Hartman]

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

This cleans up the init code a lot, and there's no need to return an
error value based on the debugfs calls, especially as it turns out no
one was even looking at that return value.  So it obviously wasn't that
important :)

Cc: Michael Turquette <mturquette@baylibre.com>
Cc: Stephen Boyd <sboyd@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/clk/clk.c | 129 +++++++++++-----------------------------------
 1 file changed, 30 insertions(+), 99 deletions(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index ea67ac81c6f9..ef701ce44439 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -2608,81 +2608,31 @@ static int possible_parents_show(struct seq_file *s, void *data)
 }
 DEFINE_SHOW_ATTRIBUTE(possible_parents);
 
-static int clk_debug_create_one(struct clk_core *core, struct dentry *pdentry)
+static void clk_debug_create_one(struct clk_core *core, struct dentry *pdentry)
 {
-	struct dentry *d;
-	int ret = -ENOMEM;
-
-	if (!core || !pdentry) {
-		ret = -EINVAL;
-		goto out;
-	}
-
-	d = debugfs_create_dir(core->name, pdentry);
-	if (!d)
-		goto out;
-
-	core->dentry = d;
-
-	d = debugfs_create_ulong("clk_rate", 0444, core->dentry, &core->rate);
-	if (!d)
-		goto err_out;
-
-	d = debugfs_create_ulong("clk_accuracy", 0444, core->dentry,
-				 &core->accuracy);
-	if (!d)
-		goto err_out;
+	struct dentry *root;
 
-	d = debugfs_create_u32("clk_phase", 0444, core->dentry, &core->phase);
-	if (!d)
-		goto err_out;
-
-	d = debugfs_create_file("clk_flags", 0444, core->dentry, core,
-				&clk_flags_fops);
-	if (!d)
-		goto err_out;
-
-	d = debugfs_create_u32("clk_prepare_count", 0444, core->dentry,
-			       &core->prepare_count);
-	if (!d)
-		goto err_out;
-
-	d = debugfs_create_u32("clk_enable_count", 0444, core->dentry,
-			       &core->enable_count);
-	if (!d)
-		goto err_out;
-
-	d = debugfs_create_u32("clk_protect_count", 0444, core->dentry,
-			       &core->protect_count);
-	if (!d)
-		goto err_out;
-
-	d = debugfs_create_u32("clk_notifier_count", 0444, core->dentry,
-			       &core->notifier_count);
-	if (!d)
-		goto err_out;
+	if (!core || !pdentry)
+		return;
 
-	if (core->num_parents > 1) {
-		d = debugfs_create_file("clk_possible_parents", 0444,
-				core->dentry, core, &possible_parents_fops);
-		if (!d)
-			goto err_out;
-	}
+	root = debugfs_create_dir(core->name, pdentry);
+	core->dentry = root;
 
-	if (core->ops->debug_init) {
-		ret = core->ops->debug_init(core->hw, core->dentry);
-		if (ret)
-			goto err_out;
-	}
+	debugfs_create_ulong("clk_rate", 0444, root, &core->rate);
+	debugfs_create_ulong("clk_accuracy", 0444, root, &core->accuracy);
+	debugfs_create_u32("clk_phase", 0444, root, &core->phase);
+	debugfs_create_file("clk_flags", 0444, root, core, &clk_flags_fops);
+	debugfs_create_u32("clk_prepare_count", 0444, root, &core->prepare_count);
+	debugfs_create_u32("clk_enable_count", 0444, root, &core->enable_count);
+	debugfs_create_u32("clk_protect_count", 0444, root, &core->protect_count);
+	debugfs_create_u32("clk_notifier_count", 0444, root, &core->notifier_count);
 
-	ret = 0;
-	goto out;
+	if (core->num_parents > 1)
+		debugfs_create_file("clk_possible_parents", 0444, root, core,
+				    &possible_parents_fops);
 
-err_out:
-	debugfs_remove_recursive(core->dentry);
-	core->dentry = NULL;
-out:
-	return ret;
+	if (core->ops->debug_init)
+		core->ops->debug_init(core->hw, core->dentry);
 }
 
 /**
@@ -2693,17 +2643,13 @@ static int clk_debug_create_one(struct clk_core *core, struct dentry *pdentry)
  * initialized.  Otherwise it bails out early since the debugfs clk directory
  * will be created lazily by clk_debug_init as part of a late_initcall.
  */
-static int clk_debug_register(struct clk_core *core)
+static void clk_debug_register(struct clk_core *core)
 {
-	int ret = 0;
-
 	mutex_lock(&clk_debug_lock);
 	hlist_add_head(&core->debug_node, &clk_debug_list);
 	if (inited)
-		ret = clk_debug_create_one(core, rootdir);
+		clk_debug_create_one(core, rootdir);
 	mutex_unlock(&clk_debug_lock);
-
-	return ret;
 }
 
  /**
@@ -2748,32 +2694,17 @@ EXPORT_SYMBOL_GPL(clk_debugfs_add_file);
 static int __init clk_debug_init(void)
 {
 	struct clk_core *core;
-	struct dentry *d;
 
 	rootdir = debugfs_create_dir("clk", NULL);
 
-	if (!rootdir)
-		return -ENOMEM;
-
-	d = debugfs_create_file("clk_summary", 0444, rootdir, &all_lists,
-				&clk_summary_fops);
-	if (!d)
-		return -ENOMEM;
-
-	d = debugfs_create_file("clk_dump", 0444, rootdir, &all_lists,
-				&clk_dump_fops);
-	if (!d)
-		return -ENOMEM;
-
-	d = debugfs_create_file("clk_orphan_summary", 0444, rootdir,
-				&orphan_list, &clk_summary_fops);
-	if (!d)
-		return -ENOMEM;
-
-	d = debugfs_create_file("clk_orphan_dump", 0444, rootdir,
-				&orphan_list, &clk_dump_fops);
-	if (!d)
-		return -ENOMEM;
+	debugfs_create_file("clk_summary", 0444, rootdir, &all_lists,
+			    &clk_summary_fops);
+	debugfs_create_file("clk_dump", 0444, rootdir, &all_lists,
+			    &clk_dump_fops);
+	debugfs_create_file("clk_orphan_summary", 0444, rootdir, &orphan_list,
+			    &clk_summary_fops);
+	debugfs_create_file("clk_orphan_dump", 0444, rootdir, &orphan_list,
+			    &clk_dump_fops);
 
 	mutex_lock(&clk_debug_lock);
 	hlist_for_each_entry(core, &clk_debug_list, debug_node)
@@ -2786,7 +2717,7 @@ static int __init clk_debug_init(void)
 }
 late_initcall(clk_debug_init);
 #else
-static inline int clk_debug_register(struct clk_core *core) { return 0; }
+static inline void clk_debug_register(struct clk_core *core) { }
 static inline void clk_debug_reparent(struct clk_core *core,
 				      struct clk_core *new_parent)
 {
-- 
2.17.0

[PATCH 2/5] clk: bcm2835: no need to check return value of debugfs_create functions

[Greg Kroah-Hartman]

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

Cc: Michael Turquette <mturquette@baylibre.com>
Cc: Stephen Boyd <sboyd@kernel.org>
Cc: Eric Anholt <eric@anholt.net>
Cc: Stefan Wahren <stefan.wahren@i2se.com>
Cc: Florian Fainelli <f.fainelli@gmail.com>
Cc: Ray Jui <rjui@broadcom.com>
Cc: Scott Branden <sbranden@broadcom.com>
Cc: bcm-kernel-feedback-list@broadcom.com
Cc: Phil Elwell <phil@raspberrypi.org>
Cc: Boris Brezillon <boris.brezillon@bootlin.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Danilo Krummrich <danilokrummrich@dk-develop.de>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/clk/bcm/clk-bcm2835.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/clk/bcm/clk-bcm2835.c b/drivers/clk/bcm/clk-bcm2835.c
index fa0d5c8611a0..1329440af59f 100644
--- a/drivers/clk/bcm/clk-bcm2835.c
+++ b/drivers/clk/bcm/clk-bcm2835.c
@@ -398,7 +398,6 @@ static int bcm2835_debugfs_regset(struct bcm2835_cprman *cprman, u32 base,
 				  struct debugfs_reg32 *regs, size_t nregs,
 				  struct dentry *dentry)
 {
-	struct dentry *regdump;
 	struct debugfs_regset32 *regset;
 
 	regset = devm_kzalloc(cprman->dev, sizeof(*regset), GFP_KERNEL);
@@ -409,10 +408,9 @@ static int bcm2835_debugfs_regset(struct bcm2835_cprman *cprman, u32 base,
 	regset->nregs = nregs;
 	regset->base = cprman->regs + base;
 
-	regdump = debugfs_create_regset32("regdump", S_IRUGO, dentry,
-					  regset);
+	debugfs_create_regset32("regdump", S_IRUGO, dentry, regset);
 
-	return regdump ? 0 : -ENOMEM;
+	return 0;
 }
 
 struct bcm2835_pll_data {
-- 
2.17.0

Re: [PATCH 2/5] clk: bcm2835: no need to check return value of debugfs_create functions

[Eric Anholt]

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

Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:

> 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.

I like it.

Reviewed-by: Eric Anholt <eric@anholt.net>

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

Re: [PATCH 2/5] clk: bcm2835: no need to check return value of debugfs_create functions

[Stephen Boyd]

Quoting Greg Kroah-Hartman (2018-05-29 09:08:01)
> 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 Turquette <mturquette@baylibre.com>
> Cc: Stephen Boyd <sboyd@kernel.org>
> Cc: Eric Anholt <eric@anholt.net>
> Cc: Stefan Wahren <stefan.wahren@i2se.com>
> Cc: Florian Fainelli <f.fainelli@gmail.com>
> Cc: Ray Jui <rjui@broadcom.com>
> Cc: Scott Branden <sbranden@broadcom.com>
> Cc: bcm-kernel-feedback-list@broadcom.com
> Cc: Phil Elwell <phil@raspberrypi.org>
> Cc: Boris Brezillon <boris.brezillon@bootlin.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Danilo Krummrich <danilokrummrich@dk-develop.de>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---

Applied to clk-next

[PATCH 3/5] clk: davinci: no need to check return value of debugfs_create functions

[Greg Kroah-Hartman]

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

Cc: David Lechner <david@lechnology.com>
Cc: Sekhar Nori <nsekhar@ti.com>
Cc: Michael Turquette <mturquette@baylibre.com>
Cc: Stephen Boyd <sboyd@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/clk/davinci/pll.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/clk/davinci/pll.c b/drivers/clk/davinci/pll.c
index 23a24c944f1d..bb9594703d4a 100644
--- a/drivers/clk/davinci/pll.c
+++ b/drivers/clk/davinci/pll.c
@@ -878,7 +878,6 @@ static int davinci_pll_debug_init(struct clk_hw *hw, struct dentry *dentry)
 {
 	struct davinci_pll_clk *pll = to_davinci_pll_clk(hw);
 	struct debugfs_regset32 *regset;
-	struct dentry *d;
 
 	regset = kzalloc(sizeof(*regset), GFP_KERNEL);
 	if (!regset)
@@ -888,11 +887,7 @@ static int davinci_pll_debug_init(struct clk_hw *hw, struct dentry *dentry)
 	regset->nregs = ARRAY_SIZE(davinci_pll_regs);
 	regset->base = pll->base;
 
-	d = debugfs_create_regset32("registers", 0400, dentry, regset);
-	if (IS_ERR(d)) {
-		kfree(regset);
-		return PTR_ERR(d);
-	}
+	debugfs_create_regset32("registers", 0400, dentry, regset);
 
 	return 0;
 }
-- 
2.17.0

Re: [PATCH 3/5] clk: davinci: no need to check return value of debugfs_create functions

[David Lechner]

On 05/29/2018 11:08 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.
> 
> Cc: David Lechner <david@lechnology.com>
> Cc: Sekhar Nori <nsekhar@ti.com>
> Cc: Michael Turquette <mturquette@baylibre.com>
> Cc: Stephen Boyd <sboyd@kernel.org>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---

Acked-by: David Lechner <david@lechnology.com>

Re: [PATCH 3/5] clk: davinci: no need to check return value of debugfs_create functions

[Stephen Boyd]

Quoting Greg Kroah-Hartman (2018-05-29 09:08:02)
> 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 Lechner <david@lechnology.com>
> Cc: Sekhar Nori <nsekhar@ti.com>
> Cc: Michael Turquette <mturquette@baylibre.com>
> Cc: Stephen Boyd <sboyd@kernel.org>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---

Applied to clk-next

[PATCH 4/5] clk: tegra: no need to check return value of debugfs_create functions

[Greg Kroah-Hartman]

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

The return value of these functions were never checked in the end
anyway, so it is obvious this does not change any functionality :)

Cc: Peter De Schrijver <pdeschrijver@nvidia.com>
Cc: Prashant Gaikwad <pgaikwad@nvidia.com>
Cc: Michael Turquette <mturquette@baylibre.com>
Cc: Stephen Boyd <sboyd@kernel.org>
Cc: Thierry Reding <thierry.reding@gmail.com>
Cc: Jonathan Hunter <jonathanh@nvidia.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/clk/tegra/clk-dfll.c | 42 ++++++++++--------------------------
 1 file changed, 11 insertions(+), 31 deletions(-)

diff --git a/drivers/clk/tegra/clk-dfll.c b/drivers/clk/tegra/clk-dfll.c
index 0a7deee74eea..48ee43734e05 100644
--- a/drivers/clk/tegra/clk-dfll.c
+++ b/drivers/clk/tegra/clk-dfll.c
@@ -1196,42 +1196,24 @@ static const struct file_operations attr_registers_fops = {
 	.release	= single_release,
 };
 
-static int dfll_debug_init(struct tegra_dfll *td)
+static void dfll_debug_init(struct tegra_dfll *td)
 {
-	int ret;
+	struct dentry *root;
 
 	if (!td || (td->mode == DFLL_UNINITIALIZED))
-		return 0;
-
-	td->debugfs_dir = debugfs_create_dir("tegra_dfll_fcpu", NULL);
-	if (!td->debugfs_dir)
-		return -ENOMEM;
-
-	ret = -ENOMEM;
-
-	if (!debugfs_create_file("enable", S_IRUGO | S_IWUSR,
-				 td->debugfs_dir, td, &enable_fops))
-		goto err_out;
-
-	if (!debugfs_create_file("lock", S_IRUGO,
-				 td->debugfs_dir, td, &lock_fops))
-		goto err_out;
+		return;
 
-	if (!debugfs_create_file("rate", S_IRUGO,
-				 td->debugfs_dir, td, &rate_fops))
-		goto err_out;
+	root = debugfs_create_dir("tegra_dfll_fcpu", NULL);
+	td->debugfs_dir = root;
 
-	if (!debugfs_create_file("registers", S_IRUGO,
-				 td->debugfs_dir, td, &attr_registers_fops))
-		goto err_out;
-
-	return 0;
-
-err_out:
-	debugfs_remove_recursive(td->debugfs_dir);
-	return ret;
+	debugfs_create_file("enable", S_IRUGO | S_IWUSR, root, td, &enable_fops);
+	debugfs_create_file("lock", S_IRUGO, root, td, &lock_fops);
+	debugfs_create_file("rate", S_IRUGO, root, td, &rate_fops);
+	debugfs_create_file("registers", S_IRUGO, root, td, &attr_registers_fops);
 }
 
+#else
+static void inline dfll_debug_init(struct tegra_dfll *td) { }
 #endif /* CONFIG_DEBUG_FS */
 
 /*
@@ -1715,9 +1697,7 @@ int tegra_dfll_register(struct platform_device *pdev,
 		return ret;
 	}
 
-#ifdef CONFIG_DEBUG_FS
 	dfll_debug_init(td);
-#endif
 
 	return 0;
 }
-- 
2.17.0

Re: [PATCH 4/5] clk: tegra: no need to check return value of debugfs_create functions

[Stephen Boyd]

Quoting Greg Kroah-Hartman (2018-05-29 09:08:03)
> 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.
> =

> The return value of these functions were never checked in the end
> anyway, so it is obvious this does not change any functionality :)
> =

> Cc: Peter De Schrijver <pdeschrijver@nvidia.com>
> Cc: Prashant Gaikwad <pgaikwad@nvidia.com>
> Cc: Michael Turquette <mturquette@baylibre.com>
> Cc: Stephen Boyd <sboyd@kernel.org>
> Cc: Thierry Reding <thierry.reding@gmail.com>
> Cc: Jonathan Hunter <jonathanh@nvidia.com>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---

Applied to clk-next

[PATCH 5/5] clk: remove clk_debugfs_add_file()

[Greg Kroah-Hartman]

No one was using this api call, so remove it.  If it is ever needed in
the future, a "raw" debugfs call can be used.

Cc: Michael Turquette <mturquette@baylibre.com>
Cc: Stephen Boyd <sboyd@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/clk/clk.c            | 13 -------------
 include/linux/clk-provider.h |  5 -----
 2 files changed, 18 deletions(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index ef701ce44439..59207cd6138a 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -2669,19 +2669,6 @@ static void clk_debug_unregister(struct clk_core *core)
 	mutex_unlock(&clk_debug_lock);
 }
 
-struct dentry *clk_debugfs_add_file(struct clk_hw *hw, char *name, umode_t mode,
-				void *data, const struct file_operations *fops)
-{
-	struct dentry *d = NULL;
-
-	if (hw->core->dentry)
-		d = debugfs_create_file(name, mode, hw->core->dentry, data,
-					fops);
-
-	return d;
-}
-EXPORT_SYMBOL_GPL(clk_debugfs_add_file);
-
 /**
  * clk_debug_init - lazily populate the debugfs clk directory
  *
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index 210a890008f9..00e55d4d07f1 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -996,10 +996,5 @@ static inline void clk_writel(u32 val, u32 __iomem *reg)
 
 #endif	/* platform dependent I/O accessors */
 
-#ifdef CONFIG_DEBUG_FS
-struct dentry *clk_debugfs_add_file(struct clk_hw *hw, char *name, umode_t mode,
-				void *data, const struct file_operations *fops);
-#endif
-
 #endif /* CONFIG_COMMON_CLK */
 #endif /* CLK_PROVIDER_H */
-- 
2.17.0

Re: [PATCH 5/5] clk: remove clk_debugfs_add_file()

[Stephen Boyd]

Quoting Greg Kroah-Hartman (2018-05-29 09:08:04)
> No one was using this api call, so remove it.  If it is ever needed in
> the future, a "raw" debugfs call can be used.
> =

> Cc: Michael Turquette <mturquette@baylibre.com>
> Cc: Stephen Boyd <sboyd@kernel.org>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---

Applied to clk-next

Re: [PATCH 1/5] clk: no need to check return value of debugfs_create functions

[Stephen Boyd]

Quoting Greg Kroah-Hartman (2018-05-29 09:08:00)
> -
> -       d =3D debugfs_create_u32("clk_protect_count", 0444, core->dentry,
> -                              &core->protect_count);
> -       if (!d)
> -               goto err_out;
> -
> -       d =3D debugfs_create_u32("clk_notifier_count", 0444, core->dentry,
> -                              &core->notifier_count);
> -       if (!d)
> -               goto err_out;
> +       if (!core || !pdentry)
> +               return;
>  =

> -       if (core->num_parents > 1) {
> -               d =3D debugfs_create_file("clk_possible_parents", 0444,
> -                               core->dentry, core, &possible_parents_fop=
s);
> -               if (!d)
> -                       goto err_out;
> -       }
> +       root =3D debugfs_create_dir(core->name, pdentry);
> +       core->dentry =3D root;
>  =

> -       if (core->ops->debug_init) {
> -               ret =3D core->ops->debug_init(core->hw, core->dentry);

This returns an int. We should go fix all the ops to return void now
because nobody should care. I can pile that patch on top.

Anyway, applied to clk-next.

Re: [PATCH 1/5] clk: no need to check return value of debugfs_create functions

[Greg Kroah-Hartman]

On Fri, Jun 01, 2018 at 07:27:15PM -0700, Stephen Boyd wrote:
> Quoting Greg Kroah-Hartman (2018-05-29 09:08:00)
> > -
> > -       d = debugfs_create_u32("clk_protect_count", 0444, core->dentry,
> > -                              &core->protect_count);
> > -       if (!d)
> > -               goto err_out;
> > -
> > -       d = debugfs_create_u32("clk_notifier_count", 0444, core->dentry,
> > -                              &core->notifier_count);
> > -       if (!d)
> > -               goto err_out;
> > +       if (!core || !pdentry)
> > +               return;
> >  
> > -       if (core->num_parents > 1) {
> > -               d = debugfs_create_file("clk_possible_parents", 0444,
> > -                               core->dentry, core, &possible_parents_fops);
> > -               if (!d)
> > -                       goto err_out;
> > -       }
> > +       root = debugfs_create_dir(core->name, pdentry);
> > +       core->dentry = root;
> >  
> > -       if (core->ops->debug_init) {
> > -               ret = core->ops->debug_init(core->hw, core->dentry);
> 
> This returns an int. We should go fix all the ops to return void now
> because nobody should care. I can pile that patch on top.

Thanks, I didn't think to do that.

> Anyway, applied to clk-next.

Wonderful, thanks for applying all of these.

greg k-h