[U-Boot] [PATCH v3 00/13] ARM: tegra: Add support for framebuffer carveouts

From: Thierry Reding treding@nvidia.com
The series adds a couple of helpers that allow passing framebuffer carveouts to the kernel on boot. This is required in order to reserve the memory region that the framebuffer resides in and that the display controller is scanning out from, so that it is not reused by the kernel for the buddy allocator. At the same time, this information can be used to set up a 1:1 mapping for the IOMMU during boot in order to allow the display controller to keep scanning out from the same address without causing IOMMU faults.
Note that the last two patches depend on this series:
http://patchwork.ozlabs.org/project/uboot/list/?series=98501
I've included them here to give an example for how they are used. Tom would need to apply these on top of that series once the remainder of the patches here have been merged.
Given the build time dependencies it might be best for Tom to pick up the fdtdec pieces as well, with Simon's ack. Or alternatively Simon could merge these through his tree and Tom can apply once the dependencies have gone into master.
Tom, Simon, any preferences?
Thierry
Thierry Reding (13): libfdt: Add phandle generation helper fdtdec: Add cpu_to_fdt_{addr,size}() macros fdtdec: Add fdt_{addr,size}_unpack() helpers fdtdec: Implement fdtdec_set_phandle() fdtdec: Implement fdtdec_add_reserved_memory() fdtdec: Implement carveout support functions fdtdec: Add Kconfig symbol for tests fdtdec: test: Fix build warning fdtdec: test: Use compound statement macros fdtdec: test: Add carveout tests sandbox: Enable fdtdec tests p2371-2180: Add support for framebuffer carveouts p2771-0000: Add support for framebuffer carveouts
.../reserved-memory/reserved-memory.txt | 136 +++++++++++ board/nvidia/p2371-2180/p2371-2180.c | 47 ++++ board/nvidia/p2771-0000/p2771-0000.c | 66 ++++- configs/sandbox64_defconfig | 1 + configs/sandbox_defconfig | 1 + include/fdtdec.h | 169 +++++++++++++ lib/Kconfig | 4 + lib/fdtdec.c | 225 ++++++++++++++++++ lib/fdtdec_test.c | 216 +++++++++++++---- lib/libfdt/fdt_ro.c | 31 +++ scripts/dtc/libfdt/fdt_ro.c | 31 +++ scripts/dtc/libfdt/libfdt.h | 19 ++ scripts/dtc/libfdt/libfdt_env.h | 1 + 13 files changed, 902 insertions(+), 45 deletions(-) create mode 100644 Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt

From: Thierry Reding treding@nvidia.com
The new fdt_generate_phandle() function can be used to generate a new, unused phandle given a specific device tree blob. The implementation is somewhat naive in that it simply walks the entire device tree to find the highest phandle value and then returns a phandle value one higher than that. A more clever implementation might try to find holes in the current set of phandle values and fill them. But this implementation is relatively simple and works reliably.
Also add a test that validates that phandles generated by this new API are indeed unique.
Signed-off-by: Thierry Reding treding@nvidia.com --- Changes in v3: - update to latest upstream commit
lib/libfdt/fdt_ro.c | 31 +++++++++++++++++++++++++++++++ scripts/dtc/libfdt/fdt_ro.c | 31 +++++++++++++++++++++++++++++++ scripts/dtc/libfdt/libfdt.h | 19 +++++++++++++++++++ scripts/dtc/libfdt/libfdt_env.h | 1 + 4 files changed, 82 insertions(+)
diff --git a/lib/libfdt/fdt_ro.c b/lib/libfdt/fdt_ro.c index b6ca4e0b0c30..693de9aa5ad8 100644 --- a/lib/libfdt/fdt_ro.c +++ b/lib/libfdt/fdt_ro.c @@ -73,6 +73,37 @@ uint32_t fdt_get_max_phandle(const void *fdt) return 0; }
+int fdt_generate_phandle(const void *fdt, uint32_t *phandle) +{ + uint32_t max = 0; + int offset = -1; + + while (true) { + uint32_t value; + + offset = fdt_next_node(fdt, offset, NULL); + if (offset < 0) { + if (offset == -FDT_ERR_NOTFOUND) + break; + + return offset; + } + + value = fdt_get_phandle(fdt, offset); + + if (value > max) + max = value; + } + + if (max == FDT_MAX_PHANDLE) + return -FDT_ERR_NOPHANDLES; + + if (phandle) + *phandle = max + 1; + + return 0; +} + int fdt_get_mem_rsv(const void *fdt, int n, uint64_t *address, uint64_t *size) { FDT_CHECK_HEADER(fdt); diff --git a/scripts/dtc/libfdt/fdt_ro.c b/scripts/dtc/libfdt/fdt_ro.c index dfb3236da388..dc499884e4d1 100644 --- a/scripts/dtc/libfdt/fdt_ro.c +++ b/scripts/dtc/libfdt/fdt_ro.c @@ -115,6 +115,37 @@ uint32_t fdt_get_max_phandle(const void *fdt) return 0; }
+int fdt_generate_phandle(const void *fdt, uint32_t *phandle) +{ + uint32_t max = 0; + int offset = -1; + + while (true) { + uint32_t value; + + offset = fdt_next_node(fdt, offset, NULL); + if (offset < 0) { + if (offset == -FDT_ERR_NOTFOUND) + break; + + return offset; + } + + value = fdt_get_phandle(fdt, offset); + + if (value > max) + max = value; + } + + if (max == FDT_MAX_PHANDLE) + return -FDT_ERR_NOPHANDLES; + + if (phandle) + *phandle = max + 1; + + return 0; +} + int fdt_get_mem_rsv(const void *fdt, int n, uint64_t *address, uint64_t *size) { FDT_CHECK_HEADER(fdt); diff --git a/scripts/dtc/libfdt/libfdt.h b/scripts/dtc/libfdt/libfdt.h index fd73688f9e9f..cf86ddba8811 100644 --- a/scripts/dtc/libfdt/libfdt.h +++ b/scripts/dtc/libfdt/libfdt.h @@ -139,6 +139,10 @@
#define FDT_ERR_MAX 17
+/* constants */ +#define FDT_MAX_PHANDLE 0xfffffffe + /* Valid values for phandles range from 1 to 2^32-2. */ + /**********************************************************************/ /* Low-level functions (you probably don't need these) */ /**********************************************************************/ @@ -313,6 +317,21 @@ const char *fdt_string(const void *fdt, int stroffset); */ uint32_t fdt_get_max_phandle(const void *fdt);
+/** + * fdt_generate_phandle - return a new, unused phandle for a device tree blob + * @fdt: pointer to the device tree blob + * @phandle: return location for the new phandle + * + * Walks the device tree blob and looks for the highest phandle value. On + * success, the new, unused phandle value (one higher than the previously + * highest phandle value in the device tree blob) will be returned in the + * @phandle parameter. + * + * Returns: + * 0 on success or a negative error-code on failure + */ +int fdt_generate_phandle(const void *fdt, uint32_t *phandle); + /** * fdt_num_mem_rsv - retrieve the number of memory reserve map entries * @fdt: pointer to the device tree blob diff --git a/scripts/dtc/libfdt/libfdt_env.h b/scripts/dtc/libfdt/libfdt_env.h index bd2474628775..3ff9e2863075 100644 --- a/scripts/dtc/libfdt/libfdt_env.h +++ b/scripts/dtc/libfdt/libfdt_env.h @@ -52,6 +52,7 @@ * EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. */
+#include <stdbool.h> #include <stddef.h> #include <stdint.h> #include <stdlib.h>

On Fri, 22 Mar 2019 at 02:10, Thierry Reding thierry.reding@gmail.com wrote:
From: Thierry Reding treding@nvidia.com
The new fdt_generate_phandle() function can be used to generate a new, unused phandle given a specific device tree blob. The implementation is somewhat naive in that it simply walks the entire device tree to find the highest phandle value and then returns a phandle value one higher than that. A more clever implementation might try to find holes in the current set of phandle values and fill them. But this implementation is relatively simple and works reliably.
Also add a test that validates that phandles generated by this new API are indeed unique.
Signed-off-by: Thierry Reding treding@nvidia.com
Changes in v3:
- update to latest upstream commit
lib/libfdt/fdt_ro.c | 31 +++++++++++++++++++++++++++++++ scripts/dtc/libfdt/fdt_ro.c | 31 +++++++++++++++++++++++++++++++ scripts/dtc/libfdt/libfdt.h | 19 +++++++++++++++++++ scripts/dtc/libfdt/libfdt_env.h | 1 + 4 files changed, 82 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org

On Fri, Mar 22, 2019 at 03:52:59PM +0800, Simon Glass wrote:
On Fri, 22 Mar 2019 at 02:10, Thierry Reding thierry.reding@gmail.com wrote:
From: Thierry Reding treding@nvidia.com
The new fdt_generate_phandle() function can be used to generate a new, unused phandle given a specific device tree blob. The implementation is somewhat naive in that it simply walks the entire device tree to find the highest phandle value and then returns a phandle value one higher than that. A more clever implementation might try to find holes in the current set of phandle values and fill them. But this implementation is relatively simple and works reliably.
Also add a test that validates that phandles generated by this new API are indeed unique.
Signed-off-by: Thierry Reding treding@nvidia.com
Changes in v3:
- update to latest upstream commit
lib/libfdt/fdt_ro.c | 31 +++++++++++++++++++++++++++++++ scripts/dtc/libfdt/fdt_ro.c | 31 +++++++++++++++++++++++++++++++ scripts/dtc/libfdt/libfdt.h | 19 +++++++++++++++++++ scripts/dtc/libfdt/libfdt_env.h | 1 + 4 files changed, 82 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org
Looks like this was reverted again upstream (for, in my opinion, dubious reasons). Shall I just move it to fdtdec again and we can convert to whatever we end up with upstream, if anything, later on?
Thierry

Hi Thierry,
On Mon, 25 Mar 2019 at 01:27, Thierry Reding thierry.reding@gmail.com wrote:
On Fri, Mar 22, 2019 at 03:52:59PM +0800, Simon Glass wrote:
On Fri, 22 Mar 2019 at 02:10, Thierry Reding thierry.reding@gmail.com wrote:
From: Thierry Reding treding@nvidia.com
The new fdt_generate_phandle() function can be used to generate a new, unused phandle given a specific device tree blob. The implementation is somewhat naive in that it simply walks the entire device tree to find the highest phandle value and then returns a phandle value one higher than that. A more clever implementation might try to find holes in the current set of phandle values and fill them. But this implementation is relatively simple and works reliably.
Also add a test that validates that phandles generated by this new API are indeed unique.
Signed-off-by: Thierry Reding treding@nvidia.com
Changes in v3:
- update to latest upstream commit
lib/libfdt/fdt_ro.c | 31 +++++++++++++++++++++++++++++++ scripts/dtc/libfdt/fdt_ro.c | 31 +++++++++++++++++++++++++++++++ scripts/dtc/libfdt/libfdt.h | 19 +++++++++++++++++++ scripts/dtc/libfdt/libfdt_env.h | 1 + 4 files changed, 82 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org
Looks like this was reverted again upstream (for, in my opinion, dubious reasons). Shall I just move it to fdtdec again and we can convert to whatever we end up with upstream, if anything, later on?
Yes that is OK with me.
Regards, Simon

Hi Thierry,
On Mon, 25 Mar 2019 at 01:27, Thierry Reding thierry.reding@gmail.com wrote:
On Fri, Mar 22, 2019 at 03:52:59PM +0800, Simon Glass wrote:
On Fri, 22 Mar 2019 at 02:10, Thierry Reding thierry.reding@gmail.com wrote:
From: Thierry Reding treding@nvidia.com
The new fdt_generate_phandle() function can be used to generate a new, unused phandle given a specific device tree blob. The implementation is somewhat naive in that it simply walks the entire device tree to find the highest phandle value and then returns a phandle value one higher than that. A more clever implementation might try to find holes in the current set of phandle values and fill them. But this implementation is relatively simple and works reliably.
Also add a test that validates that phandles generated by this new API are indeed unique.
Signed-off-by: Thierry Reding treding@nvidia.com
Changes in v3:
- update to latest upstream commit
lib/libfdt/fdt_ro.c | 31 +++++++++++++++++++++++++++++++ scripts/dtc/libfdt/fdt_ro.c | 31 +++++++++++++++++++++++++++++++ scripts/dtc/libfdt/libfdt.h | 19 +++++++++++++++++++ scripts/dtc/libfdt/libfdt_env.h | 1 + 4 files changed, 82 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org
Looks like this was reverted again upstream (for, in my opinion, dubious reasons). Shall I just move it to fdtdec again and we can convert to whatever we end up with upstream, if anything, later on?
Yes that is OK with me.
Regards, Simon
Applied to u-boot-dm, thanks!

From: Thierry Reding treding@nvidia.com
These macros are useful for converting the endianness of variables of type fdt_addr_t and fdt_size_t.
Reviewed-by: Simon Glass sjg@chromium.org Signed-off-by: Thierry Reding treding@nvidia.com --- Changes in v2: - add Reviewed-by from Simon
include/fdtdec.h | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/include/fdtdec.h b/include/fdtdec.h index b7e35cd87c55..a965c33157c9 100644 --- a/include/fdtdec.h +++ b/include/fdtdec.h @@ -27,11 +27,15 @@ typedef phys_size_t fdt_size_t; #define FDT_ADDR_T_NONE (-1U) #define fdt_addr_to_cpu(reg) be64_to_cpu(reg) #define fdt_size_to_cpu(reg) be64_to_cpu(reg) +#define cpu_to_fdt_addr(reg) cpu_to_be64(reg) +#define cpu_to_fdt_size(reg) cpu_to_be64(reg) typedef fdt64_t fdt_val_t; #else #define FDT_ADDR_T_NONE (-1U) #define fdt_addr_to_cpu(reg) be32_to_cpu(reg) #define fdt_size_to_cpu(reg) be32_to_cpu(reg) +#define cpu_to_fdt_addr(reg) cpu_to_be32(reg) +#define cpu_to_fdt_size(reg) cpu_to_be32(reg) typedef fdt32_t fdt_val_t; #endif

On Thu, 21 Mar 2019 at 12:10, Thierry Reding thierry.reding@gmail.com wrote:
From: Thierry Reding treding@nvidia.com
These macros are useful for converting the endianness of variables of type fdt_addr_t and fdt_size_t.
Reviewed-by: Simon Glass sjg@chromium.org Signed-off-by: Thierry Reding treding@nvidia.com
Changes in v2:
- add Reviewed-by from Simon
include/fdtdec.h | 4 ++++ 1 file changed, 4 insertions(+)
Applied to u-boot-dm, thanks!

From: Thierry Reding treding@nvidia.com
These helpers can be used to unpack variables of type fdt_addr_t and fdt_size_t into a pair of 32-bit variables. This is useful in cases where such variables need to be written to properties (such as "reg") of a device tree node where they need to be split into cells.
Signed-off-by: Thierry Reding treding@nvidia.com --- Changes in v2: - new patch
include/fdtdec.h | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+)
diff --git a/include/fdtdec.h b/include/fdtdec.h index a965c33157c9..a0ba57c6318b 100644 --- a/include/fdtdec.h +++ b/include/fdtdec.h @@ -23,6 +23,31 @@ */ typedef phys_addr_t fdt_addr_t; typedef phys_size_t fdt_size_t; + +static inline fdt32_t fdt_addr_unpack(fdt_addr_t addr, fdt32_t *upper) +{ + if (upper) +#ifdef CONFIG_PHYS_64BIT + *upper = addr >> 32; +#else + *upper = 0; +#endif + + return addr; +} + +static inline fdt32_t fdt_size_unpack(fdt_size_t size, fdt32_t *upper) +{ + if (upper) +#ifdef CONFIG_PHYS_64BIT + *upper = size >> 32; +#else + *upper = 0; +#endif + + return size; +} + #ifdef CONFIG_PHYS_64BIT #define FDT_ADDR_T_NONE (-1U) #define fdt_addr_to_cpu(reg) be64_to_cpu(reg)

Hi Thierry,
On Fri, 22 Mar 2019 at 02:10, Thierry Reding thierry.reding@gmail.com wrote:
From: Thierry Reding treding@nvidia.com
These helpers can be used to unpack variables of type fdt_addr_t and fdt_size_t into a pair of 32-bit variables. This is useful in cases where such variables need to be written to properties (such as "reg") of a device tree node where they need to be split into cells.
Signed-off-by: Thierry Reding treding@nvidia.com
Changes in v2:
- new patch
include/fdtdec.h | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+)
diff --git a/include/fdtdec.h b/include/fdtdec.h index a965c33157c9..a0ba57c6318b 100644 --- a/include/fdtdec.h +++ b/include/fdtdec.h @@ -23,6 +23,31 @@ */ typedef phys_addr_t fdt_addr_t; typedef phys_size_t fdt_size_t;
+static inline fdt32_t fdt_addr_unpack(fdt_addr_t addr, fdt32_t *upper) +{
if (upper)
+#ifdef CONFIG_PHYS_64BIT
Could we use 'if IS_ENABLED()' instead?
*upper = addr >> 32;
+#else
*upper = 0;
+#endif
return addr;
+}
+static inline fdt32_t fdt_size_unpack(fdt_size_t size, fdt32_t *upper) +{
if (upper)
+#ifdef CONFIG_PHYS_64BIT
*upper = size >> 32;
+#else
*upper = 0;
+#endif
return size;
+}
#ifdef CONFIG_PHYS_64BIT #define FDT_ADDR_T_NONE (-1U)
#define fdt_addr_to_cpu(reg) be64_to_cpu(reg)
2.21.0
Regards, Simon

