| Message ID | 1386296295-28658-3-git-send-email-dennis@ausil.us |
|---|---|
| State | Superseded |
| Delegated to: | Tom Rini |
| Headers | show |
On Thu, Dec 5, 2013 at 8:18 PM, Dennis Gilmore <dennis@ausil.us> wrote: > Signed-off-by: Dennis Gilmore <dennis@ausil.us> > --- > include/configs/wandboard.h | 40 +++++++++++++++++++++++++++++++++++----- > 1 file changed, 35 insertions(+), 5 deletions(-) > > diff --git a/include/configs/wandboard.h b/include/configs/wandboard.h > index e9c7e64..02d8968 100644 > --- a/include/configs/wandboard.h > +++ b/include/configs/wandboard.h > @@ -40,6 +40,9 @@ > #define CONFIG_CONS_INDEX 1 > #define CONFIG_BAUDRATE 115200 > > +/* enable generic distro config */ > +#define DISTRO_DEFAULTS 1 > + > /* Command definition */ > #include <config_cmd_default.h> > > @@ -48,7 +51,6 @@ > #define CONFIG_CMD_BMODE > #define CONFIG_CMD_SETEXPR > > -#define CONFIG_BOOTDELAY 5 > > #define CONFIG_SYS_MEMTEST_START 0x10000000 > #define CONFIG_SYS_MEMTEST_END (CONFIG_SYS_MEMTEST_START + 500 * SZ_1M) > @@ -65,6 +67,9 @@ > #define CONFIG_CMD_MMC > #define CONFIG_GENERIC_MMC > #define CONFIG_BOUNCE_BUFFER > + > +#define CONFIG_BOOTDELAY 5 > + > #define CONFIG_CMD_EXT2 > #define CONFIG_CMD_FAT > #define CONFIG_DOS_PARTITION > @@ -74,6 +79,11 @@ > #define CONFIG_CMD_DHCP > #define CONFIG_CMD_MII > #define CONFIG_CMD_NET > + > +#define CONFIG_OF_LIBFDT > +#define CONFIG_CMD_BOOTZ > + > +/* Ethernet Configuration */ > #define CONFIG_FEC_MXC > #define CONFIG_MII > #define IMX_FEC_BASE ENET_BASE_ADDR > @@ -113,8 +123,30 @@ > "fdt_high=0xffffffff\0" \ > "initrd_high=0xffffffff\0" \ > "fdt_file=" CONFIG_DEFAULT_FDT_FILE "\0" \ > - "fdt_addr=0x11000000\0" \ > + "fdt_addr=0x11100000\0" \ > + "fdt_addr_r=0x11200000\0" \ > + "pxefile_addr_r=0x11300000\0" \ > + "scr_addr_r=0x11400000\0" \ > + "kernel_addr_r=0x11500000\0" \ > + "ramdisk_addr_r=0x13500000\0" \ > "boot_fdt=try\0" \ > + "bootcmd_setup=mmc rescan\0" \ > + "bootcmd_pxe=setenv bootfile \"\" ;dhcp; tftp ${fdt_addr} /dtb/${fdt_file}; pxe get; pxe boot\0" \ > + "bootcmd_disk_scr=ext2load ${boot_ifc} ${bootdevice} ${scr_addr_r} boot.scr && Why all the non-generic "ext2load"'s? use just "load", then it'll work with both fat, ext2/3/4 & btrfs... Regards,
El Thu, 5 Dec 2013 21:47:43 -0600 Robert Nelson <robertcnelson@gmail.com> escribió: > On Thu, Dec 5, 2013 at 8:18 PM, Dennis Gilmore <dennis@ausil.us> > wrote: > > Signed-off-by: Dennis Gilmore <dennis@ausil.us> > > --- > > include/configs/wandboard.h | 40 > > +++++++++++++++++++++++++++++++++++----- 1 file changed, 35 > > insertions(+), 5 deletions(-) > > > > diff --git a/include/configs/wandboard.h > > b/include/configs/wandboard.h index e9c7e64..02d8968 100644 > > --- a/include/configs/wandboard.h > > +++ b/include/configs/wandboard.h > > @@ -40,6 +40,9 @@ > > #define CONFIG_CONS_INDEX 1 > > #define CONFIG_BAUDRATE 115200 > > > > +/* enable generic distro config */ > > +#define DISTRO_DEFAULTS 1 > > + > > /* Command definition */ > > #include <config_cmd_default.h> > > > > @@ -48,7 +51,6 @@ > > #define CONFIG_CMD_BMODE > > #define CONFIG_CMD_SETEXPR > > > > -#define CONFIG_BOOTDELAY 5 > > > > #define CONFIG_SYS_MEMTEST_START 0x10000000 > > #define CONFIG_SYS_MEMTEST_END (CONFIG_SYS_MEMTEST_START + > > 500 * SZ_1M) @@ -65,6 +67,9 @@ > > #define CONFIG_CMD_MMC > > #define CONFIG_GENERIC_MMC > > #define CONFIG_BOUNCE_BUFFER > > + > > +#define CONFIG_BOOTDELAY 5 > > + > > #define CONFIG_CMD_EXT2 > > #define CONFIG_CMD_FAT > > #define CONFIG_DOS_PARTITION > > @@ -74,6 +79,11 @@ > > #define CONFIG_CMD_DHCP > > #define CONFIG_CMD_MII > > #define CONFIG_CMD_NET > > + > > +#define CONFIG_OF_LIBFDT > > +#define CONFIG_CMD_BOOTZ > > + > > +/* Ethernet Configuration */ > > #define CONFIG_FEC_MXC > > #define CONFIG_MII > > #define IMX_FEC_BASE ENET_BASE_ADDR > > @@ -113,8 +123,30 @@ > > "fdt_high=0xffffffff\0" \ > > "initrd_high=0xffffffff\0" \ > > "fdt_file=" CONFIG_DEFAULT_FDT_FILE "\0" \ > > - "fdt_addr=0x11000000\0" \ > > + "fdt_addr=0x11100000\0" \ > > + "fdt_addr_r=0x11200000\0" \ > > + "pxefile_addr_r=0x11300000\0" \ > > + "scr_addr_r=0x11400000\0" \ > > + "kernel_addr_r=0x11500000\0" \ > > + "ramdisk_addr_r=0x13500000\0" \ > > "boot_fdt=try\0" \ > > + "bootcmd_setup=mmc rescan\0" \ > > + "bootcmd_pxe=setenv bootfile \"\" ;dhcp; tftp > > ${fdt_addr} /dtb/${fdt_file}; pxe get; pxe boot\0" \ > > + "bootcmd_disk_scr=ext2load ${boot_ifc} ${bootdevice} > > ${scr_addr_r} boot.scr && > > Why all the non-generic "ext2load"'s? use just "load", then it'll work > with both fat, ext2/3/4 & btrfs... > > Regards, the pxe code requires you specify the filesystem for the sysboot command, and as /boot needs to be ext ive kept it consistent. Ive also not narrowed down the option to enable load to work, in my testing it did not work. Dennis
El Thu, 5 Dec 2013 23:01:13 -0600 Dennis Gilmore <dennis@ausil.us> escribió: > El Thu, 5 Dec 2013 21:47:43 -0600 > Robert Nelson <robertcnelson@gmail.com> escribió: > > On Thu, Dec 5, 2013 at 8:18 PM, Dennis Gilmore <dennis@ausil.us> > > wrote: > > > Signed-off-by: Dennis Gilmore <dennis@ausil.us> > > > --- > > > include/configs/wandboard.h | 40 > > > +++++++++++++++++++++++++++++++++++----- 1 file changed, 35 > > > insertions(+), 5 deletions(-) > > > > > > diff --git a/include/configs/wandboard.h > > > b/include/configs/wandboard.h index e9c7e64..02d8968 100644 > > > --- a/include/configs/wandboard.h > > > +++ b/include/configs/wandboard.h > > > @@ -40,6 +40,9 @@ > > > #define CONFIG_CONS_INDEX 1 > > > #define CONFIG_BAUDRATE 115200 > > > > > > +/* enable generic distro config */ > > > +#define DISTRO_DEFAULTS 1 > > > + > > > /* Command definition */ > > > #include <config_cmd_default.h> > > > > > > @@ -48,7 +51,6 @@ > > > #define CONFIG_CMD_BMODE > > > #define CONFIG_CMD_SETEXPR > > > > > > -#define CONFIG_BOOTDELAY 5 > > > > > > #define CONFIG_SYS_MEMTEST_START 0x10000000 > > > #define CONFIG_SYS_MEMTEST_END (CONFIG_SYS_MEMTEST_START > > > + 500 * SZ_1M) @@ -65,6 +67,9 @@ > > > #define CONFIG_CMD_MMC > > > #define CONFIG_GENERIC_MMC > > > #define CONFIG_BOUNCE_BUFFER > > > + > > > +#define CONFIG_BOOTDELAY 5 > > > + > > > #define CONFIG_CMD_EXT2 > > > #define CONFIG_CMD_FAT > > > #define CONFIG_DOS_PARTITION > > > @@ -74,6 +79,11 @@ > > > #define CONFIG_CMD_DHCP > > > #define CONFIG_CMD_MII > > > #define CONFIG_CMD_NET > > > + > > > +#define CONFIG_OF_LIBFDT > > > +#define CONFIG_CMD_BOOTZ > > > + > > > +/* Ethernet Configuration */ > > > #define CONFIG_FEC_MXC > > > #define CONFIG_MII > > > #define IMX_FEC_BASE ENET_BASE_ADDR > > > @@ -113,8 +123,30 @@ > > > "fdt_high=0xffffffff\0" \ > > > "initrd_high=0xffffffff\0" \ > > > "fdt_file=" CONFIG_DEFAULT_FDT_FILE "\0" \ > > > - "fdt_addr=0x11000000\0" \ > > > + "fdt_addr=0x11100000\0" \ > > > + "fdt_addr_r=0x11200000\0" \ > > > + "pxefile_addr_r=0x11300000\0" \ > > > + "scr_addr_r=0x11400000\0" \ > > > + "kernel_addr_r=0x11500000\0" \ > > > + "ramdisk_addr_r=0x13500000\0" \ > > > "boot_fdt=try\0" \ > > > + "bootcmd_setup=mmc rescan\0" \ > > > + "bootcmd_pxe=setenv bootfile \"\" ;dhcp; tftp > > > ${fdt_addr} /dtb/${fdt_file}; pxe get; pxe boot\0" \ > > > + "bootcmd_disk_scr=ext2load ${boot_ifc} ${bootdevice} > > > ${scr_addr_r} boot.scr && > > > > Why all the non-generic "ext2load"'s? use just "load", then it'll > > work with both fat, ext2/3/4 & btrfs... > > > > Regards, > > the pxe code requires you specify the filesystem for the sysboot > command, and as /boot needs to be ext ive kept it consistent. Ive also > not narrowed down the option to enable load to work, in my testing it > did not work. having said that I just found CONFIG_CMD_FS_GENERIC which is not set, though cmd_pxe.c will need some patching to not require specifying a filesystem Dennis Dennis
On Thu, Dec 5, 2013 at 11:01 PM, Dennis Gilmore <dennis@ausil.us> wrote: > El Thu, 5 Dec 2013 21:47:43 -0600 > Robert Nelson <robertcnelson@gmail.com> escribió: >> On Thu, Dec 5, 2013 at 8:18 PM, Dennis Gilmore <dennis@ausil.us> >> wrote: >> > Signed-off-by: Dennis Gilmore <dennis@ausil.us> >> > --- >> > include/configs/wandboard.h | 40 >> > +++++++++++++++++++++++++++++++++++----- 1 file changed, 35 >> > insertions(+), 5 deletions(-) >> > >> > diff --git a/include/configs/wandboard.h >> > b/include/configs/wandboard.h index e9c7e64..02d8968 100644 >> > --- a/include/configs/wandboard.h >> > +++ b/include/configs/wandboard.h >> > @@ -40,6 +40,9 @@ >> > #define CONFIG_CONS_INDEX 1 >> > #define CONFIG_BAUDRATE 115200 >> > >> > +/* enable generic distro config */ >> > +#define DISTRO_DEFAULTS 1 >> > + >> > /* Command definition */ >> > #include <config_cmd_default.h> >> > >> > @@ -48,7 +51,6 @@ >> > #define CONFIG_CMD_BMODE >> > #define CONFIG_CMD_SETEXPR >> > >> > -#define CONFIG_BOOTDELAY 5 >> > >> > #define CONFIG_SYS_MEMTEST_START 0x10000000 >> > #define CONFIG_SYS_MEMTEST_END (CONFIG_SYS_MEMTEST_START + >> > 500 * SZ_1M) @@ -65,6 +67,9 @@ >> > #define CONFIG_CMD_MMC >> > #define CONFIG_GENERIC_MMC >> > #define CONFIG_BOUNCE_BUFFER >> > + >> > +#define CONFIG_BOOTDELAY 5 >> > + >> > #define CONFIG_CMD_EXT2 >> > #define CONFIG_CMD_FAT >> > #define CONFIG_DOS_PARTITION >> > @@ -74,6 +79,11 @@ >> > #define CONFIG_CMD_DHCP >> > #define CONFIG_CMD_MII >> > #define CONFIG_CMD_NET >> > + >> > +#define CONFIG_OF_LIBFDT >> > +#define CONFIG_CMD_BOOTZ >> > + >> > +/* Ethernet Configuration */ >> > #define CONFIG_FEC_MXC >> > #define CONFIG_MII >> > #define IMX_FEC_BASE ENET_BASE_ADDR >> > @@ -113,8 +123,30 @@ >> > "fdt_high=0xffffffff\0" \ >> > "initrd_high=0xffffffff\0" \ >> > "fdt_file=" CONFIG_DEFAULT_FDT_FILE "\0" \ >> > - "fdt_addr=0x11000000\0" \ >> > + "fdt_addr=0x11100000\0" \ >> > + "fdt_addr_r=0x11200000\0" \ >> > + "pxefile_addr_r=0x11300000\0" \ >> > + "scr_addr_r=0x11400000\0" \ >> > + "kernel_addr_r=0x11500000\0" \ >> > + "ramdisk_addr_r=0x13500000\0" \ >> > "boot_fdt=try\0" \ >> > + "bootcmd_setup=mmc rescan\0" \ >> > + "bootcmd_pxe=setenv bootfile \"\" ;dhcp; tftp >> > ${fdt_addr} /dtb/${fdt_file}; pxe get; pxe boot\0" \ >> > + "bootcmd_disk_scr=ext2load ${boot_ifc} ${bootdevice} >> > ${scr_addr_r} boot.scr && >> >> Why all the non-generic "ext2load"'s? use just "load", then it'll work >> with both fat, ext2/3/4 & btrfs... >> >> Regards, > > the pxe code requires you specify the filesystem for the sysboot > command, and as /boot needs to be ext ive kept it consistent. Ive also > not narrowed down the option to enable load to work, in my testing it > did not work. Ah, i see the issue, "#define CONFIG_CMD_FS_GENERIC" isn't defined yet for the wandboard in mainline u-boot, as you don't set it either in your patch, so of course a generic "load" won't work.. So if you add: #define CONFIG_CMD_EXT4 #define CONFIG_CMD_FS_GENERIC after: #define CONFIG_CMD_EXT2 in your patch you can drop all the ext2load non-sense for generic loads. I've been shipping ubuntu/debian images with '#define CONFIG_CMD_FS_GENERIC' and 'load' for the wand since v2013.07 with no issues.. Regards,
>> > >> > Why all the non-generic "ext2load"'s? use just "load", then it'll >> > work with both fat, ext2/3/4 & btrfs... >> > >> > Regards, >> >> the pxe code requires you specify the filesystem for the sysboot >> command, and as /boot needs to be ext ive kept it consistent. Ive also >> not narrowed down the option to enable load to work, in my testing it >> did not work. > having said that I just found CONFIG_CMD_FS_GENERIC which is not set, > though cmd_pxe.c will need some patching to not require specifying a > filesystem I can see fixing cmd_pxe.c could be done later. But as just a generic uEnv.txt user, I do like the idea of having a consistent variable naming setup for a bunch of boards. So i can easily convert my stuff to this setup, but the ext2load vs a generic load was the one big issue it was missing. Regards,
Dear Dennis Gilmore, In message <1386296295-28658-3-git-send-email-dennis@ausil.us> you wrote: > > - "fdt_addr=0x11000000\0" \ > + "fdt_addr=0x11100000\0" \ > + "fdt_addr_r=0x11200000\0" \ Why would you need fdt_addr and fdt_addr_r ? This makes no sense. Two different values would only be needed if there was NOR flash available where we could read the FDT from without loading it to RAM first. AFAIK ther ewill never be variants of the Wandboard with NOR flash, so you will always have to load the FDT to RAM first - thus the destinction makes no sense. Best regards, Wolfgang Denk
El Fri, 06 Dec 2013 11:59:05 +0100 Wolfgang Denk <wd@denx.de> escribió: > Dear Dennis Gilmore, > > In message <1386296295-28658-3-git-send-email-dennis@ausil.us> you > wrote: > > > > - "fdt_addr=0x11000000\0" \ > > + "fdt_addr=0x11100000\0" \ > > + "fdt_addr_r=0x11200000\0" \ > > Why would you need fdt_addr and fdt_addr_r ? This makes no sense. > Two different values would only be needed if there was NOR flash > available where we could read the FDT from without loading it to RAM > first. AFAIK ther ewill never be variants of the Wandboard with NOR > flash, so you will always have to load the FDT to RAM first - thus the > destinction makes no sense. please go and read the cmd_pxe.c file it specifies the use of the two variables. one is for a system provided dtb and the other is for a user provided dtb Dennis
Dear Dennis Gilmore, In message <20131206084854.0e0da0cd@adria.ausil.us> you wrote: > > Wolfgang Denk <wd@denx.de> escribi=F3: > > Dear Dennis Gilmore, > >=20 > > In message <1386296295-28658-3-git-send-email-dennis@ausil.us> you > > wrote: > > > > > > - "fdt_addr=0x11000000\0" \ > > > + "fdt_addr=0x11100000\0" \ > > > + "fdt_addr_r=0x11200000\0" \ > > > > Why would you need fdt_addr and fdt_addr_r ? This makes no sense. > > Two different values would only be needed if there was NOR flash > > available where we could read the FDT from without loading it to RAM > > first. AFAIK ther ewill never be variants of the Wandboard with NOR > > flash, so you will always have to load the FDT to RAM first - thus the > > destinction makes no sense. > > please go and read the cmd_pxe.c file it specifies the use of the two > variables. one is for a system provided dtb and the other is for a user > provided dtb But this is crap. The meaning of these variables has been wel-defined for a long, long time. "fdt_addr" is the FDT address in NOR flash (or similar memory except system RAM); "fdt_addr_r" is the FDT address when loaded to system RAM (hence the "_r" in the variable name). Arbitariry redefining this meaning is counterproductive and confusing. If cmd_pxe.c uses incorrect names, then please fix the bug there. Best regards, Wolfgang Denk
On Fri, Dec 06, 2013 at 04:26:51PM +0100, Wolfgang Denk wrote: > Dear Dennis Gilmore, > > In message <20131206084854.0e0da0cd@adria.ausil.us> you wrote: > > > > Wolfgang Denk <wd@denx.de> escribi=F3: > > > Dear Dennis Gilmore, > > >=20 > > > In message <1386296295-28658-3-git-send-email-dennis@ausil.us> you > > > wrote: > > > > > > > > - "fdt_addr=0x11000000\0" \ > > > > + "fdt_addr=0x11100000\0" \ > > > > + "fdt_addr_r=0x11200000\0" \ > > > > > > Why would you need fdt_addr and fdt_addr_r ? This makes no sense. > > > Two different values would only be needed if there was NOR flash > > > available where we could read the FDT from without loading it to RAM > > > first. AFAIK ther ewill never be variants of the Wandboard with NOR > > > flash, so you will always have to load the FDT to RAM first - thus the > > > destinction makes no sense. > > > > please go and read the cmd_pxe.c file it specifies the use of the two > > variables. one is for a system provided dtb and the other is for a user > > provided dtb > > But this is crap. The meaning of these variables has been wel-defined > for a long, long time. "fdt_addr" is the FDT address in NOR flash (or > similar memory except system RAM); "fdt_addr_r" is the FDT address > when loaded to system RAM (hence the "_r" in the variable name). It's a well defined and widely ignored in ARM convention then. We've got lots of 'fdt_addr' meaning RAM and no 'fdt_addr_r' and then in both ARM and PowerPC 'fdtaddr' being presumably RAM. > Arbitariry redefining this meaning is counterproductive and confusing. > > If cmd_pxe.c uses incorrect names, then please fix the bug there. I would say that 'fdt_addr' being the system provided DT, even when not found on memory-mapped flash and 'fdt_addr_r' being the user provided one is a logical extension. If we want to set some new rules on these variables we're going to live with for a long while and really document and enforce (see above about fdt_addr/fdt_addr_r/fdtaddr current usage), OK, sure.
Dear Tom, In message <20131206162854.GX420@bill-the-cat> you wrote: > > > But this is crap. The meaning of these variables has been wel-defined > > for a long, long time. "fdt_addr" is the FDT address in NOR flash (or > > similar memory except system RAM); "fdt_addr_r" is the FDT address > > when loaded to system RAM (hence the "_r" in the variable name). > > It's a well defined and widely ignored in ARM convention then. We've > got lots of 'fdt_addr' meaning RAM and no 'fdt_addr_r' and then in both > ARM and PowerPC 'fdtaddr' being presumably RAM. I think it's actually OK to omit the "_r" in NOR-less systems. The number of devices with actual NOT flash is decreasing, and if you can be sure that there is no such memory device available, then it is just overhead to always carry the "_r" suffice around, knowing all the time that there will never be any other option than RAM to store that data. I do not complain if such systems use a simplified setup without the "_r". What I don't like to see is to have "fdt_addr_r" and "fdt_addr" used with a new, totally different meaning. I don't know where the spelling "fdtaddr" is coming from; I would consider it one of the many "non-standard" variants (assuming we agree that there is actually something like a "standard"). Note that there is no "fdtaddr_r" anywhere. > I would say that 'fdt_addr' being the system provided DT, even when not > found on memory-mapped flash and 'fdt_addr_r' being the user provided > one is a logical extension. Um... you enter completely new terms here - "system provided" and "user provided". I cannot see how a "user provided" DTB in NOR flash would fit in such a concept, nor how this would work on systems with NOR if a "system provided" DTB gets loaded into RAM from a DHCP server. I understand that you are trying to give the old names a new definition that would magically cover the suggested use, but this is extremely thin ice. I recommend not to try that. Best regards, Wolfgang Denk
On Fri, Dec 06, 2013 at 09:37:59PM +0100, Wolfgang Denk wrote: > Dear Tom, > > In message <20131206162854.GX420@bill-the-cat> you wrote: > > > > > But this is crap. The meaning of these variables has been wel-defined > > > for a long, long time. "fdt_addr" is the FDT address in NOR flash (or > > > similar memory except system RAM); "fdt_addr_r" is the FDT address > > > when loaded to system RAM (hence the "_r" in the variable name). > > > > It's a well defined and widely ignored in ARM convention then. We've > > got lots of 'fdt_addr' meaning RAM and no 'fdt_addr_r' and then in both > > ARM and PowerPC 'fdtaddr' being presumably RAM. > > I think it's actually OK to omit the "_r" in NOR-less systems. The > number of devices with actual NOT flash is decreasing, and if you can > be sure that there is no such memory device available, then it is > just overhead to always carry the "_r" suffice around, knowing all > the time that there will never be any other option than RAM to store > that data. Right. So the rule is "fdt_addr means the [shipped] DT in NOR, if present. fdt_addr_r means the [shipped] DT in system RAM." > I do not complain if such systems use a simplified setup without the > "_r". > > What I don't like to see is to have "fdt_addr_r" and "fdt_addr" used > with a new, totally different meaning. Well, "fdt_addr" still means the shipped DT and "fdt_addr_r" still means a DT loaded into system RAM. The only change is that fdt_addr may also be a system RAM address. > I don't know where the spelling "fdtaddr" is coming from; I would > consider it one of the many "non-standard" variants (assuming we agree > that there is actually something like a "standard"). Note that there > is no "fdtaddr_r" anywhere. "fdtaddr" comes from somewhere along the line someone not going "Hey, you forgot a _ in your env" since it means what fdt_addr_r means or fdt_addr means when you lack NOR/similar flash for a DT. > > I would say that 'fdt_addr' being the system provided DT, even when not > > found on memory-mapped flash and 'fdt_addr_r' being the user provided > > one is a logical extension. > > Um... you enter completely new terms here - "system provided" and > "user provided". I cannot see how a "user provided" DTB in NOR flash > would fit in such a concept, nor how this would work on systems with > NOR if a "system provided" DTB gets loaded into RAM from a DHCP > server. "system provided" or "shipped" or what have you for the vendor provided DT, which previously would have been in NOR, for fdt_addr when you also have fdt_addr_r. And I believe the answer to the second question is that yes, the shipped or system provided DTB would end up in fdt_addr, so long as whatever "grab the provided default DT" puts it there. > I understand that you are trying to give the old names a new > definition that would magically cover the suggested use, but this is > extremely thin ice. I recommend not to try that. Well, lets see if we can't convince you around. Or get some better names to use for these use cases.
Hi Wolfgang, El Fri, 06 Dec 2013 21:37:59 +0100 Wolfgang Denk <wd@denx.de> escribió: > Dear Tom, > > In message <20131206162854.GX420@bill-the-cat> you wrote: > > > > > But this is crap. The meaning of these variables has been > > > wel-defined for a long, long time. "fdt_addr" is the FDT address > > > in NOR flash (or similar memory except system RAM); "fdt_addr_r" > > > is the FDT address when loaded to system RAM (hence the "_r" in > > > the variable name). > > > > It's a well defined and widely ignored in ARM convention then. > > We've got lots of 'fdt_addr' meaning RAM and no 'fdt_addr_r' and > > then in both ARM and PowerPC 'fdtaddr' being presumably RAM. > > I think it's actually OK to omit the "_r" in NOR-less systems. The > number of devices with actual NOT flash is decreasing, and if you can > be sure that there is no such memory device available, then it is > just overhead to always carry the "_r" suffice around, knowing all > the time that there will never be any other option than RAM to store > that data. > > I do not complain if such systems use a simplified setup without the > "_r". > > What I don't like to see is to have "fdt_addr_r" and "fdt_addr" used > with a new, totally different meaning. > > > I don't know where the spelling "fdtaddr" is coming from; I would > consider it one of the many "non-standard" variants (assuming we agree > that there is actually something like a "standard"). Note that there > is no "fdtaddr_r" anywhere. fdtaddr is highly prevalent in the configs grep -r fdtaddr include/configs/|wc -l 390 more so than fdt_addr grep -r fdt_addr include/configs/|wc -l 278 those counts are in my git tree with the proposed patches applied that converted soem systems from fdtaddr to fdt_addr. One of the problems right or wrong is that u-boot is seen as not having any kind of standard, which is what I am trying to fix, at least for use in the generic distro world where we need to have an standardised documented interface. > > I would say that 'fdt_addr' being the system provided DT, even when > > not found on memory-mapped flash and 'fdt_addr_r' being the user > > provided one is a logical extension. > > Um... you enter completely new terms here - "system provided" and > "user provided". I cannot see how a "user provided" DTB in NOR flash > would fit in such a concept, nor how this would work on systems with > NOR if a "system provided" DTB gets loaded into RAM from a DHCP > server. > > I understand that you are trying to give the old names a new > definition that would magically cover the suggested use, but this is > extremely thin ice. I recommend not to try that. greping through the doc directory of git I am unable to find any reference to this convention you speak of. The only references i found was in README.falcon README.pxe and README.commands.spl based on your description it would mean falcon mode could not be implemented on any system not having nand. cmd_pxe.c clearly specifies what it thinks the addresses are for fdt_addr_r - location in RAM at which 'pxe boot' will store the fdt blob it retrieves from tftp. The retrieval is possible if 'fdt' label is defined in pxe file and 'fdt_addr_r' is set. If retrieval is possible, 'fdt_addr_r' will be passed to bootm command to boot the kernel. fdt_addr - the location of a fdt blob. 'fdt_addr' will be passed to bootm command if it is set and 'fdt_addr_r' is not passed to bootm command. Which i read as fdt_addr is a system provided dtb, and fdt_addr_r is a user provided one. there is no mention of where exactly they come from. when pxe booting or installing on an arm system we do not want to put a fdt line in the config file, because we do not want to have to have the knowledge in the installer to work out what dtb file we want. that really is a job for u-boot to provide, but we do have the option to if its needed for some reason. I was purely working with what is in front of me. Dennis
Dear Tom, In message <20131206221307.GD420@bill-the-cat> you wrote: > > > I think it's actually OK to omit the "_r" in NOR-less systems. The > > number of devices with actual NOT flash is decreasing, and if you can > > be sure that there is no such memory device available, then it is > > just overhead to always carry the "_r" suffice around, knowing all > > the time that there will never be any other option than RAM to store > > that data. > > Right. So the rule is "fdt_addr means the [shipped] DT in NOR, if > present. fdt_addr_r means the [shipped] DT in system RAM." No. NAK. Delete this "[shipped]" stuff here. This is some new interpretation which you are trying to sneak in here, but there has never been any such notion before. It's just an address, and we don't care where the actual data came from or who put it there. > Well, "fdt_addr" still means the shipped DT and "fdt_addr_r" still means > a DT loaded into system RAM. The only change is that fdt_addr may also > be a system RAM address. Stop. There is no such thing as a "shipped DT". > > Um... you enter completely new terms here - "system provided" and > > "user provided". I cannot see how a "user provided" DTB in NOR flash > > would fit in such a concept, nor how this would work on systems with > > NOR if a "system provided" DTB gets loaded into RAM from a DHCP > > server. > > "system provided" or "shipped" or what have you for the vendor provided > DT, which previously would have been in NOR, for fdt_addr when you also NAK. We have never been using any such terms before. You are trying to insert completely new meaning here, and I do not agree with such interpretation. A DT is a DT is a DT, no matter who provided it or where it is coming from or who installed it where. > Well, lets see if we can't convince you around. Or get some better > names to use for these use cases. No chance. This is the first time ever such terms come up, and we've been using DTs for a long, long time before. Best regards, Wolfgang Denk
Dear Dennis, In message <20131206164420.36e5f593@adria.ausil.us> you wrote: > > fdtaddr is highly prevalent in the configs Yes, it appears some vendors favoured this name. I'm temptted to add that these very vendors often tend to define thier own ways without caring what the community has been using before ;-) That doesn't make it any better, though... > those counts are in my git tree with the proposed patches applied > that converted soem systems from fdtaddr to fdt_addr. One of the > problems right or wrong is that u-boot is seen as not having any kind of > standard, which is what I am trying to fix, at least for use in the > generic distro world where we need to have an standardised > documented interface. Even if you feel differntly, I do appreciate your efforts. But I'd also like to see things done in a consistent way. And the whole idea of using the "_r" names was to show clearly which of the addresses are supposed to be in system RAM (with "_r"), and which are not (without). This parallels function names like board_init_f() ["_f" standing for "running from [NOR] flash"] and board_init_r() ["_r" = running from system RAM]. > greping through the doc directory of git I am unable to find any > reference to this convention you speak of. Agreed. Noone wrote a document about this, yet. > The only references i found was in README.falcon README.pxe and > README.commands.spl based on your description it would mean falcon mode > could not be implemented on any system not having nand. I lost you here. What makes you think so? > cmd_pxe.c clearly specifies what it thinks the addresses are for Yes, it does. But this is confused or incorrect, misusing existing names for other purposes. This should be fixed. > Which i read as fdt_addr is a system provided dtb, and fdt_addr_r is a > user provided one. there is no mention of where exactly they come from. Stop. There has never been any such notion like "system provided" or "user provided" before. You cannot just put new meanings over existing terms. Actually, to me such terms don't even make much sense. Best regards, Wolfgang Denk
Hi Wolfgang, El Sat, 07 Dec 2013 00:16:16 +0100 Wolfgang Denk <wd@denx.de> escribió: > Dear Dennis, > > In message <20131206164420.36e5f593@adria.ausil.us> you wrote: > > > > fdtaddr is highly prevalent in the configs > > Yes, it appears some vendors favoured this name. I'm temptted to add > that these very vendors often tend to define thier own ways without > caring what the community has been using before ;-) > > That doesn't make it any better, though... Not saying that it does, just pointing out the inconsistency in the tree today. something we likely should fix. > > those counts are in my git tree with the proposed patches applied > > that converted soem systems from fdtaddr to fdt_addr. One of the > > problems right or wrong is that u-boot is seen as not having any > > kind of standard, which is what I am trying to fix, at least for > > use in the generic distro world where we need to have an > > standardised documented interface. > > Even if you feel differntly, I do appreciate your efforts. But I'd > also like to see things done in a consistent way. And the whole idea > of using the "_r" names was to show clearly which of the addresses are > supposed to be in system RAM (with "_r"), and which are not (without). Thankyou I'm trying to be consistent in the interface between u-boot and the operating system. > This parallels function names like board_init_f() ["_f" standing for > "running from [NOR] flash"] and board_init_r() ["_r" = running from > system RAM]. which makes some sense in the code, but the config is defining the interface to kernel/userland and needs to be consistent regardless of what is underneath > > greping through the doc directory of git I am unable to find any > > reference to this convention you speak of. > > Agreed. Noone wrote a document about this, yet. > > > The only references i found was in README.falcon README.pxe and > > README.commands.spl based on your description it would mean falcon > > mode could not be implemented on any system not having nand. > > I lost you here. What makes you think so? Usage: spl export <img=atags|fdt> [kernel_addr] [initrd_addr] [fdt_addr ] which to me based on what you said means everything needs to be in nand > > cmd_pxe.c clearly specifies what it thinks the addresses are for > > Yes, it does. But this is confused or incorrect, misusing existing > names for other purposes. This should be fixed. I actually think it is spot on and is how it should be. > > Which i read as fdt_addr is a system provided dtb, and fdt_addr_r > > is a user provided one. there is no mention of where exactly they > > come from. > > Stop. There has never been any such notion like "system provided" or > "user provided" before. You cannot just put new meanings over > existing terms. Actually, to me such terms don't even make much > sense. from u-boot itself there is not this notion. But from a Distro perspective its always there and is a really big thing. It really is important. in 99% of cases we want to have u-boot use a provided dtb without the need to say so and to have it work. Dennis Dennis
Dear Dennis, In message <20131206180955.32ba9f0b@adria.ausil.us> you wrote: > > > Yes, it appears some vendors favoured this name. I'm temptted to add > > that these very vendors often tend to define thier own ways without > > caring what the community has been using before ;-) > > > > That doesn't make it any better, though... > > Not saying that it does, just pointing out the inconsistency in the > tree today. something we likely should fix. Agreed, it would be nice, but it might turn out to be difficult. I think a large number of such systems is alreay out in the field, and introducing such chnges to the user interface is often just impossible or inacceptable. We should keep in mind that - until not so long ago - no attempts have been made to try to standardize these things, so we have now the results of uncontrolled growth over 13 years or so. Even though there are a number of configuratios some of us would recommend as archetypes, but again there are several "schools" of these, each with their own approaches and names. > > Even if you feel differntly, I do appreciate your efforts. But I'd > > also like to see things done in a consistent way. And the whole idea > > of using the "_r" names was to show clearly which of the addresses are > > supposed to be in system RAM (with "_r"), and which are not (without). ... > which makes some sense in the code, but the config is defining the > interface to kernel/userland and needs to be consistent regardless of > what is underneath This is no contradiction, right? > > > The only references i found was in README.falcon README.pxe and > > > README.commands.spl based on your description it would mean falcon > > > mode could not be implemented on any system not having nand. > > > > I lost you here. What makes you think so? > Usage: > > spl export <img=atags|fdt> [kernel_addr] [initrd_addr] [fdt_addr ] > > which to me based on what you said means everything needs to be in nand No. First, SPL booting is not restricted to NAND. We use this also (at least on some systems) to boot from SDCard, and even to boot from NOR flash. Second, the "spl export" step is a pretty specific thing, that does not really reflect the normal steps of booting a system - it makes preparations to do so, but this is a different task. > > > cmd_pxe.c clearly specifies what it thinks the addresses are for > > > > Yes, it does. But this is confused or incorrect, misusing existing > > names for other purposes. This should be fixed. > > I actually think it is spot on and is how it should be. I continue to disagree. The use of "fdt_addr" and "fdt_addr_r" in the same context should stick with exising practice, and as I explained before, "fdt_addr_r" then refers to an address in ROM (usually NOR flash). > > Stop. There has never been any such notion like "system provided" or > > "user provided" before. You cannot just put new meanings over > > existing terms. Actually, to me such terms don't even make much > > sense. > > from u-boot itself there is not this notion. But from a Distro > perspective its always there and is a really big thing. It really is > important. in 99% of cases we want to have u-boot use a provided dtb > without the need to say so and to have it work. When introducing new terms into a new environment, you should make sure to actually define the meaning of these terms, so everybody speaks the same language. Actually you would pprobably have to start with defining "system" (I feel you might actually mean "distro" instead?) and "user". There is a number of situations, which may (or may not) result in differing usage modes. In addition to "user" and "distro" terms like "vendor" (or "manufacturer") or "developer" might play a role here - each will probably result in specific expectations and experiences about "good practices". So please, define your terms, and as exact as possibvle, so we avoid confusion when discussing them. Best regards, Wolfgang Denk
diff --git a/include/configs/wandboard.h b/include/configs/wandboard.h index e9c7e64..02d8968 100644 --- a/include/configs/wandboard.h +++ b/include/configs/wandboard.h @@ -40,6 +40,9 @@ #define CONFIG_CONS_INDEX 1 #define CONFIG_BAUDRATE 115200 +/* enable generic distro config */ +#define DISTRO_DEFAULTS 1 + /* Command definition */ #include <config_cmd_default.h> @@ -48,7 +51,6 @@ #define CONFIG_CMD_BMODE #define CONFIG_CMD_SETEXPR -#define CONFIG_BOOTDELAY 5 #define CONFIG_SYS_MEMTEST_START 0x10000000 #define CONFIG_SYS_MEMTEST_END (CONFIG_SYS_MEMTEST_START + 500 * SZ_1M) @@ -65,6 +67,9 @@ #define CONFIG_CMD_MMC #define CONFIG_GENERIC_MMC #define CONFIG_BOUNCE_BUFFER + +#define CONFIG_BOOTDELAY 5 + #define CONFIG_CMD_EXT2 #define CONFIG_CMD_FAT #define CONFIG_DOS_PARTITION @@ -74,6 +79,11 @@ #define CONFIG_CMD_DHCP #define CONFIG_CMD_MII #define CONFIG_CMD_NET + +#define CONFIG_OF_LIBFDT +#define CONFIG_CMD_BOOTZ + +/* Ethernet Configuration */ #define CONFIG_FEC_MXC #define CONFIG_MII #define IMX_FEC_BASE ENET_BASE_ADDR @@ -113,8 +123,30 @@ "fdt_high=0xffffffff\0" \ "initrd_high=0xffffffff\0" \ "fdt_file=" CONFIG_DEFAULT_FDT_FILE "\0" \ - "fdt_addr=0x11000000\0" \ + "fdt_addr=0x11100000\0" \ + "fdt_addr_r=0x11200000\0" \ + "pxefile_addr_r=0x11300000\0" \ + "scr_addr_r=0x11400000\0" \ + "kernel_addr_r=0x11500000\0" \ + "ramdisk_addr_r=0x13500000\0" \ "boot_fdt=try\0" \ + "bootcmd_setup=mmc rescan\0" \ + "bootcmd_pxe=setenv bootfile \"\" ;dhcp; tftp ${fdt_addr} /dtb/${fdt_file}; pxe get; pxe boot\0" \ + "bootcmd_disk_scr=ext2load ${boot_ifc} ${bootdevice} ${scr_addr_r} boot.scr && source ${scr_addr_r}\0" \ + "bootcmd_disk_sysboot1=setenv bootfile /boot/extlinux/extlinux.conf; sysboot ${boot_ifc} ${bootdevice} ext2\0" \ + "bootcmd_disk_sysboot2=setenv bootfile /extlinux/extlinux.conf; sysboot ${boot_ifc} ${bootdevice} ext2\0" \ + "bootcmd_disk_uenv=ext2load ${boot_ifc} ${bootdevice} ${uenv_addr_r} uEnv.txt; env import -t ${uenv_addr_r} ${filesize}; run bootcmd_uenv\0" \ + "bootcmd_disk_kernel=ext2load ${boot_ifc} ${bootdevice} ${kernel_addr_r} vmlinuz && ext2load ${boot_ifc} ${bootdevice} ${ramdisk_addr_r} initrd.img && bootz ${kernel_addr_r} ${ramdisk_addr_r}:${filesize} ${fdt_addr}\0" \ + "bootcmd_disk=ext2load ${boot_ifc} ${bootdevice} ${fdt_addr} /boot/dtb/${fdt_file}; ext2load ${boot_ifc} ${bootdevice} ${fdt_addr} /dtb/${fdt_file};run bootcmd_disk_sysboot1; run bootcmd_disk_sysboot2; run bootcmd_disk_uenv; run bootcmd_disk_scr; run bootcmd_disk_kernel\0" \ + "bootcmd_sata=setenv boot_ifc scsi; scsi scan && run bootcmd_disk\0" \ + "bootcmd_mmc=setenv boot_ifc mmc; mmc rescan && run bootcmd_disk\0" \ + "bootcmd_default=run bootcmd_mmc; run bootcmd_sata; run bootcmd_pxe\0" \ + "localcmd=run bootcmd_mmc\0" \ + "bootdevice=0\0" \ + "bootargs=console=ttymxc0 root=LABEL=rootfs\0" \ + "bootdelay=2\0" \ + "bootretry=90\0" \ + "netretry=once\0" \ "ip_dyn=yes\0" \ "mmcdev=" __stringify(CONFIG_SYS_MMC_ENV_DEV) "\0" \ "mmcpart=1\0" \ @@ -183,6 +215,7 @@ #define CONFIG_BOOTCOMMAND \ "mmc dev ${mmcdev}; if mmc rescan; then " \ + "run bootcmd_default; " \ "if run loadbootscript; then " \ "run bootscript; " \ "else " \ @@ -230,9 +263,6 @@ #define CONFIG_ENV_OFFSET (6 * 64 * 1024) #define CONFIG_SYS_MMC_ENV_DEV 0 -#define CONFIG_OF_LIBFDT -#define CONFIG_CMD_BOOTZ - #ifndef CONFIG_SYS_DCACHE_OFF #define CONFIG_CMD_CACHE #endif
Signed-off-by: Dennis Gilmore <dennis@ausil.us> --- include/configs/wandboard.h | 40 +++++++++++++++++++++++++++++++++++----- 1 file changed, 35 insertions(+), 5 deletions(-)