Re: [U-Boot] [patch V2] U-Boot Firetux board support

2008-11-13 Thread Jürgen Schöw
Hi Jean-Christophe PLAGNIOL-VILLARD,

sorry to answer late. I've been on the CELF Conference last week and
being busy this week delays so much.

Am Thu, 6 Nov 2008 21:53:29 +0100
schrieb Jean-Christophe PLAGNIOL-VILLARD [EMAIL PROTECTED]:

 On 12:48 Wed 05 Nov , Juergen Schoew wrote:
  Hi U-Boot mailling list,

 First some general comments
 
 As you anwser it's a NXP board so please create a vendor directory

OK, I use DSPG.

 please use tab instead of space for code alignement and indentation
 please a blank line when you write block of code and comment to make
 it more readable
 please and whitespace before and after   co

OK, I try to find every position.
 
 in the code their is a lot of hard code value please use Macro or
 specify why you can not do it

Will try to find suitable places.

 please only one blank line consecutive

OK.

  diff --git a/board/firetux/Makefile b/board/firetux/Makefile
  new file mode 100644
  index 000..bf1afec
  --- /dev/null
  +++ b/board/firetux/Makefile
  +include $(TOPDIR)/config.mk
  +
  +LIB= $(obj)lib$(BOARD).a
  +
  +COBJS-y+= firetux.o ethernet.o
 as ask by Ben and I move this to drivers/net

OK, I read it more as you may and not you have to. Sorry for the
misinterpretation. I try to find some spare time to do this jobs.
Hopefully this can work on the next weekend.

  diff --git a/board/firetux/ethernet.c b/board/firetux/ethernet.c
  new file mode 100644
  index 000..a4c1bc2
  --- /dev/null
  +++ b/board/firetux/ethernet.c
  +
  +extern unsigned int boardrevision;
  +
  +int firetux_miiphy_initialize(bd_t *bis);
  +int mii_discover_phy(void);
  +int mii_negotiate_phy(void);
  +
  +#define ALIGN8 static __attribute__ ((aligned(8)))
  +#define ALIGN4 static __attribute__ ((aligned(4)))
 IMHO this make more sense
 #define align8 __attribute__ ((aligned(8))

