On 11/30/18 11:25 AM, Peter Maydell wrote:
On Mon, 26 Nov 2018 at 20:04, <miny...@acm.org> wrote:
From: Corey Minyard <cminy...@mvista.com>
i2c_recv() cannot fail, so there is no need to check the return
value. It also returns unt8_t, so comparing with < 0 is not
meaningful.
Fix up various I2C controllers to remove the unneeded code.
Signed-off-by: Corey Minyard <cminy...@mvista.com>
Suggested-by: Peter Maydell <peter.mayd...@linaro.org>
---
diff --git a/hw/i2c/exynos4210_i2c.c b/hw/i2c/exynos4210_i2c.c
index c96fa7d7be..43f284eab7 100644
--- a/hw/i2c/exynos4210_i2c.c
+++ b/hw/i2c/exynos4210_i2c.c
@@ -106,12 +106,12 @@ static inline void
exynos4210_i2c_raise_interrupt(Exynos4210I2CState *s)
static void exynos4210_i2c_data_receive(void *opaque)
{
Exynos4210I2CState *s = (Exynos4210I2CState *)opaque;
- int ret;
+ uint8_t ret;
s->i2cstat &= ~I2CSTAT_LAST_BIT;
s->scl_free = false;
ret = i2c_recv(s->bus);
- if (ret < 0 && (s->i2ccon & I2CCON_ACK_GEN)) {
+ if (s->i2ccon & I2CCON_ACK_GEN) {
s->i2cstat |= I2CSTAT_LAST_BIT; /* Data is not acknowledged */
Previously the code was doing this only if i2c_recv()
failed (returned a negative value) and ACK_GEN was set.
i2c_recv() can never fail, so this if() {} branch should
be deleted entirely ("false && something" simplifies
to "false", not "something").
Oops, yes. Plus you can just remove the ret variable and simplify
this a lot. Thanks for finding this.
-corey
} else {
s->i2cds = ret;
The rest of the patch looks good.
thanks
-- PMM
