Re: [PATCH] clocksource/drivers/renesas-ostm: Avoid reprobe after successful early probe

[Saravana Kannan]

On Wed, Mar 20, 2024 at 3:30 AM Geert Uytterhoeven
<geert+renesas@xxxxxxxxx> wrote:
>
> The Renesas OS Timer (OSTM) driver contains two probe points, of which
> only one should complete:
>   1. Early probe, using TIMER_OF_DECLARE(), to provide the sole
>      clocksource on (arm32) RZ/A1 and RZ/A2 SoCs,
>   2. Normal probe, using a platform driver, to provide additional timers
>      on (arm64 + riscv) RZ/G2L and similar SoCs.
>
> The latter is needed because using OSTM on RZ/G2L requires manipulation
> of its reset signal, which is not yet available at the time of early
> probe, causing early probe to fail with -EPROBE_DEFER.  It is only
> enabled when building a kernel with support for the RZ/G2L family, so it
> does not impact RZ/A1 and RZ/A2.  Hence only one probe method can
> complete on all affected systems.
>
> As relying on the order of initialization of subsystems inside the
> kernel is fragile, set the DT node's OF_POPULATED flag after a succesful
> early probe.  This makes sure the platform driver's probe is never
> called after a successful early probe.
>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@xxxxxxxxx>
> ---
> Tested on RZ/A2 (after force-enabling the platform driver probe).
> Regression-tested on RZ/Five (member of the RZ/G2L family).
>
> In between commit 4f41fe386a94639c ("clocksource/drivers/timer-probe:
> Avoid creating dead devices") and its revert 4479730e9263befb (both in
> v5.7), the clocksource core took care of this.  Other subsystems[1]
> still handle this, either minimally (by just setting OF_POPULATED), or
> fully (by also clearing OF_POPULATED again in case of probe failure).
>
> Note that despite the revert in the clocksource core, several
> clocksource drivers[2] still clear the OF_POPULATED flag manually, to
> force probing the same device using both TIMER_OF_DECLARE() and standard
> platform device probing (the latter may be done in a different driver).
>
> [1] See of_clk_init(), of_gpiochip_scan_gpios(), of_irq_init().
> [2] drivers/clocksource/ingenic-sysost.c
>     drivers/clocksource/ingenic-timer.c
>     drivers/clocksource/timer-versatile.c
> ---
>  drivers/clocksource/renesas-ostm.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/clocksource/renesas-ostm.c b/drivers/clocksource/renesas-ostm.c
> index 8da972dc171365bc..37db7e23a4d29135 100644
> --- a/drivers/clocksource/renesas-ostm.c
> +++ b/drivers/clocksource/renesas-ostm.c
> @@ -210,6 +210,7 @@ static int __init ostm_init(struct device_node *np)
>                 pr_info("%pOF: used for clock events\n", np);
>         }
>
> +       of_node_set_flag(np, OF_POPULATED);
>         return 0;

Couldn't you also solve this by using the more specific compatible
strings for the driver and TIMER_OF_DECLARE()?

-Saravana

>
>  err_cleanup:
> --
> 2.34.1
>