All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH] Remove extra fdt_fixup_ethernet() call
@ 2017-03-23 17:02 Joakim Tjernlund
  2017-03-28 17:47 ` Joakim Tjernlund
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Joakim Tjernlund @ 2017-03-23 17:02 UTC (permalink / raw)
  To: u-boot

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,
-- 
2.10.2

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [U-Boot] [PATCH] Remove extra fdt_fixup_ethernet() call
  2017-03-23 17:02 [U-Boot] [PATCH] Remove extra fdt_fixup_ethernet() call Joakim Tjernlund
@ 2017-03-28 17:47 ` Joakim Tjernlund
  2017-04-01  4:21 ` Simon Glass
  2017-04-09  1:14 ` [U-Boot] " Tom Rini
  2 siblings, 0 replies; 6+ messages in thread
From: Joakim Tjernlund @ 2017-03-28 17:47 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [U-Boot] [PATCH] Remove extra fdt_fixup_ethernet() call
  2017-03-23 17:02 [U-Boot] [PATCH] Remove extra fdt_fixup_ethernet() call Joakim Tjernlund
  2017-03-28 17:47 ` Joakim Tjernlund
@ 2017-04-01  4:21 ` Simon Glass
  2017-04-03  7:03   ` Joakim Tjernlund
  2017-04-09  1:14 ` [U-Boot] " Tom Rini
  2 siblings, 1 reply; 6+ messages in thread
From: Simon Glass @ 2017-04-01  4:21 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [U-Boot] [PATCH] Remove extra fdt_fixup_ethernet() call
  2017-04-01  4:21 ` Simon Glass
@ 2017-04-03  7:03   ` Joakim Tjernlund
  2017-04-28  7:27     ` Chen-Yu Tsai
  0 siblings, 1 reply; 6+ messages in thread
From: Joakim Tjernlund @ 2017-04-03  7:03 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [U-Boot] Remove extra fdt_fixup_ethernet() call
  2017-03-23 17:02 [U-Boot] [PATCH] Remove extra fdt_fixup_ethernet() call Joakim Tjernlund
  2017-03-28 17:47 ` Joakim Tjernlund
  2017-04-01  4:21 ` Simon Glass
@ 2017-04-09  1:14 ` Tom Rini
  2 siblings, 0 replies; 6+ messages in thread
From: Tom Rini @ 2017-04-09  1:14 UTC (permalink / raw)
  To: u-boot

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!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170408/4bbd81c8/attachment.sig>

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [U-Boot] [PATCH] Remove extra fdt_fixup_ethernet() call
  2017-04-03  7:03   ` Joakim Tjernlund
@ 2017-04-28  7:27     ` Chen-Yu Tsai
  0 siblings, 0 replies; 6+ messages in thread
From: Chen-Yu Tsai @ 2017-04-28  7:27 UTC (permalink / raw)
  To: u-boot

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 at lists.denx.de
> https://lists.denx.de/listinfo/u-boot

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-04-28  7:27 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-23 17:02 [U-Boot] [PATCH] Remove extra fdt_fixup_ethernet() call Joakim Tjernlund
2017-03-28 17:47 ` Joakim Tjernlund
2017-04-01  4:21 ` Simon Glass
2017-04-03  7:03   ` Joakim Tjernlund
2017-04-28  7:27     ` Chen-Yu Tsai
2017-04-09  1:14 ` [U-Boot] " Tom Rini

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.