[DNX#2006033142000285] [U-Boot-Users] PATCH : add MX21ads

[U-Boot patch tracking system]

Dear Tomko,

Thank you for your request.
Tomko <tomko at avantwave.com> wrote:

> Here is the patch adds support for mx21ads on u-boot 1.1.3.

Please read the "Coding Standards" in u-boot/README.

You have C++ comments, wrong identation, trailing whitespaces!

For Example:
./board/mx21ads/flash.c:

+typedef unsigned int    U32;    /* unsigned 32 bit data */

why didn&#180;t you use u32 from ./include/asm-arm/types.h ?

[...]
+U32 FlashSectorErase(U32 sAddress, U32 nAddress)
+{
+    volatile U32*       pBase;
^^^^^             ^^^^^^^
please use Tab for identation

[...]
+    U32                                        SA;
+    U32                                        i = 0;
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
why so much?

+        // Check the Flash Starting Address
         ^^^ Don&#180;t use C++ comments

[...]
+               for (SA = 1; SA <= 270; SA++)
+               {

Please read "Documentation/CodingStyle Chapter 2: Placing Braces"
in your Linux kernel source directory.

[...]
+
+int flash_erase (flash_info_t *info, int s_first, int s_last) {
+       int rc = ERR_OK;
+//add by tom

Nice to know, but this is out of interest here. Please comment
only useful things ...

+
+       int sadd = 0;
+       int eadd = 0;
+       int nsect =0;
+
+puts("flash_earse in syncflash.c\n");
+
+printf("first sector is %d\n",s_first);
+printf("last sector is %d\n",s_last);

This is debuginfo. Please use debug() (from ./include/common.h)

+
+       nsect = s_last + 1 - s_first;
+
+       if (nsect < 0)
+               return -1;
+
+       sadd = info->start[s_first];
+       eadd = sadd + (nsect * (FLASH_BANK_SIZE / CFG_MAX_FLASH_SECT)) -1;
+
+       FlashSectorErase(sadd,eadd);

Why didn&#180;t you check the returncode from FlashSectorErase()?

+       return rc;
+}

You didn&#180;t use rc! This is confusing.

+void FlashWrite(U32 sAddress,
+               U32* pData,
+               U32 nData)

Please in one line.

+{
+
+

Why this 2 empty lines?

Furthermore i get compiler warnings:

mx21ads.c: In function 'board_late_init':
mx21ads.c:86: warning: implicit declaration of function 'get_mcuPLLCLK'
mx21ads.c:87: warning: implicit declaration of function 'get_serialPLLCLK'
mx21ads.c:88: warning: implicit declaration of function 'get_PERCLK1'
mx21ads.c:89: warning: implicit declaration of function 'get_PERCLK2'
mx21ads.c:90: warning: implicit declaration of function 'get_PERCLK3'
mx21ads.c:91: warning: implicit declaration of function 'get_PERCLK4'

interrupts.c: In function 'interrupt_init':
interrupts.c:33: warning: 'tmp' is used uninitialized in this function

flash.c: In function 'write_buff':
flash.c:363: warning: unused variable 'i'
flash.c: In function 'FlashSectorErase':
flash.c:116: warning: 'bFailTotal' may be used uninitialized in this function

and compiling errors:

serial.c: In function 'mx21_uart_preset':
serial.c:83: error: invalid lvalue in assignment
serial.c:84: error: invalid lvalue in assignment


Please reformat your patch and resubmit it.

Your Ticket-Team

--
Heiko Schocher --
DENX Software Engineering:  Embedded and Realtime Systems, Embedded Linux
Phone: (+49)-8142-66989-0 Fax: (+49)-8142-66989-80 Email: support at denx.de
Powered by OTRS (http://otrs.org/)