This thread has been locked.

If you have a related question, please click the "Ask a related question" button in the top right corner. The newly created question will be automatically linked to this question.

Multiplier error when using an ISR on the MSP430F5529

For way too long I have been chasing a problem where the multiplier result is corrupted when using the USB. It is not a problem limited to the USB, but is a problem whenever an ISR accesses an array or preforms a multiply. CCS MAY use the multiplier to calculate the address of a value stored in an array. (Sometimes it uses the multiplier to multiply the size of an element in the array by the index into the array by calling __mpyi_f5hw. Sometimes it uses shifts. I have not figured out what its criteria is.) This is not obvious looking at the c code, which makes it hard to find that multiply, but, when looking at the disassemble, it starts to become easier to see. Thus if an ISR is executed when the main code is in the middle of the use of the multiplier, which could be when the code is accessing an array or when it is explicitly doing a multiply and that ISR accesses an array the multiply result may be wrong.

This can cause trouble if you are using the code supplied by TI to do the USB and you are using arrays in your code even if you never use the multiplier explicitly because you are using it when you access arrays.

The solution is for ISRs that use arrays or the multiplier  to save the multiplier registers (on the stack) at the beginning of the ISR and restore them at the end. This is described in Section 23.2.7.1 Save and Restore, of slau208j.

I edited the USB ISR iUsbInterruptHandler(VOID) in UsbIsr.c to do that store and restore.

Here is what I did:

typedef struct {
    int mpy32ctl0;
    int res3;
    int res2;
    int res1;
    int res0;
    int mpy32h;
    int mpy32l;
    int opl2h;
    int opl2l;
}MULTIPLY_REG_TYPE;

At the beginning of the ISR

//Save multiplier registers so that its values may be restored at end of ISR
//See section 23.2.7.1 Save and Restore in SLAU208
   MULTIPLY_REG_TYPE mreg;
    mreg.mpy32ctl0 = MPY32CTL0;// Save multiplier mode, etc.
    MPY32CTL0 &= ~(MPYSAT | MPYFRAC);// Clear MPYSAT+MPYFRAC
    mreg.res3=RES3;// Save result 3
    mreg.res2=RES2;// Save result 2
    mreg.res1=RES1;// Save result 1
    mreg.res0=RES0;// Save result 0
    mreg.mpy32h=MPY32H;//Save operand 1, high word
    mreg.mpy32l=MPY32L;// Save operand 1, low word
    mreg.opl2h=OP2H;//Save operand 2, high word
    mreg.opl2l=OP2L;// Save operand 2, low word

At the end of the ISR

//restore multiplier registers
    OP2L=mreg.opl2l;//Restore operand 2, low word
    OP2H=mreg.opl2h;//Restore operand 2, high word
    MPY32L=mreg.mpy32l;// Restore operand 1, low word
    MPY32H=mreg.mpy32h;// Restore operand 1, high word
    RES0=mreg.res0;// Restore result 0
    RES1=mreg.res1;// Restore result 1
    RES2=mreg.res2;// Restore result 2
    RES3=mreg.res3;// Restore result 3
    MPY32CTL0=mreg.mpy32ctl0;// Restore multiplier mode, etc.

The compiler puts the data on the stack

Another solution would be to modify __mpyi_f5hw so that it stop interrupts during its execution. You would have to do the same when explicitly using the multiplier.

Good luck

