* [PATCH v4 01/15] hw/input/lm832x: Move lm832x_key_event() declaration to "lm832x.h"
2021-06-16 21:42 [PATCH v4 00/15] hw/i2c: Remove confusing i2c_send_recv() API Philippe Mathieu-Daudé
@ 2021-06-16 21:42 ` Philippe Mathieu-Daudé
2021-06-16 21:42 ` [PATCH v4 02/15] hw/input/lm832x: Define TYPE_LM8323 in public header Philippe Mathieu-Daudé
` (13 subsequent siblings)
14 siblings, 0 replies; 19+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-06-16 21:42 UTC (permalink / raw)
To: qemu-devel
Cc: Corey Minyard, Richard Henderson, Philippe Mathieu-Daudé,
KONRAD Frederic, qemu-arm, qemu-ppc
lm832x_key_event() is specific go LM832x devices, not to the
I2C bus API. Move it out of "i2c.h" to a new header.
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Acked-by: Corey Minyard <cminyard@mvista.com>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
include/hw/i2c/i2c.h | 3 ---
include/hw/input/lm832x.h | 26 ++++++++++++++++++++++++++
hw/arm/nseries.c | 1 +
hw/input/lm832x.c | 1 +
MAINTAINERS | 1 +
5 files changed, 29 insertions(+), 3 deletions(-)
create mode 100644 include/hw/input/lm832x.h
diff --git a/include/hw/i2c/i2c.h b/include/hw/i2c/i2c.h
index ff4129ea704..850815e707c 100644
--- a/include/hw/i2c/i2c.h
+++ b/include/hw/i2c/i2c.h
@@ -142,9 +142,6 @@ I2CSlave *i2c_slave_create_simple(I2CBus *bus, const char *name, uint8_t addr);
*/
bool i2c_slave_realize_and_unref(I2CSlave *dev, I2CBus *bus, Error **errp);
-/* lm832x.c */
-void lm832x_key_event(DeviceState *dev, int key, int state);
-
extern const VMStateDescription vmstate_i2c_slave;
#define VMSTATE_I2C_SLAVE(_field, _state) { \
diff --git a/include/hw/input/lm832x.h b/include/hw/input/lm832x.h
new file mode 100644
index 00000000000..f47e579ff90
--- /dev/null
+++ b/include/hw/input/lm832x.h
@@ -0,0 +1,26 @@
+/*
+ * National Semiconductor LM8322/8323 GPIO keyboard & PWM chips.
+ *
+ * Copyright (C) 2008 Nokia Corporation
+ * Written by Andrzej Zaborowski <andrew@openedhand.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 or
+ * (at your option) version 3 of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef HW_INPUT_LM832X
+#define HW_INPUT_LM832X
+
+void lm832x_key_event(DeviceState *dev, int key, int state);
+
+#endif
diff --git a/hw/arm/nseries.c b/hw/arm/nseries.c
index 0aefa5d0f3e..7b82b8682e8 100644
--- a/hw/arm/nseries.c
+++ b/hw/arm/nseries.c
@@ -34,6 +34,7 @@
#include "hw/boards.h"
#include "hw/i2c/i2c.h"
#include "hw/display/blizzard.h"
+#include "hw/input/lm832x.h"
#include "hw/input/tsc2xxx.h"
#include "hw/misc/cbus.h"
#include "hw/misc/tmp105.h"
diff --git a/hw/input/lm832x.c b/hw/input/lm832x.c
index 4cb1e9de01f..d2b92b457e3 100644
--- a/hw/input/lm832x.c
+++ b/hw/input/lm832x.c
@@ -19,6 +19,7 @@
*/
#include "qemu/osdep.h"
+#include "hw/input/lm832x.h"
#include "hw/i2c/i2c.h"
#include "hw/irq.h"
#include "migration/vmstate.h"
diff --git a/MAINTAINERS b/MAINTAINERS
index 636bf2f5365..c56bc112ccf 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -792,6 +792,7 @@ F: hw/input/tsc2005.c
F: hw/misc/cbus.c
F: hw/rtc/twl92230.c
F: include/hw/display/blizzard.h
+F: include/hw/input/lm832x.h
F: include/hw/input/tsc2xxx.h
F: include/hw/misc/cbus.h
F: tests/acceptance/machine_arm_n8x0.py
--
2.31.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v4 02/15] hw/input/lm832x: Define TYPE_LM8323 in public header
2021-06-16 21:42 [PATCH v4 00/15] hw/i2c: Remove confusing i2c_send_recv() API Philippe Mathieu-Daudé
2021-06-16 21:42 ` [PATCH v4 01/15] hw/input/lm832x: Move lm832x_key_event() declaration to "lm832x.h" Philippe Mathieu-Daudé
@ 2021-06-16 21:42 ` Philippe Mathieu-Daudé
2021-06-16 21:42 ` [PATCH v4 03/15] hw/display/sm501: Simplify sm501_i2c_write() logic Philippe Mathieu-Daudé
` (12 subsequent siblings)
14 siblings, 0 replies; 19+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-06-16 21:42 UTC (permalink / raw)
To: qemu-devel
Cc: Corey Minyard, Richard Henderson, Philippe Mathieu-Daudé,
KONRAD Frederic, qemu-arm, qemu-ppc
Define TYPE_LM8323 in the public "hw/input/lm832x.h"
header and use it in hw/arm/nseries.c.
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Acked-by: Corey Minyard <cminyard@mvista.com>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
include/hw/input/lm832x.h | 2 ++
hw/arm/nseries.c | 2 +-
hw/input/lm832x.c | 1 -
3 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/include/hw/input/lm832x.h b/include/hw/input/lm832x.h
index f47e579ff90..2a58ccf8916 100644
--- a/include/hw/input/lm832x.h
+++ b/include/hw/input/lm832x.h
@@ -21,6 +21,8 @@
#ifndef HW_INPUT_LM832X
#define HW_INPUT_LM832X
+#define TYPE_LM8323 "lm8323"
+
void lm832x_key_event(DeviceState *dev, int key, int state);
#endif
diff --git a/hw/arm/nseries.c b/hw/arm/nseries.c
index 7b82b8682e8..3a51264e3cf 100644
--- a/hw/arm/nseries.c
+++ b/hw/arm/nseries.c
@@ -417,7 +417,7 @@ static void n810_kbd_setup(struct n800_s *s)
/* Attach the LM8322 keyboard to the I2C bus,
* should happen in n8x0_i2c_setup and s->kbd be initialised here. */
s->kbd = DEVICE(i2c_slave_create_simple(omap_i2c_bus(s->mpu->i2c[0]),
- "lm8323", N810_LM8323_ADDR));
+ TYPE_LM8323, N810_LM8323_ADDR));
qdev_connect_gpio_out(s->kbd, 0, kbd_irq);
}
diff --git a/hw/input/lm832x.c b/hw/input/lm832x.c
index d2b92b457e3..19a646d9bb4 100644
--- a/hw/input/lm832x.c
+++ b/hw/input/lm832x.c
@@ -28,7 +28,6 @@
#include "ui/console.h"
#include "qom/object.h"
-#define TYPE_LM8323 "lm8323"
OBJECT_DECLARE_SIMPLE_TYPE(LM823KbdState, LM8323)
struct LM823KbdState {
--
2.31.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v4 03/15] hw/display/sm501: Simplify sm501_i2c_write() logic
2021-06-16 21:42 [PATCH v4 00/15] hw/i2c: Remove confusing i2c_send_recv() API Philippe Mathieu-Daudé
2021-06-16 21:42 ` [PATCH v4 01/15] hw/input/lm832x: Move lm832x_key_event() declaration to "lm832x.h" Philippe Mathieu-Daudé
2021-06-16 21:42 ` [PATCH v4 02/15] hw/input/lm832x: Define TYPE_LM8323 in public header Philippe Mathieu-Daudé
@ 2021-06-16 21:42 ` Philippe Mathieu-Daudé
2021-06-16 23:15 ` BALATON Zoltan
2021-06-16 21:42 ` [PATCH v4 04/15] hw/display/sm501: Replace i2c_send_recv() by i2c_recv() & i2c_send() Philippe Mathieu-Daudé
` (11 subsequent siblings)
14 siblings, 1 reply; 19+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-06-16 21:42 UTC (permalink / raw)
To: qemu-devel
Cc: Corey Minyard, Richard Henderson, Philippe Mathieu-Daudé,
KONRAD Frederic, qemu-arm, qemu-ppc
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Acked-by: Corey Minyard <cminyard@mvista.com>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
hw/display/sm501.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/hw/display/sm501.c b/hw/display/sm501.c
index 8789722ef27..f276276f7f1 100644
--- a/hw/display/sm501.c
+++ b/hw/display/sm501.c
@@ -1036,8 +1036,9 @@ static void sm501_i2c_write(void *opaque, hwaddr addr, uint64_t value,
int res = i2c_start_transfer(s->i2c_bus,
s->i2c_addr >> 1,
s->i2c_addr & 1);
- s->i2c_status |= (res ? SM501_I2C_STATUS_ERROR : 0);
- if (!res) {
+ if (res) {
+ s->i2c_status |= SM501_I2C_STATUS_ERROR;
+ } else {
int i;
for (i = 0; i <= s->i2c_byte_count; i++) {
res = i2c_send_recv(s->i2c_bus, &s->i2c_data[i],
--
2.31.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH v4 03/15] hw/display/sm501: Simplify sm501_i2c_write() logic
2021-06-16 21:42 ` [PATCH v4 03/15] hw/display/sm501: Simplify sm501_i2c_write() logic Philippe Mathieu-Daudé
@ 2021-06-16 23:15 ` BALATON Zoltan
0 siblings, 0 replies; 19+ messages in thread
From: BALATON Zoltan @ 2021-06-16 23:15 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Corey Minyard, Richard Henderson, qemu-devel, KONRAD Frederic,
qemu-arm, qemu-ppc
[-- Attachment #1: Type: text/plain, Size: 1191 bytes --]
On Wed, 16 Jun 2021, Philippe Mathieu-Daudé wrote:
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> Acked-by: Corey Minyard <cminyard@mvista.com>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: BALATON Zoltan <balaton@eik.bme.hu>
> ---
> hw/display/sm501.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/hw/display/sm501.c b/hw/display/sm501.c
> index 8789722ef27..f276276f7f1 100644
> --- a/hw/display/sm501.c
> +++ b/hw/display/sm501.c
> @@ -1036,8 +1036,9 @@ static void sm501_i2c_write(void *opaque, hwaddr addr, uint64_t value,
> int res = i2c_start_transfer(s->i2c_bus,
> s->i2c_addr >> 1,
> s->i2c_addr & 1);
> - s->i2c_status |= (res ? SM501_I2C_STATUS_ERROR : 0);
> - if (!res) {
> + if (res) {
> + s->i2c_status |= SM501_I2C_STATUS_ERROR;
> + } else {
> int i;
> for (i = 0; i <= s->i2c_byte_count; i++) {
> res = i2c_send_recv(s->i2c_bus, &s->i2c_data[i],
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v4 04/15] hw/display/sm501: Replace i2c_send_recv() by i2c_recv() & i2c_send()
2021-06-16 21:42 [PATCH v4 00/15] hw/i2c: Remove confusing i2c_send_recv() API Philippe Mathieu-Daudé
` (2 preceding siblings ...)
2021-06-16 21:42 ` [PATCH v4 03/15] hw/display/sm501: Simplify sm501_i2c_write() logic Philippe Mathieu-Daudé
@ 2021-06-16 21:42 ` Philippe Mathieu-Daudé
2021-06-16 21:42 ` [PATCH v4 05/15] hw/i2c/ppc4xx_i2c: Add reference to datasheet Philippe Mathieu-Daudé
` (10 subsequent siblings)
14 siblings, 0 replies; 19+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-06-16 21:42 UTC (permalink / raw)
To: qemu-devel
Cc: Corey Minyard, Richard Henderson, Philippe Mathieu-Daudé,
KONRAD Frederic, qemu-arm, qemu-ppc
Instead of using the confuse i2c_send_recv(), rewrite to directly
call i2c_recv() & i2c_send(), resulting in code easier to review.
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Acked-by: Corey Minyard <cminyard@mvista.com>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
hw/display/sm501.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/hw/display/sm501.c b/hw/display/sm501.c
index f276276f7f1..569661a0746 100644
--- a/hw/display/sm501.c
+++ b/hw/display/sm501.c
@@ -1033,17 +1033,18 @@ static void sm501_i2c_write(void *opaque, hwaddr addr, uint64_t value,
case SM501_I2C_CONTROL:
if (value & SM501_I2C_CONTROL_ENABLE) {
if (value & SM501_I2C_CONTROL_START) {
+ bool is_recv = s->i2c_addr & 1;
int res = i2c_start_transfer(s->i2c_bus,
s->i2c_addr >> 1,
- s->i2c_addr & 1);
+ is_recv);
if (res) {
s->i2c_status |= SM501_I2C_STATUS_ERROR;
} else {
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));
- if (res) {
+ if (is_recv) {
+ s->i2c_data[i] = i2c_recv(s->i2c_bus);
+ } else if (i2c_send(s->i2c_bus, s->i2c_data[i]) < 0) {
s->i2c_status |= SM501_I2C_STATUS_ERROR;
return;
}
--
2.31.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v4 05/15] hw/i2c/ppc4xx_i2c: Add reference to datasheet
2021-06-16 21:42 [PATCH v4 00/15] hw/i2c: Remove confusing i2c_send_recv() API Philippe Mathieu-Daudé
` (3 preceding siblings ...)
2021-06-16 21:42 ` [PATCH v4 04/15] hw/display/sm501: Replace i2c_send_recv() by i2c_recv() & i2c_send() Philippe Mathieu-Daudé
@ 2021-06-16 21:42 ` Philippe Mathieu-Daudé
2021-06-16 21:42 ` [PATCH v4 06/15] hw/i2c/ppc4xx_i2c: Replace i2c_send_recv() by i2c_recv() & i2c_send() Philippe Mathieu-Daudé
` (9 subsequent siblings)
14 siblings, 0 replies; 19+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-06-16 21:42 UTC (permalink / raw)
To: qemu-devel
Cc: Corey Minyard, Richard Henderson, Philippe Mathieu-Daudé,
KONRAD Frederic, qemu-arm, qemu-ppc
It took me a while to find this model datasheet, since it is
an OCR scan. Add a reference to save other developers time.
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Acked-by: Corey Minyard <cminyard@mvista.com>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
hw/i2c/ppc4xx_i2c.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/hw/i2c/ppc4xx_i2c.c b/hw/i2c/ppc4xx_i2c.c
index c0a8e045670..f4c5bc12d36 100644
--- a/hw/i2c/ppc4xx_i2c.c
+++ b/hw/i2c/ppc4xx_i2c.c
@@ -1,6 +1,8 @@
/*
* PPC4xx I2C controller emulation
*
+ * Documentation: PPC405GP User's Manual, Chapter 22. IIC Bus Interface
+ *
* Copyright (c) 2007 Jocelyn Mayer
* Copyright (c) 2012 François Revol
* Copyright (c) 2016-2018 BALATON Zoltan
--
2.31.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v4 06/15] hw/i2c/ppc4xx_i2c: Replace i2c_send_recv() by i2c_recv() & i2c_send()
2021-06-16 21:42 [PATCH v4 00/15] hw/i2c: Remove confusing i2c_send_recv() API Philippe Mathieu-Daudé
` (4 preceding siblings ...)
2021-06-16 21:42 ` [PATCH v4 05/15] hw/i2c/ppc4xx_i2c: Add reference to datasheet Philippe Mathieu-Daudé
@ 2021-06-16 21:42 ` Philippe Mathieu-Daudé
2021-06-16 21:42 ` [PATCH v4 07/15] hw/misc/auxbus: Fix MOT/classic I2C mode Philippe Mathieu-Daudé
` (8 subsequent siblings)
14 siblings, 0 replies; 19+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-06-16 21:42 UTC (permalink / raw)
To: qemu-devel
Cc: Corey Minyard, Richard Henderson, Philippe Mathieu-Daudé,
KONRAD Frederic, qemu-arm, qemu-ppc
Instead of using the confuse i2c_send_recv(), rewrite to directly
call i2c_recv() & i2c_send(), resulting in code easier to review.
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Acked-by: Corey Minyard <cminyard@mvista.com>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
hw/i2c/ppc4xx_i2c.c | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/hw/i2c/ppc4xx_i2c.c b/hw/i2c/ppc4xx_i2c.c
index f4c5bc12d36..75d50f15158 100644
--- a/hw/i2c/ppc4xx_i2c.c
+++ b/hw/i2c/ppc4xx_i2c.c
@@ -240,11 +240,14 @@ static void ppc4xx_i2c_writeb(void *opaque, hwaddr addr, uint64_t value,
i2c->sts &= ~IIC_STS_ERR;
}
}
- if (!(i2c->sts & IIC_STS_ERR) &&
- i2c_send_recv(i2c->bus, &i2c->mdata[i], !recv)) {
- i2c->sts |= IIC_STS_ERR;
- i2c->extsts |= IIC_EXTSTS_XFRA;
- break;
+ if (!(i2c->sts & IIC_STS_ERR)) {
+ if (recv) {
+ i2c->mdata[i] = i2c_recv(i2c->bus);
+ } else if (i2c_send(i2c->bus, i2c->mdata[i]) < 0) {
+ i2c->sts |= IIC_STS_ERR;
+ i2c->extsts |= IIC_EXTSTS_XFRA;
+ break;
+ }
}
if (value & IIC_CNTL_RPST || !(value & IIC_CNTL_CHT)) {
i2c_end_transfer(i2c->bus);
--
2.31.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v4 07/15] hw/misc/auxbus: Fix MOT/classic I2C mode
2021-06-16 21:42 [PATCH v4 00/15] hw/i2c: Remove confusing i2c_send_recv() API Philippe Mathieu-Daudé
` (5 preceding siblings ...)
2021-06-16 21:42 ` [PATCH v4 06/15] hw/i2c/ppc4xx_i2c: Replace i2c_send_recv() by i2c_recv() & i2c_send() Philippe Mathieu-Daudé
@ 2021-06-16 21:42 ` Philippe Mathieu-Daudé
2021-06-16 21:42 ` [PATCH v4 08/15] hw/misc/auxbus: Explode READ_I2C / WRITE_I2C_MOT cases Philippe Mathieu-Daudé
` (7 subsequent siblings)
14 siblings, 0 replies; 19+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-06-16 21:42 UTC (permalink / raw)
To: qemu-devel
Cc: Corey Minyard, Richard Henderson, Philippe Mathieu-Daudé,
KONRAD Frederic, qemu-arm, qemu-ppc
Since its introduction in commit 6fc7f77fd2 i2c_start_transfer()
uses incorrectly the direction of the transfer (the last argument
is called 'is_recv'). Fix by inverting the argument, we now have
is_recv = !is_write.
Fixes: 6fc7f77fd2 ("introduce aux-bus")
Reported-by: BALATON Zoltan <balaton@eik.bme.hu>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Acked-by: Corey Minyard <cminyard@mvista.com>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
hw/misc/auxbus.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/hw/misc/auxbus.c b/hw/misc/auxbus.c
index 6c099ae2a2d..148b070ce4a 100644
--- a/hw/misc/auxbus.c
+++ b/hw/misc/auxbus.c
@@ -139,7 +139,7 @@ AUXReply aux_request(AUXBus *bus, AUXCommand cmd, uint32_t address,
i2c_end_transfer(i2c_bus);
}
- if (i2c_start_transfer(i2c_bus, address, is_write)) {
+ if (i2c_start_transfer(i2c_bus, address, !is_write)) {
ret = AUX_I2C_NACK;
break;
}
@@ -170,7 +170,7 @@ AUXReply aux_request(AUXBus *bus, AUXCommand cmd, uint32_t address,
/*
* No transactions started..
*/
- if (i2c_start_transfer(i2c_bus, address, is_write)) {
+ if (i2c_start_transfer(i2c_bus, address, !is_write)) {
break;
}
} else if ((address != bus->last_i2c_address) ||
@@ -179,7 +179,7 @@ AUXReply aux_request(AUXBus *bus, AUXCommand cmd, uint32_t address,
* Transaction started but we need to restart..
*/
i2c_end_transfer(i2c_bus);
- if (i2c_start_transfer(i2c_bus, address, is_write)) {
+ if (i2c_start_transfer(i2c_bus, address, !is_write)) {
break;
}
}
--
2.31.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v4 08/15] hw/misc/auxbus: Explode READ_I2C / WRITE_I2C_MOT cases
2021-06-16 21:42 [PATCH v4 00/15] hw/i2c: Remove confusing i2c_send_recv() API Philippe Mathieu-Daudé
` (6 preceding siblings ...)
2021-06-16 21:42 ` [PATCH v4 07/15] hw/misc/auxbus: Fix MOT/classic I2C mode Philippe Mathieu-Daudé
@ 2021-06-16 21:42 ` Philippe Mathieu-Daudé
2021-06-16 21:42 ` [PATCH v4 09/15] hw/misc/auxbus: Replace 'is_write' boolean by its value Philippe Mathieu-Daudé
` (6 subsequent siblings)
14 siblings, 0 replies; 19+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-06-16 21:42 UTC (permalink / raw)
To: qemu-devel
Cc: Corey Minyard, Richard Henderson, Philippe Mathieu-Daudé,
KONRAD Frederic, qemu-arm, qemu-ppc
To allow further simplifications in the following commits,
start copying WRITE_I2C code to the READ_I2C, and READ_I2C_MOT
to WRITE_I2C_MOT. No logical change.
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Acked-by: Corey Minyard <cminyard@mvista.com>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
hw/misc/auxbus.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 53 insertions(+)
diff --git a/hw/misc/auxbus.c b/hw/misc/auxbus.c
index 148b070ce4a..9cc9cf3be32 100644
--- a/hw/misc/auxbus.c
+++ b/hw/misc/auxbus.c
@@ -133,6 +133,26 @@ AUXReply aux_request(AUXBus *bus, AUXCommand cmd, uint32_t address,
* Classic I2C transactions..
*/
case READ_I2C:
+ is_write = cmd == READ_I2C ? false : true;
+ if (i2c_bus_busy(i2c_bus)) {
+ i2c_end_transfer(i2c_bus);
+ }
+
+ if (i2c_start_transfer(i2c_bus, address, !is_write)) {
+ ret = AUX_I2C_NACK;
+ break;
+ }
+
+ ret = AUX_I2C_ACK;
+ while (len > 0) {
+ if (i2c_send_recv(i2c_bus, data++, is_write) < 0) {
+ ret = AUX_I2C_NACK;
+ break;
+ }
+ len--;
+ }
+ i2c_end_transfer(i2c_bus);
+ break;
case WRITE_I2C:
is_write = cmd == READ_I2C ? false : true;
if (i2c_bus_busy(i2c_bus)) {
@@ -163,6 +183,39 @@ AUXReply aux_request(AUXBus *bus, AUXCommand cmd, uint32_t address,
* - We changed the address.
*/
case WRITE_I2C_MOT:
+ is_write = cmd == READ_I2C_MOT ? false : true;
+ ret = AUX_I2C_NACK;
+ if (!i2c_bus_busy(i2c_bus)) {
+ /*
+ * No transactions started..
+ */
+ if (i2c_start_transfer(i2c_bus, address, !is_write)) {
+ break;
+ }
+ } else if ((address != bus->last_i2c_address) ||
+ (bus->last_transaction != cmd)) {
+ /*
+ * Transaction started but we need to restart..
+ */
+ i2c_end_transfer(i2c_bus);
+ if (i2c_start_transfer(i2c_bus, address, !is_write)) {
+ break;
+ }
+ }
+
+ bus->last_transaction = cmd;
+ bus->last_i2c_address = address;
+ while (len > 0) {
+ if (i2c_send_recv(i2c_bus, data++, is_write) < 0) {
+ i2c_end_transfer(i2c_bus);
+ break;
+ }
+ len--;
+ }
+ if (len == 0) {
+ ret = AUX_I2C_ACK;
+ }
+ break;
case READ_I2C_MOT:
is_write = cmd == READ_I2C_MOT ? false : true;
ret = AUX_I2C_NACK;
--
2.31.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v4 09/15] hw/misc/auxbus: Replace 'is_write' boolean by its value
2021-06-16 21:42 [PATCH v4 00/15] hw/i2c: Remove confusing i2c_send_recv() API Philippe Mathieu-Daudé
` (7 preceding siblings ...)
2021-06-16 21:42 ` [PATCH v4 08/15] hw/misc/auxbus: Explode READ_I2C / WRITE_I2C_MOT cases Philippe Mathieu-Daudé
@ 2021-06-16 21:42 ` Philippe Mathieu-Daudé
2021-06-16 21:42 ` [PATCH v4 10/15] hw/misc/auxbus: Replace i2c_send_recv() by i2c_recv() & i2c_send() Philippe Mathieu-Daudé
` (5 subsequent siblings)
14 siblings, 0 replies; 19+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-06-16 21:42 UTC (permalink / raw)
To: qemu-devel
Cc: Corey Minyard, Richard Henderson, Philippe Mathieu-Daudé,
KONRAD Frederic, qemu-arm, qemu-ppc
Remove the 'is_write' boolean by directly using its value in place.
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Acked-by: Corey Minyard <cminyard@mvista.com>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
hw/misc/auxbus.c | 28 +++++++++++-----------------
1 file changed, 11 insertions(+), 17 deletions(-)
diff --git a/hw/misc/auxbus.c b/hw/misc/auxbus.c
index 9cc9cf3be32..d96219aef88 100644
--- a/hw/misc/auxbus.c
+++ b/hw/misc/auxbus.c
@@ -106,7 +106,6 @@ AUXReply aux_request(AUXBus *bus, AUXCommand cmd, uint32_t address,
AUXReply ret = AUX_NACK;
I2CBus *i2c_bus = aux_get_i2c_bus(bus);
size_t i;
- bool is_write = false;
DPRINTF("request at address 0x%" PRIX32 ", command %u, len %u\n", address,
cmd, len);
@@ -117,11 +116,10 @@ AUXReply aux_request(AUXBus *bus, AUXCommand cmd, uint32_t address,
*/
case WRITE_AUX:
case READ_AUX:
- is_write = cmd == READ_AUX ? false : true;
for (i = 0; i < len; i++) {
if (!address_space_rw(&bus->aux_addr_space, address++,
MEMTXATTRS_UNSPECIFIED, data++, 1,
- is_write)) {
+ cmd == WRITE_AUX)) {
ret = AUX_I2C_ACK;
} else {
ret = AUX_NACK;
@@ -133,19 +131,18 @@ AUXReply aux_request(AUXBus *bus, AUXCommand cmd, uint32_t address,
* Classic I2C transactions..
*/
case READ_I2C:
- is_write = cmd == READ_I2C ? false : true;
if (i2c_bus_busy(i2c_bus)) {
i2c_end_transfer(i2c_bus);
}
- if (i2c_start_transfer(i2c_bus, address, !is_write)) {
+ if (i2c_start_transfer(i2c_bus, address, true)) {
ret = AUX_I2C_NACK;
break;
}
ret = AUX_I2C_ACK;
while (len > 0) {
- if (i2c_send_recv(i2c_bus, data++, is_write) < 0) {
+ if (i2c_send_recv(i2c_bus, data++, false) < 0) {
ret = AUX_I2C_NACK;
break;
}
@@ -154,19 +151,18 @@ AUXReply aux_request(AUXBus *bus, AUXCommand cmd, uint32_t address,
i2c_end_transfer(i2c_bus);
break;
case WRITE_I2C:
- is_write = cmd == READ_I2C ? false : true;
if (i2c_bus_busy(i2c_bus)) {
i2c_end_transfer(i2c_bus);
}
- if (i2c_start_transfer(i2c_bus, address, !is_write)) {
+ if (i2c_start_transfer(i2c_bus, address, false)) {
ret = AUX_I2C_NACK;
break;
}
ret = AUX_I2C_ACK;
while (len > 0) {
- if (i2c_send_recv(i2c_bus, data++, is_write) < 0) {
+ if (i2c_send_recv(i2c_bus, data++, true) < 0) {
ret = AUX_I2C_NACK;
break;
}
@@ -183,13 +179,12 @@ AUXReply aux_request(AUXBus *bus, AUXCommand cmd, uint32_t address,
* - We changed the address.
*/
case WRITE_I2C_MOT:
- is_write = cmd == READ_I2C_MOT ? false : true;
ret = AUX_I2C_NACK;
if (!i2c_bus_busy(i2c_bus)) {
/*
* No transactions started..
*/
- if (i2c_start_transfer(i2c_bus, address, !is_write)) {
+ if (i2c_start_transfer(i2c_bus, address, false)) {
break;
}
} else if ((address != bus->last_i2c_address) ||
@@ -198,7 +193,7 @@ AUXReply aux_request(AUXBus *bus, AUXCommand cmd, uint32_t address,
* Transaction started but we need to restart..
*/
i2c_end_transfer(i2c_bus);
- if (i2c_start_transfer(i2c_bus, address, !is_write)) {
+ if (i2c_start_transfer(i2c_bus, address, false)) {
break;
}
}
@@ -206,7 +201,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_send_recv(i2c_bus, data++, true) < 0) {
i2c_end_transfer(i2c_bus);
break;
}
@@ -217,13 +212,12 @@ AUXReply aux_request(AUXBus *bus, AUXCommand cmd, uint32_t address,
}
break;
case READ_I2C_MOT:
- is_write = cmd == READ_I2C_MOT ? false : true;
ret = AUX_I2C_NACK;
if (!i2c_bus_busy(i2c_bus)) {
/*
* No transactions started..
*/
- if (i2c_start_transfer(i2c_bus, address, !is_write)) {
+ if (i2c_start_transfer(i2c_bus, address, true)) {
break;
}
} else if ((address != bus->last_i2c_address) ||
@@ -232,7 +226,7 @@ AUXReply aux_request(AUXBus *bus, AUXCommand cmd, uint32_t address,
* Transaction started but we need to restart..
*/
i2c_end_transfer(i2c_bus);
- if (i2c_start_transfer(i2c_bus, address, !is_write)) {
+ if (i2c_start_transfer(i2c_bus, address, true)) {
break;
}
}
@@ -240,7 +234,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_send_recv(i2c_bus, data++, false) < 0) {
i2c_end_transfer(i2c_bus);
break;
}
--
2.31.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v4 10/15] hw/misc/auxbus: Replace i2c_send_recv() by i2c_recv() & i2c_send()
2021-06-16 21:42 [PATCH v4 00/15] hw/i2c: Remove confusing i2c_send_recv() API Philippe Mathieu-Daudé
` (8 preceding siblings ...)
2021-06-16 21:42 ` [PATCH v4 09/15] hw/misc/auxbus: Replace 'is_write' boolean by its value Philippe Mathieu-Daudé
@ 2021-06-16 21:42 ` Philippe Mathieu-Daudé
2021-06-16 21:42 ` [PATCH v4 11/15] hw/i2c: Remove confusing i2c_send_recv() Philippe Mathieu-Daudé
` (4 subsequent siblings)
14 siblings, 0 replies; 19+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-06-16 21:42 UTC (permalink / raw)
To: qemu-devel
Cc: Corey Minyard, Richard Henderson, Philippe Mathieu-Daudé,
KONRAD Frederic, qemu-arm, qemu-ppc
Instead of using the confuse i2c_send_recv(), replace
i2c_send_recv(send = true) by i2c_send() and
i2c_send_recv(send = false) by i2c_recv().
During the replacement we also change a while() statement by for().
The resulting code is easier to review.
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Acked-by: Corey Minyard <cminyard@mvista.com>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
hw/misc/auxbus.c | 35 +++++++++++------------------------
1 file changed, 11 insertions(+), 24 deletions(-)
diff --git a/hw/misc/auxbus.c b/hw/misc/auxbus.c
index d96219aef88..44aa9730bc9 100644
--- a/hw/misc/auxbus.c
+++ b/hw/misc/auxbus.c
@@ -141,12 +141,8 @@ AUXReply aux_request(AUXBus *bus, AUXCommand cmd, uint32_t address,
}
ret = AUX_I2C_ACK;
- while (len > 0) {
- if (i2c_send_recv(i2c_bus, data++, false) < 0) {
- ret = AUX_I2C_NACK;
- break;
- }
- len--;
+ for (i = 0; i < len; i++) {
+ data[i] = i2c_recv(i2c_bus);
}
i2c_end_transfer(i2c_bus);
break;
@@ -161,12 +157,11 @@ AUXReply aux_request(AUXBus *bus, AUXCommand cmd, uint32_t address,
}
ret = AUX_I2C_ACK;
- while (len > 0) {
- if (i2c_send_recv(i2c_bus, data++, true) < 0) {
+ for (i = 0; i < len; i++) {
+ if (i2c_send(i2c_bus, data[i]) < 0) {
ret = AUX_I2C_NACK;
break;
}
- len--;
}
i2c_end_transfer(i2c_bus);
break;
@@ -200,15 +195,13 @@ 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++, true) < 0) {
+ ret = AUX_I2C_ACK;
+ for (i = 0; i < len; i++) {
+ if (i2c_send(i2c_bus, data[i]) < 0) {
i2c_end_transfer(i2c_bus);
+ ret = AUX_I2C_NACK;
break;
}
- len--;
- }
- if (len == 0) {
- ret = AUX_I2C_ACK;
}
break;
case READ_I2C_MOT:
@@ -233,16 +226,10 @@ 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++, false) < 0) {
- i2c_end_transfer(i2c_bus);
- break;
- }
- len--;
- }
- if (len == 0) {
- ret = AUX_I2C_ACK;
+ for (i = 0; i < len; i++) {
+ data[i] = i2c_recv(i2c_bus);
}
+ ret = AUX_I2C_ACK;
break;
default:
qemu_log_mask(LOG_UNIMP, "AUX cmd=%u not implemented\n", cmd);
--
2.31.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v4 11/15] hw/i2c: Remove confusing i2c_send_recv()
2021-06-16 21:42 [PATCH v4 00/15] hw/i2c: Remove confusing i2c_send_recv() API Philippe Mathieu-Daudé
` (9 preceding siblings ...)
2021-06-16 21:42 ` [PATCH v4 10/15] hw/misc/auxbus: Replace i2c_send_recv() by i2c_recv() & i2c_send() Philippe Mathieu-Daudé
@ 2021-06-16 21:42 ` Philippe Mathieu-Daudé
2021-06-16 21:42 ` [PATCH v4 12/15] hw/i2c: Rename i2c_set_slave_address() -> i2c_slave_set_address() Philippe Mathieu-Daudé
` (3 subsequent siblings)
14 siblings, 0 replies; 19+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-06-16 21:42 UTC (permalink / raw)
To: qemu-devel
Cc: Corey Minyard, Richard Henderson, Philippe Mathieu-Daudé,
KONRAD Frederic, qemu-arm, qemu-ppc
We replaced all the i2c_send_recv() calls by the clearer i2c_recv()
and i2c_send(), so we can remove this confusing API.
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Acked-by: Corey Minyard <cminyard@mvista.com>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
include/hw/i2c/i2c.h | 1 -
hw/i2c/core.c | 50 +++++++++++++++++++-------------------------
2 files changed, 21 insertions(+), 30 deletions(-)
diff --git a/include/hw/i2c/i2c.h b/include/hw/i2c/i2c.h
index 850815e707c..99635b837a5 100644
--- a/include/hw/i2c/i2c.h
+++ b/include/hw/i2c/i2c.h
@@ -84,7 +84,6 @@ 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);
uint8_t i2c_recv(I2CBus *bus);
bool i2c_scan_bus(I2CBus *bus, uint8_t address, bool broadcast,
diff --git a/hw/i2c/core.c b/hw/i2c/core.c
index 3a7bae311df..27a66df7f34 100644
--- a/hw/i2c/core.c
+++ b/hw/i2c/core.c
@@ -188,50 +188,42 @@ void i2c_end_transfer(I2CBus *bus)
bus->broadcast = false;
}
-int i2c_send_recv(I2CBus *bus, uint8_t *data, bool send)
+int i2c_send(I2CBus *bus, uint8_t data)
{
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;
- }
+ 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 {
- ret = 0xff;
- if (!QLIST_EMPTY(&bus->current_devs) && !bus->broadcast) {
- sc = I2C_SLAVE_GET_CLASS(QLIST_FIRST(&bus->current_devs)->elt);
- if (sc->recv) {
- s = QLIST_FIRST(&bus->current_devs)->elt;
- ret = sc->recv(s);
- trace_i2c_recv(s->address, ret);
- }
- }
- *data = ret;
- return 0;
}
-}
-int i2c_send(I2CBus *bus, uint8_t data)
-{
- return i2c_send_recv(bus, &data, true);
+ return ret ? -1 : 0;
}
uint8_t i2c_recv(I2CBus *bus)
{
uint8_t data = 0xff;
+ I2CSlaveClass *sc;
+ I2CSlave *s;
+
+ if (!QLIST_EMPTY(&bus->current_devs) && !bus->broadcast) {
+ sc = I2C_SLAVE_GET_CLASS(QLIST_FIRST(&bus->current_devs)->elt);
+ if (sc->recv) {
+ s = QLIST_FIRST(&bus->current_devs)->elt;
+ data = sc->recv(s);
+ trace_i2c_recv(s->address, data);
+ }
+ }
- i2c_send_recv(bus, &data, false);
return data;
}
--
2.31.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v4 12/15] hw/i2c: Rename i2c_set_slave_address() -> i2c_slave_set_address()
2021-06-16 21:42 [PATCH v4 00/15] hw/i2c: Remove confusing i2c_send_recv() API Philippe Mathieu-Daudé
` (10 preceding siblings ...)
2021-06-16 21:42 ` [PATCH v4 11/15] hw/i2c: Remove confusing i2c_send_recv() Philippe Mathieu-Daudé
@ 2021-06-16 21:42 ` Philippe Mathieu-Daudé
2021-06-16 21:42 ` [PATCH v4 13/15] hw/i2c: Make i2c_start_transfer() direction argument a boolean Philippe Mathieu-Daudé
` (2 subsequent siblings)
14 siblings, 0 replies; 19+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-06-16 21:42 UTC (permalink / raw)
To: qemu-devel
Cc: Corey Minyard, Richard Henderson, Philippe Mathieu-Daudé,
KONRAD Frederic, qemu-arm, qemu-ppc
Other functions from I2C slave API are named "i2c_slave_XXX()".
Follow that pattern with set_address(). Add docstring along.
No logical change.
Patch created mechanically using:
$ sed -i s/i2c_set_slave_address/i2c_slave_set_address/ \
$(git grep -l i2c_set_slave_address)
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Acked-by: Corey Minyard <cminyard@mvista.com>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
include/hw/i2c/i2c.h | 8 +++++++-
hw/arm/pxa2xx.c | 2 +-
hw/arm/spitz.c | 4 ++--
hw/display/ati.c | 2 +-
hw/display/sm501.c | 2 +-
hw/display/xlnx_dp.c | 2 +-
hw/i2c/core.c | 2 +-
hw/i2c/imx_i2c.c | 2 +-
8 files changed, 15 insertions(+), 9 deletions(-)
diff --git a/include/hw/i2c/i2c.h b/include/hw/i2c/i2c.h
index 99635b837a5..2adf521b271 100644
--- a/include/hw/i2c/i2c.h
+++ b/include/hw/i2c/i2c.h
@@ -79,7 +79,6 @@ 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);
void i2c_end_transfer(I2CBus *bus);
@@ -141,6 +140,13 @@ I2CSlave *i2c_slave_create_simple(I2CBus *bus, const char *name, uint8_t addr);
*/
bool i2c_slave_realize_and_unref(I2CSlave *dev, I2CBus *bus, Error **errp);
+/**
+ * Set the I2C bus address of a slave device
+ * @dev: I2C slave device
+ * @address: I2C address of the slave when put on a bus
+ */
+void i2c_slave_set_address(I2CSlave *dev, uint8_t address);
+
extern const VMStateDescription vmstate_i2c_slave;
#define VMSTATE_I2C_SLAVE(_field, _state) { \
diff --git a/hw/arm/pxa2xx.c b/hw/arm/pxa2xx.c
index fdc4955e95b..15a247efae2 100644
--- a/hw/arm/pxa2xx.c
+++ b/hw/arm/pxa2xx.c
@@ -1437,7 +1437,7 @@ static void pxa2xx_i2c_write(void *opaque, hwaddr addr,
break;
case ISAR:
- i2c_set_slave_address(I2C_SLAVE(s->slave), value & 0x7f);
+ i2c_slave_set_address(I2C_SLAVE(s->slave), value & 0x7f);
break;
case IDBR:
diff --git a/hw/arm/spitz.c b/hw/arm/spitz.c
index b45a929cbd9..c0f0f8193f4 100644
--- a/hw/arm/spitz.c
+++ b/hw/arm/spitz.c
@@ -769,9 +769,9 @@ static void spitz_wm8750_addr(void *opaque, int line, int level)
{
I2CSlave *wm = (I2CSlave *) opaque;
if (level)
- i2c_set_slave_address(wm, SPITZ_WM_ADDRH);
+ i2c_slave_set_address(wm, SPITZ_WM_ADDRH);
else
- i2c_set_slave_address(wm, SPITZ_WM_ADDRL);
+ i2c_slave_set_address(wm, SPITZ_WM_ADDRL);
}
static void spitz_i2c_setup(PXA2xxState *cpu)
diff --git a/hw/display/ati.c b/hw/display/ati.c
index 4c3ad8f47b0..31f22754dce 100644
--- a/hw/display/ati.c
+++ b/hw/display/ati.c
@@ -968,7 +968,7 @@ static void ati_vga_realize(PCIDevice *dev, Error **errp)
I2CBus *i2cbus = i2c_init_bus(DEVICE(s), "ati-vga.ddc");
bitbang_i2c_init(&s->bbi2c, i2cbus);
I2CSlave *i2cddc = I2C_SLAVE(qdev_new(TYPE_I2CDDC));
- i2c_set_slave_address(i2cddc, 0x50);
+ i2c_slave_set_address(i2cddc, 0x50);
qdev_realize_and_unref(DEVICE(i2cddc), BUS(i2cbus), &error_abort);
/* mmio register space */
diff --git a/hw/display/sm501.c b/hw/display/sm501.c
index 569661a0746..663c37e7f28 100644
--- a/hw/display/sm501.c
+++ b/hw/display/sm501.c
@@ -1828,7 +1828,7 @@ static void sm501_init(SM501State *s, DeviceState *dev,
s->i2c_bus = i2c_init_bus(dev, "sm501.i2c");
/* ddc */
I2CDDCState *ddc = I2CDDC(qdev_new(TYPE_I2CDDC));
- i2c_set_slave_address(I2C_SLAVE(ddc), 0x50);
+ i2c_slave_set_address(I2C_SLAVE(ddc), 0x50);
qdev_realize_and_unref(DEVICE(ddc), BUS(s->i2c_bus), &error_abort);
/* mmio */
diff --git a/hw/display/xlnx_dp.c b/hw/display/xlnx_dp.c
index 4fd6aeb18b5..2bb7a5441ad 100644
--- a/hw/display/xlnx_dp.c
+++ b/hw/display/xlnx_dp.c
@@ -1253,7 +1253,7 @@ static void xlnx_dp_init(Object *obj)
object_property_add_child(OBJECT(s), "dpcd", OBJECT(s->dpcd));
s->edid = I2CDDC(qdev_new("i2c-ddc"));
- i2c_set_slave_address(I2C_SLAVE(s->edid), 0x50);
+ i2c_slave_set_address(I2C_SLAVE(s->edid), 0x50);
object_property_add_child(OBJECT(s), "edid", OBJECT(s->edid));
fifo8_create(&s->rx_fifo, 16);
diff --git a/hw/i2c/core.c b/hw/i2c/core.c
index 27a66df7f34..6af24c9e797 100644
--- a/hw/i2c/core.c
+++ b/hw/i2c/core.c
@@ -66,7 +66,7 @@ I2CBus *i2c_init_bus(DeviceState *parent, const char *name)
return bus;
}
-void i2c_set_slave_address(I2CSlave *dev, uint8_t address)
+void i2c_slave_set_address(I2CSlave *dev, uint8_t address)
{
dev->address = address;
}
diff --git a/hw/i2c/imx_i2c.c b/hw/i2c/imx_i2c.c
index 2e02e1c4faa..9792583fea7 100644
--- a/hw/i2c/imx_i2c.c
+++ b/hw/i2c/imx_i2c.c
@@ -171,7 +171,7 @@ static void imx_i2c_write(void *opaque, hwaddr offset,
switch (offset) {
case IADR_ADDR:
s->iadr = value & IADR_MASK;
- /* i2c_set_slave_address(s->bus, (uint8_t)s->iadr); */
+ /* i2c_slave_set_address(s->bus, (uint8_t)s->iadr); */
break;
case IFDR_ADDR:
s->ifdr = value & IFDR_MASK;
--
2.31.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v4 13/15] hw/i2c: Make i2c_start_transfer() direction argument a boolean
2021-06-16 21:42 [PATCH v4 00/15] hw/i2c: Remove confusing i2c_send_recv() API Philippe Mathieu-Daudé
` (11 preceding siblings ...)
2021-06-16 21:42 ` [PATCH v4 12/15] hw/i2c: Rename i2c_set_slave_address() -> i2c_slave_set_address() Philippe Mathieu-Daudé
@ 2021-06-16 21:42 ` Philippe Mathieu-Daudé
2021-06-16 21:42 ` [PATCH v4 14/15] hw/i2c: Extract i2c_do_start_transfer() from i2c_start_transfer() Philippe Mathieu-Daudé
2021-06-16 21:42 ` [PATCH v4 15/15] hw/i2c: Introduce i2c_start_recv() and i2c_start_send() Philippe Mathieu-Daudé
14 siblings, 0 replies; 19+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-06-16 21:42 UTC (permalink / raw)
To: qemu-devel
Cc: Corey Minyard, Richard Henderson, Philippe Mathieu-Daudé,
KONRAD Frederic, qemu-arm, qemu-ppc
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]
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Acked-by: Corey Minyard <cminyard@mvista.com>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
include/hw/i2c/i2c.h | 12 +++++++++++-
hw/i2c/core.c | 4 ++--
2 files changed, 13 insertions(+), 3 deletions(-)
diff --git a/include/hw/i2c/i2c.h b/include/hw/i2c/i2c.h
index 2adf521b271..21f2dba1bf7 100644
--- a/include/hw/i2c/i2c.h
+++ b/include/hw/i2c/i2c.h
@@ -80,7 +80,17 @@ 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);
if (rv && !bus->broadcast) {
if (bus_scanned) {
/* First call, terminate the transfer. */
--
2.31.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v4 14/15] hw/i2c: Extract i2c_do_start_transfer() from i2c_start_transfer()
2021-06-16 21:42 [PATCH v4 00/15] hw/i2c: Remove confusing i2c_send_recv() API Philippe Mathieu-Daudé
` (12 preceding siblings ...)
2021-06-16 21:42 ` [PATCH v4 13/15] hw/i2c: Make i2c_start_transfer() direction argument a boolean Philippe Mathieu-Daudé
@ 2021-06-16 21:42 ` Philippe Mathieu-Daudé
2021-06-17 0:21 ` Richard Henderson
2021-06-16 21:42 ` [PATCH v4 15/15] hw/i2c: Introduce i2c_start_recv() and i2c_start_send() Philippe Mathieu-Daudé
14 siblings, 1 reply; 19+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-06-16 21:42 UTC (permalink / raw)
To: qemu-devel
Cc: Corey Minyard, Philippe Mathieu-Daudé,
KONRAD Frederic, qemu-arm, qemu-ppc
To allow further simplications, extract i2c_do_start_transfer()
from i2c_start_transfer(). This is mostly the same function,
but the former is static and takes an enum argument.
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
hw/i2c/core.c | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/hw/i2c/core.c b/hw/i2c/core.c
index 6639ca8c2e0..69df4c0df6b 100644
--- a/hw/i2c/core.c
+++ b/hw/i2c/core.c
@@ -115,12 +115,15 @@ 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, bool is_recv)
+static int i2c_do_start_transfer(I2CBus *bus, uint8_t address,
+ enum i2c_event event)
{
I2CSlaveClass *sc;
I2CNode *node;
bool bus_scanned = false;
+ assert(event == I2C_START_RECV || event == I2C_START_SEND);
+
if (address == I2C_BROADCAST) {
/*
* This is a broadcast, the current_devs will be all the devices of the
@@ -157,7 +160,7 @@ int i2c_start_transfer(I2CBus *bus, uint8_t address, bool is_recv)
if (sc->event) {
trace_i2c_event("start", s->address);
- rv = sc->event(s, is_recv ? I2C_START_RECV : I2C_START_SEND);
+ rv = sc->event(s, event);
if (rv && !bus->broadcast) {
if (bus_scanned) {
/* First call, terminate the transfer. */
@@ -170,6 +173,13 @@ int i2c_start_transfer(I2CBus *bus, uint8_t address, bool is_recv)
return 0;
}
+int i2c_start_transfer(I2CBus *bus, uint8_t address, bool is_recv)
+{
+ return i2c_do_start_transfer(bus, address, is_recv
+ ? I2C_START_RECV
+ : I2C_START_SEND);
+}
+
void i2c_end_transfer(I2CBus *bus)
{
I2CSlaveClass *sc;
--
2.31.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH v4 14/15] hw/i2c: Extract i2c_do_start_transfer() from i2c_start_transfer()
2021-06-16 21:42 ` [PATCH v4 14/15] hw/i2c: Extract i2c_do_start_transfer() from i2c_start_transfer() Philippe Mathieu-Daudé
@ 2021-06-17 0:21 ` Richard Henderson
0 siblings, 0 replies; 19+ messages in thread
From: Richard Henderson @ 2021-06-17 0:21 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Corey Minyard, KONRAD Frederic, qemu-arm, qemu-ppc
On 6/16/21 2:42 PM, Philippe Mathieu-Daudé wrote:
> +static int i2c_do_start_transfer(I2CBus *bus, uint8_t address,
> + enum i2c_event event)
> {
> I2CSlaveClass *sc;
> I2CNode *node;
> bool bus_scanned = false;
>
> + assert(event == I2C_START_RECV || event == I2C_START_SEND);
I don't think you need the assert, given that its scope is limited, and there will be
exactly 3 users that immediately follow. Document it if you like.
Otherwise,
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
r~
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v4 15/15] hw/i2c: Introduce i2c_start_recv() and i2c_start_send()
2021-06-16 21:42 [PATCH v4 00/15] hw/i2c: Remove confusing i2c_send_recv() API Philippe Mathieu-Daudé
` (13 preceding siblings ...)
2021-06-16 21:42 ` [PATCH v4 14/15] hw/i2c: Extract i2c_do_start_transfer() from i2c_start_transfer() Philippe Mathieu-Daudé
@ 2021-06-16 21:42 ` Philippe Mathieu-Daudé
2021-06-17 0:23 ` Richard Henderson
14 siblings, 1 reply; 19+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-06-16 21:42 UTC (permalink / raw)
To: qemu-devel
Cc: Corey Minyard, Richard Henderson, Philippe Mathieu-Daudé,
KONRAD Frederic, qemu-arm, qemu-ppc
To ease reviewing code using the I2C bus API, introduce the
i2c_start_recv() and i2c_start_send() helpers which don't
take the confusing 'is_recv' boolean argument.
Use these new helpers in the SMBus / AUX bus models.
Suggested-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
include/hw/i2c/i2c.h | 24 ++++++++++++++++++++++++
hw/i2c/core.c | 10 ++++++++++
hw/i2c/pm_smbus.c | 4 ++--
hw/i2c/smbus_master.c | 22 +++++++++++-----------
hw/misc/auxbus.c | 12 ++++++------
5 files changed, 53 insertions(+), 19 deletions(-)
diff --git a/include/hw/i2c/i2c.h b/include/hw/i2c/i2c.h
index 21f2dba1bf7..5ca3b708c0b 100644
--- a/include/hw/i2c/i2c.h
+++ b/include/hw/i2c/i2c.h
@@ -88,9 +88,33 @@ int i2c_bus_busy(I2CBus *bus);
* @address: address of the slave
* @is_recv: indicates the transfer direction
*
+ * When @is_recv is a known boolean constant, use the
+ * i2c_start_recv() or i2c_start_send() helper instead.
+ *
* Returns: 0 on success, -1 on error
*/
int i2c_start_transfer(I2CBus *bus, uint8_t address, bool is_recv);
+
+/**
+ * i2c_start_recv: start a 'receive' transfer on an I2C bus.
+ *
+ * @bus: #I2CBus to be used
+ * @address: address of the slave
+ *
+ * Returns: 0 on success, -1 on error
+ */
+int i2c_start_recv(I2CBus *bus, uint8_t address);
+
+/**
+ * i2c_start_send: start a 'send' transfer on an I2C bus.
+ *
+ * @bus: #I2CBus to be used
+ * @address: address of the slave
+ *
+ * Returns: 0 on success, -1 on error
+ */
+int i2c_start_send(I2CBus *bus, uint8_t address);
+
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 69df4c0df6b..7156f55ded7 100644
--- a/hw/i2c/core.c
+++ b/hw/i2c/core.c
@@ -180,6 +180,16 @@ int i2c_start_transfer(I2CBus *bus, uint8_t address, bool is_recv)
: I2C_START_SEND);
}
+int i2c_start_recv(I2CBus *bus, uint8_t address)
+{
+ return i2c_do_start_transfer(bus, address, I2C_START_RECV);
+}
+
+int i2c_start_send(I2CBus *bus, uint8_t address)
+{
+ return i2c_do_start_transfer(bus, address, I2C_START_SEND);
+}
+
void i2c_end_transfer(I2CBus *bus)
{
I2CSlaveClass *sc;
diff --git a/hw/i2c/pm_smbus.c b/hw/i2c/pm_smbus.c
index 06e1e5321b9..d7eae548cbc 100644
--- a/hw/i2c/pm_smbus.c
+++ b/hw/i2c/pm_smbus.c
@@ -128,14 +128,14 @@ static void smb_transaction(PMSMBus *s)
* So at least Linux may or may not set the read bit here.
* So just ignore the read bit for this command.
*/
- if (i2c_start_transfer(bus, addr, 0)) {
+ if (i2c_start_send(bus, addr)) {
goto error;
}
ret = i2c_send(bus, s->smb_data1);
if (ret) {
goto error;
}
- if (i2c_start_transfer(bus, addr, 1)) {
+ if (i2c_start_recv(bus, addr)) {
goto error;
}
s->in_i2c_block_read = true;
diff --git a/hw/i2c/smbus_master.c b/hw/i2c/smbus_master.c
index dc43b8637d1..6a53c34e70b 100644
--- a/hw/i2c/smbus_master.c
+++ b/hw/i2c/smbus_master.c
@@ -29,7 +29,7 @@ int smbus_receive_byte(I2CBus *bus, uint8_t addr)
{
uint8_t data;
- if (i2c_start_transfer(bus, addr, 1)) {
+ if (i2c_start_recv(bus, addr)) {
return -1;
}
data = i2c_recv(bus);
@@ -40,7 +40,7 @@ int smbus_receive_byte(I2CBus *bus, uint8_t addr)
int smbus_send_byte(I2CBus *bus, uint8_t addr, uint8_t data)
{
- if (i2c_start_transfer(bus, addr, 0)) {
+ if (i2c_start_send(bus, addr)) {
return -1;
}
i2c_send(bus, data);
@@ -51,11 +51,11 @@ int smbus_send_byte(I2CBus *bus, uint8_t addr, uint8_t data)
int smbus_read_byte(I2CBus *bus, uint8_t addr, uint8_t command)
{
uint8_t data;
- if (i2c_start_transfer(bus, addr, 0)) {
+ if (i2c_start_send(bus, addr)) {
return -1;
}
i2c_send(bus, command);
- if (i2c_start_transfer(bus, addr, 1)) {
+ if (i2c_start_recv(bus, addr)) {
i2c_end_transfer(bus);
return -1;
}
@@ -67,7 +67,7 @@ int smbus_read_byte(I2CBus *bus, uint8_t addr, uint8_t command)
int smbus_write_byte(I2CBus *bus, uint8_t addr, uint8_t command, uint8_t data)
{
- if (i2c_start_transfer(bus, addr, 0)) {
+ if (i2c_start_send(bus, addr)) {
return -1;
}
i2c_send(bus, command);
@@ -79,11 +79,11 @@ int smbus_write_byte(I2CBus *bus, uint8_t addr, uint8_t command, uint8_t data)
int smbus_read_word(I2CBus *bus, uint8_t addr, uint8_t command)
{
uint16_t data;
- if (i2c_start_transfer(bus, addr, 0)) {
+ if (i2c_start_send(bus, addr)) {
return -1;
}
i2c_send(bus, command);
- if (i2c_start_transfer(bus, addr, 1)) {
+ if (i2c_start_recv(bus, addr)) {
i2c_end_transfer(bus);
return -1;
}
@@ -96,7 +96,7 @@ int smbus_read_word(I2CBus *bus, uint8_t addr, uint8_t command)
int smbus_write_word(I2CBus *bus, uint8_t addr, uint8_t command, uint16_t data)
{
- if (i2c_start_transfer(bus, addr, 0)) {
+ if (i2c_start_send(bus, addr)) {
return -1;
}
i2c_send(bus, command);
@@ -113,12 +113,12 @@ int smbus_read_block(I2CBus *bus, uint8_t addr, uint8_t command, uint8_t *data,
int i;
if (send_cmd) {
- if (i2c_start_transfer(bus, addr, 0)) {
+ if (i2c_start_send(bus, addr)) {
return -1;
}
i2c_send(bus, command);
}
- if (i2c_start_transfer(bus, addr, 1)) {
+ if (i2c_start_recv(bus, addr)) {
if (send_cmd) {
i2c_end_transfer(bus);
}
@@ -149,7 +149,7 @@ int smbus_write_block(I2CBus *bus, uint8_t addr, uint8_t command, uint8_t *data,
len = 32;
}
- if (i2c_start_transfer(bus, addr, 0)) {
+ if (i2c_start_send(bus, addr)) {
return -1;
}
i2c_send(bus, command);
diff --git a/hw/misc/auxbus.c b/hw/misc/auxbus.c
index 44aa9730bc9..434ff8d910d 100644
--- a/hw/misc/auxbus.c
+++ b/hw/misc/auxbus.c
@@ -135,7 +135,7 @@ AUXReply aux_request(AUXBus *bus, AUXCommand cmd, uint32_t address,
i2c_end_transfer(i2c_bus);
}
- if (i2c_start_transfer(i2c_bus, address, true)) {
+ if (i2c_start_recv(i2c_bus, address)) {
ret = AUX_I2C_NACK;
break;
}
@@ -151,7 +151,7 @@ AUXReply aux_request(AUXBus *bus, AUXCommand cmd, uint32_t address,
i2c_end_transfer(i2c_bus);
}
- if (i2c_start_transfer(i2c_bus, address, false)) {
+ if (i2c_start_send(i2c_bus, address)) {
ret = AUX_I2C_NACK;
break;
}
@@ -179,7 +179,7 @@ AUXReply aux_request(AUXBus *bus, AUXCommand cmd, uint32_t address,
/*
* No transactions started..
*/
- if (i2c_start_transfer(i2c_bus, address, false)) {
+ if (i2c_start_send(i2c_bus, address)) {
break;
}
} else if ((address != bus->last_i2c_address) ||
@@ -188,7 +188,7 @@ AUXReply aux_request(AUXBus *bus, AUXCommand cmd, uint32_t address,
* Transaction started but we need to restart..
*/
i2c_end_transfer(i2c_bus);
- if (i2c_start_transfer(i2c_bus, address, false)) {
+ if (i2c_start_send(i2c_bus, address)) {
break;
}
}
@@ -210,7 +210,7 @@ AUXReply aux_request(AUXBus *bus, AUXCommand cmd, uint32_t address,
/*
* No transactions started..
*/
- if (i2c_start_transfer(i2c_bus, address, true)) {
+ if (i2c_start_recv(i2c_bus, address)) {
break;
}
} else if ((address != bus->last_i2c_address) ||
@@ -219,7 +219,7 @@ AUXReply aux_request(AUXBus *bus, AUXCommand cmd, uint32_t address,
* Transaction started but we need to restart..
*/
i2c_end_transfer(i2c_bus);
- if (i2c_start_transfer(i2c_bus, address, true)) {
+ if (i2c_start_recv(i2c_bus, address)) {
break;
}
}
--
2.31.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH v4 15/15] hw/i2c: Introduce i2c_start_recv() and i2c_start_send()
2021-06-16 21:42 ` [PATCH v4 15/15] hw/i2c: Introduce i2c_start_recv() and i2c_start_send() Philippe Mathieu-Daudé
@ 2021-06-17 0:23 ` Richard Henderson
0 siblings, 0 replies; 19+ messages in thread
From: Richard Henderson @ 2021-06-17 0:23 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Corey Minyard, KONRAD Frederic, qemu-arm, qemu-ppc
On 6/16/21 2:42 PM, Philippe Mathieu-Daudé wrote:
> To ease reviewing code using the I2C bus API, introduce the
> i2c_start_recv() and i2c_start_send() helpers which don't
> take the confusing 'is_recv' boolean argument.
>
> Use these new helpers in the SMBus / AUX bus models.
>
> Suggested-by: Richard Henderson<richard.henderson@linaro.org>
> Signed-off-by: Philippe Mathieu-Daudé<f4bug@amsat.org>
> ---
> include/hw/i2c/i2c.h | 24 ++++++++++++++++++++++++
> hw/i2c/core.c | 10 ++++++++++
> hw/i2c/pm_smbus.c | 4 ++--
> hw/i2c/smbus_master.c | 22 +++++++++++-----------
> hw/misc/auxbus.c | 12 ++++++------
> 5 files changed, 53 insertions(+), 19 deletions(-)
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
r~
^ permalink raw reply [flat|nested] 19+ messages in thread