arm: BeagleBone Black: enable fastboot support

Flash command internally uses DFU, and Fastboot command initialization is modified to add DFU and partition initialization Added oem format functionality for GPT table creation
partitioning code is added as disk/part_fastboot.c for better usability

Fastboot flash command code is enabled and being tested on BeagleBone Black.
OMAP3 Beagle configuration is modified to fix the build errors, but this configuration needs to be updated as per the flash functionality.

drivers/usb/gadget/f_fastboot.c +16 -0

+int handle_flash(char *part_name, char *response, char *transfer_buffer)
+{
+	int status = 0;
+	printf("download_bytes: 0x%x\n", download_bytes);
+	if (download_bytes) {
+		struct fastboot_ptentry *ptn;
+
+		/* Next is the partition name */
+		ptn = fastboot_flash_find_ptn(part_name);
+
+		if (ptn == 0) {
+			printf("Partition:[%s] does not exist\n", part_name);
+			sprintf(response, "FAILpartition does not exist");
+		} else if ((download_bytes > ptn->length) &&
+			!(ptn->flags & FASTBOOT_PTENTRY_FLAGS_WRITE_ENV)) {
+			printf("Image too large for the partition\n");

disk/part_fastboot.c new file +6 -0

+		mmc_read[2] = source;
+		mmc_read[3] = dest;
+		mmc_read[4] = length;
+
+		sprintf(source, "%p", entry);
+		sprintf(dest, "0x%x", 0x1+n);

Dear Dileep Katta,

Quoted 6 lines · In message <[email protected]> you…

Flash command internally uses DFU, and Fastboot command initialization is modified to add DFU and partition initialization Added oem format functionality for GPT table creation
partitioning code is added as disk/part_fastboot.c for better usability

Fastboot flash command code is enabled and being tested on BeagleBone Black.
OMAP3 Beagle configuration is modified to fix the build errors, but this configuration needs to be updated as per the flash functionality.

Please fix the line length in the commit message.

Also, checkpatch says:

        WARNING: do not add new typedefs
        #1073: FILE: include/usb/fastboot.h:98:
        +typedef struct fastboot_ptentry fastboot_ptentry;

Quoted 16 lines

drivers/usb/gadget/f_fastboot.c +16 -0

+int handle_flash(char *part_name, char *response, char *transfer_buffer)
+{
+     int status = 0;
+     printf("download_bytes: 0x%x\n", download_bytes);
+     if (download_bytes) {
+             struct fastboot_ptentry *ptn;
+
+             /* Next is the partition name */
+             ptn = fastboot_flash_find_ptn(part_name);
+
+             if (ptn == 0) {
+                     printf("Partition:[%s] does not exist\n", part_name);
+                     sprintf(response, "FAILpartition does not exist");
+             } else if ((download_bytes > ptn->length) &&
+                     !(ptn->flags & FASTBOOT_PTENTRY_FLAGS_WRITE_ENV)) {
+                     printf("Image too large for the partition\n");

drivers/usb/gadget/f_fastboot.c static void rx_handler_command(struct usb_ep *ep, struct usb_request *req) · line 646

Quoted 10 lines

drivers/usb/gadget/f_fastboot.c +10 -0

+                     sprintf(response, "FAILimage too large for partition");
+             } else if (ptn->flags & FASTBOOT_PTENTRY_FLAGS_WRITE_ENV) {
+                     /* Check if this is not really a flash write,
+                      * but instead a saveenv
+                      */
+                     unsigned int i = 0;
+                     /* Env file is expected with a NULL delimeter between
+                      * env variables So replace New line Feeds(0x0a) with
+                      * NULL (0x00)
+                      */

Incorrect multiline comment style. Please fix globally.

Also this sequence of comment - declaration - comment is highly confusing. What is the first comment ("Check if..") actually related to?

drivers/usb/gadget/f_fastboot.c static void rx_handler_command(struct usb_ep *ep, struct usb_request *req) · line 654

Quoted 8 lines

drivers/usb/gadget/f_fastboot.c +8 -0

+                     for (i = 0; i < download_bytes; i++) {
+                             if (transfer_buffer[i] == 0x0a)
+                                     transfer_buffer[i] = 0x00;
+                     }
+                     memset(env_ptr->data, 0, ENV_SIZE);
+                     memcpy(env_ptr->data, transfer_buffer, download_bytes);
+                     do_env_save(NULL, 0, 1, NULL);
+                     printf("saveenv to '%s' DONE!\n", ptn->name);

I think it is a bad idea to split formatting / reformatting envrionment data alll over the place. This should be done only once, in the environment handling code. Also, I do not like you using do_env_save() here - this should remain a static function. Why do't you simply use saveenv() here?

Finally, the "saveenv to '%s' DONE!\n" is IMO too verbose. No news is Good News - we usually do not report abot the success of operations, as this is what we expect. We should only print error messages when such occur - which points out another problem of that code: there is no error checking nor handling there...

disk/part_fastboot.c line 157

Quoted 6 lines

disk/part_fastboot.c new file +6 -0

+static void end_ptbl(struct ptable *ptbl)
+{
+     struct efi_header *hdr = &ptbl->header;
+     u32 n;
+
+     n = crc32(0, 0, 0);

What exactly is this good for?

disk/part_fastboot.c line 161

Quoted 4 lines

disk/part_fastboot.c new file +4 -0

+     n = crc32(n, (void *)ptbl->entry, sizeof(ptbl->entry));
+     hdr->entries_crc32 = n;
+
+     n = crc32(0, 0, 0);

What exactly is this good for?

disk/part_fastboot.c line 251

Quoted 10 lines

disk/part_fastboot.c new file +8 -0

+     n = crc32(0, (void *)&ptbl->header, sizeof(ptbl->header));
+     hdr->crc32 = n;
+}
 
 
+     for (n = 0; n < (128/4); n++) {
+             /* read partition */
+             source[0] = '\0';
+             dest[0] = '\0';
+             length[0] = '\0';

You overwrite source, dest, and length by the sprintf()s just a few lines below. So why are you doing this here?

Quoted 6 lines

disk/part_fastboot.c new file +6 -0

+             mmc_read[2] = source;
+             mmc_read[3] = dest;
+             mmc_read[4] = length;
+
+             sprintf(source, "%p", entry);
+             sprintf(dest, "0x%x", 0x1+n);

disk/part_fastboot.c line 276

Quoted 10 lines

disk/part_fastboot.c new file +8 -0

+             sprintf(length, "0x%x", 1);
 
 
+int board_mmc_fbtptn_init(void)
+{
+     char *mmc_init[2] = {"mmc", "rescan",};
+     char dev[2];
+     char *mmc_dev[3] = {"mmc", "dev", NULL};
+
+     mmc_dev[2] = dev;

Why do you initialize mmc_dev[2] with another value just a line above? Why don't you use the correct value right from the beginning?

disk/part_fastboot.c line 287

Quoted 9 lines

disk/part_fastboot.c new file +9 -0

+     if (do_mmcops(NULL, 0, 3, mmc_dev)) {
+             printf("MMC DEV: %d selection FAILED!\n", CONFIG_FB_MMCDEV);
+             return -1;
+     }
+
+     if (do_mmcops(NULL, 0, 2, mmc_init)) {
+                     printf("FAIL:Init of MMC card\n");
+                     return 1;
+     }

What exactly are your return codes? Here you return -1 in case of error, there +1 - what's the logic behind that?

disk/part_fastboot.c line 299

Quoted 4 lines

disk/part_fastboot.c new file +4 -0

+int do_format(void)
+{
+     struct ptable *ptbl = &the_ptable;
+     /* unsigned sector_sz; */

Please remove dead code.

Quoted 1 line

+ printf("\ndo_format ..!!");

Is this not overly verbose?

disk/part_fastboot.c line 306

Quoted 2 lines

disk/part_fastboot.c new file +2 -0

+     /* get mmc info */
+     struct mmc *mmc = find_mmc_device(CONFIG_FB_MMCDEV);

Please do not mix code and declarations. Also, always add a blank line after declarations.

disk/part_fastboot.c line 316

Quoted 4 lines

disk/part_fastboot.c new file +4 -0

+     if (mmc_init(mmc)) {
+             printf("\n mmc init FAILED");
+             return -1;
+     } else{

After the return, no else is needed here.

disk/part_fastboot.c line 352

Quoted 12 lines

disk/part_fastboot.c new file +11 -0

+             printf("\nmmc capacity is: %llu", mmc->capacity);
+             printf("\nmmc: number of blocks:0x%lx", mmc->block_dev.lba);
+             printf("\nmmc: block size:0x%lx", mmc->block_dev.blksz);
+     }
 
+     /* 10/11:modified as per PSP release support */
+     char *mmc_write[5]  = {"mmc", "write", NULL, NULL, NULL};
+     char source[32], dest[32], length[32];
+
+     char dev[2];
+     char *mmc_dev[3] = {"mmc", "dev", NULL};
+     mmc_dev[2] = dev;

See above - please do not mix code and declarations, please use the right initialization right away.

disk/part_fastboot.c line 372

Quoted 2 lines

disk/part_fastboot.c new file +2 -0

+     } else {
+             printf("Writing mbr is DONE!\n");

Less verbose, please.

Best regards,

Wolfgang Denk

--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de At the source of every error which is blamed on the computer you will find at least two human errors, including the error of blaming it on the computer.