[PATCHv1,3/3] clk: meson: g12a: set cpu clock divider flags too CLK_IS_CRITICAL
diff mbox series

Message ID 20200216173446.1823-4-linux.amoon@gmail.com
State Superseded
Delegated to: Neil Armstrong
Headers show
Series
  • Odroid N2 failes to boot using upstream kernel using microSD card
Related show

Commit Message

Anand Moon Feb. 16, 2020, 5:34 p.m. UTC
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(-)

Comments

Jerome Brunet Feb. 17, 2020, 8:02 a.m. UTC | #1
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.


>  	},
>  };
Anand Moon Feb. 17, 2020, 8:51 a.m. UTC | #2
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
Anand Moon Feb. 17, 2020, 1:30 p.m. UTC | #3
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
>
> >       },
> >  };
>
Martin Blumenstingl Feb. 20, 2020, 9:15 p.m. UTC | #4
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
Anand Moon Feb. 23, 2020, 1:34 p.m. UTC | #5
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
Jerome Brunet Feb. 24, 2020, 9:31 a.m. UTC | #6
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
Anand Moon Feb. 24, 2020, 10:17 a.m. UTC | #7
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

Patch
diff mbox series

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,
 	},
 };