[PATCH] i2c-pxa.c recover from false start

[PATCH] i2c-pxa.c recover from false start

[Chuck Kamas]

[-- Attachment #1.1: Type: text/plain, Size: 1609 bytes --]

Hi all,

 

I made this patch to recover from a false start on the bus.  This
scenario happens in my system when a device is hot-inserted onto the bus
causing a false start.  This is caused by the new device pulling SDA
low.  The PXA then thinks someone is trying to talk on the bus and will
not transmit.  My solution is to detect this fault and reset the pxa's
i2c subsystem... not nice I know.  If there is a better way, let me
know!

 

Chuck

 

--- i2c-pxa.c.orig      2008-06-12 00:22:11.000000000 -0700

+++ i2c-pxa.c   2008-06-17 15:34:45.000000000 -0700

@@ -540,6 +540,18 @@

        writel(icr | ICR_START | ICR_TB, _ICR(i2c));

 }

 

+static inline void i2c_pxa_stop_message(struct pxa_i2c *i2c)

+{

+       u32 icr;

+

+       /*

+        * Clear the STOP and ACK flags

+        */

+       icr = readl(_ICR(i2c));

+       icr &= ~(ICR_STOP | ICR_ACKNAK);

+       writel(icr, _ICR(i2c));

+}

+

 /*

  * We are protected by the adapter bus mutex.

  */

@@ -554,6 +566,7 @@

        ret = i2c_pxa_wait_bus_not_busy(i2c);

        if (ret) {

                dev_err(&i2c->adap.dev, "i2c_pxa: timeout waiting for
bus free\n");

+               i2c_pxa_reset(i2c);

                goto out;

        }

 

@@ -582,6 +595,7 @@

         * The rest of the processing occurs in the interrupt handler.

         */

        timeout = wait_event_timeout(i2c->wait, i2c->msg_num == 0, HZ *
5);

+       i2c_pxa_stop_message(i2c);

 

        /*

         * We place the return code in i2c->msg_idx.


[-- Attachment #1.2: Type: text/html, Size: 8812 bytes --]

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [PATCH] i2c-pxa.c recover from false start

[Jean Delvare]

Hi Chuck,

No HTML mails, please.

On Tue, 17 Jun 2008 15:39:16 -0700, Chuck Kamas wrote:
> I made this patch to recover from a false start on the bus.  This
> scenario happens in my system when a device is hot-inserted onto the bus
> causing a false start.  This is caused by the new device pulling SDA
> low.  The PXA then thinks someone is trying to talk on the bus and will
> not transmit.  My solution is to detect this fault and reset the pxa's
> i2c subsystem... not nice I know.  If there is a better way, let me
> know!

How do you differentiate between this case and another master starting
a real transaction on the bus?

What you have is a hardware issue. If you hotplug I2C devices, then you
must ensure that they will not pull either SCL not SDA low when you do.
Otherwise you are not only confusing the PXA master, but also quite
possibly the other slaves on the I2C bus. Maybe you have to put the
new device behind a gate, and open the gate once the device is properly
connected and initialized (and likewise close the gate before you
remove the device.)

I think you have to fix your hardware setup, not the i2c-pxa driver. 

-- 
Jean Delvare

_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [PATCH] i2c-pxa.c recover from false start

[Jean Delvare]

Hi Chuck,

On Thu, 19 Jun 2008 09:02:49 -0700, Chuck Kamas wrote:
> I have to disagree with you on the hardware issue.  The i2c bus must be
> able to recover from any and all issues.

Definitely not. To go to an extreme example, you're not going to recover
from a nuclear attach aimed at your PXA system ;) More seriously, we
only want to make our drivers robust to problems which can reasonably
happen. Otherwise the code would be slow and unmaintainable.

>                                           The fact that a hot plug is
> how I discovered this issue does not mean that that is the only why for
> the PXA to get locked up.  I can think of a few more: ESD, RFI, bus
> glitch...

My time to disagree. You should protect your system from physical
interferences at the hardware level. Compensating poor hardware design
with software workarounds is a bad practice.

You mentioned the device hot-plug as the reason why you wanted this
patch to be applied, I didn't. Now you mention other reasons why this
patch should still be applied, but I suspect you haven't experienced
any of these.

>         You are correct that the patch should look for another master
> talking on the bus before resetting itself. If another master is using
> the bus we have a few scenarios.  1) He is talking to us and we are a
> slave, in which case we would not time out because our slave process
> would be taking care of business. 2) He is talking to another.  In this
> case we can reset our engine/HW without fear of upsetting that traffic.
> I believe your concern is that resetting our engine/HW will interfere
> with the traffic.  If this is true we have another problem to contend
> with.

I am not familiar with the PXA hardware, so I don't know what really
happens when you reset the engine. I can imagine though that you lose
the notion that the bus is busy, as this is typically done by detecting
start and stop conditions on the bus. So I presume you could start a new
transaction while the bus is already in use. No good.

>     3) There is no one talking on the bus and our HW is in a bad
> state.  This is the case I am trying to cover.

But by doing so, I suspect that you're breaking case 2) which is
legitimate and more likely to happen. That's the reason why I don't
want to apply your patch.

> As to the time out issue, we should use the smbus' specification as our
> guide.

Not all I2C devices comply with the SMBus specification, so you have to
be careful if you go that route. I'm not too sure what you had in mind
anyway.

If something really needs to be done, please discuss this with someone
familiar with the i2c-pxa driver, and come up with a proper patch
explaining what it does, why, and why it is correct.

-- 
Jean Delvare

_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [PATCH] i2c-pxa.c recover from false start

[Chuck Kamas]

Jean,

> please discuss this with someone
>familiar with the i2c-pxa driver

Who is the official maintainer of the PXA code?  I would like very much
to come up with a better way to deal with bus faults, and Nukes, that
the reset work around I came up with.

Chuck


_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [PATCH] i2c-pxa.c recover from false start

[Jean Delvare]

On Tue, 24 Jun 2008 09:13:11 -0700, Chuck Kamas wrote:
> > please discuss this with someone
> >familiar with the i2c-pxa driver
> 
> Who is the official maintainer of the PXA code?  I would like very much
> to come up with a better way to deal with bus faults, and Nukes, that
> the reset work around I came up with.

File MAINTAINERS apparently has the answer to this question. You might
additionally want to check
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=history;f=drivers/i2c/busses/i2c-pxa.c
for a list of recent contributors to the i2c-pxa driver.

-- 
Jean Delvare

_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c