Getting I2C(1), i.e. USCI_B1, to work on F5529 Launchpad

[hogemark 5]

Hi

 

I want to be able to use the second I2C module on my F5529 Launchpad. My project works fine with the default module 0, USCI_B0, using Energia.

But I want to also use the CC3000 Wifi module, and that seems to be using the USCI_B0 for SPI.

 

Therefore I think the simplest thing would be to enable support for the USCI_B1 in Energia.

I've spent some time now, and have done all the code changes needed, as far as I can see.

 

I've pushed my code to https://github.com/alfh/Energia/tree/feature_i2c_uscb1, the changes are here : https://github.com/alfh/Energia/compare/feature_i2c_uscb1?expand=1

 

 

What I've really done, is to change twi.c, so instead of using UCB0* registers and controls, it uses UCBx*, where the UCBx* are set to point to either UCB0 or UCB1, based on the "USE_USCI_B1".

 

 

The code is working fine if I don't define USE_USCI_B1, because then it sticks to using USCI_B0.

So the code in twi.c is working fine when using USBx* defined as USB0*.

 

But when I try to use USB1*, my test hangs on "Wire.endTransmission".

I'm using the same launchpad, just moving wires from P3.1 to P4.2, and P3.0 to P4.1.

 

I do not have a logic analyzer or other tools to check what is actually being sent on the wires.

 

Does anyone have a clue as to what I might have forgotten to change, or where I've made a mistake ?

 

I've also edited the twi.c, pins_energia.h and usci_isr_handler.c (not committed to github), removing the UCB0 stuff and only having UCB1 stuff, since I am unsure if the "#ifndef USE_USCI_B1" is working everywhere, it seems like it is not working properly in usci_isr_handler.c. 

I've also hacked and removed all the code related to "__MSP430_HAS_USCI__", "__MSP430_HAS_EUSCI_A0__", and "__MSP430_HAS_USI__", it then still works for USCI_B0.

But I still do not get the USCB1 to work.

 

I've not been able to find anything in the documentation that points to differences between USCI_B0 and USCI_B1, expect for different registers etc.

So I hope someone can help me get further.

 

Regards

Alf H

[chicken 630]

There seem still to be several places where you did not replace UCB0 with UCBx, e.g. lines 194, 196, 778, 779 - for the last one you changed the path for all that do not have an USCI.

 

If you plan to contribute your changes back to the main project, I'd avoid to rewrite all existing #ifdef's. You risk breaking I2C for MSP430s without USCI.

[hogemark 5]

Hi, and thanks for your review.

 

Are you referring to this construct

{code}

#if defined(__MSP430_HAS_USCI__)

    /* Set I2C state change interrupt mask */

    UCB0I2CIE |= (UCALIE|UCNACKIE|UCSTTIE|UCSTPIE);

    /* Enable state change and TX/RX interrupts */

    UC0IE |= UCB0RXIE | UCB0TXIE;

#else

    /* Set I2C state change interrupt mask and TX/RX interrupts */

    UCBxIE |= (UCALIE|UCNACKIE|UCSTTIE|UCSTPIE|UCRXIE|UCTXIE);

#endif

{code}

 

As I understand it, the MSP430's having only "__MSP430_HAS_USCI__", does not have "__MSP430_HAS_USCI_B0__" or "__MSP430_HAS_USCI_B1__", at least that is the case for 5529.

Therefore I've left UC0 (note no UCB0) for the "__MSP430_HAS_USCI__".

 

Can you elaborate on "778, 779 - for the last one you changed the path for all that do not have an USCI." ?

 

 

As to rewriting those #ifdef's, I was unsure about doing it. But as I understand the code, those are exclusive options :

"

 

#if defined(__MSP430_HAS_USI__)

 

#elif defined(__MSP430_HAS_USCI__) || defined(__MSP430_HAS_USCI_B0__) || defined(__MSP430_HAS_USCI_B1__)

 

#elif defined(__MSP430_HAS_EUSCI_B0__)

"

 

From page 1035 on http://www.ti.com/lit/ug/slau208m/slau208m.pdf), it seems like the MSP430x5xx and MSP430x6xx family might have EUSCI.

