qemu-ppc
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v3 13/13] hw/i2c: Make i2c_start_transfer() direction argumen


From: Richard Henderson
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
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.8.1

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 via email to

[Prev in Thread] Current Thread [Next in Thread]