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

[Cédric Le Goater]

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é <phi...@linaro.org>
Tested-by: Cédric Le Goater <c...@redhat.com>

Reviewed-by: Cédric Le Goater <c...@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);