• State Resolved
  • Locked Locked
  • Replies 31 replies
  • Subscribers 99 subscribers
  • Views 6961 views
  • Users 0 members are here
Support feedback
Options
Options
Related

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.

How to implement the I2C "free data format"-read to free up the bus?

Ben Gardiner
Intellectual 280 points
Other Parts Discussed in Thread: OMAP-L138, TPS65070, OMAP-L137, AM1707, AM1808, TCA6416

Note: the problem is observed in the Linux kernel but the question is of a more general nature.

I and another user (Bastian Ruppert <Bastian.Ruppert@sewerin.de>) are experiencing "controller timed out" kernel messages. He has found that there is a suggested workaround for TI devices that do not have their SCL muxed with GPIOs -- the OMAP-L138 appears to be a such a device -- in the I2CTips wiki page. The suggested method follows:

"Option 2: Many devices don't mux SCL/SDA with GPIO since the I2C I/O cells are often special open drain cells. A workaround has been reported to work even on these devices. By configuring the I2C for "free data format" and then reading a byte the I2C will immediately start sending clocks to input data (rather than trying to send an address). This can be used to free up the bus."

Bastian and I have both tried to implement the workaround in the i2c-davinci driver but have not been successful. His implementation follows:

           flag |= DAVINCI_I2C_MDR_FDF;// free data format mode
           flag &= ~DAVINCI_I2C_MDR_TRX;// receive byte
           // Set STT to begin transmit now DXR is loaded
           flag |= DAVINCI_I2C_MDR_STT;
           davinci_i2c_write_reg(dev, DAVINCI_I2C_MDR_REG, flag);

Can someone provide us with a procedure for achieving SCL toggle on the OMAP-L138 using the "free data format"-read as described in the wiki page?

