[PATCH v2 1/2] lmb: Fix LMB_MEMORY_REGIONS flag usage

[Michal Simek]

On 3/22/23 19:12, 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 is 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 is defined if CONFIG_LMB_MEMORY_REGIONS is NOT defined.
> 
> This patch also removes CONFIG_LMB_XXX dependency on CONFIG_LMB as these
> defines are used in API file lmb.h and not only in library file.
> 
> Fixes: 5e2548c1d6e03 ("lmb: Fix LMB_MEMORY_REGIONS flag usage")
> Reported-by: Mark Millard <marklmi at yahoo.com>
> Signed-off-by: Patrick Delaunay <patrick.delaunay at foss.st.com>
> ---
> 
> Changes in v2:
> - Remove CONFIG_LMB_XXX dependency on CONFIG_LMB as these defines are
>    used in lmb.h file, include by default to export the LMB API
>    and not only in LMB libary code. This modification is required to
>    avoid issue in API definition when CONFIG_LMB is not set.
> - Fix some typo in commit message and in comment
> 
>   include/lmb.h | 20 +++++++++++++++++++-
>   lib/Kconfig   |  7 +++----
>   lib/lmb.c     |  2 +-
>   3 files changed, 23 insertions(+), 6 deletions(-)
> 
> diff --git a/include/lmb.h b/include/lmb.h
> index 7298c2ccc403..07bf22144eac 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,
> + *         directly in the array lmb_region.region[], with the same
> + *         configuration for memory and reserved regions.
> + *
> + * 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/Kconfig b/lib/Kconfig
> index 83e5edd73b0e..da6c7cd5f628 100644
> --- a/lib/Kconfig
> +++ b/lib/Kconfig
> @@ -1027,7 +1027,6 @@ config LMB
>   
>   config LMB_USE_MAX_REGIONS
>   	bool "Use a common number of memory and reserved regions in lmb lib"
> -	depends on LMB
>   	default y
>   	help
>   	  Define the number of supported memory regions in the library logical
> @@ -1037,7 +1036,7 @@ config LMB_USE_MAX_REGIONS
>   
>   config LMB_MAX_REGIONS
>   	int "Number of memory and reserved regions in lmb lib"
> -	depends on LMB && LMB_USE_MAX_REGIONS
> +	depends on LMB_USE_MAX_REGIONS
>   	default 16
>   	help
>   	  Define the number of supported regions, memory and reserved, in the
> @@ -1045,7 +1044,7 @@ config LMB_MAX_REGIONS
>   
>   config LMB_MEMORY_REGIONS
>   	int "Number of memory regions in lmb lib"
> -	depends on LMB && !LMB_USE_MAX_REGIONS
> +	depends on !LMB_USE_MAX_REGIONS
>   	default 8
>   	help
>   	  Define the number of supported memory regions in the library logical
> @@ -1054,7 +1053,7 @@ config LMB_MEMORY_REGIONS
>   
>   config LMB_RESERVED_REGIONS
>   	int "Number of reserved regions in lmb lib"
> -	depends on LMB && !LMB_USE_MAX_REGIONS
> +	depends on !LMB_USE_MAX_REGIONS
>   	default 8
>   	help
>   	  Define the number of supported reserved regions in the library logical
> 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;

Acked-by: Michal Simek <michal.simek at amd.com>

Thanks,
Michal