* [U-Boot] [PATCH 2/6] powerpc/85xx: fdt_set_phy_handle() should return an error code
2012-05-04 22:21 [U-Boot] [PATCH 1/6] powerpc/85xx: minor clean-ups to the P2020DS board header file Timur Tabi
@ 2012-05-04 22:21 ` Timur Tabi
2012-05-04 22:21 ` [U-Boot] [PATCH 3/6] powerpc/85xx: clean up P1022DS board configuration header file Timur Tabi
` (3 subsequent siblings)
4 siblings, 0 replies; 32+ messages in thread
From: Timur Tabi @ 2012-05-04 22:21 UTC (permalink / raw)
To: u-boot
fdt_set_phy_handle() makes several FDT calls that could fail, so it should
not be hiding these errors.
Signed-off-by: Timur Tabi <timur@freescale.com>
---
board/freescale/common/fman.c | 36 +++++++++++++++++++-----------------
board/freescale/common/fman.h | 2 +-
2 files changed, 20 insertions(+), 18 deletions(-)
diff --git a/board/freescale/common/fman.c b/board/freescale/common/fman.c
index 8a55fde..6ddf816 100644
--- a/board/freescale/common/fman.c
+++ b/board/freescale/common/fman.c
@@ -37,31 +37,33 @@
* ... update that Ethernet node's phy-handle property to point to the
* ethernet-phy node. This is how we link an Ethernet node to its PHY, so each
* PHY in a virtual MDIO node must have an alias.
+ *
+ * Returns 0 on success, or a negative FDT error code on error.
*/
-void fdt_set_phy_handle(void *fdt, char *compat, phys_addr_t addr,
+int fdt_set_phy_handle(void *fdt, char *compat, phys_addr_t addr,
const char *alias)
{
- int offset, ph;
+ int offset;
+ unsigned int ph;
const char *path;
/* Get a path to the node that 'alias' points to */
path = fdt_get_alias(fdt, alias);
- if (path) {
- /* Get the offset of that node */
- int off = fdt_path_offset(fdt, path);
- if (off > 0)
- ph = fdt_create_phandle(fdt, off);
- else
- return;
- } else {
- return ;
- }
+ if (!path)
+ return -FDT_ERR_BADPATH;
+
+ /* Get the offset of that node */
+ offset = fdt_path_offset(fdt, path);
+ if (offset < 0)
+ return offset;
- /* failed to create a phandle */
- if (ph <= 0)
- return ;
+ ph = fdt_create_phandle(fdt, offset);
+ if (!ph)
+ return -FDT_ERR_BADPHANDLE;
offset = fdt_node_offset_by_compat_reg(fdt, compat, addr);
- if (offset > 0)
- fdt_setprop(fdt, offset, "phy-handle", &ph, sizeof(ph));
+ if (offset < 0)
+ return offset;
+
+ return fdt_setprop(fdt, offset, "phy-handle", &ph, sizeof(ph));
}
diff --git a/board/freescale/common/fman.h b/board/freescale/common/fman.h
index 19ef7c4..d39ef08 100644
--- a/board/freescale/common/fman.h
+++ b/board/freescale/common/fman.h
@@ -20,7 +20,7 @@
#ifndef __FMAN_BOARD_HELPER__
#define __FMAN_BOARD_HELPER__
-void fdt_set_phy_handle(void *fdt, char *compat, phys_addr_t addr,
+int fdt_set_phy_handle(void *fdt, char *compat, phys_addr_t addr,
const char *alias);
#endif
--
1.7.3.4
^ permalink raw reply related [flat|nested] 32+ messages in thread* [U-Boot] [PATCH 3/6] powerpc/85xx: clean up P1022DS board configuration header file
2012-05-04 22:21 [U-Boot] [PATCH 1/6] powerpc/85xx: minor clean-ups to the P2020DS board header file Timur Tabi
2012-05-04 22:21 ` [U-Boot] [PATCH 2/6] powerpc/85xx: fdt_set_phy_handle() should return an error code Timur Tabi
@ 2012-05-04 22:21 ` Timur Tabi
2012-05-04 22:21 ` [U-Boot] [PATCH 4/6] lib/powerpc: addrmap_phys_to_virt() should return a pointer Timur Tabi
` (2 subsequent siblings)
4 siblings, 0 replies; 32+ messages in thread
From: Timur Tabi @ 2012-05-04 22:21 UTC (permalink / raw)
To: u-boot
Remove some unused default environment variables (memctl_intlv_ctl,
perf_mode, diuregs, dium, and diuerr), update 'tftpflash' variable,
and add videobootargs as a Linux command line variable (so that we can
easily pass video= to the kernel).
Signed-off-by: Timur Tabi <timur@freescale.com>
---
include/configs/P1022DS.h | 43 +++++++++++++++++++------------------------
1 files changed, 19 insertions(+), 24 deletions(-)
diff --git a/include/configs/P1022DS.h b/include/configs/P1022DS.h
index 70d751d..9d2f8ea 100644
--- a/include/configs/P1022DS.h
+++ b/include/configs/P1022DS.h
@@ -488,35 +488,30 @@
#define CONFIG_LOADADDR 1000000
#define CONFIG_BOOTDELAY 10 /* -1 disables auto-boot */
-#define CONFIG_BOOTARGS
#define CONFIG_BAUDRATE 115200
-#define CONFIG_EXTRA_ENV_SETTINGS \
- "perf_mode=stable\0" \
- "memctl_intlv_ctl=2\0" \
- "netdev=eth0\0" \
- "uboot=" MK_STR(CONFIG_UBOOTPATH) "\0" \
- "tftpflash=tftpboot $loadaddr $uboot; " \
- "protect off " MK_STR(CONFIG_SYS_TEXT_BASE) " +$filesize; " \
- "erase " MK_STR(CONFIG_SYS_TEXT_BASE) " +$filesize; " \
- "cp.b $loadaddr " MK_STR(CONFIG_SYS_TEXT_BASE) " $filesize; " \
- "protect on " MK_STR(CONFIG_SYS_TEXT_BASE) " +$filesize; " \
- "cmp.b $loadaddr " MK_STR(CONFIG_SYS_TEXT_BASE) " $filesize\0" \
- "consoledev=ttyS0\0" \
- "ramdiskaddr=2000000\0" \
- "ramdiskfile=uramdisk\0" \
- "fdtaddr=c00000\0" \
- "fdtfile=p1022ds.dtb\0" \
- "bdev=sda3\0" \
- "diuregs=md e002c000 1d\0" \
- "dium=mw e002c01c\0" \
- "diuerr=md e002c014 1\0" \
+#define CONFIG_EXTRA_ENV_SETTINGS \
+ "netdev=eth0\0" \
+ "uboot=" MK_STR(CONFIG_UBOOTPATH) "\0" \
+ "ubootaddr=" MK_STR(CONFIG_SYS_TEXT_BASE) "\0" \
+ "tftpflash=tftpboot $loadaddr $uboot && " \
+ "protect off $ubootaddr +$filesize && " \
+ "erase $ubootaddr +$filesize && " \
+ "cp.b $loadaddr $ubootaddr $filesize && " \
+ "protect on $ubootaddr +$filesize && " \
+ "cmp.b $loadaddr $ubootaddr $filesize\0" \
+ "consoledev=ttyS0\0" \
+ "ramdiskaddr=2000000\0" \
+ "ramdiskfile=rootfs.ext2.gz.uboot\0" \
+ "fdtaddr=c00000\0" \
+ "fdtfile=p1022ds.dtb\0" \
+ "bdev=sda3\0" \
"hwconfig=esdhc;audclk:12\0"
#define CONFIG_HDBOOT \
"setenv bootargs root=/dev/$bdev rw " \
- "console=$consoledev,$baudrate $othbootargs;" \
+ "console=$consoledev,$baudrate $othbootargs $videobootargs;" \
"tftp $loadaddr $bootfile;" \
"tftp $fdtaddr $fdtfile;" \
"bootm $loadaddr - $fdtaddr"
@@ -525,14 +520,14 @@
"setenv bootargs root=/dev/nfs rw " \
"nfsroot=$serverip:$rootpath " \
"ip=$ipaddr:$serverip:$gatewayip:$netmask:$hostname:$netdev:off " \
- "console=$consoledev,$baudrate $othbootargs;" \
+ "console=$consoledev,$baudrate $othbootargs $videobootargs;" \
"tftp $loadaddr $bootfile;" \
"tftp $fdtaddr $fdtfile;" \
"bootm $loadaddr - $fdtaddr"
#define CONFIG_RAMBOOTCOMMAND \
"setenv bootargs root=/dev/ram rw " \
- "console=$consoledev,$baudrate $othbootargs;" \
+ "console=$consoledev,$baudrate $othbootargs $videobootargs;" \
"tftp $ramdiskaddr $ramdiskfile;" \
"tftp $loadaddr $bootfile;" \
"tftp $fdtaddr $fdtfile;" \
--
1.7.3.4
^ permalink raw reply related [flat|nested] 32+ messages in thread* [U-Boot] [PATCH 4/6] lib/powerpc: addrmap_phys_to_virt() should return a pointer
2012-05-04 22:21 [U-Boot] [PATCH 1/6] powerpc/85xx: minor clean-ups to the P2020DS board header file Timur Tabi
2012-05-04 22:21 ` [U-Boot] [PATCH 2/6] powerpc/85xx: fdt_set_phy_handle() should return an error code Timur Tabi
2012-05-04 22:21 ` [U-Boot] [PATCH 3/6] powerpc/85xx: clean up P1022DS board configuration header file Timur Tabi
@ 2012-05-04 22:21 ` Timur Tabi
2012-05-04 22:21 ` [U-Boot] [PATCH 5/6] cmd_nvedit.c: allow board-specific code before/after saving the environment Timur Tabi
2012-05-04 22:21 ` [U-Boot] [PATCH 6/6] powerpc/85xx: p1022ds: use the saveenv board preparation functions Timur Tabi
4 siblings, 0 replies; 32+ messages in thread
From: Timur Tabi @ 2012-05-04 22:21 UTC (permalink / raw)
To: u-boot
addrmap_phys_to_virt() converts a physical address (phys_addr_t) to a
virtual address, so it should return a pointer instead of an unsigned long.
Its counterpart, addrmap_virt_to_phys(), takes a pointer, so now they're
orthogonal.
The only caller of addrmap_phys_to_virt() converts the return value to
a pointer anyway.
Signed-off-by: Timur Tabi <timur@freescale.com>
---
arch/powerpc/include/asm/io.h | 2 +-
include/addr_map.h | 2 +-
lib/addr_map.c | 19 +++++++++++--------
3 files changed, 13 insertions(+), 10 deletions(-)
diff --git a/arch/powerpc/include/asm/io.h b/arch/powerpc/include/asm/io.h
index 56ac9fe..6b52a94 100644
--- a/arch/powerpc/include/asm/io.h
+++ b/arch/powerpc/include/asm/io.h
@@ -295,7 +295,7 @@ static inline void *
map_physmem(phys_addr_t paddr, unsigned long len, unsigned long flags)
{
#ifdef CONFIG_ADDR_MAP
- return (void *)(addrmap_phys_to_virt(paddr));
+ return addrmap_phys_to_virt(paddr);
#else
return (void *)((unsigned long)paddr);
#endif
diff --git a/include/addr_map.h b/include/addr_map.h
index d55f5f6..36da256 100644
--- a/include/addr_map.h
+++ b/include/addr_map.h
@@ -22,7 +22,7 @@
#include <asm/types.h>
extern phys_addr_t addrmap_virt_to_phys(void *vaddr);
-extern unsigned long addrmap_phys_to_virt(phys_addr_t paddr);
+void *addrmap_phys_to_virt(phys_addr_t paddr);
extern void addrmap_set_entry(unsigned long vaddr, phys_addr_t paddr,
phys_size_t size, int idx);
diff --git a/lib/addr_map.c b/lib/addr_map.c
index ff8532c..31384d1 100644
--- a/lib/addr_map.c
+++ b/lib/addr_map.c
@@ -47,26 +47,29 @@ phys_addr_t addrmap_virt_to_phys(void * vaddr)
return (phys_addr_t)(~0);
}
-unsigned long addrmap_phys_to_virt(phys_addr_t paddr)
+void *addrmap_phys_to_virt(phys_addr_t paddr)
{
int i;
for (i = 0; i < CONFIG_SYS_NUM_ADDR_MAP; i++) {
- u64 base, upper, addr;
+ phys_addr_t base, upper;
if (address_map[i].size == 0)
continue;
- addr = (u64)paddr;
- base = (u64)(address_map[i].paddr);
- upper = (u64)(address_map[i].size) + base - 1;
+ base = address_map[i].paddr;
+ upper = address_map[i].size + base - 1;
- if (addr >= base && addr <= upper) {
- return paddr - address_map[i].paddr + address_map[i].vaddr;
+ if (paddr >= base && paddr <= upper) {
+ phys_addr_t offset;
+
+ offset = address_map[i].paddr - address_map[i].vaddr;
+
+ return (void *)(unsigned long)(paddr - offset);
}
}
- return (unsigned long)(~0);
+ return (void *)(~0);
}
void addrmap_set_entry(unsigned long vaddr, phys_addr_t paddr,
--
1.7.3.4
^ permalink raw reply related [flat|nested] 32+ messages in thread* [U-Boot] [PATCH 5/6] cmd_nvedit.c: allow board-specific code before/after saving the environment
2012-05-04 22:21 [U-Boot] [PATCH 1/6] powerpc/85xx: minor clean-ups to the P2020DS board header file Timur Tabi
` (2 preceding siblings ...)
2012-05-04 22:21 ` [U-Boot] [PATCH 4/6] lib/powerpc: addrmap_phys_to_virt() should return a pointer Timur Tabi
@ 2012-05-04 22:21 ` Timur Tabi
2012-05-14 5:20 ` Mike Frysinger
2012-05-04 22:21 ` [U-Boot] [PATCH 6/6] powerpc/85xx: p1022ds: use the saveenv board preparation functions Timur Tabi
4 siblings, 1 reply; 32+ messages in thread
From: Timur Tabi @ 2012-05-04 22:21 UTC (permalink / raw)
To: u-boot
Introduce board_start_saveenv() and board_finish_saveenv(), two "weak"
functions that are called before and after saving the environment. This
allows for board-specific functions that "prepare" the board for saving
the environment. This is useful if, for some reason, the non-volatile
storage is normally unavailable (e.g. blocked via a mux).
Signed-off-by: Timur Tabi <timur@freescale.com>
---
common/cmd_nvedit.c | 24 +++++++++++++++++++++++-
1 files changed, 23 insertions(+), 1 deletions(-)
diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c
index e1ccdd8..9637682 100644
--- a/common/cmd_nvedit.c
+++ b/common/cmd_nvedit.c
@@ -595,11 +595,33 @@ ulong getenv_ulong(const char *name, int base, ulong default_val)
}
#if defined(CONFIG_CMD_SAVEENV) && !defined(CONFIG_ENV_IS_NOWHERE)
+
+static int __board_start_saveenv(void)
+{
+ return 0;
+}
+int board_start_saveenv(void) __attribute__((weak, alias("__board_start_saveenv")));
+
+static void __board_finish_saveenv(void)
+{
+}
+void board_finish_saveenv(void) __attribute__((weak, alias("__board_finish_saveenv")));
+
int do_env_save(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
{
+ int ret;
+
printf("Saving Environment to %s...\n", env_name_spec);
- return saveenv() ? 1 : 0;
+ ret = board_start_saveenv();
+ if (ret)
+ return 0;
+
+ ret = saveenv() ? 1 : 0;
+
+ board_finish_saveenv();
+
+ return ret;
}
U_BOOT_CMD(
--
1.7.3.4
^ permalink raw reply related [flat|nested] 32+ messages in thread* [U-Boot] [PATCH 5/6] cmd_nvedit.c: allow board-specific code before/after saving the environment
2012-05-04 22:21 ` [U-Boot] [PATCH 5/6] cmd_nvedit.c: allow board-specific code before/after saving the environment Timur Tabi
@ 2012-05-14 5:20 ` Mike Frysinger
2012-05-14 16:10 ` Timur Tabi
2012-05-17 21:18 ` Timur Tabi
0 siblings, 2 replies; 32+ messages in thread
From: Mike Frysinger @ 2012-05-14 5:20 UTC (permalink / raw)
To: u-boot
On Friday 04 May 2012 18:21:31 Timur Tabi wrote:
> Introduce board_start_saveenv() and board_finish_saveenv(), two "weak"
> functions that are called before and after saving the environment. This
> allows for board-specific functions that "prepare" the board for saving
> the environment. This is useful if, for some reason, the non-volatile
> storage is normally unavailable (e.g. blocked via a mux).
all these board hooks are paper-cutting us to death with unused bloat
> --- a/common/cmd_nvedit.c
> +++ b/common/cmd_nvedit.c
>
> #if defined(CONFIG_CMD_SAVEENV) && !defined(CONFIG_ENV_IS_NOWHERE)
> +
> +static int __board_start_saveenv(void)
> +{
> + return 0;
> +}
> +int board_start_saveenv(void) __attribute__((weak,
> alias("__board_start_saveenv"))); +
> +static void __board_finish_saveenv(void)
> +{
> +}
> +void board_finish_saveenv(void) __attribute__((weak,
> alias("__board_finish_saveenv"))); +
> int do_env_save(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
> {
> + int ret;
> +
> printf("Saving Environment to %s...\n", env_name_spec);
>
> - return saveenv() ? 1 : 0;
> + ret = board_start_saveenv();
> + if (ret)
> + return 0;
> +
> + ret = saveenv() ? 1 : 0;
> +
> + board_finish_saveenv();
> +
> + return ret;
> }
this is less bloat:
int board_start_saveenv(void) __attribute__((weak, alias("saveenv")));
int do_env_save(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
{
printf("Saving Environment to %s...\n", env_name_spec);
return board_saveenv() ? 1 : 0;
}
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20120514/5fd0154f/attachment.pgp>
^ permalink raw reply [flat|nested] 32+ messages in thread* [U-Boot] [PATCH 5/6] cmd_nvedit.c: allow board-specific code before/after saving the environment
2012-05-14 5:20 ` Mike Frysinger
@ 2012-05-14 16:10 ` Timur Tabi
2012-05-15 5:14 ` Mike Frysinger
2012-05-17 22:18 ` Wolfgang Denk
2012-05-17 21:18 ` Timur Tabi
1 sibling, 2 replies; 32+ messages in thread
From: Timur Tabi @ 2012-05-14 16:10 UTC (permalink / raw)
To: u-boot
Mike Frysinger wrote:
> On Friday 04 May 2012 18:21:31 Timur Tabi wrote:
>> Introduce board_start_saveenv() and board_finish_saveenv(), two "weak"
>> functions that are called before and after saving the environment. This
>> allows for board-specific functions that "prepare" the board for saving
>> the environment. This is useful if, for some reason, the non-volatile
>> storage is normally unavailable (e.g. blocked via a mux).
>
> all these board hooks are paper-cutting us to death with unused bloat
I know, and I don't like it either. I hate how our hardware designers are
always breaking the "rules", forcing us software developers to hack up our
software more and more. The muxing on this chip is a like a cruel joke
being played on me. I've even been told that I'm trying too hard to make
it work.
> this is less bloat:
> int board_start_saveenv(void) __attribute__((weak, alias("saveenv")));
>
> int do_env_save(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
> {
> printf("Saving Environment to %s...\n", env_name_spec);
> return board_saveenv() ? 1 : 0;
> }
Ah, I see. This forces the board-specific function to call saveenv().
That gives us more flexibility in the board code.
However, I was trying to mimic what we have in the NAND layer, with
nand_get_device() and nand_release_device(). That is, before we save the
environment, we have to "get" it, and then after we save it, we can
"release" it.
Your approach, although it eliminates two weak functions, is not as
"architecturally clean" as mine, IMHO.
I'm happy to do it your way if that's the consensus. I just want everyone
to understand my approach first.
--
Timur Tabi
Linux kernel developer at Freescale
^ permalink raw reply [flat|nested] 32+ messages in thread* [U-Boot] [PATCH 5/6] cmd_nvedit.c: allow board-specific code before/after saving the environment
2012-05-14 16:10 ` Timur Tabi
@ 2012-05-15 5:14 ` Mike Frysinger
2012-05-17 22:18 ` Wolfgang Denk
1 sibling, 0 replies; 32+ messages in thread
From: Mike Frysinger @ 2012-05-15 5:14 UTC (permalink / raw)
To: u-boot
On Monday 14 May 2012 12:10:48 Timur Tabi wrote:
> Mike Frysinger wrote:
> > this is less bloat:
> > int board_start_saveenv(void) __attribute__((weak, alias("saveenv")));
> >
> > int do_env_save(cmd_tbl_t *cmdtp, int flag, int argc, char * const
> > argv[]) {
> >
> > printf("Saving Environment to %s...\n", env_name_spec);
> > return board_saveenv() ? 1 : 0;
> >
> > }
>
> Ah, I see. This forces the board-specific function to call saveenv().
> That gives us more flexibility in the board code.
>
> However, I was trying to mimic what we have in the NAND layer, with
> nand_get_device() and nand_release_device(). That is, before we save the
> environment, we have to "get" it, and then after we save it, we can
> "release" it.
i don't think the nand approach is terribly applicable. different solutions
for different cases.
> Your approach, although it eliminates two weak functions, is not as
> "architecturally clean" as mine, IMHO.
i'd prefer my approach because it keeps down the bloat for the vast majority
of people out there while not restricting boards who want this override
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20120515/8e4e7844/attachment.pgp>
^ permalink raw reply [flat|nested] 32+ messages in thread* [U-Boot] [PATCH 5/6] cmd_nvedit.c: allow board-specific code before/after saving the environment
2012-05-14 16:10 ` Timur Tabi
2012-05-15 5:14 ` Mike Frysinger
@ 2012-05-17 22:18 ` Wolfgang Denk
2012-05-17 22:35 ` Timur Tabi
2012-05-17 22:48 ` Scott Wood
1 sibling, 2 replies; 32+ messages in thread
From: Wolfgang Denk @ 2012-05-17 22:18 UTC (permalink / raw)
To: u-boot
Dear Timur Tabi,
In message <4FB12E88.1050906@freescale.com> you wrote:
>
> > all these board hooks are paper-cutting us to death with unused bloat
>
> I know, and I don't like it either. I hate how our hardware designers are
> always breaking the "rules", forcing us software developers to hack up our
> software more and more. The muxing on this chip is a like a cruel joke
> being played on me. I've even been told that I'm trying too hard to make
> it work.
I think whoever told you this was right. Let it break.
We cannot add pre- and post-hooks all ever the place for brain-dead
designs that need to do this and that before and after doing perfectly
things.
It makes no sense adding this to saveenv, because there will be othe
rplaces in the code that need to to the same - like if it's NAND
flash, you will probabaly need to do the same for all NAND related
commands.
cmd_nvedit.c is definitely the wrong place for this.
Best regards,
Wolfgang Denk
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Bradley's Bromide: If computers get too powerful, we can organize
them into a committee - that will do them in.
^ permalink raw reply [flat|nested] 32+ messages in thread
* [U-Boot] [PATCH 5/6] cmd_nvedit.c: allow board-specific code before/after saving the environment
2012-05-17 22:18 ` Wolfgang Denk
@ 2012-05-17 22:35 ` Timur Tabi
2012-05-18 2:46 ` Mike Frysinger
2012-05-18 11:28 ` Wolfgang Denk
2012-05-17 22:48 ` Scott Wood
1 sibling, 2 replies; 32+ messages in thread
From: Timur Tabi @ 2012-05-17 22:35 UTC (permalink / raw)
To: u-boot
Wolfgang Denk wrote:
> I think whoever told you this was right. Let it break.
Come on, Wolfgang. That's not acceptable.
> We cannot add pre- and post-hooks all ever the place for brain-dead
> designs that need to do this and that before and after doing perfectly
> things.
Well, I already have code in U-boot that does this. If you look at
board/freescale/p1022ds/diu.c, you'll see that I override each of the NOR
flash accessors. This is horribly inefficient, but it works.
Unfortunately, it only covers NOR flash. The new design covers NOR and NAND.
The last two patches of this patchset are a vast improvement, but they
require a board hook (and using Mike's idea, only one hook is necessary,
not two).
As for 'all over the place", I think it's unfair to say my one board hook
function is going to result in "all over the place" hacks.
> It makes no sense adding this to saveenv, because there will be othe
> rplaces in the code that need to to the same - like if it's NAND
> flash, you will probabaly need to do the same for all NAND related
> commands.
Actually, the same code works for saving the environment to NAND flash.
This is how the board code will look:
/*
* While the DIU is active, the localbus is not available. Therefore, in
* order to support the saveenv command to localbus devices, we need to
* temporarily disable the DIU and enable the localbus. To do this, we
* provide our own implementation of board_saveenv(). This function is called
* by do_env_save().
*/
#if defined(CONFIG_ENV_IS_IN_NAND) || defined(CONFIG_ENV_IS_IN_FLASH)
int board_saveenv(void)
{
int switched, ret;
switched = set_mux_to_lbc();
ret = saveenv();
if (switched)
set_mux_to_diu();
return ret;
}
#endif
> cmd_nvedit.c is definitely the wrong place for this.
If you have a better idea, then I'm all ears. I could implement my own
version of saveenv(), but the current design of U-boot does not allow me
to have two functions called saveenv().
--
Timur Tabi
Linux kernel developer at Freescale
^ permalink raw reply [flat|nested] 32+ messages in thread* [U-Boot] [PATCH 5/6] cmd_nvedit.c: allow board-specific code before/after saving the environment
2012-05-17 22:35 ` Timur Tabi
@ 2012-05-18 2:46 ` Mike Frysinger
2012-05-18 11:28 ` Wolfgang Denk
1 sibling, 0 replies; 32+ messages in thread
From: Mike Frysinger @ 2012-05-18 2:46 UTC (permalink / raw)
To: u-boot
On Thursday 17 May 2012 18:35:25 Timur Tabi wrote:
> Well, I already have code in U-boot that does this. If you look at
> board/freescale/p1022ds/diu.c, you'll see that I override each of the NOR
> flash accessors. This is horribly inefficient, but it works.
> Unfortunately, it only covers NOR flash. The new design covers NOR and
> NAND.
oh, i see what you're on about now. we do a similar thing, but to support
some address lines being controlled via GPIOs. you can see what we did here:
board/cm-bf537e/gpio_cfi_flash.c
unfortunately, we run up to the same issue you describe where doing saveenv
doesn't go through the normal flash API, so the lines don't get set right, so
we end up writing to the wrong part of flash.
when i brought this up before, the suggestion was to extend the existing mtd
framework to support this because the existing nor flash framework is supposed
to make the assumption that the entire thing is directly addressable. but i
never found time or inclination, and i'm much less likely to do so today :).
the thread was titled:
flread: new command for reading indirect mapped flashes
and dates June 2009 :)
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20120517/3f8c7f57/attachment.pgp>
^ permalink raw reply [flat|nested] 32+ messages in thread
* [U-Boot] [PATCH 5/6] cmd_nvedit.c: allow board-specific code before/after saving the environment
2012-05-17 22:35 ` Timur Tabi
2012-05-18 2:46 ` Mike Frysinger
@ 2012-05-18 11:28 ` Wolfgang Denk
2012-05-18 15:58 ` Timur Tabi
1 sibling, 1 reply; 32+ messages in thread
From: Wolfgang Denk @ 2012-05-18 11:28 UTC (permalink / raw)
To: u-boot
Dear Timur,
In message <4FB57D2D.3090902@freescale.com> you wrote:
>
> > I think whoever told you this was right. Let it break.
>
> Come on, Wolfgang. That's not acceptable.
Why not? But adding arbitrary complexity and ugliness into U-Boot
mainline is acceptable? Why?
> > We cannot add pre- and post-hooks all ever the place for brain-dead
> > designs that need to do this and that before and after doing perfectly
> > things.
>
> Well, I already have code in U-boot that does this. If you look at
> board/freescale/p1022ds/diu.c, you'll see that I override each of the NOR
> flash accessors. This is horribly inefficient, but it works.
This is already streching the code to the limits, and the only reason
you ever got this thrugh is that it's FSL specific code, you you have
to deal yourself with this ugliness. I don;t think you will find good
arguments to convince me adding such stuff into common code, though.
> Unfortunately, it only covers NOR flash. The new design covers NOR and NAND.
Well, your code does not really cover NOR flash. I think it covers
only a small subset of the use cases, but horribly fails else.
For example, what happens when I just use "md" or "itest *addr" or
similar trying to read NOR flash while the display is on?
You _do_ have a broken hardware design, and if you cannot access NOR
flash with display running, and vice versa, than just accept this
fact.
Feel free to provide commands to switch mode, but don't lard the code
with hooks here and there trying to fix what cannot be fixed.
> The last two patches of this patchset are a vast improvement, but they
> require a board hook (and using Mike's idea, only one hook is necessary,
> not two).
>
> As for 'all over the place", I think it's unfair to say my one board hook
> function is going to result in "all over the place" hacks.
Well, you try again to fix just a single use-case, leaving all the
others unsolved. It makes zero sense to fix "saveenv" while not
fixing all other access modes to the same storage device.
Yes, it is a pain if you have to run something like
=> busmux flash;saveenv;busmux diu
but the ugliness comes from the hardware design, so we have no reasoin
to camouflage it. Let people see what is going on under the hood - if
this is done in a clean way, you don't have to be ashamed.
> > It makes no sense adding this to saveenv, because there will be othe
> > rplaces in the code that need to to the same - like if it's NAND
> > flash, you will probabaly need to do the same for all NAND related
> > commands.
>
> Actually, the same code works for saving the environment to NAND flash.
> This is how the board code will look:
But this again works only for thesingle use case of the "saveenv"
command.
This makes no sense - it's still broken, and trying to fix it using
this approach does not scale and/or does not work.
> If you have a better idea, then I'm all ears. I could implement my own
> version of saveenv(), but the current design of U-boot does not allow me
> to have two functions called saveenv().
Your problem is not just "saveenv". Your problem is that you need to
switch modes. So provide a command to switch modes, and use this.
Yes, this is a pain, and it is less from perfect. But you cannot have
a perfect solution on broken hardware. It makes no sense to waste
lots of efforts on this. If you're ugly, you are still ugly even if
wearing expensive clothes.
Best regards,
Wolfgang Denk
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Keep your head and your heart going in the right direction and you
will not have to worry about your feet.
^ permalink raw reply [flat|nested] 32+ messages in thread
* [U-Boot] [PATCH 5/6] cmd_nvedit.c: allow board-specific code before/after saving the environment
2012-05-18 11:28 ` Wolfgang Denk
@ 2012-05-18 15:58 ` Timur Tabi
2012-05-18 16:02 ` Jeroen Hofstee
` (2 more replies)
0 siblings, 3 replies; 32+ messages in thread
From: Timur Tabi @ 2012-05-18 15:58 UTC (permalink / raw)
To: u-boot
Wolfgang Denk wrote:
> This is already streching the code to the limits, and the only reason
> you ever got this thrugh is that it's FSL specific code, you you have
> to deal yourself with this ugliness. I don;t think you will find good
> arguments to convince me adding such stuff into common code, though.
Well, I guess we'll just have to agree to disagree. I didn't think Mike's
idea was a bad one, but if you don't like it, then I'll drop it.
>> Unfortunately, it only covers NOR flash. The new design covers NOR and NAND.
>
> Well, your code does not really cover NOR flash. I think it covers
> only a small subset of the use cases, but horribly fails else.
>
> For example, what happens when I just use "md" or "itest *addr" or
> similar trying to read NOR flash while the display is on?
The reason I make a big deal about saveenv is because I use an environment
variable ("video-mode") to enable video support. Once the console is
switched to the video display, the only way to switch it back to the
serial port is to delete the environment variable, save the environment,
and reboot.
If we had a command that switched the console back to the serial port, I
wouldn't need any of this.
> You _do_ have a broken hardware design, and if you cannot access NOR
> flash with display running, and vice versa, than just accept this
> fact.
>
> Feel free to provide commands to switch mode, but don't lard the code
> with hooks here and there trying to fix what cannot be fixed.
Is there a way to switch the console
> Well, you try again to fix just a single use-case, leaving all the
> others unsolved. It makes zero sense to fix "saveenv" while not
> fixing all other access modes to the same storage device.
>
> Yes, it is a pain if you have to run something like
>
> => busmux flash;saveenv;busmux diu
Hmmm... that's not a bad idea. I'll think about it.
> but the ugliness comes from the hardware design, so we have no reasoin
> to camouflage it. Let people see what is going on under the hood - if
> this is done in a clean way, you don't have to be ashamed.
LOL.
--
Timur Tabi
Linux kernel developer at Freescale
^ permalink raw reply [flat|nested] 32+ messages in thread* [U-Boot] [PATCH 5/6] cmd_nvedit.c: allow board-specific code before/after saving the environment
2012-05-18 15:58 ` Timur Tabi
@ 2012-05-18 16:02 ` Jeroen Hofstee
2012-05-18 18:24 ` Wolfgang Denk
2012-05-18 18:23 ` Wolfgang Denk
2012-05-18 18:29 ` Fabio Estevam
2 siblings, 1 reply; 32+ messages in thread
From: Jeroen Hofstee @ 2012-05-18 16:02 UTC (permalink / raw)
To: u-boot
On 05/18/2012 05:58 PM, Timur Tabi wrote:
> Is there a way to switch the console
setenv stdout serial
works for me..
Regards,
Jeroen
^ permalink raw reply [flat|nested] 32+ messages in thread
* [U-Boot] [PATCH 5/6] cmd_nvedit.c: allow board-specific code before/after saving the environment
2012-05-18 16:02 ` Jeroen Hofstee
@ 2012-05-18 18:24 ` Wolfgang Denk
0 siblings, 0 replies; 32+ messages in thread
From: Wolfgang Denk @ 2012-05-18 18:24 UTC (permalink / raw)
To: u-boot
Dear Jeroen Hofstee,
In message <4FB672B0.8010506@myspectrum.nl> you wrote:
> On 05/18/2012 05:58 PM, Timur Tabi wrote:
> > Is there a way to switch the console
> setenv stdout serial
>
> works for me..
Hey. Don't share insider information like that ;-)
Best regards,
Wolfgang Denk
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Presidency: The greased pig in the field game of American politics.
- Ambrose Bierce
^ permalink raw reply [flat|nested] 32+ messages in thread
* [U-Boot] [PATCH 5/6] cmd_nvedit.c: allow board-specific code before/after saving the environment
2012-05-18 15:58 ` Timur Tabi
2012-05-18 16:02 ` Jeroen Hofstee
@ 2012-05-18 18:23 ` Wolfgang Denk
2012-05-18 18:29 ` Fabio Estevam
2 siblings, 0 replies; 32+ messages in thread
From: Wolfgang Denk @ 2012-05-18 18:23 UTC (permalink / raw)
To: u-boot
Dear Timur,
In message <4FB67191.4030000@freescale.com> you wrote:
>
> > For example, what happens when I just use "md" or "itest *addr" or
> > similar trying to read NOR flash while the display is on?
>
> The reason I make a big deal about saveenv is because I use an environment
> variable ("video-mode") to enable video support. Once the console is
> switched to the video display, the only way to switch it back to the
> serial port is to delete the environment variable, save the environment,
> and reboot.
Please try to look over the rim of your plate. Even if you make
"saveenv" work with some magic, your end users may have a zillion
other ideas why they want to access the flash - for example, to store
a copy of the environment created using "env export", or to reload
such a saved profile using "env import". Why should their needs be
ignored?
> > Feel free to provide commands to switch mode, but don't lard the code
> > with hooks here and there trying to fix what cannot be fixed.
>
> Is there a way to switch the console
Sure. Delete the env var, switch mode to flash access, run saveenv,
run reset.
Best regards,
Wolfgang Denk
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Often it is fatal to live too long. - Racine
^ permalink raw reply [flat|nested] 32+ messages in thread* [U-Boot] [PATCH 5/6] cmd_nvedit.c: allow board-specific code before/after saving the environment
2012-05-18 15:58 ` Timur Tabi
2012-05-18 16:02 ` Jeroen Hofstee
2012-05-18 18:23 ` Wolfgang Denk
@ 2012-05-18 18:29 ` Fabio Estevam
2 siblings, 0 replies; 32+ messages in thread
From: Fabio Estevam @ 2012-05-18 18:29 UTC (permalink / raw)
To: u-boot
On Fri, May 18, 2012 at 12:58 PM, Timur Tabi <timur@freescale.com> wrote:
> The reason I make a big deal about saveenv is because I use an environment
> variable ("video-mode") to enable video support. ?Once the console is
> switched to the video display, the only way to switch it back to the
> serial port is to delete the environment variable, save the environment,
> and reboot.
>
> If we had a command that switched the console back to the serial port, I
> wouldn't need any of this.
Yes, I noticed it last week when working with mx51evk.
The suggestion I got from the list and that worked for me was to force
setenv("stdout", "serial"); inside board_late_init.
Take a look at:
https://github.com/fabioestevam/u-boot-imx/commit/bbd9e6eac8dfbce2fe64e84f01c292ab708124b5
^ permalink raw reply [flat|nested] 32+ messages in thread
* [U-Boot] [PATCH 5/6] cmd_nvedit.c: allow board-specific code before/after saving the environment
2012-05-17 22:18 ` Wolfgang Denk
2012-05-17 22:35 ` Timur Tabi
@ 2012-05-17 22:48 ` Scott Wood
2012-05-17 22:53 ` Timur Tabi
1 sibling, 1 reply; 32+ messages in thread
From: Scott Wood @ 2012-05-17 22:48 UTC (permalink / raw)
To: u-boot
On 05/17/2012 05:18 PM, Wolfgang Denk wrote:
> Dear Timur Tabi,
>
> In message <4FB12E88.1050906@freescale.com> you wrote:
>>
>>> all these board hooks are paper-cutting us to death with unused bloat
>>
>> I know, and I don't like it either. I hate how our hardware designers are
>> always breaking the "rules", forcing us software developers to hack up our
>> software more and more. The muxing on this chip is a like a cruel joke
>> being played on me. I've even been told that I'm trying too hard to make
>> it work.
>
> I think whoever told you this was right. Let it break.
>
> We cannot add pre- and post-hooks all ever the place for brain-dead
> designs that need to do this and that before and after doing perfectly
> things.
>
> It makes no sense adding this to saveenv, because there will be othe
> rplaces in the code that need to to the same - like if it's NAND
> flash, you will probabaly need to do the same for all NAND related
> commands.
NAND doesn't need it because NAND goes through an API rather than direct
memory-mapped access, and has more coarse-grained operations. NAND
should be able to take care of this entirely in the driver using the
select_chip() callback.
Timur, is there any reason to use NOR rather than NAND with this chip?
-Scott
^ permalink raw reply [flat|nested] 32+ messages in thread
* [U-Boot] [PATCH 5/6] cmd_nvedit.c: allow board-specific code before/after saving the environment
2012-05-17 22:48 ` Scott Wood
@ 2012-05-17 22:53 ` Timur Tabi
2012-05-18 2:14 ` Scott Wood
0 siblings, 1 reply; 32+ messages in thread
From: Timur Tabi @ 2012-05-17 22:53 UTC (permalink / raw)
To: u-boot
Scott Wood wrote:
> NAND doesn't need it because NAND goes through an API rather than direct
> memory-mapped access, and has more coarse-grained operations. NAND
> should be able to take care of this entirely in the driver using the
> select_chip() callback.
Fair enough. How do I enable that feature? Do I create my own
board_nand_init() and then do this:
this->select_chip = p1022ds_nand_select_chip;
> Timur, is there any reason to use NOR rather than NAND with this chip?
Well, as with most of our boards, NOR is the default configuration. Also,
there's no NAND support upstream yet.
--
Timur Tabi
Linux kernel developer at Freescale
^ permalink raw reply [flat|nested] 32+ messages in thread
* [U-Boot] [PATCH 5/6] cmd_nvedit.c: allow board-specific code before/after saving the environment
2012-05-17 22:53 ` Timur Tabi
@ 2012-05-18 2:14 ` Scott Wood
2012-05-18 2:21 ` Tabi Timur-B04825
0 siblings, 1 reply; 32+ messages in thread
From: Scott Wood @ 2012-05-18 2:14 UTC (permalink / raw)
To: u-boot
On 05/17/2012 05:53 PM, Timur Tabi wrote:
> Scott Wood wrote:
>> NAND doesn't need it because NAND goes through an API rather than direct
>> memory-mapped access, and has more coarse-grained operations. NAND
>> should be able to take care of this entirely in the driver using the
>> select_chip() callback.
>
> Fair enough. How do I enable that feature? Do I create my own
> board_nand_init() and then do this:
>
> this->select_chip = p1022ds_nand_select_chip;
board_nand_init() is implemented in drivers/mtd/nand/fsl_elbc_nand.c.
You'll need to add some board hook there.
>> Timur, is there any reason to use NOR rather than NAND with this chip?
>
> Well, as with most of our boards, NOR is the default configuration. Also,
> there's no NAND support upstream yet.
What isn't upstream, besides the muxing hack? Does it need the 4K page
hack?
-Scott
^ permalink raw reply [flat|nested] 32+ messages in thread
* [U-Boot] [PATCH 5/6] cmd_nvedit.c: allow board-specific code before/after saving the environment
2012-05-18 2:14 ` Scott Wood
@ 2012-05-18 2:21 ` Tabi Timur-B04825
2012-05-18 2:30 ` Scott Wood
0 siblings, 1 reply; 32+ messages in thread
From: Tabi Timur-B04825 @ 2012-05-18 2:21 UTC (permalink / raw)
To: u-boot
Scott Wood wrote:
>> > Well, as with most of our boards, NOR is the default configuration. Also,
>> > there's no NAND support upstream yet.
> What isn't upstream, besides the muxing hack? Does it need the 4K page
> hack?
There's no NAND support at all. However, I just tried the two SDK patches
that add it, and they apply cleanly, so that's an easy fix.
I guess we can drop patches #5 and #6, and just use the existing code to
support NOR flash. I'll add NAND flash support.
--
Timur Tabi
Linux kernel developer at Freescale
^ permalink raw reply [flat|nested] 32+ messages in thread
* [U-Boot] [PATCH 5/6] cmd_nvedit.c: allow board-specific code before/after saving the environment
2012-05-18 2:21 ` Tabi Timur-B04825
@ 2012-05-18 2:30 ` Scott Wood
2012-05-18 16:00 ` Timur Tabi
0 siblings, 1 reply; 32+ messages in thread
From: Scott Wood @ 2012-05-18 2:30 UTC (permalink / raw)
To: u-boot
On 05/17/2012 09:21 PM, Tabi Timur-B04825 wrote:
> Scott Wood wrote:
>>>> Well, as with most of our boards, NOR is the default configuration. Also,
>>>> there's no NAND support upstream yet.
>> What isn't upstream, besides the muxing hack? Does it need the 4K page
>> hack?
>
> There's no NAND support at all.
Of course there's NAND support. I was asking what, besides the mux,
makes that existing support not work on this board.
> However, I just tried the two SDK patches
> that add it, and they apply cleanly, so that's an easy fix.
Which patches are those?
-Scott
^ permalink raw reply [flat|nested] 32+ messages in thread
* [U-Boot] [PATCH 5/6] cmd_nvedit.c: allow board-specific code before/after saving the environment
2012-05-18 2:30 ` Scott Wood
@ 2012-05-18 16:00 ` Timur Tabi
2012-05-18 16:13 ` Scott Wood
0 siblings, 1 reply; 32+ messages in thread
From: Timur Tabi @ 2012-05-18 16:00 UTC (permalink / raw)
To: u-boot
Scott Wood wrote:
>> There's no NAND support at all.
> Of course there's NAND support. I was asking what, besides the mux,
> makes that existing support not work on this board.
No, there is no NAND support for the P1022DS upstream.
[b04825 at efes u-boot.0]$ grep -i nand include/configs/P1022DS.h
[b04825 at efes u-boot.0]$
>> > However, I just tried the two SDK patches
>> > that add it, and they apply cleanly, so that's an easy fix.
> Which patches are those?
powerpc/85xx: add SPI and SD builds for P1022DS
powerpc/p1022ds: Add support for NAND and NAND boot
--
Timur Tabi
Linux kernel developer at Freescale
^ permalink raw reply [flat|nested] 32+ messages in thread* [U-Boot] [PATCH 5/6] cmd_nvedit.c: allow board-specific code before/after saving the environment
2012-05-18 16:00 ` Timur Tabi
@ 2012-05-18 16:13 ` Scott Wood
2012-05-18 16:17 ` Timur Tabi
0 siblings, 1 reply; 32+ messages in thread
From: Scott Wood @ 2012-05-18 16:13 UTC (permalink / raw)
To: u-boot
On 05/18/2012 11:00 AM, Timur Tabi wrote:
> Scott Wood wrote:
>>> There's no NAND support at all.
>> Of course there's NAND support. I was asking what, besides the mux,
>> makes that existing support not work on this board.
>
> No, there is no NAND support for the P1022DS upstream.
>
> [b04825 at efes u-boot.0]$ grep -i nand include/configs/P1022DS.h
> [b04825 at efes u-boot.0]$
That's the equivalent of saying Linux doesn't support something because
nobody bothered to enable it in a certain defconfig.
>>>> However, I just tried the two SDK patches
>>>> that add it, and they apply cleanly, so that's an easy fix.
>> Which patches are those?
>
> powerpc/85xx: add SPI and SD builds for P1022DS
> powerpc/p1022ds: Add support for NAND and NAND boot
I'm not sure what SPI and SD have to do with it...
Most of the latter patch is concerned with NAND boot, which is a
different issue from NAND support, but still pretty important if you're
going NAND-only. It looks like the patches were actually initially
separate, but Kumar oh-so-helpfully squashed them together.
One thing I would like to see fixed in at upstream version of p1022ds
NAND boot support is for it to use SPD like a normal p1022ds boot. This
will likely require reviving the three-stage boot discussion (TPL).
-Scott
^ permalink raw reply [flat|nested] 32+ messages in thread
* [U-Boot] [PATCH 5/6] cmd_nvedit.c: allow board-specific code before/after saving the environment
2012-05-18 16:13 ` Scott Wood
@ 2012-05-18 16:17 ` Timur Tabi
2012-05-18 16:29 ` Scott Wood
0 siblings, 1 reply; 32+ messages in thread
From: Timur Tabi @ 2012-05-18 16:17 UTC (permalink / raw)
To: u-boot
Scott Wood wrote:
> That's the equivalent of saying Linux doesn't support something because
> nobody bothered to enable it in a certain defconfig.
Well, that's exactly what I meant. When you boot an upstream U-boot on a
P1022DS, there is no support for NAND chips. The 'nand' command does not
exist. You cannot build a u-boot.bin that will boot from NAND. That
pretty much means "there is no NAND support".
>>>>> However, I just tried the two SDK patches
>>>>> that add it, and they apply cleanly, so that's an easy fix.
>>> Which patches are those?
>>
>> powerpc/85xx: add SPI and SD builds for P1022DS
>> powerpc/p1022ds: Add support for NAND and NAND boot
>
> I'm not sure what SPI and SD have to do with it...
The 2nd patch applies on top of the first.
> Most of the latter patch is concerned with NAND boot, which is a
> different issue from NAND support, but still pretty important if you're
> going NAND-only. It looks like the patches were actually initially
> separate, but Kumar oh-so-helpfully squashed them together.
It's a good thing we don't do that any more.
> One thing I would like to see fixed in at upstream version of p1022ds
> NAND boot support is for it to use SPD like a normal p1022ds boot. This
> will likely require reviving the three-stage boot discussion (TPL).
I just posted those two patches for upstream. I don't want the TPL work
to hold up these patches.
--
Timur Tabi
Linux kernel developer at Freescale
^ permalink raw reply [flat|nested] 32+ messages in thread
* [U-Boot] [PATCH 5/6] cmd_nvedit.c: allow board-specific code before/after saving the environment
2012-05-18 16:17 ` Timur Tabi
@ 2012-05-18 16:29 ` Scott Wood
2012-05-18 17:08 ` Timur Tabi
2012-05-18 18:28 ` Wolfgang Denk
0 siblings, 2 replies; 32+ messages in thread
From: Scott Wood @ 2012-05-18 16:29 UTC (permalink / raw)
To: u-boot
On 05/18/2012 11:17 AM, Timur Tabi wrote:
> Scott Wood wrote:
>
>> That's the equivalent of saying Linux doesn't support something because
>> nobody bothered to enable it in a certain defconfig.
>
> Well, that's exactly what I meant. When you boot an upstream U-boot on a
> P1022DS, there is no support for NAND chips. The 'nand' command does not
> exist. You cannot build a u-boot.bin that will boot from NAND. That
> pretty much means "there is no NAND support".
I was under the impression that this was a developer discussion about
what it would take to get this working, not an end user support forum.
>> One thing I would like to see fixed in at upstream version of p1022ds
>> NAND boot support is for it to use SPD like a normal p1022ds boot. This
>> will likely require reviving the three-stage boot discussion (TPL).
>
> I just posted those two patches for upstream. I don't want the TPL work
> to hold up these patches.
It was over a year ago that I made that request internally. And still
the answer is "I need this now now now!".
NACK any non-SPD NAND boot for a board that otherwise uses SPD,
particularly if it has socketed RAM. It's not as if we don't know how
to make this work.
Wolfgang will also probably object to adding another board to the old
nand_spl infrastrucutre.
-Scott
^ permalink raw reply [flat|nested] 32+ messages in thread
* [U-Boot] [PATCH 5/6] cmd_nvedit.c: allow board-specific code before/after saving the environment
2012-05-18 16:29 ` Scott Wood
@ 2012-05-18 17:08 ` Timur Tabi
2012-05-18 17:21 ` Scott Wood
2012-05-18 18:28 ` Wolfgang Denk
1 sibling, 1 reply; 32+ messages in thread
From: Timur Tabi @ 2012-05-18 17:08 UTC (permalink / raw)
To: u-boot
Scott Wood wrote:
> It was over a year ago that I made that request internally. And still
> the answer is "I need this now now now!".
Who's going to do that work, if not you?
> NACK any non-SPD NAND boot for a board that otherwise uses SPD,
> particularly if it has socketed RAM. It's not as if we don't know how
> to make this work.
Well, *I* don't know how to do that.
--
Timur Tabi
Linux kernel developer at Freescale
^ permalink raw reply [flat|nested] 32+ messages in thread
* [U-Boot] [PATCH 5/6] cmd_nvedit.c: allow board-specific code before/after saving the environment
2012-05-18 17:08 ` Timur Tabi
@ 2012-05-18 17:21 ` Scott Wood
2012-05-18 18:13 ` McClintock Matthew-B29882
0 siblings, 1 reply; 32+ messages in thread
From: Scott Wood @ 2012-05-18 17:21 UTC (permalink / raw)
To: u-boot
On 05/18/2012 12:08 PM, Timur Tabi wrote:
> Scott Wood wrote:
>> It was over a year ago that I made that request internally. And still
>> the answer is "I need this now now now!".
>
> Who's going to do that work, if not you?
Matthew had something working a while ago, I thought.
>> NACK any non-SPD NAND boot for a board that otherwise uses SPD,
>> particularly if it has socketed RAM. It's not as if we don't know how
>> to make this work.
>
> Well, *I* don't know how to do that.
Use a three stage boot where the middle stage runs out of L2 SRAM.
-Scott
^ permalink raw reply [flat|nested] 32+ messages in thread
* [U-Boot] [PATCH 5/6] cmd_nvedit.c: allow board-specific code before/after saving the environment
2012-05-18 17:21 ` Scott Wood
@ 2012-05-18 18:13 ` McClintock Matthew-B29882
0 siblings, 0 replies; 32+ messages in thread
From: McClintock Matthew-B29882 @ 2012-05-18 18:13 UTC (permalink / raw)
To: u-boot
On Fri, May 18, 2012 at 12:21 PM, Scott Wood <scottwood@freescale.com> wrote:
> On 05/18/2012 12:08 PM, Timur Tabi wrote:
>> Scott Wood wrote:
>>> It was over a year ago that I made that request internally. ?And still
>>> the answer is "I need this now now now!".
>>
>> Who's going to do that work, if not you?
>
> Matthew had something working a while ago, I thought.
>
>>> NACK any non-SPD NAND boot for a board that otherwise uses SPD,
>>> particularly if it has socketed RAM. ?It's not as if we don't know how
>>> to make this work.
>>
>> Well, *I* don't know how to do that.
>
> Use a three stage boot where the middle stage runs out of L2 SRAM.
Then upstream u-boot reworked SPL so we need to rework using that
implementation.
-M
^ permalink raw reply [flat|nested] 32+ messages in thread
* [U-Boot] [PATCH 5/6] cmd_nvedit.c: allow board-specific code before/after saving the environment
2012-05-18 16:29 ` Scott Wood
2012-05-18 17:08 ` Timur Tabi
@ 2012-05-18 18:28 ` Wolfgang Denk
1 sibling, 0 replies; 32+ messages in thread
From: Wolfgang Denk @ 2012-05-18 18:28 UTC (permalink / raw)
To: u-boot
Dear Scott,
In message <4FB678CF.4030501@freescale.com> you wrote:
>
> Wolfgang will also probably object to adding another board to the old
> nand_spl infrastrucutre.
True. This gives a NAK reliably.
Best regards,
Wolfgang Denk
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
To be is to program.
^ permalink raw reply [flat|nested] 32+ messages in thread
* [U-Boot] [PATCH 5/6] cmd_nvedit.c: allow board-specific code before/after saving the environment
2012-05-14 5:20 ` Mike Frysinger
2012-05-14 16:10 ` Timur Tabi
@ 2012-05-17 21:18 ` Timur Tabi
1 sibling, 0 replies; 32+ messages in thread
From: Timur Tabi @ 2012-05-17 21:18 UTC (permalink / raw)
To: u-boot
Mike Frysinger wrote:
> this is less bloat:
> int board_saveenv(void) __attribute__((weak, alias("saveenv")));
>
> int do_env_save(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
> {
> printf("Saving Environment to %s...\n", env_name_spec);
> return board_saveenv() ? 1 : 0;
> }
I don't think this can work:
cmd_nvedit.c:599:5: error: 'board_saveenv' aliased to undefined symbol
'saveenv'
cmd_nvedit.c:599:5: error: 'board_saveenv' aliased to undefined symbol
'saveenv'
make[1]: *** [/home/b04825/git/u-boot.0/1022/common/cmd_nvedit.o] Error 1
It looks like weak functions can't be in another source file?
If I change it to this, then it works:
static int __saveenv(void)
{
return saveenv();
}
int board_saveenv(void) __attribute__((weak, alias("__saveenv")));
int do_env_save(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
{
printf("Saving Environment to %s...\n", env_name_spec);
return board_saveenv() ? 1 : 0;
}
--
Timur Tabi
Linux kernel developer at Freescale
^ permalink raw reply [flat|nested] 32+ messages in thread
* [U-Boot] [PATCH 6/6] powerpc/85xx: p1022ds: use the saveenv board preparation functions
2012-05-04 22:21 [U-Boot] [PATCH 1/6] powerpc/85xx: minor clean-ups to the P2020DS board header file Timur Tabi
` (3 preceding siblings ...)
2012-05-04 22:21 ` [U-Boot] [PATCH 5/6] cmd_nvedit.c: allow board-specific code before/after saving the environment Timur Tabi
@ 2012-05-04 22:21 ` Timur Tabi
4 siblings, 0 replies; 32+ messages in thread
From: Timur Tabi @ 2012-05-04 22:21 UTC (permalink / raw)
To: u-boot
On the P1022, the DIU video signals are muxed with the localbus, so when
the DIU is active, the localbus is unavailable. The saveenv command
now supports board-specific functions that allow the DIU/LBC mux to
be switched while saving the environment.
This is much more efficient than switching the mux for each NOR flash
transaction (which is what the current code does), and it also allows
us to support any localbus device, not just NOR flash.
Signed-off-by: Timur Tabi <timur@freescale.com>
---
board/freescale/p1022ds/diu.c | 173 +++++++----------------------------------
include/configs/P1022DS.h | 6 --
2 files changed, 28 insertions(+), 151 deletions(-)
diff --git a/board/freescale/p1022ds/diu.c b/board/freescale/p1022ds/diu.c
index d5428ea..1dd0e1f 100644
--- a/board/freescale/p1022ds/diu.c
+++ b/board/freescale/p1022ds/diu.c
@@ -147,10 +147,7 @@ int platform_diu_init(unsigned int xres, unsigned int yres, const char *port)
*
* On the Freescale P1022, the DIU video signal and the LBC address/data lines
* share the same pins, which means that when the DIU is active (e.g. the
- * console is on the DVI display), NOR flash cannot be accessed. So we use the
- * weak accessor feature of the CFI flash code to temporarily switch the pin
- * mux from DIU to LBC whenever we want to read or write flash. This has a
- * significant performance penalty, but it's the only way to make it work.
+ * console is on the DVI display), NOR flash cannot be accessed.
*
* There are two muxes: one on the chip, and one on the board. The chip mux
* controls whether the pins are used for the DIU or the LBC, and it is
@@ -213,6 +210,33 @@ static void set_mux_to_diu(void)
}
/*
+ * While the DIU is active, the localbus is not available. Therefore, in
+ * order to support the saveenv command, we need to temporarily disable the
+ * DIU and enable the localbus. To do this, we provide our own
+ * implementations of the board_start_saveenv() and board_finish_saveenv()
+ * weak functions. These functions are called by do_env_save() before and
+ * after the environment is saved.
+ */
+
+/* Remember if we switched the MUX, so that we know to switch it back */
+static int switched;
+
+int board_start_saveenv(void)
+{
+ switched = set_mux_to_lbc();
+
+ return 0;
+}
+
+void board_finish_saveenv(void)
+{
+ if (switched)
+ set_mux_to_diu();
+
+ switched = 0;
+}
+
+/*
* pixis_read - board-specific function to read from the PIXIS
*
* This function overrides the generic pixis_read() function, so that it can
@@ -271,144 +295,3 @@ void pixis_bank_reset(void)
while (1);
}
-
-#ifdef CONFIG_CFI_FLASH_USE_WEAK_ACCESSORS
-
-void flash_write8(u8 value, void *addr)
-{
- int sw = set_mux_to_lbc();
-
- __raw_writeb(value, addr);
- if (sw) {
- /*
- * To ensure the post-write is completed to eLBC, software must
- * perform a dummy read from one valid address from eLBC space
- * before changing the eLBC_DIU from NOR mode to DIU mode.
- * set_mux_to_diu() includes a sync that will ensure the
- * __raw_readb() completes before it switches the mux.
- */
- __raw_readb(addr);
- set_mux_to_diu();
- }
-}
-
-void flash_write16(u16 value, void *addr)
-{
- int sw = set_mux_to_lbc();
-
- __raw_writew(value, addr);
- if (sw) {
- /*
- * To ensure the post-write is completed to eLBC, software must
- * perform a dummy read from one valid address from eLBC space
- * before changing the eLBC_DIU from NOR mode to DIU mode.
- * set_mux_to_diu() includes a sync that will ensure the
- * __raw_readb() completes before it switches the mux.
- */
- __raw_readb(addr);
- set_mux_to_diu();
- }
-}
-
-void flash_write32(u32 value, void *addr)
-{
- int sw = set_mux_to_lbc();
-
- __raw_writel(value, addr);
- if (sw) {
- /*
- * To ensure the post-write is completed to eLBC, software must
- * perform a dummy read from one valid address from eLBC space
- * before changing the eLBC_DIU from NOR mode to DIU mode.
- * set_mux_to_diu() includes a sync that will ensure the
- * __raw_readb() completes before it switches the mux.
- */
- __raw_readb(addr);
- set_mux_to_diu();
- }
-}
-
-void flash_write64(u64 value, void *addr)
-{
- int sw = set_mux_to_lbc();
- uint32_t *p = addr;
-
- /*
- * There is no __raw_writeq(), so do the write manually. We don't trust
- * the compiler, so we use inline assembly.
- */
- __asm__ __volatile__(
- "stw%U0%X0 %2,%0;\n"
- "stw%U1%X1 %3,%1;\n"
- : "=m" (*p), "=m" (*(p + 1))
- : "r" ((uint32_t) (value >> 32)), "r" ((uint32_t) (value)));
-
- if (sw) {
- /*
- * To ensure the post-write is completed to eLBC, software must
- * perform a dummy read from one valid address from eLBC space
- * before changing the eLBC_DIU from NOR mode to DIU mode. We
- * read addr+4 because we just wrote to addr+4, so that's how we
- * maintain execution order. set_mux_to_diu() includes a sync
- * that will ensure the __raw_readb() completes before it
- * switches the mux.
- */
- __raw_readb(addr + 4);
- set_mux_to_diu();
- }
-}
-
-u8 flash_read8(void *addr)
-{
- u8 ret;
-
- int sw = set_mux_to_lbc();
-
- ret = __raw_readb(addr);
- if (sw)
- set_mux_to_diu();
-
- return ret;
-}
-
-u16 flash_read16(void *addr)
-{
- u16 ret;
-
- int sw = set_mux_to_lbc();
-
- ret = __raw_readw(addr);
- if (sw)
- set_mux_to_diu();
-
- return ret;
-}
-
-u32 flash_read32(void *addr)
-{
- u32 ret;
-
- int sw = set_mux_to_lbc();
-
- ret = __raw_readl(addr);
- if (sw)
- set_mux_to_diu();
-
- return ret;
-}
-
-u64 flash_read64(void *addr)
-{
- u64 ret;
-
- int sw = set_mux_to_lbc();
-
- /* There is no __raw_readq(), so do the read manually */
- ret = *(volatile u64 *)addr;
- if (sw)
- set_mux_to_diu();
-
- return ret;
-}
-
-#endif
diff --git a/include/configs/P1022DS.h b/include/configs/P1022DS.h
index 9d2f8ea..89e8663 100644
--- a/include/configs/P1022DS.h
+++ b/include/configs/P1022DS.h
@@ -206,12 +206,6 @@
#define CONFIG_VGA_AS_SINGLE_DEVICE
#define CONFIG_VIDEO_LOGO
#define CONFIG_VIDEO_BMP_LOGO
-#define CONFIG_CFI_FLASH_USE_WEAK_ACCESSORS
-/*
- * With CONFIG_CFI_FLASH_USE_WEAK_ACCESSORS, flash I/O is really slow, so
- * disable empty flash sector detection, which is I/O-intensive.
- */
-#undef CONFIG_SYS_FLASH_EMPTY_INFO
#endif
#ifndef CONFIG_FSL_DIU_FB
--
1.7.3.4
^ permalink raw reply related [flat|nested] 32+ messages in thread