[U-Boot] [RFC PATCH 0/3] Implement "fastboot flash" for eMMC

[Steve Rae <srae@broadcom.com>]

Rob & Sebastian

I would appreciate your comments on this issue; I suspect that you had 
some ideas regarding the implementation of the fastboot "flash" and 
"erase" commands....

Thanks in advance, Steve

On 14-06-23 05:58 AM, Lukasz Majewski wrote:
> Hi Steve,
>
>>
>>
>> On 14-06-19 11:32 PM, Marek Vasut wrote:
>>> On Friday, June 20, 2014 at 08:18:42 AM, Lukasz Majewski wrote:
>>>> Hi Steve,
>>>>
>>>>> This series implements the "fastboot flash" command for eMMC
>>>>> devices. It supports both raw and sparse images.
>>>>>
>>>>> NOTES:
>>>>> - the support for the "fastboot flash" command is enabled with
>>>>> CONFIG_FASTBOOT_FLASH
>>>>> - the support for eMMC is enabled with
>>>>> CONFIG_FASTBOOT_FLASH_MMC_DEV
>>>>> - (future) the support for NAND would be enabled with
>>>>> CONFIG_FASTBOOT_FLASH_NAND(???)
>>>>> - thus the proposal is to place the code in common/fb_mmc.c and
>>>>> (future) common/fb_nand.c(???), however, this may not be the
>>>>> appropriate location....
>>>>
>>>> Would you consider another approach for providing flashing backend
>>>> for fastboot?
>>>>
>>>> I'd like to propose reusing of the dfu flashing code for this
>>>> purpose. Such approach has been used successfully with USB "thor"
>>>> downloading function.
>>>>
>>>> Since the "fastboot" is using gadget infrastructure (thanks to the
>>>> effort of Rob Herring) I think that it would be feasible to reuse
>>>> the same approach as "thor" does. In this way the low level code
>>>> would be kept in one place and we could refine and test it more
>>>> thoroughly.
>>>
>>> I'm all for this approach as well if possible.
>>>
>>> Best regards,
>>> Marek Vasut
>>> _______________________________________________
>>> U-Boot mailing list
>>> U-Boot at lists.denx.de
>>> http://lists.denx.de/mailman/listinfo/u-boot
>>>
>>
>> I have briefly investigated this suggestion....
>> And have 'hacked' some code as follows:
>>
>> --- common/fb_mmc.c_000	2014-06-20 14:13:43.271158073 -0700
>> +++ common/fb_mmc.c_001	2014-06-20 14:17:48.688072764 -0700
>>    		while (remaining_chunks) {
>>    			switch (le16_to_cpu(c_header->chunk_type)) {
>>    			case CHUNK_TYPE_RAW:
>> +#if 0
>>    				blkcnt =
>>    				    (le32_to_cpu(c_header->chunk_sz)
>> * blk_sz) / info.blksz;
>>    				buffer =
>>    				    (void *)c_header +
>>    				    le16_to_cpu(s_header->chunk_hdr_sz);
>>
>>    				blks =
>> mmc_dev->block_write(mmc_dev->dev, blk, blkcnt, buffer);
>>    				if (blks != blkcnt) {
>>    					printf("Write failed
>> %lu\n", blks); strcpy(response,
>>    					       "FAILmmc write
>> failure"); return;
>>    				}
>>
>>    				bytes_written += blkcnt *
>> info.blksz; +#else
>> +				buffer =
>> +				    (void *)c_header +
>> +
>> le16_to_cpu(s_header->chunk_hdr_sz); +
>> +				len =
>> le32_to_cpu(c_header->chunk_sz) * blk_sz;
>> +				ret_dfu = dfu_write_medium_mmc(dfu,
>> offset,
>> +
>> buffer, &len);
>> +				if (ret_dfu) {
>> +					printf("Write failed %lu\n",
>> len);
>> +					strcpy(response,
>> +					       "FAILmmc write
>> failure");
>> +					return;
>> +				}
>> +
>> +
>> +				bytes_written += len;
>> +#endif
>>    				break;
>>
>>    			case CHUNK_TYPE_FILL:
>>    			case CHUNK_TYPE_DONT_CARE:
>>    			case CHUNK_TYPE_CRC32:
>>    				/* do nothing */
>>    				break;
>>
>>    			default:
>>    				/* error */
>>    				printf("Unknown chunk type\n");
>>    				strcpy(response,
>>    				       "FAILunknown chunk type in
>> sparse image"); return;
>>    			}
>>
>> +#if 0
>>    			blk += (le32_to_cpu(c_header->chunk_sz) *
>> blk_sz) / info.blksz;
>> +#else
>> +			offset += le32_to_cpu(c_header->chunk_sz) *
>> blk_sz; +#endif
>>    			c_header = (chunk_header_t *)((void
>> *)c_header + le32_to_cpu(c_header->total_sz));
>>    			remaining_chunks--;
>>    		}
>>
>>
>> --- common/fb_mmc.c_000	2014-06-20 14:13:43.271158073 -0700
>> +++ common/fb_mmc.c_001	2014-06-20 14:17:48.688072764 -0700
>>    		/* raw image */
>>
>> +#if 0
>>    		/* determine number of blocks to write */
>>    		blkcnt =
>>    		    ((download_bytes + (info.blksz - 1)) &
>> ~(info.blksz - 1)); blkcnt = blkcnt / info.blksz;
>>
>>    		if (blkcnt > info.size) {
>>    			printf("%s: too large for partition:
>> '%s'\n", __func__, cmd);
>>    			strcpy(response, "FAILtoo large for
>> partition"); return;
>>    		}
>>
>>    		printf("Flashing Raw Image\n");
>>
>>    		blks = mmc_dev->block_write(mmc_dev->dev,
>> info.start, blkcnt, download_buffer);
>>    		if (blks != blkcnt) {
>>    			printf("%s: failed writing to mmc device
>> %d\n", __func__, mmc_dev->dev);
>>    			strcpy(response, "FAILfailed writing to mmc
>> device"); return;
>>    		}
>>
>>    		printf("........ wrote %lu bytes to '%s'\n",
>>    		       blkcnt * info.blksz, cmd);
>> +#else
>> +		printf("Flashing Raw Image\n");
>> +
>> +		ret_dfu = dfu_write_medium_mmc(dfu, offset,
>> download_buffer, &len);
>> +		if (ret_dfu) {
>> +			printf("%s: failed writing to mmc device
>> %d\n",
>> +			       __func__, mmc_dev->dev);
>> +			strcpy(response, "FAILfailed writing to mmc
>> device");
>> +			return;
>> +		}
>> +
>> +		printf("........ wrote %lu bytes to '%s'\n", len,
>> cmd); +#endif
>>    	}
>>
>> NOTE:
>> - I know that I cannot call "dfu_write_medium_mmc()" directly -- but
>> I just wanted to test this functionality
>
> Indeed, it looks like an early proof-of-concept code :-).
>
>>
>> My initial reaction is that using the DFU backend to effectively call
>> the mmc block_write() function seems to cause an unnecessary amount
>> of overhead;
>
> It also allows to access/write data to other media - like NAND memory.
>
>> and the only thing that it really provides is a proven
>> method of calculating the "number of blocks to write"...
>>
>> I would be more interested in this backend if it would provide:
>> - handling of the "sparse image format"
>> 	-- would a CONFIG option to include this in the DFU_OP_WRITE
>
> You are welcome to prepare patch which adds such functionality.
> Moreover, in the u-boot-dfu repository (master branch) you can find
> initial version of the regression tests for DFU.
> Extending the current one, or adding your own would be awesome :-)
>
>
>> case of the "mmc_block_op()" be acceptable?
>> - a method which uses "get_partition_info_efi_by_name()"
>> 	-- no ideas yet...
>>
>
> I'm looking forward for RFC.
>
>> If the consensus is to use this DFU backend, then I will continue is
>> this direction.
>
> That would be great.
>
>>
>> Please advise,
>> Thanks, Steve
>
>