| Message ID | 20191203034519.5640-2-chris.brandt@renesas.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | spi: Add Renesas SPIBSC controller | expand |
Hi Chris, On Tue, Dec 3, 2019 at 4:46 AM Chris Brandt <chris.brandt@renesas.com> wrote: > Allow critical clocks to be specified in the Device Tree. > > Signed-off-by: Chris Brandt <chris.brandt@renesas.com> Thanks for your patch! Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> Notes: 1. Assumed we really need this, 2. Minor nit below. > --- a/drivers/clk/renesas/clk-mstp.c > +++ b/drivers/clk/renesas/clk-mstp.c > @@ -187,6 +187,7 @@ static void __init cpg_mstp_clocks_init(struct device_node *np) > const char *idxname; > struct clk **clks; > unsigned int i; > + unsigned long flags; = 0 here... > > group = kzalloc(struct_size(group, clks, MSTP_MAX_CLOCKS), GFP_KERNEL); > if (!group) > @@ -239,8 +240,11 @@ static void __init cpg_mstp_clocks_init(struct device_node *np) > continue; > } > > + flags = 0; ... instead of here? > + of_clk_detect_critical(np, i, &flags); > + > clks[clkidx] = cpg_mstp_clock_register(name, parent_name, > - clkidx, group); > + clkidx, group, flags); > if (!IS_ERR(clks[clkidx])) { > group->data.clk_num = max(group->data.clk_num, > clkidx + 1); Gr{oetje,eeting}s, Geert
Hi Geert, Thank you for your review! On Tue, Dec 3, 2019, Geert Uytterhoeven wrote: > > + unsigned long flags; > > = 0 here... > > > > + flags = 0; > > ... instead of here? That was my first thought too...and it was wrong. If of_clk_detect_critical does NOT detect a critical clock, it does not touch flags at all. And since it is a loop, what you get is after the first clock is marked as CRITICAL, all the remaining clocks also get marked CRITICAL. In this case, both spibsc0 and spibsc1 get marked critical. That's why I have to reset it for each loop. Chris
Hi Chris, On Tue, Dec 3, 2019 at 7:46 PM Chris Brandt <Chris.Brandt@renesas.com> wrote: > On Tue, Dec 3, 2019, Geert Uytterhoeven wrote: > > > + unsigned long flags; > > > > = 0 here... > > > > > > + flags = 0; > > > > ... instead of here? > > That was my first thought too...and it was wrong. > > If of_clk_detect_critical does NOT detect a critical clock, it does not > touch flags at all. > And since it is a loop, what you get is after the first clock is marked > as CRITICAL, all the remaining clocks also get marked CRITICAL. In this > case, both spibsc0 and spibsc1 get marked critical. That's why I have to > reset it for each loop. Thanks, I missed this is done inside a loop. Gr{oetje,eeting}s, Geert
diff --git a/drivers/clk/renesas/clk-mstp.c b/drivers/clk/renesas/clk-mstp.c index 003e9ce45757..8e28e9671265 100644 --- a/drivers/clk/renesas/clk-mstp.c +++ b/drivers/clk/renesas/clk-mstp.c @@ -148,7 +148,7 @@ static const struct clk_ops cpg_mstp_clock_ops = { static struct clk * __init cpg_mstp_clock_register(const char *name, const char *parent_name, unsigned int index, - struct mstp_clock_group *group) + struct mstp_clock_group *group, unsigned long flags) { struct clk_init_data init; struct mstp_clock *clock; @@ -160,12 +160,12 @@ static struct clk * __init cpg_mstp_clock_register(const char *name, init.name = name; init.ops = &cpg_mstp_clock_ops; - init.flags = CLK_SET_RATE_PARENT; + init.flags = CLK_SET_RATE_PARENT | flags; /* INTC-SYS is the module clock of the GIC, and must not be disabled */ - if (!strcmp(name, "intc-sys")) { - pr_debug("MSTP %s setting CLK_IS_CRITICAL\n", name); + if (!strcmp(name, "intc-sys")) init.flags |= CLK_IS_CRITICAL; - } + if (init.flags & CLK_IS_CRITICAL) + pr_debug("MSTP %s setting CLK_IS_CRITICAL\n", name); init.parent_names = &parent_name; init.num_parents = 1; @@ -187,6 +187,7 @@ static void __init cpg_mstp_clocks_init(struct device_node *np) const char *idxname; struct clk **clks; unsigned int i; + unsigned long flags; group = kzalloc(struct_size(group, clks, MSTP_MAX_CLOCKS), GFP_KERNEL); if (!group) @@ -239,8 +240,11 @@ static void __init cpg_mstp_clocks_init(struct device_node *np) continue; } + flags = 0; + of_clk_detect_critical(np, i, &flags); + clks[clkidx] = cpg_mstp_clock_register(name, parent_name, - clkidx, group); + clkidx, group, flags); if (!IS_ERR(clks[clkidx])) { group->data.clk_num = max(group->data.clk_num, clkidx + 1);
Allow critical clocks to be specified in the Device Tree. Signed-off-by: Chris Brandt <chris.brandt@renesas.com> --- drivers/clk/renesas/clk-mstp.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-)