[U-Boot-Users] [PATCH 3/3 resubmit] ppc_4xx: Add MCU25 board specific failes

[Stefan Roese <sr@denx.de>]

Hi Niklaus,

On Monday 21 January 2008, Niklaus Giger wrote:
> Add PPC405GPr based MCU25 board from Netstal.
>
> Signed-off-by: Niklaus Giger <niklaus.giger@netstal.com>
> ---
>  board/netstal/mcu25/Makefile   |   60 +++++++++++
>  board/netstal/mcu25/README.txt |   60 +++++++++++
>  board/netstal/mcu25/config.mk  |   29 ++++++
>  board/netstal/mcu25/mcu25.c    |  212
> ++++++++++++++++++++++++++++++++++++++++ board/netstal/mcu25/u-boot.lds | 
> 141 ++++++++++++++++++++++++++
>  5 files changed, 502 insertions(+), 0 deletions(-)
>  create mode 100644 board/netstal/mcu25/Makefile
>  create mode 100644 board/netstal/mcu25/README.txt
>  create mode 100644 board/netstal/mcu25/config.mk
>  create mode 100644 board/netstal/mcu25/mcu25.c
>  create mode 100644 board/netstal/mcu25/u-boot.lds
>
> diff --git a/board/netstal/mcu25/Makefile b/board/netstal/mcu25/Makefile
> new file mode 100644
> index 0000000..f367f34
> --- /dev/null
> +++ b/board/netstal/mcu25/Makefile
> @@ -0,0 +1,60 @@
> +#
> +# (C) Copyright 2008 Netstal Maschinen AG
> +# Niklaus Giger (ng at netstal.com)
> +#
> +# This program is free software; you can redistribute it and/or
> +# modify it under the terms of the GNU General Public License as
> +# published by the Free Software Foundation; either version 2 of
> +# the License, or (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program; if not, write to the Free Software
> +# Foundation, Inc., 59 Temple Place, Suite 330, Boston,
> +# MA 02111-1307 USA
> +#
> +
> +include $(TOPDIR)/config.mk
> +
> +LIB	= $(obj)lib$(BOARD).a
> +
> +vpath fixed_sdram.c ../common
> +vpath hcu_flash.c ../common
> +vpath nm_bsp.c ../common
> +
> +# NOBJS : Netstal common objects
> +NOBJS	= fixed_sdram.o hcu_flash.o nm_bsp.o
> +COBJS	= $(BOARD).o
> +SOBJS	=
> +
> +SRCS	:= $(SOBJS:.o=.S) $(COBJS:.o=.c) ../common/$(NOBJS:.o=.c)

This patch introduces the same compilation problems as known from
the HCU4/5 boards:

[stefan at ubuntu u-boot-ppc4xx (for-1.3.2)]$ ./MAKEALL mcu25
Configuring for mcu25 board...
ppc_4xx-gcc: hcu_flash.c: No such file or directory
ppc_4xx-gcc: no input files
ppc_4xx-gcc: nm_bsp.c: No such file or directory
ppc_4xx-gcc: no input files

Please fix.

<snip>

> +++ b/board/netstal/mcu25/mcu25.c
> @@ -0,0 +1,212 @@
> +/*
> + *(C) Copyright 2005-2008 Netstal Maschinen AG
> + * ? ?Niklaus Giger (Niklaus.Giger at netstal.com)
> + *
> + * ? ?This source code is free software; you can redistribute it
> + * ? ?and/or modify it in source code form under the terms of the GNU
> + * ? ?General Public License as published by the Free Software
> + * ? ?Foundation; either version 2 of the License, or (at your option)
> + * ? ?any later version.
> + *
> + * ? ?This program is distributed in the hope that it will be useful,
> + * ? ?but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * ? ?MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. ?See the
> + * ? ?GNU General Public License for more details.
> + *
> + * ? ?You should have received a copy of the GNU General Public License
> + * ? ?along with this program; if not, write to the Free Software
> + * ? ?Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA
> 02111-1307, USA + */
> +
> +#include ?<common.h>
> +#include ?<ppc4xx.h>
> +#include ?<asm/processor.h>
> +#include ?<asm/io.h>
> +#include ?<asm-ppc/u-boot.h>
> +#include ?"../common/nm.h"
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +#define MCU25_SLOT_ADDRESS?????????????(0x7A000000 + 0x0A)
> +#define MCU25_DIGITAL_IO_REGISTER??????(0x7A000000 + 0xc0)
> +
> +#define MCU25_LED_REGISTER_ADDRESS?????(0x7C000000 + 0x10)
> +#define MCU25_VERSIONS_REGISTER????????(0x7C000000 + 0x0C)
> +#define MCU25_IO_CONFIGURATION?????????(0x7C000000 + 0x0e)
> +#define MCU_SW_INSTALL_REQUESTED???????0x08
> +#define HW_GENERATION_MCU25????????????0x09
> +
> +#define SDRAM_LEN 32*1024*1024 /* 32 MB -RAM */

I suggest to always use parentheses around such defines:

How about this version:

#define SDRAM_LEN	(32 << 20)	/* 32 MB - RAM */

