| Message ID | 20221207135352.592556-1-bmeng@tinylab.org (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Palmer Dabbelt |
| Headers | show |
| Series | [v2,1/3] serial: earlycon-arm-semihost: Move smh_putc() variants in respective arch's semihost.h | expand |
| Context | Check | Description |
|---|---|---|
| conchuod/patch_count | success | Link |
| conchuod/cover_letter | warning | Series does not have a cover letter |
| conchuod/tree_selection | success | Guessed tree name to be for-next |
| conchuod/fixes_present | success | Fixes tag not required for -next series |
| conchuod/verify_signedoff | success | Signed-off-by tag matches author and committer |
| conchuod/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| conchuod/module_param | success | Was 0 now: 0 |
| conchuod/alphanumeric_selects | success | Out of order selects before the patch: 57 and now 57 |
| conchuod/build_rv32_defconfig | success | Build OK |
| conchuod/build_warn_rv64 | success | Errors and warnings before: 0 this patch: 0 |
| conchuod/dtb_warn_rv64 | success | Errors and warnings before: 0 this patch: 0 |
| conchuod/header_inline | success | No static functions without inline keyword in header files |
| conchuod/checkpatch | warning | WARNING: added, moved or deleted file(s), does MAINTAINERS need updating? |
| conchuod/source_inline | success | Was 0 now: 0 |
| conchuod/build_rv64_nommu_k210_defconfig | success | Build OK |
| conchuod/verify_fixes | success | No Fixes tag |
| conchuod/build_rv64_nommu_virt_defconfig | success | Build OK |
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. > +#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? > +{ > + asm volatile("mov r1, %0\n" > + "mov r0, #3\n" > + "svc " SEMIHOST_SWI "\n" > + : : "r" (&c) : "r0", "r1", "memory"); > +} thanks,
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