[U-Boot] [PATCH 0/8] ARM: Add Seagate GoFlexHome support

Wolfgang Denk wd at denx.de
Mon Apr 8 07:36:28 CEST 2013


Dear Suriyan Ramasami,

In message <1365392690-8668-1-git-send-email-suriyan.r at gmail.com> you wrote:
> Most of the files except goflexhomemenu.c are direct copies
> of the dockstar files with minor modifications.
> 
> goflexhomemenu.c has code which uses the menu functionality
> of u-boot and present the user with an option to choose
> the various bootable options.

Thank you very much for your patches, but there are a number of
issues with these.

First, if you submit several patches, then please chose useful
Subjects and Commit Messages for each patch.  Something like this is
not useful:

> Suriyan Ramasami (8):
>   Add Seagate GoFlexHome support
>   Add Seagate GoFlexHome support
>   Add Seagate GoFlexHome support
>   Add Seagate GoFlexHome support
>   Add Seagate GoFlexHome support
>   Add Seagate GoFlexHome support
>   Add Seagate GoFlexHome support
>   Add Seagate GoFlexHome support

Second, splitting your submission into several patches as you did it
breaks bisectability of the code.  Please squash all these poatches
into one.

Third, there are a large number of coding style issues with your
patches.  Please make sure to read http://www.denx.de/wiki/U-Boot/Patches
and run your patches through checkpatch before resubmitting:


