clocksource: arm_arch_timer: Don't wait for probe of unexisting timers

PATCH New Laurent Pinchart 4 participants 5 messages Oct 23, 2014

Subsystems (2)

Clocksource, clockevent drivers Arm architected timer driver

lore

Share & tools

lore

Laurent Pinchart

author

2014-10-23 17:32 UTC #0 lore raw

PATCH 1/1 clocksource: arm_arch_timer: Don't wait for probe of unexisting timers

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 <[email protected]>

---
 drivers/clocksource/arm_arch_timer.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

This patch fixes a regression introduced in v3.18-rc1.

drivers/clocksource/arm_arch_timer.c +8 -7

index 2133f9d..5caf32c 100644
@@ -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;
 	}

Signature · 3 lines

Regards,

Laurent Pinchart

Stephen Boyd

2014-10-23 17:39 UTC #1 lore raw

PATCH 1/1 clocksource: arm_arch_timer: Don't wait for probe of unexisting timers

Quoted 8 lines · Laurent Pinchart

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 <[email protected]>
---

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.

Quoted 45 lines

 drivers/clocksource/arm_arch_timer.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

This patch fixes a regression introduced in v3.18-rc1.

drivers/clocksource/arm_arch_timer.c +8 -7

index 2133f9d..5caf32c 100644
@@ -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;
 	}

Signature · 2 lines

Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Laurent Pinchart

2014-10-23 17:43 UTC #2 lore raw

PATCH 1/1 clocksource: arm_arch_timer: Don't wait for probe of unexisting timers

Hi Stephen,

Quoted 11 lines · On Thursday 23 October 2014 10:39:18 Stephen Boyd wrote:

Quoted 8 lines · Laurent Pinchart
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 <[email protected]> ---
You're the third one.

Sorry about that.

Quoted 4 lines

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.

Quoted 46 lines

 drivers/clocksource/arm_arch_timer.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

This patch fixes a regression introduced in v3.18-rc1.

(quoted diff) +0 -0

b/drivers/clocksource/arm_arch_timer.c index 2133f9d..5caf32c 100644

drivers/clocksource/arm_arch_timer.c +0 -0

@@ -657,17 +657,17 @@ static const struct of_device_id

arch_timer_mem_of_match[] __initconst = {>
};

static bool __init

drivers/clocksource/arm_arch_timer.c +3 -4

-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))

drivers/clocksource/arm_arch_timer.c +5 -3

+		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;
 	}

Signature · 3 lines

Regards,

Laurent Pinchart

Mark Rutland

maintainer

2014-10-23 17:49 UTC #3 lore raw

PATCH 1/1 clocksource: arm_arch_timer: Don't wait for probe of unexisting timers

In reply to Stephen Boyd

Quoted 6 lines · Stephen Boyd

Quoted 5 lines · Laurent Pinchart
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.

Quoted 8 lines

Quoted 3 lines
Fixes c387f07e6205 ("clocksource: arm_arch_timer: Discard unavailable timers correctly")
Signed-off-by Laurent Pinchart <[email protected]>
---
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.

Quoted 53 lines

Quoted 46 lines

 drivers/clocksource/arm_arch_timer.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

This patch fixes a regression introduced in v3.18-rc1.

drivers/clocksource/arm_arch_timer.c +8 -7

index 2133f9d..5caf32c 100644
@@ -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;
 	}

Signature · 2 lines

Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Laurent Pinchart

2014-10-23 23:29 UTC #4 lore raw

PATCH 1/1 clocksource: arm_arch_timer: Don't wait for probe of unexisting timers

Hi Mark,

Quoted 16 lines · On Thursday 23 October 2014 18:49:11 Mark Rutland wrote:

Quoted 6 lines · Stephen Boyd
Quoted 5 lines · Laurent Pinchart
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.
Quoted 8 lines
Quoted 3 lines
Fixes c387f07e6205 ("clocksource: arm_arch_timer: Discard unavailable
timers correctly") Signed-off-by: Laurent Pinchart <[email protected]> ---
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.

Quoted 4 lines

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.

Signature · 3 lines

Regards,

Laurent Pinchart