OK, changed.

  +   }
  +   if (act) {
  +   etn_rxdescriptor = etn2_rxdescriptor;
  +   etn_rxstatus = etn2_rxstatus;
 ^
 please use tab instead of space on all

Yes.

  +   /* get mode from environment */
  +   mode = getenv(phymode);
  +   if (mode  != NULL) {
 please use if (mode)
  +   if (0 == strcmp(mode, auto)) {
 please invert the condition an so on
   if (strcmp(mode, auto)) == 0) {

No problem.

  +void firetux_gpio_init_A(void)
  +{
  +   /* select TXD2 for GPIOa11 and select RXD2 for GPIOa1 */
  +   writel(((readl((void *)(PNX8181_SCON_BASE +
  PNX8181_SYSMUX0))
  +0xff33) |
  0x0044),
  +   (void *)(PNX8181_SCON_BASE +
  PNX8181_SYSMUX0));
  +   /* select ETN for GPIOc0-16 */
  +   writel(readl((void *)(PNX8181_SCON_BASE + PNX8181_SYSMUX5))
  +   
  0xfffc,
  +   (void *)(PNX8181_SCON_BASE +
  PNX8181_SYSMUX5));
  +   writel(readl((void *)(PNX8181_SCON_BASE + PNX8181_SYSMUX4))
  +   
  0x3000,
  +   (void *)(PNX8181_SCON_BASE +
  PNX8181_SYSMUX4)); +
  +   /* use clk26m and fract-n for UART2 */
  +   writew(((1  7) | (1  1)),
  +   (void *)(PNX8181_UART2_BASE +
  PNX8181_UART_FDIV_CTRL));
  +   writew(0x5F37, (void *)(PNX8181_UART2_BASE +
  PNX8181_UART_FDIV_M));
  +   writew(0x32C8, (void *)(PNX8181_UART2_BASE +
  PNX8181_UART_FDIV_N)); +}
  +
  
 IMHO please use deferent function for each hardware revision
 it will allow to make it more readable and to reduce the u-boot when
 compiling it for specific revision

The different revisions are very similair. I can split the routines
into different functions, but I doubt that it make sense to split these
into different files. The overhead is IMHO too much.

  +void firetux_gpio_init_B(void)
  +{
  +   if (boardrevision  2)  /* EZ_MCP has no
  leds or keys */
  +   return;
  +
  +   /* set GPIOA10 and GPIOA2 for key input */
  +   writel(readl((void *)(PNX8181_SCON_BASE + PNX8181_SYSMUX0))
  +   
  0xffcfffcf,
  +   (void *)(PNX8181_SCON_BASE +
  PNX8181_SYSMUX0));
  +   writel(readl((void *)PNX8181_GPIOA_PAD_LOW)  0xffcfffcf,
  +   (void
  *)PNX8181_GPIOA_PAD_LOW);
  +   writel(readl((void *)PNX8181_GPIOA_DIRECTION)  ~((1  2)
  | (1  10)),
  +   (void
  *)PNX8181_GPIOA_DIRECTION); +
  +   /* set GPIOA9 and GPIOA0 for LED output */
  +   if (boardrevision  2) {
  +   writel(readl((void *)(PNX8181_SCON_BASE +
  PNX8181_SYSMUX0))
  +   
  0xfff3fffc,
  +   (void *)(PNX8181_SCON_BASE +
  PNX8181_SYSMUX0));
  +   writel(readl((void *)PNX8181_GPIOA_DIRECTION)
  +   | ((1 
  9) | (1  0)),
  +

Re: [U-Boot] [patch V2] U-Boot Firetux board support

2008-11-13 Thread Jean-Christophe PLAGNIOL-VILLARD
   diff --git a/include/configs/firetux.h b/include/configs/firetux.h
   new file mode 100644
   index 000..efa27af
   --- /dev/null
   +++ b/include/configs/firetux.h
   +/* we can have nand _or_ nor flash */
   +/* #define CONFIG_NANDFLASH  1 */
   +
   +/* #define CONFIG_SHOW_BOOT_PROGRESS 1 */
  please no dead code
 
 Sorry, I use this code for testing the release and trying to find
 problematic areas. This way I only need to change the config and do not
 need to write the code for every release cycle. So I prefer to let it
 there.

please do this via Makefile and different config

Best Regards,
J.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [patch V2] U-Boot Firetux board support

2008-11-06 Thread Jean-Christophe PLAGNIOL-VILLARD
On 12:48 Wed 05 Nov , Juergen Schoew wrote:
 Hi U-Boot mailling list,
 
 This patchset adds a new ARM board with the NXP PNX8181 cpu to u-boot.
 The PNX8181 is an ARM926ej with an internal DSP (mostly used for Audio
 processing and VOIP codecs) and a baseband processor (used for DECT). The
 chip also features dual ethernet, digital to analog interface, spi, i2c and
 other SOC peripherals.
 
 These boards have been build by NXP Semiconductors GmbH, Nuremberg, Germany
 and are now build by DSPG Technologies GmbH, Nuremberg, Germany.
 
 
 Signed-off-by: Jürgen Schöw [EMAIL PROTECTED]
 Signed-off-by: Sebastian Hess [EMAIL PROTECTED]
 Signed-off-by: Matthias Mwenzel [EMAIL PROTECTED]
 Signed-off-by: Dirk Hörner [EMAIL PROTECTED]
 Signed-off-by: Andreas Weißel [EMAIL PROTECTED]
 ---
 
 Here is the next round of the patch. Thanks for the comments so far. I tried
 to change the code to meet your requirements. Following changes have been
 done:
 
  - use writex/readx for register accesses
  - use pointer in readx/writex
  - whitespace fixes (use tabs)
  - add Maintainer field
  - adjust comments to common style
  - use conditional settings
  - relocate.S bug fixes
  - network reworks
 
 I did not move the ethernet driver to drivers/net/ because a lot of hardware
 dependencies are in that driver to work. This IP-Core is not used very often
 by NXP processors (I don't have numbers) so it may not need to be moved. This
 driver does not not use the a struct to save the parameters as suggested
 from Jean-Christophe PLAGNIOL-VILLARD. Sorry but time was too short to fix it
 right now.
 
 Regards
 
 Jürgen Schöw
 -- 
 Dipl.-Ing. Jürgen Schöw, emlix GmbH, http://www.emlix.com, mailto:[EMAIL 
 PROTECTED]
 Fon +49 551 30664-0, Fax -11, Bahnhofsallee 1b, 37081 Göttingen, Germany
 Geschäftsführung: Dr. Uwe Kracke, Dr. Cord Seele, Ust-IdNr.: DE 205 198 055
 Sitz der Gesellschaft: Göttingen, Amtsgericht Göttingen HR B 3160
 
 emlix - your embedded linux partner
 
 
  MAKEALL   |1 +
  Makefile  |7 +
  board/firetux/Makefile|   59 +++
  board/firetux/config.mk   |   44 ++
  board/firetux/ethernet.c  |  954 
 +
  board/firetux/ethernet.h  |  213 +
  board/firetux/firetux.c   |  619 ++
  board/firetux/firetux.h   |  109 +
  board/firetux/lowlevel_init.S |  404 +
  board/firetux/memsetup.S  |  381 
  board/firetux/nand.c  |   68 +++
  board/firetux/relocate.S  |  252 +++
  board/firetux/u-boot.lds  |   57 +++
  drivers/i2c/Makefile  |1 +
  drivers/i2c/pnx8181_i2c.c |  302 +
  include/configs/firetux.h |  460 
  net/bootp.c   |   11 +
  net/eth.c |7 +
  18 files changed, 3949 insertions(+), 0 deletions(-)

First some general comments

As you anwser it's a NXP board so please create a vendor directory
please use tab instead of space for code alignement and indentation
please a blank line when you write block of code and comment to make it more
readable
please and whitespace before and after   co

in the code their is a lot of hard code value please use Macro or specify why
you can not do it

please only one blank line consecutive
 
 
 
 diff --git a/MAKEALL b/MAKEALL
 index 9ccb9ac..0ba8e4d 100755
 --- a/MAKEALL
 +++ b/MAKEALL
 @@ -480,6 +480,7 @@ LIST_ARM9=   \
   cp926ejs\
   cp946es \
   cp966   \
 + firetux \
   lpd7a400\
   mx1ads  \
   mx1fs2  \
 diff --git a/Makefile b/Makefile
 index 9a132f7..f747759 100644
 --- a/Makefile
 +++ b/Makefile
 @@ -2683,6 +2683,13 @@ voiceblue_config:  unconfig
   @$(MKCONFIG) $(@:_config=) arm arm925t voiceblue
  
  #
 +## NXP PNX8181 firetux
 +#
 +
 +firetux_config:unconfig
 + @$(MKCONFIG) $(@:_config=) arm arm926ejs firetux
 +
 +#
  ## S3C44B0 Systems
  #
  
 diff --git a/board/firetux/Makefile b/board/firetux/Makefile
 new file mode 100644
 index 000..bf1afec
 --- /dev/null
 +++ b/board/firetux/Makefile
 @@ -0,0 +1,59 @@
 +# firetux makefile
 +#
 +# (C) Copyright 2007-2008, emlix GmbH, Germany
 +# Juergen Schoew [EMAIL PROTECTED]
 +#
 +# (C) Copyright 2008, DSPG Technologies GmbH, Germany
 +# (C) Copyright 2007, NXP Semiconductors Germany GmbH
 +# Matthias Wenzel, [EMAIL PROTECTED]
 +#
 +# See file CREDITS for list of people who contributed to this
 +# project.
 +#
 +# This program is free software; you can redistribute it and/or
 +#