[U-Boot] [PATCH] nios2: Set CONFIG_SYS_HZ to 1000 all nios2 boards.

[U-Boot] [PATCH] nios2: Set CONFIG_SYS_HZ to 1000 all nios2 boards.

[Scott McNutt]

   CONFIG_SYS_HZ was being calculated (incorrectly) in nios2 configuration
   headers. Updated comments to accurately describe timebase macros.

Signed-off-by: Scott McNutt <smcnutt@psyent.com>
---
 include/configs/EP1C20.h  |   14 ++++++++------
 include/configs/EP1S10.h  |   14 ++++++++------
 include/configs/EP1S40.h  |   14 ++++++++------
 include/configs/PCI5441.h |   14 ++++++++------
 include/configs/PK1C20.h  |   14 ++++++++------
 5 files changed, 40 insertions(+), 30 deletions(-)

diff --git a/include/configs/EP1C20.h b/include/configs/EP1C20.h
index dc82e54..3920d35 100644
--- a/include/configs/EP1C20.h
+++ b/include/configs/EP1C20.h
@@ -124,14 +124,16 @@
  * TIMEBASE --
  *
  * The high res timer defaults to 1 msec. Since it includes the period
- * registers, we can slow it down to 10 msec using TMRCNT. If the default
- * period is acceptable, TMRCNT can be left undefined.
+ * registers, the interrupt frequency can be reduced using TMRCNT.
+ * If the default period is acceptable, TMRCNT can be left undefined.
+ * TMRMS represents the desired mecs per tick (msecs per interrupt).
  *----------------------------------------------------------------------*/
+#define CONFIG_SYS_HZ			1000	/* Always 1000 */
 #define CONFIG_SYS_NIOS_TMRBASE	0x02120820	/* Tick timer base addr */
-#define CONFIG_SYS_NIOS_TMRIRQ		3		/* Timer IRQ num	*/
-#define CONFIG_SYS_NIOS_TMRMS		10		/* 10 msec per tick	*/
-#define CONFIG_SYS_NIOS_TMRCNT (CONFIG_SYS_NIOS_TMRMS * (CONFIG_SYS_CLK_FREQ/1000))
-#define CONFIG_SYS_HZ		(CONFIG_SYS_CLK_FREQ/(CONFIG_SYS_NIOS_TMRCNT + 1))
+#define CONFIG_SYS_NIOS_TMRIRQ		3	/* Timer IRQ num	*/
+#define CONFIG_SYS_NIOS_TMRMS		10	/* Desired period (msec)*/
+#define CONFIG_SYS_NIOS_TMRCNT \
+		(CONFIG_SYS_NIOS_TMRMS * (CONFIG_SYS_CLK_FREQ/1000))
 
 /*------------------------------------------------------------------------
  * STATUS LED -- Provides a simple blinking led. For Nios2 each board
diff --git a/include/configs/EP1S10.h b/include/configs/EP1S10.h
index 498f26d..bfbf8c1 100644
--- a/include/configs/EP1S10.h
+++ b/include/configs/EP1S10.h
@@ -119,14 +119,16 @@
  * TIMEBASE --
  *
  * The high res timer defaults to 1 msec. Since it includes the period
- * registers, we can slow it down to 10 msec using TMRCNT. If the default
- * period is acceptable, TMRCNT can be left undefined.
+ * registers, the interrupt frequency can be reduced using TMRCNT.
+ * If the default period is acceptable, TMRCNT can be left undefined.
+ * TMRMS represents the desired mecs per tick (msecs per interrupt).
  *----------------------------------------------------------------------*/
+#define CONFIG_SYS_HZ			1000	/* Always 1000 */
 #define CONFIG_SYS_NIOS_TMRBASE	0x02120820	/* Tick timer base addr */
