How to put extra clock cycles on i2c cavinci, problems on omapl138 (logic pd som)

How to put extra clock cycles on i2c cavinci, problems on omapl138 (logic pd som)

[Bastian.Ruppert-qHSRieSIjQqELgA04lAiVw]

Hello,

we have problems with the davinci-i2c driver on an omapl-138 (Logic PD 
SOM) in
conjunction with tps65070 hardware and tps6507x_ts touchscreen driver. 

(The following thread is related to this issue.
http://www.mail-archive.com/davinci-linux-open-source-VycZQUHpC/PFrsHnngEfi1aTQe2KTcn/@public.gmane.org/msg21162.html
)

After some action on the touchscreen, there are communication errors
on the i2c, and the bus stalled.
In this state the SDA line is hold low and the driver gives repeatedly
the following messages:

tps6507x 1-0048: ADC result 1 read failed
i2c_davinci i2c_davinci.1: controller timed out
i2c_davinci i2c_davinci.1: initiating i2c bus recovery
i2c_davinci i2c_davinci.1: controller timed out
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: initiating i2c bus recovery
i2c_davinci i2c_davinci.1: controller timed out
i2c_davinci i2c_davinci.1: initiating i2c bus recovery
tps6507x 1-0048: TSC mode read failed
[...]

When giving a short low impulse on the SCL wire by shortcutting
this line with ground, the i2c is running again. 

Under http://processors.wiki.ti.com/index.php/I2C_Tips 
there is article about : External Slave Device Hanging the Bus by
Holding SDA Low

Option 1: For devices that mux the SCL/SDA pins with GPIO, the easiest
thing is to configure the pins for GPIO operation and toggle SCL until
the slave releases SDA. At this point you should be able to resume
normal operation.

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.

We would like to have some extra SCL pulses in the function 
static void i2c_recover_bus(struct davinci_i2c_dev *dev).

We tried to put the I2C in "free data format" and read one byte with
the following lines:

            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); 

But that doesn`t work that way.

We have problems to use the GPIO variant to toggle SCL or using the 
generic_i2c_clock_pulse function, too.

Please tell me how to get extra clock cycles on i2c?

Thank you, regards,

Bastian Ruppert.

P.S.

[please put me on cc, i am not on the vger.kernel.org list, thx] 

Re: How to put extra clock cycles on i2c cavinci, problems on omapl138 (logic pd som)

[Ben Gardiner]

Hello Bastian,

On Fri, Mar 11, 2011 at 7:52 AM,  <Bastian.Ruppert-qHSRieSIjQqELgA04lAiVw@public.gmane.org> wrote:
> Hello,
>
> we have problems with the davinci-i2c driver on an omapl-138 (Logic PD
> SOM) in
> conjunction with tps65070 hardware and tps6507x_ts touchscreen driver.
>
> (The following thread is related to this issue.
> http://www.mail-archive.com/davinci-linux-open-source-VycZQUHpC/MDL2sXcvxsEg@public.gmane.orgp.com/msg21162.html
> )
>
> After some action on the touchscreen, there are communication errors
> on the i2c, and the bus stalled.
> In this state the SDA line is hold low and the driver gives repeatedly
> the following messages:
>
> tps6507x 1-0048: ADC result 1 read failed
> i2c_davinci i2c_davinci.1: controller timed out
> i2c_davinci i2c_davinci.1: initiating i2c bus recovery
> i2c_davinci i2c_davinci.1: controller timed out
> 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: initiating i2c bus recovery
> i2c_davinci i2c_davinci.1: controller timed out
> i2c_davinci i2c_davinci.1: initiating i2c bus recovery
> tps6507x 1-0048: TSC mode read failed
> [...]

I am also observing 'controller timed out' messages on v2.6.38-rc8 on
the da850evm with the UI Board connected (the char-lcd one, no the
touchscreen).

 The addresses 0x20 and 0x21 can be seen in the error output below
which I believe indicates that the slave(s) hanging the bus are the
gpio expanders on the baseboard and UI board.

[...]
pca953x 1-0020: 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
i2c_davinci i2c_davinci.1: controller timed out
i2c_davinci i2c_davinci.1: initiating i2c bus recovery
[...]

The messages repeat with a mix of 0x20 and 0x21

When I revert:
c5b4afec8e669bbe265daea24037fccfc42c4fe3 i2c-davinci: Fix TX setup for more SoCs
4bba0fd8d1c6d405df666e2573e1a1f917098be0 i2c-davinci: Fix race when
setting up for TX

(CC'd the author, Jon Povey) -- I get the occasional:

i2c_davinci i2c_davinci.1: abnormal termination buf_len=-1
pca953x 1-0020: failed reading register
i2c_davinci i2c_davinci.1: initiating i2c bus recovery

but not the long repeating string as before.

I'm guessing there is some incompatibility between the recovery
procedure implemented by Philby John and the fix for the setup race
implemented by Jon Povey -- but I don't know enough about the drivers
to say what that might be.

> When giving a short low impulse on the SCL wire by shortcutting
> this line with ground, the i2c is running again.
>
> Under http://processors.wiki.ti.com/index.php/I2C_Tips
> there is article about : External Slave Device Hanging the Bus by
> Holding SDA Low
>
> Option 1: For devices that mux the SCL/SDA pins with GPIO, the easiest
> thing is to configure the pins for GPIO operation and toggle SCL until
> the slave releases SDA. At this point you should be able to resume
> normal operation.
>
> 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.
>
> We would like to have some extra SCL pulses in the function
> static void i2c_recover_bus(struct davinci_i2c_dev *dev).
>
> We tried to put the I2C in "free data format" and read one byte with
> the following lines:
>
>            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);
>
> But that doesn`t work that way.
>
> We have problems to use the GPIO variant to toggle SCL or using the
> generic_i2c_clock_pulse function, too.
>
> Please tell me how to get extra clock cycles on i2c?
>
> Thank you, regards,
>
> Bastian Ruppert.
>
> P.S.
>
> [please put me on cc, i am not on the vger.kernel.org list, thx]
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Best Regards,
Ben Gardiner

---
Nanometrics Inc.
http://www.nanometrics.ca

Re: How to put extra clock cycles on i2c cavinci, problems on omapl138 (logic pd som)

[Ben Gardiner]

Spoke too soon.

On Tue, Mar 15, 2011 at 1:02 PM, Ben Gardiner
<bengardiner-ScDXFp4xN3PN+rMO2ozGnw@public.gmane.org> wrote:
> [...]
> When I revert:
> c5b4afec8e669bbe265daea24037fccfc42c4fe3 i2c-davinci: Fix TX setup for more SoCs
> 4bba0fd8d1c6d405df666e2573e1a1f917098be0 i2c-davinci: Fix race when
> setting up for TX

It just took longer than usual for the onset of the messages; my
console is now filling with

[...]
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
i2c_davinci i2c_davinci.1: controller timed out
i2c_davinci i2c_davinci.1: initiating i2c bus recovery
[...]