Any procedure at the level of the programming interface of the davinci i2c controller would be great. Example implementations of this workaround would also be great.

  • 0
    TI__Guru*** 125430 points

    That particular section is talking about a specific case where the processor has been reset, but the I2C slave device has not.  Your case sounds a bit different.  That said, here's what I recommend:

    • Do you know what happened on the bus leading up to this "controller timed out" message? Is SCL or SDA being held low?
    • What's the state of the I2C itself when this happens, i.e. can you dump the I2C registers before you try this?
    • Have you tried to reset the I2C controller, e.g. through PSC?  If so, does that cause SCL/SDA to be released?
    • After you've reset the I2C controller, then you should try your procedure for the FDF read.

     

  • 0 in reply to Brad Griffis
    Intellectual 430 points

    Hello,

    Brad Griffis said:
    • Do you know what happened on the bus leading up to this "controller timed out" message? Is SCL or SDA being held low?

    For this point i can say that the SDA wire is low in my environment! Shortcutting the SCL line with ground frees the 

    i2c bus until the next freeze.

    Regards,

     

    Bastian.

  • 0 in reply to Bastian
    TI__Guru*** 125430 points

     

    • What hardware are you using? Is it the experimenter kit, or your own hardware? 
    • Is there much noise on SCL/SDA?
    • Are you always communicating with the same slave device when the lockup occurs?  What is the slave device?
    • What clock frequency do you see on SCL, and how are CLKL/CLKH programmed?  FYI, I'm asking because there is a noise glitch removal circuit built into the I2C, but in order for it to work correctly the prescaled clock frequency (after PSC but before CLKL/CLKH) should be between 7-12 MHz.
  • 0 in reply to Brad Griffis
    Intellectual 280 points

    Hi Brad. 
    Thanks for your rapid reply.
    • What hardware are you using? Is it the experimenter kit, or your own hardware? 
    • Are you always communicating with the same slave device when the lockup occurs?  What is the slave device?
    The problem is occuring for me on the da850evm from LogicPD with the UI board connected -- but not when the UI Board is disconnected (yet).
    I have:
    • REV A SOM
    • REV 4 Baseboard and 
    • REV 7 UI Board.
    As best I can tell the problem is occuring in communication to the tca416 GPIO I2C expanders -- error messages appear for both 0x20 and 0x21 addresses. There are polling tasks, 2x @200ms, that read the expander state for the purpose of reporting the pushbutton and DIP switch inputs.
    I'm affraid I can't (yet) answer some of your questions since I haven't got a scope hooked up to the system exhibiting the problem behaviour:
    • Do you know what happened on the bus leading up to this "controller timed out" message? Is SCL or SDA being held low?
    • Is there much noise on SCL/SDA?
    • What clock frequency do you see on SCL, and how are CLKL/CLKH programmed?  FYI, I'm asking because there is a noise glitch removal circuit built into the I2C, but in order for it to work correctly the prescaled clock frequency (after PSC but before CLKL/CLKH) should be between 7-12 MHz.

    I'll come back to answer these questions as soon as I have.

    • What's the state of the I2C itself when this happens, i.e. can you dump the I2C registers before you try this?

    I can hack up a register dump routine and hook it into the bus-recovery to get you this information. I expect to have it available at the beginning of next week.

    • Have you tried to reset the I2C controller, e.g. through PSC?  If so, does that cause SCL/SDA to be released?
    • After you've reset the I2C controller, then you should try your procedure for the FDF read.

    The driver does not currently reset the i2c controller before performing bus recovery; however, the PSC register is available -- I think it should be possible to implement a PSC reset before FDF read. I'll try this after I've got you the register dump and answered the three 'signal' related questions above.

    Thanks again for your help, Brad. 

    Best Regards,
    Ben Gardiner

    Nanometrics Inc.
    http://nanometrics.ca

  • 0 in reply to Brad Griffis
    Intellectual 430 points

    Hello,

    Brad Griffis said:
    • What hardware are you using? Is it the experimenter kit, or your own hardware? 
    • Is there much noise on SCL/SDA?
    • Are you always communicating with the same slave device when the lockup occurs?  What is the slave device?


    We use the SOM OMAPL138-10-1502QHCR from logicpd on a custom board.
    The problem occurs while communicating with the pmic TPS65070RSL
    touchscreen interface (i2c address 0x48). The I2C is running on 100 kHz.
    We have a i2c repeater solded on our board (PCA9515AD). With this repeater
    i can reproduce the error pretty fast.
    Without this repeater solded on, the mean time between failure is much higher.

    The signals are looking very clean. On the oscilloscope it seems the
    slave released the SDA line after 4-8ms, nevertheless the line is low
    after that. 


    Brad Griffis said:
    What clock frequency do you see on SCL, and how are CLKL/CLKH programmed?  FYI, I'm asking because there is a noise glitch removal circuit built into the I2C, but in order for it to work correctly the prescaled clock frequency (after PSC but before CLKL/CLKH) should be between 7-12 MHz.

    Some register values after the error has been occured:
    i2c_davinci i2c_davinci.1: MDR = 0x2e20
    i2c_davinci i2c_davinci.1: CLKL = 0x23
    i2c_davinci i2c_davinci.1: CLKH = 0x23
    i2c_davinci i2c_davinci.1: PSC = 0x2
     
    The prescaled module clock frequency is with 8MHz in the valid
    range. (The I2C input clock frequency is 24MHz).
    We see the expected 100KHz I2C clock on the oscilloscope.I tried
    frequencies of 25kHz and 400kHz before, with the same behaviour.


    Can you see a problem in the Mode Register?

    Regards,

    Bastian.

  • 0 in reply to Ben Gardiner
    Intellectual 280 points

    Ben Gardiner said:

     

    • What's the state of the I2C itself when this happens, i.e. can you dump the I2C registers before you try this?

    I can hack up a register dump routine and hook it into the bus-recovery to get you this information. I expect to have it available at the beginning of next week.

    [/quote]

    I applied the attached patch to get the following output:

    pca953x 1-0021: failed reading register

    i2c_davinci i2c_davinci.1: controller timed out

    i2c_davinci i2c_davinci.1: PSC  = 00000002

    i2c_davinci i2c_davinci.1: CLKL = 00000023

    i2c_davinci i2c_davinci.1: CLKH = 00000023

    i2c_davinci i2c_davinci.1: MDR = 00002620

    i2c_davinci i2c_davinci.1: OAR = 00000008

    i2c_davinci i2c_davinci.1: IMR = 00000077

    i2c_davinci i2c_davinci.1: STR = 00000400

    i2c_davinci i2c_davinci.1: CNT = 00000001

    i2c_davinci i2c_davinci.1: DRR = 000000ff

    i2c_davinci i2c_davinci.1: SAR = 00000021

    i2c_davinci i2c_davinci.1: DXR = 00000000

    i2c_davinci i2c_davinci.1: IVR = 00000000

    i2c_davinci i2c_davinci.1: EMDR = 00000001

    i2c_davinci i2c_davinci.1: initiating i2c bus recovery

    pca953x 1-0021: failed reading register

    i2c_davinci i2c_davinci.1: controller timed out

    i2c_davinci i2c_davinci.1: PSC  = 00000002

    i2c_davinci i2c_davinci.1: CLKL = 00000023

    i2c_davinci i2c_davinci.1: CLKH = 00000023

    i2c_davinci i2c_davinci.1: MDR = 00002620

    i2c_davinci i2c_davinci.1: OAR = 00000008

    i2c_davinci i2c_davinci.1: IMR = 00000077

    i2c_davinci i2c_davinci.1: STR = 00000400

    i2c_davinci i2c_davinci.1: CNT = 00000001

    i2c_davinci i2c_davinci.1: DRR = 000000ff

    i2c_davinci i2c_davinci.1: SAR = 00000021

    i2c_davinci i2c_davinci.1: DXR = 00000000

    i2c_davinci i2c_davinci.1: IVR = 00000000

    i2c_davinci i2c_davinci.1: EMDR = 00000001

    i2c_davinci i2c_davinci.1: initiating i2c bus recovery

    pca953x 1-0021: failed reading register

    i2c_davinci i2c_davinci.1: controller timed out

    i2c_davinci i2c_davinci.1: PSC  = 00000002

    i2c_davinci i2c_davinci.1: CLKL = 00000023

    i2c_davinci i2c_davinci.1: CLKH = 00000023

    i2c_davinci i2c_davinci.1: MDR = 00002620

    i2c_davinci i2c_davinci.1: OAR = 00000008

    i2c_davinci i2c_davinci.1: IMR = 00000077

    i2c_davinci i2c_davinci.1: STR = 00000400

    i2c_davinci i2c_davinci.1: CNT = 00000001

    i2c_davinci i2c_davinci.1: DRR = 000000ff

    i2c_davinci i2c_davinci.1: SAR = 00000020

    i2c_davinci i2c_davinci.1: DXR = 00000000

    i2c_davinci i2c_davinci.1: IVR = 00000000

    i2c_davinci i2c_davinci.1: EMDR = 00000001

    i2c_davinci i2c_davinci.1: initiating i2c bus recovery

    pca953x 1-0020: failed reading register

    i2c_davinci i2c_davinci.1: controller timed out

    i2c_davinci i2c_davinci.1: PSC  = 00000002

    i2c_davinci i2c_davinci.1: CLKL = 00000023

    i2c_davinci i2c_davinci.1: CLKH = 00000023

    i2c_davinci i2c_davinci.1: MDR = 00002620

    i2c_davinci i2c_davinci.1: OAR = 00000008

    i2c_davinci i2c_davinci.1: IMR = 00000077

    i2c_davinci i2c_davinci.1: STR = 00000400

    i2c_davinci i2c_davinci.1: CNT = 00000001

    i2c_davinci i2c_davinci.1: DRR = 000000ff

    i2c_davinci i2c_davinci.1: SAR = 00000020

    i2c_davinci i2c_davinci.1: DXR = 00000000

    i2c_davinci i2c_davinci.1: IVR = 00000000

    i2c_davinci i2c_davinci.1: EMDR = 00000001

    i2c_davinci i2c_davinci.1: initiating i2c bus recovery

    pca953x 1-0020: failed reading register

    [...]

     

    diff --git a/drivers/i2c/busses/i2c-davinci.c b/drivers/i2c/busses/i2c-davinci.cindex 5795c83..7aa5cc5 100644
    --- a/drivers/i2c/busses/i2c-davinci.c
    +++ b/drivers/i2c/busses/i2c-davinci.c
    @@ -133,6 +133,37 @@ static inline u16 davinci_i2c_read_reg(struct davinci_i2c_dev *i2c_dev, int reg)
      return __raw_readw(i2c_dev->base + reg);
     }
     
    +static void dump(struct davinci_i2c_dev *dev)
    +{
    + dev_info(dev->dev, "PSC  = %08x\n",
    + davinci_i2c_read_reg(dev, DAVINCI_I2C_PSC_REG));
    + dev_info(dev->dev, "CLKL = %08x\n",
    + davinci_i2c_read_reg(dev, DAVINCI_I2C_CLKL_REG));
    + dev_info(dev->dev, "CLKH = %08x\n",
    + davinci_i2c_read_reg(dev, DAVINCI_I2C_CLKH_REG));
    + dev_info(dev->dev, "MDR = %08x\n",
    + davinci_i2c_read_reg(dev, DAVINCI_I2C_MDR_REG));
    +
    + dev_info(dev->dev, "OAR = %08x\n",
    + davinci_i2c_read_reg(dev, DAVINCI_I2C_OAR_REG));
    + dev_info(dev->dev, "IMR = %08x\n",
    + davinci_i2c_read_reg(dev, DAVINCI_I2C_IMR_REG));
    + dev_info(dev->dev, "STR = %08x\n",
    + davinci_i2c_read_reg(dev, DAVINCI_I2C_STR_REG));
    + dev_info(dev->dev, "CNT = %08x\n",
    + davinci_i2c_read_reg(dev, DAVINCI_I2C_CNT_REG));
    + dev_info(dev->dev, "DRR = %08x\n",
    + davinci_i2c_read_reg(dev, DAVINCI_I2C_DRR_REG));
    + dev_info(dev->dev, "SAR = %08x\n",
    + davinci_i2c_read_reg(dev, DAVINCI_I2C_SAR_REG));
    + dev_info(dev->dev, "DXR = %08x\n",
    + davinci_i2c_read_reg(dev, DAVINCI_I2C_DXR_REG));
    + dev_info(dev->dev, "IVR = %08x\n",
    + davinci_i2c_read_reg(dev, DAVINCI_I2C_IVR_REG));
    + dev_info(dev->dev, "EMDR = %08x\n",
    + davinci_i2c_read_reg(dev, DAVINCI_I2C_EMDR_REG));
    +}
    +
     /* Generate a pulse on the i2c clock pin. */
     static void generic_i2c_clock_pulse(unsigned int scl_pin)
     {
    @@ -157,6 +188,7 @@ static void i2c_recover_bus(struct davinci_i2c_dev *dev)
      u32 flag = 0;
      struct davinci_i2c_platform_data *pdata = dev->dev->platform_data;
     
    + dump(dev);
      dev_err(dev->dev, "initiating i2c bus recovery\n");
      /* Send NACK to the slave */
      flag = davinci_i2c_read_reg(dev, DAVINCI_I2C_MDR_REG);

     

  • 0 in reply to Ben Gardiner
    Intellectual 280 points

    Ben Gardiner said:

    • Have you tried to reset the I2C controller, e.g. through PSC?  If so, does that cause SCL/SDA to be released?
    • After you've reset the I2C controller, then you should try your procedure for the FDF read.

    The driver does not currently reset the i2c controller before performing bus recovery; however, the PSC register is available -- I think it should be possible to implement a PSC reset before FDF read. I'll try this after I've got you the register dump and answered the three 'signal' related questions above.

    [/quote]

    I noticed while getting the register dump that the 'PSC' register defined by the driver, 'DAVINCI_I2C_PSC_REG' 0x30 aka Prescaler Regsiter , is _not_ the PSC I was thinking of -- the Power State Controller --  were you referring to the former or the latter?

    Since the Power State Controller is an SoC specific element, creating a patch that will reset the controller using it will require some extra effort.

  • 0
    TI__Guru*** 125430 points

    Ben Gardiner said:
    He has found that there is a suggested workaround for TI devices that do not have their SCL muxed with GPIOs -- the OMAP-L138 appears to be a such a device

    Back up...  The OMAP-L138 does have the ability to configure SCL/SDA as GPIO.  This capability is built into the I2C module itself (for this particular device).  Here's the sort of thing I would try for this situation:

     
    // UNTESTED CODE!!!

    unsigned int counter=0;

    ICMDR = 0; // put I2C in reset
    ICPDIR = 1; // SCL output, SDA input
    ICPFUNC = 1; // change to GPIO mode
    ICPDSET = 1; // SCL high
    five_microsecond_delay();

    while ( (ICPDIN&2 != 2) && (counter<8) ) {

    // pulse SCL
    ICPDCLR = 1; // SCL low
    five_microsecond_delay();
    ICPDSET = 1; // SCL high
    five_microsecond_delay();

    counter++;

    }

    ICPFUNC = 0; // change to I2C mode

    if (ICPDIN&2 != 2)
       printf("I2C slave will not release SDA.\n");

     

     

  • 0 in reply to Brad Griffis
    TI__Guru*** 125430 points

    Brad Griffis said:
    • Have you tried to reset the I2C controller, e.g. through PSC?  If so, does that cause SCL/SDA to be released?

    Sorry, let me clarify on this point since PSC has 2 meanings.  The first and easiest way to reset the I2C is by writing ICMDR = 0.  That's probably good enough, but if you wanted to go a step further you could reset the I2C through the Power and Sleep Controller (PSC).

  • 0 in reply to Bastian
    TI__Guru*** 125430 points

    Bastian said:
    The signals are looking very clean. On the oscilloscope it seems the
    slave released the SDA line after 4-8ms, nevertheless the line is low
    after that. 

    So if I understand you correctly when this issue occurs SDA is high on the slave side of the PCA9515AD but it is being held low on the processor side.  Is that correct?  Is that always the behavior you see, or sometimes do you see SDA low on both sides of the PCA9515AD, which is what I would expect.

    I think it's important to understand in this condition whether the OMAP-L138 is holding SDA low or if the PCA9515AD is doing so.  How is the EN signal of the PCA9515AD connected?  Is it just pulled high, or does the OMAP-L138 control it?  A simple experiment might be to reset the OMAP-L138 (reset button, emulator, etc) and then see the state of SDA.  After the OMAP-L138 has been reset it would definitely release SDA if it is the culprit (though I can't think of any reason why it would do this).

    In debugging this situation you can likely workaround the problem by toggling SCL in GPIO mode, but I would recommend doing a little more investigation to see if we can determine the root cause of the issue.  Your end product will be more robust if we can prevent the issue in addition to having the workaround present just in case.

  • 0 in reply to Brad Griffis
    Intellectual 280 points

    Hi Brad,

    Thanks for clearing up that this chip does have GPIO capability in the controller and for the example code -- I hope to be experimenting with this aspect later in the week.

    Brad Griffis said:

    I think it's important to understand in this condition whether the OMAP-L138 is holding SDA low or if the PCA9515AD is doing so.  How is the EN signal of the PCA9515AD connected?  Is it just pulled high, or does the OMAP-L138 control it?  A simple experiment might be to reset the OMAP-L138 (reset button, emulator, etc) and then see the state of SDA.  After the OMAP-L138 has been reset it would definitely release SDA if it is the culprit (though I can't think of any reason why it would do this).

    I'll have to put my investigations on hold for the week; but for now I have been able to connect a logic analyzer to SDA and SCL at the baseboard i2c expander (with the UI board connected as before).

    I've noticed that the problem did not occur with the analyzer leads connected -- I waited a couple hours which has been more than long enough w/o the leads connected.

    I disconnected the leads and shortly thereafter observed the controller timeout messages.

    I re-connected the leads and observed that SDA was held low and there was no SCL activity (b/c the driver code does nothing for omap-l138 ATM).

    I reset the OMAP-L138 using the "S5 RST" pushbutton on the baseboard and observed that there was no change to SDA : it stayed low during the reset of the OMAP-L138. Note that in my case it is not a PCA9515AD on the bus.

    Best Regards,

    Ben Gardiner

  • 0 in reply to Ben Gardiner
    Intellectual 430 points

    Dear Brad Griffis, dear Ben,

    for better comparing i used Bens patch for register output,
    thank you for that:

    tps6507x 1-0048: ADC config read failed
    i2c_davinci i2c_davinci.1: controller timed out
    i2c_davinci i2c_davinci.1: PSC  = 00000002
    i2c_davinci i2c_davinci.1: CLKL = 00000023
    i2c_davinci i2c_davinci.1: CLKH = 00000023
    i2c_davinci i2c_davinci.1: MDR = 00002e20
    i2c_davinci i2c_davinci.1: OAR = 00000008
    i2c_davinci i2c_davinci.1: IMR = 00000077
    i2c_davinci i2c_davinci.1: STR = 00000400
    i2c_davinci i2c_davinci.1: CNT = 00000002
    i2c_davinci i2c_davinci.1: DRR = 00000077
    i2c_davinci i2c_davinci.1: SAR = 00000048
    i2c_davinci i2c_davinci.1: DXR = 00000007
    i2c_davinci i2c_davinci.1: IVR = 00000000
    i2c_davinci i2c_davinci.1: EMDR = 00000001
    i2c_davinci i2c_davinci.1: initiating i2c bus recovery
    i2c_davinci i2c_davinci.1: controller timed out
    i2c_davinci i2c_davinci.1: PSC  = 00000002
    i2c_davinci i2c_davinci.1: CLKL = 00000023
    i2c_davinci i2c_davinci.1: CLKH = 00000023
    i2c_davinci i2c_davinci.1: MDR = 00002e20
    i2c_davinci i2c_davinci.1: OAR = 00000008
    i2c_davinci i2c_davinci.1: IMR = 00000077
    i2c_davinci i2c_davinci.1: STR = 00000400
    i2c_davinci i2c_davinci.1: CNT = 00000002
    i2c_davinci i2c_davinci.1: DRR = 00000077
    i2c_davinci i2c_davinci.1: SAR = 00000048
    i2c_davinci i2c_davinci.1: DXR = 00000008
    i2c_davinci i2c_davinci.1: IVR = 00000000
    i2c_davinci i2c_davinci.1: EMDR = 00000001
    i2c_davinci i2c_davinci.1: initiating i2c bus recovery
    tps6507x 1-0048: TSC mode read failed
    i2c_davinci i2c_davinci.1: controller timed out
    i2c_davinci i2c_davinci.1: PSC  = 00000002


    Brad Griffis said:
    So if I understand you correctly when this
    issue occurs SDA is high on the slave side of the PCA9515AD but it is
    being held low on the processor side.  Is that correct?  Is that
    always the behavior you see, or sometimes do you see SDA low on both
    sides of the PCA9515AD, which is what I would expect.

    I think it's important to understand in this condition whether the
    OMAP-L138  is holding SDA low or if the PCA9515AD is doing so.  How is
    the EN signal of the PCA9515AD connected?  Is it just pulled high, or
    does the OMAP-L138  control it?  A simple experiment might be to reset
    the OMAP-L138  (reset button, emulator, etc) and then see the state of
    SDA.  After the OMAP-L138  has been reset it would definitely release
    SDA if it is the culprit (though I can't think of any reason why it
    would do this).


    The EA-pin of the PCA9515AD is not connected!

    A reset of the L138 doesn`t work, a power down - power up cycle is
    needed to get rid of the low SDA-line. (I think that points to slave
    holding the line?!)

    I think the PCA9515AD is working as expected:

    If the repeater is disabled via the EA-pin after error (SDA Low), SDA
    is low on master side and high on the slave side.

    If the repeater is disabled by the EA-Pin and therefore the slave side
    is disconnected (and high pegel), it is easy to generate the error. 
    It seems the PCA9515AD is intensifying the problematic but not the
    reason. When not solded the PCA9515AD on ,the error is much harder to
    generate, only, but it is still possible.

    Brad Griffis said:
    Back up...  The OMAP-L138  does have the
    ability to configure SCL/SDA as GPIO.  This capability is built into
    the I2C module itself (for this particular device).  Here's the sort
    of thing I would try for this situation:

    // UNTESTED CODE!!!
    ...


    Thank you for this workaround, i will try it as soon as i am not so busy
    any more.

    Brad Griffis said:
    In debugging this situation you can likely
    workaround the problem by toggling SCL in GPIO mode, but I would
    recommend doing a little more investigation to see if we can determine
    the root cause of the issue.  Your end product will be more robust if
    we can prevent the issue in addition to having the workaround present
    just in case.


    I absolutely agree!

    Ben Gardiner said:

    I have:

        * REV A SOM
        * REV 4 Baseboard and
        * REV 7 UI Board.


    Dear Ben,
    are you forced to follow the errata ID E0019 mentioned by Sekhar Nori before?

    Title:
    Communication issues with I2C interface on PMIC
    Affected Part Number(s):
    1014650
    1014651
    1014652
    1014613

    Description:
    The I2C interface on TPS65070 may experience communication issues
    because buffer U24 only allows unidirectional communication.

    I do not know if your som is affected by this issue.
    But if so, perhaps you can remove the bridge to interrupt the I2C part
    of the TPS65070RSL from the bus at least with the clock?

    I would like to know if the TPS65070RSL is causing trouble, but its
    not easy to seperate this device from the bus.

    Thanks and best regards,

    Bastian.

  • 0 in reply to Bastian
    Intellectual 280 points

    Bastian said:

    for better comparing i used Bens patch for register output,
    thank you for that:

     

    You're welcome.

    Bastian said:


    I have:

        * REV A SOM
        * REV 4 Baseboard and
        * REV 7 UI Board.



    Dear Ben,
    are you forced to follow the errata ID E0019 mentioned by Sekhar Nori before?

    Title:
    Communication issues with I2C interface on PMIC
    Affected Part Number(s):
    1014650
    1014651
    1014652
    1014613

    Description:
    The I2C interface on TPS65070 may experience communication issues
    because buffer U24 only allows unidirectional communication.

    I do not know if your som is affected by this issue.
    But if so, perhaps you can remove the bridge to interrupt the I2C part
    of the TPS65070RSL from the bus at least with the clock?

    [/quote]

    Thanks for the details of the errata. I wasn't able to get access to it through LogicPD's site.

    My SOM is affected by this issue -- part number 1014650.

    I guess it is possible that it could be the cause; I had ruled it out b/c I have disabled CPU frequency scaling to prevent transitions to 1.0v OPP. I don't think I will be able to get my SOM re-worked, sorry.

     

  • 0 in reply to Ben Gardiner
    Intellectual 430 points

    Hello,

    Brad Griffis said:

    // UNTESTED CODE!!!

    unsigned int counter=0;

    ICMDR = 0; // put I2C in reset
    ICPDIR = 1; // SCL output, SDA input
    ICPFUNC = 1; // change to GPIO mode
    ICPDSET = 1; // SCL high
    five_microsecond_delay();

    while ( (ICPDIN&2 != 2) && (counter<8) ) {

    // pulse SCL
    ICPDCLR = 1; // SCL low
    five_microsecond_delay();
    ICPDSET = 1; // SCL high
    five_microsecond_delay();

    counter++;

    }

    ICPFUNC = 0; // change to I2C mode

    if (ICPDIN&2 != 2)
       printf("I2C slave will not release SDA.\n");



    ...this code seems to work.

    With the following patch i have a very dirty workaround for the issue but still
    no idea where it comes from. (Not usable in field).


    Regards,

    Bastian.

    diff --git a/drivers/i2c/busses/i2c-davinci.c b/drivers/i2c/busses/i2c-davinci.c
    index 5795c83..e4102dc 100644
    --- a/drivers/i2c/busses/i2c-davinci.c
    +++ b/drivers/i2c/busses/i2c-davinci.c
    @@ -65,6 +65,11 @@
     #define DAVINCI_I2C_IVR_REG    0x28
     #define DAVINCI_I2C_EMDR_REG    0x2c
     #define DAVINCI_I2C_PSC_REG    0x30
    +#define DAVINCI_I2C_PFUNC_REG    0x48
    +#define DAVINCI_I2C_PDIR_REG    0x4c
    +#define DAVINCI_I2C_PDIN_REG    0x50
    +#define DAVINCI_I2C_DSET_REG    0x58
    +#define DAVINCI_I2C_DCLR_REG    0x5c
     
     #define DAVINCI_I2C_IVR_AAS    0x07
     #define DAVINCI_I2C_IVR_SCD    0x06
    @@ -185,6 +190,63 @@ static inline void davinci_i2c_reset_ctrl(struct davinci_i2c_dev *i2c_dev,
         davinci_i2c_write_reg(i2c_dev, DAVINCI_I2C_MDR_REG, w);
     }
     
    +/* Generate gpio a pulse on the i2c clock pin. */
    +static int generic_i2c_clock_pulse_gpio(struct davinci_i2c_dev *dev,unsigned int pulses)
    +{
    +    u32 flag = 0;
    +    unsigned int counter=0;
    +
    +    dev_err(dev->dev, "i2c_clock_pulse_gpio\n");
    +
    +    /* ICMDR = 0; // put I2C in reset
    +       Note that if IRS is reset during a transfer, it can
    +       cause the I2C bus to hang (I2Cx_SDA and I2Cx_SCL are
    +       in a high-impedance state). */
    +    davinci_i2c_reset_ctrl(dev,0);
    +
    +    davinci_i2c_write_reg(dev, DAVINCI_I2C_MDR_REG, 0x00);
    +
    +    /* ICPDIR = 1; // SCL output, SDA input */
    +    davinci_i2c_write_reg(dev, DAVINCI_I2C_PDIR_REG, 0x01);
    +
    +    /* ICPFUNC = 1; // change to GPIO mode */
    +    davinci_i2c_write_reg(dev, DAVINCI_I2C_PFUNC_REG, 0x01);
    +
    +    /* ICPDSET = 1; // SCL high */
    +    davinci_i2c_write_reg(dev, DAVINCI_I2C_DSET_REG, 0x01);
    +    udelay(5);
    +
    +    for(counter = 0;counter<pulses;counter++){
    +      /*      flag = davinci_i2c_read_reg(dev, DAVINCI_I2C_PDIN_REG);
    +      if(flag&2)
    +        {
    +          break;
    +          }*/
    +      // pulse SCL
    +      /* ICPDCLR = 1; // SCL low */
    +      davinci_i2c_write_reg(dev, DAVINCI_I2C_DCLR_REG, 0x01);
    +      udelay(5);
    +      /* ICPDSET = 1; // SCL high */
    +      davinci_i2c_write_reg(dev, DAVINCI_I2C_DSET_REG, 0x01);
    +      udelay(5);
    +    }
    +
    +    dev_err(dev->dev, "i2c_clock_pulse_gpio generated\n");
    +
    +    /* ICPFUNC = 0; // change to I2C mode */
    +    davinci_i2c_write_reg(dev, DAVINCI_I2C_PFUNC_REG, 0x00);
    +
    +    /* ICMDR = 1; // put I2C out of reset */
    +    davinci_i2c_reset_ctrl(dev,0);
    +
    +    flag = davinci_i2c_read_reg(dev, DAVINCI_I2C_PDIN_REG);
    +    if (flag&2){
    +      return 0;
    +    }
    +    dev_err(dev->dev, "I2C slave will not release SDA.\n");
    +    return -1;
    +}
    +
     static void i2c_davinci_calc_clk_dividers(struct davinci_i2c_dev *dev)
     {
         struct davinci_i2c_platform_data *pdata = dev->dev->platform_data;
    @@ -381,6 +443,11 @@ i2c_davinci_xfer_msg(struct i2c_adapter *adap, struct i2c_msg *msg, int stop)
                                   dev->adapter.timeout);
         if (r == 0) {
             dev_err(dev->dev, "controller timed out\n");
    +
    +        if(!generic_i2c_clock_pulse_gpio(dev,8)) {
    +           return -ETIMEDOUT;
    +        }
    +
             i2c_recover_bus(dev);
             i2c_davinci_init(dev);
             dev->buf_len = 0;

  • 0 in reply to Bastian
    TI__Guru*** 125430 points

    Bastian,

    I'm glad to hear you have a suitable workaround.  That's great news.  In order to find the root cause I think a great starting point would be to actually see a screenshot of one of these conditions on a scope.  Can you add some additional code (temporarily) into your workaround?  Specifically, I'd like you to toggle any GPIO that you can easily access.  That way you can connect an oscilloscope to your board.  You can hook up SCL, SDA, and the GPIO.  You would then configure the scope to trigger based on your GPIO.  If you set the trigger to be near the end of the sampling window then we should be able to look back and see the final transfer that occurred and messed things up.

    Can you get that done and post some screenshots?  If not, Ben can you do that?

    Best regards,
    Brad

  • 0 in reply to Brad Griffis
    Intellectual 280 points

    Brad Griffis said:

    Can you add some additional code (temporarily) into your workaround?  Specifically, I'd like you to toggle any GPIO that you can easily access.  That way you can connect an oscilloscope to your board.  You can hook up SCL, SDA, and the GPIO.  You would then configure the scope to trigger based on your GPIO.  If you set the trigger to be near the end of the sampling window then we should be able to look back and see the final transfer that occurred and messed things up.

    Can you get that done and post some screenshots?  If not, Ben can you do that?

    I did add code to toggle a gpio when the condition occurs -- see attached. Unfortunately I cannot reproduce the problem with either oscilloscope or (USB-) logic analyzer probes attached to either 1) SDA only , 2) SCL only or 3) both SDA and SCL.

    • Is there much noise on SCL/SDA?
    • What clock frequency do you see on SCL, and how are CLKL/CLKH programmed?  FYI, I'm asking because there is a noise glitch removal circuit built into the I2C, but in order for it to work correctly the prescaled clock frequency (after PSC but before CLKL/CLKH) should be between 7-12 MHz.

     

    There is not much noise on either SDA or SCL.

    I am observing a 99KHz SCL frequency -- according to my scope.

     

    diff --git a/arch/arm/mach-davinci/board-da850-evm.c b/arch/arm/mach-davinci/board-da850-evm.cindex b01fb2a..78cb2e8 100644

    --- a/arch/arm/mach-davinci/board-da850-evm.c

    +++ b/arch/arm/mach-davinci/board-da850-evm.c

    @@ -964,6 +964,11 @@ static int __init da850_evm_config_emac(void)

      pr_warning("da850_evm_init: emac registration failed: %d\n",

      ret);

     

    + ret = davinci_cfg_reg(DA850_GPIO7_13);

    + if (ret)

    + pr_warning("da850_evm_init:GPIO(7,13) mux setup "

    + "failed\n");

    +

      return 0;

     }

     device_initcall(da850_evm_config_emac);

    diff --git a/arch/arm/mach-davinci/da850.c b/arch/arm/mach-davinci/da850.c

    index 78b5ae2..1d051f5 100644

    --- a/arch/arm/mach-davinci/da850.c

    +++ b/arch/arm/mach-davinci/da850.c

    @@ -549,6 +549,7 @@ static const struct mux_config da850_pins[] = {

      MUX_CFG(DA850, GPIO4_0, 10, 28, 15, 8, false)

      MUX_CFG(DA850, GPIO4_1, 10, 24, 15, 8, false)

      MUX_CFG(DA850, RTC_ALARM, 0, 28, 15, 2, false)

    + MUX_CFG(DA850, GPIO7_13, 16, 16, 15, 8, false)

     #endif

     };

     

    diff --git a/arch/arm/mach-davinci/include/mach/mux.h b/arch/arm/mach-davinci/include/mach/mux.h

    index de11aac..cae5f86 100644

    --- a/arch/arm/mach-davinci/include/mach/mux.h

    +++ b/arch/arm/mach-davinci/include/mach/mux.h

    @@ -914,6 +914,8 @@ enum davinci_da850_index {

      DA850_GPIO4_0,

      DA850_GPIO4_1,

      DA850_RTC_ALARM,

    +

    + DA850_GPIO7_13,

     };

     

     enum davinci_tnetv107x_index {

    diff --git a/drivers/i2c/busses/i2c-davinci.c b/drivers/i2c/busses/i2c-davinci.c

    index 7aa5cc5..41e7598 100644

    --- a/drivers/i2c/busses/i2c-davinci.c

    +++ b/drivers/i2c/busses/i2c-davinci.c

    @@ -188,6 +188,11 @@ static void i2c_recover_bus(struct davinci_i2c_dev *dev)

      u32 flag = 0;

      struct davinci_i2c_platform_data *pdata = dev->dev->platform_data;

     

    + gpio_direction_output(125, 1);

    + gpio_set_value(125, 0);

    + udelay(20);

    + gpio_set_value(125, 1);

    +

      dump(dev);

      dev_err(dev->dev, "initiating i2c bus recovery\n");

      /* Send NACK to the slave */

     

     

     

  • 0 in reply to Ben Gardiner
    TI__Guru*** 125430 points

    Ben Gardiner said:
    I did add code to toggle a gpio when the condition occurs -- see attached. Unfortunately I cannot reproduce the problem with either oscilloscope or (USB-) logic analyzer probes attached to either 1) SDA only , 2) SCL only or 3) both SDA and SCL.

    So are you sure you still see this issue even with the new code applied (with no scope attached)?

    What pullup strength is being used on the bus?  It seems like adding a tiny bit of load (i.e. the probe) is enough to make the issue go away.  Perhaps you need to make the I2C pullups a little weaker.  ???

    I didn't see any files attached.  What do you see for rise and fall times on the SCL and SDA lines?

    FYI, I'll be traveling this week so may not be able to reply.

  • 0 in reply to Bastian
    Intellectual 280 points

    Thanks for the patch, Bastian. 

     

    I've modified it a little please see attached -- I'm going to work to get the patch integrated into mainline.

     

    There was one little wrinkle I found: 

    +    /* ICMDR = 1; // put I2C out of reset */
    +    davinci_i2c_reset_ctrl(dev,0);

     

    should pass 1 as second arg to release from reset.

     

    With the attached path applied I found that the controller was able to recover -- repeatedly -- from the 'controller timed out' errors.

    I think it is noteworthy that there seems to be no room for variations in the recovery procedure -- modifying any of the 1) initial SCL phase or length of delay in toggle results in a failure to recover the bus.

    I was, however, able to modify the number of toggles to get fewer "I2C slave will not release SDA."; that is, if I set the count to 16, the recovery procedure occurs but doesn't print the "I2C slave will not release SDA." the first but not second time as it does with count=8.

    with count=8:

    pca953x 1-0021: failed reading register

    i2c_davinci i2c_davinci.1: controller timed out

    i2c_davinci i2c_davinci.1: initiating i2c bus recovery

    i2c_davinci i2c_davinci.1: I2C slave will not release SDA.

    pca953x 1-0021: failed reading register

    i2c_davinci i2c_davinci.1: controller timed out

    i2c_davinci i2c_davinci.1: initiating i2c bus recovery

    pca953x 1-0021: failed writing register

    pca953x 1-0021: failed reading register

    i2c_davinci i2c_davinci.1: controller timed out

    i2c_davinci i2c_davinci.1: initiating i2c bus recovery

    i2c_davinci i2c_davinci.1: I2C slave will not release SDA.

    pca953x 1-0021: failed reading register

    i2c_davinci i2c_davinci.1: controller timed out

    i2c_davinci i2c_davinci.1: initiating i2c bus recovery

    pca953x 1-0021: failed writing register

     

    with count=16:

    pca953x 1-0021: failed reading register

    i2c_davinci i2c_davinci.1: controller timed out

    i2c_davinci i2c_davinci.1: initiating i2c bus recovery

    pca953x 1-0021: failed reading register

    pca953x 1-0021: failed reading register

    i2c_davinci i2c_davinci.1: controller timed out

    i2c_davinci i2c_davinci.1: initiating i2c bus recovery

    pca953x 1-0021: failed reading register

     

    But I'm still getting back-to-back "controller timed out" messages; perhaps a delay is needed after the SCL toggle?

     

     

    diff --git a/drivers/i2c/busses/i2c-davinci.c b/drivers/i2c/busses/i2c-davinci.cindex 54960d7..ef493a6 100644

    --- a/drivers/i2c/busses/i2c-davinci.c

    +++ b/drivers/i2c/busses/i2c-davinci.c

    @@ -65,6 +65,11 @@

     #define DAVINCI_I2C_IVR_REG 0x28

     #define DAVINCI_I2C_EMDR_REG 0x2c

     #define DAVINCI_I2C_PSC_REG 0x30

    +#define DAVINCI_I2C_PFUNC_REG 0x48

    +#define DAVINCI_I2C_PDIR_REG 0x4c

    +#define DAVINCI_I2C_PDIN_REG 0x50

    +#define DAVINCI_I2C_DSET_REG 0x58

    +#define DAVINCI_I2C_DCLR_REG 0x5c

     

     #define DAVINCI_I2C_IVR_AAS 0x07

     #define DAVINCI_I2C_IVR_SCD 0x06

    @@ -98,6 +103,29 @@

     #define DAVINCI_I2C_IMR_NACK BIT(1)

     #define DAVINCI_I2C_IMR_AL BIT(0)

     

    +/* set SDA and SCL as GPIO */

    +#define DAVINCI_I2C_PFUNC_PFUNC0 BIT(0)

    +

    +/* set SCL as output when used as GPIO*/

    +#define DAVINCI_I2C_PDIR_PDIR0 BIT(0)

    +/* set SDA as output when used as GPIO*/

    +#define DAVINCI_I2C_PDIR_PDIR1 BIT(1)

    +

    +/* read SCL GPIO level */

    +#define DAVINCI_I2C_PDIN_PDIN0 BIT(0)

    +/* read SDA GPIO level */

    +#define DAVINCI_I2C_PDIN_PDIN1 BIT(1)

    +

    +/*set the SCL GPIO high */

    +#define DAVINCI_I2C_DSET_PDSET0 BIT(0)

    +/*set the SDA GPIO high */

    +#define DAVINCI_I2C_DSET_PDSET1 BIT(1)

    +

    +/* set the SCL GPIO low */

    +#define DAVINCI_I2C_DCLR_PDCLR0 BIT(0)

    +/* set the SDA GPIO low */

    +#define DAVINCI_I2C_DCLR_PDCLR1 BIT(1)

    +

     struct davinci_i2c_dev {

      struct device           *dev;

      void __iomem *base;

    @@ -180,6 +208,64 @@ static void generic_i2c_clock_pulse(unsigned int scl_pin)

      }

     }

     

    +static inline void davinci_i2c_reset_ctrl(struct davinci_i2c_dev *i2c_dev,

    + int val);

    +

    +/* Generate gpio a pulse on the i2c clock pin. */

    +static int generic_i2c_clock_pulse_gpio(struct davinci_i2c_dev *dev)

    +{

    + u32 flag = 0;

    + u16 i;

    +

    + davinci_i2c_reset_ctrl(dev, 0);

    +

    + davinci_i2c_write_reg(dev, DAVINCI_I2C_MDR_REG, 0x00);

    +

    + /* SCL output, SDA input */

    + davinci_i2c_write_reg(dev, DAVINCI_I2C_PDIR_REG,

    + DAVINCI_I2C_PDIR_PDIR0);

    +

    + /* change to GPIO mode */

    + davinci_i2c_write_reg(dev, DAVINCI_I2C_PFUNC_REG,

    + DAVINCI_I2C_PFUNC_PFUNC0);

    +

    + /*

    + * NOTE: testing has shown that other variations of this SCL-toggle

    + * routine do not work:

    + * - SCL-low first,

    + * - 9 toggles,

    + * - 20us delays.

    + */

    +

    + /* SCL high */

    + davinci_i2c_write_reg(dev, DAVINCI_I2C_DSET_REG,

    + DAVINCI_I2C_DSET_PDSET0);

    + udelay(5);

    + for (i = 0; i < 8; i++) {

    + /* SCL low */

    + davinci_i2c_write_reg(dev, DAVINCI_I2C_DCLR_REG,

    + DAVINCI_I2C_DCLR_PDCLR0);

    + udelay(5);

    + /* SCL high */

    + davinci_i2c_write_reg(dev, DAVINCI_I2C_DSET_REG,

    + DAVINCI_I2C_DSET_PDSET0);

    + udelay(5);

    + }

    +

    + /* change to I2C mode */

    + davinci_i2c_write_reg(dev, DAVINCI_I2C_PFUNC_REG, 0);

    +

    + /* take the I2C dev out of reset */

    + davinci_i2c_reset_ctrl(dev, 1);

    +

    + flag = davinci_i2c_read_reg(dev, DAVINCI_I2C_PDIN_REG);

    + if (flag & DAVINCI_I2C_PDIN_PDIN1)

    + return 0;

    +

    + dev_err(dev->dev, "I2C slave will not release SDA.\n");

    + return -1;

    +}

    +

     /* This routine does i2c bus recovery as specified in the

      * i2c protocol Rev. 03 section 3.16 titled "Bus clear"

      */

    @@ -195,8 +281,10 @@ static void i2c_recover_bus(struct davinci_i2c_dev *dev)

      flag |=  DAVINCI_I2C_MDR_NACK;

      /* write the data into mode register */

      davinci_i2c_write_reg(dev, DAVINCI_I2C_MDR_REG, flag);

    - if (pdata)

    + if (pdata->scl_pin)

      generic_i2c_clock_pulse(pdata->scl_pin);

    + else

    + generic_i2c_clock_pulse_gpio(dev);

      /* Send STOP */

      flag = davinci_i2c_read_reg(dev, DAVINCI_I2C_MDR_REG);

      flag |= DAVINCI_I2C_MDR_STP;

     

     

  • 0 in reply to Ben Gardiner
    TI__Guru*** 125430 points

    Ben Gardiner said:

    + for (i = 0; i < 8; i++) {

    + /* SCL low */

    + davinci_i2c_write_reg(dev, DAVINCI_I2C_DCLR_REG,

    + DAVINCI_I2C_DCLR_PDCLR0);

    + udelay(5);

    + /* SCL high */

    + davinci_i2c_write_reg(dev, DAVINCI_I2C_DSET_REG,

    + DAVINCI_I2C_DSET_PDSET0);

    + udelay(5);

    + }

    Why don't you check the state of SDA after each loop?  Personally I think that would give you the most robust code for unlocking the bus.  The bus gets locked up as a result of the master and the slave getting out of sync.  The master is trying to send a stop condition which requires SDA to transition high.  However, the slave is holding SDA low because it thinks it's transmitting a bit of data (0).  It's not clear at any given moment exactly how far out of sync the master and the slave have gotten so you really need to keep pulsing until the slave let's SDA go high and then send a stop bit.  If you continue pulsing then the slave might transmit another zero and pull SDA low again.

    In the case of a master receiver the master is expected to NACK the final byte being received such that the slave knows to stop sending data.  According to the spec, once you get to this point the master should either issue a stop condition or a repeated start.  As far as I can tell if the master continues pulsing SCL the behavior is not defined by the spec.  So conceivably (in my opinion) the slave may keep trying to send data.  I assume this behavior will be dependent upon the individual implementation.  So in any case, I think you're best to check the state of SDA after every toggle and then stop toggling once SDA has been released.  Then you can send a stop condition.

     

     

  • 0 in reply to Brad Griffis
    Intellectual 280 points

    Brad Griffis said:

    + for (i = 0; i < 8; i++) {

    + /* SCL low */

    + davinci_i2c_write_reg(dev, DAVINCI_I2C_DCLR_REG,

    + DAVINCI_I2C_DCLR_PDCLR0);

    + udelay(5);

    + /* SCL high */

    + davinci_i2c_write_reg(dev, DAVINCI_I2C_DSET_REG,

    + DAVINCI_I2C_DSET_PDSET0);

    + udelay(5);

    + }

    Why don't you check the state of SDA after each loop?  Personally I think that would give you the most robust code for unlocking the bus.  The bus gets locked up as a result of the master and the slave getting out of sync.  The master is trying to send a stop condition which requires SDA to transition high.  However, the slave is holding SDA low because it thinks it's transmitting a bit of data (0).  It's not clear at any given moment exactly how far out of sync the master and the slave have gotten so you really need to keep pulsing until the slave let's SDA go high and then send a stop bit.  If you continue pulsing then the slave might transmit another zero and pull SDA low again.

    [/quote]

    Hi Brad,

    Thanks again for your invaluable input; I implemented the read of SDA as you suggested -- please see attached for update patch. I added a print of the number of pusles required before SDA was released; so far all the recoveries initiated are reporting only one SCL pulse before SDA is released.

     

    pca953x 1-0021: failed reading register

    i2c_davinci i2c_davinci.1: controller timed out

    i2c_davinci i2c_davinci.1: initiating i2c bus recovery

    i2c_davinci i2c_davinci.1: recovered after 1 SCL pulses

    pca953x 1-0021: failed reading register

     

    In the case of a master receiver the master is expected to NACK the final byte being received such that the slave knows to stop sending data.  According to the spec, once you get to this point the master should either issue a stop condition or a repeated start.  As far as I can tell if the master continues pulsing SCL the behavior is not defined by the spec.  So conceivably (in my opinion) the slave may keep trying to send data.  I assume this behavior will be dependent upon the individual implementation.  So in any case, I think you're best to check the state of SDA after every toggle and then stop toggling once SDA has been released.  Then you can send a stop condition.

     

     

    The existing recovery code in the driver issues  a STOP after the pulses; I am relying on this behaviour to finish the recovery.

     

    I hope to post a polished version of this implementation to i2c-dev; would you like to be on the CC for review?

     

    Best Regards,

    Ben Gardiner

    diff --git a/drivers/i2c/busses/i2c-davinci.c b/drivers/i2c/busses/i2c-davinci.cindex 54960d7..922a297 100644

    --- a/drivers/i2c/busses/i2c-davinci.c

    +++ b/drivers/i2c/busses/i2c-davinci.c

    @@ -65,6 +65,11 @@

     #define DAVINCI_I2C_IVR_REG 0x28

     #define DAVINCI_I2C_EMDR_REG 0x2c

     #define DAVINCI_I2C_PSC_REG 0x30

    +#define DAVINCI_I2C_PFUNC_REG 0x48

    +#define DAVINCI_I2C_PDIR_REG 0x4c

    +#define DAVINCI_I2C_PDIN_REG 0x50

    +#define DAVINCI_I2C_DSET_REG 0x58

    +#define DAVINCI_I2C_DCLR_REG 0x5c

     

     #define DAVINCI_I2C_IVR_AAS 0x07

     #define DAVINCI_I2C_IVR_SCD 0x06

    @@ -98,6 +103,29 @@

     #define DAVINCI_I2C_IMR_NACK BIT(1)

     #define DAVINCI_I2C_IMR_AL BIT(0)

     

    +/* set SDA and SCL as GPIO */

    +#define DAVINCI_I2C_PFUNC_PFUNC0 BIT(0)

    +

    +/* set SCL as output when used as GPIO*/

    +#define DAVINCI_I2C_PDIR_PDIR0 BIT(0)

    +/* set SDA as output when used as GPIO*/

    +#define DAVINCI_I2C_PDIR_PDIR1 BIT(1)

    +

    +/* read SCL GPIO level */

    +#define DAVINCI_I2C_PDIN_PDIN0 BIT(0)

    +/* read SDA GPIO level */

    +#define DAVINCI_I2C_PDIN_PDIN1 BIT(1)

    +

    +/*set the SCL GPIO high */

    +#define DAVINCI_I2C_DSET_PDSET0 BIT(0)

    +/*set the SDA GPIO high */

    +#define DAVINCI_I2C_DSET_PDSET1 BIT(1)

    +

    +/* set the SCL GPIO low */

    +#define DAVINCI_I2C_DCLR_PDCLR0 BIT(0)

    +/* set the SDA GPIO low */

    +#define DAVINCI_I2C_DCLR_PDCLR1 BIT(1)

    +

     struct davinci_i2c_dev {

      struct device           *dev;

      void __iomem *base;

    @@ -180,6 +208,73 @@ static void generic_i2c_clock_pulse(unsigned int scl_pin)

      }

     }

     

    +static inline void davinci_i2c_reset_ctrl(struct davinci_i2c_dev *i2c_dev,

    + int val);

    +

    +/* Generate gpio a pulse on the i2c clock pin. */

    +static int generic_i2c_clock_pulse_gpio(struct davinci_i2c_dev *dev)

    +{

    + u32 flag = 0;

    + u16 i;

    +

    + davinci_i2c_reset_ctrl(dev, 0);

    +

    + davinci_i2c_write_reg(dev, DAVINCI_I2C_MDR_REG, 0x00);

    +

    + /* SCL output, SDA input */

    + davinci_i2c_write_reg(dev, DAVINCI_I2C_PDIR_REG,

    + DAVINCI_I2C_PDIR_PDIR0);

    +

    + /* change to GPIO mode */

    + davinci_i2c_write_reg(dev, DAVINCI_I2C_PFUNC_REG,

    + DAVINCI_I2C_PFUNC_PFUNC0);

    +

    + /*

    + * NOTE: testing has shown that other variations of this SCL-toggle

    + * routine do not work:

    + * - SCL-low first,

    + * - 9 toggles,

    + * - 20us delays.

    + */

    +

    + /* SCL high */

    + davinci_i2c_write_reg(dev, DAVINCI_I2C_DSET_REG,

    + DAVINCI_I2C_DSET_PDSET0);

    + udelay(5);

    + for (i = 0; i < 16; i++) {

    + /* SCL low */

    + davinci_i2c_write_reg(dev, DAVINCI_I2C_DCLR_REG,

    + DAVINCI_I2C_DCLR_PDCLR0);

    + udelay(5);

    + /* SCL high */

    + davinci_i2c_write_reg(dev, DAVINCI_I2C_DSET_REG,

    + DAVINCI_I2C_DSET_PDSET0);

    + udelay(5);

    +

    + /* read the state of SDA */

    + flag = davinci_i2c_read_reg(dev, DAVINCI_I2C_PDIN_REG);

    + if (flag & DAVINCI_I2C_PDIN_PDIN1) {

    + dev_err(dev->dev, "recovered after %d SCL pulses",

    + i + 1);

    + break;

    + }

    + }

    +

    + /* change back to I2C mode */

    + davinci_i2c_write_reg(dev, DAVINCI_I2C_PFUNC_REG, 0);

    +

    + /* take the I2C dev out of reset */

    + davinci_i2c_reset_ctrl(dev, 1);

    +

    + /* read the state of SDA */

    + flag = davinci_i2c_read_reg(dev, DAVINCI_I2C_PDIN_REG);

    + if (flag & DAVINCI_I2C_PDIN_PDIN1)

    + return 0;

    +

    + dev_err(dev->dev, "I2C slave will not release SDA.\n");

    + return -1;

    +}

    +

     /* This routine does i2c bus recovery as specified in the

      * i2c protocol Rev. 03 section 3.16 titled "Bus clear"

      */

    @@ -195,8 +290,10 @@ static void i2c_recover_bus(struct davinci_i2c_dev *dev)

      flag |=  DAVINCI_I2C_MDR_NACK;

      /* write the data into mode register */

      davinci_i2c_write_reg(dev, DAVINCI_I2C_MDR_REG, flag);

    - if (pdata)

    + if (pdata->scl_pin)

      generic_i2c_clock_pulse(pdata->scl_pin);

    + else

    + generic_i2c_clock_pulse_gpio(dev);

      /* Send STOP */

      flag = davinci_i2c_read_reg(dev, DAVINCI_I2C_MDR_REG);

      flag |= DAVINCI_I2C_MDR_STP;

     

     

  • 0 in reply to Ben Gardiner
    TI__Guru*** 125430 points

    Ben Gardiner said:
    I hope to post a polished version of this implementation to i2c-dev; would you like to be on the CC for review?

    Thank you for your efforts in fixing this problem for EVERYONE by pushing this code to i2c-dev.  Yes, I would be happy to review.  My email is XXXXXXXXXX.  Note that I am going to remove this immediately after posting, so keep the email notification because it will disappear from here!

    Ben Gardiner said:
    I implemented the read of SDA as you suggested -- please see attached for update patch. I added a print of the number of pusles required before SDA was released; so far all the recoveries initiated are reporting only one SCL pulse before SDA is released.

    So how often do you see these recoveries occurring?  Have you looked any more at pullup strength, etc. as a potential contributor to the issues?

  • 0 in reply to Brad Griffis
    Intellectual 280 points

    Brad Griffis said:

    So how often do you see these recoveries occurring?

    With the check-SDA-after-each pulse version the recoveries are needed roughly once every 10minutes

     

    # dmesg|grep recovery; uptime

    i2c_davinci i2c_davinci.1: initiating i2c bus recovery

    i2c_davinci i2c_davinci.1: initiating i2c bus recovery

    i2c_davinci i2c_davinci.1: initiating i2c bus recovery

    i2c_davinci i2c_davinci.1: initiating i2c bus recovery

    i2c_davinci i2c_davinci.1: initiating i2c bus recovery

     06:15:09 up 41 min,  1 user,  load average: 0.00, 0.01, 0.04

     

    Brad Griffis said:

    Have you looked any more at pullup strength, etc. as a potential contributor to the issues?

    Sorry I haven't dug into this much -- it is a LogicPD EVM and SOM; the SOM has the U24 buffer that shouldn't be there so I guess I should suspect it...


     

  • 0 in reply to Brad Griffis
    Intellectual 280 points

    Brad Griffis said:

    I hope to post a polished version of this implementation to i2c-dev; would you like to be on the CC for review?

    Thank you for your efforts in fixing this problem for EVERYONE by pushing this code to i2c-dev.

    [/quote]

    My pleasure.

    Can you help me out once more: I think part of the review will be on what platform data the switch for recovery is based -- I'm thinking of introducing a 'has_pfunc' flag to indicate whether or not the i2c controller has the ICPFUNC register et. al. to control the SDA and SCL as GPIOs. Is this a definiing feature of certain revisions of the controller? i.e. are there davinci's that do not have that register?

    Best Regards, 

    Ben Gardiner

  • 0 in reply to Brad Griffis
    Intellectual 280 points

    Brad Griffis said:

    What pullup strength is being used on the bus?  It seems like adding a tiny bit of load (i.e. the probe) is enough to make the issue go away.  Perhaps you need to make the I2C pullups a little weaker.  ???

    According to the SOM schematic the pullups R92 and R93 are 1kOhm.

    Brad Griffis said:

    I didn't see any files attached.  What do you see for rise and fall times on the SCL and SDA lines?

    Sorry I guess I should have attached a scope-shot.

    I did capture a noise spike this time around but there were no "i2c controller timed out" messages reported.

  • 0 in reply to Ben Gardiner
    TI__Guru*** 125430 points

    Ben Gardiner said:
    I think part of the review will be on what platform data the switch for recovery is based -- I'm thinking of introducing a 'has_pfunc' flag to indicate whether or not the i2c controller has the ICPFUNC register et. al. to control the SDA and SCL as GPIOs. Is this a definiing feature of certain revisions of the controller? i.e. are there davinci's that do not have that register?

    The OMAP-L137/AM1707 and OMAP-L138/AM1808 are the only devices that contain the ICPFUNC registers.  Furthermore, you will most likely see another TI device that includes those registers.  So personally I think I would vote against introducing a has_pfunc flag.  In my opinion the handling needs to be done according to device family (e.g. DM644x, DM646x, OMAP-L1x, etc.).  Some devices mux the I2C pins with GPIO and so you need to use the GPIO peripheral to toggle SCL.  Other devices like OMAP-L1x have the PFUNC register.  New devices like DM814x and DM816x contain a test mode in the I2C peripheral that allows you to pulse SCL.

    Since this tends to be device-specific, perhaps instead of having a bunch of "if" statements the driver should be architected such that a function pointer to the device-specific pulse_SCL() function should be part of the driver's structure.  Does that make sense?  I've got a lot to learn about Linux drivers so pardon my ignorance!

  • 0 in reply to Ben Gardiner
    TI__Guru*** 125430 points

    Ben Gardiner said:

    According to the SOM schematic the pullups R92 and R93 are 1kOhm.

    That's a pretty stiff pullup.  Perhaps you should try 5k pullups which is my general recommendation for I2C pullups.  I don't have any specific reason why the 1k is bad, but based on the fact that you said adding the scope probe made the issue go away I am making this suggestion.

    Ben Gardiner said:

    I did capture a noise spike this time around but there were no "i2c controller timed out" messages reported.

    That looks like a pretty big noise spike on the line.  Something like that could cause a lot of problems, e.g. you might be experiencing data corruption without realizing it.  I wouldn't expect a bit glitch like that to always hang the bus.   The OMAP-L138 has some noise glitch detection built into it.  I don't know if the same is true for all the slave devices though.  Also, the bus will only hang in the case where the master and slave get out of sync AND the slave is trying to send a zero to the master.

  • 0 in reply to Brad Griffis
    Intellectual 430 points

    Dear Brad, dear Ben,

    thank you for your effort, and sorry for the delayed answer.

    It felt for me that the tps65070 makes trouble. So i coupled the tps reset with
    the board reset, but after a reset the SDA was still low (a powerdown powerup was
    still necessary). So it seems there is another slave holding this line and the
    tps65070 is OK. 

    Brad Griffis said:

    So are you sure you still see this issue even with the new code
    applied (with no scope attached)?

    What pullup strength is being used on the bus?  It seems like adding a tiny
    bit of load (i.e. the probe) is enough to make the issue go away. 
    Perhaps you need to make the I2C pullups a little weaker.  ???

    We have a new hardware revision here. With the same i2c slaves but different
    arranged. Now its very difficult to produce the error.  
    I will tell logicpd that in my opinion a higher pullup is the a better solution.
     
    Ben Gardiner said:

    I hope to post a polished version of this implementation to i2c-dev;
    would you like to be on the CC for review?


    I would be glad to be on CC, too.

    Best regards,

    Bastian.

  • 0 in reply to Brad Griffis
    Intellectual 430 points

    Dear Brad, dear Ben,

    thank you for your effort, and sorry for the delayed answer.

    It felt for me that the tps65070 makes trouble. So i coupled the tps reset with
    the board reset, but after a reset the SDA was still low (a powerdown powerup was
    still necessary). So it seems there is another slave holding this line and the
    tps65070 is OK. 

    Brad Griffis said:

    So are you sure you still see this issue even with the new code
    applied (with no scope attached)?

    What pullup strength is being used on the bus?  It seems like adding a tiny
    bit of load (i.e. the probe) is enough to make the issue go away. 
    Perhaps you need to make the I2C pullups a little weaker.  ???

    We have a new hardware revision here. With the same i2c slaves but different

    arranged. Now its very difficult to produce the error without the scope on the lines.

    With the scope, the error occurs very fast!

     
    I will tell logicpd that in my opinion a higher pullup is the a better solution.
     

    Ben Gardiner said:

    I hope to post a polished version of this implementation to i2c-dev;
    would you like to be on the CC for review?


    I would be glad to be on CC, too.

    Best regards,

    Bastian.

  • 0 in reply to Bastian
    Intellectual 280 points

     

    Brad Griffis said:

    I think part of the review will be on what platform data the switch for recovery is based -- I'm thinking of introducing a 'has_pfunc' flag to indicate whether or not the i2c controller has the ICPFUNC register et. al. to control the SDA and SCL as GPIOs. Is this a definiing feature of certain revisions of the controller? i.e. are there davinci's that do not have that register?

    The OMAP-L137/AM1707 and OMAP-L138/AM1808 are the only devices that contain the ICPFUNC registers.  Furthermore, you will most likely see another TI device that includes those registers.  So personally I think I would vote against introducing a has_pfunc flag.  In my opinion the handling needs to be done according to device family (e.g. DM644x, DM646x, OMAP-L1x, etc.).  Some devices mux the I2C pins with GPIO and so you need to use the GPIO peripheral to toggle SCL.  Other devices like OMAP-L1x have the PFUNC register.  New devices like DM814x and DM816x contain a test mode in the I2C peripheral that allows you to pulse SCL.

    Since this tends to be device-specific, perhaps instead of having a bunch of "if" statements the driver should be architected such that a function pointer to the device-specific pulse_SCL() function should be part of the driver's structure.  Does that make sense?  I've got a lot to learn about Linux drivers so pardon my ignorance!

    [/quote]

    No need to apologize -- I'm not an expert either but I think that is a reasonable architechture; I think there might be some technical limitations to putting the implementations literally in the device family platform code. But I'm sure this will all come out in the wash of the review.

    Thanks for the overview of the current and future I2C davicinci controllers; that is key information. I'm curious about the 'test mode' in the DM814x and DM816x: does it fit into the existing timeout->pulse SCL->send stop template?

     

    Brad Griffis said:

    That looks like a pretty big noise spike on the line.  Something like that could cause a lot of problems, e.g. you might be experiencing data corruption without realizing it.  I wouldn't expect a bit glitch like that to always hang the bus.   The OMAP-L138 has some noise glitch detection built into it.  I don't know if the same is true for all the slave devices though.  Also, the bus will only hang in the case where the master and slave get out of sync AND the slave is trying to send a zero to the master.

    I think I am experiencing corruption as you have predicted. I left the board running overnight with heartbeats on the user LEDs (and gpio-keys-polled polling task); when I came back this morning I found that the board had rebooted. I've seen this before and I'm starting to suspect a corrupted transfer causing reboot by the PMIC or the reset line attached to the tca6416 gpio expander on the baseboard.

    Bastian said:

    It felt for me that the tps65070 makes trouble. So i coupled the tps reset with
    the board reset, but after a reset the SDA was still low (a powerdown powerup was
    still necessary). So it seems there is another slave holding this line and the
    tps65070 is OK. 

    This coincides with my observation that SDA was held low after a reset when the buffer to enable the PMIC was disabled here.

    Bastian said:

    I hope to post a polished version of this implementation to i2c-dev;
    would you like to be on the CC for review?



    I would be glad to be on CC, too.
    [/quote]

    Excellent -- you were, of course, going to be on the CC whether you liked it or not :)

    Best Regards,

    Ben Gardiner

  • 0 in reply to Ben Gardiner
    TI__Expert 6385 points

    Ben Gardiner said:

    No need to apologize -- I'm not an expert either but I think that is a reasonable architechture; I think there might be some technical limitations to putting the implementations literally in the device family platform code. But I'm sure this will all come out in the wash of the review.

    Without a deeper analysis on this, I would think platform data member advertising this facility on the SoC would be more acceptable. This way no need to write platform specific code for every platform that has this functionality.

    Ben Gardiner said:
    I'm curious about the 'test mode' in the DM814x and DM816x: does it fit into the existing timeout->pulse SCL->send stop template?

    These devices fall into the "OMAP" family Linux kernel wise (will be supported under arch/arm/mach-omap2) and are actually served by i2c-omap.c driver.

    Thanks,

    Sekhar

     

  • 0 in reply to Sekhar Nori
    Intellectual 280 points

    Hi Sekhar,

     

    Sekhar Nori said:

    No need to apologize -- I'm not an expert either but I think that is a reasonable architechture; I think there might be some technical limitations to putting the implementations literally in the device family platform code. But I'm sure this will all come out in the wash of the review.

    Without a deeper analysis on this, I would think platform data member advertising this facility on the SoC would be more acceptable. This way no need to write platform specific code for every platform that has this functionality.

    [/quote]

    Make sense to me. Thanks for weighing-in now to short-circuit a review cycle or two. 

    Sekhar Nori said:

    I'm curious about the 'test mode' in the DM814x and DM816x: does it fit into the existing timeout->pulse SCL->send stop template?

    These devices fall into the "OMAP" family Linux kernel wise (will be supported under arch/arm/mach-omap2) and are actually served by i2c-omap.c driver.

    [/quote]

    Ok. So i2c-davinci.c won't need to handle those new DM's. Thanks, that simplifies things.

    OT: What is up with DaVinci vs. OMAP in the linux kernel? The news (to me) that those DM's are OMAP2 means there are DaVinci's served by the arch/arm/mach-omap* trees and OMAP's served by the arch/arm/mach-davinci tree?

    Best Regards,

    Ben Gardiner