SPI interrupt not triggering (MC9S12DG256 + CW 3.1)

[sjmelnikoff]

Hi.

I'm in the process of porting some code from a DG128 to a DG256. After making the minimum number of changes (mainly relating to the differences in the memory map), the code does run. However, I've found that the SPI interrupt doesn't trigger.

This worked fine on the DG128. The only clue I could find in the documentation was the fact that the SPI module has been upgraded in the DG256, such that "In SPI Master Mode, the change of a config bit during a transmission in progress, will abort the transmission and force the SPI into idle state."

The code doesn't change any configuration bits during a transmission. However, there are circumstances where the SPIE bit is cleared or set during transmission. The assembly produced by the compiler uses BSET/BCLR, which I assumed wouldn't affect any of the other bits. Is this assumption wrong?

I've experimented with the debugger, with some odd results:

1. Single stepping through the code which writes to SPIDR sets SPIF as expected, but then clears SPIF on the next instruction, even if the next instruction is a NOP! (I understand from another post that the debugger reading the SPI registers could cause this flag to be cleared).
2. Specifying a particular register in the watch window, then restarting the code, makes the problem go away! The following registers worked: SPISR, SPICR1, SPICR2, and an I2C register; SPIDR did not. It didn't matter whether the memory or watch windows were open or not.
3. It makes no difference whether the BDM module is connected or not.

All in all, this is a bit of a puzzle, especially as this code worked fine on the DG128. I would welcome any suggestions!

Steve.

[dog]

Need to be very careful with bit set and bit clear instructions on registers that have write 1 to clear bits (such as interrupt flags).

These are read modify write operations. What set bit actually does is read the register, OR the mask byte in the instruction with it and then write the result back to the register. You can see that any flags in the register that are set will get 1's written back to them and if write one to clear will get cleared - not what you expected! Clear bit does a similar thing using AND.

At the C level don't use bit field assignments and don't use |= with a bit mask, stick to plain old assignment with the bit mask, e.g. FlagRegister = 0x40;

Dog.

[sjmelnikoff]

---

dog wrote:

These are read modify write operations. What set bit actually does is read the register, OR the mask byte in the instruction with it and then write the result back to the register.

...

At the C level don't use bit field assignments and don't use |= with a bit mask, stick to plain old assignment with the bit mask, e.g. FlagRegister = 0x40;

---

Dog's point is a good one in general - though in this particular case, it doesn't apply. The HCS12 headers for CW3.1 use bitfields for bit access as well as masks, As a result, assignments made through bitfields are translated as BSET and BCLR, as mentioned in my original post.

In any case, the SPIF bit is never written to; it's cleared automatically by reading other registers; and the SPIE bit is in a control register, for which it's perfectly valid to do a read-modigy-write.

Steve.

[peg]

Hi Dog,

It would seem our clever chip designers are actually on to this, making write 1 to acknowledge bits read as 0. Do you know of specific instances where they have failed to do this?

[imajeff]

---

peg wrote:

Hi Dog,

It would seem our clever chip designers are actually on to this, making write 1 to acknowledge bits read as 0. Do you know of specific instances where they have failed to do this?

---

That sounds bad. The method works just fine the way it is now. Nearly every interrupt flag I know works in the way that if the bit reads '1', you write a '1' to it in order to clear the same bit (to make it read '0'). If they changed any of those arrangements, it would introduce a huge incompatibility that is not necessary. It already works just fine to write only a '1' to the bit(s) which need acknowledged, and not use bset.

[peg]

Woops!

It would seem I forgot which forum I was in. My comment refers to 8-bit devices. This is obviously not relevent with the 12's. The reason I checked was that I don't remember anyone being caught out by this.

[bigmac]

Hello all,

Perhaps I am missing something, but I cannot see a specific problem with using the bset instruction to clear an interrupt flag.

If the flag is set, it will read as a 1, and the instruction should write a 1 back to the bit to clear the flag.  If the flag is clear, the instruction would still write a 1, but this would obviously do nothing, as required.  However, there might be a potential problem if a bset instruction was used to set a bit, other than the flag bit - the flag could be unintentionally cleared.

A simular argument would also apply if the method of clearing a flag is to write a zero, and the bclr instruction is used (e.g. applicable to some timer flags).

My understanding would be that the instruction is a read-modify-write type in order to determine the state of the other bits that must not alter.

Regards,

Mac

[dog]

Hi Bigmac,

