Hi Sughosh, On Sat, 17 Aug 2024 at 04:06, Sughosh Ganu <[email protected]> wrote: > > On Sat, 17 Aug 2024 at 05:24, Simon Glass <[email protected]> wrote: > > > > Hi Sughosh, > > > > On Fri, 16 Aug 2024 at 11:53, Sughosh Ganu <[email protected]> wrote: > > > > > > On Fri, 16 Aug 2024 at 22:39, Sughosh Ganu <[email protected]> > > > wrote: > > > > > > > > On Fri, 16 Aug 2024 at 20:17, Simon Glass <[email protected]> wrote: > > > > > > > > > > Hi Sughosh, > > > > > > > > > > On Fri, 16 Aug 2024 at 04:34, Sughosh Ganu <[email protected]> > > > > > wrote: > > > > > > > > > > > > On Fri, 16 Aug 2024 at 02:02, Simon Glass <[email protected]> wrote: > > > > > > > > > > > > > > Hi Sughosh, > > > > > > > > > > > > > > On Wed, 14 Aug 2024 at 05:02, Sughosh Ganu > > > > > > > <[email protected]> wrote: > > > > > > > > > > > > > > > > Instead of a randomly selected address, use an LMB allocated > > > > > > > > one for > > > > > > > > reading the file into memory. With the LMB map now being > > > > > > > > persistent > > > > > > > > and global, the address used for reading the file might be > > > > > > > > already > > > > > > > > allocated as non-overwritable, resulting in a failure. Get a > > > > > > > > valid > > > > > > > > address from LMB and then read the file to that address. > > > > > > > > > > > > > > > > Signed-off-by: Sughosh Ganu <[email protected]> > > > > > > > > --- > > > > > > > > Changes since V1: > > > > > > > > * Don't use the API version with flags parameter. > > > > > > > > > > > > > > > > test/boot/cedit.c | 6 +++++- > > > > > > > > 1 file changed, 5 insertions(+), 1 deletion(-) > > > > > > > > > > > > > > > > > > > > > > No, this address needs to work fine without using lmb. Same with > > > > > > > any > > > > > > > other tests. Tests make use of the sandbox memory space memory > > > > > > > addresses and it makes things easier to code and debug. > > > > > > > > > > > > Like I had explained earlier [1], not using the LMB API for > > > > > > allocating > > > > > > the address results in issues, since the load command internally > > > > > > checks if the address can be used for reading the dtb. Without this > > > > > > patch, the cmd_ut test fails. I am not sure why you do not like this > > > > > > solution. But in any case, can you propose some other solution? I > > > > > > believe I can tweak the address to some other value, but that would > > > > > > not be a proper solution, but simply kicking the can down the road. > > > > > > Thanks. > > > > > > > > > > But this is why I suggested having lmb_push() and lmb_pop(), so you > > > > > should be able to use those in the few tests which have this problem. > > > > > > > > Ah yes, I forgot about that change. Let me try running the test again > > > > and see what I get. Thanks. > > > > > > Sorry, I think I was hasty in my reply. This would not work. We have > > > the lmb_push()/lmb_get() functions only for the lmb specific tests. In > > > this scenario, we have the load command which checks the lmb address > > > against reservations, which are the "normal operation" memory map. So, > > > if we have to have this to work, this might need a way of using a new > > > lmb structure instance for every test. So that every test that gets > > > invoked, does so like the way the lmb tests are. > > > > Why does the 'load' operation fail? When I revert this patch the tests > > still seem to pass. > > I had not run the tests then. I thought that the address might > conflict with some existing reservation made with a different flag. I > have run the tests, and I too see that the CI goes through even with > this patch dropped. I still think that getting an address from LMB and > then using it is more robust, but then the same flow is being used in > many other tests as well. This should work as long as we don't get any > conflicting reservation with a different flag. I will drop this patch > in the next version. Thanks.
OK, well let's see if it does cause problems later. We can use the push/pop feature if needed down the track. > > > > > If it becomes more widespread and affects a lot of tests, we could do > > > > > thing automatically in the test framework, but for now, that doesn't > > > > > seem necessary. > > > > > > > > > > > [1] - https://lists.denx.de/pipermail/u-boot/2024-July/560569.html Regards, Simon
