[PATCH v42 13/98] hw/sd/sdcard: Add direct reference to SDProto in SDState

Philippe Mathieu-Daudé posted 98 patches 6 months ago
There is a newer version of this series
[PATCH v42 13/98] hw/sd/sdcard: Add direct reference to SDProto in SDState
Posted by Philippe Mathieu-Daudé 6 months ago
Keep direct reference to SDProto in SDState,
remove then unnecessary sd_proto().

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Tested-by: Cédric Le Goater <clg@redhat.com>
---
 hw/sd/sd.c | 37 +++++++++++++++++--------------------
 1 file changed, 17 insertions(+), 20 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 8f441e418c..aaa50ab2c5 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -116,6 +116,8 @@ struct SDState {
     uint8_t spec_version;
     BlockBackend *blk;
 
+    const SDProto *proto;
+
     /* Runtime changeables */
 
     uint32_t mode;    /* current card mode, one of SDCardModes */
@@ -154,18 +156,11 @@ struct SDState {
 
 static void sd_realize(DeviceState *dev, Error **errp);
 
-static const struct SDProto *sd_proto(SDState *sd)
-{
-    SDCardClass *sc = SD_CARD_GET_CLASS(sd);
-
-    return sc->proto;
-}
-
 static const SDProto sd_proto_spi;
 
 static bool sd_is_spi(SDState *sd)
 {
-    return sd_proto(sd) == &sd_proto_spi;
+    return sd->proto == &sd_proto_spi;
 }
 
 static const char *sd_version_str(enum SDPhySpecificationVersion version)
@@ -1044,7 +1039,7 @@ static bool address_in_range(SDState *sd, const char *desc,
 static sd_rsp_type_t sd_invalid_state_for_cmd(SDState *sd, SDRequest req)
 {
     qemu_log_mask(LOG_GUEST_ERROR, "%s: CMD%i in a wrong state: %s (spec %s)\n",
-                  sd_proto(sd)->name, req.cmd, sd_state_name(sd->state),
+                  sd->proto->name, req.cmd, sd_state_name(sd->state),
                   sd_version_str(sd->spec_version));
 
     return sd_illegal;
@@ -1053,7 +1048,7 @@ static sd_rsp_type_t sd_invalid_state_for_cmd(SDState *sd, SDRequest req)
 static sd_rsp_type_t sd_invalid_mode_for_cmd(SDState *sd, SDRequest req)
 {
     qemu_log_mask(LOG_GUEST_ERROR, "%s: CMD%i in a wrong mode: %s (spec %s)\n",
-                  sd_proto(sd)->name, req.cmd, sd_mode_name(sd->mode),
+                  sd->proto->name, req.cmd, sd_mode_name(sd->mode),
                   sd_version_str(sd->spec_version));
 
     return sd_illegal;
@@ -1062,7 +1057,7 @@ static sd_rsp_type_t sd_invalid_mode_for_cmd(SDState *sd, SDRequest req)
 static sd_rsp_type_t sd_cmd_illegal(SDState *sd, SDRequest req)
 {
     qemu_log_mask(LOG_GUEST_ERROR, "%s: Unknown CMD%i for spec %s\n",
-                  sd_proto(sd)->name, req.cmd,
+                  sd->proto->name, req.cmd,
                   sd_version_str(sd->spec_version));
 
     return sd_illegal;
@@ -1073,7 +1068,7 @@ __attribute__((unused))
 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(sd)->name, req.cmd);
+                  sd->proto->name, req.cmd);
 
     return sd_illegal;
 }
@@ -1166,7 +1161,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
      * However there is no ACMD55, so we want to trace this particular case.
      */
     if (req.cmd != 55 || sd->expecting_acmd) {
-        trace_sdcard_normal_command(sd_proto(sd)->name,
+        trace_sdcard_normal_command(sd->proto->name,
                                     sd->last_cmd_name, req.cmd,
                                     req.arg, sd_state_name(sd->state));
     }
@@ -1185,8 +1180,8 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
         return sd_illegal;
     }
 
-    if (sd_proto(sd)->cmd[req.cmd]) {
-        return sd_proto(sd)->cmd[req.cmd](sd, req);
+    if (sd->proto->cmd[req.cmd]) {
+        return sd->proto->cmd[req.cmd](sd, req);
     }
 
     switch (req.cmd) {
@@ -1632,12 +1627,12 @@ static sd_rsp_type_t sd_app_command(SDState *sd,
                                     SDRequest req)
 {
     sd->last_cmd_name = sd_acmd_name(req.cmd);
-    trace_sdcard_app_command(sd_proto(sd)->name, sd->last_cmd_name,
+    trace_sdcard_app_command(sd->proto->name, sd->last_cmd_name,
                              req.cmd, req.arg, sd_state_name(sd->state));
     sd->card_status |= APP_CMD;
 
-    if (sd_proto(sd)->acmd[req.cmd]) {
-        return sd_proto(sd)->acmd[req.cmd](sd, req);
+    if (sd->proto->acmd[req.cmd]) {
+        return sd->proto->acmd[req.cmd](sd, req);
     }
 
     switch (req.cmd) {
@@ -1928,7 +1923,7 @@ void sd_write_byte(SDState *sd, uint8_t value)
     if (sd->card_status & (ADDRESS_ERROR | WP_VIOLATION))
         return;
 
-    trace_sdcard_write_data(sd_proto(sd)->name,
+    trace_sdcard_write_data(sd->proto->name,
                             sd->last_cmd_name,
                             sd->current_cmd, sd->data_offset, value);
     switch (sd->current_cmd) {
@@ -2083,7 +2078,7 @@ uint8_t sd_read_byte(SDState *sd)
 
     io_len = (sd->ocr & (1 << 30)) ? 512 : sd->blk_len;
 
-    trace_sdcard_read_data(sd_proto(sd)->name,
+    trace_sdcard_read_data(sd->proto->name,
                            sd->last_cmd_name,
                            sd->current_cmd, sd->data_offset, io_len);
     switch (sd->current_cmd) {
@@ -2227,7 +2222,9 @@ static const SDProto sd_proto_sd = {
 static void sd_instance_init(Object *obj)
 {
     SDState *sd = SD_CARD(obj);
+    SDCardClass *sc = SD_CARD_GET_CLASS(sd);
 
+    sd->proto = sc->proto;
     sd->last_cmd_name = "UNSET";
     sd->enable = true;
     sd->ocr_power_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, sd_ocr_powerup, sd);
-- 
2.41.0


Re: [PATCH v42 13/98] hw/sd/sdcard: Add direct reference to SDProto in SDState
Posted by Cédric Le Goater 6 months ago
On 6/28/24 9:00 AM, Philippe Mathieu-Daudé wrote:
> Keep direct reference to SDProto in SDState,
> remove then unnecessary sd_proto().
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Tested-by: Cédric Le Goater <clg@redhat.com>


Reviewed-by: Cédric Le Goater <clg@redhat.com>

Thanks,

C.


> ---
>   hw/sd/sd.c | 37 +++++++++++++++++--------------------
>   1 file changed, 17 insertions(+), 20 deletions(-)
> 
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index 8f441e418c..aaa50ab2c5 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -116,6 +116,8 @@ struct SDState {
>       uint8_t spec_version;
>       BlockBackend *blk;
>   
> +    const SDProto *proto;
> +
>       /* Runtime changeables */
>   
>       uint32_t mode;    /* current card mode, one of SDCardModes */
> @@ -154,18 +156,11 @@ struct SDState {
>   
>   static void sd_realize(DeviceState *dev, Error **errp);
>   
> -static const struct SDProto *sd_proto(SDState *sd)
> -{
> -    SDCardClass *sc = SD_CARD_GET_CLASS(sd);
> -
> -    return sc->proto;
> -}
> -
>   static const SDProto sd_proto_spi;
>   
>   static bool sd_is_spi(SDState *sd)
>   {
> -    return sd_proto(sd) == &sd_proto_spi;
> +    return sd->proto == &sd_proto_spi;
>   }
>   
>   static const char *sd_version_str(enum SDPhySpecificationVersion version)
> @@ -1044,7 +1039,7 @@ static bool address_in_range(SDState *sd, const char *desc,
>   static sd_rsp_type_t sd_invalid_state_for_cmd(SDState *sd, SDRequest req)
>   {
>       qemu_log_mask(LOG_GUEST_ERROR, "%s: CMD%i in a wrong state: %s (spec %s)\n",
> -                  sd_proto(sd)->name, req.cmd, sd_state_name(sd->state),
> +                  sd->proto->name, req.cmd, sd_state_name(sd->state),
>                     sd_version_str(sd->spec_version));
>   
>       return sd_illegal;
> @@ -1053,7 +1048,7 @@ static sd_rsp_type_t sd_invalid_state_for_cmd(SDState *sd, SDRequest req)
>   static sd_rsp_type_t sd_invalid_mode_for_cmd(SDState *sd, SDRequest req)
>   {
>       qemu_log_mask(LOG_GUEST_ERROR, "%s: CMD%i in a wrong mode: %s (spec %s)\n",
> -                  sd_proto(sd)->name, req.cmd, sd_mode_name(sd->mode),
> +                  sd->proto->name, req.cmd, sd_mode_name(sd->mode),
>                     sd_version_str(sd->spec_version));
>   
>       return sd_illegal;
> @@ -1062,7 +1057,7 @@ static sd_rsp_type_t sd_invalid_mode_for_cmd(SDState *sd, SDRequest req)
>   static sd_rsp_type_t sd_cmd_illegal(SDState *sd, SDRequest req)
>   {
>       qemu_log_mask(LOG_GUEST_ERROR, "%s: Unknown CMD%i for spec %s\n",
> -                  sd_proto(sd)->name, req.cmd,
> +                  sd->proto->name, req.cmd,
>                     sd_version_str(sd->spec_version));
>   
>       return sd_illegal;
> @@ -1073,7 +1068,7 @@ __attribute__((unused))
>   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(sd)->name, req.cmd);
> +                  sd->proto->name, req.cmd);
>   
>       return sd_illegal;
>   }
> @@ -1166,7 +1161,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
>        * However there is no ACMD55, so we want to trace this particular case.
>        */
>       if (req.cmd != 55 || sd->expecting_acmd) {
> -        trace_sdcard_normal_command(sd_proto(sd)->name,
> +        trace_sdcard_normal_command(sd->proto->name,
>                                       sd->last_cmd_name, req.cmd,
>                                       req.arg, sd_state_name(sd->state));
>       }
> @@ -1185,8 +1180,8 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
>           return sd_illegal;
>       }
>   
> -    if (sd_proto(sd)->cmd[req.cmd]) {
> -        return sd_proto(sd)->cmd[req.cmd](sd, req);
> +    if (sd->proto->cmd[req.cmd]) {
> +        return sd->proto->cmd[req.cmd](sd, req);
>       }
>   
>       switch (req.cmd) {
> @@ -1632,12 +1627,12 @@ static sd_rsp_type_t sd_app_command(SDState *sd,
>                                       SDRequest req)
>   {
>       sd->last_cmd_name = sd_acmd_name(req.cmd);
> -    trace_sdcard_app_command(sd_proto(sd)->name, sd->last_cmd_name,
> +    trace_sdcard_app_command(sd->proto->name, sd->last_cmd_name,
>                                req.cmd, req.arg, sd_state_name(sd->state));
>       sd->card_status |= APP_CMD;
>   
> -    if (sd_proto(sd)->acmd[req.cmd]) {
> -        return sd_proto(sd)->acmd[req.cmd](sd, req);
> +    if (sd->proto->acmd[req.cmd]) {
> +        return sd->proto->acmd[req.cmd](sd, req);
>       }
>   
>       switch (req.cmd) {
> @@ -1928,7 +1923,7 @@ void sd_write_byte(SDState *sd, uint8_t value)
>       if (sd->card_status & (ADDRESS_ERROR | WP_VIOLATION))
>           return;
>   
> -    trace_sdcard_write_data(sd_proto(sd)->name,
> +    trace_sdcard_write_data(sd->proto->name,
>                               sd->last_cmd_name,
>                               sd->current_cmd, sd->data_offset, value);
>       switch (sd->current_cmd) {
> @@ -2083,7 +2078,7 @@ uint8_t sd_read_byte(SDState *sd)
>   
>       io_len = (sd->ocr & (1 << 30)) ? 512 : sd->blk_len;
>   
> -    trace_sdcard_read_data(sd_proto(sd)->name,
> +    trace_sdcard_read_data(sd->proto->name,
>                              sd->last_cmd_name,
>                              sd->current_cmd, sd->data_offset, io_len);
>       switch (sd->current_cmd) {
> @@ -2227,7 +2222,9 @@ static const SDProto sd_proto_sd = {
>   static void sd_instance_init(Object *obj)
>   {
>       SDState *sd = SD_CARD(obj);
> +    SDCardClass *sc = SD_CARD_GET_CLASS(sd);
>   
> +    sd->proto = sc->proto;
>       sd->last_cmd_name = "UNSET";
>       sd->enable = true;
>       sd->ocr_power_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, sd_ocr_powerup, sd);