* [Qemu-devel] [PATCH] hw/lan9118: Add save/load support
@ 2011-12-19 22:01 Peter Maydell
2012-01-04 10:41 ` Peter Maydell
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Peter Maydell @ 2011-12-19 22:01 UTC (permalink / raw)
To: qemu-devel; +Cc: patches
Implement save/load for the LAN9118.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
Does anybody have a nicer solution to the "can't put an enum in a
VMStateDescription" problem?
hw/lan9118.c | 126 +++++++++++++++++++++++++++++++++++++++++++++++-----------
1 files changed, 103 insertions(+), 23 deletions(-)
diff --git a/hw/lan9118.c b/hw/lan9118.c
index 7e64c5d..6542a26 100644
--- a/hw/lan9118.c
+++ b/hw/lan9118.c
@@ -136,17 +136,36 @@ enum tx_state {
};
typedef struct {
- enum tx_state state;
+ /* state is a tx_state but we can't put enums in VMStateDescriptions. */
+ uint32_t state;
uint32_t cmd_a;
uint32_t cmd_b;
- int buffer_size;
- int offset;
- int pad;
- int fifo_used;
- int len;
+ int32_t buffer_size;
+ int32_t offset;
+ int32_t pad;
+ int32_t fifo_used;
+ int32_t len;
uint8_t data[2048];
} LAN9118Packet;
+static const VMStateDescription vmstate_lan9118_packet = {
+ .name = "lan9118_packet",
+ .version_id = 1,
+ .minimum_version_id = 1,
+ .fields = (VMStateField[]) {
+ VMSTATE_UINT32(state, LAN9118Packet),
+ VMSTATE_UINT32(cmd_a, LAN9118Packet),
+ VMSTATE_UINT32(cmd_b, LAN9118Packet),
+ VMSTATE_INT32(buffer_size, LAN9118Packet),
+ VMSTATE_INT32(offset, LAN9118Packet),
+ VMSTATE_INT32(pad, LAN9118Packet),
+ VMSTATE_INT32(fifo_used, LAN9118Packet),
+ VMSTATE_INT32(len, LAN9118Packet),
+ VMSTATE_UINT8_ARRAY(data, LAN9118Packet, 2048),
+ VMSTATE_END_OF_LIST()
+ }
+};
+
typedef struct {
SysBusDevice busdev;
NICState *nic;
@@ -186,34 +205,95 @@ typedef struct {
uint32_t phy_int;
uint32_t phy_int_mask;
- int eeprom_writable;
+ int32_t eeprom_writable;
uint8_t eeprom[128];
- int tx_fifo_size;
+ int32_t tx_fifo_size;
LAN9118Packet *txp;
LAN9118Packet tx_packet;
- int tx_status_fifo_used;
- int tx_status_fifo_head;
+ int32_t tx_status_fifo_used;
+ int32_t tx_status_fifo_head;
uint32_t tx_status_fifo[512];
- int rx_status_fifo_size;
- int rx_status_fifo_used;
- int rx_status_fifo_head;
+ int32_t rx_status_fifo_size;
+ int32_t rx_status_fifo_used;
+ int32_t rx_status_fifo_head;
uint32_t rx_status_fifo[896];
- int rx_fifo_size;
- int rx_fifo_used;
- int rx_fifo_head;
+ int32_t rx_fifo_size;
+ int32_t rx_fifo_used;
+ int32_t rx_fifo_head;
uint32_t rx_fifo[3360];
- int rx_packet_size_head;
- int rx_packet_size_tail;
- int rx_packet_size[1024];
+ int32_t rx_packet_size_head;
+ int32_t rx_packet_size_tail;
+ int32_t rx_packet_size[1024];
- int rxp_offset;
- int rxp_size;
- int rxp_pad;
+ int32_t rxp_offset;
+ int32_t rxp_size;
+ int32_t rxp_pad;
} lan9118_state;
+static const VMStateDescription vmstate_lan9118 = {
+ .name = "lan9118",
+ .version_id = 1,
+ .minimum_version_id = 1,
+ .fields = (VMStateField[]) {
+ VMSTATE_PTIMER(timer, lan9118_state),
+ VMSTATE_UINT32(irq_cfg, lan9118_state),
+ VMSTATE_UINT32(int_sts, lan9118_state),
+ VMSTATE_UINT32(int_en, lan9118_state),
+ VMSTATE_UINT32(fifo_int, lan9118_state),
+ VMSTATE_UINT32(rx_cfg, lan9118_state),
+ VMSTATE_UINT32(tx_cfg, lan9118_state),
+ VMSTATE_UINT32(hw_cfg, lan9118_state),
+ VMSTATE_UINT32(pmt_ctrl, lan9118_state),
+ VMSTATE_UINT32(gpio_cfg, lan9118_state),
+ VMSTATE_UINT32(gpt_cfg, lan9118_state),
+ VMSTATE_UINT32(word_swap, lan9118_state),
+ VMSTATE_UINT32(free_timer_start, lan9118_state),
+ VMSTATE_UINT32(mac_cmd, lan9118_state),
+ VMSTATE_UINT32(mac_data, lan9118_state),
+ VMSTATE_UINT32(afc_cfg, lan9118_state),
+ VMSTATE_UINT32(e2p_cmd, lan9118_state),
+ VMSTATE_UINT32(e2p_data, lan9118_state),
+ VMSTATE_UINT32(mac_cr, lan9118_state),
+ VMSTATE_UINT32(mac_hashh, lan9118_state),
+ VMSTATE_UINT32(mac_hashl, lan9118_state),
+ VMSTATE_UINT32(mac_mii_acc, lan9118_state),
+ VMSTATE_UINT32(mac_mii_data, lan9118_state),
+ VMSTATE_UINT32(mac_flow, lan9118_state),
+ VMSTATE_UINT32(phy_status, lan9118_state),
+ VMSTATE_UINT32(phy_control, lan9118_state),
+ VMSTATE_UINT32(phy_advertise, lan9118_state),
+ VMSTATE_UINT32(phy_int, lan9118_state),
+ VMSTATE_UINT32(phy_int_mask, lan9118_state),
+ VMSTATE_INT32(eeprom_writable, lan9118_state),
+ VMSTATE_UINT8_ARRAY(eeprom, lan9118_state, 128),
+ VMSTATE_INT32(tx_fifo_size, lan9118_state),
+ /* txp always points at tx_packet so need not be saved */
+ VMSTATE_STRUCT(tx_packet, lan9118_state, 0,
+ vmstate_lan9118_packet, LAN9118Packet),
+ VMSTATE_INT32(tx_status_fifo_used, lan9118_state),
+ VMSTATE_INT32(tx_status_fifo_head, lan9118_state),
+ VMSTATE_UINT32_ARRAY(tx_status_fifo, lan9118_state, 512),
+ VMSTATE_INT32(rx_status_fifo_size, lan9118_state),
+ VMSTATE_INT32(rx_status_fifo_used, lan9118_state),
+ VMSTATE_INT32(rx_status_fifo_head, lan9118_state),
+ VMSTATE_UINT32_ARRAY(rx_status_fifo, lan9118_state, 896),
+ VMSTATE_INT32(rx_fifo_size, lan9118_state),
+ VMSTATE_INT32(rx_fifo_used, lan9118_state),
+ VMSTATE_INT32(rx_fifo_head, lan9118_state),
+ VMSTATE_UINT32_ARRAY(rx_fifo, lan9118_state, 3360),
+ VMSTATE_INT32(rx_packet_size_head, lan9118_state),
+ VMSTATE_INT32(rx_packet_size_tail, lan9118_state),
+ VMSTATE_INT32_ARRAY(rx_packet_size, lan9118_state, 1024),
+ VMSTATE_INT32(rxp_offset, lan9118_state),
+ VMSTATE_INT32(rxp_size, lan9118_state),
+ VMSTATE_INT32(rxp_pad, lan9118_state),
+ VMSTATE_END_OF_LIST()
+ }
+};
+
static void lan9118_update(lan9118_state *s)
{
int level;
@@ -1151,7 +1231,6 @@ static int lan9118_init1(SysBusDevice *dev)
ptimer_set_freq(s->timer, 10000);
ptimer_set_limit(s->timer, 0xffff, 1);
- /* ??? Save/restore. */
return 0;
}
@@ -1160,6 +1239,7 @@ static SysBusDeviceInfo lan9118_info = {
.qdev.name = "lan9118",
.qdev.size = sizeof(lan9118_state),
.qdev.reset = lan9118_reset,
+ .qdev.vmsd = &vmstate_lan9118,
.qdev.props = (Property[]) {
DEFINE_NIC_PROPERTIES(lan9118_state, conf),
DEFINE_PROP_END_OF_LIST(),
--
1.7.5.4
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] hw/lan9118: Add save/load support
2011-12-19 22:01 [Qemu-devel] [PATCH] hw/lan9118: Add save/load support Peter Maydell
@ 2012-01-04 10:41 ` Peter Maydell
2012-01-10 16:55 ` Peter Maydell
2012-01-12 4:23 ` Andreas Färber
2012-01-12 10:16 ` Mitsyanko Igor
2 siblings, 1 reply; 7+ messages in thread
From: Peter Maydell @ 2012-01-04 10:41 UTC (permalink / raw)
To: qemu-devel; +Cc: patches
Ping?
-- PMM
On 19 December 2011 22:01, Peter Maydell <peter.maydell@linaro.org> wrote:
> Implement save/load for the LAN9118.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> Does anybody have a nicer solution to the "can't put an enum in a
> VMStateDescription" problem?
>
> hw/lan9118.c | 126 +++++++++++++++++++++++++++++++++++++++++++++++-----------
> 1 files changed, 103 insertions(+), 23 deletions(-)
>
> diff --git a/hw/lan9118.c b/hw/lan9118.c
> index 7e64c5d..6542a26 100644
> --- a/hw/lan9118.c
> +++ b/hw/lan9118.c
> @@ -136,17 +136,36 @@ enum tx_state {
> };
>
> typedef struct {
> - enum tx_state state;
> + /* state is a tx_state but we can't put enums in VMStateDescriptions. */
> + uint32_t state;
> uint32_t cmd_a;
> uint32_t cmd_b;
> - int buffer_size;
> - int offset;
> - int pad;
> - int fifo_used;
> - int len;
> + int32_t buffer_size;
> + int32_t offset;
> + int32_t pad;
> + int32_t fifo_used;
> + int32_t len;
> uint8_t data[2048];
> } LAN9118Packet;
>
> +static const VMStateDescription vmstate_lan9118_packet = {
> + .name = "lan9118_packet",
> + .version_id = 1,
> + .minimum_version_id = 1,
> + .fields = (VMStateField[]) {
> + VMSTATE_UINT32(state, LAN9118Packet),
> + VMSTATE_UINT32(cmd_a, LAN9118Packet),
> + VMSTATE_UINT32(cmd_b, LAN9118Packet),
> + VMSTATE_INT32(buffer_size, LAN9118Packet),
> + VMSTATE_INT32(offset, LAN9118Packet),
> + VMSTATE_INT32(pad, LAN9118Packet),
> + VMSTATE_INT32(fifo_used, LAN9118Packet),
> + VMSTATE_INT32(len, LAN9118Packet),
> + VMSTATE_UINT8_ARRAY(data, LAN9118Packet, 2048),
> + VMSTATE_END_OF_LIST()
> + }
> +};
> +
> typedef struct {
> SysBusDevice busdev;
> NICState *nic;
> @@ -186,34 +205,95 @@ typedef struct {
> uint32_t phy_int;
> uint32_t phy_int_mask;
>
> - int eeprom_writable;
> + int32_t eeprom_writable;
> uint8_t eeprom[128];
>
> - int tx_fifo_size;
> + int32_t tx_fifo_size;
> LAN9118Packet *txp;
> LAN9118Packet tx_packet;
>
> - int tx_status_fifo_used;
> - int tx_status_fifo_head;
> + int32_t tx_status_fifo_used;
> + int32_t tx_status_fifo_head;
> uint32_t tx_status_fifo[512];
>
> - int rx_status_fifo_size;
> - int rx_status_fifo_used;
> - int rx_status_fifo_head;
> + int32_t rx_status_fifo_size;
> + int32_t rx_status_fifo_used;
> + int32_t rx_status_fifo_head;
> uint32_t rx_status_fifo[896];
> - int rx_fifo_size;
> - int rx_fifo_used;
> - int rx_fifo_head;
> + int32_t rx_fifo_size;
> + int32_t rx_fifo_used;
> + int32_t rx_fifo_head;
> uint32_t rx_fifo[3360];
> - int rx_packet_size_head;
> - int rx_packet_size_tail;
> - int rx_packet_size[1024];
> + int32_t rx_packet_size_head;
> + int32_t rx_packet_size_tail;
> + int32_t rx_packet_size[1024];
>
> - int rxp_offset;
> - int rxp_size;
> - int rxp_pad;
> + int32_t rxp_offset;
> + int32_t rxp_size;
> + int32_t rxp_pad;
> } lan9118_state;
>
> +static const VMStateDescription vmstate_lan9118 = {
> + .name = "lan9118",
> + .version_id = 1,
> + .minimum_version_id = 1,
> + .fields = (VMStateField[]) {
> + VMSTATE_PTIMER(timer, lan9118_state),
> + VMSTATE_UINT32(irq_cfg, lan9118_state),
> + VMSTATE_UINT32(int_sts, lan9118_state),
> + VMSTATE_UINT32(int_en, lan9118_state),
> + VMSTATE_UINT32(fifo_int, lan9118_state),
> + VMSTATE_UINT32(rx_cfg, lan9118_state),
> + VMSTATE_UINT32(tx_cfg, lan9118_state),
> + VMSTATE_UINT32(hw_cfg, lan9118_state),
> + VMSTATE_UINT32(pmt_ctrl, lan9118_state),
> + VMSTATE_UINT32(gpio_cfg, lan9118_state),
> + VMSTATE_UINT32(gpt_cfg, lan9118_state),
> + VMSTATE_UINT32(word_swap, lan9118_state),
> + VMSTATE_UINT32(free_timer_start, lan9118_state),
> + VMSTATE_UINT32(mac_cmd, lan9118_state),
> + VMSTATE_UINT32(mac_data, lan9118_state),
> + VMSTATE_UINT32(afc_cfg, lan9118_state),
> + VMSTATE_UINT32(e2p_cmd, lan9118_state),
> + VMSTATE_UINT32(e2p_data, lan9118_state),
> + VMSTATE_UINT32(mac_cr, lan9118_state),
> + VMSTATE_UINT32(mac_hashh, lan9118_state),
> + VMSTATE_UINT32(mac_hashl, lan9118_state),
> + VMSTATE_UINT32(mac_mii_acc, lan9118_state),
> + VMSTATE_UINT32(mac_mii_data, lan9118_state),
> + VMSTATE_UINT32(mac_flow, lan9118_state),
> + VMSTATE_UINT32(phy_status, lan9118_state),
> + VMSTATE_UINT32(phy_control, lan9118_state),
> + VMSTATE_UINT32(phy_advertise, lan9118_state),
> + VMSTATE_UINT32(phy_int, lan9118_state),
> + VMSTATE_UINT32(phy_int_mask, lan9118_state),
> + VMSTATE_INT32(eeprom_writable, lan9118_state),
> + VMSTATE_UINT8_ARRAY(eeprom, lan9118_state, 128),
> + VMSTATE_INT32(tx_fifo_size, lan9118_state),
> + /* txp always points at tx_packet so need not be saved */
> + VMSTATE_STRUCT(tx_packet, lan9118_state, 0,
> + vmstate_lan9118_packet, LAN9118Packet),
> + VMSTATE_INT32(tx_status_fifo_used, lan9118_state),
> + VMSTATE_INT32(tx_status_fifo_head, lan9118_state),
> + VMSTATE_UINT32_ARRAY(tx_status_fifo, lan9118_state, 512),
> + VMSTATE_INT32(rx_status_fifo_size, lan9118_state),
> + VMSTATE_INT32(rx_status_fifo_used, lan9118_state),
> + VMSTATE_INT32(rx_status_fifo_head, lan9118_state),
> + VMSTATE_UINT32_ARRAY(rx_status_fifo, lan9118_state, 896),
> + VMSTATE_INT32(rx_fifo_size, lan9118_state),
> + VMSTATE_INT32(rx_fifo_used, lan9118_state),
> + VMSTATE_INT32(rx_fifo_head, lan9118_state),
> + VMSTATE_UINT32_ARRAY(rx_fifo, lan9118_state, 3360),
> + VMSTATE_INT32(rx_packet_size_head, lan9118_state),
> + VMSTATE_INT32(rx_packet_size_tail, lan9118_state),
> + VMSTATE_INT32_ARRAY(rx_packet_size, lan9118_state, 1024),
> + VMSTATE_INT32(rxp_offset, lan9118_state),
> + VMSTATE_INT32(rxp_size, lan9118_state),
> + VMSTATE_INT32(rxp_pad, lan9118_state),
> + VMSTATE_END_OF_LIST()
> + }
> +};
> +
> static void lan9118_update(lan9118_state *s)
> {
> int level;
> @@ -1151,7 +1231,6 @@ static int lan9118_init1(SysBusDevice *dev)
> ptimer_set_freq(s->timer, 10000);
> ptimer_set_limit(s->timer, 0xffff, 1);
>
> - /* ??? Save/restore. */
> return 0;
> }
>
> @@ -1160,6 +1239,7 @@ static SysBusDeviceInfo lan9118_info = {
> .qdev.name = "lan9118",
> .qdev.size = sizeof(lan9118_state),
> .qdev.reset = lan9118_reset,
> + .qdev.vmsd = &vmstate_lan9118,
> .qdev.props = (Property[]) {
> DEFINE_NIC_PROPERTIES(lan9118_state, conf),
> DEFINE_PROP_END_OF_LIST(),
> --
> 1.7.5.4
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] hw/lan9118: Add save/load support
2012-01-04 10:41 ` Peter Maydell
@ 2012-01-10 16:55 ` Peter Maydell
2012-01-11 22:53 ` Peter Maydell
0 siblings, 1 reply; 7+ messages in thread
From: Peter Maydell @ 2012-01-10 16:55 UTC (permalink / raw)
To: qemu-devel; +Cc: patches
Ping^2 (slightly early but there's another patch on list which will
conflict so we ought to get one committed at least)
thanks
-- PMM
On 4 January 2012 10:41, Peter Maydell <peter.maydell@linaro.org> wrote:
> Ping?
>
> -- PMM
>
> On 19 December 2011 22:01, Peter Maydell <peter.maydell@linaro.org> wrote:
>> Implement save/load for the LAN9118.
>>
>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>> ---
>> Does anybody have a nicer solution to the "can't put an enum in a
>> VMStateDescription" problem?
>>
>> hw/lan9118.c | 126 +++++++++++++++++++++++++++++++++++++++++++++++-----------
>> 1 files changed, 103 insertions(+), 23 deletions(-)
>>
>> diff --git a/hw/lan9118.c b/hw/lan9118.c
>> index 7e64c5d..6542a26 100644
>> --- a/hw/lan9118.c
>> +++ b/hw/lan9118.c
>> @@ -136,17 +136,36 @@ enum tx_state {
>> };
>>
>> typedef struct {
>> - enum tx_state state;
>> + /* state is a tx_state but we can't put enums in VMStateDescriptions. */
>> + uint32_t state;
>> uint32_t cmd_a;
>> uint32_t cmd_b;
>> - int buffer_size;
>> - int offset;
>> - int pad;
>> - int fifo_used;
>> - int len;
>> + int32_t buffer_size;
>> + int32_t offset;
>> + int32_t pad;
>> + int32_t fifo_used;
>> + int32_t len;
>> uint8_t data[2048];
>> } LAN9118Packet;
>>
>> +static const VMStateDescription vmstate_lan9118_packet = {
>> + .name = "lan9118_packet",
>> + .version_id = 1,
>> + .minimum_version_id = 1,
>> + .fields = (VMStateField[]) {
>> + VMSTATE_UINT32(state, LAN9118Packet),
>> + VMSTATE_UINT32(cmd_a, LAN9118Packet),
>> + VMSTATE_UINT32(cmd_b, LAN9118Packet),
>> + VMSTATE_INT32(buffer_size, LAN9118Packet),
>> + VMSTATE_INT32(offset, LAN9118Packet),
>> + VMSTATE_INT32(pad, LAN9118Packet),
>> + VMSTATE_INT32(fifo_used, LAN9118Packet),
>> + VMSTATE_INT32(len, LAN9118Packet),
>> + VMSTATE_UINT8_ARRAY(data, LAN9118Packet, 2048),
>> + VMSTATE_END_OF_LIST()
>> + }
>> +};
>> +
>> typedef struct {
>> SysBusDevice busdev;
>> NICState *nic;
>> @@ -186,34 +205,95 @@ typedef struct {
>> uint32_t phy_int;
>> uint32_t phy_int_mask;
>>
>> - int eeprom_writable;
>> + int32_t eeprom_writable;
>> uint8_t eeprom[128];
>>
>> - int tx_fifo_size;
>> + int32_t tx_fifo_size;
>> LAN9118Packet *txp;
>> LAN9118Packet tx_packet;
>>
>> - int tx_status_fifo_used;
>> - int tx_status_fifo_head;
>> + int32_t tx_status_fifo_used;
>> + int32_t tx_status_fifo_head;
>> uint32_t tx_status_fifo[512];
>>
>> - int rx_status_fifo_size;
>> - int rx_status_fifo_used;
>> - int rx_status_fifo_head;
>> + int32_t rx_status_fifo_size;
>> + int32_t rx_status_fifo_used;
>> + int32_t rx_status_fifo_head;
>> uint32_t rx_status_fifo[896];
>> - int rx_fifo_size;
>> - int rx_fifo_used;
>> - int rx_fifo_head;
>> + int32_t rx_fifo_size;
>> + int32_t rx_fifo_used;
>> + int32_t rx_fifo_head;
>> uint32_t rx_fifo[3360];
>> - int rx_packet_size_head;
>> - int rx_packet_size_tail;
>> - int rx_packet_size[1024];
>> + int32_t rx_packet_size_head;
>> + int32_t rx_packet_size_tail;
>> + int32_t rx_packet_size[1024];
>>
>> - int rxp_offset;
>> - int rxp_size;
>> - int rxp_pad;
>> + int32_t rxp_offset;
>> + int32_t rxp_size;
>> + int32_t rxp_pad;
>> } lan9118_state;
>>
>> +static const VMStateDescription vmstate_lan9118 = {
>> + .name = "lan9118",
>> + .version_id = 1,
>> + .minimum_version_id = 1,
>> + .fields = (VMStateField[]) {
>> + VMSTATE_PTIMER(timer, lan9118_state),
>> + VMSTATE_UINT32(irq_cfg, lan9118_state),
>> + VMSTATE_UINT32(int_sts, lan9118_state),
>> + VMSTATE_UINT32(int_en, lan9118_state),
>> + VMSTATE_UINT32(fifo_int, lan9118_state),
>> + VMSTATE_UINT32(rx_cfg, lan9118_state),
>> + VMSTATE_UINT32(tx_cfg, lan9118_state),
>> + VMSTATE_UINT32(hw_cfg, lan9118_state),
>> + VMSTATE_UINT32(pmt_ctrl, lan9118_state),
>> + VMSTATE_UINT32(gpio_cfg, lan9118_state),
>> + VMSTATE_UINT32(gpt_cfg, lan9118_state),
>> + VMSTATE_UINT32(word_swap, lan9118_state),
>> + VMSTATE_UINT32(free_timer_start, lan9118_state),
>> + VMSTATE_UINT32(mac_cmd, lan9118_state),
>> + VMSTATE_UINT32(mac_data, lan9118_state),
>> + VMSTATE_UINT32(afc_cfg, lan9118_state),
>> + VMSTATE_UINT32(e2p_cmd, lan9118_state),
>> + VMSTATE_UINT32(e2p_data, lan9118_state),
>> + VMSTATE_UINT32(mac_cr, lan9118_state),
>> + VMSTATE_UINT32(mac_hashh, lan9118_state),
>> + VMSTATE_UINT32(mac_hashl, lan9118_state),
>> + VMSTATE_UINT32(mac_mii_acc, lan9118_state),
>> + VMSTATE_UINT32(mac_mii_data, lan9118_state),
>> + VMSTATE_UINT32(mac_flow, lan9118_state),
>> + VMSTATE_UINT32(phy_status, lan9118_state),
>> + VMSTATE_UINT32(phy_control, lan9118_state),
>> + VMSTATE_UINT32(phy_advertise, lan9118_state),
>> + VMSTATE_UINT32(phy_int, lan9118_state),
>> + VMSTATE_UINT32(phy_int_mask, lan9118_state),
>> + VMSTATE_INT32(eeprom_writable, lan9118_state),
>> + VMSTATE_UINT8_ARRAY(eeprom, lan9118_state, 128),
>> + VMSTATE_INT32(tx_fifo_size, lan9118_state),
>> + /* txp always points at tx_packet so need not be saved */
>> + VMSTATE_STRUCT(tx_packet, lan9118_state, 0,
>> + vmstate_lan9118_packet, LAN9118Packet),
>> + VMSTATE_INT32(tx_status_fifo_used, lan9118_state),
>> + VMSTATE_INT32(tx_status_fifo_head, lan9118_state),
>> + VMSTATE_UINT32_ARRAY(tx_status_fifo, lan9118_state, 512),
>> + VMSTATE_INT32(rx_status_fifo_size, lan9118_state),
>> + VMSTATE_INT32(rx_status_fifo_used, lan9118_state),
>> + VMSTATE_INT32(rx_status_fifo_head, lan9118_state),
>> + VMSTATE_UINT32_ARRAY(rx_status_fifo, lan9118_state, 896),
>> + VMSTATE_INT32(rx_fifo_size, lan9118_state),
>> + VMSTATE_INT32(rx_fifo_used, lan9118_state),
>> + VMSTATE_INT32(rx_fifo_head, lan9118_state),
>> + VMSTATE_UINT32_ARRAY(rx_fifo, lan9118_state, 3360),
>> + VMSTATE_INT32(rx_packet_size_head, lan9118_state),
>> + VMSTATE_INT32(rx_packet_size_tail, lan9118_state),
>> + VMSTATE_INT32_ARRAY(rx_packet_size, lan9118_state, 1024),
>> + VMSTATE_INT32(rxp_offset, lan9118_state),
>> + VMSTATE_INT32(rxp_size, lan9118_state),
>> + VMSTATE_INT32(rxp_pad, lan9118_state),
>> + VMSTATE_END_OF_LIST()
>> + }
>> +};
>> +
>> static void lan9118_update(lan9118_state *s)
>> {
>> int level;
>> @@ -1151,7 +1231,6 @@ static int lan9118_init1(SysBusDevice *dev)
>> ptimer_set_freq(s->timer, 10000);
>> ptimer_set_limit(s->timer, 0xffff, 1);
>>
>> - /* ??? Save/restore. */
>> return 0;
>> }
>>
>> @@ -1160,6 +1239,7 @@ static SysBusDeviceInfo lan9118_info = {
>> .qdev.name = "lan9118",
>> .qdev.size = sizeof(lan9118_state),
>> .qdev.reset = lan9118_reset,
>> + .qdev.vmsd = &vmstate_lan9118,
>> .qdev.props = (Property[]) {
>> DEFINE_NIC_PROPERTIES(lan9118_state, conf),
>> DEFINE_PROP_END_OF_LIST(),
>> --
>> 1.7.5.4
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] hw/lan9118: Add save/load support
2012-01-10 16:55 ` Peter Maydell
@ 2012-01-11 22:53 ` Peter Maydell
0 siblings, 0 replies; 7+ messages in thread
From: Peter Maydell @ 2012-01-11 22:53 UTC (permalink / raw)
To: qemu-devel; +Cc: patches
On consideration, unless somebody wishes to:
(a) commit this
(b) object
I'm going to put it in the next arm-devs pullreq I do. This is
reasonably justifiable since the only boards using lan9118 are the
ARM realview and vexpress devboard models.
-- PMM
On 10 January 2012 16:55, Peter Maydell <peter.maydell@linaro.org> wrote:
> Ping^2 (slightly early but there's another patch on list which will
> conflict so we ought to get one committed at least)
>
> thanks
> -- PMM
>
> On 4 January 2012 10:41, Peter Maydell <peter.maydell@linaro.org> wrote:
>> Ping?
>>
>> -- PMM
>>
>> On 19 December 2011 22:01, Peter Maydell <peter.maydell@linaro.org> wrote:
>>> Implement save/load for the LAN9118.
>>>
>>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>>> ---
>>> Does anybody have a nicer solution to the "can't put an enum in a
>>> VMStateDescription" problem?
>>>
>>> hw/lan9118.c | 126 +++++++++++++++++++++++++++++++++++++++++++++++-----------
>>> 1 files changed, 103 insertions(+), 23 deletions(-)
>>>
>>> diff --git a/hw/lan9118.c b/hw/lan9118.c
>>> index 7e64c5d..6542a26 100644
>>> --- a/hw/lan9118.c
>>> +++ b/hw/lan9118.c
>>> @@ -136,17 +136,36 @@ enum tx_state {
>>> };
>>>
>>> typedef struct {
>>> - enum tx_state state;
>>> + /* state is a tx_state but we can't put enums in VMStateDescriptions. */
>>> + uint32_t state;
>>> uint32_t cmd_a;
>>> uint32_t cmd_b;
>>> - int buffer_size;
>>> - int offset;
>>> - int pad;
>>> - int fifo_used;
>>> - int len;
>>> + int32_t buffer_size;
>>> + int32_t offset;
>>> + int32_t pad;
>>> + int32_t fifo_used;
>>> + int32_t len;
>>> uint8_t data[2048];
>>> } LAN9118Packet;
>>>
>>> +static const VMStateDescription vmstate_lan9118_packet = {
>>> + .name = "lan9118_packet",
>>> + .version_id = 1,
>>> + .minimum_version_id = 1,
>>> + .fields = (VMStateField[]) {
>>> + VMSTATE_UINT32(state, LAN9118Packet),
>>> + VMSTATE_UINT32(cmd_a, LAN9118Packet),
>>> + VMSTATE_UINT32(cmd_b, LAN9118Packet),
>>> + VMSTATE_INT32(buffer_size, LAN9118Packet),
>>> + VMSTATE_INT32(offset, LAN9118Packet),
>>> + VMSTATE_INT32(pad, LAN9118Packet),
>>> + VMSTATE_INT32(fifo_used, LAN9118Packet),
>>> + VMSTATE_INT32(len, LAN9118Packet),
>>> + VMSTATE_UINT8_ARRAY(data, LAN9118Packet, 2048),
>>> + VMSTATE_END_OF_LIST()
>>> + }
>>> +};
>>> +
>>> typedef struct {
>>> SysBusDevice busdev;
>>> NICState *nic;
>>> @@ -186,34 +205,95 @@ typedef struct {
>>> uint32_t phy_int;
>>> uint32_t phy_int_mask;
>>>
>>> - int eeprom_writable;
>>> + int32_t eeprom_writable;
>>> uint8_t eeprom[128];
>>>
>>> - int tx_fifo_size;
>>> + int32_t tx_fifo_size;
>>> LAN9118Packet *txp;
>>> LAN9118Packet tx_packet;
>>>
>>> - int tx_status_fifo_used;
>>> - int tx_status_fifo_head;
>>> + int32_t tx_status_fifo_used;
>>> + int32_t tx_status_fifo_head;
>>> uint32_t tx_status_fifo[512];
>>>
>>> - int rx_status_fifo_size;
>>> - int rx_status_fifo_used;
>>> - int rx_status_fifo_head;
>>> + int32_t rx_status_fifo_size;
>>> + int32_t rx_status_fifo_used;
>>> + int32_t rx_status_fifo_head;
>>> uint32_t rx_status_fifo[896];
>>> - int rx_fifo_size;
>>> - int rx_fifo_used;
>>> - int rx_fifo_head;
>>> + int32_t rx_fifo_size;
>>> + int32_t rx_fifo_used;
>>> + int32_t rx_fifo_head;
>>> uint32_t rx_fifo[3360];
>>> - int rx_packet_size_head;
>>> - int rx_packet_size_tail;
>>> - int rx_packet_size[1024];
>>> + int32_t rx_packet_size_head;
>>> + int32_t rx_packet_size_tail;
>>> + int32_t rx_packet_size[1024];
>>>
>>> - int rxp_offset;
>>> - int rxp_size;
>>> - int rxp_pad;
>>> + int32_t rxp_offset;
>>> + int32_t rxp_size;
>>> + int32_t rxp_pad;
>>> } lan9118_state;
>>>
>>> +static const VMStateDescription vmstate_lan9118 = {
>>> + .name = "lan9118",
>>> + .version_id = 1,
>>> + .minimum_version_id = 1,
>>> + .fields = (VMStateField[]) {
>>> + VMSTATE_PTIMER(timer, lan9118_state),
>>> + VMSTATE_UINT32(irq_cfg, lan9118_state),
>>> + VMSTATE_UINT32(int_sts, lan9118_state),
>>> + VMSTATE_UINT32(int_en, lan9118_state),
>>> + VMSTATE_UINT32(fifo_int, lan9118_state),
>>> + VMSTATE_UINT32(rx_cfg, lan9118_state),
>>> + VMSTATE_UINT32(tx_cfg, lan9118_state),
>>> + VMSTATE_UINT32(hw_cfg, lan9118_state),
>>> + VMSTATE_UINT32(pmt_ctrl, lan9118_state),
>>> + VMSTATE_UINT32(gpio_cfg, lan9118_state),
>>> + VMSTATE_UINT32(gpt_cfg, lan9118_state),
>>> + VMSTATE_UINT32(word_swap, lan9118_state),
>>> + VMSTATE_UINT32(free_timer_start, lan9118_state),
>>> + VMSTATE_UINT32(mac_cmd, lan9118_state),
>>> + VMSTATE_UINT32(mac_data, lan9118_state),
>>> + VMSTATE_UINT32(afc_cfg, lan9118_state),
>>> + VMSTATE_UINT32(e2p_cmd, lan9118_state),
>>> + VMSTATE_UINT32(e2p_data, lan9118_state),
>>> + VMSTATE_UINT32(mac_cr, lan9118_state),
>>> + VMSTATE_UINT32(mac_hashh, lan9118_state),
>>> + VMSTATE_UINT32(mac_hashl, lan9118_state),
>>> + VMSTATE_UINT32(mac_mii_acc, lan9118_state),
>>> + VMSTATE_UINT32(mac_mii_data, lan9118_state),
>>> + VMSTATE_UINT32(mac_flow, lan9118_state),
>>> + VMSTATE_UINT32(phy_status, lan9118_state),
>>> + VMSTATE_UINT32(phy_control, lan9118_state),
>>> + VMSTATE_UINT32(phy_advertise, lan9118_state),
>>> + VMSTATE_UINT32(phy_int, lan9118_state),
>>> + VMSTATE_UINT32(phy_int_mask, lan9118_state),
>>> + VMSTATE_INT32(eeprom_writable, lan9118_state),
>>> + VMSTATE_UINT8_ARRAY(eeprom, lan9118_state, 128),
>>> + VMSTATE_INT32(tx_fifo_size, lan9118_state),
>>> + /* txp always points at tx_packet so need not be saved */
>>> + VMSTATE_STRUCT(tx_packet, lan9118_state, 0,
>>> + vmstate_lan9118_packet, LAN9118Packet),
>>> + VMSTATE_INT32(tx_status_fifo_used, lan9118_state),
>>> + VMSTATE_INT32(tx_status_fifo_head, lan9118_state),
>>> + VMSTATE_UINT32_ARRAY(tx_status_fifo, lan9118_state, 512),
>>> + VMSTATE_INT32(rx_status_fifo_size, lan9118_state),
>>> + VMSTATE_INT32(rx_status_fifo_used, lan9118_state),
>>> + VMSTATE_INT32(rx_status_fifo_head, lan9118_state),
>>> + VMSTATE_UINT32_ARRAY(rx_status_fifo, lan9118_state, 896),
>>> + VMSTATE_INT32(rx_fifo_size, lan9118_state),
>>> + VMSTATE_INT32(rx_fifo_used, lan9118_state),
>>> + VMSTATE_INT32(rx_fifo_head, lan9118_state),
>>> + VMSTATE_UINT32_ARRAY(rx_fifo, lan9118_state, 3360),
>>> + VMSTATE_INT32(rx_packet_size_head, lan9118_state),
>>> + VMSTATE_INT32(rx_packet_size_tail, lan9118_state),
>>> + VMSTATE_INT32_ARRAY(rx_packet_size, lan9118_state, 1024),
>>> + VMSTATE_INT32(rxp_offset, lan9118_state),
>>> + VMSTATE_INT32(rxp_size, lan9118_state),
>>> + VMSTATE_INT32(rxp_pad, lan9118_state),
>>> + VMSTATE_END_OF_LIST()
>>> + }
>>> +};
>>> +
>>> static void lan9118_update(lan9118_state *s)
>>> {
>>> int level;
>>> @@ -1151,7 +1231,6 @@ static int lan9118_init1(SysBusDevice *dev)
>>> ptimer_set_freq(s->timer, 10000);
>>> ptimer_set_limit(s->timer, 0xffff, 1);
>>>
>>> - /* ??? Save/restore. */
>>> return 0;
>>> }
>>>
>>> @@ -1160,6 +1239,7 @@ static SysBusDeviceInfo lan9118_info = {
>>> .qdev.name = "lan9118",
>>> .qdev.size = sizeof(lan9118_state),
>>> .qdev.reset = lan9118_reset,
>>> + .qdev.vmsd = &vmstate_lan9118,
>>> .qdev.props = (Property[]) {
>>> DEFINE_NIC_PROPERTIES(lan9118_state, conf),
>>> DEFINE_PROP_END_OF_LIST(),
>>> --
>>> 1.7.5.4
--
12345678901234567890123456789012345678901234567890123456789012345678901234567890
1 2 3 4 5 6 7 8
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] hw/lan9118: Add save/load support
2011-12-19 22:01 [Qemu-devel] [PATCH] hw/lan9118: Add save/load support Peter Maydell
2012-01-04 10:41 ` Peter Maydell
@ 2012-01-12 4:23 ` Andreas Färber
2012-01-12 10:16 ` Mitsyanko Igor
2 siblings, 0 replies; 7+ messages in thread
From: Andreas Färber @ 2012-01-12 4:23 UTC (permalink / raw)
To: Peter Maydell; +Cc: qemu-devel, patches
Am 19.12.2011 23:01, schrieb Peter Maydell:
> Implement save/load for the LAN9118.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Andreas Färber <afaerber@suse.de>
The type conversions are okay. The VMState fields look sane on a short
look, didn't check for completeness though.
Andreas
> ---
> Does anybody have a nicer solution to the "can't put an enum in a
> VMStateDescription" problem?
>
> hw/lan9118.c | 126 +++++++++++++++++++++++++++++++++++++++++++++++-----------
> 1 files changed, 103 insertions(+), 23 deletions(-)
>
> diff --git a/hw/lan9118.c b/hw/lan9118.c
> index 7e64c5d..6542a26 100644
> --- a/hw/lan9118.c
> +++ b/hw/lan9118.c
> @@ -136,17 +136,36 @@ enum tx_state {
> };
>
> typedef struct {
> - enum tx_state state;
> + /* state is a tx_state but we can't put enums in VMStateDescriptions. */
> + uint32_t state;
> uint32_t cmd_a;
> uint32_t cmd_b;
> - int buffer_size;
> - int offset;
> - int pad;
> - int fifo_used;
> - int len;
> + int32_t buffer_size;
> + int32_t offset;
> + int32_t pad;
> + int32_t fifo_used;
> + int32_t len;
> uint8_t data[2048];
> } LAN9118Packet;
>
> +static const VMStateDescription vmstate_lan9118_packet = {
> + .name = "lan9118_packet",
> + .version_id = 1,
> + .minimum_version_id = 1,
> + .fields = (VMStateField[]) {
> + VMSTATE_UINT32(state, LAN9118Packet),
> + VMSTATE_UINT32(cmd_a, LAN9118Packet),
> + VMSTATE_UINT32(cmd_b, LAN9118Packet),
> + VMSTATE_INT32(buffer_size, LAN9118Packet),
> + VMSTATE_INT32(offset, LAN9118Packet),
> + VMSTATE_INT32(pad, LAN9118Packet),
> + VMSTATE_INT32(fifo_used, LAN9118Packet),
> + VMSTATE_INT32(len, LAN9118Packet),
> + VMSTATE_UINT8_ARRAY(data, LAN9118Packet, 2048),
> + VMSTATE_END_OF_LIST()
> + }
> +};
> +
> typedef struct {
> SysBusDevice busdev;
> NICState *nic;
> @@ -186,34 +205,95 @@ typedef struct {
> uint32_t phy_int;
> uint32_t phy_int_mask;
>
> - int eeprom_writable;
> + int32_t eeprom_writable;
> uint8_t eeprom[128];
>
> - int tx_fifo_size;
> + int32_t tx_fifo_size;
> LAN9118Packet *txp;
> LAN9118Packet tx_packet;
>
> - int tx_status_fifo_used;
> - int tx_status_fifo_head;
> + int32_t tx_status_fifo_used;
> + int32_t tx_status_fifo_head;
> uint32_t tx_status_fifo[512];
>
> - int rx_status_fifo_size;
> - int rx_status_fifo_used;
> - int rx_status_fifo_head;
> + int32_t rx_status_fifo_size;
> + int32_t rx_status_fifo_used;
> + int32_t rx_status_fifo_head;
> uint32_t rx_status_fifo[896];
> - int rx_fifo_size;
> - int rx_fifo_used;
> - int rx_fifo_head;
> + int32_t rx_fifo_size;
> + int32_t rx_fifo_used;
> + int32_t rx_fifo_head;
> uint32_t rx_fifo[3360];
> - int rx_packet_size_head;
> - int rx_packet_size_tail;
> - int rx_packet_size[1024];
> + int32_t rx_packet_size_head;
> + int32_t rx_packet_size_tail;
> + int32_t rx_packet_size[1024];
>
> - int rxp_offset;
> - int rxp_size;
> - int rxp_pad;
> + int32_t rxp_offset;
> + int32_t rxp_size;
> + int32_t rxp_pad;
> } lan9118_state;
>
> +static const VMStateDescription vmstate_lan9118 = {
> + .name = "lan9118",
> + .version_id = 1,
> + .minimum_version_id = 1,
> + .fields = (VMStateField[]) {
> + VMSTATE_PTIMER(timer, lan9118_state),
> + VMSTATE_UINT32(irq_cfg, lan9118_state),
> + VMSTATE_UINT32(int_sts, lan9118_state),
> + VMSTATE_UINT32(int_en, lan9118_state),
> + VMSTATE_UINT32(fifo_int, lan9118_state),
> + VMSTATE_UINT32(rx_cfg, lan9118_state),
> + VMSTATE_UINT32(tx_cfg, lan9118_state),
> + VMSTATE_UINT32(hw_cfg, lan9118_state),
> + VMSTATE_UINT32(pmt_ctrl, lan9118_state),
> + VMSTATE_UINT32(gpio_cfg, lan9118_state),
> + VMSTATE_UINT32(gpt_cfg, lan9118_state),
> + VMSTATE_UINT32(word_swap, lan9118_state),
> + VMSTATE_UINT32(free_timer_start, lan9118_state),
> + VMSTATE_UINT32(mac_cmd, lan9118_state),
> + VMSTATE_UINT32(mac_data, lan9118_state),
> + VMSTATE_UINT32(afc_cfg, lan9118_state),
> + VMSTATE_UINT32(e2p_cmd, lan9118_state),
> + VMSTATE_UINT32(e2p_data, lan9118_state),
> + VMSTATE_UINT32(mac_cr, lan9118_state),
> + VMSTATE_UINT32(mac_hashh, lan9118_state),
> + VMSTATE_UINT32(mac_hashl, lan9118_state),
> + VMSTATE_UINT32(mac_mii_acc, lan9118_state),
> + VMSTATE_UINT32(mac_mii_data, lan9118_state),
> + VMSTATE_UINT32(mac_flow, lan9118_state),
> + VMSTATE_UINT32(phy_status, lan9118_state),
> + VMSTATE_UINT32(phy_control, lan9118_state),
> + VMSTATE_UINT32(phy_advertise, lan9118_state),
> + VMSTATE_UINT32(phy_int, lan9118_state),
> + VMSTATE_UINT32(phy_int_mask, lan9118_state),
> + VMSTATE_INT32(eeprom_writable, lan9118_state),
> + VMSTATE_UINT8_ARRAY(eeprom, lan9118_state, 128),
> + VMSTATE_INT32(tx_fifo_size, lan9118_state),
> + /* txp always points at tx_packet so need not be saved */
> + VMSTATE_STRUCT(tx_packet, lan9118_state, 0,
> + vmstate_lan9118_packet, LAN9118Packet),
> + VMSTATE_INT32(tx_status_fifo_used, lan9118_state),
> + VMSTATE_INT32(tx_status_fifo_head, lan9118_state),
> + VMSTATE_UINT32_ARRAY(tx_status_fifo, lan9118_state, 512),
> + VMSTATE_INT32(rx_status_fifo_size, lan9118_state),
> + VMSTATE_INT32(rx_status_fifo_used, lan9118_state),
> + VMSTATE_INT32(rx_status_fifo_head, lan9118_state),
> + VMSTATE_UINT32_ARRAY(rx_status_fifo, lan9118_state, 896),
> + VMSTATE_INT32(rx_fifo_size, lan9118_state),
> + VMSTATE_INT32(rx_fifo_used, lan9118_state),
> + VMSTATE_INT32(rx_fifo_head, lan9118_state),
> + VMSTATE_UINT32_ARRAY(rx_fifo, lan9118_state, 3360),
> + VMSTATE_INT32(rx_packet_size_head, lan9118_state),
> + VMSTATE_INT32(rx_packet_size_tail, lan9118_state),
> + VMSTATE_INT32_ARRAY(rx_packet_size, lan9118_state, 1024),
> + VMSTATE_INT32(rxp_offset, lan9118_state),
> + VMSTATE_INT32(rxp_size, lan9118_state),
> + VMSTATE_INT32(rxp_pad, lan9118_state),
> + VMSTATE_END_OF_LIST()
> + }
> +};
> +
> static void lan9118_update(lan9118_state *s)
> {
> int level;
> @@ -1151,7 +1231,6 @@ static int lan9118_init1(SysBusDevice *dev)
> ptimer_set_freq(s->timer, 10000);
> ptimer_set_limit(s->timer, 0xffff, 1);
>
> - /* ??? Save/restore. */
> return 0;
> }
>
> @@ -1160,6 +1239,7 @@ static SysBusDeviceInfo lan9118_info = {
> .qdev.name = "lan9118",
> .qdev.size = sizeof(lan9118_state),
> .qdev.reset = lan9118_reset,
> + .qdev.vmsd = &vmstate_lan9118,
> .qdev.props = (Property[]) {
> DEFINE_NIC_PROPERTIES(lan9118_state, conf),
> DEFINE_PROP_END_OF_LIST(),
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] hw/lan9118: Add save/load support
2011-12-19 22:01 [Qemu-devel] [PATCH] hw/lan9118: Add save/load support Peter Maydell
2012-01-04 10:41 ` Peter Maydell
2012-01-12 4:23 ` Andreas Färber
@ 2012-01-12 10:16 ` Mitsyanko Igor
2012-01-12 10:18 ` Peter Maydell
2 siblings, 1 reply; 7+ messages in thread
From: Mitsyanko Igor @ 2012-01-12 10:16 UTC (permalink / raw)
To: Peter Maydell; +Cc: qemu-devel, patches
On 12/20/2011 02:01 AM, Peter Maydell wrote:
> Implement save/load for the LAN9118.
>
> Signed-off-by: Peter Maydell<peter.maydell@linaro.org>
> ---
> Does anybody have a nicer solution to the "can't put an enum in a
> VMStateDescription" problem?
>
> hw/lan9118.c | 126 +++++++++++++++++++++++++++++++++++++++++++++++-----------
> 1 files changed, 103 insertions(+), 23 deletions(-)
>
> diff --git a/hw/lan9118.c b/hw/lan9118.c
> index 7e64c5d..6542a26 100644
> --- a/hw/lan9118.c
> +++ b/hw/lan9118.c
> @@ -136,17 +136,36 @@ enum tx_state {
> };
Hi Peter, I have a small question. Wouldn't it be better to use uint32_t
for variables that actually have positive values only? That would make
code easier to understand. For example in this state
> typedef struct {
> - enum tx_state state;
> + /* state is a tx_state but we can't put enums in VMStateDescriptions. */
> + uint32_t state;
> uint32_t cmd_a;
> uint32_t cmd_b;
> - int buffer_size;
> - int offset;
> - int pad;
> - int fifo_used;
> - int len;
> + int32_t buffer_size;
> + int32_t offset;
> + int32_t pad;
> + int32_t fifo_used;
> + int32_t len;
> uint8_t data[2048];
> } LAN9118Packet;
all member variables (except maybe buffer_size) can have positive values
only. buffer_size is probably positive-only too because this code
while (n--) {
s->txp->data[s->txp->len] = val & 0xff;
s->txp->len++;
val >>= 8;
s->txp->buffer_size--;
}
looks wrong (I haven't dug dip into it though) and must be
...
while (s->txp->buffer_size && (n--)) {
...
--
Mitsyanko Igor
ASWG, Moscow R&D center, Samsung Electronics
email: i.mitsyanko@samsung.com
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] hw/lan9118: Add save/load support
2012-01-12 10:16 ` Mitsyanko Igor
@ 2012-01-12 10:18 ` Peter Maydell
0 siblings, 0 replies; 7+ messages in thread
From: Peter Maydell @ 2012-01-12 10:18 UTC (permalink / raw)
To: i.mitsyanko; +Cc: qemu-devel, patches
On 12 January 2012 10:16, Mitsyanko Igor <i.mitsyanko@samsung.com> wrote:
> Hi Peter, I have a small question. Wouldn't it be better to use uint32_t for
> variables that actually have positive values only? That would make code
> easier to understand.
The idea is to do a straightforward and easy to review conversion.
Switching from signed to unsigned changes semantics of the variable
in a way that requires review of all the code that used that variable,
so it's riskier.
-- PMM
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2012-01-12 10:18 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-12-19 22:01 [Qemu-devel] [PATCH] hw/lan9118: Add save/load support Peter Maydell
2012-01-04 10:41 ` Peter Maydell
2012-01-10 16:55 ` Peter Maydell
2012-01-11 22:53 ` Peter Maydell
2012-01-12 4:23 ` Andreas Färber
2012-01-12 10:16 ` Mitsyanko Igor
2012-01-12 10:18 ` Peter Maydell
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).