[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