as before.

Best Regards,
Ben Gardiner

---
Nanometrics Inc.
http://www.nanometrics.ca

(unknown),

[Ben Gardiner]

I tried the following patch based on the description of the steps you took to 
recover the bus. Is there anything you can add? I still get lots of 
'controller timed out' errors.

Best Regards,

Ben Gardiner

Nanometrics Inc.
http://www.nanometrics.ca

---

diff --git a/drivers/i2c/busses/i2c-davinci.c b/drivers/i2c/busses/i2c-davinci.c
index 5795c83..52e5e60 100644
--- a/drivers/i2c/busses/i2c-davinci.c
+++ b/drivers/i2c/busses/i2c-davinci.c
@@ -89,6 +89,7 @@
 #define DAVINCI_I2C_MDR_XA	BIT(8)
 #define DAVINCI_I2C_MDR_RM	BIT(7)
 #define DAVINCI_I2C_MDR_IRS	BIT(5)
+#define DAVINCI_I2C_MDR_FDF	BIT(3)
 
 #define DAVINCI_I2C_IMR_AAS	BIT(6)
 #define DAVINCI_I2C_IMR_SCD	BIT(5)
@@ -149,6 +150,36 @@ static void generic_i2c_clock_pulse(unsigned int scl_pin)
 	}
 }
 
+static void alternate_i2c_clock_pulse(struct davinci_i2c_dev *dev)
+{
+	u32 flag = davinci_i2c_read_reg(dev, DAVINCI_I2C_MDR_REG);
+	/* free data format mode */
+	flag |= DAVINCI_I2C_MDR_FDF;
+	/* receive byte */
+	flag &= ~DAVINCI_I2C_MDR_TRX;
+	/* repeat */
+	flag |= DAVINCI_I2C_MDR_RM;
+
+	/*
+	 * Write mode register first as needed for correct behaviour
+	 * on OMAP-L138, but don't set STT yet to avoid a race with XRDY
+	 * occuring before we have loaded DXR
+	 */
+	davinci_i2c_write_reg(dev, DAVINCI_I2C_MDR_REG, flag);
+
+	/* Disable receive and transmit interrupts */
+	flag = davinci_i2c_read_reg(dev, DAVINCI_I2C_IMR_REG);
+	flag &= ~DAVINCI_I2C_IMR_RRDY;
+	flag &= ~DAVINCI_I2C_IMR_XRDY;
+	davinci_i2c_write_reg(dev, DAVINCI_I2C_IMR_REG, flag);
+
+	/* Set STT to begin transmit now DXR is loaded */
+	flag |= DAVINCI_I2C_MDR_STT;
+	davinci_i2c_write_reg(dev, DAVINCI_I2C_MDR_REG, flag);
+
+	udelay(40*9);
+}
+
 /* This routine does i2c bus recovery as specified in the
  * i2c protocol Rev. 03 section 3.16 titled "Bus clear"
  */
@@ -163,8 +194,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
+		alternate_i2c_clock_pulse(dev);
 	/* Send STOP */
 	flag = davinci_i2c_read_reg(dev, DAVINCI_I2C_MDR_REG);
 	flag |= DAVINCI_I2C_MDR_STP;

Re: How to put extra clock cycles on i2c cavinci, problems on omapl138 (logic pd som)

[Bastian.Ruppert-qHSRieSIjQqELgA04lAiVw]

Hello,

[...]
> >
> > After some action on the touchscreen, there are communication errors
> > on the i2c, and the bus stalled.
> > In this state the SDA line is hold low and the driver gives repeatedly
> > the following messages:
> >
> > tps6507x 1-0048: ADC result 1 read failed
> > i2c_davinci i2c_davinci.1: controller timed out
> > i2c_davinci i2c_davinci.1: initiating i2c bus recovery
> > i2c_davinci i2c_davinci.1: controller timed out
> > 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: initiating i2c bus recovery
> > i2c_davinci i2c_davinci.1: controller timed out
> > i2c_davinci i2c_davinci.1: initiating i2c bus recovery
> > tps6507x 1-0048: TSC mode read failed
> > [...]
> 
> I am also observing 'controller timed out' messages on v2.6.38-rc8 on
> the da850evm with the UI Board connected (the char-lcd one, no the
> touchscreen).
> 
>  The addresses 0x20 and 0x21 can be seen in the error output below
> which I believe indicates that the slave(s) hanging the bus are the
> gpio expanders on the baseboard and UI board.
> 
> [...]
> pca953x 1-0020: 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
> i2c_davinci i2c_davinci.1: controller timed out
> i2c_davinci i2c_davinci.1: initiating i2c bus recovery
> [...]
> 
> The messages repeat with a mix of 0x20 and 0x21
> 
> When I revert:
> c5b4afec8e669bbe265daea24037fccfc42c4fe3 i2c-davinci: Fix TX setup 
> for more SoCs
> 4bba0fd8d1c6d405df666e2573e1a1f917098be0 i2c-davinci: Fix race when
> setting up for TX
> 
> (CC'd the author, Jon Povey) -- I get the occasional:
> 
> i2c_davinci i2c_davinci.1: abnormal termination buf_len=-1
> pca953x 1-0020: failed reading register
> i2c_davinci i2c_davinci.1: initiating i2c bus recovery
> 
> but not the long repeating string as before.
> 
> I'm guessing there is some incompatibility between the recovery
> procedure implemented by Philby John and the fix for the setup race
> implemented by Jon Povey -- but I don't know enough about the drivers
> to say what that might be.
> 

Have you tested to recover the i2c bus by adding some SCL pulses manually
(Shorten SCL to ground)?


I tried your patch:

> Ben Gardiner <bengardiner-ScDXFp4xN3PN+rMO2ozGnw@public.gmane.org> 
> 15.03.2011 22:08
[...]
> I tried the following patch based on the description of the steps you 
took to 
> recover the bus. Is there anything you can add? I still get lots of 
> 'controller timed out' errors.
> 
> Best Regards,
> 
> Ben Gardiner
> 
> Nanometrics Inc.
> http://www.nanometrics.ca
> 
> ---
> 
> diff --git a/drivers/i2c/busses/i2c-davinci.c b/drivers/i2c/busses/
> i2c-davinci.c
> index 5795c83..52e5e60 100644
> --- a/drivers/i2c/busses/i2c-davinci.c
> +++ b/drivers/i2c/busses/i2c-davinci.c
> @@ -89,6 +89,7 @@
>  #define DAVINCI_I2C_MDR_XA   BIT(8)
>  #define DAVINCI_I2C_MDR_RM   BIT(7)
>  #define DAVINCI_I2C_MDR_IRS   BIT(5)
> +#define DAVINCI_I2C_MDR_FDF   BIT(3)
> 
>  #define DAVINCI_I2C_IMR_AAS   BIT(6)
>  #define DAVINCI_I2C_IMR_SCD   BIT(5)
> @@ -149,6 +150,36 @@ static void generic_i2c_clock_pulse(unsigned int 
scl_pin)
>     }
>  }
> 
> +static void alternate_i2c_clock_pulse(struct davinci_i2c_dev *dev)
> +{
> +   u32 flag = davinci_i2c_read_reg(dev, DAVINCI_I2C_MDR_REG);
> +   /* free data format mode */
> +   flag |= DAVINCI_I2C_MDR_FDF;
> +   /* receive byte */
> +   flag &= ~DAVINCI_I2C_MDR_TRX;
> +   /* repeat */
> +   flag |= DAVINCI_I2C_MDR_RM;
> +
> +   /*
> +    * Write mode register first as needed for correct behaviour
> +    * on OMAP-L138, but don't set STT yet to avoid a race with XRDY
> +    * occuring before we have loaded DXR
> +    */
> +   davinci_i2c_write_reg(dev, DAVINCI_I2C_MDR_REG, flag);
> +
> +   /* Disable receive and transmit interrupts */
> +   flag = davinci_i2c_read_reg(dev, DAVINCI_I2C_IMR_REG);
> +   flag &= ~DAVINCI_I2C_IMR_RRDY;
> +   flag &= ~DAVINCI_I2C_IMR_XRDY;
> +   davinci_i2c_write_reg(dev, DAVINCI_I2C_IMR_REG, flag);
> +
> +   /* Set STT to begin transmit now DXR is loaded */
> +   flag |= DAVINCI_I2C_MDR_STT;
> +   davinci_i2c_write_reg(dev, DAVINCI_I2C_MDR_REG, flag);
> +
> +   udelay(40*9);
> +}
> +
>  /* This routine does i2c bus recovery as specified in the
>   * i2c protocol Rev. 03 section 3.16 titled "Bus clear"
>   */
> @@ -163,8 +194,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
> +      alternate_i2c_clock_pulse(dev);
>     /* Send STOP */
>     flag = davinci_i2c_read_reg(dev, DAVINCI_I2C_MDR_REG);
>     flag |= DAVINCI_I2C_MDR_STP;

