Am 3. Juni 2025 18:22:46 MESZ schrieb Tom Rini <[email protected]>: >On Tue, Jun 03, 2025 at 12:14:44PM +0530, Sughosh Ganu wrote: >> On Fri, 30 May 2025 at 15:41, Sughosh Ganu <[email protected]> wrote: >> > >> > On Fri, 30 May 2025 at 13:17, Ilias Apalodimas >> > <[email protected]> wrote: >> > > >> > > > +static phys_addr_t lmb_alloc(phys_size_t size) >> > > > +{ >> > > > + int ret; >> > > > + phys_addr_t addr; >> > > > + >> > > > + /* All memory regions allocated with a 2MiB alignment */ >> > > > + ret = lmb_alloc_mem(LMB_MEM_ALLOC_ANY, SZ_2M, &addr, size, >> > > > LMB_NONE); >> > > > + if (ret) >> > > > + return 0; >> > > > + >> > > > + return addr; >> > > > +} >> > > > + >> > > >> > > I think we need a better naming for these. Right now we have >> > > lmb_alloc() here and in tests, addr_alloc() in snapdragon code. >> > > I'd say either export them as API if you think they would be useful, >> > > or get rid of the wrappers. >> > >> > I kept these functions as is to reduce the amount of change resulting >> > from introducing the API. Also, the newly introduced API has >> > parameters which are common across all the callers, which also was why >> > I kept a wrapper. This is especially true in the tests, where it would >> > be required to change the function names in a bunch of places without >> > much benefit. >> > >> > > >> > > [...] >> > > >> > > > +static phys_addr_t lmb_alloc(phys_size_t size, ulong align) >> > > > +{ >> > > > + int err; >> > > > + phys_addr_t addr; >> > > > + >> > > > + err = lmb_alloc_mem(LMB_MEM_ALLOC_ANY, align, &addr, size, >> > > > LMB_NONE); >> > > > + if (err) >> > > > + return 0;
Would lmb ever allocate at (phys_addr_t)-1 ? I guess this should work for all systems as error indicator. Best regards Heinrich >> > > >> > > >> > > This tends to blow up in random ways. See >> > > commit 67be24906fe. TL;DR 0 is a valid address in some systems. >> > >> > Yes, I see your point. I think the calling function will have to be >> > re-written such that the env variables get stored only when the API >> > returns successfully. Then at least the platform will not have an env >> > variable with some junk value. >> >> Thinking about this a bit, I think in these two instances, returning a >> value of 0 might not be an issue if the DRAM memory does not start at >> 0x0. Don't get me wrong, what you are suggesting is definitely >> correct. I am only thinking about increasing code size on these >> platforms if 0x0 is not a valid address, and moreover since the >> platforms were already setting 0x0 in case of an error. If it is okay >> to increase code size on these platforms, I will change the calling >> function, such that the variable does not get set in case of an error. >> Maybe Casey and Mark can comment? Thanks. > >Systems where 0x0 is not valid memory are common enough I think, yes. >You can see what the size growth is by checking say j721e_evm_a72 >before/after, and if you haven't been using buildman for size >comparisons already, I posted >https://source.denx.de/u-boot/u-boot-extras/-/blob/master/contrib/trini/u-boot-size-test.sh?ref_type=heads >a while back now. Thanks. >