-#define CONFIG_SYS_NIOS_TMRIRQ		3		/* Timer IRQ num	*/
-#define CONFIG_SYS_NIOS_TMRMS		10		/* 10 msec per tick	*/
-#define CONFIG_SYS_NIOS_TMRCNT (CONFIG_SYS_NIOS_TMRMS * (CONFIG_SYS_CLK_FREQ/1000))
-#define CONFIG_SYS_HZ		(CONFIG_SYS_CLK_FREQ/(CONFIG_SYS_NIOS_TMRCNT + 1))
+#define CONFIG_SYS_NIOS_TMRIRQ		3	/* Timer IRQ num */
+#define CONFIG_SYS_NIOS_TMRMS		10	/* Desired period (msec)*/
+#define CONFIG_SYS_NIOS_TMRCNT \
+		(CONFIG_SYS_NIOS_TMRMS * (CONFIG_SYS_CLK_FREQ/1000))
 
 /*------------------------------------------------------------------------
  * STATUS LED -- Provides a simple blinking led. For Nios2 each board
diff --git a/include/configs/EP1S40.h b/include/configs/EP1S40.h
index 4ad65d8..4d905fe 100644
--- a/include/configs/EP1S40.h
+++ b/include/configs/EP1S40.h
@@ -119,14 +119,16 @@
  * TIMEBASE --
  *
  * The high res timer defaults to 1 msec. Since it includes the period
- * registers, we can slow it down to 10 msec using TMRCNT. If the default
- * period is acceptable, TMRCNT can be left undefined.
+ * registers, the interrupt frequency can be reduced using TMRCNT.
+ * If the default period is acceptable, TMRCNT can be left undefined.
+ * TMRMS represents the desired mecs per tick (msecs per interrupt).
  *----------------------------------------------------------------------*/
+#define CONFIG_SYS_HZ			1000	/* Always 1000 */
 #define CONFIG_SYS_NIOS_TMRBASE	0x02120820	/* Tick timer base addr */
-#define CONFIG_SYS_NIOS_TMRIRQ		3		/* Timer IRQ num	*/
-#define CONFIG_SYS_NIOS_TMRMS		10		/* 10 msec per tick	*/
-#define CONFIG_SYS_NIOS_TMRCNT (CONFIG_SYS_NIOS_TMRMS * (CONFIG_SYS_CLK_FREQ/1000))
-#define CONFIG_SYS_HZ		(CONFIG_SYS_CLK_FREQ/(CONFIG_SYS_NIOS_TMRCNT + 1))
+#define CONFIG_SYS_NIOS_TMRIRQ		3	/* Timer IRQ num */
+#define CONFIG_SYS_NIOS_TMRMS		10	/* Desired period (msec) */
+#define CONFIG_SYS_NIOS_TMRCNT \
+		(CONFIG_SYS_NIOS_TMRMS * (CONFIG_SYS_CLK_FREQ/1000))
 
 /*------------------------------------------------------------------------
  * STATUS LED -- Provides a simple blinking led. For Nios2 each board
diff --git a/include/configs/PCI5441.h b/include/configs/PCI5441.h
index d06b7f8..c60a9f7 100644
--- a/include/configs/PCI5441.h
+++ b/include/configs/PCI5441.h
@@ -114,14 +114,16 @@
  * TIMEBASE --
  *
  * The high res timer defaults to 1 msec. Since it includes the period
- * registers, we can slow it down to 10 msec using TMRCNT. If the default
- * period is acceptable, TMRCNT can be left undefined.
+ * registers, the interrupt frequency can be reduced using TMRCNT.
+ * If the default period is acceptable, TMRCNT can be left undefined.
+ * TMRMS represents the desired mecs per tick (msecs per interrupt).
  *----------------------------------------------------------------------*/
+#define CONFIG_SYS_HZ			1000	/* Always 1000 */
 #define CONFIG_SYS_NIOS_TMRBASE	0x00920860	/* Tick timer base addr	*/
