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.

TWL access bug

Other Parts Discussed in Thread: TPS65950

I came across a very obscure bug in the twl access routines in the A8_1.00.00 BSP release.

I was trying to determine why my RTC time from the TPS65950 was not working. I determined I was reading bad data from the TWL RTC registers (TWL I2C address group 0x4B). It was NOT throwing an access violations or anything like that ... it was just returning bad data.  The RTC routines in rtc.c use:

s_rtc.hTWL = TWLOpen();

TWLReadByteReg(s_rtc.hTWL, TWL_YEARS_REG, &bcdTime[5]);

etc. etc.

I tried rewriting the access routines using just the low level I2C1 routines and the data was returning correctly.

It seems the bug is in the twl_access.c file in the COMMON_TI_V1\TRITON\TWL\OAL directory. Upon further digging I determined the slaveAddress in the Device_t local static structure variable is not being initialized at all. It just so happens, in my case, it was being randomly set to "3". When I subsequently called TWLReadByteReg with an RTC register as the address the HIWORD is also "3" (see tps659xx_internals.h for example:

 #define TWL_SECURED_REG_B 0x00030001   its HIWORD would also be "3")

This causes the TWLReadRegs to not have the correct slave address leading to some nasty results.

 

Could someone at TI verify my findings?

 

This is how I corrected the problem.

4670.twl_access.c.txt

 

  • Also, Its not creating the critical section even thought it looks like it was intended.

  • David,

    The uninitialized variables you reported are real problems, we will address the issues in next release.

    FYI, the critical section is not actually used, since csInitialized is set to FALSE.

    Thanks,

    Tao

  • This issue is being tracked as SDOCM00077422

    (If the link does not work please flush your browser cache and try again).

  •  SDOCM00077422

    More information.. the root cause appears to be the access routines in the oal are not thread safe.

    The setting of the slave address and the data transfer need to be atomic. Also, the accesses are called from many places, rndis kitl (USB), RTC and power routines.

    I am not sure critical sections solves everything if they are called from power routines while they are running single threaded.

    My problem was the slave address was changing unintentually.

  • Hi David:

       Are you sure the bsp's version you used is A8_1.00.00? And I find your method  have been updated in the  A8_1.00.10.

      But I have a doubt with it. Before the A8_1.00.00, the TWL Group Address defined in the  head files is address + Group   . And  from A8_1.00.00,the define is the address in group.

    for example:

     In the old ,before A8_1.00.00:

    // usb
    #define TWL_VENDOR_ID_LO                0x00480000
    #define TWL_VENDOR_ID_HI                0x00480001
    #define TWL_PRODUCT_ID_LO               0x00480002
    #define TWL_PRODUCT_ID_HI               0x00480003
    #define TWL_FUNC_CTRL                   0x00480004
    #define TWL_FUNC_CTRL_SET               0x00480005
    #define TWL_FUNC_CTRL_CLR               0x00480006
    #define TWL_IFC_CTRL                    0x00480007
    #define TWL_IFC_CTRL_SET                0x00480008
    #define TWL_IFC_CTRL_CLR                0x00480009
    #define TWL_OTG_CTRL                    0x0048000A
    #define TWL_OTG_CTRL_SET                0x0048000B
    #define TWL_OTG_CTRL_CLR                0x0048000C
    #define TWL_USB_INT_EN_RISE             0x0048000D
    #define TWL_USB_INT_EN_RISE_SET         0x0048000E
    #define TWL_USB_INT_EN_RISE_CLR         0x0048000F
    #define TWL_USB_INT_EN_FALL             0x00480010
    #define TWL_USB_INT_EN_FALL_SET         0x00480011
    #define TWL_USB_INT_EN_FALL_CLR         0x00480012
    #define TWL_USB_INT_STS                 0x00480013
    #define TWL_USB_INT_LATCH               0x00480014
    #define TWL_DEBUG                       0x00480015
    #define TWL_SCRATCH_REG                 0x00480016
    #define TWL_SCRATCH_REG_SET             0x00480017

     

     

    In A8_1.00.00 is:

    //------------------------------------------------------------------------------
    // address group 0x48

    // usb
    #define TWL_VENDOR_ID_LO                0x00000000
    #define TWL_VENDOR_ID_HI                0x00000001
    #define TWL_PRODUCT_ID_LO               0x00000002
    #define TWL_PRODUCT_ID_HI               0x00000003
    #define TWL_FUNC_CTRL                   0x00000004
    #define TWL_FUNC_CTRL_SET               0x00000005
    #define TWL_FUNC_CTRL_CLR               0x00000006
    #define TWL_IFC_CTRL                    0x00000007
    #define TWL_IFC_CTRL_SET                0x00000008
    #define TWL_IFC_CTRL_CLR                0x00000009
    #define TWL_OTG_CTRL                    0x0000000A
    #define TWL_OTG_CTRL_SET                0x0000000B
    #define TWL_OTG_CTRL_CLR                0x0000000C
    #define TWL_USB_INT_EN_RISE             0x0000000D
    #define TWL_USB_INT_EN_RISE_SET         0x0000000E
    #define TWL_USB_INT_EN_RISE_CLR         0x0000000F
    #define TWL_USB_INT_EN_FALL             0x00000010
    #define TWL_USB_INT_EN_FALL_SET         0x00000011
    #define TWL_USB_INT_EN_FALL_CLR         0x00000012
    #define TWL_USB_INT_STS                 0x00000013
    #define TWL_USB_INT_LATCH               0x00000014

      and in the  A8_1.00.00,    Device.slaveAddress means address in group, and ((I2CContext_t*) Device->hI2C )->slaveAddress  is the real I2C Addr.  in the TWL.

    So I think ,there

    //BUG!!
    //        I2CSetSlaveAddress(Device.hI2C, BSPGetTritonSlaveAddress());
    //Correction: DV
    //++
            Device.slaveAddress = BSPGetTritonSlaveAddress();
            I2CSetSlaveAddress(Device.hI2C, Device.slaveAddress);
    //--
      

    should  be

            Device.slaveAddress =  0 ;
            I2CSetSlaveAddress(Device.hI2C,  BSPGetTritonSlaveAddress() );  //