> +
> +#undef DEBUG
> +/*
> + * 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
> + * of being in the init_sequence array.
> + *
> + * The SDRAM has been initialized already -- start.S:start called
> + * init.S:init_sdram early on -- but it is not yet being used for
> + * anything, not even stack. So be careful.
> + */
> +
> +#define CPC0_CR0 ? ? ? ?0xb1???/* Chip control register 0 */
> +#define CPC0_CR1 ? ? ? ?0xb2???/* Chip control register 1 */

Those should be defined in include/ppc405.h

> +/* Attention: If you want 1 microsecs times from the external oscillator
> + * 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?0x0007F03C
> +#define CPC0_CR1_VALUE?0x00004051
> +#define CPC0_ECR???????0xaa????/* Edge condition register */
> +#define EBC0_CFG???????0x23????/* External Peripheral Control Register */

This is already defined:

include/ppc405.h:#define EBC0_CFG       0x23    /* external bus configuration reg       */

Could be that this definition is restricted for another PPC405
version. Please clean this up and use the general define.

> +#define CPC0_EIRR??????0xb6????/* External Interrupt Register */
> +
> +#if defined(DEBUG)
> +void show_sdram_registers(void);
> +#endif
> +long int fixed_hcu4_sdram (unsigned int dram_size);
> +
> +int board_early_init_f (void)
> +{
> +???????/* Documented in A-1171
> +??????? *
> +??????? * Interrupt controller setup for the MCU25 board.
> +??????? * Note: IRQ 0-15 ?405GP internally generated; high; level
> sensitive +??????? * ? ? ? IRQ 16 ? ?405GP internally generated; low; level
> sensitive +??????? * ? ? ?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(CPC0_CR1, ?CPC0_CR1_VALUE);

You are mixing different conding styles here:

	funcabc();

and

	funcabc ();

Please only use one style in a file. My preference is the Linux version
without the space.

> +???????mtdcr(CPC0_ECR, ?0x60606000);
> +???????mtdcr(CPC0_EIRR, 0x7C000000);
> +???????out32(GPIO0_OR,?????????CFG_GPIO0_OR );
> + ? ? ? out32(GPIO0_TCR,????????CFG_GPIO0_TCR);
> + ? ? ? out32(GPIO0_ODR,????????CFG_GPIO0_ODR);
> +???????mtspr(ccr0, ? ? ?0x00700000);

Alignment doesn't look good to me.

> +
> +???????return 0;
> +}
> +
> +#ifdef CONFIG_BOARD_PRE_INIT
> +int board_pre_init (void)
> +{
> +???????return board_early_init_f ();
> +}
> +#endif
> +
> +int sys_install_requested(void)
> +{
> +???????u16 *ioValuePtr = (u16 *)MCU25_DIGITAL_IO_REGISTER;
> +???????return (*ioValuePtr & MCU_SW_INSTALL_REQUESTED) != 0;

Use in_be16() here.

> +}
> +
> +int checkboard (void)
> +{
> +???????u16 *boardVersReg = (u16 *) MCU25_VERSIONS_REGISTER;
> +???????u16 *hwConfig ?= (u16 *) MCU25_IO_CONFIGURATION;
> +???????u16 generation = *boardVersReg & 0x0f;
> +???????u16 index ? ? ?= *boardVersReg & 0xf0;

Empty line missing here.

And please use the correct accessor functions: in_be16().

> +???????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. */

Wrong multi-line comment style.

> +???????writeb (readb (0xef600404) | 0x03, 0xef600404);

in_8() and out_8() please.

> +???????/* reset ANSI terminal color mode */
> +???????nm_show_print(generation, index, *hwConfig);
> +???????return 0;
> +}
> +
> +u32 hcu_led_get(void)
> +{
> +???????return (*(u16 *)MCU25_LED_REGISTER_ADDRESS)& 0x3ff;

in_be16()

> +}
> +
> +/*
> + * hcu_led_set ?value to be placed into the LEDs (max 6 bit)
> + */
> +void hcu_led_set(u32 value)
> +{
> + ? *(u16 *)MCU25_LED_REGISTER_ADDRESS = value;

in_be16()

> +}
> +
> +/*
> + * sdram_init - Dummy implementation for start.S, spd_sdram ?or initdram
> + *?????????????used for HCUx
> + */
> +void sdram_init(void)
> +{
> +???????return;
> +}
> +
> +/*
> + * hcu_get_slot
> + */
> +u32 hcu_get_slot(void)
> +{
> +???????u16 *slot = (u16 *)MCU25_SLOT_ADDRESS;
> +???????return (*slot) & 0x7f;

in_be16()

> +}
> +
> +/*
> + * get_serial_number
> + */
> +u32 get_serial_number(void)
> +{
> +???????u32 *serial = (u32 *)CFG_FLASH_BASE;
> +
> +???????if (*serial == 0xffffffff)
> +???????????????return 0;
> +
> +???????return *serial;
> +}
> +
> +
> +/*
> + * misc_init_r.
> + */
> +
> +int misc_init_r(void)
> +{
> +???????common_misc_init_r();
> +???????set_params_for_sw_install( sys_install_requested(), "mcu25" );
> +???????return 0;
> +}
> +
> +long int initdram(int board_type)
> +{
> +???????long int dram_size = 64*1024*1024;

So here is different dram_size than defined above? Why the define then?

Please fix 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: office at denx.de
=====================================================================