[PATCH v2 1/3] i2c: Match parameters of i2c_start_transfer and i2c_send_recv

[Philippe Mathieu-Daudé]

From: BALATON Zoltan <balaton@eik.bme.hu>

These functions have a parameter that decides the direction of
transfer but totally confusingly they don't match but inverted sense.
To avoid frequent mistakes when using these functions change
i2c_send_recv to match i2c_start_transfer.

Rename i2c_send_recv() as i2c_recv_send() and rename the 'send'
argument as 'is_recv' (its boolean opposite). Invert the logic
in i2c_recv_send() to not use inverted boolean value check.

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/display/sm501.c   |  4 ++--
 hw/i2c/core.c        | 32 ++++++++++++++++----------------
 hw/i2c/ppc4xx_i2c.c  |  2 +-
 hw/misc/auxbus.c     |  4 ++--
 5 files changed, 31 insertions(+), 22 deletions(-)

diff --git a/include/hw/i2c/i2c.h b/include/hw/i2c/i2c.h
index 4117211565..bc70c5b43d 100644
--- a/include/hw/i2c/i2c.h
+++ b/include/hw/i2c/i2c.h
@@ -75,7 +75,16 @@ 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);
+/**
+ * i2c_recv_send: receive from or send to an I2C bus.
+ *
+ * @bus: #I2CBus to be used
+ * @data: buffer with the data transferred
+ * @is_recv: indicates the transfer direction
+ *
+ * Returns: 0 on success, -1 on error
+ */
+int i2c_recv_send(I2CBus *bus, uint8_t *data, bool is_recv);
 int i2c_send(I2CBus *bus, uint8_t data);
 uint8_t i2c_recv(I2CBus *bus);
 
diff --git a/hw/display/sm501.c b/hw/display/sm501.c
index a7fc08c52b..56e35cf4ae 100644
--- a/hw/display/sm501.c
+++ b/hw/display/sm501.c
@@ -1040,8 +1040,8 @@ static void sm501_i2c_write(void *opaque, hwaddr addr, 
uint64_t value,
                     SM501_DPRINTF("sm501 i2c : transferring %d bytes to 
0x%x\n",
                                   s->i2c_byte_count + 1, s->i2c_addr >> 1);
                     for (i = 0; i <= s->i2c_byte_count; i++) {
-                        res = i2c_send_recv(s->i2c_bus, &s->i2c_data[i],
-                                            !(s->i2c_addr & 1));
+                        res = i2c_recv_send(s->i2c_bus, &s->i2c_data[i],
+                                            s->i2c_addr & 1);
                         if (res) {
                             SM501_DPRINTF("sm501 i2c : transfer failed"
                                           " i=%d, res=%d\n", i, res);
diff --git a/hw/i2c/core.c b/hw/i2c/core.c
index 1aac457a2a..b83c2b5b89 100644
--- a/hw/i2c/core.c
+++ b/hw/i2c/core.c
@@ -175,26 +175,14 @@ void i2c_end_transfer(I2CBus *bus)
     bus->broadcast = false;
 }
 
-int i2c_send_recv(I2CBus *bus, uint8_t *data, bool send)
+int i2c_recv_send(I2CBus *bus, uint8_t *data, bool is_recv)
 {
     I2CSlaveClass *sc;
     I2CSlave *s;
     I2CNode *node;
     int ret = 0;
 
-    if (send) {
-        QLIST_FOREACH(node, &bus->current_devs, next) {
-            s = node->elt;
-            sc = I2C_SLAVE_GET_CLASS(s);
-            if (sc->send) {
-                trace_i2c_send(s->address, *data);
-                ret = ret || sc->send(s, *data);
-            } else {
-                ret = -1;
-            }
-        }
-        return ret ? -1 : 0;
-    } else {
+    if (is_recv) {
         ret = 0xff;
         if (!QLIST_EMPTY(&bus->current_devs) && !bus->broadcast) {
             sc = I2C_SLAVE_GET_CLASS(QLIST_FIRST(&bus->current_devs)->elt);
@@ -206,19 +194,31 @@ int i2c_send_recv(I2CBus *bus, uint8_t *data, bool send)
         }
         *data = ret;
         return 0;
+    } else {
+        QLIST_FOREACH(node, &bus->current_devs, next) {
+            s = node->elt;
+            sc = I2C_SLAVE_GET_CLASS(s);
+            if (sc->send) {
+                trace_i2c_send(s->address, *data);
+                ret = ret || sc->send(s, *data);
+            } else {
+                ret = -1;
+            }
+        }
+        return ret ? -1 : 0;
     }
 }
 
 int i2c_send(I2CBus *bus, uint8_t data)
 {
-    return i2c_send_recv(bus, &data, true);
+    return i2c_recv_send(bus, &data, false);
 }
 
 uint8_t i2c_recv(I2CBus *bus)
 {
     uint8_t data = 0xff;
 
-    i2c_send_recv(bus, &data, false);
+    i2c_recv_send(bus, &data, true);
     return data;
 }
 
diff --git a/hw/i2c/ppc4xx_i2c.c b/hw/i2c/ppc4xx_i2c.c
index c0a8e04567..f0b0de19b0 100644
--- a/hw/i2c/ppc4xx_i2c.c
+++ b/hw/i2c/ppc4xx_i2c.c
@@ -239,7 +239,7 @@ static void ppc4xx_i2c_writeb(void *opaque, hwaddr addr, 
uint64_t value,
                     }
                 }
                 if (!(i2c->sts & IIC_STS_ERR) &&
-                    i2c_send_recv(i2c->bus, &i2c->mdata[i], !recv)) {
+                    i2c_recv_send(i2c->bus, &i2c->mdata[i], recv)) {
                     i2c->sts |= IIC_STS_ERR;
                     i2c->extsts |= IIC_EXTSTS_XFRA;
                     break;
diff --git a/hw/misc/auxbus.c b/hw/misc/auxbus.c
index da361baa32..cef0d0d6d0 100644
--- a/hw/misc/auxbus.c
+++ b/hw/misc/auxbus.c
@@ -148,7 +148,7 @@ AUXReply aux_request(AUXBus *bus, AUXCommand cmd, uint32_t 
address,
 
         ret = AUX_I2C_ACK;
         while (len > 0) {
-            if (i2c_send_recv(i2c_bus, data++, is_write) < 0) {
+            if (i2c_recv_send(i2c_bus, data++, is_write) < 0) {
                 ret = AUX_I2C_NACK;
                 break;
             }
@@ -189,7 +189,7 @@ AUXReply aux_request(AUXBus *bus, AUXCommand cmd, uint32_t 
address,
         bus->last_transaction = cmd;
         bus->last_i2c_address = address;
         while (len > 0) {
-            if (i2c_send_recv(i2c_bus, data++, is_write) < 0) {
+            if (i2c_recv_send(i2c_bus, data++, is_write) < 0) {
                 i2c_end_transfer(i2c_bus);
                 break;
             }
-- 
2.21.3