Am 20. Januar 2026 18:55:55 MEZ schrieb Quentin Schulz <[email protected]>: >Hi Frank, > >On 1/20/26 5:09 PM, Frank Wunderlich wrote: >> Hi Quentin, >> >> Am 20. Januar 2026 13:47:00 MEZ schrieb Quentin Schulz >> <[email protected]>: >>> Hi Frank, >>> >>> On 1/20/26 1:09 PM, Frank Wunderlich wrote: >>>> Hi >>>> >>>> Thank you for your review >>>> >>>> Am 19. Januar 2026 11:22:07 MEZ schrieb Quentin Schulz >>>> <[email protected]>: >>>>> Hi Frank, >>>>> >>>>> On 1/18/26 11:50 AM, Frank Wunderlich wrote: >>> > >[...] >>>>> - what if I want to save the byte size in a variable? With the current >>>>> implementation, only mebibyte or gibibyte sizes can be stored. I would >>>>> suggest to argc-- if m or g is passed, and then compare against argc > 1 >>>>> in which case you'd use this third (in case m or g is passed) or second >>>>> (if they aren't) argument to store the value. >>>> >>>> Good idea, thinking about how to implement this the correct way :) as >>>> varname could be "g" or "b" too. >>>> >>> >>> You could also force the use of the first argument to be able to dump to a >>> variable. >>> >>> memsize >>> memsize b|m|g [varname] >>> >>> memsize would be an alias to memsize b. >>> memsize b var stores the byte size in var. >>> memsize m var stores the (truncated) mebibyte size in var. >>> memsize g var stores the (truncated) gibibyte size in var. >> >> I could also drop the first param and always handle as MiB...uboot also >> prints ram as MiB and not GiB on initialization. >> > >It does print GiB on Rockchip SoCs, so there doesn't seem to be a "standard" >for that. > >> So only choose between print or set variable and no "floating point" >> calculation needs to be >> done...and result is simply an int. Afaik there should be no more boards >> with less than 1MB >> ram,or am i wrong? > >Not aware but that would be quite the headache (you need to load *some* stuff >in RAM, e.g. U-Boot itself (well maybe it could be in SRAM only?), the >kernel). I would say, it's fine to truncate at MiB level. > >> I think this makes most sense..and keeps code small and clean. >> >>> You could also decide the prefix your size argument with a dash and if no >>> argument with a dash is passed, it means it's a variable name and the >>> default is in byte size. >>> >>>>> - missing documentation on the base for the environment variable. We have >>>>> some commands setting hex digits in variables, some others in decimal, so >>>>> this needs to be made explicit so that people know if they need to >>>>> translate stuff before using it (or how to compare two numbers against >>>>> each other). Here it's decimal according to the code (maybe we should >>>>> have it in hex instead? I am not sure what is the expected base for >>>>> variables :/). >>>> >>>> As i want to use it in scripts to choose correct bl2 file for bananapi r4. >>>> Imho decimal is better to read here,but it should be mentioned in doc. >>>> >>> >>> OK. Considering test >>> (https://docs.u-boot.org/en/latest/usage/cmd/test.html#numbers) compares >>> numbers assuming they're decimal if they don't start with 0x, this is fine >>> to do (we would need to prefix the number with 0x if we were to store it as >>> an hex value). >> >> But this means numbers can be no fload and filesize is wrong as it returns >> hex value without 0x prefix. >> > >Yes to both. I was actually agreeing with your implementation (using decimal) >being the best option because we can directly use it with the test command. > >>> Can you tell us more about the initial issue and how you plan on fixing it >>> here? >>> >>> That sounds like a board file (board/....) would be potentially more >>> appropriate for this? What is this bl2, why does it need to be different >>> based on DRAM size, isn't there anyway for U-boot to pass the DRAM size to >>> bl2 and only have one bl2 binary for example? >> >> BL2 is bootloader after bootrom (ATF). The board can have 2 different ram hw > >Is this a typo? You say later that you want U-Boot to select the proper BL2, >so necessarily U-Boot is loaded before BL2 by the BootROM? > >Do you mean ATF to be "bootrom" or bl2 here? (I'm assuming the latter).
Bootrom (hardware bootup) -> atf bl2 -> uboot packed into bl31. >> configurations which require BL2 compiled with different options set. The >> purpose for uboot >> command is checking available ram for updating the BL2 via tftp and picking >> the right >> file based on current installed bl2 via detected ramsize. >> >> The issue is that 4gb board does not boot with 8gb ram bl2, 8gb board boots >> with 4gb bl2,but >> cannot use the 8gb (only 4gb). So i tried to make a detection mechanism to >> select the right >> file (tftp,usb,...) for current ram configuration. >> > >If BL2 is TF-A, do you have an open-source implementation for your board >(maybe https://github.com/mtk-openwrt/arm-trusted-firmware?)? Would it be >possible to pass e.g. a Device Tree to TF-A to pass some information? I'm >assuming we must be configuring somewhere the DRAM address ranges so the >kernel knows where to put stuff. On Rockchip, we use the Device Tree in TF-A >(ATF) to know to which serial controller to write to and at which baudrate >(downstream TF-A blob from Rockchip doesn't support that), wondering if we can >do something similar for Mediatek? This way you don't need to know at runtime >what to do, it'll automagically do it, and that for every board with the SoC >supported in TF-A. Yes this repo and yes TF-A,but there is no way to detect ramsize / or config automaticly in bl2. Dram calibration is a binary blob there. So we have to flash the right bl2 and use uboot to update to newer version or to another bootdevice (sdmmc -> spi nand -> emmc. So my usecase is for installing/updating bl2 only. >>>>>> + >>>>>> + return 0; >>>>>> +} >>>>>> +#endif /* CONFIG_CMD_MEMSIZE */ >>>>>> + >>>>>> #ifdef CONFIG_CMD_MEMTEST >>>>>> static ulong mem_test_alt(volatile ulong *buf, ulong start_addr, >>>>>> ulong end_addr, >>>>>> volatile ulong *dummy) >>>>>> @@ -1404,6 +1428,14 @@ U_BOOT_CMD( >>>>>> ); >>>>>> #endif /* CONFIG_LOOPW */ >>>>>> +#ifdef CONFIG_CMD_MEMSIZE >>>>>> +U_BOOT_CMD( >>>>>> + msize, 3, 1, do_mem_size, >>>>>> + "get detected ram size, optional set env variable with value", >>>>>> + "[m, g] [envvar]" >>>>> >>>>> Explain m and g so the user knows when typing help msize. >>>>> >>>>> We need tests in test/cmd/ and a new entry in doc/usage/cmd/ for this new >>>>> command. >>>> >>>> I see this note in checkpatch,but as i rely on dynamic value and i saw no >>>> test for mem yet. >>>> >>> >>> There's one for meminfo, but honestly I'm not sure it's a test that makes >>> sense (just checking whether some fixed strings are printed and not >>> associated values that are printed afterwards). I haven't written a C unit >>> test in U-Boot so far so won't be able to help there. But we really do want >>> docs in doc/usage/cmd for any new command. >> >> Not sure if such test makes sense except checking if function returns a >> string based on unknown gd->ram_size (which depends on the host running the >> test i guess),or can it set before runing the test so that i can just check >> the formatting/calculation of the string? >> > >I don't know. It's fine for me if you don't write a test for this, I'm not >sure if you can mock the DRAM size for example. The point would have been to >check you're properly truncating and reporting the right size as well as >writing to the specified variable the appropriate value in decimal. Maybe Tom can answer this? As far as i see meminfo also uses gd->ram_size and called without parameter and seems to check for fixed strings (container which runs the tests seems like having 256MiB RAM...when this container changes,test will fail i guess. <https://elixir.bootlin.com/u-boot/v2026.01/source/cmd/meminfo.c#L60> <https://elixir.bootlin.com/u-boot/v2026.01/source/test/cmd/meminfo.c#L17> >Cheers, >Quentin regards Frank
