diff mbox series

[v4,3/5] sf: Tidy up code to avoid #ifdef

Message ID 20210919214937.3829422-4-sjg@chromium.org
State Accepted
Commit 3512e1570d4376a4581575546cee7b5d3af54370
Delegated to: Simon Glass
Headers show
Series sf: Add documentation and an 'sf mmap' command | expand

Commit Message

Simon Glass Sept. 19, 2021, 9:49 p.m. UTC 
Update this code to use IS_ENABLED() instead.

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

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

(no changes since v1)

 cmd/sf.c | 30 +++++++++++++-----------------
 1 file changed, 13 insertions(+), 17 deletions(-)

Comments

Pratyush Yadav Sept. 20, 2021, 11:08 a.m. UTC | #1 
Hi Simon,

On 19/09/21 03:49PM, Simon Glass wrote:
> Update this code to use IS_ENABLED() instead.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> 
> Reviewed-by: Pratyush Yadav <p.yadav@ti.com>

Nitpick: Trailers shouldn't have a blank line between them. I see it for 
this patch and 4/5 as well. It probably doesn't matter, but I wonder if 
it will trip up some tools that work on commit trailers like 
git-interpret-trailers. Something you might want to fix in your 
workflow...
Simon Glass Sept. 24, 2021, 2:48 a.m. UTC | #2 
Hi Pratyush,

On Mon, 20 Sept 2021 at 05:08, Pratyush Yadav <p.yadav@ti.com> wrote:
>
> Hi Simon,
>
> On 19/09/21 03:49PM, Simon Glass wrote:
> > Update this code to use IS_ENABLED() instead.
> >
> > Signed-off-by: Simon Glass <sjg@chromium.org>
> >
> > Reviewed-by: Pratyush Yadav <p.yadav@ti.com>
>
> Nitpick: Trailers shouldn't have a blank line between them. I see it for
> this patch and 4/5 as well. It probably doesn't matter, but I wonder if
> it will trip up some tools that work on commit trailers like
> git-interpret-trailers. Something you might want to fix in your
> workflow...

The fix to 'patman status -d <new_branch>' is in mainline but was not
in the tree where I ran this tool, unfortunately.

So hopefully this won't happen again.

Regards,
Simon
Jagan Teki Oct. 8, 2021, 12:41 p.m. UTC | #3 
On Mon, Sep 20, 2021 at 3:19 AM Simon Glass <sjg@chromium.org> wrote:
>
> Update this code to use IS_ENABLED() instead.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
>
> Reviewed-by: Pratyush Yadav <p.yadav@ti.com>
> ---
>

Reviewed-by: Jagan Teki <jagan@amarulasolutions.com>
Simon Glass Nov. 24, 2021, 5:47 p.m. UTC | #4 
On Mon, Sep 20, 2021 at 3:19 AM Simon Glass <sjg@chromium.org> wrote:
>
> Update this code to use IS_ENABLED() instead.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
>
> Reviewed-by: Pratyush Yadav <p.yadav@ti.com>
> ---
>

Reviewed-by: Jagan Teki <jagan@amarulasolutions.com>

Applied to u-boot-dm, thanks!
Simon Glass Nov. 24, 2021, 5:47 p.m. UTC | #5 
On Mon, Sep 20, 2021 at 3:19 AM Simon Glass <sjg@chromium.org> wrote:
>
> Update this code to use IS_ENABLED() instead.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
>
> Reviewed-by: Pratyush Yadav <p.yadav@ti.com>
> ---
>

Reviewed-by: Jagan Teki <jagan@amarulasolutions.com>

Applied to u-boot-dm, thanks!
Applied to u-boot-dm/next, thanks!
diff mbox series

Patch

diff --git a/cmd/sf.c b/cmd/sf.c
index 15361a4bddf..72246d912fe 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,
@@ -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
 );