| Message ID | 20240825122617.3708982-2-dario.binacchi@amarulasolutions.com |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
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 To unsubscribe from this group and stop receiving emails from it, send an email to linux-amarula+unsubscribe@amarulasolutions.com.
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
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 To unsubscribe from this group and stop receiving emails from it, send an email to linux-amarula+unsubscribe@amarulasolutions.com.
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.
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 To unsubscribe from this group and stop receiving emails from it, send an email to linux-amarula+unsubscribe@amarulasolutions.com.
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); }
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(-)