CHECK: Alignment should match open parenthesis
#161: FILE: board/Seagate/goflexhome/goflexhome.c:50:
+	kw_config_gpio(GOFLEXHOME_OE_VAL_LOW,
+			GOFLEXHOME_OE_VAL_HIGH,

CHECK: No space is necessary after a cast
#247: FILE: board/Seagate/goflexhome/goflexhome.c:136:
+	/* command to read PHY dev address */
+	if (miiphy_read(name, 0xEE, 0xEE, (u16 *) &devadr)) {

CHECK: Alignment should match open parenthesis
#249: FILE: board/Seagate/goflexhome/goflexhome.c:138:
+		printf("Err..%s could not read PHY dev address\n",
+			__FUNCTION__);

WARNING: __func__ should be used instead of gcc specific __FUNCTION__
#249: FILE: board/Seagate/goflexhome/goflexhome.c:138:
+			__FUNCTION__);

total: 0 errors, 1 warnings, 3 checks, 187 lines checked

NOTE: Ignored message types: COMPLEX_MACRO CONSIDER_KSTRTO MINMAX MULTISTATEMENT_MACRO_USE_DO_WHILE USLEEP_RANGE

/home/wd/Mail/U-Boot/8248 has style problems, please review.

WARNING: line over 80 characters
#214: FILE: include/configs/goflexhome.h:103:
+#define CONFIG_MTDPARTS		"mtdparts=orion_nand:1m(uboot),6M(uImage),-(root)\0"

total: 0 errors, 1 warnings, 0 checks, 140 lines checked

NOTE: Ignored message types: COMPLEX_MACRO CONSIDER_KSTRTO MINMAX MULTISTATEMENT_MACRO_USE_DO_WHILE USLEEP_RANGE

/home/wd/Mail/U-Boot/8251 has style problems, please review.

If any of these errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.
WARNING: do not add new typedefs
#151: FILE: board/Seagate/goflexhome/goflexhomemenu.c:40:
+typedef struct menu_bootables {

ERROR: code indent should use tabs where possible
#162: FILE: board/Seagate/goflexhome/goflexhomemenu.c:51:
+        printf("%s\n", (char *) print_buffer);$

CHECK: No space is necessary after a cast
#162: FILE: board/Seagate/goflexhome/goflexhomemenu.c:51:
+{
+        printf("%s\n", (char *) print_buffer);

WARNING: please, no spaces at the start of a line
#162: FILE: board/Seagate/goflexhome/goflexhomemenu.c:51:
+        printf("%s\n", (char *) print_buffer);$

WARNING: line over 80 characters
#200: FILE: board/Seagate/goflexhome/goflexhomemenu.c:89:
+static int goflexhome_handle_choice(menu_bootables_t menu_bootlist[], char *choice)

ERROR: do not initialise globals to 0 or NULL
#206: FILE: board/Seagate/goflexhome/goflexhomemenu.c:95:
+int call_saveenv = 0;

WARNING: line over 80 characters
#218: FILE: board/Seagate/goflexhome/goflexhomemenu.c:107:
+		setenv("menu_bootargs", "setenv bootargs ${console} ubi.mtd=2,2048 root=ubi0:root rootfstype=ubifs debug");

WARNING: line over 80 characters
#219: FILE: board/Seagate/goflexhome/goflexhomemenu.c:108:
+		setenv("menu_bootcmd", "setenv bootcmd nand read.e 0x800000 0x100000 0x600000");

WARNING: line over 80 characters
#234: FILE: board/Seagate/goflexhome/goflexhomemenu.c:123:
+		if (strcmp(s, "setenv bootcmd echo Dropping you to u-boot prompt") !=  0) {

WARNING: line over 80 characters
#235: FILE: board/Seagate/goflexhome/goflexhomemenu.c:124:
+			setenv("menu_bootcmd", "setenv bootcmd echo Dropping you to u-boot prompt");

WARNING: line over 80 characters
#243: FILE: board/Seagate/goflexhome/goflexhomemenu.c:132:
+	sprintf(menu_command, "/dev/sd%c%d", menu_bootlist[index].drive, menu_bootlist[index].partition);

WARNING: line over 80 characters
#250: FILE: board/Seagate/goflexhome/goflexhomemenu.c:139:
+	if (strcmp(s, "setenv bootargs $console rootdelay=10 root=${menu_root} debug") != 0) {

WARNING: line over 80 characters
#251: FILE: board/Seagate/goflexhome/goflexhomemenu.c:140:
+		setenv("menu_bootargs", "setenv bootargs ${console} rootdelay=10 root=${menu_root} debug");

CHECK: Alignment should match open parenthesis
#267: FILE: board/Seagate/goflexhome/goflexhomemenu.c:156:
+	sprintf(menu_command, "%s %s %d:%d %x %s",
+			load_command,

CHECK: Alignment should match open parenthesis
#286: FILE: board/Seagate/goflexhome/goflexhomemenu.c:175:
+	sprintf(menu_command, "setenv bootcmd %s %s %d:%d %x %s",
+			load_command,

ERROR: trailing statements should be on next line
#297: FILE: board/Seagate/goflexhome/goflexhomemenu.c:186:
+	if (call_saveenv) saveenv();

WARNING: line over 80 characters
#312: FILE: board/Seagate/goflexhome/goflexhomemenu.c:201:
+	m = menu_create("Bootables:\nChoice\tIntface\tDrive\tDevice\tPart\tFS\tFileName\n---------------------------------------------------------------", 60, 1, goflexhome_menuprint, NULL, NULL);

ERROR: trailing statements should be on next line
#314: FILE: board/Seagate/goflexhome/goflexhomemenu.c:203:
+		if (menu_bootlist[index].fstype == '0') break;

WARNING: line over 80 characters
#316: FILE: board/Seagate/goflexhome/goflexhomemenu.c:205:
+		snprintf(menu_entry[index], sizeof(menu_entry[index]), "%d\t%s\t%c\t%d\t%d\t%c\t%s", index, menu_bootlist[index].interface, menu_bootlist[index].drive, menu_bootlist[index].device, menu_bootlist[index].partition, menu_bootlist[index].fstype, menu_bootlist[index].filename);

ERROR: trailing whitespace
#328: FILE: board/Seagate/goflexhome/goflexhomemenu.c:217:
+ $

WARNING: please, no spaces at the start of a line
#328: FILE: board/Seagate/goflexhome/goflexhomemenu.c:217:
+ $

WARNING: line over 80 characters
#332: FILE: board/Seagate/goflexhome/goflexhomemenu.c:221:
+		sprintf(choice_menu_entry, "* Last boot options (%s)", last_menu_choice);

ERROR: else should follow close brace '}'
#334: FILE: board/Seagate/goflexhome/goflexhomemenu.c:223:
+	}
+	else {

ERROR: else should follow close brace '}'
#341: FILE: board/Seagate/goflexhome/goflexhomemenu.c:230:
+		}
+		else {

WARNING: line over 80 characters
#342: FILE: board/Seagate/goflexhome/goflexhomemenu.c:231:
+	 		sprintf(choice_menu_entry, "* Last boot options (None, and no bootables found!)");

ERROR: code indent should use tabs where possible
#342: FILE: board/Seagate/goflexhome/goflexhomemenu.c:231:
+^I ^I^Isprintf(choice_menu_entry, "* Last boot options (None, and no bootables found!)");$

WARNING: please, no space before tabs
#342: FILE: board/Seagate/goflexhome/goflexhomemenu.c:231:
+^I ^I^Isprintf(choice_menu_entry, "* Last boot options (None, and no bootables found!)");$

CHECK: No space is necessary after a cast
#356: FILE: board/Seagate/goflexhome/goflexhomemenu.c:245:
+
+	menu_get_choice(m, (void **) &menu_choice);

ERROR: return is not a function, parentheses are not required
#357: FILE: board/Seagate/goflexhome/goflexhomemenu.c:246:
+	return(goflexhome_handle_choice(menu_bootlist, menu_choice));

WARNING: line over 80 characters
#360: FILE: board/Seagate/goflexhome/goflexhomemenu.c:249:
+static void goflexhome_filesearch(menu_bootables_t menu_bootlist[], int *bootindex) {

ERROR: open brace '{' following function declarations go on the next line
#360: FILE: board/Seagate/goflexhome/goflexhomemenu.c:249:
+static void goflexhome_filesearch(menu_bootables_t menu_bootlist[], int *bootindex) {

ERROR: do not initialise globals to 0 or NULL
#362: FILE: board/Seagate/goflexhome/goflexhomemenu.c:251:
+int index = 0;

ERROR: trailing statements should be on next line
#381: FILE: board/Seagate/goflexhome/goflexhomemenu.c:270:
+		if (*bootindex >= MENU_MAX_BOOTABLES) break;

WARNING: line over 80 characters
#383: FILE: board/Seagate/goflexhome/goflexhomemenu.c:272:
+		memcpy(&menu_bootlist[*bootindex], &menu_bootlist[*bootindex - 1], sizeof(menu_bootables_t));

WARNING: line over 80 characters
#387: FILE: board/Seagate/goflexhome/goflexhomemenu.c:276:
+static void goflexhome_populate_partitions(menu_bootables_t menu_bootlist[], block_dev_desc_t *dev_desc, int *bootindex)

WARNING: braces {} are not necessary for single statement blocks
#395: FILE: board/Seagate/goflexhome/goflexhomemenu.c:284:
+	if (get_partition_info(dev_desc, part, &disk_part)) {
+		return;
+	}

WARNING: line over 80 characters
#408: FILE: board/Seagate/goflexhome/goflexhomemenu.c:297:
+static void goflexhome_populate_devices(menu_bootables_t menu_bootlist[], int *bootindex)

ERROR: trailing statements should be on next line
#417: FILE: board/Seagate/goflexhome/goflexhomemenu.c:306:
+		if (dev_desc == NULL) continue;

WARNING: line over 80 characters
#421: FILE: board/Seagate/goflexhome/goflexhomemenu.c:310:
+			goflexhome_populate_partitions(menu_bootlist, dev_desc, bootindex);

ERROR: do not initialise globals to 0 or NULL
#431: FILE: board/Seagate/goflexhome/goflexhomemenu.c:320:
+int bootindex = 0;

ERROR: do not initialise globals to 0 or NULL
#432: FILE: board/Seagate/goflexhome/goflexhomemenu.c:321:
+int i = 0;

ERROR: trailing whitespace
#442: FILE: board/Seagate/goflexhome/goflexhomemenu.c:331:
+        while ((interfaces[i][0] != '\0') && $

ERROR: code indent should use tabs where possible
#442: FILE: board/Seagate/goflexhome/goflexhomemenu.c:331:
+        while ((interfaces[i][0] != '\0') && $

WARNING: please, no spaces at the start of a line
#442: FILE: board/Seagate/goflexhome/goflexhomemenu.c:331:
+        while ((interfaces[i][0] != '\0') && $

ERROR: code indent should use tabs where possible
#443: FILE: board/Seagate/goflexhome/goflexhomemenu.c:332:
+               (bootindex < MENU_MAX_BOOTABLES)) {$

WARNING: please, no spaces at the start of a line
#443: FILE: board/Seagate/goflexhome/goflexhomemenu.c:332:
+               (bootindex < MENU_MAX_BOOTABLES)) {$

ERROR: trailing statements should be on next line
#456: FILE: board/Seagate/goflexhome/goflexhomemenu.c:345:
+		if (menu_bootlist[i].fstype == '0') break;

WARNING: line over 80 characters
#460: FILE: board/Seagate/goflexhome/goflexhomemenu.c:349:
+		if (strcmp(menu_bootlist[i].interface, menu_bootlist[i - 1].interface) != 0) {

ERROR: else should follow close brace '}'
#463: FILE: board/Seagate/goflexhome/goflexhomemenu.c:352:
+		}
+		else {

WARNING: line over 80 characters
#464: FILE: board/Seagate/goflexhome/goflexhomemenu.c:353:
+			if (menu_bootlist[i].device > menu_bootlist[i - 1].device) {

WARNING: braces {} are not necessary for single statement blocks
#464: FILE: board/Seagate/goflexhome/goflexhomemenu.c:353:
+			if (menu_bootlist[i].device > menu_bootlist[i - 1].device) {
+				menu_bootlist[i].drive++;
+			}

ERROR: trailing whitespace
#469: FILE: board/Seagate/goflexhome/goflexhomemenu.c:358:
+} $

CHECK: if this code is redundant consider removing it
#476: FILE: board/Seagate/goflexhome/goflexhomemenu.c:365:
+#if 0

ERROR: code indent should use tabs where possible
#481: FILE: board/Seagate/goflexhome/goflexhomemenu.c:370:
+        goflexhome_populate_bootlist(menu_bootlist);$

WARNING: please, no spaces at the start of a line
#481: FILE: board/Seagate/goflexhome/goflexhomemenu.c:370:
+        goflexhome_populate_bootlist(menu_bootlist);$

WARNING: braces {} are not necessary for single statement blocks
#484: FILE: board/Seagate/goflexhome/goflexhomemenu.c:373:
+		if (retval == MENU_EXIT) {
+			retval = goflexhome_evaluate_env();
+		}

total: 22 errors, 29 warnings, 5 checks, 381 lines checked

NOTE: whitespace errors detected, you may wish to use scripts/cleanpatch or
      scripts/cleanfile

NOTE: Ignored message types: COMPLEX_MACRO CONSIDER_KSTRTO MINMAX MULTISTATEMENT_MACRO_USE_DO_WHILE USLEEP_RANGE

/home/wd/Mail/U-Boot/8252 has style problems, please review.


Thanks.

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
How many Unix hacks does it take to change a light bulb?  Let's  see,
   can you use a shell script for that or does it need a C program?


More information about the U-Boot mailing list