Re: [Qemu-devel] [PATCH v3 04/16] i2c: Don't check return value from i2c_recv()

[Peter Maydell]

On Mon, 26 Nov 2018 at 20:04, <address@hidden> wrote:
>
> From: Corey Minyard <address@hidden>
>
> 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 <address@hidden>
> Suggested-by: Peter Maydell <address@hidden>
> ---

> 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").

>      } else {
>          s->i2cds = ret;

The rest of the patch looks good.

thanks
-- PMM