* [PATCH] clocksource/drivers/renesas-ostm: Avoid reprobe after successful early probe
@ 2024-03-20 10:30 Geert Uytterhoeven
2024-03-20 17:54 ` Lad, Prabhakar
2024-03-20 20:17 ` Saravana Kannan
0 siblings, 2 replies; 4+ messages in thread
From: Geert Uytterhoeven @ 2024-03-20 10:30 UTC (permalink / raw
To: Daniel Lezcano, Thomas Gleixner, Saravana Kannan, Biju Das,
Lad Prabhakar
Cc: 周琰杰, Paul Cercueil, Liviu Dudau,
Sudeep Holla, Lorenzo Pieralisi, linux-renesas-soc, linux-kernel,
Geert Uytterhoeven
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/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;
err_cleanup:
--
2.34.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] clocksource/drivers/renesas-ostm: Avoid reprobe after successful early probe
2024-03-20 10:30 [PATCH] clocksource/drivers/renesas-ostm: Avoid reprobe after successful early probe Geert Uytterhoeven
@ 2024-03-20 17:54 ` Lad, Prabhakar
2024-03-20 20:17 ` Saravana Kannan
1 sibling, 0 replies; 4+ messages in thread
From: Lad, Prabhakar @ 2024-03-20 17:54 UTC (permalink / raw
To: Geert Uytterhoeven
Cc: Daniel Lezcano, Thomas Gleixner, Saravana Kannan, Biju Das,
Lad Prabhakar, 周琰杰, Paul Cercueil,
Liviu Dudau, Sudeep Holla, Lorenzo Pieralisi, linux-renesas-soc,
linux-kernel
On Wed, Mar 20, 2024 at 10:40 AM 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(+)
>
Reviwed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Cheers,
Prabhakar
> 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;
>
> err_cleanup:
> --
> 2.34.1
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] clocksource/drivers/renesas-ostm: Avoid reprobe after successful early probe
2024-03-20 10:30 [PATCH] clocksource/drivers/renesas-ostm: Avoid reprobe after successful early probe Geert Uytterhoeven
2024-03-20 17:54 ` Lad, Prabhakar
@ 2024-03-20 20:17 ` Saravana Kannan
2024-03-21 8:40 ` Geert Uytterhoeven
1 sibling, 1 reply; 4+ messages in thread
From: Saravana Kannan @ 2024-03-20 20:17 UTC (permalink / raw
To: Geert Uytterhoeven
Cc: Daniel Lezcano, Thomas Gleixner, Biju Das, Lad Prabhakar,
周琰杰, Paul Cercueil, Liviu Dudau,
Sudeep Holla, Lorenzo Pieralisi, linux-renesas-soc, linux-kernel
On Wed, Mar 20, 2024 at 3:30 AM 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/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
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] clocksource/drivers/renesas-ostm: Avoid reprobe after successful early probe
2024-03-20 20:17 ` Saravana Kannan
@ 2024-03-21 8:40 ` Geert Uytterhoeven
0 siblings, 0 replies; 4+ messages in thread
From: Geert Uytterhoeven @ 2024-03-21 8:40 UTC (permalink / raw
To: Saravana Kannan
Cc: Daniel Lezcano, Thomas Gleixner, Biju Das, Lad Prabhakar,
周琰杰, Paul Cercueil, Liviu Dudau,
Sudeep Holla, Lorenzo Pieralisi, linux-renesas-soc, linux-kernel
Hi Saravana,
On Wed, Mar 20, 2024 at 9:18 PM Saravana Kannan <saravanak@google.com> wrote:
> On Wed, Mar 20, 2024 at 3:30 AM 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/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()?
That's another option, but would considerably grow the number of
compatible values to match against.
Note that the actual OSTM module is (assumed to be) identical. The
differences lie in the integration into the SoC (wiring of the
module's reset). Hence using the compatible value to differentiate
looks wrong to me. It would be nice if this could be handled
automatically, which is what the reverted commit 4f41fe386a94639c
("clocksource/drivers/timer-probe: Avoid creating dead devices") did...
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@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
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2024-03-21 8:40 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-03-20 10:30 [PATCH] clocksource/drivers/renesas-ostm: Avoid reprobe after successful early probe Geert Uytterhoeven
2024-03-20 17:54 ` Lad, Prabhakar
2024-03-20 20:17 ` Saravana Kannan
2024-03-21 8:40 ` Geert Uytterhoeven
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.