[U-Boot] [PATCH 1/3] Add support for the virtex4fx12 minimodul
Ricardo
ricardo.ribalda at gmail.com
Sun Sep 7 23:50:52 CEST 2008
Hello Georg
First of all, it is great to see more people sending fpgas paths:
Some comments
1) The V4 FX12 MM is by avnet, not by Xilinx, please locate your code
under the folder board/avnet . Maybe it is not in WD branch yet. Take
a look to the ppc4xx board by Stefan
http://git.denx.de/?p=u-boot/u-boot-ppc4xx.git;a=summary
2) We had a big discussions some days ago about how to handle fpga
boards. I have uploaded a new "method" for the virtex5 ppc440 boards.
The idea was to use this method also in Xilinx ppc400.
http://git.denx.de/?p=u-boot/u-boot-ppc4xx.git;a=commit;h=e07f4a8033b6270b8103049adb6456f660ff4a89
To sum up. The idea is to crate 1 generic board for every processor
(ppc440, ppc400 and microblace) and base all the boards with that
processor on that board.
3) Remove all the unnecessary lines in the xparameters file.
4) The xilinx_iic driver will be removed sooner or later. I would
remove the i2c support of your board or create a flattened driver
under /drivers/i2c
5) You are using a "propietary" way of locating the serial number and
mac address. I know that it is the way Xilinx use to do it, but on
u-boot we have a common way of saving that variables using the
environment. On my boards I use it.
Best regards.
On Sat, Sep 6, 2008 at 09:34, Wolfgang Denk <wd at denx.de> wrote:
> Dear Georg Schardt,
>
> In message <12206967521515-git-send-email-schardt at team-ctech.de> you wrote:
>> ---
>> board/xilinx/fx12mm/Makefile | 64 +++++++++++
>> board/xilinx/fx12mm/config.mk | 28 +++++
>> board/xilinx/fx12mm/fx12mm.c | 118 +++++++++++++++++++
>> board/xilinx/fx12mm/init.S | 48 ++++++++
>> board/xilinx/fx12mm/serial.c | 154 +++++++++++++++++++++++++
>> board/xilinx/fx12mm/u-boot.lds | 149 ++++++++++++++++++++++++
>> board/xilinx/fx12mm/xparameters.h | 225 +++++++++++++++++++++++++++++++++++++
>> include/configs/FX12MM.h | 103 +++++++++++++++++
>> 8 files changed, 889 insertions(+), 0 deletions(-)
>
> There are a couple of coding style issues with your code; trailing
> white space, indentation not by TAB, etc. Please clean up.
>
>
>> diff --git a/board/xilinx/fx12mm/Makefile b/board/xilinx/fx12mm/Makefile
>> new file mode 100644
>> index 0000000..61aba24
>> --- /dev/null
>> +++ b/board/xilinx/fx12mm/Makefile
>> @@ -0,0 +1,64 @@
>> +#
>> +# (C) Copyright 2000-2006
>> +# Wolfgang Denk, DENX Software Engineering, wd at denx.de.
>> +#
>> +# 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
>> +ifneq ($(OBJTREE),$(SRCTREE))
>> +$(shell mkdir -p $(obj)../common)
>> +$(shell mkdir -p $(obj)../xilinx_iic)
>> +endif
>> +
>> +INCS := -I../common -I../xilinx_iic
>> +CFLAGS += $(INCS)
>> +HOST_CFLAGS += $(INCS)
>> +
>> +LIB = $(obj)lib$(BOARD).a
>> +
>> +COBJS = $(BOARD).o \
>> + ../common/xbasic_types.o ../common/xdma_channel.o \
>> + ../common/xdma_channel_sg.o \
>> + ../common/xversion.o \
>> + serial.o \
>> +
>> +SOBJS = init.o
>> +
>> +SRCS := $(SOBJS:.o=.S) $(COBJS:.o=.c)
>> +OBJS := $(addprefix $(obj),$(COBJS))
>> +SOBJS := $(addprefix $(obj),$(SOBJS))
>> +
>> +$(LIB): $(OBJS) $(SOBJS)
>> + $(AR) $(ARFLAGS) $@ $^
>> +
>> +clean:
>> + rm -f $(SOBJS) $(OBJS)
>> +
>> +distclean: clean
>> + rm -f $(LIB) core *.bak .depend
>> +
>> +#########################################################################
>> +
>> +# defines $(obj).depend target
>> +include $(SRCTREE)/rules.mk
>> +
>> +sinclude $(obj).depend
>> +
>> +#########################################################################
>> diff --git a/board/xilinx/fx12mm/config.mk b/board/xilinx/fx12mm/config.mk
>> new file mode 100644
>> index 0000000..69490fb
>> --- /dev/null
>> +++ b/board/xilinx/fx12mm/config.mk
>> @@ -0,0 +1,28 @@
>> +#
>> +# (C) Copyright 2000
>> +# Wolfgang Denk, DENX Software Engineering, wd at denx.de.
>> +#
>> +# 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
>> +#
>> +
>> +#
>> +# Memec/Avnet Virtex4FX12 MiniModul
>> +# Standard EDK 10.1 Flash Address
>> +#
>> +TEXT_BASE = 0xFF800000
>> diff --git a/board/xilinx/fx12mm/fx12mm.c b/board/xilinx/fx12mm/fx12mm.c
>> new file mode 100644
>> index 0000000..0f2dd76
>> --- /dev/null
>> +++ b/board/xilinx/fx12mm/fx12mm.c
>> @@ -0,0 +1,118 @@
>> +/*
>> + * fx12mm.c: U-Boot platform support for Avnet/Memec FX12 MiniModul
>> + *
>> + * Author: Xilinx, Inc.
>> + *
>> + *
>> + * 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.
>> + *
>> + *
>> + * XILINX IS PROVIDING THIS DESIGN, CODE, OR INFORMATION "AS IS" AS A
>> + * COURTESY TO YOU. BY PROVIDING THIS DESIGN, CODE, OR INFORMATION AS
>> + * ONE POSSIBLE IMPLEMENTATION OF THIS FEATURE, APPLICATION OR STANDARD,
>> + * XILINX IS MAKING NO REPRESENTATION THAT THIS IMPLEMENTATION IS FREE
>> + * FROM ANY CLAIMS OF INFRINGEMENT, AND YOU ARE RESPONSIBLE FOR OBTAINING
>> + * ANY THIRD PARTY RIGHTS YOU MAY REQUIRE FOR YOUR IMPLEMENTATION.
>> + * XILINX EXPRESSLY DISCLAIMS ANY WARRANTY WHATSOEVER WITH RESPECT TO
>> + * THE ADEQUACY OF THE IMPLEMENTATION, INCLUDING BUT NOT LIMITED TO ANY
>> + * WARRANTIES OR REPRESENTATIONS THAT THIS IMPLEMENTATION IS FREE FROM
>> + * CLAIMS OF INFRINGEMENT, IMPLIED WARRANTIES OF MERCHANTABILITY AND
>> + * FITNESS FOR A PARTICULAR PURPOSE.
>> + *
>> + *
>> + * Xilinx hardware products are not intended for use in life support
>> + * appliances, devices, or systems. Use in such applications is
>> + * expressly prohibited.
>
> This is a restriction in the rights you have with the software which
> is incompatible with the rights the GPL grants you. There must be no
> such restrictions in GPLed code.
>
> The same applies for other files as well.
>
>
>> +int
>> +checkboard(void)
>> +{
>> + char tmp[64]; /* long enough for environment variables */
>> + char *s, *e;
>> + int i = getenv_r("L", tmp, sizeof(tmp));
>> +
>> + if (i < 0) {
>> + printf("### No HW ID - assuming FX12MM");
>> + } else {
>> + for (e = tmp; *e; ++e) {
>> + if (*e == ' ')
>> + break;
>> + }
>> +
>> + printf("### Board Serial# is ");
>> +
>> + for (s = tmp; s < e; ++s) {
>> + putc(*s);
>> + }
>
> No braces for one line loops, please.
>
>> diff --git a/board/xilinx/fx12mm/serial.c b/board/xilinx/fx12mm/serial.c
>> new file mode 100644
>> index 0000000..0c3e868
>> --- /dev/null
>> +++ b/board/xilinx/fx12mm/serial.c
>> @@ -0,0 +1,154 @@
>> +/*
>> + * Author: Xilinx, Inc.
>> + *
>> + *
>> + * 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.
>> + *
>> + *
>> + * XILINX IS PROVIDING THIS DESIGN, CODE, OR INFORMATION "AS IS" AS A
>> + * COURTESY TO YOU. BY PROVIDING THIS DESIGN, CODE, OR INFORMATION AS
>> + * ONE POSSIBLE IMPLEMENTATION OF THIS FEATURE, APPLICATION OR STANDARD,
>> + * XILINX IS MAKING NO REPRESENTATION THAT THIS IMPLEMENTATION IS FREE
>> + * FROM ANY CLAIMS OF INFRINGEMENT, AND YOU ARE RESPONSIBLE FOR OBTAINING
>> + * ANY THIRD PARTY RIGHTS YOU MAY REQUIRE FOR YOUR IMPLEMENTATION.
>> + * XILINX EXPRESSLY DISCLAIMS ANY WARRANTY WHATSOEVER WITH RESPECT TO
>> + * THE ADEQUACY OF THE IMPLEMENTATION, INCLUDING BUT NOT LIMITED TO ANY
>> + * WARRANTIES OR REPRESENTATIONS THAT THIS IMPLEMENTATION IS FREE FROM
>> + * CLAIMS OF INFRINGEMENT, IMPLIED WARRANTIES OF MERCHANTABILITY AND
>> + * FITNESS FOR A PARTICULAR PURPOSE.
>> + *
>> + *
>> + * Xilinx hardware products are not intended for use in life support
>> + * appliances, devices, or systems. Use in such applications is
>> + * expressly prohibited.
>> + *
>> + *
>> + * (c) Copyright 2002-2004 Xilinx Inc.
>> + * All rights reserved.
>> + *
>> + *
>> + * 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.,
>> + * 675 Mass Ave, Cambridge, MA 02139, USA.
>> + *
>> + */
>> +
>> +#include <asm/u-boot.h>
>> +#include <asm/processor.h>
>> +#include <common.h>
>> +#include <command.h>
>> +#include <config.h>
>> +
>> +DECLARE_GLOBAL_DATA_PTR;
>> +
>> +#define USE_CHAN1 \
>> + ((defined XPAR_UARTNS550_0_BASEADDR) && (defined CFG_INIT_CHAN1))
>> +#define USE_CHAN2 \
>> + ((defined XPAR_UARTNS550_1_BASEADDR) && (defined CFG_INIT_CHAN2))
>> +
>> +#if USE_CHAN1
>> +#include <ns16550.h>
>> +#endif
>> +
>> +#if USE_CHAN1
>> +const NS16550_t COM_PORTS[] = { (NS16550_t) (XPAR_UARTNS550_0_BASEADDR + 3)
>> +#if USE_CHAN2
>> + , (NS16550_t) (XPAR_UARTNS550_1_BASEADDR + 3)
>> +#endif
>> +};
>> +#endif
>
> This code is ugly and close to unreadable.
>
>> +int
>> +serial_init(void)
>> +{
>> +#if USE_CHAN1
>> + int clock_divisor;
>> +
>> + clock_divisor = XPAR_UARTNS550_0_CLOCK_FREQ_HZ / 16 / gd->baudrate;
>> + (void) NS16550_init(COM_PORTS[0], clock_divisor);
>> +#if USE_CHAN2
>> + clock_divisor = XPAR_UARTNS550_1_CLOCK_FREQ_HZ / 16 / gd->baudrate;
>> + (void) NS16550_init(COM_PORTS[1], clock_divisor);
>> +#endif
>> +#endif
>> + return 0;
>
> Please get rid of all these #if's in your code.
>
>> index 0000000..c197186
>> --- /dev/null
>> +++ b/board/xilinx/fx12mm/xparameters.h
>> @@ -0,0 +1,225 @@
>> +
>> +/*******************************************************************
>> +*
>> +* CAUTION: This file is automatically generated by libgen.
>> +* Version: Xilinx EDK 10.1.02 EDK_K_SP2.5
>> +* DO NOT EDIT.
>> +*
>> +* Copyright (c) 2005 Xilinx, Inc. All rights reserved.
>> +*
>> +* Description: Driver parameters
>
> Not GPL conformant.
>
>> diff --git a/include/configs/FX12MM.h b/include/configs/FX12MM.h
>> new file mode 100644
>> index 0000000..b47e403
>> --- /dev/null
>> +++ b/include/configs/FX12MM.h
>> @@ -0,0 +1,103 @@
>> +#ifndef __CONFIG_H
>> +#define __CONFIG_H
>> +
>> +
>
> GPL conformant license ,issing.
>
>
> This code needs some major cleanup before it can go into mainline.
>
> 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
> "Who is the oldest inhabitant of this village?"
> "We haven't got one; we had one, but he died three weeks ago."
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>
--
Ricardo Ribalda
http://www.eps.uam.es/~rribalda/
More information about the U-Boot
mailing list