Shop OBEX P1 Docs P2 Docs Learn Events
help with i2c — Parallax Forums

help with i2c

Mark MaraMark Mara Posts: 64
edited 2013-06-11 22:56 in Propeller 1
I am having a problem getting i2c to work with XMMC. I am trying to read a DS3231 real time clock. The attached code works fine when compiled using LMM, however, it never returns from the i2cOpen call when compiled using XMMC.

The attached example uses pins 0 and 1 for the i2c bus. I get the same result if I move the DS3231 to pins 28 and 29 and use i2cBootOpen; it works fine using LMM, but never returns from the i2cBootOpen call using XMMC.

Any help would be greatly appreciated.

--markM
#include <stdio.h>
#include <i2c.h>
/* DS3231 pin assignments */
#define  _rtcDataPin   0
#define  _rtcClockPin  1
/* i2c addresses */
#define DS3231_I2C_WRITE_ADDR 0xd0 
#define DS3231_I2C_READ_ADDR  0xd1
/* DS3231 register offsets  */
#define DS3231_SECONDS_REG  0 
#define DS3231_MINUTES_REG  1 
#define DS3231_HOURS_REG    2  
#define DS3231_DATE_TIME_BYTE_COUNT 7 // Includes bytes 0-6
#define MAX_I2C_BUS 16 
#define TRUE    1
char bcd2bin(char bcd_value) 
{ 
  char temp; 
  temp = bcd_value; 
  temp >>= 1; 
  temp &= 0x78; 
  return(temp + (temp >> 2) + (bcd_value & 0x0f)); 
}
void read_DS3231(I2C *bus)
{
    int hour, minute, second, wday, mday, month, year;
    uint8_t  buffer[DS3231_DATE_TIME_BYTE_COUNT];
    printf("data regesters selected\n");       
/* read the data registers */
    if (i2cRead(bus, DS3231_I2C_READ_ADDR, buffer, DS3231_DATE_TIME_BYTE_COUNT, TRUE) != 0)
        printf("read failed\n");
    else 
    { 
        printf("good read\n");
        second = bcd2bin(buffer[DS3231_SECONDS_REG]);
        minute = bcd2bin(buffer[DS3231_MINUTES_REG]);
        hour = bcd2bin(buffer[DS3231_HOURS_REG]);
        printf("\d:d:d\n", hour, minute,second);            
    }
    return;
}
int main(void)
{
    I2C  *bus;
    I2C_COGDRIVER dev;
/* open i2c bus */    
    bus = i2cOpen(&dev, _rtcClockPin,_rtcDataPin, 4000);
    printf("i2cOpen returned x\n",bus);
/* select the DS3231 data registers */
    if (i2cWrite(bus, DS3231_I2C_WRITE_ADDR, 0, 1, TRUE) != 0)
        printf("Write failed\n"); 
    else
        read_DS3231(bus);    
    return 0;
}

