On 2/24/25 11:01 AM, Quentin Schulz wrote:
Hi Marek,

Hi,

On 2/21/25 7:47 PM, Marek Vasut wrote:
Introduce a new function mmc_env_is_redundant_in_both_boot_hwparts()
which replaces IS_ENABLED(ENV_MMC_HWPART_REDUND) and internally does
almost the same check as the macro which assigned ENV_MMC_HWPART_REDUND
did, and call it in place of IS_ENABLED(ENV_MMC_HWPART_REDUND).

The difference compared to IS_ENABLED(ENV_MMC_HWPART_REDUND) is
in the last conditional, which does not do plain macro compare
(CONFIG_ENV_OFFSET == CONFIG_ENV_OFFSET_REDUND), but instead does
mmc_offset(mmc, 0) == mmc_offset(mmc, 1). If OF_CONTROL is not
in use, this gets optimized back to original macro compare, but
if OF_CONTROL is in use, this also takes into account the DT
properties u-boot,mmc-env-offset and u-boot,mmc-env-offset-redundant.

Signed-off-by: Marek Vasut <ma...@denx.de>
---
Cc: Dragan Simic <dsi...@manjaro.org>
Cc: Joe Hershberger <joe.hershber...@ni.com>
Cc: Mattijs Korpershoek <mkorpersh...@baylibre.com>
Cc: Quentin Schulz <quentin.sch...@cherry.de>
Cc: Rasmus Villemoes <rasmus.villem...@prevas.dk>
Cc: Simon Glass <s...@chromium.org>
Cc: Tom Rini <tr...@konsulko.com>
Cc: u-boot@lists.denx.de
---
V2: - Rename mmc_env_hwpart_redund() to mmc_env_is_redundant_in_both_boot_hwparts()
     - Return bool
V3: - Hide the mmc_env_is_redundant_in_both_boot_hwparts symbol behind __maybe_unused        as this symbol is called from code gated behind ifdef CONFIG_SYS_REDUNDAND_ENVIRONMENT
       in env_mmc_load()
---
  env/mmc.c | 37 +++++++++++++++++++++----------------
  1 file changed, 21 insertions(+), 16 deletions(-)

diff --git a/env/mmc.c b/env/mmc.c
index 379f5ec9be7..353a7ce72fb 100644
--- a/env/mmc.c
+++ b/env/mmc.c
@@ -40,18 +40,6 @@
  DECLARE_GLOBAL_DATA_PTR;
-/*
- * In case the environment is redundant, stored in eMMC hardware boot
- * partition and the environment and redundant environment offsets are
- * identical, store the environment and redundant environment in both
- * eMMC boot partitions, one copy in each.
- * */
-#if (defined(CONFIG_SYS_REDUNDAND_ENVIRONMENT) && \
-     (CONFIG_SYS_MMC_ENV_PART == 1) && \
-     (CONFIG_ENV_OFFSET == CONFIG_ENV_OFFSET_REDUND))
-#define ENV_MMC_HWPART_REDUND    1
-#endif
-
  #if CONFIG_IS_ENABLED(OF_CONTROL)
  static int mmc_env_partition_by_name(struct blk_desc *desc, const char *str, @@ -217,6 +205,23 @@ static inline s64 mmc_offset(struct mmc *mmc, int copy)
  }
  #endif
+static bool __maybe_unused mmc_env_is_redundant_in_both_boot_hwparts(struct mmc *mmc)
+{
+    /*
+     * In case the environment is redundant, stored in eMMC hardware boot +     * partition and the environment and redundant environment offsets are +     * identical, store the environment and redundant environment in both
+     * eMMC boot partitions, one copy in each.
+     */
+    if (!IS_ENABLED(CONFIG_SYS_REDUNDAND_ENVIRONMENT))
+        return false;
+
+    if (CONFIG_SYS_MMC_ENV_PART != 1)
+        return false;
+
+    return mmc_offset(mmc, 0) == mmc_offset(mmc, 1);

This is not always equivalent to the current test of

CONFIG_ENV_OFFSET == CONFIG_ENV_OFFSET_REDUND

Indeed, it only is for when OF_CONTROL isn't set.

This is what the patch fixes and this is what the commit message states.

I would recommend to keep this check in patch 1, then add another patch that swaps CONFIG_ENV_OFFSET == CONFIG_ENV_OFFSET_REDUND for mmc_offset(mmc, 0) == mmc_offset(mmc, 1)

What benefit would that bring ?

This will allow to use DT properties like u-boot,mmc-env-offset- redundant and u-boot,mmc-env-offset, which isn't supported today.

This is what the patch fixes and this is what the commit message states.

[...]

Reply via email to