Re: [RFC PATCH v2 19/30] drivers/tty: sh-sci fix SH4 OF support.

On Tue, 19 Sep 2023 21:25:06 +0900,
Geert Uytterhoeven wrote:
> 
> Hi Sato-san,
> 
> Thanks for your patch!
> 
> On Wed, Sep 13, 2023 at 11:26 AM Yoshinori Sato
> <[email protected]> wrote:
> > - fix earlycon name.
> 
> I guess you mean earlyprintk?
> 
> "Earlyprintk expects that all names used in OF_EARLYCON_DECLARE()
>  are unique".
> 
> > - fix earlyprintk hung (NULL pointer reference).
> > - clocks property support.
> >
> > Signed-off-by: Yoshinori Sato <[email protected]>
> 
> > --- a/drivers/tty/serial/sh-sci.c
> > +++ b/drivers/tty/serial/sh-sci.c
> 
> > @@ -2842,6 +2842,8 @@ static int sci_init_clocks(struct sci_port *sci_port, struct device *dev)
> >                          * global "peripheral_clk" clock.
> >                          */
> >                         clk = devm_clk_get(dev, "peripheral_clk");
> > +                       if (IS_ERR(clk))
> > +                               clk = devm_clk_get(dev, NULL);
> 
> This should not be needed.
> I guess this is a workaround for the lack of
> 
>         clock-names = "fck";
> 
> in arch/sh/boot/dts/sh7751.dtsi?
> 
> "make dtbs_check
> DT_SCHEMA_FILES=Documentation/devicetree/bindings/serial/renesas,scif.yaml"
> would have told you ;-)
> 
>     serial@ffe80000: 'clock-names' is a required property
> 
> >                         if (IS_ERR(clk))
> >                                 return dev_err_probe(dev, PTR_ERR(clk),
> >                                                      "failed to get %s\n",
> > @@ -3555,8 +3557,8 @@ static int __init hscif_early_console_setup(struct earlycon_device *device,
> >
> >  OF_EARLYCON_DECLARE(sci, "renesas,sci", sci_early_console_setup);
> >  OF_EARLYCON_DECLARE(scif, "renesas,scif", scif_early_console_setup);
> > -OF_EARLYCON_DECLARE(scif, "renesas,scif-r7s9210", rzscifa_early_console_setup);
> > -OF_EARLYCON_DECLARE(scif, "renesas,scif-r9a07g044", rzscifa_early_console_setup);
> > +OF_EARLYCON_DECLARE(rzscif, "renesas,scif-r7s9210", rzscifa_early_console_setup);
> > +OF_EARLYCON_DECLARE(rzscif, "renesas,scif-r9a07g044", rzscifa_early_console_setup);
> 
> Perhaps "rzscifa", to match the setup function prefix?

With this change, specifying "earlycon=rzscif,0xe8007000" in
the boot parameter will call rzscifa_early_console_setup.
But it would be confusing if the names were not unified, so use "rzscifa"
I'll make it.

> >  OF_EARLYCON_DECLARE(scifa, "renesas,scifa", scifa_early_console_setup);
> >  OF_EARLYCON_DECLARE(scifb, "renesas,scifb", scifb_early_console_setup);
> >  OF_EARLYCON_DECLARE(hscif, "renesas,hscif", hscif_early_console_setup);
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> -- 
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
> 
> 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

-- 
Yosinori Sato