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

["Michael Walle" <mwalle@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 5055 bytes --]

Hi,

> > 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>
> > ---
> >   env/Kconfig | 42 +++++++++++++++++++++++++-----------------
> >   env/mmc.c   |  8 ++++++++
> >   2 files changed, 33 insertions(+), 17 deletions(-)
> > 
> > diff --git a/env/Kconfig b/env/Kconfig
> > index 4438f0b392c..49888bf700c 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
> > @@ -592,9 +581,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
> > +	  partition. This can be useful if your board may be fitted with
>
> I guess by MMC partition you meant hardware MMC partitions, as opposed 
> to GPT/MBR partitions?

Yes exactly.

> It's not clear from JEDEC standard if there's a word that would apply to 
> the boot partitions, User Area (I think it can be split also?), RPMB, 
> etc.? But if there's one, maybe use that term?

I didn't come up with that, as I just moved the help text. But I can
certainly add "(hardware) MMC partition".

>
> > +	  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.
> >   
> >   config ENV_OFFSET_REDUND
> >   	hex "Redundant environment offset"
> > @@ -607,9 +605,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 redundnat environment offset as relative to the end of the
>
> s/redundnat/redundant/
>
> > +	  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.
> >   
>
> Same remark as for ENV_OFFSET_RELATIVE_END.
>
> Looks good to me otherwise!

Thanks for reviewing.

Let's give this patch a couple more day, then I'll resend it with
your remarks being addressed.

-michael

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 297 bytes --]