The problem is not the intended action of the BSET clearing the flag but the unintended side effect of clearing any other write 1 to clear flags that are set in the same register.

If you know that there is only one such flag in a specific register then it works fine, but only for the flag bit, (and I've used it occasionally, but only with very careful consideration). 

As you point out, if you use BSET it for any other bit in the register you also can accidentally clear flag bit(s) - so the simplest rule is avoid using read-modify-write operations on any register that contains any write one to clear bits.

I am not aware of any instance of S12(X) designers changing this feature, it works very effectively the way it is and you need to be able to read these flags as 1 else you couldn't test which flag had been set (to cause your interrupt for example). 

And, I have used the CodeWarrior compiler a lot and the simple assignment does not generate BSET.

cheers,

Dog

[kef]

Dog,

why using very carefully BSET if you can use safely BCLR with inversed bit mask? Check Motorola HC11 pink book, you will find there asm examples.

Clearing bit N of TFLG1 can be done safely two ways:

a) using non RMW instruction like this:

MOVB  #(1<<N), TFLG1  ; in asm

TFLG1 = (1<<N);  //in C

b) using BCLR

BCLR  TFLG1, #~(1<<N)   ; in asm

TFLG1 &= (1<<N);          // in C. Pls don't mix with  TFLG1 &= ~(1<<N);

^^ this can clear (write 1 to) only Nth bit and nothing else. Only two cases are possible:

1) Nth bit is 0 before TFLG1 &=(1<<N);. Then  CPU will write to register

xxxxxxx0 & 00000001 = 00000000.

2) Nth bit is 1 before TFLG1 &=(1<<N);. Then 

xxxxxxx1 & 00000001 = 00000001

Regards

[dog]

Thanks Kef,

Good point!

It's mostly because I'm lazy and I use C headers that have nice easy to to use bit field definitions, so if I can say

TFLG1.bit1 = 1;

and know it is not going to cause any problems for what I'm doing (possibly because I'm not using any other timer channels) then that's what I'll do sometimes.

Not as clean as the pink books &= solution though, I certainly admit.

Dog

[kef]

Oh no, you shouldn't use bitfields to clear flags. Just think how C handles bitfields. When you write access one field - C has to preserve bits outside the field. This means C will automatically clear all set bits. Let's disassemble it

   71:     TFLG1_C0F=1;
  0000 4c0001       [4]     BSET  _TFLG1,#1

Ouch. Maybe clearing the bitfield could somehow magically help:

   72:     TFLG1_C0F=0;
  0003 4d0001       [4]     BCLR  _TFLG1,#1

Ouch again.

Are there other solutions with bit fields? Nope. Maybe there should be extension to C that could allow using bitfields with flags?

Regards

[dog]

I know that it generates BSET, that's the point. Like I said, only when I'm being lazy and know that this is not going to cause me problems.

In general I very much agree with you - you'll see in my previous posts that I advise that the simplest (and safest) rule is to stick to plain old assignment and avoid using read-modify-write operations on any register that contains write one to clear bits - and this is what I do in more complex code (say where I have several timer channels in use) or code I expect to get reused.

dog

[sjmelnikoff]

Having recently revisited the issues discussed in this thread, I think that some of the confusion may be based on the assumption that a BSET instruction is atomic, and sets a bit in a register without touching any of the other bits.

Unfortunately, as mentioned above, this is not how the instruction is implemented on the HCS12. It appears to be implemented as a read-modify-write, which is implied, though not stated explicity, in the CPU reference manual (S12CPUV2 rev 4.0, p140), where it describes the operation as:

Operation: (M) + (Mask) ⇒ M

Unhelpfully, it also states:

Description: Sets bits in memory location M. To set a bit, set the corresponding bit in the mask byte. All other bits in M are unchanged.

...which is not the case for flag registers.

So to reiterate what others have said here: C bitfield assignments are compiled to BSET, so are unsafe for flag registers if you're using other bits in the same register. Simple assignments have the desired effect.

Steve.

[allawtterb]

---

sjmelnikoff wrote:

Unfortunately, as mentioned above, this is not how the instruction is implemented on the HCS12. It appears to be implemented as a read-modify-write, which is implied, though not stated explicity, in the CPU reference manual (S12CPUV2 rev 4.0, p140), where it describes the operation as:

While it doesn't say anything about a read-modify-write in the text describing this, the Access Details does show this.  For all addressing modes a 'r' is present indicating an 8-bit read.  It would be nice for them to state it in the description of the command as they do in the HCS08 Reference Manual(Which also contains a section on the BSET and BCLR commands). 

