| Message ID | 20170323170241.19485-1-joakim.tjernlund@infinera.com |
|---|---|
| State | Accepted |
| Delegated to: | Tom Rini |
| Headers | show |
On Thu, 2017-03-23 at 18:02 +0100, Joakim Tjernlund wrote: > ft_cpu_setup() already calls fdt_fixup_ethernet(), calling it > in image_setup_libfdt() is both redundant and breaks any modifications > done by ft_board_setup(). Restore the old behavior by removing > the call in image_setup_libfdt() > > Signed-off-by: Joakim Tjernlund <joakim.tjernlund@infinera.com> > --- > common/image-fdt.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/common/image-fdt.c b/common/image-fdt.c > index 80e3e63..b8f5654 100644 > --- a/common/image-fdt.c > +++ b/common/image-fdt.c > @@ -498,7 +498,6 @@ int image_setup_libfdt(bootm_headers_t *images, void *blob, > goto err; > } > } > - fdt_fixup_ethernet(blob); > > /* Delete the old LMB reservation */ > lmb_free(lmb, (phys_addr_t)(u32)(uintptr_t)blob, Ping ? Jocke
Hi Joakim, On 23 March 2017 at 11:02, Joakim Tjernlund <joakim.tjernlund@infinera.com> wrote: > ft_cpu_setup() already calls fdt_fixup_ethernet(), calling it > in image_setup_libfdt() is both redundant and breaks any modifications > done by ft_board_setup(). Restore the old behavior by removing > the call in image_setup_libfdt() Which old behaviour? Can you please add a Fixes: tag with the details? Also, which ft_cpu_setup()? > > Signed-off-by: Joakim Tjernlund <joakim.tjernlund@infinera.com> > --- > common/image-fdt.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/common/image-fdt.c b/common/image-fdt.c > index 80e3e63..b8f5654 100644 > --- a/common/image-fdt.c > +++ b/common/image-fdt.c > @@ -498,7 +498,6 @@ int image_setup_libfdt(bootm_headers_t *images, void *blob, > goto err; > } > } > - fdt_fixup_ethernet(blob); > > /* Delete the old LMB reservation */ > lmb_free(lmb, (phys_addr_t)(u32)(uintptr_t)blob, > -- > 2.10.2 > Regards, Simon
On Fri, 2017-03-31 at 22:21 -0600, Simon Glass wrote: > Hi Joakim, > > On 23 March 2017 at 11:02, Joakim Tjernlund > <joakim.tjernlund@infinera.com> wrote: > > ft_cpu_setup() already calls fdt_fixup_ethernet(), calling it > > in image_setup_libfdt() is both redundant and breaks any modifications > > done by ft_board_setup(). Restore the old behavior by removing > > the call in image_setup_libfdt() > > Which old behaviour? Can you please add a Fixes: tag with the details? The feature of having the possibility to rewrite the dev trees MAC addresses in ft_board_setup(). Currently such rewrites are overwritten by the call to dt_fixup_ethernet(blob) in image_setup_libfdt(). Looking into the history I see that commit 13d06981(by you as it happens :) is the one adding the extra call to fdt_fixup_ethernet() > > Also, which ft_cpu_setup()? # > git grep fdt_fixup_ethernet arch/arm/cpu/armv7/ls102xa/fdt.c: fdt_fixup_ethernet(blob); arch/mips/lib/bootm.c: fdt_fixup_ethernet(images->ft_addr); arch/nios2/cpu/fdt.c: fdt_fixup_ethernet(blob); arch/powerpc/cpu/mpc512x/cpu.c: fdt_fixup_ethernet(blob); arch/powerpc/cpu/mpc8260/cpu.c: fdt_fixup_ethernet(blob); arch/powerpc/cpu/mpc83xx/fdt.c: fdt_fixup_ethernet(blob); arch/powerpc/cpu/mpc85xx/fdt.c: fdt_fixup_ethernet(blob); arch/powerpc/cpu/mpc86xx/fdt.c: fdt_fixup_ethernet(blob); arch/powerpc/cpu/mpc8xx/fdt.c: fdt_fixup_ethernet(blob); arch/powerpc/cpu/ppc4xx/fdt.c: fdt_fixup_ethernet(blob); Seems like it is mostly PPC which has fdt_fixup_ethernet() call though. Jocke > > > > > Signed-off-by: Joakim Tjernlund <joakim.tjernlund@infinera.com> > > --- > > common/image-fdt.c | 1 - > > 1 file changed, 1 deletion(-) > > > > diff --git a/common/image-fdt.c b/common/image-fdt.c > > index 80e3e63..b8f5654 100644 > > --- a/common/image-fdt.c > > +++ b/common/image-fdt.c > > @@ -498,7 +498,6 @@ int image_setup_libfdt(bootm_headers_t *images, void *blob, > > goto err; > > } > > } > > - fdt_fixup_ethernet(blob); > > > > /* Delete the old LMB reservation */ > > lmb_free(lmb, (phys_addr_t)(u32)(uintptr_t)blob, > > -- > > 2.10.2 > > > > Regards, > Simon
On Thu, Mar 23, 2017 at 06:02:41PM +0100, Joakim Tjernlund wrote: > ft_cpu_setup() already calls fdt_fixup_ethernet(), calling it > in image_setup_libfdt() is both redundant and breaks any modifications > done by ft_board_setup(). Restore the old behavior by removing > the call in image_setup_libfdt() > > Signed-off-by: Joakim Tjernlund <joakim.tjernlund@infinera.com> Applied to u-boot/master, thanks!
On Mon, Apr 3, 2017 at 3:03 PM, Joakim Tjernlund <Joakim.Tjernlund@infinera.com> wrote: > On Fri, 2017-03-31 at 22:21 -0600, Simon Glass wrote: >> Hi Joakim, >> >> On 23 March 2017 at 11:02, Joakim Tjernlund >> <joakim.tjernlund@infinera.com> wrote: >> > ft_cpu_setup() already calls fdt_fixup_ethernet(), calling it >> > in image_setup_libfdt() is both redundant and breaks any modifications >> > done by ft_board_setup(). Restore the old behavior by removing >> > the call in image_setup_libfdt() >> >> Which old behaviour? Can you please add a Fixes: tag with the details? > > The feature of having the possibility to rewrite the dev trees MAC addresses in > ft_board_setup(). Currently such rewrites are overwritten by the call to dt_fixup_ethernet(blob) > in image_setup_libfdt(). > Looking into the history I see that commit 13d06981(by you as it happens :) is the one adding > the extra call to fdt_fixup_ethernet() >> >> Also, which ft_cpu_setup()? > # > git grep fdt_fixup_ethernet > arch/arm/cpu/armv7/ls102xa/fdt.c: fdt_fixup_ethernet(blob); > arch/mips/lib/bootm.c: fdt_fixup_ethernet(images->ft_addr); > arch/nios2/cpu/fdt.c: fdt_fixup_ethernet(blob); > arch/powerpc/cpu/mpc512x/cpu.c: fdt_fixup_ethernet(blob); > arch/powerpc/cpu/mpc8260/cpu.c: fdt_fixup_ethernet(blob); > arch/powerpc/cpu/mpc83xx/fdt.c: fdt_fixup_ethernet(blob); > arch/powerpc/cpu/mpc85xx/fdt.c: fdt_fixup_ethernet(blob); > arch/powerpc/cpu/mpc86xx/fdt.c: fdt_fixup_ethernet(blob); > arch/powerpc/cpu/mpc8xx/fdt.c: fdt_fixup_ethernet(blob); > arch/powerpc/cpu/ppc4xx/fdt.c: fdt_fixup_ethernet(blob); > > Seems like it is mostly PPC which has fdt_fixup_ethernet() call though. I think that's because everyone else is depending on the call in image_setup_libfdt... So far RPi and sunxi are broken. I'm sure there are others. IMHO, the commit message offers little context on the attempts to check existing users would break or not. Just because someone is not explicitly calling it does not mean they aren't expecting that behavior from the system. There's also no context on what _your_ platform is, how it's affected and and why actually needs the explicit call. Yes the 2 redundant calls are confusing, and one overwrites the other. However it was introduced in 2007, and device tree usage has increased a lot, particularly for ARM, since then. For us, the "new" behavior was standard from day 1. Regards ChenYu > > Jocke >> >> > >> > Signed-off-by: Joakim Tjernlund <joakim.tjernlund@infinera.com> >> > --- >> > common/image-fdt.c | 1 - >> > 1 file changed, 1 deletion(-) >> > >> > diff --git a/common/image-fdt.c b/common/image-fdt.c >> > index 80e3e63..b8f5654 100644 >> > --- a/common/image-fdt.c >> > +++ b/common/image-fdt.c >> > @@ -498,7 +498,6 @@ int image_setup_libfdt(bootm_headers_t *images, void *blob, >> > goto err; >> > } >> > } >> > - fdt_fixup_ethernet(blob); >> > >> > /* Delete the old LMB reservation */ >> > lmb_free(lmb, (phys_addr_t)(u32)(uintptr_t)blob, >> > -- >> > 2.10.2 >> > >> >> Regards, >> Simon > _______________________________________________ > U-Boot mailing list > U-Boot@lists.denx.de > https://lists.denx.de/listinfo/u-boot
diff --git a/common/image-fdt.c b/common/image-fdt.c index 80e3e63..b8f5654 100644 --- a/common/image-fdt.c +++ b/common/image-fdt.c @@ -498,7 +498,6 @@ int image_setup_libfdt(bootm_headers_t *images, void *blob, goto err; } } - fdt_fixup_ethernet(blob); /* Delete the old LMB reservation */ lmb_free(lmb, (phys_addr_t)(u32)(uintptr_t)blob,
ft_cpu_setup() already calls fdt_fixup_ethernet(), calling it in image_setup_libfdt() is both redundant and breaks any modifications done by ft_board_setup(). Restore the old behavior by removing the call in image_setup_libfdt() Signed-off-by: Joakim Tjernlund <joakim.tjernlund@infinera.com> --- common/image-fdt.c | 1 - 1 file changed, 1 deletion(-)