* [PATCH] clocksource: arm_arch_timer: Don't wait for probe of unexisting timers
@ 2014-10-23 17:32 Laurent Pinchart
2014-10-23 17:39 ` Stephen Boyd
0 siblings, 1 reply; 5+ messages in thread
From: Laurent Pinchart @ 2014-10-23 17:32 UTC (permalink / raw)
To: linux-arm-kernel
Commit c387f07e6205 ("clocksource: arm_arch_timer: Discard unavailable
timers correctly") attempted to avoid waiting for probe of disabled
timers, but got its condition check wrong and resulted in the driver
waiting for probe of timers not present in DT at all. Fix it.
Fixes: c387f07e6205 ("clocksource: arm_arch_timer: Discard unavailable timers correctly")
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
drivers/clocksource/arm_arch_timer.c | 15 ++++++++-------
1 file changed, 8 insertions(+), 7 deletions(-)
This patch fixes a regression introduced in v3.18-rc1.
diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index 2133f9d..5caf32c 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -657,17 +657,17 @@ static const struct of_device_id arch_timer_mem_of_match[] __initconst = {
};
static bool __init
-arch_timer_probed(int type, const struct of_device_id *matches)
+arch_timer_need_probe(int type, const struct of_device_id *matches)
{
struct device_node *dn;
- bool probed = false;
+ bool need_probe = false;
dn = of_find_matching_node(NULL, matches);
- if (dn && of_device_is_available(dn) && (arch_timers_present & type))
- probed = true;
+ if (dn && of_device_is_available(dn) && !(arch_timers_present & type))
+ need_probe = true;
of_node_put(dn);
- return probed;
+ return need_probe;
}
static void __init arch_timer_common_init(void)
@@ -676,9 +676,10 @@ static void __init arch_timer_common_init(void)
/* Wait until both nodes are probed if we have two timers */
if ((arch_timers_present & mask) != mask) {
- if (!arch_timer_probed(ARCH_MEM_TIMER, arch_timer_mem_of_match))
+ if (arch_timer_need_probe(ARCH_MEM_TIMER,
+ arch_timer_mem_of_match))
return;
- if (!arch_timer_probed(ARCH_CP15_TIMER, arch_timer_of_match))
+ if (arch_timer_need_probe(ARCH_CP15_TIMER, arch_timer_of_match))
return;
}
--
Regards,
Laurent Pinchart
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH] clocksource: arm_arch_timer: Don't wait for probe of unexisting timers
2014-10-23 17:32 [PATCH] clocksource: arm_arch_timer: Don't wait for probe of unexisting timers Laurent Pinchart
@ 2014-10-23 17:39 ` Stephen Boyd
2014-10-23 17:43 ` Laurent Pinchart
2014-10-23 17:49 ` Mark Rutland
0 siblings, 2 replies; 5+ messages in thread
From: Stephen Boyd @ 2014-10-23 17:39 UTC (permalink / raw)
To: linux-arm-kernel
On 10/23/2014 10:32 AM, Laurent Pinchart wrote:
> Commit c387f07e6205 ("clocksource: arm_arch_timer: Discard unavailable
> timers correctly") attempted to avoid waiting for probe of disabled
> timers, but got its condition check wrong and resulted in the driver
> waiting for probe of timers not present in DT at all. Fix it.
>
> Fixes: c387f07e6205 ("clocksource: arm_arch_timer: Discard unavailable timers correctly")
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
You're the third one. I like how the function has been renamed in this
patch. arch_timer_probed() is not accurately describing the intention of
the code so arch_timer_need_probe() reads better. Maybe your rename of
the function can be squashed into Marc's patch.
> drivers/clocksource/arm_arch_timer.c | 15 ++++++++-------
> 1 file changed, 8 insertions(+), 7 deletions(-)
>
> This patch fixes a regression introduced in v3.18-rc1.
>
> diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
> index 2133f9d..5caf32c 100644
> --- a/drivers/clocksource/arm_arch_timer.c
> +++ b/drivers/clocksource/arm_arch_timer.c
> @@ -657,17 +657,17 @@ static const struct of_device_id arch_timer_mem_of_match[] __initconst = {
> };
>
> static bool __init
> -arch_timer_probed(int type, const struct of_device_id *matches)
> +arch_timer_need_probe(int type, const struct of_device_id *matches)
> {
> struct device_node *dn;
> - bool probed = false;
> + bool need_probe = false;
>
> dn = of_find_matching_node(NULL, matches);
> - if (dn && of_device_is_available(dn) && (arch_timers_present & type))
> - probed = true;
> + if (dn && of_device_is_available(dn) && !(arch_timers_present & type))
> + need_probe = true;
> of_node_put(dn);
>
> - return probed;
> + return need_probe;
> }
>
> static void __init arch_timer_common_init(void)
> @@ -676,9 +676,10 @@ static void __init arch_timer_common_init(void)
>
> /* Wait until both nodes are probed if we have two timers */
> if ((arch_timers_present & mask) != mask) {
> - if (!arch_timer_probed(ARCH_MEM_TIMER, arch_timer_mem_of_match))
> + if (arch_timer_need_probe(ARCH_MEM_TIMER,
> + arch_timer_mem_of_match))
> return;
> - if (!arch_timer_probed(ARCH_CP15_TIMER, arch_timer_of_match))
> + if (arch_timer_need_probe(ARCH_CP15_TIMER, arch_timer_of_match))
> return;
> }
>
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH] clocksource: arm_arch_timer: Don't wait for probe of unexisting timers
2014-10-23 17:39 ` Stephen Boyd
@ 2014-10-23 17:43 ` Laurent Pinchart
2014-10-23 17:49 ` Mark Rutland
1 sibling, 0 replies; 5+ messages in thread
From: Laurent Pinchart @ 2014-10-23 17:43 UTC (permalink / raw)
To: linux-arm-kernel
Hi Stephen,
On Thursday 23 October 2014 10:39:18 Stephen Boyd wrote:
> On 10/23/2014 10:32 AM, Laurent Pinchart wrote:
> > Commit c387f07e6205 ("clocksource: arm_arch_timer: Discard unavailable
> > timers correctly") attempted to avoid waiting for probe of disabled
> > timers, but got its condition check wrong and resulted in the driver
> > waiting for probe of timers not present in DT at all. Fix it.
> >
> > Fixes: c387f07e6205 ("clocksource: arm_arch_timer: Discard unavailable
> > timers correctly") Signed-off-by: Laurent Pinchart
> > <laurent.pinchart+renesas@ideasonboard.com> ---
>
> You're the third one.
Sorry about that.
> I like how the function has been renamed in this patch. arch_timer_probed()
> is not accurately describing the intention of the code so
> arch_timer_need_probe() reads better. Maybe your rename of the function can
> be squashed into Marc's patch.
Sure, that's fine with me.
> > drivers/clocksource/arm_arch_timer.c | 15 ++++++++-------
> > 1 file changed, 8 insertions(+), 7 deletions(-)
> >
> > This patch fixes a regression introduced in v3.18-rc1.
> >
> > diff --git a/drivers/clocksource/arm_arch_timer.c
> > b/drivers/clocksource/arm_arch_timer.c index 2133f9d..5caf32c 100644
> > --- a/drivers/clocksource/arm_arch_timer.c
> > +++ b/drivers/clocksource/arm_arch_timer.c
> > @@ -657,17 +657,17 @@ static const struct of_device_id
> > arch_timer_mem_of_match[] __initconst = {>
> > };
> >
> > static bool __init
> > -arch_timer_probed(int type, const struct of_device_id *matches)
> > +arch_timer_need_probe(int type, const struct of_device_id *matches)
> > {
> > struct device_node *dn;
> > - bool probed = false;
> > + bool need_probe = false;
> >
> > dn = of_find_matching_node(NULL, matches);
> > - if (dn && of_device_is_available(dn) && (arch_timers_present & type))
> > - probed = true;
> > + if (dn && of_device_is_available(dn) && !(arch_timers_present &
> > type))
> > + need_probe = true;
> > of_node_put(dn);
> >
> > - return probed;
> > + return need_probe;
> > }
> >
> > static void __init arch_timer_common_init(void)
> > @@ -676,9 +676,10 @@ static void __init arch_timer_common_init(void)
> >
> > /* Wait until both nodes are probed if we have two timers */
> > if ((arch_timers_present & mask) != mask) {
> > - if (!arch_timer_probed(ARCH_MEM_TIMER, arch_timer_mem_of_match))
> > + if (arch_timer_need_probe(ARCH_MEM_TIMER,
> > + arch_timer_mem_of_match))
> > return;
> > - if (!arch_timer_probed(ARCH_CP15_TIMER, arch_timer_of_match))
> > + if (arch_timer_need_probe(ARCH_CP15_TIMER, arch_timer_of_match))
> > return;
> > }
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH] clocksource: arm_arch_timer: Don't wait for probe of unexisting timers
2014-10-23 17:39 ` Stephen Boyd
2014-10-23 17:43 ` Laurent Pinchart
@ 2014-10-23 17:49 ` Mark Rutland
2014-10-23 23:29 ` Laurent Pinchart
1 sibling, 1 reply; 5+ messages in thread
From: Mark Rutland @ 2014-10-23 17:49 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Oct 23, 2014 at 06:39:18PM +0100, Stephen Boyd wrote:
> On 10/23/2014 10:32 AM, Laurent Pinchart wrote:
> > Commit c387f07e6205 ("clocksource: arm_arch_timer: Discard unavailable
> > timers correctly") attempted to avoid waiting for probe of disabled
> > timers, but got its condition check wrong and resulted in the driver
> > waiting for probe of timers not present in DT at all. Fix it.
> >
> > Fixes: c387f07e6205 ("clocksource: arm_arch_timer: Discard unavailable timers correctly")
> > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> > ---
>
> You're the third one. I like how the function has been renamed in this
> patch. arch_timer_probed() is not accurately describing the intention of
> the code so arch_timer_need_probe() reads better. Maybe your rename of
> the function can be squashed into Marc's patch.
I'm happy with that as a cleanup, but right now I'd just like to see
Marc's patch hit mainline as-is. It's days old and tested, and it would
be nice to avoid another potential bug (not that I believe this patch is
in any way broken).
Thanks,
Mark.
>
> > drivers/clocksource/arm_arch_timer.c | 15 ++++++++-------
> > 1 file changed, 8 insertions(+), 7 deletions(-)
> >
> > This patch fixes a regression introduced in v3.18-rc1.
> >
> > diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
> > index 2133f9d..5caf32c 100644
> > --- a/drivers/clocksource/arm_arch_timer.c
> > +++ b/drivers/clocksource/arm_arch_timer.c
> > @@ -657,17 +657,17 @@ static const struct of_device_id arch_timer_mem_of_match[] __initconst = {
> > };
> >
> > static bool __init
> > -arch_timer_probed(int type, const struct of_device_id *matches)
> > +arch_timer_need_probe(int type, const struct of_device_id *matches)
> > {
> > struct device_node *dn;
> > - bool probed = false;
> > + bool need_probe = false;
> >
> > dn = of_find_matching_node(NULL, matches);
> > - if (dn && of_device_is_available(dn) && (arch_timers_present & type))
> > - probed = true;
> > + if (dn && of_device_is_available(dn) && !(arch_timers_present & type))
> > + need_probe = true;
> > of_node_put(dn);
> >
> > - return probed;
> > + return need_probe;
> > }
> >
> > static void __init arch_timer_common_init(void)
> > @@ -676,9 +676,10 @@ static void __init arch_timer_common_init(void)
> >
> > /* Wait until both nodes are probed if we have two timers */
> > if ((arch_timers_present & mask) != mask) {
> > - if (!arch_timer_probed(ARCH_MEM_TIMER, arch_timer_mem_of_match))
> > + if (arch_timer_need_probe(ARCH_MEM_TIMER,
> > + arch_timer_mem_of_match))
> > return;
> > - if (!arch_timer_probed(ARCH_CP15_TIMER, arch_timer_of_match))
> > + if (arch_timer_need_probe(ARCH_CP15_TIMER, arch_timer_of_match))
> > return;
> > }
> >
>
>
> --
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> a Linux Foundation Collaborative Project
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH] clocksource: arm_arch_timer: Don't wait for probe of unexisting timers
2014-10-23 17:49 ` Mark Rutland
@ 2014-10-23 23:29 ` Laurent Pinchart
0 siblings, 0 replies; 5+ messages in thread
From: Laurent Pinchart @ 2014-10-23 23:29 UTC (permalink / raw)
To: linux-arm-kernel
Hi Mark,
On Thursday 23 October 2014 18:49:11 Mark Rutland wrote:
> On Thu, Oct 23, 2014 at 06:39:18PM +0100, Stephen Boyd wrote:
> > On 10/23/2014 10:32 AM, Laurent Pinchart wrote:
> > > Commit c387f07e6205 ("clocksource: arm_arch_timer: Discard unavailable
> > > timers correctly") attempted to avoid waiting for probe of disabled
> > > timers, but got its condition check wrong and resulted in the driver
> > > waiting for probe of timers not present in DT at all. Fix it.
> > >
> > > Fixes: c387f07e6205 ("clocksource: arm_arch_timer: Discard unavailable
> > > timers correctly") Signed-off-by: Laurent Pinchart
> > > <laurent.pinchart+renesas@ideasonboard.com> ---
> >
> > You're the third one. I like how the function has been renamed in this
> > patch. arch_timer_probed() is not accurately describing the intention of
> > the code so arch_timer_need_probe() reads better. Maybe your rename of
> > the function can be squashed into Marc's patch.
>
> I'm happy with that as a cleanup, but right now I'd just like to see
> Marc's patch hit mainline as-is. It's days old and tested, and it would
> be nice to avoid another potential bug (not that I believe this patch is
> in any way broken).
Sure. I'll resend my patch as a cleanup on top of Marc's fix.
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2014-10-23 23:29 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-10-23 17:32 [PATCH] clocksource: arm_arch_timer: Don't wait for probe of unexisting timers Laurent Pinchart
2014-10-23 17:39 ` Stephen Boyd
2014-10-23 17:43 ` Laurent Pinchart
2014-10-23 17:49 ` Mark Rutland
2014-10-23 23:29 ` Laurent Pinchart
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).