[U-Boot] [PATCH 2/2] arm64: zynqmp: Define distro boot commnads for qspi and nand

[Alexander Graf]

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 at 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 at xilinx.com>
>>> Signed-off-by: Michal Simek <michal.simek at 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
>