diff mbox

[U-Boot] fdt: prevent clearing memory node if there are no banks

Message ID 1434192193-32126-1-git-send-email-osp@andrep.de
State Superseded
Delegated to: Simon Glass
Headers show

Commit Message

Andre Przywara June 13, 2015, 10:43 a.m. UTC
Avoid clearing the reg property in the memory DT node if no memory
banks have been specified for a board (CONFIG_NR_DRAM_BANKS == 0).
This allows boards to let U-Boot skip the DT memory tinkering in case
other firmware has already setup the node properly before.
This should be safe as all callers of fdt_fixup_memory_banks that use
a computed <banks> value put at least 1 in there.
Add some documentation comments to the header file.

Signed-off-by: Andre Przywara <osp@andrep.de>
---
Hi,

this replaces my former "6/8: ARM: bootm: allow skipping fdt memory node fixup"
patch, as Simon had some (justified) concerns. If this is too generic now,
let me know and I will rework it.

Cheers,
Andre.

 common/fdt_support.c  | 14 ++++++++------
 include/fdt_support.h | 26 ++++++++++++++++++++++++++
 2 files changed, 34 insertions(+), 6 deletions(-)

Comments

Rob Herring (Arm) June 15, 2015, 3:57 p.m. UTC | #1
On Sat, Jun 13, 2015 at 5:43 AM, Andre Przywara <osp@andrep.de> wrote:
> Avoid clearing the reg property in the memory DT node if no memory
> banks have been specified for a board (CONFIG_NR_DRAM_BANKS == 0).
> This allows boards to let U-Boot skip the DT memory tinkering in case
> other firmware has already setup the node properly before.
> This should be safe as all callers of fdt_fixup_memory_banks that use
> a computed <banks> value put at least 1 in there.
> Add some documentation comments to the header file.
>
> Signed-off-by: Andre Przywara <osp@andrep.de>
> ---
> Hi,
>
> this replaces my former "6/8: ARM: bootm: allow skipping fdt memory node fixup"
> patch, as Simon had some (justified) concerns. If this is too generic now,
> let me know and I will rework it.
>
> Cheers,
> Andre.
>
>  common/fdt_support.c  | 14 ++++++++------
>  include/fdt_support.h | 26 ++++++++++++++++++++++++++
>  2 files changed, 34 insertions(+), 6 deletions(-)
>
> diff --git a/common/fdt_support.c b/common/fdt_support.c
> index 9e50148..6ad532c 100644
> --- a/common/fdt_support.c
> +++ b/common/fdt_support.c
> @@ -429,13 +429,15 @@ int fdt_fixup_memory_banks(void *blob, u64 start[], u64 size[], int banks)
>                 return err;
>         }
>
> -       len = fdt_pack_reg(blob, tmp, start, size, banks);
> +       if (banks) {
> +               len = fdt_pack_reg(blob, tmp, start, size, banks);

Save some indentation:

if (!banks)
  return 0;

>
> -       err = fdt_setprop(blob, nodeoffset, "reg", tmp, len);
> -       if (err < 0) {
> -               printf("WARNING: could not set %s %s.\n",
> -                               "reg", fdt_strerror(err));
> -               return err;
> +               err = fdt_setprop(blob, nodeoffset, "reg", tmp, len);
> +               if (err < 0) {
> +                       printf("WARNING: could not set %s %s.\n",
> +                                       "reg", fdt_strerror(err));
> +                       return err;
> +               }
>         }
>         return 0;
>  }
Simon Glass June 16, 2015, 6:07 p.m. UTC | #2
Hi,

On 13 June 2015 at 04:43, Andre Przywara <osp@andrep.de> wrote:
>
> Avoid clearing the reg property in the memory DT node if no memory
> banks have been specified for a board (CONFIG_NR_DRAM_BANKS == 0).
> This allows boards to let U-Boot skip the DT memory tinkering in case
> other firmware has already setup the node properly before.
> This should be safe as all callers of fdt_fixup_memory_banks that use
> a computed <banks> value put at least 1 in there.
> Add some documentation comments to the header file.
>
> Signed-off-by: Andre Przywara <osp@andrep.de>
> ---
> Hi,
>
> this replaces my former "6/8: ARM: bootm: allow skipping fdt memory node fixup"
> patch, as Simon had some (justified) concerns. If this is too generic now,
> let me know and I will rework it.
>
> Cheers,
> Andre.
>
>  common/fdt_support.c  | 14 ++++++++------
>  include/fdt_support.h | 26 ++++++++++++++++++++++++++
>  2 files changed, 34 insertions(+), 6 deletions(-)

Acked-by: Simon Glass <sjg@chromium.org>

Tom, you can pick this up if you like?

- Simon
diff mbox

Patch

diff --git a/common/fdt_support.c b/common/fdt_support.c
index 9e50148..6ad532c 100644
--- a/common/fdt_support.c
+++ b/common/fdt_support.c
@@ -429,13 +429,15 @@  int fdt_fixup_memory_banks(void *blob, u64 start[], u64 size[], int banks)
 		return err;
 	}
 
-	len = fdt_pack_reg(blob, tmp, start, size, banks);
+	if (banks) {
+		len = fdt_pack_reg(blob, tmp, start, size, banks);
 
-	err = fdt_setprop(blob, nodeoffset, "reg", tmp, len);
-	if (err < 0) {
-		printf("WARNING: could not set %s %s.\n",
-				"reg", fdt_strerror(err));
-		return err;
+		err = fdt_setprop(blob, nodeoffset, "reg", tmp, len);
+		if (err < 0) {
+			printf("WARNING: could not set %s %s.\n",
+					"reg", fdt_strerror(err));
+			return err;
+		}
 	}
 	return 0;
 }
diff --git a/include/fdt_support.h b/include/fdt_support.h
index 5d4f28d..0296ed3 100644
--- a/include/fdt_support.h
+++ b/include/fdt_support.h
@@ -40,8 +40,34 @@  void do_fixup_by_compat(void *fdt, const char *compat,
 			const char *prop, const void *val, int len, int create);
 void do_fixup_by_compat_u32(void *fdt, const char *compat,
 			    const char *prop, u32 val, int create);
+/**
+ * Setup the memory node in the DT. Creates one if none was existing before.
+ * Calls fdt_fixup_memory_banks() to populate a single reg pair covering the
+ * whole memory.
+ *
+ * @param blob		FDT blob to update
+ * @param start		Begin of DRAM mapping in physical memory
+ * @param size		Size of the single memory bank
+ * @return 0 if ok, or -1 or -FDT_ERR_... on error
+ */
 int fdt_fixup_memory(void *blob, u64 start, u64 size);
+
+/**
+ * Fill the DT memory node with multiple memory banks.
+ * Creates the node if none was existing before.
+ * If banks is 0, it will not touch the existing reg property. This allows
+ * boards to not mess with the existing DT setup, which may have been
+ * filled in properly before.
+ *
+ * @param blob		FDT blob to update
+ * @param start		Array of size <banks> to hold the start addresses.
+ * @param size		Array of size <banks> to hold the size of each region.
+ * @param banks		Number of memory banks to create. If 0, the reg
+ *			property will be left untouched.
+ * @return 0 if ok, or -1 or -FDT_ERR_... on error
+ */
 int fdt_fixup_memory_banks(void *blob, u64 start[], u64 size[], int banks);
+
 void fdt_fixup_ethernet(void *fdt);
 int fdt_find_and_setprop(void *fdt, const char *node, const char *prop,
 			 const void *val, int len, int create);