| Message ID | 1405437890-6468-2-git-send-email-pdeschrijver@nvidia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Jul 15, 2014 at 06:24:31PM +0300, Peter De Schrijver wrote: > Just continue initializing clocks if there's an error on one of them. This > is useful if there's a mistake in the inittable, because the system could > hang if clk_disable_unused() disables some of the critical clocks in this > table. > > Signed-off-by: Peter De Schrijver <pdeschrijver@nvidia.com> > --- > drivers/clk/tegra/clk.c | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/drivers/clk/tegra/clk.c b/drivers/clk/tegra/clk.c > index c0a7d77..d081732 100644 > --- a/drivers/clk/tegra/clk.c > +++ b/drivers/clk/tegra/clk.c > @@ -207,7 +207,7 @@ void __init tegra_init_from_table(struct tegra_clk_init_table *tbl, > for (; tbl->clk_id < clk_max; tbl++) { > clk = clks[tbl->clk_id]; > if (IS_ERR_OR_NULL(clk)) > - return; > + continue; Perhaps rather than silently ignoring, should this at least print out an error? I'd even go as far as make it a full-blown WARN to make sure people notice and this gets fixed early. Thierry
On 07/15/2014 09:24 AM, Peter De Schrijver wrote: > Just continue initializing clocks if there's an error on one of them. This > is useful if there's a mistake in the inittable, because the system could > hang if clk_disable_unused() disables some of the critical clocks in this > table. If there's a problem in the init table, we should simply fix it instead of working around it. At the very least, we need to WARN on this rather than just ignoring problems.
On Tue, Jul 22, 2014 at 07:16:15PM +0200, Stephen Warren wrote: > On 07/15/2014 09:24 AM, Peter De Schrijver wrote: > > Just continue initializing clocks if there's an error on one of them. This > > is useful if there's a mistake in the inittable, because the system could > > hang if clk_disable_unused() disables some of the critical clocks in this > > table. > > If there's a problem in the init table, we should simply fix it instead > of working around it. > Yes, ofcourse. However today we silently stop processing the init_table if a clock cannot be found. That doesn't sound right either to me and makes detecting wrong entries in the table more complex than it should be. > At the very least, we need to WARN on this rather than just ignoring > problems. Cheers, Peter.
diff --git a/drivers/clk/tegra/clk.c b/drivers/clk/tegra/clk.c index c0a7d77..d081732 100644 --- a/drivers/clk/tegra/clk.c +++ b/drivers/clk/tegra/clk.c @@ -207,7 +207,7 @@ void __init tegra_init_from_table(struct tegra_clk_init_table *tbl, for (; tbl->clk_id < clk_max; tbl++) { clk = clks[tbl->clk_id]; if (IS_ERR_OR_NULL(clk)) - return; + continue; if (tbl->parent_id < clk_max) { struct clk *parent = clks[tbl->parent_id];
Just continue initializing clocks if there's an error on one of them. This is useful if there's a mistake in the inittable, because the system could hang if clk_disable_unused() disables some of the critical clocks in this table. Signed-off-by: Peter De Schrijver <pdeschrijver@nvidia.com> --- drivers/clk/tegra/clk.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)