| Message ID | 1356109151-6623-8-git-send-email-otavio@ossystems.com.br |
|---|---|
| State | Changes Requested |
| Delegated to: | Stefano Babic |
| Headers | show |
On 21/12/2012 17:59, Otavio Salvador wrote: > Signed-off-by: Otavio Salvador <otavio@ossystems.com.br> > --- > include/configs/mx53loco.h | 15 +++++++++++++-- > 1 file changed, 13 insertions(+), 2 deletions(-) > Hi Otavio, > diff --git a/include/configs/mx53loco.h b/include/configs/mx53loco.h > index 4cfdc0b..505957c 100644 > --- a/include/configs/mx53loco.h > +++ b/include/configs/mx53loco.h > @@ -118,6 +118,8 @@ > #define CONFIG_EXTRA_ENV_SETTINGS \ > "script=boot.scr\0" \ > "uimage=uImage\0" \ > + "ftd_file=imx53-qsb.dtb\0" \ > + "ftd_addr=0x71000000\0" \ > "mmcdev=0\0" \ > "mmcpart=2\0" \ > "mmcroot=/dev/mmcblk0p3 rw rootwait\0" \ > @@ -129,13 +131,22 @@ > "loaduimage=fatload mmc ${mmcdev}:${mmcpart} ${loadaddr} ${uimage}\0" \ > "mmcboot=echo Booting from mmc ...; " \ > "run mmcargs; " \ > - "bootm\0" \ > + "if fatload mmc ${mmcdev}:${mmcpart} ${ftd_addr} ${ftd_file}; then " \ > + "bootm ${loadaddr} - ${ftd_addr}; " \ > + "else " \ > + "bootm; " \ > + "fi;\0" \ > "netargs=setenv bootargs console=ttymxc0,${baudrate} " \ > "root=/dev/nfs " \ > "ip=dhcp nfsroot=${serverip}:${nfsroot},v3,tcp\0" \ > "netboot=echo Booting from net ...; " \ > "run netargs; " \ > - "dhcp ${uimage}; bootm\0" \ > + "dhcp ${uimage}; " \ > + "if dhcp ${ftd_addr} ${ftd_file}; then " \ > + "bootm ${loadaddr} - ${ftd_addr}; " \ > + "else " \ > + "bootm; " \ > + "fi;\0" > Only to remark that this is the correct behavior. The kernel with fdt is booted independently if this is wanted or not, but if the fdt file is loaded successfully from MMC or network. This can have some drawback effects if, for example, the fdt is simply stored on the TFTP server, but we want to boot a kernel without DT. For example, when someone wants to test both kernels or there are multiple instances of the same board type (mx53loco in this case) loading from the same TFTP server. In your patch, the behavior depends if the fdt is simply present on the media, but this does not always mean that the file must be loaded. Should be not better to set variable as flag to force the desired behavior and to be sure that the system does not boot in a different way as the user thinks ? In other words, something like : "if boot_fdt; then " \ "if dhcp ${ftd_addr} ${ftd_file}; then " \ "bootm ${loadaddr} - ${ftd_addr}; " \ "else " \ "echo Error: I cannot boot a DT kernel; \" "fi; \" " else bootm; fi;\0" Best regards, Stefano Babic
On Wed, Dec 26, 2012 at 9:31 AM, Stefano Babic <sbabic@denx.de> wrote: > Only to remark that this is the correct behavior. The kernel with fdt is > booted independently if this is wanted or not, but if the fdt file is > loaded successfully from MMC or network. This can have some drawback > effects if, for example, the fdt is simply stored on the TFTP server, > but we want to boot a kernel without DT. For example, when someone wants > to test both kernels or there are multiple instances of the same board > type (mx53loco in this case) loading from the same TFTP server. > > In your patch, the behavior depends if the fdt is simply present on the > media, but this does not always mean that the file must be loaded. > Should be not better to set variable as flag to force the desired > behavior and to be sure that the system does not boot in a different way > as the user thinks ? > > In other words, something like : > > "if boot_fdt; then " \ > "if dhcp ${ftd_addr} ${ftd_file}; then " \ > "bootm ${loadaddr} - ${ftd_addr}; " \ > "else " \ > "echo Error: I cannot boot a DT kernel; \" > "fi; \" > " else bootm; fi;\0" Yes, I think this is a good idea and would allow us to easily switch from dt to non-dt kernel during tests. Regards, Fabio Estevam
On Wed, Dec 26, 2012 at 10:14 AM, Fabio Estevam <festevam@gmail.com> wrote: > On Wed, Dec 26, 2012 at 9:31 AM, Stefano Babic <sbabic@denx.de> wrote: > >> Only to remark that this is the correct behavior. The kernel with fdt is >> booted independently if this is wanted or not, but if the fdt file is >> loaded successfully from MMC or network. This can have some drawback >> effects if, for example, the fdt is simply stored on the TFTP server, >> but we want to boot a kernel without DT. For example, when someone wants >> to test both kernels or there are multiple instances of the same board >> type (mx53loco in this case) loading from the same TFTP server. >> >> In your patch, the behavior depends if the fdt is simply present on the >> media, but this does not always mean that the file must be loaded. >> Should be not better to set variable as flag to force the desired >> behavior and to be sure that the system does not boot in a different way >> as the user thinks ? >> >> In other words, something like : >> >> "if boot_fdt; then " \ >> "if dhcp ${ftd_addr} ${ftd_file}; then " \ >> "bootm ${loadaddr} - ${ftd_addr}; " \ >> "else " \ >> "echo Error: I cannot boot a DT kernel; \" >> "fi; \" >> " else bootm; fi;\0" > > > Yes, I think this is a good idea and would allow us to easily switch > from dt to non-dt kernel during tests. I like the idea but maybe we could have three states? boot_fdt=yes It'd imply force fdt and would behave as you said above; boot_fdt=no Would ignore the fdt completely boot_fdt=auto The current code, which try to load fdt and do not fail otherwise. Comments? -- Otavio Salvador O.S. Systems E-mail: otavio@ossystems.com.br http://www.ossystems.com.br Mobile: +55 53 9981-7854 http://projetos.ossystems.com.br
On 26/12/2012 19:23, Otavio Salvador wrote: > On Wed, Dec 26, 2012 at 10:14 AM, Fabio Estevam <festevam@gmail.com> wrote: >> On Wed, Dec 26, 2012 at 9:31 AM, Stefano Babic <sbabic@denx.de> wrote: >> >>> Only to remark that this is the correct behavior. The kernel with fdt is >>> booted independently if this is wanted or not, but if the fdt file is >>> loaded successfully from MMC or network. This can have some drawback >>> effects if, for example, the fdt is simply stored on the TFTP server, >>> but we want to boot a kernel without DT. For example, when someone wants >>> to test both kernels or there are multiple instances of the same board >>> type (mx53loco in this case) loading from the same TFTP server. >>> >>> In your patch, the behavior depends if the fdt is simply present on the >>> media, but this does not always mean that the file must be loaded. >>> Should be not better to set variable as flag to force the desired >>> behavior and to be sure that the system does not boot in a different way >>> as the user thinks ? >>> >>> In other words, something like : >>> >>> "if boot_fdt; then " \ >>> "if dhcp ${ftd_addr} ${ftd_file}; then " \ >>> "bootm ${loadaddr} - ${ftd_addr}; " \ >>> "else " \ >>> "echo Error: I cannot boot a DT kernel; \" >>> "fi; \" >>> " else bootm; fi;\0" >> >> >> Yes, I think this is a good idea and would allow us to easily switch >> from dt to non-dt kernel during tests. > > I like the idea but maybe we could have three states? > > boot_fdt=yes > > It'd imply force fdt and would behave as you said above; > > boot_fdt=no > > Would ignore the fdt completely > > boot_fdt=auto > > The current code, which try to load fdt and do not fail otherwise. > > Comments? Reading again your environment, it seems to me that you strictly bind the network configuration with fdt. But they are two distinct setup. Let's say, if I desire to start a fdt kernel but I have a static ip address. In this case, I should load with "tftp" instead using "dhcp". So the necessity to have a three state is due to the fact you are setting more as one state in one shot: fdt or not, and dhcp or not. IMHO you need two variables to manage them independently. I think I did something like this in another board, but I do not remember which one ;-(. So you can have: boot_fdt ip_dyn to discriminate what the script should do. "if boot_fdt; then " \ "if ip_dyn;then" \ "if dhcp ${ftd_addr} ${ftd_file}; then " \ "bootm ${loadaddr} - ${ftd_addr}; " \ "else " \ "echo Error: I cannot boot a DT kernel; \" "fi;" \ "else tftp ${ftd_addr} ${ftd_file} ..... Regards, Stefano
diff --git a/include/configs/mx53loco.h b/include/configs/mx53loco.h index 4cfdc0b..505957c 100644 --- a/include/configs/mx53loco.h +++ b/include/configs/mx53loco.h @@ -118,6 +118,8 @@ #define CONFIG_EXTRA_ENV_SETTINGS \ "script=boot.scr\0" \ "uimage=uImage\0" \ + "ftd_file=imx53-qsb.dtb\0" \ + "ftd_addr=0x71000000\0" \ "mmcdev=0\0" \ "mmcpart=2\0" \ "mmcroot=/dev/mmcblk0p3 rw rootwait\0" \ @@ -129,13 +131,22 @@ "loaduimage=fatload mmc ${mmcdev}:${mmcpart} ${loadaddr} ${uimage}\0" \ "mmcboot=echo Booting from mmc ...; " \ "run mmcargs; " \ - "bootm\0" \ + "if fatload mmc ${mmcdev}:${mmcpart} ${ftd_addr} ${ftd_file}; then " \ + "bootm ${loadaddr} - ${ftd_addr}; " \ + "else " \ + "bootm; " \ + "fi;\0" \ "netargs=setenv bootargs console=ttymxc0,${baudrate} " \ "root=/dev/nfs " \ "ip=dhcp nfsroot=${serverip}:${nfsroot},v3,tcp\0" \ "netboot=echo Booting from net ...; " \ "run netargs; " \ - "dhcp ${uimage}; bootm\0" \ + "dhcp ${uimage}; " \ + "if dhcp ${ftd_addr} ${ftd_file}; then " \ + "bootm ${loadaddr} - ${ftd_addr}; " \ + "else " \ + "bootm; " \ + "fi;\0" #define CONFIG_BOOTCOMMAND \ "mmc dev ${mmcdev}; if mmc rescan; then " \
Signed-off-by: Otavio Salvador <otavio@ossystems.com.br> --- include/configs/mx53loco.h | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-)