It seems we are working on different trees. 

git log drivers/i2c/busses/i2c-davinci.c

gives me the following output.

commit c5b4afec8e669bbe265daea24037fccfc42c4fe3
Author: Jon Povey <jon.povey-Ean/AyPsLtfkYMGBc/C6ZA@public.gmane.org>
Date:   Tue Oct 12 13:47:05 2010 +0900

    i2c-davinci: Fix TX setup for more SoCs
[...] 
commit 4bba0fd8d1c6d405df666e2573e1a1f917098be0
Author: Jon Povey <jon.povey-Ean/AyPsLtfkYMGBc/C6ZA@public.gmane.org>
Date:   Fri Sep 17 12:02:11 2010 +0900

    i2c-davinci: Fix race when setting up for TX
[...]
commit 8574faf9a5ae71fdd84c6413779a9b076138eb9e
Author: Philby John <pjohn-k0rHJ+Hhz/SB+jHODAdFcQ@public.gmane.org>
[...]

...so the patch doesn ´t apply smooth. 
I put your changes in manually.

The alternate_i2c_clock_pulse function does not generate extra pulses on 
the clock line.
After applying this pach, i am not able to recreate the i2c bus with 
shortcutting SCL to ground anymore. 

Are there extra Clock cycles on your EVM UI Board? 

Regards,

Bastian.

Re: How to put extra clock cycles on i2c cavinci, problems on omapl138 (logic pd som)

[Ben Gardiner]

Hi Bastian,

On Thu, Mar 17, 2011 at 5:08 AM,  <Bastian.Ruppert-qHSRieSIjQqELgA04lAiVw@public.gmane.org> wrote:
> Hello,
>
> [...]
>> >
>> > After some action on the touchscreen, there are communication errors
>> > on the i2c, and the bus stalled.
>> > In this state the SDA line is hold low and the driver gives repeatedly
>> > the following messages:
>> >
>> > tps6507x 1-0048: ADC result 1 read failed
>> > i2c_davinci i2c_davinci.1: controller timed out
>> > i2c_davinci i2c_davinci.1: initiating i2c bus recovery
>> > i2c_davinci i2c_davinci.1: controller timed out
>> > 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: initiating i2c bus recovery
>> > i2c_davinci i2c_davinci.1: controller timed out
>> > i2c_davinci i2c_davinci.1: initiating i2c bus recovery
>> > tps6507x 1-0048: TSC mode read failed
>> > [...]
>>
>> I am also observing 'controller timed out' messages on v2.6.38-rc8 on
>> the da850evm with the UI Board connected (the char-lcd one, no the
>> touchscreen).
>>
>>  The addresses 0x20 and 0x21 can be seen in the error output below
>> which I believe indicates that the slave(s) hanging the bus are the
>> gpio expanders on the baseboard and UI board.
>>
>> [...]
>> pca953x 1-0020: 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
>> i2c_davinci i2c_davinci.1: controller timed out
>> i2c_davinci i2c_davinci.1: initiating i2c bus recovery
>> [...]
>>
>> The messages repeat with a mix of 0x20 and 0x21
>>
>> When I revert:
>> c5b4afec8e669bbe265daea24037fccfc42c4fe3 i2c-davinci: Fix TX setup
>> for more SoCs
>> 4bba0fd8d1c6d405df666e2573e1a1f917098be0 i2c-davinci: Fix race when
>> setting up for TX
>>
>> (CC'd the author, Jon Povey) -- I get the occasional:
>>
>> i2c_davinci i2c_davinci.1: abnormal termination buf_len=-1
>> pca953x 1-0020: failed reading register
>> i2c_davinci i2c_davinci.1: initiating i2c bus recovery
>>
>> but not the long repeating string as before.
>>
>> I'm guessing there is some incompatibility between the recovery
>> procedure implemented by Philby John and the fix for the setup race
>> implemented by Jon Povey -- but I don't know enough about the drivers
>> to say what that might be.
>>
>
> Have you tested to recover the i2c bus by adding some SCL pulses manually
> (Shorten SCL to ground)?

I shorted I2C0_SCL at pin 22 of the PH146(tca416) on the baseboard.
The first time it appeared to work -- the repeating messages on the
console stopped. Sometime the messages started up again; I repeated
the procedure but this time the messages did not stop.

> I tried your patch:
>
>> Ben Gardiner <bengardiner-ScDXFp4xN3PN+rMO2ozGnw@public.gmane.org>
>> 15.03.2011 22:08
> [...]
>> I tried the following patch based on the description of the steps you
> took to
>> recover the bus. Is there anything you can add? I still get lots of
>> 'controller timed out' errors.
>>
>> Best Regards,
>>
>> Ben Gardiner
>>
>> Nanometrics Inc.
>> http://www.nanometrics.ca
>>
>> ---
>>
>> diff --git a/drivers/i2c/busses/i2c-davinci.c b/drivers/i2c/busses/
>> i2c-davinci.c
>> index 5795c83..52e5e60 100644
>> --- a/drivers/i2c/busses/i2c-davinci.c
>> +++ b/drivers/i2c/busses/i2c-davinci.c
>> @@ -89,6 +89,7 @@
>>  #define DAVINCI_I2C_MDR_XA   BIT(8)
>>  #define DAVINCI_I2C_MDR_RM   BIT(7)
>>  #define DAVINCI_I2C_MDR_IRS   BIT(5)
>> +#define DAVINCI_I2C_MDR_FDF   BIT(3)
>>
>>  #define DAVINCI_I2C_IMR_AAS   BIT(6)
>>  #define DAVINCI_I2C_IMR_SCD   BIT(5)
>> @@ -149,6 +150,36 @@ static void generic_i2c_clock_pulse(unsigned int
> scl_pin)
>>     }
>>  }
>>
>> +static void alternate_i2c_clock_pulse(struct davinci_i2c_dev *dev)
>> +{
>> +   u32 flag = davinci_i2c_read_reg(dev, DAVINCI_I2C_MDR_REG);
>> +   /* free data format mode */
>> +   flag |= DAVINCI_I2C_MDR_FDF;
>> +   /* receive byte */
>> +   flag &= ~DAVINCI_I2C_MDR_TRX;
>> +   /* repeat */
>> +   flag |= DAVINCI_I2C_MDR_RM;
>> +
>> +   /*
>> +    * Write mode register first as needed for correct behaviour
>> +    * on OMAP-L138, but don't set STT yet to avoid a race with XRDY
>> +    * occuring before we have loaded DXR
>> +    */
>> +   davinci_i2c_write_reg(dev, DAVINCI_I2C_MDR_REG, flag);
>> +
>> +   /* Disable receive and transmit interrupts */
>> +   flag = davinci_i2c_read_reg(dev, DAVINCI_I2C_IMR_REG);
>> +   flag &= ~DAVINCI_I2C_IMR_RRDY;
>> +   flag &= ~DAVINCI_I2C_IMR_XRDY;
>> +   davinci_i2c_write_reg(dev, DAVINCI_I2C_IMR_REG, flag);
>> +
>> +   /* Set STT to begin transmit now DXR is loaded */
>> +   flag |= DAVINCI_I2C_MDR_STT;
>> +   davinci_i2c_write_reg(dev, DAVINCI_I2C_MDR_REG, flag);
>> +
>> +   udelay(40*9);
>> +}
>> +
>>  /* This routine does i2c bus recovery as specified in the
>>   * i2c protocol Rev. 03 section 3.16 titled "Bus clear"
>>   */
>> @@ -163,8 +194,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
>> +      alternate_i2c_clock_pulse(dev);
>>     /* Send STOP */
>>     flag = davinci_i2c_read_reg(dev, DAVINCI_I2C_MDR_REG);
>>     flag |= DAVINCI_I2C_MDR_STP;
>
> It seems we are working on different trees.

