| Message ID | b9e56d12fd351ed805a9cf2bddbb48712415f393.1548343860.git.michal.simek@xilinx.com |
|---|---|
| State | Deferred |
| Delegated to: | Michal Simek |
| Headers | show |
| Series | [U-Boot,1/2] ARM: zynq: Run distribution boot commands first | expand |
On 24.01.19 16:31, Michal Simek wrote: > From: Siva Durga Prasad Paladugu <siva.durga.paladugu@xilinx.com> > > This patch adds distro boot commands for qspi and nand. The > distro boot commands now reads the script from flash offset > of 63.5MB and executes it. > > Setup default location via script_offset_f to 63.5MB to match the most > xilinx reference boards. 512kB allocated space for script size > (script_size_f) should be more than enough to cover custom boot logic. > > Signed-off-by: Siva Durga Prasad Paladugu <siva.durga.paladugu@xilinx.com> > Signed-off-by: Michal Simek <michal.simek@xilinx.com> > --- > > include/configs/xilinx_zynqmp.h | 32 ++++++++++++++++++++++++++++++++ > 1 file changed, 32 insertions(+) > > diff --git a/include/configs/xilinx_zynqmp.h b/include/configs/xilinx_zynqmp.h > index d0515fa9bc46..5eeae6331e1c 100644 > --- a/include/configs/xilinx_zynqmp.h > +++ b/include/configs/xilinx_zynqmp.h > @@ -129,6 +129,8 @@ > "kernel_addr_r=0x18000000\0" \ > "scriptaddr=0x02000000\0" \ > "ramdisk_addr_r=0x02100000\0" \ > + "script_offset_f=0x3e80000\0" \ > + "script_size_f=0x80000\0" \ > > #if defined(CONFIG_MMC_SDHCI_ZYNQ) > # define BOOT_TARGET_DEVICES_MMC(func) func(MMC, mmc, 0) func(MMC, mmc, 1) > @@ -160,8 +162,38 @@ > # define BOOT_TARGET_DEVICES_DHCP(func) > #endif > > +#if defined(CONFIG_ZYNQMP_GQSPI) > +# define BOOT_TARGET_DEVICES_QSPI(func) func(QSPI, qspi, 0) > +#else > +# define BOOT_TARGET_DEVICES_QSPI(func) > +#endif > + > +#if defined(CONFIG_NAND_ARASAN) > +# define BOOT_TARGET_DEVICES_NAND(func) func(NAND, nand, 0) > +#else > +# define BOOT_TARGET_DEVICES_NAND(func) > +#endif > + > +#define BOOTENV_DEV_QSPI(devtypeu, devtypel, instance) \ > + "bootcmd_qspi0=sf probe 0 0 0 && " \ This > + "sf read $scriptaddr $script_offset_f $script_size_f && " \ > + "source ${scriptaddr}; echo SCRIPT FAILED: continuing...;\0" > + > +#define BOOTENV_DEV_NAME_QSPI(devtypeu, devtypel, instance) \ > + "qspi " > + > +#define BOOTENV_DEV_NAND(devtypeu, devtypel, instance) \ > + "bootcmd_nand0= nand info && " \ and this will emit errors if no device was found, right? That might be confusing for a user, if all he sees is a scary "device not found" message which simply only occurs because he doesn't have nand populated. Alex > + "nand read $scriptaddr $script_offset_f $script_size_f && " \ > + "source ${scriptaddr}; echo SCRIPT FAILED: continuing...;\0" > + > +#define BOOTENV_DEV_NAME_NAND(devtypeu, devtypel, instance) \ > + "nand " > + > #define BOOT_TARGET_DEVICES(func) \ > BOOT_TARGET_DEVICES_MMC(func) \ > + BOOT_TARGET_DEVICES_QSPI(func) \ > + BOOT_TARGET_DEVICES_NAND(func) \ > BOOT_TARGET_DEVICES_USB(func) \ > BOOT_TARGET_DEVICES_SCSI(func) \ > BOOT_TARGET_DEVICES_PXE(func) \ >
On 25. 01. 19 12:25, Alexander Graf wrote: > > > On 24.01.19 16:31, Michal Simek wrote: >> From: Siva Durga Prasad Paladugu <siva.durga.paladugu@xilinx.com> >> >> This patch adds distro boot commands for qspi and nand. The >> distro boot commands now reads the script from flash offset >> of 63.5MB and executes it. >> >> Setup default location via script_offset_f to 63.5MB to match the most >> xilinx reference boards. 512kB allocated space for script size >> (script_size_f) should be more than enough to cover custom boot logic. >> >> Signed-off-by: Siva Durga Prasad Paladugu <siva.durga.paladugu@xilinx.com> >> Signed-off-by: Michal Simek <michal.simek@xilinx.com> >> --- >> >> include/configs/xilinx_zynqmp.h | 32 ++++++++++++++++++++++++++++++++ >> 1 file changed, 32 insertions(+) >> >> diff --git a/include/configs/xilinx_zynqmp.h b/include/configs/xilinx_zynqmp.h >> index d0515fa9bc46..5eeae6331e1c 100644 >> --- a/include/configs/xilinx_zynqmp.h >> +++ b/include/configs/xilinx_zynqmp.h >> @@ -129,6 +129,8 @@ >> "kernel_addr_r=0x18000000\0" \ >> "scriptaddr=0x02000000\0" \ >> "ramdisk_addr_r=0x02100000\0" \ >> + "script_offset_f=0x3e80000\0" \ >> + "script_size_f=0x80000\0" \ >> >> #if defined(CONFIG_MMC_SDHCI_ZYNQ) >> # define BOOT_TARGET_DEVICES_MMC(func) func(MMC, mmc, 0) func(MMC, mmc, 1) >> @@ -160,8 +162,38 @@ >> # define BOOT_TARGET_DEVICES_DHCP(func) >> #endif >> >> +#if defined(CONFIG_ZYNQMP_GQSPI) >> +# define BOOT_TARGET_DEVICES_QSPI(func) func(QSPI, qspi, 0) >> +#else >> +# define BOOT_TARGET_DEVICES_QSPI(func) >> +#endif >> + >> +#if defined(CONFIG_NAND_ARASAN) >> +# define BOOT_TARGET_DEVICES_NAND(func) func(NAND, nand, 0) >> +#else >> +# define BOOT_TARGET_DEVICES_NAND(func) >> +#endif >> + >> +#define BOOTENV_DEV_QSPI(devtypeu, devtypel, instance) \ >> + "bootcmd_qspi0=sf probe 0 0 0 && " \ > > This > >> + "sf read $scriptaddr $script_offset_f $script_size_f && " \ >> + "source ${scriptaddr}; echo SCRIPT FAILED: continuing...;\0" >> + >> +#define BOOTENV_DEV_NAME_QSPI(devtypeu, devtypel, instance) \ >> + "qspi " >> + >> +#define BOOTENV_DEV_NAND(devtypeu, devtypel, instance) \ >> + "bootcmd_nand0= nand info && " \ > > and this will emit errors if no device was found, right? That might be > confusing for a user, if all he sees is a scary "device not found" > message which simply only occurs because he doesn't have nand populated. As you see above it is enabled only when devices are enabled. For generic u-boot for all zynqmp targets it should be correct to emit error message that nand is not detected if it is not present on the board. :-) All these boot modes will be tried if the first boot mode fails. I don't think it is an issue to report that booting from flash fails if it is not detected and it shouldn't matter if flash is broken or it is not even on the board because simply booting from flash fails. Thanks, Michal
On 25.01.19 12:56, Michal Simek wrote: > On 25. 01. 19 12:25, Alexander Graf wrote: >> >> >> On 24.01.19 16:31, Michal Simek wrote: >>> From: Siva Durga Prasad Paladugu <siva.durga.paladugu@xilinx.com> >>> >>> This patch adds distro boot commands for qspi and nand. The >>> distro boot commands now reads the script from flash offset >>> of 63.5MB and executes it. >>> >>> Setup default location via script_offset_f to 63.5MB to match the most >>> xilinx reference boards. 512kB allocated space for script size >>> (script_size_f) should be more than enough to cover custom boot logic. >>> >>> Signed-off-by: Siva Durga Prasad Paladugu <siva.durga.paladugu@xilinx.com> >>> Signed-off-by: Michal Simek <michal.simek@xilinx.com> >>> --- >>> >>> include/configs/xilinx_zynqmp.h | 32 ++++++++++++++++++++++++++++++++ >>> 1 file changed, 32 insertions(+) >>> >>> diff --git a/include/configs/xilinx_zynqmp.h b/include/configs/xilinx_zynqmp.h >>> index d0515fa9bc46..5eeae6331e1c 100644 >>> --- a/include/configs/xilinx_zynqmp.h >>> +++ b/include/configs/xilinx_zynqmp.h >>> @@ -129,6 +129,8 @@ >>> "kernel_addr_r=0x18000000\0" \ >>> "scriptaddr=0x02000000\0" \ >>> "ramdisk_addr_r=0x02100000\0" \ >>> + "script_offset_f=0x3e80000\0" \ >>> + "script_size_f=0x80000\0" \ >>> >>> #if defined(CONFIG_MMC_SDHCI_ZYNQ) >>> # define BOOT_TARGET_DEVICES_MMC(func) func(MMC, mmc, 0) func(MMC, mmc, 1) >>> @@ -160,8 +162,38 @@ >>> # define BOOT_TARGET_DEVICES_DHCP(func) >>> #endif >>> >>> +#if defined(CONFIG_ZYNQMP_GQSPI) >>> +# define BOOT_TARGET_DEVICES_QSPI(func) func(QSPI, qspi, 0) >>> +#else >>> +# define BOOT_TARGET_DEVICES_QSPI(func) >>> +#endif >>> + >>> +#if defined(CONFIG_NAND_ARASAN) >>> +# define BOOT_TARGET_DEVICES_NAND(func) func(NAND, nand, 0) >>> +#else >>> +# define BOOT_TARGET_DEVICES_NAND(func) >>> +#endif >>> + >>> +#define BOOTENV_DEV_QSPI(devtypeu, devtypel, instance) \ >>> + "bootcmd_qspi0=sf probe 0 0 0 && " \ >> >> This >> >>> + "sf read $scriptaddr $script_offset_f $script_size_f && " \ >>> + "source ${scriptaddr}; echo SCRIPT FAILED: continuing...;\0" >>> + >>> +#define BOOTENV_DEV_NAME_QSPI(devtypeu, devtypel, instance) \ >>> + "qspi " >>> + >>> +#define BOOTENV_DEV_NAND(devtypeu, devtypel, instance) \ >>> + "bootcmd_nand0= nand info && " \ >> >> and this will emit errors if no device was found, right? That might be >> confusing for a user, if all he sees is a scary "device not found" >> message which simply only occurs because he doesn't have nand populated. > > As you see above it is enabled only when devices are enabled. > For generic u-boot for all zynqmp targets it should be correct to emit > error message that nand is not detected if it is not present on the > board. :-) > All these boot modes will be tried if the first boot mode fails. Well, it's conditional on whether the driver is available, not whether the board actually exposes a flash device, right? So think about the generic case - one U-Boot binary for all targets. What are you going to do there? That needs to have all possible boot target devices enabled, but how do you know whether an actual QSPI chip is attached to the SPI controller? I also think this problem is bigger than this patch, so don't take my comment as a nack. I'm just trying to raise that we should start to think about cases where "probe fails" is not an error and thus should not print an error message. Alex > I don't think it is an issue to report that booting from flash fails if > it is not detected and it shouldn't matter if flash is broken or it is > not even on the board because simply booting from flash fails. > > Thanks, > Michal >
On 25. 01. 19 13:00, Alexander Graf wrote: > > > On 25.01.19 12:56, Michal Simek wrote: >> On 25. 01. 19 12:25, Alexander Graf wrote: >>> >>> >>> On 24.01.19 16:31, Michal Simek wrote: >>>> From: Siva Durga Prasad Paladugu <siva.durga.paladugu@xilinx.com> >>>> >>>> This patch adds distro boot commands for qspi and nand. The >>>> distro boot commands now reads the script from flash offset >>>> of 63.5MB and executes it. >>>> >>>> Setup default location via script_offset_f to 63.5MB to match the most >>>> xilinx reference boards. 512kB allocated space for script size >>>> (script_size_f) should be more than enough to cover custom boot logic. >>>> >>>> Signed-off-by: Siva Durga Prasad Paladugu <siva.durga.paladugu@xilinx.com> >>>> Signed-off-by: Michal Simek <michal.simek@xilinx.com> >>>> --- >>>> >>>> include/configs/xilinx_zynqmp.h | 32 ++++++++++++++++++++++++++++++++ >>>> 1 file changed, 32 insertions(+) >>>> >>>> diff --git a/include/configs/xilinx_zynqmp.h b/include/configs/xilinx_zynqmp.h >>>> index d0515fa9bc46..5eeae6331e1c 100644 >>>> --- a/include/configs/xilinx_zynqmp.h >>>> +++ b/include/configs/xilinx_zynqmp.h >>>> @@ -129,6 +129,8 @@ >>>> "kernel_addr_r=0x18000000\0" \ >>>> "scriptaddr=0x02000000\0" \ >>>> "ramdisk_addr_r=0x02100000\0" \ >>>> + "script_offset_f=0x3e80000\0" \ >>>> + "script_size_f=0x80000\0" \ >>>> >>>> #if defined(CONFIG_MMC_SDHCI_ZYNQ) >>>> # define BOOT_TARGET_DEVICES_MMC(func) func(MMC, mmc, 0) func(MMC, mmc, 1) >>>> @@ -160,8 +162,38 @@ >>>> # define BOOT_TARGET_DEVICES_DHCP(func) >>>> #endif >>>> >>>> +#if defined(CONFIG_ZYNQMP_GQSPI) >>>> +# define BOOT_TARGET_DEVICES_QSPI(func) func(QSPI, qspi, 0) >>>> +#else >>>> +# define BOOT_TARGET_DEVICES_QSPI(func) >>>> +#endif >>>> + >>>> +#if defined(CONFIG_NAND_ARASAN) >>>> +# define BOOT_TARGET_DEVICES_NAND(func) func(NAND, nand, 0) >>>> +#else >>>> +# define BOOT_TARGET_DEVICES_NAND(func) >>>> +#endif >>>> + >>>> +#define BOOTENV_DEV_QSPI(devtypeu, devtypel, instance) \ >>>> + "bootcmd_qspi0=sf probe 0 0 0 && " \ >>> >>> This >>> >>>> + "sf read $scriptaddr $script_offset_f $script_size_f && " \ >>>> + "source ${scriptaddr}; echo SCRIPT FAILED: continuing...;\0" >>>> + >>>> +#define BOOTENV_DEV_NAME_QSPI(devtypeu, devtypel, instance) \ >>>> + "qspi " >>>> + >>>> +#define BOOTENV_DEV_NAND(devtypeu, devtypel, instance) \ >>>> + "bootcmd_nand0= nand info && " \ >>> >>> and this will emit errors if no device was found, right? That might be >>> confusing for a user, if all he sees is a scary "device not found" >>> message which simply only occurs because he doesn't have nand populated. >> >> As you see above it is enabled only when devices are enabled. >> For generic u-boot for all zynqmp targets it should be correct to emit >> error message that nand is not detected if it is not present on the >> board. :-) >> All these boot modes will be tried if the first boot mode fails. > > Well, it's conditional on whether the driver is available, not whether > the board actually exposes a flash device, right? > > So think about the generic case - one U-Boot binary for all targets. > What are you going to do there? That needs to have all possible boot > target devices enabled, but how do you know whether an actual QSPI chip > is attached to the SPI controller? yes - sure all devices needs to be enabled. You don't know that. You need to try to probe it and access it. > > I also think this problem is bigger than this patch, so don't take my > comment as a nack. I'm just trying to raise that we should start to > think about cases where "probe fails" is not an error and thus should > not print an error message. What is the current behavior if you failed to access network or SD and images is on SATA which is the last (or different order). I expect that scanning finds nothing. Nand/qspi should be just behave the same. Thanks, Michal
On 25.01.19 13:13, Michal Simek wrote: > On 25. 01. 19 13:00, Alexander Graf wrote: >> >> >> On 25.01.19 12:56, Michal Simek wrote: >>> On 25. 01. 19 12:25, Alexander Graf wrote: >>>> >>>> >>>> On 24.01.19 16:31, Michal Simek wrote: >>>>> From: Siva Durga Prasad Paladugu <siva.durga.paladugu@xilinx.com> >>>>> >>>>> This patch adds distro boot commands for qspi and nand. The >>>>> distro boot commands now reads the script from flash offset >>>>> of 63.5MB and executes it. >>>>> >>>>> Setup default location via script_offset_f to 63.5MB to match the most >>>>> xilinx reference boards. 512kB allocated space for script size >>>>> (script_size_f) should be more than enough to cover custom boot logic. >>>>> >>>>> Signed-off-by: Siva Durga Prasad Paladugu <siva.durga.paladugu@xilinx.com> >>>>> Signed-off-by: Michal Simek <michal.simek@xilinx.com> >>>>> --- >>>>> >>>>> include/configs/xilinx_zynqmp.h | 32 ++++++++++++++++++++++++++++++++ >>>>> 1 file changed, 32 insertions(+) >>>>> >>>>> diff --git a/include/configs/xilinx_zynqmp.h b/include/configs/xilinx_zynqmp.h >>>>> index d0515fa9bc46..5eeae6331e1c 100644 >>>>> --- a/include/configs/xilinx_zynqmp.h >>>>> +++ b/include/configs/xilinx_zynqmp.h >>>>> @@ -129,6 +129,8 @@ >>>>> "kernel_addr_r=0x18000000\0" \ >>>>> "scriptaddr=0x02000000\0" \ >>>>> "ramdisk_addr_r=0x02100000\0" \ >>>>> + "script_offset_f=0x3e80000\0" \ >>>>> + "script_size_f=0x80000\0" \ >>>>> >>>>> #if defined(CONFIG_MMC_SDHCI_ZYNQ) >>>>> # define BOOT_TARGET_DEVICES_MMC(func) func(MMC, mmc, 0) func(MMC, mmc, 1) >>>>> @@ -160,8 +162,38 @@ >>>>> # define BOOT_TARGET_DEVICES_DHCP(func) >>>>> #endif >>>>> >>>>> +#if defined(CONFIG_ZYNQMP_GQSPI) >>>>> +# define BOOT_TARGET_DEVICES_QSPI(func) func(QSPI, qspi, 0) >>>>> +#else >>>>> +# define BOOT_TARGET_DEVICES_QSPI(func) >>>>> +#endif >>>>> + >>>>> +#if defined(CONFIG_NAND_ARASAN) >>>>> +# define BOOT_TARGET_DEVICES_NAND(func) func(NAND, nand, 0) >>>>> +#else >>>>> +# define BOOT_TARGET_DEVICES_NAND(func) >>>>> +#endif >>>>> + >>>>> +#define BOOTENV_DEV_QSPI(devtypeu, devtypel, instance) \ >>>>> + "bootcmd_qspi0=sf probe 0 0 0 && " \ >>>> >>>> This >>>> >>>>> + "sf read $scriptaddr $script_offset_f $script_size_f && " \ >>>>> + "source ${scriptaddr}; echo SCRIPT FAILED: continuing...;\0" >>>>> + >>>>> +#define BOOTENV_DEV_NAME_QSPI(devtypeu, devtypel, instance) \ >>>>> + "qspi " >>>>> + >>>>> +#define BOOTENV_DEV_NAND(devtypeu, devtypel, instance) \ >>>>> + "bootcmd_nand0= nand info && " \ >>>> >>>> and this will emit errors if no device was found, right? That might be >>>> confusing for a user, if all he sees is a scary "device not found" >>>> message which simply only occurs because he doesn't have nand populated. >>> >>> As you see above it is enabled only when devices are enabled. >>> For generic u-boot for all zynqmp targets it should be correct to emit >>> error message that nand is not detected if it is not present on the >>> board. :-) >>> All these boot modes will be tried if the first boot mode fails. >> >> Well, it's conditional on whether the driver is available, not whether >> the board actually exposes a flash device, right? >> >> So think about the generic case - one U-Boot binary for all targets. >> What are you going to do there? That needs to have all possible boot >> target devices enabled, but how do you know whether an actual QSPI chip >> is attached to the SPI controller? > > yes - sure all devices needs to be enabled. You don't know that. You > need to try to probe it and access it. > >> >> I also think this problem is bigger than this patch, so don't take my >> comment as a nack. I'm just trying to raise that we should start to >> think about cases where "probe fails" is not an error and thus should >> not print an error message. > > What is the current behavior if you failed to access network or SD and > images is on SATA which is the last (or different order). > I expect that scanning finds nothing. > Nand/qspi should be just behave the same. It depends on the target; some will just not complain if no device was found, others will. It's a bigger task, let's defer it to later :) Alex
diff --git a/include/configs/xilinx_zynqmp.h b/include/configs/xilinx_zynqmp.h index d0515fa9bc46..5eeae6331e1c 100644 --- a/include/configs/xilinx_zynqmp.h +++ b/include/configs/xilinx_zynqmp.h @@ -129,6 +129,8 @@ "kernel_addr_r=0x18000000\0" \ "scriptaddr=0x02000000\0" \ "ramdisk_addr_r=0x02100000\0" \ + "script_offset_f=0x3e80000\0" \ + "script_size_f=0x80000\0" \ #if defined(CONFIG_MMC_SDHCI_ZYNQ) # define BOOT_TARGET_DEVICES_MMC(func) func(MMC, mmc, 0) func(MMC, mmc, 1) @@ -160,8 +162,38 @@ # define BOOT_TARGET_DEVICES_DHCP(func) #endif +#if defined(CONFIG_ZYNQMP_GQSPI) +# define BOOT_TARGET_DEVICES_QSPI(func) func(QSPI, qspi, 0) +#else +# define BOOT_TARGET_DEVICES_QSPI(func) +#endif + +#if defined(CONFIG_NAND_ARASAN) +# define BOOT_TARGET_DEVICES_NAND(func) func(NAND, nand, 0) +#else +# define BOOT_TARGET_DEVICES_NAND(func) +#endif + +#define BOOTENV_DEV_QSPI(devtypeu, devtypel, instance) \ + "bootcmd_qspi0=sf probe 0 0 0 && " \ + "sf read $scriptaddr $script_offset_f $script_size_f && " \ + "source ${scriptaddr}; echo SCRIPT FAILED: continuing...;\0" + +#define BOOTENV_DEV_NAME_QSPI(devtypeu, devtypel, instance) \ + "qspi " + +#define BOOTENV_DEV_NAND(devtypeu, devtypel, instance) \ + "bootcmd_nand0= nand info && " \ + "nand read $scriptaddr $script_offset_f $script_size_f && " \ + "source ${scriptaddr}; echo SCRIPT FAILED: continuing...;\0" + +#define BOOTENV_DEV_NAME_NAND(devtypeu, devtypel, instance) \ + "nand " + #define BOOT_TARGET_DEVICES(func) \ BOOT_TARGET_DEVICES_MMC(func) \ + BOOT_TARGET_DEVICES_QSPI(func) \ + BOOT_TARGET_DEVICES_NAND(func) \ BOOT_TARGET_DEVICES_USB(func) \ BOOT_TARGET_DEVICES_SCSI(func) \ BOOT_TARGET_DEVICES_PXE(func) \