diff mbox series

[1/2] lmb: Fix LMB_MEMORY_REGIONS flag usage

Message ID 20230321135805.1.I2a3a1c4e853d66857091cbef8d3a2ec87d65daba@changeid
State Changes Requested
Delegated to: Michal Simek
Headers show
Series [1/2] lmb: Fix LMB_MEMORY_REGIONS flag usage | expand

Commit Message

Patrick Delaunay March 21, 2023, 12:58 p.m. UTC
Remove test on CONFIG_LMB_MEMORY_REGIONS introduced by commit
7c1860fce4e3 ("lmb: Fix lmb property's defination under struct lmb").

This code in lmb_init() is strange, because if CONFIG_LMB_USE_MAX_REGIONS
and CONFIG_LMB_MEMORY_REGIONS are not defined, the implicit #else is empty
and the required initialization are not done:
lmb->memory.max = ?
lmb->reserved.max = ?

But this setting is not possible:
- CONFIG_LMB_USE_MAX_REGIONS not defined
- CONFIG_LMB_MEMORY_REGIONS not defined
because CONFIG_LMB_MEMORY_REGIONS and CONFIG_LMB_RESERVED_REGIONS are
defined as soon as the CONFIG_LMB_USE_MAX_REGIONS is not defined.

This patch removes this impossible case #elif and I add some
explanation in lmb.h to explain why in the struct lmb {} the lmb
property's should is defined if CONFIG_LMB_MEMORY_REGIONS is NOT defined.

Fixes: 5e2548c1d6e03 ("lmb: Fix LMB_MEMORY_REGIONS flag usage")
Reported-by: Mark Millard <marklmi@yahoo.com>
Signed-off-by: Patrick Delaunay <patrick.delaunay@foss.st.com>
---

 include/lmb.h | 20 +++++++++++++++++++-
 lib/lmb.c     |  2 +-
 2 files changed, 20 insertions(+), 2 deletions(-)

Comments

