[U-Boot] [PATCH 33/42] Blackfin: cm-bf537e: new board port

[Wolfgang Denk]

Dear Mike Frysinger,

In message <1234246880-32438-34-git-send-email-vapier at gentoo.org> you wrote:
> Signed-off-by: Mike Frysinger <vapier at gentoo.org>
> ---
>  MAINTAINERS                     |    1 +
>  MAKEALL                         |    1 +
>  Makefile                        |    4 +-
>  board/cm-bf537e/.gitignore      |    1 +
>  board/cm-bf537e/Makefile        |   58 +++++
>  board/cm-bf537e/cm-bf537e.c     |   44 ++++
>  board/cm-bf537e/config.mk       |   33 +++
>  board/cm-bf537e/flash-defines.h |   50 +++++
>  board/cm-bf537e/flash.c         |  449 +++++++++++++++++++++++++++++++++++++++
>  board/cm-bf537e/spi_flash.c     |    2 +
>  board/cm-bf537e/u-boot.lds.S    |  143 +++++++++++++
>  include/configs/cm-bf537e.h     |  133 ++++++++++++
>  12 files changed, 917 insertions(+), 2 deletions(-)
>  create mode 100644 board/cm-bf537e/.gitignore
>  create mode 100644 board/cm-bf537e/Makefile
>  create mode 100644 board/cm-bf537e/cm-bf537e.c
>  create mode 100644 board/cm-bf537e/config.mk
>  create mode 100644 board/cm-bf537e/flash-defines.h
>  create mode 100644 board/cm-bf537e/flash.c
>  create mode 100644 board/cm-bf537e/spi_flash.c
>  create mode 100644 board/cm-bf537e/u-boot.lds.S
>  create mode 100644 include/configs/cm-bf537e.h

Global comment for all patches in this series:

> diff --git a/board/cm-bf537e/flash.c b/board/cm-bf537e/flash.c
> new file mode 100644
> index 0000000..1806be6
> --- /dev/null
> +++ b/board/cm-bf537e/flash.c
> @@ -0,0 +1,449 @@
> +/*
> + * U-boot - flash.c Flash driver for MT28F320J3FS-11
...
> +#define SWITCH_BANK do { if (memIndex >= 0x20200000) {\
> +			  *pPORTFIO_SET = GPIO_4; \
> +			  memIndex -= 0x200000; \
> +			  crossed = 1;\
> +			} else { \
> +			  *pPORTFIO_CLEAR = GPIO_4; \
> +			} \
> +			asm("ssync;"); } while (0)
> +
> +#define SWITCH_BACK do {if (crossed) {\
> +			*pPORTFIO_CLEAR = GPIO_4; \
> +			memIndex += 0x200000; \
> +			crossed = 0;\
> +			asm("ssync;");\
> +			} } while (0)

Better use inline functions instead of such macros.

Please use accessor functions instead of pointer accesses (which alslo
will allow you to get rid of the "ssync" stuff here).

> +	for (i = 0; i < CONFIG_SYS_MAX_FLASH_SECT; i++)
> +		flash_info[0].start[i] =
> +		    (CONFIG_SYS_FLASH_BASE + (i * FLASH_SECTOR_SIZE));

Please use curly braces for multiline statements.

...
> +int flash_erase(flash_info_t *info, int s_first, int s_last)
> +{
> +	int prot, sect, i;
> +
> +	prot = 0;
> +	for (sect = s_first; sect <= s_last; ++sect) {
> +		if (info->protect[sect])
> +			prot++;
> +	}
> +	if (prot)
> +		printf("- Warning: %d protected sectors will not be erased!\n",
> +		       prot);
> +	else

Please use curly braces for multiline statements.

> +int write_buff(flash_info_t *info, unsigned char *src, ulong addr, ulong cnt)
> +{
> +	long i = 0;
> +	unsigned long ulOffset = addr - CONFIG_SYS_FLASH_BASE;
> +	int nLeftover = cnt % 4;
> +	int iSectors = 0;
> +	int iFirst_sector = 0;
> +	int iProgress = cnt / (4 * 20);

Please do not use camel-case variable names.


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
Q:  Do you know what the death rate around here is?
A:  One per person.