This is the mail archive of the ecos-patches@sourceware.org mailing list for the eCos project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: "Harmless" bugs in AT91 PIT code


On Wed, Jan 24, 2007 at 05:15:22PM -0800, Jim Seymour wrote:
> New mailing list subscriber here, so please forgive me if I've muddled 
> up the patch submission procedure...
> 
> The AT91 Periodic Interval Timer module has two "mostly harmless" bugs 
> that the attached patch corrects.  This patch is for:
>    .../packages/hal/arm/at91/var/current/src/timer_pit.c
> 
> First, in hal_clock_initialize(), "period" is stored directly in the 
> PIT_MR register.  It should be "(period - 1)".  This error is corrected 
> by the first call to hal_clock_reset(), but it still should be set up 
> correctly the first time.
> 
> Second, there are four places where the PIV field in PIT_MR is masked 
> with 0xffffff (24 bits), instead of 0xfffff (20 bits).  This has no ill 
> effect that I can see - except that the assert in hal_clock_reset() will 
> not get triggered for certain illegal values.

Hi Jim

Please could you test this patch. I decided to add a mask #define to
var_io.h to replace the hard coded numbers. So it should now be right
(or wrong) everywhere.

    Thanks
        Andrew
Index: hal/arm/at91/var/current/ChangeLog
===================================================================
RCS file: /cvs/ecos/ecos/packages/hal/arm/at91/var/current/ChangeLog,v
retrieving revision 1.38
diff -u -r1.38 ChangeLog
--- hal/arm/at91/var/current/ChangeLog	9 Sep 2006 13:26:22 -0000	1.38
+++ hal/arm/at91/var/current/ChangeLog	25 Jan 2007 08:37:51 -0000
@@ -1,3 +1,12 @@
+2007-01-25  Andrew Lunn  <andrew.lunn@ascom.ch>
+
+	* include/var_io.h (AT91_PITC_VALUE_MASK): New - mask to access
+	the PITC value which is a 20 bit number.
+	* src/timer_pit.c: Change all hard coded mask for the period, 
+	some of which were wrong, to use AT91_PITC_VALUE_MASK. 
+	When initializing the PIT, remember to decrement the period first.
+	Bugs found by Jim Seymour.
+
 2006-09-08  John Eigelaar <jeigelaar@mweb.co.za>
 
 	* include/var_io.h: Added definition for SPI MODFDIS bit 
Index: hal/arm/at91/var/current/include/var_io.h
===================================================================
RCS file: /cvs/ecos/ecos/packages/hal/arm/at91/var/current/include/var_io.h,v
retrieving revision 1.18
diff -u -r1.18 var_io.h
--- hal/arm/at91/var/current/include/var_io.h	9 Sep 2006 13:26:22 -0000	1.18
+++ hal/arm/at91/var/current/include/var_io.h	25 Jan 2007 08:37:53 -0000
@@ -1618,6 +1618,7 @@
 #define AT91_PITC_PISR_PITS   (1 << 0)  // Periodic Interval Timer Status
 #define AT91_PITC_PIVR 0x08  // Period Interval Status Register
 #define AT91_PITC_PIIR 0x0C  // Period Interval Image Register
+#define AT91_PITC_VALUE_MASK 0x000fffff  // 20-bit period value
 #endif
 
 //=============================================================================
Index: hal/arm/at91/var/current/src/timer_pit.c
===================================================================
RCS file: /cvs/ecos/ecos/packages/hal/arm/at91/var/current/src/timer_pit.c,v
retrieving revision 1.3
diff -u -r1.3 timer_pit.c
--- hal/arm/at91/var/current/src/timer_pit.c	26 Mar 2006 11:02:59 -0000	1.3
+++ hal/arm/at91/var/current/src/timer_pit.c	25 Jan 2007 08:37:53 -0000
@@ -66,7 +66,7 @@
   
   /* Set Period Interval timer and enable interrupt */
   HAL_WRITE_UINT32((AT91_PITC + AT91_PITC_PIMR), 
-                   period |  
+                   (period - 1) |  
                    AT91_PITC_PIMR_PITEN |
                    AT91_PITC_PIMR_PITIEN);
   
@@ -81,11 +81,11 @@
   cyg_uint32 reg;
   cyg_uint32 pimr;
   
-  CYG_ASSERT(period < 0xffffff, "Invalid HAL clock configuration");
+  CYG_ASSERT(period < AT91_PITC_VALUE_MASK, "Invalid HAL clock configuration");
   
   // Check that the PIT has the right period.
   HAL_READ_UINT32((AT91_PITC + AT91_PITC_PIMR), pimr);
-  if ((pimr & 0xffffff) != (period - 1)){
+  if ((pimr & AT91_PITC_VALUE_MASK) != (period - 1)) {
     HAL_WRITE_UINT32((AT91_PITC + AT91_PITC_PIMR), 
                      (period - 1) |  
                      AT91_PITC_PIMR_PITEN |
@@ -109,11 +109,11 @@
   HAL_READ_UINT32((AT91_PITC + AT91_PITC_PIMR),pimr);
   if (!(pimr & AT91_PITC_PIMR_PITEN)) {
     HAL_WRITE_UINT32((AT91_PITC + AT91_PITC_PIMR), 
-                     0xffffff | AT91_PITC_PIMR_PITEN);
+                     AT91_PITC_VALUE_MASK | AT91_PITC_PIMR_PITEN);
   }
   
   HAL_READ_UINT32(AT91_PITC + AT91_PITC_PIIR, ir);
-  *pvalue = ir & 0xfffff;
+  *pvalue = ir & AT91_PITC_VALUE_MASK;
 }
 
 // -------------------------------------------------------------------------
@@ -135,7 +135,7 @@
   
   // Calculate the wrap around period. 
   HAL_READ_UINT32(AT91_PITC + AT91_PITC_PIMR, piv);
-  piv = (piv & 0xffffff) - 1; 
+  piv = (piv & AT91_PITC_VALUE_MASK) - 1; 
   
   hal_clock_read(&val1);
   while (ticks > 0) {

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]