sh-sci regression without scif_clk in 4.6-rc5

Commit Message

Jürg Billeter April 25, 2016, 12:24 p.m. UTC

Hi Geert,

testing 4.6-rc5 on a custom R-Car H3 board, I noticed that the Linux
console attached to SCIF2 no longer works (it works on 4.5). earlycon
works fine, though. Unlike Salvator-X, this H3 board does not use an
external SCIF clock and thus, scif_clk is not enabled in the board
device tree.

The first "bad" commit is
commit 3e5dd6f6e690048d0fd1c913397506648724474e
Author: Geert Uytterhoeven <[e-mail]>
Date:   Fri Feb 26 16:54:31 2016 +0100

    clk: Ignore disabled DT clock providers

As far as I can tell, the issue is that, with the above commit,
devm_clk_get() returns -EPROBE_DEFER for disabled clocks and the sh-sci 
driver aborts probing with -EPROBE_DEFER even though scif_clk is
optional.

I can work around this issue with the patch below but I'm not sure
about the correct fix. Intuitively, I would expect devm_clk_get() to
return -ENOENT instead of -EPROBE_DEFER for disabled clocks but I don't
know whether this can be changed in the common clock framework without
risking issues in other places.

What do you think is the best solution to this issue?

Regards,
Jürg

Comments

Geert Uytterhoeven April 25, 2016, 12:44 p.m. UTC | #1

Hi Jürg,

On Mon, Apr 25, 2016 at 2:24 PM, Jürg Billeter <[e-mail]> wrote:
> testing 4.6-rc5 on a custom R-Car H3 board, I noticed that the Linux
> console attached to SCIF2 no longer works (it works on 4.5). earlycon
> works fine, though. Unlike Salvator-X, this H3 board does not use an
> external SCIF clock and thus, scif_clk is not enabled in the board
> device tree.
>
> The first "bad" commit is
> commit 3e5dd6f6e690048d0fd1c913397506648724474e
> Author: Geert Uytterhoeven <[e-mail]>
> Date:   Fri Feb 26 16:54:31 2016 +0100
>
>     clk: Ignore disabled DT clock providers
>
> As far as I can tell, the issue is that, with the above commit,
> devm_clk_get() returns -EPROBE_DEFER for disabled clocks and the sh-sci
> driver aborts probing with -EPROBE_DEFER even though scif_clk is
> optional.
>
> I can work around this issue with the patch below but I'm not sure
> about the correct fix. Intuitively, I would expect devm_clk_get() to
> return -ENOENT instead of -EPROBE_DEFER for disabled clocks but I don't
> know whether this can be changed in the common clock framework without
> risking issues in other places.
>
> What do you think is the best solution to this issue?

The real issue is that scif_clk in r8a7795.dtsi should not be marked
disabled.

Cfr. "ARM: dts: r8a7791: Don't disable referenced optional clocks"
https://git.kernel.org/cgit/linux/kernel/git/horms/renesas.git/commit/?h=fixes-for-v4.6&id=ac6908b3049397b10bcfd8143d79cbdbbd266f02

Will fix and send patches for all other affected SoCs...

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [e-mail]

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

Patch

diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index 8e966d9..0916159 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -2462,7 +2462,7 @@  static int sci_init_clocks(struct sci_port *sci_port, struct device *dev)
 
        for (i = 0; i < SCI_NUM_CLKS; i++) {
                clk = devm_clk_get(dev, clk_names[i]);
-               if (PTR_ERR(clk) == -EPROBE_DEFER)
+               if (PTR_ERR(clk) == -EPROBE_DEFER && i != SCI_SCIF_CLK)
                        return -EPROBE_DEFER;
 
                if (IS_ERR(clk) && i == SCI_FCK) {