All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] CONFIG_SYS_FDT_PAD: Why do we need this?
@ 2012-03-25  6:59 Dirk Behme
  2012-04-11 22:06 ` Simon Glass
  0 siblings, 1 reply; 2+ messages in thread
From: Dirk Behme @ 2012-03-25  6:59 UTC (permalink / raw)
  To: u-boot


In boot_relocate_fdt() [1] we increase the size of the loaded fdt by a 
predefined, hard coded CONFIG_SYS_FDT_PAD. The basic idea seem to be 
to create some space for adding additional information later, like the 
bootargs.

I wonder why we have to do this by a hard coded, predefined value 
(which is most probably always to large)? Why can't we increase 
'totalsize' in the fdt_header just if we add anything? Individually by 
the size of the element added, at runtime? E.g. by anything like [2]?

I debugged a system were

#define CONFIG_SYS_FDT_PAD 0x3000

was used. These 0x3000 are added to totalsize before something is 
added at all. Then, if an element is added, always the whole 0x3000 
bytes have to be memmoved additionally. And in the end only ~0x100 
additional space was really needed.

Best regards

Dirk

[1] 
http://git.denx.de/cgi-bin/gitweb.cgi?p=u-boot.git;a=blob;f=common/image.c;h=8c644b7dacdc9b7a196a5238f4dc5d94bd615d07;hb=HEAD#l1285

[2]

static int _fdt_splice_struct(void *fdt, void *p,
			      int oldlen, int newlen)
{
	int delta = newlen - oldlen;
	int err;

	if ((err = _fdt_splice(fdt, p, oldlen, newlen)))
		return err;

	fdt_set_size_dt_struct(fdt, fdt_size_dt_struct(fdt) + delta);
	fdt_set_off_dt_strings(fdt, fdt_off_dt_strings(fdt) + delta);
+	fdt_set_totalsize(fdt, fdt_totalsize(fdt) + delta);
	return 0;
}

in

http://git.denx.de/cgi-bin/gitweb.cgi?p=u-boot.git;a=blob;f=lib/libfdt/fdt_rw.c;h=5c27a677e3536ed7320ee399113666975e5d545b;hb=HEAD#l127

^ permalink raw reply	[flat|nested] 2+ messages in thread

* [U-Boot] CONFIG_SYS_FDT_PAD: Why do we need this?
  2012-03-25  6:59 [U-Boot] CONFIG_SYS_FDT_PAD: Why do we need this? Dirk Behme
@ 2012-04-11 22:06 ` Simon Glass
  0 siblings, 0 replies; 2+ messages in thread
From: Simon Glass @ 2012-04-11 22:06 UTC (permalink / raw)
  To: u-boot

Hi Dirk,

On Sat, Mar 24, 2012 at 11:59 PM, Dirk Behme <dirk.behme@googlemail.com> wrote:
>
> In boot_relocate_fdt() [1] we increase the size of the loaded fdt by a
> predefined, hard coded CONFIG_SYS_FDT_PAD. The basic idea seem to be to
> create some space for adding additional information later, like the
> bootargs.
>
> I wonder why we have to do this by a hard coded, predefined value (which is
> most probably always to large)? Why can't we increase 'totalsize' in the
> fdt_header just if we add anything? Individually by the size of the element
> added, at runtime? E.g. by anything like [2]?
>
> I debugged a system were
>
> #define CONFIG_SYS_FDT_PAD 0x3000
>
> was used. These 0x3000 are added to totalsize before something is added at
> all. Then, if an element is added, always the whole 0x3000 bytes have to be
> memmoved additionally. And in the end only ~0x100 additional space was
> really needed.

I don't really know the answer, but I guess that just increasing the
FDT is dangerous, since we don't know how big it can grow before
running into something else. This way we set a maximum size that it
can grow to and will produce an error if it is exceeded.

There is also the practical problem that we would need to check for
FDT_ERR_NOSPACE, expand the space, and then retry. A bit of a
hassle...

I suppose you could use fdt_pack() afterwards, although I don't see a
command for this at present.

Regards,
Simon

>
> Best regards
>
> Dirk
>
> [1]
> http://git.denx.de/cgi-bin/gitweb.cgi?p=u-boot.git;a=blob;f=common/image.c;h=8c644b7dacdc9b7a196a5238f4dc5d94bd615d07;hb=HEAD#l1285
>
> [2]
>
> static int _fdt_splice_struct(void *fdt, void *p,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?int oldlen, int newlen)
> {
> ? ? ? ?int delta = newlen - oldlen;
> ? ? ? ?int err;
>
> ? ? ? ?if ((err = _fdt_splice(fdt, p, oldlen, newlen)))
> ? ? ? ? ? ? ? ?return err;
>
> ? ? ? ?fdt_set_size_dt_struct(fdt, fdt_size_dt_struct(fdt) + delta);
> ? ? ? ?fdt_set_off_dt_strings(fdt, fdt_off_dt_strings(fdt) + delta);
> + ? ? ? fdt_set_totalsize(fdt, fdt_totalsize(fdt) + delta);
> ? ? ? ?return 0;
> }
>
> in
>
> http://git.denx.de/cgi-bin/gitweb.cgi?p=u-boot.git;a=blob;f=lib/libfdt/fdt_rw.c;h=5c27a677e3536ed7320ee399113666975e5d545b;hb=HEAD#l127
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2012-04-11 22:06 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-03-25  6:59 [U-Boot] CONFIG_SYS_FDT_PAD: Why do we need this? Dirk Behme
2012-04-11 22:06 ` Simon Glass

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.