On 6/12/24 15:57, Vasileios Amoiridis wrote:
On Wed, 2024-06-12 at 15:42 +0200, Michal Simek wrote:
Hi Vasileios,
út 4. 6. 2024 v 15:21 odesílatel Vasileios Amoiridis
<vassilisa...@gmail.com> napsal:
From: Vasileios Amoiridis <vasileios.amoiri...@cern.ch>
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/20240522174738.73522-1-vassilisa...@gmail.com/
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.
But that is the desired behavior, if the environment is not in one
location to check to the others.
That's how u-boot is written but if device doesn't exist or not accessible by
u-boot it is clear that it shouldn't be used and then behavior is correct.
If device exists and simple varibles are not yet saved there going to different
device is IMHO problematic.
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
This is the message that we get as well when this patch is not added.
It means you are booting out of QSPI but qspi is not accessible by u-boot.
Correct?
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().
You mean that there should be some type of new U-Boot node that
describes where the environment should be located and go and
search in this device?
Not sure if node or just dt property which points where that variables are
stored.
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
You mean to basically disable the board env_get_location() that
exists in board/xilinx/zynqmp/zynqmp.c for example?
yes. Something like this.
config BOARD_ENV_LOCATION
bool "Use board defined ENV location"
default y
...
if defined (BOARD_ENV_SETUP/LOCATION)
enum env_location env_get_location(enum env_operation op, int prio)
{
...
}
endif
M
--
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/Versal ARM64 SoCs
U-Boot custodian - Xilinx Microblaze/Zynq/ZynqMP/Versal/Versal NET SoCs
TF-A maintainer - Xilinx ZynqMP/Versal/Versal NET SoCs
