MSP432P401R: alignment errors from driverlib

I strongly suspect one can fix this by changing HWREG16 to HWREG8 and that will fix the problem.

but its too late at this point for the current version of the ROM. Any plans to rev the rom?

[A diff for a suggested change is at: 

https://github.com/tinyos/tinyos-main/commit/196f3883894739230b80778c5cd8e6fba84033f7?diff=split

]

Clemens Ladisch said:

The MSP430 driverlib code supports both 8-bit (P1,2,3,...) and 16-bit (PA,B,C,...) ports, and takes care to adjust the 'odd' 8-bit register values to work with the 16-bit accesses.

ah no.  that is the problem. they weren't careful enough.  What they do is when accessing an EVEN port they bump the base address by one.  ie   when accessing P1 they use base address P1 (4000_4c00), for P2 they use (4000_4c01).

And then when they actually do the memory access they use HWREG16.  ie.  HWREG16(baseAddress + OFS_LIB_PASEL0).    This is what causes the alignment trap.  They got away with it because by default alignment traps are turned off.

But there are good reasons to use alignment traps, has to do with looking for memory access performance problems.  (see my original post).

Clemens Ladisch said:

The MSP432 driverlib code is written so that it works only for 16-bit ports, but documents only the 8-bit ports.

I don't understand what you mean.  Could you elaborate?  I've been through the published driverlib code as of version 3.21.0.5 and I don't see anything that indicates it only works for 16 bit ports.  P1 is an 8 bit port.  Yes TI extended some 8 bit wide ports to 16 but they are still 8 bit ports.  The combination ports ie.  P1/P2 have always been laid out so a 16 bit access will work  (this started with the msp430f2618, the msp430f1611 didn't do it).  I've yet to see anyone ever actually use the ports this way.  You access the bits individually and don't really care that you can get them as 8 bits or 16 bits.

The documentation seems to imply 8 bit return values.  And the code actually returns 16 bit values (they used HWREG16 to do the accesses).  But the upper 8 bits are usually zero (depends on what is being accessed).

Clemens Ladisch said:

The examples use 8-bit ports, too, so this looks like a bug in the code.

Its a bug because they are using HWREG16  (cast to (uint16_t *)) to access an unaligned memory location.  When alignment traps are turned on this faults.  The fix is to change the HWREG16 to HWREG8.

However, changing from HWREG16 to HWREG8 breaks the 16 bit port accesses, ie PA ....  PJ.  It is my OSHO that the interface is broken.  It is trying to do too much.  A better abstraction would be a different call for the 16 bit ports.  ie.    GPIO16_<routine_name>.

Even better would be to change the code to eliminate the HWREG16 casts and convert to using the structures defined in msp432p401r.h.  Its a pain because of the interleaved ports but it works for all cases and doesn't generate unaligned traps if they are turned on.  Using casts and modifying offsets is problematic.

Here is how I handle it in TinyOS code.  The syntax is a little weird (nesc, Network Extended System C, a superset of C) but you get the idea.  And because of how the compiler works, it doesn't generate any extra code because the compiler does eliminates dead code.  It only generates instructions for the type of the port it is which is known at compile time.

I was talking not about MSP432 but MSP430. That code is correct, and similar logic should have been used for MSP432:

void GPIO_setAsOutputPin(uint8_t selectedPort,
                         uint16_t selectedPins) {
    uint16_t baseAddress = GPIO_PORT_TO_BASE[selectedPort];

    // Shift by 8 if port is even (upper 8-bits)
    if((selectedPort & 1) ^ 1)
    {
        selectedPins <<= 8;
    }

    HWREG16(baseAddress + OFS_PASEL) &= ~selectedPins;
    HWREG16(baseAddress + OFS_PADIR) |= selectedPins;
}

Clemens Ladisch said:

I was talking not about MSP432 but MSP430. That code is correct, and similar logic should have been used for MSP432:

my apologies, I saw msp430 and read msp432.  I understand what you are saying.

Using similar logic probably won't work on the 432.  The problem is TI is using a forced cast (HWREG16) to access an unaligned address.  This issue comes with using a real RISC.  I know TI calls the MSP430 a RISC machine but it isn't.