[JimDon]

This is correct if and only if read the register clears a flag, and you need to test the state of that flag.
In most cases, this is not an issue.
Even if the BSET instruction was not used, there would have to be a read of a register or memory location if a bit was to be set without changing the state of the other bits,  C bitfields are not a special case.

If there was not a BSET instruction, and we are not use using C bitfields this:

  *x |= 4;

Would still generate a read modify cycle of some sort, even if it was to load a register, or the bit in and save it back.

How is it possible to set a bit with out first reading the rest of the byte?
Are proposing an new kind of memory that is bit addressable - because in this architecture io registers appear as memory.

In those few special cases, the user needs to know read the register first. It really does not matter how you do it, there will still be a read.




Message Edited by JimDon on 2008-04-21 08:29 AM

[sjmelnikoff]

Agreed. As an aside:

---

JimDon wrote:

Are proposing an new kind of memory that is bit addressable - because in this architecture io registers appear as memory.

---

Funny you should mention that, as we're doing some work with the STM32 processor, and it does indeed have bit-addressable memory, of a sort.

Each RAM location and reigster has an address in the memory map as normal. But in addition, each bit has its own address in a special "bit-banding" area of the memory space.

So if you want to write to one bit without affecting any others in the same word, you can write to its bit-band address instead. It's a bit different from using bitfields or masks, but does avoid some of the issues mentioned here.

Steve.

[JimDon]

---

Funny you should mention that, as we're doing some work with the STM32 processor, and it does indeed have bit-addressable memory, of a sort.

---

I've seen processors with instructions like that, but in the end they still have to read at least a byte of memory to do that. If you look at the guts of a memory array, they only decode to the byte level. Wide memories access more than a byte.

To add the logic to just pull a just bit from ram would be very costly, and hardly worth it. For one thing, you would have to add 8 more bits to the address bus to specify which bit you wanted.

[rocco]

Hi Mac,

---

BigMac wrote:

However, there might be a potential problem if a bset instruction was used to set a bit, other than the flag bit - the flag could be unintentionally cleared.

---

I believe that IS the problem. Inadvertantly clearing a flag.

Couple that with the fact that the C compiler generates BSET and BCLR instructions from statements like:

TSC_TSTOP=1;

An unsuspecting C programmer would never know where his/her firmware was going awry, without getting into the nuts & bolts of the MCU. Many companies have chosen C to avoid the nuts & bolts of the MCU.

Message Edited by rocco on 2007-08-23 02:55 PM

[bigmac]

Hello Steve,

I wonder if the problem might be related to the use of different SPI clock rates in each instance, especially if you are using a fast rate.  You seem to be saying that you are using SPI interrupt for a SPI master, and that you are enabling and disabling this interrupt at various times.

Keep in mind that, for a fast clock rate, it is possible for the ISR processing period to easily exceed the SPI transmission period, so receive overrun might occur, depending on your actual code.  This would apply to the processing period of any ISR that may occur once a SPI transmission is initiated.  You could try reducing the SPI clock frequency, and see if the problem persists.

Regards,

Mac

[sjmelnikoff]

Hi bigmac; thanks for your response.

I submitted this as a Technical Request to Freescale, and they suggested this (reproduced with permission):

---

Please try the following tip:

I remember similar problem with SPI after migration from old Dx256B to newer Dx256 device.

Please try to read SPISR register immediately after reset before writing to SPI config. registers:

(void)SPI1SR; // SPISR init

Unfortunately this problem is not documented yet.

You could add this line also after SPI configuration is completed before the first writing to SPIDR register.

---

As suggested, I added a dummy read of SPISR prior to SPI initialisation, and it worked perfectly!

Freescale have indicated that this issue will now be documented.

Steve.

[bigmac]

Hello Steve,

This appears to be a related issue to the following post, even though the latter refers to 8-bit devices.  Obviously, there is some commonality between the SPI modules for the S12 and 9S08 devices.

http://forums.freescale.com/freescale/board/message?board.id=8BITCOMM&thread.id=6713

In fact, the issue may already be documented for some MCUs, but not very prominently.

My understanding is that the read of SPISR is not related to writing to the configuration registers, but must occur somewhere prior to attempting the first write to SPIDR, but not necessarily within the SPI initialisation function.

Regards,

Mac

Message Edited by bigmac on 2007-07-25 11:15 PM