| Message ID | 20210624142209.1193073-7-f4bug@amsat.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | hw/sd: Start splitting SD vs SPI protocols | expand |
On Thu, Jun 24, 2021 at 10:28 PM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote: > > Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > --- > hw/sd/sd.c | 21 ++++++++++++--------- > 1 file changed, 12 insertions(+), 9 deletions(-) > > diff --git a/hw/sd/sd.c b/hw/sd/sd.c > index 0215bdb3689..2647fd26566 100644 > --- a/hw/sd/sd.c > +++ b/hw/sd/sd.c > @@ -973,6 +973,15 @@ static sd_rsp_type_t sd_cmd_illegal(SDState *sd, SDRequest req) > return sd_illegal; > } > > +/* Commands that are recognised but not yet implemented. */ > +static sd_rsp_type_t sd_cmd_unimplemented(SDState *sd, SDRequest req) > +{ > + qemu_log_mask(LOG_UNIMP, "%s: CMD%i not implemented\n", > + sd->proto->name, req.cmd); > + > + return sd_illegal; > +} > + > static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req) > { > uint32_t rca = 0x0000; > @@ -1522,9 +1531,6 @@ static sd_rsp_type_t sd_app_command(SDState *sd, > > switch (req.cmd) { > case 6: /* ACMD6: SET_BUS_WIDTH */ > - if (sd->spi) { > - goto unimplemented_spi_cmd; > - } > switch (sd->state) { > case sd_transfer_state: > sd->sd_status[0] &= 0x3f; > @@ -1655,12 +1661,6 @@ static sd_rsp_type_t sd_app_command(SDState *sd, > default: > /* Fall back to standard commands. */ > return sd_normal_command(sd, req); > - > - unimplemented_spi_cmd: > - /* Commands that are recognised but not yet implemented in SPI mode. */ > - qemu_log_mask(LOG_UNIMP, "SD: CMD%i not implemented in SPI mode\n", > - req.cmd); > - return sd_illegal; > } > > qemu_log_mask(LOG_GUEST_ERROR, "SD: ACMD%i in a wrong state\n", req.cmd); > @@ -2096,6 +2096,9 @@ static const SDProto sd_proto_spi = { > [26] = sd_cmd_illegal, > [52 ... 54] = sd_cmd_illegal, > }, > + .cmd = { > + [6] = sd_cmd_unimplemented, > + }, > }; Does this compile? Or is this another GCC extension to C? But I think you wanted to say .acmd = ? Regards, Bin
On 6/25/21 3:49 PM, Bin Meng wrote: > On Thu, Jun 24, 2021 at 10:28 PM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote: >> >> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> >> --- >> hw/sd/sd.c | 21 ++++++++++++--------- >> 1 file changed, 12 insertions(+), 9 deletions(-) >> qemu_log_mask(LOG_GUEST_ERROR, "SD: ACMD%i in a wrong state\n", req.cmd); >> @@ -2096,6 +2096,9 @@ static const SDProto sd_proto_spi = { >> [26] = sd_cmd_illegal, >> [52 ... 54] = sd_cmd_illegal, >> }, >> + .cmd = { >> + [6] = sd_cmd_unimplemented, >> + }, >> }; > > Does this compile? Yes. > Or is this another GCC extension to C? I haven't checked when this was introduced, but QEMU uses it since quite some time now. Apparently this is: https://gcc.gnu.org/onlinedocs/gcc/Designated-Inits.html "In ISO C99 you can give the elements in any order, specifying the array indices or structure field names they apply to, and GNU C allows this as an extension in C90 mode as well." > But I think you wanted to say .acmd = ? Oops! Thanks for the review, Phil.
On Sat, Jun 26, 2021 at 1:17 AM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote: > > On 6/25/21 3:49 PM, Bin Meng wrote: > > On Thu, Jun 24, 2021 at 10:28 PM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote: > >> > >> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > >> --- > >> hw/sd/sd.c | 21 ++++++++++++--------- > >> 1 file changed, 12 insertions(+), 9 deletions(-) > > >> qemu_log_mask(LOG_GUEST_ERROR, "SD: ACMD%i in a wrong state\n", req.cmd); > >> @@ -2096,6 +2096,9 @@ static const SDProto sd_proto_spi = { > >> [26] = sd_cmd_illegal, > >> [52 ... 54] = sd_cmd_illegal, > >> }, > >> + .cmd = { > >> + [6] = sd_cmd_unimplemented, > >> + }, > >> }; > > > > Does this compile? > > Yes. > > > Or is this another GCC extension to C? > > I haven't checked when this was introduced, but QEMU uses it since > quite some time now. > > Apparently this is: > https://gcc.gnu.org/onlinedocs/gcc/Designated-Inits.html Yep, I know designated initialization of a C array, but I don't know gcc does not complain two .cmd here > > "In ISO C99 you can give the elements in any order, specifying > the array indices or structure field names they apply to, and > GNU C allows this as an extension in C90 mode as well." > > > But I think you wanted to say .acmd = ? > > Oops! > > Thanks for the review, Regards, Bin
On 6/26/21 5:31 AM, Bin Meng wrote: > On Sat, Jun 26, 2021 at 1:17 AM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote: >> >> On 6/25/21 3:49 PM, Bin Meng wrote: >>> On Thu, Jun 24, 2021 at 10:28 PM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote: >>>> >>>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> >>>> --- >>>> hw/sd/sd.c | 21 ++++++++++++--------- >>>> 1 file changed, 12 insertions(+), 9 deletions(-) >> >>>> qemu_log_mask(LOG_GUEST_ERROR, "SD: ACMD%i in a wrong state\n", req.cmd); >>>> @@ -2096,6 +2096,9 @@ static const SDProto sd_proto_spi = { >>>> [26] = sd_cmd_illegal, >>>> [52 ... 54] = sd_cmd_illegal, >>>> }, >>>> + .cmd = { >>>> + [6] = sd_cmd_unimplemented, >>>> + }, >>>> }; >>> >>> Does this compile? >> >> Yes. >> >>> Or is this another GCC extension to C? >> >> I haven't checked when this was introduced, but QEMU uses it since >> quite some time now. >> >> Apparently this is: >> https://gcc.gnu.org/onlinedocs/gcc/Designated-Inits.html > > Yep, I know designated initialization of a C array, but I don't know > gcc does not complain two .cmd here IIUC GCC would warn if we were using -Woverride-init (but we are not): -Woverride-init (C and Objective-C only) Warn if an initialized field without side effects is overridden when using designated initializers. >> "In ISO C99 you can give the elements in any order, specifying >> the array indices or structure field names they apply to, and >> GNU C allows this as an extension in C90 mode as well." >> >>> But I think you wanted to say .acmd = ? >> >> Oops! >> >> Thanks for the review, > > Regards, > Bin >
diff --git a/hw/sd/sd.c b/hw/sd/sd.c index 0215bdb3689..2647fd26566 100644 --- a/hw/sd/sd.c +++ b/hw/sd/sd.c @@ -973,6 +973,15 @@ static sd_rsp_type_t sd_cmd_illegal(SDState *sd, SDRequest req) return sd_illegal; } +/* Commands that are recognised but not yet implemented. */ +static sd_rsp_type_t sd_cmd_unimplemented(SDState *sd, SDRequest req) +{ + qemu_log_mask(LOG_UNIMP, "%s: CMD%i not implemented\n", + sd->proto->name, req.cmd); + + return sd_illegal; +} + static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req) { uint32_t rca = 0x0000; @@ -1522,9 +1531,6 @@ static sd_rsp_type_t sd_app_command(SDState *sd, switch (req.cmd) { case 6: /* ACMD6: SET_BUS_WIDTH */ - if (sd->spi) { - goto unimplemented_spi_cmd; - } switch (sd->state) { case sd_transfer_state: sd->sd_status[0] &= 0x3f; @@ -1655,12 +1661,6 @@ static sd_rsp_type_t sd_app_command(SDState *sd, default: /* Fall back to standard commands. */ return sd_normal_command(sd, req); - - unimplemented_spi_cmd: - /* Commands that are recognised but not yet implemented in SPI mode. */ - qemu_log_mask(LOG_UNIMP, "SD: CMD%i not implemented in SPI mode\n", - req.cmd); - return sd_illegal; } qemu_log_mask(LOG_GUEST_ERROR, "SD: ACMD%i in a wrong state\n", req.cmd); @@ -2096,6 +2096,9 @@ static const SDProto sd_proto_spi = { [26] = sd_cmd_illegal, [52 ... 54] = sd_cmd_illegal, }, + .cmd = { + [6] = sd_cmd_unimplemented, + }, }; static const SDProto sd_proto_sd = {
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> --- hw/sd/sd.c | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-)