This is the mail archive of the
ecos-patches@sourceware.org
mailing list for the eCos project.
Re: "Harmless" bugs in AT91 PIT code
- From: Andrew Lunn <andrew at lunn dot ch>
- To: Jim Seymour <jim at cipher dot com>
- Cc: ecos-patches at ecos dot sourceware dot org
- Date: Thu, 25 Jan 2007 09:39:47 +0100
- Subject: Re: "Harmless" bugs in AT91 PIT code
- References: <45B804AA.6050902@cipher.com>
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) {