Hi Morimoto-san,
On Tue, Dec 15, 2020 at 1:06 AM Kuninori Morimoto
<kuninori.morimoto.gx@xxxxxxxxxxx> wrote:
> We need to care clock accessibility,
> because we might can't use clock for some reasons.
>
> It sets clk_rate for each clocks when enabled.
> This means it doesn't have clk_rate if we can't use.
> We can avoid to call clk_disable_unprepare() in such case.
>
> Reported-by: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>
Feel free to use geert+renesas@xxxxxxxxx instead ;-)
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@xxxxxxxxxxx>
> ---
>
> Hi Geert.
>
> Thank you for your reporting.
> I have never seen this kind of error, but it possible to happen.
> Unfortunately, I can't reproduce this but I hope this patch can solve it.
> Could you please check this ?
> I added [RFC] on this patch Subject.
The patch looks good to me, but I also cannot trigger the issue at will.
I went through my old boot logs, and found 2 other occurrences, also
on Ebisu. In all cases, it happened while a lot of output was printed to
the serial console (either a WARN() splat, or DEBUG_PINCTRL output).
My guess is that console output or disabling interrupts too long is
triggering a race condition or other issue in the i2c driver (clk 1 is the
cs2000 clock generator, controlled through i2c).
> --- a/sound/soc/sh/rcar/adg.c
> +++ b/sound/soc/sh/rcar/adg.c
> @@ -366,25 +366,25 @@ void rsnd_adg_clk_control(struct rsnd_priv *priv, int enable)
> struct rsnd_adg *adg = rsnd_priv_to_adg(priv);
> struct device *dev = rsnd_priv_to_dev(priv);
> struct clk *clk;
> - int i, ret;
> + int i;
>
> for_each_rsnd_clk(clk, adg, i) {
> - ret = 0;
> if (enable) {
> - ret = clk_prepare_enable(clk);
> + int ret = clk_prepare_enable(clk);
>
> /*
> * We shouldn't use clk_get_rate() under
> * atomic context. Let's keep it when
> * rsnd_adg_clk_enable() was called
> */
> - adg->clk_rate[i] = clk_get_rate(adg->clk[i]);
> + if (ret < 0)
> + dev_warn(dev, "can't use clk %d\n", i);
> + else
> + adg->clk_rate[i] = clk_get_rate(adg->clk[i]);
> } else {
> - clk_disable_unprepare(clk);
> + if (adg->clk_rate[i])
> + clk_disable_unprepare(clk);
As pointed out by Mark, you may want to clear adg->clk_rate[i] here?
> }
> -
> - if (ret < 0)
> - dev_warn(dev, "can't use clk %d\n", i);
> }
> }
With the above sorted out:
Reviewed-by: Geert Uytterhoeven <geert+renesas@xxxxxxxxx>
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
