| Message ID | 20180224110951.3446-1-marek.vasut+renesas@gmail.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Simon Glass |
| Headers | show |
| Series | [U-Boot,RFC] cmd: fdt: Fix fdt address information after the movement | expand |
Hi Marek, On 24 February 2018 at 19:09, Marek Vasut <marek.vasut@gmail.com> wrote: > From: Hiroyuki Yokoyama <hiroyuki.yokoyama.vx@renesas.com> > > This patch fixes the address information of fdt. > > wrong case: > => fdt addr 0x48000000 > => fdt move 0x48000000 0x41000000 0xa000 > => fdt addr > The address of the fdt is 48000000 > > Active address in this case is 0x41000000. > > Signed-off-by: Hiroyuki Yokoyama <hiroyuki.yokoyama.vx@renesas.com> > Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com> > Cc: Hiroyuki Yokoyama <hiroyuki.yokoyama.vx@renesas.com> > Cc: Nobuhiro Iwamatsu <iwamatsu@nigauri.org> > Cc: Pantelis Antoniou <pantelis.antoniou@konsulko.com> > --- > cmd/fdt.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/cmd/fdt.c b/cmd/fdt.c > index b783b0df42..1245bc24eb 100644 > --- a/cmd/fdt.c > +++ b/cmd/fdt.c > @@ -204,6 +204,8 @@ static int do_fdt(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) > return 1; > } > working_fdt = newaddr; > + env_set_hex("fdtaddr", (ulong)working_fdt); Shouldn't this be map_to_sysmem(working_fdt)? > + > #ifdef CONFIG_OF_SYSTEM_SETUP > /* Call the board-specific fixup routine */ > } else if (strncmp(argv[1], "sys", 3) == 0) { > -- > 2.16.1 Regards, Simon
On 04/01/2018 04:14 PM, Simon Glass wrote: > Hi Marek, Hi, > On 24 February 2018 at 19:09, Marek Vasut <marek.vasut@gmail.com> wrote: >> From: Hiroyuki Yokoyama <hiroyuki.yokoyama.vx@renesas.com> >> >> This patch fixes the address information of fdt. >> >> wrong case: >> => fdt addr 0x48000000 >> => fdt move 0x48000000 0x41000000 0xa000 >> => fdt addr >> The address of the fdt is 48000000 >> >> Active address in this case is 0x41000000. >> >> Signed-off-by: Hiroyuki Yokoyama <hiroyuki.yokoyama.vx@renesas.com> >> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com> >> Cc: Hiroyuki Yokoyama <hiroyuki.yokoyama.vx@renesas.com> >> Cc: Nobuhiro Iwamatsu <iwamatsu@nigauri.org> >> Cc: Pantelis Antoniou <pantelis.antoniou@konsulko.com> >> --- >> cmd/fdt.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/cmd/fdt.c b/cmd/fdt.c >> index b783b0df42..1245bc24eb 100644 >> --- a/cmd/fdt.c >> +++ b/cmd/fdt.c >> @@ -204,6 +204,8 @@ static int do_fdt(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) >> return 1; >> } >> working_fdt = newaddr; >> + env_set_hex("fdtaddr", (ulong)working_fdt); > > Shouldn't this be map_to_sysmem(working_fdt)? Should it ? The other question I have is, is this possibly changing the U-Boot API and is that a problem ? btw sorry, must've missed this mail.
Hi Marek, On 13 April 2018 at 16:07, Marek Vasut <marek.vasut@gmail.com> wrote: > On 04/01/2018 04:14 PM, Simon Glass wrote: >> Hi Marek, > > Hi, > >> On 24 February 2018 at 19:09, Marek Vasut <marek.vasut@gmail.com> wrote: >>> From: Hiroyuki Yokoyama <hiroyuki.yokoyama.vx@renesas.com> >>> >>> This patch fixes the address information of fdt. >>> >>> wrong case: >>> => fdt addr 0x48000000 >>> => fdt move 0x48000000 0x41000000 0xa000 >>> => fdt addr >>> The address of the fdt is 48000000 >>> >>> Active address in this case is 0x41000000. >>> >>> Signed-off-by: Hiroyuki Yokoyama <hiroyuki.yokoyama.vx@renesas.com> >>> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com> >>> Cc: Hiroyuki Yokoyama <hiroyuki.yokoyama.vx@renesas.com> >>> Cc: Nobuhiro Iwamatsu <iwamatsu@nigauri.org> >>> Cc: Pantelis Antoniou <pantelis.antoniou@konsulko.com> >>> --- >>> cmd/fdt.c | 2 ++ >>> 1 file changed, 2 insertions(+) >>> >>> diff --git a/cmd/fdt.c b/cmd/fdt.c >>> index b783b0df42..1245bc24eb 100644 >>> --- a/cmd/fdt.c >>> +++ b/cmd/fdt.c >>> @@ -204,6 +204,8 @@ static int do_fdt(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) >>> return 1; >>> } >>> working_fdt = newaddr; >>> + env_set_hex("fdtaddr", (ulong)working_fdt); >> >> Shouldn't this be map_to_sysmem(working_fdt)? > > Should it ? Yes, because you are converting a pointer to a ulong address. That's what that function is for. If you use it, you will allow this code to work on sandbox. > > The other question I have is, is this possibly changing the U-Boot API > and is that a problem ? Probably this code should use set_working_fdt_addr() instead. Then we have all the env_sets in one place. But no I don't think it is a problem to make this change. After all, working_fdt should match the 'fdtaddr' environment variable, right? > > btw sorry, must've missed this mail. That's OK, I do that a lot I suspect... Regards, Simon
diff --git a/cmd/fdt.c b/cmd/fdt.c index b783b0df42..1245bc24eb 100644 --- a/cmd/fdt.c +++ b/cmd/fdt.c @@ -204,6 +204,8 @@ static int do_fdt(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) return 1; } working_fdt = newaddr; + env_set_hex("fdtaddr", (ulong)working_fdt); + #ifdef CONFIG_OF_SYSTEM_SETUP /* Call the board-specific fixup routine */ } else if (strncmp(argv[1], "sys", 3) == 0) {