Hi Niklaus,

On Monday 04 February 2008, Niklaus Giger wrote:
> - Fix some coding style violations.
> - Use in/out_u16/32 where appropriate.
> - Use register names from ppc405.h.
> - Fix trace useage for Lauterbach.
> - Fix detection of vxWorks EDR.
> - Remove obsolete generation HCU2.
> - Renamed fixed_hcu4_sdram to init_ppc405_sdram.

Thanks. Look really good now. Just a few nitpicking comments below.

> Signed-off-by: Niklaus Giger <[EMAIL PROTECTED]>
> ---
>  board/netstal/common/fixed_sdram.c |    2 +-
>  board/netstal/common/hcu_flash.c   |   14 ------
>  board/netstal/common/nm.h          |   11 ++++-
>  board/netstal/common/nm_bsp.c      |    4 +-
>  board/netstal/hcu4/hcu4.c          |   78
> ++++++++++++++++-------------------- board/netstal/hcu5/hcu5.c          |  
>  8 +--
>  board/netstal/hcu5/sdram.c         |   32 +++++++++------
>  7 files changed, 68 insertions(+), 81 deletions(-)
>
> diff --git a/board/netstal/common/fixed_sdram.c
> b/board/netstal/common/fixed_sdram.c index 8082f60..1df790c 100644
> --- a/board/netstal/common/fixed_sdram.c
> +++ b/board/netstal/common/fixed_sdram.c
> @@ -44,7 +44,7 @@ void show_sdram_registers(void)
>  }
>  #endif
>
> -long int fixed_hcu4_sdram (unsigned int dram_size)
> +long int init_ppc405_sdram (unsigned int dram_size)
>  {
>  #ifdef DEBUG
>       printf(__FUNCTION__);
> diff --git a/board/netstal/common/hcu_flash.c
> b/board/netstal/common/hcu_flash.c index be2cb37..d0322f2 100644
> --- a/board/netstal/common/hcu_flash.c
> +++ b/board/netstal/common/hcu_flash.c
> @@ -21,18 +21,6 @@
>   * MA 02111-1307 USA
>   */
>
> -/*
> - * Modified 4/5/2001
> - * Wait for completion of each sector erase command issued
> - * 4/5/2001
> - * Chris Hallinan - DS4.COM, Inc. - [EMAIL PROTECTED]
> - *
> - * Modified 6/6/2007
> - * Added isync
> - * Niklaus Giger, Netstal Maschinen, [EMAIL PROTECTED]
> - *
> - */
> -
>  #include <common.h>
>  #include <ppc4xx.h>
>  #include <asm/processor.h>
> @@ -387,7 +375,6 @@ int flash_erase (flash_info_t * info, int s_first, int
> s_last) /* wait at least 80us - let's wait 1 ms */
>       udelay (1000);
>
> -#if 0
>       /*
>        * We wait for the last triggered sector
>        */
> @@ -396,7 +383,6 @@ int flash_erase (flash_info_t * info, int s_first, int
> s_last) wait_for_DQ7 (info, l_sect);
>
>  DONE:
> -#endif
>       /* reset to read mode */
>       addr = (FLASH_WORD_SIZE *) info->start[0];
>       addr[0] = (FLASH_WORD_SIZE) 0x00F000F0; /* reset bank */
> diff --git a/board/netstal/common/nm.h b/board/netstal/common/nm.h
> index 2801e13..f6acd47 100644
> --- a/board/netstal/common/nm.h
> +++ b/board/netstal/common/nm.h
> @@ -27,8 +27,7 @@ extern void set_params_for_sw_install(int
> install_requested, char *board_name ); extern void
> common_misc_init_r(void);
>
>  enum {
> -     /* HW_GENERATION_HCU1 is no longer supported */
> -     HW_GENERATION_HCU2  = 0x10,
> +     /* HW_GENERATION_HCU1/2 is no longer supported */
>       HW_GENERATION_HCU3  = 0x10,
>       HW_GENERATION_HCU4  = 0x20,
>       HW_GENERATION_HCU5  = 0x30,
> @@ -36,3 +35,11 @@ enum {
>       HW_GENERATION_MCU20 = 0x0a,
>       HW_GENERATION_MCU25 = 0x09,
>  };
> +
> +#ifdef CONFIG_405GPr

Why this check here? You should never need 405GPr. CONFIG_405GP should be 
enough since they are nearly compatible.

> +#if defined(DEBUG)
> +void show_sdram_registers(void);
> +#endif
> +long int init_ppc405_sdram (unsigned int dram_size);

Again you are mixing here coding styles: func() and func ().

> +#endif
> +
> diff --git a/board/netstal/common/nm_bsp.c b/board/netstal/common/nm_bsp.c
> index c4265bb..89c697c 100644
> --- a/board/netstal/common/nm_bsp.c
> +++ b/board/netstal/common/nm_bsp.c
> @@ -29,8 +29,7 @@ DECLARE_GLOBAL_DATA_PTR;
>
>  typedef struct {u8   id;     char *name;} generation_info;
>
> -generation_info generations[7] = {
> -     {HW_GENERATION_HCU2,    "HCU2"},
> +generation_info generations[6] = {

I think you can remove the "6" here.

>       {HW_GENERATION_HCU3,    "HCU3"},
>       {HW_GENERATION_HCU4,    "HCU4"},
>       {HW_GENERATION_HCU5,    "HCU5"},
> @@ -134,3 +133,4 @@ void common_misc_init_r(void)
>               saveenv();
>       }
>  }
> +
> diff --git a/board/netstal/hcu4/hcu4.c b/board/netstal/hcu4/hcu4.c
> index 4fbe701..43057a0 100644
> --- a/board/netstal/hcu4/hcu4.c
> +++ b/board/netstal/hcu4/hcu4.c
> @@ -1,5 +1,5 @@
>  /*
> - *(C) Copyright 2005-2007 Netstal Maschinen AG
> + *(C) Copyright 2005-2008 Netstal Maschinen AG
>   *    Niklaus Giger ([EMAIL PROTECTED])
>   *
>   *    This source code is free software; you can redistribute it
> @@ -28,17 +28,10 @@
>  DECLARE_GLOBAL_DATA_PTR;
>
>  #define HCU_MACH_VERSIONS_REGISTER   (0x7C000000 + 0xF00000)
> -#define SYS_SLOT_ADDRESS             (0x7C000000 + 0x400000)
> -#define HCU3_DIGITAL_IO_REGISTER     (0x7C000000 + 0x500000)
> +#define HCU_SLOT_ADDRESS             (0x7C000000 + 0x400000)
> +#define HCU_DIGITAL_IO_REGISTER              (0x7C000000 + 0x500000)
>  #define HCU_SW_INSTALL_REQUESTED     0x10
>
> -#undef DEBUG
> -
> -#if defined(DEBUG)
> -void show_sdram_registers(void);
> -#endif
> -long int fixed_hcu4_sdram (unsigned int dram_size);
> -
>  /*
>   * This function is run very early, out of flash, and before devices are
>   * initialized. It is called by lib_ppc/board.c:board_init_f by virtue
> @@ -49,17 +42,12 @@ long int fixed_hcu4_sdram (unsigned int dram_size);
>   * anything, not even stack. So be careful.
>   */
>
> -#define CPC0_CR0     0xb1    /* Chip control register 0 */
> -#define CPC0_CR1        0xb2 /* Chip control register 1 */
>  /* Attention: If you want 1 microsecs times from the external oscillator
> - * use  0x00804051. But this causes problems with u-boot and linux!
> + * 0x00004051 is okay for u-boot/linux, but different from old vxworks
> values + * 0x00804051 causes problems with u-boot and linux!
>   */
>  #define CPC0_CR0_VALUE       0x0030103c
>  #define CPC0_CR1_VALUE       0x00004051
> -#define CPC0_ECR     0xaa    /* Edge condition register */
> -#define EBC0_CFG     0x23    /* External Peripheral Control Register */
> -#define CPC0_EIRR    0xb6    /* External Interrupt Register */
> -
>
>  int board_early_init_f (void)
>  {
> @@ -70,16 +58,16 @@ int board_early_init_f (void)
>        *      IRQ 17-24 RESERVED/UNUSED
>        *      IRQ 31 (EXT IRQ 6) (unused)
>        */
> -     mtdcr (uicsr, 0xFFFFFFFF); /* clear all ints */
> -     mtdcr (uicer, 0x00000000); /* disable all ints */
> -     mtdcr (uiccr, 0x00000000); /* set all to be non-critical */
> -     mtdcr (uicpr, 0xFFFFE000); /* set int polarities */
> -     mtdcr (uictr, 0x00000000); /* set int trigger levels */
> -     mtdcr (uicsr, 0xFFFFFFFF); /* clear all ints */
> +     mtdcr(uicsr, 0xFFFFFFFF); /* clear all ints */
> +     mtdcr(uicer, 0x00000000); /* disable all ints */
> +     mtdcr(uiccr, 0x00000000); /* set all to be non-critical */
> +     mtdcr(uicpr, 0xFFFFE000); /* set int polarities */
> +     mtdcr(uictr, 0x00000000); /* set int trigger levels */
> +     mtdcr(uicsr, 0xFFFFFFFF); /* clear all ints */
>
> -     mtdcr(CPC0_CR1,  CPC0_CR1_VALUE);
> -     mtdcr(CPC0_ECR,  0x60606000);
> -     mtdcr(CPC0_EIRR, 0x7c000000);
> +     mtdcr(cntrl1,  CPC0_CR1_VALUE);
> +     mtdcr(ecr,  0x60606000);
> +     mtdcr(eirr, 0x7C000000);
>
>       return 0;
>  }
> @@ -93,18 +81,19 @@ int board_pre_init (void)
>
>  int sys_install_requested(void)
>  {
> -     u16 *ioValuePtr = (u16 *)HCU3_DIGITAL_IO_REGISTER;
> -     return (in_be16(ioValuePtr) & HCU_SW_INSTALL_REQUESTED) != 0;
> +     u16 ioValue = in_be16((u16 *)HCU_DIGITAL_IO_REGISTER);
> +     return (ioValue & HCU_SW_INSTALL_REQUESTED) != 0;
>  }
>
>  int checkboard (void)
>  {
> -     u16 *boardVersReg = (u16 *)HCU_MACH_VERSIONS_REGISTER;
> -     u16 generation = in_be16(boardVersReg) & 0xf0;
> -     u16 index      = in_be16(boardVersReg) & 0x0f;
> +     u16 boardVersReg = in_be16((u16 *)HCU_MACH_VERSIONS_REGISTER);
> +     u16 generation = boardVersReg & 0xf0;
> +     u16 index      = boardVersReg & 0x0f;
> +
> +     /* Cannot be done in board_early_init */
> +     mtdcr(cntrl0,  CPC0_CR0_VALUE);
>
> -     /* Cannot be done, in board_early_init */
> -     mtdcr(CPC0_CR0,  CPC0_CR0_VALUE);
>       /* Force /RTS to active. The board it not wired quite
>        *  correctly to use cts/rtc flow control, so just force the
>        *  /RST active and forget about it.
> @@ -145,8 +134,8 @@ void sdram_init(void)
>   */
>  u32 hcu_get_slot(void)
>  {
> -     u16 *slot = (u16 *)SYS_SLOT_ADDRESS;
> -     return in_be16(slot) & 0x7f;
> +     u16 slot = in_be16((u16 *)HCU_SLOT_ADDRESS);
> +     return slot & 0x7f;
>  }
>
>  /*
> @@ -154,12 +143,12 @@ u32 hcu_get_slot(void)
>   */
>  u32 get_serial_number(void)
>  {
> -     u32 *serial = (u32 *)CFG_FLASH_BASE;
> +     u32 serial = in_be32((u32 *)CFG_FLASH_BASE);
>
> -     if (in_be32(serial) == 0xffffffff)
> +     if (serial == 0xffffffff)
>               return 0;
>
> -     return in_be32(serial);
> +     return serial;
>  }
>
>
> @@ -177,12 +166,13 @@ int misc_init_r(void)
>  long int initdram(int board_type)
>  {
>       long dram_size = 0;
> -     u16 *boardVersReg = (u16 *) HCU_MACH_VERSIONS_REGISTER;
> -     u16 generation = in_be16(boardVersReg) & 0xf0;
> -     if (generation == HW_GENERATION_HCU3)
> -             dram_size = 32*1024*1024;
> -     else dram_size = 64*1024*1024;
> -     fixed_hcu4_sdram(dram_size);
> +     u16 boardVersReg = in_be16((u16 *)HCU_MACH_VERSIONS_REGISTER);
> +     u16 generation = boardVersReg & 0xf0;
> +     u16 index      = boardVersReg & 0x0f;
> +     if (generation == HW_GENERATION_HCU3 && index < 0xf)
> +             dram_size = 32 << 20; /* 32 MB - RAM */
> +     else dram_size = 64 << 20;    /* 64 MB - RAM */

        if (generation == HW_GENERATION_HCU3 && index < 0xf)
                dram_size = 32 << 20;   /* 32 MB - RAM */
        else
                dram_size = 64 << 20;   /* 64 MB - RAM */

please.

> +     init_ppc405_sdram(dram_size);
>
>  #ifdef DEBUG
>       show_sdram_registers();
> diff --git a/board/netstal/hcu5/hcu5.c b/board/netstal/hcu5/hcu5.c
> index 2c7afe2..c494e93 100644
> --- a/board/netstal/hcu5/hcu5.c
> +++ b/board/netstal/hcu5/hcu5.c
> @@ -309,15 +309,13 @@ int misc_init_r(void)
>        */
>       if (mfspr(dbcr0) & 0x80000000) {
>               /* External debugger alive
> -              * enable trace facilty for Lauterback
> -              * CCR0[DAPUIB]=0       Enable broadcast of instruction data
> -              *                      to auxiliary processor interface
> +              * enable trace facilty for Lauterbach
>                * CCR0[DTB]=0          Enable broadcast of trace information
>                * SDR0_PFC0[TRE]       Trace signals are enabled instead of
>                *                      GPIO49-63
>                */
> -             mtspr(ccr0, mfspr(ccr0)  &~ 0x00108000);
> -             mtsdr(SDR0_PFC0, sdr0_pfc1 | 0x00000100);
> +             mtspr(ccr0, mfspr(ccr0)  &~ (CCR0_DTB));
> +             mtsdr(SDR0_PFC0, sdr0_pfc1 | SDR0_PFC0_TRE_ENABLE);
>       }
>       return 0;
>  }
> diff --git a/board/netstal/hcu5/sdram.c b/board/netstal/hcu5/sdram.c
> index 5435de1..be9a4cb 100644
> --- a/board/netstal/hcu5/sdram.c
> +++ b/board/netstal/hcu5/sdram.c
> @@ -165,19 +165,25 @@ static void program_ecc(unsigned long start_address,
> unsigned long num_bytes) u32 val;
>       char str[] = "ECC generation -";
>  #if defined(CONFIG_PRAM)
> -     u32 *magic;
> -
> -     /* Check whether vxWorks is using EDR logging, if yes zero */
> -     /* also PostMortem and user reserved memory */
> -     magic = (u32 *)in_be32((u32 *)(start_address + num_bytes -
> -                                    (CONFIG_PRAM*1024) + sizeof(u32)));
> -
> -     debug("\n%s:  CONFIG_PRAM %d kB magic 0x%x 0x%p -> 0x%x\n", 
> __FUNCTION__,
> -            CONFIG_PRAM,
> -            start_address + num_bytes - (CONFIG_PRAM*1024) + sizeof(u32),
> -            magic, in_be32(magic));
> -     if (in_be32(magic) == 0xbeefbabe)
> -             num_bytes -= (CONFIG_PRAM*1024) - PM_RESERVED_MEM;
> +     u32 *magicPtr;
> +     u32 magic;
> +
> +     if ((mfspr(dbcr0) & 0x80000000) == 0) {
> +             /* only if no external debugger is alive!
> +              * Check whether vxWorks is using EDR logging, if yes zero
> +              * also PostMortem and user reserved memory
> +              */
> +             magicPtr =  (u32 *)(start_address + num_bytes -

One space too much after the "=".

> +                             (CONFIG_PRAM*1024) + sizeof(u32));
> +             magic = in_be32(magicPtr);
> +             debug("%s:  CONFIG_PRAM %d kB magic 0x%x 0x%p\n",
> +                   __FUNCTION__, CONFIG_PRAM,
> +                   magicPtr, magic);
> +             if (magic == 0xbeefbabe) {
> +                     printf("%s: preserving at %p\n", __FUNCTION__, 
> magicPtr);
> +                     num_bytes -= (CONFIG_PRAM*1024) - PM_RESERVED_MEM;
> +             }
> +     }
>  #endif
>
>       sync();

Looks good otherwise. Please clean up and resubmit.

Thanks.

Best regards,
Stefan

=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: [EMAIL PROTECTED]
=====================================================================

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
_______________________________________________
U-Boot-Users mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/u-boot-users

Reply via email to