I was working off of v2.6.38-rc8 here.

> git log drivers/i2c/busses/i2c-davinci.c
>
> gives me the following output.
>
> commit c5b4afec8e669bbe265daea24037fccfc42c4fe3
> Author: Jon Povey <jon.povey-Ean/AyPsLtfkYMGBc/C6ZA@public.gmane.org>
> Date:   Tue Oct 12 13:47:05 2010 +0900
>
>    i2c-davinci: Fix TX setup for more SoCs
> [...]
> commit 4bba0fd8d1c6d405df666e2573e1a1f917098be0
> Author: Jon Povey <jon.povey-Ean/AyPsLtfkYMGBc/C6ZA@public.gmane.org>
> Date:   Fri Sep 17 12:02:11 2010 +0900
>
>    i2c-davinci: Fix race when setting up for TX
> [...]
> commit 8574faf9a5ae71fdd84c6413779a9b076138eb9e
> Author: Philby John <pjohn-k0rHJ+Hhz/SB+jHODAdFcQ@public.gmane.org>
> [...]
>
> ...so the patch doesn ´t apply smooth.

You are missing the following commit

commit c5b4afec8e669bbe265daea24037fccfc42c4fe3
Author: Jon Povey <jon.povey-Ean/AyPsLtfkYMGBc/C6ZA@public.gmane.org>
Date:   Tue Oct 12 13:47:05 2010 +0900

    i2c-davinci: Fix TX setup for more SoCs

    This patch is an improvement to 4bba0fd8d1c6d405df666e2573e1a1f917098be0
    which got to mainline a little early.

    Sudhakar Rajashekhara explains that at least OMAP-L138 requires MDR mode
    settings before DXR for correct behaviour, so load MDR first with
    STT cleared and later load again with STT set.

    Tested on DM355 connected to Techwell TW2836 and Wolfson WM8985

    Signed-off-by: Jon Povey <jon.povey-Ean/AyPsLtfkYMGBc/C6ZA@public.gmane.org>
    Acked-by: Troy Kisky <troy.kisky-Q5RJGjKts06CY9SHAMCTRUEOCMrvLtNR@public.gmane.org>
    Tested-by: Sudhakar Rajashekhara <sudhakar.raj-l0cyMroinI0@public.gmane.org>
    Acked-by: Kevin Hilman <khilman-1D3HCaltpLuhEniVeURVKkEOCMrvLtNR@public.gmane.org>
    Signed-off-by: Ben Dooks <ben-linux-elnMNo+KYs3YtjvyW6yDsg@public.gmane.org>

> The alternate_i2c_clock_pulse function does not generate extra pulses on
> the clock line.

Ok. I made the patch from the short snip of the method you described;
I was more wondering whether the patch was the same as what you've
tested.

> After applying this pach, i am not able to recreate the i2c bus with
> shortcutting SCL to ground anymore.

Yeah -- I can confirm also that with the patch applied I am unable to
free the bus by shorting SCL to ground. The patch is doing more harm
than good.

> Are there extra Clock cycles on your EVM UI Board?

I'm not sure. I'm sorry I don't have a logic analyzer available ATM.

I noticed on the e2e forums that there is a reason why the PSP uses a
bitbanging i2c interface -- since the 1.0v OPP makes the i2c
controller unusable [1] (I've added Sekhar to the CC since he pointed
out the problem in that post).

I had my kernel configured to use CPU frequency scaling -- I disabled
the feature but this did not fix the problem.

I wonder if there is YA hardware problem with i2c on the LogicPD
boards. I've noticed that when I don't have the UBI board connected
then I don't get the controller timeouts.

Have you tried using the bitbanging i2c driver? I suppose that using
the bitbanging i2c driver would be too much CPU overhead for polling
your touchscreen...

Have you asked about how to implement this FDF-SCL-toggle method on
the e2e forums?

Best Regards,
Ben Gardiner

[1] https://e2e.ti.com/support/embedded/f/354/p/68559/248084.aspx#248084

---
Nanometrics Inc.
http://www.nanometrics.ca

