[PATCH] env: mmc: specify that the address is a block address when erasing

[E Shattow]

On 6/27/25 07:04, Quentin Schulz wrote:
> From: Quentin Schulz <quentin.schulz at cherry.de>
> 
> 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 <quentin.schulz at cherry.de>
> ---
> 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
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
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.

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.

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?

-E