Re: [PATCH 01/10] stm32mp: stm32prog: remove all the header check for UART download

Hi Patrick

On 5/18/21 3:12 PM, Patrick Delaunay wrote:

This patch removes the header check for UART download; the check of checksum is not mandatory with even parity and chuck checksum for each 256 received bytes and it is only done for STM32 image (FSBL = TF-A BL2), not for FIT image.

This patch solve issue of duplicated 0x100 byte written with FIP header.

Fixes: 4fb7b3e10891 ("stm32mp: stm32prog: add FIP header support") Signed-off-by: Patrick Delaunay patrick.delaunay@foss.st.com

---

.../mach-stm32mp/cmd_stm32prog/stm32prog.c | 14 +- .../mach-stm32mp/cmd_stm32prog/stm32prog.h | 5 - .../cmd_stm32prog/stm32prog_serial.c | 151 ++---------------- 3 files changed, 22 insertions(+), 148 deletions(-)

diff --git a/arch/arm/mach-stm32mp/cmd_stm32prog/stm32prog.c b/arch/arm/mach-stm32mp/cmd_stm32prog/stm32prog.c index 4c4d8a7a69..84b880261a 100644 --- a/arch/arm/mach-stm32mp/cmd_stm32prog/stm32prog.c +++ b/arch/arm/mach-stm32mp/cmd_stm32prog/stm32prog.c @@ -369,23 +369,24 @@ static int parse_flash_layout(struct stm32prog_data *data, bool end_of_line, eof; char *p, *start, *last, *col; struct stm32prog_part_t *part;

• struct image_header_s header; int part_list_size; int i;

  data->part_nb = 0;

  /* check if STM32image is detected */

• stm32prog_header_check((struct raw_header_s *)addr, &data->header);
• if (data->header.type == HEADER_STM32IMAGE) {

• stm32prog_header_check((struct raw_header_s *)addr, &header);

• if (header.type == HEADER_STM32IMAGE) { u32 checksum;

  addr = addr + BL_HEADER_SIZE;

• size = data->header.image_length;
  

• size = header.image_length;
  
  

• checksum = stm32prog_header_checksum(addr, &data->header);
  

• if (checksum != data->header.image_checksum) {
  

• checksum = stm32prog_header_checksum(addr, &header);
  

• if (checksum != header.image_checksum) {
  stm32prog_err("Layout: invalid checksum : 0x%x expected 0x%x",
  

• 		      checksum, data->header.image_checksum);
  

• 		      checksum, header.image_checksum);
  return -EIO;
  

  } }

@@ -1727,7 +1728,6 @@ void stm32prog_clean(struct stm32prog_data *data) free(data->part_array); free(data->otp_part); free(data->buffer);

• free(data->header_data);

}

/* DFU callback: used after serial and direct DFU USB access */ diff --git a/arch/arm/mach-stm32mp/cmd_stm32prog/stm32prog.h b/arch/arm/mach-stm32mp/cmd_stm32prog/stm32prog.h index 581b10d0ac..ad404879a7 100644 --- a/arch/arm/mach-stm32mp/cmd_stm32prog/stm32prog.h +++ b/arch/arm/mach-stm32mp/cmd_stm32prog/stm32prog.h @@ -132,14 +132,9 @@ struct stm32prog_data { u32 *otp_part; u8 pmic_part[PMIC_SIZE];

• /* STM32 header information */
• struct raw_header_s *header_data;
• struct image_header_s header;
• /* SERIAL information */ u32 cursor; u32 packet_number;
• u32 checksum; u8 *buffer; /* size = USART_RAM_BUFFER_SIZE*/ int dfu_seq; u8 read_phase;

diff --git a/arch/arm/mach-stm32mp/cmd_stm32prog/stm32prog_serial.c b/arch/arm/mach-stm32mp/cmd_stm32prog/stm32prog_serial.c index 2b92e3b149..7eca86c11b 100644 --- a/arch/arm/mach-stm32mp/cmd_stm32prog/stm32prog_serial.c +++ b/arch/arm/mach-stm32mp/cmd_stm32prog/stm32prog_serial.c @@ -292,56 +292,6 @@ static void stm32prog_serial_putc(u8 w_byte) }

/* Helper function ************************************************/

-static u8 stm32prog_header(struct stm32prog_data *data) -{

• u8 ret;
• u8 boot = 0;
• struct dfu_entity *dfu_entity;
• u64 size = 0;
• dfu_entity = stm32prog_get_entity(data);
• if (!dfu_entity)

• return -ENODEV;
  

• printf("\nSTM32 download write %s\n", dfu_entity->name);
• /* force cleanup to avoid issue with previous read */
• dfu_transaction_cleanup(dfu_entity);
• stm32prog_header_check(data->header_data, &data->header);
• /* no stm32 image header : max size is partition size */
• if (data->header.type != HEADER_STM32IMAGE) {

• dfu_entity->get_medium_size(dfu_entity, &size);
  

• data->header.image_length = size;
  

• }
• /**** Flash the header if necessary for boot partition */
• if (data->phase < PHASE_FIRST_USER)

• boot = 1;
  

• /* write header if boot partition */
• if (boot) {

• if (ret) {
  

• 	stm32prog_err("invalid header (error %d)", ret);
  

• } else {
  

• 	ret = stm32prog_write(data,
  

• 			      (u8 *)data->header_data,
  

• 			      BL_HEADER_SIZE);
  

• }
  

• } else {

• if (ret)
  

• 	printf("  partition without checksum\n");
  

• ret = 0;
  

• }
• free(data->header_data);
• data->header_data = NULL;
• return ret;

-}

static u8 stm32prog_start(struct stm32prog_data *data, u32 address) { u8 ret = 0; @@ -388,23 +338,6 @@ static u8 stm32prog_start(struct stm32prog_data *data, u32 address) data->dfu_seq = 0;

printf("\n  received length = 0x%x\n", data->cursor);

• if (data->header.type == HEADER_STM32IMAGE) {
  

• 	if (data->cursor !=
  

• 	    (data->header.image_length + BL_HEADER_SIZE)) {
  

• 		stm32prog_err("transmission interrupted (length=0x%x expected=0x%x)",
  

• 			      data->cursor,
  

• 			      data->header.image_length +
  

• 			      BL_HEADER_SIZE);
  

• 		return -EIO;
  

• 	}
  

• 	if (data->header.image_checksum != data->checksum) {
  

• 		stm32prog_err("invalid checksum received (0x%x expected 0x%x)",
  

• 			      data->checksum,
  

• 			      data->header.image_checksum);
  

• 		return -EIO;
  

• 	}
  

• 	printf("\n  checksum OK (0x%x)\n", data->checksum);
  

• }
  
  

  /* update DFU with received flashlayout */ if (data->phase == PHASE_FLASHLAYOUT)

@@ -627,14 +560,12 @@ static void download_command(struct stm32prog_data *data) u32 counter = 0x0, codesize = 0x0; u8 *ramaddress = 0; u8 rcv_data = 0x0;

• struct image_header_s *image_header = &data->header; u32 cursor = data->cursor; long size = 0; u8 operation; u32 packet_number; u32 result = ACK_BYTE; u8 ret;
• unsigned int i; bool error; int rcv;

@@ -668,13 +599,8 @@ static void download_command(struct stm32prog_data *data) if (packet_number == 0) { /* erase: re-initialize the image_header struct */ data->packet_number = 0;

• if (data->header_data)
  

• 	memset(data->header_data, 0, BL_HEADER_SIZE);
  

• else
  

• 	data->header_data = calloc(1, BL_HEADER_SIZE);
  

  cursor = 0; data->cursor = 0;

• data->checksum = 0;
  

  /*idx = cursor;*/ } else { data->packet_number++;

@@ -746,74 +672,27 @@ static void download_command(struct stm32prog_data *data) goto end; }

• /* Update current position in buffer */
• data->cursor += codesize;
• if (operation == PHASE_OTP) {

• size = data->cursor - cursor;
  

• /* no header for OTP */
  

• if (stm32prog_otp_write(data, cursor,
  

• 			data->buffer, &size))
  

• 	result = ABORT_BYTE;
  

• goto end;
  

• }

• switch (operation) {
• case PHASE_OTP:

• size = codesize;
  

• ret = stm32prog_otp_write(data, cursor, data->buffer, &size);
  

• break;
  
  

• if (operation == PHASE_PMIC) {

• size = data->cursor - cursor;
  

• /* no header for PMIC */
  

• if (stm32prog_pmic_write(data, cursor,
  

• 			 data->buffer, &size))
  

• 	result = ABORT_BYTE;
  

• goto end;
  

• }

• case PHASE_PMIC:

• size = codesize;
  

• ret = stm32prog_pmic_write(data, cursor, data->buffer, &size);
  

• break;
  
  

• if (cursor < BL_HEADER_SIZE) {

• /* size = portion of header in this chunck */
  

• if (data->cursor >= BL_HEADER_SIZE)
  

• 	size = BL_HEADER_SIZE - cursor;
  

• else
  

• 	size = data->cursor - cursor;
  

• memcpy((void *)((u32)(data->header_data) + cursor),
  

•        data->buffer, size);
  

• cursor += size;
  

• if (cursor == BL_HEADER_SIZE) {
  

• 	/* Check and Write the header */
  

• 	if (stm32prog_header(data)) {
  

• 		result = ABORT_BYTE;
  

• 		goto end;
  

• 	}
  

• } else {
  

• 	goto end;
  

• }
  

• default:

• ret = stm32prog_write(data, data->buffer, codesize);
  

• break;
  

  }

• if (data->header.type == HEADER_STM32IMAGE) {

• if (data->cursor <= BL_HEADER_SIZE)
  

• 	goto end;
  

• /* compute checksum on payload */
  

• for (i = (unsigned long)size; i < codesize; i++)
  

• 	data->checksum += data->buffer[i];
  

• if (data->cursor >
  

•     image_header->image_length + BL_HEADER_SIZE) {
  

• 	log_err("expected size exceeded\n");
  

• 	result = ABORT_BYTE;
  

• 	goto end;
  

• }
  

• /* write data (payload) */
  

• ret = stm32prog_write(data,
  

• 		      &data->buffer[size],
  

• 		      codesize - size);
  

• } else {

• /* write all */
  

• ret = stm32prog_write(data,
  

• 		      data->buffer,
  

• 		      codesize);
  

• } if (ret) result = ABORT_BYTE;

• else

• /* Update current position in buffer */
  

• data->cursor += codesize;
  
  

end: stm32prog_serial_result(result);

Reviewed-by: Patrice Chotard patrice.chotard@foss.st.com

Thanks Patrice