public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
* [U-Boot] [PATCH v3 1/2] m68k: add support for mcf5307 cpu
@ 2012-11-04 19:59 angelo
  2012-11-04 21:50 ` Wolfgang Denk
  0 siblings, 1 reply; 5+ messages in thread
From: angelo @ 2012-11-04 19:59 UTC (permalink / raw)
  To: u-boot

Add support for freescale coldfire mcf5307 cpu.

Signed-off-by: Angelo Dureghello <sysamfw@gmail.com>
Cc: Jason Jin <jason.jin@freescale.com>
---
Changes for v2:
- Add MAINTAINERS entry
- Add boards.cfg entry
Changes for v3:
- Fix code format issues
---
 MAINTAINERS                        |    4 +
 arch/m68k/cpu/mcf530x/Makefile     |   48 ++++
 arch/m68k/cpu/mcf530x/config.mk    |   30 +++
 arch/m68k/cpu/mcf530x/cpu.c        |   48 ++++
 arch/m68k/cpu/mcf530x/cpu.h        |   33 +++
 arch/m68k/cpu/mcf530x/cpu_init.c   |  160 +++++++++++++
 arch/m68k/cpu/mcf530x/interrupts.c |   43 ++++
 arch/m68k/cpu/mcf530x/speed.c      |   38 +++
 arch/m68k/cpu/mcf530x/start.S      |  263 +++++++++++++++++++++
 arch/m68k/include/asm/cache.h      |    3 +-
 arch/m68k/include/asm/immap.h      |   23 ++
 arch/m68k/include/asm/immap_5307.h |   78 +++++++
 arch/m68k/include/asm/m5307.h      |  127 +++++++++++
 arch/m68k/include/asm/timer.h      |    3 +-
 14 files changed, 899 insertions(+), 2 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 2daee7d..5cb951f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1118,6 +1118,10 @@ Wolfgang Wegner <w.wegner@astro-kom.de>
 
 	astro_mcf5373l	MCF5373L
 
+Angelo Dureghello <sysamfw@gmail.com>
+
+	amcore		mcf5307
+
 #########################################################################
 # AVR32 Systems:							#
 #									#
diff --git a/arch/m68k/cpu/mcf530x/Makefile b/arch/m68k/cpu/mcf530x/Makefile
new file mode 100644
index 0000000..3c5a1c2
--- /dev/null
+++ b/arch/m68k/cpu/mcf530x/Makefile
@@ -0,0 +1,48 @@
+#
+# Copyright (c) 2011 Angelo Dureghello <sysamfw@gmail.com>
+#
+# See file CREDITS for list of people who contributed to this
+# project.
+#
+# 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
+
+# CFLAGS += -DET_DEBUG
+
+LIB	= $(obj)lib$(CPU).o
+
+START	= start.o
+COBJS	= interrupts.o cpu.o speed.o cpu_init.o
+
+SRCS	:= $(START:.o=.S) $(SOBJS:.o=.S) $(COBJS:.o=.c)
+OBJS	:= $(addprefix $(obj),$(SOBJS) $(COBJS))
+START	:= $(addprefix $(obj),$(START))
+
+all:	$(obj).depend $(START) $(LIB)
+
+$(LIB):	$(OBJS)
+	$(call cmd_link_o_target, $(OBJS))
+
+#########################################################################
+
+# defines $(obj).depend target
+include $(SRCTREE)/rules.mk
+
+sinclude $(obj).depend
+
+#########################################################################
diff --git a/arch/m68k/cpu/mcf530x/config.mk b/arch/m68k/cpu/mcf530x/config.mk
new file mode 100644
index 0000000..f7b01c6
--- /dev/null
+++ b/arch/m68k/cpu/mcf530x/config.mk
@@ -0,0 +1,30 @@
+#
+# Copyright (c) 2011 Angelo Dureghello <sysamfw@gmail.com>
+#
+# See file CREDITS for list of people who contributed to this
+# project.
+#
+# 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
+#
+
+PLATFORM_RELFLAGS += -ffixed-d7 -msep-data
+
+cfg=$(shell grep configs $(OBJTREE)/include/config.h | sed 's/.*<\(configs.*\)>/\1/')
+is5307:=$(shell grep CONFIG_M5307 $(TOPDIR)/include/$(cfg))
+
+ifneq (,$(findstring CONFIG_M5307,$(is5307)))
+PLATFORM_CPPFLAGS += -mcpu=5307
+endif
diff --git a/arch/m68k/cpu/mcf530x/cpu.c b/arch/m68k/cpu/mcf530x/cpu.c
new file mode 100644
index 0000000..6f8ca3b
--- /dev/null
+++ b/arch/m68k/cpu/mcf530x/cpu.c
@@ -0,0 +1,48 @@
+/*
+ * Copyright (c) 2011 Angelo Dureghello <sysamfw@gmail.com>
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * 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 <common.h>
+#include <command.h>
+#include <asm/immap.h>
+
+#include "cpu.h"
+
+#ifdef CONFIG_M5307
+int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+{
+	/* enable watchdog, set timeout to 0 and wait */
+	mbar_writeByte(MCFSIM_SYPCR, 0xc0);
+	while (1);
+
+	/* we don't return! */
+	return 0;
+}
+
+int checkcpu(void)
+{
+	char buf[32];
+
+	printf("CPU:   Freescale Coldfire MCF5307 at %s MHz\n",
+	       strmhz(buf, CONFIG_SYS_CLK));
+	return 0;
+}
+#endif
diff --git a/arch/m68k/cpu/mcf530x/cpu.h b/arch/m68k/cpu/mcf530x/cpu.h
new file mode 100644
index 0000000..1accfcc
--- /dev/null
+++ b/arch/m68k/cpu/mcf530x/cpu.h
@@ -0,0 +1,33 @@
+/*
+ *  cpu.h
+ *
+ *  Copyright (c) 2011 Angelo Dureghello <sysamfw@gmail.com>
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * 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., 51 Franklin St, Fifth Floor, Boston,
+ * MA 02110-1301 USA
+ */
+
+#ifndef _CPU_H_
+#define _CPU_H_
+
+#include <command.h>
+
+/* Use this to create board specific reset functions */
+void board_reset(void) __attribute__((__weak__));
+
+#endif /* _CPU_H_ */
diff --git a/arch/m68k/cpu/mcf530x/cpu_init.c b/arch/m68k/cpu/mcf530x/cpu_init.c
new file mode 100644
index 0000000..f4a4ccb
--- /dev/null
+++ b/arch/m68k/cpu/mcf530x/cpu_init.c
@@ -0,0 +1,160 @@
+/*
+ * Copyright (c) 2011 Angelo Dureghello <sysamfw@gmail.com>
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * 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 <common.h>
+#include <watchdog.h>
+#include <asm/immap.h>
+
+#if defined(CONFIG_M5307)
+/*
+ * Simple mcf5307 chip select module init.
+ *
+ * Note: this chip has an issue reported in the device "errata":
+ * MCF5307ER Rev 4.2 reports @ section 35:
+ * Corrupted Return PC in Exception Stack Frame
+ * When processing an autovectored interrupt an error can occur that
+ * causes 0xFFFFFFFF to be written as the return PC value in the
+ * exception stack frame. The problem is caused by a conflict between
+ * an internal autovector access and a chip select mapped to the IACK
+ * address space (0xFFFFXXXX).
+ * Workaround:
+ * Set the C/I bit in the chip select mask register (CSMR) for the
+ * chip select that is mapped to 0xFFFFXXXX.
+ * This will prevent the chip select from asserting for IACK accesses.
+ */
+#define MCF5307_SP_ERR_FIX(cs_base,mask) \
+        if((cs_base+(mask&0xffff0000))>=0xffff0000)mask|=0x20
+
+void init_csm(void)
+{
+	volatile csm_t *csm = (csm_t *) (MMAP_CSM);
+
+#if (defined(CONFIG_SYS_CS0_BASE) && defined(CONFIG_SYS_CS0_MASK) \
+     && defined(CONFIG_SYS_CS0_CTRL))
+	csm->csar0 = (CONFIG_SYS_CS0_BASE>>16);
+	csm->cscr0 = CONFIG_SYS_CS0_CTRL;
+	csm->csmr0 = CONFIG_SYS_CS0_MASK;
+	MCF5307_SP_ERR_FIX(CONFIG_SYS_CS0_BASE,csm->csmr0);
+#else
+#warning "Chip Select 0 are not initialized/used"
+#endif
+#if (defined(CONFIG_SYS_CS1_BASE) && defined(CONFIG_SYS_CS1_MASK) \
+     && defined(CONFIG_SYS_CS1_CTRL))
+	csm->csar1 = (CONFIG_SYS_CS1_BASE>>16);
+	csm->cscr1 = CONFIG_SYS_CS1_CTRL;
+	csm->csmr1 = CONFIG_SYS_CS1_MASK;
+	MCF5307_SP_ERR_FIX(CONFIG_SYS_CS1_BASE,csm->csmr1);
+#endif
+#if (defined(CONFIG_SYS_CS2_BASE) && defined(CONFIG_SYS_CS2_MASK) \
+     && defined(CONFIG_SYS_CS2_CTRL))
+	csm->csar2 = (CONFIG_SYS_CS2_BASE>>16);
+	csm->cscr2 = CONFIG_SYS_CS2_CTRL;
+	csm->csmr2 = CONFIG_SYS_CS2_MASK;
+	MCF5307_SP_ERR_FIX(CONFIG_SYS_CS2_BASE,csm->csmr2);
+#endif
+#if (defined(CONFIG_SYS_CS3_BASE) && defined(CONFIG_SYS_CS3_MASK) \
+     && defined(CONFIG_SYS_CS3_CTRL))
+	csm->csar3 =(CONFIG_SYS_CS3_BASE>>16);
+	csm->cscr3 = CONFIG_SYS_CS3_CTRL;
+	csm->csmr3 = CONFIG_SYS_CS3_MASK;
+	MCF5307_SP_ERR_FIX(CONFIG_SYS_CS3_BASE,csm->csmr3);
+#endif
+#if (defined(CONFIG_SYS_CS4_BASE) && defined(CONFIG_SYS_CS4_MASK) \
+     && defined(CONFIG_SYS_CS4_CTRL))
+	csm->csar4 = (CONFIG_SYS_CS4_BASE>>16);
+	csm->cscr4 = CONFIG_SYS_CS4_CTRL;
+	csm->csmr4 = CONFIG_SYS_CS4_MASK;
+	MCF5307_SP_ERR_FIX(CONFIG_SYS_CS4_BASE,csm->csmr4);
+#endif
+#if (defined(CONFIG_SYS_CS5_BASE) && defined(CONFIG_SYS_CS5_MASK) \
+     && defined(CONFIG_SYS_CS5_CTRL))
+	csm->csar5 = (CONFIG_SYS_CS5_BASE>>16);
+	csm->cscr5 = CONFIG_SYS_CS5_CTRL;
+	csm->csmr5 = CONFIG_SYS_CS5_MASK;
+	MCF5307_SP_ERR_FIX(CONFIG_SYS_CS5_BASE,csm->csmr5);
+#endif
+#if (defined(CONFIG_SYS_CS6_BASE) && defined(CONFIG_SYS_CS6_MASK) \
+     && defined(CONFIG_SYS_CS6_CTRL))
+	csm->csar6 = (CONFIG_SYS_CS6_BASE>>16);
+	csm->cscr6 = CONFIG_SYS_CS6_CTRL;
+	csm->csmr6 = CONFIG_SYS_CS6_MASK;
+	MCF5307_SP_ERR_FIX(CONFIG_SYS_CS6_BASE,csm->csmr6);
+#endif
+#if (defined(CONFIG_SYS_CS7_BASE) && defined(CONFIG_SYS_CS7_MASK) \
+     && defined(CONFIG_SYS_CS7_CTRL))
+	csm->csar7 = (CONFIG_SYS_CS7_BASE>>16);
+	csm->cscr7 = CONFIG_SYS_CS7_CTRL;
+	csm->csmr7 = CONFIG_SYS_CS7_MASK;
+	MCF5307_SP_ERR_FIX(CONFIG_SYS_CS7_BASE,csm->csmr7);
+#endif
+}
+
+/*
+ * Set up the memory map and initialize registers
+ */
+void cpu_init_f(void)
+{
+	mbar_writeByte(MCFSIM_MPARK, 0x00);
+	mbar_writeByte(MCFSIM_SYPCR, 0x00);
+	mbar_writeByte(MCFSIM_SWIVR, 0x0f);
+	mbar_writeByte(MCFSIM_SWSR, 0x00);
+	mbar_writeLong(MCFSIM_IMR, 0xfffffbff);
+	mbar_writeByte(MCFSIM_SWDICR, 0x00);
+	mbar_writeByte(MCFSIM_TIMER1ICR, 0x00);
+	mbar_writeByte(MCFSIM_TIMER2ICR, 0x88);
+	mbar_writeByte(MCFSIM_I2CICR, 0x00);
+	mbar_writeByte(MCFSIM_UART1ICR, 0x00);
+	mbar_writeByte(MCFSIM_UART2ICR, 0x00);
+	mbar_writeByte(MCFSIM_ICR6, 0x00);
+	mbar_writeByte(MCFSIM_ICR7, 0x00);
+	mbar_writeByte(MCFSIM_ICR8, 0x00);
+	mbar_writeByte(MCFSIM_ICR9, 0x00);
+
+	/* Chipselect Init */
+ 	init_csm();
+
+	/* enable instruction cache now */
+	icache_enable();
+}
+
+/*
+ * initialize higher level parts of CPU like timers
+ */
+int cpu_init_r(void)
+{
+	return (0);
+}
+
+void uart_port_conf(int port)
+{
+}
+
+void arch_preboot_os(void)
+{
+	/*
+	 * OS can change interrupt offsets and are about to boot the OS so
+	 * we need to make sure we disable all async interrupts.
+	 */
+	mbar_writeByte(MCFSIM_TIMER1ICR, 0x00);
+	mbar_writeByte(MCFSIM_TIMER2ICR, 0x00);
+}
+#endif
diff --git a/arch/m68k/cpu/mcf530x/interrupts.c b/arch/m68k/cpu/mcf530x/interrupts.c
new file mode 100644
index 0000000..17a438f
--- /dev/null
+++ b/arch/m68k/cpu/mcf530x/interrupts.c
@@ -0,0 +1,43 @@
+/*
+ * Copyright (c) 2011 Angelo Dureghello <sysamfw@gmail.com>
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * 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 <common.h>
+#include <watchdog.h>
+#include <asm/processor.h>
+#include <asm/immap.h>
+
+#ifdef  CONFIG_M5307
+int interrupt_init(void)
+{
+	enable_interrupts();
+
+	return 0;
+}
+
+void dtimer_intr_setup(void)
+{
+	/* clearing TIMER2 mask, so enabling the related interrupt */
+	mbar_writeLong(MCFSIM_IMR, mbar_readLong(MCFSIM_IMR) & ~0x00000400);
+	/* set TIMER2 interrupt priority */
+	mbar_writeByte(MCFSIM_TIMER2ICR, CONFIG_SYS_TMRINTR_PRI);
+}
+#endif
diff --git a/arch/m68k/cpu/mcf530x/speed.c b/arch/m68k/cpu/mcf530x/speed.c
new file mode 100644
index 0000000..42b55dd
--- /dev/null
+++ b/arch/m68k/cpu/mcf530x/speed.c
@@ -0,0 +1,38 @@
+/*
+ * Copyright (c) 2011 Angelo Dureghello <sysamfw@gmail.com>
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * 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 <common.h>
+#include <asm/processor.h>
+#include <asm/immap.h>
+
+DECLARE_GLOBAL_DATA_PTR;
+
+/* get_clocks() fills in gd->cpu_clock and gd->bus_clk */
+int get_clocks (void)
+{
+#if defined(CONFIG_M5307)
+	gd->cpu_clk = CONFIG_SYS_CLK;
+	gd->bus_clk = gd->cpu_clk / 2;
+#endif
+
+	return (0);
+}
diff --git a/arch/m68k/cpu/mcf530x/start.S b/arch/m68k/cpu/mcf530x/start.S
new file mode 100644
index 0000000..576368f
--- /dev/null
+++ b/arch/m68k/cpu/mcf530x/start.S
@@ -0,0 +1,263 @@
+/*
+ * Copyright (c) 2011 Angelo Dureghello <sysamfw@gmail.com>
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * 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 <asm-offsets.h>
+#include <config.h>
+#include <timestamp.h>
+#include "version.h"
+#include <asm/cache.h>
+
+#ifndef	 CONFIG_IDENT_STRING
+#define	 CONFIG_IDENT_STRING ""
+#endif
+
+#define _START	_start
+#define _FAULT	_fault
+
+#define SAVE_ALL						\
+	move.w	#0x2700,%sr;		/* disable intrs */	\
+	subl	#60,%sp;		/* space for 15 regs */ \
+	moveml	%d0-%d7/%a0-%a6,%sp@;				\
+
+#define RESTORE_ALL						\
+	moveml	%sp@,%d0-%d7/%a0-%a6;				\
+	addl	#60,%sp;		/* space for 15 regs */ \
+	rte
+
+/* 
+ * If we come from a pre-loader we don't need an initial exception
+ * table.
+ */
+#if !defined(CONFIG_MONITOR_IS_IN_RAM)
+
+.text
+/*
+ *	Vector table. This is used for initial platform startup.
+ *	These vectors are to catch any un-intended traps.
+ */
+_vectors:
+
+.long	0x00000000	/* Flash offset is 0 until we setup CS0 */
+.long	_START
+
+.long	_FAULT, _FAULT, _FAULT, _FAULT, _FAULT, _FAULT, _FAULT, _FAULT
+.long	_FAULT, _FAULT, _FAULT, _FAULT, _FAULT, _FAULT, _FAULT, _FAULT
+.long	_FAULT, _FAULT, _FAULT, _FAULT, _FAULT, _FAULT, _FAULT, _FAULT
+.long	_FAULT, _FAULT, _FAULT, _FAULT, _FAULT, _FAULT, _FAULT, _FAULT
+.long	_FAULT, _FAULT, _FAULT, _FAULT, _FAULT, _FAULT, _FAULT, _FAULT
+.long	_FAULT, _FAULT, _FAULT, _FAULT, _FAULT, _FAULT, _FAULT, _FAULT
+.long	_FAULT, _FAULT, _FAULT, _FAULT, _FAULT, _FAULT, _FAULT, _FAULT
+.long	_FAULT, _FAULT, _FAULT, _FAULT, _FAULT, _FAULT
+
+.long	_FAULT, _FAULT, _FAULT, _FAULT, _FAULT, _FAULT, _FAULT, _FAULT
+.long	_FAULT, _FAULT, _FAULT, _FAULT, _FAULT, _FAULT, _FAULT, _FAULT
+.long	_FAULT, _FAULT, _FAULT, _FAULT, _FAULT, _FAULT, _FAULT, _FAULT
+.long	_FAULT, _FAULT, _FAULT, _FAULT, _FAULT, _FAULT, _FAULT, _FAULT
+.long	_FAULT, _FAULT, _FAULT, _FAULT, _FAULT, _FAULT, _FAULT, _FAULT
+.long	_FAULT, _FAULT, _FAULT, _FAULT, _FAULT, _FAULT, _FAULT, _FAULT
+.long	_FAULT, _FAULT, _FAULT, _FAULT, _FAULT, _FAULT, _FAULT, _FAULT
+.long	_FAULT, _FAULT, _FAULT, _FAULT, _FAULT, _FAULT, _FAULT, _FAULT
+
+.long	_FAULT, _FAULT, _FAULT, _FAULT, _FAULT, _FAULT, _FAULT, _FAULT
+.long	_FAULT, _FAULT, _FAULT, _FAULT, _FAULT, _FAULT, _FAULT, _FAULT
+.long	_FAULT, _FAULT, _FAULT, _FAULT, _FAULT, _FAULT, _FAULT, _FAULT
+.long	_FAULT, _FAULT, _FAULT, _FAULT, _FAULT, _FAULT, _FAULT, _FAULT
+.long	_FAULT, _FAULT, _FAULT, _FAULT, _FAULT, _FAULT, _FAULT, _FAULT
+.long	_FAULT, _FAULT, _FAULT, _FAULT, _FAULT, _FAULT, _FAULT, _FAULT
+.long	_FAULT, _FAULT, _FAULT, _FAULT, _FAULT, _FAULT, _FAULT, _FAULT
+.long	_FAULT, _FAULT, _FAULT, _FAULT, _FAULT, _FAULT, _FAULT, _FAULT
+
+.long	_FAULT, _FAULT, _FAULT, _FAULT, _FAULT, _FAULT, _FAULT, _FAULT
+.long	_FAULT, _FAULT, _FAULT, _FAULT, _FAULT, _FAULT, _FAULT, _FAULT
+.long	_FAULT, _FAULT, _FAULT, _FAULT, _FAULT, _FAULT, _FAULT, _FAULT
+.long	_FAULT, _FAULT, _FAULT, _FAULT, _FAULT, _FAULT, _FAULT, _FAULT
+.long	_FAULT, _FAULT, _FAULT, _FAULT, _FAULT, _FAULT, _FAULT, _FAULT
+.long	_FAULT, _FAULT, _FAULT, _FAULT, _FAULT, _FAULT, _FAULT, _FAULT
+.long	_FAULT, _FAULT, _FAULT, _FAULT, _FAULT, _FAULT, _FAULT, _FAULT
+.long	_FAULT, _FAULT, _FAULT, _FAULT, _FAULT, _FAULT, _FAULT, _FAULT
+
+#endif
+
+	.text
+	.globl	_start
+_start:
+	nop
+	nop
+	move.w  #0x2700,%sr
+
+	/* set MBAR address + valid flag */
+	move.l	#(CONFIG_SYS_MBAR + 1), %d0		
+	move.c	%d0, %MBAR
+
+	move.l	#(CONFIG_SYS_INIT_RAM_ADDR + 1), %d0
+	move.c	%d0, %RAMBAR
+
+	/* 
+	 * if we come from a pre-loader we have no exception table and
+	 * therefore no VBR to set
+	 */
+#if !defined(CONFIG_MONITOR_IS_IN_RAM)
+	move.l	#CONFIG_SYS_FLASH_BASE, %d0
+	movec	%d0, %VBR
+#endif
+
+	/* initialize general use internal ram */
+	move.l #0, %d0
+	move.l #(ICACHE_STATUS), %a1	/* icache */
+	move.l #(DCACHE_STATUS), %a2	/* icache */
+	move.l %d0, (%a1)
+	move.l %d0, (%a2)
+
+	/* 
+	 * set stackpointer to internal sram end - 80 
+	 * (global data struct size + some bytes)
+	 * get some stackspace for the first c-code,
+	 */
+	move.l	#(CONFIG_SYS_INIT_RAM_ADDR + CONFIG_SYS_INIT_SP_OFFSET), %sp
+	clr.l   %sp at -
+
+	/* put relocation table address to a5 */
+	move.l #__got_start, %a5
+
+	/* run low-level CPU init code (from flash) */
+	bsr cpu_init_f
+
+	/* run low-level board init code (from flash) */
+	bsr board_init_f
+
+	/* board_init_f() does not return */
+
+/*----------------------------------------------------------------------------*/
+
+/*
+ * void relocate_code (addr_sp, gd, addr_moni)
+ *
+ * This "function" does not return, instead it continues in RAM
+ * after relocating the monitor code.
+ *
+ * r3 = dest
+ * r4 = src
+ * r5 = length in bytes
+ * r6 = cachelinesize
+ */
+	.globl	relocate_code
+relocate_code:
+	link.w %a6,#0
+	move.l 8(%a6), %sp	/* set new stack pointer */
+	move.l 12(%a6), %d0	/* Save copy of Global Data pointer */
+	move.l 16(%a6), %a0	/* Save copy of Destination Address */
+
+	move.l #CONFIG_SYS_MONITOR_BASE, %a1
+	move.l #__init_end, %a2
+	move.l %a0, %a3
+
+	/* copy the code to RAM */
+1:
+	move.l (%a1)+, (%a3)+
+	cmp.l  %a1,%a2
+	bgt.s	 1b
+
+/*
+ * We are done. Do not return, instead branch to second part of board
+ * initialization, now running from RAM.
+ */
+	move.l	%a0, %a1
+	add.l	#(in_ram - CONFIG_SYS_MONITOR_BASE), %a1
+	jmp	(%a1)
+
+in_ram:
+
+clear_bss:
+	/*
+	 * Now clear BSS segment
+	 */
+	move.l	%a0, %a1
+	add.l	#(_sbss - CONFIG_SYS_MONITOR_BASE),%a1
+	move.l	%a0, %d1
+	add.l	#(_ebss - CONFIG_SYS_MONITOR_BASE),%d1
+6:
+	clr.l	(%a1)+
+	cmp.l	%a1,%d1
+	bgt.s	6b
+
+	/*
+	 * fix got table in RAM
+	 */
+	move.l	%a0, %a1
+	add.l	#(__got_start - CONFIG_SYS_MONITOR_BASE),%a1
+	/* * fix got pointer register a5 */
+	move.l	%a1,%a5
+
+	move.l	%a0, %a2
+	add.l	#(__got_end - CONFIG_SYS_MONITOR_BASE),%a2
+
+7:
+	move.l	(%a1),%d1
+	sub.l	#_start,%d1
+	add.l	%a0,%d1
+	move.l	%d1,(%a1)+
+	cmp.l	%a2, %a1
+	bne	7b
+
+	/* calculate relative jump to board_init_r in ram */
+	move.l %a0, %a1
+	add.l #(board_init_r - CONFIG_SYS_MONITOR_BASE), %a1
+
+	/* set parameters for board_init_r */
+	move.l %a0,-(%sp)	/* dest_addr */
+	move.l %d0,-(%sp)	/* gd */
+#if defined(DEBUG) && (CONFIG_SYS_TEXT_BASE!=CONFIG_SYS_INT_FLASH_BASE) && \
+    defined(CONFIG_SYS_HALT_BEFOR_RAM_JUMP)
+	halt
+#endif
+	jsr	(%a1)
+
+/*----------------------------------------------------------------------------*/
+/* exception code */
+	.globl _fault
+_fault: 
+	jmp _fault
+
+	.globl	_exc_handler
+_exc_handler:
+	SAVE_ALL
+	movel	%sp,%sp at -
+	bsr 	exc_handler
+	addql	#4,%sp
+	RESTORE_ALL
+
+	.globl	_int_handler
+_int_handler:
+	SAVE_ALL
+	movel	%sp,%sp at -
+	bsr 	int_handler
+	addql	#4,%sp
+	RESTORE_ALL
+
+/*----------------------------------------------------------------------------*/
+
+	.globl	version_string
+version_string:
+	.ascii U_BOOT_VERSION
+	.ascii " (", U_BOOT_DATE, " - ", U_BOOT_TIME, ")"
+	.ascii CONFIG_IDENT_STRING, "\0"
+	.align 4
diff --git a/arch/m68k/include/asm/cache.h b/arch/m68k/include/asm/cache.h
index f9e2d15..dd932c3 100644
--- a/arch/m68k/include/asm/cache.h
+++ b/arch/m68k/include/asm/cache.h
@@ -31,7 +31,8 @@
 #define CONFIG_CF_V2
 #endif
 
-#if defined(CONFIG_MCF532x) || defined(CONFIG_MCF5301x)
+#if defined(CONFIG_MCF530x) || defined(CONFIG_MCF532x) || \
+    defined(CONFIG_MCF5301x)
 #define CONFIG_CF_V3
 #endif
 
diff --git a/arch/m68k/include/asm/immap.h b/arch/m68k/include/asm/immap.h
index 2aab463..5cc81c2 100644
--- a/arch/m68k/include/asm/immap.h
+++ b/arch/m68k/include/asm/immap.h
@@ -256,6 +256,29 @@
 #endif
 #endif				/* CONFIG_M5282 */
 
+#ifdef CONFIG_M5307
+#include <asm/immap_5307.h>
+#include <asm/m5307.h>
+
+#define CONFIG_SYS_UART_BASE            (MMAP_UART0 + \
+					(CONFIG_SYS_UART_PORT * 0x40))
+#define CONFIG_SYS_INTR_BASE            (MMAP_INTC)
+#define CONFIG_SYS_NUM_IRQS             (64)
+
+/* Timer */
+#ifdef CONFIG_MCFTMR
+#define CONFIG_SYS_UDELAY_BASE          (MMAP_DTMR0)
+#define CONFIG_SYS_TMR_BASE             (MMAP_DTMR1)
+#define CONFIG_SYS_TMRPND_REG           (mbar_readLong(MCFSIM_IPR))
+#define CONFIG_SYS_TMRINTR_NO           (31)
+#define CONFIG_SYS_TMRINTR_MASK		(0x00000400)
+#define CONFIG_SYS_TMRINTR_PEND		(CONFIG_SYS_TMRINTR_MASK)
+#define CONFIG_SYS_TMRINTR_PRI          (MCFSIM_ICR_AUTOVEC | \
+					 MCFSIM_ICR_LEVEL7 | MCFSIM_ICR_PRI3)
+#define CONFIG_SYS_TIMER_PRESCALER      (((gd->bus_clk / 1000000) - 1) << 8)
+#endif
+#endif                          /* CONFIG_M5307 */
+
 #if defined(CONFIG_MCF5301x)
 #include <asm/immap_5301x.h>
 #include <asm/m5301x.h>
diff --git a/arch/m68k/include/asm/immap_5307.h b/arch/m68k/include/asm/immap_5307.h
new file mode 100644
index 0000000..24283dc
--- /dev/null
+++ b/arch/m68k/include/asm/immap_5307.h
@@ -0,0 +1,78 @@
+/*
+ * MCF5307 Internal Memory Map
+ *
+ * Copyright (c) 2011 Angelo Dureghello <sysamfw@gmail.com>
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * 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
+ */
+
+#ifndef __IMMAP_5307__
+#define __IMMAP_5307__
+
+#define MMAP_INTC	(CONFIG_SYS_MBAR + 0x00000040)
+#define MMAP_CSM	(CONFIG_SYS_MBAR + 0x00000080)
+#define MMAP_DTMR0	(CONFIG_SYS_MBAR + 0x00000140)
+#define MMAP_DTMR1	(CONFIG_SYS_MBAR + 0x00000180)
+#define MMAP_UART0	(CONFIG_SYS_MBAR + 0x000001C0)
+#define MMAP_UART1	(CONFIG_SYS_MBAR + 0x00000200)
+
+typedef struct csm {
+	u16 csar0; /* Chip-select Address */
+	u16 res0a;
+	u32 csmr0; /* Chip-select Mask */
+	u16 res0b;
+	u16 cscr0; /* Chip-select Control */
+	u16 csar1;
+	u16 res1a;
+	u32 csmr1;
+	u16 res1b;
+	u16 cscr1;
+	u16 csar2;
+	u16 res2a;
+	u32 csmr2;
+	u16 res2b;
+	u16 cscr2;
+	u16 csar3;
+	u16 res3a;
+	u32 csmr3;
+	u16 res3b;
+	u16 cscr3;
+	u16 csar4;
+	u16 res4a;
+	u32 csmr4;
+	u16 res4b;
+	u16 cscr4;
+	u16 csar5;
+	u16 res5a;
+	u32 csmr5;
+	u16 res5b;
+	u16 cscr5;
+	u16 csar6;
+	u16 res6a;
+	u32 csmr6;
+	u16 res6b;
+	u16 cscr6;
+	u16 csar7;
+	u16 res7a;
+	u32 csmr7;
+	u16 res7b;
+	u16 cscr7;
+} csm_t;
+
+#endif				/* __IMMAP_5307__ */
diff --git a/arch/m68k/include/asm/m5307.h b/arch/m68k/include/asm/m5307.h
new file mode 100644
index 0000000..9c69def
--- /dev/null
+++ b/arch/m68k/include/asm/m5307.h
@@ -0,0 +1,127 @@
+/*
+ * mcf5307.h -- Definitions for Motorola Coldfire 5307
+ *
+ * Copyright (c) 2011 Angelo Dureghello <sysamfw@gmail.com>
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * 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
+ */
+
+#ifndef	mcf5307_h
+#define	mcf5307_h
+/****************************************************************************/
+
+/*
+ * useful definitions for reading/writing MBAR offset memory
+ */
+#define mbar_readLong(x) \
+	*((volatile unsigned long *) (CONFIG_SYS_MBAR + x))
+#define mbar_writeLong(x,y) \
+	*((volatile unsigned long *) (CONFIG_SYS_MBAR + x)) = y
+#define mbar_writeShort(x,y) \
+	*((volatile unsigned short *) (CONFIG_SYS_MBAR + x)) = y
+#define mbar_writeByte(x,y) \
+	*((volatile unsigned char *) (CONFIG_SYS_MBAR + x)) = y
+
+/*
+ * Size of internal RAM
+ */
+
+#define INT_RAM_SIZE 4096	/* RAMBAR - 4k */
+
+/*
+ *	Define the 5249 SIM register set addresses.
+ */
+
+/*
+ ***** MBAR  *****
+ */
+#define MCFSIM_RSR		0x00	/* Reset Status reg (r/w) */
+#define MCFSIM_SYPCR		0x01	/* System Protection reg (r/w) */
+#define MCFSIM_SWIVR		0x02	/* SW Watchdog intr reg (r/w) */
+#define MCFSIM_SWSR		0x03	/* SW Watchdog service (r/w) */
+#define MCFSIM_PAR		0x04    /* Parallel pin assignment reg */
+#define MCFSIM_PLLCR		0x08	/* PLL Control register */
+#define MCFSIM_MPARK		0x0c	/* Bus master park register (r/w) */
+
+#define MCFSIM_SIMR		0x00	/* SIM Config reg (r/w) */
+#define MCFSIM_ICR0		0x4c	/* Intr Ctrl reg 0 (r/w) */
+#define MCFSIM_ICR1		0x4d	/* Intr Ctrl reg 1 (r/w) */
+#define MCFSIM_ICR2		0x4e	/* Intr Ctrl reg 2 (r/w) */
+#define MCFSIM_ICR3		0x4f	/* Intr Ctrl reg 3 (r/w) */
+#define MCFSIM_ICR4		0x50	/* Intr Ctrl reg 4 (r/w) */
+#define MCFSIM_ICR5		0x51	/* Intr Ctrl reg 5 (r/w) */
+#define MCFSIM_ICR6		0x52	/* Intr Ctrl reg 6 (r/w) */
+#define MCFSIM_ICR7		0x53	/* Intr Ctrl reg 7 (r/w) */
+#define MCFSIM_ICR8		0x54	/* Intr Ctrl reg 8 (r/w) */
+#define MCFSIM_ICR9		0x55	/* Intr Ctrl reg 9 (r/w) */
+#define MCFSIM_ICR10		0x56	/* Intr Ctrl reg 10 (r/w) */
+#define MCFSIM_ICR11		0x57	/* Intr Ctrl reg 11 (r/w) */
+
+#define MCFSIM_IPR		0x40	/* Interrupt Pend reg (r/w) */
+#define MCFSIM_IMR		0x44	/* Interrupt Mask reg (r/w) */
+
+#define MCFSIM_DCR		0x100	/* DRAM Control reg (r/w) */
+#define MCFSIM_DACR0		0x108	/* DRAM 0 Addr and Ctrl (r/w) */
+#define MCFSIM_DMR0		0x10c	/* DRAM 0 Mask reg (r/w) */
+#define MCFSIM_DACR1		0x110	/* DRAM 1 Addr and Ctrl (r/w) */
+#define MCFSIM_DMR1		0x114	/* DRAM 1 Mask reg (r/w) */
+
+#define MCFSIM_PADDR		0x244   /* Parallel data direction reg */
+#define MCFSIM_PADAT		0x248   /* Parallel data direction reg */
+
+/*
+ *  Some symbol defines for the above...
+ */
+#define	MCFSIM_SWDICR		MCFSIM_ICR0	/* Watchdog timer ICR */
+#define	MCFSIM_TIMER1ICR	MCFSIM_ICR1	/* Timer 1 ICR */
+#define	MCFSIM_TIMER2ICR	MCFSIM_ICR2	/* Timer 2 ICR */
+#define	MCFSIM_I2CICR		MCFSIM_ICR3	/* I2C ICR */
+#define	MCFSIM_UART1ICR		MCFSIM_ICR4	/* UART 1 ICR */
+#define	MCFSIM_UART2ICR		MCFSIM_ICR5	/* UART 2 ICR */
+/* XXX - If needed, DMA ICRs go here */
+
+/*
+ *	Bit definitions for the ICR family of registers.
+ */
+#define	MCFSIM_ICR_AUTOVEC	0x80	/* Auto-vectored intr */
+#define	MCFSIM_ICR_LEVEL0	0x00	/* Level 0 intr */
+#define	MCFSIM_ICR_LEVEL1	0x04	/* Level 1 intr */
+#define	MCFSIM_ICR_LEVEL2	0x08	/* Level 2 intr */
+#define	MCFSIM_ICR_LEVEL3	0x0c	/* Level 3 intr */
+#define	MCFSIM_ICR_LEVEL4	0x10	/* Level 4 intr */
+#define	MCFSIM_ICR_LEVEL5	0x14	/* Level 5 intr */
+#define	MCFSIM_ICR_LEVEL6	0x18	/* Level 6 intr */
+#define	MCFSIM_ICR_LEVEL7	0x1c	/* Level 7 intr */
+
+#define	MCFSIM_ICR_PRI0		0x00	/* Priority 0 intr */
+#define	MCFSIM_ICR_PRI1		0x01	/* Priority 1 intr */
+#define	MCFSIM_ICR_PRI2		0x02	/* Priority 2 intr */
+#define	MCFSIM_ICR_PRI3		0x03	/* Priority 3 intr */
+
+/*
+ *  Macros to read/set IMR register. It is 32 bits on the 5307.
+ */
+
+#define	mcf_getimr() \
+	*((volatile unsigned long *) (MCF_MBAR + MCFSIM_IMR))
+
+#define	mcf_setimr(imr) \
+	*((volatile unsigned long *) (MCF_MBAR + MCFSIM_IMR)) = (imr);
+
+#endif	/* mcf5307_h */
diff --git a/arch/m68k/include/asm/timer.h b/arch/m68k/include/asm/timer.h
index 1a5de05..2638574 100644
--- a/arch/m68k/include/asm/timer.h
+++ b/arch/m68k/include/asm/timer.h
@@ -33,7 +33,8 @@
 /****************************************************************************/
 /* DMA Timer module registers */
 typedef struct dtimer_ctrl {
-#if defined(CONFIG_M5249) || defined(CONFIG_M5253) || defined(CONFIG_M5272)
+#if defined(CONFIG_M5307) || defined(CONFIG_M5249) || \
+    defined(CONFIG_M5253) || defined(CONFIG_M5272)
 	u16 tmr;		/* 0x00 Mode register */
 	u16 res1;		/* 0x02 */
 	u16 trr;		/* 0x04 Reference register */

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [U-Boot] [PATCH v3 1/2] m68k: add support for mcf5307 cpu
  2012-11-04 19:59 [U-Boot] [PATCH v3 1/2] m68k: add support for mcf5307 cpu angelo
@ 2012-11-04 21:50 ` Wolfgang Denk
  2012-11-05 14:50   ` Angelo Dureghello
  2012-11-05 20:52   ` Angelo Dureghello
  0 siblings, 2 replies; 5+ messages in thread
