[U-Boot] [PATCH 1/1] cmd: add exception command
The 'exception' command allows to test exception handling.
This implementation supports ARM, x86, RISC-V and the following exceptions: * 'breakpoint' - prefetch abort exception (ARM 32bit only) * 'unaligned' - data abort exception (ARM only) * 'undefined' - undefined instruction exception
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- Currently RISC-V 64bit gets into an endless loop when hitting the undefined instruction.
So it makes sense to have a testing capability. --- cmd/Kconfig | 6 ++ cmd/Makefile | 1 + cmd/exception.c | 161 ++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 168 insertions(+) create mode 100644 cmd/exception.c
diff --git a/cmd/Kconfig b/cmd/Kconfig index e2973b3c51..9d2b8199b8 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -1388,6 +1388,12 @@ config CMD_DISPLAY displayed on a simple board-specific display. Implement display_putc() to use it.
+config CMD_EXCEPTION + bool "exception - raise exception" + depends on ARM || RISCV || X86 + help + Enable the 'exception' command which allows to raise an exception. + config CMD_LED bool "led" default y if LED diff --git a/cmd/Makefile b/cmd/Makefile index 0534ddc679..cd67a79170 100644 --- a/cmd/Makefile +++ b/cmd/Makefile @@ -46,6 +46,7 @@ endif obj-$(CONFIG_CMD_DISPLAY) += display.o obj-$(CONFIG_CMD_DTIMG) += dtimg.o obj-$(CONFIG_CMD_ECHO) += echo.o +obj-$(CONFIG_CMD_EXCEPTION) += exception.o obj-$(CONFIG_ENV_IS_IN_EEPROM) += eeprom.o obj-$(CONFIG_CMD_EEPROM) += eeprom.o obj-$(CONFIG_EFI_STUB) += efi.o diff --git a/cmd/exception.c b/cmd/exception.c new file mode 100644 index 0000000000..fb6a15573e --- /dev/null +++ b/cmd/exception.c @@ -0,0 +1,161 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * The 'exception' command can be used for testing exception handling. + * + * Copyright (c) 2018, Heinrich Schuchardt xypron.glpk@gmx.de + */ +#include <common.h> +#include <command.h> + +enum exception { + UNDEFINED_INSTRUCTION = 1, + DATA_ABORT, + BREAKPOINT, +}; + +struct exception_str { + enum exception id; + char *text; + void (*func)(void); +}; + +#if defined(CONFIG_ARM) +static void data_abort(void) +{ +#if defined(CONFIG_ARM64) + /* + * The LDR instruction requires the data source to be eight byte + * aligned. + */ + asm volatile ( + "MOV x1, sp\n" + "ADD x1, x1, 1\n" + "LDR x3, [x1]\n"); +#else + /* + * The LDRD instruction requires the data source to be four byte aligned + * even if strict alignment fault checking is disabled in the system + * control register. + */ + asm volatile ( + "MOV r5, sp\n" + "ADD r5, #1\n" + "LDRD r6, r7, [r5]\n"); +#endif +} + +#if !defined(CONFIG_ARM64) +static void breakpoint(void) +{ + asm volatile ("BKPT #123\n"); +} +#endif +#endif + +static void undefined_instruction(void) +{ +#if defined(CONFIG_ARM) + /* + * 0xe7f...f. is undefined in ARM mode + * 0xde.. is undefined in Thumb mode + */ + asm volatile (".word 0xe7f7defb\n"); +#elif defined(CONFIG_RISCV) + asm volatile (".word 0xffffffff\n"); +#elif defined(CONFIG_X86) + asm volatile (".word 0xffff\n"); +#endif +} + +struct exception_str exceptions[] = { +#if defined(CONFIG_ARM) +#if !defined(CONFIG_ARM64) + { BREAKPOINT, "breakpoint", breakpoint }, +#endif + { DATA_ABORT, "unaligned", data_abort }, +#endif + { UNDEFINED_INSTRUCTION, "undefined", undefined_instruction }, + { 0, NULL, NULL }, +}; + +static int do_exception(cmd_tbl_t *cmdtp, int flag, int argc, + char * const argv[]) +{ + struct exception_str *ex; + enum exception id = 0; + + if (argc != 2) + return CMD_RET_USAGE; + + for (ex = exceptions; ex->func ; ++ex) { + if (!strcmp(argv[1], ex->text)) { + id = ex->id; + break; + } + } + switch (id) { +#if defined(CONFIG_ARM) +#if !defined(CONFIG_ARM64) + case BREAKPOINT: + breakpoint(); + break; +#endif + case DATA_ABORT: + data_abort(); + break; +#endif + case UNDEFINED_INSTRUCTION: + undefined_instruction(); + break; + default: + return CMD_RET_USAGE; + } + + return CMD_RET_FAILURE; +} + +static int exception_complete(int argc, char * const argv[], char last_char, + int maxv, char *cmdv[]) +{ + int len = 0; + struct exception_str *ex; + int i = 0; + + switch (argc) { + case 1: + break; + case 2: + len = strlen(argv[1]); + break; + default: + return 0; + } + for (ex = exceptions; ex->func ; ++ex) { + if (i >= maxv - 1) + return i; + if (!strncmp(argv[1], ex->text, len)) + cmdv[i++] = ex->text; + } + cmdv[i] = NULL; + return i; +} + +#ifdef CONFIG_SYS_LONGHELP +static char exception_help_text[] = + "<ex>\n" + " The following exceptions are available:\n" +#if defined(CONFIG_ARM) +#if !defined(CONFIG_ARM64) + " breakpoint - prefetch abort\n" +#endif + " unaligned - data abort\n" +#endif +#endif + " undefined - undefined instruction\n" + ; + +U_BOOT_CMD_COMPLETE( + exception, 2, 0, do_exception, + "Forces an exception to occur", + exception_help_text, exception_complete +);
Hi Heinrich,
On Sat, 17 Nov 2018 at 07:29, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
The 'exception' command allows to test exception handling.
This implementation supports ARM, x86, RISC-V and the following exceptions:
- 'breakpoint' - prefetch abort exception (ARM 32bit only)
- 'unaligned' - data abort exception (ARM only)
- 'undefined' - undefined instruction exception
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de
Currently RISC-V 64bit gets into an endless loop when hitting the undefined instruction.
So it makes sense to have a testing capability.
cmd/Kconfig | 6 ++ cmd/Makefile | 1 + cmd/exception.c | 161 ++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 168 insertions(+) create mode 100644 cmd/exception.c
diff --git a/cmd/Kconfig b/cmd/Kconfig index e2973b3c51..9d2b8199b8 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -1388,6 +1388,12 @@ config CMD_DISPLAY displayed on a simple board-specific display. Implement display_putc() to use it.
+config CMD_EXCEPTION
bool "exception - raise exception"depends on ARM || RISCV || X86helpEnable the 'exception' command which allows to raise an exception.config CMD_LED bool "led" default y if LED diff --git a/cmd/Makefile b/cmd/Makefile index 0534ddc679..cd67a79170 100644 --- a/cmd/Makefile +++ b/cmd/Makefile @@ -46,6 +46,7 @@ endif obj-$(CONFIG_CMD_DISPLAY) += display.o obj-$(CONFIG_CMD_DTIMG) += dtimg.o obj-$(CONFIG_CMD_ECHO) += echo.o +obj-$(CONFIG_CMD_EXCEPTION) += exception.o obj-$(CONFIG_ENV_IS_IN_EEPROM) += eeprom.o obj-$(CONFIG_CMD_EEPROM) += eeprom.o obj-$(CONFIG_EFI_STUB) += efi.o diff --git a/cmd/exception.c b/cmd/exception.c new file mode 100644 index 0000000000..fb6a15573e --- /dev/null +++ b/cmd/exception.c @@ -0,0 +1,161 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*
- The 'exception' command can be used for testing exception handling.
- Copyright (c) 2018, Heinrich Schuchardt xypron.glpk@gmx.de
- */
+#include <common.h> +#include <command.h>
+enum exception {
UNDEFINED_INSTRUCTION = 1,DATA_ABORT,BREAKPOINT,+};
+struct exception_str {
enum exception id;char *text;void (*func)(void);+};
Can you use the normal subcommand approach for this, as with other commands?
+#if defined(CONFIG_ARM)
How about creating a uclass for this? It isn't nice to have a arch-specific #ifdefs in commands. Something like we did with sysreset.
+static void data_abort(void) +{ +#if defined(CONFIG_ARM64)
/** The LDR instruction requires the data source to be eight byte* aligned.*/asm volatile ("MOV x1, sp\n""ADD x1, x1, 1\n""LDR x3, [x1]\n");
Regards, Simon
On 11/27/18 1:08 AM, Simon Glass wrote:
Hi Heinrich,
On Sat, 17 Nov 2018 at 07:29, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
The 'exception' command allows to test exception handling.
This implementation supports ARM, x86, RISC-V and the following exceptions:
- 'breakpoint' - prefetch abort exception (ARM 32bit only)
- 'unaligned' - data abort exception (ARM only)
- 'undefined' - undefined instruction exception
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de
Currently RISC-V 64bit gets into an endless loop when hitting the undefined instruction.
So it makes sense to have a testing capability.
cmd/Kconfig | 6 ++ cmd/Makefile | 1 + cmd/exception.c | 161 ++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 168 insertions(+) create mode 100644 cmd/exception.c
diff --git a/cmd/Kconfig b/cmd/Kconfig index e2973b3c51..9d2b8199b8 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -1388,6 +1388,12 @@ config CMD_DISPLAY displayed on a simple board-specific display. Implement display_putc() to use it.
+config CMD_EXCEPTION
bool "exception - raise exception"depends on ARM || RISCV || X86helpEnable the 'exception' command which allows to raise an exception.config CMD_LED bool "led" default y if LED diff --git a/cmd/Makefile b/cmd/Makefile index 0534ddc679..cd67a79170 100644 --- a/cmd/Makefile +++ b/cmd/Makefile @@ -46,6 +46,7 @@ endif obj-$(CONFIG_CMD_DISPLAY) += display.o obj-$(CONFIG_CMD_DTIMG) += dtimg.o obj-$(CONFIG_CMD_ECHO) += echo.o +obj-$(CONFIG_CMD_EXCEPTION) += exception.o obj-$(CONFIG_ENV_IS_IN_EEPROM) += eeprom.o obj-$(CONFIG_CMD_EEPROM) += eeprom.o obj-$(CONFIG_EFI_STUB) += efi.o diff --git a/cmd/exception.c b/cmd/exception.c new file mode 100644 index 0000000000..fb6a15573e --- /dev/null +++ b/cmd/exception.c @@ -0,0 +1,161 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*
- The 'exception' command can be used for testing exception handling.
- Copyright (c) 2018, Heinrich Schuchardt xypron.glpk@gmx.de
- */
+#include <common.h> +#include <command.h>
+enum exception {
UNDEFINED_INSTRUCTION = 1,DATA_ABORT,BREAKPOINT,+};
+struct exception_str {
enum exception id;char *text;void (*func)(void);+};
Can you use the normal subcommand approach for this, as with other commands?
Could you give an example, please. I looked at cmd/scsi. and cmd/mmc.c And the only difference I spot is that the names of subcommand functions start with "do_".
+#if defined(CONFIG_ARM)
How about creating a uclass for this? It isn't nice to have a arch-specific #ifdefs in commands. Something like we did with sysreset.
If you want separate files per architecture, why would we need a uclass?
We could simply put the architecture specific U_BOOT_CMD into the implementation file, e.g arch/arm/cpu/exception.c and arch/arm/cpu/exception_64.c
With a uclass I would not know how to implement an architecture specific help output.
Best regards
Heinrich
+static void data_abort(void) +{ +#if defined(CONFIG_ARM64)
/** The LDR instruction requires the data source to be eight byte* aligned.*/asm volatile ("MOV x1, sp\n""ADD x1, x1, 1\n""LDR x3, [x1]\n");Regards, Simon
Hi Heinrich,
On Thu, 20 Dec 2018 at 05:23, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 11/27/18 1:08 AM, Simon Glass wrote:
Hi Heinrich,
On Sat, 17 Nov 2018 at 07:29, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
The 'exception' command allows to test exception handling.
This implementation supports ARM, x86, RISC-V and the following exceptions:
- 'breakpoint' - prefetch abort exception (ARM 32bit only)
- 'unaligned' - data abort exception (ARM only)
- 'undefined' - undefined instruction exception
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de
Currently RISC-V 64bit gets into an endless loop when hitting the undefined instruction.
So it makes sense to have a testing capability.
cmd/Kconfig | 6 ++ cmd/Makefile | 1 + cmd/exception.c | 161 ++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 168 insertions(+) create mode 100644 cmd/exception.c
diff --git a/cmd/Kconfig b/cmd/Kconfig index e2973b3c51..9d2b8199b8 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -1388,6 +1388,12 @@ config CMD_DISPLAY displayed on a simple board-specific display. Implement display_putc() to use it.
+config CMD_EXCEPTION
bool "exception - raise exception"depends on ARM || RISCV || X86helpEnable the 'exception' command which allows to raise an exception.config CMD_LED bool "led" default y if LED diff --git a/cmd/Makefile b/cmd/Makefile index 0534ddc679..cd67a79170 100644 --- a/cmd/Makefile +++ b/cmd/Makefile @@ -46,6 +46,7 @@ endif obj-$(CONFIG_CMD_DISPLAY) += display.o obj-$(CONFIG_CMD_DTIMG) += dtimg.o obj-$(CONFIG_CMD_ECHO) += echo.o +obj-$(CONFIG_CMD_EXCEPTION) += exception.o obj-$(CONFIG_ENV_IS_IN_EEPROM) += eeprom.o obj-$(CONFIG_CMD_EEPROM) += eeprom.o obj-$(CONFIG_EFI_STUB) += efi.o diff --git a/cmd/exception.c b/cmd/exception.c new file mode 100644 index 0000000000..fb6a15573e --- /dev/null +++ b/cmd/exception.c @@ -0,0 +1,161 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*
- The 'exception' command can be used for testing exception handling.
- Copyright (c) 2018, Heinrich Schuchardt xypron.glpk@gmx.de
- */
+#include <common.h> +#include <command.h>
+enum exception {
UNDEFINED_INSTRUCTION = 1,DATA_ABORT,BREAKPOINT,+};
+struct exception_str {
enum exception id;char *text;void (*func)(void);+};
Can you use the normal subcommand approach for this, as with other commands?
Could you give an example, please. I looked at cmd/scsi. and cmd/mmc.c And the only difference I spot is that the names of subcommand functions start with "do_".
Yes, see for example:
static cmd_tbl_t cmd_mmc[] = {
It defines the subcommands in a standard way.
+#if defined(CONFIG_ARM)
How about creating a uclass for this? It isn't nice to have a arch-specific #ifdefs in commands. Something like we did with sysreset.
If you want separate files per architecture, why would we need a uclass?
We could simply put the architecture specific U_BOOT_CMD into the implementation file, e.g arch/arm/cpu/exception.c and arch/arm/cpu/exception_64.c
That that is one option although perhaps you might end up with multiple ARM implementation (e.g. for ARMv47, ARMv7 and ARMv8)?
With a uclass I would not know how to implement an architecture specific help output.
One option is something like sysreset_walk(). It allows us to have a common function for ARM undefined instruction, for example, but finer granularity for breakpoint.
Regards, Simon
On 12/20/18 10:17 PM, Simon Glass wrote:
Hi Heinrich,
On Thu, 20 Dec 2018 at 05:23, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 11/27/18 1:08 AM, Simon Glass wrote:
Hi Heinrich,
On Sat, 17 Nov 2018 at 07:29, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
The 'exception' command allows to test exception handling.
This implementation supports ARM, x86, RISC-V and the following exceptions:
- 'breakpoint' - prefetch abort exception (ARM 32bit only)
- 'unaligned' - data abort exception (ARM only)
- 'undefined' - undefined instruction exception
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de
Currently RISC-V 64bit gets into an endless loop when hitting the undefined instruction.
So it makes sense to have a testing capability.
cmd/Kconfig | 6 ++ cmd/Makefile | 1 + cmd/exception.c | 161 ++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 168 insertions(+) create mode 100644 cmd/exception.c
diff --git a/cmd/Kconfig b/cmd/Kconfig index e2973b3c51..9d2b8199b8 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -1388,6 +1388,12 @@ config CMD_DISPLAY displayed on a simple board-specific display. Implement display_putc() to use it.
+config CMD_EXCEPTION
bool "exception - raise exception"depends on ARM || RISCV || X86helpEnable the 'exception' command which allows to raise an exception.config CMD_LED bool "led" default y if LED diff --git a/cmd/Makefile b/cmd/Makefile index 0534ddc679..cd67a79170 100644 --- a/cmd/Makefile +++ b/cmd/Makefile @@ -46,6 +46,7 @@ endif obj-$(CONFIG_CMD_DISPLAY) += display.o obj-$(CONFIG_CMD_DTIMG) += dtimg.o obj-$(CONFIG_CMD_ECHO) += echo.o +obj-$(CONFIG_CMD_EXCEPTION) += exception.o obj-$(CONFIG_ENV_IS_IN_EEPROM) += eeprom.o obj-$(CONFIG_CMD_EEPROM) += eeprom.o obj-$(CONFIG_EFI_STUB) += efi.o diff --git a/cmd/exception.c b/cmd/exception.c new file mode 100644 index 0000000000..fb6a15573e --- /dev/null +++ b/cmd/exception.c @@ -0,0 +1,161 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*
- The 'exception' command can be used for testing exception handling.
- Copyright (c) 2018, Heinrich Schuchardt xypron.glpk@gmx.de
- */
+#include <common.h> +#include <command.h>
+enum exception {
UNDEFINED_INSTRUCTION = 1,DATA_ABORT,BREAKPOINT,+};
+struct exception_str {
enum exception id;char *text;void (*func)(void);+};
Can you use the normal subcommand approach for this, as with other commands?
Could you give an example, please. I looked at cmd/scsi. and cmd/mmc.c And the only difference I spot is that the names of subcommand functions start with "do_".
Yes, see for example:
static cmd_tbl_t cmd_mmc[] = {
It defines the subcommands in a standard way.
Hello Simon,
thanks for pointing me to the right place. I will adjust my patch.
I think we have some design issues with sub-commands:
I want to have auto-completion for the sub-commands. Unfortunately the "standard way" does not offer it out of the box.
The standard way further forces the creation of redundant code like:
static int do_zynq(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) { cmd_tbl_t *zynq_cmd; int ret;
if (!ARRAY_SIZE(zynq_commands)) { puts("No zynq specific command enabled\n"); return CMD_RET_USAGE; }
if (argc < 2) return CMD_RET_USAGE; zynq_cmd = find_cmd_tbl(argv[1], zynq_commands, ARRAY_SIZE(zynq_commands)); if (!zynq_cmd || argc != zynq_cmd->maxargs) return CMD_RET_USAGE;
ret = zynq_cmd->cmd(zynq_cmd, flag, argc, argv);
return cmd_process_error(zynq_cmd, ret); }
It would be preferable to reference cmd_tbl_t in U_BOOT_CMD. Furthermore U_BOOT_CMD_MKENT should allow references to a cmd_tbl_t for sub-sub commands. This way autocompletion of subcommands could be provided.
This would also allow to place the help texts for subcommands into U_BOOT_CMD_MKENT where they belong.
Why are U_BOOT_SUBCMD_START and U_BOOT_SUBCMD_END only defined for CONFIG_CMDLINE=n? This way we cannot use them.
Why do we check CONFIG_SYS_LONGHELP inside cmd/*.c for many commands? Hasn't this been superseded by _CMD_HELP in include/commands.h?
Best regards
Heinrich
+#if defined(CONFIG_ARM)
How about creating a uclass for this? It isn't nice to have a arch-specific #ifdefs in commands. Something like we did with sysreset.
If you want separate files per architecture, why would we need a uclass?
We could simply put the architecture specific U_BOOT_CMD into the implementation file, e.g arch/arm/cpu/exception.c and arch/arm/cpu/exception_64.c
That that is one option although perhaps you might end up with multiple ARM implementation (e.g. for ARMv47, ARMv7 and ARMv8)?
With a uclass I would not know how to implement an architecture specific help output.
One option is something like sysreset_walk(). It allows us to have a common function for ARM undefined instruction, for example, but finer granularity for breakpoint.
Regards, Simon
Hi Heinrich,
On Thu, 20 Dec 2018 at 18:58, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 12/20/18 10:17 PM, Simon Glass wrote:
Hi Heinrich,
On Thu, 20 Dec 2018 at 05:23, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 11/27/18 1:08 AM, Simon Glass wrote:
Hi Heinrich,
On Sat, 17 Nov 2018 at 07:29, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
The 'exception' command allows to test exception handling.
This implementation supports ARM, x86, RISC-V and the following exceptions:
- 'breakpoint' - prefetch abort exception (ARM 32bit only)
- 'unaligned' - data abort exception (ARM only)
- 'undefined' - undefined instruction exception
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de
Currently RISC-V 64bit gets into an endless loop when hitting the undefined instruction.
So it makes sense to have a testing capability.
cmd/Kconfig | 6 ++ cmd/Makefile | 1 + cmd/exception.c | 161 ++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 168 insertions(+) create mode 100644 cmd/exception.c
diff --git a/cmd/Kconfig b/cmd/Kconfig index e2973b3c51..9d2b8199b8 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -1388,6 +1388,12 @@ config CMD_DISPLAY displayed on a simple board-specific display. Implement display_putc() to use it.
+config CMD_EXCEPTION
bool "exception - raise exception"depends on ARM || RISCV || X86helpEnable the 'exception' command which allows to raise an exception.config CMD_LED bool "led" default y if LED diff --git a/cmd/Makefile b/cmd/Makefile index 0534ddc679..cd67a79170 100644 --- a/cmd/Makefile +++ b/cmd/Makefile @@ -46,6 +46,7 @@ endif obj-$(CONFIG_CMD_DISPLAY) += display.o obj-$(CONFIG_CMD_DTIMG) += dtimg.o obj-$(CONFIG_CMD_ECHO) += echo.o +obj-$(CONFIG_CMD_EXCEPTION) += exception.o obj-$(CONFIG_ENV_IS_IN_EEPROM) += eeprom.o obj-$(CONFIG_CMD_EEPROM) += eeprom.o obj-$(CONFIG_EFI_STUB) += efi.o diff --git a/cmd/exception.c b/cmd/exception.c new file mode 100644 index 0000000000..fb6a15573e --- /dev/null +++ b/cmd/exception.c @@ -0,0 +1,161 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*
- The 'exception' command can be used for testing exception handling.
- Copyright (c) 2018, Heinrich Schuchardt xypron.glpk@gmx.de
- */
+#include <common.h> +#include <command.h>
+enum exception {
UNDEFINED_INSTRUCTION = 1,DATA_ABORT,BREAKPOINT,+};
+struct exception_str {
enum exception id;char *text;void (*func)(void);+};
Can you use the normal subcommand approach for this, as with other commands?
Could you give an example, please. I looked at cmd/scsi. and cmd/mmc.c And the only difference I spot is that the names of subcommand functions start with "do_".
Yes, see for example:
static cmd_tbl_t cmd_mmc[] = {
It defines the subcommands in a standard way.
Hello Simon,
thanks for pointing me to the right place. I will adjust my patch.
I think we have some design issues with sub-commands:
I want to have auto-completion for the sub-commands. Unfortunately the "standard way" does not offer it out of the box.
The standard way further forces the creation of redundant code like:
static int do_zynq(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) { cmd_tbl_t *zynq_cmd; int ret;
if (!ARRAY_SIZE(zynq_commands)) { puts("No zynq specific command enabled\n"); return CMD_RET_USAGE; } if (argc < 2) return CMD_RET_USAGE; zynq_cmd = find_cmd_tbl(argv[1], zynq_commands, ARRAY_SIZE(zynq_commands)); if (!zynq_cmd || argc != zynq_cmd->maxargs) return CMD_RET_USAGE; ret = zynq_cmd->cmd(zynq_cmd, flag, argc, argv); return cmd_process_error(zynq_cmd, ret);}
It would be preferable to reference cmd_tbl_t in U_BOOT_CMD. Furthermore U_BOOT_CMD_MKENT should allow references to a cmd_tbl_t for sub-sub commands. This way autocompletion of subcommands could be provided.
This would also allow to place the help texts for subcommands into U_BOOT_CMD_MKENT where they belong.
Yes indeed. There was a recent series sent to introduce auto-completion for that I think. I have not looked at it though.
Why are U_BOOT_SUBCMD_START and U_BOOT_SUBCMD_END only defined for CONFIG_CMDLINE=n? This way we cannot use them.
Well it was a feature designed to provide a way to drop commands. I have no objection to changing this.
Why do we check CONFIG_SYS_LONGHELP inside cmd/*.c for many commands? Hasn't this been superseded by _CMD_HELP in include/commands.h?
Looks like it. Yes this could be cleaned up.
Regards, Simon
Best regards
Heinrich
+#if defined(CONFIG_ARM)
How about creating a uclass for this? It isn't nice to have a arch-specific #ifdefs in commands. Something like we did with sysreset.
If you want separate files per architecture, why would we need a uclass?
We could simply put the architecture specific U_BOOT_CMD into the implementation file, e.g arch/arm/cpu/exception.c and arch/arm/cpu/exception_64.c
That that is one option although perhaps you might end up with multiple ARM implementation (e.g. for ARMv47, ARMv7 and ARMv8)?
With a uclass I would not know how to implement an architecture specific help output.
One option is something like sysreset_walk(). It allows us to have a common function for ARM undefined instruction, for example, but finer granularity for breakpoint.
Regards, Simon
participants (2)
-
Heinrich Schuchardt -
Simon Glass