On 5/28/21 2:53 PM, Patrice CHOTARD wrote:
> 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.delau...@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.chot...@foss.st.com>
>
> Thanks
> Patrice
> _______________________________________________
> Uboot-stm32 mailing list
> uboot-st...@st-md-mailman.stormreply.com
> https://st-md-mailman.stormreply.com/mailman/listinfo/uboot-stm32
>
Applied on u-boot-stm32/next
Thanks
