[Qemu-devel] [PATCH] bitbang_i2c: Fix spurious slave read after NACK

[Qemu-devel] [PATCH] bitbang_i2c: Fix spurious slave read after NACK

[Marcus Comstedt]

After NACKing a read operation, a raising SCL should not trigger a new
read from the slave.  Introduce a new state which just waits for a stop
or start condition after NACK.

Signed-off-by: Marcus Comstedt <marcus@mc.pp.se>
---
 hw/bitbang_i2c.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/hw/bitbang_i2c.c b/hw/bitbang_i2c.c
index 4ee99a1..2937b5c 100644
--- a/hw/bitbang_i2c.c
+++ b/hw/bitbang_i2c.c
@@ -38,7 +38,8 @@ typedef enum bitbang_i2c_state {
     RECEIVING_BIT2,
     RECEIVING_BIT1,
     RECEIVING_BIT0,
-    SENDING_ACK
+    SENDING_ACK,
+    SENT_NACK
 } bitbang_i2c_state;
 
 struct bitbang_i2c_interface {
@@ -115,6 +116,7 @@ int bitbang_i2c_set(bitbang_i2c_interface *i2c, int line, int level)
     }
     switch (i2c->state) {
     case STOPPED:
+    case SENT_NACK:
         return bitbang_i2c_ret(i2c, 1);
 
     case SENDING_BIT7 ... SENDING_BIT0:
@@ -155,6 +157,7 @@ int bitbang_i2c_set(bitbang_i2c_interface *i2c, int line, int level)
         i2c->state = RECEIVING_BIT7;
         if (data != 0) {
             DPRINTF("NACKED\n");
+            i2c->state = SENT_NACK;
             i2c_nack(i2c->bus);
         } else {
             DPRINTF("ACKED\n");
-- 
1.7.4.1

Re: [Qemu-devel] [Qemu-trivial] [PATCH] bitbang_i2c: Fix spurious slave read after NACK

[Stefan Hajnoczi]

On Sat, May 28, 2011 at 3:55 PM, Marcus Comstedt <marcus@mc.pp.se> wrote:
> After NACKing a read operation, a raising SCL should not trigger a new
> read from the slave.  Introduce a new state which just waits for a stop
> or start condition after NACK.
>
> Signed-off-by: Marcus Comstedt <marcus@mc.pp.se>
> ---
>  hw/bitbang_i2c.c |    5 ++++-
>  1 files changed, 4 insertions(+), 1 deletions(-)

Seems like a simple change but I'm not familiar with the code or I2C
details.  Andrzej, care to take a look?

> diff --git a/hw/bitbang_i2c.c b/hw/bitbang_i2c.c
> index 4ee99a1..2937b5c 100644
> --- a/hw/bitbang_i2c.c
> +++ b/hw/bitbang_i2c.c
> @@ -38,7 +38,8 @@ typedef enum bitbang_i2c_state {
>     RECEIVING_BIT2,
>     RECEIVING_BIT1,
>     RECEIVING_BIT0,
> -    SENDING_ACK
> +    SENDING_ACK,
> +    SENT_NACK
>  } bitbang_i2c_state;
>
>  struct bitbang_i2c_interface {
> @@ -115,6 +116,7 @@ int bitbang_i2c_set(bitbang_i2c_interface *i2c, int line, int level)
>     }
>     switch (i2c->state) {
>     case STOPPED:
> +    case SENT_NACK:
>         return bitbang_i2c_ret(i2c, 1);
>
>     case SENDING_BIT7 ... SENDING_BIT0:
> @@ -155,6 +157,7 @@ int bitbang_i2c_set(bitbang_i2c_interface *i2c, int line, int level)
>         i2c->state = RECEIVING_BIT7;
>         if (data != 0) {
>             DPRINTF("NACKED\n");
> +            i2c->state = SENT_NACK;
>             i2c_nack(i2c->bus);
>         } else {
>             DPRINTF("ACKED\n");
> --
> 1.7.4.1
>
>
>

Re: [Qemu-devel] [Qemu-trivial] [PATCH] bitbang_i2c: Fix spurious slave read after NACK

[andrzej zaborowski]

Hi,

On 28 May 2011 21:52, Stefan Hajnoczi <stefanha@gmail.com> wrote:
> On Sat, May 28, 2011 at 3:55 PM, Marcus Comstedt <marcus@mc.pp.se> wrote:
>> After NACKing a read operation, a raising SCL should not trigger a new
>> read from the slave.  Introduce a new state which just waits for a stop
>> or start condition after NACK.

Makes sense, I pushed this change to master.

Cheers