Hi

On 3/19/20 10:59 AM, Patrick Delaunay wrote:

The output of the function mmc_offset_try_partition must be a byte offset in mmc and not a multiple of blksz.

This function is used in mmc_offset(), called by mmc_get_env_addr() and the offset is used in write_env(), erase_env() and read_env().

In these function, blk_start = offset / mmc->read_bl_len or /write_bl_len so this offset is not a multiple of blksz.

Signed-off-by: Patrick Delaunay patrick.delaunay@st.com

env/mmc.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/env/mmc.c b/env/mmc.c index 902cca23ad..c24b169f3e 100644 --- a/env/mmc.c +++ b/env/mmc.c @@ -56,10 +56,10 @@ static inline int mmc_offset_try_partition(const char *str, s64 *val) }

/* round up to info.blksz */

• len = (CONFIG_ENV_SIZE + info.blksz - 1) & ~(info.blksz - 1);

• len = DIV_ROUND_UP(CONFIG_ENV_SIZE, info.blksz);

  /* use the top of the partion for the environment */

• *val = (info.start + info.size - 1) - len / info.blksz;

• *val = (info.start + info.size - len) * info.blksz;

  return 0;

}

Reviewed-by: Patrice Chotard patrice.chotard@st.com

Thanks

Hi

On 3/19/20 10:59 AM, Patrick Delaunay wrote:

Manage 2 copy at the end of the partition selected by config "u-boot,mmc-env-partition" to save the U-Boot environment, with CONFIG_ENV_SIZE and 2*CONFIG_ENV_SIZE offset.

This patch allows to support redundancy (CONFIG_ENV_OFFSET_REDUND).

Signed-off-by: Patrick Delaunay patrick.delaunay@st.com

env/mmc.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/env/mmc.c b/env/mmc.c index c24b169f3e..677a3d4668 100644 --- a/env/mmc.c +++ b/env/mmc.c @@ -34,7 +34,7 @@ __weak int mmc_get_env_dev(void) }

#if CONFIG_IS_ENABLED(OF_CONTROL) -static inline int mmc_offset_try_partition(const char *str, s64 *val) +static inline int mmc_offset_try_partition(const char *str, int copy, s64 *val) { disk_partition_t info; struct blk_desc *desc; @@ -59,7 +59,7 @@ static inline int mmc_offset_try_partition(const char *str, s64 *val) len = DIV_ROUND_UP(CONFIG_ENV_SIZE, info.blksz);

/* use the top of the partion for the environment */

• *val = (info.start + info.size - len) * info.blksz;

• *val = (info.start + info.size - (1 + copy) * len) * info.blksz;

  return 0;

} @@ -84,7 +84,7 @@ static inline s64 mmc_offset(int copy) str = fdtdec_get_config_string(gd->fdt_blob, dt_prop.partition); if (str) { /* try to place the environment at end of the partition */

• err = mmc_offset_try_partition(str, &val);
  

• err = mmc_offset_try_partition(str, copy, &val);
  

  if (!err) return val; }

Reviewed-by: Patrice Chotard patrice.chotard@st.com

Thanks

On Tue, Apr 21, 2020 at 10:35:40AM +0000, Peng Fan wrote:

...

Subject: Re: [Uboot-stm32] [PATCH 1/3] env: mmc: allow support of mmc_get_env_dev with OF_CONTROL

Hi Tom

I just noticed that this env series is delegated to Peng Fan (mmc maintainer) instead of Joe Hershberger (env maintainer).

Is there any reason for this or perhaps is it just an error ?

Ah, I not notice the delegation issue. if you are concerned about this. I'll leave this to Joe and drop the patchset from my CI.

I looked over the patch and figured it was clear enough in implementation to go either way, sorry for the confusion.

Thanks, Peng.

...

Thanks

Patrice

On 3/19/20 10:59 AM, Patrick Delaunay wrote:

...

Use the weak function mmc_get_env_dev in mmc_offset_try_partition function to allow dynamic selection of mmc device to use and no more use directly the define CONFIG_SYS_MMC_ENV_DEV.

Signed-off-by: Patrick Delaunay patrick.delaunay@st.com

env/mmc.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/env/mmc.c b/env/mmc.c index 251ad07d7c..902cca23ad 100644 --- a/env/mmc.c +++ b/env/mmc.c @@ -24,14 +24,25 @@

DECLARE_GLOBAL_DATA_PTR;

+#if !defined(CONFIG_SYS_MMC_ENV_DEV) +#define CONFIG_SYS_MMC_ENV_DEV 0 +#endif

+__weak int mmc_get_env_dev(void) +{

• return CONFIG_SYS_MMC_ENV_DEV;

+}

#if CONFIG_IS_ENABLED(OF_CONTROL) static inline int mmc_offset_try_partition(const char *str, s64 *val) { disk_partition_t info; struct blk_desc *desc; int len, i, ret;

• char dev_str[4];

• ret = blk_get_device_by_str("mmc", STR(CONFIG_SYS_MMC_ENV_DEV),

&desc);

...

• snprintf(dev_str, sizeof(dev_str), "%d", mmc_get_env_dev());
• ret = blk_get_device_by_str("mmc", dev_str, &desc); if (ret < 0) return (ret);

@@ -114,11 +125,6 @@ __weak int mmc_get_env_addr(struct mmc

*mmc, int copy, u32 *env_addr)

...

return 0; }

-__weak int mmc_get_env_dev(void) -{

• return CONFIG_SYS_MMC_ENV_DEV;

-}

#ifdef CONFIG_SYS_MMC_ENV_PART __weak uint mmc_get_env_part(struct mmc *mmc){