From: BALATON Zoltan <balaton@eik.bme.hu>
To: qemu-devel@nongnu.org
Cc: Corey Minyard <cminyard@mvista.com>,
KONRAD Frederic <frederic.konrad@adacore.com>,
David Gibson <david@gibson.dropbear.id.au>
Subject: [PATCH] i2c: Match parameters of i2c_start_transfer and i2c_send_recv
Date: Sun, 21 Jun 2020 16:43:38 +0200 [thread overview]
Message-ID: <20200621145235.9E241745712@zero.eik.bme.hu> (raw)
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. Also use bool in
i2c_start_transfer instead of int to match i2c_send_recv.
Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
Looks like hw/misc/auxbus.c already got this wrong and calls both
i2c_start_transfer and i2c_send_recv with same is_write parameter.
Although the name of the is_write variable suggest this may need to be
inverted I'm not sure what that value actially means and which usage
was correct so I did not touch it. Someone knowing this device might
want to review and fix it.
hw/display/sm501.c | 2 +-
hw/i2c/core.c | 34 +++++++++++++++++-----------------
hw/i2c/ppc4xx_i2c.c | 2 +-
include/hw/i2c/i2c.h | 4 ++--
4 files changed, 21 insertions(+), 21 deletions(-)
diff --git a/hw/display/sm501.c b/hw/display/sm501.c
index 2db347dcbc..ccd0a6e376 100644
--- a/hw/display/sm501.c
+++ b/hw/display/sm501.c
@@ -1034,7 +1034,7 @@ static void sm501_i2c_write(void *opaque, hwaddr addr, uint64_t value,
int i;
for (i = 0; i <= s->i2c_byte_count; i++) {
res = i2c_send_recv(s->i2c_bus, &s->i2c_data[i],
- !(s->i2c_addr & 1));
+ s->i2c_addr & 1);
if (res) {
s->i2c_status |= SM501_I2C_STATUS_ERROR;
return;
diff --git a/hw/i2c/core.c b/hw/i2c/core.c
index 1aac457a2a..c9d01df427 100644
--- a/hw/i2c/core.c
+++ b/hw/i2c/core.c
@@ -91,7 +91,7 @@ int i2c_bus_busy(I2CBus *bus)
* 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 recv)
{
BusChild *kid;
I2CSlaveClass *sc;
@@ -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_send_recv(I2CBus *bus, uint8_t *data, bool 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 (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_send_recv(bus, &data, false);
}
uint8_t i2c_recv(I2CBus *bus)
{
uint8_t data = 0xff;
- i2c_send_recv(bus, &data, false);
+ i2c_send_recv(bus, &data, true);
return data;
}
diff --git a/hw/i2c/ppc4xx_i2c.c b/hw/i2c/ppc4xx_i2c.c
index c0a8e04567..d3899203a4 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_send_recv(i2c->bus, &i2c->mdata[i], recv)) {
i2c->sts |= IIC_STS_ERR;
i2c->extsts |= IIC_EXTSTS_XFRA;
break;
diff --git a/include/hw/i2c/i2c.h b/include/hw/i2c/i2c.h
index 4117211565..a09ab9230b 100644
--- a/include/hw/i2c/i2c.h
+++ b/include/hw/i2c/i2c.h
@@ -72,10 +72,10 @@ struct I2CBus {
I2CBus *i2c_init_bus(DeviceState *parent, const char *name);
void i2c_set_slave_address(I2CSlave *dev, uint8_t address);
int i2c_bus_busy(I2CBus *bus);
-int i2c_start_transfer(I2CBus *bus, uint8_t address, int recv);
+int i2c_start_transfer(I2CBus *bus, uint8_t address, bool 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_recv(I2CBus *bus, uint8_t *data, bool recv);
int i2c_send(I2CBus *bus, uint8_t data);
uint8_t i2c_recv(I2CBus *bus);
--
2.21.3
next reply other threads:[~2020-06-21 14:53 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-06-21 14:43 BALATON Zoltan [this message]
2020-06-22 21:32 ` [PATCH] i2c: Match parameters of i2c_start_transfer and i2c_send_recv Corey Minyard
2020-06-23 2:06 ` Corey Minyard
2020-06-23 5:15 ` Philippe Mathieu-Daudé
2020-06-23 5:23 ` Philippe Mathieu-Daudé
2020-06-23 5:29 ` Philippe Mathieu-Daudé
2020-06-26 10:20 ` Fred Konrad
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=20200621145235.9E241745712@zero.eik.bme.hu \
--to=balaton@eik.bme.hu \
--cc=cminyard@mvista.com \
--cc=david@gibson.dropbear.id.au \
--cc=frederic.konrad@adacore.com \
--cc=qemu-devel@nongnu.org \
/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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).