Re: [PATCH 09/22] hw/sd: ssi-sd: Use macros for the dummy value and tokens in the transfer

[Alistair Francis]

On Thu, Dec 31, 2020 at 3:38 AM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> From: Bin Meng <bin.meng@windriver.com>
>
> At present the codes use hardcoded numbers (0xff/0xfe) for the dummy
> value and block start token. Replace them with macros, and add more
> tokens for multiple block write.
>
> Signed-off-by: Bin Meng <bin.meng@windriver.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>
>  hw/sd/ssi-sd.c | 30 +++++++++++++++++++++---------
>  1 file changed, 21 insertions(+), 9 deletions(-)
>
> diff --git a/hw/sd/ssi-sd.c b/hw/sd/ssi-sd.c
> index 889260bd8f..8eb48550cf 100644
> --- a/hw/sd/ssi-sd.c
> +++ b/hw/sd/ssi-sd.c
> @@ -78,6 +78,18 @@ OBJECT_DECLARE_SIMPLE_TYPE(ssi_sd_state, SSI_SD)
>  #define SSI_SDR_ADDRESS_ERROR   0x2000
>  #define SSI_SDR_PARAMETER_ERROR 0x4000
>
> +/* reading and writing blocks start with these tokens and end with crc16 */
> +
> +/* multiple block write */
> +#define SSI_TOKEN_MULTI_WRITE   0xfc
> +/* terminate multiple block write */
> +#define SSI_TOKEN_STOP_TRAN     0xfd
> +/* single block read/write, multiple block read */
> +#define SSI_TOKEN_SINGLE        0xfe
> +
> +/* dummy value - don't care */
> +#define SSI_DUMMY               0xff
> +
>  static uint32_t ssi_sd_transfer(SSIPeripheral *dev, uint32_t val)
>  {
>      ssi_sd_state *s = SSI_SD(dev);
> @@ -91,14 +103,14 @@ static uint32_t ssi_sd_transfer(SSIPeripheral *dev, 
> uint32_t val)
>
>      switch (s->mode) {
>      case SSI_SD_CMD:
> -        if (val == 0xff) {
> +        if (val == SSI_DUMMY) {
>              DPRINTF("NULL command\n");
> -            return 0xff;
> +            return SSI_DUMMY;
>          }
>          s->cmd = val & 0x3f;
>          s->mode = SSI_SD_CMDARG;
>          s->arglen = 0;
> -        return 0xff;
> +        return SSI_DUMMY;
>      case SSI_SD_CMDARG:
>          if (s->arglen == 4) {
>              SDRequest request;
> @@ -173,14 +185,14 @@ static uint32_t ssi_sd_transfer(SSIPeripheral *dev, 
> uint32_t val)
>          } else {
>              s->cmdarg[s->arglen++] = val;
>          }
> -        return 0xff;
> +        return SSI_DUMMY;
>      case SSI_SD_PREP_RESP:
>          s->mode = SSI_SD_RESPONSE;
> -        return 0xff;
> +        return SSI_DUMMY;
>      case SSI_SD_RESPONSE:
>          if (s->stopping) {
>              s->stopping = 0;
> -            return 0xff;
> +            return SSI_DUMMY;
>          }
>          if (s->response_pos < s->arglen) {
>              DPRINTF("Response 0x%02x\n", s->response[s->response_pos]);
> @@ -193,12 +205,12 @@ static uint32_t ssi_sd_transfer(SSIPeripheral *dev, 
> uint32_t val)
>              DPRINTF("End of command\n");
>              s->mode = SSI_SD_CMD;
>          }
> -        return 0xff;
> +        return SSI_DUMMY;
>      case SSI_SD_DATA_START:
>          DPRINTF("Start read block\n");
>          s->mode = SSI_SD_DATA_READ;
>          s->response_pos = 0;
> -        return 0xfe;
> +        return SSI_TOKEN_SINGLE;
>      case SSI_SD_DATA_READ:
>          val = sdbus_read_byte(&s->sdbus);
>          s->read_bytes++;
> @@ -225,7 +237,7 @@ static uint32_t ssi_sd_transfer(SSIPeripheral *dev, 
> uint32_t val)
>          return val;
>      }
>      /* Should never happen.  */
> -    return 0xff;
> +    return SSI_DUMMY;
>  }
>
>  static int ssi_sd_post_load(void *opaque, int version_id)
> --
> 2.25.1
>
>