Ping? This is a bugfix and a coverity warning fix so it would be nice if it could go into 2.8.
thanks -- PMM On 24 October 2016 at 19:12, Peter Maydell <[email protected]> wrote: > If the guest attempts to talk to a nonexistent device over i2c, > the i2c_start_transfer() function will return non-zero, indicating > that the bus is signalling a NACK. Similarly, if the i2c_send() > function returns nonzero then the target device returned a NACK. > Handle this possibility in the bitbang_i2c code, by returning > the state machine to the STOPPED state and returning the NACK > bit to the guest. > > This bit of missing functionality was spotted by Coverity > (it noticed that we weren't checking the return value from > i2c_start_transfer()). > > Signed-off-by: Peter Maydell <[email protected]> > --- > Lightly tested using the musicpal board, which is the only one > using the bitbang_i2c code. If you make the board put the > Wolfson 8750 at the wrong I2C address then the guest will > retry the transaction a few times and give up, as expected. > Without this patch it will send a bunch of data out into > the void without realizing there's a problem. > --- > hw/i2c/bitbang_i2c.c | 19 +++++++++++++++---- > 1 file changed, 15 insertions(+), 4 deletions(-) > > diff --git a/hw/i2c/bitbang_i2c.c b/hw/i2c/bitbang_i2c.c > index d3a2989..8be88ee 100644 > --- a/hw/i2c/bitbang_i2c.c > +++ b/hw/i2c/bitbang_i2c.c > @@ -130,14 +130,25 @@ int bitbang_i2c_set(bitbang_i2c_interface *i2c, int > line, int level) > return bitbang_i2c_ret(i2c, 1); > > case WAITING_FOR_ACK: > + { > + int ret; > + > if (i2c->current_addr < 0) { > i2c->current_addr = i2c->buffer; > DPRINTF("Address 0x%02x\n", i2c->current_addr); > - i2c_start_transfer(i2c->bus, i2c->current_addr >> 1, > - i2c->current_addr & 1); > + ret = i2c_start_transfer(i2c->bus, i2c->current_addr >> 1, > + i2c->current_addr & 1); > } else { > DPRINTF("Sent 0x%02x\n", i2c->buffer); > - i2c_send(i2c->bus, i2c->buffer); > + ret = i2c_send(i2c->bus, i2c->buffer); > + } > + if (ret) { > + /* NACK (either addressing a nonexistent device, or the > + * device we were sending to decided to NACK us). > + */ > + DPRINTF("Got NACK\n"); > + bitbang_i2c_enter_stop(i2c); > + return bitbang_i2c_ret(i2c, 1); > } > if (i2c->current_addr & 1) { > i2c->state = RECEIVING_BIT7; > @@ -145,7 +156,7 @@ int bitbang_i2c_set(bitbang_i2c_interface *i2c, int line, > int level) > i2c->state = SENDING_BIT7; > } > return bitbang_i2c_ret(i2c, 0); > - > + } > case RECEIVING_BIT7: > i2c->buffer = i2c_recv(i2c->bus); > DPRINTF("RX byte 0x%02x\n", i2c->buffer); > -- > 2.7.4 > >
