diff mbox series

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

Message ID 20210801211245.2208037-3-sjg@chromium.org
State Superseded
Delegated to: Jagannadha Sutradharudu Teki
Headers show
Series sf: Add documentation and an 'sf mmap' command | expand

Commit Message

Simon Glass Aug. 1, 2021, 9:12 p.m. UTC
Update this code to use IS_ENABLED() instead.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 cmd/sf.c | 32 ++++++++++++++------------------
 1 file changed, 14 insertions(+), 18 deletions(-)

Comments

Pratyush Yadav Aug. 2, 2021, 4:28 p.m. UTC | #1
On 01/08/21 03:12PM, Simon Glass wrote:
> Update this code to use IS_ENABLED() instead.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> 
> (no changes since v1)
> 
>  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] = {

Unrelated change. Dunno if it is worth its own separate patch though. Up 
to you.

>  	"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 */
>  
>  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"))

Ok.

>  		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

Dunno if this is better or worse than before. Should be fine either way 
I guess

Other than the comment above,

Reviewed-by: Pratyush Yadav <p.yadav@ti.com>

> +#endif /* CONFIG_SYS_LONGHELP */
> +	;
> +
> +U_BOOT_CMD(
> +	sf,	5,	1,	do_spi_flash,
> +	"SPI flash sub-system", long_help
>  );
> -- 
> 2.32.0.554.ge1b32706d8-goog
>
diff mbox series

Patch

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 */
 
 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
 );