Comments

  • David BetzDavid Betz Posts: 14,516
    edited 2013-06-03 07:02
    Mark Mara wrote: »
    I am having a problem getting i2c to work with XMMC. I am trying to read a DS3231 real time clock. The attached code works fine when compiled using LMM, however, it never returns from the i2cOpen call when compiled using XMMC.

    The attached example uses pins 0 and 1 for the i2c bus. I get the same result if I move the DS3231 to pins 28 and 29 and use i2cBootOpen; it works fine using LMM, but never returns from the i2cBootOpen call using XMMC.

    Any help would be greatly appreciated.

    --markM
    #include <stdio.h>
    #include <i2c.h>
    /* DS3231 pin assignments */
    #define  _rtcDataPin   0
    #define  _rtcClockPin  1
    /* i2c addresses */
    #define DS3231_I2C_WRITE_ADDR 0xd0 
    #define DS3231_I2C_READ_ADDR  0xd1
    /* DS3231 register offsets  */
    #define DS3231_SECONDS_REG  0 
    #define DS3231_MINUTES_REG  1 
    #define DS3231_HOURS_REG    2  
    #define DS3231_DATE_TIME_BYTE_COUNT 7 // Includes bytes 0-6
    #define MAX_I2C_BUS 16 
    #define TRUE    1
    char bcd2bin(char bcd_value) 
    { 
      char temp; 
      temp = bcd_value; 
      temp >>= 1; 
      temp &= 0x78; 
      return(temp + (temp >> 2) + (bcd_value & 0x0f)); 
    }
    void read_DS3231(I2C *bus)
    {
        int hour, minute, second, wday, mday, month, year;
        uint8_t  buffer[DS3231_DATE_TIME_BYTE_COUNT];
        printf("data regesters selected\n");       
    /* read the data registers */
        if (i2cRead(bus, DS3231_I2C_READ_ADDR, buffer, DS3231_DATE_TIME_BYTE_COUNT, TRUE) != 0)
            printf("read failed\n");
        else 
        { 
            printf("good read\n");
            second = bcd2bin(buffer[DS3231_SECONDS_REG]);
            minute = bcd2bin(buffer[DS3231_MINUTES_REG]);
            hour = bcd2bin(buffer[DS3231_HOURS_REG]);
            printf("\d:d:d\n", hour, minute,second);            
        }
        return;
    }
    int main(void)
    {
        I2C  *bus;
        I2C_COGDRIVER dev;
    /* open i2c bus */    
        bus = i2cOpen(&dev, _rtcClockPin,_rtcDataPin, 4000);
        printf("i2cOpen returned x\n",bus);
    /* select the DS3231 data registers */
        if (i2cWrite(bus, DS3231_I2C_WRITE_ADDR, 0, 1, TRUE) != 0)
            printf("Write failed\n"); 
        else
            read_DS3231(bus);    
        return 0;
    }
    
    Well, one minor problem is that you probably want %x in your printf statement although that doesn't explain why i2cOpen isn't returning. I may be able to try this late tonight or tomorrow if no one else comes up with a solution before that. Sorry for the delay! Today is a busy day for me.
  • Mark MaraMark Mara Posts: 64
    edited 2013-06-03 07:29
    Thanks for taking a look.

    If you paste <%02d> into the reply to thread box then switch to advanced mode it is changed to <d> that is why my printf(s) look weird.

    --markM
  • jazzedjazzed Posts: 11,803
    edited 2013-06-03 16:27
    Hi Mark. Welcome to the forums.

    There is some I2C discussion in the Learn forum http://forums.parallax.com/showthread.php/148348-Any-example-code-for-I2C
  • RaymanRayman Posts: 14,663
    edited 2013-06-04 09:52
    Sounds like maybe that I2C driver is missing the required "volatile", "HUBDATA", and "HUBTEXT" labels that will make it work in xmm mode...
  • David BetzDavid Betz Posts: 14,516
    edited 2013-06-04 10:03
    Rayman wrote: »
    Sounds like maybe that I2C driver is missing the required "volatile", "HUBDATA", and "HUBTEXT" labels that will make it work in xmm mode...
    volatile would be required in LMM and CMM modes as well and I believe his sample code allocates the I2C_COGDRIVER structure on the stack which is always in hub memory.
  • RaymanRayman Posts: 14,663
    edited 2013-06-04 10:36
    I'm curious about what's going on here then... I'm assuming that the i2c.h is an I2C driver that comes with propgcc...

    Should it work in xmm mode? Or, is there a known problem using it that way?
  • David BetzDavid Betz Posts: 14,516
    edited 2013-06-04 11:32
    Rayman wrote: »
    I'm curious about what's going on here then... I'm assuming that the i2c.h is an I2C driver that comes with propgcc...

    Should it work in xmm mode? Or, is there a known problem using it that way?
    Yes, it should work in xmm mode and I'm not sure why it doesn't. I'm planning on trying it tonight. I was busy last night and didn't have time.
  • David BetzDavid Betz Posts: 14,516
    edited 2013-06-04 11:35
    Here is one problem:
        if (i2cWrite(bus, DS3231_I2C_WRITE_ADDR, 0, 1, TRUE) != 0)
            printf("Write failed\n"); 
        else
            read_DS3231(bus);    
    
    This code passes zero as the third parameter to i2cWrite. That parameter is supposed to be a pointer to a buffer containing the data to write. That would be okay if the fourth parameter, number of bytes to write, was zero but in this case it is 1.
  • jazzedjazzed Posts: 11,803
    edited 2013-06-04 11:43
    Sounds like the i2c drivers could use some API candy for simple things.
    Most likely we will end up with libsimplei2c and other drivers ....
  • RaymanRayman Posts: 14,663
    edited 2013-06-04 12:12
    Ok, sounds like the problem isn't with the I2C driver then. That's good news.
    There are some things that I would like to code with this instead of the Spin2Cpp version of my own driver...

    BTW: Is there a list somewhere of all the different drivers that come with PropGCC.
    I didn't know about i2c.h. I'd guess there's an spi.h around somewhere too, right?
  • David BetzDavid Betz Posts: 14,516
    edited 2013-06-04 12:15
    Rayman wrote: »
    Ok, sounds like the problem isn't with the I2C driver then. That's good news.
    There are some things that I would like to code with this instead of the Spin2Cpp version of my own driver...

    BTW: Is there a list somewhere of all the different drivers that come with PropGCC.
    I didn't know about i2c.h. I'd guess there's an spi.h around somewhere too, right?
    I'm not saying there isn't a problem with the i2c driver. I will try it tonight to see if there is a bug in xmmc mode. At first glance it looks like it has volatile in the right places but there could certainly be a problem.

    No, there is no spi.h at the moment. Someone was working on a generic SPI driver but I don't think they ever checked it in and I don't recall who it was. Do you have a proposed API for SPI? It wouldn't be too hard to create a driver.
  • David BetzDavid Betz Posts: 14,516
    edited 2013-06-04 14:33
    Okay, there *is* a problem with the i2c driver in xmm mode. The problem is that the driver COG image gets placed in external memory by the linker and hence cognew doesn't work to load it since it can only handle COG images in hub memory. There are two possible solutions to this. One is to place the i2c driver image in hub memory. The other is to copy the image from external memory to a hub buffer before doing cognew. Both consume an additional 2k of hub memory but in the later case we can reuse the same 2k buffer for every COG image. I think that is probably the better approach. In fact, I'm pretty sure that's the way we load the SD driver now so I'm going to look into using the same mechanism for the i2c driver. Sorry about this!
  • RaymanRayman Posts: 14,663
    edited 2013-06-04 15:06
    Maybe PropGCC should reserve a cog launch pad area for all the drivers to share...
  • Mark MaraMark Mara Posts: 64
    edited 2013-06-04 16:55
    Thanks - It makes me feel better to know that there is a real problem and it was not just something stupid that I was doing.

    Regarding this code
    &#8203;
    if (i2cWrite(bus, DS3231_I2C_WRITE_ADDR, 0, 1, TRUE) != 0)
    
    It is writing a 1 byte of zero. I copied it from someone else's code. It seems to work fine. However, from your comment I assume that it is bad technique. I'll change it.

    --markM
  • jazzedjazzed Posts: 11,803
    edited 2013-06-04 17:31
    It's useful to look in a header (*.h) file to find the usage of a function. Often headers will have nothing more than function declarations which are useful for finding compile time errors and warnings. In this particular case the header gives lots more detail than just function declarations.

    From C:\propgcc\propeller-elf\include\i2c.h ....
    /**
     * @brief Write to an I2C device
     *
     * @details Write to an I2C device at the specified address.
     * The address should be the device address in bits 7:1 and
     * a zero in bit 0. If count is zero only the address byte
     * will be sent. Set the stop parameter to TRUE to cause an
     * I2C stop sequence to be emitted after the data. Setting it
     * to FALSE omits the stop sequence.
     *
     * @param dev I2C device to write to
     * @param address I2C address in bits 7:1, zero in bit 0
     * @param buffer Address of the buffer containing data to write
     * @param count Number of bytes of data to write
     * @param stop TRUE to send a stop sequence after the data
     *
     * @returns 0 on success, -1 on failure.
     *
     */
    static inline int i2cWrite(I2C *dev, int address, uint8_t *buffer, int count, int stop)
    {
        return (*dev->ops->write)(dev, address, buffer, count, stop);
    }
    


    That tells you everything you need to know about how to use the function. To me, it is glorious!
  • David BetzDavid Betz Posts: 14,516
    edited 2013-06-04 17:34
    Mark Mara wrote: »
    Thanks - It makes me feel better to know that there is a real problem and it was not just something stupid that I was doing.

    Regarding this code
    &#8203;
    if (i2cWrite(bus, DS3231_I2C_WRITE_ADDR, 0, 1, TRUE) != 0)
    
    It is writing a 1 byte of zero. I copied it from someone else's code. It seems to work fine. However, from your comment I assume that it is bad technique. I'll change it.

    --markM
    It only works fine because the byte at location zero is probably zero. It's just a happy coincidence! :-)

    Anyway, you found a real problem with i2cOpen that I am in the process of fixing. Thanks!!!!
  • David BetzDavid Betz Posts: 14,516
    edited 2013-06-04 20:41
    David Betz wrote: »
    It only works fine because the byte at location zero is probably zero. It's just a happy coincidence! :-)

    Anyway, you found a real problem with i2cOpen that I am in the process of fixing. Thanks!!!!
    Okay, I have a fix for this. It brings up some other driver loading issues thought that we should probably resolve before releasing the fix. Again, thanks for reporting this!
  • SRLMSRLM Posts: 5,045
    edited 2013-06-04 22:01
    David Betz wrote: »
    Okay, there *is* a problem with the i2c driver in xmm mode. The problem is that the driver COG image gets placed in external memory by the linker and hence cognew doesn't work to load it since it can only handle COG images in hub memory. There are two possible solutions to this. One is to place the i2c driver image in hub memory. The other is to copy the image from external memory to a hub buffer before doing cognew. Both consume an additional 2k of hub memory but in the later case we can reuse the same 2k buffer for every COG image. I think that is probably the better approach. In fact, I'm pretty sure that's the way we load the SD driver now so I'm going to look into using the same mechanism for the i2c driver. Sorry about this!

    Would it be possible to have cognew work in CMM/LMM and XMM?

    My guess is that it would be possible to use compile time flags to select the appropriate cognew at compile time. In CMM/LMM the cognew would be the same as it is now. In XMM, it would check out a lock, copy the 2K bytes, use the CMM/LMM cognew, and then release the lock.

    Anyway, that's just a suggestion. Otherwise, the onus is each and every developer to provide two methods of starting cog drivers, and that sounds like a hassle.
  • David BetzDavid Betz Posts: 14,516
    edited 2013-06-05 04:53
    SRLM wrote: »
    Would it be possible to have cognew work in CMM/LMM and XMM?

    My guess is that it would be possible to use compile time flags to select the appropriate cognew at compile time. In CMM/LMM the cognew would be the same as it is now. In XMM, it would check out a lock, copy the 2K bytes, use the CMM/LMM cognew, and then release the lock.

    Anyway, that's just a suggestion. Otherwise, the onus is each and every developer to provide two methods of starting cog drivers, and that sounds like a hassle.
    That is certainly possible but so far we've tried to keep the functions that have names that mach Propeller opcodes just be aliases for those opcodes. They currently translate directly into the corresponding instruction. Another complication is that you really need the size of the driver if you're going to copy to a hub memory buffer to avoid copying more data than is really required. We have some macros that help with this that I'm in the process of adding to propeller.h. They depend on a mechanism we setup to support COG C and PASM drivers that makes use of a linker feature to define symbols at the start and end of each driver image:
    use_cog_driverx(id)
    load_cog_driverx(id, param)
    
    The id is the name of the file containing the driver with ".c" for COG C drivers or ".spin" for PASM drivers replaced by "_cog". For example, the i2c driver is in the file "i2c_driver.c" and the id for it is "i2c_driver_cog". This id scheme is used because the linker produces symbols like "_load_start_i2c_driver_cog" and "_load_stop_i2c_driver_cog" to mark the start and end of the driver image in memory.
  • David BetzDavid Betz Posts: 14,516
    edited 2013-06-05 06:24
    I pushed a fix to the problem of using the i2c driver in XMM modes to the default branch of the PropGCC repository if anyone wants to try it. You'll have to check out the branch and build it yourself until we fold this fix into an official release.
  • Mark MaraMark Mara Posts: 64
    edited 2013-06-11 14:57
    Thanks, I built the modified release and my code now works correctly when compiled using XMMC. I now can move ahead on my project.

    A side effect of the change is that code that uses i2cBootOpen() gets the following error messages when attempting to compile. This code complies cleanly under the old version of propgcc. This is not a problem for me.
    1024 x 188 - 49K
  • SRLMSRLM Posts: 5,045
    edited 2013-06-11 16:35
    Are you using --gc-sections? That's one of the symptoms.
  • jazzedjazzed Posts: 11,803
    edited 2013-06-11 20:30
    Mark Mara wrote: »
    A side effect of the change is that code that uses i2cBootOpen() gets the following error messages when attempting to compile. This code complies cleanly under the old version of propgcc. This is not a problem for me.

    It seems like this is an issue David is working on. He's on vacation right now though.
  • David BetzDavid Betz Posts: 14,516
    edited 2013-06-11 22:09
    Mark Mara wrote: »
    Thanks, I built the modified release and my code now works correctly when compiled using XMMC. I now can move ahead on my project.

    A side effect of the change is that code that uses i2cBootOpen() gets the following error messages when attempting to compile. This code complies cleanly under the old version of propgcc. This is not a problem for me.
    The symbols for the start and end of a COG driver have changed since I'm no longer using the symbols that the linker generates automatically. I must have forgotten to change the symbol names in the i2cBootOpen function. I'll check into it. Thanks for noticing!
  • David BetzDavid Betz Posts: 14,516
    edited 2013-06-11 22:56
    David Betz wrote: »
    The symbols for the start and end of a COG driver have changed since I'm no longer using the symbols that the linker generates automatically. I must have forgotten to change the symbol names in the i2cBootOpen function. I'll check into it. Thanks for noticing!
    I've just pushed a fix for the i2cBootOpen() bug to the default branch. I'll update the kernel-work branch when I get back home on June 20th.
Sign In or Register to comment.