From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39163) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aOo7N-0007vP-0d for qemu-devel@nongnu.org; Thu, 28 Jan 2016 10:04:21 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aOo7G-0003Hz-JN for qemu-devel@nongnu.org; Thu, 28 Jan 2016 10:04:16 -0500 Received: from greensocs.com ([193.104.36.180]:42437) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aOo7G-0003H0-8r for qemu-devel@nongnu.org; Thu, 28 Jan 2016 10:04:10 -0500 Message-ID: <56AA2DE6.4020407@greensocs.com> Date: Thu, 28 Jan 2016 16:04:06 +0100 From: Frederic Konrad MIME-Version: 1.0 References: <56372862.9030004@greensocs.com> In-Reply-To: <56372862.9030004@greensocs.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [RFC v1 1/1] i2c: Factor our send() and recv() common logic List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Crosthwaite , qemu-devel@nongnu.org On 02/11/2015 10:09, Frederic Konrad wrote: > On 19/10/2015 06:09, Peter Crosthwaite wrote: >> Most of the control flow logic between send and recv (error checking >> etc) is the same. Factor this out into a common send_recv() API. >> This is then usable by clients, where the control logic for send >> and receive differs only by a boolean. E.g. >> >> if (send) >> i2c_send(...): >> else >> i2c_recv(...); >> >> becomes: >> >> i2c_send_recv(... , send); >> >> Signed-off-by: Peter Crosthwaite >> --- >> >> hw/i2c/core.c | 34 +++++++++++++++++++--------------- >> include/hw/i2c/i2c.h | 1 + >> 2 files changed, 20 insertions(+), 15 deletions(-) >> >> diff --git a/hw/i2c/core.c b/hw/i2c/core.c >> index 5a64026..d4a8cbb 100644 >> --- a/hw/i2c/core.c >> +++ b/hw/i2c/core.c >> @@ -129,7 +129,7 @@ void i2c_end_transfer(I2CBus *bus) >> bus->current_dev = NULL; >> } >> >> -int i2c_send(I2CBus *bus, uint8_t data) >> +int i2c_send_recv(I2CBus *bus, uint8_t *data, bool send) > Double space here between I2CBus and *bus. > > Otherwise it looks ok for me. Is everybody ok with this patch? If it's the case I can pick it with the dpdma series. Thanks, Fred > Fred >> { >> I2CSlave *dev = bus->current_dev; >> I2CSlaveClass *sc; >> @@ -139,28 +139,32 @@ int i2c_send(I2CBus *bus, uint8_t data) >> } >> >> sc = I2C_SLAVE_GET_CLASS(dev); >> - if (sc->send) { >> - return sc->send(dev, data); >> + if (send && sc->send) { >> + return sc->send(dev, *data); >> + } else if (!send && sc->recv) { >> + int ret = sc->recv(dev); >> + if (ret < 0) { >> + return ret; >> + } else { >> + *data = ret; >> + return 0; >> + } >> } >> >> return -1; >> } >> >> -int i2c_recv(I2CBus *bus) >> +int i2c_send(I2CBus *bus, uint8_t data) >> { >> - I2CSlave *dev = bus->current_dev; >> - I2CSlaveClass *sc; >> - >> - if (!dev) { >> - return -1; >> - } >> + return i2c_send_recv(bus, &data, true); >> +} >> >> - sc = I2C_SLAVE_GET_CLASS(dev); >> - if (sc->recv) { >> - return sc->recv(dev); >> - } >> +int i2c_recv(I2CBus *bus) >> +{ >> + uint8_t data; >> + int ret = i2c_send_recv(bus, &data, false); >> >> - return -1; >> + return ret < 0 ? ret : data; >> } >> >> void i2c_nack(I2CBus *bus) >> diff --git a/include/hw/i2c/i2c.h b/include/hw/i2c/i2c.h >> index 4986ebc..c4085aa 100644 >> --- a/include/hw/i2c/i2c.h >> +++ b/include/hw/i2c/i2c.h >> @@ -56,6 +56,7 @@ 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); >> int i2c_recv(I2CBus *bus); >> >