From mboxrd@z Thu Jan 1 00:00:00 1970 From: Sean Anderson Date: Mon, 1 Mar 2021 18:10:35 -0500 Subject: [PATCH v3 1/1] env: sf: single function env_sf_save() In-Reply-To: <60bb9cc5-1bfb-4957-7c20-7dcf2de28760@men.de> References: <20210202082156.21056-1-Harry.Waschkeit@men.de> <60bb9cc5-1bfb-4957-7c20-7dcf2de28760@men.de> Message-ID: <4857412c-0a63-3c0d-aeda-db3620792c43@gmail.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On 3/1/21 11:39 AM, Harry Waschkeit wrote: > ?Hi again, > > gentle ping for that patch, also in view of subsequently sent patch ... > > https://lists.denx.de/pipermail/u-boot/2021-February/439797.html > > ... which saw a competing patch by Patrick Delaunay a week later: > > https://lists.denx.de/pipermail/u-boot/2021-February/440769.html > > However, the latter doesn't seem to take embedded environments into account. Can you give an example of where your patch would work while Patrick's wouldn't? Thanks. --Sean > > Best regards, > Harry > > > On 02.02.21 09:21, Harry Waschkeit wrote: >> Instead of implementing redundant environments in two very similar >> functions env_sf_save(), handle redundancy in one function, placing the >> few differences in appropriate pre-compiler sections depending on config >> option CONFIG_ENV_OFFSET_REDUND. >> >> Additionally, several checkpatch complaints were addressed. >> >> This patch is in preparation for adding support for env erase. >> >> Signed-off-by: Harry Waschkeit >> Reviewed-by: Stefan Roese >> --- >> Change in v3: >> - no change in patch, only added "reviewed-by" to commit log >> >> Change in v2: >> - remove one more #ifdef, instead take advantage of compiler attribute >> __maybe_unused for one variable used only in case of redundant >> environments >> >> env/sf.c | 130 ++++++++++++++++++------------------------------------- >> 1 file changed, 41 insertions(+), 89 deletions(-) >> >> diff --git a/env/sf.c b/env/sf.c >> index 937778aa37..199114fd3b 100644 >> --- a/env/sf.c >> +++ b/env/sf.c >> @@ -66,13 +66,14 @@ static int setup_flash_device(void) >> return 0; >> } >> -#if defined(CONFIG_ENV_OFFSET_REDUND) >> static int env_sf_save(void) >> { >> env_t env_new; >> - char *saved_buffer = NULL, flag = ENV_REDUND_OBSOLETE; >> + char *saved_buffer = NULL; >> u32 saved_size, saved_offset, sector; >> + ulong offset; >> int ret; >> + __maybe_unused char flag = ENV_REDUND_OBSOLETE; >> ret = setup_flash_device(); >> if (ret) >> @@ -81,27 +82,33 @@ static int env_sf_save(void) >> ret = env_export(&env_new); >> if (ret) >> return -EIO; >> - env_new.flags = ENV_REDUND_ACTIVE; >> - if (gd->env_valid == ENV_VALID) { >> - env_new_offset = CONFIG_ENV_OFFSET_REDUND; >> - env_offset = CONFIG_ENV_OFFSET; >> - } else { >> - env_new_offset = CONFIG_ENV_OFFSET; >> - env_offset = CONFIG_ENV_OFFSET_REDUND; >> - } >> +#if CONFIG_IS_ENABLED(SYS_REDUNDAND_ENVIRONMENT) >> + env_new.flags = ENV_REDUND_ACTIVE; >> + >> + if (gd->env_valid == ENV_VALID) { >> + env_new_offset = CONFIG_ENV_OFFSET_REDUND; >> + env_offset = CONFIG_ENV_OFFSET; >> + } else { >> + env_new_offset = CONFIG_ENV_OFFSET; >> + env_offset = CONFIG_ENV_OFFSET_REDUND; >> + } >> + offset = env_new_offset; >> +#else >> + offset = CONFIG_ENV_OFFSET; >> +#endif >> /* Is the sector larger than the env (i.e. embedded) */ >> if (CONFIG_ENV_SECT_SIZE > CONFIG_ENV_SIZE) { >> saved_size = CONFIG_ENV_SECT_SIZE - CONFIG_ENV_SIZE; >> - saved_offset = env_new_offset + CONFIG_ENV_SIZE; >> + saved_offset = offset + CONFIG_ENV_SIZE; >> saved_buffer = memalign(ARCH_DMA_MINALIGN, saved_size); >> if (!saved_buffer) { >> ret = -ENOMEM; >> goto done; >> } >> - ret = spi_flash_read(env_flash, saved_offset, >> - saved_size, saved_buffer); >> + ret = spi_flash_read(env_flash, saved_offset, saved_size, >> + saved_buffer); >> if (ret) >> goto done; >> } >> @@ -109,35 +116,39 @@ static int env_sf_save(void) >> sector = DIV_ROUND_UP(CONFIG_ENV_SIZE, CONFIG_ENV_SECT_SIZE); >> puts("Erasing SPI flash..."); >> - ret = spi_flash_erase(env_flash, env_new_offset, >> - sector * CONFIG_ENV_SECT_SIZE); >> + ret = spi_flash_erase(env_flash, offset, >> + sector * CONFIG_ENV_SECT_SIZE); >> if (ret) >> goto done; >> puts("Writing to SPI flash..."); >> - ret = spi_flash_write(env_flash, env_new_offset, >> - CONFIG_ENV_SIZE, &env_new); >> + ret = spi_flash_write(env_flash, offset, >> + CONFIG_ENV_SIZE, &env_new); >> if (ret) >> goto done; >> if (CONFIG_ENV_SECT_SIZE > CONFIG_ENV_SIZE) { >> - ret = spi_flash_write(env_flash, saved_offset, >> - saved_size, saved_buffer); >> + ret = spi_flash_write(env_flash, saved_offset, saved_size, >> + saved_buffer); >> if (ret) >> goto done; >> } >> - ret = spi_flash_write(env_flash, env_offset + offsetof(env_t, flags), >> - sizeof(env_new.flags), &flag); >> - if (ret) >> - goto done; >> +#if CONFIG_IS_ENABLED(SYS_REDUNDAND_ENVIRONMENT) >> + ret = spi_flash_write(env_flash, env_offset + offsetof(env_t, flags), >> + sizeof(env_new.flags), &flag); >> + if (ret) >> + goto done; >> - puts("done\n"); >> + puts("done\n"); >> - gd->env_valid = gd->env_valid == ENV_REDUND ? ENV_VALID : ENV_REDUND; >> + gd->env_valid = gd->env_valid == ENV_REDUND ? ENV_VALID : ENV_REDUND; >> - printf("Valid environment: %d\n", (int)gd->env_valid); >> + printf("Valid environment: %d\n", (int)gd->env_valid); >> +#else >> + puts("done\n"); >> +#endif >> done: >> if (saved_buffer) >> @@ -146,6 +157,7 @@ static int env_sf_save(void) >> return ret; >> } >> +#if CONFIG_IS_ENABLED(SYS_REDUNDAND_ENVIRONMENT) >> static int env_sf_load(void) >> { >> int ret; >> @@ -183,66 +195,6 @@ out: >> return ret; >> } >> #else >> -static int env_sf_save(void) >> -{ >> - u32 saved_size, saved_offset, sector; >> - char *saved_buffer = NULL; >> - int ret = 1; >> - env_t env_new; >> - >> - ret = setup_flash_device(); >> - if (ret) >> - return ret; >> - >> - /* Is the sector larger than the env (i.e. embedded) */ >> - if (CONFIG_ENV_SECT_SIZE > CONFIG_ENV_SIZE) { >> - saved_size = CONFIG_ENV_SECT_SIZE - CONFIG_ENV_SIZE; >> - saved_offset = CONFIG_ENV_OFFSET + CONFIG_ENV_SIZE; >> - saved_buffer = malloc(saved_size); >> - if (!saved_buffer) >> - goto done; >> - >> - ret = spi_flash_read(env_flash, saved_offset, >> - saved_size, saved_buffer); >> - if (ret) >> - goto done; >> - } >> - >> - ret = env_export(&env_new); >> - if (ret) >> - goto done; >> - >> - sector = DIV_ROUND_UP(CONFIG_ENV_SIZE, CONFIG_ENV_SECT_SIZE); >> - >> - puts("Erasing SPI flash..."); >> - ret = spi_flash_erase(env_flash, CONFIG_ENV_OFFSET, >> - sector * CONFIG_ENV_SECT_SIZE); >> - if (ret) >> - goto done; >> - >> - puts("Writing to SPI flash..."); >> - ret = spi_flash_write(env_flash, CONFIG_ENV_OFFSET, >> - CONFIG_ENV_SIZE, &env_new); >> - if (ret) >> - goto done; >> - >> - if (CONFIG_ENV_SECT_SIZE > CONFIG_ENV_SIZE) { >> - ret = spi_flash_write(env_flash, saved_offset, >> - saved_size, saved_buffer); >> - if (ret) >> - goto done; >> - } >> - >> - ret = 0; >> - puts("done\n"); >> - >> - done: >> - if (saved_buffer) >> - free(saved_buffer); >> - >> - return ret; >> -} >> - >> static int env_sf_load(void) >> { >> int ret; >> @@ -258,8 +210,8 @@ static int env_sf_load(void) >> if (ret) >> goto out; >> - ret = spi_flash_read(env_flash, >> - CONFIG_ENV_OFFSET, CONFIG_ENV_SIZE, buf); >> + ret = spi_flash_read(env_flash, CONFIG_ENV_OFFSET, CONFIG_ENV_SIZE, >> + buf); >> if (ret) { >> env_set_default("spi_flash_read() failed", 0); >> goto err_read; >> @@ -292,7 +244,7 @@ static int env_sf_init(void) >> env_t *env_ptr = (env_t *)env_sf_get_env_addr(); >> if (crc32(0, env_ptr->data, ENV_SIZE) == env_ptr->crc) { >> - gd->env_addr = (ulong)&(env_ptr->data); >> + gd->env_addr = (ulong)&env_ptr->data; >> gd->env_valid = 1; >> } else { >> gd->env_addr = (ulong)&default_environment[0]; >> > >