Re: How to put extra clock cycles on i2c cavinci, problems on omapl138 (logic pd som)

[Bastian.Ruppert-qHSRieSIjQqELgA04lAiVw]

> 
> Hi Bastian,

Hello Ben,

[...]
> >>
> >
> > Have you tested to recover the i2c bus by adding some SCL pulses 
manually
> > (Shorten SCL to ground)?
> 
> I shorted I2C0_SCL at pin 22 of the PH146(tca416) on the baseboard.
> The first time it appeared to work -- the repeating messages on the
> console stopped. Sometime the messages started up again; I repeated
> the procedure but this time the messages did not stop.
> 

Sometimes, when a slave "holds" the SDA it need some drive through i2c 
clock.

> > I tried your patch:
> >
> >> Ben Gardiner <bengardiner-ScDXFp4xN3PN+rMO2ozGnw@public.gmane.org>
> >> 15.03.2011 22:08
> > [...]
> >> I tried the following patch based on the description of the steps you
> > took to
> >> recover the bus. Is there anything you can add? I still get lots of
> >> 'controller timed out' errors.
> >>
> >> Best Regards,
> >>
> >> Ben Gardiner
> >>
> >> Nanometrics Inc.
> >> http://www.nanometrics.ca
> >>
> >> ---
> >>
> >> diff --git a/drivers/i2c/busses/i2c-davinci.c b/drivers/i2c/busses/
> >> i2c-davinci.c
> >> index 5795c83..52e5e60 100644
> >> --- a/drivers/i2c/busses/i2c-davinci.c
> >> +++ b/drivers/i2c/busses/i2c-davinci.c
> >> @@ -89,6 +89,7 @@
> >>  #define DAVINCI_I2C_MDR_XA   BIT(8)
> >>  #define DAVINCI_I2C_MDR_RM   BIT(7)
> >>  #define DAVINCI_I2C_MDR_IRS   BIT(5)
> >> +#define DAVINCI_I2C_MDR_FDF   BIT(3)
> >>
> >>  #define DAVINCI_I2C_IMR_AAS   BIT(6)
> >>  #define DAVINCI_I2C_IMR_SCD   BIT(5)
> >> @@ -149,6 +150,36 @@ static void generic_i2c_clock_pulse(unsigned int
> > scl_pin)
> >>     }
> >>  }
> >>
> >> +static void alternate_i2c_clock_pulse(struct davinci_i2c_dev *dev)
> >> +{
> >> +   u32 flag = davinci_i2c_read_reg(dev, DAVINCI_I2C_MDR_REG);
> >> +   /* free data format mode */
> >> +   flag |= DAVINCI_I2C_MDR_FDF;
> >> +   /* receive byte */
> >> +   flag &= ~DAVINCI_I2C_MDR_TRX;
> >> +   /* repeat */
> >> +   flag |= DAVINCI_I2C_MDR_RM;
> >> +
> >> +   /*
> >> +    * Write mode register first as needed for correct behaviour
> >> +    * on OMAP-L138, but don't set STT yet to avoid a race with XRDY
> >> +    * occuring before we have loaded DXR
> >> +    */
> >> +   davinci_i2c_write_reg(dev, DAVINCI_I2C_MDR_REG, flag);
> >> +
> >> +   /* Disable receive and transmit interrupts */
> >> +   flag = davinci_i2c_read_reg(dev, DAVINCI_I2C_IMR_REG);
> >> +   flag &= ~DAVINCI_I2C_IMR_RRDY;
> >> +   flag &= ~DAVINCI_I2C_IMR_XRDY;
> >> +   davinci_i2c_write_reg(dev, DAVINCI_I2C_IMR_REG, flag);
> >> +
> >> +   /* Set STT to begin transmit now DXR is loaded */
> >> +   flag |= DAVINCI_I2C_MDR_STT;
> >> +   davinci_i2c_write_reg(dev, DAVINCI_I2C_MDR_REG, flag);
> >> +
> >> +   udelay(40*9);
> >> +}
> >> +
> >>  /* This routine does i2c bus recovery as specified in the
> >>   * i2c protocol Rev. 03 section 3.16 titled "Bus clear"
> >>   */
> >> @@ -163,8 +194,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
> >> +      alternate_i2c_clock_pulse(dev);
> >>     /* Send STOP */
> >>     flag = davinci_i2c_read_reg(dev, DAVINCI_I2C_MDR_REG);
> >>     flag |= DAVINCI_I2C_MDR_STP;
> >
> > It seems we are working on different trees.
> 
> I was working off of v2.6.38-rc8 here.
> 
> > git log drivers/i2c/busses/i2c-davinci.c
> >
> > gives me the following output.
> >
> > commit c5b4afec8e669bbe265daea24037fccfc42c4fe3
> > Author: Jon Povey <jon.povey-Ean/AyPsLtfkYMGBc/C6ZA@public.gmane.org>
> > Date:   Tue Oct 12 13:47:05 2010 +0900
> >
> >    i2c-davinci: Fix TX setup for more SoCs
> > [...]
> > commit 4bba0fd8d1c6d405df666e2573e1a1f917098be0
> > Author: Jon Povey <jon.povey-Ean/AyPsLtfkYMGBc/C6ZA@public.gmane.org>
> > Date:   Fri Sep 17 12:02:11 2010 +0900
> >
> >    i2c-davinci: Fix race when setting up for TX
> > [...]
> > commit 8574faf9a5ae71fdd84c6413779a9b076138eb9e
> > Author: Philby John <pjohn-k0rHJ+Hhz/SB+jHODAdFcQ@public.gmane.org>
> > [...]
> >
> > ...so the patch doesn ´t apply smooth.
> 
> You are missing the following commit
> 
> commit c5b4afec8e669bbe265daea24037fccfc42c4fe3
> Author: Jon Povey <jon.povey-Ean/AyPsLtfkYMGBc/C6ZA@public.gmane.org>
> Date:   Tue Oct 12 13:47:05 2010 +0900
> 
>     i2c-davinci: Fix TX setup for more SoCs
> 
>     This patch is an improvement to 
4bba0fd8d1c6d405df666e2573e1a1f917098be0
>     which got to mainline a little early.
> 
>     Sudhakar Rajashekhara explains that at least OMAP-L138 requires MDR 
mode
>     settings before DXR for correct behaviour, so load MDR first with
>     STT cleared and later load again with STT set.
> 
>     Tested on DM355 connected to Techwell TW2836 and Wolfson WM8985
> 
>     Signed-off-by: Jon Povey <jon.povey-Ean/AyPsLtfkYMGBc/C6ZA@public.gmane.org>
>     Acked-by: Troy Kisky <troy.kisky-Q5RJGjKts06CY9SHAMCTRUEOCMrvLtNR@public.gmane.org>
>     Tested-by: Sudhakar Rajashekhara <sudhakar.raj-l0cyMroinI0@public.gmane.org>
>     Acked-by: Kevin Hilman <khilman-1D3HCaltpLuhEniVeURVKkEOCMrvLtNR@public.gmane.org>
>     Signed-off-by: Ben Dooks <ben-linux-elnMNo+KYs3YtjvyW6yDsg@public.gmane.org>
> 

