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 |
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 --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;
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(-)