subject:"\[Qemu\-devel\] \[PATCH V2 2\/7\] i2c\: implement broadcast write."

Re: [Qemu-devel] [PATCH V2 2/7] i2c: implement broadcast write.

2015-07-06 Thread Frederic Konrad


On 24/06/2015 08:35, Peter Crosthwaite wrote:

On Mon, Jun 15, 2015 at 8:15 AM,  fred.kon...@greensocs.com wrote:

From: KONRAD Frederic fred.kon...@greensocs.com

This does a write to every slaves when the I2C bus get a write to address 0.

Signed-off-by: KONRAD Frederic fred.kon...@greensocs.com
---
  hw/i2c/core.c | 46 +-
  1 file changed, 45 insertions(+), 1 deletion(-)

diff --git a/hw/i2c/core.c b/hw/i2c/core.c
index 5a64026..db1cbdd 100644
--- a/hw/i2c/core.c
+++ b/hw/i2c/core.c
@@ -15,6 +15,7 @@ struct I2CBus
  I2CSlave *current_dev;
  I2CSlave *dev;
  uint8_t saved_address;
+bool broadcast;
  };

  static Property i2c_props[] = {
@@ -67,6 +68,8 @@ I2CBus *i2c_init_bus(DeviceState *parent, const char *name)

  bus = I2C_BUS(qbus_create(TYPE_I2C_BUS, parent, name));
  vmstate_register(NULL, -1, vmstate_i2c_bus, bus);
+
+bus-broadcast = false;

0 initialiser should not be needed for new QOM object.


  return bus;
  }

@@ -89,6 +92,21 @@ int i2c_start_transfer(I2CBus *bus, uint8_t address, int 
recv)
  I2CSlave *slave = NULL;
  I2CSlaveClass *sc;

