Hi Jerome, Thanks for your review comments. On Mon, 17 Feb 2020 at 13:32, Jerome Brunet <jbrunet@xxxxxxxxxxxx> wrote: > > > On Sun 16 Feb 2020 at 18:34, Anand Moon <linux.amoon@xxxxxxxxx> wrote: > > > Odroid N2 would fail to boot using microSD unless we set > > cpu freq clk divider flags to CLK_IS_CRITICAL to avoid stalling of > > cpu when booting, most likely because of PWM module linked to > > Where did you see a PWM ? > > > the CPU for DVFS is getting disabled in between the late_init call, > > between the late_init call and what ? > > > so gaiting the clock source shuts down the power to the codes. > > what code ? > > > Setting clk divider flags to CLK_IS_CRITICAL help resolve the issue. > > > > Cc: Martin Blumenstingl <martin.blumenstingl@xxxxxxxxxxxxxx> > > Cc: Jerome Brunet <jbrunet@xxxxxxxxxxxx> > > Cc: Neil Armstrong <narmstrong@xxxxxxxxxxxx> > > Suggested-by: Neil Armstrong <narmstrong@xxxxxxxxxxxx> > > Signed-off-by: Anand Moon <linux.amoon@xxxxxxxxx> > > --- > > > > Following Neil's suggestion, I have prepared this patch. > > https://patchwork.kernel.org/patch/11177441/#22964889 > > --- > > drivers/clk/meson/g12a.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/clk/meson/g12a.c b/drivers/clk/meson/g12a.c > > index d2760a021301..accae3695fe5 100644 > > --- a/drivers/clk/meson/g12a.c > > +++ b/drivers/clk/meson/g12a.c > > @@ -283,6 +283,7 @@ static struct clk_fixed_factor g12a_fclk_div2_div = { > > .ops = &clk_fixed_factor_ops, > > .parent_hws = (const struct clk_hw *[]) { &g12a_fixed_pll.hw }, > > .num_parents = 1, > > + .flags = CLK_IS_CRITICAL, > > This makes no sense for because: > * This clock cannot gate and none of its parents can either. IOW, the > output of this clock is never disabled. > * I cannot guess the relation between fdiv2 and the commit description > > > }, > > }; > > > > @@ -681,7 +682,7 @@ static struct clk_regmap g12b_cpub_clk = { > > &g12a_sys_pll.hw > > }, > > .num_parents = 2, > > - .flags = CLK_SET_RATE_PARENT, > > + .flags = CLK_SET_RATE_PARENT | CLK_IS_CRITICAL, > > Why not. Neil what do you think of this ? > If nothing is claiming this clock and enabling it then I suppose it > could make sense. > > > > }, > > }; > Sorry for the noise, I should not have send this patch in first place. -Anand
