| Message ID | 20221207135352.592556-1-bmeng@tinylab.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2,1/3] serial: earlycon-arm-semihost: Move smh_putc() variants in respective arch's semihost.h | expand |
On 2022/12/8 14:08:33, "Jiri Slaby" <jirislaby@kernel.org> wrote: >On 07. 12. 22, 14:53, Bin Meng wrote: >>Move smh_putc() variants in respective arch/*/include/asm/semihost.h, >>in preparation to add RISC-V support. >> >>Signed-off-by: Bin Meng <bmeng@tinylab.org> >... >>--- /dev/null >>+++ b/arch/arm/include/asm/semihost.h >>@@ -0,0 +1,23 @@ >>+/* SPDX-License-Identifier: GPL-2.0 */ >>+/* >>+ * Copyright (C) 2012 ARM Ltd. >>+ * Author: Marc Zyngier <marc.zyngier@arm.com> >>+ * >>+ * Adapted for ARM and earlycon: >>+ * Copyright (C) 2014 Linaro Ltd. >>+ * Author: Rob Herring <robh@kernel.org> >>+ */ > >Much better. There are three minor issues: >1) protection against multiple #include-s is missing here. Oops, will add in v3. > > >>+#ifdef CONFIG_THUMB2_KERNEL >>+#define SEMIHOST_SWI "0xab" >>+#else >>+#define SEMIHOST_SWI "0x123456" >>+#endif >>+ >>+static inline void smh_putc(struct uart_port *port, unsigned char c) > >2) port is unused in all implementations. So it should be dropped. >3) can you make "c" an explicit u8? The smh_putc function signature is defined by the uart_console_write helper. I don't think we can change it. > >>+{ >>+ asm volatile("mov r1, %0\n" >>+ "mov r0, #3\n" >>+ "svc " SEMIHOST_SWI "\n" >>+ : : "r" (&c) : "r0", "r1", "memory"); >>+} Regards, Bin
On 08. 12. 22, 10:32, Bin Meng wrote: >>> +#ifdef CONFIG_THUMB2_KERNEL >>> +#define SEMIHOST_SWI "0xab" >>> +#else >>> +#define SEMIHOST_SWI "0x123456" >>> +#endif >>> + >>> +static inline void smh_putc(struct uart_port *port, unsigned char c) >> >> 2) port is unused in all implementations. So it should be dropped. >> 3) can you make "c" an explicit u8? > > The smh_putc function signature is defined by the uart_console_write > helper. I don't think we can change it. Ah. Of course. Then at least forward-declare struct uart_port here. So that it works also when someone decides to include the header outside serial. thanks,
diff --git a/arch/arm/include/asm/semihost.h b/arch/arm/include/asm/semihost.h new file mode 100644 index 000000000000..c33cb5124376 --- /dev/null +++ b/arch/arm/include/asm/semihost.h @@ -0,0 +1,23 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * Copyright (C) 2012 ARM Ltd. + * Author: Marc Zyngier <marc.zyngier@arm.com> + * + * Adapted for ARM and earlycon: + * Copyright (C) 2014 Linaro Ltd. + * Author: Rob Herring <robh@kernel.org> + */ + +#ifdef CONFIG_THUMB2_KERNEL +#define SEMIHOST_SWI "0xab" +#else +#define SEMIHOST_SWI "0x123456" +#endif + +static inline void smh_putc(struct uart_port *port, unsigned char c) +{ + asm volatile("mov r1, %0\n" + "mov r0, #3\n" + "svc " SEMIHOST_SWI "\n" + : : "r" (&c) : "r0", "r1", "memory"); +} diff --git a/arch/arm64/include/asm/semihost.h b/arch/arm64/include/asm/semihost.h new file mode 100644 index 000000000000..9e56d38fe5fd --- /dev/null +++ b/arch/arm64/include/asm/semihost.h @@ -0,0 +1,17 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * Copyright (C) 2012 ARM Ltd. + * Author: Marc Zyngier <marc.zyngier@arm.com> + * + * Adapted for ARM and earlycon: + * Copyright (C) 2014 Linaro Ltd. + * Author: Rob Herring <robh@kernel.org> + */ + +static inline void smh_putc(struct uart_port *port, unsigned char c) +{ + asm volatile("mov x1, %0\n" + "mov x0, #3\n" + "hlt 0xf000\n" + : : "r" (&c) : "x0", "x1", "memory"); +} diff --git a/drivers/tty/serial/earlycon-arm-semihost.c b/drivers/tty/serial/earlycon-arm-semihost.c index fcdec5f42376..e4692a8433f9 100644 --- a/drivers/tty/serial/earlycon-arm-semihost.c +++ b/drivers/tty/serial/earlycon-arm-semihost.c @@ -11,30 +11,7 @@ #include <linux/console.h> #include <linux/init.h> #include <linux/serial_core.h> - -#ifdef CONFIG_THUMB2_KERNEL -#define SEMIHOST_SWI "0xab" -#else -#define SEMIHOST_SWI "0x123456" -#endif - -/* - * Semihosting-based debug console - */ -static void smh_putc(struct uart_port *port, unsigned char c) -{ -#ifdef CONFIG_ARM64 - asm volatile("mov x1, %0\n" - "mov x0, #3\n" - "hlt 0xf000\n" - : : "r" (&c) : "x0", "x1", "memory"); -#else - asm volatile("mov r1, %0\n" - "mov r0, #3\n" - "svc " SEMIHOST_SWI "\n" - : : "r" (&c) : "r0", "r1", "memory"); -#endif -} +#include <asm/semihost.h> static void smh_write(struct console *con, const char *s, unsigned n) {
Move smh_putc() variants in respective arch/*/include/asm/semihost.h, in preparation to add RISC-V support. Signed-off-by: Bin Meng <bmeng@tinylab.org> --- Changes in v2: - new patch: "serial: earlycon-arm-semihost: Move smh_putc() variants in respective arch's semihost.h" arch/arm/include/asm/semihost.h | 23 ++++++++++++++++++++ arch/arm64/include/asm/semihost.h | 17 +++++++++++++++ drivers/tty/serial/earlycon-arm-semihost.c | 25 +--------------------- 3 files changed, 41 insertions(+), 24 deletions(-) create mode 100644 arch/arm/include/asm/semihost.h create mode 100644 arch/arm64/include/asm/semihost.h