From: Wolfgang Denk @ 2012-11-04 21:50 UTC (permalink / raw)
  To: u-boot

Dear angelo,

In message <20121104195901.GA5141@angel3> you wrote:
> Add support for freescale coldfire mcf5307 cpu.
...
> --- /dev/null
> +++ b/arch/m68k/cpu/mcf530x/cpu_init.c
...
> +#define MCF5307_SP_ERR_FIX(cs_base,mask) \
> +        if((cs_base+(mask&0xffff0000))>=0xffff0000)mask|=0x20

Please never do this.  Please ALWAYS use the standard "do { ... }
while (0)" construct to make sure such macros can be used savely in
any call envionment.

> +void init_csm(void)
> +{
> +	volatile csm_t *csm = (csm_t *) (MMAP_CSM);

NAK.  Please so not use volatile.  Hey, did you run your ptches
through checkpatch?  What did it say?


> +#if (defined(CONFIG_SYS_CS0_BASE) && defined(CONFIG_SYS_CS0_MASK) \
> +     && defined(CONFIG_SYS_CS0_CTRL))
> +	csm->csar0 = (CONFIG_SYS_CS0_BASE>>16);
> +	csm->cscr0 = CONFIG_SYS_CS0_CTRL;
> +	csm->csmr0 = CONFIG_SYS_CS0_MASK;
> +	MCF5307_SP_ERR_FIX(CONFIG_SYS_CS0_BASE,csm->csmr0);