Ok.

> > The alternate_i2c_clock_pulse function does not generate extra pulses 
on
> > the clock line.
> 
> Ok. I made the patch from the short snip of the method you described;
> I was more wondering whether the patch was the same as what you've
> tested.
> 
> > After applying this pach, i am not able to recreate the i2c bus with
> > shortcutting SCL to ground anymore.
> 
> Yeah -- I can confirm also that with the patch applied I am unable to
> free the bus by shorting SCL to ground. The patch is doing more harm
> than good.
> 
> > Are there extra Clock cycles on your EVM UI Board?
> 
> I'm not sure. I'm sorry I don't have a logic analyzer available ATM.
> 
> I noticed on the e2e forums that there is a reason why the PSP uses a
> bitbanging i2c interface -- since the 1.0v OPP makes the i2c
> controller unusable [1] (I've added Sekhar to the CC since he pointed
> out the problem in that post).
> 

Yes, i saw this post before, but our chip is driving 1.2V and we still 
have 
our problems.

> I had my kernel configured to use CPU frequency scaling -- I disabled
> the feature but this did not fix the problem.
> 
> I wonder if there is YA hardware problem with i2c on the LogicPD
> boards. I've noticed that when I don't have the UBI board connected
> then I don't get the controller timeouts.
> 

I have some effects here, too.
We have a i2c repeater solded on our board (PCA9515AD). With this repeater 

i can reproduce the errer pretty fast.
Without this repeater solded on, the mean time before failure is much 
higher.

I can produce the error pretty fast with the ts_test tool. Without any 
userland 
tool using the touchscreen info or with the tool evtest, i can see the i2c 

action on the oscilloscope but it is hard to produce the error.
What does this indicate to? 
Is it possible that userland can influence the kernel driver timing to 
promote 
the error? 

> Have you tried using the bitbanging i2c driver? I suppose that using
> the bitbanging i2c driver would be too much CPU overhead for polling
> your touchscreen...
> 

Yes, the bitbanging driver gives me "boiled NACK" errors or so sometimes, 
but the system do not stall. (In my opinion, bitbanging is not the elegant 
way 
to do i2c).

> Have you asked about how to implement this FDF-SCL-toggle method on
> the e2e forums?

No, i did not.

> Best Regards,
> Ben Gardiner
> 

Regards,
 
Bastian.

RE: How to put extra clock cycles on i2c cavinci, problems on omapl138 (logic pd som)

[Nori, Sekhar]

On Thu, Mar 17, 2011 at 18:55:00, Ben Gardiner wrote:

> I noticed on the e2e forums that there is a reason why the PSP uses a
> bitbanging i2c interface -- since the 1.0v OPP makes the i2c
> controller unusable [1] (I've added Sekhar to the CC since he pointed
> out the problem in that post).

There was a problem with the way I2C was implemented on the SoM. Here is
the information I have for the modifications required on the SoM.

"
It is likely that the errors seen were a result of the buffer on the I2C clock.  The hardware fix for this issue is to remove U24 and R161 and short U24 pins 2 to 4.
"

There should be an EVM with these fixes already, though I have not
tested it myself.

Also, recently a bug in the I2C driver was brought to my attention.
In the bus busy function:

/*
 * Waiting for bus not busy
 */
static int i2c_davinci_wait_bus_not_busy(struct davinci_i2c_dev *dev,
					 char allow_sleep)
{
	unsigned long timeout;
	static u16 to_cnt;

	timeout = jiffies + dev->adapter.timeout;
	while (davinci_i2c_read_reg(dev, DAVINCI_I2C_STR_REG)
	       & DAVINCI_I2C_STR_BB) {
		if (to_cnt <= DAVINCI_I2C_MAX_TRIES) {
			if (time_after(jiffies, timeout)) {
				dev_warn(dev->dev,
				"timeout waiting for bus ready\n");
				to_cnt++;
				return -ETIMEDOUT;
			} else {
				to_cnt = 0;
				i2c_recover_bus(dev);
				i2c_davinci_init(dev);
			}
		}
		if (allow_sleep)
			schedule_timeout(1);
	}

	return 0;
}

If there is a context switch immediately after the while statement,
the loop can return a timeout error while there is none. The
simple solution would be to check for bus busy again if the
timeout condition occurs. There should be a more elegant way
to write the loop, but it is too late in the evening to think
about it :)

I doubt if this is related to the errors you guys are seeing,
but it is worth trying anyway.

Thanks,
Sekhar

Re: How to put extra clock cycles on i2c cavinci, problems on omapl138 (logic pd som)

[Ben Gardiner]

On Thu, Mar 17, 2011 at 10:13 AM,  <Bastian.Ruppert-qHSRieSIjQqELgA04lAiVw@public.gmane.org> wrote:
> [...]
>> I wonder if there is YA hardware problem with i2c on the LogicPD
>> boards. I've noticed that when I don't have the UBI board connected
>> then I don't get the controller timeouts.
>>
>
> I have some effects here, too.
> We have a i2c repeater solded on our board (PCA9515AD). With this repeater
>
> i can reproduce the errer pretty fast.
> Without this repeater solded on, the mean time before failure is much
> higher.
>
> I can produce the error pretty fast with the ts_test tool. Without any
> userland
> tool using the touchscreen info or with the tool evtest, i can see the i2c
>
> action on the oscilloscope but it is hard to produce the error.
> What does this indicate to?
> Is it possible that userland can influence the kernel driver timing to
> promote
> the error?

I don't think that userland can influence the kernel driver timing;
based on my understanding of the driver [1] polling occurs from the
moment the device is probe()'d. Are you modprob'ing the driver right
before ts_test? Where can I see the ts_test source?

>> Have you tried using the bitbanging i2c driver? I suppose that using
>> the bitbanging i2c driver would be too much CPU overhead for polling
>> your touchscreen...
>>
>
> Yes, the bitbanging driver gives me "boiled NACK" errors or so sometimes,
> but the system do not stall. (In my opinion, bitbanging is not the elegant
> way
> to do i2c).
>
>> Have you asked about how to implement this FDF-SCL-toggle method on
>> the e2e forums?
>
> No, i did not.

Ok. I'll post then.

Best Regards,
Ben Gardiner

[1] http://lxr.linux.no/linux+v2.6.38/drivers/input/touchscreen/tps6507x-ts.c

---
Nanometrics Inc.
http://www.nanometrics.ca

Re: How to put extra clock cycles on i2c cavinci, problems on omapl138 (logic pd som)

[Bastian.Ruppert-qHSRieSIjQqELgA04lAiVw]

Dear Ben,

