Re: [Qemu-devel] [PATCH V7 3/9] i2c: Factor our send() and recv() common logic

[Alistair Francis]

On Mon, Feb 1, 2016 at 7:43 AM,  <address@hidden> wrote:
> From: Peter Crosthwaite <address@hidden>
>
> Most of the control flow logic between send and recv (error checking
> etc) is the same. Factor this out into a common send_recv() API.
> This is then usable by clients, where the control logic for send
> and receive differs only by a boolean. E.g.
>
> if (send)
>    i2c_send(...):
> else
>    i2c_recv(...);
>
> becomes:
>
> i2c_send_recv(... , send);
>
> Signed-off-by: Peter Crosthwaite <address@hidden>
> Changes from FK:
>   * Rebased on master.
>   * Rebased on my i2c broadcast patch.
> Signed-off-by: KONRAD Frederic <address@hidden>

Reviewed-by: Alistair Francis <address@hidden>

Thanks,

Alistair

> ---
>  hw/i2c/core.c        | 48 ++++++++++++++++++++++++++++++++----------------
>  include/hw/i2c/i2c.h |  1 +
>  2 files changed, 33 insertions(+), 16 deletions(-)
>
> diff --git a/hw/i2c/core.c b/hw/i2c/core.c
> index a3921d9..a49138f 100644
> --- a/hw/i2c/core.c
> +++ b/hw/i2c/core.c
> @@ -148,34 +148,50 @@ void i2c_end_transfer(I2CBus *bus)
>      bus->broadcast = false;
>  }
>
> -int i2c_send(I2CBus *bus, uint8_t data)
> +int i2c_send_recv(I2CBus *bus, uint8_t *data, bool send)
>  {
>      I2CSlaveClass *sc;
>      I2CNode *node;
>      int ret = -1;
>
> -    QLIST_FOREACH(node, &bus->current_devs, next) {
> -        sc = I2C_SLAVE_GET_CLASS(node->elt);
> -        if (sc->send) {
> -            ret |= sc->send(node->elt, data);
> +    if (send) {
> +        QLIST_FOREACH(node, &bus->current_devs, next) {
> +            sc = I2C_SLAVE_GET_CLASS(node->elt);
> +            if (sc->send) {
> +                ret |= sc->send(node->elt, *data);
> +            }
> +        }
> +        return ret;
> +    } else {
> +        if ((QLIST_EMPTY(&bus->current_devs)) || (bus->broadcast)) {
> +            return -1;
>          }
> +
> +        sc = I2C_SLAVE_GET_CLASS(QLIST_FIRST(&bus->current_devs)->elt);
> +        if (sc->recv) {
> +            ret = sc->recv(QLIST_FIRST(&bus->current_devs)->elt);
> +            if (ret < 0) {
> +                return ret;
> +            } else {
> +                *data = ret;
> +                return 0;
> +            }
> +        }
> +        return -1;
>      }
> -    return ret;
>  }
>
> -int i2c_recv(I2CBus *bus)
> +int i2c_send(I2CBus *bus, uint8_t data)
>  {
> -    I2CSlaveClass *sc;
> +    return i2c_send_recv(bus, &data, true);
> +}
>
> -    if ((QLIST_EMPTY(&bus->current_devs)) || (bus->broadcast)) {
> -        return -1;
> -    }
> +int i2c_recv(I2CBus *bus)
> +{
> +    uint8_t data;
> +    int ret = i2c_send_recv(bus, &data, false);
>
> -    sc = I2C_SLAVE_GET_CLASS(QLIST_FIRST(&bus->current_devs)->elt);
> -    if (sc->recv) {
> -        return sc->recv(QLIST_FIRST(&bus->current_devs)->elt);
> -    }
> -    return -1;
> +    return ret < 0 ? ret : data;
>  }
>
>  void i2c_nack(I2CBus *bus)
> diff --git a/include/hw/i2c/i2c.h b/include/hw/i2c/i2c.h
> index 4986ebc..c4085aa 100644
> --- a/include/hw/i2c/i2c.h
> +++ b/include/hw/i2c/i2c.h
> @@ -56,6 +56,7 @@ int i2c_bus_busy(I2CBus *bus);
>  int i2c_start_transfer(I2CBus *bus, uint8_t address, int recv);
>  void i2c_end_transfer(I2CBus *bus);
>  void i2c_nack(I2CBus *bus);
> +int i2c_send_recv(I2CBus *bus, uint8_t *data, bool send);
>  int i2c_send(I2CBus *bus, uint8_t data);
>  int i2c_recv(I2CBus *bus);
>
> --
> 1.9.0
>
>