Hi Vasileios, út 4. 6. 2024 v 15:21 odesílatel Vasileios Amoiridis <[email protected]> napsal: > > From: Vasileios Amoiridis <[email protected]> > > Changes in v2: > - Remove duplication of custom hardcoded env_locations[] code. > - Add implementation with general arch_env_get_location(op, prio) > > v1: > https://lore.kernel.org/u-boot/[email protected]/ > > Vasileios Amoiridis (1): > xilinx: Add option to load environment from outside of boot media > > board/xilinx/versal-net/board.c | 47 ++++++++++++++-------------- > board/xilinx/versal/board.c | 47 ++++++++++++++-------------- > board/xilinx/zynq/board.c | 49 +++++++++++++++-------------- > board/xilinx/zynqmp/zynqmp.c | 55 +++++++++++++++++---------------- > 4 files changed, 101 insertions(+), 97 deletions(-) > > -- > 2.34.1 >
I have to remove this patch from my queue because it is actually breaking current behavior. CI is reporting an issue with it and I did a closer look and what is happening is that if one location has no valid data it goes and looks at another location from the list. On Zynq it behaves like this. MMC: mmc@e0100000: 0 prio 0 Loading Environment from SPIFlash... SF: Detected n25q128a13 with page size 256 Bytes, erase size 64 KiB, total 16 MiB *** Warning - bad CRC, using default environment prio 1 Loading Environment from NAND... *** Error - No Valid Environment Area found *** Warning - bad env area, using default environment prio 2 Loading Environment from SPIFlash... *** Warning - bad CRC, using default environment prio 3 Loading Environment from nowhere... OK prio is my print to show where code is. Qemu boots out in QSPI boot mode and SPI is tried first and because this is xilinx_zynq_virt defconfig drivers/env locations for other devices are present too. That's why it goes over the list and it always ends in nowhere which never fails. If this runs on real HW then the same behavior is visible and I don't think it is right. I think this solution should be rethought. In product current behavior from our code is not the best and current U-Boot implementation is not allowing flexibility too. We are enabling redundant variables which can be only on the same device but not that you have one copy in QSPI and second in EMMC. That's why I think location should be pretty much read from DT. There is options/u-boot node where location of variables should be described and this should replace env_get_location(). It is a lot of work that's why I think second solution is more reasonable for you which is simply create new Kconfig symbol to disable board env_get_location() implementation. Thanks, Michal -- Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91 w: www.monstr.eu p: +42-0-721842854 Maintainer of Linux kernel - Xilinx Microblaze Maintainer of Linux kernel - Xilinx Zynq ARM and ZynqMP ARM64 SoCs U-Boot custodian - Xilinx Microblaze/Zynq/ZynqMP/Versal SoCs