We do not allow accesses to device registers through plain (even
volatile) pointers.  Please make sure to use proper I/O accessors
instead.

...
> +/*
> + *	Define the 5249 SIM register set addresses.
> + */
> +
> +/*
> + ***** MBAR  *****
> + */
> +#define MCFSIM_RSR		0x00	/* Reset Status reg (r/w) */
> +#define MCFSIM_SYPCR		0x01	/* System Protection reg (r/w) */
> +#define MCFSIM_SWIVR		0x02	/* SW Watchdog intr reg (r/w) */
> +#define MCFSIM_SWSR		0x03	/* SW Watchdog service (r/w) */
> +#define MCFSIM_PAR		0x04    /* Parallel pin assignment reg */
> +#define MCFSIM_PLLCR		0x08	/* PLL Control register */
> +#define MCFSIM_MPARK		0x0c	/* Bus master park register (r/w) */
> +
> +#define MCFSIM_SIMR		0x00	/* SIM Config reg (r/w) */
> +#define MCFSIM_ICR0		0x4c	/* Intr Ctrl reg 0 (r/w) */
> +#define MCFSIM_ICR1		0x4d	/* Intr Ctrl reg 1 (r/w) */
> +#define MCFSIM_ICR2		0x4e	/* Intr Ctrl reg 2 (r/w) */
> +#define MCFSIM_ICR3		0x4f	/* Intr Ctrl reg 3 (r/w) */
> +#define MCFSIM_ICR4		0x50	/* Intr Ctrl reg 4 (r/w) */
> +#define MCFSIM_ICR5		0x51	/* Intr Ctrl reg 5 (r/w) */
> +#define MCFSIM_ICR6		0x52	/* Intr Ctrl reg 6 (r/w) */
> +#define MCFSIM_ICR7		0x53	/* Intr Ctrl reg 7 (r/w) */
> +#define MCFSIM_ICR8		0x54	/* Intr Ctrl reg 8 (r/w) */
> +#define MCFSIM_ICR9		0x55	/* Intr Ctrl reg 9 (r/w) */
> +#define MCFSIM_ICR10		0x56	/* Intr Ctrl reg 10 (r/w) */
> +#define MCFSIM_ICR11		0x57	/* Intr Ctrl reg 11 (r/w) */
> +
> +#define MCFSIM_IPR		0x40	/* Interrupt Pend reg (r/w) */
> +#define MCFSIM_IMR		0x44	/* Interrupt Mask reg (r/w) */
> +
> +#define MCFSIM_DCR		0x100	/* DRAM Control reg (r/w) */
> +#define MCFSIM_DACR0		0x108	/* DRAM 0 Addr and Ctrl (r/w) */
> +#define MCFSIM_DMR0		0x10c	/* DRAM 0 Mask reg (r/w) */
> +#define MCFSIM_DACR1		0x110	/* DRAM 1 Addr and Ctrl (r/w) */
> +#define MCFSIM_DMR1		0x114	/* DRAM 1 Mask reg (r/w) */
> +
> +#define MCFSIM_PADDR		0x244   /* Parallel data direction reg */
> +#define MCFSIM_PADAT		0x248   /* Parallel data direction reg */

