Hi Niklas,
On Thu, Nov 29, 2018 at 1:16 AM Niklas Söderlund
<niklas.soderlund+renesas@xxxxxxxxxxxx> wrote:
> The driver tries to figure out which state a SD clock is in when the
> clock is register instead of setting a known state. This can be
> problematic for two reasons.
>
> 1. If the clock driver can't figure out the state of the clock
> registration of the clock fails and setting of a known state by a
> clock user is not possible.
>
> 2. The state of the clock depends on if and how the bootloader
> configured it. The driver only checks that the rate is known not if
> the clock is stopped or not for example.
>
> Fix this by setting a known state and make sure the clock is stopped.
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@xxxxxxxxxxxx>
Thanks for your patch!
Reviewed-by: Geert Uytterhoeven <geert+renesas@xxxxxxxxx>
One minor nit below.
> --- a/drivers/clk/renesas/rcar-gen3-cpg.c
> +++ b/drivers/clk/renesas/rcar-gen3-cpg.c
> @@ -360,7 +360,7 @@ static struct clk * __init cpg_sd_clk_register(const struct cpg_core_clk *core,
> struct sd_clock *clock;
> struct clk *clk;
> unsigned int i;
> - u32 sd_fc;
> + u32 val;
>
> clock = kzalloc(sizeof(*clock), GFP_KERNEL);
> if (!clock)
> @@ -377,17 +377,11 @@ static struct clk * __init cpg_sd_clk_register(const struct cpg_core_clk *core,
> clock->div_table = cpg_sd_div_table;
> clock->div_num = ARRAY_SIZE(cpg_sd_div_table);
>
> - sd_fc = readl(clock->csn.reg) & CPG_SD_FC_MASK;
> - for (i = 0; i < clock->div_num; i++)
> - if (sd_fc == (clock->div_table[i].val & CPG_SD_FC_MASK))
> - break;
> -
> - if (WARN_ON(i >= clock->div_num)) {
> - kfree(clock);
> - return ERR_PTR(-EINVAL);
> - }
> + val = readl(clock->csn.reg) & ~CPG_SD_FC_MASK;
> + val |= CPG_SD_STP_MASK | (clock->div_table[0].val & CPG_SD_FC_MASK);
> + writel(val, clock->csn.reg);
>
> - clock->cur_div_idx = i;
> + clock->cur_div_idx = 0;
I think this line can just be dropped, as the object was allocated using
kzalloc().
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
