| Message ID | 20200216173446.1823-4-linux.amoon@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Odroid N2 failes to boot using upstream kernel using microSD card | expand |
On Sun 16 Feb 2020 at 18:34, Anand Moon <linux.amoon@gmail.com> 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@googlemail.com> > Cc: Jerome Brunet <jbrunet@baylibre.com> > Cc: Neil Armstrong <narmstrong@baylibre.com> > Suggested-by: Neil Armstrong <narmstrong@baylibre.com> > Signed-off-by: Anand Moon <linux.amoon@gmail.com> > --- > > 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. > }, > };
Hi Jerome, Thanks for your review comments. On Mon, 17 Feb 2020 at 13:32, Jerome Brunet <jbrunet@baylibre.com> wrote: > > > On Sun 16 Feb 2020 at 18:34, Anand Moon <linux.amoon@gmail.com> 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@googlemail.com> > > Cc: Jerome Brunet <jbrunet@baylibre.com> > > Cc: Neil Armstrong <narmstrong@baylibre.com> > > Suggested-by: Neil Armstrong <narmstrong@baylibre.com> > > Signed-off-by: Anand Moon <linux.amoon@gmail.com> > > --- > > > > 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
Hi Jeronme, Thanks for your review comments. On Mon, 17 Feb 2020 at 13:32, Jerome Brunet <jbrunet@baylibre.com> wrote: > > > On Sun 16 Feb 2020 at 18:34, Anand Moon <linux.amoon@gmail.com> 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 ? > Sorry, I was really upset about my self. I tried to improvise this commit message based on previous mails. sorry about that. > > Setting clk divider flags to CLK_IS_CRITICAL help resolve the issue. > > > > Cc: Martin Blumenstingl <martin.blumenstingl@googlemail.com> > > Cc: Jerome Brunet <jbrunet@baylibre.com> > > Cc: Neil Armstrong <narmstrong@baylibre.com> > > Suggested-by: Neil Armstrong <narmstrong@baylibre.com> > > Signed-off-by: Anand Moon <linux.amoon@gmail.com> > > --- > > > > 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 > Ok I check this code changes is not needed for this fix. > > }, > > }; > > > > @@ -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. > I would like core developers to handle this. Sorry for the noise. -Anand > > > }, > > }; >
Hi Anand, On Mon, Feb 17, 2020 at 2:30 PM Anand Moon <linux.amoon@gmail.com> wrote: [...] > > > @@ -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. > > > I would like core developers to handle this. > Sorry for the noise. can you please resend this patch with only the change to g12b_cpub_clk? I have no G12B board myself so it would be great if you could take care of this! Martin
Hi Martin / Jerome / Neil, On Fri, 21 Feb 2020 at 02:45, Martin Blumenstingl <martin.blumenstingl@googlemail.com> wrote: > > Hi Anand, > > On Mon, Feb 17, 2020 at 2:30 PM Anand Moon <linux.amoon@gmail.com> wrote: > [...] > > > > @@ -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. > > > > > I would like core developers to handle this. > > Sorry for the noise. > can you please resend this patch with only the change to g12b_cpub_clk? > I have no G12B board myself so it would be great if you could take care of this! > > > Martin Thanks, yes I will try again, but I have a question. On eMMC module *cpub_clk* is not getting enabled, see below is clk_summay of eMMC. [...] fclk_div2_div 1 1 0 999999985 0 0 50000 fclk_div2 2 2 0 999999985 0 0 50000 ff3f0000.ethernet#m250_sel 1 1 0 999999985 0 0 50000 ff3f0000.ethernet#m250_div 1 1 0 249999997 0 0 50000 ff3f0000.ethernet#fixed_div2 1 1 0 124999998 0 0 50000 ff3f0000.ethernet#rgmii_tx_en 1 1 0 124999998 0 0 50000 ffe07000.mmc#mux 1 1 0 999999985 0 0 50000 ffe07000.mmc#div 1 1 0 199999997 0 0 50000 cpub_clk_dyn1_sel 0 0 0 999999985 0 0 50000 cpub_clk_dyn1 0 0 0 999999985 0 0 50000 cpub_clk_dyn 0 0 0 999999985 0 0 50000 cpub_clk 0 0 0 999999985 0 0 50000 cpub_clk_div8 0 0 0 124999998 0 0 50000 cpub_clk_div7 0 0 0 142857140 0 0 50000 cpub_clk_div6 0 0 0 166666664 0 0 50000 cpub_clk_trace_sel 0 0 0 166666664 0 0 50000 cpub_clk_trace 0 0 0 166666664 0 0 50000 cpub_clk_div5 0 0 0 199999997 0 0 50000 cpub_clk_apb_sel 0 0 0 199999997 0 0 50000 cpub_clk_apb 0 0 0 199999997 0 0 50000 cpub_clk_div4 0 0 0 249999996 0 0 50000 cpub_clk_div3 0 0 0 333333328 0 0 50000 cpub_clk_atb_sel 0 0 0 333333328 0 0 50000 cpub_clk_atb 0 0 0 333333328 0 0 50000 cpub_clk_div2 0 0 0 499999992 0 0 50000 cpub_clk_axi_sel 0 0 0 499999992 0 0 50000 cpub_clk_axi 0 0 0 499999992 0 0 50000 cpub_clk_div16_en 0 0 0 999999985 0 0 50000 cpub_clk_div16 0 0 0 62499999 0 0 50000 After enable *cpub_clk* flags with .flags = CLK_SET_RATE_PARENT | CLK_IS_CRITICAL, this clk is enabled on microSD card see clk_summary below. [...] fclk_div2_div 1 1 0 999999985 0 0 50000 fclk_div2 3 3 0 999999985 0 0 50000 ff3f0000.ethernet#m250_sel 1 1 0 999999985 0 0 50000 ff3f0000.ethernet#m250_div 1 1 0 249999997 0 0 50000 ff3f0000.ethernet#fixed_div2 1 1 0 124999998 0 0 50000 ff3f0000.ethernet#rgmii_tx_en 1 1 0 124999998 0 0 50000 ffe05000.sd#mux 1 1 0 999999985 0 0 50000 ffe05000.sd#div 1 1 0 50000000 0 0 50000 cpub_clk_dyn1_sel 1 1 0 999999985 0 0 50000 cpub_clk_dyn1 1 1 0 999999985 0 0 50000 cpub_clk_dyn 1 1 0 999999985 0 0 50000 cpub_clk 1 1 0 999999985 0 0 50000 cpub_clk_div8 0 0 0 124999998 0 0 50000 cpub_clk_div7 0 0 0 142857140 0 0 50000 cpub_clk_div6 0 0 0 166666664 0 0 50000 cpub_clk_trace_sel 0 0 0 166666664 0 0 50000 cpub_clk_trace 0 0 0 166666664 0 0 50000 cpub_clk_div5 0 0 0 199999997 0 0 50000 cpub_clk_apb_sel 0 0 0 199999997 0 0 50000 cpub_clk_apb 0 0 0 199999997 0 0 50000 cpub_clk_div4 0 0 0 249999996 0 0 50000 cpub_clk_div3 0 0 0 333333328 0 0 50000 cpub_clk_atb_sel 0 0 0 333333328 0 0 50000 cpub_clk_atb 0 0 0 333333328 0 0 50000 cpub_clk_div2 0 0 0 499999992 0 0 50000 cpub_clk_axi_sel 0 0 0 499999992 0 0 50000 cpub_clk_axi 0 0 0 499999992 0 0 50000 cpub_clk_div16_en 0 0 0 999999985 0 0 50000 cpub_clk_div16 0 0 0 62499999 0 0 50000 cpub_clk_dyn1_div 0 0 0 999999985 0 0 50000 Is this correct approach to set the flags to enable *cpub_clk*. .flags = CLK_SET_RATE_PARENT | CLK_IS_CRITICAL, What I meant is their *Dyn_enable[26]* field for enable/disable for HHI_SYS_CPU_CLK_CNTL0 and HHI_SYS_CPUB_CLK_CNTL clk controller. in the S922X datasheets which could help resolve this issue. Any thought on this. -Anand
On Sun 23 Feb 2020 at 14:34, Anand Moon <linux.amoon@gmail.com> wrote: > Hi Martin / Jerome / Neil, > > On Fri, 21 Feb 2020 at 02:45, Martin Blumenstingl > <martin.blumenstingl@googlemail.com> wrote: >> >> Hi Anand, >> >> On Mon, Feb 17, 2020 at 2:30 PM Anand Moon <linux.amoon@gmail.com> wrote: >> [...] >> > > > @@ -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. >> > > >> > I would like core developers to handle this. >> > Sorry for the noise. >> can you please resend this patch with only the change to g12b_cpub_clk? >> I have no G12B board myself so it would be great if you could take care of this! >> >> >> Martin > > Thanks, yes I will try again, but I have a question. > > On eMMC module *cpub_clk* is not getting enabled, see below is > clk_summay of eMMC. I'm sorry but I don't understand the link between the cpu clock of the second cluster and MMC. > [...] > fclk_div2_div 1 1 0 999999985 > 0 0 50000 > fclk_div2 2 2 0 999999985 > 0 0 50000 > ff3f0000.ethernet#m250_sel 1 1 0 > 999999985 0 0 50000 > ff3f0000.ethernet#m250_div 1 1 > 0 249999997 0 0 50000 > ff3f0000.ethernet#fixed_div2 1 1 > 0 124999998 0 0 50000 > ff3f0000.ethernet#rgmii_tx_en 1 > 1 0 124999998 0 0 50000 > ffe07000.mmc#mux 1 1 0 999999985 > 0 0 50000 > ffe07000.mmc#div 1 1 0 > 199999997 0 0 50000 > cpub_clk_dyn1_sel 0 0 0 > 999999985 0 0 50000 > cpub_clk_dyn1 0 0 0 999999985 > 0 0 50000 > cpub_clk_dyn 0 0 0 > 999999985 0 0 50000 > cpub_clk 0 0 0 > 999999985 0 0 50000 > cpub_clk_div8 0 0 0 > 124999998 0 0 50000 > cpub_clk_div7 0 0 0 > 142857140 0 0 50000 > cpub_clk_div6 0 0 0 > 166666664 0 0 50000 > cpub_clk_trace_sel 0 0 > 0 166666664 0 0 50000 > cpub_clk_trace 0 0 > 0 166666664 0 0 50000 > cpub_clk_div5 0 0 0 > 199999997 0 0 50000 > cpub_clk_apb_sel 0 0 > 0 199999997 0 0 50000 > cpub_clk_apb 0 0 > 0 199999997 0 0 50000 > cpub_clk_div4 0 0 0 > 249999996 0 0 50000 > cpub_clk_div3 0 0 0 > 333333328 0 0 50000 > cpub_clk_atb_sel 0 0 > 0 333333328 0 0 50000 > cpub_clk_atb 0 0 > 0 333333328 0 0 50000 > cpub_clk_div2 0 0 0 > 499999992 0 0 50000 > cpub_clk_axi_sel 0 0 > 0 499999992 0 0 50000 > cpub_clk_axi 0 0 > 0 499999992 0 0 50000 > cpub_clk_div16_en 0 0 > 0 999999985 0 0 50000 > cpub_clk_div16 0 0 > 0 62499999 0 0 50000 I can't read that. > > After enable *cpub_clk* flags with > .flags = CLK_SET_RATE_PARENT | CLK_IS_CRITICAL, > this clk is enabled on microSD card see clk_summary below. Again, I don't get the relationship between cpub and sdcard (or eMMC) > [...] > fclk_div2_div 1 1 0 999999985 > 0 0 50000 > fclk_div2 3 3 0 999999985 > 0 0 50000 > ff3f0000.ethernet#m250_sel 1 1 0 > 999999985 0 0 50000 > ff3f0000.ethernet#m250_div 1 1 > 0 249999997 0 0 50000 > ff3f0000.ethernet#fixed_div2 1 1 > 0 124999998 0 0 50000 > ff3f0000.ethernet#rgmii_tx_en 1 > 1 0 124999998 0 0 50000 > ffe05000.sd#mux 1 1 0 999999985 > 0 0 50000 > ffe05000.sd#div 1 1 0 > 50000000 0 0 50000 > cpub_clk_dyn1_sel 1 1 0 > 999999985 0 0 50000 > cpub_clk_dyn1 1 1 0 999999985 > 0 0 50000 > cpub_clk_dyn 1 1 0 > 999999985 0 0 50000 > cpub_clk 1 1 0 > 999999985 0 0 50000 > cpub_clk_div8 0 0 0 > 124999998 0 0 50000 > cpub_clk_div7 0 0 0 > 142857140 0 0 50000 > cpub_clk_div6 0 0 0 > 166666664 0 0 50000 > cpub_clk_trace_sel 0 0 > 0 166666664 0 0 50000 > cpub_clk_trace 0 0 > 0 166666664 0 0 50000 > cpub_clk_div5 0 0 0 > 199999997 0 0 50000 > cpub_clk_apb_sel 0 0 > 0 199999997 0 0 50000 > cpub_clk_apb 0 0 > 0 199999997 0 0 50000 > cpub_clk_div4 0 0 0 > 249999996 0 0 50000 > cpub_clk_div3 0 0 0 > 333333328 0 0 50000 > cpub_clk_atb_sel 0 0 > 0 333333328 0 0 50000 > cpub_clk_atb 0 0 > 0 333333328 0 0 50000 > cpub_clk_div2 0 0 0 > 499999992 0 0 50000 > cpub_clk_axi_sel 0 0 > 0 499999992 0 0 50000 > cpub_clk_axi 0 0 > 0 499999992 0 0 50000 > cpub_clk_div16_en 0 0 > 0 999999985 0 0 50000 > cpub_clk_div16 0 0 > 0 62499999 0 0 50000 > cpub_clk_dyn1_div 0 0 0 > 999999985 0 0 50000 > > Is this correct approach to set the flags to enable *cpub_clk*. > .flags = CLK_SET_RATE_PARENT | CLK_IS_CRITICAL, > > What I meant is their *Dyn_enable[26]* field for enable/disable for > HHI_SYS_CPU_CLK_CNTL0 and HHI_SYS_CPUB_CLK_CNTL clk controller. > in the S922X datasheets which could help resolve this issue. > Any thought on this. I sorry but I'm just lost. I don't understand anything above so I can't comment. > > -Anand
Hi Jerome, On Mon, 24 Feb 2020 at 15:01, Jerome Brunet <jbrunet@baylibre.com> wrote: > > > On Sun 23 Feb 2020 at 14:34, Anand Moon <linux.amoon@gmail.com> wrote: > > > Hi Martin / Jerome / Neil, > > > > On Fri, 21 Feb 2020 at 02:45, Martin Blumenstingl > > <martin.blumenstingl@googlemail.com> wrote: > >> > >> Hi Anand, > >> > >> On Mon, Feb 17, 2020 at 2:30 PM Anand Moon <linux.amoon@gmail.com> wrote: > >> [...] > >> > > > @@ -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. > >> > > > >> > I would like core developers to handle this. > >> > Sorry for the noise. > >> can you please resend this patch with only the change to g12b_cpub_clk? > >> I have no G12B board myself so it would be great if you could take care of this! > >> > >> > >> Martin > > > > Thanks, yes I will try again, but I have a question. > > > > On eMMC module *cpub_clk* is not getting enabled, see below is > > clk_summay of eMMC. > > I'm sorry but I don't understand the link between the cpu clock of the > second cluster and MMC. > > > [...] > > fclk_div2_div 1 1 0 999999985 > > 0 0 50000 > > fclk_div2 2 2 0 999999985 > > 0 0 50000 > > ff3f0000.ethernet#m250_sel 1 1 0 > > 999999985 0 0 50000 > > ff3f0000.ethernet#m250_div 1 1 > > 0 249999997 0 0 50000 > > ff3f0000.ethernet#fixed_div2 1 1 > > 0 124999998 0 0 50000 > > ff3f0000.ethernet#rgmii_tx_en 1 > > 1 0 124999998 0 0 50000 > > ffe07000.mmc#mux 1 1 0 999999985 > > 0 0 50000 > > ffe07000.mmc#div 1 1 0 > > 199999997 0 0 50000 > > cpub_clk_dyn1_sel 0 0 0 > > 999999985 0 0 50000 > > cpub_clk_dyn1 0 0 0 999999985 > > 0 0 50000 > > cpub_clk_dyn 0 0 0 > > 999999985 0 0 50000 > > cpub_clk 0 0 0 > > 999999985 0 0 50000 > > cpub_clk_div8 0 0 0 > > 124999998 0 0 50000 > > cpub_clk_div7 0 0 0 > > 142857140 0 0 50000 > > cpub_clk_div6 0 0 0 > > 166666664 0 0 50000 > > cpub_clk_trace_sel 0 0 > > 0 166666664 0 0 50000 > > cpub_clk_trace 0 0 > > 0 166666664 0 0 50000 > > cpub_clk_div5 0 0 0 > > 199999997 0 0 50000 > > cpub_clk_apb_sel 0 0 > > 0 199999997 0 0 50000 > > cpub_clk_apb 0 0 > > 0 199999997 0 0 50000 > > cpub_clk_div4 0 0 0 > > 249999996 0 0 50000 > > cpub_clk_div3 0 0 0 > > 333333328 0 0 50000 > > cpub_clk_atb_sel 0 0 > > 0 333333328 0 0 50000 > > cpub_clk_atb 0 0 > > 0 333333328 0 0 50000 > > cpub_clk_div2 0 0 0 > > 499999992 0 0 50000 > > cpub_clk_axi_sel 0 0 > > 0 499999992 0 0 50000 > > cpub_clk_axi 0 0 > > 0 499999992 0 0 50000 > > cpub_clk_div16_en 0 0 > > 0 999999985 0 0 50000 > > cpub_clk_div16 0 0 > > 0 62499999 0 0 50000 > > I can't read that. > > > > > After enable *cpub_clk* flags with > > .flags = CLK_SET_RATE_PARENT | CLK_IS_CRITICAL, > > this clk is enabled on microSD card see clk_summary below. > > Again, I don't get the relationship between cpub and sdcard (or eMMC) > Yes their is not relation with the cpub and sdcard and eMMC, I understood that cpub_clk is not getting enable which is causing the staling at booting using sdcard. sorry about this logs. > > [...] > > fclk_div2_div 1 1 0 999999985 > > 0 0 50000 > > fclk_div2 3 3 0 999999985 > > 0 0 50000 > > ff3f0000.ethernet#m250_sel 1 1 0 > > 999999985 0 0 50000 > > ff3f0000.ethernet#m250_div 1 1 > > 0 249999997 0 0 50000 > > ff3f0000.ethernet#fixed_div2 1 1 > > 0 124999998 0 0 50000 > > ff3f0000.ethernet#rgmii_tx_en 1 > > 1 0 124999998 0 0 50000 > > ffe05000.sd#mux 1 1 0 999999985 > > 0 0 50000 > > ffe05000.sd#div 1 1 0 > > 50000000 0 0 50000 > > cpub_clk_dyn1_sel 1 1 0 > > 999999985 0 0 50000 > > cpub_clk_dyn1 1 1 0 999999985 > > 0 0 50000 > > cpub_clk_dyn 1 1 0 > > 999999985 0 0 50000 > > cpub_clk 1 1 0 > > 999999985 0 0 50000 > > cpub_clk_div8 0 0 0 > > 124999998 0 0 50000 > > cpub_clk_div7 0 0 0 > > 142857140 0 0 50000 > > cpub_clk_div6 0 0 0 > > 166666664 0 0 50000 > > cpub_clk_trace_sel 0 0 > > 0 166666664 0 0 50000 > > cpub_clk_trace 0 0 > > 0 166666664 0 0 50000 > > cpub_clk_div5 0 0 0 > > 199999997 0 0 50000 > > cpub_clk_apb_sel 0 0 > > 0 199999997 0 0 50000 > > cpub_clk_apb 0 0 > > 0 199999997 0 0 50000 > > cpub_clk_div4 0 0 0 > > 249999996 0 0 50000 > > cpub_clk_div3 0 0 0 > > 333333328 0 0 50000 > > cpub_clk_atb_sel 0 0 > > 0 333333328 0 0 50000 > > cpub_clk_atb 0 0 > > 0 333333328 0 0 50000 > > cpub_clk_div2 0 0 0 > > 499999992 0 0 50000 > > cpub_clk_axi_sel 0 0 > > 0 499999992 0 0 50000 > > cpub_clk_axi 0 0 > > 0 499999992 0 0 50000 > > cpub_clk_div16_en 0 0 > > 0 999999985 0 0 50000 > > cpub_clk_div16 0 0 > > 0 62499999 0 0 50000 > > cpub_clk_dyn1_div 0 0 0 > > 999999985 0 0 50000 > > > > Is this correct approach to set the flags to enable *cpub_clk*. > > .flags = CLK_SET_RATE_PARENT | CLK_IS_CRITICAL, > > > > What I meant is their *Dyn_enable[26]* field for enable/disable for > > HHI_SYS_CPU_CLK_CNTL0 and HHI_SYS_CPUB_CLK_CNTL clk controller. > > in the S922X datasheets which could help resolve this issue. > > Any thought on this. > > I sorry but I'm just lost. I don't understand anything above so I can't > comment. I am not able to express my self clearly, I will try to submit the patch by enable cpub_clk with following flags. .flags = CLK_SET_RATE_PARENT | CLK_IS_CRITICAL, -Anand
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, }, }; @@ -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, }, };
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 the CPU for DVFS is getting disabled in between the late_init call, so gaiting the clock source shuts down the power to the codes. Setting clk divider flags to CLK_IS_CRITICAL help resolve the issue. Cc: Martin Blumenstingl <martin.blumenstingl@googlemail.com> Cc: Jerome Brunet <jbrunet@baylibre.com> Cc: Neil Armstrong <narmstrong@baylibre.com> Suggested-by: Neil Armstrong <narmstrong@baylibre.com> Signed-off-by: Anand Moon <linux.amoon@gmail.com> --- 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(-)