Re: [PATCH 20/27] ppc: Remove xpedite boards

2021-05-17 Thread Peter Tyser
Acked-by: Peter Tyser 

- Original Message -
> From: "Tom Rini" 
> To: "U-Boot Mailing List" 
> Cc: "ptyser" 
> Sent: Friday, May 14, 2021 8:34:25 PM
> Subject: [PATCH 20/27] ppc: Remove xpedite boards

> These boards have not been converted to CONFIG_DM_PCI by the deadline and is
> also missing conversion to CONFIG_DM.  Remove them.  As this includes
> the last ARCH_MPC8572 platform, remove that as well.
> 
> Cc: Peter Tyser 
> Signed-off-by: Tom Rini 
> ---
> arch/powerpc/cpu/mpc85xx/Kconfig  |  40 +-
> arch/powerpc/cpu/mpc85xx/Makefile |   1 -
> arch/powerpc/cpu/mpc85xx/mpc8572_serdes.c |  74 ---
> arch/powerpc/cpu/mpc86xx/Kconfig  |   5 -
> arch/powerpc/include/asm/fsl_law.h|   4 +-
> arch/powerpc/include/asm/immap_85xx.h |   2 +-
> board/xes/common/Makefile |   1 -
> board/xes/xpedite517x/Kconfig |  12 -
> board/xes/xpedite517x/MAINTAINERS |   6 -
> board/xes/xpedite517x/Makefile|   8 -
> board/xes/xpedite517x/ddr.c   | 124 -
> board/xes/xpedite517x/law.c   |  27 -
> board/xes/xpedite517x/xpedite517x.c   |  86 ---
> board/xes/xpedite520x/Kconfig |  12 -
> board/xes/xpedite520x/MAINTAINERS |   6 -
> board/xes/xpedite520x/Makefile|  11 -
> board/xes/xpedite520x/ddr.c   |  68 ---
> board/xes/xpedite520x/law.c   |  26 -
> board/xes/xpedite520x/tlb.c   |  68 ---
> board/xes/xpedite520x/xpedite520x.c   |  82 ---
> board/xes/xpedite537x/Kconfig |  12 -
> board/xes/xpedite537x/MAINTAINERS |   6 -
> board/xes/xpedite537x/Makefile|  11 -
> board/xes/xpedite537x/ddr.c   | 234 
> board/xes/xpedite537x/law.c   |  25 -
> board/xes/xpedite537x/tlb.c   |  82 ---
> board/xes/xpedite537x/xpedite537x.c   |  82 ---
> board/xes/xpedite550x/Kconfig |  12 -
> board/xes/xpedite550x/MAINTAINERS |   6 -
> board/xes/xpedite550x/Makefile|   8 -
> board/xes/xpedite550x/ddr.c   | 135 -
> board/xes/xpedite550x/law.c   |  25 -
> board/xes/xpedite550x/tlb.c   |  81 ---
> board/xes/xpedite550x/xpedite550x.c   |  82 ---
> configs/xpedite517x_defconfig |  54 --
> configs/xpedite520x_defconfig |  54 --
> configs/xpedite537x_defconfig |  57 --
> configs/xpedite550x_defconfig |  57 --
> drivers/ddr/fsl/Kconfig   |   1 -
> env/Kconfig   |   2 +-
> include/configs/xpedite517x.h | 646 --
> include/configs/xpedite520x.h | 445 ---
> include/configs/xpedite537x.h | 496 -
> include/configs/xpedite550x.h | 494 -
> 44 files changed, 5 insertions(+), 3765 deletions(-)
> delete mode 100644 arch/powerpc/cpu/mpc85xx/mpc8572_serdes.c
> delete mode 100644 board/xes/xpedite517x/Kconfig
> delete mode 100644 board/xes/xpedite517x/MAINTAINERS
> delete mode 100644 board/xes/xpedite517x/Makefile
> delete mode 100644 board/xes/xpedite517x/ddr.c
> delete mode 100644 board/xes/xpedite517x/law.c
> delete mode 100644 board/xes/xpedite517x/xpedite517x.c
> delete mode 100644 board/xes/xpedite520x/Kconfig
> delete mode 100644 board/xes/xpedite520x/MAINTAINERS
> delete mode 100644 board/xes/xpedite520x/Makefile
> delete mode 100644 board/xes/xpedite520x/ddr.c
> delete mode 100644 board/xes/xpedite520x/law.c
> delete mode 100644 board/xes/xpedite520x/tlb.c
> delete mode 100644 board/xes/xpedite520x/xpedite520x.c
> delete mode 100644 board/xes/xpedite537x/Kconfig
> delete mode 100644 board/xes/xpedite537x/MAINTAINERS
> delete mode 100644 board/xes/xpedite537x/Makefile
> delete mode 100644 board/xes/xpedite537x/ddr.c
> delete mode 100644 board/xes/xpedite537x/law.c
> delete mode 100644 board/xes/xpedite537x/tlb.c
> delete mode 100644 board/xes/xpedite537x/xpedite537x.c
> delete mode 100644 board/xes/xpedite550x/Kconfig
> delete mode 100644 board/xes/xpedite550x/MAINTAINERS
> delete mode 100644 board/xes/xpedite550x/Makefile
> delete mode 100644 board/xes/xpedite550x/ddr.c
> delete mode 100644 board/xes/xpedite550x/law.c
> delete mode 100644 board/xes/xpedite550x/tlb.c
> delete mode 100644 board/xes/xpedite550x/xpedite550x.c
> delete mode 100644 configs/xpedite517x_defconfig
> delete mode 100644 configs/xpedite520x_defconfig
> delete mode 100644 configs/xpedite537x_defconfig
> delete mode 100644 configs/xpedite550x_defconfig
> delete mode 100644 include/configs/xpedite517x.h
> delete mode 100644 include/configs/xpedite520x.h
> delete mode 100644 include/configs/xpedite537x.h
> delete mode 100644 include/configs/xpedite550x.h
> 




Re: [PATCH v3 15/15] MAINTAINERS: update maintainers for broadcom ns3 platform

2020-06-26 Thread Peter Tyser


> diff --git a/MAINTAINERS b/MAINTAINERS
> index 1fd975c72f..0c72deaa44 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -11,7 +11,7 @@ Descriptions of section entries:
>  Type is one of: git, hg, quilt, stgit, topgit
>   S: Status, one of the following:
>  Supported:   Someone is actually paid to look after this.
> -Maintained:  Someone actually looks after it.
> +Mne actually looks after it.
>  Orphan:  No current maintainer [but maybe you could take the
>   role as you write your new code].
>   F: Files and directories with wildcard patterns.

This looks to be an accidental change that should be fixed?

Regards,
Peter


Re: [U-Boot] Using MinGW gcc cross-compiling host tools for Windows broken

2019-10-14 Thread Peter Tyser
Hi Bin,

- Original Message -
> From: "Bin Meng" 
> To: "ptyser" , "Vladimir Yakovlev" , 
> "Mike Frysinger" , "Remy
> Bohmer" , "U-Boot Mailing List" 
> Cc: "Tom Rini" 
> Sent: Monday, October 14, 2019 4:46:16 AM
> Subject: Using MinGW gcc cross-compiling host tools for Windows broken

> Hi Peter,
> 
> I noticed that you were the first one that added support to build
> native Win32 tools using MinGW GCC via:
> 
> commit 2f8d396b9302eddcd8d552648e101a46b7a80acd
> Author: Peter Tyser 
> Date:   Fri Mar 13 18:54:51 2009 -0500
> 
>Add support for building native win32 tools
> 
>Add support for compiling the host tools in the tools directory using
>the MinGW toolchain.  This produces executables which can be used on
>standard Windows computers without requiring cygwin.
> 
>One must specify the MinGW compiler and strip utilities as if they
>were the host toolchain in order to build win32 executables, eg:
> 
>make HOSTCC=i586-mingw32msvc-gcc HOSTSTRIP=i586-mingw32msvc-strip tools
> 
>Signed-off-by: Peter Tyser 
> 
> There are also several follow-up commits that fixed the build errors
> from time to time:
> 
> commit 8b6a4952e6064dc558cb7d5d375990b17491f26f
> Author: Vladimir Yakovlev 
> Date:   Sat Jul 7 10:05:06 2012 +
> 
>tools: Fix mingw tools build
> 
>mkenvimage does not build due to missed os_support.o and unsupported
>file modes S_IRGRP S_IWGRP.
>Tested with mingw 4.2.1 on ubuntu 12.04.
> 
>Signed-off-by: Vladimir Yakovlev 
> 
> commit b050c72d52c4e30d5b978ab6758f8dcdbe5c690c
> Author: Mike Frysinger 
> Date:   Tue Apr 20 05:49:30 2010 -0400
> 
>compiler.h: add uint typedef
> 
>Recent crc changes started using the "uint" type in headers that are used
>on the build system.  This subsequently broke mingw targets as they do not
>provide such a type.  So add this basic typedef to compiler.h so that we
>do not have to worry about this breaking again in the future.
> 
>Signed-off-by: Mike Frysinger 
> 
> commit faf36c1437c95e4a86835633d9801c5f6396a3c7
> Author: Remy Bohmer 
> Date:   Wed Oct 28 22:13:36 2009 +0100
> 
>Fix mingw tools build
> 
>mkimage does not build due to missing strtok_r() and getline()
> implementation
> 
>Signed-off-by: Remy Bohmer 
> 
> Today I tried to build the Windows tools by following the README, and
> got build errors.
> 
> $ make HOSTCC=/usr/bin/x86_64-w64-mingw32-gcc tools
> 
> note: I am running this from Linux. It's not clear to me whether we
> should run this from Windows natively or cross-compile on Linux.

It was intended to be cross-compiled on a Linux box.

> It seems that this MinGW Windows build has been broken for quite a long time.
> 
> Will you address this? Thanks!

I had a quick look at getting compilation working, and my vote is to remove
support for the MinGW compilation.
- It can't be too popular since the compilation issues haven't been
flagged for a long time.

- The motivation 10 years ago was to support customers that used Windows as
their OS development environment, mostly for VxWorks.  Development under a
100% Windows environment seems a lot less common nowadays.  Virtual machines
and Windows Subsystem for Linux also make it much easier to run Linux apps
in a Windows development environment.  My company hasn't been distributing
the MinGW binary for a number of years now as a data point.

- The feature set of some of the U-Boot tools has increased quite a bit
which makes it non-trivial to support with MinGW.  For example mkimage links
with libssl and libcrypto.

I'm guessing more duct tape on the make system would be needed to get
MinGW working, and my thought is that ugliness isn't worth the benefit
anymore.

I'm happy to gin up a patch if others agree and think removing
MinGW support makes sense.

Regards,
Peter


___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH] drivers/ddr/fsl: Dual-license DDR driver

2018-02-13 Thread Peter Tyser
On Tue, 2018-02-13 at 16:05 +, York Sun wrote:
> On 02/13/2018 04:49 AM, Wolfgang Denk wrote:
> > 
> > Dear York,
> > 
> > In message  you wrote:
> > > 
> > > 
> > > Nobody said anything. Some addresses bounced. And most changes made out
> > > people outside Freescale/NXP are minor changes, except twice the files
> > > were moved during U-Boot structure change. What options do I have?
> > Ask all people who contributed to that code for their explicit
> > permission.  Legally it is a huge difference between actively
> > confirming approval and not reacting at all.
> > 
> All people (except Freescale and NXP employees) contributed to this code
> are in the CC list. Please give your explicit approval for this license
> change. Thanks.

Acked-by: Peter Tyser <pty...@xes-inc.com>
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH 1/1] Broadwell-DE Implementation

2018-02-05 Thread Peter Tyser
Hi Vincenzo,

On Thu, 2018-01-25 at 13:58 -0500, vnktux wrote:
> Hi all,
> 
> This patch contain a working implementation of Broadwell-DE for U-
> Boot, and support memory down with external SPD binary file. However
> there is only one issue that I couldn't solve, the booting process
> takes 1 hour. When the FSP reach "DDRIO Initialization" it take a lot
> before the memory is initialized and you can reach the U-Boot shell.
> I would like a review of my implementation since I am out of options.
> I tried to Enable/Disable MRC Cache and ACPI Resume but the problem
> is still there.

Thanks for the Broadwell-DE work.  It'd be nice to have a port!  I
tried your code on one of our Broadwell-DE board this weekend and saw
similar behavior to you with the slow boot time.  I believe this is
related to cache configuration.  A modification like the following
makes my board boot quickly (sub 10 seconds):

--- a/arch/x86/lib/fsp/fsp_car.S
+++ b/arch/x86/lib/fsp/fsp_car.S
@@ -106,5 +106,5 @@ _dt_ucode_base_size:
 ucode_base:/* Declared in micrcode.h */
.long   0   /* microcode base */
.long   0   /* microcode size */
-   .long   CONFIG_SYS_MONITOR_BASE /* code region base */
-   .long   CONFIG_SYS_MONITOR_LEN  /* code region size */
+   .long   0x - 0x100 + 1  /* code region base */
+   .long   0x100   /* code region size */

The above is just a proof of concept meant to show the problem area - a
more elegant solution should likely be used.  The "code region" above
is passed into the FSP, which I believe enables caching in that region.
 Increasing it from 1MB to 16MB (the size of my SPI flash) made the
boot time more sane.

The latest patch series also seems to have whitespace issues so that it
can't apply cleanly without modification.  As an example,
arch/x86/dts/u-boot.dtsi has indentation that is not present in  https:
//patchwork.ozlabs.org/patch/866257/ which makes it not apply.  It'd be
good to fix that on future submissions.

The patch series seemed to mostly work out of the box to get up to a
command prompt, which is great!  It could boot a Linuz zImage as well.
 There seemed to be some rough edges (eg multi processor support), but
I didn't kick the tires too much and assume those edges shouldn't be
too bad to smooth out later.

Regards,
Peter
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] Xpedite boards

2017-05-11 Thread Peter Tyser
Hi Simon,

> 
> I am trying to help with the U-Boot conversion of CONFIG options to
> Kconfig. I notice that the Xpedite boards use a ds4510 driver which
> defines quite a few of these options. I could potentially convert it
> to Kconfig.

Thanks, and thanks for your work on U-Boot improvements in general over
the last few years!  I've been out of the development loop, but it's
neat to see the improvements you've been shepherding.

> However, many of the options in these boards define I2C addresses,
> which these days should be done with driver model / device tree.
> 
> I wonder if you have any interest in converting these boards as part
> of ongoing work?

I definitely have interest, but am iffy on the time:)  From what I
understand PowerPC has been pretty tardy on moving to the driver model
/ device tree, and no PowerPC board uses the device tree control at
this point.  To make sure I understand the end goal, you're asking
about defining CONFIG_OF_CONTROL for the XPedite boards?  This would
primarily entail getting CONFIG_OF_CONTROL working, at which point the
driver model should "just work"?  Let me know if I'm missing the high
level picture.

Regards,
Peter
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


[U-Boot] [PATCH] nand: Remove unused read/write structures

2015-02-03 Thread Peter Tyser
The use of the nand_write_options and nand_read_options structures were
removed in commit dfbf617ff055e4216f78d358b0867c548916d14b.  Remove the
now-unused structures too.

Signed-off-by: Peter Tyser pty...@xes-inc.com
---
 include/nand.h | 26 --
 1 file changed, 26 deletions(-)

diff --git a/include/nand.h b/include/nand.h
index bf8e538..621c004 100644
--- a/include/nand.h
+++ b/include/nand.h
@@ -82,34 +82,8 @@ static inline int nand_erase(nand_info_t *info, loff_t off, 
size_t size)
  * declarations from nand_util.c
  /
 
-struct nand_write_options {
-   u_char *buffer; /* memory block containing image to write */
-   ulong length;   /* number of bytes to write */
-   ulong offset;   /* start address in NAND */
-   int quiet;  /* don't display progress messages */
-   int autoplace;  /* if true use auto oob layout */
-   int forcejffs2; /* force jffs2 oob layout */
-   int forceyaffs; /* force yaffs oob layout */
-   int noecc;  /* write without ecc */
-   int writeoob;   /* image contains oob data */
-   int pad;/* pad to page size */
-   int blockalign; /* 1|2|4 set multiple of eraseblocks
-* to align to */
-};
-
-typedef struct nand_write_options nand_write_options_t;
 typedef struct mtd_oob_ops mtd_oob_ops_t;
 
-struct nand_read_options {
-   u_char *buffer; /* memory block in which read image is written*/
-   ulong length;   /* number of bytes to read */
-   ulong offset;   /* start address in NAND */
-   int quiet;  /* don't display progress messages */
-   int readoob;/* put oob data in image */
-};
-
-typedef struct nand_read_options nand_read_options_t;
-
 struct nand_erase_options {
loff_t length;  /* number of bytes to erase */
loff_t offset;  /* first address in NAND to erase */
-- 
1.9.1

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


[U-Boot] [PATCH] cmd_yaffs: Clean up command usage messages

2015-02-03 Thread Peter Tyser
Remove duplicate command names in usage messages to fix issues such as:
  = help yls
  yls - yaffs ls

  Usage:
  yls yls [-l] dirname

Signed-off-by: Peter Tyser pty...@xes-inc.com
---
 common/cmd_yaffs2.c | 26 +-
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/common/cmd_yaffs2.c b/common/cmd_yaffs2.c
index d43a9d4..9244606 100644
--- a/common/cmd_yaffs2.c
+++ b/common/cmd_yaffs2.c
@@ -281,46 +281,46 @@ int do_ymv(cmd_tbl_t *cmdtp, int flag, int argc, char 
*const argv[])
 
 U_BOOT_CMD(ytrace, 2, 0, do_ytrace,
   show/set yaffs trace mask,
-  ytrace [new_mask]  show/set yaffs trace mask);
+  [new_mask]  show/set yaffs trace mask);
 
 U_BOOT_CMD(ydevls, 1, 0, do_ydevls,
   list yaffs mount points, list yaffs mount points);
 
 U_BOOT_CMD(ydevconfig, 5, 0, do_ydevconfig,
   configure yaffs mount point,
-  ydevconfig mtpoint mtd_id start_block end_block   configures a 
yaffs2 mount point);
+  mtpoint mtd_id start_block end_block   configures a yaffs2 mount 
point);
 
 U_BOOT_CMD(ymount, 2, 0, do_ymount,
-  mount yaffs, ymount mtpoint  mounts a yaffs2 mount point);
+  mount yaffs, mtpoint  mounts a yaffs2 mount point);
 
 U_BOOT_CMD(yumount, 2, 0, do_yumount,
-  unmount yaffs, yunmount mtpoint  unmounts a yaffs2 mount point);
+  unmount yaffs, mtpoint  unmounts a yaffs2 mount point);
 
-U_BOOT_CMD(yls, 3, 0, do_yls, yaffs ls, yls [-l] dirname);
+U_BOOT_CMD(yls, 3, 0, do_yls, yaffs ls, [-l] dirname);
 
 U_BOOT_CMD(yrd, 2, 0, do_yrd,
-  read file from yaffs, yrd path   read file from yaffs);
+  read file from yaffs, path   read file from yaffs);
 
 U_BOOT_CMD(ywr, 4, 0, do_ywr,
   write file to yaffs,
-  ywr filename value num_vlues   write values to yaffs file);
+  filename value num_vlues   write values to yaffs file);
 
 U_BOOT_CMD(yrdm, 3, 0, do_yrdm,
   read file to memory from yaffs,
-  yrdm filename offsetreads yaffs file into memory);
+  filename offsetreads yaffs file into memory);
 
 U_BOOT_CMD(ywrm, 4, 0, do_ywrm,
   write file from memory to yaffs,
-  ywrm filename offset size  writes memory to yaffs file);
+  filename offset size  writes memory to yaffs file);
 
 U_BOOT_CMD(ymkdir, 2, 0, do_ymkdir,
-  YAFFS mkdir, ymkdir dircreate a yaffs directory);
+  YAFFS mkdir, dircreate a yaffs directory);
 
 U_BOOT_CMD(yrmdir, 2, 0, do_yrmdir,
-  YAFFS rmdir, yrmdir dirname   removes a yaffs directory);
+  YAFFS rmdir, dirname   removes a yaffs directory);
 
-U_BOOT_CMD(yrm, 2, 0, do_yrm, YAFFS rm, yrm path   removes a yaffs file);
+U_BOOT_CMD(yrm, 2, 0, do_yrm, YAFFS rm, path   removes a yaffs file);
 
 U_BOOT_CMD(ymv, 4, 0, do_ymv,
   YAFFS mv,
-  ymv old_path new_path   moves/rename files within a yaffs mount 
point);
+  old_path new_path   moves/rename files within a yaffs mount point);
-- 
1.9.1

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


[U-Boot] [PATCH] nand: Remove unused CONFIG_MTD_NAND_ECC_JFFS2 option

2015-02-03 Thread Peter Tyser
This option was removed along with legacy NAND support in
be33b046b549ad88c204c209508cd7657232ffbd.  Clean up some remnants.

Signed-off-by: Peter Tyser pty...@xes-inc.com
---
 doc/README.nand | 12 
 include/configs/ethernut5.h |  1 -
 2 files changed, 13 deletions(-)

diff --git a/doc/README.nand b/doc/README.nand
index dee0e00..46d7edd 100644
--- a/doc/README.nand
+++ b/doc/README.nand
@@ -99,12 +99,6 @@ Configuration Options:
CONFIG_CMD_NAND_TORTURE
   Enables the torture command (see description of this command below).
 
-   CONFIG_MTD_NAND_ECC_JFFS2
-  Define this if you want the Error Correction Code information in
-  the out-of-band data to be formatted to match the JFFS2 file system.
-  CONFIG_MTD_NAND_ECC_YAFFS would be another useful choice for
-  someone to implement.
-
CONFIG_SYS_MAX_NAND_DEVICE
   The maximum number of NAND devices you want to support.
 
@@ -312,12 +306,6 @@ Platform specific options
 NOTE:
 =
 
-The current NAND implementation is based on what is in recent
-Linux kernels.  The old legacy implementation has been removed.
-
-If you have board code which used CONFIG_NAND_LEGACY, you'll need
-to convert to the current NAND interface for it to continue to work.
-
 The Disk On Chip driver is currently broken and has been for some time.
 There is a driver in drivers/mtd/nand, taken from Linux, that works with
 the current NAND system but has not yet been adapted to the u-boot
diff --git a/include/configs/ethernut5.h b/include/configs/ethernut5.h
index ce61a16..b79000e 100644
--- a/include/configs/ethernut5.h
+++ b/include/configs/ethernut5.h
@@ -153,7 +153,6 @@
 
 /* JFFS2 */
 #ifdef CONFIG_CMD_JFFS2
-#define CONFIG_MTD_NAND_ECC_JFFS2
 #define CONFIG_JFFS2_CMDLINE
 #define CONFIG_JFFS2_NAND
 #endif
-- 
1.9.1

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


[U-Boot] [PATCH 1/5] nand: Add verification functions

2015-02-03 Thread Peter Tyser
Add nand_verify() and nand_verify_page_oob().  nand_verify() verifies
NAND contents against an arbitrarily sized buffer using ECC while
nand_verify_page_oob() verifies a NAND page's contents and OOB.

Signed-off-by: Peter Tyser pty...@xes-inc.com
---

 drivers/mtd/nand/nand_util.c | 97 +++-
 include/nand.h   |  4 ++
 2 files changed, 100 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/nand_util.c b/drivers/mtd/nand/nand_util.c
index afdd160..f487756 100644
--- a/drivers/mtd/nand/nand_util.c
+++ b/drivers/mtd/nand/nand_util.c
@@ -464,6 +464,87 @@ static size_t drop_ffs(const nand_info_t *nand, const 
u_char *buf,
 #endif
 
 /**
+ * nand_verify_page_oob:
+ *
+ * Verify a page of NAND flash, including the OOB.
+ * Reads page of NAND and verifies the contents and OOB against the
+ * values in ops.
+ *
+ * @param nand NAND device
+ * @param ops  MTD operations, including data to verify
+ * @param ofs  offset in flash
+ * @return 0 in case of success
+ */
+int nand_verify_page_oob(nand_info_t *nand, struct mtd_oob_ops *ops, loff_t 
ofs)
+{
+   int rval;
+   struct mtd_oob_ops vops;
+   size_t verlen = nand-writesize + nand-oobsize;
+
+   memcpy(vops, ops, sizeof(vops));
+
+   vops.datbuf = malloc(verlen);
+
+   if (!vops.datbuf)
+   return -ENOMEM;
+
+   vops.oobbuf = vops.datbuf + nand-writesize;
+
+   rval = mtd_read_oob(nand, ofs, vops);
+   if (!rval)
+   rval = memcmp(ops-datbuf, vops.datbuf, vops.len);
+   if (!rval)
+   rval = memcmp(ops-oobbuf, vops.oobbuf, vops.ooblen);
+
+   free(vops.datbuf);
+
+   return rval ? -EIO : 0;
+}
+
+/**
+ * nand_verify:
+ *
+ * Verify a region of NAND flash.
+ * Reads NAND in page-sized chunks and verifies the contents against
+ * the contents of a buffer.  The offset into the NAND must be
+ * page-aligned, and the function doesn't handle skipping bad blocks.
+ *
+ * @param nand NAND device
+ * @param ofs  offset in flash
+ * @param len  buffer length
+ * @param buf  buffer to read from
+ * @return 0 in case of success
+ */
+int nand_verify(nand_info_t *nand, loff_t ofs, size_t len, u_char *buf)
+{
+   int rval = 0;
+   size_t verofs;
+   size_t verlen = nand-writesize;
+   uint8_t *verbuf = malloc(verlen);
+
+   if (!verbuf)
+   return -ENOMEM;
+
+   /* Read the NAND back in page-size groups to limit malloc size */
+   for (verofs = ofs; verofs  ofs + len;
+verofs += verlen, buf += verlen) {
+   verlen = min(nand-writesize, (uint32_t)(ofs + len - verofs));
+   rval = nand_read(nand, verofs, verlen, verbuf);
+   if (!rval || (rval == -EUCLEAN))
+   rval = memcmp(buf, verbuf, verlen);
+
+   if (rval)
+   break;
+   }
+
+   free(verbuf);
+
+   return rval ? -EIO : 0;
+}
+
+
+
+/**
  * nand_write_skip_bad:
  *
  * Write image to NAND flash.
@@ -501,7 +582,7 @@ int nand_write_skip_bad(nand_info_t *nand, loff_t offset, 
size_t *length,
 
 #ifdef CONFIG_CMD_NAND_YAFFS
if (flags  WITH_YAFFS_OOB) {
-   if (flags  ~WITH_YAFFS_OOB)
+   if (flags  (~WITH_YAFFS_OOB  ~WITH_WR_VERIFY))
return -EINVAL;
 
int pages;
@@ -554,6 +635,10 @@ int nand_write_skip_bad(nand_info_t *nand, loff_t offset, 
size_t *length,
 
if (!need_skip  !(flags  WITH_DROP_FFS)) {
rval = nand_write(nand, offset, length, buffer);
+
+   if ((flags  WITH_WR_VERIFY)  !rval)
+   rval = nand_verify(nand, offset, *length, buffer);
+
if (rval == 0)
return 0;
 
@@ -601,6 +686,11 @@ int nand_write_skip_bad(nand_info_t *nand, loff_t offset, 
size_t *length,
ops.oobbuf = ops.datbuf + pagesize;
 
rval = mtd_write_oob(nand, offset, ops);
+
+   if ((flags  WITH_WR_VERIFY)  !rval)
+   rval = nand_verify_page_oob(nand,
+   ops, offset);
+
if (rval != 0)
break;
 
@@ -620,6 +710,11 @@ int nand_write_skip_bad(nand_info_t *nand, loff_t offset, 
size_t *length,
 
rval = nand_write(nand, offset, truncated_write_size,
p_buffer);
+
+   if ((flags  WITH_WR_VERIFY)  !rval)
+   rval = nand_verify(nand, offset,
+   truncated_write_size, p_buffer);
+
offset += write_size;
p_buffer += write_size;
}
diff --git a/include/nand.h b/include/nand.h
index

[U-Boot] [PATCH 3/5] dfu: nand: Verify writes

2015-02-03 Thread Peter Tyser
Previously NAND writes were not verified and could fail silently.  Add
a verification step after all writes to NAND.

Signed-off-by: Peter Tyser pty...@xes-inc.com
---
I don't have a board with DFU support, so this change is untested.

 drivers/dfu/dfu_nand.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/dfu/dfu_nand.c b/drivers/dfu/dfu_nand.c
index f9ee189..a975492 100644
--- a/drivers/dfu/dfu_nand.c
+++ b/drivers/dfu/dfu_nand.c
@@ -64,7 +64,7 @@ static int nand_block_op(enum dfu_op op, struct dfu_entity 
*dfu,
return ret;
/* then write */
ret = nand_write_skip_bad(nand, start, count, actual,
-   lim, buf, 0);
+   lim, buf, WITH_WR_VERIFY);
}
 
if (ret != 0) {
-- 
1.9.1

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


[U-Boot] [PATCH 2/5] cmd_nand: Verify writes to NAND

2015-02-03 Thread Peter Tyser
Previously NAND writes were only verified when CONFIG_MTD_NAND_VERIFY_WRITE
was defined.  On boards without this define writes could fail silently.
Boards with CONFIG_MTD_NAND_VERIFY_WRITE could prematurely report
failures which ECC could correct.

Add a verification step after all nand write[.x] commands to ensure the
writes were successful.  The verification uses ECC for for normal
writes, but does not for raw and yaffs writes.  Some test cases which
inject fake bad bits on a 2K page flash are below.

Test cases with CONFIG_MTD_NAND_VERIFY_WRITE defined:
  Example of an ECC write which previously failed when
  CONFIG_MTD_NAND_VERIFY_WRITE was defined, but now succeeds because ECC
  is used during verification:
  nand erase 0 0x1
  dhcp /somefile
  mw.b 0x1 0xff 0x2000
  mw.b 0x10020 0xfe 1
  nand write.raw 0x1 0x800 1
  mw.b 0x120 0x01 1
  nand write 0x100 0x800 0x1800

Test cases without CONFIG_MTD_NAND_VERIFY_WRITE defined:
  Example of an ECC write which previously silently failed:
  nand erase 0 0x1
  dhcp /somefile
  mw.b 0x1 0xff 0x2000
  mw.b 0x10020 0x00 1
  nand write.raw 0x1 0x800 1
  mw.b 0x120 0xff 1
  nand write 0x100 0x800 0x1800

  Example of a raw write which previously failed silently due to stuck
  data bit, but now errors out:
  nand erase 0 0x1
  dhcp /somefile
  mw.b 0x1 0xff 0x2000
  mw.b 0x10020 0xfe 1
  nand write.raw 0x1 0x800 1
  mw.b 0x120 0x01 1
  nand write.raw 0x100 0x800 3

  Example of a raw write which previously failed silently due to stuck OOB
  bit, but now errors out:
  nand erase 0 0x1
  dhcp /somefile
  mw.b 0x1 0xff 0x2000
  mw.b 0x10810 0xfe 1
  nand write.raw 0x1 0x800 1
  mw.b 0x1000810 0x01 1
  nand write.raw 0x100 0x800 3

Signed-off-by: Peter Tyser pty...@xes-inc.com
---

 common/cmd_nand.c | 14 +-
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/common/cmd_nand.c b/common/cmd_nand.c
index 7f962dc..bada28c 100644
--- a/common/cmd_nand.c
+++ b/common/cmd_nand.c
@@ -419,10 +419,13 @@ static int raw_access(nand_info_t *nand, ulong addr, 
loff_t off, ulong count,
.mode = MTD_OPS_RAW
};
 
-   if (read)
+   if (read) {
ret = mtd_read_oob(nand, off, ops);
-   else
+   } else {
ret = mtd_write_oob(nand, off, ops);
+   if (!ret)
+   ret = nand_verify_page_oob(nand, ops, off);
+   }
 
if (ret) {
printf(%s: error at offset %llx, ret %d\n,
@@ -690,7 +693,8 @@ static int do_nand(cmd_tbl_t *cmdtp, int flag, int argc, 
char * const argv[])
else
ret = nand_write_skip_bad(nand, off, rwsize,
  NULL, maxsize,
- (u_char *)addr, 0);
+ (u_char *)addr,
+ WITH_WR_VERIFY);
 #ifdef CONFIG_CMD_NAND_TRIMFFS
} else if (!strcmp(s, .trimffs)) {
if (read) {
@@ -699,7 +703,7 @@ static int do_nand(cmd_tbl_t *cmdtp, int flag, int argc, 
char * const argv[])
}
ret = nand_write_skip_bad(nand, off, rwsize, NULL,
maxsize, (u_char *)addr,
-   WITH_DROP_FFS);
+   WITH_DROP_FFS | WITH_WR_VERIFY);
 #endif
 #ifdef CONFIG_CMD_NAND_YAFFS
} else if (!strcmp(s, .yaffs)) {
@@ -709,7 +713,7 @@ static int do_nand(cmd_tbl_t *cmdtp, int flag, int argc, 
char * const argv[])
}
ret = nand_write_skip_bad(nand, off, rwsize, NULL,
maxsize, (u_char *)addr,
-   WITH_YAFFS_OOB);
+   WITH_YAFFS_OOB | 
WITH_WR_VERIFY);
 #endif
} else if (!strcmp(s, .oob)) {
/* out-of-band data */
-- 
1.9.1

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


[U-Boot] [PATCH 5/5] nand: yaffs: Remove the nand write.yaffs command

2015-02-03 Thread Peter Tyser
This command is only enabled by one board, complicates the NAND code,
and doesn't appear to have been functioning properly for several
years.  If there are no bad blocks in the NAND region being written
nand_write_skip_bad() will take the shortcut of calling nand_write()
which bypasses the special yaffs handling.  This causes invalid YAFFS
data to be written. See
http://lists.denx.de/pipermail/u-boot/2011-September/102830.html for
an example and a potential workaround.

U-Boot still retains the ability to mount and access YAFFS partitions
via CONFIG_YAFFS2.

Signed-off-by: Peter Tyser pty...@xes-inc.com
---
If people are actively using nand write.yaffs, feel free to chime in
if you'd like it to be kept in.  I spent a little time trying to use
it and ran into a handful of issues which made me think it wasn't
actively used.

It's also not clear to me why nand write.raw wouldn't be usable since
it should allow writing any arbitrary data?

 common/cmd_nand.c| 15 -
 drivers/mtd/nand/nand_util.c | 77 +++-
 include/configs/M54418TWR.h  |  1 -
 include/configs/VCMA9.h  |  1 -
 include/nand.h   |  7 ++--
 5 files changed, 14 insertions(+), 87 deletions(-)

diff --git a/common/cmd_nand.c b/common/cmd_nand.c
index bada28c..17fa7ea 100644
--- a/common/cmd_nand.c
+++ b/common/cmd_nand.c
@@ -705,16 +705,6 @@ static int do_nand(cmd_tbl_t *cmdtp, int flag, int argc, 
char * const argv[])
maxsize, (u_char *)addr,
WITH_DROP_FFS | WITH_WR_VERIFY);
 #endif
