Fix imx time reprogramming issue

PATCH In review Michael Trimarchi 2 participants 9 messages Apr 24, 2012 – Apr 25, 2012

Subsystems (3)

Arm (32-bit) Arm sub-architectures Arm i.MX and Layerscape

lore

Share & tools

lore

Michael Trimarchi

author

2012-04-24 08:09 UTC #0 lore raw

PATCH 3/3 Fix imx time reprogramming issue

This patch includes:

* Fix invalid virtual address base of MX1_2_TCM
* Fix the minimum delay below which the hardware timer can not be reprogrammed. The value is the same of that one that is used to calculate the min_delta_ns

arch/arm/plat-mxc/time.c
414: clockevent_mxc.min_delta_ns = clockevent_delta2ns(0xff, &clockevent_mxc);

Signed-off-by Michael Trimarchi <[email protected]>

Signed-off-by Bruno Morelli <[email protected]>

---
 arch/arm/plat-mxc/time.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

arch/arm/plat-mxc/time.c +2 -2

index aae2717..30916f8 100644
@@ -400,11 +400,11 @@ mxc_timer_init(struct clk *timer_clk,
 	__ipipe_mach_timerint = irq;
 	__ipipe_mach_ticks_per_jiffy = (clk_get_rate(timer_clk) + HZ/2) / HZ;
 	tsc_info.freq = clk_get_rate(timer_clk);
-	mxc_min_delay = ((__ipipe_cpu_freq + 500000) / 1000000) ?: 1;
+	mxc_min_delay = 0xff;
 
 	if (timer_is_v1()) {
 		tsc_info.u.counter_paddr = phys + MX1_2_TCN;
-		tsc_info.counter_vaddr =(unsigned long)(phys + MX1_2_TCN);
+		tsc_info.counter_vaddr = (unsigned long)(timer_base + MX1_2_TCN);
 	} else {
 		tsc_info.u.counter_paddr = phys + V2_TCN;
 		tsc_info.counter_vaddr = (unsigned long)(timer_base + V2_TCN);

Signature · 6 lines

1.7.5.4

-- 
| Michael Nazzareno Trimarchi                     Amarula Solutions BV |
| COO  -  Founder                                      Cruquiuskade 47 |
| +31(0)851119172                                 Amsterdam 1018 AM NL |

Gilles Chanteperdrix

2012-04-24 08:13 UTC #1 lore raw

Quoted 8 lines · Michael Trimarchi

This patch includes:
* Fix invalid virtual address base of MX1_2_TCM
* Fix the minimum delay below which the hardware timer can not be reprogrammed. The value is the same of that one that is used to calculate the min_delta_ns
arch/arm/plat-mxc/time.c
414: clockevent_mxc.min_delta_ns = clockevent_delta2ns(0xff, &clockevent_mxc);

I do not think we can use this value, it is way to high for xenomai needs. In latest releases we use 2us instead of 1us, and according to what a user posted recently, this should be enough, could you test with 2us? And 2us is already what we have in the 3.2 repository.

Signature · 1 line

                                                                Gilles.

Michael Trimarchi

author

2012-04-24 08:32 UTC #2 lore raw

Quoted 7 lines · Gilles Chanteperdrix

Quoted 6 lines · Michael Trimarchi
This patch includes:
* Fix invalid virtual address base of MX1_2_TCM
* Fix the minimum delay below which the hardware timer can not be reprogrammed. The value is the same of that one that is used to calculate the min_delta_ns

Quoted 8 lines

Quoted 2 lines
arch/arm/plat-mxc/time.c
414: clockevent_mxc.min_delta_ns = clockevent_delta2ns(0xff, &clockevent_mxc);
I do not think we can use this value, it is way to high for xenomai needs. In latest releases we use 2us instead of 1us, and according to what a user posted recently, this should be enough, could you test with 2us? And 2us is already what we have in the 3.2 repository.

As I understood the min delays is in tick and not depend on the clock rate.

This is from atmel at91_ipipe_time

min_delta_ticks = ((unsigned long long) clkevt.min_delta_ns
* clkevt.mult) >> clkevt.shift;

this is exaclty the reverse calculation of clockevent_mxc.min_delta_ns

I have already the result in the previous calculation: 0xff is the minimun in ticks

Michael

Gilles Chanteperdrix

2012-04-24 08:53 UTC #3 lore raw

Quoted 18 lines · Michael Trimarchi

Quoted 7 lines · Gilles Chanteperdrix
Quoted 6 lines · Michael Trimarchi
This patch includes:
* Fix invalid virtual address base of MX1_2_TCM
* Fix the minimum delay below which the hardware timer can not be reprogrammed. The value is the same of that one that is used to calculate the min_delta_ns
Quoted 8 lines
Quoted 2 lines
arch/arm/plat-mxc/time.c
414: clockevent_mxc.min_delta_ns = clockevent_delta2ns(0xff, &clockevent_mxc);
I do not think we can use this value, it is way to high for xenomai needs. In latest releases we use 2us instead of 1us, and according to what a user posted recently, this should be enough, could you test with 2us? And 2us is already what we have in the 3.2 repository.
As I understood the min delays is in tick and not depend on the clock rate.

Quoted 8 lines

This is from atmel at91_ipipe_time
min_delta_ticks = ((unsigned long long) clkevt.min_delta_ns
* clkevt.mult) >> clkevt.shift;
this is exaclty the reverse calculation of clockevent_mxc.min_delta_ns
I have already the result in the previous calculation: 0xff is the minimun in ticks

0xff is too large, it corresponds to 30us on some platforms, could you please try with 2us? The delay is given as a count of ticks or as a duration in ns depending on the architecture, it does not really matter.

Quoted 3 lines

Michael

Signature · 1 line

                                                                Gilles.

Michael Trimarchi

author

2012-04-24 09:31 UTC #4 lore raw

Quoted 24 lines · Gilles Chanteperdrix

Quoted 23 lines · Michael Trimarchi
Quoted 7 lines · Gilles Chanteperdrix
Quoted 6 lines · Michael Trimarchi
This patch includes:
* Fix invalid virtual address base of MX1_2_TCM
* Fix the minimum delay below which the hardware timer can not be reprogrammed. The value is the same of that one that is used to calculate the min_delta_ns
Quoted 8 lines
Quoted 2 lines
arch/arm/plat-mxc/time.c
414: clockevent_mxc.min_delta_ns = clockevent_delta2ns(0xff, &clockevent_mxc);
I do not think we can use this value, it is way to high for xenomai needs. In latest releases we use 2us instead of 1us, and according to what a user posted recently, this should be enough, could you test with 2us? And 2us is already what we have in the 3.2 repository.
As I understood the min delays is in tick and not depend on the clock rate.
This is from atmel at91_ipipe_time
min_delta_ticks = ((unsigned long long) clkevt.min_delta_ns
* clkevt.mult) >> clkevt.shift;

Quoted 8 lines

Quoted 3 lines
this is exaclty the reverse calculation of clockevent_mxc.min_delta_ns
I have already the result in the previous calculation: 0xff is the minimun in ticks
0xff is too large, it corresponds to 30us on some platforms, could you please try with 2us? The delay is given as a count of ticks or as a duration in ns depending on the architecture, it does not really matter.

Ok, with 0x85 is working (2 uS). I don't find in the documentation the min_delta_ns for this architecture. I will rebase the patch on core-3.2

Michael

Quoted 5 lines

Quoted 3 lines
Michael

Signature · 3 lines

| Michael Nazzareno Trimarchi                     Amarula Solutions BV |
| COO  -  Founder                                      Cruquiuskade 47 |
| +31(0)851119172                                 Amsterdam 1018 AM NL |

Gilles Chanteperdrix

2012-04-24 09:41 UTC #5 lore raw

Quoted 42 lines · Michael Trimarchi

Hi
Quoted 30 lines · Gilles Chanteperdrix
Quoted 29 lines · Michael Trimarchi
Quoted 8 lines · Gilles Chanteperdrix
Quoted 7 lines · Michael Trimarchi
This patch includes:
* Fix invalid virtual address base of MX1_2_TCM * Fix the minimum delay below which the hardware timer can not be reprogrammed. The value is the same of that one that is used to calculate the min_delta_ns
Quoted 9 lines
Quoted 2 lines
arch/arm/plat-mxc/time.c 414: clockevent_mxc.min_delta_ns = clockevent_delta2ns(0xff, &clockevent_mxc);
I do not think we can use this value, it is way to high for xenomai needs. In latest releases we use 2us instead of 1us, and according to what a user posted recently, this should be enough, could you test with 2us? And 2us is already what we have in the 3.2 repository.
As I understood the min delays is in tick and not depend on the clock rate.
This is from atmel at91_ipipe_time
min_delta_ticks = ((unsigned long long) clkevt.min_delta_ns * clkevt.mult) >> clkevt.shift;
this is exaclty the reverse calculation of
clockevent_mxc.min_delta_ns
Quoted 8 lines
Quoted 2 lines
I have already the result in the previous calculation: 0xff is the minimun in ticks
0xff is too large, it corresponds to 30us on some platforms, could you please try with 2us? The delay is given as a count of ticks or as a duration in ns depending on the architecture, it does not really matter.

Quoted 3 lines

Ok, with 0x85 is working (2 uS). I don't find in the documentation the min_delta_ns for this architecture. I will rebase the patch on core-3.2

This is not really a specified constant, finding this minimum value is more of a hit and miss. Anyway, on core 3.2, the timers have been substantially reworked, and we are able to reuse the clockevent callback which has a safety mechanism in addition to the minimum value to avoid loosing ticks (in short, the timer is re-read after programming it, and if the delay has already passed, the callback returns a negative value, which the timer subsystem uses to trigger the irq).

Core-3.2 is not yet supported on ARM for xenomai 2.6. I will do the changes and keep you informed.

Signature · 1 line

					    Gilles.

Gilles Chanteperdrix

2012-04-24 20:34 UTC #6 lore raw

Quoted 55 lines · Gilles Chanteperdrix

Quoted 42 lines · Michael Trimarchi
Hi
Quoted 30 lines · Gilles Chanteperdrix
Quoted 29 lines · Michael Trimarchi
Quoted 8 lines · Gilles Chanteperdrix
Quoted 7 lines · Michael Trimarchi
This patch includes:
* Fix invalid virtual address base of MX1_2_TCM * Fix the minimum delay below which the hardware timer can not be reprogrammed. The value is the same of that one that is used to calculate the min_delta_ns
Quoted 9 lines
Quoted 2 lines
arch/arm/plat-mxc/time.c 414: clockevent_mxc.min_delta_ns = clockevent_delta2ns(0xff, &clockevent_mxc);
I do not think we can use this value, it is way to high for xenomai needs. In latest releases we use 2us instead of 1us, and according to what a user posted recently, this should be enough, could you test with 2us? And 2us is already what we have in the 3.2 repository.
As I understood the min delays is in tick and not depend on the clock rate.
This is from atmel at91_ipipe_time
min_delta_ticks = ((unsigned long long) clkevt.min_delta_ns * clkevt.mult) >> clkevt.shift;
this is exaclty the reverse calculation of
clockevent_mxc.min_delta_ns
Quoted 8 lines
Quoted 2 lines
I have already the result in the previous calculation: 0xff is the minimun in ticks
0xff is too large, it corresponds to 30us on some platforms, could you please try with 2us? The delay is given as a count of ticks or as a duration in ns depending on the architecture, it does not really matter.
Quoted 3 lines
Ok, with 0x85 is working (2 uS). I don't find in the documentation the min_delta_ns for this architecture. I will rebase the patch on core-3.2
This is not really a specified constant, finding this minimum value is more of a hit and miss. Anyway, on core 3.2, the timers have been substantially reworked, and we are able to reuse the clockevent callback which has a safety mechanism in addition to the minimum value to avoid loosing ticks (in short, the timer is re-read after programming it, and if the delay has already passed, the callback returns a negative value, which the timer subsystem uses to trigger the irq).

Quoted 3 lines

Core-3.2 is not yet supported on ARM for xenomai 2.6. I will do the changes and keep you informed.

Ok, I have just pushed the changes to the 2.6 branch repository to support I-pipe core on ARM.

So, the repository for xenomai is:
git://git.xenomai.org/xenoami-2.6.git branch master and the repository for the core-3.2 ipipe series is:
git://git.denx.de/ipipe.git branch core-3.2

Signature · 1 line

                                                                Gilles.

Michael Trimarchi

author

2012-04-25 08:12 UTC #7 lore raw

Hi Gilles

Quoted 59 lines · Gilles Chanteperdrix

Quoted 55 lines · Gilles Chanteperdrix
Quoted 42 lines · Michael Trimarchi
Hi
Quoted 30 lines · Gilles Chanteperdrix
Quoted 29 lines · Michael Trimarchi
Quoted 8 lines · Gilles Chanteperdrix
Quoted 7 lines · Michael Trimarchi
This patch includes:
* Fix invalid virtual address base of MX1_2_TCM * Fix the minimum delay below which the hardware timer can not be reprogrammed. The value is the same of that one that is used to calculate the min_delta_ns
Quoted 9 lines
Quoted 2 lines
arch/arm/plat-mxc/time.c 414: clockevent_mxc.min_delta_ns = clockevent_delta2ns(0xff, &clockevent_mxc);
I do not think we can use this value, it is way to high for xenomai needs. In latest releases we use 2us instead of 1us, and according to what a user posted recently, this should be enough, could you test with 2us? And 2us is already what we have in the 3.2 repository.
As I understood the min delays is in tick and not depend on the clock rate.
This is from atmel at91_ipipe_time
min_delta_ticks = ((unsigned long long) clkevt.min_delta_ns * clkevt.mult) >> clkevt.shift;
this is exaclty the reverse calculation of
clockevent_mxc.min_delta_ns
Quoted 8 lines
Quoted 2 lines
I have already the result in the previous calculation: 0xff is the minimun in ticks
0xff is too large, it corresponds to 30us on some platforms, could you please try with 2us? The delay is given as a count of ticks or as a duration in ns depending on the architecture, it does not really matter.
Quoted 3 lines
Ok, with 0x85 is working (2 uS). I don't find in the documentation the min_delta_ns for this architecture. I will rebase the patch on core-3.2
This is not really a specified constant, finding this minimum value is more of a hit and miss. Anyway, on core 3.2, the timers have been substantially reworked, and we are able to reuse the clockevent callback which has a safety mechanism in addition to the minimum value to avoid loosing ticks (in short, the timer is re-read after programming it, and if the delay has already passed, the callback returns a negative value, which the timer subsystem uses to trigger the irq).
Quoted 3 lines
Core-3.2 is not yet supported on ARM for xenomai 2.6. I will do the changes and keep you informed.

Quoted 8 lines

Ok, I have just pushed the changes to the 2.6 branch repository to support I-pipe core on ARM.
So, the repository for xenomai is:
git://git.xenomai.org/xenoami-2.6.git branch master and the repository for the core-3.2 ipipe series is:
git://git.denx.de/ipipe.git branch core-3.2

Thanks
I will rebase the the embedded board on top of the ipipe core-3.2 so I can test again the ipipe layer.

Michael

Gilles Chanteperdrix

2012-04-25 08:18 UTC #8 lore raw

Quoted 71 lines · Michael Trimarchi

Hi Gilles
Quoted 59 lines · Gilles Chanteperdrix
Quoted 55 lines · Gilles Chanteperdrix
Quoted 42 lines · Michael Trimarchi
Hi
Quoted 30 lines · Gilles Chanteperdrix
Quoted 29 lines · Michael Trimarchi
Quoted 8 lines · Gilles Chanteperdrix
Quoted 7 lines · Michael Trimarchi
This patch includes:
* Fix invalid virtual address base of MX1_2_TCM * Fix the minimum delay below which the hardware timer can not be reprogrammed. The value is the same of that one that is used to calculate the min_delta_ns
Quoted 9 lines
Quoted 2 lines
arch/arm/plat-mxc/time.c 414: clockevent_mxc.min_delta_ns = clockevent_delta2ns(0xff, &clockevent_mxc);
I do not think we can use this value, it is way to high for xenomai needs. In latest releases we use 2us instead of 1us, and according to what a user posted recently, this should be enough, could you test with 2us? And 2us is already what we have in the 3.2 repository.
As I understood the min delays is in tick and not depend on the clock rate.
This is from atmel at91_ipipe_time
min_delta_ticks = ((unsigned long long) clkevt.min_delta_ns * clkevt.mult) >> clkevt.shift;
this is exaclty the reverse calculation of
clockevent_mxc.min_delta_ns
Quoted 8 lines
Quoted 2 lines
I have already the result in the previous calculation: 0xff is the minimun in ticks
0xff is too large, it corresponds to 30us on some platforms, could you please try with 2us? The delay is given as a count of ticks or as a duration in ns depending on the architecture, it does not really matter.
Quoted 3 lines
Ok, with 0x85 is working (2 uS). I don't find in the documentation the min_delta_ns for this architecture. I will rebase the patch on core-3.2
This is not really a specified constant, finding this minimum value is more of a hit and miss. Anyway, on core 3.2, the timers have been substantially reworked, and we are able to reuse the clockevent callback which has a safety mechanism in addition to the minimum value to avoid loosing ticks (in short, the timer is re-read after programming it, and if the delay has already passed, the callback returns a negative value, which the timer subsystem uses to trigger the irq).
Quoted 3 lines
Core-3.2 is not yet supported on ARM for xenomai 2.6. I will do the changes and keep you informed.
Quoted 8 lines
Ok, I have just pushed the changes to the 2.6 branch repository to support I-pipe core on ARM.
So, the repository for xenomai is:
git://git.xenomai.org/xenoami-2.6.git branch master and the repository for the core-3.2 ipipe series is:
git://git.denx.de/ipipe.git branch core-3.2

Quoted 3 lines

Thanks
I will rebase the the embedded board on top of the ipipe core-3.2 so I can test again the ipipe layer.

Ok. In fact, please use the ipipe-core from this branch:
git://git.xenomai.org/ipipe-gch.git branch for-ipipe-3.2-arm, it is generally more up to date for arm (in this case, it contains some fixes for imx).

Quoted 3 lines

Michael

Signature · 1 line

                                                                Gilles.