[PATCH v2] env: mmc: fix offsets relative to the end of the partition

[PATCH v2] env: mmc: fix offsets relative to the end of the partition

[Michael Walle]

According to the help text, you can set negative offsets to indicated
that the offset is relative to the end of the parition. But kconfig
doesn't let you specify negative hex values. I think this fell through
the cracks when converting the symbol from a '#define' to a kconfig
option.

Introduce a new boolean kconfig option to switch on the "relative to the
end" behavior.

Signed-off-by: Michael Walle <mwalle@kernel.org>
---
v2:
 - made the Kconfig help text clearer on MMC hardware partitions.
   I've used the term "MMC hardware partition" as it was already
   used in some help texts. Thanks Quentin.
 - fixed typo. Thanks Quentin.

 env/Kconfig | 42 +++++++++++++++++++++++++-----------------
 env/mmc.c   |  8 ++++++++
 2 files changed, 33 insertions(+), 17 deletions(-)

diff --git a/env/Kconfig b/env/Kconfig
index d7a7e81144b..58500becbb7 100644
--- a/env/Kconfig
+++ b/env/Kconfig
@@ -231,14 +231,6 @@ config ENV_IS_IN_MMC
 	  These two #defines specify the offset and size of the environment
 	  area within the specified MMC device.
 
-	  If offset is positive (the usual case), it is treated as relative to
-	  the start of the MMC partition. If offset is negative, it is treated
-	  as relative to the end of the MMC partition. This can be useful if
-	  your board may be fitted with different MMC devices, which have
-	  different sizes for the MMC partitions, and you always want the
-	  environment placed at the very end of the partition, to leave the
-	  maximum possible space before it, to store other data.
-
 	  These two values are in units of bytes, but must be aligned to an
 	  MMC sector boundary.
 
@@ -249,9 +241,6 @@ config ENV_IS_IN_MMC
 	  valid backup copy in case the other copy is corrupted, e.g. due
 	  to a power failure during a "saveenv" operation.
 
-	  This value may also be positive or negative; this is handled in the
-	  same way as CONFIG_ENV_OFFSET.
-
 	  In case CONFIG_SYS_MMC_ENV_PART is 1 (i.e. environment in eMMC boot
 	  partition) then setting CONFIG_ENV_OFFSET_REDUND to the same value
 	  as CONFIG_ENV_OFFSET makes use of the second eMMC boot partition for
@@ -611,9 +600,18 @@ config ENV_OFFSET
 	  Offset from the start of the device (or partition).
 
 	  This offset may be interpreted differently depending on the chosen
-	  ENV_IS_IN_* options. For example, for ENV_IS_IN_MMC=y, this offset may
-	  be negative to indicate an offset backwards from the end of the
-	  partition. See the relevant help messages for more details.
+	  ENV_IS_IN_* options. See the relevant help messages for more details.
+
+config ENV_OFFSET_RELATIVE_END
+	bool "Offset is relative to the end of the partition"
+	depends on ENV_IS_IN_MMC
+	help
+	  Treat the environment offset as relative to the end of the MMC
+	  hardware partition. This can be useful if your board may be fitted
+	  with different MMC devices, which have different sizes for the MMC
+	  hardware partitions, and you always want the environment placed at the
+	  very end of the partition, to leave the maximum possible space before
+	  it, to store other data.
 
 config ENV_OFFSET_REDUND
 	hex "Redundant environment offset"
@@ -626,9 +624,19 @@ config ENV_OFFSET_REDUND
 	  environment location.
 
 	  This offset may be interpreted differently depending on the chosen
-	  ENV_IS_IN_* options. For example, for ENV_IS_IN_MMC=y, this offset may
-	  be negative to indicate an offset backwards from the end of the
-	  partition. See the relevant help messages for more details.
+	  ENV_IS_IN_* options. See the relevant help messages for more details.
+
+config ENV_OFFSET_REDUND_RELATIVE_END
+	bool "Offset is relative to the end of the partition"
+	depends on SYS_REDUNDAND_ENVIRONMENT
+	depends on ENV_IS_IN_MMC
+	help
+	  Treat the redundant environment offset as relative to the end of the
+	  MMC hardware partition. This can be useful if your board may be
+	  fitted with different MMC devices, which have different sizes for the
+	  MMC hardware partitions, and you always want the environment placed at
+	  the very end of the partition, to leave the maximum possible space
+	  before it, to store other data.
 
 config ENV_SIZE
 	hex "Environment Size"
diff --git a/env/mmc.c b/env/mmc.c
index 8848371eb4f..8211470a08f 100644
--- a/env/mmc.c
+++ b/env/mmc.c
@@ -29,11 +29,19 @@
 
 #else
 /* Default ENV offset when not defined in Device Tree */
+#if !defined(CONFIG_ENV_OFFSET_RELATIVE_END)
 #define ENV_MMC_OFFSET		CONFIG_ENV_OFFSET
+#else
+#define ENV_MMC_OFFSET		(-(CONFIG_ENV_OFFSET))
+#endif
 
 #if defined(CONFIG_ENV_OFFSET_REDUND)
+#if !defined(CONFIG_ENV_OFFSET_REDUND_RELATIVE_END)
 #define ENV_MMC_OFFSET_REDUND	CONFIG_ENV_OFFSET_REDUND
 #else
+#define ENV_MMC_OFFSET_REDUND	(-(CONFIG_ENV_OFFSET_REDUND))
+#endif
+#else
 #define ENV_MMC_OFFSET_REDUND	ENV_MMC_INVALID_OFFSET
 #endif
 #endif
-- 
2.39.5

Re: [PATCH v2] env: mmc: fix offsets relative to the end of the partition

[Quentin Schulz]