-#ifdef CONFIG_CMD_NAND_YAFFS
-   } else if (!strcmp(s, .yaffs)) {
-   if (read) {
-   printf(Unknown nand command suffix '%s'.\n, 
s);
-   return 1;
-   }
-   ret = nand_write_skip_bad(nand, off, rwsize, NULL,
-   maxsize, (u_char *)addr,
-   WITH_YAFFS_OOB | 
WITH_WR_VERIFY);
-#endif
} else if (!strcmp(s, .oob)) {
/* out-of-band data */
mtd_oob_ops_t ops = {
@@ -857,11 +847,6 @@ static char nand_help_text[] =
'addr', skipping bad blocks and dropping any pages at the end\n
of eraseblocks that contain only 0xFF\n
 #endif
-#ifdef CONFIG_CMD_NAND_YAFFS
-   nand write.yaffs - addr off|partition size\n
-   write 'size' bytes starting at offset 'off' with yaffs format\n
-   from memory address 'addr', skipping bad blocks.\n
-#endif
nand erase[.spread] [clean] off size - erase 'size' bytes 
from offset 'off'\n
With '.spread', erase enough for given file size, otherwise,\n
diff --git a/drivers/mtd/nand/nand_util.c b/drivers/mtd/nand/nand_util.c
index f487756..12dd26a 100644
--- a/drivers/mtd/nand/nand_util.c
+++ b/drivers/mtd/nand/nand_util.c
@@ -580,24 +580,7 @@ int nand_write_skip_bad(nand_info_t *nand, loff_t offset, 
size_t *length,
if (actual)
*actual = 0;
 
-#ifdef CONFIG_CMD_NAND_YAFFS
-   if (flags  WITH_YAFFS_OOB) {
-   if (flags  (~WITH_YAFFS_OOB  ~WITH_WR_VERIFY))
-   return -EINVAL;
-
-   int pages;
-   pages = nand-erasesize / nand-writesize;
-   blocksize = (pages * nand-oobsize) + nand-erasesize;
-   if (*length % (nand-writesize + nand-oobsize)) {
-   printf(Attempt to write incomplete page
-in yaffs mode\n);
-   return -EINVAL;
-   }
-   } else
-#endif
-   {
-   blocksize = nand-erasesize;
-   }
+   blocksize = nand-erasesize;
 
/*
 * nand_write() handles unaligned, partial page writes.
@@ -666,58 +649,22 @@ int nand_write_skip_bad(nand_info_t *nand, loff_t offset, 
size_t *length,
else
write_size = blocksize - block_offset;
 
-#ifdef CONFIG_CMD_NAND_YAFFS
-   if (flags  WITH_YAFFS_OOB) {
-   int page, pages;
-   size_t pagesize = nand-writesize;
-   size_t pagesize_oob = pagesize + nand-oobsize;
-   struct mtd_oob_ops ops;
-
-   ops.len = pagesize;
-   ops.ooblen = nand-oobsize;
-   ops.mode = MTD_OPS_AUTO_OOB;
-   ops.ooboffs = 0;
-
-   pages = write_size / pagesize_oob;
-   for (page = 0; page  pages; page++) {
-   WATCHDOG_RESET();
-
-   ops.datbuf = p_buffer;
-   ops.oobbuf = ops.datbuf + pagesize;
-
-   rval = mtd_write_oob(nand, offset, ops

[U-Boot] [PATCH 4/5] nand: Remove CONFIG_MTD_NAND_VERIFY_WRITE

2015-02-03 Thread Peter Tyser
The CONFIG_MTD_NAND_VERIFY_WRITE has been removed from Linux for some
time and a more generic method of NAND verification now exists in U-Boot.

Signed-off-by: Peter Tyser pty...@xes-inc.com
---

 README  |  3 --
 board/prodrive/alpr/nand.c  | 16 -
 board/socrates/nand.c   | 25 --
 drivers/mtd/nand/davinci_nand.c | 12 ---
 drivers/mtd/nand/fsl_elbc_nand.c| 38 --
 drivers/mtd/nand/fsl_ifc_nand.c | 38 --
 drivers/mtd/nand/fsl_upm.c  | 18 --
 drivers/mtd/nand/mpc5121_nfc.c  | 26 ---
 drivers/mtd/nand/mxc_nand.c | 33 ---
 drivers/mtd/nand/nand_base.c| 65 -
 drivers/mtd/nand/ndfc.c | 18 --
 include/configs/B4860QDS.h  |  1 -
 include/configs/BSC9131RDB.h|  1 -
 include/configs/BSC9132QDS.h|  1 -
 include/configs/C29XPCIE.h  |  1 -
 include/configs/MPC8313ERDB.h   |  1 -
 include/configs/MPC8315ERDB.h   |  1 -
 include/configs/MPC837XEMDS.h   |  1 -
 include/configs/MPC8536DS.h |  1 -
 include/configs/MPC8569MDS.h|  1 -
 include/configs/MPC8572DS.h |  1 -
 include/configs/P1010RDB.h  |  1 -
 include/configs/P1022DS.h   |  1 -
 include/configs/P1023RDB.h  |  1 -
 include/configs/P2041RDB.h  |  1 -
 include/configs/T102xQDS.h  |  1 -
 include/configs/T102xRDB.h  |  1 -
 include/configs/T1040QDS.h  |  1 -
 include/configs/T104xRDB.h  |  1 -
 include/configs/T208xQDS.h  |  1 -
 include/configs/T208xRDB.h  |  1 -
 include/configs/T4240QDS.h  |  1 -
 include/configs/T4240RDB.h  |  1 -
 include/configs/corenet_ds.h|  1 -
 include/configs/ids8313.h   |  1 -
 include/configs/km/kmp204x-common.h |  1 -
 include/configs/ls1021aqds.h|  1 -
 include/configs/ls2085a_common.h|  1 -
 include/configs/p1_p2_rdb_pc.h  |  1 -
 include/configs/ve8313.h|  1 -
 include/configs/xpedite537x.h   |  1 -
 include/configs/xpedite550x.h   |  1 -
 include/linux/mtd/nand.h|  5 ---
 43 files changed, 328 deletions(-)

diff --git a/README b/README
index fefa71c..88a21ff 100644
--- a/README
+++ b/README
@@ -3493,9 +3493,6 @@ FIT uImage format:
Adds the MTD partitioning infrastructure from the Linux
kernel. Needed for UBI support.
 
-   CONFIG_MTD_NAND_VERIFY_WRITE
-   verify if the written data is correct reread.
-
 - UBI support
CONFIG_CMD_UBI
 
diff --git a/board/prodrive/alpr/nand.c b/board/prodrive/alpr/nand.c
index 5427de5..ca40cea 100644
--- a/board/prodrive/alpr/nand.c
+++ b/board/prodrive/alpr/nand.c
@@ -93,19 +93,6 @@ static void alpr_nand_read_buf(struct mtd_info *mtd, u_char 
*buf, int len)
}
 }
 
-#if defined(CONFIG_MTD_NAND_VERIFY_WRITE)
-static int alpr_nand_verify_buf(struct mtd_info *mtd, const u_char *buf, int 
len)
-{
-   int i;
-
-   for (i = 0; i  len; i++)
-   if (buf[i] != readb((alpr_ndfc-data)))
-   return i;
-
-   return 0;
-}
-#endif
-
 static int alpr_nand_dev_ready(struct mtd_info *mtd)
 {
/*
@@ -130,9 +117,6 @@ int board_nand_init(struct nand_chip *nand)
nand-read_byte  = alpr_nand_read_byte;
nand-write_buf  = alpr_nand_write_buf;
nand-read_buf   = alpr_nand_read_buf;
-#if defined(CONFIG_MTD_NAND_VERIFY_WRITE)
-   nand-verify_buf = alpr_nand_verify_buf;
-#endif
nand-dev_ready  = alpr_nand_dev_ready;
 
return 0;
diff --git a/board/socrates/nand.c b/board/socrates/nand.c
index 7394478..15e6ea6 100644
--- a/board/socrates/nand.c
+++ b/board/socrates/nand.c
@@ -18,9 +18,6 @@ static void sc_nand_write_buf(struct mtd_info *mtd, const 
u_char *buf, int len);
 static u_char sc_nand_read_byte(struct mtd_info *mtd);
 static u16 sc_nand_read_word(struct mtd_info *mtd);
 static void sc_nand_read_buf(struct mtd_info *mtd, u_char *buf, int len);
-#if defined(CONFIG_MTD_NAND_VERIFY_WRITE)
-static int sc_nand_verify_buf(struct mtd_info *mtd, const u_char *buf, int 
len);
-#endif
 static int sc_nand_device_ready(struct mtd_info *mtdinfo);
 
 #define FPGA_NAND_CMD_MASK (0x7  28)
@@ -102,25 +99,6 @@ static void sc_nand_read_buf(struct mtd_info *mtd, u_char 
*buf, int len)
}
 }
 
-#if defined(CONFIG_MTD_NAND_VERIFY_WRITE)
-/**
- * sc_nand_verify_buf -  Verify chip data against buffer
- * @mtd:   MTD device structure
- * @buf:   buffer containing the data to compare
- * @len:   number of bytes to compare
- */
-static int sc_nand_verify_buf(struct mtd_info *mtd, const u_char *buf, int len)
-{
-   int i;
-
-   for (i = 0; i  len; i++) {
-   if (buf[i] != sc_nand_read_byte(mtd));
-   return -EFAULT;
-   }
-   return 0

[U-Boot] [PATCH 1/3] dm: Prevent demo hello and demo status segfaults

2015-02-03 Thread Peter Tyser
Segfaults can occur when a mandatory argument is not provided to
demo hello and demo status.  Eg:

   = demo hello
   Segmentation fault (core dumped)

Add a check to ensure all required arguments are provided.

Signed-off-by: Peter Tyser pty...@xes-inc.com
---
 common/cmd_demo.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/common/cmd_demo.c b/common/cmd_demo.c
index 652c61c..b8b8578 100644
--- a/common/cmd_demo.c
+++ b/common/cmd_demo.c
@@ -76,7 +76,9 @@ static int do_demo(cmd_tbl_t *cmdtp, int flag, int argc, char 
* const argv[])
ARRAY_SIZE(demo_commands));
argc -= 2;
argv += 2;
-   if (!demo_cmd || argc  demo_cmd-maxargs)
+
+   if ((!demo_cmd || argc  demo_cmd-maxargs) ||
+   ((demo_cmd-name[0] != 'l')  (argc  1)))
return CMD_RET_USAGE;
 
if (argc) {
-- 
1.9.1

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


Re: [U-Boot] [PATCH 1/3] dm: Prevent demo hello and demo status segfaults

2015-02-03 Thread Peter Tyser
Hi Simon,

On Tue, 2015-02-03 at 13:18 -0600, Peter Tyser wrote:
 Segfaults can occur when a mandatory argument is not provided to
 demo hello and demo status.  Eg:
 
= demo hello
Segmentation fault (core dumped)
 
 Add a check to ensure all required arguments are provided.
 
 Signed-off-by: Peter Tyser pty...@xes-inc.com

The 1/3 in the subject line is an error - there are no other
patches in the series.

Peter
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH 1/5] nand: Use common read function instead of verify_buf()

2015-01-29 Thread Peter Tyser

On Thu, 2015-01-29 at 17:02 -0600, Scott Wood wrote:
 On Tue, 2015-01-27 at 17:47 -0600, Peter Tyser wrote:
  Hi Scott,
  
  
I waffled about removing it, but leaned towards leaving it in because:
- I didn't want to change the existing U-Boot behavior for other
users.  A google of 'u-boot nand write' shows a lot of examples that
don't include verification of writes, and they should if we remove
auto-verification.
   
   How many configs actually enable this option?  I don't see many beyond
   the FSL PPC boards (which are so full of copy-and-paste that it probably
   wasn't deliberate).
  
  Yeah, the majority are FSL 83xx and 85xx, with 2 or so random ARM boards.
  
- The reason it was removed in Linux was Both UBI and JFFS2 are able
to read verify what they wrote already.  There are also MTD tests
which do this verification.  I thought U-Boot was more likely than
Linux to use raw NAND writes without a filesystem, so leaving it in U-
Boot made sense since the UBI/JFFS2 logic didn't apply as much here.
   
   Right, though raw writes ought to be limited to blocks that aren't
   written often enough to fail.
   
- I didn't think a lot of people would know they have to explicitly
verify NAND contents after a write, since they'd assume it was like
other memories that aren't as lossy.

- The penalty of slightly different code from Linux and a small
performance hit was worth the gain of auto-verification to me.  I
viewed consolidating it into one small chunk of code as a happy medium.
   
   The davinci patches show that there can still be driver dependencies
   depending on what the driver overrides.  I'm not hugely opposed, but it
   seems like it would be better to do it at a higher level (e.g. in
   nand_util.c with a flag to enable, and either make support mandatory, or
   if you try to use that command variant without support it fails rather
   than silently not verifying).
  
  That seems like a good idea.  How about:
  - Remove all CONFIG_MTD_NAND_VERIFY_WRITE references
  
  - Add a new flag WITH_WR_VERIFY and have nand_write_skip_bad() in
  nand_util.c verify writes only when it is set.
  
  - Update the calls to nand_write_skip_bad() in cmd_nand.c to include
  the new WITH_WR_VERIFY flag.  I'd vote to enable it for all boards,
  but let me know if you disagree.
  
  That would make all nand write commands verify writes, with the
  exception of nand write.raw.  Any opinion on if this should also
  be verified?  I only use it for development/testing, so don't have
  a strong opinion.
 
 raw refers to the absence of ECC, and I'd rather not overload it to
 mean don't verify.  Should it also be possible to request non-raw
 non-verified accesses?  Or should we always verify and wait until
 someone complains about performance?

OK, I'll add verification to the nand write.raw functionality too.
I'd lean towards always verifying and waiting until/if someone
complains about performance.  I doubt many (any?) people are doing
timing critical writes in U-Boot.  I think the argument that writes
should be verified carries some water, and it'd be nice to not
make the command arguments more complicated than they already are.

 What about DFU and other non-cmd_nand NAND accesses?

Are there other non-cmd NAND accesses other than DFU?  None jumped
out at me.  I'm not too familiar with DFU, but in theory the DFU
programming utilities could already be doing their own verification.
I took a quick look at the dfu-util source, and it doesn't appear
to be doing its own.  I'd vote to verify the DFU writes too, since
even more than nand write its performance shouldn't be very
critical.  I'll break DFU verification out into a separate patch,
and you or others can ACK or reject it then.

Let me know if the above sounds good and I'll make the changes.

Thanks,
Peter
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH 1/5] nand: Use common read function instead of verify_buf()

2015-01-27 Thread Peter Tyser
Hi Scott,


  I waffled about removing it, but leaned towards leaving it in because:
  - I didn't want to change the existing U-Boot behavior for other
  users.  A google of 'u-boot nand write' shows a lot of examples that
  don't include verification of writes, and they should if we remove
  auto-verification.
 
 How many configs actually enable this option?  I don't see many beyond
 the FSL PPC boards (which are so full of copy-and-paste that it probably
 wasn't deliberate).

Yeah, the majority are FSL 83xx and 85xx, with 2 or so random ARM boards.

  - The reason it was removed in Linux was Both UBI and JFFS2 are able
  to read verify what they wrote already.  There are also MTD tests
  which do this verification.  I thought U-Boot was more likely than
  Linux to use raw NAND writes without a filesystem, so leaving it in U-
  Boot made sense since the UBI/JFFS2 logic didn't apply as much here.
 
 Right, though raw writes ought to be limited to blocks that aren't
 written often enough to fail.
 
  - I didn't think a lot of people would know they have to explicitly
  verify NAND contents after a write, since they'd assume it was like
  other memories that aren't as lossy.
  
  - The penalty of slightly different code from Linux and a small
  performance hit was worth the gain of auto-verification to me.  I
  viewed consolidating it into one small chunk of code as a happy medium.
 
 The davinci patches show that there can still be driver dependencies
 depending on what the driver overrides.  I'm not hugely opposed, but it
 seems like it would be better to do it at a higher level (e.g. in
 nand_util.c with a flag to enable, and either make support mandatory, or
 if you try to use that command variant without support it fails rather
 than silently not verifying).

That seems like a good idea.  How about:
- Remove all CONFIG_MTD_NAND_VERIFY_WRITE references

- Add a new flag WITH_WR_VERIFY and have nand_write_skip_bad() in 
nand_util.c verify writes only when it is set.

- Update the calls to nand_write_skip_bad() in cmd_nand.c to include
the new WITH_WR_VERIFY flag.  I'd vote to enable it for all boards,
but let me know if you disagree.

That would make all nand write commands verify writes, with the
exception of nand write.raw.  Any opinion on if this should also
be verified?  I only use it for development/testing, so don't have
a strong opinion.

Regards,
Peter



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


Re: [U-Boot] [PATCH] patman: Make dry-run output match real functionality

2015-01-27 Thread Peter Tyser

On Mon, 2015-01-26 at 22:21 -0700, Simon Glass wrote:
 Hi Peter,
 
 On 26 January 2015 at 10:42, Peter Tyser pty...@xes-inc.com wrote:
  When run with the --dry-run argument patman prints out information
  showing what it would do.  This information currently doesn't line up
  with what patman/git send-email really do.  Some basic examples:
  - If an email address is addressed via Series-cc and Patch-cc patman
shows that email address would be CC-ed two times.
  - If an email address is addressed via Series-to and Patch-cc patman
shows that email address would be sent TO and CC-ed.
  - If an email address is addressed from a combination of tag aliases,
get_maintainer.pl output, Series-cc, Patch-cc, etc patman shows
that the email address would be CC-ed multiple times.
  
  Patman currently does try to send duplicate emails like the --dry-run
  output shows, but git send-email intelligently removes duplicate
  addresses so this patch shouldn't change the non-dry-run functionality.
  
  Change patman's output and email addressing to line up with the
  git send-email logic.  This trims down patman's dry-run output and
  prevents confusion about what patman will do when emails are actually
  sent.
 
 Thanks for the patch, it's good to match up with git send-email.
 
 Are the rules that git send-email follows documented or obtained by
 trial and error?

Trial and error initially.  The git source code lined up with what I
saw (see the send_message function in git-send-email.perl).  I didn't
see the policy documented officially, but what git does makes sense
to me:
- remove any duplicate addresses in the to: field
- remove all the to: addresses from the cc: addresses
- remove any duplicate cc: addresses

This makes sure each email is only sent to an address one time,
with the to: field taking precedence over the cc: field.


For a recent NAND patch series it looked like I was going to send Scott
2-4 emails per patch which is why I looked into it.

Regards,
Peter


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


[U-Boot] [PATCH] patman: Make dry-run output match real functionality

2015-01-26 Thread Peter Tyser
When run with the --dry-run argument patman prints out information
showing what it would do.  This information currently doesn't line up
with what patman/git send-email really do.  Some basic examples:
- If an email address is addressed via Series-cc and Patch-cc patman
  shows that email address would be CC-ed two times.
- If an email address is addressed via Series-to and Patch-cc patman
  shows that email address would be sent TO and CC-ed.
- If an email address is addressed from a combination of tag aliases,
  get_maintainer.pl output, Series-cc, Patch-cc, etc patman shows
  that the email address would be CC-ed multiple times.

Patman currently does try to send duplicate emails like the --dry-run
output shows, but git send-email intelligently removes duplicate
addresses so this patch shouldn't change the non-dry-run functionality.

Change patman's output and email addressing to line up with the
git send-email logic.  This trims down patman's dry-run output and
prevents confusion about what patman will do when emails are actually
sent.

Signed-off-by: Peter Tyser pty...@xes-inc.com
---

 tools/patman/gitutil.py |  3 ++-
 tools/patman/series.py  | 21 -
 2 files changed, 10 insertions(+), 14 deletions(-)

diff --git a/tools/patman/gitutil.py b/tools/patman/gitutil.py
index cc5a55a..c593070 100644
--- a/tools/patman/gitutil.py
+++ b/tools/patman/gitutil.py
@@ -392,7 +392,8 @@ def EmailPatches(series, cover_fname, args, dry_run, 
raise_on_error, cc_fname,
Or do something like this\n
git config sendemail.to u-boot@lists.denx.de)
 return
-cc = BuildEmailList(series.get('cc'), '--cc', alias, raise_on_error)
+cc = BuildEmailList(list(set(series.get('cc')) - set(series.get('to'))),
+'--cc', alias, raise_on_error)
 if self_only:
 to = BuildEmailList([os.getenv('USER')], '--to', alias, raise_on_error)
 cc = []
diff --git a/tools/patman/series.py b/tools/patman/series.py
index b67f870..60ebc76 100644
--- a/tools/patman/series.py
+++ b/tools/patman/series.py
@@ -94,6 +94,9 @@ class Series(dict):
 cmd: The git command we would have run
 process_tags: Process tags as if they were aliases
 
+to_set = set(gitutil.BuildEmailList(self.to));
+cc_set = set(gitutil.BuildEmailList(self.cc));
+
 col = terminal.Color()
 print 'Dry run, so not doing much. But I would do this:'
 print
@@ -106,24 +109,16 @@ class Series(dict):
 commit = self.commits[upto]
 print col.Color(col.GREEN, '   %s' % args[upto])
 cc_list = list(self._generated_cc[commit.patch])
-
-# Skip items in To list
-if 'to' in self:
-try:
-map(cc_list.remove, gitutil.BuildEmailList(self.to))
-except ValueError:
-pass
-
-for email in cc_list:
+for email in set(cc_list) - to_set - cc_set:
 if email == None:
 email = col.Color(col.YELLOW, alias '%s' not found
 % tag)
 if email:
 print '  Cc: ',email
 print
-for item in gitutil.BuildEmailList(self.get('to', 'none')):
+for item in to_set:
 print 'To:\t ', item
-for item in gitutil.BuildEmailList(self.cc):
+for item in cc_set - to_set:
 print 'Cc:\t ', item
 print 'Version: ', self.get('version')
 print 'Prefix:\t ', self.get('prefix')
@@ -131,7 +126,7 @@ class Series(dict):
 print 'Cover: %d lines' % len(self.cover)
 cover_cc = gitutil.BuildEmailList(self.get('cover_cc', ''))
 all_ccs = itertools.chain(cover_cc, *self._generated_cc.values())
-for email in set(all_ccs):
+for email in set(all_ccs) - to_set - cc_set:
 print '  Cc: ',email
 if cmd:
 print 'Git command: %s' % cmd
@@ -230,7 +225,7 @@ class Series(dict):
if add_maintainers:
 list += get_maintainer.GetMaintainer(commit.patch)
 all_ccs += list
-print fd, commit.patch, ', '.join(list)
+print fd, commit.patch, ', '.join(set(list))
 self._generated_cc[commit.patch] = list
 
 if cover_fname:
-- 
1.9.1

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


[U-Boot] [PATCH 3/5] mtd: nand: Remove nand_verify_buf() function

2015-01-26 Thread Peter Tyser
The nand_verify_buf() function is no longer used, so remove it.  This
function has been removed in mainline Linux for a long time, so it
brings U-Boot's NAND implementation a bit closer to its source.

Signed-off-by: Peter Tyser pty...@xes-inc.com
---

 board/prodrive/alpr/nand.c   | 16 -
 board/socrates/nand.c| 25 
 drivers/mtd/nand/fsl_elbc_nand.c | 38 --
 drivers/mtd/nand/fsl_ifc_nand.c  | 38 --
 drivers/mtd/nand/fsl_upm.c   | 19 +--
 drivers/mtd/nand/mpc5121_nfc.c   | 26 
 drivers/mtd/nand/mxc_nand.c  | 33 --
 drivers/mtd/nand/nand_base.c | 51 
 drivers/mtd/nand/ndfc.c  | 20 +---
 include/linux/mtd/nand.h |  5 
 10 files changed, 2 insertions(+), 269 deletions(-)

diff --git a/board/prodrive/alpr/nand.c b/board/prodrive/alpr/nand.c
index 5427de5..ca40cea 100644
--- a/board/prodrive/alpr/nand.c
+++ b/board/prodrive/alpr/nand.c
@@ -93,19 +93,6 @@ static void alpr_nand_read_buf(struct mtd_info *mtd, u_char 
*buf, int len)
}
 }
 
-#if defined(CONFIG_MTD_NAND_VERIFY_WRITE)
-static int alpr_nand_verify_buf(struct mtd_info *mtd, const u_char *buf, int 
len)
-{
-   int i;
-
-   for (i = 0; i  len; i++)
-   if (buf[i] != readb((alpr_ndfc-data)))
-   return i;
-
-   return 0;
-}
-#endif
-
 static int alpr_nand_dev_ready(struct mtd_info *mtd)
 {
/*
@@ -130,9 +117,6 @@ int board_nand_init(struct nand_chip *nand)
nand-read_byte  = alpr_nand_read_byte;
nand-write_buf  = alpr_nand_write_buf;
nand-read_buf   = alpr_nand_read_buf;
-#if defined(CONFIG_MTD_NAND_VERIFY_WRITE)
-   nand-verify_buf = alpr_nand_verify_buf;
-#endif
nand-dev_ready  = alpr_nand_dev_ready;
 
return 0;
diff --git a/board/socrates/nand.c b/board/socrates/nand.c
index 7394478..15e6ea6 100644
--- a/board/socrates/nand.c
+++ b/board/socrates/nand.c
@@ -18,9 +18,6 @@ static void sc_nand_write_buf(struct mtd_info *mtd, const 
u_char *buf, int len);
 static u_char sc_nand_read_byte(struct mtd_info *mtd);
 static u16 sc_nand_read_word(struct mtd_info *mtd);
 static void sc_nand_read_buf(struct mtd_info *mtd, u_char *buf, int len);
-#if defined(CONFIG_MTD_NAND_VERIFY_WRITE)
-static int sc_nand_verify_buf(struct mtd_info *mtd, const u_char *buf, int 
len);
-#endif
 static int sc_nand_device_ready(struct mtd_info *mtdinfo);
 
 #define FPGA_NAND_CMD_MASK (0x7  28)
@@ -102,25 +99,6 @@ static void sc_nand_read_buf(struct mtd_info *mtd, u_char 
*buf, int len)
}
 }
 
-#if defined(CONFIG_MTD_NAND_VERIFY_WRITE)
-/**
- * sc_nand_verify_buf -  Verify chip data against buffer
- * @mtd:   MTD device structure
- * @buf:   buffer containing the data to compare
- * @len:   number of bytes to compare
- */
-static int sc_nand_verify_buf(struct mtd_info *mtd, const u_char *buf, int len)
-{
-   int i;
-
-   for (i = 0; i  len; i++) {
-   if (buf[i] != sc_nand_read_byte(mtd));
-   return -EFAULT;
-   }
-   return 0;
-}
-#endif
-
 /**
  * sc_nand_device_ready - Check the NAND device is ready for next command.
  * @mtd:   MTD device structure
@@ -178,9 +156,6 @@ int board_nand_init(struct nand_chip *nand)
nand-read_word = sc_nand_read_word;
nand-write_buf = sc_nand_write_buf;
nand-read_buf = sc_nand_read_buf;
-#if defined(CONFIG_MTD_NAND_VERIFY_WRITE)
-   nand-verify_buf = sc_nand_verify_buf;
-#endif
 
return 0;
 }
diff --git a/drivers/mtd/nand/fsl_elbc_nand.c b/drivers/mtd/nand/fsl_elbc_nand.c
index 3372b64..e85832d 100644
--- a/drivers/mtd/nand/fsl_elbc_nand.c
+++ b/drivers/mtd/nand/fsl_elbc_nand.c
@@ -561,41 +561,6 @@ static void fsl_elbc_read_buf(struct mtd_info *mtd, u8 
*buf, int len)
   len, avail);
 }
 
-#if defined(CONFIG_MTD_NAND_VERIFY_WRITE)
-/*
- * Verify buffer against the FCM Controller Data Buffer
- */
-static int fsl_elbc_verify_buf(struct mtd_info *mtd,
-  const u_char *buf, int len)
-{
-   struct nand_chip *chip = mtd-priv;
-   struct fsl_elbc_mtd *priv = chip-priv;
-   struct fsl_elbc_ctrl *ctrl = priv-ctrl;
-   int i;
-
-   if (len  0) {
-   printf(write_buf of %d bytes, len);
-   return -EINVAL;
-   }
-
-   if ((unsigned int)len  ctrl-read_bytes - ctrl-index) {
-   printf(verify_buf beyond end of buffer 
-  (%d requested, %u available)\n,
-  len, ctrl-read_bytes - ctrl-index);
-
-   ctrl-index = ctrl-read_bytes;
-   return -EINVAL;
-   }
-
-   for (i = 0; i  len; i++)
-   if (in_8(ctrl-addr[ctrl-index + i]) != buf[i])
-   break;
-
-   ctrl-index += len;
-   return i == len

Re: [U-Boot] [PATCH 1/5] nand: Use common read function instead of verify_buf()

2015-01-26 Thread Peter Tyser

On Mon, 2015-01-26 at 16:33 -0600, Scott Wood wrote:
 On Mon, 2015-01-26 at 16:24 -0600, Peter Tyser wrote:
  The driver-specific verify_buf() function can be replaced with the
  standard read_page_raw() function to verify writes.  This will 
  allow
  verify_buf() to be removed from individual drivers.  verify_buf() 
  is no
  longer supported in mainline Linux, so it is a pain to continue
  supporting.
 
 Any reason not to just remove this feature from U-Boot, as Linux has
 done?

The Linux change for reference: 
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=657f28f8811c92724db10d18bbbec70d540147d6

I waffled about removing it, but leaned towards leaving it in because:
- I didn't want to change the existing U-Boot behavior for other 
users.  A google of 'u-boot nand write' shows a lot of examples that 
don't include verification of writes, and they should if we remove 
auto-verification.

- The reason it was removed in Linux was Both UBI and JFFS2 are able 
to read verify what they wrote already.  There are also MTD tests 
which do this verification.  I thought U-Boot was more likely than 
Linux to use raw NAND writes without a filesystem, so leaving it in U-
Boot made sense since the UBI/JFFS2 logic didn't apply as much here.



- I didn't think a lot of people would know they have to explicitly 
verify NAND contents after a write, since they'd assume it was like 
other memories that aren't as lossy.

- The penalty of slightly different code from Linux and a small 
performance hit was worth the gain of auto-verification to me.  I 
viewed consolidating it into one small chunk of code as a happy medium.


The philosophical side of me said to remove it altogether, so I can 
see that perspective too.
Peter
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


[U-Boot] [PATCH 5/5] mtd: davinci nand: Use ECC for NAND write verification

2015-01-26 Thread Peter Tyser
From: Joe Schaack jscha...@xes-inc.com

Modify the nand_davinci_write_page() function to use ECC when appropriate
to verify writes.  Previously if a single bit error occured and software
ECC was used the write verification would report a failure.  However,
the write really did succeed, since ECC can handle the error.

Signed-off-by: Joe Schaack jscha...@xes-inc.com
Signed-off-by: Peter Tyser pty...@xes-inc.com
---
I don't have davinci hardware to test on, but the change should
mirror the nand_base.c change.

 drivers/mtd/nand/davinci_nand.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/davinci_nand.c b/drivers/mtd/nand/davinci_nand.c
index b4e22d1..cd0b07c 100644
--- a/drivers/mtd/nand/davinci_nand.c
+++ b/drivers/mtd/nand/davinci_nand.c
@@ -415,7 +415,10 @@ static int nand_davinci_write_page(struct mtd_info *mtd, 
struct nand_chip *chip,
 
/* Send command to read back the data */
chip-cmdfunc(mtd, NAND_CMD_READ0, 0, page);
-   chip-ecc.read_page_raw(mtd, chip, vfy_buf, oob_required, page);
+   if (unlikely(raw))
+   chip-ecc.read_page_raw(mtd, chip, vfy_buf, oob_required, page);
+   else
+   chip-ecc.read_page(mtd, chip, vfy_buf, oob_required, page);
 
if (memcmp(buf, vfy_buf, mtd-writesize))
ret = -EIO;
-- 
1.9.1

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


[U-Boot] [PATCH 4/5] mtd: nand: Use ECC for NAND write verification

2015-01-26 Thread Peter Tyser
From: Joe Schaack jscha...@xes-inc.com

Modify the nand_write_page() function to use ECC when appropriate to verify
writes.  Previously if a single bit error occured and software ECC was
used the write verification would report a failure.  However, the write
really did succeed, since ECC can handle the error.

The issue can be simulated with a sequence of commands such as:

   # Inject a fake bit that is stuck low (2K page flash being used)
   nand erase 0 0x2
   mw.b 0x1 0xff 0x1000
   mw.b 0x1 0xfe 1
   nand write.raw 0x1 0x0 0x1

   # Write some data which needs to toggle the fake stuck bit
   mw.b 0x1 0xab 1
   nand write 0x1 0x0 0x800

An error will occur:
NAND write: device 0 offset 0x0, size 0x800
NAND write to offset 0 failed -5
0 bytes written: ERROR

But you can verify data was correctly written:
# Show data is correct when ECC is used
nand read 0x1 0x0 0x800
md 0x1

# Show the bit is still stuck low
nand read.raw 0x1 0x0 0x1
md 0x1

Change the behavior so ECC is used when verifying non-raw writes.
This only impacts boards that have COFNIG_MTD_NAND_VERIFY_WRITE
defined.

Signed-off-by: Joe Schaack jscha...@xes-inc.com
Signed-off-by: Peter Tyser pty...@xes-inc.com
---

 drivers/mtd/nand/nand_base.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index abcb84a..aa039ef 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -2405,7 +2405,10 @@ static int nand_write_page(struct mtd_info *mtd, struct 
nand_chip *chip,
 
/* Send command to read back the data */
chip-cmdfunc(mtd, NAND_CMD_READ0, 0, page);
-   chip-ecc.read_page_raw(mtd, chip, vfy_buf, oob_required, page);
+   if (unlikely(raw))
+   chip-ecc.read_page_raw(mtd, chip, vfy_buf, oob_required, page);
+   else
+   chip-ecc.read_page(mtd, chip, vfy_buf, oob_required, page);
 
status = memcmp(buf, vfy_buf, mtd-writesize);
 
-- 
1.9.1

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


[U-Boot] [PATCH 2/5] mtd: davinci_nand: Use common read function instead of verify_buf()

2015-01-26 Thread Peter Tyser
The driver-specific verify_buf() function can be replaced with the
standard read_page_raw() function to verify writes.  This will allow
verify_buf() to be removed.  verify_buf() is no longer supported in
mainline Linux, so it is a pain to continue supporting.

Signed-off-by: Peter Tyser pty...@xes-inc.com
---
I don't have davinci hardware to test on, but the change should
mirror the nand_base.c change.

 drivers/mtd/nand/davinci_nand.c | 17 +
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/mtd/nand/davinci_nand.c b/drivers/mtd/nand/davinci_nand.c
index 41689b5..b4e22d1 100644
--- a/drivers/mtd/nand/davinci_nand.c
+++ b/drivers/mtd/nand/davinci_nand.c
@@ -370,6 +370,7 @@ static int nand_davinci_write_page(struct mtd_info *mtd, 
struct nand_chip *chip,
int status;
int ret = 0;
struct nand_ecclayout *saved_ecc_layout;
+   __maybe_unused uint8_t *vfy_buf;
 
/* save current ECC layout and assign Keystone RBL ECC layout */
if (page  CONFIG_KEYSTONE_NAND_MAX_RBL_PAGE) {
@@ -406,16 +407,24 @@ static int nand_davinci_write_page(struct mtd_info *mtd, 
struct nand_chip *chip,
}
 
 #ifdef CONFIG_MTD_NAND_VERIFY_WRITE
+   vfy_buf = malloc(mtd-writesize);
+   if (!vfy_buf) {
+   ret = -ENOMEM;
+   goto err;
+   }
+
/* Send command to read back the data */
chip-cmdfunc(mtd, NAND_CMD_READ0, 0, page);
+   chip-ecc.read_page_raw(mtd, chip, vfy_buf, oob_required, page);
 
-   if (chip-verify_buf(mtd, buf, mtd-writesize)) {
+   if (memcmp(buf, vfy_buf, mtd-writesize))
ret = -EIO;
-   goto err;
-   }
+
+   free(vfy_buf);
 
/* Make sure the next page prog is preceded by a status read */
-   chip-cmdfunc(mtd, NAND_CMD_STATUS, -1, -1);
+   if (!ret)
+   chip-cmdfunc(mtd, NAND_CMD_STATUS, -1, -1);
 #endif
 err:
/* restore ECC layout */
-- 
1.9.1

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


[U-Boot] [PATCH 1/5] nand: Use common read function instead of verify_buf()

2015-01-26 Thread Peter Tyser
The driver-specific verify_buf() function can be replaced with the
standard read_page_raw() function to verify writes.  This will allow
verify_buf() to be removed from individual drivers.  verify_buf() is no
longer supported in mainline Linux, so it is a pain to continue
supporting.

Signed-off-by: Peter Tyser pty...@xes-inc.com
---

 drivers/mtd/nand/nand_base.c | 12 +++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index 63bdf65..788846a 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -2394,6 +2394,7 @@ static int nand_write_page(struct mtd_info *mtd, struct 
nand_chip *chip,
int oob_required, int page, int cached, int raw)
 {
int status, subpage;
+   __maybe_unused uint8_t *vfy_buf;
 
if (!(chip-options  NAND_NO_SUBPAGE_WRITE) 
chip-ecc.write_subpage)
@@ -2443,10 +2444,19 @@ static int nand_write_page(struct mtd_info *mtd, struct 
nand_chip *chip,
 
 #ifdef __UBOOT__
 #if defined(CONFIG_MTD_NAND_VERIFY_WRITE)
+   vfy_buf = malloc(mtd-writesize);
+   if (!vfy_buf)
+   return -ENOMEM;
+
/* Send command to read back the data */
chip-cmdfunc(mtd, NAND_CMD_READ0, 0, page);
+   chip-ecc.read_page_raw(mtd, chip, vfy_buf, oob_required, page);
+
+   status = memcmp(buf, vfy_buf, mtd-writesize);
 
-   if (chip-verify_buf(mtd, buf, mtd-writesize))
+   free(vfy_buf);
+
+   if (status)
return -EIO;
 
/* Make sure the next page prog is preceded by a status read */
-- 
1.9.1

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


Re: [U-Boot] Standalone API examples on the P4080DS

2015-01-16 Thread Peter Tyser
Hi David,


 It looks like the entry point is 0x4. Why does the tutorial state
 that the example needs to be run from a 4 byte offset? In this case 
 this
 will result in a failure to create a stack frame for hello_world.

For PowerPC the entry point used to be non-deterministic, but was 
generally 0x40004 which is where the wiki value came from.  The 
following 2 patches made the entry point consistently 0x4:

http://git.denx.de/cgi-bin/gitweb.cgi?p=u-boot.git;a=commit;h=620bbba524fbaa26971a5004793010b169824f1b

http://git.denx.de/cgi-bin/gitweb.cgi?p=u-boot.git;a=commit;h=c91d456c055237bdadb99a80e198748b8cf32595

I updated the wiki to match the newer 0x4 address, which is valid 
for PowerPC boards with a comment to check out 
CONFIG_STANDALONE_LOAD_ADDR for other architectures.

Peter
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


[U-Boot] [PATCH 2/2] powerpc: xes: Add maintainer

2015-01-09 Thread Peter Tyser
Add Peter Tyser as the maintainer of Extreme Engineering Solutions products.

Signed-off-by: Peter Tyser pty...@xes-inc.com
---
 board/xes/xpedite517x/MAINTAINERS | 2 +-
 board/xes/xpedite520x/MAINTAINERS | 2 +-
 board/xes/xpedite537x/MAINTAINERS | 2 +-
 board/xes/xpedite550x/MAINTAINERS | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/board/xes/xpedite517x/MAINTAINERS 
b/board/xes/xpedite517x/MAINTAINERS
index 035cb14..26e0acc 100644
--- a/board/xes/xpedite517x/MAINTAINERS
+++ b/board/xes/xpedite517x/MAINTAINERS
@@ -1,5 +1,5 @@
 XPEDITE517X BOARD
-#M:-
+M: Peter Tyser pty...@xes-inc.com
 S: Maintained
 F: board/xes/xpedite517x/
 F: include/configs/xpedite517x.h
diff --git a/board/xes/xpedite520x/MAINTAINERS 
b/board/xes/xpedite520x/MAINTAINERS
index 2fd4ac0..f7bd437 100644
--- a/board/xes/xpedite520x/MAINTAINERS
+++ b/board/xes/xpedite520x/MAINTAINERS
@@ -1,5 +1,5 @@
 XPEDITE520X BOARD
-#M:-
+M: Peter Tyser pty...@xes-inc.com
 S: Maintained
 F: board/xes/xpedite520x/
 F: include/configs/xpedite520x.h
diff --git a/board/xes/xpedite537x/MAINTAINERS 
b/board/xes/xpedite537x/MAINTAINERS
index 45a420d..b6123ac 100644
--- a/board/xes/xpedite537x/MAINTAINERS
+++ b/board/xes/xpedite537x/MAINTAINERS
@@ -1,5 +1,5 @@
 XPEDITE537X BOARD
-#M:-
+M: Peter Tyser pty...@xes-inc.com
 S: Maintained
 F: board/xes/xpedite537x/
 F: include/configs/xpedite537x.h
diff --git a/board/xes/xpedite550x/MAINTAINERS 
b/board/xes/xpedite550x/MAINTAINERS
index b22f0e6..017f368 100644
--- a/board/xes/xpedite550x/MAINTAINERS
+++ b/board/xes/xpedite550x/MAINTAINERS
@@ -1,5 +1,5 @@
 XPEDITE550X BOARD
-#M:-
+M: Peter Tyser pty...@xes-inc.com
 S: Maintained
 F: board/xes/xpedite550x/
 F: include/configs/xpedite550x.h
-- 
1.9.1

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


[U-Boot] [PATCH 1/2] powerpc: xes: Convert to generic board

2015-01-09 Thread Peter Tyser
From: John Schmoller jschmol...@xes-inc.com

Convert Extreme Engineering Solutions products to use generic board
support.

Signed-off-by: John Schmoller jschmol...@xes-inc.com
Signed-off-by: Peter Tyser pty...@xes-inc.com
---
 include/configs/xpedite1000.h | 2 ++
 include/configs/xpedite517x.h | 2 ++
 include/configs/xpedite520x.h | 2 ++
 include/configs/xpedite537x.h | 2 ++
 include/configs/xpedite550x.h | 2 ++
 5 files changed, 10 insertions(+)

diff --git a/include/configs/xpedite1000.h b/include/configs/xpedite1000.h
index ca322b2..15c9176 100644
--- a/include/configs/xpedite1000.h
+++ b/include/configs/xpedite1000.h
@@ -22,6 +22,8 @@
 #define CONFIG_440GX   1   /* 440 GX */
 #define CONFIG_BOARD_EARLY_INIT_F 1/* Call board_pre_init  */
 #define CONFIG_SYS_CLK_FREQ/* external freq to pll */
+#define CONFIG_SYS_GENERIC_BOARD
+#define CONFIG_DISPLAY_BOARDINFO
 
 #defineCONFIG_SYS_TEXT_BASE0xFFF8
 
diff --git a/include/configs/xpedite517x.h b/include/configs/xpedite517x.h
index cbf4b8e..3414230 100644
--- a/include/configs/xpedite517x.h
+++ b/include/configs/xpedite517x.h
@@ -23,6 +23,8 @@
 #define CONFIG_BAT_RW  1   /* Use common BAT rw code */
 #define CONFIG_HIGH_BATS   1   /* High BATs supported and enabled */
 #define CONFIG_ALTIVEC 1
+#define CONFIG_SYS_GENERIC_BOARD
+#define CONFIG_DISPLAY_BOARDINFO
 
 #defineCONFIG_SYS_TEXT_BASE0xfff0
 
diff --git a/include/configs/xpedite520x.h b/include/configs/xpedite520x.h
index baa3039..f966a8a 100644
--- a/include/configs/xpedite520x.h
+++ b/include/configs/xpedite520x.h
@@ -21,6 +21,8 @@
 #define CONFIG_SYS_BOARD_NAME  XPedite5200
 #define CONFIG_SYS_FORM_PMC_XMC1
 #define CONFIG_BOARD_EARLY_INIT_R  /* Call board_pre_init */
+#define CONFIG_SYS_GENERIC_BOARD
+#define CONFIG_DISPLAY_BOARDINFO
 
 #ifndef CONFIG_SYS_TEXT_BASE
 #define CONFIG_SYS_TEXT_BASE   0xfff8
diff --git a/include/configs/xpedite537x.h b/include/configs/xpedite537x.h
index bdf5576..d6b6143 100644
--- a/include/configs/xpedite537x.h
+++ b/include/configs/xpedite537x.h
@@ -21,6 +21,8 @@
 #define CONFIG_SYS_BOARD_NAME  XPedite5370
 #define CONFIG_SYS_FORM_3U_VPX 1
 #define CONFIG_BOARD_EARLY_INIT_R  /* Call board_pre_init */
+#define CONFIG_SYS_GENERIC_BOARD
+#define CONFIG_DISPLAY_BOARDINFO
 
 #ifndef CONFIG_SYS_TEXT_BASE
 #define CONFIG_SYS_TEXT_BASE   0xfff8
diff --git a/include/configs/xpedite550x.h b/include/configs/xpedite550x.h
index 0b24f3e..4536b94 100644
--- a/include/configs/xpedite550x.h
+++ b/include/configs/xpedite550x.h
@@ -22,6 +22,8 @@
 #define CONFIG_SYS_FORM_PMC_XMC1
 #define CONFIG_PRPMC_PCI_ALIAS pci0  /* Processor PMC interface on pci0 */
 #define CONFIG_BOARD_EARLY_INIT_R  /* Call board_pre_init */
+#define CONFIG_SYS_GENERIC_BOARD
+#define CONFIG_DISPLAY_BOARDINFO
 
 #ifndef CONFIG_SYS_TEXT_BASE
 #define CONFIG_SYS_TEXT_BASE   0xfff8
-- 
1.9.1

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


Re: [U-Boot] Sparc build warnings

2012-07-22 Thread Peter Tyser
Hi Marek,

 I just recently got the following issues when building Sparc platforms with 
 debian gcc 4.7.1-5. Peter, you seems to be involved, can you please check on 
 these?

I'm not involved with these boards or Sparc, so not sure if I'm much
help.  I believe Dan Hellstrom is the best cantidate (added to CC).

Most the the warnings look pretty trivial to fix, so I could gin
something up, but I don't have a Sparc toolchain to actually test any
modifications.

Best,
Peter

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


Re: [U-Boot] USB only works in Linux if started in UBoot

2011-05-12 Thread Peter Tyser
On Thu, 2011-05-12 at 15:36 +, Einar Már Björgvinsson wrote:
 hi
 
 I was wondering what I was doing wrong here !
 
 The USB functionality in Linux only works if I run 'usb start' in UBoot 
 before starting the kernel.
 
 Not sure what will resolve this, of course I can make automatic start command 
 during starting of the bootloader but I think that's not optimal.
 
 Any thoughts on this ?

What hardware are you running on?  What kernel version are you using?
What U-Boot version are you using?

This issue was present on some Freescale CPUs, but has been fixed in the
mainline kernel by the following commit in case you're running on one of
their CPUs:
http://git.kernel.org/?p=linux/kernel/git/next/linux-next.git;a=commitdiff;h=cc604ddd118cf4a699c12bc41a5fa2d2f225f702;hp=ad84e4a9efb7c8ed322bafb6ebdb9c3a49a3d3a8

Best,
Peter

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


[U-Boot] [PATCH] cfi_flash: Fix CONFIG_SYS_FLASH_AUTOPROTECT_LIST usage

2011-04-13 Thread Peter Tyser
Commit 6ee1416e8184b4d9ebe6087d396a60bcecf3551c (mtd, cfi: introduce
void flash_protect_default(void)) introduced a bug which resulted in
boards that define CONFIG_SYS_FLASH_AUTOPROTECT_LIST not compiling with
the the following errors and warning:
  ptyser@petert u-boot $ make -s xpedite520x
  Configuring for xpedite520x board...
  cfi_flash.c: In function 'flash_protect_default':
  cfi_flash.c:2118: error: 'i' undeclared (first use in this function)
  cfi_flash.c:2118: error: (Each undeclared identifier is reported only once
  cfi_flash.c:2118: error: for each function it appears in.)
  cfi_flash.c:2118: error: 'apl' undeclared (first use in this function)
  cfi_flash.c:2118: error: invalid application of 'sizeof' to incomplete type 
'struct apl_s'
  cfi_flash.c: In function 'flash_init':
  cfi_flash.c:2137: warning: unused variable 'apl'

Signed-off-by: Peter Tyser pty...@xes-inc.com
Reported-by: Kumar Gala ga...@kernel.crashing.org
Cc: Heiko Schocher h...@denx.de
---
Thanks for noticing this Kumar.

 drivers/mtd/cfi_flash.c |   14 --
 1 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/mtd/cfi_flash.c b/drivers/mtd/cfi_flash.c
index 5788328..91ddcb4 100644
--- a/drivers/mtd/cfi_flash.c
+++ b/drivers/mtd/cfi_flash.c
@@ -2089,6 +2089,14 @@ static void cfi_flash_set_config_reg(u32 base, u16 val)
 
 void flash_protect_default(void)
 {
+#if defined(CONFIG_SYS_FLASH_AUTOPROTECT_LIST)
+   int i;
+   struct apl_s {
+   ulong start;
+   ulong size;
+   } apl[] = CONFIG_SYS_FLASH_AUTOPROTECT_LIST;
+#endif
+
/* Monitor protection ON by default */
 #if (CONFIG_SYS_MONITOR_BASE = CONFIG_SYS_FLASH_BASE)  \
(!defined(CONFIG_MONITOR_IS_IN_RAM))
@@ -2130,12 +2138,6 @@ unsigned long flash_init (void)
 {
unsigned long size = 0;
int i;
-#if defined(CONFIG_SYS_FLASH_AUTOPROTECT_LIST)
-   struct apl_s {
-   ulong start;
-   ulong size;
-   } apl[] = CONFIG_SYS_FLASH_AUTOPROTECT_LIST;
-#endif
 
 #ifdef CONFIG_SYS_FLASH_PROTECTION
/* read environment from EEPROM */
-- 
1.7.0.4

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


Re: [U-Boot] [PATCH v4] common: add a grepenv command

2011-04-05 Thread Peter Tyser
Hi Kim,

 +#ifdef CONFIG_CMD_GREPENV
 +static int do_env_grep (cmd_tbl_t *cmdtp, int flag, int argc, char * const 
 argv[])
 +{
 + ENTRY *match;
 + int matched[env_htab.size];
 + int rcode = 1, idx;
 +
 + if (argc  2) {
 + cmd_usage(cmdtp);
 + return 1;
 + }

This could be:
if (argc  2)
return cmd_usage(cmdtp);

 + for (idx = 0; idx  env_htab.size; idx++)
 + matched[idx] = 0;

memset()?

snip

 +#ifdef CONFIG_CMD_GREPENV
 +U_BOOT_CMD_COMPLETE(
 + grepenv, CONFIG_SYS_MAXARGS, 0,  do_env_grep,
 + search environment variables,
 + string ...\n
 + - list environment name=value pairs matching 'string',
 + var_complete
 +);
 +#endif

Support for env grep should also be added to the env command in
cmd_nvedit.c.  My understanding was that the individual printenv,
setenv, etc commands were deprecated in favor of the unified env
command.

Cool feature!

Peter


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


Re: [U-Boot] [PATCH V2] arm: Tegra2: add support for A9 CPU init

2011-03-25 Thread Peter Tyser
Hi Tom,
Things look pretty good.  Minor comments/questions below.

snip

 +/*
 + * TBD: Move cold_boot() to assembly file.
 + * Values/offsets of the table vars make this difficult.
 + */
 +
 +void cold_boot(void)
 +{
 + asm volatile(
 + msrcpsr_c, #0xD3   \n
 + /*
 + * Check current processor: CPU or AVP?
 + * If CPU, go to CPU boot code, else continue on AVP path.
 + */
 + movr0, %0  \n
 + ldrb   r2, [r0, %1]\n
 + /* are we the CPU? */
 + cmpr2, %2  \n
 + movsp, %3  \n
 + /*  yep, we are the CPU */
 + bxeq   %4  \n
 +
 + /* AVP initialization follows this path */
 + movsp, %5  \n
 + /* Init and start CPU */
 + b  startup_cpu \n
 + :
 + : i(NV_PA_PG_UP_BASE),
 + i(PG_UP_TAG_0),
 + r(proc_tag),
 + r(cpu_boot_stack),
 + r(_armboot_start),
 + r(avp_boot_stack)
 + : r0, r2, cc, lr
 + );
 +}

What errors did you encounter when this was in the assembly file?  It'd
be nice to put it there now.  Likely it will never get fixed if it
doesn't implemented correctly off the bat.  If you post the errors
perhaps someone on the list can provide insight.

snip

 +.globl startup_cpu
 +startup_cpu:
 + @ Initialize the AVP, clocks, and memory controller
 + @ SDRAM is guaranteed to be on at this point
 +
 + ldr r0, =cold_boot  @ R0 = reset vector for CPU
 + bl  start_cpu   @ start the CPU
 +
 + @ Transfer control to the AVP code */
 + bl  halt_avp
 +
 + @ Should never get here
 +_loop_forever2:
 + b   _loop_forever2
 +
 +.globl cache_configure
 +cache_configure:
 + stmdb r13!,{r14}
 +@/* invalidate instruction cache */

It looks like there's a combination of comment forms @, @ */, and @ /*
*/.  Is there a reason not to use the normal /* */ universally?

Best,
Peter

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


Re: [U-Boot] [PATCH V2] arm: Tegra2: add support for A9 CPU init

2011-03-25 Thread Peter Tyser
On Fri, 2011-03-25 at 09:16 -0700, Tom Warren wrote:
 Peter,
 
 On Fri, Mar 25, 2011 at 9:02 AM, Peter Tyser pty...@xes-inc.com wrote:
  Hi Tom,
  Things look pretty good.  Minor comments/questions below.
 
  snip
 
  +/*
  + * TBD: Move cold_boot() to assembly file.
  + * Values/offsets of the table vars make this difficult.
  + */
  +
  +void cold_boot(void)
  +{
  + asm volatile(
  + msrcpsr_c, #0xD3   \n
  + /*
  + * Check current processor: CPU or AVP?
  + * If CPU, go to CPU boot code, else continue on AVP path.
  + */
  + movr0, %0  \n
  + ldrb   r2, [r0, %1]\n
  + /* are we the CPU? */
  + cmpr2, %2  \n
  + movsp, %3  \n
  + /*  yep, we are the CPU */
  + bxeq   %4  \n
  +
  + /* AVP initialization follows this path */
  + movsp, %5  \n
  + /* Init and start CPU */
  + b  startup_cpu \n
  + :
  + : i(NV_PA_PG_UP_BASE),
  + i(PG_UP_TAG_0),
  + r(proc_tag),
  + r(cpu_boot_stack),
  + r(_armboot_start),
  + r(avp_boot_stack)
  + : r0, r2, cc, lr
  + );
  +}
 
  What errors did you encounter when this was in the assembly file?  It'd
  be nice to put it there now.  Likely it will never get fixed if it
  doesn't implemented correctly off the bat.  If you post the errors
  perhaps someone on the list can provide insight.
 I didn't capture a log of the errors when I was trying to put the
 cold_boot code into lowlevel_init.S. But I saw fixup errors and
 undefined constant errors, all related to the #defines (NV_PG_UP_BASE,
 avp/cpu_boot_stack, etc.) and how the compiler/assembler references
 indirect and relative constants.
 
 Note that this code works perfectly as-is, so there's no pressing need
 to move it to assembly now, except for a cosmetic/procedural one. I'd
 rather get this accepted into mainline, so I can move on to the
 eMMC/SPI/USB drivers so people can use the code to boot an OS on our
 (many) Tegra2 boards coming to market RSN.
 
 If some ARM / gcc assembly wizard wants to attempt moving this code to
 a .S file, I welcome the help - I may even attack it at a later date,
 when I've got more bandwidth. But it isn't a priority for me right
 now, unless someone on the list adamantly opposes the code as-s. But
 I'd expect anyone with that strong an opinion about to be able to fix
 it, or at least attempt it and see why I decided to defer moving it to
 assembly for now.

I understand your perspective, but why not spend the extra 30 minutes
and do it the right way?  Passing the buck to someone else who cares
about maintaining high quality code isn't the right thing to do in my
opinion.  This patch isn't going to make it into the upcoming release,
so it won't gate the other eMMC/SPI/USB drivers you want to add.  The
bar to get code into open source project generally is higher than it
works - it has to adhere to the project's design principles and
guidelines.  U-Boot already needs cleanup as is without adding more
cruft.  Solving this small issue now results in cleaner code, less
headache down the road, and shouldn't take long.  As usual, I'm not the
maintainer, so its just my $0.02.

Best,
Peter

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


Re: [U-Boot] [PATCH V2] arm: Tegra2: add support for A9 CPU init

2011-03-25 Thread Peter Tyser
On Fri, 2011-03-25 at 11:05 -0700, Tom Warren wrote:
 Peter,
 
 On Fri, Mar 25, 2011 at 10:22 AM, Peter Tyser pty...@xes-inc.com wrote:
  On Fri, 2011-03-25 at 09:16 -0700, Tom Warren wrote:
  Peter,
 
  On Fri, Mar 25, 2011 at 9:02 AM, Peter Tyser pty...@xes-inc.com wrote:
   Hi Tom,
   Things look pretty good.  Minor comments/questions below.
  
   snip
  
   +/*
   + * TBD: Move cold_boot() to assembly file.
   + * Values/offsets of the table vars make this difficult.
   + */
   +
   +void cold_boot(void)
   +{
   + asm volatile(
   + msrcpsr_c, #0xD3   \n
   + /*
   + * Check current processor: CPU or AVP?
   + * If CPU, go to CPU boot code, else continue on AVP path.
   + */
   + movr0, %0  \n
   + ldrb   r2, [r0, %1]\n
   + /* are we the CPU? */
   + cmpr2, %2  \n
   + movsp, %3  \n
   + /*  yep, we are the CPU */
   + bxeq   %4  \n
   +
   + /* AVP initialization follows this path */
   + movsp, %5  \n
   + /* Init and start CPU */
   + b  startup_cpu \n
   + :
   + : i(NV_PA_PG_UP_BASE),
   + i(PG_UP_TAG_0),
   + r(proc_tag),
   + r(cpu_boot_stack),
   + r(_armboot_start),
   + r(avp_boot_stack)
   + : r0, r2, cc, lr
   + );
   +}
  
   What errors did you encounter when this was in the assembly file?  It'd
   be nice to put it there now.  Likely it will never get fixed if it
   doesn't implemented correctly off the bat.  If you post the errors
   perhaps someone on the list can provide insight.
  I didn't capture a log of the errors when I was trying to put the
  cold_boot code into lowlevel_init.S. But I saw fixup errors and
  undefined constant errors, all related to the #defines (NV_PG_UP_BASE,
  avp/cpu_boot_stack, etc.) and how the compiler/assembler references
  indirect and relative constants.
 
  Note that this code works perfectly as-is, so there's no pressing need
  to move it to assembly now, except for a cosmetic/procedural one. I'd
  rather get this accepted into mainline, so I can move on to the
  eMMC/SPI/USB drivers so people can use the code to boot an OS on our
  (many) Tegra2 boards coming to market RSN.
 
  If some ARM / gcc assembly wizard wants to attempt moving this code to
  a .S file, I welcome the help - I may even attack it at a later date,
  when I've got more bandwidth. But it isn't a priority for me right
  now, unless someone on the list adamantly opposes the code as-s. But
  I'd expect anyone with that strong an opinion about to be able to fix
  it, or at least attempt it and see why I decided to defer moving it to
  assembly for now.
 
  I understand your perspective, but why not spend the extra 30 minutes
  and do it the right way?  Passing the buck to someone else who cares
  about maintaining high quality code isn't the right thing to do in my
  opinion.  This patch isn't going to make it into the upcoming release,
  so it won't gate the other eMMC/SPI/USB drivers you want to add.  The
  bar to get code into open source project generally is higher than it
  works - it has to adhere to the project's design principles and
  guidelines.  U-Boot already needs cleanup as is without adding more
  cruft.  Solving this small issue now results in cleaner code, less
  headache down the road, and shouldn't take long.  As usual, I'm not the
  maintainer, so its just my $0.02.
 FWIW, I spent _far_ more than 30 minutes on this .. close to a full
 day of frustration/banging my head against the wall.  I have other
 priorities besides upstreaming Tegra2 U-Boot support, and I can't
 justify spending days on this. As I originally stated, I'm no expert
 in the intricacies of ARM asm programming - my expertise is in x86
 CPUs, and hit a wall with this port to assembly.

Understood.  When situations like that happen its good to ask others for
input rather than using a non-optimal solution.  Then either
- Someone suggests how to fix the code, and the code improves.
- No one helps, which implies your method isn't easy to improve upon,
you gave it a good effort, and so it gets included.

As is, the current patch looks like a non-optimal solution that someone
familiar with ARM asm could have quickly helped with.

 I'm not passing the buck, and while I agree that this code is not the
 cleanest I've seen, I don't think I'm pushing low-quality code here.

Based on the fact that you tried to fix it for a day, it sounds like you
are aware its less than ideal implementation, and the TBD comment for
the function further implies that this function doesn't belong here, and
someone should fix it down the road.  My guess is that this will likely
never happen unless it gets fixed now.

 It's 8 lines of embedded assembly, plus a table

Re: [U-Boot] [PATCH] arm: Tegra2: add support for A9 CPU init

2011-03-17 Thread Peter Tyser
Hi Tom,

  
   + /* Is PLL-X already running? */
   + reg = readl(clkrst-crc_pllx_base);
   + if (reg  PLL_ENABLE)
   + return;
   +
   + /* Do PLLX init if it isn't running, but BootROM sets it, so TBD 
   */
   +}
  
   The above function looks incorrect.
  What looks incorrect? It checks to see if the PLL is already
  running/enabled, and returns if it is.
  Tegra2 mask ROM (BootROM) currently always sets PLLX, so this will
  always return, but the comment is there for future chips that may not
  set up PLLX.
 
  It looks like a function that does a read of a value it doesn't care
  about, does an unnecessary comparison, and then returns either way,
  without doing anything:)  If/when you want to do future stuff with the
  PLL-X, code should be added then - as is it doesn't do anything and is
  confusing.  The general policy of U-Boot is not to add dead code.
 OK, so not really incorrect, just unnecessary. Agreed - again a
 vestigial leftover from our in-house code. I'll remove it.

Unnecessary/misleading/dead code is inherently not correct:)

snip

   +#include asm/types.h
   +
   +#ifndef  FALSE
   +#define FALSE0
   +#endif
   +#ifndef TRUE
   +#define TRUE 1
   +#endif
  
   These shouldn't be added here.  They should be somewhere common, or
   shouldn't be used (my preference).
  I would think they'd be in the ARM tree somewhere, but I couldn't find
  them so I added 'em here.
  My preference is to use TRUE/FALSE - it carries more context than 1/0 to 
  me.
 
  If you prefer to use TRUE/FALSE, they should be moved into a common
  location so everywhere uses the same, once-defined definition.  Their
  definitions are already littered over a few files, which would ideally
  be cleaned up.  Perhaps moving all definitions into include/common.h, or
  somewhere similar would work.  Others may have opinions about TRUE/FALSE
  vs 1/0 - it seems like TRUE/FALSE aren't generally used.
 I don't want to pollute all builds by adding to include/common.h.
 I'll try to find a more central header in my own tree.

My point is that there are already 32 definitions of TRUE/FALSE - adding
a 33rd doesn't seem like a good thing to do.  I view a 33rd identical
definition as polluting the code more than 1 common definition that most
people won't generally use.

Its not my decision, but I assume the powers that be would recommend one
of:
- Not using TRUE/FALSE since using non-zero values and 0 are widely
accepted as TRUE/FALSE.  I think using TRUE/FALSE makes the code harder
to understand and more open to bugs.  Eg for other code that interacts
with your code, or someone reviewing your code, they either have to
either assume you defined TRUE as 1, FALSE as 0, or import your
definitions.  Anyway, I view their use as another layer of unnecessary
abstraction with very little benefit.

- Consolidating the definition of TRUE/FALSE.

Wolfgang, do you have a preference about how TRUE/FALSE are generally
used/defined?

Best,
Peter



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


Re: [U-Boot] [PATCH V4 4/4] various cleanups/code style fixes

2011-03-14 Thread Peter Tyser
Hi David,
It'd be good to include smdk2410 in this patch titles, eg smdk2410:
Various cleanups/code style fixes. or arm: smdk2410: Various...

Without the prefix, its unclear what code these patches affect, and
somewhat implies you are changing common code.

snip

 -#define  CONFIG_RTC_S3C24X0  1
 +#define  CONFIG_RTC_S3C24X0

Looks like there are a number of tabs in odd locations, it'd be nice to
clean those up while you're modifying this file.

Best,
Peter

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


Re: [U-Boot] [PATCH] arm: Tegra2: add support for A9 CPU init

2011-03-14 Thread Peter Tyser
Hi Tom,
I'm not too familiar with the architecture, so many comments are
aesthetic.  It would be a good idea to run the patch through
checkpatch.pl too.

 diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S
 index 684f2d2..50a1725 100644
 --- a/arch/arm/cpu/armv7/start.S
 +++ b/arch/arm/cpu/armv7/start.S
 @@ -70,6 +70,12 @@ _end_vect:
  _TEXT_BASE:
   .word   CONFIG_SYS_TEXT_BASE
  
 +#ifdef CONFIG_TEGRA2
 +.globl _armboot_start
 +_armboot_start:
 +.word _start
 +#endif
 +

It'd be good to add a comment about what's going on above, and why its
tegra2-specific.  Eg why is it needed?

snip

 +static void init_pll_x(void)
 +{
 + struct clk_rst_ctlr *clkrst = (struct clk_rst_ctlr *)NV_PA_CLK_RST_BASE;
 + u32 reg;

The spaces in front of reg can be removed.  Ditto for all
functions/variables.

 + /* Is PLL-X already running? */
 + reg = readl(clkrst-crc_pllx_base);
 + if (reg  PLL_ENABLE)
 + return;
 +
 + /* Do PLLX init if it isn't running, but BootROM sets it, so TBD */
 +}

The above function looks incorrect.

 +static void set_cpu_reset_vector(u32 vector)
 +{
 + writel(vector, EXCEP_VECTOR_CPU_RESET_VECTOR);
 +}

Is it worth breaking this out into its own function?  The define names
make it pretty clear what is being done, and its only called from 1
location.

 +static void enable_cpu_clock(int enable)
 +{
 + struct clk_rst_ctlr *clkrst = (struct clk_rst_ctlr *)NV_PA_CLK_RST_BASE;
 + u32   reg, clk;
 +
 + /*
 +  * NOTE:
 +  * Regardless of whether the request is to enable or disable the CPU
 +  * clock, every processor in the CPU complex except the master (CPU 0)
 +  * will have it's clock stopped because the AVP only talks to the
 +  * master. The AVP does not know (nor does it need to know) that there
 +  * are multiple processors in the CPU complex.
 +  */
 +
 + /* Need to initialize PLLX? */
 + if (enable) {
 + /* Initialize PLL */
 + init_pll_x();
 +
 + /* Wait until stable */
 + udelay(NVBOOT_CLOCKS_PLL_STABILIZATION_DELAY);
 +
 + reg = CCLK_BURST_POLICY;
 + writel(reg, clkrst-crc_cclk_brst_pol);

It'd look cleaner to not set reg for each access, just use the define
directly.  I'd personally use less temporary variables in general and
only use them when it made the code cleaner and easier to understand.

 + reg = SUPER_CCLK_DIVIDER;
 + writel(reg, clkrst-crc_super_cclk_div);
 + }
 +
 + /* Fetch the register containing the main CPU complex clock enable */
 + reg = readl(clkrst-crc_clk_out_enb_l);

Is this read necessary?  You overwrite reg unconditionally below.

 + /*
 +  * Read the register containing the individual CPU clock enables and
 +  * always stop the clock to CPU 1.
 +  */
 + clk = readl(clkrst-crc_clk_cpu_cmplx);
 + clk |= CPU1_CLK_STP;
 +
 + if (enable) {
 + /* Enable the CPU clock */
 + reg = readl(clkrst-crc_clk_out_enb_l);
 + reg |= CLK_ENB_CPU;
 + clk = readl(clkrst-crc_clk_cpu_cmplx);
 + clk = ~CPU0_CLK_STP;
 + } else {
 + /* Disable the CPU clock */
 + reg = readl(clkrst-crc_clk_out_enb_l);
 + reg |= CLK_ENB_CPU;
 + clk = readl(clkrst-crc_clk_cpu_cmplx);
 + clk |= CPU0_CLK_STP;
 + }

For if/elses that share common code, the common code should be broken
out.  eg above only the one-line |=/= operations should be conditional.

 + writel(clk, clkrst-crc_clk_cpu_cmplx);
 + writel(reg, clkrst-crc_clk_out_enb_l);
 +}
 +
 +static int is_cpu_powered(void)
 +{
 + struct pmc_ctlr *pmc = (struct pmc_ctlr *)NV_PA_PMC_BASE;
 + u32   reg;
 +
 + reg = readl(pmc-pmc_pwrgate_status);
 +
 + if (reg  CPU_PWRED)
 + return TRUE;

I'd remove the reg variable.  eg something like:
return readl(pmc-pmc_pwrgate_status  CPU_PWRED ? 1 : 0);

 + return FALSE;
 +}
 +
 +static void remove_cpu_io_clamps(void)
 +{
 + struct pmc_ctlr *pmc = (struct pmc_ctlr *)NV_PA_PMC_BASE;
 + u32   reg;
 +
 + /* Remove the clamps on the CPU I/O signals */
 + reg = readl(pmc-pmc_remove_clamping);
 + reg |= CPU_CLMP;
 + writel(reg, pmc-pmc_remove_clamping);
 +
 + /* Give I/O signals time to stabilize */
 + udelay(1000);

For magic delays it'd be good to reference why you're waiting a specific
amount of time.  Does a manual state 1 ms?  Did testing show 1ms was
required?

 +}
 +
 +static void powerup_cpu(void)
 +{
 + struct pmc_ctlr *pmc = (struct pmc_ctlr *)NV_PA_PMC_BASE;
 + u32   reg;
 +
 + if (!is_cpu_powered()) {
 + /* Toggle the CPU power state (OFF - ON) */
 + reg = readl(pmc-pmc_pwrgate_toggle);
 + reg = (PARTID_CP);
 + reg |= START_CP;

Why ()'s on one operation, but not the other?

 + writel(reg, 

Re: [U-Boot] [PATCH] arm: Tegra2: add support for A9 CPU init

2011-03-14 Thread Peter Tyser
Hi Tom,

snip

  +static void init_pll_x(void)
  +{
  + struct clk_rst_ctlr *clkrst = (struct clk_rst_ctlr 
  *)NV_PA_CLK_RST_BASE;
  + u32 reg;
 
  The spaces in front of reg can be removed.  Ditto for all
  functions/variables.
 Not sure what you mean, here, Peter. There are no spaces here in my
 ap20.c file, just tabs.
 Please clarify.

My email client converted to spaces I was referring to the u32reg;
above.  I think it should be u32spacereg instead of u32tabreg.
Other places in this patch spaces are used (eg in enable_cpu_clock, and
in the struct declaration above), so at a minimum the tabs/spaces should
be changed to be consistent, and typespacename seems cleanest to
me.

 
  + /* Is PLL-X already running? */
  + reg = readl(clkrst-crc_pllx_base);
  + if (reg  PLL_ENABLE)
  + return;
  +
  + /* Do PLLX init if it isn't running, but BootROM sets it, so TBD */
  +}
 
  The above function looks incorrect.
 What looks incorrect? It checks to see if the PLL is already
 running/enabled, and returns if it is.
 Tegra2 mask ROM (BootROM) currently always sets PLLX, so this will
 always return, but the comment is there for future chips that may not
 set up PLLX.

It looks like a function that does a read of a value it doesn't care
about, does an unnecessary comparison, and then returns either way,
without doing anything:)  If/when you want to do future stuff with the
PLL-X, code should be added then - as is it doesn't do anything and is
confusing.  The general policy of U-Boot is not to add dead code.

snip

  +static void enable_cpu_power_rail(void)
  +{
  + struct pmc_ctlr *pmc = (struct pmc_ctlr *)NV_PA_PMC_BASE;
  + u32   reg;
  +
  + reg = readl(pmc-pmc_cntrl);
  + reg |= CPUPWRREQ_OE;
  + writel(reg, pmc-pmc_cntrl);
  +
  + /* Delay to allow for stable power (CPU_PWR_REQ - VDD_CPU from PMU) 
  */
  + udelay(3750);
 
  Ditto for description.
 Ditto on why the delay? In this case, I did find a reference to the
 time needed between the request from the chipset to the PMU, hence the
 comment.

It'd be nice mention that reference in the comment.  For those designing
boards around your chips, during debug they will look through the
initialization code and wonder I wonder if we need to delay longer, or
I'm not using the same power supply sequencer, I wonder if this might
be a problem.  If you more explicitly state where the delay came from,
or what the delay specifically accomplishes, it saves others from having
to guess and investigate.

  +}
  +
  +static void reset_A9_cpu(int reset)
  +{
  + struct clk_rst_ctlr *clkrst = (struct clk_rst_ctlr 
  *)NV_PA_CLK_RST_BASE;
  + u32   reg, cpu;
  +
  + /*
  + * NOTE:  Regardless of whether the request is to hold the CPU in 
  reset
  + *or take it out of reset, every processor in the CPU complex
  + *except the master (CPU 0) will be held in reset because the
  + *AVP only talks to the master. The AVP does not know that 
  there
  + *are multiple processors in the CPU complex.
  + */
  +
  + /* Hold CPU 1 in reset */
  + cpu = SET_DBGRESET1 | SET_DERESET1 | SET_CPURESET1;
  + writel(cpu, clkrst-crc_cpu_cmplx_set);
 
  The cpu variable can be removed.
 It could be, sure. But it makes the line longer, 80 chars, and hence
 it would have to be broken into two lines.
 Actually, now that I look at the code again, it appears that 'cpu'
 later should be OR'd with the SET_/CLR_DBGRESET0, etc. bits, depending
 on the state of 'reset'.
 I'll give it a rewrite for the next patch.

Its a matter of preference, but is acceptable either way.  I think:
+   writel(SET_DBGRESET1 | SET_DERESET1 | SET_CPURESET1,
+   clkrst-crc_cpu_cmplx_set);

makes it clearer what is going on.  Setting 'cpu', then writing would
imply to me that 'cpu' has some additional purpose, or is being used
later.  If you restructure the code, this comment will likely be moot.

snip

  + if (enable) {
  + /*
  +  * Put CoreSight on PLLP_OUT0 (216 MHz) and divide it down by
  +  *  1.5, giving an effective frequency of 144MHz.
  +  * Set PLLP_OUT0 [bits31:30 = 00], and use a 7.1 divisor
  +  *  (bits 7:0), so 0001b == 1.5 (n+1 + .5)
  +  */
  + src = CLK_DIVIDER(NVBL_PLLP_KHZ, 144000);
  + writel(src, clkrst-crc_clk_src_csite);
  +
  + /* Unlock the CPU CoreSight interfaces */
  + rst = 0xC5ACCE55;
 
  What's this number?  Is there a define that could be used instead?
 Sure, I can add a #define. Some magic ARM access sequence to unlock
 the CoreSight I/F, as the comment says

Its not clear what the number is from the comment.  Is it a bitmask
where each bit has some significance?  Or is it just a magic number of
some sort?  If its a magic number with no other significance, a more
verbose comment is fine stating so without 

[U-Boot] [PATCH] mpc8[5/6]xx: Ensure POST word does not get reset

2011-03-10 Thread Peter Tyser
From: John Schmoller jschmol...@xes-inc.com

The POST word is stored in a spare register in the PIC on MPC8[5/6]xx
processors.  When interrupt_init() is called, this register gets reset
which resulted in all POST_RAM POSTs not being ran due to the corrupted
POST word.  To resolve this, store off POST word before the PIC is
reset, and restore it after the PIC has been initialized.

Signed-off-by: John Schmoller jschmol...@xes-inc.com
Signed-off-by: Peter Tyser pty...@xes-inc.com
---
This is a bugfix, so it'd be nice if it made it into the upcoming release
if possible.

 arch/powerpc/cpu/mpc85xx/interrupts.c |   16 
 arch/powerpc/cpu/mpc86xx/interrupts.c |   16 
 2 files changed, 32 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/cpu/mpc85xx/interrupts.c 
b/arch/powerpc/cpu/mpc85xx/interrupts.c
index a62b031..7ab7113 100644
--- a/arch/powerpc/cpu/mpc85xx/interrupts.c
+++ b/arch/powerpc/cpu/mpc85xx/interrupts.c
@@ -32,11 +32,23 @@
 #include command.h
 #include asm/processor.h
 #include asm/io.h
+#ifdef CONFIG_POST
+#include post.h
+#endif
 
 int interrupt_init_cpu(unsigned int *decrementer_count)
 {
ccsr_pic_t __iomem *pic = (void *)CONFIG_SYS_MPC8xxx_PIC_ADDR;
 
+#ifdef CONFIG_POST
+   /*
+* The POST word is stored in the PIC's TFRR register which gets
+* cleared when the PIC is reset.  Save it off so we can restore it
+* later.
+*/
+   ulong post_word = post_word_load();
+#endif
+
out_be32(pic-gcr, MPC85xx_PICGCR_RST);
while (in_be32(pic-gcr)  MPC85xx_PICGCR_RST)
;
@@ -78,6 +90,10 @@ int interrupt_init_cpu(unsigned int *decrementer_count)
pic-ctpr=0;/* 40080 clear current task priority register */
 #endif
 
+#ifdef CONFIG_POST
+   post_word_store(post_word);
+#endif
+
return (0);
 }
 
diff --git a/arch/powerpc/cpu/mpc86xx/interrupts.c 
b/arch/powerpc/cpu/mpc86xx/interrupts.c
index d8ad6d3..14821f4 100644
--- a/arch/powerpc/cpu/mpc86xx/interrupts.c
+++ b/arch/powerpc/cpu/mpc86xx/interrupts.c
@@ -35,12 +35,24 @@
 #include mpc86xx.h
 #include command.h
 #include asm/processor.h
+#ifdef CONFIG_POST
+#include post.h
+#endif
 
 int interrupt_init_cpu(unsigned long *decrementer_count)
 {
volatile immap_t *immr = (immap_t *)CONFIG_SYS_IMMR;
volatile ccsr_pic_t *pic = immr-im_pic;
 
+#ifdef CONFIG_POST
+   /*
+* The POST word is stored in the PIC's TFRR register which gets
+* cleared when the PIC is reset.  Save it off so we can restore it
+* later.
+*/
+   ulong post_word = post_word_load();
+#endif
+
pic-gcr = MPC86xx_PICGCR_RST;
while (pic-gcr  MPC86xx_PICGCR_RST)
;
@@ -74,6 +86,10 @@ int interrupt_init_cpu(unsigned long *decrementer_count)
pic-ctpr = 0;  /* 40080 clear current task priority register */
 #endif
 
+#ifdef CONFIG_POST
+   post_word_store(post_word);
+#endif
+
return 0;
 }
 
-- 
1.7.0.4

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


[U-Boot] [PATCH] 8xxx: ddr3: Adjust timings for tRRD, tWTR, and tRTP

2011-03-10 Thread Peter Tyser
From: John Schmoller jschmol...@xes-inc.com

The JEDEC DDR3 specification states that the above parameters should
be set to a minimum of 4 clocks.  The SPD defines the values in
nanoseconds, and depending on the clock frequency the value in the
SPD can be less than 4 clocks.

Previously, we were only using the values from the SPD, regardless if
they were less than 4 clocks.  Now, set the timing values to the greater
of 4 clocks or the SPD value.

Invalid tRRD, tWTR, and tRTP values were observed on a variety of
Freescale CPUs, although no negative side effects were seen from their
use.

Signed-off-by: John Schmoller jschmol...@xes-inc.com
Signed-off-by: Peter Tyser pty...@xes-inc.com
---
 arch/powerpc/cpu/mpc8xxx/ddr/ddr3_dimm_params.c |7 ---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/cpu/mpc8xxx/ddr/ddr3_dimm_params.c 
b/arch/powerpc/cpu/mpc8xxx/ddr/ddr3_dimm_params.c
index 29cea53..8669898 100644
--- a/arch/powerpc/cpu/mpc8xxx/ddr/ddr3_dimm_params.c
+++ b/arch/powerpc/cpu/mpc8xxx/ddr/ddr3_dimm_params.c
@@ -230,8 +230,9 @@ ddr_compute_dimm_parameters(const ddr3_spd_eeprom_t *spd,
 * eg: tRRD_min =
 * DDR3-800(1KB page)   80 MTB (10ns)
 * DDR3-1333(1KB page)  48 MTB (6ns)
+* tRRD is at least 4 mclk independent of operating freq.
 */
-   pdimm-tRRD_ps = spd-tRRD_min * mtb_ps;
+   pdimm-tRRD_ps = max(spd-tRRD_min * mtb_ps, mclk_to_picos(4));
 
/*
 * min row precharge delay time
@@ -276,14 +277,14 @@ ddr_compute_dimm_parameters(const ddr3_spd_eeprom_t *spd,
 * eg: tWTR_min = 40 MTB (7.5ns) - all speed bins.
 * tWRT is at least 4 mclk independent of operating freq.
 */
-   pdimm-tWTR_ps = spd-tWTR_min * mtb_ps;
+   pdimm-tWTR_ps = max(spd-tWTR_min * mtb_ps, mclk_to_picos(4));
 
/*
 * min internal read to precharge command delay time
 * eg: tRTP_min = 40 MTB (7.5ns) - all speed bins.
 * tRTP is at least 4 mclk independent of operating freq.
 */
-   pdimm-tRTP_ps = spd-tRTP_min * mtb_ps;
+   pdimm-tRTP_ps = max(spd-tRTP_min * mtb_ps, mclk_to_picos(4));
 
/*
 * Average periodic refresh interval
-- 
1.7.0.4

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


Re: [U-Boot] organizing doc/ dir?

2011-02-22 Thread Peter Tyser
On Wed, 2011-02-23 at 00:02 -0600, Kumar Gala wrote:
 Wolfgang,
 
 Any issue that added a little hierarchy to the doc/ dir?
 
 maybe something mimic board/
 
 for example:
 
 doc/README.mpc8313erdb  -  doc/freescale/README.mpc8313erdb
 doc/README.mpc8315erdb  -  doc/freescale/README.mpc8315erdb
 doc/README.mpc8323erdb  -  doc/freescale/README.mpc8323erdb
 doc/README.mpc832xemds  -  doc/freescale/README.mpc832xemds
 doc/README.mpc8349itx   -  doc/freescale/README.mpc8349itx
 doc/README.mpc8360emds  -  doc/freescale/README.mpc8360emds
 doc/README.mpc837xemds  -  doc/freescale/README.mpc837xemds
 doc/README.mpc837xerdb  -  doc/freescale/README.mpc837xerdb
 doc/README.mpc83xxads   -  doc/freescale/README.mpc83xxads
 doc/README.mpc83xx.ddrecc- doc/freescale/README.mpc83xx.ddrecc
 doc/README.mpc8536ds-  doc/freescale/README.mpc8536ds
 doc/README.mpc8544ds-  doc/freescale/README.mpc8544ds
 doc/README.mpc8569mds   -  doc/freescale/README.mpc8569mds
 doc/README.mpc8572ds-  doc/freescale/README.mpc8572ds
 doc/README.mpc85xxads   -  doc/freescale/README.mpc85xxads
 doc/README.mpc85xxcds   -  doc/freescale/README.mpc85xxcds
 doc/README.mpc8610hpcd  -  doc/freescale/README.mpc8610hpcd
 doc/README.mpc8641hpcn  -  doc/freescale/README.mpc8641hpcn

What about adding a doc/board directory?  eg:
doc/README.mpc8313erdb  -  doc/board/freescale/README.mpc8313erdb
doc/README.mpc8315erdb  -  doc/board/freescale/README.mpc8315erdb
doc/README.ocotea   -  doc/board/amcc/README.ocotea
doc/README.ebony-  doc/board/amcc/README.ebony
doc/README.zeus -  doc/board/README.zeus
doc/README.xpedite1k-  doc/board/README.xpedite1k

Since the majority of documentation is board specific at this point,
it'd be nice if it was all moved into a subdirectory to clean up the
top-level doc/ dir.

Also, without the board subdirectory it'd be unclear where general
Freescale documentation that was relevant to multiple board vendors
would go.  Eg README.fsl-ddr - doc/freescale?  or doc/?.

A reorganization sounds nice either way!

Peter

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


Re: [U-Boot] [PATCH] unzip: return uncompressed size in `filesize', and print it.

2011-02-11 Thread Peter Tyser
Hi Wolfgang,

 - return !!gunzip((void *) dst, dst_len, (void *) src, src_len);
 + rc = gunzip((void *) dst, dst_len, (void *) src, src_len);
 +
 + printf(Uncompressed size: %ld = 0x%lX\n, src_len, src_len);
 + sprintf(buf, %lX, src_len);
 + setenv(filesize, buf);
 +
 + return !!rc;

What about:
if (rc)
return rc;

printf(Uncompressed size: %ld = 0x%lX\n, src_len, src_len);
sprintf(buf, %lX, src_len);
setenv(filesize, buf);

return 0;


This will prevent printing and setting of bogus values when an invalid
or overly large image is unzipped.

Best,
Peter

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


Re: [U-Boot] P1022 eTSEC

2011-02-09 Thread Peter Tyser
Hi Renaud,

On Wed, 2011-02-09 at 20:21 +, Renaud Barbier wrote:
 We have a system with a P1022 connected to a 5461S in SGMII mode.
 
 In order to make it work in SGMII mode, I set TBI ANA to 0x4001 as per
 AN3869. Note that those bit are described as reserved in the P1022 doc 
 that I have.
 I was then able to transfer data at 100/1000 (10 not tested).
 
 As per AN3869 a value of 0x1a0 is  for 1000BASE-X.
 
 
 Looking at the tsec driver (drivers/net/tsec.c), one can see:
 
 #define TBIANA_SETTINGS ( \
  TBIANA_ASYMMETRIC_PAUSE \
  | TBIANA_SYMMETRIC_PAUSE \
  | TBIANA_FULL_DUPLEX \
  )
 == 0x1a0
 
 if (regs-ecntrl  ECNTRL_SGMII_MODE)
  tsec_configure_serdes(priv);
 
 That would mean the TBI ANA is not set correctly when SGMII
 is reported.
 
 Please can you verify this.

Gotta love those undocumented register bits:)  This same issue has been
discussed a number of times, but no one ever noticed the 0x4001 in
AN3869, or assumed it was an error.  The bug also didn't seem to affect
some PHYs, eg Vitesse models:

http://old.nabble.com/-U-Boot---PATCH--tsec:-Force-TBI-PHY-to-1000Mbps-full-duplex-in-SGMII-mode-td26188785.html
http://comments.gmane.org/gmane.comp.boot-loaders.u-boot/89059
http://comments.gmane.org/gmane.comp.boot-loaders.u-boot/80256

My company worked around the issue by not enabling auto-negotiation in
the TBI control register via a custom CONFIG_TSEC_TBICR_SETTINGS value.
I just tried removing our workaround and setting TBIANA_SETTINGS to
0x4001, and it looks like it works on a P2020-based board with a
BCM5482S PHY.

It'd be ideal if someone from from Freescale chimed in so we knew what
bits we were hitting in the TBIANA register.  The change has my ack
though.  Let me know if you don't plan on submitting a change and I'll
update our boards, as well as the value of TBIANA_SETTINGS.

Best,
Peter

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


Re: [U-Boot] [PATCH V5 2/4] serial: Add Tegra2 serial port support

2011-01-26 Thread Peter Tyser
On Wed, 2011-01-26 at 10:13 +0200, Mike Rapoport wrote:
 On 01/26/11 00:24, Peter Tyser wrote:
  As I've already pointed out (1) this covers only one possibility of 
  UART pin
  muxing options. I agree that it makes sense when this is a part of the
  board-specific code. However, forcing specific pins configuration in 
  the generic
  driver seems problematic to me, even if most Tegra2 boards use the same
  configuration.
  As a side note, I'd prefer to have all the pin multiplexing defined in 
  one place
  in board file rather than scattered among different drivers.
 
  Alright. I'd like to get this wrapped up and accepted before the merge 
  window
  closes so I can get on with the important bits (drivers, etc.). What
  would you like
  here? A generic function like 'pinmux_set_config(reg, val, bit)' that
  lives in the board
  files and gets called from the driver code with specifics of that
  board's/drivers pinmux
  config? Or do you see the pinmux code living entirely in the board
  files, and being done
  as part of board init? That's where it was before, BTW.
 
  I think that the pinmux code should live entirely in the board file and
  performed as part of board init. The drivers than can assume that the 
  pins are
  configured properly.
  OK. I'll move the clock/pinmux init code into armv7/tegra/board.c
  (since it's SoC-centric),
  and call it during board_init().
  Actually, I think it makes more sense to move pinmux_init_uart and
  clock_init_uart to board/nvidia/common/board.c.
  These routines are more board-specific than SoC-specific - they depend
  on how the UART is routed on a board.
  Let me do that  gen up a new patchset.
  
  You previously mentioned that To date, all of our Tegra boards use
  these pinmux options for both UARTs.  If a board vendor chooses to use
  different pinmuxes, then they can override these funcs in their board
  files, or use their own code triggered by their own defines. But
  according to our HW guys, the vast majority will use these pins
  
  If the vast majority of boards really will use the same pinmuxing, it
  would be nice to provide that common muxing as a default which can be
  overridden.  Perhaps moving pinmux_init_uart() and uart_clock_init()
  into armv7/tegra/*, and making them weak functions would make everyone
  happy.
 
 My point was that pin muxing belongs to the board code rather than to the
 driver. Driver should just assume that pins are configured elsewhere and it 
 does
 not need to deal with pin muxing at all.

I understand your point, but think if 9/10 boards use the same pin
muxing its good to provide a default so we don't have 9 chunks of
duplicate code floating around in board/*.  What's the harm in providing
a default?  It can be overridden if needed.

 Moreover, I'd prefer to see pinmux_board_init or something similar that
 configures all the pins at once rather than collection of pinmux_init_uart,
 pinmux_init_sdmmc, pinmux_init_gmi etc that will grow as more drivers are 
 added.

I can see that point but its a different discussion.  I don't know
enough about the Tegra2 to comment on this, but it seems like a good
idea based on previous experiences with boards with similar pinmuxing
(eg mpc8260).  In my last email I mentioned a table-driven approach
(similar to the mpc8260 implementation?) which sounds somewhat like
you're proposing.

Best,
Peter

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


Re: [U-Boot] [PATCH V5 2/4] serial: Add Tegra2 serial port support

2011-01-25 Thread Peter Tyser
  [ snip ]
 
  +/*
  + * Routine: pin_mux_uart
  + * Description: setup the pin muxes/tristate values for a UART
  + */
  +static void pin_mux_uart(void)
  +{
  + pinmux_tri_ctlr *const pmt = (pinmux_tri_ctlr 
  *)NV_PA_APB_MISC_BASE;
  + u32 reg;
  +
  +#if CONFIG_TEGRA2_ENABLE_UARTA
  + reg = readl(pmt-pmt_ctl_c);
  + reg = 0xFFF0;  /* IRRX_/IRTX_SEL [19:16] = 00 
  UARTA */
  + writel(reg, pmt-pmt_ctl_c);
  +
  + reg = readl(pmt-pmt_tri_a);
  + reg = ~Z_IRRX; /* Z_IRRX = normal (0) */
  + reg = ~Z_IRTX; /* Z_IRTX = normal (0) */
  + writel(reg, pmt-pmt_tri_a);
  +#endif   /* CONFIG_TEGRA2_ENABLE_UARTA */
  +#if CONFIG_TEGRA2_ENABLE_UARTD
  + reg = readl(pmt-pmt_ctl_b);
  + reg = 0xFFF3;  /* GMC_SEL [3:2] = 00, UARTD */
  + writel(reg, pmt-pmt_ctl_b);
  +
  + reg = readl(pmt-pmt_tri_a);
  + reg = ~Z_GMC;  /* Z_GMC = normal (0) */
  + writel(reg, pmt-pmt_tri_a);
  +#endif   /* CONFIG_TEGRA2_ENABLE_UARTD */
 
  As I've already pointed out (1) this covers only one possibility of UART 
  pin
  muxing options. I agree that it makes sense when this is a part of the
  board-specific code. However, forcing specific pins configuration in the 
  generic
  driver seems problematic to me, even if most Tegra2 boards use the same
  configuration.
  As a side note, I'd prefer to have all the pin multiplexing defined in 
  one place
  in board file rather than scattered among different drivers.
 
  Alright. I'd like to get this wrapped up and accepted before the merge 
  window
  closes so I can get on with the important bits (drivers, etc.). What
  would you like
  here? A generic function like 'pinmux_set_config(reg, val, bit)' that
  lives in the board
  files and gets called from the driver code with specifics of that
  board's/drivers pinmux
  config? Or do you see the pinmux code living entirely in the board
  files, and being done
  as part of board init? That's where it was before, BTW.
 
  I think that the pinmux code should live entirely in the board file and
  performed as part of board init. The drivers than can assume that the pins 
  are
  configured properly.
  OK. I'll move the clock/pinmux init code into armv7/tegra/board.c
  (since it's SoC-centric),
  and call it during board_init().
 Actually, I think it makes more sense to move pinmux_init_uart and
 clock_init_uart to board/nvidia/common/board.c.
 These routines are more board-specific than SoC-specific - they depend
 on how the UART is routed on a board.
 Let me do that  gen up a new patchset.

You previously mentioned that To date, all of our Tegra boards use
these pinmux options for both UARTs.  If a board vendor chooses to use
different pinmuxes, then they can override these funcs in their board
files, or use their own code triggered by their own defines. But
according to our HW guys, the vast majority will use these pins

If the vast majority of boards really will use the same pinmuxing, it
would be nice to provide that common muxing as a default which can be
overridden.  Perhaps moving pinmux_init_uart() and uart_clock_init()
into armv7/tegra/*, and making them weak functions would make everyone
happy.

Or could you make default defines that board's could override as needed?
eg in pin_mux_uart():
#ifndef CONFIG_TEGRA2_PMT_CTLC_MASK
#define CONFIG_TEGRA2_PMC_CTLC_MASK 0xfff0
#endif
#ifndef CONFIG_TEGRA2_PMT_TRI_A_MASK
#define CONFIG_TEGRA2_PMC_TRI_A_MASK ~(Z_IRRX | Z_IRTX)
#endif
pin_mux_uart() {
reg = readl(pmt-pmt_ctl_c);
reg = CONFIG_TEGRA2_PMT_CTLC_MASK;
writel(reg, pmt-pmt_ctl_c);

reg = readl(pmt-pmt_tri_a);
reg = CONFIG_TEGRA2_PMC_TRI_A_MASK;
writel(reg, pmt-pmt_tri_a);
}

Or make the functions table driven so each board would define a
pinmux/clock configuration table?

I think its worthwhile to try and reduce duplicated code whenever
possible.

Best,
Peter

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


Re: [U-Boot] [PATCH V5 2/4] serial: Add Tegra2 serial port support

2011-01-24 Thread Peter Tyser
snip

  It looks like arch/arm/cpu/armv7/tegra2/board.h and
  arch/arm/cpu/armv7/tegra2/uart.c are added in the first patch, then
  moved in this patch.  It'd be ideal to just add them once in the proper
  location.
 
  On a side note, if you pass git format-patch the -M and -C options it
  will make pretty diffs that only show what lines changed during a move.
  In the case that you do move files in the future its nice to use those
  options to ease review.
 
 I'll use those options in the future (thanks!) to keep things cleaner.
 Hopefully we can just go with this set of patches so I can get to the
 other, more interesting code (drivers, A9 CPU init, etc.).

Its up to the ARM maintainer and Wolfgang to decide if adding code in
one patch just to move it around in the 2nd is acceptable.  I'm guess it
won't be acceptable since its considered bad form, and its unclear
what patch in this series contains what functionality.  Eg this isn't
really meant to Add Tegra2 serial port support, it moves existing
serial port code around?  And more?  Its not really just adding serial
port support as the patch title states in any case.

  snip
 
  +void uart_init(void)
  +{
  + /* Init each UART - there may be more than 1 on a board/build */
  +#if (CONFIG_TEGRA2_ENABLE_UARTA)
  + init_uart();
  +#endif
  +#if (CONFIG_TEGRA2_ENABLE_UARTD)
  + init_uart();
  +#endif
  +}
 
  How about:
  #if defined(CONFIG_TEGRA2_ENABLE_UARTA) || 
  defined(CONFIG_TEGRA2_ENABLE_UARTD)
 init_uart();
  #endif
 That won't work for a board like Harmony where the developer wants
 both UARTs active, since uart_init is only called once.

Why should init_uart() be called two times?  It looks to initialize both
ports, meaning it should only be called once, right?

Also, just noticed:
+static void init_uart(void)
+{
+#if CONFIG_TEGRA2_ENABLE_UARTA
+   uart_ctlr *const uart = (uart_ctlr *)NV_PA_APB_UARTA_BASE;
+
+   uart_clock_init();
+
+   /* Enable UARTA - uses config 0 */
+   pin_mux_uart();
+
+   setup_uart(uart);
+#endif /* CONFIG_TEGRA2_ENABLE_UARTD */
+#if CONFIG_TEGRA2_ENABLE_UARTD
+   uart_ctlr *const uart = (uart_ctlr *)NV_PA_APB_UARTD_BASE;
+

Have you compiled with both UARTA and UARTD enabled?  Redeclaring 'uart'
in the middle of the function should throw an error or warning.

+   uart_clock_init();
+
+   /* Enable UARTD - uses config 0 */
+   pin_mux_uart();
+
+   setup_uart(uart);
+#endif /* CONFIG_TEGRA2_ENABLE_UARTD */
+}

Best,
Peter

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


Re: [U-Boot] [PATCH V5 2/4] serial: Add Tegra2 serial port support

2011-01-24 Thread Peter Tyser
snip

 I see what you're talking about now - I've got uart.c in 2 patch files - 
 created
 in 0001 and then moved in 0002. My bad - that wasn't the intent, just what
 happened when I applied my V4 patches to a new branch to get the V5 patchset
 built.  I'll fix it and resubmit.
 
 As to 0002 not adding serial port support for Tegra2, that's all it does - 
 adds
 TEGRA2 defines to serial.h/serial.c for the eserial* tables, and then adds
 code to turn on Tegra2-specific UART HW.  If I remove any mention of uart.c
 in patch 0001 (add basic Tegra2 support), then does patch 0002 make
 sense to you?

Yeah, that'd make more sense.  Patch 2 would just contain:
 common/serial.c|3 +-
 drivers/serial/Makefile|1 +
 drivers/serial/serial_tegra2.c |  205 ++
 drivers/serial/serial_tegra2.h |   49 
 include/serial.h   |3 +-

   snip
  
   +void uart_init(void)
   +{
   + /* Init each UART - there may be more than 1 on a board/build */
   +#if (CONFIG_TEGRA2_ENABLE_UARTA)
   + init_uart();
   +#endif
   +#if (CONFIG_TEGRA2_ENABLE_UARTD)
   + init_uart();
   +#endif
   +}
  
   How about:
   #if defined(CONFIG_TEGRA2_ENABLE_UARTA) || 
   defined(CONFIG_TEGRA2_ENABLE_UARTD)
  init_uart();
   #endif
  That won't work for a board like Harmony where the developer wants
  both UARTs active, since uart_init is only called once.
 
  Why should init_uart() be called two times?  It looks to initialize both
  ports, meaning it should only be called once, right?
 Correct, again (need more coffee!)  Your if defined construct wouldn't work
 as written, though, because CONFIG_TEGRA2_ENABLE_UARTx is always
 defined (as 0 or 1). I'd have to rework it.

You could also just get rid of uart_init() altogether and rename
init_uart() to uart_init().  That would get rid of some idefs and
simplify the flow.

Best,
Peter

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


Re: [U-Boot] [PATCH V5 2/4] serial: Add Tegra2 serial port support

2011-01-21 Thread Peter Tyser
Hi Tom,

On Fri, 2011-01-21 at 16:06 -0700, Tom Warren wrote:
 Signed-off-by: Tom Warren twar...@nvidia.com
 ---
 Changes for V2:
   - Move serial driver to separate patch
 
 Changes for V5:
   - Move arch/arm/cpu/armv7/uart.c  board.h to drivers/serial and
   rename to serial_tegra2.c
   - Remove use of uart_num  UART_A/D in serial_tegra2, simplify code
 
  arch/arm/cpu/armv7/tegra2/Makefile |2 +-
  arch/arm/cpu/armv7/tegra2/board.c  |2 +-
  arch/arm/cpu/armv7/tegra2/board.h  |   58 --
  arch/arm/cpu/armv7/tegra2/uart.c   |  216 
 
  common/serial.c|3 +-
  drivers/serial/Makefile|1 +
  drivers/serial/serial_tegra2.c |  205 ++
  drivers/serial/serial_tegra2.h |   49 
  include/serial.h   |3 +-
  9 files changed, 261 insertions(+), 278 deletions(-)
  delete mode 100644 arch/arm/cpu/armv7/tegra2/board.h
  delete mode 100644 arch/arm/cpu/armv7/tegra2/uart.c
  create mode 100644 drivers/serial/serial_tegra2.c
  create mode 100644 drivers/serial/serial_tegra2.h

It looks like arch/arm/cpu/armv7/tegra2/board.h and
arch/arm/cpu/armv7/tegra2/uart.c are added in the first patch, then
moved in this patch.  It'd be ideal to just add them once in the proper
location.

On a side note, if you pass git format-patch the -M and -C options it
will make pretty diffs that only show what lines changed during a move.
In the case that you do move files in the future its nice to use those
options to ease review.

snip

+void uart_init(void)
 +{
 + /* Init each UART - there may be more than 1 on a board/build */
 +#if (CONFIG_TEGRA2_ENABLE_UARTA)
 + init_uart();
 +#endif
 +#if (CONFIG_TEGRA2_ENABLE_UARTD)
 + init_uart();
 +#endif
 +}

How about:
#if defined(CONFIG_TEGRA2_ENABLE_UARTA) || defined(CONFIG_TEGRA2_ENABLE_UARTD)
init_uart();
#endif

Best,
Peter

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


Re: [U-Boot] [PATCH v3 1/4] arm: Tegra2: Add basic NVIDIA Tegra2 SoC support

2011-01-20 Thread Peter Tyser
On Thu, 2011-01-20 at 09:41 -0700, Tom Warren wrote:
 On Wed, Jan 19, 2011 at 5:04 PM, Peter Tyser pty...@xes-inc.com wrote:
  Hi Tom,
  Some last minutes nits:
 
  It looks like some of the new functions can be declared statically.
  It'd be nice to do so where possible.
 Which functions, Peter? Please point them out specifically, thanks.

Any function that won't be called from outside the scope of the file.
Eg it looks like init_uart() and setup_uart() are local functions and
should be static.  Those are the 2 that jumped out initially, but you
should review to see if there are others.

snip

  +/*
  + * Routine: uart_clock_init
  + * Description: init the PLL and clock for the UART in uart_num
  + */
  +void uart_clock_init(int uart_num)
  +{
 
  Are all these uart functions board-specific?  They look more
  CPU-specific.  If that's the case they should be moved somewhere in
  arch/arm/*.  Other boards that use the Tegra2 don't want to duplicate
  this code or link into Nvidia's board/nvidia directory.
 It's Tegra2 SoC-specific - that's not the CPU, per se.  I guess I could move
 it to arch/arm/cpu/armv7/tegra2, if you think it's important enough.

I think they should be moved.  If they aren't, the next board vendor (eg
my company) that uses the Tegra2 will copy your board.[ch] into their
board/vendor directory and use them as a starting point, which is a
large duplication of code.  Moving it somewhere in arch/arm is the
right thing to do and will make every future tegra2 board port cleaner
and easier.

Best,
Peter


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


Re: [U-Boot] [PATCH v3 1/4] arm: Tegra2: Add basic NVIDIA Tegra2 SoC support

2011-01-19 Thread Peter Tyser
Hi Tom,
Some last minutes nits:

It looks like some of the new functions can be declared statically.
It'd be nice to do so where possible.

snip

 --- /dev/null
 +++ b/arch/arm/cpu/armv7/tegra2/lowlevel_init.S
 @@ -0,0 +1,66 @@
 +/*
 + * Board specific setup info

This is CPU-specific code, correct?

 + *
 + * (C) Copyright 2010,2011
 + * NVIDIA Corporation www.nvidia.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 config.h
 +#include version.h
 +
 +_TEXT_BASE:
 + .word   CONFIG_SYS_TEXT_BASE@ sdram load addr from config file
 +
 +.global invalidate_dcache
 +invalidate_dcache:
 + mov pc, lr
 +
 +
 + .align  5
 +.global reset_cpu
 +reset_cpu:
 + ldr r1, rstctl  @ get addr for global reset
 + @ reg
 + ldr r3, [r1]
 + orr r3, r3, #0x10
 + str r3, [r1]@ force reset
 + mov r0, r0
 +_loop_forever:
 + b   _loop_forever
 +rstctl:
 + .word   PRM_RSTCTRL
 +
 +.globl lowlevel_init
 +lowlevel_init:
 + ldr sp, SRAM_STACK
 + str ip, [sp]
 + mov ip, lr
 + bl  s_init  @ go setup pll, mux  memory
 + ldr ip, [sp]
 + mov lr, ip
 +
 + mov pc, lr  @ back to arch calling code
 +
 + @ the literal pools origin
 + .ltorg
 +
 +SRAM_STACK:
 + .word LOW_LEVEL_SRAM_STACK
 diff --git a/arch/arm/cpu/armv7/tegra2/sys_info.c 
 b/arch/arm/cpu/armv7/tegra2/sys_info.c
 new file mode 100644
 index 000..6d11dc1
 --- /dev/null
 +++ b/arch/arm/cpu/armv7/tegra2/sys_info.c
 @@ -0,0 +1,35 @@
 +/*
 + * (C) Copyright 2010,2011
 + * NVIDIA Corporation www.nvidia.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
 +
 +#ifdef CONFIG_DISPLAY_CPUINFO
 +/* Print CPU information */
 +int print_cpuinfo(void)
 +{
 + puts(TEGRA2\n);
 +
 + /* TBD: Add printf of major/minor rev info, stepping, etc. */
 + return 0;
 +}
 +#endif   /* CONFIG_DISPLAY_CPUINFO */
 diff --git a/arch/arm/cpu/armv7/tegra2/timer.c 
 b/arch/arm/cpu/armv7/tegra2/timer.c
 new file mode 100644
 index 000..858af0f
 --- /dev/null
 +++ b/arch/arm/cpu/armv7/tegra2/timer.c
 @@ -0,0 +1,122 @@
 +/*
 + * (C) Copyright 2010,2011
 + * NVIDIA Corporation www.nvidia.com
 + *
 + * (C) Copyright 2008
 + * Texas Instruments
 + *
 + * Richard Woodruff r-woodru...@ti.com
 + * Syed Moahmmed Khasim kha...@ti.com
 + *
 + * (C) Copyright 2002
 + * Sysgo Real-Time Solutions, GmbH www.elinos.com
 + * Marius Groeger mgroe...@sysgo.de
 + * Alex Zuepke a...@sysgo.de
 + *
 + * (C) Copyright 2002
 + * Gary Jennejohn, DENX Software Engineering, ga...@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 

Re: [U-Boot] [RFC PATCH] ARM: print gcc version

2011-01-18 Thread Peter Tyser
  Anyway, I would found it a nice feature, at startup or when running the
  version command, but both aren't a must.
 
  I think it would be a really useful extension to the version command.
  Looking forwad to seeing your patch.
 
 Maybe if someone could feed me with what to use for the version. E.g. my 
 gcc here defines __VERSION__ as 4.5.2 but when I'm looking at
 u-boot/lib/asm-offsets.s I see
 
 .ident  GCC: (Gentoo 4.5.2 p1.0, pie-0.4.5) 4.5.2
 
 So I would like to display that which would be in line with the comment 
 section of generated binaries.
 
 But when I use gcc -E -dM empty.c to print all predefined macros, I 
 don't see the text found in .ident. Not even something else which 
 includes Gentoo.

I believe the output of $(CC) --version should contain the same data
as your .ident string.  You could echo it into a file like:
diff --git a/Makefile b/Makefile
index 0685ef9..e070d40 100644
--- a/Makefile
+++ b/Makefile
@@ -416,6 +416,8 @@ $(U_BOOT_ONENAND):  $(ONENAND_IPL) $(obj)u-boot.bin
 $(VERSION_FILE):
@( printf '#define U_BOOT_VERSION U-Boot %s%s\n' 
$(U_BOOT_VERSION) \
 '$(shell $(TOPDIR)/tools/setlocalversion $(TOPDIR))' )  $@.tmp
+   @( printf '#define CC_VERSION_STRING %s\n' \
+'$(shell $(CC) --version | head -1)' )  $@.tmp
@cmp -s $@ $@.tmp  rm -f $@.tmp || mv -f $@.tmp $@
 
 $(TIMESTAMP_FILE):

Best,
Peter

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


Re: [U-Boot] [PATCH 0/4 V2] Add basic NVIDIA Tegra2 SoC support

2011-01-14 Thread Peter Tyser
Hi Tom,

On Fri, 2011-01-14 at 10:11 -0700, Tom Warren wrote:
 This series of patches adds preliminary/baseline support for NVIDIA's
 Tegra2 SoC.  Basic CPU (AVP), RAM and UART init are covered so that the
 system (Harmony or Seaboard) can boot to the U-Boot serial cmd prompt.
 
 Further support (for Cortex-A9 CPU(s), USB, SD/MMC, etc.) to follow.
 
 V2: Make changes based on feedback from Peter Tyser and Sandeep Paulraj.

If you didn't use all the feedback to the original patches, you should
state explicitly what you changed here, or respond to the original
comment email as to why they weren't made.  For example, I see you
didn't make the suggested change to use IO access functions, or allow
compiling out of support for UARTA and UARTD.  That should be made clear
somewhere (and the logic of why the changes weren't made) so that those
reviewing the patches know what changed between v1 and v2 and why.  As
is its unclear why the v1 comments weren't implemented, so they also
apply to this series.

Best,
Peter

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


Re: [U-Boot] [PATCH 0/4 V2] Add basic NVIDIA Tegra2 SoC support

2011-01-14 Thread Peter Tyser
On Fri, 2011-01-14 at 13:41 -0700, Tom Warren wrote:
 On Fri, Jan 14, 2011 at 12:59 PM, Peter Tyser pty...@xes-inc.com wrote:
  Hi Tom,
 
  On Fri, 2011-01-14 at 10:11 -0700, Tom Warren wrote:
  This series of patches adds preliminary/baseline support for NVIDIA's
  Tegra2 SoC.  Basic CPU (AVP), RAM and UART init are covered so that the
  system (Harmony or Seaboard) can boot to the U-Boot serial cmd prompt.
 
  Further support (for Cortex-A9 CPU(s), USB, SD/MMC, etc.) to follow.
 
  V2: Make changes based on feedback from Peter Tyser and Sandeep Paulraj.
 
  If you didn't use all the feedback to the original patches, you should
  state explicitly what you changed here, or respond to the original
  comment email as to why they weren't made.  For example, I see you
  didn't make the suggested change to use IO access functions, or allow
  compiling out of support for UARTA and UARTD.  That should be made clear
  somewhere (and the logic of why the changes weren't made) so that those
  reviewing the patches know what changed between v1 and v2 and why.  As
  is its unclear why the v1 comments weren't implemented, so they also
  apply to this series.
 
 Peter,
 
 Sorry, since this is my first patch series to U-Boot upstream, I'm
 still learning the proper etiquette.
 Let me respond here rather than in the patch comments since there are
 only 2 unchanged areas WRT comments.

No worries.  I'm glad to see you're pushing your changes upstream.

 1) IO access functions - I pre-reviewed my patch series with Wolfgang
 (to hopefully catch any blatant errors and smooth
 the process) and he indicated that C structs and I/O accessor funcs or
 macros were preferred to my base+offset original code.
 Since the ARM is 32-bit, and all of our registers are I/O mapped, it
 made sense just to cast the necessary HW mem-mapped
 regs as volatile structs and access the members directly. Works well,
 is easy to read  understand, etc. Let me know (with
 examples, if possible) how I can make it better.

Both Linux and U-Boot recommend using IO access functions instead
pointer accesses, at least in PPC-land, and even for memory mapped
registers.  I'm not too familiar with ARM, but assume they have the same
recommendation.  If pointers are used, some CPUs may optimize the access
order, thus causing problems.  Using IO accessors also ensures that the
code is portable.  Even if the Tegra doesn't re-order accesses, a driver
you write for use in a Tegra could be used on other CPUs that do.

eg:
http://www.mail-archive.com/u-boot@lists.denx.de/msg18435.html
http://lists.denx.de/pipermail/u-boot/2007-December/027595.html

It looks like other ARM processors use IO accessors too, eg the recent
armada CPU:
arch/arm/cpu/arm926ejs/armada100/*
A grep of writel in arch/arm shows a number of references.  Similarly in
the Linux code in arch/arm.

ARM maintainers, feel free to chime in if you have comments.

 2) Compiling out support for UARTA or UARTD - didn't seem necessary -
 size isn't an issue at this point with Tegra2 U-Boot,
 and some boards (Harmony, for example) are populated w/hardware for
 both UARTA and UARTD, and can have both on at
 U-Boot runtime (perhaps for debug out to UARTA whilst normal console
 I/O goes to UARTD), so I chose to leave the init code
 for both intact. Plus I've never liked code with too many unnecessary
 ifdef's - makes it less readable, IMO.

Yeah, this is grey - I see both sides of the coin.  U-Boot generally
strives to be as small as possible.  You may use a large flash on your
eval boards and not care about space, but other system designers may
place as small of flash as possible to save cost.  It looked like
UART_A-UART_E were also going to be supported at some time, which is
quite a chunk of code.  You also already are defining which serial ports
are available at compile time via CONFIG_TEGRA2_ENABLE_UARTA and
CONFIG_TEGRA2_ENABLE_UARTD.

Anyway, I don't really care much which way you go, just wanted to make
sure you were making a conscious decision to not add the ifdefs.

 I was going to respond to your review w/a direct, inline reply, but I
 thought it better to get the V2 patch out there before the
 weekend (we're off for MLK, as well). I'm under some pressure to get a
 baseline Tegra2 patchset in before the merge window
 closes.  I'll be sure to respond to each issue directly on the list in
 the future, though.

As long as you initially submit your patches during the merge window
they will generally be merged, even if there are multiple revisions and
discussion that lasts past the end of official merge window.  So this
stuff should get in.

Best,
Peter

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


Re: [U-Boot] [PATCH 0/4 V2] Add basic NVIDIA Tegra2 SoC support

2011-01-14 Thread Peter Tyser
On Sat, 2011-01-15 at 00:00 +0100, Albert ARIBAUD wrote:
 Hello,
 
 Le 14/01/2011 23:39, Tom Warren a écrit :
 
  So instead of, say uart-lcr = 0, you'd prefer writel(0, uart-lcr),
  where writel = __arch_putl(v, a) = (*(volatile unsigned int *)(a) =
  (v))?
  Is that different enough from 'uart-lcr = 0' to warrant the change?
  Does it add some HW barriers or forced read-before-write that the
  'volatile' struct doesn't?
 
 writel() and readl() do not introduce read-before-write, that is, they 
 do not perform any more than what their names imply, but yes they do 
 introduce barriers, or more precisely, they force the compiler to do so.

Agreed, I should have dug deeper.  On PPC we use out_be32() or similar
to access memory mapped registers, which does have an explicit barrier.
I'm not familiar with ARM so don't know what the proper access functions
are, but it looks like the defacto standard writel()/readl().

I'd personally use writel()/readl() in this patch.  Functionally it may
be the same as your volatile accesses, but its the generally recommended
practice.  It looks like most of the Tegra support in the Linux kernel
also uses writel()/readl() for what its worth.  Ultimately its up to the
ARM U-Boot maintainer to decide if they require the access functions or
not - I'm just giving my opinions from PPC experience.

Regards,
Peter

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


Re: [U-Boot] [PATCH 1/3] arm: Tegra2: Add generic Tegra2 SoC support Signed-off-by: Tom Warren twar...@nvidia.com

2011-01-12 Thread Peter Tyser
Hi,

Your Signed-off-by seems to have made its way into the subject line.  It
should be moved into the email contents.  A brief description of what
this patch does, what peripherals are supported, etc would be nice too.

snip

  create mode 100644 board/tegra2/common/board.c
  create mode 100644 board/tegra2/common/board.h

The format of the board directory is generally board/vendor/board or
board/board if the vendor only plans on maintaining 1 board.  If these
are nvidia-supported boards I'd think board/nvidia is the proper place
to add files board-specific files.

snip

 +/*
 + * Boot ROM initializes the odmdata in APBDEV_PMC_SCRATCH20_0,
 + * so we are using this value to identify memory size.
 + */
 +
 +static unsigned int query_sdram_size(void)
 +{
 + pmc_ctlr *const pmc = (pmc_ctlr *)NV_PA_PMC_BASE;
 + u32 reg;
 +
 + reg = pmc-pmc_scratch20;

In powerpc-land we use io access functions to access registers, eg
in_be32().  I'm not familiar with ARM, but I'd guess they should be used
in general.  Someone with ARM experience feel free to chime in here.

 + debug(pmc-pmc_scratch20 (ODMData) = 0x%08lX\n, reg);
 +
 + /* bits 31:28 in OdmData are used for RAM size  */
 + switch ((reg)  28) {
 + case 1:
 + return 0x1000;  /* 256 MB */
 + case 2:
 + return 0x2000;  /* 512 MB */
 + case 3:
 + default:
 + return 0x4000;  /* 1GB */
 + }
 +}

snip

 +#ifndef _CLK_RST_H_
 +#define _CLK_RST_H_
 +
 +/* Clock/Reset Controller (CLK_RST_CONTROLLER_) regs */
 +

Instead of adding a new vol_uint typedef and making each field of this
struct vol_uint, you can make each one uint, then instantiate the
structure as volatile.

There are a mixture of lower and upper case hex digits in the comments
below too - it'd be good to be consistent.

 +typedef struct clk_rst_ctlr {
 + vol_uint crc_rst_src;   /* _RST_SOURCE_0,   0x00*/
 + vol_uint crc_rst_dev_l; /* _RST_DEVICES_L_0,0x04*/
 + vol_uint crc_rst_dev_h; /* _RST_DEVICES_H_0,0x08*/
 + vol_uint crc_rst_dev_u; /* _RST_DEVICES_U_0,0x0C*/
 + vol_uint crc_clk_out_enb_l; /* _CLK_OUT_ENB_L_0,0x10*/
 + vol_uint crc_clk_out_enb_h; /* _CLK_OUT_ENB_H_0,0x14*/
 + vol_uint crc_clk_out_enb_u; /* _CLK_OUT_ENB_U_0,0x18*/
 + vol_uint crc_reserved0; /* reserved_0,  0x1c*/
 + vol_uint crc_cclk_brst_pol; /* _CCLK_BURST_POLICY_0,0x20*/
 + vol_uint crc_super_cclk_div;/* _SUPER_CCLK_DIVIDER_0,0x24*/
 + vol_uint crc_sclk_brst_pol; /* _SCLK_BURST_POLICY_0, 0x28*/
 + vol_uint crc_super_sclk_div;/* _SUPER_SCLK_DIVIDER_0,0x2C*/
 + vol_uint crc_clk_sys_rate;  /* _CLK_SYSTEM_RATE_0,  0x30*/
 + vol_uint crc_prog_dly_clk;  /* _PROG_DLY_CLK_0, 0x34*/
 + vol_uint crc_aud_sync_clk_rate; /* _AUDIO_SYNC_CLK_RATE_0,0x38*/
 + vol_uint crc_reserved1; /* reserved_1,  0x3c*/
 + vol_uint crc_cop_clk_skip_plcy; /* _COP_CLK_SKIP_POLICY_0,0x40*/
 + vol_uint crc_clk_mask_arm;  /* _CLK_MASK_ARM_0, 0x44*/
 + vol_uint crc_misc_clk_enb;  /* _MISC_CLK_ENB_0, 0x48*/
 + vol_uint crc_clk_cpu_cmplx; /* _CLK_CPU_CMPLX_0,0x4C*/
 + vol_uint crc_osc_ctrl;  /* _OSC_CTRL_0, 0x50*/
 + vol_uint crc_pll_lfsr;  /* _PLL_LFSR_0, 0x54*/
 + vol_uint crc_osc_freq_det;  /* _OSC_FREQ_DET_0, 0x58*/
 + vol_uint crc_osc_freq_det_stat; /* _OSC_FREQ_DET_STATUS_0,0x5C*/
 + vol_uint crc_reserved2[8];  /* reserved_2[8],   0x60-7C*/
 +
 + vol_uint crc_pllc_base; /* _PLLC_BASE_0,0x80*/
 + vol_uint crc_pllc_out;  /* _PLLC_OUT_0, 0x84*/
 + vol_uint crc_reserved3; /* reserved_3,  0x88*/
 + vol_uint crc_pllc_misc; /* _PLLC_MISC_0,0x8C*/
 +
 + vol_uint crc_pllm_base; /* _PLLM_BASE_0,0x90*/
 + vol_uint crc_pllm_out;  /* _PLLM_OUT_0, 0x94*/
 + vol_uint crc_reserved4; /* reserved_4,  0x98*/
 + vol_uint crc_pllm_misc; /* _PLLM_MISC_0,0x9C*/
 +
 + vol_uint crc_pllp_base; /* _PLLP_BASE_0,0xA0*/
 + vol_uint crc_pllp_outa; /* _PLLP_OUTA_0,0xA4*/
 + vol_uint crc_pllp_outb; /* _PLLP_OUTB_0,0xA8*/
 + vol_uint crc_pllp_misc; /* _PLLP_MISC_0,0xAC*/
 +
 + vol_uint crc_plla_base; /* _PLLA_BASE_0,0xB0*/
 + vol_uint crc_plla_out;  /* _PLLA_OUT_0, 0xB4*/
 + vol_uint crc_reserved5; /* reserved_5,  0xB8*/
 + vol_uint crc_plla_misc; /* _PLLA_MISC_0,0xBC*/
 +
 + vol_uint crc_pllu_base; /* _PLLU_BASE_0,0xC0*/
 + vol_uint crc_reserved6; /* _reserved_6, 0xC4*/
 + vol_uint crc_reserved7; /* _reserved_7, 0xC8*/
 + 

Re: [U-Boot] [PATCH 2/3] arm: Tegra2: Add support for NVIDIA Harmony board Signed-off-by: Tom Warren twar...@nvidia.com

2011-01-12 Thread Peter Tyser
snip

 --- a/boards.cfg
 +++ b/boards.cfg
 @@ -122,6 +122,7 @@ omap4_panda  arm armv7   
 panda   ti
  omap4_sdp4430arm armv7   sdp4430 ti  
omap4
  s5p_goni arm armv7   goni
 samsungs5pc1xx
  smdkc100 arm armv7   smdkc100
 samsungs5pc1xx
 +harmony  arm armv7   harmony 
 tegra2 tegra2

I think 1st tegra2 above should be changed to nvidia.

  actux1   arm ixp
  actux2   arm ixp
  actux3   arm ixp

snip

 diff --git a/include/configs/nv-common.h b/include/configs/nv-common.h

Perhaps a more descriptive name could be used?  NV == non-volatile to me
at first glance, and if this is tegra2-specific maybe tegra2-common.h
would be more appropriate?

 new file mode 100644
 index 000..46872d3
 --- /dev/null
 +++ b/include/configs/nv-common.h
 @@ -0,0 +1,166 @@
 +/*
 + *  (C) Copyright 2010,2011
 + *  NVIDIA Corporation www.nvidia.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 __TEGRA2_COMMON_H
 +#define __TEGRA2_COMMON_H

This define should match the name of the file.

 +#include asm/sizes.h
 +
 +/*
 + * High Level Configuration Options
 + */
 +#define CONFIG_ARMCORTEXA9   /* This is an ARM V7 CPU core */
 +#define CONFIG_TEGRA2/* in a NVidia Tegra2 core */
 +#define CONFIG_MACH_TEGRA_GENERIC/* which is a Tegra generic machine */
 +#define CONFIG_L2_OFF/* No L2 cache */
 +
 +#include asm/arch/tegra2.h /* get chip and board defs */
 +
 +/*
 + * Display CPU and Board information
 + */
 +#define CONFIG_DISPLAY_CPUINFO
 +#define CONFIG_DISPLAY_BOARDINFO
 +
 +#undef CONFIG_USE_IRQ

The general policy is not to undef things unless its necessary.
Assuming CONFIG_USE_IRQ isn't defined elsewhere I'd remove the above
line.

 +
 +#define CONFIG_SKIP_RELOCATE_UBOOT
 +#define CONFIG_SKIP_LOWLEVEL_INIT
 +
 +#define CONFIG_CMDLINE_TAG   /* enable passing of ATAGs */
 +
 +/* Environment */
 +#define CONFIG_ENV_IS_NOWHERE
 +#define CONFIG_ENV_SIZE  0x2 /* Total Size 
 Environment */
 +
 +/*
 + * Size of malloc() pool
 + */
 +#define CONFIG_SYS_MALLOC_LEN(4  20)   /* 4MB  */
 +
 +/*
 + * PllX Configuration
 + */
 +#define CONFIG_SYS_CPU_OSC_FREQUENCY 100 /* Set CPU clock to 1GHz */
 +
 +/*
 + * NS16550 Configuration
 + */
 +#define V_NS16550_CLK21600   /* 216MHz 
 (pllp_out0) */
 +
 +#define CONFIG_SYS_NS16550
 +#define CONFIG_SYS_NS16550_SERIAL
 +#define CONFIG_SYS_NS16550_REG_SIZE  (-4)
 +#define CONFIG_SYS_NS16550_CLK   V_NS16550_CLK
 +
 +/*
 + * select serial console configuration
 + */
 +#define CONFIG_CONS_INDEX1
 +
 +/* allow to overwrite serial and ethaddr */
 +#define CONFIG_ENV_OVERWRITE
 +#define CONFIG_BAUDRATE  115200
 +#define CONFIG_SYS_BAUDRATE_TABLE{4800, 9600, 19200, 38400, 57600,\
 + 115200}
 +
 +/* include default commands */
 +#include config_cmd_default.h
 +
 +/* remove unused commands */
 +#undef CONFIG_CMD_FLASH  /* flinfo, erase, protect */
 +#undef CONFIG_CMD_FPGA   /* FPGA configuration support */
 +#undef CONFIG_CMD_IMI
 +#undef CONFIG_CMD_IMLS
 +#undef CONFIG_CMD_NFS/* NFS support */
 +#undef CONFIG_CMD_NET/* network support */
 +
 +/* turn on command-line edit/hist/auto */
 +#define CONFIG_CMDLINE_EDITING
 +#define CONFIG_COMMAND_HISTORY
 +#define CONFIG_AUTOCOMPLETE
 +
 +#define CONFIG_SYS_NO_FLASH
 +
 +/* Environment information */
 +#define CONFIG_EXTRA_ENV_SETTINGS \
 + console=ttyS0,115200n8\0 \
 + mem= TEGRA2_SYSMEM \0 \
 + smpflag=smp\0 \
 +
 +#define CONFIG_LOADADDR  0x408000/* def. location for 
 kernel */
 +#define CONFIG_BOOTDELAY 2   /* -1 to disable auto boot */
 +
 +/*
 + * Miscellaneous configurable options
 + */
 +#define 

Re: [U-Boot] [U-Boot,PATCHv4] pca953x: support 16-pin devices

2011-01-12 Thread Peter Tyser
On Thu, 2011-01-13 at 17:02 +1300, Chris Packham wrote:
 On Mon, Jan 10, 2011 at 8:02 PM, Heiko Schocher h...@denx.de wrote:
  Hello Chris,
 
  Sorry for the late reply, but just looked in patchwork and found that
  I am responsible for your patch, so ...
 
  Chris Packham wrote:
  This adds support for for the PCA9535/PCA9539 family of gpio devices which
  have 16 output pins.
 
  To let the driver know which devices are 16-pin it is necessary to define
  CONFIG_SYS_I2C_PCA953X_WIDTH in your board config file. This is used to
  create an array of {chip, ngpio} tuples that are used to determine the
  width of a particular chip. For backwards compatibility it is assumed that
  any chip not defined in CONFIG_SYS_I2C_PCA953X_WIDTH has 8 pins.
 
  Acked-by: Peter Tyser pty...@xes-inc.com
  Tested-by: Peter Tyser pty...@xes-inc.com
  Signed-off-by: Chris Packham chris.pack...@alliedtelesis.co.nz
 
  ---
  Changes since v3:
  - pca953x_ngpio is now a function in all cases.
  - Added Peter's ACK to the commit message.
 
   README |4 ++
   drivers/gpio/pca953x.c |  114 
  ++--
   2 files changed, 95 insertions(+), 23 deletions(-)
 
  Compiles with actual u-boot the xpedite* boards, so added to
  u-boot-i2c.git
 
  Thanks!
 
  bye,
  Heiko
 
 Thanks I was about to poke the mailing list to find out where things
 had got to. Did I miss something on
 http://www.denx.de/wiki/U-Boot/Patches that says look here list for
 a custodian and CC them?

There's a list of custodians at
http://www.denx.de/wiki/U-Boot/Custodians , but your GPIO driver doesn't
really fall neatly into any one of their areas of responsibility, so
it'd be tough to know who to CC.  I added a comment to
http://www.denx.de/wiki/U-Boot/Patches to mention that appropriate
maintainers should be CC-ed, as well as people who might be familiar
with the change based on past commits for what its worth.

Best,
Peter

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


[U-Boot] [PATCH] cmd_jffs2: Fix get_part_sector_size_nor() overflow bug

2010-12-30 Thread Peter Tyser
When a flash partition was positioned at the very top of a 32-bit memory
map (eg located at 0xf800 with a size of 0x800)
get_part_sector_size_nor() would incorrectly calculate the partition's
ending address to 0x0 due to overflow.  When the overflow occurred
get_part_sector_size_nor() would falsely return a sector size of 0.
A sector size of 0 results in subsequent jffs2 operations failing.

To workaround the overflow subtract 1 from calculated address of
the partition endpoint.

Signed-off-by: Peter Tyser pty...@xes-inc.com
---
 common/cmd_jffs2.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/common/cmd_jffs2.c b/common/cmd_jffs2.c
index 0e7a6b0..27296dd 100644
--- a/common/cmd_jffs2.c
+++ b/common/cmd_jffs2.c
@@ -281,7 +281,7 @@ static inline u32 get_part_sector_size_nor(struct mtdids 
*id, struct part_info *
flash = flash_info[id-num];
 
start_phys = flash-start[0] + part-offset;
-   end_phys = start_phys + part-size;
+   end_phys = start_phys + part-size - 1;
 
for (i = 0; i  flash-sector_count; i++) {
if (flash-start[i] = end_phys)
-- 
1.7.0.4

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


Re: [U-Boot] [PATCH 08/15] powerpc/8xxx: Rework XES boards pci_init_board to use common FSL PCIe code

2010-12-28 Thread Peter Tyser

  Any direction on how these should be applied for testing?
  
  snip
 
 I've pushed a 'dev' branch on u-boot-85xx.git on denx.de with the current set 
 of patches applied.

Thanks.  Things are much tidier now.  I had a few comments.  Let me know
if you'd like me to submit patches to address any of them.

* There's a large chunk of defines that are no longer needed in the X-ES
board code:

--- a/board/xes/common/fsl_8xxx_pci.c
+++ b/board/xes/common/fsl_8xxx_pci.c
@@ -35,29 +35,6 @@
 static struct pci_controller pci1_hose;
 #endif
 
-/*
- * 85xx and 86xx share naming conventions, but different layout.
- * Correlate names to CPU-specific values to share common
- * PCI code.
- */
-#if defined(CONFIG_MPC85xx)
-#define MPC8xxx_DEVDISR_PCIE1  MPC85xx_DEVDISR_PCIE
-#define MPC8xxx_DEVDISR_PCIE2  MPC85xx_DEVDISR_PCIE2
-#define MPC8xxx_DEVDISR_PCIE3  MPC85xx_DEVDISR_PCIE3
-#define MPC8xxx_PORDEVSR_IO_SELMPC85xx_PORDEVSR_IO_SEL
-#define MPC8xxx_PORDEVSR_IO_SEL_SHIFT  MPC85xx_PORDEVSR_IO_SEL_SHIFT
-#define MPC8xxx_PORBMSR_HA MPC85xx_PORBMSR_HA
-#define MPC8xxx_PORBMSR_HA_SHIFT   MPC85xx_PORBMSR_HA_SHIFT
-#elif defined(CONFIG_MPC86xx)
-#define MPC8xxx_DEVDISR_PCIE1  MPC86xx_DEVDISR_PCIEX1
-#define MPC8xxx_DEVDISR_PCIE2  MPC86xx_DEVDISR_PCIEX2
-#define MPC8xxx_DEVDISR_PCIE3  0   /* 8641 doesn't have PCIe3 */
-#define MPC8xxx_PORDEVSR_IO_SELMPC8641_PORDEVSR_IO_SEL
-#define MPC8xxx_PORDEVSR_IO_SEL_SHIFT  MPC8641_PORDEVSR_IO_SEL_SHIFT
-#define MPC8xxx_PORBMSR_HA MPC8641_PORBMSR_HA
-#define MPC8xxx_PORBMSR_HA_SHIFT   MPC8641_PORBMSR_HA_SHIFT
-#endif
-
 void pci_init_board(void)
 {
int first_free_busno = 0;



* It'd be nice if boards didn't have to define board_serdes_name(), or
at least a sane print statement was used if it wasn't.  When I booted an
X-ES board the first time I got:
PCIE1: connected to NULL as Endpoint (base addr ef008000)
PCIE1: Bus 00 - 00
PCIE2: connected to NULL as Endpoint (base addr ef009000)
PCIE2: Bus 01 - 01

Other boards will have the same issue.  Something like the following?:
--- a/drivers/pci/fsl_pci_init.c
+++ b/drivers/pci/fsl_pci_init.c
@@ -526,8 +526,8 @@ int fsl_configure_pcie(struct fsl_pci_info *info,
set_next_law(info-mem_phys, law_size_bits(info-mem_size), info-law);
set_next_law(info-io_phys, law_size_bits(info-io_size), info-law);
is_endpoint = fsl_setup_hose(hose, info-regs);
-   printf(PCIE%u: connected to %s as %s (base addr %lx)\n,
-  info-pci_num, connected,
+   printf(PCIE%u: connected%s%s as %s (base addr %lx)\n,
+  info-pci_num, connected ?  to  : , connected ? connected : 
,
   is_endpoint ? Endpoint : Root Complex, info-regs);
return fsl_pci_init_port(info, hose, busno);
 }


* Lastly, what about using a new define to specify the PCIe port's name
instead of making each board add a board_serdes_name() function?  This
reduces each board directory's clutter and makes it so all PCIe-related
configuration and naming occurs in the board's config.h file.  As an
example that uses the xpedite517x and mpc8572ds:
--- a/board/freescale/mpc8572ds/mpc8572ds.c
+++ b/board/freescale/mpc8572ds/mpc8572ds.c
@@ -149,17 +149,6 @@ phys_size_t fixed_sdram (void)
 #endif
 
 #ifdef CONFIG_PCI
-static const char *slot_names[] = {
-   [PCIE1] = Slot 2,
-   [PCIE2] = Slot 1,
-   [PCIE3] = ULI,
-};
-
-const char *board_serdes_name(enum srds_prtcl device)
-{
-   return slot_names[device];
-}
-
 void pci_init_board(void)
 {
struct pci_controller *hose;
--- a/drivers/pci/fsl_pci_init.c
+++ b/drivers/pci/fsl_pci_init.c
@@ -558,7 +558,26 @@ int fsl_configure_pcie(struct fsl_pci_info *info,
 /* Implement a dummy function for those platforms w/o SERDES */
 static const char *__board_serdes_name(enum srds_prtcl device)
 {
-   return NULL;
+   switch (device) {
+#ifdef CONFIG_SYS_PCIE1_NAME
+   case PCIE1:
+   return CONFIG_SYS_PCIE1_NAME;
+#endif
+#ifdef CONFIG_SYS_PCIE2_NAME
+   case PCIE2:
+   return CONFIG_SYS_PCIE2_NAME;
+#endif
+#ifdef CONFIG_SYS_PCIE3_NAME
+   case PCIE3:
+   return CONFIG_SYS_PCIE3_NAME;
+#endif
+#ifdef CONFIG_SYS_PCIE4_NAME
+   case PCIE4:
+   return CONFIG_SYS_PCIE4_NAME;
+#endif
+   default:
+   return NULL;
+   }
 }
 
 __attribute__((weak, alias(__board_serdes_name))) const char *
--- a/include/configs/xpedite517x.h
+++ b/include/configs/xpedite517x.h
@@ -347,6 +347,7 @@ extern unsigned long get_board_sys_clk(unsigned long dummy);
 #define CONFIG_SYS_PCIE1_IO_BUS0x
 #define CONFIG_SYS_PCIE1_IO_PHYS   0xe800
 #define CONFIG_SYS_PCIE1_IO_SIZE   0x0080  /* 8M */
+#define CONFIG_SYS_PCIE1_NAME  PEX8518 Switch
 
 /* PCIE2 - VPX P1 */
 #define CONFIG_SYS_PCIE2_MEM_BUS   0xc000
@@ -355,6 +356,7 @@ extern unsigned 

[U-Boot] [PATCH] fsl_pci: Update PCIe boot ouput

2010-12-28 Thread Peter Tyser
This change does the following:
- Adds printing of negotiated link width.  This information can be
  useful when debugging PCIe issues.
- Makes it optional for boards to implement board_serdes_name().
  Previously boards that did not implement it would print unsightly
  output such as PCIE1: Connected to NULL...
- Rewords the PCIe boot output to reduce line length and to make it
  clear that the base address XYZ value refers to the base address of
  the internal processor PCIe registers and not a standard PCI BAR
  value.
- Changes PCIE output to the standard PCIe

Before change:
PCIE1: connected to NULL as Root Complex (base addr ef008000)
  01:00.0 - 10b5:8518 - Bridge device
   02:01.0- 10b5:8518 - Bridge device
   02:02.0- 10b5:8518 - Bridge device
   02:03.0- 10b5:8518 - Bridge device
PCIE1: Bus 00 - 05
PCIE2: connected to NULL as Endpoint (base addr ef009000)
PCIE2: Bus 06 - 06

After change:
PCIe1: Root Complex of PEX8518 Switch, x4, regs @ 0xef008000
  01:00.0 - 10b5:8518 - Bridge device
   02:01.0- 10b5:8518 - Bridge device
   02:02.0- 10b5:8518 - Bridge device
   02:03.0- 10b5:8518 - Bridge device
PCIe1: Bus 00 - 05
PCIe2: Endpoint of VPX Fabric A, x2, regs @ 0xef009000
PCIe2: Bus 06 - 06

Signed-off-by: Peter Tyser pty...@xes-inc.com
---
This applies to Kumar's u-boot-85xx.git 'dev' branch.

 arch/powerpc/include/asm/fsl_pci.h |2 +-
 drivers/pci/fsl_pci_init.c |   38 ++-
 2 files changed, 21 insertions(+), 19 deletions(-)

diff --git a/arch/powerpc/include/asm/fsl_pci.h 
b/arch/powerpc/include/asm/fsl_pci.h
index 15ab50d..0a98bde 100644
--- a/arch/powerpc/include/asm/fsl_pci.h
+++ b/arch/powerpc/include/asm/fsl_pci.h
@@ -27,7 +27,6 @@
 
 int fsl_setup_hose(struct pci_controller *hose, unsigned long addr);
 int fsl_is_pci_agent(struct pci_controller *hose);
-void fsl_pci_init(struct pci_controller *hose, u32 cfg_addr, u32 cfg_data);
 void fsl_pci_config_unlock(struct pci_controller *hose);
 void ft_fsl_pci_setup(void *blob, const char *compat, unsigned long ctrl_addr);
 
@@ -172,6 +171,7 @@ struct fsl_pci_info {
int pci_num;
 };
 
+void fsl_pci_init(struct pci_controller *hose, struct fsl_pci_info *pci_info);
 int fsl_pci_init_port(struct fsl_pci_info *pci_info,
struct pci_controller *hose, int busno);
 int fsl_pcie_init_ctrl(int busno, u32 devdisr, enum srds_prtcl dev,
diff --git a/drivers/pci/fsl_pci_init.c b/drivers/pci/fsl_pci_init.c
index 64c0198..fa23992 100644
--- a/drivers/pci/fsl_pci_init.c
+++ b/drivers/pci/fsl_pci_init.c
@@ -217,8 +217,10 @@ static int fsl_pci_setup_inbound_windows(struct 
pci_controller *hose,
return 1;
 }
 
-void fsl_pci_init(struct pci_controller *hose, u32 cfg_addr, u32 cfg_data)
+void fsl_pci_init(struct pci_controller *hose, struct fsl_pci_info *pci_info)
 {
+   u32 cfg_addr = (u32)((ccsr_fsl_pci_t *)pci_info-regs)-cfg_addr;
+   u32 cfg_data = (u32)((ccsr_fsl_pci_t *)pci_info-regs)-cfg_data;
u16 temp16;
u32 temp32;
int enabled, r, inbound = 0;
@@ -235,10 +237,6 @@ void fsl_pci_init(struct pci_controller *hose, u32 
cfg_addr, u32 cfg_data)
u64 out_hi = 0, out_lo = -1ULL;
u32 pcicsrbar, pcicsrbar_sz;
 
-#ifdef DEBUG
-   int neg_link_w;
-#endif
-
pci_setup_indirect(hose, cfg_addr, cfg_data);
 
/* Handle setup of outbound windows first */
@@ -354,20 +352,20 @@ void fsl_pci_init(struct pci_controller *hose, u32 
cfg_addr, u32 cfg_data)
 #endif
 
if (!enabled) {
-   debug(PCIE link error.  Skipping scan.
- LTSSM=0x%02x\n, ltssm);
+   /* Let the user know there's no PCIe link */
+   printf(no link, regs @ 0x%lx\n, pci_info-regs);
hose-last_busno = hose-first_busno;
return;
}
 
out_be32(pci-pme_msg_det, 0x);
out_be32(pci-pme_msg_int_en, 0x);
-#ifdef DEBUG
+
+   /* Print the negotiated PCIe link width */
pci_hose_read_config_word(hose, dev, PCI_LSR, temp16);
-   neg_link_w = (temp16  0x3f0 )  4;
-   printf(...PCIE LTSSM=0x%x, Negotiated link width=%d\n,
- ltssm, neg_link_w);
-#endif
+   printf(x%d, regs @ 0x%lx\n, (temp16  0x3f0 )  4,
+   pci_info-regs);
+
hose-current_busno++; /* Start scan with secondary */
pciauto_prescan_setup_bridge(hose, dev, hose-current_busno);
}
@@ -476,7 +474,7 @@ int fsl_pci_init_port(struct fsl_pci_info *pci_info,
hose-region_count = r - hose-regions;
hose-first_busno = busno;
 
-   fsl_pci_init(hose, (u32)pci-cfg_addr, (u32)pci-cfg_data);
+   fsl_pci_init(hose, pci_info);
 
if (fsl_is_pci_agent(hose)) {
fsl_pci_config_unlock(hose);
@@ -485,7 +483,7

[U-Boot] [PATCH] Replace FLASH strings with Flash or flash

2010-12-28 Thread Peter Tyser
There's no compelling reason to have the output on bootup or the
flinfo command print flash in uppercase, so use the proper case
where appropriate.

Signed-off-by: Peter Tyser pty...@xes-inc.com
---
 arch/arm/lib/board.c|2 +-
 arch/m68k/lib/board.c   |2 +-
 arch/microblaze/lib/board.c |2 +-
 arch/powerpc/lib/board.c|2 +-
 arch/sh/lib/board.c |2 +-
 arch/sparc/lib/board.c  |2 +-
 board/xes/xpedite517x/xpedite517x.c |2 +-
 board/xes/xpedite520x/xpedite520x.c |2 +-
 board/xes/xpedite537x/xpedite537x.c |2 +-
 board/xes/xpedite550x/xpedite550x.c |2 +-
 drivers/mtd/cfi_flash.c |4 ++--
 11 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/arch/arm/lib/board.c b/arch/arm/lib/board.c
index 96c0e30..c620d2c 100644
--- a/arch/arm/lib/board.c
+++ b/arch/arm/lib/board.c
@@ -481,7 +481,7 @@ void board_init_r (gd_t *id, ulong dest_addr)
mem_malloc_init (malloc_start, TOTAL_MALLOC_LEN);
 
 #if !defined(CONFIG_SYS_NO_FLASH)
-   puts (FLASH: );
+   puts (Flash: );
 
if ((flash_size = flash_init ())  0) {
 # ifdef CONFIG_SYS_FLASH_CHECKSUM
diff --git a/arch/m68k/lib/board.c b/arch/m68k/lib/board.c
index 9a51908..7867ba5 100644
--- a/arch/m68k/lib/board.c
+++ b/arch/m68k/lib/board.c
@@ -460,7 +460,7 @@ void board_init_r (gd_t *id, ulong dest_addr)
malloc_bin_reloc ();
 
 #if !defined(CONFIG_SYS_NO_FLASH)
-   puts (FLASH: );
+   puts (Flash: );
 
if ((flash_size = flash_init ())  0) {
 # ifdef CONFIG_SYS_FLASH_CHECKSUM
diff --git a/arch/microblaze/lib/board.c b/arch/microblaze/lib/board.c
index eeef579..3cd2352 100644
--- a/arch/microblaze/lib/board.c
+++ b/arch/microblaze/lib/board.c
@@ -125,7 +125,7 @@ void board_init (void)
printf (\tU-Boot Start:0x%08x\n, CONFIG_SYS_TEXT_BASE);
 
 #if defined(CONFIG_CMD_FLASH)
-   puts (FLASH: );
+   puts (Flash: );
bd-bi_flashstart = CONFIG_SYS_FLASH_BASE;
if (0  (flash_size = flash_init ())) {
bd-bi_flashsize = flash_size;
diff --git a/arch/powerpc/lib/board.c b/arch/powerpc/lib/board.c
index 9759e23..b88cf6b 100644
--- a/arch/powerpc/lib/board.c
+++ b/arch/powerpc/lib/board.c
@@ -717,7 +717,7 @@ void board_init_r (gd_t *id, ulong dest_addr)
mem_malloc_init (malloc_start, TOTAL_MALLOC_LEN);
 
 #if !defined(CONFIG_SYS_NO_FLASH)
-   puts (FLASH: );
+   puts (Flash: );
 
if (board_flash_wp_on()) {
printf(Uninitialized - Write Protect On\n);
diff --git a/arch/sh/lib/board.c b/arch/sh/lib/board.c
index 3d201b2..ea98b32 100644
--- a/arch/sh/lib/board.c
+++ b/arch/sh/lib/board.c
@@ -46,7 +46,7 @@ unsigned long monitor_flash_len = CONFIG_SYS_MONITOR_LEN;
 static int sh_flash_init(void)
 {
gd-bd-bi_flashsize = flash_init();
-   printf(FLASH: %ldMB\n, gd-bd-bi_flashsize / (1024*1024));
+   printf(Flash: %ldMB\n, gd-bd-bi_flashsize / (1024*1024));
 
return 0;
 }
diff --git a/arch/sparc/lib/board.c b/arch/sparc/lib/board.c
index ab31cfb..386cd04 100644
--- a/arch/sparc/lib/board.c
+++ b/arch/sparc/lib/board.c
@@ -284,7 +284,7 @@ void board_init_f(ulong bootflag)
malloc_bin_reloc();
 
 #if !defined(CONFIG_SYS_NO_FLASH)
-   puts(FLASH: );
+   puts(Flash: );
 
if ((flash_size = flash_init())  0) {
 # ifdef CONFIG_SYS_FLASH_CHECKSUM
diff --git a/board/xes/xpedite517x/xpedite517x.c 
b/board/xes/xpedite517x/xpedite517x.c
index 0f7fa6c..57ef5e3 100644
--- a/board/xes/xpedite517x/xpedite517x.c
+++ b/board/xes/xpedite517x/xpedite517x.c
@@ -47,7 +47,7 @@ static void flash_cs_fixup(void)
 */
flash_sel = !((pca953x_get_val(CONFIG_SYS_I2C_PCA953X_ADDR0) 
CONFIG_SYS_PCA953X_C0_FLASH_PASS_CS));
-   printf(FLASH: Executed from FLASH%d\n, flash_sel ? 2 : 1);
+   printf(Flash: Executed from flash%d\n, flash_sel ? 2 : 1);
 
if (flash_sel) {
set_lbc_br(0, CONFIG_SYS_BR1_PRELIM);
diff --git a/board/xes/xpedite520x/xpedite520x.c 
b/board/xes/xpedite520x/xpedite520x.c
index dc5c965..c79171d 100644
--- a/board/xes/xpedite520x/xpedite520x.c
+++ b/board/xes/xpedite520x/xpedite520x.c
@@ -47,7 +47,7 @@ static void flash_cs_fixup(void)
 */
flash_sel = !((pca953x_get_val(CONFIG_SYS_I2C_PCA953X_ADDR0) 
CONFIG_SYS_PCA953X_FLASH_PASS_CS));
-   printf(FLASH: Executed from FLASH%d\n, flash_sel ? 2 : 1);
+   printf(Flash: Executed from flash%d\n, flash_sel ? 2 : 1);
 
if (flash_sel) {
set_lbc_br(0, CONFIG_SYS_BR1_PRELIM);
diff --git a/board/xes/xpedite537x/xpedite537x.c 
b/board/xes/xpedite537x/xpedite537x.c
index 89fa6c7..d074495 100644
--- a/board/xes/xpedite537x/xpedite537x.c
+++ b/board/xes/xpedite537x/xpedite537x.c
@@ -47,7 +47,7 @@ static void flash_cs_fixup(void)
 */
flash_sel = !((pca953x_get_val(CONFIG_SYS_I2C_PCA953X_ADDR0

Re: [U-Boot] [PATCH 08/15] powerpc/8xxx: Rework XES boards pci_init_board to use common FSL PCIe code

2010-12-21 Thread Peter Tyser
On Tue, 2010-12-21 at 11:49 -0600, Kumar Gala wrote:
 On Dec 20, 2010, at 10:49 AM, Peter Tyser wrote:
 
  Thanks for the cleanup.  What branch should this series be applied to?
  And are there prerequisites?  I'm having issues applying them to test
  and review.

Any direction on how these should be applied for testing?

snip

  --- a/board/xes/common/fsl_8xxx_pci.c
  +++ b/board/xes/common/fsl_8xxx_pci.c
  @@ -34,15 +34,6 @@
  #ifdef CONFIG_PCI1
  static struct pci_controller pci1_hose;
  #endif
  
  Is there a reason PCI1 wasn't changed over too?  I see pci1_hose is
  still referenced below, but other boards with a PCI1 don't use similar
  code.
 
 I was trying to limit how much clean up I did so left this to just PCIe 
 interfaces.  Normal PCI and PCI-X is something I might get around to but one 
 thing at a time

Ah, OK.  If we're removing the LAW entries for PCI1 in law.c below, how
is a LAW being set for PCI1?  It looks like PCIe laws are set in
fsl_configure_pcie(), and PCI LAWs are set via set_next_law() in
board-specific code?  I'm not seeing the call to set_next_law() in X-ES
board specific code after this change though.

Best,
Peter

  snip
  
  diff --git a/board/xes/xpedite520x/law.c b/board/xes/xpedite520x/law.c
  index bbfcb9d..3afb3ae 100644
  --- a/board/xes/xpedite520x/law.c
  +++ b/board/xes/xpedite520x/law.c
  @@ -38,10 +38,6 @@ struct law_entry law_table[] = {
 /* LBC window - maps 256M 0xf000 - 0x */
 SET_LAW(CONFIG_SYS_FLASH_BASE2, LAW_SIZE_256M, LAW_TRGT_IF_LBC),
 SET_LAW(CONFIG_SYS_NAND_BASE, LAW_SIZE_1M, LAW_TRGT_IF_LBC),
  -#if CONFIG_SYS_PCI1_MEM_PHYS
  -  SET_LAW(CONFIG_SYS_PCI1_MEM_PHYS, LAW_SIZE_1G, LAW_TRGT_IF_PCI_1),
  -  SET_LAW(CONFIG_SYS_PCI1_IO_PHYS, LAW_SIZE_8M, LAW_TRGT_IF_PCI_1),
  -#endif
  #if CONFIG_SYS_PCI2_MEM_PHYS
 SET_LAW(CONFIG_SYS_PCI2_MEM_PHYS, LAW_SIZE_256M, LAW_TRGT_IF_PCI_2),
 SET_LAW(CONFIG_SYS_PCI2_IO_PHYS, LAW_SIZE_8M, LAW_TRGT_IF_PCI_2),


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


Re: [U-Boot] [PATCH 08/15] powerpc/8xxx: Rework XES boards pci_init_board to use common FSL PCIe code

2010-12-20 Thread Peter Tyser
Thanks for the cleanup.  What branch should this series be applied to?
And are there prerequisites?  I'm having issues applying them to test
and review.

On Fri, 2010-12-17 at 17:50 -0600, Kumar Gala wrote:
 Remove duplicated code in MPC8572 DS board and utliize the common
 fsl_pcie_init_board().

Looks like a copy/paste from the MPC8572.

On all the patches in the series s/utliize/utilize/.

snip

 --- a/board/xes/common/fsl_8xxx_pci.c
 +++ b/board/xes/common/fsl_8xxx_pci.c
 @@ -34,15 +34,6 @@
  #ifdef CONFIG_PCI1
  static struct pci_controller pci1_hose;
  #endif

Is there a reason PCI1 wasn't changed over too?  I see pci1_hose is
still referenced below, but other boards with a PCI1 don't use similar
code.

 -#ifdef CONFIG_PCIE1
 -static struct pci_controller pcie1_hose;
 -#endif
 -#ifdef CONFIG_PCIE2
 -static struct pci_controller pcie2_hose;
 -#endif
 -#ifdef CONFIG_PCIE3
 -static struct pci_controller pcie3_hose;
 -#endif

snip

 diff --git a/board/xes/xpedite520x/law.c b/board/xes/xpedite520x/law.c
 index bbfcb9d..3afb3ae 100644
 --- a/board/xes/xpedite520x/law.c
 +++ b/board/xes/xpedite520x/law.c
 @@ -38,10 +38,6 @@ struct law_entry law_table[] = {
   /* LBC window - maps 256M 0xf000 - 0x */
   SET_LAW(CONFIG_SYS_FLASH_BASE2, LAW_SIZE_256M, LAW_TRGT_IF_LBC),
   SET_LAW(CONFIG_SYS_NAND_BASE, LAW_SIZE_1M, LAW_TRGT_IF_LBC),
 -#if CONFIG_SYS_PCI1_MEM_PHYS
 - SET_LAW(CONFIG_SYS_PCI1_MEM_PHYS, LAW_SIZE_1G, LAW_TRGT_IF_PCI_1),
 - SET_LAW(CONFIG_SYS_PCI1_IO_PHYS, LAW_SIZE_8M, LAW_TRGT_IF_PCI_1),
 -#endif
  #if CONFIG_SYS_PCI2_MEM_PHYS
   SET_LAW(CONFIG_SYS_PCI2_MEM_PHYS, LAW_SIZE_256M, LAW_TRGT_IF_PCI_2),
   SET_LAW(CONFIG_SYS_PCI2_IO_PHYS, LAW_SIZE_8M, LAW_TRGT_IF_PCI_2),

The PCI2 law can be removed too.  Its not currently used on any boards
supported by mainline U-Boot.

Thanks,
Peter

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


Re: [U-Boot] [PATCHv3] pca953x: support 16-pin devices

2010-12-09 Thread Peter Tyser
On Thu, 2010-12-09 at 22:11 +1300, Chris Packham wrote:
 This adds support for for the PCA9535/PCA9539 family of gpio devices which
 have 16 output pins.
 
 To let the driver know which devices are 16-pin it is necessary to define
 CONFIG_SYS_I2C_PCA953X_WIDTH in your board config file. This is used to
 create an array of {chip, ngpio} tuples that are used to determine the
 width of a particular chip. For backwards compatibility it is assumed that
 any chip not defined in CONFIG_SYS_I2C_PCA953X_WIDTH has 8 pins.
 
 Signed-off-by: Chris Packham chris.pack...@alliedtelesis.co.nz

Looks good to me and works as advertised.

Acked-by: Peter Tyser pty...@xes-inc.com
Tested-by: Peter Tyser pty...@xes-inc.com

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


Re: [U-Boot] [PATCH] Honor /memory/reg node in DTB files

2010-12-08 Thread Peter Tyser
On Wed, 2010-12-08 at 23:34 +0100, Wolfgang Denk wrote:
 Dear Dan,
 
 In message 0ddcbda1-188f-433d-bdcc-5fdcf709a...@digitaldans.com you wrote:
  
   If you want to make this switchable at runtime, then we should
   probably use an environment setting.
  
  I experimented with this, but could never determine the
  best way to cover all behavior.  Do we have a variable that
  indicates don't update DT?  If so, it means we have to
  place all values in the DT, when it's really nice for U-Boot
  to do some of that for us.  In fact, we want U-Boot to update
  many things it discovers, just not the few we wish to actually
  (sometimes) define for ourselves.
 
 You can please all the people some of the time and some of the people
 all of the time but you can't please all the people all of the time.
 
  The other alternative (granted, I'm not as smart as I used
  to be :-)) was to have an environment variable that specified
  which node we didn't want updated by U-Boot.  For now,
  that seems reasonable since there is only one, but is that
  the general approach we want in the future?  It also presents
  the challenge of having to update both environment and
  the provided DT.
 
 I guess we can argue that the normal situation is that U-Boot will
 know how to update the DT such as needed to boot the OS. So what we
 are dealing with is a small percentage of cases where we need special
 behaviour, and where it may be acceptable if the solution is only
 semi-perfect ;-)
 
 My current thinking is to introduce something like
 
   dt_skip=memory,mac-address

I haven't followed the this thread too closely, but I was curious if you
could do manual tweaks by using the 'bootm' and 'fdt' sub-commands.
This could allow more fine grained control of the boot process and let
you manually modify the DTB before booting to Linux.  Eg make the
'bootcmd' environment variable be something like:
bootm start $loadaddr; \
bootm loados; \
bootm ramdisk; \
bootm fdt; \
fdt boardsetup; \
fdt set memory reg 0 1000; \
bootm prep; \
bootm go

Some dual core Freescale board's do somewhat similar operations for AMP
operation (see doc/README.p2020rdb), although I haven't personally tried
AMP.

Best,
Peter

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


Re: [U-Boot] [PATCHv2] pca953x: support 16-pin devices

2010-12-08 Thread Peter Tyser
The patch looks good.  I had a few minor nitpicky style comments below:

 As suggested by Peter I've implemented the 16-pin support in the existing
 pca953x driver. So this is pretty much a re-write of the v1 patch. Is the 
 commit
 message sufficient to document CONFIG_SYS_I2C_PCA953X_WIDTH or is
 there something under doc/ that I should be adding to.

You could add a brief description to the top-level README file.  There
is currently a bit of info in the GPIO Support section that could be
added to.

  #include common.h
 @@ -38,22 +38,78 @@ enum {
   PCA953X_CMD_INVERT,
  };
 
 +#ifdef CONFIG_SYS_I2C_PCA953X_WIDTH
 +struct chip_ngpio {
 + uint8_t chip;
 + uint8_t ngpio;
 +};

Since this structure is 953x-specific I'd rename chip_ngpio
pca953x_chip_ngpio to clarify its a driver-specific structure and to
prevent the (unlikely) name collision down the road.

The same comment applies to ngpio()-pca953x_ngpio(),
chip_ngpios[]-pca953x_chip_ngpios[].

 +static struct chip_ngpio chip_ngpios[] = CONFIG_SYS_I2C_PCA953X_WIDTH;
 +#define NUM_CHIP_GPIOS (sizeof(chip_ngpios) / sizeof(struct chip_ngpio))
 +
 +/*
 + * Determine the number of GPIO pins supported. If we don't know we assume
 + * 8 pins.
 + */
 +static int ngpio(uint8_t chip)
 +{
 + int i;
 +
 + for (i = 0; i  NUM_CHIP_GPIOS; i++) {
 + if (chip_ngpios[i].chip == chip)
 + return chip_ngpios[i].ngpio;
 + }

I'd remove the for loop braces above per the Linux CodingStyle
documentation that U-Boot adheres to.

There should also be an empty line above the return 8 below.

 + return 8;
 +}
 +#else
 +#define ngpio(chip)  8
 +#endif
 +
  /*
   * Modify masked bits in register
   */
  static int pca953x_reg_write(uint8_t chip, uint addr, uint mask, uint data)
  {
 - uint8_t val;
 + uint8_t  valb;
 + uint16_t valw;

I'd remove one of the spaces between valb above.  My understanding is
spaces shouldn't be used for such vertical alignment.

 
 - if (i2c_read(chip, addr, 1, val, 1))
 - return -1;
 + if (ngpio(chip) = 8) {
 + if (i2c_read(chip, addr, 1, valb, 1))
 + return -1;
 +
 + valb = ~mask;
 + valb |= data;
 +
 + return i2c_write(chip, addr, 1, valb, 1);
 + } else {
 + if (i2c_read(chip, addr  1, 1, (u8*)valw, 2))
 + return -1;
 +
 + valw = ~mask;
 + valw |= data;
 +
 + return i2c_write(chip, addr  1, 1, (u8*)valw, 2);
 + }
 +}
 
 - val = ~mask;
 - val |= data;
 +static int pca953x_reg_read(uint8_t chip, uint addr, uint *data)
 +{
 + uint8_t  valb;
 + uint16_t valw;
 
 - return i2c_write(chip, addr, 1, val, 1);
 + if (ngpio(chip) = 8) {
 + if (i2c_read(chip, addr, 1, valb, 1))
 + return -1;
 + *data = (int) valb;

The space in (int) valb should be removed.  Same below.

 + } else {
 + if (i2c_read(chip, addr  1, 1, (u8*)valw, 2))
 + return -1;
 + *data = (int) valw;
 + }
 + return 0;
  }

snip

 + for (i = msb; i = 0; i--)
 + printf(%x,i);
 + printf(\n);
 + if (nr_gpio  8)
 + printf();
   printf(---\n);

What about changing the printing of '-'s to a for loop like
for (i = 19 + nr_gpio; i 0; i++)
puts(-);

I'll give the next iteration of the patch a shot, it looks like it
should work just fine.

Best,
Peter

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


Re: [U-Boot] [PATCH 1/2] Add support for XZ decompression algorithm.

2010-12-06 Thread Peter Tyser
Hi Luigi,

On Sun, 2010-12-05 at 16:16 +0100, Luigi 'Comio' Mantellini wrote:
 XZ (aka LZMA2) is the new version of lzma compression format.
 The following patch add a cut-down version of XZ Embedded library (v20100702)
 that supports only single-call API.
 
 In order to enable XZ support, the CONFIG_XZ must be defined by board
 configuration file.
 
 For any details, please refer to XZ Embedded homesite
 (http://tukaani.org/xz/embedded.html)

If I understand the xz format correctly, its nearly identical to lzma,
and the 'xz' utility can decompress .lzma files too.  Any chance the xz
code you're adding can be used to handle lzma-compressed archives?  If
so, can we deprecate CONFIG_LZMA and have the new CONFIG_XZ support
handle both .xz and .lzma archives?  Or at least share a significant
chunk of code between the two formats?

Best,
Peter

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


Re: [U-Boot] [PATCH] gpio: add driver for PCA9539 16-bit I2C gpio expander

2010-12-06 Thread Peter Tyser
Hi Chris,

On Tue, 2010-12-07 at 10:48 +1300, Chris Packham wrote:
 This adds support for the PCA9539 family of gpio devices which have 16
 output pins. The devices are similar to chips that use the pca953x driver
 except the register map is expanded (in a non-backwards compatible manner)
 to allow for the extra 8 pins.
 
 This driver has one gotcha that you can only set the top or bottom 8 bits
 at a time. For example to set the 16 pins to be outputs you need to make
 the function calls
 
   pca9539_set_dir(PCA9539_ADDR, 0xff00, 0x);
   pca9539_set_dir(PCA9539_ADDR, 0x00ff, 0x);
 
 Signed-off-by: Chris Packham chris.pack...@alliedtelesis.co.nz
 ---
 Hi,
 
 I have a couple of questions related to my patch. Firstly is anyone
 interested in
 support for these gpio devices? Secondly is my gotcha acceptable or should I
 put some effort into making it handle a 16-bit argument in 1 hit?
 
 I toyed with the idea of making this part of the pca953x driver. I
 didn't because
 the register layout is different so I'd still need to define PCA9539 specific
 functions and macros (although pca953x_reg_write could be re-used). I also
 figured that some people may want to define only CONFIG_PCA9539 (that is
 the case for me at the moment but the overhead of CONFIG_PCA953X wouldn't
 push the size of my image over any boundary).

I think it'd be nice to add support for 16-bit pca953x devices.  I don't
personally use them, but others might in the future, or they may choose
to use them since they are well supported in U-Boot and Linux.  If you
already use the pca9539, getting it in mainline U-Boot will be nice for
you in the long run either way.

I think it'd be worth attempting to combine your changes in the existing
pca953x driver though.  They are similar enough that it should be doable
without too much headache, and it'll be more maintainable to have a
unified driver.

Linux combines the 2 for what its worth (see drivers/gpio/pca953x.c).
It uses the number of GPIO pins to conditionally support 8 or 16-bit
expanders.  You could do the same thing to the U-Boot pca953x driver.
Eg at the top you could add:
#ifdef CONFIG_PCA953X_16BIT
#define NGPIO = 16
#else
#define NGPIO = 8
#endif

You could also change the read/write parameters to be uint16's instead
of the current uint8's to support 16-bits of GPIO.  I think with those 2
changes, plus some additional conditionals you could combine the 2
drivers without too much effort.

Best,
Peter

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


Re: [U-Boot] [PATCH] gpio: add driver for PCA9539 16-bit I2C gpio expander

2010-12-06 Thread Peter Tyser
snip

  You could do the same thing to the U-Boot pca953x driver.
  Eg at the top you could add:
  #ifdef CONFIG_PCA953X_16BIT
  #define NGPIO = 16
  #else
  #define NGPIO = 8
  #endif
 
 I have a small problem with this due to the fact that we have some
 designs here that use a pca9534 and a pca9539. Having a compile-time
 conditional like this means you can only support one or the other.
 Linux can handle this because it's actually got a structure that
 contains the 8 vs 16 whereas u-boot only has the address an no other
 information. Any suggestions on handling this would be welcome.

Some ideas:
- Do a series of byte reads and determine the number of GPIOs based on
mirroring of registers, or reads failing if they are past the end of
the device.  eg reading register 4 on a 8-bit device might fail, or wrap
around and return the same data as register 0.

- Make CONFIG_PCA953X_16BIT an array that lists which I2C address
support 16 bits, then use it to determine which devices are 8 vs 16 bits
dynamically.

- Convert the driver to be more intelligent.  eg add an interface like
pca953x_add_dev(u8 addr, u8 bits) which each board calls to add a device
to a list of devices maintained by the pca953x driver.  Eg in my board
code I'd do:
pca953x_add_dev(0x18, 8);
pca953x_add_dev(0x1c, 8);
pca953x_add_dev(0x1e, 8);
pca953x_add_dev(0x1f, 8);

Best,
Peter



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


Re: [U-Boot] [PATCH] gpio: add driver for PCA9539 16-bit I2C gpio expander

2010-12-06 Thread Peter Tyser
Chris Packham wrote:
 On Tue, Dec 7, 2010 at 2:34 PM, Peter Tyser pty...@xes-inc.com wrote:
   
 snip

 
 You could do the same thing to the U-Boot pca953x driver.
 Eg at the top you could add:
 #ifdef CONFIG_PCA953X_16BIT
 #define NGPIO = 16
 #else
 #define NGPIO = 8
 #endif
 
 I have a small problem with this due to the fact that we have some
 designs here that use a pca9534 and a pca9539. Having a compile-time
 conditional like this means you can only support one or the other.
 Linux can handle this because it's actually got a structure that
 contains the 8 vs 16 whereas u-boot only has the address an no other
 information. Any suggestions on handling this would be welcome.
   
 Some ideas:
 - Do a series of byte reads and determine the number of GPIOs based on
 mirroring of registers, or reads failing if they are past the end of
 the device.  eg reading register 4 on a 8-bit device might fail, or wrap
 around and return the same data as register 0.
 

 Hmm, I can see problems ahead for that approach.

   
 - Make CONFIG_PCA953X_16BIT an array that lists which I2C address
 support 16 bits, then use it to determine which devices are 8 vs 16 bits
 dynamically.
 

 I think this is probably the best option. It shouldn't be that hard to
 implement.

   
 - Convert the driver to be more intelligent.  eg add an interface like
 pca953x_add_dev(u8 addr, u8 bits) which each board calls to add a device
 to a list of devices maintained by the pca953x driver.  Eg in my board
 code I'd do:
 pca953x_add_dev(0x18, 8);
 pca953x_add_dev(0x1c, 8);
 pca953x_add_dev(0x1e, 8);
 pca953x_add_dev(0x1f, 8);
 

 I like the idea but I was hoping not to affect other boards, but if
 people are on-board and able to test I could look into this option.

 Another approach I though about was having a global variable which
 would default to 8 but could be modified via an appropriate setter
 prior to making calls to pca953x_set_xyz.
   

I believe the only mainline users of the pca953x driver are my company's 
boards.  I'm more than happy to test any modifications to the driver and 
to tweak our boards as needed, so I'd recommend choosing whichever 
approach is the cleanest in the long run.

Best,
Peter


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


Re: [U-Boot] [PATCH 12/13] mpc85xx boards: initdram() cleanup/bugfix

2010-12-03 Thread Peter Tyser
Hi Becky,

 +/* Common ddr init for non-corenet fsl 85xx platforms */
 +#ifndef CONFIG_FSL_CORENET
 +phys_size_t initdram(int board_type)
 +{
 + phys_size_t dram_size = 0;
 +
 + puts(Initializing\n);

Any chance we can remove the puts() above?  DRAM: is always printed
out directly before initdram is called, so I don't think the
Initializing message adds much benefit and slightly dirties the
output.  For reference:

I2C:   ready
DRAM:  Initializing
DDR: 2 GiB (DDR2, 64-bit, CL=5, ECC on)
FLASH: Executed from FLASH1
FLASH: 256 MiB

vs

I2C:   ready
DRAM:  DDR: 2 GiB (DDR2, 64-bit, CL=5, ECC on)
FLASH: Executed from FLASH1
FLASH: 256 MiB

Otherwise it looked good.  I tested on the xpedite5170 that was a corner
case, and the xpedite5370 (mpc8572-based).

Acked-by: Peter Tyser pty...@xes-inc.com
Tested-by: Peter Tyser pty...@xes-inc.com

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


[U-Boot] [PATCH/next] 74xx_7xx/mpc86xx/ppmc7xx: Fix do_reset() declaration

2010-12-03 Thread Peter Tyser
The following commit:

commit 882b7d726febe65579d6502c271412ecb05821d7
Author: Mike Frysinger vap...@gentoo.org
Date:   Wed Oct 20 03:41:17 2010 -0400

do_reset: unify duplicate prototypes

missed the 74xx_7xx and mpc86xx arches and the ppmc7xx board do_reset()
functions which resulted in build errors such as:
  cpu.c:128: error: conflicting types for 'do_reset'
  include/command.h:102: error: previous declaration of 'do_reset' was here

Signed-off-by: Peter Tyser pty...@xes-inc.com
---
 arch/powerpc/cpu/74xx_7xx/cpu.c |   10 +++---
 arch/powerpc/cpu/mpc86xx/cpu.c  |5 +++--
 board/ppmc7xx/ppmc7xx.c |7 +--
 3 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/arch/powerpc/cpu/74xx_7xx/cpu.c b/arch/powerpc/cpu/74xx_7xx/cpu.c
index ab6f11d..b6a31b4 100644
--- a/arch/powerpc/cpu/74xx_7xx/cpu.c
+++ b/arch/powerpc/cpu/74xx_7xx/cpu.c
@@ -234,8 +234,7 @@ soft_restart(unsigned long addr)
 !defined(CONFIG_ELPPC)\
 !defined(CONFIG_PPMC7XX)
 /* no generic way to do board reset. simply call soft_reset. */
-void
-do_reset (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 {
ulong addr;
/* flush and disable I/D cache */
@@ -263,7 +262,12 @@ do_reset (cmd_tbl_t *cmdtp, int flag, int argc, char * 
const argv[])
addr = CONFIG_SYS_MONITOR_BASE - sizeof (ulong);
 #endif
soft_restart(addr);
-   while(1);   /* not reached */
+
+   /* not reached */
+   while(1)
+   ;
+
+   return 1;
 }
 #endif
 
diff --git a/arch/powerpc/cpu/mpc86xx/cpu.c b/arch/powerpc/cpu/mpc86xx/cpu.c
index 4e90fd2..ffcc8e6 100644
--- a/arch/powerpc/cpu/mpc86xx/cpu.c
+++ b/arch/powerpc/cpu/mpc86xx/cpu.c
@@ -123,8 +123,7 @@ checkcpu(void)
 }
 
 
-void
-do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 {
volatile immap_t *immap = (immap_t *)CONFIG_SYS_IMMR;
volatile ccsr_gur_t *gur = immap-im_gur;
@@ -137,6 +136,8 @@ do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const 
argv[])
 
while (1)
;
+
+   return 1;
 }
 
 
diff --git a/board/ppmc7xx/ppmc7xx.c b/board/ppmc7xx/ppmc7xx.c
index 5e7427f..432d366 100644
--- a/board/ppmc7xx/ppmc7xx.c
+++ b/board/ppmc7xx/ppmc7xx.c
@@ -88,7 +88,7 @@ int misc_init_r( void )
  *
  * Shell command to reset the board.
  */
-void do_reset( cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[] )
+int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 {
printf( Resetting...\n );
 
@@ -100,7 +100,10 @@ void do_reset( cmd_tbl_t *cmdtp, int flag, int argc, char 
* const argv[] )
_start();
 
/* Should never get here */
-   while(1);
+   while(1)
+   ;
+
+   return 1;
 }
 
 int board_eth_init(bd_t *bis)
-- 
1.7.0.4

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


Re: [U-Boot] [PATCH 08/13] SBC8560: #define CONFIG_SYS_LBC_NO_SDRAM_INIT

2010-12-03 Thread Peter Tyser
On Fri, 2010-12-03 at 14:54 -0600, Becky Bruce wrote:
 On Dec 2, 2010, at 8:26 PM, Peter Tyser wrote:
 
  Hi Becky,
  
  +/* Common ddr init for non-corenet fsl 85xx platforms */
  +#ifndef CONFIG_FSL_CORENET
  +phys_size_t initdram(int board_type)
  +{
  +  phys_size_t dram_size = 0;
  +
  +  puts(Initializing\n);
  
  Any chance we can remove the puts() above?  DRAM: is always printed
  out directly before initdram is called, so I don't think the
  Initializing message adds much benefit and slightly dirties the
  output.  For reference:
 
 That's fine with me does anybody object to this?  It's certainly easy to 
 change this now.

I just noticed it now, but I'd also be in favor of getting rid of the
DDR: puts() at the end of initdram().  It doesn't add much value since
the specific DDR type is already printed out in board_add_ram_info().

Best,
Peter

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


Re: [U-Boot] [PATCH] tsec: Revert to setting TBICR_ANEG_ENABLE by default for SGMII

2010-12-02 Thread Peter Tyser
Hi Kumar,

 Signed-off-by: Kumar Gala ga...@kernel.crashing.org

Acked-by: Peter Tyser pty...@xes-inc.com
Tested-by: Peter Tyser pty...@xes-inc.com

snip

 index e0a1fa4..9d87eaf 100644
 --- a/include/configs/xpedite537x.h
 +++ b/include/configs/xpedite537x.h
 @@ -375,6 +375,12 @@ extern unsigned long get_board_ddr_clk(unsigned long 
 dummy);
  #define CONFIG_MII_DEFAULT_TSEC  1   /* Allow unregistered phys */
  #define CONFIG_ETHPRIME  eTSEC2

Would you mind adding a comment along the lines of the following to
xpedite537x.h and xpedite550x.h above the CONFIG_TSEC_TBICR_SETTINGS
define?:
/*
 * In-band SGMII auto-negotiation between TBI and BCM5482S PHY fails, force
 * 1000mbps SGMII link
 */

 +#define CONFIG_TSEC_TBICR_SETTINGS ( \
 + TBICR_PHY_RESET \
 + | TBICR_FULL_DUPLEX \
 + | TBICR_SPEED1_SET \
 + )
 +

Thanks,
Peter


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


[U-Boot] [PATCH] fsl_upm: Add MxMR/MDR synchronization

2010-12-02 Thread Peter Tyser
From: John Schmoller jschmol...@xes-inc.com

According to Freescale reference manuals (eg section 13.4.4.2
Programming the UPMs of the P4080 Reference Manual):

Since the result of any update to the MxMR/MDR register must be in
effect before the dummy read or write to the UPM region, a write to
MxMR/MDR should be followed immediately by a read of MxMR/MDR.

The UPM on a custom P4080-based board did not work without performing
a read of MxMR/MDR after a write.

Signed-off-by: John Schmoller jschmol...@xes-inc.com
Signed-off-by: Peter Tyser pty...@xes-inc.com
---
 drivers/mtd/nand/fsl_upm.c |2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/mtd/nand/fsl_upm.c b/drivers/mtd/nand/fsl_upm.c
index b76c673..be00555 100644
--- a/drivers/mtd/nand/fsl_upm.c
+++ b/drivers/mtd/nand/fsl_upm.c
@@ -21,6 +21,7 @@
 static void fsl_upm_start_pattern(struct fsl_upm *upm, u32 pat_offset)
 {
clrsetbits_be32(upm-mxmr, MxMR_MAD_MSK, MxMR_OP_RUNP | pat_offset);
+   (void)in_be32(upm-mxmr);
 }
 
 static void fsl_upm_end_pattern(struct fsl_upm *upm)
@@ -35,6 +36,7 @@ static void fsl_upm_run_pattern(struct fsl_upm *upm, int 
width,
void __iomem *io_addr, u32 mar)
 {
out_be32(upm-mar, mar);
+   (void)in_be32(upm-mar);
switch (width) {
case 8:
out_8(io_addr, 0x0);
-- 
1.7.0.4

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


Re: [U-Boot] [PATCH] powerpc/8xxx: Fix _POST_WORD_ADDR on 85xx 86xx systems

2010-12-02 Thread Peter Tyser
Hi Kumar,

  #elif defined (CONFIG_MPC85xx)
  #include asm/immap_85xx.h
 -#define _POST_WORD_ADDR  (CONFIG_SYS_IMMR + offsetof(ccsr_pic_t, tfrr))
 +#define _POST_WORD_ADDR  (CONFIG_SYS_IMMR + 
 CONFIG_SYS_MPC85xx_PIC_OFFSET + \
 + offsetof(ccsr_pic_t, tfrr))
  #elif defined (CONFIG_MPC86xx)
  #include asm/immap_86xx.h
 -#define _POST_WORD_ADDR  (CONFIG_SYS_IMMR + offsetof(ccsr_pic_t, tfrr))
 +#define _POST_WORD_ADDR  (CONFIG_SYS_IMMR + 
 CONFIG_SYS_MPC86xx_PIC_OFFSET + \
 + offsetof(ccsr_pic_t, tfrr))
  
  #elif defined (CONFIG_4xx)
  #define _POST_WORD_ADDR \

John Schmoller just submitted the same patch internally, but used
CONFIG_SYS_MPC8xxx_PIC_ADDR instead of (IMMR + OFFSET).  Its a bit
cleaner, and allows combining the 85xx and 86xx case.  common.h should
already include asm/immap8[56]xx.h.

Best,
Peter

diff --git a/include/post.h b/include/post.h
index 957ce3b..15a20c1 100644
--- a/include/post.h
+++ b/include/post.h
@@ -56,14 +56,8 @@
 #include asm/immap_qe.h
 #define _POST_WORD_ADDR(CONFIG_SYS_IMMR + CPM_POST_WORD_ADDR)
 
-#elif defined (CONFIG_MPC85xx)
-#include asm/immap_85xx.h
-#define _POST_WORD_ADDR(CONFIG_SYS_IMMR + offsetof(ccsr_pic_t, tfrr))
-
-#elif defined (CONFIG_MPC86xx)
-#include asm/immap_86xx.h
-#define _POST_WORD_ADDR(CONFIG_SYS_IMMR + offsetof(ccsr_pic_t, tfrr))
-
+#elif defined(CONFIG_MPC85xx) || defined(CONFIG_MPC86xx)
+#define _POST_WORD_ADDR(CONFIG_SYS_MPC8xxx_PIC_ADDR + 
offsetof(ccsr_pic_t, tfrr))
 #elif defined (CONFIG_4xx)
 #define _POST_WORD_ADDR \
(CONFIG_SYS_OCM_DATA_ADDR + CONFIG_SYS_GBL_DATA_OFFSET - 0x4)


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


Re: [U-Boot] [PATCH 6/7] POWERPC: enable --gc-sections and -ffunction-sections -fdata-sections

2010-11-21 Thread Peter Tyser
Hi Wolfgang,

On Sun, 2010-11-21 at 22:03 +0100, Wolfgang Denk wrote:
 The switch from archive libraries to partial linking has introduced a
 number of problems, that are non-trivial to solve.  For example, it is
 no longer possible to include individual object files in the linker
 script as we did before for example in the case of boards with
 embedded environment to fill up the gap caused by the need to align
 the environment on flash erase block boundaries.
 
 The best (but unfortunately not easiest) approach to address this
 problem is to enable -ffunction-sections (and -ffunction-sections) so

's/function-sections/data-sections/' in the 2nd location above.

snip

 --- a/arch/powerpc/config.mk
 +++ b/arch/powerpc/config.mk
 @@ -25,9 +25,9 @@ CROSS_COMPILE ?= ppc_8xx-
  
  STANDALONE_LOAD_ADDR = 0x4
  
 -PLATFORM_RELFLAGS += -mrelocatable
 +PLATFORM_RELFLAGS += -mrelocatable -ffunction-sections -fdata-sections
  PLATFORM_CPPFLAGS += -DCONFIG_PPC -D__powerpc__
 -PLATFORM_LDFLAGS  += -n
 +PLATFORM_LDFLAGS  += -n --gc-sections

The above changes already exist in arch/powerpc/cpu/mpc85xx/config.mk
and arch/powerpc/cpu/mpc86xx/config.mk.  It'd be nice to remove those
references in this patch so they aren't duplicated.

Best,
Peter

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


Re: [U-Boot] [PATCH] revert tsec: Force TBI PHY to 1000Mbps fullduplex in SGMII mode

2010-11-19 Thread Peter Tyser
On Fri, 2010-11-19 at 13:53 +0800, Li Yang wrote:
 snip
 
  My understanding is that the SGMII link is always at 1000Mbps speed -
  see figure 1 from the app note.  Additionally look at figure 3.  My
  understand from it, and the app note's text is that SGMII
  auto-negotiation doesn't really occur - its just passing the PHY-side
  auto-negotiation results to the Freescale MAC, which software then
  configures.  I'm not sure what the purpose of the SGMII
  auto-negotiation is - its not really auto- negotiating and the same
  information can be read from the PHY via its MDIO interface, which is
 what is happening on X-ES boards currently.
 
  I guess the point is to save MDIO signals to the external PHY and read
  the negotiated result from the internal TBI PHY.
 
 Do the P2020DS, MPC8572DS and P1/P2 RDB boards not have an MDIO interface
 to their PHYs'?  I've never heard of a board that doesn't, and would guess
 the tsec driver in U-Boot requires a PHY to be accessible via MDIO to use
 the corresponding network interface.
 
 I'm not sure which specific silicon omits the MDIO signal.  But quoting from
 the app note you mentioned:
 
  Depending on the PowerQUICC part used, some eTSEC’s may or may not
 provide an external
 MDIO management interface. However, the TBI block for each eTSEC is
 associated in a one to one
 manner, even if that eTSEC does not have an external MDIO connection.

I believe some parts don't have an external MDIO interface for every
eTSEC.  if my memory is correct the MPC8572 only has 2 MDIO ports and 3
eTSECS.  But 1 MDIO port can be used to communicate with all 3 eTSECS'
PHYs assuming each PHY has a unique address.  (eg eTSEC1, 2, and 3 use
eTSEC1's MDIO port).

 The auto-negotiation mechanism between TBI PHY and real PHY does make
 it possible
 to save external MDIO signals if only SGMII interface is used.

That seems like an awfully big sacrifice to save routing 2 signals.
I've never heard of anyone doing this.  I don't think the current tsec
driver in U-Boot would support this configuration either, so I'm not
sure how much weight it holds.

 I can forward you the email thread about SGMII auto-negotiation with
 Freescale's FAE off-list if you'd like as well.
 
 Yes, please.  Thanks

I just forwarded it.

Best,
Peter


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


Re: [U-Boot] [PATCH] revert tsec: Force TBI PHY to 1000Mbps fullduplex in SGMII mode

2010-11-18 Thread Peter Tyser
snip

 My understanding is that the SGMII link is always at 1000Mbps speed - see
 figure 1 from the app note.  Additionally look at figure 3.  My understand
 from it, and the app note's text is that SGMII auto-negotiation doesn't
 really occur - its just passing the PHY-side auto-negotiation results to
 the Freescale MAC, which software then configures.  I'm not sure what the
 purpose of the SGMII auto-negotiation is - its not really auto-
 negotiating and the same information can be read from the PHY via its MDIO
 interface, which is what is happening on X-ES boards currently.
 
 I guess the point is to save MDIO signals to the external PHY and read the
 negotiated result from the internal TBI PHY.

Do the P2020DS, MPC8572DS and P1/P2 RDB boards not have an MDIO
interface to their PHYs'?  I've never heard of a board that doesn't, and
would guess the tsec driver in U-Boot requires a PHY to be accessible
via MDIO to use the corresponding network interface.

I can forward you the email thread about SGMII auto-negotiation with
Freescale's FAE off-list if you'd like as well.

Best,
Peter

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


Re: [U-Boot] [PATCH] revert tsec: Force TBI PHY to 1000Mbps full duplex in SGMII mode

2010-11-17 Thread Peter Tyser
Hi Zhao,

 In message 1289986984-2314-1-git-send-email-b26...@freescale.com you wrote:
  On P2020DS and MPC8572DS, the link to SGMII card which use Vitesse
  VSC8234 PHY can't come up. Current TBI PHY settings(TBICR_SETTINGS)
  for SGMII mode cause link problems.
  
  Revert commit 46e91674fb4b6d06c6a4984c0b5ac7d9a16923f4, and fix it.

Based on my company's discussions with Freescale and Freescale's
appnotes I believe that the current implementation is correct.  I make
reference to some of this in the original patch:
http://old.nabble.com/-U-Boot---PATCH--tsec:-Force-TBI-PHY-to-1000Mbps-full-duplex-in-SGMII-mode-td26188785.html

We use Broadcom PHYs on our boards, which likely has something to do
with the discrepancies between the P2020DS/MPC8572DS vs the
XPedite5370/XPedite5500.  Unless you have additional information that
shows that in-band SGMII auto-negotiation does work per the spec I'd
prefer to keep the current tsec.c code and add
CONFIG_TSEC_TBICR_SETTINGS workarounds to the P2020DS (already done in
commit 90b5bf211b85eee10c34cbeb907ce381142b7c99?) and MPC8572DS.

Best,
Peter

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


Re: [U-Boot] [PATCH] revert tsec: Force TBI PHY to 1000Mbps full duplex in SGMII mode

2010-11-17 Thread Peter Tyser
On Wed, 2010-11-17 at 21:13 -0700, Li Yang-R58472 wrote:
 Subject: Re: [U-Boot] [PATCH] revert tsec: Force TBI PHY to 1000Mbps full
 duplex in SGMII mode
 
 Hi Zhao,
 
  In message 1289986984-2314-1-git-send-email-b26...@freescale.com you
 wrote:
   On P2020DS and MPC8572DS, the link to SGMII card which use Vitesse
   VSC8234 PHY can't come up. Current TBI PHY settings(TBICR_SETTINGS)
   for SGMII mode cause link problems.
  
   Revert commit 46e91674fb4b6d06c6a4984c0b5ac7d9a16923f4, and fix it.
 
 Based on my company's discussions with Freescale and Freescale's appnotes
 I believe that the current implementation is correct.  I make reference to
 some of this in the original patch:
 http://old.nabble.com/-U-Boot---PATCH--tsec:-Force-TBI-PHY-to-1000Mbps-
 full-duplex-in-SGMII-mode-td26188785.html
 
 We use Broadcom PHYs on our boards, which likely has something to do with
 the discrepancies between the P2020DS/MPC8572DS vs the
 XPedite5370/XPedite5500.  Unless you have additional information that
 shows that in-band SGMII auto-negotiation does work per the spec I'd
 prefer to keep the current tsec.c code and add CONFIG_TSEC_TBICR_SETTINGS
 workarounds to the P2020DS (already done in commit
 90b5bf211b85eee10c34cbeb907ce381142b7c99?) and MPC8572DS.
 
 
 Hi Peter,
 
 From the App Note you mentioned, I didn't find that the auto-negotiation 
 can't be used.

My understanding is that the SGMII link is always at 1000Mbps speed -
see figure 1 from the app note.  Additionally look at figure 3.  My
understand from it, and the app note's text is that SGMII
auto-negotiation doesn't really occur - its just passing the PHY-side
auto-negotiation results to the Freescale MAC, which software then
configures.  I'm not sure what the purpose of the SGMII
auto-negotiation is - its not really auto-negotiating and the same
information can be read from the PHY via its MDIO interface, which is
what is happening on X-ES boards currently.

We were told by a Freescale FAE that SGMII auto-negotiation wasn't
supported, although 1000 BASE-X auto-negotation was (which mirrored our
test results).  I can dig up the old emails tomorrow at the office with
the details.

 Actually we need the auto-negotiation enabled for almost all Freescale 
 reference boards such as P2020DS, MPC8572DS and P1/P2 RDB boards.  If we 
 disable the auto-negotiation on these boards, the SGMII link won't work.  So 
 I guess it might be more common to use auto-negotiation, and a fixed 1000M 
 link is more like a special case.  I'm not sure what's the recommended way 
 for SGMII PHY interconnect though.

And auto-negotation doesn't work on X-ES hardware which all use Broadcom
PHYs - which is why I made the original change after consulting a
Freescale FAE.  Its not a huge deal to me either way as long as the
XPedite5370 and XPedite5500 continue to not use SGMII auto-negotiation.
Can someone at Freescale provide a definitive answer about what the
proper SGMII auto-negotiation scheme is?  And has anyone used it
successfully or unsuccessfully with a non-Vitesse PHY?

Best,
Peter

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


Re: [U-Boot] [PATCH] fsl_pci_init: Make fsl_pci_init_port() PCI/PCIe aware

2010-11-14 Thread Peter Tyser
On Sat, 2010-11-13 at 23:53 +0100, Wolfgang Denk wrote:
 Dear Kumar,
 
 In message 1288297499-21417-1-git-send-email-pty...@xes-inc.com Peter Tyser 
 wrote:
  Previously fsl_pci_init_port() always assumed that a port was a PCIe
  port and would incorrectly print messages for a PCI port such as the
  following on bootup:
  PCI1:  32 bit, 33 MHz, sync, host, arbiter
  Scanning PCI bus 00
  PCIE1 on bus 00 - 00
  
  This change corrects the output of fsl_pci_init_port():
  PCI1:  32 bit, 33 MHz, sync, host, arbiter
  Scanning PCI bus 00
  PCI1 on bus 00 - 00
  
  Signed-off-by: Peter Tyser pty...@xes-inc.com
  ---
  Tested on a MPC8548 with PCI, and a MPC8640 with PCIe
  
   drivers/pci/fsl_pci_init.c |6 +-
   1 files changed, 5 insertions(+), 1 deletions(-)
 
 I thought this should go through your tree, as it affects all the FSL
 boards.  Or are you waiting for me to pull this in?
 
 
 Also, there is still this other unresolved issue about indentation of
 the PCI messages.  Would you agree to clean this up as I suggested?

I submitted a 6-patch series starting with fsl: Clean up printing of
PCI boot info a while back which should address the indentation issue.
Ideally the fist 3 patches in that series would make it into the
upcoming release.  The last 3 are RFCs and could go in this release, or
the next.

Best,
Peter



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


Re: [U-Boot] [PATCH] Switch from library archives to partial linking

2010-11-07 Thread Peter Tyser
On Sat, 2010-11-06 at 18:21 +0100, Albert ARIBAUD wrote:
 Le 06/11/2010 15:28, Sebastien Carlier a écrit :
  Hello all,
 
  My previous patch missed some places that create library archives.  A
  new 100% $(AR)-free version is available here:
 
http://io.oiioiio.com/~sebc/0001-Use-partial-linking-v2.patch
 
  I have tested this patch with MAKEALL -A arm and checked that it does
  not break any build on this architecture.  I have not yet tested the
  resulting binaries, but in theory they should work.  :-)  Feedback on
  other architectures is also welcome.
 
  For a few boards (balloon3, palmld, palmtc, pleb2, zipitz2) that disable
  CONFIG_CMD_NET, this patch also disables CONFIG_CMD_NFS to prevent
  net/nfs.o from being compiled and causing undefined symbols.
 
 I guess if this patch is ready to be pulled in a git repo, you should 
 submit it using git format-patch / git send-email, ideally as a patchset 
 with each patch dealing with one lib, because clearly each ex-library 
 will require its own set of custodian ack(s), thus require its own patch 
 in the set.

You shouldn't need to send the patch using git send-email.  The patch
is greater than U-Boot's mailing list limit (100k) and posting the patch
on a website is perfectly acceptable.  Also, it shouldn't be necessary
to split the patch into each separate patch's to address each lib.  It'd
be a lot of work on Sebastien's part to do this and not break bisection,
and most maintainers can either ack this patch, or probably don't need
to since its more of a build change, not low-level change that a
maintainer has insight into.

I had a couple of comments though:
- You need to add your Signed-of-by:  line to the patch.
- A patch description illustrating why this approach is better than the
current approach would be appreciated.
- You shouldn't be making changes to stuff like CONFIG_CMD_NFS in this
patch.  Its unrelated, and should be dealt with in another patch.  eg
your patches could be:
1/2: Fix boards with CONFIG_CMD_NFS but !CONFIG_CMD_NET
2/2: Switch from library archives to partial linking

I just tried compiling for PowerPC and ran into this:
Configuring for cmi_mpc5xx board...
net/libnet.o: In function `rpc_req':
/home/ptyser/u-boot/u-boot/net/nfs.c:193: undefined reference to `NetEthHdrSize'
/home/ptyser/u-boot/u-boot/net/nfs.c:202: undefined reference to 
`NetSendUDPPacket'
net/libnet.o: In function `NfsStart':
/home/ptyser/u-boot/u-boot/net/nfs.c:741: undefined reference to `NetSetTimeout'
/home/ptyser/u-boot/u-boot/net/nfs.c:742: undefined reference to `NetSetHandler'
net/libnet.o: In function `NfsHandler':
/home/ptyser/u-boot/u-boot/net/nfs.c:656: undefined reference to `NetSetTimeout'
net/libnet.o: In function `NfsTimeout':
/home/ptyser/u-boot/u-boot/net/nfs.c:574: undefined reference to `NetStartAgain'
/home/ptyser/u-boot/u-boot/net/nfs.c:577: undefined reference to `NetSetTimeout'
net/libnet.o:(.got2+0x8): undefined reference to `NetTxPacket'
net/libnet.o:(.got2+0xc): undefined reference to `NetServerEther'
net/libnet.o:(.got2+0x18): undefined reference to `NetServerIP'
net/libnet.o:(.got2+0x1c): undefined reference to `BootFile'
net/libnet.o:(.got2+0x20): undefined reference to `NetOurIP'
net/libnet.o:(.got2+0x30): undefined reference to `NetOurGatewayIP'
net/libnet.o:(.got2+0x34): undefined reference to `NetOurSubnetMask'
net/libnet.o:(.got2+0x40): undefined reference to `NetBootFileSize'
net/libnet.o:(.got2+0x64): undefined reference to `NetState'
net/libnet.o:(.got2+0x7c): undefined reference to `NetBootFileXferSize'
make: *** [u-boot] Error 1
powerpc-linux-size: './u-boot': No such file

I'm guessing lots of boards will have this same issue.  I imagine its
due to include/config_cmd_defaults.h, so maybe if you fix the issue in
that one place all the compile issues will go away.

Best,
Peter

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


Re: [U-Boot] Standalone API

2010-11-07 Thread Peter Tyser
On Sun, 2010-11-07 at 20:44 +, Andrew Holt (SE) wrote:
 Hi,
 
 I hadn't, but I will, Thanks.
 
 I did find include/exports.h
 
 Another question would be, I guess exports.h holds a subset of routines that 
 'could' be called by a standalone.  Is there a list of potential candidates ?
 
 Can you give me any guidance with regard to implementing the U-Boot API on 
 SuperH ?
 
 What are the licences implications of a standalone calling these things ?
 
 Thanks for your help, it's good to know there is somebody out there :)

Its worth mentioning there are 2 external application interfaces, the
older Standalone Application described in doc/README.standalone, and
newer Standalone API described in api/README.  I don't use the
interfaces for non-GPL code or with the SuperH, so can't provide much
input on the above questions.

Best,
Peter

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


Re: [U-Boot] [PATCH] Switch from library archives to partial linking

2010-11-07 Thread Peter Tyser
Hi Wolfgang,

 In message 4cd56618.4010...@gmail.com you wrote:
 
  My previous patch missed some places that create library archives.  A 
  new 100% $(AR)-free version is available here:
 
 Please post your patch on the mailing list for review.

For what its worth, Sebastien's patch is significantly larger than the
100K mailing list limit and I assume he was following the instructions
at http://www.denx.de/wiki/U-Boot/Patches.

He's been told conflicting stories about how to submit his patch - I
just wanted to clarify for him (and me) what the current preferred
method really is.

Thanks,
Peter

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


Re: [U-Boot] patch for gc-sections

2010-11-04 Thread Peter Tyser
On Thu, 2010-11-04 at 10:19 -0400, Haiying Wang wrote:
 On Wed, 2010-11-03 at 13:38 -0700, Peter Tyser wrote:
  I'd guess none of the functions in the SPL binary are referenced in
  the
  linker script or linker command line, so the linker thinks none of
  them
  are necessary and removes them.
  
  Can you try the following patch:
  I did a quick compile test, and it seemed to work, as well as stripped
  out a few unused functions.
 
 Thanks for your patch, it did work to generate the binary, however,
 there are two problems: 
 1. The u-boot size for nand_spl is not cut down as expected.
 /* before apply your new patch */
  29292 2010-11-04 09:29 nand_spl/u-boot-spl
  0 2010-11-04 09:29 nand_spl/u-boot-spl-16k.bin
  0 2010-11-04 09:29 nand_spl/u-boot-spl.bin
  13931 2010-11-04 09:29 nand_spl/u-boot-spl.map
 
 /* After apply your new patch */
 35636 2010-11-04 09:49 nand_spl/u-boot-spl
 116912128 2010-11-04 09:49 nand_spl/u-boot-spl-16k.bin
  4096 2010-11-04 09:49 nand_spl/u-boot-spl.bin
 16381 2010-11-04 09:49 nand_spl/u-boot-spl.map
 
 /* Remove your two patches(remove gc-section patch and this new patch */
 34094 2010-11-04 09:51 nand_spl/u-boot-spl
 116912128 2010-11-04 09:51 nand_spl/u-boot-spl-16k.bin
  4096 2010-11-04 09:51 nand_spl/u-boot-spl.bin
 14097 2010-11-04 09:51 nand_spl/u-boot-spl.map

Can you explain what you mean?  The binary needs to be 4K, right?  So it
can't be trimmed down.  But there should be more available space in that
4K region, eg (all tests on MPC8536DS_NAND_config):

/* After apply my patch sent yesterday */
pty...@petert u-boot $ size after/u-boot-spl
   textdata bss dec hex filename
   3440 460   03900 f3c after/u-boot-spl

/* Remove my two patches(remove gc-section patch and this new patch) */
pty...@petert u-boot $ size before/u-boot-spl
   textdata bss dec hex filename
   3620 460   04080 ff0 before/u-boot-spl

The above says 180 bytes were removed between
fbe53f59bd40b3b1ab66dc98859e26589d64d1b7 and the current HEAD.  I'm
assuming some of that savings is due to -gc-sections (I think 2-3
functions were stripped out via -gc-sections).

You can just look at filesizes of ELF and map files to determine
savings, you'll have to use size/objdump/readelf to determine how the
code size is really affected.

 2.the u-boot-spl.bin is 4096 bytes, and u-boot-spl-16.bin which should
 be padded to 4K bytes is 116912128 bytes. You can take a look at
 nand_spl/board/freescale/mpc8536ds/Makefile to see how
 u-boot-spl.bin/u-boot-spl-16k.bin is generated. I don't know which
 patch(not your gc-section for sure)  cause this problem. I just reset my
 git tree to 2010.06, the size is:
  33512 2010-11-04 10:07 nand_spl/u-boot-spl
  4096 2010-11-04 10:07 nand_spl/u-boot-spl-16k.bin
  4096 2010-11-04 10:07 nand_spl/u-boot-spl.bin
  14037 2010-11-04 10:07 nand_spl/u-boot-spl.map

I batted an eye when I saw the 112M file to, but it looks like that is
unrelated to my change:

/* Remove my two patches(remove gc-section patch and this new patch) */
pty...@petert u-boot $ ls -lh after/
-rwxr-xr-x 1 ptyser users  36K 2010-11-04 10:25 u-boot-spl
-rwxr-xr-x 1 ptyser users 112M 2010-11-04 10:25 u-boot-spl-16k.bin
-rwxr-xr-x 1 ptyser users 4.0K 2010-11-04 10:25 u-boot-spl.bin
-rw-r--r-- 1 ptyser users  16K 2010-11-04 10:25 u-boot-spl.map

/* Remove my two patches(remove gc-section patch and this new patch) */
pty...@petert u-boot $ ls -lh before/
-rwxr-xr-x 1 ptyser users  35K 2010-11-04 10:26 u-boot-spl
-rwxr-xr-x 1 ptyser users 112M 2010-11-04 10:26 u-boot-spl-16k.bin
-rwxr-xr-x 1 ptyser users 4.0K 2010-11-04 10:26 u-boot-spl.bin
-rw-r--r-- 1 ptyser users  13K 2010-11-04 10:26 u-boot-spl.map

The PAD_TO address is set to 0xfff01000, and each of
include/configs/MPC8536DS.h, include/configs/MPC8569MDS.h,
include/configs/P1_P2_RDB.h set CONFIG_SYS_TEXT_BASE to 0xf8f82000.

0xff01000 - 0xff82000 = 0x6F7F000 = 116912128 (as seen above).

Are these boards mis-configured?  In any case, everything seems to be
working as expected from what I can tell after the example patch I
sent yesterday.  Let me know if I'm missing something.

Best,
Peter

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


Re: [U-Boot] patch for gc-sections

2010-11-04 Thread Peter Tyser
 Yes, you are right, the size is down. I only noticed the u-boot-spl size
 by using ls -l, not via size. Now I get the same result as yours. 
 
 It is very good to see this trim-down size because I know some board
 developers are fighting with the 4k limitation of the nand-spl size. Are
 you going to submit your new patch to upstream?

Glad to hear.  I'll submit an official patch shortly.  Just to make
sure, have you tried running one of the nand-spl images after the patch
I sent yesterday?  It'd be good to get confirmation that the
-gc-sections doesn't have any accidental side effects as I wasn't able
to test it.

 Btw, the filesize I see here is smaller than yours, I guess you are
 using the most updated gcc version. I am using gcc-4.3.2.

I was using gcc 4.2.2 in my tests for what its worth.

  I batted an eye when I saw the 112M file to, but it looks like that is
  unrelated to my change:
 It is not related to your change, I see it happens on 2010.09 which
 doesn't have your change. Something between 2010.06 and 2010.09 makes
 this happen.

One more bug to track down:)

Best,
Peter

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


Re: [U-Boot] patch for gc-sections

2010-11-03 Thread Peter Tyser
On Wed, 2010-11-03 at 15:07 -0400, Haiying Wang wrote:
 Peter,
 
 Do you have any idea on why your commit:
 
 commit fbe53f59bd40b3b1ab66dc98859e26589d64d1b7
 Author: Peter Tyser pty...@xes-inc.com
 Date:   Wed Sep 29 14:05:56 2010 -0500
 
 85xx: Use gc-sections to reduce image size

I'd guess none of the functions in the SPL binary are referenced in the
linker script or linker command line, so the linker thinks none of them
are necessary and removes them.

Can you try the following patch:
diff --git a/arch/powerpc/cpu/mpc85xx/u-boot-nand_spl.lds 
b/arch/powerpc/cpu/mpc85xx/u-boot-nand_spl.lds
index 7d9cee9..53d33ee 100644
--- a/arch/powerpc/cpu/mpc85xx/u-boot-nand_spl.lds
+++ b/arch/powerpc/cpu/mpc85xx/u-boot-nand_spl.lds
@@ -54,7 +54,7 @@ SECTIONS
__init_end = .;
 
.resetvec ADDR(.text) + 0xffc : {
-   *(.resetvec)
+   KEEP(*(.resetvec))
} = 0x
 
__bss_start = .;

I did a quick compile test, and it seemed to work, as well as stripped
out a few unused functions.

Best,
Peter

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


[U-Boot] [PATCH] MAKEALL: Do a sanity check on user-supplied arguments

2010-10-29 Thread Peter Tyser
Add a check to make sure that the user's arguments actually find a board
in boards.cfg.  Previously, if a user misspelled an argument the
argument would be discarded without warning.  For example, running
'MAKEALL -c 85xx' with the intention of compiling all Freescale 85xx
boards would instead silently discard the '-c 85xx' argument since the
proper cpu name is 'mpc85xx' and then proceed to compile all PowerPC
boards (MAKEALL's default).

Also fix an unrelated typo.

Signed-off-by: Peter Tyser pty...@xes-inc.com
---
 MAKEALL |8 +++-
 1 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/MAKEALL b/MAKEALL
index c54c6e8..767d561 100755
--- a/MAKEALL
+++ b/MAKEALL
@@ -11,7 +11,7 @@
 # line; without any arguments, MAKEALL defaults to building all Power
 # Architecture systems (i. e. same as for MAKEALL powerpc).
 #
-# With the iontroduction of the board.cfg file, it has become possible
+# With the introduction of the board.cfg file, it has become possible
 # to provide additional selections.  We use standard command line
 # options for this:
 #
@@ -125,6 +125,12 @@ FILTER=\$1 !~ /^#/
 
 if [ $SELECTED ] ; then
SELECTED=$(awk '('$FILTER') { print $1 }' boards.cfg)
+
+   # Make sure some boards from boards.cfg are actually found
+   if [ -z $SELECTED ] ; then
+   echo Error: No boards selected, invalid arguments
+   exit 1
+   fi
 fi
 
 #
-- 
1.7.0.4

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


[U-Boot] [PATCH 2/6] mpc85xx: Fix SERDES/eTSEC message indentation

2010-10-29 Thread Peter Tyser
Previously some mpc85xx boards printed indented messages such as the
following on bootup:
  printf(eTSEC4 is in sgmii mode.\n);
  printf(Serdes2 disalbed\n);

The bootup appearance looks cleaner if the indentation is removed which
aligns these messages with other bootup output.

Signed-off-by: Peter Tyser pty...@xes-inc.com
CC: ga...@kernel.crashing.org
---
 board/atum8548/atum8548.c |8 
 board/freescale/mpc8536ds/mpc8536ds.c |8 
 board/freescale/mpc8544ds/mpc8544ds.c |4 ++--
 board/freescale/mpc8572ds/mpc8572ds.c |8 
 board/freescale/p1_p2_rdb/pci.c   |2 +-
 board/freescale/p2020ds/p2020ds.c |4 ++--
 6 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/board/atum8548/atum8548.c b/board/atum8548/atum8548.c
index 4fcbb18..96581aa 100644
--- a/board/atum8548/atum8548.c
+++ b/board/atum8548/atum8548.c
@@ -193,13 +193,13 @@ void pci_init_board(void)
 
if (io_sel  1) {
if (!(gur-pordevsr  MPC85xx_PORDEVSR_SGMII1_DIS))
-   printf (eTSEC1 is in sgmii mode.\n);
+   printf(eTSEC1 is in sgmii mode.\n);
if (!(gur-pordevsr  MPC85xx_PORDEVSR_SGMII2_DIS))
-   printf (eTSEC2 is in sgmii mode.\n);
+   printf(eTSEC2 is in sgmii mode.\n);
if (!(gur-pordevsr  MPC85xx_PORDEVSR_SGMII3_DIS))
-   printf (eTSEC3 is in sgmii mode.\n);
+   printf(eTSEC3 is in sgmii mode.\n);
if (!(gur-pordevsr  MPC85xx_PORDEVSR_SGMII4_DIS))
-   printf (eTSEC4 is in sgmii mode.\n);
+   printf(eTSEC4 is in sgmii mode.\n);
}
 
 #ifdef CONFIG_PCIE1
diff --git a/board/freescale/mpc8536ds/mpc8536ds.c 
b/board/freescale/mpc8536ds/mpc8536ds.c
index 8ad7549..cf92ba1 100644
--- a/board/freescale/mpc8536ds/mpc8536ds.c
+++ b/board/freescale/mpc8536ds/mpc8536ds.c
@@ -211,12 +211,12 @@ void pci_init_board(void)
devdisr, sdrs2_io_sel, io_sel);
 
if (sdrs2_io_sel == 7)
-   printf(Serdes2 disalbed\n);
+   printf(Serdes2 disalbed\n);
else if (sdrs2_io_sel == 4) {
-   printf(eTSEC1 is in sgmii mode.\n);
-   printf(eTSEC3 is in sgmii mode.\n);
+   printf(eTSEC1 is in sgmii mode.\n);
+   printf(eTSEC3 is in sgmii mode.\n);
} else if (sdrs2_io_sel == 6)
-   printf(eTSEC1 is in sgmii mode.\n);
+   printf(eTSEC1 is in sgmii mode.\n);
 
puts(\n);
 #ifdef CONFIG_PCIE3
diff --git a/board/freescale/mpc8544ds/mpc8544ds.c 
b/board/freescale/mpc8544ds/mpc8544ds.c
index 3bbf0c2..fae1341 100644
--- a/board/freescale/mpc8544ds/mpc8544ds.c
+++ b/board/freescale/mpc8544ds/mpc8544ds.c
@@ -120,9 +120,9 @@ void pci_init_board(void)
 
if (io_sel  1) {
if (!(gur-pordevsr  MPC85xx_PORDEVSR_SGMII1_DIS))
-   printf (eTSEC1 is in sgmii mode.\n);
+   printf(eTSEC1 is in sgmii mode.\n);
if (!(gur-pordevsr  MPC85xx_PORDEVSR_SGMII3_DIS))
-   printf (eTSEC3 is in sgmii mode.\n);
+   printf(eTSEC3 is in sgmii mode.\n);
}
puts(\n);
 
diff --git a/board/freescale/mpc8572ds/mpc8572ds.c 
b/board/freescale/mpc8572ds/mpc8572ds.c
index 2125274..b712e24 100644
--- a/board/freescale/mpc8572ds/mpc8572ds.c
+++ b/board/freescale/mpc8572ds/mpc8572ds.c
@@ -177,13 +177,13 @@ void pci_init_board(void)
debug (   pci_init_board: devdisr=%x, io_sel=%x\n, devdisr, io_sel);
 
if (!(pordevsr  MPC85xx_PORDEVSR_SGMII1_DIS))
-   printf (eTSEC1 is in sgmii mode.\n);
+   printf(eTSEC1 is in sgmii mode.\n);
if (!(pordevsr  MPC85xx_PORDEVSR_SGMII2_DIS))
-   printf (eTSEC2 is in sgmii mode.\n);
+   printf(eTSEC2 is in sgmii mode.\n);
if (!(pordevsr  MPC85xx_PORDEVSR_SGMII3_DIS))
-   printf (eTSEC3 is in sgmii mode.\n);
+   printf(eTSEC3 is in sgmii mode.\n);
if (!(pordevsr  MPC85xx_PORDEVSR_SGMII4_DIS))
-   printf (eTSEC4 is in sgmii mode.\n);
+   printf(eTSEC4 is in sgmii mode.\n);
 
puts(\n);
 #ifdef CONFIG_PCIE3
diff --git a/board/freescale/p1_p2_rdb/pci.c b/board/freescale/p1_p2_rdb/pci.c
index e2ed29c..5fd414e 100644
--- a/board/freescale/p1_p2_rdb/pci.c
+++ b/board/freescale/p1_p2_rdb/pci.c
@@ -56,7 +56,7 @@ void pci_init_board(void)
debug (   pci_init_board: devdisr=%x, io_sel=%x\n, devdisr, io_sel);
 
if (!(pordevsr  MPC85xx_PORDEVSR_SGMII2_DIS))
-   printf (eTSEC2 is in sgmii mode.\n);
+   printf(eTSEC2 is in sgmii mode.\n);
 
puts(\n);
 #ifdef CONFIG_PCIE2
diff --git a/board/freescale/p2020ds/p2020ds.c 
b/board/freescale/p2020ds/p2020ds.c
index f988272..b507677 100644

[U-Boot] [PATCH 3/6] fsl_pci_init: Quiet scanning printf()

2010-10-29 Thread Peter Tyser
The Scanning PCI bus X message doesn't provide any real useful
information, so remove it.

Original output:
  PCIE1: connected as Root Complex
 Scanning PCI bus 01
  04  01  8086  1010  0200  00
  04  01  8086  1010  0200  00
  03  00  10b5  8112  0604  00
  02  01  10b5  8518  0604  00
  02  02  10b5  8518  0604  00
  08  00  1957  0040  0b20  00
  07  00  10b5  8518  0604  00
  09  00  10b5  8112  0604  00
  07  01  10b5  8518  0604  00
  07  02  10b5  8518  0604  00
  06  00  10b5  8518  0604  00
  02  03  10b5  8518  0604  00
  01  00  10b5  8518  0604  00
  PCIE1: Bus 00 - 0b
  PCIE2: connected as Root Complex
 Scanning PCI bus 0d
  0d  00  1957  0040  0b20  00
  PCIE2: Bus 0c - 0d

Updated output:
  PCIE1: connected as Root Complex
  04  01  8086  1010  0200  00
  04  01  8086  1010  0200  00
  03  00  10b5  8112  0604  00
  02  01  10b5  8518  0604  00
  02  02  10b5  8518  0604  00
  08  00  1957  0040  0b20  00
  07  00  10b5  8518  0604  00
  09  00  10b5  8112  0604  00
  07  01  10b5  8518  0604  00
  07  02  10b5  8518  0604  00
  06  00  10b5  8518  0604  00
  02  03  10b5  8518  0604  00
  01  00  10b5  8518  0604  00
  PCIE1: Bus 00 - 0b
  PCIE2: connected as Root Complex
  0d  00  1957  0040  0b20  00
  PCIE2: Bus 0c - 0d

Signed-off-by: Peter Tyser pty...@xes-inc.com
CC: ga...@kernel.crashing.org
---
 drivers/pci/fsl_pci_init.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/pci/fsl_pci_init.c b/drivers/pci/fsl_pci_init.c
index 21cbb3b..5b34dcb 100644
--- a/drivers/pci/fsl_pci_init.c
+++ b/drivers/pci/fsl_pci_init.c
@@ -391,7 +391,7 @@ void fsl_pci_init(struct pci_controller *hose, u32 
cfg_addr, u32 cfg_data)
 * 1 == pci agent or pcie end-point
 */
if (!temp8) {
-   printf(   Scanning PCI bus %02x\n,
+   debug(   Scanning PCI bus %02x\n,
hose-current_busno);
hose-last_busno = pci_hose_scan_bus(hose, hose-current_busno);
} else {
-- 
1.7.0.4

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


[U-Boot] [PATCH/RFC 4/6] pci: Clean up PCI info when CONFIG_PCI_SCAN_SHOW

2010-10-29 Thread Peter Tyser
This change does the following:
- Removes the printing of the PCI interrupt line value.  This is
  normally set to 0 by U-Boot on bootup and is rarely used during
  everyday operation.

- Prints out the PCI function number of a device.  Previously a device
  with multiple functions would be printed identically 2 times, which is
  generally confusing.  For example, on an Intel 2 port gigabit Ethernet
  card the following was displayed:
...
04  01  8086  1010  0200  00
04  01  8086  1010  0200  00
...

- Prints a text description of each device's PCI class instead of the
  raw PCI class code.  The textual description makes it much easier to
  determine what devices are installed on a PCI bus.

- Changes the general formatting of the PCI device output.

Previous output:
  PCIE1: connected as Root Complex
  04  01  8086  1010  0200  00
  04  01  8086  1010  0200  00
  03  00  10b5  8112  0604  00
  02  01  10b5  8518  0604  00
  02  02  10b5  8518  0604  00
  08  00  1957  0040  0b20  00
  07  00  10b5  8518  0604  00
  09  00  10b5  8112  0604  00
  07  01  10b5  8518  0604  00
  07  02  10b5  8518  0604  00
  06  00  10b5  8518  0604  00
  02  03  10b5  8518  0604  00
  01  00  10b5  8518  0604  00
  PCIE1: Bus 00 - 0b
  PCIE2: connected as Root Complex
  0d  00  1957  0040  0b20  00
  PCIE2: Bus 0c - 0d

Updated output:
  PCIE1: connected as Root Complex
  04:01.0 - 8086:1010 - Network controller
  04:01.1 - 8086:1010 - Network controller
  03:00.0 - 10b5:8112 - Bridge device
  02:01.0 - 10b5:8518 - Bridge device
  02:02.0 - 10b5:8518 - Bridge device
  08:00.0 - 1957:0040 - Processor
  07:00.0 - 10b5:8518 - Bridge device
  09:00.0 - 10b5:8112 - Bridge device
  07:01.0 - 10b5:8518 - Bridge device
  07:02.0 - 10b5:8518 - Bridge device
  06:00.0 - 10b5:8518 - Bridge device
  02:03.0 - 10b5:8518 - Bridge device
  01:00.0 - 10b5:8518 - Bridge device
  PCIE1: Bus 00 - 0b
  PCIE2: connected as Root Complex
  0d:00.0 - 1957:0040 - Processor
  PCIE2: Bus 0c - 0d

Signed-off-by: Peter Tyser pty...@xes-inc.com
---
 common/cmd_pci.c  |   66 +-
 drivers/pci/pci.c |  117 -
 include/pci.h |1 +
 3 files changed, 92 insertions(+), 92 deletions(-)

diff --git a/common/cmd_pci.c b/common/cmd_pci.c
index ccf5ada..92631ea 100644
--- a/common/cmd_pci.c
+++ b/common/cmd_pci.c
@@ -104,68 +104,6 @@ void pciinfo(int BusNum, int ShortPCIListing)
 }
 }
 
-static char *pci_classes_str(u8 class)
-{
-   switch (class) {
-   case PCI_CLASS_NOT_DEFINED:
-   return Build before PCI Rev2.0;
-   break;
-   case PCI_BASE_CLASS_STORAGE:
-   return Mass storage controller;
-   break;
-   case PCI_BASE_CLASS_NETWORK:
-   return Network controller;
-   break;
-   case PCI_BASE_CLASS_DISPLAY:
-   return Display controller;
-   break;
-   case PCI_BASE_CLASS_MULTIMEDIA:
-   return Multimedia device;
-   break;
-   case PCI_BASE_CLASS_MEMORY:
-   return Memory controller;
-   break;
-   case PCI_BASE_CLASS_BRIDGE:
-   return Bridge device;
-   break;
-   case PCI_BASE_CLASS_COMMUNICATION:
-   return Simple comm. controller;
-   break;
-   case PCI_BASE_CLASS_SYSTEM:
-   return Base system peripheral;
-   break;
-   case PCI_BASE_CLASS_INPUT:
-   return Input device;
-   break;
-   case PCI_BASE_CLASS_DOCKING:
-   return Docking station;
-   break;
-   case PCI_BASE_CLASS_PROCESSOR:
-   return Processor;
-   break;
-   case PCI_BASE_CLASS_SERIAL:
-   return Serial bus controller;
-   break;
-   case PCI_BASE_CLASS_INTELLIGENT:
-   return Intelligent controller;
-   break;
-   case PCI_BASE_CLASS_SATELLITE:
-   return Satellite controller;
-   break;
-   case PCI_BASE_CLASS_CRYPT:
-   return Cryptographic device;
-   break;
-   case PCI_BASE_CLASS_SIGNAL_PROCESSING:
-   return DSP;
-   break;
-   case PCI_CLASS_OTHERS:
-   return Does not fit any class;
-   break;
-   default:
-   return  ???;
-   break;
-   };
-}
 
 /*
  * Subroutine:  pci_header_show_brief
@@ -190,7 +128,7 @@ void pci_header_show_brief(pci_dev_t dev)
 
printf(0x%.4x 0x%.4x %-23s 0x%.2x\n,
   vendor, device,
-  pci_classes_str(class), subclass);
+  pci_class_str(class), subclass

[U-Boot] [PATCH/RFC 5/6] pci: Fix ordering of devices when CONFIG_PCI_SCAN_SHOW

2010-10-29 Thread Peter Tyser
Move the printing of PCI device information to before the PCI device is
configured.  This prevents the case where recursive scanning results in
the deepest devices being printed first.

This change also makes PCI lockups during enumeration easier to
diagnose since the device that is being configured is printed out prior
to configuration.  Previously, it was not possible to determine which
device caused the PCI lockup.

Original example:
  PCIE1: connected as Root Complex
04:01.0 - 8086:1010 - Network controller
04:01.1 - 8086:1010 - Network controller
03:00.0 - 10b5:8112 - Bridge device
02:01.0 - 10b5:8518 - Bridge device
02:02.0 - 10b5:8518 - Bridge device
08:00.0 - 1957:0040 - Processor
07:00.0 - 10b5:8518 - Bridge device
09:00.0 - 10b5:8112 - Bridge device
07:01.0 - 10b5:8518 - Bridge device
07:02.0 - 10b5:8518 - Bridge device
06:00.0 - 10b5:8518 - Bridge device
02:03.0 - 10b5:8518 - Bridge device
01:00.0 - 10b5:8518 - Bridge device
  PCIE1: Bus 00 - 0b

Updated example:
  PCIE1: connected as Root Complex
01:00.0 - 10b5:8518 - Bridge device
02:01.0 - 10b5:8518 - Bridge device
03:00.0 - 10b5:8112 - Bridge device
04:01.0 - 8086:1010 - Network controller
04:01.1 - 8086:1010 - Network controller
02:02.0 - 10b5:8518 - Bridge device
02:03.0 - 10b5:8518 - Bridge device
06:00.0 - 10b5:8518 - Bridge device
07:00.0 - 10b5:8518 - Bridge device
08:00.0 - 1957:0040 - Processor
07:01.0 - 10b5:8518 - Bridge device
09:00.0 - 10b5:8112 - Bridge device
07:02.0 - 10b5:8518 - Bridge device
  PCIE1: Bus 00 - 0b

Signed-off-by: Peter Tyser pty...@xes-inc.com
---
 drivers/pci/pci.c |   17 +
 1 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 3dccf88..78f7339 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -645,6 +645,14 @@ int pci_hose_scan_bus(struct pci_controller *hose, int bus)
pci_hose_read_config_word(hose, dev, PCI_DEVICE_ID, device);
pci_hose_read_config_word(hose, dev, PCI_CLASS_DEVICE, class);
 
+#ifdef CONFIG_PCI_SCAN_SHOW
+   if (pci_print_dev(hose, dev)) {
+   printf(%02x:%02x.%x - %04x:%04x - %s\n,
+  PCI_BUS(dev), PCI_DEV(dev), PCI_FUNC(dev),
+  vendor, device, pci_class_str(class  8));
+   }
+#endif
+
cfg = pci_find_config(hose, class, vendor, device,
  PCI_BUS(dev), PCI_DEV(dev), 
PCI_FUNC(dev));
if (cfg) {
@@ -657,16 +665,9 @@ int pci_hose_scan_bus(struct pci_controller *hose, int bus)
sub_bus = max(sub_bus, n);
 #endif
}
+
if (hose-fixup_irq)
hose-fixup_irq(hose, dev);
-
-#ifdef CONFIG_PCI_SCAN_SHOW
-   if (pci_print_dev(hose, dev)) {
-   printf(%02x:%02x.%x - %04x:%04x - %s\n,
-  PCI_BUS(dev), PCI_DEV(dev), PCI_FUNC(dev),
-  vendor, device, pci_class_str(class  8));
-   }
-#endif
}
 
return sub_bus;
-- 
1.7.0.4

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


[U-Boot] [PATCH/RFC 6/6] pci: Use intelligent indentation for CONFIG_PCI_SCAN_SHOW

2010-10-29 Thread Peter Tyser
When CONFIG_PCI_SCAN_SHOW is defined U-Boot prints out PCI devices as
they are found during bootup, eg:
  PCIE1: connected as Root Complex
01:00.0 - 10b5:8518 - Bridge device
02:01.0 - 10b5:8518 - Bridge device
03:00.0 - 10b5:8112 - Bridge device
04:01.0 - 8086:1010 - Network controller
04:01.1 - 8086:1010 - Network controller
02:02.0 - 10b5:8518 - Bridge device
02:03.0 - 10b5:8518 - Bridge device
06:00.0 - 10b5:8518 - Bridge device
07:00.0 - 10b5:8518 - Bridge device
08:00.0 - 1957:0040 - Processor
07:01.0 - 10b5:8518 - Bridge device
09:00.0 - 10b5:8112 - Bridge device
07:02.0 - 10b5:8518 - Bridge device
  PCIE1: Bus 00 - 0b
  PCIE2: connected as Root Complex
0d:00.0 - 1957:0040 - Processor
  PCIE2: Bus 0c - 0d

This information is useful, but its difficult to determine the PCI bus
topology.  To things clearer, we can use indention to make it more
obvious how the PCI bus is organized.  For the example above, the
updated output with this change is:

  PCIE1: connected as Root Complex
01:00.0 - 10b5:8518 - Bridge device
 02:01.0- 10b5:8518 - Bridge device
  03:00.0   - 10b5:8112 - Bridge device
   04:01.0  - 8086:1010 - Network controller
   04:01.1  - 8086:1010 - Network controller
 02:02.0- 10b5:8518 - Bridge device
 02:03.0- 10b5:8518 - Bridge device
  06:00.0   - 10b5:8518 - Bridge device
   07:00.0  - 10b5:8518 - Bridge device
08:00.0 - 1957:0040 - Processor
   07:01.0  - 10b5:8518 - Bridge device
09:00.0 - 10b5:8112 - Bridge device
   07:02.0  - 10b5:8518 - Bridge device
  PCIE1: Bus 00 - 0b
  PCIE2: connected as Root Complex
0d:00.0 - 1957:0040 - Processor
  PCIE2: Bus 0c - 0d

In the examples above, an MPC8640 is connected to a PEX8518 PCIe switch
(01:00 and 02:0x), which is connected to another PEX8518 PCIe switch
(06:00 and 07:0x), which then connects to a MPC8572 processor (08:00).
Also, the MPC8640's PEX8518 PCIe switch is connected to a PCI ethernet
card (04:01) via a PEX8112 PCIe-to-PCI bridge (03:00).

Signed-off-by: Peter Tyser pty...@xes-inc.com
---
 drivers/pci/pci.c |   16 ++--
 1 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 78f7339..702ac67 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -616,6 +616,9 @@ int pci_hose_scan_bus(struct pci_controller *hose, int bus)
unsigned char header_type;
struct pci_config_table *cfg;
pci_dev_t dev;
+#ifdef CONFIG_PCI_SCAN_SHOW
+   static int indent = 0;
+#endif
 
sub_bus = bus;
 
@@ -646,9 +649,14 @@ int pci_hose_scan_bus(struct pci_controller *hose, int bus)
pci_hose_read_config_word(hose, dev, PCI_CLASS_DEVICE, class);
 
 #ifdef CONFIG_PCI_SCAN_SHOW
+   indent++;
+
+   /* Print leading space, including bus indentation */
+   printf(%*c, indent + 1, ' ');
+
if (pci_print_dev(hose, dev)) {
-   printf(%02x:%02x.%x - %04x:%04x - %s\n,
-  PCI_BUS(dev), PCI_DEV(dev), PCI_FUNC(dev),
+   printf(%02x:%02x.%-*x - %04x:%04x - %s\n,
+  PCI_BUS(dev), PCI_DEV(dev), 6 - indent, 
PCI_FUNC(dev),
   vendor, device, pci_class_str(class  8));
}
 #endif
@@ -666,6 +674,10 @@ int pci_hose_scan_bus(struct pci_controller *hose, int bus)
 #endif
}
 
+#ifdef CONFIG_PCI_SCAN_SHOW
+   indent--;
+#endif
+
if (hose-fixup_irq)
hose-fixup_irq(hose, dev);
}
-- 
1.7.0.4

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


[U-Boot] [PATCH 1/6] fsl: Clean up printing of PCI boot info

2010-10-29 Thread Peter Tyser
Previously boards used a variety of indentations, newline styles, and
colon styles for the PCI information that is printed on bootup.  This
patch unifies the style to look like:

...
NAND:  1024 MiB
PCIE1: connected as Root Complex
   Scanning PCI bus 01
04  01  8086  1010  0200  00
04  01  8086  1010  0200  00
03  00  10b5  8112  0604  00
02  01  10b5  8518  0604  00
02  02  10b5  8518  0604  00
08  00  1957  0040  0b20  00
07  00  10b5  8518  0604  00
09  00  10b5  8112  0604  00
07  01  10b5  8518  0604  00
07  02  10b5  8518  0604  00
06  00  10b5  8518  0604  00
02  03  10b5  8518  0604  00
01  00  10b5  8518  0604  00
PCIE1: Bus 00 - 0b
PCIE2: connected as Root Complex
   Scanning PCI bus 0d
0d  00  1957  0040  0b20  00
PCIE2: Bus 0c - 0d
In:serial
...

Signed-off-by: Peter Tyser pty...@xes-inc.com
CC: w...@denx.de
CC: s...@denx.de
CC: ga...@kernel.crashing.org
---
This series assumes fsl_pci_init: Make fsl_pci_init_port() PCI/PCIe aware
has already been applied.

 board/atum8548/atum8548.c |   12 ++--
 board/freescale/corenet_ds/pci.c  |   16 
 board/freescale/mpc8536ds/mpc8536ds.c |   16 
 board/freescale/mpc8540ads/mpc8540ads.c   |4 ++--
 board/freescale/mpc8541cds/mpc8541cds.c   |6 +++---
 board/freescale/mpc8544ds/mpc8544ds.c |   24 
 board/freescale/mpc8548cds/mpc8548cds.c   |   16 
 board/freescale/mpc8555cds/mpc8555cds.c   |6 +++---
 board/freescale/mpc8560ads/mpc8560ads.c   |4 ++--
 board/freescale/mpc8568mds/mpc8568mds.c   |8 
 board/freescale/mpc8569mds/mpc8569mds.c   |8 
 board/freescale/mpc8572ds/mpc8572ds.c |   20 ++--
 board/freescale/mpc8610hpcd/mpc8610hpcd.c |   20 ++--
 board/freescale/mpc8641hpcn/mpc8641hpcn.c |   18 +-
 board/freescale/p1022ds/p1022ds.c |8 
 board/freescale/p1_p2_rdb/pci.c   |   16 
 board/freescale/p2020ds/p2020ds.c |   24 
 board/pm854/pm854.c   |4 ++--
 board/pm856/pm856.c   |4 ++--
 board/sbc8548/sbc8548.c   |8 
 board/sbc8641d/sbc8641d.c |   18 +-
 board/tqc/tqm85xx/tqm85xx.c   |8 
 board/xes/common/fsl_8xxx_pci.c   |   16 
 drivers/pci/fsl_pci_init.c|   10 +-
 24 files changed, 147 insertions(+), 147 deletions(-)

diff --git a/board/atum8548/atum8548.c b/board/atum8548/atum8548.c
index 671f9e9..4fcbb18 100644
--- a/board/atum8548/atum8548.c
+++ b/board/atum8548/atum8548.c
@@ -218,14 +218,14 @@ void pci_init_board(void)
 
pcie1_hose.region_count = 1;
 #endif
-   printf (PCIE1 connected to Slot as %s (base addr %lx)\n,
+   printf (PCIE1: connected to Slot as %s (base addr %lx)\n,
pcie_ep ? Endpoint : Root Complex,
pci_info[num].regs);
 
first_free_busno = fsl_pci_init_port(pci_info[num++],
pcie1_hose, first_free_busno);
} else {
-   printf (PCIE1: disabled\n);
+   printf(PCIE1: disabled\n);
}
 
puts(\n);
@@ -242,7 +242,7 @@ void pci_init_board(void)
if (!(devdisr  MPC85xx_DEVDISR_PCI1)) {
SET_STD_PCI_INFO(pci_info[num], 1);
pci_agent = fsl_setup_hose(pci1_hose, pci_info[num].regs);
-   printf (\nPCI1: %d bit, %s MHz, %s, %s, %s (base address 
%lx)\n,
+   printf(PCI1: %d bit, %s MHz, %s, %s, %s (base address %lx)\n,
(pci_32) ? 32 : 64,
(pci_speed == 3000) ? 33 :
(pci_speed == 6000) ? 66 : unknown,
@@ -254,7 +254,7 @@ void pci_init_board(void)
first_free_busno = fsl_pci_init_port(pci_info[num++],
pci1_hose, first_free_busno);
} else {
-   printf (PCI: disabled\n);
+   printf(PCI1: disabled\n);
}
 
puts(\n);
@@ -267,11 +267,11 @@ void pci_init_board(void)
SET_STD_PCI_INFO(pci_info[num], 2);
pci_agent = fsl_setup_hose(pci2_hose, pci_info[num].regs);
 
-   puts (PCI2\n);
+   puts(PCI2\n);
first_free_busno = fsl_pci_init_port(pci_info[num++],
pci1_hose, first_free_busno);
} else {
-   printf (PCI2: disabled\n);
+   printf(PCI2: disabled\n);
}
puts(\n);
 #else
diff --git a/board/freescale/corenet_ds/pci.c b/board/freescale/corenet_ds/pci.c
index e1bca19

[U-Boot] [PATCH] fsl_pci_init: Make fsl_pci_init_port() PCI/PCIe aware

2010-10-28 Thread Peter Tyser
Previously fsl_pci_init_port() always assumed that a port was a PCIe
port and would incorrectly print messages for a PCI port such as the
following on bootup:
PCI1:  32 bit, 33 MHz, sync, host, arbiter
Scanning PCI bus 00
PCIE1 on bus 00 - 00

This change corrects the output of fsl_pci_init_port():
PCI1:  32 bit, 33 MHz, sync, host, arbiter
Scanning PCI bus 00
PCI1 on bus 00 - 00

Signed-off-by: Peter Tyser pty...@xes-inc.com
---
Tested on a MPC8548 with PCI, and a MPC8640 with PCIe

 drivers/pci/fsl_pci_init.c |6 +-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/drivers/pci/fsl_pci_init.c b/drivers/pci/fsl_pci_init.c
index 1f02103..45794da 100644
--- a/drivers/pci/fsl_pci_init.c
+++ b/drivers/pci/fsl_pci_init.c
@@ -441,6 +441,8 @@ int fsl_pci_init_port(struct fsl_pci_info *pci_info,
 {
volatile ccsr_fsl_pci_t *pci;
struct pci_region *r;
+   pci_dev_t dev = PCI_BDF(busno,0,0);
+   u8 pcie_cap;
 
pci = (ccsr_fsl_pci_t *) pci_info-regs;
 
@@ -479,7 +481,9 @@ int fsl_pci_init_port(struct fsl_pci_info *pci_info,
hose-last_busno = hose-first_busno;
}
 
-   printf(PCIE%x on bus %02x - %02x\n, pci_info-pci_num,
+   pci_hose_read_config_byte(hose, dev, FSL_PCIE_CAP_ID, pcie_cap);
+   printf(PCI%s%x on bus %02x - %02x\n, pcie_cap == PCI_CAP_ID_EXP ?
+   E : , pci_info-pci_num,
hose-first_busno, hose-last_busno);
 
return(hose-last_busno + 1);
-- 
1.7.0.4

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


Re: [U-Boot] [PATCH 3/3] tqm85xx: Update PCI code

2010-10-28 Thread Peter Tyser
On Wed, 2010-10-27 at 08:47 +0200, Wolfgang Denk wrote:
 Dear Peter Tyser,
 
 In message 1288156533.1971.6.ca...@ptyser-laptop you wrote:
 
  Can you send the entire bootup output?  The code is based on Freescale
  reference boards, eg the mpc8568mds, so I'd guess the problem is not
  tqm85xx-specific.
 
 Sure. Here it is:

Thanks.

 U-Boot 2010.09-00558-g79e6313 (Oct 26 2010 - 21:31:41)
 
 CPU:   8555E, Version: 1.1, (0x80790011)
 Core:  Unknown, Version: 2.0, (0x80200020)
 Clock Configuration:
CPU0:833.333 MHz, 
CCB:333.333 MHz,
DDR:166.667 MHz (333.333 MT/s data rate), LBC:41.667 MHz
 CPM:   333.333 MHz
 L1:D-cache 32 kB enabled
I-cache 32 kB enabled
 Board: TQM8555, serial# ABC0555 casl=25
 I2C:   ready
 DRAM:  128 MiB
 FLASH: 128 MiB
 L2:256 KB already enabled
 
PCI1:  32 bit, 33 MHz, sync, host, arbiter

Its unclear what the correct behavior is here.  I just did a quick
sample of recent Freescale reference boards and generally see:
p1020ds/p2020dsmpc8572: printf(PCIE%u connected to %s as %s (base addr 
%lx)\n,

Older boards with PCI often look like:
mpc8548/mpc8568mds: printf (\nPCI: %d bit, %s MHz, %s, %s, %s (base 
address %lx)\n,

The TQM boards, socrates, and mpc8349 are/were:
printf (PCI1:  %d bit, %s MHz, %s, %s, %s\n);

So the original behavior of the TQM board was out of sync with the
majority of other boards, and some boards have a newline.

I agree we should get rid of the newline on all these printfs, but the
indentation issue is murkier to me.  The common Freescale PCI code
currently assumes there is an indentation, so we should really sync
boards'/FSL indentation up to be consistent.  Anyone have a strong
preference for the indentation?  p2020 way, or socrates way above?

Scanning PCI bus 00
 PCIE1 on bus 00 - 00

I just sent a patch to address this issue.

Best,
Peter



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


Re: [U-Boot] [PATCH 3/3] tqm85xx: Update PCI code

2010-10-26 Thread Peter Tyser
On Tue, 2010-10-26 at 21:54 +0200, Wolfgang Denk wrote:
 Dear Peter Tyser,
 
 In message 1285785448-4703-3-git-send-email-pty...@xes-inc.com you wrote:
  Update to use the recent, common FSL PCI initialization code.
  
  Signed-off-by: Peter Tyser pty...@xes-inc.com
  CC: s...@denx.de
  ---
   board/tqc/tqm85xx/law.c |4 +-
   board/tqc/tqm85xx/tlb.c |   10 ++--
   board/tqc/tqm85xx/tqm85xx.c |  151 
  ---
   include/configs/TQM85xx.h   |   20 +++---
   4 files changed, 59 insertions(+), 126 deletions(-)
 
 This commit needs fixing.
 
 First, it corrupts the output. Some patch like this should be added:
 
 diff --git a/board/tqc/tqm85xx/tqm85xx.c b/board/tqc/tqm85xx/tqm85xx.c
 index 2c3885f..027c429 100644
 --- a/board/tqc/tqm85xx/tqm85xx.c
 +++ b/board/tqc/tqm85xx/tqm85xx.c
 @@ -567,7 +567,7 @@ void pci_init_board (void)
   if (!(devdisr  MPC85xx_DEVDISR_PCI1)) {
   SET_STD_PCI_INFO(pci_info[num], 1);
   pcie_ep = fsl_setup_hose(pci1_hose, pci_info[num].regs);
 - printf (\n   PCI1:  %d bit, %s MHz, %s, %s, %s\n,
 + printf (PCI1:  %d bit, %s MHz, %s, %s, %s\n,
   (pci_32) ? 32 : 64,
   (pci_speed == ) ? 33 :
   (pci_speed == ) ? 66 : unknown,
 @@ -591,7 +591,7 @@ void pci_init_board (void)
   }
  #endif
   } else {
 - printf(PCI1: disabled\n);
 + printf(PCI1:  disabled\n);
   }
  #else
   setbits_be32(gur-devdisr, MPC85xx_DEVDISR_PCI1);
 
 
 Even worse, we now see a (badly formatted, but this is just an added
 bonus)
 
   Scanning PCI bus 00
 PCIE1 on bus 00 - 00
 
 which is completely bogus as there on these boards nor on these
 processors.
 
 
 Can you please provide a fix?

Can you send the entire bootup output?  The code is based on Freescale
reference boards, eg the mpc8568mds, so I'd guess the problem is not
tqm85xx-specific.

Best,
Pete

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


  1   2   3   4   5   6   7   8   9   10   >