[U-Boot-Users] [PATCH]ppc_4xx:netstal/common define routines used by all boards

[Wolfgang Denk]

In message <200801141526.09784.niklausgiger at gmx.ch> you wrote:
> Added some routines used by all Netstal boards  nm_show_print and
> common_misc_init_r and the very specific code to handle our SW installation
> procedure (set_params_for_sw_install).
> 
> Signed-off-by: Niklaus Giger <niklaus.giger at netstal.com>
> ---
>  board/netstal/common/nm_bsp.c |  131 ++++++++++++++++++++++++++++++++++++-----
>  1 files changed, 116 insertions(+), 15 deletions(-)
> 
> diff --git a/board/netstal/common/nm_bsp.c b/board/netstal/common/nm_bsp.c
> index a9de45e..2c1d483 100644
> --- a/board/netstal/common/nm_bsp.c
> +++ b/board/netstal/common/nm_bsp.c
> @@ -20,22 +20,123 @@
>  
>  #include <common.h>
>  #include <command.h>
> +#include <net.h>
>  
> -#ifdef CONFIG_CMD_BSP
> -/*
> - * Command nm_bsp: Netstal Maschinen BSP specific command
> - */
> -int nm_bsp(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +#define DEFAULT_ETH_ADDR  "ethaddr"
> +
> +extern void hcu_led_set(u32 value);
> +extern u32 get_serial_number(void);
> +extern u32 hcu_get_slot(void);
> +
> +enum {
> +	/* HW_GENERATION_HCU1 is no longer supported */
> +	HW_GENERATION_HCU2  = 0x10,
> +	HW_GENERATION_HCU3  = 0x10,
> +	HW_GENERATION_HCU4  = 0x20,
> +	HW_GENERATION_HCU5  = 0x30,
> +	HW_GENERATION_MCU   = 0x08,
> +	HW_GENERATION_MCU20 = 0x0a,
> +	HW_GENERATION_MCU25 = 0x09,
> +};
> +
> +
> +void nm_show_print(int generation, int index, int hw_capabilities)
> +{
> +	int j;
> +	/* reset ANSI terminal color mode */
> +	printf ("\x1B""[0m""Netstal Maschinen AG: ");
> +	if (generation == HW_GENERATION_HCU3)
> +		printf("HCU3: index %d", index);
> +	else if (generation == HW_GENERATION_HCU4)
> +		printf("HCU4: index %d", index);
> +	else if (generation == HW_GENERATION_HCU5)
> +		printf("HCU5: index %d", index);
> +	else if (generation == HW_GENERATION_MCU25)
> +		printf("MCU25: index %d", index);

	else
		???
	
You probably want to do this in a loop over an array of structs
matching "generation" IDs with names - will become more readable and
less error prone. And eventually smaller code, too.

> +	}
> +
> +	/* IP-Adress update */
> +	{
> +		IPaddr_t ipaddr;
> +		char *ipstring;

Please don't do this. Declare the vaiable at entry into this funtion.

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
A direct quote from the Boss: "We passed over a lot of good people to
get the ones we hired."