Hi Michael,

On 6/5/25 9:46 AM, Michael Walle wrote:
> According to the help text, you can set negative offsets to indicated

s/indicated/indicate/

> that the offset is relative to the end of the parition. But kconfig

s/parition/partition/

> doesn't let you specify negative hex values. I think this fell through
> the cracks when converting the symbol from a '#define' to a kconfig
> option.
> 

Maybe a Fixes: tag would be nice there, but since we don't have stable 
releases like the kernel, not sure it matters much :)

> Introduce a new boolean kconfig option to switch on the "relative to the
> end" behavior.
> 
> Signed-off-by: Michael Walle <mwalle@kernel.org>

Not sure about the ramifications, but the change looks reasonable to me, so:

Reviewed-by: Quentin Schulz <quentin.schulz@cherry.de>

Maybe we should have some test for that to make sure it works as 
expected and keeps working as expected?

Thanks!
Quentin

Re: [PATCH v2] env: mmc: fix offsets relative to the end of the partition

[Andrew Goodbody]

On 06/06/2025 10:45, Quentin Schulz wrote:
> Hi Michael,
> 
> On 6/5/25 9:46 AM, Michael Walle wrote:
>> According to the help text, you can set negative offsets to indicated
> 
> s/indicated/indicate/
> 
>> that the offset is relative to the end of the parition. But kconfig
> 
> s/parition/partition/
> 
>> doesn't let you specify negative hex values. I think this fell through
>> the cracks when converting the symbol from a '#define' to a kconfig
>> option.
>>
> 
> Maybe a Fixes: tag would be nice there, but since we don't have stable 
> releases like the kernel, not sure it matters much :)

Please always include a Fixes tag when appropriate.

Thank you,
Andrew

>> Introduce a new boolean kconfig option to switch on the "relative to the
>> end" behavior.
>>
>> Signed-off-by: Michael Walle <mwalle@kernel.org>
> 
> Not sure about the ramifications, but the change looks reasonable to me, 
> so:
> 
> Reviewed-by: Quentin Schulz <quentin.schulz@cherry.de>
> 
> Maybe we should have some test for that to make sure it works as 
> expected and keeps working as expected?
> 
> Thanks!
> Quentin

Re: [PATCH v2] env: mmc: fix offsets relative to the end of the partition

[Michael Walle]

Hi Andrew,

>> Maybe a Fixes: tag would be nice there, but since we don't have stable
> releases like the kernel, not sure it matters much :)
> 
> Please always include a Fixes tag when appropriate.

Will do. But please use reply-to-all as not everyone is subscribed to
all the MLs and I've just found your reply by accident.

-michael

Re: [PATCH v2] env: mmc: fix offsets relative to the end of the partition

[Javier Viguera]

On 5/6/25 9:46, Michael Walle wrote:
> According to the help text, you can set negative offsets to indicated
> that the offset is relative to the end of the parition. But kconfig
> doesn't let you specify negative hex values. I think this fell through
> the cracks when converting the symbol from a '#define' to a kconfig
> option.

Not sure with u-boot upstream master, but with a downstream NXP-based 
tree (version v2024.04) I think it just works with something like:

ccimx91-dvk_defconfig:CONFIG_ENV_OFFSET=0xFFFFFFFFFFFFC000
ccimx91-dvk_defconfig:CONFIG_ENV_OFFSET_REDUND=0xFFFFFFFFFFFFC000
ccimx93-dvk_defconfig:CONFIG_ENV_OFFSET=0xFFFFFFFFFFFFC000
ccimx93-dvk_defconfig:CONFIG_ENV_OFFSET_REDUND=0xFFFFFFFFFFFFC000

After all, the symbol gets translated to a signed s64 variable.

--
Javier Viguera
Digi International Inc.

Re: [PATCH v2] env: mmc: fix offsets relative to the end of the partition

[Michael Walle]

Hi Javier,

>> According to the help text, you can set negative offsets to indicated
>> that the offset is relative to the end of the parition. But kconfig
>> doesn't let you specify negative hex values. I think this fell through
>> the cracks when converting the symbol from a '#define' to a kconfig
>> option.
> 
> Not sure with u-boot upstream master, but with a downstream NXP-based 
> tree (version v2024.04) I think it just works with something like:
> 
> ccimx91-dvk_defconfig:CONFIG_ENV_OFFSET=0xFFFFFFFFFFFFC000
> ccimx91-dvk_defconfig:CONFIG_ENV_OFFSET_REDUND=0xFFFFFFFFFFFFC000
> ccimx93-dvk_defconfig:CONFIG_ENV_OFFSET=0xFFFFFFFFFFFFC000
> ccimx93-dvk_defconfig:CONFIG_ENV_OFFSET_REDUND=0xFFFFFFFFFFFFC000

Yeah, but I wouldn't call that very user friendly if you have to
convert the offset the two's complement representation yourself.

> After all, the symbol gets translated to a signed s64 variable.

Also I've just noticed, that env/sf.c uses ulong to store ENV_OFFSET.

-michael

Re: [PATCH v2] env: mmc: fix offsets relative to the end of the partition

[Peng Fan]

On Thu, Jun 05, 2025 at 09:46:10AM +0200, Michael Walle wrote:
>According to the help text, you can set negative offsets to indicated
>that the offset is relative to the end of the parition. But kconfig
>doesn't let you specify negative hex values. I think this fell through
>the cracks when converting the symbol from a '#define' to a kconfig
>option.
>
>Introduce a new boolean kconfig option to switch on the "relative to the
>end" behavior.
>
>Signed-off-by: Michael Walle <mwalle@kernel.org>

Applied to mmc-next after resolved conflicts. Sorry for late.

Thanks,
Peng