But from what I can see from the hardware/tools/msp430/msp430/include files (where the capabilities of each specific MSP430 is defined), there are no MSP430s that have both USCI_Bx and EUSCI_Bx, so I think they are exclusive, although I am not able to find that information in the pdf.

 

I think it makes the code clearer to have the #elif, if those options are in fact exclusive.

 

 

I've also edited, but not checked in, a twi.c file where all those #if are removed, and only code kept code for the  "__MSP430_HAS_USCI_B0__" or "__MSP430_HAS_USCI_B1__" case. It then works for USCI_B0, but not for USCI_B1.

The only thing I can think of, it that it has something to do with clocks, and the clock is not running for B1.

I think I will have a look at the DriverLib code and examples for I2C, and see if I can write a simple example for that for USCI_B0, and then change it to USCI_B1.

 

 

Regards

Alf

[hogemark 5]

Hi

 

I've now got a working driverlib example, where I2C master send is working for both UCSI_B0 and UCSI_B1, sending data to a TMP102 temperature sensor.

The code is here : https://github.com/alfh/Energia/commit/5c565e6430398633ccb436d38b01752a254f8a60

Unfortunately, I am not skilled enough to get the I2C master read, reading the temperature from the TMP102, to work, so I cannot use my driverlib example to confirm that I am able to use I2C to read temperature from TMP102.

I am a bit unsure on what slave address to use when receiving data from TMP102. The problem seems to be that I never get any interrupts on receive, i.e. I do not get any data back.

 

If I use 0x48 as slave address when reading data, the code hangs when receiving.

If I use 0x49, it never hangs, but it does not get any data.

 

I think maybe this is causing trouble

"

    __bis_SR_register(LPM0_bits + GIE);

"

since Energia might be using the interrupts for the "millis" time keeping.

 

I found a driverlib example for TMP102 here https://gist.github.com/donghee/5202156#file-tmp102-c-L56, but it seems to be for the driverlib for Tiva.

That code does not seem to be using interrupts at all.

 

 

But I think my example shows that USCI_B0 and USCI_B1 can be used, and that it might be something in the  Energia core that prevents my "twi.c" implementation using USCI_B1 from working.

I think maybe the Serial or SPI og Wire is somehow influencing the use of USCI_B0 and USCI_B1.

 

 

Regards

Alf

[hogemark 5]

Hi again

I've spent the day debugging this.

I've used CCS6 beta4, I have imported my Energia sketch there.

To me there seems to be some bugs in the i2c "state machine" implementation in Energia.

 

When I developer a pure CCS6 project using the msp430 driverlib, I get both the USCI_B0 and USCI_B1 to work.

 

Using the debugger in CCS6, I have learned about I2C today, and I think the debugger is showing me what happens, although I have observed some hiccups in debugger GUI, so I am not sure if I should fully trust it.

 

I am just trying to write "0b00000000" to the TMP102 sensor, by doing a master write.

 

To me, there seems to be some problems with how the interrupts are handled, and the setting of the twi_state.

 

Using the USCI_B1, the code gets stuck in this part of twi_writeTo :

"

    /* Wait for the transaction to complete */
    while(twi_state != TWI_IDLE) {
        __bis_SR_register(LPM0_bits);
    }
"

 

