Received-SPF: pass (google.com: domain of linux-kernel+bounces-109389-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Precedence: bulk
MIME-Version: 1.0
References: <bd027379713cbaafa21ffe9e848ebb7f475ca0e7.1710930542.git.geert+renesas@glider.be>
In-Reply-To: <bd027379713cbaafa21ffe9e848ebb7f475ca0e7.1710930542.git.geert+renesas@glider.be>
From: Saravana Kannan <saravanak@google.com>
Date: Wed, 20 Mar 2024 13:17:58 -0700
Message-ID: <CAGETcx_KNvY4NyQ+HSfkgVJS625R-LVNh_tsoZMM0or78ryEWg@mail.gmail.com>
Subject: Re: [PATCH] clocksource/drivers/renesas-ostm: Avoid reprobe after
 successful early probe
To: Geert Uytterhoeven <geert+renesas@glider.be>
Cc: Daniel Lezcano <daniel.lezcano@linaro.org>, Thomas Gleixner <tglx@linutronix.de>, 
	Biju Das <biju.das.jz@bp.renesas.com>, 
	Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>, 
	=?UTF-8?B?5ZGo55Cw5p2w?= <zhouyanjie@wanyeetech.com>, 
	Paul Cercueil <paul@crapouillou.net>, Liviu Dudau <liviu.dudau@arm.com>, 
	Sudeep Holla <sudeep.holla@arm.com>, Lorenzo Pieralisi <lpieralisi@kernel.org>, 
	linux-renesas-soc@vger.kernel.org, linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Wed, Mar 20, 2024 at 3:30=E2=80=AFAM Geert Uytterhoeven
<geert+renesas@glider.be> 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@glider.be>
> ---
> 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/ren=
esas-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
>