[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [RFC PATCH v2 4/6] hw/i2c: add asynchronous send
|
From: |
Corey Minyard |
|
Subject: |
Re: [RFC PATCH v2 4/6] hw/i2c: add asynchronous send |
|
Date: |
Wed, 1 Jun 2022 17:05:21 -0500 |
On Wed, Jun 01, 2022 at 11:08:29PM +0200, Klaus Jensen wrote:
> From: Klaus Jensen <k.jensen@samsung.com>
>
> Add an asynchronous version of i2c_send() that requires the slave to
> explicitly acknowledge on the bus with i2c_ack().
>
> The current master must use the new i2c_start_send_async() to indicate
> that it wants to do an asynchronous transfer. This allows the i2c core
> to check if the target slave supports this or not. This approach relies
> on adding a new enum i2c_event member, which is why a bunch of other
> devices needs changes in their event handling switches.
This would be easier to read if you split out the default return of -1
in all the devices to a separate patch.
You've already pointed out the lack of nack support.
I think this is ok outside of that.
-corey
>
> Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
> ---
> hw/arm/pxa2xx.c | 2 ++
> hw/display/sii9022.c | 2 ++
> hw/display/ssd0303.c | 2 ++
> hw/i2c/core.c | 36 +++++++++++++++++++++++++++++++++++-
> hw/i2c/smbus_slave.c | 4 ++++
> hw/i2c/trace-events | 2 ++
> hw/misc/ibm-cffps.c | 2 ++
> hw/misc/ir35221.c | 2 ++
> hw/nvram/eeprom_at24c.c | 2 ++
> hw/sensor/lsm303dlhc_mag.c | 2 ++
> include/hw/i2c/i2c.h | 16 ++++++++++++++++
> 11 files changed, 71 insertions(+), 1 deletion(-)
>
> diff --git a/hw/arm/pxa2xx.c b/hw/arm/pxa2xx.c
> index f4f687df68ef..93dda83d7aa9 100644
> --- a/hw/arm/pxa2xx.c
> +++ b/hw/arm/pxa2xx.c
> @@ -1305,6 +1305,8 @@ static int pxa2xx_i2c_event(I2CSlave *i2c, enum
> i2c_event event)
> case I2C_NACK:
> s->status |= 1 << 1; /* set ACKNAK */
> break;
> + default:
> + return -1;
> }
> pxa2xx_i2c_update(s);
>
> diff --git a/hw/display/sii9022.c b/hw/display/sii9022.c
> index b591a5878901..664fd4046d82 100644
> --- a/hw/display/sii9022.c
> +++ b/hw/display/sii9022.c
> @@ -76,6 +76,8 @@ static int sii9022_event(I2CSlave *i2c, enum i2c_event
> event)
> break;
> case I2C_NACK:
> break;
> + default:
> + return -1;
> }
>
> return 0;
> diff --git a/hw/display/ssd0303.c b/hw/display/ssd0303.c
> index aeae22da9c29..d67b0ad7b529 100644
> --- a/hw/display/ssd0303.c
> +++ b/hw/display/ssd0303.c
> @@ -196,6 +196,8 @@ static int ssd0303_event(I2CSlave *i2c, enum i2c_event
> event)
> case I2C_NACK:
> /* Nothing to do. */
> break;
> + default:
> + return -1;
> }
>
> return 0;
> diff --git a/hw/i2c/core.c b/hw/i2c/core.c
> index 145dce60782a..d4ba8146bffb 100644
> --- a/hw/i2c/core.c
> +++ b/hw/i2c/core.c
> @@ -161,7 +161,8 @@ static int i2c_do_start_transfer(I2CBus *bus, uint8_t
> address,
> start condition. */
>
> if (sc->event) {
> - trace_i2c_event("start", s->address);
> + trace_i2c_event(event == I2C_START_SEND ? "start" :
> "start_async",
> + s->address);
> rv = sc->event(s, event);
> if (rv && !bus->broadcast) {
> if (bus_scanned) {
> @@ -212,6 +213,11 @@ int i2c_start_send(I2CBus *bus, uint8_t address)
> return i2c_do_start_transfer(bus, address, I2C_START_SEND);
> }
>
> +int i2c_start_send_async(I2CBus *bus, uint8_t address)
> +{
> + return i2c_do_start_transfer(bus, address, I2C_START_SEND_ASYNC);
> +}
> +
> void i2c_end_transfer(I2CBus *bus)
> {
> I2CSlaveClass *sc;
> @@ -261,6 +267,23 @@ int i2c_send(I2CBus *bus, uint8_t data)
> return ret ? -1 : 0;
> }
>
> +int i2c_send_async(I2CBus *bus, uint8_t data)
> +{
> + I2CNode *node = QLIST_FIRST(&bus->current_devs);
> + I2CSlave *slave = node->elt;
> + I2CSlaveClass *sc = I2C_SLAVE_GET_CLASS(slave);
> +
> + if (!sc->send_async) {
> + return -1;
> + }
> +
> + trace_i2c_send_async(slave->address, data);
> +
> + sc->send_async(slave, data);
> +
> + return 0;
> +}
> +
> uint8_t i2c_recv(I2CBus *bus)
> {
> uint8_t data = 0xff;
> @@ -297,6 +320,17 @@ void i2c_nack(I2CBus *bus)
> }
> }
>
> +void i2c_ack(I2CBus *bus)
> +{
> + if (!bus->bh) {
> + return;
> + }
> +
> + trace_i2c_ack();
> +
> + qemu_bh_schedule(bus->bh);
> +}
> +
> static int i2c_slave_post_load(void *opaque, int version_id)
> {
> I2CSlave *dev = opaque;
> diff --git a/hw/i2c/smbus_slave.c b/hw/i2c/smbus_slave.c
> index 5d10e27664db..feb3ec633350 100644
> --- a/hw/i2c/smbus_slave.c
> +++ b/hw/i2c/smbus_slave.c
> @@ -143,6 +143,10 @@ static int smbus_i2c_event(I2CSlave *s, enum i2c_event
> event)
> dev->mode = SMBUS_CONFUSED;
> break;
> }
> + break;
> +
> + default:
> + return -1;
> }
>
> return 0;
> diff --git a/hw/i2c/trace-events b/hw/i2c/trace-events
> index 209275ed2dc8..af181d43ee64 100644
> --- a/hw/i2c/trace-events
> +++ b/hw/i2c/trace-events
> @@ -4,7 +4,9 @@
>
> i2c_event(const char *event, uint8_t address) "%s(addr:0x%02x)"
> i2c_send(uint8_t address, uint8_t data) "send(addr:0x%02x) data:0x%02x"
> +i2c_send_async(uint8_t address, uint8_t data) "send_async(addr:0x%02x)
> data:0x%02x"
> i2c_recv(uint8_t address, uint8_t data) "recv(addr:0x%02x) data:0x%02x"
> +i2c_ack(void) ""
>
> # aspeed_i2c.c
>
> diff --git a/hw/misc/ibm-cffps.c b/hw/misc/ibm-cffps.c
> index 042155bb7838..d69a727fd5f9 100644
> --- a/hw/misc/ibm-cffps.c
> +++ b/hw/misc/ibm-cffps.c
> @@ -152,6 +152,8 @@ static int ibm_cffps_event(I2CSlave *i2c, enum i2c_event
> event)
> case I2C_FINISH:
> s->pointer = 0xFF;
> break;
> + default:
> + return -1;
> }
>
> s->len = 0;
> diff --git a/hw/misc/ir35221.c b/hw/misc/ir35221.c
> index 5e01d3655059..c46b9ee1c3bf 100644
> --- a/hw/misc/ir35221.c
> +++ b/hw/misc/ir35221.c
> @@ -117,6 +117,8 @@ static int ir35221_event(I2CSlave *i2c, enum i2c_event
> event)
> case I2C_FINISH:
> s->pointer = 0xFF;
> break;
> + default:
> + return -1;
> }
>
> s->len = 0;
> diff --git a/hw/nvram/eeprom_at24c.c b/hw/nvram/eeprom_at24c.c
> index 01a3093600fa..d695f6ae894a 100644
> --- a/hw/nvram/eeprom_at24c.c
> +++ b/hw/nvram/eeprom_at24c.c
> @@ -75,6 +75,8 @@ int at24c_eeprom_event(I2CSlave *s, enum i2c_event event)
> break;
> case I2C_NACK:
> break;
> + default:
> + return -1;
> }
> return 0;
> }
> diff --git a/hw/sensor/lsm303dlhc_mag.c b/hw/sensor/lsm303dlhc_mag.c
> index 4c98ddbf207c..bb8d48b2fdb0 100644
> --- a/hw/sensor/lsm303dlhc_mag.c
> +++ b/hw/sensor/lsm303dlhc_mag.c
> @@ -427,6 +427,8 @@ static int lsm303dlhc_mag_event(I2CSlave *i2c, enum
> i2c_event event)
> break;
> case I2C_NACK:
> break;
> + default:
> + return -1;
> }
>
> s->len = 0;
> diff --git a/include/hw/i2c/i2c.h b/include/hw/i2c/i2c.h
> index be8bb8b78a60..9b9581d23097 100644
> --- a/include/hw/i2c/i2c.h
> +++ b/include/hw/i2c/i2c.h
> @@ -12,6 +12,7 @@
> enum i2c_event {
> I2C_START_RECV,
> I2C_START_SEND,
> + I2C_START_SEND_ASYNC,
> I2C_FINISH,
> I2C_NACK /* Masker NACKed a receive byte. */
> };
> @@ -28,6 +29,9 @@ struct I2CSlaveClass {
> /* Master to slave. Returns non-zero for a NAK, 0 for success. */
> int (*send)(I2CSlave *s, uint8_t data);
>
> + /* Master to slave (asynchronous). Receiving slave must call i2c_ack().
> */
> + void (*send_async)(I2CSlave *s, uint8_t data);
> +
> /*
> * Slave to master. This cannot fail, the device should always
> * return something here.
> @@ -127,11 +131,23 @@ int i2c_start_recv(I2CBus *bus, uint8_t address);
> */
> int i2c_start_send(I2CBus *bus, uint8_t address);
>
> +/**
> + * i2c_start_send_async: start an asynchronous 'send' transfer on an I2C bus.
> + *
> + * @bus: #I2CBus to be used
> + * @address: address of the slave
> + *
> + * Return: 0 on success, -1 on error
> + */
> +int i2c_start_send_async(I2CBus *bus, uint8_t address);
> +
> void i2c_end_transfer(I2CBus *bus);
> void i2c_nack(I2CBus *bus);
> +void i2c_ack(I2CBus *bus);
> void i2c_bus_master(I2CBus *bus, QEMUBH *bh);
> void i2c_bus_release(I2CBus *bus);
> int i2c_send(I2CBus *bus, uint8_t data);
> +int i2c_send_async(I2CBus *bus, uint8_t data);
> uint8_t i2c_recv(I2CBus *bus);
> bool i2c_scan_bus(I2CBus *bus, uint8_t address, bool broadcast,
> I2CNodeList *current_devs);
> --
> 2.36.1
>
smime.p7s
Description: S/MIME Cryptographic Signature