[PATCH 2/2] clocksource/drivers/renesas-ostm: Add RZ/G2L OSTM support

[Geert Uytterhoeven]

Hi Biju,

On Wed, Nov 10, 2021 at 9:32 AM Biju Das <biju.das.jz at bp.renesas.com> wrote:
> RZ/G2L SoC has Generic Timer Module(a.k.a OSTM) which needs to
> deassert the reset line before accessing any registers.
>
> This patch adds an entry point for RZ/G2L so that we can deassert
> the reset line in probe callback.
>
> Signed-off-by: Biju Das <biju.das.jz at bp.renesas.com>
> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj at bp.renesas.com>

Thanks for your patch!

> --- a/drivers/clocksource/renesas-ostm.c
> +++ b/drivers/clocksource/renesas-ostm.c
> @@ -209,3 +211,39 @@ static int __init ostm_init(struct device_node *np)
>  }
>
>  TIMER_OF_DECLARE(ostm, "renesas,ostm", ostm_init);

Background: this driver uses TIMER_OF_DECLARE() because the OSTM
is the system timer on RZ/A SoCs, which do not have the ARM architectured
timer.  RZ/G2L does have the ARM architectured timer.

> +
> +#ifdef CONFIG_ARCH_R9A07G044
> +static int __init ostm_probe(struct platform_device *pdev)
> +{
> +       struct device *dev = &pdev->dev;
> +       struct reset_control *rstc;
> +       int ret;
> +
> +       rstc = devm_reset_control_get_exclusive(dev, NULL);
> +       if (IS_ERR(rstc))
> +               return dev_err_probe(dev, PTR_ERR(rstc), "failed to get reset");
> +
> +       reset_control_deassert(rstc);
> +
> +       ret = ostm_init(dev->of_node);
> +       if (ret) {
> +               reset_control_assert(rstc);
> +               return ret;
> +       }
> +
> +       return 0;
> +}
> +
> +static const struct of_device_id ostm_of_table[] = {
> +       { .compatible = "renesas,rzg2l-ostm", },

I believe the OSTM block on RZ/G2L is identical to the one on RZ/A,
and the requirement to deassert its module reset is an SoC integration
feature on RZ/G2L.  Hence the driver should match on "renesas,ostm"
for both?

So my suggestion would be to include the reset handling in ostm_init()
instead, but make it optional, and error out in case of -EPROBE_DEFER.
In case initialization from TIMER_OF_DECLARE() failed, the platform
driver can kick in later and retry.

However, it seems __of_reset_control_get() ignores all errors,
including -EPROBE_DEFER, if optional is true, so this won't work?
Philipp: is that correct? If yes, ostm_init() has to check the presence
of a resets property to see if the reset is optional or required.

> +       { }
> +};
> +
> +static struct platform_driver ostm_device_driver = {
> +       .driver         = {
> +               .name   = "rzg2l_ostm",
> +               .of_match_table = of_match_ptr(ostm_of_table),
> +       },
> +};
> +builtin_platform_driver_probe(ostm_device_driver, ostm_probe);
> +#endif

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org

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