-#define CONFIG_SYS_NIOS_TMRIRQ		3		/* Timer IRQ num	*/
-#define CONFIG_SYS_NIOS_TMRMS		10		/* 10 msec per tick	*/
-#define CONFIG_SYS_NIOS_TMRCNT	(CONFIG_SYS_NIOS_TMRMS * (CONFIG_SYS_CLK_FREQ/1000))
-#define	CONFIG_SYS_HZ		(CONFIG_SYS_CLK_FREQ/(CONFIG_SYS_NIOS_TMRCNT + 1))
+#define CONFIG_SYS_NIOS_TMRIRQ		3	/* Timer IRQ num */
+#define CONFIG_SYS_NIOS_TMRMS		10	/* Desired period (msec)*/
+#define CONFIG_SYS_NIOS_TMRCNT \
+		(CONFIG_SYS_NIOS_TMRMS * (CONFIG_SYS_CLK_FREQ/1000))
 
 
 /*
diff --git a/include/configs/PK1C20.h b/include/configs/PK1C20.h
index 165dde0..874c20b 100644
--- a/include/configs/PK1C20.h
+++ b/include/configs/PK1C20.h
@@ -124,14 +124,16 @@
  * TIMEBASE --
  *
  * The high res timer defaults to 1 msec. Since it includes the period
- * registers, we can slow it down to 10 msec using TMRCNT. If the default
- * period is acceptable, TMRCNT can be left undefined.
+ * registers, the interrupt frequency can be reduced using TMRCNT.
+ * If the default period is acceptable, TMRCNT can be left undefined.
+ * TMRMS represents the desired mecs per tick (msecs per interrupt).
  *----------------------------------------------------------------------*/
+#define CONFIG_SYS_HZ			1000	/* Always 1000 */
 #define CONFIG_SYS_NIOS_TMRBASE	0x02120820	/* Tick timer base addr */