> 
> On Thu, Mar 17, 2011 at 10:13 AM,  <Bastian.Ruppert-qHSRieSIjQqELgA04lAiVw@public.gmane.org> wrote:
> > [...]
> >> I wonder if there is YA hardware problem with i2c on the LogicPD
> >> boards. I've noticed that when I don't have the UBI board connected
> >> then I don't get the controller timeouts.
> >>
> >
> > I have some effects here, too.
> > We have a i2c repeater solded on our board (PCA9515AD). With this 
repeater
> >
> > i can reproduce the errer pretty fast.
> > Without this repeater solded on, the mean time before failure is much
> > higher.
> >
> > I can produce the error pretty fast with the ts_test tool. Without any
> > userland
> > tool using the touchscreen info or with the tool evtest, i can see the 
i2c
> >
> > action on the oscilloscope but it is hard to produce the error.
> > What does this indicate to?
> > Is it possible that userland can influence the kernel driver timing to
> > promote
> > the error?
> 
> I don't think that userland can influence the kernel driver timing;
> based on my understanding of the driver [1] polling occurs from the
> moment the device is probe()'d. Are you modprob'ing the driver right
> before ts_test? Where can I see the ts_test source?

I have the driver build in.
I have the ts_test source from here [2], but this [3] is a more official 
source. 

> 
> Best Regards,
> Ben Gardiner
> 
> [1] 
http://lxr.linux.no/linux+v2.6.38/drivers/input/touchscreen/tps6507x-ts.c
> 

Cheers,

Bastian.

[2] https://github.com/kergoth/tslib/blob/master/tests/ts_test.c

[3] 
http://tslib.sourcearchive.com/documentation/1.0-7build1/ts__test_8c-source.html

Re: How to put extra clock cycles on i2c cavinci, problems on omapl138 (logic pd som)

[Ben Gardiner]

Hi Sekhar,

Thanks for rapidly weighing-in here.

On Thu, Mar 17, 2011 at 10:28 AM, Nori, Sekhar <nsekhar-l0cyMroinI0@public.gmane.org> wrote:
> On Thu, Mar 17, 2011 at 18:55:00, Ben Gardiner wrote:
>
>> I noticed on the e2e forums that there is a reason why the PSP uses a
>> bitbanging i2c interface -- since the 1.0v OPP makes the i2c
>> controller unusable [1] (I've added Sekhar to the CC since he pointed
>> out the problem in that post).
>
> There was a problem with the way I2C was implemented on the SoM. Here is
> the information I have for the modifications required on the SoM.
>
> "
> It is likely that the errors seen were a result of the buffer on the I2C clock.  The hardware fix for this issue is to remove U24 and R161 and short U24 pins 2 to 4.
> "
>
> There should be an EVM with these fixes already, though I have not
> tested it myself.
>
> Also, recently a bug in the I2C driver was brought to my attention.
> In the bus busy function:
>
> /*
>  * Waiting for bus not busy
>  */
> static int i2c_davinci_wait_bus_not_busy(struct davinci_i2c_dev *dev,
>                                         char allow_sleep)
> {
>        unsigned long timeout;
>        static u16 to_cnt;
>
>        timeout = jiffies + dev->adapter.timeout;
>        while (davinci_i2c_read_reg(dev, DAVINCI_I2C_STR_REG)
>               & DAVINCI_I2C_STR_BB) {
>                if (to_cnt <= DAVINCI_I2C_MAX_TRIES) {
>                        if (time_after(jiffies, timeout)) {
>                                dev_warn(dev->dev,
>                                "timeout waiting for bus ready\n");
>                                to_cnt++;
>                                return -ETIMEDOUT;
>                        } else {
>                                to_cnt = 0;
>                                i2c_recover_bus(dev);
>                                i2c_davinci_init(dev);
>                        }
>                }
>                if (allow_sleep)
>                        schedule_timeout(1);
>        }
>
>        return 0;
> }
>
> If there is a context switch immediately after the while statement,
> the loop can return a timeout error while there is none. The
> simple solution would be to check for bus busy again if the
> timeout condition occurs. There should be a more elegant way
> to write the loop, but it is too late in the evening to think
> about it :)
>
> I doubt if this is related to the errors you guys are seeing,
> but it is worth trying anyway.

Thanks. I tried it (anyways) but there was no change in behaviour here
(diff attached).

I've posted to the e2e forum [1]. I hope it turns up some more info.

Best Regards,
Ben Gardiner

[1] http://e2e.ti.com/support/dsp/omap_applications_processors/f/42/p/99895/350080.aspx

---
Nanometrics Inc.
http://www.nanometrics.ca

diff --git a/drivers/i2c/busses/i2c-davinci.c b/drivers/i2c/busses/i2c-davinci.c
index 5795c83..ab8e0f3 100644
--- a/drivers/i2c/busses/i2c-davinci.c
+++ b/drivers/i2c/busses/i2c-davinci.c
@@ -44,7 +44,7 @@

 /* ----- global defines ----------------------------------------------- */

-#define DAVINCI_I2C_TIMEOUT    (1*HZ)
+#define DAVINCI_I2C_TIMEOUT    (2*HZ)
 #define DAVINCI_I2C_MAX_TRIES  2
 #define I2C_DAVINCI_INTR_ALL    (DAVINCI_I2C_IMR_AAS | \
                                 DAVINCI_I2C_IMR_SCD | \
