[U-Boot] [PATCH 5/6] cmd_nvedit.c: allow board-specific code before/after saving the environment

[Timur Tabi]

Wolfgang Denk wrote:

> I think whoever told you this was right.  Let it break.

Come on, Wolfgang.  That's not acceptable.

> We cannot add pre- and post-hooks all ever the place for brain-dead
> designs that need to do this and that before and after doing perfectly
> things.

Well, I already have code in U-boot that does this.  If you look at
board/freescale/p1022ds/diu.c, you'll see that I override each of the NOR
flash accessors.  This is horribly inefficient, but it works.
Unfortunately, it only covers NOR flash.  The new design covers NOR and NAND.

The last two patches of this patchset are a vast improvement, but they
require a board hook (and using Mike's idea, only one hook is necessary,
not two).

As for 'all over the place", I think it's unfair to say my one board hook
function is going to result in "all over the place" hacks.

> It makes no sense adding this to saveenv, because there will be othe
> rplaces in the code that need to to the same - like if it's NAND
> flash, you will probabaly need to do the same for all NAND related
> commands.

Actually, the same code works for saving the environment to NAND flash.
This is how the board code will look:

/*
 * While the DIU is active, the localbus is not available.  Therefore, in
 * order to support the saveenv command to localbus devices, we need to
 * temporarily disable the DIU and enable the localbus.  To do this, we
 * provide our own implementation of board_saveenv(). This function is called
 * by do_env_save().
 */
#if defined(CONFIG_ENV_IS_IN_NAND) || defined(CONFIG_ENV_IS_IN_FLASH)
int board_saveenv(void)
{
	int switched, ret;

	switched = set_mux_to_lbc();

	ret = saveenv();

	if (switched)
		set_mux_to_diu();

	return ret;
}
#endif


> cmd_nvedit.c is definitely the wrong place for this.

If you have a better idea, then I'm all ears.  I could implement my own
version of saveenv(), but the current design of U-boot does not allow me
to have two functions called saveenv().

-- 
Timur Tabi
Linux kernel developer at Freescale