| Message ID | 20190505230140.5661-1-philmd@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | hw/sd/sdcard: Use the available enums | expand |
Philippe Mathieu-Daudé <philmd@redhat.com> writes: > We already define SDCardModes/SDCardStates as enums. Declare > the mode/state as enums too, this make gdb debugging sessions > friendlier: instead of numbers, the mode/state name is displayed. > > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> > --- > hw/sd/sd.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/hw/sd/sd.c b/hw/sd/sd.c > index aaab15f3868..a66b3d5b45e 100644 > --- a/hw/sd/sd.c > +++ b/hw/sd/sd.c > @@ -96,8 +96,8 @@ struct SDState { > BlockBackend *blk; > bool spi; > > - uint32_t mode; /* current card mode, one of SDCardModes */ > - int32_t state; /* current card state, one of SDCardStates */ > + enum SDCardModes mode; > + enum SDCardStates state; > uint32_t vhs; > bool wp_switch; > unsigned long *wp_groups; > @@ -1640,7 +1640,7 @@ static int cmd_valid_while_locked(SDState *sd, SDRequest *req) > > int sd_do_command(SDState *sd, SDRequest *req, > uint8_t *response) { > - int last_state; > + enum SDCardStates last_state; > sd_rsp_type_t rtype; > int rsplen; These guys are part of the migration state: static const VMStateDescription sd_vmstate = { .name = "sd-card", .version_id = 1, .minimum_version_id = 1, .pre_load = sd_vmstate_pre_load, .fields = (VMStateField[]) { VMSTATE_UINT32(mode, SDState), VMSTATE_INT32(state, SDState), [...] Juan, David, are VMSTATE_UINT32() and VMSTATE_INT32() safe to use with enums?
On 5/6/19 10:55 AM, Markus Armbruster wrote: > Philippe Mathieu-Daudé <philmd@redhat.com> writes: > >> We already define SDCardModes/SDCardStates as enums. Declare >> the mode/state as enums too, this make gdb debugging sessions >> friendlier: instead of numbers, the mode/state name is displayed. >> >> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> >> --- >> hw/sd/sd.c | 6 +++--- >> 1 file changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/hw/sd/sd.c b/hw/sd/sd.c >> index aaab15f3868..a66b3d5b45e 100644 >> --- a/hw/sd/sd.c >> +++ b/hw/sd/sd.c >> @@ -96,8 +96,8 @@ struct SDState { >> BlockBackend *blk; >> bool spi; >> >> - uint32_t mode; /* current card mode, one of SDCardModes */ >> - int32_t state; /* current card state, one of SDCardStates */ >> + enum SDCardModes mode; >> + enum SDCardStates state; >> uint32_t vhs; >> bool wp_switch; >> unsigned long *wp_groups; >> @@ -1640,7 +1640,7 @@ static int cmd_valid_while_locked(SDState *sd, SDRequest *req) >> >> int sd_do_command(SDState *sd, SDRequest *req, >> uint8_t *response) { >> - int last_state; >> + enum SDCardStates last_state; >> sd_rsp_type_t rtype; >> int rsplen; > > These guys are part of the migration state: > > static const VMStateDescription sd_vmstate = { > .name = "sd-card", > .version_id = 1, > .minimum_version_id = 1, > .pre_load = sd_vmstate_pre_load, > .fields = (VMStateField[]) { > VMSTATE_UINT32(mode, SDState), > VMSTATE_INT32(state, SDState), > [...] Oh good point, I missed that. > Juan, David, are VMSTATE_UINT32() and VMSTATE_INT32() safe to use with > enums? I'll wait Juan/Dave to enlighten us, else I can use anonymous union to ease my debugging sessions: -- >8 -- diff --git a/hw/sd/sd.c b/hw/sd/sd.c @@ -96,7 +96,10 @@ struct SDState { BlockBackend *blk; bool spi; - uint32_t mode; /* current card mode, one of SDCardModes */ + union { + uint32_t migratable_mode; + enum SDCardModes mode; + }; int32_t state; /* current card state, one of SDCardStates */ uint32_t vhs; bool wp_switch; @@ -659,7 +662,7 @@ static const VMStateDescription sd_vmstate = { .minimum_version_id = 1, .pre_load = sd_vmstate_pre_load, .fields = (VMStateField[]) { - VMSTATE_UINT32(mode, SDState), + VMSTATE_UINT32(migratable_mode, SDState), VMSTATE_INT32(state, SDState), VMSTATE_UINT8_ARRAY(cid, SDState, 16), VMSTATE_UINT8_ARRAY(csd, SDState, 16), --- Thanks for the review! Phil.
* Markus Armbruster (armbru@redhat.com) wrote: > Philippe Mathieu-Daudé <philmd@redhat.com> writes: > > > We already define SDCardModes/SDCardStates as enums. Declare > > the mode/state as enums too, this make gdb debugging sessions > > friendlier: instead of numbers, the mode/state name is displayed. > > > > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> > > --- > > hw/sd/sd.c | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/hw/sd/sd.c b/hw/sd/sd.c > > index aaab15f3868..a66b3d5b45e 100644 > > --- a/hw/sd/sd.c > > +++ b/hw/sd/sd.c > > @@ -96,8 +96,8 @@ struct SDState { > > BlockBackend *blk; > > bool spi; > > > > - uint32_t mode; /* current card mode, one of SDCardModes */ > > - int32_t state; /* current card state, one of SDCardStates */ > > + enum SDCardModes mode; > > + enum SDCardStates state; > > uint32_t vhs; > > bool wp_switch; > > unsigned long *wp_groups; > > @@ -1640,7 +1640,7 @@ static int cmd_valid_while_locked(SDState *sd, SDRequest *req) > > > > int sd_do_command(SDState *sd, SDRequest *req, > > uint8_t *response) { > > - int last_state; > > + enum SDCardStates last_state; > > sd_rsp_type_t rtype; > > int rsplen; > > These guys are part of the migration state: > > static const VMStateDescription sd_vmstate = { > .name = "sd-card", > .version_id = 1, > .minimum_version_id = 1, > .pre_load = sd_vmstate_pre_load, > .fields = (VMStateField[]) { > VMSTATE_UINT32(mode, SDState), > VMSTATE_INT32(state, SDState), > [...] > > Juan, David, are VMSTATE_UINT32() and VMSTATE_INT32() safe to use with > enums? I think that is compiler dependent, so no. Dave -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
diff --git a/hw/sd/sd.c b/hw/sd/sd.c index aaab15f3868..a66b3d5b45e 100644 --- a/hw/sd/sd.c +++ b/hw/sd/sd.c @@ -96,8 +96,8 @@ struct SDState { BlockBackend *blk; bool spi; - uint32_t mode; /* current card mode, one of SDCardModes */ - int32_t state; /* current card state, one of SDCardStates */ + enum SDCardModes mode; + enum SDCardStates state; uint32_t vhs; bool wp_switch; unsigned long *wp_groups; @@ -1640,7 +1640,7 @@ static int cmd_valid_while_locked(SDState *sd, SDRequest *req) int sd_do_command(SDState *sd, SDRequest *req, uint8_t *response) { - int last_state; + enum SDCardStates last_state; sd_rsp_type_t rtype; int rsplen;
We already define SDCardModes/SDCardStates as enums. Declare the mode/state as enums too, this make gdb debugging sessions friendlier: instead of numbers, the mode/state name is displayed. Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> --- hw/sd/sd.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)