Michal Simek March 21, 2023, 2:10 p.m. UTC | #1
On 3/21/23 13:58, Patrick Delaunay wrote:
> Remove test on CONFIG_LMB_MEMORY_REGIONS introduced by commit
> 7c1860fce4e3 ("lmb: Fix lmb property's defination under struct lmb").
> 
> This code in lmb_init() is strange, because if CONFIG_LMB_USE_MAX_REGIONS
> and CONFIG_LMB_MEMORY_REGIONS are not defined, the implicit #else is empty
> and the required initialization are not done:
> lmb->memory.max = ?
> lmb->reserved.max = ?
> 
> But this setting is not possible:
> - CONFIG_LMB_USE_MAX_REGIONS not defined
> - CONFIG_LMB_MEMORY_REGIONS not defined
> because CONFIG_LMB_MEMORY_REGIONS and CONFIG_LMB_RESERVED_REGIONS are
> defined as soon as the CONFIG_LMB_USE_MAX_REGIONS is not defined.
> 
> This patch removes this impossible case #elif and I add some
> explanation in lmb.h to explain why in the struct lmb {} the lmb
> property's should is defined if CONFIG_LMB_MEMORY_REGIONS is NOT defined.
> 
> Fixes: 5e2548c1d6e03 ("lmb: Fix LMB_MEMORY_REGIONS flag usage")
> Reported-by: Mark Millard <marklmi@yahoo.com>
> Signed-off-by: Patrick Delaunay <patrick.delaunay@foss.st.com>
> ---
> 
>   include/lmb.h | 20 +++++++++++++++++++-
>   lib/lmb.c     |  2 +-
>   2 files changed, 20 insertions(+), 2 deletions(-)
> 
> diff --git a/include/lmb.h b/include/lmb.h
> index 7298c2ccc403..f70463ac5440 100644
> --- a/include/lmb.h
> +++ b/include/lmb.h
> @@ -35,6 +35,24 @@ struct lmb_property {
>   	enum lmb_flags flags;
>   };
>   
> +/*
> + * For regions size management, see LMB configuration in KConfig
> + * all the #if test are done with CONFIG_LMB_USE_MAX_REGIONS (boolean)
> + *
> + * case 1. CONFIG_LMB_USE_MAX_REGIONS is defined (legacy mode)
> + *         => CONFIG_LMB_MAX_REGIONS is used to configure the region size,
> + *         direclty in the array lmb_region.region[], with the same
> + *         configuration for memory reion and reseserved region.
> + *
> + * case 2. CONFIG_LMB_USE_MAX_REGIONS is not defined, the size of each
> + *         region is configurated *independently* with
> + *         => CONFIG_LMB_MEMORY_REGIONS: struct lmb.memory_regions
> + *         => CONFIG_LMB_RESERVED_REGIONS: struct lmb.reserved_regions
> + *         lmb_region.region is only a pointer to the correct buffer,
> + *         initialized in lmb_init(). This configuration is useful to manage
> + *         more reserved memory regions with CONFIG_LMB_RESERVED_REGIONS.
> + */
> +
>   /**
>    * struct lmb_region - Description of a set of region.
>    *
> @@ -68,7 +86,7 @@ struct lmb_region {
>   struct lmb {
>   	struct lmb_region memory;
>   	struct lmb_region reserved;
> -#ifdef CONFIG_LMB_MEMORY_REGIONS
> +#if !IS_ENABLED(CONFIG_LMB_USE_MAX_REGIONS)
>   	struct lmb_property memory_regions[CONFIG_LMB_MEMORY_REGIONS];
>   	struct lmb_property reserved_regions[CONFIG_LMB_RESERVED_REGIONS];
>   #endif
> diff --git a/lib/lmb.c b/lib/lmb.c
> index 2444b2a62121..8fbe453dfa9d 100644
> --- a/lib/lmb.c
> +++ b/lib/lmb.c
> @@ -110,7 +110,7 @@ void lmb_init(struct lmb *lmb)
>   #if IS_ENABLED(CONFIG_LMB_USE_MAX_REGIONS)
>   	lmb->memory.max = CONFIG_LMB_MAX_REGIONS;
>   	lmb->reserved.max = CONFIG_LMB_MAX_REGIONS;
> -#elif defined(CONFIG_LMB_MEMORY_REGIONS)
> +#else
>   	lmb->memory.max = CONFIG_LMB_MEMORY_REGIONS;
>   	lmb->reserved.max = CONFIG_LMB_RESERVED_REGIONS;
>   	lmb->memory.region = lmb->memory_regions;

I just build this patch for our platforms and got this.

M


02: lmb: Fix LMB_MEMORY_REGIONS flag usage
    aarch64:  +   xilinx_versal_mini xilinx_versal_mini_emmc0 
xilinx_versal_mini_emmc1 xilinx_versal_mini_ospi xilinx_versal_mini_qspi 
xilinx_versal_net_mini xilinx_zynqmp_mini xilinx_zynqmp_mini_emmc0 
xilinx_zynqmp_mini_emmc1 xilinx_zynqmp_mini_nand xilinx_zynqmp_mini_nand_single 
xilinx_zynqmp_mini_qspi
        arm:  +   zynq_cse_nand zynq_cse_nor zynq_cse_qspi
+In file included from ../include/image.h:34:0,
+                 from ../include/bootm.h:10,
+                 from ../arch/arm/cpu/armv8/exception_level.c:12:
+../include/lmb.h:90:37: error: ‘CONFIG_LMB_MEMORY_REGIONS’ undeclared here (not 
in a function)
+  struct lmb_property memory_regions[CONFIG_LMB_MEMORY_REGIONS];
+                                     ^~~~~~~~~~~~~~~~~~~~~~~~~
+../include/lmb.h:91:39: error: ‘CONFIG_LMB_RESERVED_REGIONS’ undeclared here 
(not in a function)
+  struct lmb_property reserved_regions[CONFIG_LMB_RESERVED_REGIONS];
+                                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~
+make[2]: *** [../scripts/Makefile.build:257: 
arch/arm/cpu/armv8/exception_level.o] Error 1
+make[1]: *** [Makefile:1847: arch/arm/cpu/armv8] Error 2
+make: *** [Makefile:177: sub-make] Error 2
+In file included from ../arch/arm/lib/stack.c:15:0:
+make[2]: *** [../scripts/Makefile.build:257: arch/arm/lib/stack.o] Error 1
+make[1]: *** [Makefile:1847: arch/arm/lib] Error 2
diff mbox series

Patch

diff --git a/include/lmb.h b/include/lmb.h
index 7298c2ccc403..f70463ac5440 100644
--- a/include/lmb.h
+++ b/include/lmb.h
@@ -35,6 +35,24 @@  struct lmb_property {
 	enum lmb_flags flags;
 };
 
+/*
+ * For regions size management, see LMB configuration in KConfig
+ * all the #if test are done with CONFIG_LMB_USE_MAX_REGIONS (boolean)
+ *
+ * case 1. CONFIG_LMB_USE_MAX_REGIONS is defined (legacy mode)
+ *         => CONFIG_LMB_MAX_REGIONS is used to configure the region size,
+ *         direclty in the array lmb_region.region[], with the same
+ *         configuration for memory reion and reseserved region.
+ *
+ * case 2. CONFIG_LMB_USE_MAX_REGIONS is not defined, the size of each
+ *         region is configurated *independently* with
+ *         => CONFIG_LMB_MEMORY_REGIONS: struct lmb.memory_regions
+ *         => CONFIG_LMB_RESERVED_REGIONS: struct lmb.reserved_regions
+ *         lmb_region.region is only a pointer to the correct buffer,
+ *         initialized in lmb_init(). This configuration is useful to manage
+ *         more reserved memory regions with CONFIG_LMB_RESERVED_REGIONS.
+ */
+
 /**
  * struct lmb_region - Description of a set of region.
  *
@@ -68,7 +86,7 @@  struct lmb_region {
 struct lmb {
 	struct lmb_region memory;
 	struct lmb_region reserved;
-#ifdef CONFIG_LMB_MEMORY_REGIONS
+#if !IS_ENABLED(CONFIG_LMB_USE_MAX_REGIONS)
 	struct lmb_property memory_regions[CONFIG_LMB_MEMORY_REGIONS];
 	struct lmb_property reserved_regions[CONFIG_LMB_RESERVED_REGIONS];
 #endif
diff --git a/lib/lmb.c b/lib/lmb.c
index 2444b2a62121..8fbe453dfa9d 100644
--- a/lib/lmb.c
+++ b/lib/lmb.c
@@ -110,7 +110,7 @@  void lmb_init(struct lmb *lmb)
 #if IS_ENABLED(CONFIG_LMB_USE_MAX_REGIONS)
 	lmb->memory.max = CONFIG_LMB_MAX_REGIONS;
 	lmb->reserved.max = CONFIG_LMB_MAX_REGIONS;
-#elif defined(CONFIG_LMB_MEMORY_REGIONS)
+#else
 	lmb->memory.max = CONFIG_LMB_MEMORY_REGIONS;
 	lmb->reserved.max = CONFIG_LMB_RESERVED_REGIONS;
 	lmb->memory.region = lmb->memory_regions;