-#define CONFIG_SYS_NIOS_TMRIRQ		3		/* Timer IRQ num	*/
-#define CONFIG_SYS_NIOS_TMRMS		10		/* 10 msec per tick	*/
-#define CONFIG_SYS_NIOS_TMRCNT (CONFIG_SYS_NIOS_TMRMS * (CONFIG_SYS_CLK_FREQ/1000))
-#define CONFIG_SYS_HZ		(CONFIG_SYS_CLK_FREQ/(CONFIG_SYS_NIOS_TMRCNT + 1))
+#define CONFIG_SYS_NIOS_TMRIRQ		3	/* Timer IRQ num */
+#define CONFIG_SYS_NIOS_TMRMS		10	/* Desired period */
+#define CONFIG_SYS_NIOS_TMRCNT \
+		(CONFIG_SYS_NIOS_TMRMS * (CONFIG_SYS_CLK_FREQ/1000))
 
 /*------------------------------------------------------------------------
  * STATUS LED -- Provides a simple blinking led. For Nios2 each board
-- 
1.6.0.6

[U-Boot] [PATCH] nios2: Set CONFIG_SYS_HZ to 1000 all nios2 boards.

[Scott McNutt]

> So it might be cleaner to let user define the HZ as the actual tick rate 
> and increase the tick count by one in the tmr_isr.

This was discussed/debated thoroughly over the past year:
CONFIG_SYS_HZ at 1000 is mandatory.

> 
> -    timestamp += CONFIG_SYS_NIOS_TMRMS;
> +    timestamp++;

This means that each interrupt is exactly 1 msec. That's not what
was intended and not what was implemented Forcing the interrupt
period to 1 msec is an unnecessary constraint. If you want the isr
to increment the timestamp by 1, then set CONFIG_SYS_NIOS_TMRMS to 1
... and set your timer accordingly.

--Scott

[U-Boot] [PATCH] nios2: Set CONFIG_SYS_HZ to 1000 all nios2 boards.

[Thomas Chou]

On 03/31/2010 10:09 AM, Scott McNutt wrote:
>> So it might be cleaner to let user define the HZ as the actual tick rate
>> and increase the tick count by one in the tmr_isr.
>>      
> This was discussed/debated thoroughly over the past year:
> CONFIG_SYS_HZ at 1000 is mandatory.
>
>    
>> -    timestamp += CONFIG_SYS_NIOS_TMRMS;
>> +    timestamp++;
>>      
> This means that each interrupt is exactly 1 msec. That's not what
> was intended and not what was implemented Forcing the interrupt
> period to 1 msec is an unnecessary constraint. If you want the isr
> to increment the timestamp by 1, then set CONFIG_SYS_NIOS_TMRMS to 1
> ... and set your timer accordingly.
>
> -
>    
Hi Scott,

Thanks for the explanation. I will set HZ to 1000 and TMRMS to 1.

I tried to follow your patch "nios2: Set CONFIG_SYS_HZ to 1000 all nios2 
boards". But got cfi flash buffer write timeout on EP2C35 board. If I 
set TMRMS to 1, it works with HZ at 100, 1000 and 2000.

Best regards,
Thomas

[U-Boot] [PATCH] nios2: Set CONFIG_SYS_HZ to 1000 all nios2 boards.

[Scott McNutt]

Hi Thomas,

Thomas Chou wrote:
> On 03/31/2010 10:09 AM, Scott McNutt wrote:
>>> So it might be cleaner to let user define the HZ as the actual tick rate
>>> and increase the tick count by one in the tmr_isr.
>>>      
>> This was discussed/debated thoroughly over the past year:
>> CONFIG_SYS_HZ at 1000 is mandatory.
>>
>>   
>>> -    timestamp += CONFIG_SYS_NIOS_TMRMS;
>>> +    timestamp++;
>>>      
>> This means that each interrupt is exactly 1 msec. That's not what
>> was intended and not what was implemented Forcing the interrupt
>> period to 1 msec is an unnecessary constraint. If you want the isr
>> to increment the timestamp by 1, then set CONFIG_SYS_NIOS_TMRMS to 1
>> ... and set your timer accordingly.

> I tried to follow your patch "nios2: Set CONFIG_SYS_HZ to 1000 all nios2 
> boards". But got cfi flash buffer write timeout on EP2C35 board. If I 
> set TMRMS to 1, it works with HZ at 100, 1000 and 2000.

The patch should not affect your EP2C35 board configuration since it
hasn't been added yet. So I'm not sure I understand the scenario you
are describing. If your TMRMS is set to 1 and your TMRCNT is set to
match, then you should be ticking at 1000 Hz. And if CONFIG_SYS_HZ
is set to 1000, we should achieve oneness with u-boot ;-)

In any case, for the benefit of the nios2 folks who might be
following this thread:

CONFIG_SYS_TMRMS -- this is the _actual_ period (in msec) of the
timer interrupt that is added to the timestamp on every interrupt.
It's what board developers choose based on their requirements. It's
intended to be cpu clock and timer configuration agnostic.

CONFIG_SYS_NIOS_TMRCNT -- this is what (optionally) gets loaded into
the timer period registers. If the default configuration of the timer
matches TMRMS, then there's no need to define this value. However,
if for some reason you want to slow down the interrupt frequency,
you can do so by defining an appropriate value.

How to calculate these values is up to the board developer. For the
Altera EP1xxx boards, we start with a desired period of 10 ms, so
TMRMS is set to 10. Then TMRCNT is calculated based on the system
clock frequency.

And none of the above has anything to do with CONFIG_SYS_HZ ... which
for the most part is just a synonym for the value 1000. If you set
CONFIG_SYS_HZ to anything less than 1000 you end up with a timeout
of zero in cfi_flash ... at least until we rebase to the mothership
to get the latest cfi patches ... at which point it should be 1.
A timeout of zero is probably not what we want. ;-)

In any case, I'll try to fire up my 2C35 board and have a look.

Best Regards,
--Scott

[U-Boot] [PATCH] nios2: Set CONFIG_SYS_HZ to 1000 all nios2 boards.

[Michal Simek]

Scott McNutt wrote:
>    CONFIG_SYS_HZ was being calculated (incorrectly) in nios2 configuration
>    headers. Updated comments to accurately describe timebase macros.
> 
> Signed-off-by: Scott McNutt <smcnutt@psyent.com>
> ---
>  include/configs/EP1C20.h  |   14 ++++++++------
>  include/configs/EP1S10.h  |   14 ++++++++------
>  include/configs/EP1S40.h  |   14 ++++++++------
>  include/configs/PCI5441.h |   14 ++++++++------
>  include/configs/PK1C20.h  |   14 ++++++++------
>  5 files changed, 40 insertions(+), 30 deletions(-)

This is the next "nice" patch around altera boards. If you do the same 
changes (because of the same reason) in 5 files it points me that 
something is wrong. Please create one generic nios2 file and move there 
these common macros. Look at ppc405/440 which use that style.

Michal


-- 
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/
Microblaze U-BOOT custodian

[U-Boot] [PATCH] nios2: Set CONFIG_SYS_HZ to 1000 all nios2 boards.

[Scott McNutt]

Michal Simek wrote:
> Scott McNutt wrote:
>>    CONFIG_SYS_HZ was being calculated (incorrectly) in nios2 
>> configuration
>>    headers. Updated comments to accurately describe timebase macros.
>>
>> Signed-off-by: Scott McNutt <smcnutt@psyent.com>
>> ---
>>  include/configs/EP1C20.h  |   14 ++++++++------
>>  include/configs/EP1S10.h  |   14 ++++++++------
>>  include/configs/EP1S40.h  |   14 ++++++++------
>>  include/configs/PCI5441.h |   14 ++++++++------
>>  include/configs/PK1C20.h  |   14 ++++++++------
>>  5 files changed, 40 insertions(+), 30 deletions(-)
> 
> This is the next "nice" patch around altera boards.

Agreed.

> If you do the same 
> changes (because of the same reason) in 5 files it points me that 
> something is wrong.

IMO, it is inefficient, not wrong. But only because each of these
boards worked fine ... until some things changed externally ... that
required a common resolution.

> Please create one generic nios2 file and move there 
> these common macros. Look at ppc405/440 which use that style.

It would indeed be more efficient -- perhaps a common header for
the Altera boards when time permits.

Regards,
--Scott

[U-Boot] [PATCH] nios2: Set CONFIG_SYS_HZ to 1000 all nios2 boards.

[Michal Simek]

Scott McNutt wrote:
> 
> 
> Michal Simek wrote:
>> Scott McNutt wrote:
>>>    CONFIG_SYS_HZ was being calculated (incorrectly) in nios2 
>>> configuration
>>>    headers. Updated comments to accurately describe timebase macros.
>>>
>>> Signed-off-by: Scott McNutt <smcnutt@psyent.com>
>>> ---
>>>  include/configs/EP1C20.h  |   14 ++++++++------
>>>  include/configs/EP1S10.h  |   14 ++++++++------
>>>  include/configs/EP1S40.h  |   14 ++++++++------
>>>  include/configs/PCI5441.h |   14 ++++++++------
>>>  include/configs/PK1C20.h  |   14 ++++++++------
>>>  5 files changed, 40 insertions(+), 30 deletions(-)
>>
>> This is the next "nice" patch around altera boards.
> 
> Agreed.
> 
>> If you do the same changes (because of the same reason) in 5 files it 
>> points me that something is wrong.
> 
> IMO, it is inefficient, not wrong. But only because each of these
> boards worked fine ... until some things changed externally ... that
> required a common resolution.

I don't have anything against the purpose that patch because fix the 
real issue. I have just problem with that style of doing it.

I am really not sure what you mean by "until some things changed 
externally"!


> 
>> Please create one generic nios2 file and move there these common 
>> macros. Look at ppc405/440 which use that style.
> 
> It would indeed be more efficient -- perhaps a common header for
> the Altera boards when time permits.

:-) It is not just to be more efficient.
1. Save u-boot size
2. If is only one place where it is defined than you don't need to spend 
time to fix all boards which are very similar. It is more bug-free solution.

Regards,
Michal




> 
> Regards,
> --Scott
> 


-- 
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/
Microblaze U-BOOT custodian

[U-Boot] [PATCH] nios2: Set CONFIG_SYS_HZ to 1000 all nios2 boards.

[Scott McNutt]

Michal,

>> It would indeed be more efficient -- perhaps a common header for
>> the Altera boards when time permits.
> 
> :-) It is not just to be more efficient.
> 1. Save u-boot size
> 2. If is only one place where it is defined than you don't need to spend 
> time to fix all boards which are very similar. It is more bug-free 
> solution.

Exactly! That's what I meant by "more efficient." Efficiency of size,
efficiency of effort. I'm confident that we are in complete agreement.

Best Regards,
--Scott