2022-12-07 15:16:09

by Bin Meng

[permalink] [raw]
Subject: [PATCH v2 1/3] serial: earlycon-arm-semihost: Move smh_putc() variants in respective arch's semihost.h

Move smh_putc() variants in respective arch/*/include/asm/semihost.h,
in preparation to add RISC-V support.

Signed-off-by: Bin Meng <[email protected]>

---

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

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 <[email protected]>
+ *
+ * Adapted for ARM and earlycon:
+ * Copyright (C) 2014 Linaro Ltd.
+ * Author: Rob Herring <[email protected]>
+ */
+
+#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 <[email protected]>
+ *
+ * Adapted for ARM and earlycon:
+ * Copyright (C) 2014 Linaro Ltd.
+ * Author: Rob Herring <[email protected]>
+ */
+
+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)
{
--
2.34.1


2022-12-08 06:41:48

by Jiri Slaby

[permalink] [raw]
Subject: Re: [PATCH v2 1/3] serial: earlycon-arm-semihost: Move smh_putc() variants in respective arch's semihost.h

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 <[email protected]>
...
> --- /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 <[email protected]>
> + *
> + * Adapted for ARM and earlycon:
> + * Copyright (C) 2014 Linaro Ltd.
> + * Author: Rob Herring <[email protected]>
> + */

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,
--
js
suse labs

2022-12-08 10:07:42

by Bin Meng

[permalink] [raw]
Subject: Re: [PATCH v2 1/3] serial: earlycon-arm-semihost: Move smh_putc() variants in respective arch's semihost.h

On 2022/12/8 14:08:33, "Jiri Slaby" <[email protected]> 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 <[email protected]>
>...
>>--- /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 <[email protected]>
>>+ *
>>+ * Adapted for ARM and earlycon:
>>+ * Copyright (C) 2014 Linaro Ltd.
>>+ * Author: Rob Herring <[email protected]>
>>+ */
>
>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

2022-12-08 10:11:27

by Jiri Slaby

[permalink] [raw]
Subject: Re: [PATCH v2 1/3] serial: earlycon-arm-semihost: Move smh_putc() variants in respective arch's semihost.h

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,
--
js
suse labs