[PATCH 1/2] bootm: adjust the print format

[PATCH 1/2] bootm: adjust the print format

[Dario Binacchi]

All three addresses printed are in hexadecimal format, but only the
first two have the "0x" prefix. The patch aligns the format of the
"end" address with the other two by adding the "0x" prefix.

Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
---

 boot/bootm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/boot/bootm.c b/boot/bootm.c
index 480f8e6a0e6e..951e549f19ff 100644
--- a/boot/bootm.c
+++ b/boot/bootm.c
@@ -703,7 +703,7 @@ static int bootm_load_os(struct bootm_headers *images, int boot_progress)
 
 		/* Handle BOOTM_STATE_LOADOS */
 		if (relocated_addr != load) {
-			printf("Moving Image from 0x%lx to 0x%lx, end=%lx\n",
+			printf("Moving Image from 0x%lx to 0x%lx, end=0x%lx\n",
 			       load, relocated_addr,
 			       relocated_addr + image_size);
 			memmove((void *)relocated_addr, load_buf, image_size);
-- 
2.43.0

[PATCH 2/2] cmd: booti: adjust the print format

[Dario Binacchi]

All three addresses printed are in hexadecimal format, but only the
first two have the "0x" prefix. The patch aligns the format of the
"end" address with the other two by adding the "0x" prefix.

Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>

---

 cmd/booti.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/cmd/booti.c b/cmd/booti.c
index 62b19e834366..ea811244a0a9 100644
--- a/cmd/booti.c
+++ b/cmd/booti.c
@@ -78,7 +78,7 @@ static int booti_start(struct bootm_info *bmi)
 
 	/* Handle BOOTM_STATE_LOADOS */
 	if (relocated_addr != ld) {
-		printf("Moving Image from 0x%lx to 0x%lx, end=%lx\n", ld,
+		printf("Moving Image from 0x%lx to 0x%lx, end=0x%lx\n", ld,
 		       relocated_addr, relocated_addr + image_size);
 		memmove((void *)relocated_addr, (void *)ld, image_size);
 	}
-- 
2.43.0

Re: [PATCH 1/2] bootm: adjust the print format

[E Shattow]

On Sun, Aug 25, 2024 at 5:26 AM Dario Binacchi
<dario.binacchi@amarulasolutions.com> wrote:
>
> All three addresses printed are in hexadecimal format, but only the
> first two have the "0x" prefix. The patch aligns the format of the
> "end" address with the other two by adding the "0x" prefix.
>
> Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
> ---
>
>  boot/bootm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/boot/bootm.c b/boot/bootm.c
> index 480f8e6a0e6e..951e549f19ff 100644
> --- a/boot/bootm.c
> +++ b/boot/bootm.c
> @@ -703,7 +703,7 @@ static int bootm_load_os(struct bootm_headers *images, int boot_progress)
>
>                 /* Handle BOOTM_STATE_LOADOS */
>                 if (relocated_addr != load) {
> -                       printf("Moving Image from 0x%lx to 0x%lx, end=%lx\n",
> +                       printf("Moving Image from 0x%lx to 0x%lx, end=0x%lx\n",
>                                load, relocated_addr,
>                                relocated_addr + image_size);
>                         memmove((void *)relocated_addr, load_buf, image_size);
> --
> 2.43.0
>

From U-Boot documentation, alpha-numeric input is assumed to be
hexadecimal except when it is not, and generally does not accept "0x"
prefix on input. So the correct action would be to make this
consistent over the whole U-Boot code base, or remove the "0x"
prefixes (not add more of them) ?

-E

Re: [PATCH 1/2] bootm: adjust the print format

[Mattijs Korpershoek]

Hi Dario,

Thank you for the patch.

On dim., août 25, 2024 at 14:26, Dario Binacchi <dario.binacchi@amarulasolutions.com> wrote:

> All three addresses printed are in hexadecimal format, but only the
> first two have the "0x" prefix. The patch aligns the format of the
> "end" address with the other two by adding the "0x" prefix.
>
> Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>

Reviewed-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>

> ---
>
>  boot/bootm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/boot/bootm.c b/boot/bootm.c
> index 480f8e6a0e6e..951e549f19ff 100644
> --- a/boot/bootm.c
> +++ b/boot/bootm.c
> @@ -703,7 +703,7 @@ static int bootm_load_os(struct bootm_headers *images, int boot_progress)
>  
>  		/* Handle BOOTM_STATE_LOADOS */
>  		if (relocated_addr != load) {
> -			printf("Moving Image from 0x%lx to 0x%lx, end=%lx\n",
> +			printf("Moving Image from 0x%lx to 0x%lx, end=0x%lx\n",
>  			       load, relocated_addr,
>  			       relocated_addr + image_size);
>  			memmove((void *)relocated_addr, load_buf, image_size);
> -- 
> 2.43.0

Re: [PATCH 1/2] bootm: adjust the print format

[Caleb Connolly]

On 25/08/2024 19:36, E Shattow wrote:
> On Sun, Aug 25, 2024 at 5:26 AM Dario Binacchi
> <dario.binacchi@amarulasolutions.com> wrote:
>>
>> All three addresses printed are in hexadecimal format, but only the
>> first two have the "0x" prefix. The patch aligns the format of the
>> "end" address with the other two by adding the "0x" prefix.
>>
>> Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
>> ---
>>
>>   boot/bootm.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/boot/bootm.c b/boot/bootm.c
>> index 480f8e6a0e6e..951e549f19ff 100644
>> --- a/boot/bootm.c
>> +++ b/boot/bootm.c
>> @@ -703,7 +703,7 @@ static int bootm_load_os(struct bootm_headers *images, int boot_progress)
>>
>>                  /* Handle BOOTM_STATE_LOADOS */
>>                  if (relocated_addr != load) {
>> -                       printf("Moving Image from 0x%lx to 0x%lx, end=%lx\n",
>> +                       printf("Moving Image from 0x%lx to 0x%lx, end=0x%lx\n",
>>                                 load, relocated_addr,
>>                                 relocated_addr + image_size);
>>                          memmove((void *)relocated_addr, load_buf, image_size);
>> --
>> 2.43.0
>>
> 
>  From U-Boot documentation, alpha-numeric input is assumed to be
> hexadecimal except when it is not, and generally does not accept "0x"
> prefix on input. So the correct action would be to make this
> consistent over the whole U-Boot code base, or remove the "0x"
> prefixes (not add more of them) ?

Most(?) U-Boot commands accept the 0x prefix. I don't think stripping it 
is sensible, I myself have gotten confused many times over hex values 
that lack the leading 0x in U-Boot output.

Maybe unavailable in SPL (not sure) but I prefer the "%#lx" format which 
prepends the 0x automatically.
> 
> -E

-- 
// Caleb (they/them)

Re: [PATCH 1/2] bootm: adjust the print format

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 2868 bytes --]

On Mon, Aug 26, 2024 at 02:26:10PM +0100, Caleb Connolly wrote:
> 
> 
> On 25/08/2024 19:36, E Shattow wrote:
> > On Sun, Aug 25, 2024 at 5:26 AM Dario Binacchi
> > <dario.binacchi@amarulasolutions.com> wrote:
> > > 
> > > All three addresses printed are in hexadecimal format, but only the
> > > first two have the "0x" prefix. The patch aligns the format of the
> > > "end" address with the other two by adding the "0x" prefix.
> > > 
> > > Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
> > > ---
> > > 
> > >   boot/bootm.c | 2 +-
> > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/boot/bootm.c b/boot/bootm.c
> > > index 480f8e6a0e6e..951e549f19ff 100644
> > > --- a/boot/bootm.c
> > > +++ b/boot/bootm.c
> > > @@ -703,7 +703,7 @@ static int bootm_load_os(struct bootm_headers *images, int boot_progress)
> > > 
> > >                  /* Handle BOOTM_STATE_LOADOS */
> > >                  if (relocated_addr != load) {
> > > -                       printf("Moving Image from 0x%lx to 0x%lx, end=%lx\n",
> > > +                       printf("Moving Image from 0x%lx to 0x%lx, end=0x%lx\n",
> > >                                 load, relocated_addr,
> > >                                 relocated_addr + image_size);
> > >                          memmove((void *)relocated_addr, load_buf, image_size);
> > > --
> > > 2.43.0
> > > 
> > 
> >  From U-Boot documentation, alpha-numeric input is assumed to be
> > hexadecimal except when it is not, and generally does not accept "0x"
> > prefix on input. So the correct action would be to make this

While there was some point in history where I'm sure we got confused by
"0x" input I don't think that's true anymore (and everything should be
using some strto function that works as expected, not a custom parser).
So the docs should be updated there.

> > consistent over the whole U-Boot code base, or remove the "0x"
> > prefixes (not add more of them) ?
> 
> Most(?) U-Boot commands accept the 0x prefix. I don't think stripping it is
> sensible, I myself have gotten confused many times over hex values that lack
> the leading 0x in U-Boot output.
> 
> Maybe unavailable in SPL (not sure) but I prefer the "%#lx" format which
> prepends the 0x automatically.

That we assume input is hex is just what it is these days. Output really
ought to be prefixed with 0x because that's just common convention (and
whatever we assumed people would Just Know 25+ years ago may not be true
today). Since updating this output really shouldn't change our ABI, it's
conceptually fine with me but we don't use "%#lx" a lot and so I don't
know if tiny-printf handles it and so that might not be the right call
for SPL code and so lets not change this patch.

Reviewed-by: Tom Rini <trini@konsulko.com>

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 1/2] bootm: adjust the print format

[Simon Glass]

Hi,

On Sun, 25 Aug 2024 at 12:36, E Shattow <lucent@gmail.com> wrote:
>
> On Sun, Aug 25, 2024 at 5:26 AM Dario Binacchi
> <dario.binacchi@amarulasolutions.com> wrote:
> >
> > All three addresses printed are in hexadecimal format, but only the
> > first two have the "0x" prefix. The patch aligns the format of the
> > "end" address with the other two by adding the "0x" prefix.
> >
> > Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
> > ---
> >
> >  boot/bootm.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/boot/bootm.c b/boot/bootm.c
> > index 480f8e6a0e6e..951e549f19ff 100644
> > --- a/boot/bootm.c
> > +++ b/boot/bootm.c
> > @@ -703,7 +703,7 @@ static int bootm_load_os(struct bootm_headers *images, int boot_progress)
> >
> >                 /* Handle BOOTM_STATE_LOADOS */
> >                 if (relocated_addr != load) {
> > -                       printf("Moving Image from 0x%lx to 0x%lx, end=%lx\n",
> > +                       printf("Moving Image from 0x%lx to 0x%lx, end=0x%lx\n",
> >                                load, relocated_addr,
> >                                relocated_addr + image_size);
> >                         memmove((void *)relocated_addr, load_buf, image_size);
> > --
> > 2.43.0
> >
>
> From U-Boot documentation, alpha-numeric input is assumed to be
> hexadecimal except when it is not, and generally does not accept "0x"
> prefix on input. So the correct action would be to make this
> consistent over the whole U-Boot code base, or remove the "0x"
> prefixes (not add more of them) ?

Yes, we should avoid these prefixes as they can confuse people into
thinking that hex is not the default.

In other cases where this is needed, for 0x you can use %#x

Regards,
Simon

Re: [PATCH 2/2] cmd: booti: adjust the print format

[Simon Glass]

Hi Dario,

On Sun, 25 Aug 2024 at 06:26, Dario Binacchi
<dario.binacchi@amarulasolutions.com> wrote:
>
> All three addresses printed are in hexadecimal format, but only the
> first two have the "0x" prefix. The patch aligns the format of the
> "end" address with the other two by adding the "0x" prefix.
>
> Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
>
> ---
>
>  cmd/booti.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/cmd/booti.c b/cmd/booti.c
> index 62b19e834366..ea811244a0a9 100644
> --- a/cmd/booti.c
> +++ b/cmd/booti.c
> @@ -78,7 +78,7 @@ static int booti_start(struct bootm_info *bmi)
>
>         /* Handle BOOTM_STATE_LOADOS */
>         if (relocated_addr != ld) {
> -               printf("Moving Image from 0x%lx to 0x%lx, end=%lx\n", ld,
> +               printf("Moving Image from 0x%lx to 0x%lx, end=0x%lx\n", ld,
>                        relocated_addr, relocated_addr + image_size);
>                 memmove((void *)relocated_addr, (void *)ld, image_size);
>         }
> --
> 2.43.0
>

I really don't like this...numbers are hex in U-Boot and this just
adds confusion.

Regards,
Simon

Re: [PATCH 2/2] cmd: booti: adjust the print format

[Dario Binacchi]

Hi Simon,

On Thu, Aug 29, 2024 at 4:05 PM Simon Glass <sjg@chromium.org> wrote:
>
> Hi Dario,
>
> On Sun, 25 Aug 2024 at 06:26, Dario Binacchi
> <dario.binacchi@amarulasolutions.com> wrote:
> >
> > All three addresses printed are in hexadecimal format, but only the
> > first two have the "0x" prefix. The patch aligns the format of the
> > "end" address with the other two by adding the "0x" prefix.
> >
> > Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
> >
> > ---
> >
> >  cmd/booti.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/cmd/booti.c b/cmd/booti.c
> > index 62b19e834366..ea811244a0a9 100644
> > --- a/cmd/booti.c
> > +++ b/cmd/booti.c
> > @@ -78,7 +78,7 @@ static int booti_start(struct bootm_info *bmi)
> >
> >         /* Handle BOOTM_STATE_LOADOS */
> >         if (relocated_addr != ld) {
> > -               printf("Moving Image from 0x%lx to 0x%lx, end=%lx\n", ld,
> > +               printf("Moving Image from 0x%lx to 0x%lx, end=0x%lx\n", ld,
> >                        relocated_addr, relocated_addr + image_size);
> >                 memmove((void *)relocated_addr, (void *)ld, image_size);
> >         }
> > --
> > 2.43.0
> >
>
> I really don't like this...numbers are hex in U-Boot and this just
> adds confusion.

Sorry, but I'm quite confused.
Doesn't printing 3 numbers in hexadecimal format with different
formatting (two with `0x` and
 one without) create more confusion?
At least we should ensure formatting consistency.
Also, it seems to me that this patch:
https://patchwork.ozlabs.org/project/uboot/patch/20240825122617.3708982-1-dario.binacchi@amarulasolutions.com/
has been considered correct.

Thanks and regards,
Dario
>
> Regards,
> Simon



-- 

Dario Binacchi

Senior Embedded Linux Developer

dario.binacchi@amarulasolutions.com

__________________________________


Amarula Solutions SRL

Via Le Canevare 30, 31100 Treviso, Veneto, IT

T. +39 042 243 5310
info@amarulasolutions.com

www.amarulasolutions.com

Re: [PATCH 2/2] cmd: booti: adjust the print format

[Simon Glass]

Hi Dario,

On Thu, 29 Aug 2024 at 08:25, Dario Binacchi
<dario.binacchi@amarulasolutions.com> wrote:
>
> Hi Simon,
>
> On Thu, Aug 29, 2024 at 4:05 PM Simon Glass <sjg@chromium.org> wrote:
> >
> > Hi Dario,
> >
> > On Sun, 25 Aug 2024 at 06:26, Dario Binacchi
> > <dario.binacchi@amarulasolutions.com> wrote:
> > >
> > > All three addresses printed are in hexadecimal format, but only the
> > > first two have the "0x" prefix. The patch aligns the format of the
> > > "end" address with the other two by adding the "0x" prefix.
> > >
> > > Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
> > >
> > > ---
> > >
> > >  cmd/booti.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/cmd/booti.c b/cmd/booti.c
> > > index 62b19e834366..ea811244a0a9 100644
> > > --- a/cmd/booti.c
> > > +++ b/cmd/booti.c
> > > @@ -78,7 +78,7 @@ static int booti_start(struct bootm_info *bmi)
> > >
> > >         /* Handle BOOTM_STATE_LOADOS */
> > >         if (relocated_addr != ld) {
> > > -               printf("Moving Image from 0x%lx to 0x%lx, end=%lx\n", ld,
> > > +               printf("Moving Image from 0x%lx to 0x%lx, end=0x%lx\n", ld,
> > >                        relocated_addr, relocated_addr + image_size);
> > >                 memmove((void *)relocated_addr, (void *)ld, image_size);
> > >         }
> > > --
> > > 2.43.0
> > >
> >
> > I really don't like this...numbers are hex in U-Boot and this just
> > adds confusion.
>
> Sorry, but I'm quite confused.
> Doesn't printing 3 numbers in hexadecimal format with different
> formatting (two with `0x` and
>  one without) create more confusion?
> At least we should ensure formatting consistency.
> Also, it seems to me that this patch:
> https://patchwork.ozlabs.org/project/uboot/patch/20240825122617.3708982-1-dario.binacchi@amarulasolutions.com/
> has been considered correct.
>
> Thanks and regards,

IMO we should avoid adding 0x to things...particularly for addresses.
Better to remove it when it has crept in.

Regards,
SImon

Re: [PATCH 2/2] cmd: booti: adjust the print format

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 2423 bytes --]

On Thu, Aug 29, 2024 at 09:00:30AM -0600, Simon Glass wrote:
> Hi Dario,
> 
> On Thu, 29 Aug 2024 at 08:25, Dario Binacchi
> <dario.binacchi@amarulasolutions.com> wrote:
> >
> > Hi Simon,
> >
> > On Thu, Aug 29, 2024 at 4:05 PM Simon Glass <sjg@chromium.org> wrote:
> > >
> > > Hi Dario,
> > >
> > > On Sun, 25 Aug 2024 at 06:26, Dario Binacchi
> > > <dario.binacchi@amarulasolutions.com> wrote:
> > > >
> > > > All three addresses printed are in hexadecimal format, but only the
> > > > first two have the "0x" prefix. The patch aligns the format of the
> > > > "end" address with the other two by adding the "0x" prefix.
> > > >
> > > > Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
> > > >
> > > > ---
> > > >
> > > >  cmd/booti.c | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/cmd/booti.c b/cmd/booti.c
> > > > index 62b19e834366..ea811244a0a9 100644
> > > > --- a/cmd/booti.c
> > > > +++ b/cmd/booti.c
> > > > @@ -78,7 +78,7 @@ static int booti_start(struct bootm_info *bmi)
> > > >
> > > >         /* Handle BOOTM_STATE_LOADOS */
> > > >         if (relocated_addr != ld) {
> > > > -               printf("Moving Image from 0x%lx to 0x%lx, end=%lx\n", ld,
> > > > +               printf("Moving Image from 0x%lx to 0x%lx, end=0x%lx\n", ld,
> > > >                        relocated_addr, relocated_addr + image_size);
> > > >                 memmove((void *)relocated_addr, (void *)ld, image_size);
> > > >         }
> > > > --
> > > > 2.43.0
> > > >
> > >
> > > I really don't like this...numbers are hex in U-Boot and this just
> > > adds confusion.
> >
> > Sorry, but I'm quite confused.
> > Doesn't printing 3 numbers in hexadecimal format with different
> > formatting (two with `0x` and
> >  one without) create more confusion?
> > At least we should ensure formatting consistency.
> > Also, it seems to me that this patch:
> > https://patchwork.ozlabs.org/project/uboot/patch/20240825122617.3708982-1-dario.binacchi@amarulasolutions.com/
> > has been considered correct.
> >
> > Thanks and regards,
> 
> IMO we should avoid adding 0x to things...particularly for addresses.
> Better to remove it when it has crept in.

That we don't prefix with "0x" like humans generally expect is why
people have been confused why partition 10 is in fact not 10-in-decimal
but 0x10.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 2/2] cmd: booti: adjust the print format

[Simon Glass]

Hi Tom,

On Thu, 29 Aug 2024 at 09:03, Tom Rini <trini@konsulko.com> wrote:
>
> On Thu, Aug 29, 2024 at 09:00:30AM -0600, Simon Glass wrote:
> > Hi Dario,
> >
> > On Thu, 29 Aug 2024 at 08:25, Dario Binacchi
> > <dario.binacchi@amarulasolutions.com> wrote:
> > >
> > > Hi Simon,
> > >
> > > On Thu, Aug 29, 2024 at 4:05 PM Simon Glass <sjg@chromium.org> wrote:
> > > >
> > > > Hi Dario,
> > > >
> > > > On Sun, 25 Aug 2024 at 06:26, Dario Binacchi
> > > > <dario.binacchi@amarulasolutions.com> wrote:
> > > > >
> > > > > All three addresses printed are in hexadecimal format, but only the
> > > > > first two have the "0x" prefix. The patch aligns the format of the
> > > > > "end" address with the other two by adding the "0x" prefix.
> > > > >
> > > > > Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
> > > > >
> > > > > ---
> > > > >
> > > > >  cmd/booti.c | 2 +-
> > > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/cmd/booti.c b/cmd/booti.c
> > > > > index 62b19e834366..ea811244a0a9 100644
> > > > > --- a/cmd/booti.c
> > > > > +++ b/cmd/booti.c
> > > > > @@ -78,7 +78,7 @@ static int booti_start(struct bootm_info *bmi)
> > > > >
> > > > >         /* Handle BOOTM_STATE_LOADOS */
> > > > >         if (relocated_addr != ld) {
> > > > > -               printf("Moving Image from 0x%lx to 0x%lx, end=%lx\n", ld,
> > > > > +               printf("Moving Image from 0x%lx to 0x%lx, end=0x%lx\n", ld,
> > > > >                        relocated_addr, relocated_addr + image_size);
> > > > >                 memmove((void *)relocated_addr, (void *)ld, image_size);
> > > > >         }
> > > > > --
> > > > > 2.43.0
> > > > >
> > > >
> > > > I really don't like this...numbers are hex in U-Boot and this just
> > > > adds confusion.
> > >
> > > Sorry, but I'm quite confused.
> > > Doesn't printing 3 numbers in hexadecimal format with different
> > > formatting (two with `0x` and
> > >  one without) create more confusion?
> > > At least we should ensure formatting consistency.
> > > Also, it seems to me that this patch:
> > > https://patchwork.ozlabs.org/project/uboot/patch/20240825122617.3708982-1-dario.binacchi@amarulasolutions.com/
> > > has been considered correct.
> > >
> > > Thanks and regards,
> >
> > IMO we should avoid adding 0x to things...particularly for addresses.
> > Better to remove it when it has crept in.
>
> That we don't prefix with "0x" like humans generally expect is why
> people have been confused why partition 10 is in fact not 10-in-decimal
> but 0x10.

Yes, that's the one example which was in my head when reviewing this patch.

Regards,
Simon

Re: [PATCH 1/2] bootm: adjust the print format

[Dario Binacchi]

Hello Tom,

On Mon, Aug 26, 2024 at 5:01 PM Tom Rini <trini@konsulko.com> wrote:
>
> On Mon, Aug 26, 2024 at 02:26:10PM +0100, Caleb Connolly wrote:
> >
> >
> > On 25/08/2024 19:36, E Shattow wrote:
> > > On Sun, Aug 25, 2024 at 5:26 AM Dario Binacchi
> > > <dario.binacchi@amarulasolutions.com> wrote:
> > > >
> > > > All three addresses printed are in hexadecimal format, but only the
> > > > first two have the "0x" prefix. The patch aligns the format of the
> > > > "end" address with the other two by adding the "0x" prefix.
> > > >
> > > > Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
> > > > ---
> > > >
> > > >   boot/bootm.c | 2 +-
> > > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/boot/bootm.c b/boot/bootm.c
> > > > index 480f8e6a0e6e..951e549f19ff 100644
> > > > --- a/boot/bootm.c
> > > > +++ b/boot/bootm.c
> > > > @@ -703,7 +703,7 @@ static int bootm_load_os(struct bootm_headers *images, int boot_progress)
> > > >
> > > >                  /* Handle BOOTM_STATE_LOADOS */
> > > >                  if (relocated_addr != load) {
> > > > -                       printf("Moving Image from 0x%lx to 0x%lx, end=%lx\n",
> > > > +                       printf("Moving Image from 0x%lx to 0x%lx, end=0x%lx\n",
> > > >                                 load, relocated_addr,
> > > >                                 relocated_addr + image_size);
> > > >                          memmove((void *)relocated_addr, load_buf, image_size);
> > > > --
> > > > 2.43.0
> > > >
> > >
> > >  From U-Boot documentation, alpha-numeric input is assumed to be
> > > hexadecimal except when it is not, and generally does not accept "0x"
> > > prefix on input. So the correct action would be to make this
>
> While there was some point in history where I'm sure we got confused by
> "0x" input I don't think that's true anymore (and everything should be
> using some strto function that works as expected, not a custom parser).
> So the docs should be updated there.
>
> > > consistent over the whole U-Boot code base, or remove the "0x"
> > > prefixes (not add more of them) ?
> >
> > Most(?) U-Boot commands accept the 0x prefix. I don't think stripping it is
> > sensible, I myself have gotten confused many times over hex values that lack
> > the leading 0x in U-Boot output.
> >
> > Maybe unavailable in SPL (not sure) but I prefer the "%#lx" format which
> > prepends the 0x automatically.
>
> That we assume input is hex is just what it is these days. Output really
> ought to be prefixed with 0x because that's just common convention (and
> whatever we assumed people would Just Know 25+ years ago may not be true
> today). Since updating this output really shouldn't change our ABI, it's
> conceptually fine with me but we don't use "%#lx" a lot and so I don't
> know if tiny-printf handles it and so that might not be the right call
> for SPL code and so lets not change this patch.
>
> Reviewed-by: Tom Rini <trini@konsulko.com>
>
> --
> Tom

Can this patch be merged?
I've seen both Review tags and conflicting opinions, and I haven't understood
whether it can be accepted or not.

Thanks and regards,
Dario

-- 

Dario Binacchi

Senior Embedded Linux Developer

dario.binacchi@amarulasolutions.com

__________________________________


Amarula Solutions SRL

Via Le Canevare 30, 31100 Treviso, Veneto, IT

T. +39 042 243 5310
info@amarulasolutions.com

www.amarulasolutions.com

Re: [PATCH 1/2] bootm: adjust the print format

[Tom Rini]

On Sun, 25 Aug 2024 14:26:07 +0200, Dario Binacchi wrote:

> All three addresses printed are in hexadecimal format, but only the
> first two have the "0x" prefix. The patch aligns the format of the
> "end" address with the other two by adding the "0x" prefix.
> 
> 

Applied to u-boot/next, thanks!

-- 
Tom