diff mbox series

[v2,10/10] i2c: Add vmstate handling to the smbus eeprom

Message ID 1534796770-10295-11-git-send-email-minyard@acm.org
State Superseded
Headers show
Series [v2,01/10] i2c:pm_smbus: Clean up some style issues | expand

Commit Message

Corey Minyard Aug. 20, 2018, 8:26 p.m. UTC
From: Corey Minyard <cminyard@mvista.com>


Signed-off-by: Corey Minyard <cminyard@mvista.com>

Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: "Michael S . Tsirkin" <mst@redhat.com>
Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 hw/i2c/smbus_eeprom.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

-- 
2.7.4

Comments

Dr. David Alan Gilbert Aug. 29, 2018, 11:44 a.m. UTC | #1
* minyard@acm.org (minyard@acm.org) wrote:
> From: Corey Minyard <cminyard@mvista.com>

> 

> Signed-off-by: Corey Minyard <cminyard@mvista.com>

> Cc: Paolo Bonzini <pbonzini@redhat.com>

> Cc: "Michael S . Tsirkin" <mst@redhat.com>

> Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>

> ---

>  hw/i2c/smbus_eeprom.c | 16 +++++++++++++++-

>  1 file changed, 15 insertions(+), 1 deletion(-)

> 

> diff --git a/hw/i2c/smbus_eeprom.c b/hw/i2c/smbus_eeprom.c

> index f18aa3d..d4430b0 100644

> --- a/hw/i2c/smbus_eeprom.c

> +++ b/hw/i2c/smbus_eeprom.c

> @@ -29,6 +29,8 @@

>  

>  //#define DEBUG

>  

> +#define TYPE_SMBUS_EEPROM_DEVICE "smbus-eeprom"

> +

>  typedef struct SMBusEEPROMDevice {

>      SMBusDevice smbusdev;

>      void *data;

> @@ -97,6 +99,17 @@ static uint8_t eeprom_read_data(SMBusDevice *dev, uint8_t cmd, int n)

>      return eeprom_receive_byte(dev);

>  }

>  

> +static const VMStateDescription vmstate_smbus_eeprom = {

> +    .name = TYPE_SMBUS_EEPROM_DEVICE,

> +    .version_id = 1,

> +    .minimum_version_id = 1,


Can you add a .needed and wire it up to a property that we can then wire
to machine types; that way we don't break migration compatibility.

> +    .fields      = (VMStateField[]) {

> +        VMSTATE_SMBUS_DEVICE(smbusdev, SMBusEEPROMDevice),

> +        VMSTATE_UINT8(offset, SMBusEEPROMDevice),


What about the data?

Dave

> +        VMSTATE_END_OF_LIST()

> +    }

> +};

> +

>  static void smbus_eeprom_realize(DeviceState *dev, Error **errp)

>  {

>      SMBusEEPROMDevice *eeprom = (SMBusEEPROMDevice *)dev;

> @@ -121,12 +134,13 @@ static void smbus_eeprom_class_initfn(ObjectClass *klass, void *data)

>      sc->write_data = eeprom_write_data;

>      sc->read_data = eeprom_read_data;

>      dc->props = smbus_eeprom_properties;

> +    dc->vmsd = &vmstate_smbus_eeprom;

>      /* Reason: pointer property "data" */

>      dc->user_creatable = false;

>  }

>  

