[PATCH 2/4] sf: Tidy up code to avoid #ifdef

[Pratyush Yadav]

On 06/04/21 01:32PM, Heinrich Schuchardt wrote:
> On 06.04.21 12:22, Pratyush Yadav wrote:
> > On 06/04/21 04:30PM, Simon Glass wrote:
> >> Update this code to use IS_ENABLED() instead.
> >>
> >> Signed-off-by: Simon Glass <sjg at chromium.org>
> >> ---
> >>
> >>  cmd/sf.c | 32 ++++++++++++++------------------
> >>  1 file changed, 14 insertions(+), 18 deletions(-)
> >>
> >> diff --git a/cmd/sf.c b/cmd/sf.c
> >> index 46346fb9d43..d4f5ecee38c 100644
> >> --- a/cmd/sf.c
> >> +++ b/cmd/sf.c
> >> @@ -384,7 +384,6 @@ static int do_spi_protect(int argc, char *const argv[])
> >>  	return ret == 0 ? 0 : 1;
> >>  }
> >>
> >> -#ifdef CONFIG_CMD_SF_TEST
> >>  enum {
> >>  	STAGE_ERASE,
> >>  	STAGE_CHECK,
> >> @@ -394,7 +393,7 @@ enum {
> >>  	STAGE_COUNT,
> >>  };
> >>
> >> -static char *stage_name[STAGE_COUNT] = {
> >> +static const char *stage_name[STAGE_COUNT] = {
> >>  	"erase",
> >>  	"check",
> >>  	"write",
> >> @@ -548,7 +547,6 @@ static int do_spi_flash_test(int argc, char *const argv[])
> >>
> >>  	return 0;
> >>  }
> >> -#endif /* CONFIG_CMD_SF_TEST */
> >
> > Won't this cause all the test related functions to always be compiled
> > into the binary? Then what's the point of having it behind a config at
> > all?
> 
> We compile all functions into separate sections. The linker eliminates
> unused functions. Look for -ffunction-sections in files
> arch/<arch>/config.mk.

Ah, clever! This would rely on the compiler eliminating the dead if 
branch but the build uses -O2 or -Os, both of which should eliminate 
dead code.

> 
> Best regards
> 
> Heinrich
> 
> >
> >>
> >>  static int do_spi_flash(struct cmd_tbl *cmdtp, int flag, int argc,
> >>  			char *const argv[])
> >> @@ -582,10 +580,8 @@ static int do_spi_flash(struct cmd_tbl *cmdtp, int flag, int argc,
> >>  		ret = do_spi_flash_erase(argc, argv);
> >>  	else if (strcmp(cmd, "protect") == 0)
> >>  		ret = do_spi_protect(argc, argv);
> >> -#ifdef CONFIG_CMD_SF_TEST
> >> -	else if (!strcmp(cmd, "test"))
> >> +	else if (IS_ENABLED(CONFIG_CMD_SF_TEST) && !strcmp(cmd, "test"))
> >>  		ret = do_spi_flash_test(argc, argv);
> >> -#endif
> >>  	else
> >>  		ret = -1;
> >>
> >> @@ -597,16 +593,8 @@ usage:
> >>  	return CMD_RET_USAGE;
> >>  }
> >>
> >> -#ifdef CONFIG_CMD_SF_TEST
> >> -#define SF_TEST_HELP "\nsf test offset len		" \
> >> -		"- run a very basic destructive test"
> >> -#else
> >> -#define SF_TEST_HELP
> >> -#endif
> >> -
> >> -U_BOOT_CMD(
> >> -	sf,	5,	1,	do_spi_flash,
> >> -	"SPI flash sub-system",
> >> +#ifdef CONFIG_SYS_LONGHELP
> >> +static const char long_help[] =
> >>  	"probe [[bus:]cs] [hz] [mode]	- init flash device on given SPI bus\n"
> >>  	"				  and chip select\n"
> >>  	"sf read addr offset|partition len	- read `len' bytes starting at\n"
> >> @@ -622,6 +610,14 @@ U_BOOT_CMD(
> >>  	"					  at `addr' to flash at `offset'\n"
> >>  	"					  or to start of mtd `partition'\n"
> >>  	"sf protect lock/unlock sector len	- protect/unprotect 'len' bytes starting\n"
> >> -	"					  at address 'sector'\n"
> >> -	SF_TEST_HELP
> >> +	"					  at address 'sector'"
> >> +#ifdef CONFIG_CMD_SF_TEST
> >> +	"\nsf test offset len		- run a very basic destructive test"
> >> +#endif
> >> +#endif /* CONFIG_SYS_LONGHELP */
> >> +	;
> >> +
> >> +U_BOOT_CMD(
> >> +	sf,	5,	1,	do_spi_flash,
> >> +	"SPI flash sub-system", long_help
> >>  );
> >> --
> >> 2.31.0.208.g409f899ff0-goog
> >>
> >
> 

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.