| Message ID | 20210616161418.2514095-14-f4bug@amsat.org |
|---|---|
| State | New |
| Headers | show |
| Series | hw/i2c: Remove confusing i2c_send_recv() API | expand |
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~
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); if (rv && !bus->broadcast) { if (bus_scanned) { /* First call, terminate the transfer. */