Skip to contentSkip to conversation

clocksource: sh_tmu: Set cpu_possible_mask to fix SMP broadcast

PATCH Applied Magnus Damm 4 participants 28 messages Dec 16, 2014 – Dec 19, 2014

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 clocksource: sh_tmu: Set cpu_possible_mask to fix SMP broadcast

From: Magnus Damm <[email protected]>

Update the TMU driver to use cpu_possible_mask as cpumask to make r8a7779 SMP work as expected with or without the ARM TWD timer.

---

 drivers/clocksource/sh_tmu.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/clocksource/sh_tmu.c +1 -1
@@ -428,7 +428,7 @@ static void sh_tmu_register_clockevent(s
 	ced->features = CLOCK_EVT_FEAT_PERIODIC;
 	ced->features |= CLOCK_EVT_FEAT_ONESHOT;
 	ced->rating = 200;
-	ced->cpumask = cpumask_of(0);
+	ced->cpumask = cpu_possible_mask;
 	ced->set_next_event = sh_tmu_clock_event_next;
 	ced->set_mode = sh_tmu_clock_event_mode;
 	ced->suspend = sh_tmu_clock_event_suspend;
#1 lore raw
Quoted 6 lines · Magnus Damm

From: Magnus Damm <[email protected]>

Update the TMU driver to use cpu_possible_mask as cpumask to make r8a7779 SMP work as expected with or without the ARM TWD timer.

Applied as a 3.18 fix.

Thanks !

-- Daniel

ps: May I suggest to use the CLOCK_EVT_FEAT_DYNIRQ flag for this driver ?

Quoted 17 lines

---

  drivers/clocksource/sh_tmu.c |    2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
drivers/clocksource/sh_tmu.c +1 -1
@@ -428,7 +428,7 @@ static void sh_tmu_register_clockevent(s
  	ced->features = CLOCK_EVT_FEAT_PERIODIC;
  	ced->features |= CLOCK_EVT_FEAT_ONESHOT;
  	ced->rating = 200;
-	ced->cpumask = cpumask_of(0);
+	ced->cpumask = cpu_possible_mask;
  	ced->set_next_event = sh_tmu_clock_event_next;
  	ced->set_mode = sh_tmu_clock_event_mode;
  	ced->suspend = sh_tmu_clock_event_suspend;
Signature · 5 lines
  <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
#2 lore raw

Hi Daniel,

Quoted 9 lines · On Tuesday 16 December 2014 12:14:40 Daniel Lezcano wrote:
Quoted 6 lines · Magnus Damm

From: Magnus Damm <[email protected]>

Update the TMU driver to use cpu_possible_mask as cpumask to make r8a7779 SMP work as expected with or without the ARM TWD timer.

Applied as a 3.18 fix.

You're a bit too fast, I haven't had time to review the patch yet.

Quoted 11 lines

ps: May I suggest to use the CLOCK_EVT_FEAT_DYNIRQ flag for this driver ?

Quoted 9 lines

---

  drivers/clocksource/sh_tmu.c |    2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
drivers/clocksource/sh_tmu.c +0 -0
@@ -428,7 +428,7 @@ static void sh_tmu_register_clockevent(s
 
Quoted 5 lines

drivers/clocksource/sh_tmu.c static void sh_tmu_register_clockevent(s · line 431

Quoted 5 lines
drivers/clocksource/sh_tmu.c +1 -1
  	ced->features = CLOCK_EVT_FEAT_PERIODIC;
  	ced->features |= CLOCK_EVT_FEAT_ONESHOT;
  	ced->rating = 200;
-	ced->cpumask = cpumask_of(0);
+	ced->cpumask = cpu_possible_mask;

Magnus, how thoroughly have you tested this ? The TMU is indeed usable by all CPUs, so setting the CPU mask to cpu_possible_mask makes sense, but last time I've tried that it broke the broadcast timer due to the heuristics used by the clock events core code.

Could you please confirm that you've tested both CONFIG_PREEMPT_NONE and CONFIG_PREEMPT with and without the ARM TWD times, and that you've booted to userspace and tested timer broadcast on all CPUs ?

Quoted 3 lines
Quoted 3 lines
drivers/clocksource/sh_tmu.c +0 -0
  	ced->set_next_event = sh_tmu_clock_event_next;
  	ced->set_mode = sh_tmu_clock_event_mode;
  	ced->suspend = sh_tmu_clock_event_suspend;
Signature · 3 lines
Regards,

Laurent Pinchart
#3 lore raw

Hi Magnus,

Quoted 37 lines · On Tuesday 16 December 2014 13:20:56 Laurent Pinchart wrote:
Quoted 9 lines · On Tuesday 16 December 2014 12:14:40 Daniel Lezcano wrote:
Quoted 6 lines · Magnus Damm

From: Magnus Damm <[email protected]>

Update the TMU driver to use cpu_possible_mask as cpumask to make r8a7779 SMP work as expected with or without the ARM TWD timer.

Applied as a 3.18 fix.

You're a bit too fast, I haven't had time to review the patch yet.

Quoted 12 lines

ps: May I suggest to use the CLOCK_EVT_FEAT_DYNIRQ flag for this driver ?

Quoted 10 lines

---

  drivers/clocksource/sh_tmu.c |    2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
drivers/clocksource/sh_tmu.c +1 -0
+0900
@@ -428,7 +428,7 @@ static void sh_tmu_register_clockevent(s
 
Quoted 6 lines

drivers/clocksource/sh_tmu.c static void sh_tmu_register_clockevent(s · line 431

Quoted 6 lines
drivers/clocksource/sh_tmu.c +1 -1
  	ced->features = CLOCK_EVT_FEAT_PERIODIC;
  	ced->features |= CLOCK_EVT_FEAT_ONESHOT;
  	ced->rating = 200;
 
-	ced->cpumask = cpumask_of(0);
+	ced->cpumask = cpu_possible_mask;

Magnus, how thoroughly have you tested this ? The TMU is indeed usable by all CPUs, so setting the CPU mask to cpu_possible_mask makes sense, but last time I've tried that it broke the broadcast timer due to the heuristics used by the clock events core code.

Quoted 3 lines

Could you please confirm that you've tested both CONFIG_PREEMPT_NONE and CONFIG_PREEMPT with and without the ARM TWD times, and that you've booted to userspace and tested timer broadcast on all CPUs ?

I forgot to mention that this should be tested on R8A7740 with TMU only and both TMU and CMT, and on r8A7779 with TMU only both TMU and TWD.

Without getting test results I'm afraid I must NACK the patch.

Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
drivers/clocksource/sh_tmu.c +0 -0
  	ced->set_next_event = sh_tmu_clock_event_next;
  	ced->set_mode = sh_tmu_clock_event_mode;
  	ced->suspend = sh_tmu_clock_event_suspend;
Signature · 3 lines
Regards,

Laurent Pinchart
#4 lore raw
In reply to Laurent Pinchart
Quoted 6 lines · Laurent Pinchart

Hi Daniel,

Quoted 3 lines · On Tuesday 16 December 2014 12:14:40 Daniel Lezcano wrote:
Quoted 2 lines · Magnus Damm

From: Magnus Damm <[email protected]>

Quoted 8 lines
Quoted 6 lines
Quoted 4 lines

Update the TMU driver to use cpu_possible_mask as cpumask to make r8a7779 SMP work as expected with or without the ARM TWD timer.

Applied as a 3.18 fix.

You're a bit too fast, I haven't had time to review the patch yet.

Ah, ok. Hanging on then :)

Quoted 31 lines
Quoted 12 lines

ps: May I suggest to use the CLOCK_EVT_FEAT_DYNIRQ flag for this driver ?

Quoted 9 lines

---

   drivers/clocksource/sh_tmu.c |    2 +-
   1 file changed, 1 insertion(+), 1 deletion(-)
drivers/clocksource/sh_tmu.c +0 -0
@@ -428,7 +428,7 @@ static void sh_tmu_register_clockevent(s
 
Quoted 5 lines

drivers/clocksource/sh_tmu.c static void sh_tmu_register_clockevent(s · line 431

Quoted 5 lines
drivers/clocksource/sh_tmu.c +1 -1
   	ced->features = CLOCK_EVT_FEAT_PERIODIC;
   	ced->features |= CLOCK_EVT_FEAT_ONESHOT;
   	ced->rating = 200;
-	ced->cpumask = cpumask_of(0);
+	ced->cpumask = cpu_possible_mask;

Magnus, how thoroughly have you tested this ? The TMU is indeed usable by all CPUs, so setting the CPU mask to cpu_possible_mask makes sense, but last time I've tried that it broke the broadcast timer due to the heuristics used by the clock events core code.

Could you please confirm that you've tested both CONFIG_PREEMPT_NONE and CONFIG_PREEMPT with and without the ARM TWD times, and that you've booted to userspace and tested timer broadcast on all CPUs ?

Quoted 3 lines
Quoted 3 lines
drivers/clocksource/sh_tmu.c +0 -0
   	ced->set_next_event = sh_tmu_clock_event_next;
   	ced->set_mode = sh_tmu_clock_event_mode;
   	ced->suspend = sh_tmu_clock_event_suspend;
Signature · 5 lines
  <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
#5 lore raw
In reply to Laurent Pinchart

Hi Laurent,

Quoted 26 lines · Laurent Pinchart

Hi Daniel,

Quoted 9 lines · On Tuesday 16 December 2014 12:14:40 Daniel Lezcano wrote:
Quoted 6 lines · Magnus Damm

From: Magnus Damm <[email protected]>

Update the TMU driver to use cpu_possible_mask as cpumask to make r8a7779 SMP work as expected with or without the ARM TWD timer.

Applied as a 3.18 fix.

You're a bit too fast, I haven't had time to review the patch yet.

Quoted 11 lines

ps: May I suggest to use the CLOCK_EVT_FEAT_DYNIRQ flag for this driver ?

Quoted 9 lines

---

  drivers/clocksource/sh_tmu.c |    2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
drivers/clocksource/sh_tmu.c 2014-12-16 17:49:49.000000000 +0900 renamed from drivers/clocksource/sh_tmu.c +0 -0
@@ -428,7 +428,7 @@ static void sh_tmu_register_clockevent(s
 
Quoted 10 lines
Quoted 5 lines

drivers/clocksource/sh_tmu.c 2014-12-16 17:49:49.000000000 +0900 static void sh_tmu_register_clockevent(s · line 432

Quoted 5 lines
drivers/clocksource/sh_tmu.c 2014-12-16 17:49:49.000000000 +0900 renamed from drivers/clocksource/sh_tmu.c +1 -1
    ced->features = CLOCK_EVT_FEAT_PERIODIC;
    ced->features |= CLOCK_EVT_FEAT_ONESHOT;
    ced->rating = 200;
-   ced->cpumask = cpumask_of(0);
+   ced->cpumask = cpu_possible_mask;

Magnus, how thoroughly have you tested this ? The TMU is indeed usable by all CPUs, so setting the CPU mask to cpu_possible_mask makes sense, but last time I've tried that it broke the broadcast timer due to the heuristics used by the clock events core code.

Uhm, so I've tested this particular patch on r8a7779 but I do agree that the TMU is used on a bunch of SoCs if that's what you mean. I don't see how it is different from any other of our timers though, and those got fixed like this earlier.

I wonder if you may recall an earlier issue with incorrect clock event priorities and code somehow working-by-accident without the mask set as expected?

Quoted 3 lines

Could you please confirm that you've tested both CONFIG_PREEMPT_NONE and CONFIG_PREEMPT with and without the ARM TWD times, and that you've booted to userspace and tested timer broadcast on all CPUs ?

No I have not. I've booted to user space in initramfs with DT-based TWD on Multiplatform for r8a7779. Without this fix (and other r8a7779 TWD bits) I see a lot of breakage. For instance, TWD and SMP boot is broken on r8a7779 - both legacy and non-legacy. I have not gotten to sh73a0 yet, but I assume it is busted too.

Can you please explain to me how the TMU is any different compared to the CMT, MTU2 or STI? =)

And no, I don't have any r8a7740 board anymore. Can anyone else test?

Cheers,

/ magnus

#6 lore raw
Quoted 41 lines · Magnus Damm

Hi Laurent,

Quoted 26 lines · Laurent Pinchart

Hi Daniel,

Quoted 9 lines · On Tuesday 16 December 2014 12:14:40 Daniel Lezcano wrote:
Quoted 6 lines · Magnus Damm

From: Magnus Damm <[email protected]>

Update the TMU driver to use cpu_possible_mask as cpumask to make r8a7779 SMP work as expected with or without the ARM TWD timer.

Applied as a 3.18 fix.

You're a bit too fast, I haven't had time to review the patch yet.

Quoted 11 lines

ps: May I suggest to use the CLOCK_EVT_FEAT_DYNIRQ flag for this driver ?

Quoted 9 lines

---

   drivers/clocksource/sh_tmu.c |    2 +-
   1 file changed, 1 insertion(+), 1 deletion(-)
drivers/clocksource/sh_tmu.c 2014-12-16 17:49:49.000000000 +0900 renamed from drivers/clocksource/sh_tmu.c +0 -0
@@ -428,7 +428,7 @@ static void sh_tmu_register_clockevent(s
 
Quoted 10 lines
Quoted 5 lines

drivers/clocksource/sh_tmu.c 2014-12-16 17:49:49.000000000 +0900 static void sh_tmu_register_clockevent(s · line 432

Quoted 5 lines
drivers/clocksource/sh_tmu.c 2014-12-16 17:49:49.000000000 +0900 renamed from drivers/clocksource/sh_tmu.c +1 -1
     ced->features = CLOCK_EVT_FEAT_PERIODIC;
     ced->features |= CLOCK_EVT_FEAT_ONESHOT;
     ced->rating = 200;
-   ced->cpumask = cpumask_of(0);
+   ced->cpumask = cpu_possible_mask;

Magnus, how thoroughly have you tested this ? The TMU is indeed usable by all CPUs, so setting the CPU mask to cpu_possible_mask makes sense, but last time I've tried that it broke the broadcast timer due to the heuristics used by the clock events core code.

Quoted 8 lines

Uhm, so I've tested this particular patch on r8a7779 but I do agree that the TMU is used on a bunch of SoCs if that's what you mean. I don't see how it is different from any other of our timers though, and those got fixed like this earlier.

I wonder if you may recall an earlier issue with incorrect clock event priorities and code somehow working-by-accident without the mask set as expected?

Could have been fixed with : ?

commit 70e5975d3a04be5479a28eec4a2fb10f98ad2785

Author: Stephen Boyd <[email protected]>
Date:   Thu Jun 13 11:39:50 2013 -0700

clockevents: Prefer CPU local devices over global devices

On an SMP system with only one global clockevent and a dummy clockevent per CPU we run into problems. We want the dummy clockevents to be registered as the per CPU tick devices, but we can only achieve that if we register the dummy clockevents before the global clockevent or if we artificially inflate the rating of the dummy clockevents to be higher than the rating of the global clockevent. Failure to do so leads to boot hangs when the dummy timers are registered on all other CPUs besides the CPU that accepted the global clockevent as its tick device and there is no broadcast timer to poke the dummy devices.

If we're registering multiple clockevents and one clockevent is global and the other is local to a particular CPU we should choose to use the local clockevent regardless of the rating of the device. This way, if the clockevent is a dummy it will take the tick device duty as long as there isn't a higher rated tick device and any global clockevent will be bumped out into broadcast mode, fixing the problem described above.

Quoted 19 lines
Quoted 3 lines

Could you please confirm that you've tested both CONFIG_PREEMPT_NONE and CONFIG_PREEMPT with and without the ARM TWD times, and that you've booted to userspace and tested timer broadcast on all CPUs ?

No I have not. I've booted to user space in initramfs with DT-based TWD on Multiplatform for r8a7779. Without this fix (and other r8a7779 TWD bits) I see a lot of breakage. For instance, TWD and SMP boot is broken on r8a7779 - both legacy and non-legacy. I have not gotten to sh73a0 yet, but I assume it is busted too.

Can you please explain to me how the TMU is any different compared to the CMT, MTU2 or STI? =)

And no, I don't have any r8a7740 board anymore. Can anyone else test?

Cheers,

/ magnus

Signature · 5 lines
  <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
#7 lore raw

Hi Daniel,

Quoted 66 lines · On Tuesday 16 December 2014 12:54:56 Daniel Lezcano wrote:
Quoted 36 lines · Magnus Damm
Quoted 20 lines · Laurent Pinchart
Quoted 9 lines · On Tuesday 16 December 2014 12:14:40 Daniel Lezcano wrote:
Quoted 6 lines · Magnus Damm

From: Magnus Damm <[email protected]>

Update the TMU driver to use cpu_possible_mask as cpumask to make r8a7779 SMP work as expected with or without the ARM TWD timer.

Applied as a 3.18 fix.

You're a bit too fast, I haven't had time to review the patch yet.

Quoted 7 lines

ps: May I suggest to use the CLOCK_EVT_FEAT_DYNIRQ flag for this driver ?

Quoted 4 lines
---
   drivers/clocksource/sh_tmu.c |    2 +-
   1 file changed, 1 insertion(+), 1 deletion(-)
Quoted 14 lines
Quoted 9 lines

drivers/clocksource/sh_tmu.c 2014-12-16

Quoted 9 lines
drivers/clocksource/sh_tmu.c 2014-12-16 renamed from drivers/clocksource/sh_tmu.c +0 -0

17:49:49.000000000 +0900 @@ -428,7 +428,7 @@ static void sh_tmu_register_clockevent(s

    ced->features = CLOCK_EVT_FEAT_PERIODIC;
    ced->features |= CLOCK_EVT_FEAT_ONESHOT;
    ced->rating = 200;
drivers/clocksource/sh_tmu.c 2014-12-16 renamed from drivers/clocksource/sh_tmu.c +1 -1
-   ced->cpumask = cpumask_of(0);
+   ced->cpumask = cpu_possible_mask;

Magnus, how thoroughly have you tested this ? The TMU is indeed usable by all CPUs, so setting the CPU mask to cpu_possible_mask makes sense, but last time I've tried that it broke the broadcast timer due to the heuristics used by the clock events core code.

Quoted 8 lines

Uhm, so I've tested this particular patch on r8a7779 but I do agree that the TMU is used on a bunch of SoCs if that's what you mean. I don't see how it is different from any other of our timers though, and those got fixed like this earlier.

I wonder if you may recall an earlier issue with incorrect clock event priorities and code somehow working-by-accident without the mask set as expected?

Could have been fixed with : ?

commit 70e5975d3a04be5479a28eec4a2fb10f98ad2785

Author: Stephen Boyd <[email protected]>
Date:   Thu Jun 13 11:39:50 2013 -0700

clockevents: Prefer CPU local devices over global devices

On an SMP system with only one global clockevent and a dummy clockevent per CPU we run into problems. We want the dummy clockevents to be registered as the per CPU tick devices, but we can only achieve that if we register the dummy clockevents before the global clockevent or if we artificially inflate the rating of the dummy clockevents to be higher than the rating of the global clockevent. Failure to do so leads to boot hangs when the dummy timers are registered on all other CPUs besides the CPU that accepted the global clockevent as its tick device and there is no broadcast timer to poke the dummy devices.

Quoted 7 lines

If we're registering multiple clockevents and one clockevent is global and the other is local to a particular CPU we should choose to use the local clockevent regardless of the rating of the device. This way, if the clockevent is a dummy it will take the tick device duty as long as there isn't a higher rated tick device and any global clockevent will be bumped out into broadcast mode, fixing the problem described above.

I think I had that patch in my tree when I noticed the breakage, since my own tmu patch dates from February 2014. I'll retest it though and will let you know.

Quoted 14 lines
Quoted 14 lines
Quoted 3 lines

Could you please confirm that you've tested both CONFIG_PREEMPT_NONE and CONFIG_PREEMPT with and without the ARM TWD times, and that you've booted to userspace and tested timer broadcast on all CPUs ?

No I have not. I've booted to user space in initramfs with DT-based TWD on Multiplatform for r8a7779. Without this fix (and other r8a7779 TWD bits) I see a lot of breakage. For instance, TWD and SMP boot is broken on r8a7779 - both legacy and non-legacy. I have not gotten to sh73a0 yet, but I assume it is busted too.

Can you please explain to me how the TMU is any different compared to the CMT, MTU2 or STI? =)

And no, I don't have any r8a7740 board anymore. Can anyone else test?

Signature · 3 lines
Regards,

Laurent Pinchart
#8 lore raw
In reply to Magnus Damm

Hi Magnus,

Quoted 9 lines · Magnus Damm
Quoted 3 lines

Could you please confirm that you've tested both CONFIG_PREEMPT_NONE and CONFIG_PREEMPT with and without the ARM TWD times, and that you've booted to userspace and tested timer broadcast on all CPUs ?

No I have not. I've booted to user space in initramfs with DT-based TWD on Multiplatform for r8a7779. Without this fix (and other r8a7779 TWD bits) I see a lot of breakage. For instance, TWD and SMP boot is broken on r8a7779 - both legacy and non-legacy. I have not gotten to sh73a0 yet, but I assume it is busted too.

FWIW, I applied this patch, and booted the result successfully and without any user-visible changes on:
- koelsch
- armadillo-multiplatform
- kzm9g-legacy
- kzm9g-multiplatform

Armadillo-legacy is broken, and probably won't come back (cfr.
https://lkml.org/lkml/2014/12/2/339).

Kzm9g-reference still hangs at "Calibrating local timer..." (it did work at some point in the past).

Note that my local tree is based on renesas-drivers-2014-12-08-v3.18, renesas-devel-20141212-v3.18, and yesterday's upstream, with CCF, PM domain, TWD, and lots of WIP patches from the kitchen sink applied.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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

#9 lore raw

Hi Geert,

Quoted 13 lines · Geert Uytterhoeven

Hi Magnus,

Quoted 9 lines · Magnus Damm
Quoted 3 lines

Could you please confirm that you've tested both CONFIG_PREEMPT_NONE and CONFIG_PREEMPT with and without the ARM TWD times, and that you've booted to userspace and tested timer broadcast on all CPUs ?

No I have not. I've booted to user space in initramfs with DT-based TWD on Multiplatform for r8a7779. Without this fix (and other r8a7779 TWD bits) I see a lot of breakage. For instance, TWD and SMP boot is broken on r8a7779 - both legacy and non-legacy. I have not gotten to sh73a0 yet, but I assume it is busted too.

Quoted 6 lines

FWIW, I applied this patch, and booted the result successfully and without any user-visible changes on:
- koelsch
- armadillo-multiplatform
- kzm9g-legacy
- kzm9g-multiplatform

Thanks for testing - now let us wait and see what Laurent says.

Quoted 5 lines

Armadillo-legacy is broken, and probably won't come back (cfr.
https://lkml.org/lkml/2014/12/2/339).

Kzm9g-reference still hangs at "Calibrating local timer..." (it did work at some point in the past).

When the time allows I'd like us to try to fix up these somehow. At least I'll give it a go - I guess Armadillo is difficult without any board.

Quoted 3 lines

Note that my local tree is based on renesas-drivers-2014-12-08-v3.18, renesas-devel-20141212-v3.18, and yesterday's upstream, with CCF, PM domain, TWD, and lots of WIP patches from the kitchen sink applied.

The TWD portion above - is it driver code or integration stuff? If the latter - is there any reason why these can't be picked up by Simon and merged right away.

My plan is to dig into sh73a0 (maybe including TWD) later today.

Thanks for your help!

/ magnus

#10 lore raw

Hi Magnus and Geert,

Quoted 14 lines · On Wednesday 17 December 2014 10:30:33 Magnus Damm wrote:
Quoted 13 lines · Geert Uytterhoeven
Quoted 9 lines · Magnus Damm
Quoted 3 lines

Could you please confirm that you've tested both CONFIG_PREEMPT_NONE and CONFIG_PREEMPT with and without the ARM TWD times, and that you've booted to userspace and tested timer broadcast on all CPUs ?

No I have not. I've booted to user space in initramfs with DT-based TWD on Multiplatform for r8a7779. Without this fix (and other r8a7779 TWD bits) I see a lot of breakage. For instance, TWD and SMP boot is broken on r8a7779 - both legacy and non-legacy. I have not gotten to sh73a0 yet, but I assume it is busted too.

FWIW, I applied this patch, and booted the result successfully and without

Quoted 13 lines
Quoted 5 lines

any user-visible changes on:
- koelsch
- armadillo-multiplatform
- kzm9g-legacy
- kzm9g-multiplatform

Thanks for testing - now let us wait and see what Laurent says.

Quoted 5 lines

Armadillo-legacy is broken, and probably won't come back (cfr.
https://lkml.org/lkml/2014/12/2/339).

Kzm9g-reference still hangs at "Calibrating local timer..." (it did work at some point in the past).

kzm9g-reference boots for me with kzm9g_defconfig on Simon's devel branch with CONFIG_PREEMPT_NONE but fails to boot with CONFIG_PREEMPT. The platform doesn't instantiate the TMU at the moment anyway, so it's unrelated to this patch.

Given that kzm9g-legacy works for me regardless of the preempt configuration and on the TMU cpumask, that kzm9g-reference doesn't use TMU, and that you've tested kzm9g-multiplaform successfully, I think we can consider that this patch is safe from a kzm9g point of view.

(By the way, how have you tested kzm9g-multiplatform given that none of renesas-drivers-2014-12-08-v3.18, renesas-devel-20141212-v3.18 or today's upstream support multiplatform kernels for sh73a0 ?)

Magnus, you have told me that you've performed tests on Marzen with CONFIG_PREEMPT_NONE and with both TWD enabled and disabled. If you could perform the same tests with CONFIG_PREEMPT instead without noticing any regression I think we can merge your patch. Whatever breakage it has caused in the past is likely hidden somewhere beyond eyesight.

Quoted 3 lines

When the time allows I'd like us to try to fix up these somehow. At least I'll give it a go - I guess Armadillo is difficult without any board.

I'd like that very much. Let's get rid of r8a73a4 legacy (Ulrich's patches should be ready) first, and possible sh73a0 and r8a7740 as well, and then retest our various timers configurations. We should test both CONFIG_PREEMPT and CONFIG_PREEMPT none, with all combination of the CMT, TMU, MTU2, TWD and ARM architected timer enabled.

Quoted 9 lines
Quoted 3 lines

Note that my local tree is based on renesas-drivers-2014-12-08-v3.18, renesas-devel-20141212-v3.18, and yesterday's upstream, with CCF, PM domain, TWD, and lots of WIP patches from the kitchen sink applied.

The TWD portion above - is it driver code or integration stuff? If the latter - is there any reason why these can't be picked up by Simon and merged right away.

My plan is to dig into sh73a0 (maybe including TWD) later today.

Signature · 3 lines
Regards,

Laurent Pinchart
#11 lore raw

Hi Laurent,

Quoted 4 lines · Laurent Pinchart
Quoted 2 lines
Quoted 2 lines

Kzm9g-reference still hangs at "Calibrating local timer..." (it did work at some point in the past).

kzm9g-reference boots for me with kzm9g_defconfig on Simon's devel branch with

OK.

I had expected the breakage to be something in my tree, either an issue in a -next branch I'm using, an interaction with the CCF patches or so, or a config issue (e.g. CONFIG_CPU_IDLE became broken lately if the TWD is not in DT).

Quoted 3 lines

(By the way, how have you tested kzm9g-multiplatform given that none of renesas-drivers-2014-12-08-v3.18, renesas-devel-20141212-v3.18 or today's upstream support multiplatform kernels for sh73a0 ?)

As I said, my local tree contains lots of extra patches (erhm... 233).

Quoted 9 lines

Magnus, you have told me that you've performed tests on Marzen with CONFIG_PREEMPT_NONE and with both TWD enabled and disabled. If you could perform the same tests with CONFIG_PREEMPT instead without noticing any regression I think we can merge your patch. Whatever breakage it has caused in the past is likely hidden somewhere beyond eyesight.

Quoted 3 lines

When the time allows I'd like us to try to fix up these somehow. At least I'll give it a go - I guess Armadillo is difficult without any board.

I do have an Armadillo.

Quoted 5 lines

I'd like that very much. Let's get rid of r8a73a4 legacy (Ulrich's patches should be ready) first, and possible sh73a0 and r8a7740 as well, and then retest our various timers configurations. We should test both CONFIG_PREEMPT and CONFIG_PREEMPT none, with all combination of the CMT, TMU, MTU2, TWD and ARM architected timer enabled.

Yes, getting rid of more legacy is good. It's just a pity for the Penguin on the Armadillo's LCD, which doesn't want to visit armadillo-multiplatform.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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

#12 lore raw

Hi Laurent, Magnus,

Quoted 5 lines · Geert Uytterhoeven
Quoted 3 lines · Laurent Pinchart
Quoted 2 lines
Quoted 2 lines

Kzm9g-reference still hangs at "Calibrating local timer..." (it did work at some point in the past).

Quoted 8 lines
Quoted 1 line

kzm9g-reference boots for me with kzm9g_defconfig on Simon's devel branch with

OK.

I had expected the breakage to be something in my tree, either an issue in a -next branch I'm using, an interaction with the CCF patches or so, or a config issue (e.g. CONFIG_CPU_IDLE became broken lately if the TWD is not in DT).

Kzm9g-reference hangs at "Calibrating local timer..." because I added the TWD to sh73a0.dtsi (http://www.spinics.net/lists/arm-kernel/msg383413.html).

As twd_get_clock() does

        if (np)
                twd_clk = of_clk_get(np, 0);
        else
                twd_clk = clk_get_sys("smp_twd", NULL);

it's not DT-without-CCF proof, and kzm9g-reference cannot get its clock :-(

So the TWD node should be in sh73a0-kzm9g-multiplatform.dts, and we need more dts files instead of less...

Note that I added the TWD node to DT to fix a hang on kzm9g-multiplatform with CONFIG_CPU_IDLE=y (recently introduced "regression" in core code) after:

DMA: preallocated 256 KiB pool for atomic coherent allocations

On kzm9g-legacy the TWD is instantiated from C board code (machine_desc.init_time = sh73a0_earlytimer_init), so CONFIG_CPU_IDLE=y does not hang. Despite https://lkml.org/lkml/2014/12/2/339.

On kzm9g-reference, the TWD is not instantiated, causing the same hang. If I instantiate the TWD from C board code there, it fails with

twd: can't register interrupt 29 (-22)

which looks like a symptom of https://lkml.org/lkml/2014/12/2/339

So no cookies for users of several -legacy and -reference platforms in v3.19-rc1 this Monday...

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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

#13 lore raw

Hi Geert,

Quoted 43 lines · On Wednesday 17 December 2014 10:42:52 Geert Uytterhoeven wrote:
Quoted 7 lines · Geert Uytterhoeven
Quoted 6 lines · Laurent Pinchart
Quoted 2 lines
Quoted 2 lines

Kzm9g-reference still hangs at "Calibrating local timer..." (it did work at some point in the past).

kzm9g-reference boots for me with kzm9g_defconfig on Simon's devel branch with

Quoted 6 lines

OK.

I had expected the breakage to be something in my tree, either an issue in a -next branch I'm using, an interaction with the CCF patches or so, or a config issue (e.g. CONFIG_CPU_IDLE became broken lately if the TWD is not in DT).

Kzm9g-reference hangs at "Calibrating local timer..." because I added the TWD to sh73a0.dtsi (http://www.spinics.net/lists/arm-kernel/msg383413.html).

As twd_get_clock() does

        if (np)
                twd_clk = of_clk_get(np, 0);
        else
                twd_clk = clk_get_sys("smp_twd", NULL);

it's not DT-without-CCF proof, and kzm9g-reference cannot get its clock :-(

So the TWD node should be in sh73a0-kzm9g-multiplatform.dts, and we need more dts files instead of less...

Note that I added the TWD node to DT to fix a hang on kzm9g-multiplatform with CONFIG_CPU_IDLE=y (recently introduced "regression" in core code) after:

DMA: preallocated 256 KiB pool for atomic coherent allocations

On kzm9g-legacy the TWD is instantiated from C board code (machine_desc.init_time = sh73a0_earlytimer_init), so CONFIG_CPU_IDLE=y does not hang. Despite https://lkml.org/lkml/2014/12/2/339.

On kzm9g-reference, the TWD is not instantiated, causing the same hang. If I instantiate the TWD from C board code there, it fails with

Quoted 6 lines

twd: can't register interrupt 29 (-22)

which looks like a symptom of https://lkml.org/lkml/2014/12/2/339

So no cookies for users of several -legacy and -reference platforms in v3.19-rc1 this Monday...

I'm not sure if fixing that would be worth it. Should we instead try to replace kzm9g-legacy and kzm9g-reference by kzm9g-multiplatform in v3.20 ? What are we missing ?

- The DIV6 multiparent series has been merged by Mike, and the sh73a0 CCF is nearly ready.

- We need proper BSC support to get LAN working, although it could be worked around temporarily by specifying the BSC clock in the LAN node if I recall correctly.

- MMCIF and SDHI are present.

- Sound support is there as far as I can tell, but possibly without DMA.

- Touchscreen and magnetometer have DT bindings but are not instantiated in the dts at the moment. That should be easy to fix.

- Accelerometer and RTC have no DT bindings, but they should work with the I2C core OF match support.

- We're loosing USBHS (driver supports OF but doesn't cover sh73a0), USB host (no OF support in driver) and LCDC (no OF support in driver).

All but USBHS, USB host and LCDC should be possible to support in v3.20 without too much issues, depending on the effort we can put in this.

Signature · 3 lines
Regards,

Laurent Pinchart
#14 lore raw

Hi Laurent,

Quoted 46 lines · Laurent Pinchart
Quoted 45 lines · On Wednesday 17 December 2014 10:42:52 Geert Uytterhoeven wrote:
Quoted 7 lines · Geert Uytterhoeven
Quoted 6 lines · Laurent Pinchart
Quoted 2 lines
Quoted 2 lines

Kzm9g-reference still hangs at "Calibrating local timer..." (it did work at some point in the past).

kzm9g-reference boots for me with kzm9g_defconfig on Simon's devel branch with

Quoted 6 lines

OK.

I had expected the breakage to be something in my tree, either an issue in a -next branch I'm using, an interaction with the CCF patches or so, or a config issue (e.g. CONFIG_CPU_IDLE became broken lately if the TWD is not in DT).

Kzm9g-reference hangs at "Calibrating local timer..." because I added the TWD to sh73a0.dtsi (http://www.spinics.net/lists/arm-kernel/msg383413.html).

As twd_get_clock() does

        if (np)
                twd_clk = of_clk_get(np, 0);
        else
                twd_clk = clk_get_sys("smp_twd", NULL);

it's not DT-without-CCF proof, and kzm9g-reference cannot get its clock :-(

So the TWD node should be in sh73a0-kzm9g-multiplatform.dts, and we need more dts files instead of less...

Note that I added the TWD node to DT to fix a hang on kzm9g-multiplatform with CONFIG_CPU_IDLE=y (recently introduced "regression" in core code) after:

DMA: preallocated 256 KiB pool for atomic coherent allocations

On kzm9g-legacy the TWD is instantiated from C board code (machine_desc.init_time = sh73a0_earlytimer_init), so CONFIG_CPU_IDLE=y does not hang. Despite https://lkml.org/lkml/2014/12/2/339.

On kzm9g-reference, the TWD is not instantiated, causing the same hang. If I instantiate the TWD from C board code there, it fails with

twd: can't register interrupt 29 (-22)

Quoted 7 lines
Quoted 4 lines

which looks like a symptom of https://lkml.org/lkml/2014/12/2/339

So no cookies for users of several -legacy and -reference platforms in v3.19-rc1 this Monday...

I'm not sure if fixing that would be worth it. Should we instead try to replace kzm9g-legacy and kzm9g-reference by kzm9g-multiplatform in v3.20 ?

v3.20 is reasonable. Except that some breakage will arrive in v3.19-rc1. So there will be a brokenness window for the kzm9g platform between v3.18 and v3.20.

Quoted 4 lines

What are we missing ?

- The DIV6 multiparent series has been merged by Mike, and the sh73a0 CCF is nearly ready.

Yep.

Quoted 3 lines

- We need proper BSC support to get LAN working, although it could be worked around temporarily by specifying the BSC clock in the LAN node if I recall correctly.

Working on that. I hope to send out v2 later today...

Quoted 2 lines

- Accelerometer and RTC have no DT bindings, but they should work with the I2C core OF match support.

I have RTC in my DTS, and it works.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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

#15 lore raw

Hi Geert,

Quoted 54 lines · On Wednesday 17 December 2014 13:11:05 Geert Uytterhoeven wrote:
Quoted 53 lines · Laurent Pinchart
Quoted 51 lines · On Wednesday 17 December 2014 10:42:52 Geert Uytterhoeven wrote:
Quoted 7 lines · Geert Uytterhoeven
Quoted 5 lines · Laurent Pinchart
Quoted 2 lines
Quoted 2 lines

Kzm9g-reference still hangs at "Calibrating local timer..." (it did work at some point in the past).

kzm9g-reference boots for me with kzm9g_defconfig on Simon's devel branch with

Quoted 6 lines

OK.

I had expected the breakage to be something in my tree, either an issue in a -next branch I'm using, an interaction with the CCF patches or so, or a config issue (e.g. CONFIG_CPU_IDLE became broken lately if the TWD is not in DT).

Kzm9g-reference hangs at "Calibrating local timer..." because I added the TWD to sh73a0.dtsi
(http://www.spinics.net/lists/arm-kernel/msg383413.html).

As twd_get_clock() does

        if (np)
                twd_clk = of_clk_get(np, 0);
        else
                twd_clk = clk_get_sys("smp_twd", NULL);

it's not DT-without-CCF proof, and kzm9g-reference cannot get its clock :-(

So the TWD node should be in sh73a0-kzm9g-multiplatform.dts, and we need more dts files instead of less...

Note that I added the TWD node to DT to fix a hang on kzm9g-multiplatform with CONFIG_CPU_IDLE=y (recently introduced "regression" in core code) after:

DMA: preallocated 256 KiB pool for atomic coherent allocations

On kzm9g-legacy the TWD is instantiated from C board code (machine_desc.init_time = sh73a0_earlytimer_init), so CONFIG_CPU_IDLE=y does not hang. Despite https://lkml.org/lkml/2014/12/2/339.

On kzm9g-reference, the TWD is not instantiated, causing the same hang. If I instantiate the TWD from C board code there, it fails with

twd: can't register interrupt 29 (-22)

which looks like a symptom of https://lkml.org/lkml/2014/12/2/339

So no cookies for users of several -legacy and -reference platforms in v3.19-rc1 this Monday...

Quoted 6 lines
Quoted 2 lines

I'm not sure if fixing that would be worth it. Should we instead try to replace kzm9g-legacy and kzm9g-reference by kzm9g-multiplatform in v3.20 ?

v3.20 is reasonable. Except that some breakage will arrive in v3.19-rc1. So there will be a brokenness window for the kzm9g platform between v3.18 and v3.20.

I know, and that's unfortunate, but I wonder if we should really spend time on fixing that if we can get multiplatform support merged in v3.20. We could consider multiplatform to be a regression fix and get it merged in v3.19, but that might be a bit far-fetched :-)

Quoted 11 lines
Quoted 4 lines

What are we missing ?

- The DIV6 multiparent series has been merged by Mike, and the sh73a0 CCF is nearly ready.

Yep.

Quoted 3 lines

- We need proper BSC support to get LAN working, although it could be worked around temporarily by specifying the BSC clock in the LAN node if I recall correctly.

Quoted 6 lines

Working on that. I hope to send out v2 later today...

Quoted 2 lines

- Accelerometer and RTC have no DT bindings, but they should work with the I2C core OF match support.

I have RTC in my DTS, and it works.

Great.

I can help with at least some of the remaining pieces if you share your working branch.

Signature · 3 lines
Regards,

Laurent Pinchart
#16 lore raw
In reply to Geert Uytterhoeven

Hi Geert,

Quoted 14 lines · On Wednesday 17 December 2014 09:30:57 Geert Uytterhoeven wrote:
Quoted 5 lines · Laurent Pinchart
Quoted 2 lines
Quoted 2 lines

Kzm9g-reference still hangs at "Calibrating local timer..." (it did work at some point in the past).

kzm9g-reference boots for me with kzm9g_defconfig on Simon's devel branch with

OK.

I had expected the breakage to be something in my tree, either an issue in a -next branch I'm using, an interaction with the CCF patches or so, or a config issue (e.g. CONFIG_CPU_IDLE became broken lately if the TWD is not in DT).

Quoted 5 lines
Quoted 3 lines

(By the way, how have you tested kzm9g-multiplatform given that none of renesas-drivers-2014-12-08-v3.18, renesas-devel-20141212-v3.18 or today's upstream support multiplatform kernels for sh73a0 ?)

As I said, my local tree contains lots of extra patches (erhm... 233).

We should probably synchronize at some point and get pending patches merged, or you'll loose sanity :-)

What multiplatform support patches do you plan to submit for v3.20 ?

Quoted 20 lines
Quoted 9 lines

Magnus, you have told me that you've performed tests on Marzen with CONFIG_PREEMPT_NONE and with both TWD enabled and disabled. If you could perform the same tests with CONFIG_PREEMPT instead without noticing any regression I think we can merge your patch. Whatever breakage it has caused in the past is likely hidden somewhere beyond eyesight.

Quoted 3 lines

When the time allows I'd like us to try to fix up these somehow. At least I'll give it a go - I guess Armadillo is difficult without any board.

I do have an Armadillo.

Quoted 5 lines

I'd like that very much. Let's get rid of r8a73a4 legacy (Ulrich's patches should be ready) first, and possible sh73a0 and r8a7740 as well, and then retest our various timers configurations. We should test both CONFIG_PREEMPT and CONFIG_PREEMPT none, with all combination of the CMT, TMU, MTU2, TWD and ARM architected timer enabled.

Yes, getting rid of more legacy is good. It's just a pity for the Penguin on the Armadillo's LCD, which doesn't want to visit armadillo-multiplatform.

Signature · 3 lines
Regards,

Laurent Pinchart
#17 lore raw
In reply to Laurent Pinchart

Hi Laurent,

Quoted 28 lines · Laurent Pinchart

Hi Magnus and Geert,

Quoted 25 lines · On Wednesday 17 December 2014 10:30:33 Magnus Damm wrote:
Quoted 11 lines · Geert Uytterhoeven
Quoted 9 lines · Magnus Damm
Quoted 3 lines

Could you please confirm that you've tested both CONFIG_PREEMPT_NONE and CONFIG_PREEMPT with and without the ARM TWD times, and that you've booted to userspace and tested timer broadcast on all CPUs ?

No I have not. I've booted to user space in initramfs with DT-based TWD on Multiplatform for r8a7779. Without this fix (and other r8a7779 TWD bits) I see a lot of breakage. For instance, TWD and SMP boot is broken on r8a7779 - both legacy and non-legacy. I have not gotten to sh73a0 yet, but I assume it is busted too.

Quoted 7 lines

FWIW, I applied this patch, and booted the result successfully and without

any user-visible changes on:
- koelsch
- armadillo-multiplatform
- kzm9g-legacy
- kzm9g-multiplatform

Thanks for testing - now let us wait and see what Laurent says.

Quoted 3 lines

Armadillo-legacy is broken, and probably won't come back (cfr.
https://lkml.org/lkml/2014/12/2/339).

Quoted 7 lines
Quoted 2 lines
Quoted 2 lines

Kzm9g-reference still hangs at "Calibrating local timer..." (it did work at some point in the past).

kzm9g-reference boots for me with kzm9g_defconfig on Simon's devel branch with CONFIG_PREEMPT_NONE but fails to boot with CONFIG_PREEMPT. The platform doesn't instantiate the TMU at the moment anyway, so it's unrelated to this patch.

Ok, thanks for testing. That makes sense.

Quoted 4 lines

Given that kzm9g-legacy works for me regardless of the preempt configuration and on the TMU cpumask, that kzm9g-reference doesn't use TMU, and that you've tested kzm9g-multiplaform successfully, I think we can consider that this patch is safe from a kzm9g point of view.

I think so too.

Quoted 3 lines

(By the way, how have you tested kzm9g-multiplatform given that none of renesas-drivers-2014-12-08-v3.18, renesas-devel-20141212-v3.18 or today's upstream support multiplatform kernels for sh73a0 ?)

My recently posted patch set:
[PATCH v2 00/05] ARM: shmobile: sh73a0 and kzm9g Multiplatform revisit

Quoted 5 lines

Magnus, you have told me that you've performed tests on Marzen with CONFIG_PREEMPT_NONE and with both TWD enabled and disabled. If you could perform the same tests with CONFIG_PREEMPT instead without noticing any regression I think we can merge your patch. Whatever breakage it has caused in the past is likely hidden somewhere beyond eyesight.

Sorry, but I have not had any time to test this today. What I can tell from yesterday is that legacy Marzen seemed busted and without my patch(es) (including this one) multiplatform Marzen was busted as well.

I'll most likely be buried in paper work tomorrow, but I'll see if I can find time dealing with Marzen legacy tomorrow. In general I find legacy upkeep rather time consuming since it seems to degrade rather quickly, so I much rather spend time fixing up multiplatform.

Quoted 9 lines
Quoted 3 lines

When the time allows I'd like us to try to fix up these somehow. At least I'll give it a go - I guess Armadillo is difficult without any board.

I'd like that very much. Let's get rid of r8a73a4 legacy (Ulrich's patches should be ready) first, and possible sh73a0 and r8a7740 as well, and then retest our various timers configurations. We should test both CONFIG_PREEMPT and CONFIG_PREEMPT none, with all combination of the CMT, TMU, MTU2, TWD and ARM architected timer enabled.

Sure, those are the ones I usually care about. From my side I find the TWD and arch timer upkeep to be heavy enough as-is without considering the preempt case. But you're right - we should cover all of them.

Thanks,

/ magnus

#18 lore raw
In reply to Laurent Pinchart

Hi Laurent,

Quoted 5 lines · Laurent Pinchart

Magnus, you have told me that you've performed tests on Marzen with CONFIG_PREEMPT_NONE and with both TWD enabled and disabled. If you could perform the same tests with CONFIG_PREEMPT instead without noticing any regression I think we can merge your patch. Whatever breakage it has caused in the past is likely hidden somewhere beyond eyesight.

I've now successfully boot tested the different PREEMPT settings on r8a7779 Marzen multiplatform. Everything seems to work as expected.

Compared to before I've been a bit more lazy, so I've kept the timer config to default which means TWD and TMU enabled. The kernel tree is renesas-devel-20141218-v3.18 which is v3.18 plus local integration stuff including TWD DT. I've applied this patch too.

Thanks,

/ magnus

#19 lore raw

Hi Magnus,

Quoted 7 lines · On Friday 19 December 2014 09:03:00 Magnus Damm wrote:
Quoted 5 lines · Laurent Pinchart

Magnus, you have told me that you've performed tests on Marzen with CONFIG_PREEMPT_NONE and with both TWD enabled and disabled. If you could perform the same tests with CONFIG_PREEMPT instead without noticing any regression I think we can merge your patch. Whatever breakage it has caused in the past is likely hidden somewhere beyond eyesight.

Quoted 5 lines

I've now successfully boot tested the different PREEMPT settings on r8a7779 Marzen multiplatform. Everything seems to work as expected.

Compared to before I've been a bit more lazy, so I've kept the timer config to default which means TWD and TMU enabled.

If your test setup is still around could you quickly test PREEMPT + TMU without TWD ?

Quoted 2 lines

The kernel tree is renesas-devel-20141218-v3.18 which is v3.18 plus local integration stuff including TWD DT. I've applied this patch too.

Signature · 3 lines
Regards,

Laurent Pinchart
#20 lore raw
In reply to Magnus Damm

Hi Magnus,

Quoted 9 lines · Magnus Damm
Quoted 5 lines

Armadillo-legacy is broken, and probably won't come back (cfr.
https://lkml.org/lkml/2014/12/2/339).

Kzm9g-reference still hangs at "Calibrating local timer..." (it did work at some point in the past).

When the time allows I'd like us to try to fix up these somehow. At least I'll give it a go - I guess Armadillo is difficult without any board.

To fix Armadillo, we have to instantiate the GIC from C code, which is a step backwards.

Quoted 7 lines
Quoted 3 lines

Note that my local tree is based on renesas-drivers-2014-12-08-v3.18, renesas-devel-20141212-v3.18, and yesterday's upstream, with CCF, PM domain, TWD, and lots of WIP patches from the kitchen sink applied.

The TWD portion above - is it driver code or integration stuff? If the latter - is there any reason why these can't be picked up by Simon and merged right away.

Integration stuff. Basically it's
http://www.spinics.net/lists/arm-kernel/msg383413.html

In the mean time, I found the right clock, but I still have to repost that (sorry, recovering from being ill --- merge windows don't help much ;-)

Quoted 1 line

My plan is to dig into sh73a0 (maybe including TWD) later today.

Okay, thanks!

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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

#21 lore raw
In reply to Magnus Damm

Hi Magnus,

Quoted 36 lines · On Tuesday 16 December 2014 20:46:33 Magnus Damm wrote:
Quoted 20 lines · Laurent Pinchart
Quoted 9 lines · On Tuesday 16 December 2014 12:14:40 Daniel Lezcano wrote:
Quoted 6 lines · Magnus Damm

From: Magnus Damm <[email protected]>

Update the TMU driver to use cpu_possible_mask as cpumask to make r8a7779 SMP work as expected with or without the ARM TWD timer.

Applied as a 3.18 fix.

You're a bit too fast, I haven't had time to review the patch yet.

Quoted 7 lines

ps: May I suggest to use the CLOCK_EVT_FEAT_DYNIRQ flag for this driver ?

Quoted 5 lines

---

  drivers/clocksource/sh_tmu.c |    2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
Quoted 14 lines
Quoted 9 lines

drivers/clocksource/sh_tmu.c 2014-12-16

Quoted 9 lines
drivers/clocksource/sh_tmu.c 2014-12-16 renamed from drivers/clocksource/sh_tmu.c +0 -0

17:49:49.000000000 +0900 @@ -428,7 +428,7 @@ static void sh_tmu_register_clockevent(s

    ced->features = CLOCK_EVT_FEAT_PERIODIC;
    ced->features |= CLOCK_EVT_FEAT_ONESHOT;
    ced->rating = 200;
drivers/clocksource/sh_tmu.c 2014-12-16 renamed from drivers/clocksource/sh_tmu.c +1 -1
-   ced->cpumask = cpumask_of(0);
+   ced->cpumask = cpu_possible_mask;

Magnus, how thoroughly have you tested this ? The TMU is indeed usable by all CPUs, so setting the CPU mask to cpu_possible_mask makes sense, but last time I've tried that it broke the broadcast timer due to the heuristics used by the clock events core code.

Quoted 8 lines

Uhm, so I've tested this particular patch on r8a7779 but I do agree that the TMU is used on a bunch of SoCs if that's what you mean. I don't see how it is different from any other of our timers though, and those got fixed like this earlier.

I wonder if you may recall an earlier issue with incorrect clock event priorities and code somehow working-by-accident without the mask set as expected?

As discussed privately today, I've had this exact same patch in one of my private branches, and I haven't pushed it upstream at the same time as the similar cmt and mtu2 patches due to a regression. Unfortunately I don't remember the details :-/

I'll test the patch on Marzen and KZM9G. I unfortunately can't test it on Armadillo as I don't have access to that board anymore.

Quoted 4 lines
Quoted 3 lines

Could you please confirm that you've tested both CONFIG_PREEMPT_NONE and CONFIG_PREEMPT with and without the ARM TWD times, and that you've booted to userspace and tested timer broadcast on all CPUs ?

Quoted 8 lines

No I have not. I've booted to user space in initramfs with DT-based TWD on Multiplatform for r8a7779. Without this fix (and other r8a7779 TWD bits) I see a lot of breakage. For instance, TWD and SMP boot is broken on r8a7779 - both legacy and non-legacy. I have not gotten to sh73a0 yet, but I assume it is busted too.

Can you please explain to me how the TMU is any different compared to the CMT, MTU2 or STI? =)

I don't know yet :-) I'll hopefully have more information after testing on KZM9G.

Quoted 1 line

And no, I don't have any r8a7740 board anymore. Can anyone else test?

Signature · 3 lines
Regards,

Laurent Pinchart
#22 lore raw
In reply to Magnus Damm

Hi Laurent,

[reduced cc-list]

Quoted 16 lines · Laurent Pinchart
Quoted 16 lines
Quoted 4 lines

What are we missing ?

- The DIV6 multiparent series has been merged by Mike, and the sh73a0 CCF is nearly ready.

Yep.

Quoted 3 lines

- We need proper BSC support to get LAN working, although it could be worked around temporarily by specifying the BSC clock in the LAN node if I recall correctly.

Working on that. I hope to send out v2 later today...

Quoted 2 lines

- Accelerometer and RTC have no DT bindings, but they should work with the I2C core OF match support.

Quoted 6 lines
Quoted 1 line

I have RTC in my DTS, and it works.

Great.

I can help with at least some of the remaining pieces if you share your working branch.

I pushed my local working branch to
https://git.kernel.org/cgit/linux/kernel/git/geert/renesas-drivers.git/log/?h=wip

Please wear sun glasses, and keep the kbuild test robot away ;-)

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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

#23 lore raw
In reply to Magnus Damm

Hi Geert,

Quoted 21 lines · On Wednesday 17 December 2014 14:59:21 Geert Uytterhoeven wrote:
Quoted 20 lines · Laurent Pinchart
Quoted 11 lines
Quoted 4 lines

What are we missing ?

- The DIV6 multiparent series has been merged by Mike, and the sh73a0 CCF is nearly ready.

Yep.

Quoted 3 lines

- We need proper BSC support to get LAN working, although it could be worked around temporarily by specifying the BSC clock in the LAN node if I recall correctly.

Quoted 6 lines

Working on that. I hope to send out v2 later today...

Quoted 2 lines

- Accelerometer and RTC have no DT bindings, but they should work with the I2C core OF match support.

I have RTC in my DTS, and it works.

Great.

Quoted 8 lines
Quoted 2 lines

I can help with at least some of the remaining pieces if you share your working branch.

I pushed my local working branch to
https://git.kernel.org/cgit/linux/kernel/git/geert/renesas-drivers.git/log/?
h=wip

Please wear sun glasses, and keep the kbuild test robot away ;-)

I will :-) Thank you. Is there a way to selectively keep kbuild away from branches by the way ?

Do avoid duplicating work, what do you plan to work on in the context of multiplatform kzm9g ?

Magnus, how much time do you think would be worth spending on DT support for the lcdc drm driver ?

Signature · 3 lines
Regards,

Laurent Pinchart
#24 lore raw
In reply to Magnus Damm

Hi Geert,

Quoted 13 lines · On Wednesday 17 December 2014 16:43:09 Laurent Pinchart wrote:
Quoted 12 lines · On Wednesday 17 December 2014 14:59:21 Geert Uytterhoeven wrote:
Quoted 11 lines · Laurent Pinchart
Quoted 11 lines
Quoted 4 lines

What are we missing ?

- The DIV6 multiparent series has been merged by Mike, and the sh73a0 CCF is nearly ready.

Yep.

Quoted 3 lines

- We need proper BSC support to get LAN working, although it could be worked around temporarily by specifying the BSC clock in the LAN node if I recall correctly.

Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines

Working on that. I hope to send out v2 later today...

Quoted 2 lines

- Accelerometer and RTC have no DT bindings, but they should work with the I2C core OF match support.

I have RTC in my DTS, and it works.

I'll test compass, touch screen and accelerometer.

Quoted 19 lines
Quoted 10 lines
Quoted 4 lines

Great.

I can help with at least some of the remaining pieces if you share your working branch.

I pushed my local working branch to
https://git.kernel.org/cgit/linux/kernel/git/geert/renesas-drivers.git/log
/? h=wip

Please wear sun glasses, and keep the kbuild test robot away ;-)

I will :-) Thank you. Is there a way to selectively keep kbuild away from branches by the way ?

Do avoid duplicating work, what do you plan to work on in the context of multiplatform kzm9g ?

Magnus, how much time do you think would be worth spending on DT support for the lcdc drm driver ?

Signature · 3 lines
Regards,

Laurent Pinchart
#25 lore raw
In reply to Magnus Damm

Hi Laurent,

Quoted 6 lines · Laurent Pinchart
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 2 lines

- Accelerometer and RTC have no DT bindings, but they should work with the I2C core OF match support.

I have RTC in my DTS, and it works.

I'll test compass, touch screen and accelerometer.

Thanks!

Getting the accelerometer to work probably needs some changes to the driver. On both legacy and multi-platform, reading from /dev/input/event1 causes an interrupt storm, which can only be stopped by power-cycling the board (the adxl345 doesn't have a reset line).

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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

#26 lore raw
In reply to Magnus Damm

Hi Laurent,

Quoted 33 lines · Laurent Pinchart

Hi Geert,

Quoted 21 lines · On Wednesday 17 December 2014 14:59:21 Geert Uytterhoeven wrote:
Quoted 20 lines · Laurent Pinchart
Quoted 11 lines
Quoted 4 lines

What are we missing ?

- The DIV6 multiparent series has been merged by Mike, and the sh73a0 CCF is nearly ready.

Yep.

Quoted 3 lines

- We need proper BSC support to get LAN working, although it could be worked around temporarily by specifying the BSC clock in the LAN node if I recall correctly.

Quoted 6 lines

Working on that. I hope to send out v2 later today...

Quoted 2 lines

- Accelerometer and RTC have no DT bindings, but they should work with the I2C core OF match support.

I have RTC in my DTS, and it works.

Great.

Quoted 8 lines
Quoted 2 lines

I can help with at least some of the remaining pieces if you share your working branch.

I pushed my local working branch to
https://git.kernel.org/cgit/linux/kernel/git/geert/renesas-drivers.git/log/?
h=wip

Please wear sun glasses, and keep the kbuild test robot away ;-)

Quoted 8 lines

I will :-) Thank you. Is there a way to selectively keep kbuild away from branches by the way ?

Do avoid duplicating work, what do you plan to work on in the context of multiplatform kzm9g ?

Magnus, how much time do you think would be worth spending on DT support for the lcdc drm driver ?

Not much, but perhaps it is good to have a working separate reference implementation if it can be done with little effort.

I guess if you can re-use the LCDC driver between sh73a0 and r8a7740 then there may be _some_ point, but unless I'm mistaken the sh73a0 has the LCDC interrupt only routed to INTCS which in turn lacks DT support. Not sure if r8a7740 is any better - at least it has the GIC together with the Cortex-A9.

Cheers,

/ magnus

#27 lore raw
In reply to Magnus Damm

Hi Magnus,

Quoted 38 lines · On Thursday 18 December 2014 00:46:59 Magnus Damm wrote:
Quoted 37 lines · Laurent Pinchart
Quoted 21 lines · On Wednesday 17 December 2014 14:59:21 Geert Uytterhoeven wrote:
Quoted 20 lines · Laurent Pinchart
Quoted 11 lines
Quoted 4 lines

What are we missing ?

- The DIV6 multiparent series has been merged by Mike, and the sh73a0 CCF is nearly ready.

Yep.

Quoted 3 lines

- We need proper BSC support to get LAN working, although it could be worked around temporarily by specifying the BSC clock in the LAN node if I recall correctly.

Quoted 6 lines

Working on that. I hope to send out v2 later today...

Quoted 2 lines

- Accelerometer and RTC have no DT bindings, but they should work with the I2C core OF match support.

I have RTC in my DTS, and it works.

Great.

Quoted 8 lines
Quoted 2 lines

I can help with at least some of the remaining pieces if you share your working branch.

I pushed my local working branch to
https://git.kernel.org/cgit/linux/kernel/git/geert/renesas-drivers.git/lo
g/? h=wip

Please wear sun glasses, and keep the kbuild test robot away ;-)

I will :-) Thank you. Is there a way to selectively keep kbuild away from branches by the way ?

Do avoid duplicating work, what do you plan to work on in the context of multiplatform kzm9g ?

Quoted 8 lines
Quoted 2 lines

Magnus, how much time do you think would be worth spending on DT support for the lcdc drm driver ?

Not much, but perhaps it is good to have a working separate reference implementation if it can be done with little effort.

I guess if you can re-use the LCDC driver between sh73a0 and r8a7740 then there may be _some_ point,

The driver should be usable on both platforms, yes.

Quoted 2 lines

but unless I'm mistaken the sh73a0 has the LCDC interrupt only routed to INTCS which in turn lacks DT support.

You're correct :-( It shouldn't be difficult to implement a proper INTCS irqchip driver. As usual, it's a matter of time. I'm afraid I won't be able to tackle that in the very near future.

Quoted 2 lines

Not sure if r8a7740 is any better - at least it has the GIC together with the Cortex-A9.

Signature · 3 lines
Regards,

Laurent Pinchart