On Fri, Mar 22, 2019 at 03:53:00PM +0800, Simon Glass wrote:
Hi Thierry,
On Fri, 22 Mar 2019 at 02:10, Thierry Reding thierry.reding@gmail.com wrote:
From: Thierry Reding treding@nvidia.com
These helpers can be used to unpack variables of type fdt_addr_t and fdt_size_t into a pair of 32-bit variables. This is useful in cases where such variables need to be written to properties (such as "reg") of a device tree node where they need to be split into cells.
Signed-off-by: Thierry Reding treding@nvidia.com
Changes in v2:
- new patch
include/fdtdec.h | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+)
diff --git a/include/fdtdec.h b/include/fdtdec.h index a965c33157c9..a0ba57c6318b 100644 --- a/include/fdtdec.h +++ b/include/fdtdec.h @@ -23,6 +23,31 @@ */ typedef phys_addr_t fdt_addr_t; typedef phys_size_t fdt_size_t;
+static inline fdt32_t fdt_addr_unpack(fdt_addr_t addr, fdt32_t *upper) +{
if (upper)
+#ifdef CONFIG_PHYS_64BIT
Could we use 'if IS_ENABLED()' instead?
Are you suggesting to use IS_ENABLED() in a preprocessor #if or a compiler if (IS_ENABLED(...)) { ... } construct? For the former, yes we could certainly do that.
The latter would probably work as well if we did something like this:
*upper = addr >> 32;
*upper = upper_32_bits(addr);
where upper_32_bits() is a macro such as the one defined in Linux' include/linux/kernel.h. That prevents the right-shift of 32 bits for 32-bit quantities to not produce a warning.
That said, I see that we already have that macro in U-Boot's kernel.h header file, so I could switch to using that. With that I think we could even leave out the conditional. The compiler would hopefully optimize it (the upper_32_bits() invocation) out by itself.
I'll do some testing and respin if that works out.
Thierry
+#else
*upper = 0;
+#endif
return addr;
+}
+static inline fdt32_t fdt_size_unpack(fdt_size_t size, fdt32_t *upper) +{
if (upper)
+#ifdef CONFIG_PHYS_64BIT
*upper = size >> 32;
+#else
*upper = 0;
+#endif
return size;
+}
#ifdef CONFIG_PHYS_64BIT #define FDT_ADDR_T_NONE (-1U)
#define fdt_addr_to_cpu(reg) be64_to_cpu(reg)
2.21.0
Regards, Simon

