Re: [Qemu-devel] [PATCH 17/20] sdcard: Add SDFrameData struct and data frame checksum functions

["Philippe Mathieu-Daudé" <f4bug@amsat.org>]

On 05/04/2018 12:59 PM, Philippe Mathieu-Daudé wrote:
> The block transfers data frames are upto 512 bytes and use a 16-bit CRC.
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  include/hw/sd/sd.h     | 29 +++++++++++++++++++++++++++++
>  hw/sd/sdmmc-internal.c | 10 ++++++++++
>  2 files changed, 39 insertions(+)
> 
> diff --git a/include/hw/sd/sd.h b/include/hw/sd/sd.h
> index c76be51b32..6bf9daf559 100644
> --- a/include/hw/sd/sd.h
> +++ b/include/hw/sd/sd.h
> @@ -102,6 +102,17 @@ typedef struct SDFrame136 {
>      uint8_t crc;
>  } SDFrame136;
>  
> +/**
> + * SDFrameData: 512 bytes block transfers
> + *
> + * @content: block data
> + * @crc:     16-bit CRC checksum
> + */
> +typedef struct SDFrameData {
> +    uint8_t content[512];
> +    uint16_t crc;
> +} SDFrameData;
> +
>  typedef struct SDFrame48 SDRequest;
>  
>  typedef struct SDState SDState;
> @@ -212,6 +223,14 @@ void sd_update_frame48_checksum(SDFrame48 *frame, bool is_response);
>   */
>  void sd_update_frame136_checksum(SDFrame136 *frame);
>  
> +/**
> + * sd_update_framedata_checksum:
> + * @frame: the #SDFrameData to verify
> + *
> + * Update the 16-bit CRC checksum of a SD data frame (up to 512 bytes).
> + */
> +void sd_update_framedata_checksum(SDFrameData *frame);

Since the frame size is variable, this function needs a size_t argument.

> +
>  /**
>   * sd_verify_frame48_checksum:
>   * @frame: the #SDFrame48 to verify
> @@ -233,6 +252,16 @@ bool sd_verify_frame48_checksum(SDFrame48 *frame, bool is_response);
>   */
>  bool sd_verify_frame136_checksum(SDFrame136 *frame);
>  
> +/**
> + * sd_verify_framedata_checksum:
> + * @frame: the #SDFrameData to verify
> + *
> + * Verify the 16-bit CRC checksum of a SD data frame.
> + *
> + * Returns: A boolean indicating whether the frame 16-bit CRC is correct.
> + */
> +bool sd_verify_framedata_checksum(SDFrameData *frame);

Ditto.

> +
>  /* Legacy functions to be used only by non-qdevified callers */
>  SDState *sd_init(BlockBackend *bs, bool is_spi);
>  int sd_do_command(SDState *sd, SDRequest *req,
> diff --git a/hw/sd/sdmmc-internal.c b/hw/sd/sdmmc-internal.c
> index 68350a2304..0e82e69d99 100644
> --- a/hw/sd/sdmmc-internal.c
> +++ b/hw/sd/sdmmc-internal.c
> @@ -134,6 +134,11 @@ bool sd_verify_frame136_checksum(SDFrame136 *frame)
>      return sd_calc_frame136_crc7(frame) == frame->crc;
>  }
>  
> +bool sd_verify_framedata_checksum(SDFrameData *frame)
> +{
> +    return sd_crc16(frame->content, sizeof(frame->content)) == frame->crc;

I'll probably replace sizeof(frame->content) by a 'length' argument,
or add a 'length' member to SDFrameData.

> +}
> +
>  void sd_update_frame48_checksum(SDFrame48 *frame, bool is_response)
>  {
>      frame->crc = sd_calc_frame48_crc7(frame->cmd, frame->arg, is_response);
> @@ -144,6 +149,11 @@ void sd_update_frame136_checksum(SDFrame136 *frame)
>      frame->crc = (sd_crc7(frame->content, sizeof(frame->content)) << 1) | 1;
>  }
>  
> +void sd_update_framedata_checksum(SDFrameData *frame)
> +{
> +    frame->crc = sd_crc16(frame->content, sizeof(frame->content));
> +}
> +
>  void sd_prepare_frame48(SDFrame48 *frame, uint8_t cmd, uint32_t arg,
>                          bool is_response, bool gen_crc)
>  {
>