Dear "kevin.morfitt@fearnside-systems.co.uk",
In message 4B2548F7.2060605@fearnside-systems.co.uk you wrote:
Cleans up the s3c24x0 header files by changing the upper case members of the s3c24x0 register structures to lower case and changing all code that uses these register structures.
Signed-off-by: Kevin Morfitt kevin.morfitt@fearnside-systems.co.uk
board/mpl/vcma9/vcma9.c | 264 ++++++++++--------- board/mpl/vcma9/vcma9.h | 91 +++--- board/samsung/smdk2400/smdk2400.c | 53 ++-- board/samsung/smdk2410/smdk2410.c | 85 +++--- board/sbc2410x/sbc2410x.c | 131 +++++----- board/trab/cmd_trab.c | 547 +++++++++++++++++------------------- board/trab/rs485.c | 92 ++++--- 7 files changed, 626 insertions(+), 637 deletions(-)
diff --git a/board/mpl/vcma9/vcma9.c b/board/mpl/vcma9/vcma9.c index 1835677..84338eb 100644 --- a/board/mpl/vcma9/vcma9.c +++ b/board/mpl/vcma9/vcma9.c @@ -39,32 +39,31 @@ DECLARE_GLOBAL_DATA_PTR; #define FCLK_SPEED 1
#if FCLK_SPEED==0 /* Fout = 203MHz, Fin = 12MHz for Audio */ -#define M_MDIV 0xC3 -#define M_PDIV 0x4 -#define M_SDIV 0x1
- #define M_MDIV 0xC3
- #define M_PDIV 0x4
- #define M_SDIV 0x1
#elif FCLK_SPEED==1 /* Fout = 202.8MHz */ -#define M_MDIV 0xA1 -#define M_PDIV 0x3 -#define M_SDIV 0x1
- #define M_MDIV 0xA1
- #define M_PDIV 0x3
- #define M_SDIV 0x1
#endif
#define USB_CLOCK 1
#if USB_CLOCK==0 -#define U_M_MDIV 0xA1 -#define U_M_PDIV 0x3 -#define U_M_SDIV 0x1
- #define U_M_MDIV 0xA1
- #define U_M_PDIV 0x3
- #define U_M_SDIV 0x1
#elif USB_CLOCK==1 -#define U_M_MDIV 0x48 -#define U_M_PDIV 0x3 -#define U_M_SDIV 0x2
- #define U_M_MDIV 0x48
- #define U_M_PDIV 0x3
- #define U_M_SDIV 0x2
#endif
NAK. C preprocessor lines always start in the first column. Do not do this.
int board_init(void) {
- struct s3c24x0_clock_power * const clk_power =
s3c24x0_get_base_clock_power();- struct s3c24x0_gpio * const gpio = s3c24x0_get_base_gpio();
- struct s3c24x0_clock_power *const clk_power =
s3c24x0_get_base_clock_power();
Indentation not by TAB.
/* set up the I/O ports */
- gpio->GPACON = 0x007FFFFF;
- gpio->GPBCON = 0x002AAAAA;
- gpio->GPBUP = 0x000002BF;
- gpio->GPCCON = 0xAAAAAAAA;
- gpio->GPCUP = 0x0000FFFF;
- gpio->GPDCON = 0xAAAAAAAA;
- gpio->GPDUP = 0x0000FFFF;
- gpio->GPECON = 0xAAAAAAAA;
- gpio->GPEUP = 0x000037F7;
- gpio->GPFCON = 0x00000000;
- gpio->GPFUP = 0x00000000;
- gpio->GPGCON = 0xFFEAFF5A;
- gpio->GPGUP = 0x0000F0DC;
- gpio->GPHCON = 0x0028AAAA;
- gpio->GPHUP = 0x00000656;
- writel(0x007FFFFF, &gpio->gpacon);
- writel(0x002AAAAA, &gpio->gpbcon);
- writel(0x000002BF, &gpio->gpbup);
- writel(0xAAAAAAAA, &gpio->gpccon);
- writel(0x0000FFFF, &gpio->gpcup);
- writel(0xAAAAAAAA, &gpio->gpdcon);
- writel(0x0000FFFF, &gpio->gpdup);
- writel(0xAAAAAAAA, &gpio->gpecon);
- writel(0x000037F7, &gpio->gpeup);
- writel(0x00000000, &gpio->gpfcon);
- writel(0x00000000, &gpio->gpfup);
- writel(0xFFEAFF5A, &gpio->gpgcon);
- writel(0x0000F0DC, &gpio->gpgup);
- writel(0x0028AAAA, &gpio->gphcon);
- writel(0x00000656, &gpio->gphup);
Such a change should introduce symbolic constants for all thes magic numbers, and add comments what they actually do.
#if defined(CONFIG_CMD_NAND) -extern ulong -nand_probe(ulong physadr);
+extern ulong nand_probe(ulong physadr);
Please move proprtypes to appropriate header files.
...
- NF_Conf((1<<15)|(0<<14)|(0<<13)|(1<<12)|(1<<11)|(TACLS<<8)|(TWRPH0<<4)|(TWRPH1<<0));
- /*nand->NFCONF = (1<<15)|(1<<14)|(1<<13)|(1<<12)|(1<<11)|(TACLS<<8)|(TWRPH0<<4)|(TWRPH1<<0); */
- /* 1 1 1 1, 1 xxx, r xxx, r xxx */
- /* En 512B 4step ECCR nFCE=H tACLS tWRPH0 tWRPH1 */
- nf_conf((1 << 15) | (0 << 14) | (0 << 13) | (1 << 12) | (1 << 11) |
(TACLS << 8) | (TWRPH0 << 4) | (TWRPH1 << 0));- /*nand->NFCONF = (1<<15)|(1<<14)|(1<<13)|(1<<12)|(1<<11)|(TACLS<<8)|(TWRPH0<<4)|(TWRPH1<<0); */
Line too long. Please check globally.
#ifdef DEBUG
- printf("NAND flash probing at 0x%.8lX\n", (ulong)nand);
- printf("NAND flash probing at 0x%.8lX\n", (ulong) nand);
#endif
- printf ("%4lu MB\n", nand_probe((ulong)nand) >> 20);
- printf("%4lu MB\n", nand_probe((ulong) nand) >> 20);
No additional spaces here.
-static inline void NF_WaitRB(void) +static inline void nf_waitrb(void) {
- struct s3c2410_nand * const nand = s3c2410_get_base_nand();
- struct s3c2410_nand *const nand = s3c2410_get_base_nand();
- while (!(nand->NFSTAT & (1<<0)));
- while (!(readl(&nand->nfstat) & (1 << 0)));
Please write as:
while (!(readl(&nand->nfstat) & 1)) ;
@@ -105,16 +106,16 @@ int dram_init (void) static int key_pressed(void) { int rc;
- if (1) { /* check for button push here, now just return 1 */
- if (1) { /* check for button push here, now just return 1 */
Line too long. Please check globally.
Hey, why do you change this line at all? It was OK before!
#ifdef CONFIG_CMD_NET -int board_eth_init(bd_t *bis) +int board_eth_init(bd_t * bis)
No additional spaces here. Please check globally.
...
-static inline void delay (unsigned long loops) +static inline void delay(unsigned long loops) {
- __asm__ volatile ("1:\n"
"subs %0, %1, #1\n""bne 1b":"=r" (loops):"0" (loops));
- __asm__ volatile("1:\n"
"subs %0, %1, #1\n" "bne 1b":"=r" (loops):"0"(loops));
Line too long, and much less readable. Don't do this!!!
I give up here.
The same comments apply in many p[laces, please make sure to check globally and fix all ocurrences.
Best regards,
Wolfgang Denk