Re: [PATCH 20/27] ppc: Remove xpedite boards
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
> 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
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
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
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
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
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
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
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
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
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
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
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
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
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
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()
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()
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
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
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
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()
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
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
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()
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()
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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?
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.
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
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
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
[ 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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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.
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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()
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
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
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
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
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
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
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
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
