Re: [Qemu-devel] [PATCH v3 12/12] sdcard: use the registerfields API to access the OCR register

[Alistair Francis]

On Mon, Jan 22, 2018 at 7:21 PM, Philippe Mathieu-Daudé <address@hidden> wrote:
> Signed-off-by: Philippe Mathieu-Daudé <address@hidden>

Reviewed-by: Alistair Francis <address@hidden>

Alistair

> ---
>  include/hw/sd/sd.h |  1 -
>  hw/sd/sd.c         | 21 +++++++++++++--------
>  2 files changed, 13 insertions(+), 9 deletions(-)
>
> diff --git a/include/hw/sd/sd.h b/include/hw/sd/sd.h
> index bf1eb0713c..9bdb3c9285 100644
> --- a/include/hw/sd/sd.h
> +++ b/include/hw/sd/sd.h
> @@ -53,7 +53,6 @@
>  #define READY_FOR_DATA         (1 << 8)
>  #define APP_CMD                        (1 << 5)
>  #define AKE_SEQ_ERROR          (1 << 3)
> -#define OCR_CCS_BITN        30
>
>  typedef enum {
>      SD_VOLTAGE_0_4V     = 400,  /* currently not supported */
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index f87e543f8f..437ce25f79 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -32,6 +32,7 @@
>  #include "qemu/osdep.h"
>  #include "hw/qdev.h"
>  #include "hw/hw.h"
> +#include "hw/registerfields.h"
>  #include "sysemu/block-backend.h"
>  #include "hw/sd/sd.h"
>  #include "qapi/error.h"
> @@ -47,8 +48,6 @@
>  //#define DEBUG_SD 1
>
>  #define ACMD41_ENQUIRY_MASK     0x00ffffff
> -#define OCR_POWER_UP            0x80000000
> -#define OCR_POWER_DELAY_NS      500000 /* 0.5ms */
>
>  typedef enum {
>      sd_r0 = 0,    /* no response */
> @@ -270,6 +269,11 @@ static uint16_t sd_crc16(void *message, size_t width)
>      return shift_reg;
>  }
>
> +#define OCR_POWER_DELAY_NS      500000 /* 0.5ms */
> +
> +FIELD(OCR, CARD_CAPACITY,              30,  1) /* 0:SDSC, 1:SDHC/SDXC */
> +FIELD(OCR, CARD_POWER_UP,              31,  1)
> +
>  static void sd_reset_ocr(SDState *sd)
>  {
>      /* All voltages OK, Standard Capacity SD Memory Card, not yet powered up 
> */
> @@ -281,9 +285,10 @@ static void sd_ocr_powerup(void *opaque)
>      SDState *sd = opaque;
>
>      trace_sdcard_powerup();
> -    /* Set powered up bit in OCR */
> -    assert(!(sd->ocr & OCR_POWER_UP));
> -    sd->ocr |= OCR_POWER_UP;
> +    assert(!FIELD_EX32(sd->ocr, OCR, CARD_POWER_UP));
> +
> +    /* card power-up OK */
> +    sd->ocr = FIELD_DP32(sd->ocr, OCR, CARD_POWER_UP, 1);
>  }
>
>  static void sd_reset_scr(SDState *sd)
> @@ -574,7 +579,7 @@ static bool sd_ocr_vmstate_needed(void *opaque)
>      SDState *sd = opaque;
>
>      /* Include the OCR state (and timer) if it is not yet powered up */
> -    return !(sd->ocr & OCR_POWER_UP);
> +    return !FIELD_EX32(sd->ocr, OCR, CARD_POWER_UP);
>  }
>
>  static const VMStateDescription sd_ocr_vmstate = {
> @@ -684,7 +689,7 @@ static void sd_erase(SDState *sd)
>          return;
>      }
>
> -    if (extract32(sd->ocr, OCR_CCS_BITN, 1)) {
> +    if (FIELD_EX32(sd->ocr, OCR, CARD_CAPACITY)) {
>          /* High capacity memory card: erase units are 512 byte blocks */
>          erase_start *= 512;
>          erase_end *= 512;
> @@ -1476,7 +1481,7 @@ static sd_rsp_type_t sd_app_command(SDState *sd,
>               * UEFI, which sends an initial enquiry ACMD41, but
>               * assumes that the card is in ready state as soon as it
>               * sees the power up bit set. */
> -            if (!(sd->ocr & OCR_POWER_UP)) {
> +            if (!FIELD_EX32(sd->ocr, OCR, CARD_POWER_UP)) {
>                  if ((req.arg & ACMD41_ENQUIRY_MASK) != 0) {
>                      timer_del(sd->ocr_power_timer);
>                      sd_ocr_powerup(sd);
> --
> 2.15.1
>
>