Patchwork [v1,1/6] i2c: support address ranges

login
register
mail settings
Submitter Peter Crosthwaite
Date Feb. 20, 2013, 5:29 a.m.
Message ID <29889099adf9f3347cd43b8c95f1201de5fa8e71.1361337686.git.peter.crosthwaite@xilinx.com>
Download mbox | patch
Permalink /patch/221948/
State New
Headers show

Comments

Peter Crosthwaite - Feb. 20, 2013, 5:29 a.m.
Some I2C devices (eg m24c08) can decode a linear range of addresses
(e.g. 0b10100xx). Add the address_range field to I2C slave that specifies the
number of consecutive addresses the device decodes.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---

 hw/i2c.c |   14 ++++++++++----
 hw/i2c.h |    6 ++++++
 2 files changed, 16 insertions(+), 4 deletions(-)
Paolo Bonzini - Feb. 20, 2013, 7:27 a.m.
Il 20/02/2013 06:29, Peter Crosthwaite ha scritto:
> @@ -192,12 +197,13 @@ static int i2c_slave_post_load(void *opaque, int version_id)
>  
>  const VMStateDescription vmstate_i2c_slave = {
>      .name = "I2CSlave",
> -    .version_id = 1,
> -    .minimum_version_id = 1,
> -    .minimum_version_id_old = 1,
> +    .version_id = 2,
> +    .minimum_version_id = 2,
> +    .minimum_version_id_old = 2,
>      .post_load = i2c_slave_post_load,
>      .fields      = (VMStateField []) {
>          VMSTATE_UINT8(address, I2CSlave),
> +        VMSTATE_UINT8(address_range, I2CSlave),
>          VMSTATE_END_OF_LIST()

Properties do not need to be serialized.

Paolo
Peter Crosthwaite - Feb. 20, 2013, 12:30 p.m.
On Wed, Feb 20, 2013 at 5:27 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Il 20/02/2013 06:29, Peter Crosthwaite ha scritto:
>> @@ -192,12 +197,13 @@ static int i2c_slave_post_load(void *opaque, int version_id)
>>
>>  const VMStateDescription vmstate_i2c_slave = {
>>      .name = "I2CSlave",
>> -    .version_id = 1,
>> -    .minimum_version_id = 1,
>> -    .minimum_version_id_old = 1,
>> +    .version_id = 2,
>> +    .minimum_version_id = 2,
>> +    .minimum_version_id_old = 2,
>>      .post_load = i2c_slave_post_load,
>>      .fields      = (VMStateField []) {
>>          VMSTATE_UINT8(address, I2CSlave),
>> +        VMSTATE_UINT8(address_range, I2CSlave),
>>          VMSTATE_END_OF_LIST()
>
> Properties do not need to be serialized.
>

Ok,

I do wonder why the address itselt in VMSDs however and my best guess
is the intention is that devices can modify their address at runtime
(phyiscally very possible and valid with I2C). So what happens if a
device changes the value of one of its props at runtime? E.G. in this
case what happens if my device decides to change its I2CSlave->address
in response to some arbitrary event? Will that play foul with VMSD or
can i just save the prop in my VMSD with the rest of my device state
to implement restoration of the modified prop?

Regards,
Peter

> Paolo
>
Paolo Bonzini - Feb. 20, 2013, 1:04 p.m.
Il 20/02/2013 13:30, Peter Crosthwaite ha scritto:
> Ok,
> 
> I do wonder why the address itselt in VMSDs however and my best guess
> is the intention is that devices can modify their address at runtime
> (phyiscally very possible and valid with I2C). So what happens if a
> device changes the value of one of its props at runtime? E.G. in this
> case what happens if my device decides to change its I2CSlave->address
> in response to some arbitrary event? Will that play foul with VMSD or
> can i just save the prop in my VMSD with the rest of my device state
> to implement restoration of the modified prop?

Ok, in this case this would be valid, but the implementation is still a
bit weird.  What is the behavior on reset?  Should it go back to the
original value of the address?

If so, you need to use a separate variable to store the initial value of
the address and the current value.  Then the initial value is immutable
and thus not serialized; the current value instead is part of the
VMState.  More importantly, the reset callback can then reset the
current address back to the initial value.

Paolo

Patch

diff --git a/hw/i2c.c b/hw/i2c.c
index ec314a4..a9004e6 100644
--- a/hw/i2c.c
+++ b/hw/i2c.c
@@ -19,6 +19,7 @@  struct i2c_bus
 
 static Property i2c_props[] = {
     DEFINE_PROP_UINT8("address", struct I2CSlave, address, 0),
+    DEFINE_PROP_UINT8("address-range", struct I2CSlave, address_range, 1),
     DEFINE_PROP_END_OF_LIST(),
 };
 
@@ -93,7 +94,8 @@  int i2c_start_transfer(i2c_bus *bus, uint8_t address, int recv)
     QTAILQ_FOREACH(kid, &bus->qbus.children, sibling) {
         DeviceState *qdev = kid->child;
         I2CSlave *candidate = I2C_SLAVE(qdev);
-        if (candidate->address == address) {
+        if (address >= candidate->address &&
+                address < candidate->address + candidate->address_range) {
             slave = candidate;
             break;
         }
@@ -110,6 +112,9 @@  int i2c_start_transfer(i2c_bus *bus, uint8_t address, int recv)
     if (sc->event) {
         sc->event(slave, recv ? I2C_START_RECV : I2C_START_SEND);
     }
+    if (sc->decode_address) {
+        sc->decode_address(slave, address);
+    }
     return 0;
 }
 
@@ -192,12 +197,13 @@  static int i2c_slave_post_load(void *opaque, int version_id)
 
 const VMStateDescription vmstate_i2c_slave = {
     .name = "I2CSlave",
-    .version_id = 1,
-    .minimum_version_id = 1,
-    .minimum_version_id_old = 1,
+    .version_id = 2,
+    .minimum_version_id = 2,
+    .minimum_version_id_old = 2,
     .post_load = i2c_slave_post_load,
     .fields      = (VMStateField []) {
         VMSTATE_UINT8(address, I2CSlave),
+        VMSTATE_UINT8(address_range, I2CSlave),
         VMSTATE_END_OF_LIST()
     }
 };
diff --git a/hw/i2c.h b/hw/i2c.h
index 0e80d5a..0021125 100644
--- a/hw/i2c.h
+++ b/hw/i2c.h
@@ -40,6 +40,11 @@  typedef struct I2CSlaveClass
 
     /* Notify the slave of a bus state change.  */
     void (*event)(I2CSlave *s, enum i2c_event event);
+
+    /* Notify the slave what address was decoded. Only needed for slaves that
+     * decode multiple addresses. Called after event() for I2C_START_RECV/SEND
+     */
+    void (*decode_address)(I2CSlave *s, uint8_t address);
 } I2CSlaveClass;
 
 struct I2CSlave
@@ -48,6 +53,7 @@  struct I2CSlave
 
     /* Remaining fields for internal use by the I2C code.  */
     uint8_t address;
+    uint8_t address_range;
 };
 
 i2c_bus *i2c_init_bus(DeviceState *parent, const char *name);