[PATCH 1/2] lmb: Fix lmb property's defination under struct lmb

From: Ashok Reddy Soma ashok.reddy.soma@xilinx.com
Under struct lmb {} the lmb property's should be defined only if CONFIG_LMB_MEMORY_REGIONS is defined.
Signed-off-by: Ashok Reddy Soma ashok.reddy.soma@xilinx.com Signed-off-by: Michal Simek michal.simek@xilinx.com ---
include/lmb.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/lmb.h b/include/lmb.h index ab277ca80004..1476d78c2823 100644 --- a/include/lmb.h +++ b/include/lmb.h @@ -68,7 +68,7 @@ struct lmb_region { struct lmb { struct lmb_region memory; struct lmb_region reserved; -#if !IS_ENABLED(CONFIG_LMB_USE_MAX_REGIONS) +#if IS_ENABLED(CONFIG_LMB_MEMORY_REGIONS) struct lmb_property memory_regions[CONFIG_LMB_MEMORY_REGIONS]; struct lmb_property reserved_regions[CONFIG_LMB_RESERVED_REGIONS]; #endif

From: Ashok Reddy Soma ashok.reddy.soma@xilinx.com
With commit ce39ee28ec31 ("zynqmp: Do not place u-boot to reserved memory location"), the function board_get_usable_ram_top() is allocating MMU_SECTION_SIZE of about 2MB using lmb_alloc(). But we dont have this much memory in case of mini U-Boot.
Keep these functions which use lmb under CONFIG_LMB so that they are compiled and used only when LMB is enabled.
Signed-off-by: Ashok Reddy Soma ashok.reddy.soma@xilinx.com Signed-off-by: Michal Simek michal.simek@xilinx.com ---
board/xilinx/zynqmp/zynqmp.c | 3 +++ boot/image-board.c | 4 ++++ 2 files changed, 7 insertions(+)
diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c index 2b5239ccb475..c69fb939a83e 100644 --- a/board/xilinx/zynqmp/zynqmp.c +++ b/board/xilinx/zynqmp/zynqmp.c @@ -483,6 +483,7 @@ int dram_init(void) return 0; }
+#if defined(CONFIG_LMB) ulong board_get_usable_ram_top(ulong total_size) { phys_size_t size; @@ -504,6 +505,8 @@ ulong board_get_usable_ram_top(ulong total_size)
return reg + size; } +#endif + #else int dram_init_banksize(void) { diff --git a/boot/image-board.c b/boot/image-board.c index bf8817165cab..9cdfbc44154c 100644 --- a/boot/image-board.c +++ b/boot/image-board.c @@ -546,6 +546,7 @@ int boot_get_ramdisk(int argc, char *const argv[], bootm_headers_t *images, return 0; }
+#if defined(CONFIG_LMB) /** * boot_ramdisk_high - relocate init ramdisk * @lmb: pointer to lmb handle, will be used for memory mgmt @@ -639,6 +640,7 @@ int boot_ramdisk_high(struct lmb *lmb, ulong rd_data, ulong rd_len, error: return -1; } +#endif
int boot_get_setup(bootm_headers_t *images, u8 arch, ulong *setup_start, ulong *setup_len) @@ -832,6 +834,7 @@ int boot_get_loadable(int argc, char *const argv[], bootm_headers_t *images, return 0; }
+#if defined(CONFIG_LMB) /** * boot_get_cmdline - allocate and initialize kernel cmdline * @lmb: pointer to lmb handle, will be used for memory mgmt @@ -939,6 +942,7 @@ int image_setup_linux(bootm_headers_t *images)
return 0; } +#endif
void genimg_print_size(uint32_t size) {

pá 14. 1. 2022 v 13:14 odesílatel Michal Simek michal.simek@xilinx.com napsal:
From: Ashok Reddy Soma ashok.reddy.soma@xilinx.com
With commit ce39ee28ec31 ("zynqmp: Do not place u-boot to reserved memory location"), the function board_get_usable_ram_top() is allocating MMU_SECTION_SIZE of about 2MB using lmb_alloc(). But we dont have this much memory in case of mini U-Boot.
Keep these functions which use lmb under CONFIG_LMB so that they are compiled and used only when LMB is enabled.
Signed-off-by: Ashok Reddy Soma ashok.reddy.soma@xilinx.com Signed-off-by: Michal Simek michal.simek@xilinx.com
board/xilinx/zynqmp/zynqmp.c | 3 +++ boot/image-board.c | 4 ++++ 2 files changed, 7 insertions(+)
diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c index 2b5239ccb475..c69fb939a83e 100644 --- a/board/xilinx/zynqmp/zynqmp.c +++ b/board/xilinx/zynqmp/zynqmp.c @@ -483,6 +483,7 @@ int dram_init(void) return 0; }
+#if defined(CONFIG_LMB) ulong board_get_usable_ram_top(ulong total_size) { phys_size_t size; @@ -504,6 +505,8 @@ ulong board_get_usable_ram_top(ulong total_size)
return reg + size;
} +#endif
#else int dram_init_banksize(void) { diff --git a/boot/image-board.c b/boot/image-board.c index bf8817165cab..9cdfbc44154c 100644 --- a/boot/image-board.c +++ b/boot/image-board.c @@ -546,6 +546,7 @@ int boot_get_ramdisk(int argc, char *const argv[], bootm_headers_t *images, return 0; }
+#if defined(CONFIG_LMB) /**
- boot_ramdisk_high - relocate init ramdisk
- @lmb: pointer to lmb handle, will be used for memory mgmt
@@ -639,6 +640,7 @@ int boot_ramdisk_high(struct lmb *lmb, ulong rd_data, ulong rd_len, error: return -1; } +#endif
int boot_get_setup(bootm_headers_t *images, u8 arch, ulong *setup_start, ulong *setup_len) @@ -832,6 +834,7 @@ int boot_get_loadable(int argc, char *const argv[], bootm_headers_t *images, return 0; }
+#if defined(CONFIG_LMB) /**
- boot_get_cmdline - allocate and initialize kernel cmdline
- @lmb: pointer to lmb handle, will be used for memory mgmt
@@ -939,6 +942,7 @@ int image_setup_linux(bootm_headers_t *images)
return 0;
} +#endif
void genimg_print_size(uint32_t size) { -- 2.34.1
Applied. M

Hi,
On 1/14/22 1:14 PM, Michal Simek wrote:
From: Ashok Reddy Soma ashok.reddy.soma@xilinx.com
Under struct lmb {} the lmb property's should be defined only if CONFIG_LMB_MEMORY_REGIONS is defined.
Signed-off-by: Ashok Reddy Soma ashok.reddy.soma@xilinx.com Signed-off-by: Michal Simek michal.simek@xilinx.com
include/lmb.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/lmb.h b/include/lmb.h index ab277ca80004..1476d78c2823 100644 --- a/include/lmb.h +++ b/include/lmb.h @@ -68,7 +68,7 @@ struct lmb_region { struct lmb { struct lmb_region memory; struct lmb_region reserved; -#if !IS_ENABLED(CONFIG_LMB_USE_MAX_REGIONS) +#if IS_ENABLED(CONFIG_LMB_MEMORY_REGIONS) struct lmb_property memory_regions[CONFIG_LMB_MEMORY_REGIONS]; struct lmb_property reserved_regions[CONFIG_LMB_RESERVED_REGIONS]; #endif
I think this patch don't change the exiting code
because in lib/Kconfig we have also the dependancy with LMB_USE_MAX_REGIONS:
config LMB_MEMORY_REGIONS int "Number of memory regions in lmb lib" depends on LMB && !LMB_USE_MAX_REGIONS
=> memory_regions and reserved_regions are needed in struc lmb only if CONFIG_LMB_USE_MAX_REGIONS is not defined else it is defined in "struct lmb_region" under the SAME compilation flag
struct lmb_region { unsigned long cnt; unsigned long max; #if IS_ENABLED(CONFIG_LMB_USE_MAX_REGIONS) struct lmb_property region[CONFIG_LMB_MAX_REGIONS]; #else struct lmb_property *region; #endif };
with .region = pointer to 'memory_regions[]' or 'reserved_regions[]' in "struct lmb"
I think it is more clear to have the compilation flag in "struct lmb_region" and in "struct lmb_region".
but I have no objection to change it.
PS: I introduce this flag to keep the previous behavior and previous struct size on other platform
when I push the commit 6d66502bc741 ("lmb: Add 2 config to define the max number of regions")
Regards
Patrick

On 1/17/22 17:23, Patrick DELAUNAY wrote:
Hi,
On 1/14/22 1:14 PM, Michal Simek wrote:
From: Ashok Reddy Soma ashok.reddy.soma@xilinx.com
Under struct lmb {} the lmb property's should be defined only if CONFIG_LMB_MEMORY_REGIONS is defined.
Signed-off-by: Ashok Reddy Soma ashok.reddy.soma@xilinx.com Signed-off-by: Michal Simek michal.simek@xilinx.com
include/lmb.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/lmb.h b/include/lmb.h index ab277ca80004..1476d78c2823 100644 --- a/include/lmb.h +++ b/include/lmb.h @@ -68,7 +68,7 @@ struct lmb_region { struct lmb { struct lmb_region memory; struct lmb_region reserved; -#if !IS_ENABLED(CONFIG_LMB_USE_MAX_REGIONS) +#if IS_ENABLED(CONFIG_LMB_MEMORY_REGIONS) struct lmb_property memory_regions[CONFIG_LMB_MEMORY_REGIONS]; struct lmb_property reserved_regions[CONFIG_LMB_RESERVED_REGIONS]; #endif
I think this patch don't change the exiting code
because in lib/Kconfig we have also the dependancy with LMB_USE_MAX_REGIONS:
config LMB_MEMORY_REGIONS int "Number of memory regions in lmb lib" depends on LMB && !LMB_USE_MAX_REGIONS
=> memory_regions and reserved_regions are needed in struc lmb only if CONFIG_LMB_USE_MAX_REGIONS is not defined else it is defined in "struct lmb_region" under the SAME compilation flag
struct lmb_region { unsigned long cnt; unsigned long max; #if IS_ENABLED(CONFIG_LMB_USE_MAX_REGIONS) struct lmb_property region[CONFIG_LMB_MAX_REGIONS]; #else struct lmb_property *region; #endif };
with .region = pointer to 'memory_regions[]' or 'reserved_regions[]' in "struct lmb"
I think it is more clear to have the compilation flag in "struct lmb_region" and in "struct lmb_region".
but I have no objection to change it.
PS: I introduce this flag to keep the previous behavior and previous struct size on other platform
when I push the commit 6d66502bc741 ("lmb: Add 2 config to define the max number of regions")
When I run make xilinx_zynqmp_virt_defconfig and then disable LMB I am getting compilation failure just simply because of CONFIG_LMB_USE_MAX_REGIONS is disabled but because there is #if !IS_ENABLED(CONFIG_LMB_USE_MAX_REGIONS) which is simply #if 1 but CONFIG_LMB_MEMORY_REGIONS is not defined.
That's why patch is just correcting that if statement to be aligned with macro which should be defined.
Thanks, Michal

On 1/17/22 17:23, Patrick DELAUNAY wrote:
Hi,
On 1/14/22 1:14 PM, Michal Simek wrote:
From: Ashok Reddy Soma ashok.reddy.soma@xilinx.com
Under struct lmb {} the lmb property's should be defined only if CONFIG_LMB_MEMORY_REGIONS is defined.
Signed-off-by: Ashok Reddy Soma ashok.reddy.soma@xilinx.com Signed-off-by: Michal Simek michal.simek@xilinx.com
include/lmb.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/lmb.h b/include/lmb.h index ab277ca80004..1476d78c2823 100644 --- a/include/lmb.h +++ b/include/lmb.h @@ -68,7 +68,7 @@ struct lmb_region { struct lmb { struct lmb_region memory; struct lmb_region reserved; -#if !IS_ENABLED(CONFIG_LMB_USE_MAX_REGIONS) +#if IS_ENABLED(CONFIG_LMB_MEMORY_REGIONS) struct lmb_property memory_regions[CONFIG_LMB_MEMORY_REGIONS]; struct lmb_property reserved_regions[CONFIG_LMB_RESERVED_REGIONS]; #endif
I think this patch don't change the exiting code
because in lib/Kconfig we have also the dependancy with LMB_USE_MAX_REGIONS:
config LMB_MEMORY_REGIONS int "Number of memory regions in lmb lib" depends on LMB && !LMB_USE_MAX_REGIONS
=> memory_regions and reserved_regions are needed in struc lmb only if CONFIG_LMB_USE_MAX_REGIONS is not defined else it is defined in "struct lmb_region" under the SAME compilation flag
struct lmb_region { unsigned long cnt; unsigned long max; #if IS_ENABLED(CONFIG_LMB_USE_MAX_REGIONS) struct lmb_property region[CONFIG_LMB_MAX_REGIONS]; #else struct lmb_property *region; #endif };
with .region = pointer to 'memory_regions[]' or 'reserved_regions[]' in "struct lmb"
I think it is more clear to have the compilation flag in "struct lmb_region" and in "struct lmb_region".
but I have no objection to change it.
PS: I introduce this flag to keep the previous behavior and previous struct size on other platform
when I push the commit 6d66502bc741 ("lmb: Add 2 config to define the max number of regions")
Regards
Patrick
Should we really use #ifdef for these structures to save some bytes on the stack. Isn't it preferable to move the code from using '#ifdef CONFIG' to 'if IS_ENABLED(CONFIG'?
Best regards
Heinrich

pá 14. 1. 2022 v 13:14 odesílatel Michal Simek michal.simek@xilinx.com napsal:
From: Ashok Reddy Soma ashok.reddy.soma@xilinx.com
Under struct lmb {} the lmb property's should be defined only if CONFIG_LMB_MEMORY_REGIONS is defined.
Signed-off-by: Ashok Reddy Soma ashok.reddy.soma@xilinx.com Signed-off-by: Michal Simek michal.simek@xilinx.com
include/lmb.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/lmb.h b/include/lmb.h index ab277ca80004..1476d78c2823 100644 --- a/include/lmb.h +++ b/include/lmb.h @@ -68,7 +68,7 @@ struct lmb_region { struct lmb { struct lmb_region memory; struct lmb_region reserved; -#if !IS_ENABLED(CONFIG_LMB_USE_MAX_REGIONS) +#if IS_ENABLED(CONFIG_LMB_MEMORY_REGIONS) struct lmb_property memory_regions[CONFIG_LMB_MEMORY_REGIONS]; struct lmb_property reserved_regions[CONFIG_LMB_RESERVED_REGIONS];
#endif
2.34.1
applied. M

po 7. 2. 2022 v 10:40 odesílatel Michal Simek monstr@monstr.eu napsal:
pá 14. 1. 2022 v 13:14 odesílatel Michal Simek michal.simek@xilinx.com napsal:
From: Ashok Reddy Soma ashok.reddy.soma@xilinx.com
Under struct lmb {} the lmb property's should be defined only if CONFIG_LMB_MEMORY_REGIONS is defined.
Signed-off-by: Ashok Reddy Soma ashok.reddy.soma@xilinx.com Signed-off-by: Michal Simek michal.simek@xilinx.com
include/lmb.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/lmb.h b/include/lmb.h index ab277ca80004..1476d78c2823 100644 --- a/include/lmb.h +++ b/include/lmb.h @@ -68,7 +68,7 @@ struct lmb_region { struct lmb { struct lmb_region memory; struct lmb_region reserved; -#if !IS_ENABLED(CONFIG_LMB_USE_MAX_REGIONS) +#if IS_ENABLED(CONFIG_LMB_MEMORY_REGIONS) struct lmb_property memory_regions[CONFIG_LMB_MEMORY_REGIONS]; struct lmb_property reserved_regions[CONFIG_LMB_RESERVED_REGIONS];
#endif
2.34.1
applied. M
ci loop found an issue that's why I have sent v2 and remove these patches from my queue.
M
participants (4)
-
Heinrich Schuchardt
-
Michal Simek
-
Michal Simek
-
Patrick DELAUNAY