Dear Macpaul Lin,

In message <1303797876-28548-2-git-send-email-macp...@andestech.com> you wrote:
> Support registers definitions of ftsdmc021 SDRAM controller.
> 
> Signed-off-by: Macpaul Lin <macp...@andestech.com>
> ---
> Changes for v2:
>  - Add __ASSEMBLY__ protecton to register offset for supporting lowlevel_init
> Changes for v3:
>  - Patch: no change. Changed a mail server to resend this.
...
> +#ifdef __ASSEMBLY__
> +#define FTSDMC021_OFFSET_TP1         0x00    /* SDRAM Timing Parameter 1 */
> +#define FTSDMC021_OFFSET_TP2         0x04    /* SDRAM Timing Parameter 2 */
> +#define FTSDMC021_OFFSET_CR1         0x08    /* SDRAM Configuration Reg 1 */
> +#define FTSDMC021_OFFSET_CR2         0x0C    /* SDRAM Configuration Reg 2 */
> +#define FTSDMC021_OFFSET_BANK0_BSR   0x10    /* External Bank Base/Size Reg 
> 0 */
> +#define FTSDMC021_OFFSET_BANK1_BSR   0x14    /* External Bank Base/Size Reg 
> 1 */
> +#define FTSDMC021_OFFSET_BANK2_BSR   0x18    /* External Bank Base/Size Reg 
> 2 */
> +#define FTSDMC021_OFFSET_BANK3_BSR   0x1C    /* External Bank Base/Size Reg 
> 3 */
> +#define FTSDMC021_OFFSET_BANK4_BSR   0x20    /* External Bank Base/Size Reg 
> 4 */
> +#define FTSDMC021_OFFSET_BANK5_BSR   0x24    /* External Bank Base/Size Reg 
> 5 */
> +#define FTSDMC021_OFFSET_BANK6_BSR   0x28    /* External Bank Base/Size Reg 
> 6 */
> +#define FTSDMC021_OFFSET_BANK7_BSR   0x2C    /* External Bank Base/Size Reg 
> 7 */

Lines too long.  Please fix globally.

I think it is generally wrong to manually define these offsets here.
You should use a C struct instead, and auto-generate the offsets if
needed using the asm-offsets approach (see top level Makefile for
details).

> +/* 12-bit base address of external bank.
> + * Default value is 0x800.
> + * The 12-bit equals to the haddr[31:20] of AHB address bus. */
> +#define FTSDMC021_BANK_BASE(x)               ((x) & 0xfff)
> +
> +#define FTSDMC021_BANK_SIZE_1M               0x0
> +#define FTSDMC021_BANK_SIZE_2M               0x1
> +#define FTSDMC021_BANK_SIZE_4M               0x2
> +#define FTSDMC021_BANK_SIZE_8M               0x3
> +#define FTSDMC021_BANK_SIZE_16M              0x4
> +#define FTSDMC021_BANK_SIZE_32M              0x5
> +#define FTSDMC021_BANK_SIZE_64M              0x6
> +#define FTSDMC021_BANK_SIZE_128M     0x7
> +#define FTSDMC021_BANK_SIZE_256M     0x8
> +#define FTSDMC021_BANK_SIZE_512M     0x9

Why don't you use a generic macro here, like

        #define FTSDMC021_BANK_SIZE(sz) (ffs(x) - 21)

?


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: w...@denx.de
Accident: A condition in which presence of mind is good, but  absence
of body is better.
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to