clk: no need to check return value of debugfs_create functions
Subsystems (6)
PATCH 1/5 clk: no need to check return value of debugfs_create functions
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 :)
--- drivers/clk/clk.c | 129 +++++++++++----------------------------------- 1 file changed, 30 insertions(+), 99 deletions(-)
drivers/clk/clk.c +30 -99
@@ -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) {
Signature · 1 line
2.17.0
PATCH 2/5 clk: bcm2835: no need to check return value of debugfs_create functions
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.
--- drivers/clk/bcm/clk-bcm2835.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
drivers/clk/bcm/clk-bcm2835.c +2 -4
@@ -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 {
Signature · 1 line
2.17.0
Re: [PATCH 2/5] clk: bcm2835: no need to check return value of debugfs_create functions
Quoted 3 lines · Greg Kroah-Hartman
When calling debugfs functions, there is no need to ever check the return value. The function can work or not, but the code logic should never do something different based on this.
I like it.
Re: [PATCH 2/5] clk: bcm2835: no need to check return value of debugfs_create functions
In reply to Greg Kroah-HartmanQuoting Greg Kroah-Hartman (2018-05-29 09:08:01)
Quoted 18 lines
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 <[email protected]>Cc Stephen Boyd <[email protected]>Cc Eric Anholt <[email protected]>Cc Stefan Wahren <[email protected]>Cc Florian Fainelli <[email protected]>Cc Ray Jui <[email protected]>Cc Scott Branden <[email protected]>Cc Phil Elwell <[email protected]>Cc Boris Brezillon <[email protected]>Cc Greg Kroah-Hartman <[email protected]>Cc Danilo Krummrich <[email protected]>Signed-off-by Greg Kroah-Hartman <[email protected]>
Quoted 1 line
---
Applied to clk-next
PATCH 3/5 clk: davinci: no need to check return value of debugfs_create functions
In reply to Greg Kroah-HartmanWhen 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.
--- drivers/clk/davinci/pll.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-)
drivers/clk/davinci/pll.c +1 -6
@@ -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 ; }
Signature · 1 line
2.17.0
Re: [PATCH 3/5] clk: davinci: no need to check return value of debugfs_create functions
Quoted 10 lines · Greg Kroah-Hartman
When calling debugfs functions, there is no need to ever check the return value. The function can work or not, but the code logic should never do something different based on this.
Cc David Lechner <[email protected]>Cc Sekhar Nori <[email protected]>Cc Michael Turquette <[email protected]>Cc Stephen Boyd <[email protected]>Signed-off-by Greg Kroah-Hartman <[email protected]>---
Re: [PATCH 3/5] clk: davinci: no need to check return value of debugfs_create functions
In reply to Greg Kroah-HartmanQuoting Greg Kroah-Hartman (2018-05-29 09:08:02)
Quoted 11 lines
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 <[email protected]>Cc Sekhar Nori <[email protected]>Cc Michael Turquette <[email protected]>Cc Stephen Boyd <[email protected]>Signed-off-by Greg Kroah-Hartman <[email protected]>---
Applied to clk-next
PATCH 4/5 clk: tegra: no need to check return value of debugfs_create functions
In reply to Greg Kroah-HartmanWhen 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 :)
--- drivers/clk/tegra/clk-dfll.c | 42 ++++++++++-------------------------- 1 file changed, 11 insertions(+), 31 deletions(-)
drivers/clk/tegra/clk-dfll.c +11 -31
@@ -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 ; }
Signature · 1 line
2.17.0
Re: [PATCH 4/5] clk: tegra: no need to check return value of debugfs_create functions
Quoting Greg Kroah-Hartman (2018-05-29 09:08:03)
Quoted 9 lines
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 :) =
Quoted 8 lines
Cc Peter De Schrijver <[email protected]>Cc Prashant Gaikwad <[email protected]>Cc Michael Turquette <[email protected]>Cc Stephen Boyd <[email protected]>Cc Thierry Reding <[email protected]>Cc Jonathan Hunter <[email protected]>Signed-off-by Greg Kroah-Hartman <[email protected]>---
Applied to clk-next
PATCH 5/5 clk: remove clk_debugfs_add_file()
In reply to Greg Kroah-HartmanNo one was using this api call, so remove it. If it is ever needed in the future, a "raw" debugfs call can be used.
--- drivers/clk/clk.c | 13 ------------- include/linux/clk-provider.h | 5 ----- 2 files changed, 18 deletions(-)
drivers/clk/clk.c +0 -13
@@ -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 *
include/linux/clk-provider.h +0 -5
@@ -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 */
Signature · 1 line
2.17.0
Re: [PATCH 5/5] clk: remove clk_debugfs_add_file()
Quoting Greg Kroah-Hartman (2018-05-29 09:08:04)
Quoted 8 lines
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 <[email protected]>Cc Stephen Boyd <[email protected]>Signed-off-by Greg Kroah-Hartman <[email protected]>---
Applied to clk-next
Stephen Boyd
maintainerQuoting Greg Kroah-Hartman (2018-05-29 09:08:00)
Quoted 7 lines
drivers/clk/clk.c +0
-7- - 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,
drivers/clk/clk.c static int possible_parents_show(struct seq_file *s, void *data)
Quoted 10 lines
drivers/clk/clk.c +2
-6- &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);
drivers/clk/clk.c static int possible_parents_show(struct seq_file *s, void *data)
Quoted 9 lines
drivers/clk/clk.c +2
-5-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.
Quoted 23 lines · Stephen Boyd
Quoting Greg Kroah-Hartman (2018-05-29 09:08:00)
Quoted 22 lines
drivers/clk/clk.c +4
-16- - 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;
Quoted 5 lines
drivers/clk/clk.c static int possible_parents_show(struct seq_file *s, void *data) · line 2673
Quoted 2 lines
drivers/clk/clk.c +0
-2-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.
Quoted 1 line
Anyway, applied to clk-next.
Wonderful, thanks for applying all of these.
greg k-h