This code seems dangerrous (I've added the USCI_B1 stuff myself in git branch mentioned in first post)

"

#if defined(__MSP430_HAS_USCI_B0__)
__attribute__((interrupt(USCI_B0_VECTOR)))
void USCIB0_ISR(void)
{
    /* USCI_B0 I2C state change interrupt. */
    if ((UCB0CTL0 & UCMODE_3) == UCMODE_3 && (UCB0IFG & (UCALIFG | UCNACKIFG | UCSTTIFG | UCSTPIFG)) != 0)
        i2c_state_isr();
    /* USCI_B0 I2C TX RX interrupt. */
    if ((UCB0CTL0 & UCMODE_3) == UCMODE_3 && (UCB0IFG & (UCTXIFG | UCRXIFG)) != 0)
        i2c_txrx_isr();
}
#endif
#if defined(__MSP430_HAS_USCI_B1__)
__attribute__((interrupt(USCI_B1_VECTOR)))
void USCIB1_ISR(void)
{
    /* USCI_B1 I2C state change interrupt. */
    if ((UCB1CTL0 & UCMODE_3) == UCMODE_3 && (UCB1IFG & (UCALIFG | UCNACKIFG | UCSTTIFG | UCSTPIFG)) != 0)
        i2c_state_isr();
    /* USCI_B1 I2C TX RX interrupt. */
    if ((UCB1CTL0 & UCMODE_3) == UCMODE_3 && (UCB1IFG & (UCTXIFG | UCRXIFG)) != 0)
        i2c_txrx_isr();
}
#endif

"

 

I observe that for USCI_B0, "i2c_state_isr()" is called first, setting the twi_state to TWI_IDLE, and then when "i2c_txrx_isr" is being called, that method is actually performing a "slave transmit" and not a "master transmit", because of this if test, and the fact that the twi_state has been changed to TWI_IDLE from the original TWI_MTX :

"

        /* Master transmit mode */
        if (twi_state == TWI_MTX) {

.....
        /* Slave transmit mode (twi_state = TWI_STX) */
        } else {
...

"

Using USCI_B0, I get both a UCNACKIFG and a UCTXIFG as soon as the code does this :

"

    twi_state =  TWI_MTX;                  // Master Transmit mode
    UCBxCTL1 |= UCTXSTT;                   // I2C start condition

"

 

But what is strange, is that when I use USCI_B1, the interrupt to trigger "i2c_state_isr()" does not happen, and therefore somehow the status is never changed to TWI_IDLE, and therefore the code gets stuck in the while loop mentioned above. This is because on USCI_B1, I only get "UCTXIFG".

One time when running the debugger using USCI_B1, I saw the interrupt to trigger the "i2c_state_isr" happen.

 

When only one byte is transferred in master transit mode, how is the code meant to be triggered to set the twi_state to TWI_IDLE ?

 

Just wanted to keep you updated with some of my progress, and appreciate feedback.

 

My conclusions so far, is that there is some difference in the chip itself for USCI_B0 and USCI_B1, and that some bug in the Eneriga twi.c (state machine?) is causing the code to not work for USCI_B1.

 

Regards

Alf

[hogemark 5]

Hi

Some more progress.

I got it to work once more in the debugger. But thereafter not. And now I notice that when it does not work, after the "Wire.begin" has been executed, the UCB1STAT says "UCBBUSY" == 1. When it worked the one time in the debugger, the "UCBBUSY" was not 0.

 

I set the pin 9/10 (for USCI_B1) in the same way as pin 14/15 (for USCI_B0).

Could it be something floating ? (I have external pullup, and the setup works fine with the standalone driverlib example).

 

The bus is signalled as busy right after enabling by doing "UCBxCTL1 &= ~(UCSWRST)" :

"

    UCBxBR0 = (unsigned char)((F_CPU / TWI_FREQ) & 0xFF);
    UCBxBR1 = (unsigned char)((F_CPU / TWI_FREQ) >> 8);

    UCBxCTL1 &= ~(UCSWRST);

"

Just need to figure out why the "bus is busy".

 

Regards

Alf

[hogemark 5]

Ok, after reading the datasheet for F5529 and the family guide, I am pretty sure I know the reason.

For port 4.1 and 4.2 for USCI_B1, I have to use the port map controller to set up the ports correctly.

I'll investigate more, and let you know the results.

 

Alf

[hogemark 5]

Finally, got it to work !

I now have USCI_B1 working in Energia on F5529 Launchpad...

 

The problem is actually happening in wiring_digital.c in the method "pinMode_int", and is related to the port map controller for port4.

I noticed that my standalone driverlib example did not do any work against the port map controller.

When debugging the energia code, I could not see that the wiring_digital made any changes to the PortMap4 controller, except for presenting the password and unlocking the port map controller for a brief moment. But it does not seem to change the config for the P4.1 and P4.2.

Still, because of the port map controller code in "pinMode_int", the USCI_B1 was always (98% of cases when I debugged) telling "UCBBUSY".

 

So I just commented out all of this code :

"

//#if defined(__MSP430_HAS_PORT_MAPPING__)
//    volatile uint8_t *pmreg;
//    pmreg = portPMReg(port);
//
//    if(pmreg == NOT_A_PIN) return;
//
//    PMAPPWD = PMAPKEY;
//    PMAPCTL |= PMAPRECFG;
//    *(pmreg + bit_pos(bit)) = (mode >> 8) & 0xff;
//    PMAPPWD = 0x0;
//#endif
"

and now things are working.

 

I have read about the port map controller on page 426 in http://www.ti.com/lit/ug/slau208m/slau208m.pdf.

I have some doubts about the port map controller code above, because I observed using the debugger that port map controller stayed locked and would not unlock at the second attempt at writing to it, for set second call to "pinMode_int".

But I also have trouble understanding what input I should send to "pinMode_int" to set P4.1 and P4.2 to SDA / SDL and have the port map controller configure it correctly.

I think I need to use PORT_SELECTION0 and PM_UCB1SDA, PM_UCB1CLK somehow.

 

"

hardware/tools/msp430/msp430/include/msp430f5529.h:#define PM_UCB1SDA    15
hardware/tools/msp430/msp430/include/msp430f5529.h:#define PM_UCB1CLK    16
"

 

I would grealy appreciate if someone could suggest what input I should send to pinMode_int to configure the USCI_B1 SDA and SCL, or if someone could verify that the code for updating the port map controller in pinMode_int is correct and working properly.

 

 

While investigating, I read the documentation as good as I could, and found this in the family guide :

"

NOTE: Initializing or re-configuring the USCI module
The recommended USCI initialization/reconfiguration process is:
1.Set UCSWRST
2. Initialize all USCI registers with UCSWRST = 1.
3. Configure ports.
4. Clear UCSWRST via software
5. Enable interrupts(optional).
"

So I moved the code for setting ports a bit down in twi.c.
 

I'll take a break now, but will try to update my github repo in the coming days to have my latest updates, to show how to get USCI_B1 to work.

 

And then I'll have to investigate some more how to structure the code properly, so it is possible to use both USCI_B0 and USCI_B1 at the same time from Energia, this is https://github.com/energia/Energia/issues/298.

 

 

 

Regards

Alf

[hogemark 5]

Hi again

 

I've now pushed my latest code to GitHub, https://github.com/alfh/Energia/tree/feature_i2c_uscb1.

So using this code, I can use a #define to choose if I want to use USCI_B0 and USCI_B1 module.

The code it still not ready to be merged.

 

I need to figure out how to handle the "port map controller" code in wiring_digital.c, as of now I've had to comment that code out in order for my code to work.

 

And I want to be able to use both modules at the same time, so I guess more code restructuring is needed for that.

I would appreciate feedback on how you think support for more than one I2C module should be addded.

 

There are some general I2C initialization improvements in my code, so I should perhaps make a separate pull request for that.

 

I am also starting to wonder if the whole Wire/twi implementation for msp430 should be rewritten to resemble more the implementation for lm4f, where the "DriverLib" is being used, since the "driverlib for msp430" is now a part of Energia.

Comments to this ?

 

Regards

Alf

[hogemark 5]

Hi

I have now looked into the I2C implementation for Tiva in Energia, and investigated DriverLib for MSP430, which is only supported for 5x and 6x devices as of now.
The Tiva implementation seems to rely less on interrupts, and the code seems simpler and cleaner.


After doing some more investigation, I see the following alternatives for implementing support for having multiple I2C modules active on the MSP430 in Energia.

1.
Make a TwoWire class for MSP430, inspired by the Tiva TwoWire class in Energia, and use DriverLib for the implementation for 5x and 6x devices.
Just leave the current Wire implementation as is for non DriverLib devices.
If the Tiva TwoWire is good quality, and the DriverLib for MSP430 is good quality, this should reduce the amount of code in Energia for 5x and 6x devices.
But it would mean two different implementations of I2C for MSP430 devices, so a larger code base to maintain in total, I think.

2.
Make a TwoWire class, like in alternative 1, but do not use DriverLib at all, so then we could use the same implementation for all MSP430 devices.
This would involve the most effort because of rewriting, I think.

3.
Expand the current Wire class for MSP430, and move the code in twi.c/twi.h into the Wire.cpp, Wire.h class.
Make the Wire class handle multiple modules like the HardwareSerial.cpp currently does for MSP430.
I do not fully understand why the code is splitted in such a large degree among Wire.cpp and twi.c.

I think the usci_isr_handler.c should only contain the ISR handler for the "__MSP430_HAS_USCI__" case, i.e. where the USCI_Ax and USCI_Bx are sharing the same interupt handler.
The other interrupt handlers in that file should either be part of HardwareSerial or Wire.

4.
Leave the structure mainly as it is today, but add support for having two Wire instances, like it is done for HardwareSerial.
I have working code to have either USCI_B0 or USCI_B1 active, so just need to add code so there can be one instance per module.
My feeling is that it would not be very much new code that would be needed, but I feel the code is cluttered / complicated currently, because of all the different modules USI, USCI, USCI_Bx, EUSCI_Bx modules that needs to be supported.
I'm guessing there will never be a device with two USI modules or two USCI modules, so no need to pay attention to add support for such a possible scenario.

But my fear is that the twi.c will be even more cluttered, so therefore alternative 3, with more code in the Wire class and using instance member, would lead to cleaner code, I think.


I would appreciate feedback on what you think are the best approach to follow.
My favorites are alternatives 1 and 4, with number 3 also a good candidate.

Regards
Alf
 

[hogemark 5]

Hi again

 

I've tried to pursue option 1. and 2. above, but the I2C code in driverlib for Tiva and MSP430 are quite different, and my driverlib and i2c skills are not good enough, so I am abandoning those options.

 

I think I will now pursue option 4.

 

I've come a few steps further today.

 

I use CCSv6 for debugging, it is useful.

But debugging the port mapping setting proved to be difficult. At least until I found out that when running the debugger and stepping through, the port mapping register would always turn back to read-only before the code was able to manipulate the registers. So now I set a breakpoint after the changes to port map register has been set, and run until that breakpoint.

So I now think I understand the port mapping control fairly well.

 

So now I know what to send as input to the pinMode_int for USCI_B1, I use :

#define TWISDA_SET_MODE1 (PORT_SELECTION0 | (PM_UCB1SDA << 8) | INPUT)
#define TWISCL_SET_MODE1 (PORT_SELECTION0 | (PM_UCB1SCL << 8) | INPUT)
 

I've also changed the implementation in wiring_digital.c for pinMode_int regarding the setting of the port map control registers, and disable interrupts there, like suggested in the family guide.

The code now looks like this :

#if defined(__MSP430_HAS_PORT_MAPPING__)
    volatile uint8_t *pmreg;
    pmreg = portPMReg(port);

    if(pmreg == NOT_A_PIN) return;

    // Disable interrupts to avoid that the port map is put into read only mode
    __disable_interrupt();

    PMAPKEYID = PMAPKEY;
    PMAPCTL |= PMAPRECFG;
    *(pmreg + bit_pos(bit)) = (mode >> 8) & 0xff;
    // Make port map control read only by writing invalid password
    PMAPKEYID = 0x0;
    // We can now enable interrupts again
    __enable_interrupt();
#endif
 

 

I'm continuing to commit and push code to : https://github.com/alfh/Energia/tree/feature_i2c_uscb1.

My steps are small, but I think they are in forward direction.

 

Regards

Alf

[hogemark 5]

Hi

 

Finally, I have working code that allows me to use Wire1 for using the USCI_B1 module, and Wire for using the USCI_B0 module.

 

The code is here : https://github.com/alfh/Energia/tree/feature_i2c_uscb1

 

I still need to clean up the code, and do more initial testing, I have now just got it to work in my sample program, to use Wire1 for accessing the USCI_B1.

I am also confident that it will work using Wire in the same sample program for accessing the USCI_B0, but I have not tested that yet.

 

The code will also need review by experts..

I know some simple optimizations can be made to reduce the RAM usage, by having less instance variables in the Wire class.

I think the code got cleaner when getting rid of the twi.c/h, and having the Wire class handling everything.

 

The usci_isr_handler also got simpler, since it now only has to handle the case where uart and i2c are sharing the interrupt handler.

 

I just wanted to let you know about the progress, and appreciate feedback.

 

Regards

Alf

[Rei Vilo 695]

I've downloaded and compiled your Energia fork and I'm trying to test the library.

 

Could you please provide an example?

 

With the following code for the MSP430F5529,

#include "Wire.h"

I have the following error when compiling 

[/Users/ReiVilo/Desktop/Energia.app/Contents/Resources/Java/hardware/tools/msp430/bin/msp430-g++, -c, -g, -Os, -w, -ffunction-sections, -fdata-sections, -mmcu=msp430f5529, -DF_CPU=25000000L, -MMD, -DARDUINO=101, -DENERGIA=12, -I/Users/ReiVilo/Desktop/Energia.app/Contents/Resources/Java/hardware/msp430/cores/msp430, -I/Users/ReiVilo/Desktop/Energia.app/Contents/Resources/Java/hardware/msp430/variants/launchpad_f5529, /var/folders/5d/dky9xc691m32kztqnhkn8bhw0000gn/T/build8620168210632241345.tmp/F5529_two_I2C.cpp, -o, /var/folders/5d/dky9xc691m32kztqnhkn8bhw0000gn/T/build8620168210632241345.tmp/F5529_two_I2C.cpp.o]
In file included from F5529_two_I2C.ino:1:0:
/Users/ReiVilo/Desktop/Energia.app/Contents/Resources/Java/hardware/msp430/cores/msp430/Wire.h:36:2: error: #error "********** USI not available"
/Users/ReiVilo/Desktop/Energia.app/Contents/Resources/Java/hardware/msp430/cores/msp430/Wire.h:40:2: error: #error "********** USCI not available"

Thank you!

[hogemark 5]

Hi

 

Here is the sample program I've used for testing so far, testing on a F5529 Launchpad :

#include "Energia.h"
#include <Wire.h>


#define TMP102_ADDRESS 0x48


void setup()
{
  Serial.begin(9600);
    
  Serial.println("ready");
  delay(2000);
  Serial.println("go");
  delay(1000);
  Wire1.begin();
  delay(1000);
  Serial.println("done setup");
}

void loop()
{
  Serial.println("start reading");
  
  Wire1.beginTransmission(TMP102_ADDRESS);
  Wire1.requestFrom(TMP102_ADDRESS, 2);
  int16_t t=(Wire1.read() << 8 | Wire1.read()) >> 4;
  Wire1.endTransmission();
  
  Serial.println("done reading");

  if (t>0b100000000000) t -= 4096;

  // Float adds 6 kB
  Serial.print(t*0.0625, DEC);
  Serial.print(" ");

  // Integer
  t *= 10; // two decimal places
  t += 8; // rounding
  Serial.print(t/160, DEC);
  Serial.print(".");
  Serial.println((t%160)/160, DEC);
  delay(1500);
}

The delays in startup and loop are not needed, I've just added them for use when I debug.

 

I've running on energia-0101E0012 with the changes done on my branch by myself.

I think I got the same error as you at one point, but cannot remember what fixed it.

Maybe you miss the "Energia.h" include ?

There are some cleanup I have to do for the "includes".

I think that is causing your problem, because when I do not include it in my sample, I get the same error as you.

 

Have you removed the twi.c/.h file, like I've removed them from my branch ?

 

I will be busy for the next week, but thereafter I will do proper testing, and clean up and improve the code, so that it is ready for a pull request and a code review.

 

Regards

Alf

[Rei Vilo 695]

Thank you for your answer.

 

Maybe you miss the "Energia.h" include ?

 

Adding #include "Energia.h" solved the problem.

 

However, this pre-processing statement is optional, as the Energia IDE adds it automatically. So something goes wrong here.

 

I'm going to test my example now and keep you updated.