Hi Biju,
On Wed, Nov 10, 2021 at 9:32 AM Biju Das <biju.das.jz@xxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxx>
> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx>
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@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
