qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [RFC v1 1/1] i2c: Factor our send() and recv() common logic
@ 2015-10-19  4:09 Peter Crosthwaite
  2015-11-02  9:09 ` Frederic Konrad
  0 siblings, 1 reply; 3+ messages in thread
From: Peter Crosthwaite @ 2015-10-19  4:09 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Crosthwaite, fred.konrad

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 <crosthwaite.peter@gmail.com>
---

 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)
 {
     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);
 
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [Qemu-devel] [RFC v1 1/1] i2c: Factor our send() and recv() common logic
  2015-10-19  4:09 [Qemu-devel] [RFC v1 1/1] i2c: Factor our send() and recv() common logic Peter Crosthwaite
@ 2015-11-02  9:09 ` Frederic Konrad
  2016-01-28 15:04   ` Frederic Konrad
  0 siblings, 1 reply; 3+ messages in thread
From: Frederic Konrad @ 2015-11-02  9:09 UTC (permalink / raw)
  To: Peter Crosthwaite, qemu-devel; +Cc: Peter Crosthwaite

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 <crosthwaite.peter@gmail.com>
> ---
>
>  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.

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);
>  

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [Qemu-devel] [RFC v1 1/1] i2c: Factor our send() and recv() common logic
  2015-11-02  9:09 ` Frederic Konrad
@ 2016-01-28 15:04   ` Frederic Konrad
  0 siblings, 0 replies; 3+ messages in thread
From: Frederic Konrad @ 2016-01-28 15:04 UTC (permalink / raw)
  To: Peter Crosthwaite, qemu-devel

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 <crosthwaite.peter@gmail.com>
>> ---
>>
>>  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);
>>  
>

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2016-01-28 15:04 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-10-19  4:09 [Qemu-devel] [RFC v1 1/1] i2c: Factor our send() and recv() common logic Peter Crosthwaite
2015-11-02  9:09 ` Frederic Konrad
2016-01-28 15:04   ` Frederic Konrad

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).