@@ -282,7 +282,10 @@ static int i2c_davinci_wait_bus_not_busy(struct
davinci_i2c_dev *dev,
        while (davinci_i2c_read_reg(dev, DAVINCI_I2C_STR_REG)
               & DAVINCI_I2C_STR_BB) {
                if (to_cnt <= DAVINCI_I2C_MAX_TRIES) {
-                       if (time_after(jiffies, timeout)) {
+                       if (time_after(jiffies, timeout)
+                               && (davinci_i2c_read_reg(dev,
+                                               DAVINCI_I2C_STR_REG)
+                                       & DAVINCI_I2C_STR_BB)) {
                                dev_warn(dev->dev,
                                "timeout waiting for bus ready\n");
                                to_cnt++;

RE: How to put extra clock cycles on i2c cavinci, problems on omapl138 (logic pd som)

[Bastian.Ruppert-qHSRieSIjQqELgA04lAiVw]

Dear Sekhar,

> 
> RE: How to put extra clock cycles on i2c cavinci, problems on 
> omapl138 (logic pd som)
> 
> On Thu, Mar 17, 2011 at 18:55:00, Ben Gardiner wrote:
> 
> > I noticed on the e2e forums that there is a reason why the PSP uses a
> > bitbanging i2c interface -- since the 1.0v OPP makes the i2c
> > controller unusable [1] (I've added Sekhar to the CC since he pointed
> > out the problem in that post).
> 
> There was a problem with the way I2C was implemented on the SoM. Here is
> the information I have for the modifications required on the SoM.
> 
> "
> It is likely that the errors seen were a result of the buffer on the
> I2C clock.  The hardware fix for this issue is to remove U24 and 
> R161 and short U24 pins 2 to 4.
> "

I have a hardware here without this buffer in the I2C clock wire!
 
> There should be an EVM with these fixes already, though I have not
> tested it myself.
> 
> Also, recently a bug in the I2C driver was brought to my attention.
> In the bus busy function:
> 
> /*
>  * Waiting for bus not busy
>  */
> static int i2c_davinci_wait_bus_not_busy(struct davinci_i2c_dev *dev,
>                 char allow_sleep)
> {
>    unsigned long timeout;
>    static u16 to_cnt;
> 
>    timeout = jiffies + dev->adapter.timeout;
>    while (davinci_i2c_read_reg(dev, DAVINCI_I2C_STR_REG)
>           & DAVINCI_I2C_STR_BB) {
>       if (to_cnt <= DAVINCI_I2C_MAX_TRIES) {
>          if (time_after(jiffies, timeout)) {
>             dev_warn(dev->dev,
>             "timeout waiting for bus ready\n");
>             to_cnt++;
>             return -ETIMEDOUT;
>          } else {
>             to_cnt = 0;
>             i2c_recover_bus(dev);
>             i2c_davinci_init(dev);
>          }
>       }
>       if (allow_sleep)
>          schedule_timeout(1);
>    }
> 
>    return 0;
> }
> 
> If there is a context switch immediately after the while statement,
> the loop can return a timeout error while there is none. The
> simple solution would be to check for bus busy again if the
> timeout condition occurs. 

It seems the following patch suggests to your presumption, but 
it is not the solution yet.

diff --git a/drivers/i2c/busses/i2c-davinci.c 
b/drivers/i2c/busses/i2c-davinci.c
index 5795c83..5ddefd6 100644
--- a/drivers/i2c/busses/i2c-davinci.c
+++ b/drivers/i2c/busses/i2c-davinci.c
@@ -283,11 +283,13 @@ static int i2c_davinci_wait_bus_not_busy(struct 
davinci_i2c_dev *dev,
               & DAVINCI_I2C_STR_BB) {
                if (to_cnt <= DAVINCI_I2C_MAX_TRIES) {
                        if (time_after(jiffies, timeout)) {
-                               dev_warn(dev->dev,
+                               dev_err(dev->dev,
                                "timeout waiting for bus ready\n");
                                to_cnt++;
                                return -ETIMEDOUT;
                        } else {
+                               dev_err(dev->dev,
+                               "timeout waiting for bus ready2\n");
                                to_cnt = 0;
                                i2c_recover_bus(dev);
                                i2c_davinci_init(dev);
@@ -381,6 +383,15 @@ i2c_davinci_xfer_msg(struct i2c_adapter *adap, struct 
i2c_msg *msg, int sto
 dev->adapter.timeout);
        if (r == 0) {
                dev_err(dev->dev, "controller timed out\n");
+               if(i2c_davinci_wait_bus_not_busy(dev, 1))
+ 
+                   dev_err(dev->dev, "there is really a timeout\n");
+ 
+               else
+ 
+                   dev_err(dev->dev, "there is no timeout\n");
+ 
+                 }


                dev->buf_len = 0;

This is the result:

tps6507x 1-0048: ADC config write failed
i2c_davinci i2c_davinci.1: controller timed out
i2c_davinci i2c_davinci.1: there is no timeout
i2c_davinci i2c_davinci.1: controller timed out
i2c_davinci i2c_davinci.1: there is no timeout
tps6507x 1-0048: TSC mode read failed
i2c_davinci i2c_davinci.1: controller timed out
i2c_davinci i2c_davinci.1: there is no timeout
i2c_davinci i2c_davinci.1: controller timed out
i2c_davinci i2c_davinci.1: there is no timeout
tps6507x 1-0048: TSC mode read failed

...and so on.

> There should be a more elegant way
> to write the loop, but it is too late in the evening to think
> about it :)
> 

Perhaps you can help us in the morning, please. 

> I doubt if this is related to the errors you guys are seeing,
> but it is worth trying anyway.
> 
> Thanks,
> Sekhar
> 

Best Regards,

Bastian.

RE: How to put extra clock cycles on i2c cavinci, problems on omapl138 (logic pd som)

[Nori, Sekhar]

Hi Bastian,

On Fri, Mar 18, 2011 at 12:13:31, Bastian.Ruppert-qHSRieSIjQqELgA04lAiVw@public.gmane.org wrote:
> Dear Sekhar,
> 
> > 
> > RE: How to put extra clock cycles on i2c cavinci, problems on 
> > omapl138 (logic pd som)
> > 
> > On Thu, Mar 17, 2011 at 18:55:00, Ben Gardiner wrote:
> > 
> > > I noticed on the e2e forums that there is a reason why the PSP uses a
> > > bitbanging i2c interface -- since the 1.0v OPP makes the i2c
> > > controller unusable [1] (I've added Sekhar to the CC since he pointed
> > > out the problem in that post).
> > 
> > There was a problem with the way I2C was implemented on the SoM. Here is
> > the information I have for the modifications required on the SoM.
> > 
> > "
> > It is likely that the errors seen were a result of the buffer on the
> > I2C clock.  The hardware fix for this issue is to remove U24 and 
> > R161 and short U24 pins 2 to 4.
> > "
> 
> I have a hardware here without this buffer in the I2C clock wire!

Okay. So, I guess the two issues have different root causes.

>  
> > There should be an EVM with these fixes already, though I have not
> > tested it myself.
> > 
> > Also, recently a bug in the I2C driver was brought to my attention.
> > In the bus busy function:
> > 
> > /*
> >  * Waiting for bus not busy
> >  */
> > static int i2c_davinci_wait_bus_not_busy(struct davinci_i2c_dev *dev,
> >                 char allow_sleep)
> > {
> >    unsigned long timeout;
> >    static u16 to_cnt;
> > 
> >    timeout = jiffies + dev->adapter.timeout;
> >    while (davinci_i2c_read_reg(dev, DAVINCI_I2C_STR_REG)
> >           & DAVINCI_I2C_STR_BB) {
> >       if (to_cnt <= DAVINCI_I2C_MAX_TRIES) {
> >          if (time_after(jiffies, timeout)) {
> >             dev_warn(dev->dev,
> >             "timeout waiting for bus ready\n");
> >             to_cnt++;
> >             return -ETIMEDOUT;
> >          } else {
> >             to_cnt = 0;
> >             i2c_recover_bus(dev);
> >             i2c_davinci_init(dev);
> >          }
> >       }
> >       if (allow_sleep)
> >          schedule_timeout(1);
> >    }
> > 
> >    return 0;
> > }
> > 
> > If there is a context switch immediately after the while statement,
> > the loop can return a timeout error while there is none. The
> > simple solution would be to check for bus busy again if the
> > timeout condition occurs. 
> 
> It seems the following patch suggests to your presumption, but 
> it is not the solution yet.

Yes, I don't think this bug is the root cause of this issue
either. Ben already tried a fix for this, but it has not
helped him as well.

Thanks,
Sekhar