Kent

  • I do not use c or CCS, but I understand the problem you described.

    I have the impression that some of the various MSP430 chips have a bug and save-then-restore MAC registers would not work correctly.

    Is it possible to tell the compiler to save-and-clear GIE before using MAC and restore GIE afterwards?

    Alternatively, is it possible to put all ISRs in a different module and tell the compiler to not use MAC unit for that module?

  • Kent Deines said:
    For way too long I have been chasing a problem where the multiplier result is corrupted when using the USB. It is not a problem limited to the USB, but is a problem whenever an ISR accesses an array or preforms a multiply.

    From MSP430F552x, MSP430F551x Device Erratasheet SLAZ054L there is erratum MPY1 which describes that problem:

    MPY1 Hardware Multiplier (MPY) Module

    Function Save and Restore feature on MPY32 not functional

    Description The MPY32 module uses the Save and Restore method which involves saving the multiplier state by pushing the MPY configuration/operand values to the stack before using the multiplier inside an Interrupt Service Routine (ISR) and then restoring the state by popping the configuration/operand values back to the MPY registers at the end of the ISR. However due to the erratum the Save and Restore operation fails causing the write operation to the OP2H register right after the restore operation to be ignored as it is not preceded by a write to OP2L register resulting in an invalid multiply operation.

    Workaround None. Disable interrupts when writing to OP2L and OP2H registers.

    Kent Deines said:
    Another solution would be to modify __mpyi_f5hw so that it stop interrupts during its execution. You would have to do the same when explicitly using the multiplier.

     The MPY1 erratum also contains the note:

    NOTE: When using the C-compiler, the interrupts are automatically disabled while using the MPY32.

    Thus, I would have expected the code generated by CCS to have disabled interrupts during use of the multiplier, and therefore you shouldn't have seen the problem.

    Which version of CCS and the MSP430 Compiler Tools are you using?

  • Kent Deines said:
    Sometimes it uses shifts. I have not figured out what its criteria is.

    If the array is an array of standard types, the element size is 1,2 or 4. In this case a simple on or two bit shift does the job. Only if the size of an element is different (e.g. because it is an array of structs of any size, inciuding 1,2 or 4), a multiplicaiton is neccessary. Even then, a clever combination of adds and shifts might still be faster in some cases, but detecting these cases is maybe too complex. :)

    Normally, the compiler should put a "push SR; DINT();...;pop SR" around any use of the MPY module. Doesn't help for NMI ISRs :(

    As already posted, saving the MPY registers will fail if you're right in the process of programming the MPY. Not to mention that saving the MPY state and restorign it adds maybe more latency to the ISR than just waiting for the current MPY operation to complete.
    Unless you have to do a complex MAC operation, but since C doesn't support this natively, a speedy MAC algorithm would be an assembly operation anyway.

    However, it may be that the multiplication for getting the array index does not use the normal multiplyer functions (as these are rather inefficient, something that has to do with the C language specs) but produces inline multiplyer code - and this code doesn't block ISRs as it should.

  • Thank you for responding.

    I am using Version: 5.2.0.00069.

    So far saving the multiplier data and restoring them has worked. That is not corrupting my multiplier result. Of course, I am only using 16 bit multiplies. Here is the the code the compiler uses when it calculated the address for an array, and it does disable the interrupts, so the only problem is that the programmer must disable interrupt when using the multiplier.

     __mpyi_f5hw:

    007834:   1202                PUSH    SR
    007836:   C232                DINT    
    007838:   4303                NOP     
    00783a:   4C82 04C0           MOV.W   R12,&MPY_16__Multiplier__16_Bit_Mode_MPY
    00783e:   4D82 04C8           MOV.W   R13,&MPY_16__Multiplier__16_Bit_Mode_OP2
    007842:   421C 04CA           MOV.W   &MPY_16__Multiplier__16_Bit_Mode_RESLO,R12
    007846:   4132                POP.W   SR
    007848:   0110                RETA  

    So the conclusion that the the only problem is that the programmer must disable interrupts when using the multiplier. Right?

     Kent

  • Kent Deines said:
    so the only problem is that the programmer must disable interrupt when using the multiplier.

    That's a bigger problem than one might think.
    You cannot simply use DINT and EINT around your multiplyer code. As thsi would enable interrutp seven if tehy have been disabled before on purpose.
    But you also cannot simply push the status register on stack as the compiler does. Because when pushing the SR to stack, you're changing the stack pointer and all references to local variables. Until you pop it back again, you must not use any local variables.

    I fell into this trap when trying to implement critical sections (not just the MPY) and finally came up with a macro that increments a counter, doe sDINT, then after the critical section decrements the counter and only does EINT when the counter reached 0.

    This allows nested critical sections. I think I posted this macro in this forum some time ago.

**Attention** This is a public forum