Hi Thierry,
On Fri, 22 Mar 2019 at 02:31, Thierry Reding thierry.reding@gmail.com wrote:
On Fri, Mar 22, 2019 at 03:53:00PM +0800, Simon Glass wrote:
Hi Thierry,
On Fri, 22 Mar 2019 at 02:10, Thierry Reding thierry.reding@gmail.com wrote:
From: Thierry Reding treding@nvidia.com
These helpers can be used to unpack variables of type fdt_addr_t and fdt_size_t into a pair of 32-bit variables. This is useful in cases where such variables need to be written to properties (such as "reg") of a device tree node where they need to be split into cells.
Signed-off-by: Thierry Reding treding@nvidia.com
Changes in v2:
- new patch
include/fdtdec.h | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+)
diff --git a/include/fdtdec.h b/include/fdtdec.h index a965c33157c9..a0ba57c6318b 100644 --- a/include/fdtdec.h +++ b/include/fdtdec.h @@ -23,6 +23,31 @@ */ typedef phys_addr_t fdt_addr_t; typedef phys_size_t fdt_size_t;
+static inline fdt32_t fdt_addr_unpack(fdt_addr_t addr, fdt32_t *upper) +{
if (upper)
+#ifdef CONFIG_PHYS_64BIT
Could we use 'if IS_ENABLED()' instead?
Are you suggesting to use IS_ENABLED() in a preprocessor #if or a compiler if (IS_ENABLED(...)) { ... } construct? For the former, yes we could certainly do that.
The latter would probably work as well if we did something like this:
*upper = addr >> 32;
*upper = upper_32_bits(addr);
where upper_32_bits() is a macro such as the one defined in Linux' include/linux/kernel.h. That prevents the right-shift of 32 bits for 32-bit quantities to not produce a warning.
That said, I see that we already have that macro in U-Boot's kernel.h header file, so I could switch to using that. With that I think we could even leave out the conditional. The compiler would hopefully optimize it (the upper_32_bits() invocation) out by itself.
I'll do some testing and respin if that works out.
Applied to u-boot-dm, thanks!
Please do a fixup patch if this works out. I hope I didn't miss an email/patch.
Regards, Simon

On Fri, Apr 12, 2019 at 03:45:53PM -0600, Simon Glass wrote:
Hi Thierry,
On Fri, 22 Mar 2019 at 02:31, Thierry Reding thierry.reding@gmail.com wrote:
On Fri, Mar 22, 2019 at 03:53:00PM +0800, Simon Glass wrote:
Hi Thierry,
On Fri, 22 Mar 2019 at 02:10, Thierry Reding thierry.reding@gmail.com wrote:
From: Thierry Reding treding@nvidia.com
These helpers can be used to unpack variables of type fdt_addr_t and fdt_size_t into a pair of 32-bit variables. This is useful in cases where such variables need to be written to properties (such as "reg") of a device tree node where they need to be split into cells.
Signed-off-by: Thierry Reding treding@nvidia.com
Changes in v2:
- new patch
include/fdtdec.h | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+)
diff --git a/include/fdtdec.h b/include/fdtdec.h index a965c33157c9..a0ba57c6318b 100644 --- a/include/fdtdec.h +++ b/include/fdtdec.h @@ -23,6 +23,31 @@ */ typedef phys_addr_t fdt_addr_t; typedef phys_size_t fdt_size_t;
+static inline fdt32_t fdt_addr_unpack(fdt_addr_t addr, fdt32_t *upper) +{
if (upper)
+#ifdef CONFIG_PHYS_64BIT
Could we use 'if IS_ENABLED()' instead?
Are you suggesting to use IS_ENABLED() in a preprocessor #if or a compiler if (IS_ENABLED(...)) { ... } construct? For the former, yes we could certainly do that.
The latter would probably work as well if we did something like this:
*upper = addr >> 32;
*upper = upper_32_bits(addr);
where upper_32_bits() is a macro such as the one defined in Linux' include/linux/kernel.h. That prevents the right-shift of 32 bits for 32-bit quantities to not produce a warning.
That said, I see that we already have that macro in U-Boot's kernel.h header file, so I could switch to using that. With that I think we could even leave out the conditional. The compiler would hopefully optimize it (the upper_32_bits() invocation) out by itself.
I'll do some testing and respin if that works out.
Applied to u-boot-dm, thanks!
Please do a fixup patch if this works out. I hope I didn't miss an email/patch.
Sorry, I had been meaning to send out a v4 of the series, but it took me too long. I just sent out a couple of follow-up patches for this:
http://patchwork.ozlabs.org/project/uboot/list/?series=102732
Thierry

From: Thierry Reding treding@nvidia.com
This function can be used to set a phandle for a given node.
Signed-off-by: Thierry Reding treding@nvidia.com --- Changes in v2: - don't emit deprecated linux,phandle property
include/fdtdec.h | 11 +++++++++++ lib/fdtdec.c | 7 +++++++ 2 files changed, 18 insertions(+)
diff --git a/include/fdtdec.h b/include/fdtdec.h index a0ba57c6318b..55600026c488 100644 --- a/include/fdtdec.h +++ b/include/fdtdec.h @@ -981,6 +981,17 @@ int fdtdec_setup_mem_size_base(void); */ int fdtdec_setup_memory_banksize(void);
+/** + * fdtdec_set_phandle() - sets the phandle of a given node + * + * @param blob FDT blob + * @param node offset in the FDT blob of the node whose phandle is to + * be set + * @param phandle phandle to set for the given node + * @return 0 on success or a negative error code on failure + */ +int fdtdec_set_phandle(void *blob, int node, uint32_t phandle); + /** * Set up the device tree ready for use */ diff --git a/lib/fdtdec.c b/lib/fdtdec.c index 09a7e133a539..00db90e3cdfd 100644 --- a/lib/fdtdec.c +++ b/lib/fdtdec.c @@ -1243,6 +1243,13 @@ __weak void *board_fdt_blob_setup(void) } #endif
+int fdtdec_set_phandle(void *blob, int node, uint32_t phandle) +{ + fdt32_t value = cpu_to_fdt32(phandle); + + return fdt_setprop(blob, node, "phandle", &value, sizeof(value)); +} + int fdtdec_setup(void) { #if CONFIG_IS_ENABLED(OF_CONTROL)

Hi Thierry,
On Fri, 22 Mar 2019 at 02:10, Thierry Reding thierry.reding@gmail.com wrote:
From: Thierry Reding treding@nvidia.com
This function can be used to set a phandle for a given node.
Signed-off-by: Thierry Reding treding@nvidia.com
Changes in v2:
- don't emit deprecated linux,phandle property
include/fdtdec.h | 11 +++++++++++ lib/fdtdec.c | 7 +++++++ 2 files changed, 18 insertions(+)
diff --git a/include/fdtdec.h b/include/fdtdec.h index a0ba57c6318b..55600026c488 100644 --- a/include/fdtdec.h +++ b/include/fdtdec.h @@ -981,6 +981,17 @@ int fdtdec_setup_mem_size_base(void); */ int fdtdec_setup_memory_banksize(void);
+/**
- fdtdec_set_phandle() - sets the phandle of a given node
- @param blob FDT blob
- @param node offset in the FDT blob of the node whose phandle is to
be set
- @param phandle phandle to set for the given node
- @return 0 on success or a negative error code on failure
- */
+int fdtdec_set_phandle(void *blob, int node, uint32_t phandle);
/**
- Set up the device tree ready for use
*/ diff --git a/lib/fdtdec.c b/lib/fdtdec.c index 09a7e133a539..00db90e3cdfd 100644 --- a/lib/fdtdec.c +++ b/lib/fdtdec.c @@ -1243,6 +1243,13 @@ __weak void *board_fdt_blob_setup(void) } #endif
+int fdtdec_set_phandle(void *blob, int node, uint32_t phandle) +{
fdt32_t value = cpu_to_fdt32(phandle);
return fdt_setprop(blob, node, "phandle", &value, sizeof(value));
Can we use fdt_setprop_u32() instead?
+}
int fdtdec_setup(void) {
#if CONFIG_IS_ENABLED(OF_CONTROL)
2.21.0
Regards, Simon

On Fri, Mar 22, 2019 at 03:53:01PM +0800, Simon Glass wrote:
Hi Thierry,
On Fri, 22 Mar 2019 at 02:10, Thierry Reding thierry.reding@gmail.com wrote:
From: Thierry Reding treding@nvidia.com
This function can be used to set a phandle for a given node.
Signed-off-by: Thierry Reding treding@nvidia.com
Changes in v2:
- don't emit deprecated linux,phandle property
include/fdtdec.h | 11 +++++++++++ lib/fdtdec.c | 7 +++++++ 2 files changed, 18 insertions(+)
diff --git a/include/fdtdec.h b/include/fdtdec.h index a0ba57c6318b..55600026c488 100644 --- a/include/fdtdec.h +++ b/include/fdtdec.h @@ -981,6 +981,17 @@ int fdtdec_setup_mem_size_base(void); */ int fdtdec_setup_memory_banksize(void);
+/**
- fdtdec_set_phandle() - sets the phandle of a given node
- @param blob FDT blob
- @param node offset in the FDT blob of the node whose phandle is to
be set
- @param phandle phandle to set for the given node
- @return 0 on success or a negative error code on failure
- */
+int fdtdec_set_phandle(void *blob, int node, uint32_t phandle);
/**
- Set up the device tree ready for use
*/ diff --git a/lib/fdtdec.c b/lib/fdtdec.c index 09a7e133a539..00db90e3cdfd 100644 --- a/lib/fdtdec.c +++ b/lib/fdtdec.c @@ -1243,6 +1243,13 @@ __weak void *board_fdt_blob_setup(void) } #endif
+int fdtdec_set_phandle(void *blob, int node, uint32_t phandle) +{
fdt32_t value = cpu_to_fdt32(phandle);
return fdt_setprop(blob, node, "phandle", &value, sizeof(value));
Can we use fdt_setprop_u32() instead?
Yeah, we could. I'm not sure if after that optimization it's even worth keeping the extra wrapper. There may be some benefit in having a separate name for this because it's a somewhat special purpose. Maybe I should make it a static inline function instead?
Thierry
+}
int fdtdec_setup(void) {
#if CONFIG_IS_ENABLED(OF_CONTROL)
2.21.0
Regards, Simon

Hi Thierry,
On Fri, 22 Mar 2019 at 16:34, Thierry Reding thierry.reding@gmail.com wrote:
On Fri, Mar 22, 2019 at 03:53:01PM +0800, Simon Glass wrote:
Hi Thierry,
On Fri, 22 Mar 2019 at 02:10, Thierry Reding thierry.reding@gmail.com wrote:
From: Thierry Reding treding@nvidia.com
This function can be used to set a phandle for a given node.
Signed-off-by: Thierry Reding treding@nvidia.com
Changes in v2:
- don't emit deprecated linux,phandle property
include/fdtdec.h | 11 +++++++++++ lib/fdtdec.c | 7 +++++++ 2 files changed, 18 insertions(+)
diff --git a/include/fdtdec.h b/include/fdtdec.h index a0ba57c6318b..55600026c488 100644 --- a/include/fdtdec.h +++ b/include/fdtdec.h @@ -981,6 +981,17 @@ int fdtdec_setup_mem_size_base(void); */ int fdtdec_setup_memory_banksize(void);
+/**
- fdtdec_set_phandle() - sets the phandle of a given node
- @param blob FDT blob
- @param node offset in the FDT blob of the node whose phandle is to
be set
- @param phandle phandle to set for the given node
- @return 0 on success or a negative error code on failure
- */
+int fdtdec_set_phandle(void *blob, int node, uint32_t phandle);
/**
- Set up the device tree ready for use
*/ diff --git a/lib/fdtdec.c b/lib/fdtdec.c index 09a7e133a539..00db90e3cdfd 100644 --- a/lib/fdtdec.c +++ b/lib/fdtdec.c @@ -1243,6 +1243,13 @@ __weak void *board_fdt_blob_setup(void) } #endif
+int fdtdec_set_phandle(void *blob, int node, uint32_t phandle) +{
fdt32_t value = cpu_to_fdt32(phandle);
return fdt_setprop(blob, node, "phandle", &value, sizeof(value));
Can we use fdt_setprop_u32() instead?
Yeah, we could. I'm not sure if after that optimization it's even worth keeping the extra wrapper. There may be some benefit in having a separate name for this because it's a somewhat special purpose. Maybe I should make it a static inline function instead?
I think it is worth it. But perhaps a static inline makes sense, up to you.
Regards, Simon

Hi Thierry,
On Fri, 22 Mar 2019 at 16:34, Thierry Reding thierry.reding@gmail.com wrote:
On Fri, Mar 22, 2019 at 03:53:01PM +0800, Simon Glass wrote:
Hi Thierry,
On Fri, 22 Mar 2019 at 02:10, Thierry Reding thierry.reding@gmail.com wrote:
From: Thierry Reding treding@nvidia.com
This function can be used to set a phandle for a given node.
Signed-off-by: Thierry Reding treding@nvidia.com
Changes in v2:
- don't emit deprecated linux,phandle property
include/fdtdec.h | 11 +++++++++++ lib/fdtdec.c | 7 +++++++ 2 files changed, 18 insertions(+)
Applied to u-boot-dm, thanks!

From: Thierry Reding treding@nvidia.com
This function can be used to add subnodes in the /reserved-memory node.
Reviewed-by: Simon Glass sjg@chromium.org Signed-off-by: Thierry Reding treding@nvidia.com --- Changes in v3: - use fdt_generate_phandle() instead of fdtdec_generate_phandle() - add device tree bindings for /reserved-memory - add examples to code comments
Changes in v2: - split fdt_{addr,size}_unpack() helpers into separate patch - use name@x,y notation only if the upper cell is > 0 - use debug() instead of printf() to save code size - properly compute number of cells in reg property - fix carveout size computations, was off by one - use #size-cells where appropriate
.../reserved-memory/reserved-memory.txt | 136 ++++++++++++++++++ include/fdtdec.h | 48 +++++++ lib/fdtdec.c | 131 +++++++++++++++++ 3 files changed, 315 insertions(+) create mode 100644 Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
diff --git a/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt new file mode 100644 index 000000000000..bac4afa3b197 --- /dev/null +++ b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt @@ -0,0 +1,136 @@ +*** Reserved memory regions *** + +Reserved memory is specified as a node under the /reserved-memory node. +The operating system shall exclude reserved memory from normal usage +one can create child nodes describing particular reserved (excluded from +normal use) memory regions. Such memory regions are usually designed for +the special usage by various device drivers. + +Parameters for each memory region can be encoded into the device tree +with the following nodes: + +/reserved-memory node +--------------------- +#address-cells, #size-cells (required) - standard definition + - Should use the same values as the root node +ranges (required) - standard definition + - Should be empty + +/reserved-memory/ child nodes +----------------------------- +Each child of the reserved-memory node specifies one or more regions of +reserved memory. Each child node may either use a 'reg' property to +specify a specific range of reserved memory, or a 'size' property with +optional constraints to request a dynamically allocated block of memory. + +Following the generic-names recommended practice, node names should +reflect the purpose of the node (ie. "framebuffer" or "dma-pool"). Unit +address (@<address>) should be appended to the name if the node is a +static allocation. + +Properties: +Requires either a) or b) below. +a) static allocation + reg (required) - standard definition +b) dynamic allocation + size (required) - length based on parent's #size-cells + - Size in bytes of memory to reserve. + alignment (optional) - length based on parent's #size-cells + - Address boundary for alignment of allocation. + alloc-ranges (optional) - prop-encoded-array (address, length pairs). + - Specifies regions of memory that are + acceptable to allocate from. + +If both reg and size are present, then the reg property takes precedence +and size is ignored. + +Additional properties: +compatible (optional) - standard definition + - may contain the following strings: + - shared-dma-pool: This indicates a region of memory meant to be + used as a shared pool of DMA buffers for a set of devices. It can + be used by an operating system to instantiate the necessary pool + management subsystem if necessary. + - vendor specific string in the form <vendor>,[<device>-]<usage> +no-map (optional) - empty property + - Indicates the operating system must not create a virtual mapping + of the region as part of its standard mapping of system memory, + nor permit speculative access to it under any circumstances other + than under the control of the device driver using the region. +reusable (optional) - empty property + - The operating system can use the memory in this region with the + limitation that the device driver(s) owning the region need to be + able to reclaim it back. Typically that means that the operating + system can use that region to store volatile or cached data that + can be otherwise regenerated or migrated elsewhere. + +Linux implementation note: +- If a "linux,cma-default" property is present, then Linux will use the + region for the default pool of the contiguous memory allocator. + +- If a "linux,dma-default" property is present, then Linux will use the + region for the default pool of the consistent DMA allocator. + +Device node references to reserved memory +----------------------------------------- +Regions in the /reserved-memory node may be referenced by other device +nodes by adding a memory-region property to the device node. + +memory-region (optional) - phandle, specifier pairs to children of /reserved-memory + +Example +------- +This example defines 3 contiguous regions are defined for Linux kernel: +one default of all device drivers (named linux,cma@72000000 and 64MiB in size), +one dedicated to the framebuffer device (named framebuffer@78000000, 8MiB), and +one for multimedia processing (named multimedia-memory@77000000, 64MiB). + +/ { + #address-cells = <1>; + #size-cells = <1>; + + memory { + reg = <0x40000000 0x40000000>; + }; + + reserved-memory { + #address-cells = <1>; + #size-cells = <1>; + ranges; + + /* global autoconfigured region for contiguous allocations */ + linux,cma { + compatible = "shared-dma-pool"; + reusable; + size = <0x4000000>; + alignment = <0x2000>; + linux,cma-default; + }; + + display_reserved: framebuffer@78000000 { + reg = <0x78000000 0x800000>; + }; + + multimedia_reserved: multimedia@77000000 { + compatible = "acme,multimedia-memory"; + reg = <0x77000000 0x4000000>; + }; + }; + + /* ... */ + + fb0: video@12300000 { + memory-region = <&display_reserved>; + /* ... */ + }; + + scaler: scaler@12500000 { + memory-region = <&multimedia_reserved>; + /* ... */ + }; + + codec: codec@12600000 { + memory-region = <&multimedia_reserved>; + /* ... */ + }; +}; diff --git a/include/fdtdec.h b/include/fdtdec.h index 55600026c488..b54ed38fb362 100644 --- a/include/fdtdec.h +++ b/include/fdtdec.h @@ -992,6 +992,54 @@ int fdtdec_setup_memory_banksize(void); */ int fdtdec_set_phandle(void *blob, int node, uint32_t phandle);
+/** + * fdtdec_add_reserved_memory() - add or find a reserved-memory node + * + * If a reserved-memory node already exists for the given carveout, a phandle + * for that node will be returned. Otherwise a new node will be created and a + * phandle corresponding to it will be returned. + * + * See Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt + * for details on how to use reserved memory regions. + * + * As an example, consider the following code snippet: + * + * struct fdt_memory fb = { + * .start = 0x92cb3000, + * .end = 0x934b2fff, + * }; + * uint32_t phandle; + * + * fdtdec_add_reserved_memory(fdt, "framebuffer", &fb, &phandle); + * + * This results in the following subnode being added to the top-level + * /reserved-memory node: + * + * reserved-memory { + * #address-cells = <0x00000002>; + * #size-cells = <0x00000002>; + * ranges; + * + * framebuffer@92cb3000 { + * reg = <0x00000000 0x92cb3000 0x00000000 0x00800000>; + * phandle = <0x0000004d>; + * }; + * }; + * + * If the top-level /reserved-memory node does not exist, it will be created. + * The phandle returned from the function call can be used to reference this + * reserved memory region from other nodes. + * + * @param blob FDT blob + * @param basename base name of the node to create + * @param carveout information about the carveout region + * @param phandlep return location for the phandle of the carveout region + * @return 0 on success or a negative error code on failure + */ +int fdtdec_add_reserved_memory(void *blob, const char *basename, + const struct fdt_memory *carveout, + uint32_t *phandlep); + /** * Set up the device tree ready for use */ diff --git a/lib/fdtdec.c b/lib/fdtdec.c index 00db90e3cdfd..be54ad5bd092 100644 --- a/lib/fdtdec.c +++ b/lib/fdtdec.c @@ -1250,6 +1250,137 @@ int fdtdec_set_phandle(void *blob, int node, uint32_t phandle) return fdt_setprop(blob, node, "phandle", &value, sizeof(value)); }
+static int fdtdec_init_reserved_memory(void *blob) +{ + int na, ns, node, err; + fdt32_t value; + + /* inherit #address-cells and #size-cells from the root node */ + na = fdt_address_cells(blob, 0); + ns = fdt_size_cells(blob, 0); + + node = fdt_add_subnode(blob, 0, "reserved-memory"); + if (node < 0) + return node; + + err = fdt_setprop(blob, node, "ranges", NULL, 0); + if (err < 0) + return err; + + value = cpu_to_fdt32(ns); + + err = fdt_setprop(blob, node, "#size-cells", &value, sizeof(value)); + if (err < 0) + return err; + + value = cpu_to_fdt32(na); + + err = fdt_setprop(blob, node, "#address-cells", &value, sizeof(value)); + if (err < 0) + return err; + + return node; +} + +int fdtdec_add_reserved_memory(void *blob, const char *basename, + const struct fdt_memory *carveout, + uint32_t *phandlep) +{ + fdt32_t cells[4] = {}, *ptr = cells; + uint32_t upper, lower, phandle; + int parent, node, na, ns, err; + char name[64]; + + /* create an empty /reserved-memory node if one doesn't exist */ + parent = fdt_path_offset(blob, "/reserved-memory"); + if (parent < 0) { + parent = fdtdec_init_reserved_memory(blob); + if (parent < 0) + return parent; + } + + /* only 1 or 2 #address-cells and #size-cells are supported */ + na = fdt_address_cells(blob, parent); + if (na < 1 || na > 2) + return -FDT_ERR_BADNCELLS; + + ns = fdt_size_cells(blob, parent); + if (ns < 1 || ns > 2) + return -FDT_ERR_BADNCELLS; + + /* find a matching node and return the phandle to that */ + fdt_for_each_subnode(node, blob, parent) { + const char *name = fdt_get_name(blob, node, NULL); + phys_addr_t addr, size; + + addr = fdtdec_get_addr_size(blob, node, "reg", &size); + if (addr == FDT_ADDR_T_NONE) { + debug("failed to read address/size for %s\n", name); + continue; + } + + if (addr == carveout->start && (addr + size) == carveout->end) { + *phandlep = fdt_get_phandle(blob, node); + return 0; + } + } + + /* + * Unpack the start address and generate the name of the new node + * base on the basename and the unit-address. + */ + lower = fdt_addr_unpack(carveout->start, &upper); + + if (na > 1 && upper > 0) + snprintf(name, sizeof(name), "%s@%x,%x", basename, upper, + lower); + else { + if (upper > 0) { + debug("address %08x:%08x exceeds addressable space\n", + upper, lower); + return -FDT_ERR_BADVALUE; + } + + snprintf(name, sizeof(name), "%s@%x", basename, lower); + } + + node = fdt_add_subnode(blob, parent, name); + if (node < 0) + return node; + + err = fdt_generate_phandle(blob, &phandle); + if (err < 0) + return err; + + err = fdtdec_set_phandle(blob, node, phandle); + if (err < 0) + return err; + + /* store one or two address cells */ + if (na > 1) + *ptr++ = cpu_to_fdt32(upper); + + *ptr++ = cpu_to_fdt32(lower); + + /* store one or two size cells */ + lower = fdt_size_unpack(carveout->end - carveout->start + 1, &upper); + + if (ns > 1) + *ptr++ = cpu_to_fdt32(upper); + + *ptr++ = cpu_to_fdt32(lower); + + err = fdt_setprop(blob, node, "reg", cells, (na + ns) * sizeof(*cells)); + if (err < 0) + return err; + + /* return the phandle for the new node for the caller to use */ + if (phandlep) + *phandlep = phandle; + + return 0; +} + int fdtdec_setup(void) { #if CONFIG_IS_ENABLED(OF_CONTROL)

From: Thierry Reding treding@nvidia.com
This function can be used to add subnodes in the /reserved-memory node.
Reviewed-by: Simon Glass sjg@chromium.org Signed-off-by: Thierry Reding treding@nvidia.com --- Changes in v3: - use fdt_generate_phandle() instead of fdtdec_generate_phandle() - add device tree bindings for /reserved-memory - add examples to code comments
Changes in v2: - split fdt_{addr,size}_unpack() helpers into separate patch - use name@x,y notation only if the upper cell is > 0 - use debug() instead of printf() to save code size - properly compute number of cells in reg property - fix carveout size computations, was off by one - use #size-cells where appropriate
.../reserved-memory/reserved-memory.txt | 136 ++++++++++++++++++ include/fdtdec.h | 48 +++++++ lib/fdtdec.c | 131 +++++++++++++++++ 3 files changed, 315 insertions(+) create mode 100644 Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
Applied to u-boot-dm, thanks!

From: Thierry Reding treding@nvidia.com
The fdtdec_get_carveout() and fdtdec_set_carveout() function can be used to read a carveout from a given node or add a carveout to a given node using the standard device tree bindings (involving reserved-memory nodes and the memory-region property).
Reviewed-by: Simon Glass sjg@chromium.org Signed-off-by: Thierry Reding treding@nvidia.com --- Changes in v3: - add examples to code comments
Changes in v2: - use debug() instead of printf() to save code size - fix carveout size computations, was off by one - use fdtdec_get_addr_size_auto_noparent()
include/fdtdec.h | 81 ++++++++++++++++++++++++++++++++++++++++++++ lib/fdtdec.c | 87 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 168 insertions(+)
diff --git a/include/fdtdec.h b/include/fdtdec.h index b54ed38fb362..13b743f59ab1 100644 --- a/include/fdtdec.h +++ b/include/fdtdec.h @@ -1030,6 +1030,8 @@ int fdtdec_set_phandle(void *blob, int node, uint32_t phandle); * The phandle returned from the function call can be used to reference this * reserved memory region from other nodes. * + * See fdtdec_set_carveout() for a more elaborate example. + * * @param blob FDT blob * @param basename base name of the node to create * @param carveout information about the carveout region @@ -1040,6 +1042,85 @@ int fdtdec_add_reserved_memory(void *blob, const char *basename, const struct fdt_memory *carveout, uint32_t *phandlep);
+/** + * fdtdec_get_carveout() - reads a carveout from an FDT + * + * Reads information about a carveout region from an FDT. The carveout is a + * referenced by its phandle that is read from a given property in a given + * node. + * + * @param blob FDT blob + * @param node name of a node + * @param name name of the property in the given node that contains + * the phandle for the carveout + * @param index index of the phandle for which to read the carveout + * @param carveout return location for the carveout information + * @return 0 on success or a negative error code on failure + */ +int fdtdec_get_carveout(const void *blob, const char *node, const char *name, + unsigned int index, struct fdt_memory *carveout); + +/** + * fdtdec_set_carveout() - sets a carveout region for a given node + * + * Sets a carveout region for a given node. If a reserved-memory node already + * exists for the carveout, the phandle for that node will be reused. If no + * such node exists, a new one will be created and a phandle to it stored in + * a specified property of the given node. + * + * As an example, consider the following code snippet: + * + * const char *node = "/host1x@50000000/dc@54240000"; + * struct fdt_memory fb = { + * .start = 0x92cb3000, + * .end = 0x934b2fff, + * }; + * + * fdtdec_set_carveout(fdt, node, "memory-region", 0, "framebuffer", &fb); + * + * dc@54200000 is a display controller and was set up by the bootloader to + * scan out the framebuffer specified by "fb". This would cause the following + * reserved memory region to be added: + * + * reserved-memory { + * #address-cells = <0x00000002>; + * #size-cells = <0x00000002>; + * ranges; + * + * framebuffer@92cb3000 { + * reg = <0x00000000 0x92cb3000 0x00000000 0x00800000>; + * phandle = <0x0000004d>; + * }; + * }; + * + * A "memory-region" property will also be added to the node referenced by the + * offset parameter. + * + * host1x@50000000 { + * ... + * + * dc@54240000 { + * ... + * memory-region = <0x0000004d>; + * ... + * }; + * + * ... + * }; + * + * @param blob FDT blob + * @param node name of the node to add the carveout to + * @param prop_name name of the property in which to store the phandle of + * the carveout + * @param index index of the phandle to store + * @param name base name of the reserved-memory node to create + * @param carveout information about the carveout to add + * @return 0 on success or a negative error code on failure + */ +int fdtdec_set_carveout(void *blob, const char *node, const char *prop_name, + unsigned int index, const char *name, + const struct fdt_memory *carveout); + /** * Set up the device tree ready for use */ diff --git a/lib/fdtdec.c b/lib/fdtdec.c index be54ad5bd092..bd05ab90fce1 100644 --- a/lib/fdtdec.c +++ b/lib/fdtdec.c @@ -1381,6 +1381,93 @@ int fdtdec_add_reserved_memory(void *blob, const char *basename, return 0; }
+int fdtdec_get_carveout(const void *blob, const char *node, const char *name, + unsigned int index, struct fdt_memory *carveout) +{ + const fdt32_t *prop; + uint32_t phandle; + int offset, len; + fdt_size_t size; + + offset = fdt_path_offset(blob, node); + if (offset < 0) + return offset; + + prop = fdt_getprop(blob, offset, name, &len); + if (!prop) { + debug("failed to get %s for %s\n", name, node); + return -FDT_ERR_NOTFOUND; + } + + if ((len % sizeof(phandle)) != 0) { + debug("invalid phandle property\n"); + return -FDT_ERR_BADPHANDLE; + } + + if (len < (sizeof(phandle) * (index + 1))) { + debug("invalid phandle index\n"); + return -FDT_ERR_BADPHANDLE; + } + + phandle = fdt32_to_cpu(prop[index]); + + offset = fdt_node_offset_by_phandle(blob, phandle); + if (offset < 0) { + debug("failed to find node for phandle %u\n", phandle); + return offset; + } + + carveout->start = fdtdec_get_addr_size_auto_noparent(blob, offset, + "reg", 0, &size, + true); + if (carveout->start == FDT_ADDR_T_NONE) { + debug("failed to read address/size from "reg" property\n"); + return -FDT_ERR_NOTFOUND; + } + + carveout->end = carveout->start + size - 1; + + return 0; +} + +int fdtdec_set_carveout(void *blob, const char *node, const char *prop_name, + unsigned int index, const char *name, + const struct fdt_memory *carveout) +{ + uint32_t phandle; + int err, offset; + fdt32_t value; + + /* XXX implement support for multiple phandles */ + if (index > 0) { + debug("invalid index %u\n", index); + return -FDT_ERR_BADOFFSET; + } + + err = fdtdec_add_reserved_memory(blob, name, carveout, &phandle); + if (err < 0) { + debug("failed to add reserved memory: %d\n", err); + return err; + } + + offset = fdt_path_offset(blob, node); + if (offset < 0) { + debug("failed to find offset for node %s: %d\n", node, offset); + return offset; + } + + value = cpu_to_fdt32(phandle); + + err = fdt_setprop(blob, offset, prop_name, &value, sizeof(value)); + if (err < 0) { + debug("failed to set %s property for node %s: %d\n", prop_name, + node, err); + return err; + } + + return 0; +} + int fdtdec_setup(void) { #if CONFIG_IS_ENABLED(OF_CONTROL)

From: Thierry Reding treding@nvidia.com
The fdtdec_get_carveout() and fdtdec_set_carveout() function can be used to read a carveout from a given node or add a carveout to a given node using the standard device tree bindings (involving reserved-memory nodes and the memory-region property).
Reviewed-by: Simon Glass sjg@chromium.org Signed-off-by: Thierry Reding treding@nvidia.com --- Changes in v3: - add examples to code comments
Changes in v2: - use debug() instead of printf() to save code size - fix carveout size computations, was off by one - use fdtdec_get_addr_size_auto_noparent()
include/fdtdec.h | 81 ++++++++++++++++++++++++++++++++++++++++++++ lib/fdtdec.c | 87 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 168 insertions(+)
Applied to u-boot-dm, thanks!

From: Thierry Reding treding@nvidia.com
Runtime tests are provided as a test_fdtdec command implementation. Add a Kconfig symbol that allows this command to be built so that the tests can be used.
Signed-off-by: Thierry Reding treding@nvidia.com --- Changes in v2: - new patch
lib/Kconfig | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/lib/Kconfig b/lib/Kconfig index 366d164cd760..b1fccf7e8dff 100644 --- a/lib/Kconfig +++ b/lib/Kconfig @@ -423,4 +423,8 @@ source lib/efi/Kconfig source lib/efi_loader/Kconfig source lib/optee/Kconfig
+config TEST_FDTDEC + bool "enable fdtdec test" + depends on OF_LIBFDT + endmenu

On Fri, 22 Mar 2019 at 02:10, Thierry Reding thierry.reding@gmail.com wrote:
From: Thierry Reding treding@nvidia.com
Runtime tests are provided as a test_fdtdec command implementation. Add a Kconfig symbol that allows this command to be built so that the tests can be used.
Signed-off-by: Thierry Reding treding@nvidia.com
Changes in v2:
- new patch
lib/Kconfig | 4 ++++ 1 file changed, 4 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org

On Fri, 22 Mar 2019 at 02:10, Thierry Reding thierry.reding@gmail.com wrote:
From: Thierry Reding treding@nvidia.com
Runtime tests are provided as a test_fdtdec command implementation. Add a Kconfig symbol that allows this command to be built so that the tests can be used.
Signed-off-by: Thierry Reding treding@nvidia.com
Changes in v2:
- new patch
lib/Kconfig | 4 ++++ 1 file changed, 4 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org
Applied to u-boot-dm, thanks!

From: Thierry Reding treding@nvidia.com
Hide the declaration of the "fd" variable When not building a DEBUG configuration, to avoid the variable being unused.
Signed-off-by: Thierry Reding treding@nvidia.com --- Changes in v2: - new patch
lib/fdtdec_test.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/lib/fdtdec_test.c b/lib/fdtdec_test.c index a82e27de942f..065fed278cf3 100644 --- a/lib/fdtdec_test.c +++ b/lib/fdtdec_test.c @@ -79,7 +79,9 @@ static int make_fdt(void *fdt, int size, const char *aliases, { char name[20], value[20]; const char *s; +#if defined(DEBUG) && defined(CONFIG_SANDBOX) int fd; +#endif
CHECK(fdt_create(fdt, size)); CHECK(fdt_finish_reservemap(fdt));

On Fri, 22 Mar 2019 at 02:10, Thierry Reding thierry.reding@gmail.com wrote:
From: Thierry Reding treding@nvidia.com
Hide the declaration of the "fd" variable When not building a DEBUG configuration, to avoid the variable being unused.
Signed-off-by: Thierry Reding treding@nvidia.com
Changes in v2:
- new patch
lib/fdtdec_test.c | 2 ++ 1 file changed, 2 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org

On Fri, 22 Mar 2019 at 02:10, Thierry Reding thierry.reding@gmail.com wrote:
From: Thierry Reding treding@nvidia.com
Hide the declaration of the "fd" variable When not building a DEBUG configuration, to avoid the variable being unused.
Signed-off-by: Thierry Reding treding@nvidia.com
Changes in v2:
- new patch
lib/fdtdec_test.c | 2 ++ 1 file changed, 2 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org
Applied to u-boot-dm, thanks!

From: Thierry Reding treding@nvidia.com
This eliminates the need for intermediate helper functions and allow the macros to return a value so that it can be used subsequently.
Signed-off-by: Thierry Reding treding@nvidia.com --- Changes in v2: - new patch
lib/fdtdec_test.c | 64 ++++++++++++++++------------------------------- 1 file changed, 22 insertions(+), 42 deletions(-)
diff --git a/lib/fdtdec_test.c b/lib/fdtdec_test.c index 065fed278cf3..928950918413 100644 --- a/lib/fdtdec_test.c +++ b/lib/fdtdec_test.c @@ -15,48 +15,28 @@ /* The size of our test fdt blob */ #define FDT_SIZE (16 * 1024)
-/** - * Check if an operation failed, and if so, print an error - * - * @param oper_name Name of operation - * @param err Error code to check - * - * @return 0 if ok, -1 if there was an error - */ -static int fdt_checkerr(const char *oper_name, int err) -{ - if (err) { - printf("%s: %s: %s\n", __func__, oper_name, fdt_strerror(err)); - return -1; - } - - return 0; -} - -/** - * Check the result of an operation and if incorrect, print an error - * - * @param oper_name Name of operation - * @param expected Expected value - * @param value Actual value - * - * @return 0 if ok, -1 if there was an error - */ -static int checkval(const char *oper_name, int expected, int value) -{ - if (expected != value) { - printf("%s: %s: expected %d, but returned %d\n", __func__, - oper_name, expected, value); - return -1; - } - - return 0; -} +#define CHECK(op) ({ \ + int err = op; \ + if (err < 0) { \ + printf("%s: %s: %s\n", __func__, #op, \ + fdt_strerror(err)); \ + return err; \ + } \ + \ + err; \ + }) + +#define CHECKVAL(op, expected) ({ \ + int err = op; \ + if (err != expected) { \ + printf("%s: %s: expected %d, but returned %d\n",\ + __func__, #op, expected, err); \ + return err; \ + } \ + \ + err; \ + })
-#define CHECK(op) if (fdt_checkerr(#op, op)) return -1 -#define CHECKVAL(op, expected) \ - if (checkval(#op, expected, op)) \ - return -1 #define CHECKOK(op) CHECKVAL(op, 0)
/* maximum number of nodes / aliases to generate */ @@ -138,7 +118,7 @@ static int run_test(const char *aliases, const char *nodes, const char *expect) CHECKVAL(make_fdt(blob, FDT_SIZE, aliases, nodes), 0); CHECKVAL(fdtdec_find_aliases_for_id(blob, "i2c", COMPAT_UNKNOWN, - list, ARRAY_SIZE(list)), strlen(expect)); + list, ARRAY_SIZE(list)), (int)strlen(expect));
/* Check we got the right ones */ for (i = 0, s = expect; *s; s++, i++) {

On Fri, 22 Mar 2019 at 02:10, Thierry Reding thierry.reding@gmail.com wrote:
From: Thierry Reding treding@nvidia.com
This eliminates the need for intermediate helper functions and allow the macros to return a value so that it can be used subsequently.
Signed-off-by: Thierry Reding treding@nvidia.com
Changes in v2:
- new patch
lib/fdtdec_test.c | 64 ++++++++++++++++------------------------------- 1 file changed, 22 insertions(+), 42 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org

On Fri, 22 Mar 2019 at 02:10, Thierry Reding thierry.reding@gmail.com wrote:
From: Thierry Reding treding@nvidia.com
This eliminates the need for intermediate helper functions and allow the macros to return a value so that it can be used subsequently.
Signed-off-by: Thierry Reding treding@nvidia.com
Changes in v2:
- new patch
lib/fdtdec_test.c | 64 ++++++++++++++++------------------------------- 1 file changed, 22 insertions(+), 42 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
Applied to u-boot-dm, thanks!

From: Thierry Reding treding@nvidia.com
Implement carveout tests for 32-bit and 64-bit builds.
Signed-off-by: Thierry Reding treding@nvidia.com --- Changes in v2: - new patch
lib/fdtdec_test.c | 152 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 152 insertions(+)
diff --git a/lib/fdtdec_test.c b/lib/fdtdec_test.c index 928950918413..f6defe16c5a6 100644 --- a/lib/fdtdec_test.c +++ b/lib/fdtdec_test.c @@ -141,6 +141,156 @@ static int run_test(const char *aliases, const char *nodes, const char *expect) return 0; }
+static int make_fdt_carveout_device(void *fdt, uint32_t na, uint32_t ns) +{ + const char *basename = "/display"; + struct fdt_memory carveout = { +#ifdef CONFIG_PHYS_64BIT + .start = 0x180000000, + .end = 0x18fffffff, +#else + .start = 0x80000000, + .end = 0x8fffffff, +#endif + }; + fdt32_t cells[4], *ptr = cells; + uint32_t upper, lower; + char name[32]; + int offset; + + /* store one or two address cells */ + lower = fdt_addr_unpack(carveout.start, &upper); + + if (na > 1 && upper > 0) + snprintf(name, sizeof(name), "%s@%x,%x", basename, upper, + lower); + else + snprintf(name, sizeof(name), "%s@%x", basename, lower); + + if (na > 1) + *ptr++ = cpu_to_fdt32(upper); + + *ptr++ = cpu_to_fdt32(lower); + + /* store one or two size cells */ + lower = fdt_size_unpack(carveout.end - carveout.start + 1, &upper); + + if (ns > 1) + *ptr++ = cpu_to_fdt32(upper); + + *ptr++ = cpu_to_fdt32(lower); + + offset = CHECK(fdt_add_subnode(fdt, 0, name + 1)); + CHECK(fdt_setprop(fdt, offset, "reg", cells, (na + ns) * sizeof(*cells))); + + return fdtdec_set_carveout(fdt, name, "memory-region", 0, + "framebuffer", &carveout); +} + +static int check_fdt_carveout(void *fdt, uint32_t address_cells, + uint32_t size_cells) +{ +#ifdef CONFIG_PHYS_64BIT + const char *name = "/display@1,80000000"; + const struct fdt_memory expected = { + .start = 0x180000000, + .end = 0x18fffffff, + }; +#else + const char *name = "/display@80000000"; + const struct fdt_memory expected = { + .start = 0x80000000, + .end = 0x8fffffff, + }; +#endif + struct fdt_memory carveout; + + printf("carveout: %pap-%pap na=%u ns=%u: ", &expected.start, + &expected.end, address_cells, size_cells); + + CHECK(fdtdec_get_carveout(fdt, name, "memory-region", 0, &carveout)); + + if ((carveout.start != expected.start) || + (carveout.end != expected.end)) { + printf("carveout: %pap-%pap, expected %pap-%pap\n", + &carveout.start, &carveout.end, + &expected.start, &expected.end); + return 1; + } + + printf("pass\n"); + return 0; +} + +static int make_fdt_carveout(void *fdt, int size, uint32_t address_cells, + uint32_t size_cells) +{ + fdt32_t na = cpu_to_fdt32(address_cells); + fdt32_t ns = cpu_to_fdt32(size_cells); +#if defined(DEBUG) && defined(CONFIG_SANDBOX) + char filename[512]; + int fd; +#endif + int err; + + CHECK(fdt_create(fdt, size)); + CHECK(fdt_finish_reservemap(fdt)); + CHECK(fdt_begin_node(fdt, "")); + CHECK(fdt_property(fdt, "#address-cells", &na, sizeof(na))); + CHECK(fdt_property(fdt, "#size-cells", &ns, sizeof(ns))); + CHECK(fdt_end_node(fdt)); + CHECK(fdt_finish(fdt)); + CHECK(fdt_pack(fdt)); + + CHECK(fdt_open_into(fdt, fdt, FDT_SIZE)); + + err = make_fdt_carveout_device(fdt, address_cells, size_cells); + +#if defined(DEBUG) && defined(CONFIG_SANDBOX) + snprintf(filename, sizeof(filename), "/tmp/fdtdec-carveout-%u-%u.dtb", + address_cells, size_cells); + + fd = os_open(filename, OS_O_CREAT | OS_O_WRONLY); + if (fd < 0) { + printf("could not open .dtb file to write\n"); + goto out; + } + + os_write(fd, fdt, size); + os_close(fd); + +out: +#endif + return err; +} + +static int check_carveout(void) +{ + void *fdt; + + fdt = malloc(FDT_SIZE); + if (!fdt) { + printf("%s: out of memory\n", __func__); + return 1; + } + +#ifndef CONFIG_PHYS_64BIT + CHECKVAL(make_fdt_carveout(fdt, FDT_SIZE, 1, 1), 0); + CHECKOK(check_fdt_carveout(fdt, 1, 1)); + CHECKVAL(make_fdt_carveout(fdt, FDT_SIZE, 1, 2), 0); + CHECKOK(check_fdt_carveout(fdt, 1, 2)); +#else + CHECKVAL(make_fdt_carveout(fdt, FDT_SIZE, 1, 1), -FDT_ERR_BADVALUE); + CHECKVAL(make_fdt_carveout(fdt, FDT_SIZE, 1, 2), -FDT_ERR_BADVALUE); +#endif + CHECKVAL(make_fdt_carveout(fdt, FDT_SIZE, 2, 1), 0); + CHECKOK(check_fdt_carveout(fdt, 2, 1)); + CHECKVAL(make_fdt_carveout(fdt, FDT_SIZE, 2, 2), 0); + CHECKOK(check_fdt_carveout(fdt, 2, 2)); + + return 0; +} + static int do_test_fdtdec(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) { @@ -182,6 +332,8 @@ static int do_test_fdtdec(cmd_tbl_t *cmdtp, int flag, int argc, CHECKOK(run_test("2a 1a 0a", "a", " a")); CHECKOK(run_test("0a 1a 2a", "a", "a"));
+ CHECKOK(check_carveout()); + printf("Test passed\n"); return 0; }

Hi Thierry,
On Fri, 22 Mar 2019 at 02:10, Thierry Reding thierry.reding@gmail.com wrote:
From: Thierry Reding treding@nvidia.com
Implement carveout tests for 32-bit and 64-bit builds.
Signed-off-by: Thierry Reding treding@nvidia.com
Changes in v2:
- new patch
lib/fdtdec_test.c | 152 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 152 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org
Just an idea - as an alternative you could use the built-in device tree as your base rather than creating a new one. But perhaps that would only be safe on sandbox?
Regards, Simon

On Fri, Mar 22, 2019 at 03:53:07PM +0800, Simon Glass wrote:
Hi Thierry,
On Fri, 22 Mar 2019 at 02:10, Thierry Reding thierry.reding@gmail.com wrote:
From: Thierry Reding treding@nvidia.com
Implement carveout tests for 32-bit and 64-bit builds.
Signed-off-by: Thierry Reding treding@nvidia.com
Changes in v2:
- new patch
lib/fdtdec_test.c | 152 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 152 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org
Just an idea - as an alternative you could use the built-in device tree as your base rather than creating a new one. But perhaps that would only be safe on sandbox?
Yeah, running that test on a live system would mess up the reserved memory regions. If U-Boot does something based on the reserved-memory node, or passes it on to the kernel, that's going to cause issues.
The test also uses rather arbitrary values for the carveout, which may not point at system memory at all.
Thierry

On Fri, Mar 22, 2019 at 03:53:07PM +0800, Simon Glass wrote:
Hi Thierry,
On Fri, 22 Mar 2019 at 02:10, Thierry Reding thierry.reding@gmail.com wrote:
From: Thierry Reding treding@nvidia.com
Implement carveout tests for 32-bit and 64-bit builds.
Signed-off-by: Thierry Reding treding@nvidia.com
Changes in v2:
- new patch
lib/fdtdec_test.c | 152 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 152 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org
Just an idea - as an alternative you could use the built-in device tree as your base rather than creating a new one. But perhaps that would only be safe on sandbox?
Yeah, running that test on a live system would mess up the reserved memory regions. If U-Boot does something based on the reserved-memory node, or passes it on to the kernel, that's going to cause issues.
The test also uses rather arbitrary values for the carveout, which may not point at system memory at all.
Thierry
Applied to u-boot-dm, thanks!

From: Thierry Reding treding@nvidia.com
Enable fdtdec tests on sandbox configurations so that they can be run to validate the fdtdec implementation.
Signed-off-by: Thierry Reding treding@nvidia.com --- Changes in v2: - new patch
configs/sandbox64_defconfig | 1 + configs/sandbox_defconfig | 1 + 2 files changed, 2 insertions(+)
diff --git a/configs/sandbox64_defconfig b/configs/sandbox64_defconfig index da4bdced3105..c04ecd915ae7 100644 --- a/configs/sandbox64_defconfig +++ b/configs/sandbox64_defconfig @@ -194,6 +194,7 @@ CONFIG_CMD_DHRYSTONE=y CONFIG_TPM=y CONFIG_LZ4=y CONFIG_ERRNO_STR=y +CONFIG_TEST_FDTDEC=y CONFIG_UNIT_TEST=y CONFIG_UT_TIME=y CONFIG_UT_DM=y diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig index 193e41896cb7..bb508a8d02e2 100644 --- a/configs/sandbox_defconfig +++ b/configs/sandbox_defconfig @@ -215,6 +215,7 @@ CONFIG_CMD_DHRYSTONE=y CONFIG_TPM=y CONFIG_LZ4=y CONFIG_ERRNO_STR=y +CONFIG_TEST_FDTDEC=y CONFIG_UNIT_TEST=y CONFIG_UT_TIME=y CONFIG_UT_DM=y

On Fri, 22 Mar 2019 at 02:10, Thierry Reding thierry.reding@gmail.com wrote:
From: Thierry Reding treding@nvidia.com
Enable fdtdec tests on sandbox configurations so that they can be run to validate the fdtdec implementation.
Signed-off-by: Thierry Reding treding@nvidia.com
Changes in v2:
- new patch
configs/sandbox64_defconfig | 1 + configs/sandbox_defconfig | 1 + 2 files changed, 2 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org

On Fri, 22 Mar 2019 at 02:10, Thierry Reding thierry.reding@gmail.com wrote:
From: Thierry Reding treding@nvidia.com
Enable fdtdec tests on sandbox configurations so that they can be run to validate the fdtdec implementation.
Signed-off-by: Thierry Reding treding@nvidia.com
Changes in v2:
- new patch
configs/sandbox64_defconfig | 1 + configs/sandbox_defconfig | 1 + 2 files changed, 2 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org
Applied to u-boot-dm, thanks!

From: Thierry Reding treding@nvidia.com
If early firmware initialized the display hardware and the display controllers are scanning out a framebuffer (e.g. a splash screen), make sure to pass information about the memory location of that framebuffer to the kernel before booting to avoid the kernel from using that memory for the buddy allocator.
This same mechanism can also be used in the kernel to set up early SMMU mappings and avoid SMMU faults caused by the display controller reading from memory for which it has no mapping.
Signed-off-by: Thierry Reding treding@nvidia.com --- board/nvidia/p2371-2180/p2371-2180.c | 47 ++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+)
diff --git a/board/nvidia/p2371-2180/p2371-2180.c b/board/nvidia/p2371-2180/p2371-2180.c index a444d692d7ea..4985302d6bc2 100644 --- a/board/nvidia/p2371-2180/p2371-2180.c +++ b/board/nvidia/p2371-2180/p2371-2180.c @@ -6,6 +6,7 @@
#include <common.h> #include <environment.h> +#include <fdtdec.h> #include <i2c.h> #include <linux/libfdt.h> #include <asm/arch/gpio.h> @@ -138,9 +139,55 @@ static void ft_mac_address_setup(void *fdt) } }
+static int ft_copy_carveout(void *dst, const void *src, const char *node) +{ + struct fdt_memory fb; + int err; + + err = fdtdec_get_carveout(src, node, "memory-region", 0, &fb); + if (err < 0) { + if (err != -FDT_ERR_NOTFOUND) + printf("failed to get carveout for %s: %d\n", node, + err); + + return err; + } + + err = fdtdec_set_carveout(dst, node, "memory-region", 0, "framebuffer", + &fb); + if (err < 0) { + printf("failed to set carveout for %s: %d\n", node, err); + return err; + } + + return 0; +} + +static void ft_carveout_setup(void *fdt) +{ + const void *cboot_fdt = (const void *)cboot_boot_x0; + static const char * const nodes[] = { + "/host1x@50000000/dc@54200000", + "/host1x@50000000/dc@54240000", + }; + unsigned int i; + int err; + + for (i = 0; i < ARRAY_SIZE(nodes); i++) { + err = ft_copy_carveout(fdt, cboot_fdt, nodes[i]); + if (err < 0) { + if (err != -FDT_ERR_NOTFOUND) + printf("failed to copy carveout for %s: %d\n", + nodes[i], err); + continue; + } + } +} + int ft_board_setup(void *fdt, bd_t *bd) { ft_mac_address_setup(fdt); + ft_carveout_setup(fdt);
return 0; }

On Fri, 22 Mar 2019 at 02:10, Thierry Reding thierry.reding@gmail.com wrote:
From: Thierry Reding treding@nvidia.com
If early firmware initialized the display hardware and the display controllers are scanning out a framebuffer (e.g. a splash screen), make sure to pass information about the memory location of that framebuffer to the kernel before booting to avoid the kernel from using that memory for the buddy allocator.
This same mechanism can also be used in the kernel to set up early SMMU mappings and avoid SMMU faults caused by the display controller reading from memory for which it has no mapping.
Signed-off-by: Thierry Reding treding@nvidia.com
board/nvidia/p2371-2180/p2371-2180.c | 47 ++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org

Hi Thierry,
On Fri, 22 Mar 2019 at 01:53, Simon Glass sjg@chromium.org wrote:
On Fri, 22 Mar 2019 at 02:10, Thierry Reding thierry.reding@gmail.com wrote:
From: Thierry Reding treding@nvidia.com
If early firmware initialized the display hardware and the display controllers are scanning out a framebuffer (e.g. a splash screen), make sure to pass information about the memory location of that framebuffer to the kernel before booting to avoid the kernel from using that memory for the buddy allocator.
This same mechanism can also be used in the kernel to set up early SMMU mappings and avoid SMMU faults caused by the display controller reading from memory for which it has no mapping.
Signed-off-by: Thierry Reding treding@nvidia.com
board/nvidia/p2371-2180/p2371-2180.c | 47 ++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org
This doesn't apply for me. Can you please resend it?
Regards, Simon

On Thu, Apr 11, 2019 at 08:12:03PM -0600, Simon Glass wrote:
Hi Thierry,
On Fri, 22 Mar 2019 at 01:53, Simon Glass sjg@chromium.org wrote:
On Fri, 22 Mar 2019 at 02:10, Thierry Reding thierry.reding@gmail.com wrote:
From: Thierry Reding treding@nvidia.com
If early firmware initialized the display hardware and the display controllers are scanning out a framebuffer (e.g. a splash screen), make sure to pass information about the memory location of that framebuffer to the kernel before booting to avoid the kernel from using that memory for the buddy allocator.
This same mechanism can also be used in the kernel to set up early SMMU mappings and avoid SMMU faults caused by the display controller reading from memory for which it has no mapping.
Signed-off-by: Thierry Reding treding@nvidia.com
board/nvidia/p2371-2180/p2371-2180.c | 47 ++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org
This doesn't apply for me. Can you please resend it?
This (and patch 13/13) depend on another series that I had expected to go into Tom's tree earlier than this series. However, there were some unexpected regressions caused by the other series, so I've got a couple of minor changes to that, so maybe I should just add these two patches to the other series for Tom to pick up?
Thierry

From: Thierry Reding treding@nvidia.com
If early firmware initialized the display hardware and the display controllers are scanning out a framebuffer (e.g. a splash screen), make sure to pass information about the memory location of that framebuffer to the kernel before booting to avoid the kernel from using that memory for the buddy allocator.
This same mechanism can also be used in the kernel to set up early SMMU mappings and avoid SMMU faults caused by the display controller reading from memory for which it has no mapping.
Signed-off-by: Thierry Reding treding@nvidia.com --- board/nvidia/p2771-0000/p2771-0000.c | 66 ++++++++++++++++++++++++++-- 1 file changed, 62 insertions(+), 4 deletions(-)
diff --git a/board/nvidia/p2771-0000/p2771-0000.c b/board/nvidia/p2771-0000/p2771-0000.c index fe22067f6571..d294c7ae0136 100644 --- a/board/nvidia/p2771-0000/p2771-0000.c +++ b/board/nvidia/p2771-0000/p2771-0000.c @@ -5,6 +5,7 @@
#include <common.h> #include <environment.h> +#include <fdtdec.h> #include <i2c.h> #include <linux/libfdt.h> #include <asm/arch-tegra/cboot.h> @@ -56,7 +57,7 @@ int tegra_pcie_board_init(void) } #endif
-int ft_board_setup(void *fdt, bd_t *bd) +static void ft_mac_address_setup(void *fdt) { const void *cboot_fdt = (const void *)cboot_boot_x0; uint8_t mac[ETH_ALEN], local_mac[ETH_ALEN]; @@ -69,13 +70,15 @@ int ft_board_setup(void *fdt, bd_t *bd)
path = fdt_get_alias(fdt, "ethernet"); if (!path) - return 0; + return;
debug("ethernet alias found: %s\n", path);
offset = fdt_path_offset(fdt, path); - if (offset < 0) - return 0; + if (offset < 0) { + printf("ethernet alias points to absent node %s\n", path); + return; + }
if (is_valid_ethaddr(local_mac)) { err = fdt_setprop(fdt, offset, "local-mac-address", local_mac, @@ -92,6 +95,61 @@ int ft_board_setup(void *fdt, bd_t *bd) debug("MAC address set: %pM\n", mac); } } +} + +static int ft_copy_carveout(void *dst, const void *src, const char *node) +{ + struct fdt_memory fb; + int err; + + err = fdtdec_get_carveout(src, node, "memory-region", 0, &fb); + if (err < 0) { + if (err != -FDT_ERR_NOTFOUND) + printf("failed to get carveout for %s: %d\n", node, + err); + + return err; + } + + err = fdtdec_set_carveout(dst, node, "memory-region", 0, "framebuffer", + &fb); + if (err < 0) { + printf("failed to set carveout for %s: %d\n", node, err); + return err; + } + + return 0; +} + +static void ft_carveout_setup(void *fdt) +{ + const void *cboot_fdt = (const void *)cboot_boot_x0; + static const char * const nodes[] = { + "/host1x@13e00000/display-hub@15200000/display@15200000", + "/host1x@13e00000/display-hub@15200000/display@15210000", + "/host1x@13e00000/display-hub@15200000/display@15220000", + }; + unsigned int i; + int err; + + for (i = 0; i < ARRAY_SIZE(nodes); i++) { + printf("copying carveout for %s...\n", nodes[i]); + + err = ft_copy_carveout(fdt, cboot_fdt, nodes[i]); + if (err < 0) { + if (err != -FDT_ERR_NOTFOUND) + printf("failed to copy carveout for %s: %d\n", + nodes[i], err); + + continue; + } + } +} + +int ft_board_setup(void *fdt, bd_t *bd) +{ + ft_mac_address_setup(fdt); + ft_carveout_setup(fdt);
return 0; }

On Fri, 22 Mar 2019 at 02:10, Thierry Reding thierry.reding@gmail.com wrote:
From: Thierry Reding treding@nvidia.com
If early firmware initialized the display hardware and the display controllers are scanning out a framebuffer (e.g. a splash screen), make sure to pass information about the memory location of that framebuffer to the kernel before booting to avoid the kernel from using that memory for the buddy allocator.
This same mechanism can also be used in the kernel to set up early SMMU mappings and avoid SMMU faults caused by the display controller reading from memory for which it has no mapping.
Signed-off-by: Thierry Reding treding@nvidia.com
board/nvidia/p2771-0000/p2771-0000.c | 66 ++++++++++++++++++++++++++-- 1 file changed, 62 insertions(+), 4 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org

Hi Thierry,
On Fri, 22 Mar 2019 at 01:53, Simon Glass sjg@chromium.org wrote:
On Fri, 22 Mar 2019 at 02:10, Thierry Reding thierry.reding@gmail.com wrote:
From: Thierry Reding treding@nvidia.com
If early firmware initialized the display hardware and the display controllers are scanning out a framebuffer (e.g. a splash screen), make sure to pass information about the memory location of that framebuffer to the kernel before booting to avoid the kernel from using that memory for the buddy allocator.
This same mechanism can also be used in the kernel to set up early SMMU mappings and avoid SMMU faults caused by the display controller reading from memory for which it has no mapping.
Signed-off-by: Thierry Reding treding@nvidia.com
board/nvidia/p2771-0000/p2771-0000.c | 66 ++++++++++++++++++++++++++-- 1 file changed, 62 insertions(+), 4 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
This doesn't apply for me. Please can you resend it?
Regards, Simon
participants (3)
-
Simon Glass
-
sjg@google.com
-
Thierry Reding