On 15/02/22, Michal Simek wrote: > Hi, > > On 2/14/22 21:10, Ricardo Salveti wrote: > > Hi Michal, > > > > This is a bit similar to the issue raised on iMX8-based targets a few > > days ago, which is forcing the environment location based on the boot > > mode and not allowing the user to use a different option via other > > CONFIG options. > > > > Should we really force the env location based on boot mode? Currently > > there is no way to boot out of QSPI and save the environment on > > emmc/fat/ext4, and removing CONFIG_ENV_IS_IN_SPI_FLASH causes the env > > location to be set as ENVL_NOWHERE, which is not ideal, especially > > when other env target locations are available at build time. > > > > diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c > > index f0be9c022a7..08afb49570a 100644 > > --- a/board/xilinx/zynqmp/zynqmp.c > > +++ b/board/xilinx/zynqmp/zynqmp.c > > @@ -969,6 +969,10 @@ enum env_location env_get_location(enum > > env_operation op, int prio) > > case QSPI_MODE_32BIT: > > if (IS_ENABLED(CONFIG_ENV_IS_IN_SPI_FLASH)) > > return ENVL_SPI_FLASH; > > + if (IS_ENABLED(CONFIG_ENV_IS_IN_FAT)) > > + return ENVL_FAT; > > + if (IS_ENABLED(CONFIG_ENV_IS_IN_EXT4)) > > + return ENVL_EXT4; > > return ENVL_NOWHERE; > > case JTAG_MODE: > > default: > > > > A change like this one would allow other locations to be set, and just > > use the boot mode to assign the priority instead. > > > > Since I couldn't really find out what is the expected behavior on > > functions defined at soc/board level (env.c sets based on priority, > > depending on what is enabled at build time), I was wondering if we > > shouldn't just drop this function entirely. > > > > While I agree the board should have a set of defaults, not allowing > > the user to change something like env location via CONFIGs seems wrong > > to me. > > > > Let me know what you think. > > Default location is setup based on how Xilinx sees where that variables > should be saved for the most cases that's why that function was done like > this. > That's why I think that it is very reasonable default setup.
I disagree. I see no actual need to hardcode the environment location the way it is done. It serves no purpose. > And as is hopefully known we are using pretty generic u-boot which should > work on all configurations it means default defconfig have all options > enabled. > > Does it make sense to have an option to change it to any configuration? > Definitely. > > How to do it? > DT is for us source of truth that's why I prefer to have DT description > which is providing all information. It means say where variables should be > stored, where redundant variables should be stored. IIRC as of today I think > they have to be on the same device which can be also more flexible. You can > specify different start/end in qspis, etc. > > What has to happen? > Someone has to take a lead and come up with generic universal DT binding to > be able describe it. Some days ago linaro had similar issue with DT in > connection to A/B update via GPT partition. And description for it should be > pretty much the same as is for variables. are you saying that unless you have a DT change you wont let the user choose where the place the environment? if so, I'd like to undestand why. As I said, it seems biased and unreasoanble hence why I think it deserves further justification. > > Thanks, > Michal