On Tue, Apr 17, 2012 at 5:02 AM, David Brown <da...@westcontrol.com> wrote:
> On 17/04/2012 09:53, Wayne Uroda wrote:
>> Hi,
>>
>> I feel quite foolish asking the following question... Please don't judge me
>> too harshly...
>>
>> In my MSPGCC based system I have got the following #defines which I use for
>> critical (interrupts disabled) access around volatile memory which is
>> modified by interrupt routines:
>>
>> #define criticalStart() __asm__ __volatile__("push r2"::); __asm__
>> __volatile__("dint"::)
>> #define criticalEnd() __asm__ __volatile__("pop r2"::)
>>
>
> /Never/ mess with the stack inside a C function. You are asking for
> trouble. Simply save the old SR in a local variable (as you do in the
> new versions below).
>
>> Usually I use this code to write to a counter which is decremented by my
>> timer ISR, eg
>>
>> void setTimeout(UInt16 timeout)
>> {
>> // Other work before setting the volatile, ISR driven value
>> criticalStart();
>> busyTimeout = timeout;
>> criticalEnd();
>> }
>
> Of course, it is pointless disabling interrupts here. The msp430 does
> the 16-bit write as an atomic operation anyway. If there is only one
> instruction, it cannot be interrupted. You only need the critical
> section if you are trying to do a 32-bit write.
>
>>
>> Today I am using Code composer for a new project and I tried a similar
>> trick. After several hours of debugging, it turns out that my little "push
>> r2" trick causes code composer to access local variables incorrectly, since
>> these are accessed relative to the stack pointer.
>>
>> For example, the following code does not function correctly because the
>> "localTimer" variable is not written correctly (the memory access is one
>> word off owing to the extra stack push).
>>
>> while(true)
>> {
>> UInt32 localTimer;
>> criticalStart();
>> localTimer = globalTimer;
>> criticalEnd();
>> if (localTimer == 0)
>> {
>> break;
>> }
>> }
>>
>>
>> My question is, does GCC analyse the inline assembly code and notice that I
>> have altered the stack, or have I just been *extremely* lucky for many
>> years? Certainly code composer does not see that I have modified the stack.
>
> You've just been lucky.
>>
>> I am going to change my critical macros like so (this is how I am now doing
>> critical access in code composer, which, unless I am missing something, has
>> no builtin way to do critical functions):
>>
>> #define criticalStart() { UInt16 __oldInterruptStatus__ = READ_SR(); __asm__
>> __volatile__("dint"::)
>> #define criticalEnd() WRITE_SR(__oldInterruptStatus__); }
>>
>
> That should work. Like many people (and all serious coding standards),
> I don't like putting brackets in macros - but that's your choice.
Nothing wrong with brackets in macros. Mismatched brackets that
require macro pairs, now---that's a Bad Thing(tm).
>
>> Any major issues with doing this? Should I instead wrap all reads and writes
>> to volatile memory/variables with critical functions? I usually only wrap
>> one line of code with the critical macros.
>>
>
> You would be better off learning what critical sections actually /are/,
> and when you need them, rather than wasting time, effort, code space and
> run time when they are not necessary. As a very general indication, you
> need interrupt disabling like this if you have code that needs atomic
> access to several parts of a complex memory structure, and the code
> could be interrupted by other code that also then accesses the same
> structure.
>
> In particular, you don't need them within interrupt functions (or other
> places with interrupts disabled), and you don't need them to access data
> that the cpu can access atomically (like 8-bit and 16-bit data).
One clarification: you don't need a critical section around a data
access that the mcu *does* atomically. That it could (even should)
have been atomic, and the compiler didn't make it so, should always be
taken into account. Trust, but verify.
>> Also if anybody knows code composer v3.1 and knows a better way, please let
>> me know.
>> Thanks,
>>
>
> With gcc for the msp430, there is also a "critical" function attribute -
> you can put your critical accesses within a small static inline
> "critical" function.
Interesting thought: I haven't tried to see whether the "critical"
attribute (which is expanded when the function prolog and epilog are
emitted) is preserved if a function is inlined. Does that actually
work?
Peter
> With Code Composter 3.1, the best option is to throw away the tools and
> use something else. The toolchain has major problems, and you need to
> work hard to get around them. At the very least, use CCS 5, as it has a
> much better IDE. But the toolchain still has the critical failing that
> it doesn't zero out uninitialised statically allocated data, and it has
> the most appalling choices you could imagine of which issues are
> warnings, errors, or ignored. It will give errors or warnings on
> perfectly good code, while accepting total crap without question.
>
>
>
> ------------------------------------------------------------------------------
> Better than sec? Nothing is better than sec when it comes to
> monitoring Big Data applications. Try Boundary one-second
> resolution app monitoring today. Free.
> http://p.sf.net/sfu/Boundary-dev2dev
> _______________________________________________
> Mspgcc-users mailing list
> Mspgcc-users@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/mspgcc-users
------------------------------------------------------------------------------
Better than sec? Nothing is better than sec when it comes to
monitoring Big Data applications. Try Boundary one-second
resolution app monitoring today. Free.
http://p.sf.net/sfu/Boundary-dev2dev
_______________________________________________
Mspgcc-users mailing list
Mspgcc-users@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/mspgcc-users
