[ECOS] Cortex M3 idle sleep

[ECOS] Cortex M3 idle sleep

[Simon Kallweit]

Hello

Would it make sense to include support for sleeping ("wfi" instruction) 
in idle thread for the cortex m3 architecture? The current code in 
hal_arch.h looks like:

#if 0 //ndef HAL_IDLE_THREAD_ACTION
#define HAL_IDLE_THREAD_ACTION(__count) __asm__ volatile ( "wfi\n" )
#else
#define HAL_IDLE_THREAD_ACTION(__count) CYG_EMPTY_STATEMENT
#endif

I guess this is not what it was meant to be. I would propose to add an option like CYGHWR_HAL_CORTEXM_IDLE_SLEEP to enable the wfi instruction:

#ifndef HAL_IDLE_THREAD_ACTION
# ifdef CYGHWR_HAL_CORTEXM_IDLE_SLEEP
#  define HAL_IDLE_THREAD_ACTION(__count) __asm__ volatile ( "wfi\n" )
# else
#  define HAL_IDLE_THREAD_ACTION(__count) CYG_EMPTY_STATEMENT
# endif
#endif

Also, currently var_arch.h gets not included in the hal_arch.h header, so it's impossible to override for example the HAL_IDLE_THREAD_ACTION macro. I guess this was just overlooked, or is it intentional?

I can provide a patch if there are no objections.

Best regards
Simon


-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

Re: [ECOS] Cortex M3 idle sleep

[Nick Garnett]

Simon Kallweit <simon.kallweit@intefo.ch> writes:

> Hello
> 
> Would it make sense to include support for sleeping ("wfi"
> instruction) in idle thread for the cortex m3 architecture? The
> current code in hal_arch.h looks like:
> 
> #if 0 //ndef HAL_IDLE_THREAD_ACTION
> #define HAL_IDLE_THREAD_ACTION(__count) __asm__ volatile ( "wfi\n" )
> #else
> #define HAL_IDLE_THREAD_ACTION(__count) CYG_EMPTY_STATEMENT
> #endif
> 
> I guess this is not what it was meant to be. I would propose to add an option like CYGHWR_HAL_CORTEXM_IDLE_SLEEP to enable the wfi instruction:
> 
> #ifndef HAL_IDLE_THREAD_ACTION
> # ifdef CYGHWR_HAL_CORTEXM_IDLE_SLEEP
> #  define HAL_IDLE_THREAD_ACTION(__count) __asm__ volatile ( "wfi\n" )
> # else
> #  define HAL_IDLE_THREAD_ACTION(__count) CYG_EMPTY_STATEMENT
> # endif
> #endif
> 
> Also, currently var_arch.h gets not included in the hal_arch.h header, so it's impossible to override for example the HAL_IDLE_THREAD_ACTION macro. I guess this was just overlooked, or is it intentional?
> 
> I can provide a patch if there are no objections.


The "#if 0" is an oversight on my part. When debugging via JTAG I was
having difficulties breaking in to an idle system. Stopping it using
WFI solved this. The #else part is also a consequence of this, so that
code should really be:

#ifndef HAL_IDLE_THREAD_ACTION
#define HAL_IDLE_THREAD_ACTION(__count) __asm__ volatile ( "wfi\n" )
#endif

Given this I don't really see any need for the
CYGHWR_HAL_CORTEXM_IDLE_SLEEP option. If a variant or platform HAL
needs to override this it can very easily by redefining
HAL_IDLE_THREAD_ACTION().

var_arch.h should certainly be included in hal_arch.h. It is only
because the STM32 has an empty var_arch.h that we haven't noticed this
before.


-- 
Nick Garnett                                        eCos Kernel Architect
eCosCentric Limited    http://www.eCosCentric.com        The eCos experts
Barnwell House, Barnwell Drive, Cambridge, UK.       Tel: +44 1223 245571
Registered in England and Wales:                          Reg No: 4422071
Besuchen Sie uns vom 3.-5.03.09 auf der Embedded World 2009, Stand 11-300
Visit us at Embedded World 2009, Nürnberg, Germany, 3-5 Mar, Stand 11-300


--
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

Re: [ECOS] Cortex M3 idle sleep

[Simon Kallweit]

Nick Garnett wrote:
> Simon Kallweit <simon.kallweit@intefo.ch> writes:
>
>   
>> Hello
>>
>> Would it make sense to include support for sleeping ("wfi"
>> instruction) in idle thread for the cortex m3 architecture? The
>> current code in hal_arch.h looks like:
>>
>> #if 0 //ndef HAL_IDLE_THREAD_ACTION
>> #define HAL_IDLE_THREAD_ACTION(__count) __asm__ volatile ( "wfi\n" )
>> #else
>> #define HAL_IDLE_THREAD_ACTION(__count) CYG_EMPTY_STATEMENT
>> #endif
>>
>> I guess this is not what it was meant to be. I would propose to add an option like CYGHWR_HAL_CORTEXM_IDLE_SLEEP to enable the wfi instruction:
>>
>> #ifndef HAL_IDLE_THREAD_ACTION
>> # ifdef CYGHWR_HAL_CORTEXM_IDLE_SLEEP
>> #  define HAL_IDLE_THREAD_ACTION(__count) __asm__ volatile ( "wfi\n" )
>> # else
>> #  define HAL_IDLE_THREAD_ACTION(__count) CYG_EMPTY_STATEMENT
>> # endif
>> #endif
>>
>> Also, currently var_arch.h gets not included in the hal_arch.h header, so it's impossible to override for example the HAL_IDLE_THREAD_ACTION macro. I guess this was just overlooked, or is it intentional?
>>
>> I can provide a patch if there are no objections.
>>     
>
>
> The "#if 0" is an oversight on my part. When debugging via JTAG I was
> having difficulties breaking in to an idle system. Stopping it using
> WFI solved this. The #else part is also a consequence of this, so that
> code should really be:
>   

Well that's actually the reason for me to propose a 
CYGHWR_HAL_CORTEXM_IDLE_SLEEP configuration option. During debugging, 
wfi is rather troublesome, so I thought it might be a good idea to make 
it configurable. Or is this too much cruft?

Simon

-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

Re: [ECOS] Cortex M3 idle sleep

[Nick Garnett]

Simon Kallweit <simon.kallweit@intefo.ch> writes:

> Nick Garnett wrote:
> > The "#if 0" is an oversight on my part. When debugging via JTAG I was
> > having difficulties breaking in to an idle system. Stopping it using
> > WFI solved this. The #else part is also a consequence of this, so that
> > code should really be:
> >
> 
> Well that's actually the reason for me to propose a
> CYGHWR_HAL_CORTEXM_IDLE_SLEEP configuration option. During debugging,
> wfi is rather troublesome, so I thought it might be a good idea to
> make it configurable. Or is this too much cruft?

I think it might be too much cruft.

I don't know how other people work but I tend to like
enabling/disabling debug stuff by editing the source rather than
messing with the configuration. The latter is often more complicated
and it takes a lot longer to recompile everything. CVS usually reminds
me to undo the change before checking in -- except in this case where
there was no original to diff against, and I forgot about this edit.

So I would actually prefer something more like this:

#if 1
# ifndef HAL_IDLE_THREAD_ACTION
#  define HAL_IDLE_THREAD_ACTION(__count) __asm__ volatile ( "wfi\n" )
# endif
#else
# define HAL_IDLE_THREAD_ACTION(__count) CYG_EMPTY_STATEMENT
#endif

So that I can easily override any idle thread action when I need
to. However I appreciate that this is not necessarily to everyone's
taste, and would usually make sure any such construct never escaped my
own source tree.

-- 
Nick Garnett                                        eCos Kernel Architect
eCosCentric Limited    http://www.eCosCentric.com        The eCos experts
Barnwell House, Barnwell Drive, Cambridge, UK.       Tel: +44 1223 245571
Registered in England and Wales:                          Reg No: 4422071
Besuchen Sie uns vom 3.-5.03.09 auf der Embedded World 2009, Stand 11-300
Visit us at Embedded World 2009, Nürnberg, Germany, 3-5 Mar, Stand 11-300


--
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

Re: [ECOS] Cortex M3 idle sleep

[Simon Kallweit]

Nick Garnett wrote:
> Simon Kallweit <simon.kallweit@intefo.ch> writes:
>
>   
>> Nick Garnett wrote:
>>     
>>> The "#if 0" is an oversight on my part. When debugging via JTAG I was
>>> having difficulties breaking in to an idle system. Stopping it using
>>> WFI solved this. The #else part is also a consequence of this, so that
>>> code should really be:
>>>
>>>       
>> Well that's actually the reason for me to propose a
>> CYGHWR_HAL_CORTEXM_IDLE_SLEEP configuration option. During debugging,
>> wfi is rather troublesome, so I thought it might be a good idea to
>> make it configurable. Or is this too much cruft?
>>     
>
> I think it might be too much cruft.
>
> I don't know how other people work but I tend to like
> enabling/disabling debug stuff by editing the source rather than
> messing with the configuration. The latter is often more complicated
> and it takes a lot longer to recompile everything. CVS usually reminds
> me to undo the change before checking in -- except in this case where
> there was no original to diff against, and I forgot about this edit.
>   

Well you're probably right. So let's just clean things up then :)

Simon

-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

Re: [ECOS] Cortex M3 idle sleep

[Nick Garnett]

Simon Kallweit <simon.kallweit@intefo.ch> writes:

> Nick Garnett wrote:
> > Simon Kallweit <simon.kallweit@intefo.ch> writes:
> >
> >
> >> Nick Garnett wrote:
> >>
> >>> The "#if 0" is an oversight on my part. When debugging via JTAG I was
> >>> having difficulties breaking in to an idle system. Stopping it using
> >>> WFI solved this. The #else part is also a consequence of this, so that
> >>> code should really be:
> >>>
> >>>
> >> Well that's actually the reason for me to propose a
> >> CYGHWR_HAL_CORTEXM_IDLE_SLEEP configuration option. During debugging,
> >> wfi is rather troublesome, so I thought it might be a good idea to
> >> make it configurable. Or is this too much cruft?
> >>
> >
> > I think it might be too much cruft.
> >
> > I don't know how other people work but I tend to like
> > enabling/disabling debug stuff by editing the source rather than
> > messing with the configuration. The latter is often more complicated
> > and it takes a lot longer to recompile everything. CVS usually reminds
> > me to undo the change before checking in -- except in this case where
> > there was no original to diff against, and I forgot about this edit.
> >
> 
> Well you're probably right. So let's just clean things up then :)

Yep. I'll do this right now.


-- 
Nick Garnett                                        eCos Kernel Architect
eCosCentric Limited    http://www.eCosCentric.com        The eCos experts
Barnwell House, Barnwell Drive, Cambridge, UK.       Tel: +44 1223 245571
Registered in England and Wales:                          Reg No: 4422071
Besuchen Sie uns vom 3.-5.03.09 auf der Embedded World 2009, Stand 11-300
Visit us at Embedded World 2009, Nürnberg, Germany, 3-5 Mar, Stand 11-300


--
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss