On 12/20/18 10:17 PM, Simon Glass wrote:
> Hi Heinrich,
>
> On Thu, 20 Dec 2018 at 05:23, Heinrich Schuchardt <xypron.g...@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.g...@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.g...@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.g...@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
>
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
