On 14:03 Sat 07 Feb , Dirk Behme wrote:
Dear Jean-Christophe,
Jean-Christophe PLAGNIOL-VILLARD wrote:
Do you like to add some words going to git describing this patch here? E.g. what DCC is and what it might be used for?
You maybe need to read arm documentation
Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD plagnioj@jcrosoft.com
common/devices.c | 3 + drivers/serial/Makefile | 1 + drivers/serial/arm_dcc.c | 270 ++++++++++++++++++++++++++++++++++++++++++++++ include/devices.h | 3 + lib_arm/board.c | 3 + 5 files changed, 280 insertions(+), 0 deletions(-) create mode 100644 drivers/serial/arm_dcc.c
diff --git a/common/devices.c b/common/devices.c index 38f1bbc..ba53c9b 100644 --- a/common/devices.c +++ b/common/devices.c @@ -215,6 +215,9 @@ int devices_init (void) /* Initialize the list */ INIT_LIST_HEAD(&(devs.list)); +#ifdef CONFIG_ARM_DCC_MULTI
- drv_arm_dcc_init ();
+#endif #if defined(CONFIG_HARD_I2C) || defined(CONFIG_SOFT_I2C) i2c_init (CONFIG_SYS_I2C_SPEED, CONFIG_SYS_I2C_SLAVE); #endif diff --git a/drivers/serial/Makefile b/drivers/serial/Makefile index b6fd0d7..af121a2 100644 --- a/drivers/serial/Makefile +++ b/drivers/serial/Makefile @@ -25,6 +25,7 @@ include $(TOPDIR)/config.mk LIB := $(obj)libserial.a +COBJS-$(CONFIG_ARM_DCC) += arm_dcc.o
Above you use CONFIG_ARM_DCC_MULTI, here you use CONFIG_ARM_DCC.
maybe you need to re-read the code it's an option of the driver
Why do you introduce two (new?) macros? Wouldn't one be sufficient?
no you may also need to read u-boot implementation about multiple serial and std serial
which both are implemented here
For which board do want to enable this? There is no enable of these two macros in this patch?
at91 which will come after or nearly any arm cpu
--- /dev/null
+++ b/drivers/serial/arm_dcc.c @@ -0,0 +1,270 @@ +/*
- Copyright (C) 2004-2007 ARM Limited.
- Copyright (C) 2008 Jean-Christophe PLAGNIOL-VILLARD plagnioj@jcrosoft.com
2009?
no I've wrote it in 2008
- This program is free software; you can redistribute it and/or
- modify it under the terms of the GNU General Public License version 2
- as published by the Free Software Foundation.
- 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
- As a special exception, if other files instantiate templates or use macros
- or inline functions from this file, or you compile this file and link it
- with other works to produce a work based on this file, this file does not
- by itself cause the resulting work to be covered by the GNU General Public
- License. However the source code for this file must still be made available
- in accordance with section (3) of the GNU General Public License.
- This exception does not invalidate any other reasons why a work based on
- this file might be covered by the GNU General Public License.
- */
+#include <common.h> +#include <devices.h>
+#define DCC_ARM9_RBIT (1 << 0) +#define DCC_ARM9_WBIT (1 << 1) +#define DCC_ARM10_RBIT (1 << 7) +#define DCC_ARM10_WBIT (1 << 6) +#define DCC_ARM11_RBIT (1 << 30) +#define DCC_ARM11_WBIT (1 << 29)
Should something like this go to a header file?
+#define read_core_id(x) do { \
__asm__ ("mrc p15, 0, %0, c0, c0, 0\n" : "=r" (x)); \x = (x >> 4) & 0xFFF; \What's about macros instead of hard coded values?
no way it's a mask
} while (0);+/*
- ARM9
- */
+#define write_arm9_dcc(x) \
__asm__ volatile ("mcr p14, 0, %0, c1, c0, 0\n" : : "r" (x))+#define read_arm9_dcc(x) \
__asm__ volatile ("mrc p14, 0, %0, c1, c0, 0\n" : "=r" (x))+#define status_arm9_dcc(x) \
__asm__ volatile ("mrc p14, 0, %0, c0, c0, 0\n" : "=r" (x))+#define can_read_arm9_dcc(x) do { \
status_arm9_dcc(x); \x &= DCC_ARM9_RBIT; \} while (0);+#define can_write_arm9_dcc(x) do { \
status_arm9_dcc(x); \x &= DCC_ARM9_WBIT; \x = (x == 0); \} while (0);+/*
- ARM10
- */
+#define write_arm10_dcc(x) \
__asm__ volatile ("mcr p14, 0, %0, c0, c5, 0\n" : : "r" (x))+#define read_arm10_dcc(x) \
__asm__ volatile ("mrc p14, 0, %0, c0, c5, 0\n" : "=r" (x))+#define status_arm10_dcc(x) \
__asm__ volatile ("mrc p14, 0, %0, c0, c1, 0\n" : "=r" (x))+#define can_read_arm10_dcc(x) do { \
status_arm10_dcc(x); \x &= DCC_ARM10_RBIT; \} while (0);+#define can_write_arm10_dcc(x) do { \
status_arm10_dcc(x); \x &= DCC_ARM10_WBIT; \x = (x == 0); \} while (0);+/*
- ARM11
- */
+#define write_arm11_dcc(x) \
__asm__ volatile ("mcr p14, 0, %0, c0, c5, 0\n" : : "r" (x))+#define read_arm11_dcc(x) \
__asm__ volatile ("mrc p14, 0, %0, c0, c5, 0\n" : "=r" (x))+#define status_arm11_dcc(x) \
__asm__ volatile ("mrc p14, 0, %0, c0, c1, 0\n" : "=r" (x))+#define can_read_arm11_dcc(x) do { \
status_arm11_dcc(x); \x &= DCC_ARM11_RBIT; \} while (0);+#define can_write_arm11_dcc(x) do { \
status_arm11_dcc(x); \x &= DCC_ARM11_WBIT; \x = (x == 0); \} while (0);+#define TIMEOUT_COUNT 0x4000000
+static enum {arm9_and_earlier, arm10, arm11_and_later} arm_type = arm9_and_earlier;
+#ifndef CONFIG_ARM_DCC_MULTI +#define arm_dcc_init serial_init +void serial_setbrg(void) {} +#define arm_dcc_getc serial_getc +#define arm_dcc_putc serial_putc +#define arm_dcc_puts serial_puts +#define arm_dcc_tstc serial_tstc +#endif
Why are all these #defines above in a .c file and not in a header file?
maybe because it's dual api
+int arm_dcc_init(void) +{
- register unsigned int id;
- read_core_id(id);
- if ((id & 0xF00) == 0xA00)
What's about macros instead of hard coded values?
arm_type = arm10;- else if (id >= 0xb00)
ditto
arm_type = arm11_and_later;- else
arm_type = arm9_and_earlier;- return 0;
Can this function return something else than 0 ? If not, why then have a return value?
why? it will not fail
- if (rc == 0) {
arm_dcc_init();Here you don't check the return value of arm_dcc_init(), so it makes no sense to have arm_dcc_init() returning anything?
no it will never fail
return 1;- }
Best Regards, J.