Dear Macpaul Lin,

In message <1303797876-28548-1-git-send-email-macp...@andestech.com> you wrote:
> ftahbc020s.h provides basic definitions of this controller
> to help a SoC which use this AHB Controller could
> do scalable software settings in lowlevel_init.S.
> 
> 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.
...
> +/* this section is used by lowlevel_init.S */
> +#define FTAHBC020S_SLAVE_BSR_0               0x00    /* Slave n Base/Size 
> Reg */
> +#define FTAHBC020S_SLAVE_BSR_1               0x04
> +#define FTAHBC020S_SLAVE_BSR_2               0x08
> +#define FTAHBC020S_SLAVE_BSR_3               0x0C
> +#define FTAHBC020S_SLAVE_BSR_4               0x10
> +#define FTAHBC020S_SLAVE_BSR_5               0x14
> +#define FTAHBC020S_SLAVE_BSR_6               0x18
> +#define FTAHBC020S_SLAVE_BSR_7               0x1C
> +#define FTAHBC020S_SLAVE_BSR_8               0x20
> +#define FTAHBC020S_SLAVE_BSR_9               0x24
> +#define FTAHBC020S_SLAVE_BSR_10              0x28

See previous comment: I think this should be done using asm-offsets
instead.

> +#define FTAHBC020S_SLAVE_BSR_BASE(x) (((x) & 0xFFF) << 20)
> +#define FTAHBC020S_SLAVE_BSR_SIZE(x) (((x) & 0xF) << 16)
> +
> +#define FTAHBC020S_SLAVE_BSR_SIZE_1M 0x0
> +#define FTAHBC020S_SLAVE_BSR_SIZE_2M 0x1
> +#define FTAHBC020S_SLAVE_BSR_SIZE_4M 0x2
> +#define FTAHBC020S_SLAVE_BSR_SIZE_8M 0x3
> +#define FTAHBC020S_SLAVE_BSR_SIZE_16M        0x4
> +#define FTAHBC020S_SLAVE_BSR_SIZE_32M        0x5
> +#define FTAHBC020S_SLAVE_BSR_SIZE_64M        0x6
> +#define FTAHBC020S_SLAVE_BSR_SIZE_128M       0x7
> +#define FTAHBC020S_SLAVE_BSR_SIZE_256M       0x8
> +#define FTAHBC020S_SLAVE_BSR_SIZE_512M       0x9
> +#define FTAHBC020S_SLAVE_BSR_SIZE_1G 0xA
> +#define FTAHBC020S_SLAVE_BSR_SIZE_2G 0xB

I recommend to use a generic macro here, as recommended for the other
patch.

> +/*
> + * FTAHBC020S_PCR - Priority Control Register
> + */
> +#define FTAHBC020S_PCR_PLEVEL_15     (1 << 15)
> +#define FTAHBC020S_PCR_PLEVEL_14     (1 << 14)
> +#define FTAHBC020S_PCR_PLEVEL_13     (1 << 13)
> +#define FTAHBC020S_PCR_PLEVEL_12     (1 << 12)
> +#define FTAHBC020S_PCR_PLEVEL_11     (1 << 11)
> +#define FTAHBC020S_PCR_PLEVEL_10     (1 << 10)
> +#define FTAHBC020S_PCR_PLEVEL_09     (1 << 9)
> +#define FTAHBC020S_PCR_PLEVEL_08     (1 << 8)
> +#define FTAHBC020S_PCR_PLEVEL_07     (1 << 7)
> +#define FTAHBC020S_PCR_PLEVEL_06     (1 << 6)
> +#define FTAHBC020S_PCR_PLEVEL_05     (1 << 5)
> +#define FTAHBC020S_PCR_PLEVEL_04     (1 << 4)
> +#define FTAHBC020S_PCR_PLEVEL_03     (1 << 3)
> +#define FTAHBC020S_PCR_PLEVEL_02     (1 << 2)
> +#define FTAHBC020S_PCR_PLEVEL_01     (1 << 1)

Ditto here.  Why flooding the code with tons of (mostly) unused
defines?

Use:

        #define FTAHBC020S_PCR_PLEVEL(x)        (1 << (x))


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
Vor allem kein Gedanke! Nichts ist kompromittierender als ein  Gedan-
ke!            - Friedrich Wilhelm Nietzsche _Der Fall Wagner_ (1888)
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to