All of lore.kernel.org
 help / color / mirror / Atom feed
From: Richard Henderson <richard.henderson@linaro.org>
To: "Philippe Mathieu-Daudé" <f4bug@amsat.org>, qemu-devel@nongnu.org
Cc: Corey Minyard <cminyard@mvista.com>,
	"Signed-off-by : Frederic Konrad" <frederic.konrad@adacore.com>,
	qemu-arm@nongnu.org, qemu-ppc@nongnu.org
Subject: Re: [PATCH v3 13/13] hw/i2c: Make i2c_start_transfer() direction argument a boolean
Date: Wed, 16 Jun 2021 12:02:47 -0700	[thread overview]
Message-ID: <31490775-a16e-1e4e-a180-d3c473cc87be@linaro.org> (raw)
In-Reply-To: <20210616161418.2514095-14-f4bug@amsat.org>

On 6/16/21 9:14 AM, Philippe Mathieu-Daudé wrote:
> From: BALATON Zoltan <balaton@eik.bme.hu>
> 
> Make the argument representing the direction of the transfer a
> boolean type.
> Rename the boolean argument as 'is_recv' to match i2c_recv_send().
> Document the function prototype.
> 
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> Message-Id: <20200621145235.9E241745712@zero.eik.bme.hu>
> [PMD: Split patch, added docstring]
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>   include/hw/i2c/i2c.h | 11 ++++++++++-
>   hw/i2c/core.c        |  4 ++--
>   2 files changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/include/hw/i2c/i2c.h b/include/hw/i2c/i2c.h
> index 2adf521b271..5fe94c62c00 100644
> --- a/include/hw/i2c/i2c.h
> +++ b/include/hw/i2c/i2c.h
> @@ -80,7 +80,16 @@ struct I2CBus {
>   
>   I2CBus *i2c_init_bus(DeviceState *parent, const char *name);
>   int i2c_bus_busy(I2CBus *bus);
> -int i2c_start_transfer(I2CBus *bus, uint8_t address, int recv);
> +/**
> + * i2c_start_transfer: start a transfer on an I2C bus.
> + *
> + * @bus: #I2CBus to be used
> + * @address: address of the slave
> + * @is_recv: indicates the transfer direction
> + *
> + * Returns: 0 on success, -1 on error
> + */
> +int i2c_start_transfer(I2CBus *bus, uint8_t address, bool is_recv);
>   void i2c_end_transfer(I2CBus *bus);
>   void i2c_nack(I2CBus *bus);
>   int i2c_send(I2CBus *bus, uint8_t data);
> diff --git a/hw/i2c/core.c b/hw/i2c/core.c
> index 6af24c9e797..6639ca8c2e0 100644
> --- a/hw/i2c/core.c
> +++ b/hw/i2c/core.c
> @@ -115,7 +115,7 @@ bool i2c_scan_bus(I2CBus *bus, uint8_t address, bool broadcast,
>    * without releasing the bus.  If that fails, the bus is still
>    * in a transaction.
>    */
> -int i2c_start_transfer(I2CBus *bus, uint8_t address, int recv)
> +int i2c_start_transfer(I2CBus *bus, uint8_t address, bool is_recv)
>   {
>       I2CSlaveClass *sc;
>       I2CNode *node;
> @@ -157,7 +157,7 @@ int i2c_start_transfer(I2CBus *bus, uint8_t address, int recv)
>   
>           if (sc->event) {
>               trace_i2c_event("start", s->address);
> -            rv = sc->event(s, recv ? I2C_START_RECV : I2C_START_SEND);
> +            rv = sc->event(s, is_recv ? I2C_START_RECV : I2C_START_SEND);

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

I did wonder about passing in the I2C_START_{RECV,SEND} enum directly, as that 
auto-documents the sense of the argument.  But there are quite a few instances remaining 
which would need updating.

Perhaps one more patch would be nice: introduce i2c_start_{send,recv} as inline wrappers?


r~


  reply	other threads:[~2021-06-16 19:03 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-16 16:14 [PATCH v3 00/13] hw/i2c: Remove confusing i2c_send_recv() API Philippe Mathieu-Daudé
2021-06-16 16:14 ` [PATCH v3 01/13] hw/input/lm832x: Move lm832x_key_event() declaration to "lm832x.h" Philippe Mathieu-Daudé
2021-06-16 18:38   ` Richard Henderson
2021-06-16 19:23     ` Philippe Mathieu-Daudé
2021-06-16 16:14 ` [PATCH v3 02/13] hw/input/lm832x: Define TYPE_LM8323 in public header Philippe Mathieu-Daudé
2021-06-16 18:39   ` Richard Henderson
2021-06-16 16:14 ` [PATCH v3 03/13] hw/display/sm501: Simplify sm501_i2c_write() logic Philippe Mathieu-Daudé
2021-06-16 18:39   ` Richard Henderson
2021-06-16 16:14 ` [PATCH v3 04/13] hw/display/sm501: Replace i2c_send_recv() by i2c_recv() & i2c_send() Philippe Mathieu-Daudé
2021-06-16 18:40   ` Richard Henderson
2021-06-16 19:14   ` Corey Minyard
2021-06-16 16:14 ` [PATCH v3 05/13] hw/i2c/ppc4xx_i2c: Add reference to datasheet Philippe Mathieu-Daudé
2021-06-16 18:40   ` Richard Henderson
2021-06-16 16:14 ` [PATCH v3 06/13] hw/i2c/ppc4xx_i2c: Replace i2c_send_recv() by i2c_recv() & i2c_send() Philippe Mathieu-Daudé
2021-06-16 18:41   ` Richard Henderson
2021-06-16 19:16   ` Corey Minyard
2021-06-16 19:25     ` Philippe Mathieu-Daudé
2021-06-16 20:01       ` BALATON Zoltan
2021-06-16 20:28         ` Philippe Mathieu-Daudé
2021-06-16 23:09           ` BALATON Zoltan
2021-06-16 23:42             ` Corey Minyard
2021-06-17 23:49           ` BALATON Zoltan
2021-06-18  8:54             ` Philippe Mathieu-Daudé
2021-06-16 16:14 ` [PATCH v3 07/13] hw/misc/auxbus: Fix MOT/classic I2C mode Philippe Mathieu-Daudé
2021-06-16 18:41   ` Richard Henderson
2021-06-16 16:14 ` [PATCH v3 08/13] hw/misc/auxbus: Explode READ_I2C / WRITE_I2C_MOT cases Philippe Mathieu-Daudé
2021-06-16 18:41   ` Richard Henderson
2021-06-16 16:14 ` [PATCH v3 09/13] hw/misc/auxbus: Replace 'is_write' boolean by its value Philippe Mathieu-Daudé
2021-06-16 18:43   ` Richard Henderson
2021-06-16 16:14 ` [PATCH v3 10/13] hw/misc/auxbus: Replace i2c_send_recv() by i2c_recv() & i2c_send() Philippe Mathieu-Daudé
2021-06-16 18:46   ` Richard Henderson
2021-06-16 19:28     ` Philippe Mathieu-Daudé
2021-06-16 20:06       ` BALATON Zoltan
2021-06-16 16:14 ` [PATCH v3 11/13] hw/i2c: Remove confusing i2c_send_recv() Philippe Mathieu-Daudé
2021-06-16 18:47   ` Richard Henderson
2021-06-16 16:14 ` [PATCH v3 12/13] hw/i2c: Rename i2c_set_slave_address() -> i2c_slave_set_address() Philippe Mathieu-Daudé
2021-06-16 18:50   ` Richard Henderson
2021-06-16 16:14 ` [PATCH v3 13/13] hw/i2c: Make i2c_start_transfer() direction argument a boolean Philippe Mathieu-Daudé
2021-06-16 19:02   ` Richard Henderson [this message]
2021-06-16 19:28 ` [PATCH v3 00/13] hw/i2c: Remove confusing i2c_send_recv() API Corey Minyard

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=31490775-a16e-1e4e-a180-d3c473cc87be@linaro.org \
    --to=richard.henderson@linaro.org \
    --cc=cminyard@mvista.com \
    --cc=f4bug@amsat.org \
    --cc=frederic.konrad@adacore.com \
    --cc=qemu-arm@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-ppc@nongnu.org \
    --subject='Re: [PATCH v3 13/13] hw/i2c: Make i2c_start_transfer() direction argument a boolean' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.