+if (address == 0x00) {
+/*
+ * This is a broadcast.
+ */

One line comment.


+QTAILQ_FOREACH(kid, bus-qbus.children, sibling) {
+I2CSlave *dev = I2C_SLAVE(kid-child);
+sc = I2C_SLAVE_GET_CLASS(dev);
+bus-broadcast = true;

Move outside loop.


+if (sc-event) {
+sc-event(dev, recv ? I2C_START_RECV : I2C_START_SEND);
+}
+}
+return 0;
+}
+
  QTAILQ_FOREACH(kid, bus-qbus.children, sibling) {
  DeviceState *qdev = kid-child;
  I2CSlave *candidate = I2C_SLAVE(qdev);
@@ -114,9 +132,21 @@ int i2c_start_transfer(I2CBus *bus, uint8_t address, int 
recv)

  void i2c_end_transfer(I2CBus *bus)
  {
+BusChild *kid;
  I2CSlave *dev = bus-current_dev;
  I2CSlaveClass *sc;

+if (bus-broadcast) {
+QTAILQ_FOREACH(kid, bus-qbus.children, sibling) {
+I2CSlave *dev = I2C_SLAVE(kid-child);
+sc = I2C_SLAVE_GET_CLASS(dev);
+if (sc-event) {
+sc-event(dev, I2C_FINISH);
+}
+}
+bus-broadcast = false;
+}
+
  if (!dev) {
  return;
  }
@@ -131,8 +161,22 @@ void i2c_end_transfer(I2CBus *bus)

  int i2c_send(I2CBus *bus, uint8_t data)
  {
+BusChild *kid;
  I2CSlave *dev = bus-current_dev;
  I2CSlaveClass *sc;
+int ret = 0;
+
+if (bus-broadcast) {
+QTAILQ_FOREACH(kid, bus-qbus.children, sibling) {
+I2CSlave *dev = I2C_SLAVE(kid-child);
+sc = I2C_SLAVE_GET_CLASS(dev);
+bus-broadcast = true;
+if (sc-send) {
+ret |= sc-send(dev, data);
+}
+}

Still not sure about the duped core functionality of each of these
APIs. That is, the same code is needed in both a looped form and a 1
form. Can this be solved by listifying current_dev? That is, -current
dev is turned into a list which in the normal case will be populated
with 1 element by start_transfer() for the current dev. In the
broadcast case, all qbus.children are added to the list. The broadcast
bool is then removed. start() send() and end_transfer() then just loop
through the list unconditionally.


I think better keeping this broadcast as we need it for VMSD anyway?


Regards,
Peter


+return ret;
+}

  if (!dev) {
  return -1;
@@ -151,7 +195,7 @@ int i2c_recv(I2CBus *bus)
  I2CSlave *dev = bus-current_dev;
  I2CSlaveClass *sc;

-if (!dev) {
+if ((!dev) || (bus-broadcast)) {
  return -1;
  }

--
1.9.0

Re: [Qemu-devel] [PATCH V2 2/7] i2c: implement broadcast write.

2015-07-06 Thread Peter Crosthwaite

On Mon, Jul 6, 2015 at 9:28 AM, Frederic Konrad
fred.kon...@greensocs.com wrote:
 On 24/06/2015 08:35, Peter Crosthwaite wrote:

 On Mon, Jun 15, 2015 at 8:15 AM,  fred.kon...@greensocs.com wrote:

 From: KONRAD Frederic fred.kon...@greensocs.com

 This does a write to every slaves when the I2C bus get a write to address
 0.

 Signed-off-by: KONRAD Frederic fred.kon...@greensocs.com
 ---
   hw/i2c/core.c | 46 +-
   1 file changed, 45 insertions(+), 1 deletion(-)

 diff --git a/hw/i2c/core.c b/hw/i2c/core.c
 index 5a64026..db1cbdd 100644
 --- a/hw/i2c/core.c
 +++ b/hw/i2c/core.c
 @@ -15,6 +15,7 @@ struct I2CBus
   I2CSlave *current_dev;
   I2CSlave *dev;
   uint8_t saved_address;
 +bool broadcast;
   };

   static Property i2c_props[] = {
 @@ -67,6 +68,8 @@ I2CBus *i2c_init_bus(DeviceState *parent, const char
 *name)

   bus = I2C_BUS(qbus_create(TYPE_I2C_BUS, parent, name));
   vmstate_register(NULL, -1, vmstate_i2c_bus, bus);
 +
 +bus-broadcast = false;

 0 initialiser should not be needed for new QOM object.

   return bus;
   }

 @@ -89,6 +92,21 @@ int i2c_start_transfer(I2CBus *bus, uint8_t address,
 int recv)
   I2CSlave *slave = NULL;
   I2CSlaveClass *sc;

 +if (address == 0x00) {
 +/*
 + * This is a broadcast.
 + */

 One line comment.

 +QTAILQ_FOREACH(kid, bus-qbus.children, sibling) {
 +I2CSlave *dev = I2C_SLAVE(kid-child);
 +sc = I2C_SLAVE_GET_CLASS(dev);
 +bus-broadcast = true;

 Move outside loop.

 +if (sc-event) {
 +sc-event(dev, recv ? I2C_START_RECV : I2C_START_SEND);
 +}
 +}
 +return 0;
 +}
 +
   QTAILQ_FOREACH(kid, bus-qbus.children, sibling) {
   DeviceState *qdev = kid-child;
   I2CSlave *candidate = I2C_SLAVE(qdev);
 @@ -114,9 +132,21 @@ int i2c_start_transfer(I2CBus *bus, uint8_t address,
 int recv)

   void i2c_end_transfer(I2CBus *bus)
   {
 +BusChild *kid;
   I2CSlave *dev = bus-current_dev;
   I2CSlaveClass *sc;

 +if (bus-broadcast) {
 +QTAILQ_FOREACH(kid, bus-qbus.children, sibling) {
 +I2CSlave *dev = I2C_SLAVE(kid-child);
 +sc = I2C_SLAVE_GET_CLASS(dev);
 +if (sc-event) {
 +sc-event(dev, I2C_FINISH);
 +}
 +}
 +bus-broadcast = false;
 +}
 +
   if (!dev) {
   return;
   }
 @@ -131,8 +161,22 @@ void i2c_end_transfer(I2CBus *bus)

   int i2c_send(I2CBus *bus, uint8_t data)
   {
 +BusChild *kid;
   I2CSlave *dev = bus-current_dev;
   I2CSlaveClass *sc;
 +int ret = 0;
 +
 +if (bus-broadcast) {
 +QTAILQ_FOREACH(kid, bus-qbus.children, sibling) {
 +I2CSlave *dev = I2C_SLAVE(kid-child);
 +sc = I2C_SLAVE_GET_CLASS(dev);
 +bus-broadcast = true;
 +if (sc-send) {
 +ret |= sc-send(dev, data);
 +}
 +}

 Still not sure about the duped core functionality of each of these
 APIs. That is, the same code is needed in both a looped form and a 1
 form. Can this be solved by listifying current_dev? That is, -current
 dev is turned into a list which in the normal case will be populated
 with 1 element by start_transfer() for the current dev. In the
 broadcast case, all qbus.children are added to the list. The broadcast
 bool is then removed. start() send() and end_transfer() then just loop
 through the list unconditionally.


 I think better keeping this broadcast as we need it for VMSD anyway?


Ok I see the VMSD issue and can keep the boolean, but I'm more
concerned about the duped code. It's hard to patch this reliably, if
the same core of code is duped.

Regards,
Peter


 Regards,
 Peter

 +return ret;
 +}

   if (!dev) {
   return -1;
 @@ -151,7 +195,7 @@ int i2c_recv(I2CBus *bus)
   I2CSlave *dev = bus-current_dev;
   I2CSlaveClass *sc;

 -if (!dev) {
 +if ((!dev) || (bus-broadcast)) {
   return -1;
   }

 --
 1.9.0

Re: [Qemu-devel] [PATCH V2 2/7] i2c: implement broadcast write.

2015-06-24 Thread Peter Crosthwaite

On Mon, Jun 15, 2015 at 8:15 AM,  fred.kon...@greensocs.com wrote:
 From: KONRAD Frederic fred.kon...@greensocs.com

 This does a write to every slaves when the I2C bus get a write to address 0.

 Signed-off-by: KONRAD Frederic fred.kon...@greensocs.com
 ---
  hw/i2c/core.c | 46 +-
  1 file changed, 45 insertions(+), 1 deletion(-)

 diff --git a/hw/i2c/core.c b/hw/i2c/core.c
 index 5a64026..db1cbdd 100644
 --- a/hw/i2c/core.c
 +++ b/hw/i2c/core.c
 @@ -15,6 +15,7 @@ struct I2CBus
  I2CSlave *current_dev;
  I2CSlave *dev;
  uint8_t saved_address;
 +bool broadcast;
  };

  static Property i2c_props[] = {
 @@ -67,6 +68,8 @@ I2CBus *i2c_init_bus(DeviceState *parent, const char *name)

  bus = I2C_BUS(qbus_create(TYPE_I2C_BUS, parent, name));
  vmstate_register(NULL, -1, vmstate_i2c_bus, bus);
 +
 +bus-broadcast = false;

0 initialiser should not be needed for new QOM object.

  return bus;
  }

 @@ -89,6 +92,21 @@ int i2c_start_transfer(I2CBus *bus, uint8_t address, int 
 recv)
  I2CSlave *slave = NULL;
  I2CSlaveClass *sc;

 +if (address == 0x00) {
 +/*
 + * This is a broadcast.
 + */

One line comment.

 +QTAILQ_FOREACH(kid, bus-qbus.children, sibling) {
 +I2CSlave *dev = I2C_SLAVE(kid-child);
 +sc = I2C_SLAVE_GET_CLASS(dev);
 +bus-broadcast = true;

Move outside loop.

 +if (sc-event) {
 +sc-event(dev, recv ? I2C_START_RECV : I2C_START_SEND);
 +}
 +}
 +return 0;
 +}
 +
  QTAILQ_FOREACH(kid, bus-qbus.children, sibling) {
  DeviceState *qdev = kid-child;
  I2CSlave *candidate = I2C_SLAVE(qdev);
 @@ -114,9 +132,21 @@ int i2c_start_transfer(I2CBus *bus, uint8_t address, int 
 recv)

  void i2c_end_transfer(I2CBus *bus)
  {
 +BusChild *kid;
  I2CSlave *dev = bus-current_dev;
  I2CSlaveClass *sc;

 +if (bus-broadcast) {
 +QTAILQ_FOREACH(kid, bus-qbus.children, sibling) {
 +I2CSlave *dev = I2C_SLAVE(kid-child);
 +sc = I2C_SLAVE_GET_CLASS(dev);
 +if (sc-event) {
 +sc-event(dev, I2C_FINISH);
 +}
 +}
 +bus-broadcast = false;
 +}
 +
  if (!dev) {
  return;
  }
 @@ -131,8 +161,22 @@ void i2c_end_transfer(I2CBus *bus)

  int i2c_send(I2CBus *bus, uint8_t data)
  {
 +BusChild *kid;
  I2CSlave *dev = bus-current_dev;
  I2CSlaveClass *sc;
 +int ret = 0;
 +
 +if (bus-broadcast) {
 +QTAILQ_FOREACH(kid, bus-qbus.children, sibling) {
 +I2CSlave *dev = I2C_SLAVE(kid-child);
 +sc = I2C_SLAVE_GET_CLASS(dev);
 +bus-broadcast = true;
 +if (sc-send) {
 +ret |= sc-send(dev, data);
 +}
 +}

Still not sure about the duped core functionality of each of these
APIs. That is, the same code is needed in both a looped form and a 1
form. Can this be solved by listifying current_dev? That is, -current
dev is turned into a list which in the normal case will be populated
with 1 element by start_transfer() for the current dev. In the
broadcast case, all qbus.children are added to the list. The broadcast
bool is then removed. start() send() and end_transfer() then just loop
through the list unconditionally.

Regards,
Peter

 +return ret;
 +}

  if (!dev) {
  return -1;
 @@ -151,7 +195,7 @@ int i2c_recv(I2CBus *bus)
  I2CSlave *dev = bus-current_dev;
  I2CSlaveClass *sc;

 -if (!dev) {
 +if ((!dev) || (bus-broadcast)) {
  return -1;
  }

 --
 1.9.0

[Qemu-devel] [PATCH V2 2/7] i2c: implement broadcast write.

2015-06-15 Thread fred . konrad

From: KONRAD Frederic fred.kon...@greensocs.com

This does a write to every slaves when the I2C bus get a write to address 0.

Signed-off-by: KONRAD Frederic fred.kon...@greensocs.com
---
 hw/i2c/core.c | 46 +-
 1 file changed, 45 insertions(+), 1 deletion(-)

diff --git a/hw/i2c/core.c b/hw/i2c/core.c
index 5a64026..db1cbdd 100644
--- a/hw/i2c/core.c
+++ b/hw/i2c/core.c
@@ -15,6 +15,7 @@ struct I2CBus
 I2CSlave *current_dev;
 I2CSlave *dev;
 uint8_t saved_address;
+bool broadcast;
 };
 
 static Property i2c_props[] = {
@@ -67,6 +68,8 @@ I2CBus *i2c_init_bus(DeviceState *parent, const char *name)
 
 bus = I2C_BUS(qbus_create(TYPE_I2C_BUS, parent, name));
 vmstate_register(NULL, -1, vmstate_i2c_bus, bus);
+
+bus-broadcast = false;
 return bus;
 }
 
@@ -89,6 +92,21 @@ int i2c_start_transfer(I2CBus *bus, uint8_t address, int 
recv)
 I2CSlave *slave = NULL;
 I2CSlaveClass *sc;
 
+if (address == 0x00) {
+/*
+ * This is a broadcast.
+ */
+QTAILQ_FOREACH(kid, bus-qbus.children, sibling) {
+I2CSlave *dev = I2C_SLAVE(kid-child);
+sc = I2C_SLAVE_GET_CLASS(dev);
+bus-broadcast = true;
+if (sc-event) {
+sc-event(dev, recv ? I2C_START_RECV : I2C_START_SEND);
+}
+}
+return 0;
+}
+
 QTAILQ_FOREACH(kid, bus-qbus.children, sibling) {
 DeviceState *qdev = kid-child;
 I2CSlave *candidate = I2C_SLAVE(qdev);
@@ -114,9 +132,21 @@ int i2c_start_transfer(I2CBus *bus, uint8_t address, int 
recv)
 
 void i2c_end_transfer(I2CBus *bus)
 {
+BusChild *kid;
 I2CSlave *dev = bus-current_dev;
 I2CSlaveClass *sc;
 
+if (bus-broadcast) {
+QTAILQ_FOREACH(kid, bus-qbus.children, sibling) {
+I2CSlave *dev = I2C_SLAVE(kid-child);
+sc = I2C_SLAVE_GET_CLASS(dev);
+if (sc-event) {
+sc-event(dev, I2C_FINISH);
+}
+}
+bus-broadcast = false;
+}
+
 if (!dev) {
 return;
 }
@@ -131,8 +161,22 @@ void i2c_end_transfer(I2CBus *bus)
 
 int i2c_send(I2CBus *bus, uint8_t data)
 {
+BusChild *kid;
 I2CSlave *dev = bus-current_dev;
 I2CSlaveClass *sc;
+int ret = 0;
+
+if (bus-broadcast) {
+QTAILQ_FOREACH(kid, bus-qbus.children, sibling) {
+I2CSlave *dev = I2C_SLAVE(kid-child);
+sc = I2C_SLAVE_GET_CLASS(dev);
+bus-broadcast = true;
+if (sc-send) {
+ret |= sc-send(dev, data);
+}
+}
+return ret;
+}
 
 if (!dev) {
 return -1;
@@ -151,7 +195,7 @@ int i2c_recv(I2CBus *bus)
 I2CSlave *dev = bus-current_dev;
 I2CSlaveClass *sc;
 
-if (!dev) {
+if ((!dev) || (bus-broadcast)) {
 return -1;
 }
 
-- 
1.9.0

Re: [Qemu-devel] [PATCH V2 2/7] i2c: implement broadcast write.

Re: [Qemu-devel] [PATCH V2 2/7] i2c: implement broadcast write.

Re: [Qemu-devel] [PATCH V2 2/7] i2c: implement broadcast write.

[Qemu-devel] [PATCH V2 2/7] i2c: implement broadcast write.

4 matches

Site Navigation

Mail list logo

Footer information