>  static const TypeInfo smbus_eeprom_info = {

> -    .name          = "smbus-eeprom",

> +    .name          = TYPE_SMBUS_EEPROM_DEVICE,

>      .parent        = TYPE_SMBUS_DEVICE,

>      .instance_size = sizeof(SMBusEEPROMDevice),

>      .class_init    = smbus_eeprom_class_initfn,

> -- 

> 2.7.4

> 

--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
Dr. David Alan Gilbert Aug. 29, 2018, 12:01 p.m. UTC | #2
* Dr. David Alan Gilbert (dgilbert@redhat.com) wrote:
> * minyard@acm.org (minyard@acm.org) wrote:

> > From: Corey Minyard <cminyard@mvista.com>

> > 

> > Signed-off-by: Corey Minyard <cminyard@mvista.com>

> > Cc: Paolo Bonzini <pbonzini@redhat.com>

> > Cc: "Michael S . Tsirkin" <mst@redhat.com>

> > Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>

> > ---

> >  hw/i2c/smbus_eeprom.c | 16 +++++++++++++++-

> >  1 file changed, 15 insertions(+), 1 deletion(-)

> > 

> > diff --git a/hw/i2c/smbus_eeprom.c b/hw/i2c/smbus_eeprom.c

> > index f18aa3d..d4430b0 100644

> > --- a/hw/i2c/smbus_eeprom.c

> > +++ b/hw/i2c/smbus_eeprom.c

> > @@ -29,6 +29,8 @@

> >  

> >  //#define DEBUG

> >  

> > +#define TYPE_SMBUS_EEPROM_DEVICE "smbus-eeprom"

> > +

> >  typedef struct SMBusEEPROMDevice {

> >      SMBusDevice smbusdev;

> >      void *data;

> > @@ -97,6 +99,17 @@ static uint8_t eeprom_read_data(SMBusDevice *dev, uint8_t cmd, int n)

> >      return eeprom_receive_byte(dev);

> >  }

> >  

> > +static const VMStateDescription vmstate_smbus_eeprom = {

> > +    .name = TYPE_SMBUS_EEPROM_DEVICE,

> > +    .version_id = 1,

> > +    .minimum_version_id = 1,

> 

> Can you add a .needed and wire it up to a property that we can then wire

> to machine types; that way we don't break migration compatibility.

> 

> > +    .fields      = (VMStateField[]) {

> > +        VMSTATE_SMBUS_DEVICE(smbusdev, SMBusEEPROMDevice),

> > +        VMSTATE_UINT8(offset, SMBusEEPROMDevice),

> 

> What about the data?


Also; now I'm looking - I'm confused by 'offset' being an 8bit int
when I can see hw/arm/aspeed.c calling smbus_eeprom_init_one passing 8KB
and 32KB buffers in.

Dave

> Dave

> 

> > +        VMSTATE_END_OF_LIST()

> > +    }

> > +};

> > +

> >  static void smbus_eeprom_realize(DeviceState *dev, Error **errp)

> >  {

> >      SMBusEEPROMDevice *eeprom = (SMBusEEPROMDevice *)dev;

> > @@ -121,12 +134,13 @@ static void smbus_eeprom_class_initfn(ObjectClass *klass, void *data)

> >      sc->write_data = eeprom_write_data;

> >      sc->read_data = eeprom_read_data;

> >      dc->props = smbus_eeprom_properties;

> > +    dc->vmsd = &vmstate_smbus_eeprom;

> >      /* Reason: pointer property "data" */

> >      dc->user_creatable = false;

> >  }

> >  

> >  static const TypeInfo smbus_eeprom_info = {

> > -    .name          = "smbus-eeprom",

> > +    .name          = TYPE_SMBUS_EEPROM_DEVICE,

> >      .parent        = TYPE_SMBUS_DEVICE,

> >      .instance_size = sizeof(SMBusEEPROMDevice),

> >      .class_init    = smbus_eeprom_class_initfn,

> > -- 

> > 2.7.4

> > 

> --

> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

> 

--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
diff mbox series

Patch

diff --git a/hw/i2c/smbus_eeprom.c b/hw/i2c/smbus_eeprom.c
index f18aa3d..d4430b0 100644
--- a/hw/i2c/smbus_eeprom.c
+++ b/hw/i2c/smbus_eeprom.c
@@ -29,6 +29,8 @@ 
 
 //#define DEBUG
 
+#define TYPE_SMBUS_EEPROM_DEVICE "smbus-eeprom"
+
 typedef struct SMBusEEPROMDevice {
     SMBusDevice smbusdev;
     void *data;
@@ -97,6 +99,17 @@  static uint8_t eeprom_read_data(SMBusDevice *dev, uint8_t cmd, int n)
     return eeprom_receive_byte(dev);
 }
 
+static const VMStateDescription vmstate_smbus_eeprom = {
+    .name = TYPE_SMBUS_EEPROM_DEVICE,
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .fields      = (VMStateField[]) {
+        VMSTATE_SMBUS_DEVICE(smbusdev, SMBusEEPROMDevice),
+        VMSTATE_UINT8(offset, SMBusEEPROMDevice),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 static void smbus_eeprom_realize(DeviceState *dev, Error **errp)
 {
     SMBusEEPROMDevice *eeprom = (SMBusEEPROMDevice *)dev;
@@ -121,12 +134,13 @@  static void smbus_eeprom_class_initfn(ObjectClass *klass, void *data)
     sc->write_data = eeprom_write_data;
     sc->read_data = eeprom_read_data;
     dc->props = smbus_eeprom_properties;
+    dc->vmsd = &vmstate_smbus_eeprom;
     /* Reason: pointer property "data" */
     dc->user_creatable = false;
 }
 
 static const TypeInfo smbus_eeprom_info = {
-    .name          = "smbus-eeprom",
+    .name          = TYPE_SMBUS_EEPROM_DEVICE,
     .parent        = TYPE_SMBUS_DEVICE,
     .instance_size = sizeof(SMBusEEPROMDevice),
     .class_init    = smbus_eeprom_class_initfn,