Hi E,
On 6/27/25 9:15 PM, E Shattow wrote:
On 6/27/25 07:04, Quentin Schulz wrote:
From: Quentin Schulz <[email protected]>
I got very confused by the address printed when erasing the environment
because I clearly didn't remember setting it at that location.
It turns out, it's the block offset and not the byte offset that is
reported, so let's make this a bit more obvious to the user.
Signed-off-by: Quentin Schulz <[email protected]>
---
Another option would be to report the byte offset by multiplying
blk_start by desc->blksz I guess. The result needs to be stored in a u64
though instead of uint otherwise we'll be limited to 4GiB offset (which
should be fine in most cases but eh).
---
env/mmc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/env/mmc.c b/env/mmc.c
index
8848371eb4f5df58da2be5398f159fdd0a00da21..f16926dc74c594b15f0bc82301aa89e1d4456d59
100644
--- a/env/mmc.c
+++ b/env/mmc.c
@@ -383,7 +383,7 @@ static inline int erase_env(struct mmc *mmc, unsigned long
size,
blk_cnt = ALIGN(size, erase_size) / desc->blksz;
n = blk_derase(desc, blk_start, blk_cnt);
- printf("%d blocks erased at 0x%x: %s\n", n, blk_start,
+ printf("%d blocks erased starting from block at 0x%x: %s\n", n,
blk_start,
(n == blk_cnt) ? "OK" : "ERROR");
return (n == blk_cnt) ? 0 : 1;
---
base-commit: 359e3012921f2fc2d43f3c4e320a752173f82b82
change-id: 20250627-env-mmc-erase-blk-7a46a3e63c78
Best regards,
NAK. This is more text on the line and less readable; by doing this here
I can suggest "%d blocks erased from block 0x%x: %s\n" instead which I
still find more helpful than what we currently have?
and nowhere else the overall effect is adding to the confusion.
Look at this (topically related) snippet output:
"Bytes transferred = 534773760 (1fe00000 hex)
MMC read: dev # 0, block # 8355840, count 1044480 ... 1044480 blocks
read: OK
Total of 534773760 byte(s) were the same"
What units and packet lengths are these numbers? It's hopeless. All of
dev # 0 is the device number, it is unlikely you'll have more than 10
MMC devices, so whether it's hex or dec base doesn't matter much as the
value in both bases would be the same.
Block number doesn't contain any hex character, so I would assume that
to be dec but as you rightfully reported, we have a mix of hex and dec
values all over the code base with and without the 0x prefix to
differentiate which is which. Nothing I can do about that though. MMC
has a block size of 512B, considering the total transfer size in bytes
is 534773760 which matches the decimal number in the Bytes transferred
line, it is decimal and 1044480 is in decimal format as well since
1044480 * 512 = 534773760.
I didn't read the code and got the information anyway, it's not great UX
but aside from the block number, it should be fine?
that output is garbage to me as the user, missing context required to
make sense of it. Adding word-qualifiers I suppose attempts to add
clarity but the result lacks any meaning. The command which does get it
right about hexadecimal v. decimal units does so with a displaying a
literal word qualifier "hex" suffix, not consistent with what we're
doing elsewhere.
There's no consistency in U-Boot's code base AFAIK.
I do agree with your premise that hex address locations and sizes, and
block counts, are presented in a confusing mix-and-match to the user.
Decimal numbering is the exception and not the rule but we somehow all
decide that it's *not important* to make this obvious to the user who
will have to input everything (except only some things, some times, but
non-obviously) as hexadecimal. We insist on showing "0x" prefix but user
input without this prefix is still considered as hexadecimal which is
just terrible UX all around.
Again, not consistent across the code base AFAIR, so it's even worse
than that.
Instead of this one patch in one place and nowhere else - I suggest to
fix the problem which is U-Boot code base (and code style) has no
discernible convention for documenting and displaying these outputs. I
understand the technical limitations of decimal user input/output
exceeding the memory type sizes, but...
#1 rule: DO NOT show me decimal output while only accepting hexadecimal
at input, it is ridiculous.
also #1 rule: Packet length not-equal-to-one? Display the packet length
with the output.
Our consistency of units in both documentation and display of
information is the problem - which I really do want to see resolved...
can we?
Anything that is displayed and not parsed should be ok, but then you'd
have commands such as
u-boot> commandx bebe
Successfully did something with 48830
Is that really less confusing?
Also, what you are asking is to check and rewrite all printfs, log
messages, etc... from all drivers, subsystems, commands, etc... which
1) isn't a small feat
2) would require all maintainers to agree on that (and a single one
disagreeing would still make everything confusing enough as it would be
rule for thee but not for me)
Also, this is entirely unrelated to this patch as I'm providing the unit
of the value and not the base (which for once is explicit).
Cheers,
Quentin