We don't allow any base address + offset notation. Please define a
proper C structure instead.

Please fix globally.


> -#if defined(CONFIG_M5249) || defined(CONFIG_M5253) || defined(CONFIG_M5272)
> +#if defined(CONFIG_M5307) || defined(CONFIG_M5249) || \
> +    defined(CONFIG_M5253) || defined(CONFIG_M5272)

Please keep the list sorted.

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
I believe you find life such a problem because you  think  there  are
the  good  people  and the bad people. You're wrong, of course. There
are, always and only, the bad people, but some of them are  on  oppo-
site sides.                      - Terry Pratchett, _Guards! Guards!_

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [U-Boot] [PATCH v3 1/2] m68k: add support for mcf5307 cpu
  2012-11-04 21:50 ` Wolfgang Denk
@ 2012-11-05 14:50   ` Angelo Dureghello
  2012-11-05 20:52   ` Angelo Dureghello
  1 sibling, 0 replies; 5+ messages in thread
From: Angelo Dureghello @ 2012-11-05 14:50 UTC (permalink / raw)
  To: u-boot

Hi Wolfgang,

thanks for reviewing.
See my comments blow.


On Sun, Nov 04, 2012 at 10:50:14PM +0100, Wolfgang Denk wrote:
> Dear angelo,
> 
> In message <20121104195901.GA5141@angel3> you wrote:
> > Add support for freescale coldfire mcf5307 cpu.
> ...
> > --- /dev/null
> > +++ b/arch/m68k/cpu/mcf530x/cpu_init.c
> ...
> > +#define MCF5307_SP_ERR_FIX(cs_base,mask) \
> > +        if((cs_base+(mask&0xffff0000))>=0xffff0000)mask|=0x20
> 

Done.

> Please never do this.  Please ALWAYS use the standard "do { ... }
> while (0)" construct to make sure such macros can be used savely in
> any call envionment.
> 
> > +void init_csm(void)
> > +{
> > +	volatile csm_t *csm = (csm_t *) (MMAP_CSM);
> 
> NAK.  Please so not use volatile.  Hey, did you run your ptches
> through checkpatch?  What did it say?

Ok, i forgot pass through checkpatch, will do it always.
 
> 
> > +#if (defined(CONFIG_SYS_CS0_BASE) && defined(CONFIG_SYS_CS0_MASK) \
> > +     && defined(CONFIG_SYS_CS0_CTRL))
> > +	csm->csar0 = (CONFIG_SYS_CS0_BASE>>16);
> > +	csm->cscr0 = CONFIG_SYS_CS0_CTRL;
> > +	csm->csmr0 = CONFIG_SYS_CS0_MASK;
> > +	MCF5307_SP_ERR_FIX(CONFIG_SYS_CS0_BASE,csm->csmr0);
> 
> We do not allow accesses to device registers through plain (even
> volatile) pointers.  Please make sure to use proper I/O accessors
> instead.

Done.

> ...
> > +/*
> > + *	Define the 5249 SIM register set addresses.
> > + */
> > +
> > +/*
> > + ***** MBAR  *****
> > + */
> > +#define MCFSIM_RSR		0x00	/* Reset Status reg (r/w) */
> > +#define MCFSIM_SYPCR		0x01	/* System Protection reg (r/w) */
> > +#define MCFSIM_SWIVR		0x02	/* SW Watchdog intr reg (r/w) */
> > +#define MCFSIM_SWSR		0x03	/* SW Watchdog service (r/w) */
> > +#define MCFSIM_PAR		0x04    /* Parallel pin assignment reg */
> > +#define MCFSIM_PLLCR		0x08	/* PLL Control register */
> > +#define MCFSIM_MPARK		0x0c	/* Bus master park register (r/w) */
> > +
> > +#define MCFSIM_SIMR		0x00	/* SIM Config reg (r/w) */
> > +#define MCFSIM_ICR0		0x4c	/* Intr Ctrl reg 0 (r/w) */
> > +#define MCFSIM_ICR1		0x4d	/* Intr Ctrl reg 1 (r/w) */
> > +#define MCFSIM_ICR2		0x4e	/* Intr Ctrl reg 2 (r/w) */
> > +#define MCFSIM_ICR3		0x4f	/* Intr Ctrl reg 3 (r/w) */
> > +#define MCFSIM_ICR4		0x50	/* Intr Ctrl reg 4 (r/w) */
> > +#define MCFSIM_ICR5		0x51	/* Intr Ctrl reg 5 (r/w) */
> > +#define MCFSIM_ICR6		0x52	/* Intr Ctrl reg 6 (r/w) */
> > +#define MCFSIM_ICR7		0x53	/* Intr Ctrl reg 7 (r/w) */
> > +#define MCFSIM_ICR8		0x54	/* Intr Ctrl reg 8 (r/w) */
> > +#define MCFSIM_ICR9		0x55	/* Intr Ctrl reg 9 (r/w) */
> > +#define MCFSIM_ICR10		0x56	/* Intr Ctrl reg 10 (r/w) */
> > +#define MCFSIM_ICR11		0x57	/* Intr Ctrl reg 11 (r/w) */
> > +
> > +#define MCFSIM_IPR		0x40	/* Interrupt Pend reg (r/w) */
> > +#define MCFSIM_IMR		0x44	/* Interrupt Mask reg (r/w) */
> > +
> > +#define MCFSIM_DCR		0x100	/* DRAM Control reg (r/w) */
> > +#define MCFSIM_DACR0		0x108	/* DRAM 0 Addr and Ctrl (r/w) */
> > +#define MCFSIM_DMR0		0x10c	/* DRAM 0 Mask reg (r/w) */
> > +#define MCFSIM_DACR1		0x110	/* DRAM 1 Addr and Ctrl (r/w) */
> > +#define MCFSIM_DMR1		0x114	/* DRAM 1 Mask reg (r/w) */
> > +
> > +#define MCFSIM_PADDR		0x244   /* Parallel data direction reg */
> > +#define MCFSIM_PADAT		0x248   /* Parallel data direction reg */
> 
> We don't allow any base address + offset notation. Please define a
> proper C structure instead.
> 
> Please fix globally.

Clear, will fix it.
> 
> 
> > -#if defined(CONFIG_M5249) || defined(CONFIG_M5253) || defined(CONFIG_M5272)
> > +#if defined(CONFIG_M5307) || defined(CONFIG_M5249) || \
> > +    defined(CONFIG_M5253) || defined(CONFIG_M5272)
> 
> Please keep the list sorted.
> 

Done.

> 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
> I believe you find life such a problem because you  think  there  are
> the  good  people  and the bad people. You're wrong, of course. There
> are, always and only, the bad people, but some of them are  on  oppo-
> site sides.                      - Terry Pratchett, _Guards! Guards!_

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [U-Boot] [PATCH v3 1/2] m68k: add support for mcf5307 cpu
  2012-11-04 21:50 ` Wolfgang Denk
  2012-11-05 14:50   ` Angelo Dureghello
@ 2012-11-05 20:52   ` Angelo Dureghello
  2012-11-06  7:21     ` Wolfgang Denk
  1 sibling, 1 reply; 5+ messages in thread
From: Angelo Dureghello @ 2012-11-05 20:52 UTC (permalink / raw)
  To: u-boot

Hi Wolfgang,

i have still a question here, please see below.

On Sun, Nov 04, 2012 at 10:50:14PM +0100, Wolfgang Denk wrote:
> Dear angelo,
> 
> In message <20121104195901.GA5141@angel3> you wrote:
> > Add support for freescale coldfire mcf5307 cpu.
> ...
> > --- /dev/null
> > +++ b/arch/m68k/cpu/mcf530x/cpu_init.c
> ...
> > +#define MCF5307_SP_ERR_FIX(cs_base,mask) \
> > +        if((cs_base+(mask&0xffff0000))>=0xffff0000)mask|=0x20
> 
> Please never do this.  Please ALWAYS use the standard "do { ... }
> while (0)" construct to make sure such macros can be used savely in
> any call envionment.
> 
> > +void init_csm(void)
> > +{
> > +	volatile csm_t *csm = (csm_t *) (MMAP_CSM);
> 
> NAK.  Please so not use volatile.  Hey, did you run your ptches
> through checkpatch?  What did it say?
> 
> 
> > +#if (defined(CONFIG_SYS_CS0_BASE) && defined(CONFIG_SYS_CS0_MASK) \
> > +     && defined(CONFIG_SYS_CS0_CTRL))
> > +	csm->csar0 = (CONFIG_SYS_CS0_BASE>>16);
> > +	csm->cscr0 = CONFIG_SYS_CS0_CTRL;
> > +	csm->csmr0 = CONFIG_SYS_CS0_MASK;
> > +	MCF5307_SP_ERR_FIX(CONFIG_SYS_CS0_BASE,csm->csmr0);
> 
> We do not allow accesses to device registers through plain (even
> volatile) pointers.  Please make sure to use proper I/O accessors
> instead.
> 
> ...
> > +/*
> > + *	Define the 5249 SIM register set addresses.
> > + */
> > +
> > +/*
> > + ***** MBAR  *****
> > + */
> > +#define MCFSIM_RSR		0x00	/* Reset Status reg (r/w) */
> > +#define MCFSIM_SYPCR		0x01	/* System Protection reg (r/w) */
> > +#define MCFSIM_SWIVR		0x02	/* SW Watchdog intr reg (r/w) */
> > +#define MCFSIM_SWSR		0x03	/* SW Watchdog service (r/w) */
> > +#define MCFSIM_PAR		0x04    /* Parallel pin assignment reg */
> > +#define MCFSIM_PLLCR		0x08	/* PLL Control register */
> > +#define MCFSIM_MPARK		0x0c	/* Bus master park register (r/w) */
> > +
> > +#define MCFSIM_SIMR		0x00	/* SIM Config reg (r/w) */
> > +#define MCFSIM_ICR0		0x4c	/* Intr Ctrl reg 0 (r/w) */
> > +#define MCFSIM_ICR1		0x4d	/* Intr Ctrl reg 1 (r/w) */
> > +#define MCFSIM_ICR2		0x4e	/* Intr Ctrl reg 2 (r/w) */
> > +#define MCFSIM_ICR3		0x4f	/* Intr Ctrl reg 3 (r/w) */
> > +#define MCFSIM_ICR4		0x50	/* Intr Ctrl reg 4 (r/w) */
> > +#define MCFSIM_ICR5		0x51	/* Intr Ctrl reg 5 (r/w) */
> > +#define MCFSIM_ICR6		0x52	/* Intr Ctrl reg 6 (r/w) */
> > +#define MCFSIM_ICR7		0x53	/* Intr Ctrl reg 7 (r/w) */
> > +#define MCFSIM_ICR8		0x54	/* Intr Ctrl reg 8 (r/w) */
> > +#define MCFSIM_ICR9		0x55	/* Intr Ctrl reg 9 (r/w) */
> > +#define MCFSIM_ICR10		0x56	/* Intr Ctrl reg 10 (r/w) */
> > +#define MCFSIM_ICR11		0x57	/* Intr Ctrl reg 11 (r/w) */
> > +
> > +#define MCFSIM_IPR		0x40	/* Interrupt Pend reg (r/w) */
> > +#define MCFSIM_IMR		0x44	/* Interrupt Mask reg (r/w) */
> > +
> > +#define MCFSIM_DCR		0x100	/* DRAM Control reg (r/w) */
> > +#define MCFSIM_DACR0		0x108	/* DRAM 0 Addr and Ctrl (r/w) */
> > +#define MCFSIM_DMR0		0x10c	/* DRAM 0 Mask reg (r/w) */
> > +#define MCFSIM_DACR1		0x110	/* DRAM 1 Addr and Ctrl (r/w) */
> > +#define MCFSIM_DMR1		0x114	/* DRAM 1 Mask reg (r/w) */
> > +
> > +#define MCFSIM_PADDR		0x244   /* Parallel data direction reg */
> > +#define MCFSIM_PADAT		0x248   /* Parallel data direction reg */
> 
> We don't allow any base address + offset notation. Please define a
> proper C structure instead.
> 
> Please fix globally.
> 


I used this approach looking all other coldfire cpu files, and this notation
seems accepted.
I started m5307.h looking those files (i.e. m5249.h) maintaing the same scheme.

Let me know how i have to proceed.

Best Regards,
Angelo Dureghello

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [U-Boot] [PATCH v3 1/2] m68k: add support for mcf5307 cpu
  2012-11-05 20:52   ` Angelo Dureghello
@ 2012-11-06  7:21     ` Wolfgang Denk
  0 siblings, 0 replies; 5+ messages in thread
From: Wolfgang Denk @ 2012-11-06  7:21 UTC (permalink / raw)
  To: u-boot

Dear Angelo,

In message <20121105205220.GA24682@angel3> you wrote:
> 
> > We don't allow any base address + offset notation. Please define a
> > proper C structure instead.
...
> I used this approach looking all other coldfire cpu files, and this notation
> seems accepted.
> I started m5307.h looking those files (i.e. m5249.h) maintaing the same scheme.
> 
> Let me know how i have to proceed.

We have been less strict in the past, but we learned a lesson and
moved on.

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
The existence of god implies a violation of causality.

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2012-11-06  7:21 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-11-04 19:59 [U-Boot] [PATCH v3 1/2] m68k: add support for mcf5307 cpu angelo
2012-11-04 21:50 ` Wolfgang Denk
2012-11-05 14:50   ` Angelo Dureghello
2012-11-05 20:52   ` Angelo Dureghello
2012-11-06  7:21     ` Wolfgang Denk

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox