All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/6] hw/char: Implement the STM32L4x5 USART, UART and LPUART
@ 2024-03-24 16:55 Arnaud Minier
  2024-03-24 16:55 ` [PATCH v2 1/6] hw/misc/stm32l4x5_rcc: Propagate period when enabling a clock Arnaud Minier
                   ` (6 more replies)
  0 siblings, 7 replies; 14+ messages in thread
From: Arnaud Minier @ 2024-03-24 16:55 UTC (permalink / raw)
  To: qemu-devel
  Cc: Laurent Vivier, Marc-André Lureau, qemu-arm, Paolo Bonzini,
	Alistair Francis, Arnaud Minier, Thomas Huth, Inès Varhol,
	Samuel Tardieu, Peter Maydell

This patch adds the STM32L4x5 USART
(Universal Synchronous/Asynchronous Receiver/Transmitter)
device and is part of a series implementing the
STM32L4x5 with a few peripherals.

It implements the necessary functionalities to receive/send
characters over the serial port, which are useful to
communicate with the program currently running.

Many thanks Peter for your review, I think I addressed almost
everything.
I'm just unsure about how to handle the waiting time in the tests.
I understand your concerns about the unreliability of using the wallclock
time but I don't understand how using clock_step() would make it
more reliable. We will always be waiting on something
that is out of our control (i.e. the OS).
I increased the delay from 5s to 10min to match the microbit test
and added a comment (I paraphrased your comment, is that okay ?).

I also saw that Philippe Mathieu-Daudé have sent a patchset with
the first commit of this patchset and another commit to make
clock_set_mul_div() return a boolean.
If this is merged before this patchset (which will probably be
the case), I will remove the first commit.

Changes from v1 to v2:
- Use asynchronous transmission for serial communication
  (based on cmsdk-apb-uart implementation)
- Use qemu_log_mask instead of error_report
- Squash the commit that renamed the base struct
- Use switch statements where appropriate
- Fix RDR and TDR mask size
- Increase time limit in tests
- Remove the global qtest in the tests
- Use assert when checking the interrupt number in the tests
- Correct usage of g_autofree in the SoC

Arnaud Minier (6):
  hw/misc/stm32l4x5_rcc: Propagate period when enabling a clock
  hw/char: Implement STM32L4x5 USART skeleton
  hw/char/stm32l4x5_usart: Enable serial read and write
  hw/char/stm32l4x5_usart: Add options for serial parameters setting
  hw/arm: Add the USART to the stm32l4x5 SoC
  tests/qtest: Add tests for the STM32L4x5 USART

 MAINTAINERS                        |   1 +
 docs/system/arm/b-l475e-iot01a.rst |   2 +-
 hw/arm/Kconfig                     |   1 +
 hw/arm/stm32l4x5_soc.c             |  82 +++-
 hw/char/Kconfig                    |   3 +
 hw/char/meson.build                |   1 +
 hw/char/stm32l4x5_usart.c          | 632 +++++++++++++++++++++++++++++
 hw/char/trace-events               |  12 +
 hw/misc/stm32l4x5_rcc.c            |   7 +-
 include/hw/arm/stm32l4x5_soc.h     |  13 +
 include/hw/char/stm32l4x5_usart.h  |  67 +++
 tests/qtest/meson.build            |   3 +-
 tests/qtest/stm32l4x5_usart-test.c | 326 +++++++++++++++
 13 files changed, 1141 insertions(+), 9 deletions(-)
 create mode 100644 hw/char/stm32l4x5_usart.c
 create mode 100644 include/hw/char/stm32l4x5_usart.h
 create mode 100644 tests/qtest/stm32l4x5_usart-test.c

-- 
2.34.1



^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v2 1/6] hw/misc/stm32l4x5_rcc: Propagate period when enabling a clock
  2024-03-24 16:55 [PATCH v2 0/6] hw/char: Implement the STM32L4x5 USART, UART and LPUART Arnaud Minier
@ 2024-03-24 16:55 ` Arnaud Minier
  2024-03-24 16:55 ` [PATCH v2 2/6] hw/char: Implement STM32L4x5 USART skeleton Arnaud Minier
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 14+ messages in thread
From: Arnaud Minier @ 2024-03-24 16:55 UTC (permalink / raw)
  To: qemu-devel
  Cc: Laurent Vivier, Marc-André Lureau, qemu-arm, Paolo Bonzini,
	Alistair Francis, Arnaud Minier, Thomas Huth, Inès Varhol,
	Samuel Tardieu, Peter Maydell

The "clock_set_mul_div" function doesn't propagate the clock period to
the children if it is changed (e.g. by enabling/disabling
a clock multiplexer).
This was overlooked during the implementation due to late changes.

This commit propagates the change if the multiplier or divider changes.

The usart tests will ensure that this behavior will not regress.

Signed-off-by: Arnaud Minier <arnaud.minier@telecom-paris.fr>
Signed-off-by: Inès Varhol <ines.varhol@telecom-paris.fr>
---
 hw/misc/stm32l4x5_rcc.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/hw/misc/stm32l4x5_rcc.c b/hw/misc/stm32l4x5_rcc.c
index bc2d63528b..4725ba4f1c 100644
--- a/hw/misc/stm32l4x5_rcc.c
+++ b/hw/misc/stm32l4x5_rcc.c
@@ -59,7 +59,12 @@ static void clock_mux_update(RccClockMuxState *mux, bool bypass_source)
         freq_multiplier = mux->divider;
     }
 
-    clock_set_mul_div(mux->out, freq_multiplier, mux->multiplier);
+    if ((mux->out->multiplier != freq_multiplier) ||
+        mux->out->divider != mux->multiplier) {
+        clock_set_mul_div(mux->out, freq_multiplier, mux->multiplier);
+        clock_propagate(mux->out);
+    }
+
     clock_update(mux->out, clock_get(current_source));
 
     src_freq = clock_get_hz(current_source);
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 2/6] hw/char: Implement STM32L4x5 USART skeleton
  2024-03-24 16:55 [PATCH v2 0/6] hw/char: Implement the STM32L4x5 USART, UART and LPUART Arnaud Minier
  2024-03-24 16:55 ` [PATCH v2 1/6] hw/misc/stm32l4x5_rcc: Propagate period when enabling a clock Arnaud Minier
@ 2024-03-24 16:55 ` Arnaud Minier
  2024-03-28 15:55   ` Peter Maydell
  2024-03-24 16:55 ` [PATCH v2 3/6] hw/char/stm32l4x5_usart: Enable serial read and write Arnaud Minier
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 14+ messages in thread
From: Arnaud Minier @ 2024-03-24 16:55 UTC (permalink / raw)
  To: qemu-devel
  Cc: Laurent Vivier, Marc-André Lureau, qemu-arm, Paolo Bonzini,
	Alistair Francis, Arnaud Minier, Thomas Huth, Inès Varhol,
	Samuel Tardieu, Peter Maydell

Add the basic infrastructure (register read/write, type...)
to implement the STM32L4x5 USART.

Also create different types for the USART, UART and LPUART
of the STM32L4x5 to deduplicate code and enable the
implementation of different behaviors depending on the type.

Signed-off-by: Arnaud Minier <arnaud.minier@telecom-paris.fr>
Signed-off-by: Inès Varhol <ines.varhol@telecom-paris.fr>
---
 MAINTAINERS                       |   1 +
 hw/char/Kconfig                   |   3 +
 hw/char/meson.build               |   1 +
 hw/char/stm32l4x5_usart.c         | 395 ++++++++++++++++++++++++++++++
 hw/char/trace-events              |   4 +
 include/hw/char/stm32l4x5_usart.h |  66 +++++
 6 files changed, 470 insertions(+)
 create mode 100644 hw/char/stm32l4x5_usart.c
 create mode 100644 include/hw/char/stm32l4x5_usart.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 409d7db4d4..deba4a54ce 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1128,6 +1128,7 @@ M: Inès Varhol <ines.varhol@telecom-paris.fr>
 L: qemu-arm@nongnu.org
 S: Maintained
 F: hw/arm/stm32l4x5_soc.c
+F: hw/char/stm32l4x5_usart.c
 F: hw/misc/stm32l4x5_exti.c
 F: hw/misc/stm32l4x5_syscfg.c
 F: hw/misc/stm32l4x5_rcc.c
diff --git a/hw/char/Kconfig b/hw/char/Kconfig
index 6b6cf2fc1d..4fd74ea878 100644
--- a/hw/char/Kconfig
+++ b/hw/char/Kconfig
@@ -41,6 +41,9 @@ config VIRTIO_SERIAL
 config STM32F2XX_USART
     bool
 
+config STM32L4X5_USART
+    bool
+
 config CMSDK_APB_UART
     bool
 
diff --git a/hw/char/meson.build b/hw/char/meson.build
index 006d20f1e2..e5b13b6958 100644
--- a/hw/char/meson.build
+++ b/hw/char/meson.build
@@ -31,6 +31,7 @@ system_ss.add(when: 'CONFIG_RENESAS_SCI', if_true: files('renesas_sci.c'))
 system_ss.add(when: 'CONFIG_SIFIVE_UART', if_true: files('sifive_uart.c'))
 system_ss.add(when: 'CONFIG_SH_SCI', if_true: files('sh_serial.c'))
 system_ss.add(when: 'CONFIG_STM32F2XX_USART', if_true: files('stm32f2xx_usart.c'))
+system_ss.add(when: 'CONFIG_STM32L4X5_USART', if_true: files('stm32l4x5_usart.c'))
 system_ss.add(when: 'CONFIG_MCHP_PFSOC_MMUART', if_true: files('mchp_pfsoc_mmuart.c'))
 system_ss.add(when: 'CONFIG_HTIF', if_true: files('riscv_htif.c'))
 system_ss.add(when: 'CONFIG_GOLDFISH_TTY', if_true: files('goldfish_tty.c'))
diff --git a/hw/char/stm32l4x5_usart.c b/hw/char/stm32l4x5_usart.c
new file mode 100644
index 0000000000..46e69bb096
--- /dev/null
+++ b/hw/char/stm32l4x5_usart.c
@@ -0,0 +1,395 @@
+/*
+ * STM32L4X5 USART (Universal Synchronous Asynchronous Receiver Transmitter)
+ *
+ * Copyright (c) 2023 Arnaud Minier <arnaud.minier@telecom-paris.fr>
+ * Copyright (c) 2023 Inès Varhol <ines.varhol@telecom-paris.fr>
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ * The STM32L4X5 USART is heavily inspired by the stm32f2xx_usart
+ * by Alistair Francis.
+ * The reference used is the STMicroElectronics RM0351 Reference manual
+ * for STM32L4x5 and STM32L4x6 advanced Arm ® -based 32-bit MCUs.
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/log.h"
+#include "qemu/module.h"
+#include "qapi/error.h"
+#include "chardev/char-fe.h"
+#include "chardev/char-serial.h"
+#include "migration/vmstate.h"
+#include "hw/char/stm32l4x5_usart.h"
+#include "hw/clock.h"
+#include "hw/irq.h"
+#include "hw/qdev-clock.h"
+#include "hw/qdev-properties.h"
+#include "hw/qdev-properties-system.h"
+#include "hw/registerfields.h"
+#include "trace.h"
+
+
+REG32(CR1, 0x00)
+    FIELD(CR1, M1, 28, 1)    /* Word length (part 2, see M0)*/
+    FIELD(CR1, EOBIE, 27, 1) /* End of Block interrupt enable */
+    FIELD(CR1, RTOIE, 26, 1) /* Receiver timeout interrupt enable */
+    FIELD(CR1, DEAT, 21, 5)  /* Driver Enable assertion time */
+    FIELD(CR1, DEDT, 16, 5)  /* Driver Enable de-assertion time */
+    FIELD(CR1, OVER8, 15, 1) /* Oversampling mode */
+    FIELD(CR1, CMIE, 14, 1)  /* Character match interrupt enable */
+    FIELD(CR1, MME, 13, 1)   /* Mute mode enable */
+    FIELD(CR1, M0, 12, 1)    /* Word length (part 1, see M1) */
+    FIELD(CR1, WAKE, 11, 1)  /* Receiver wakeup method */
+    FIELD(CR1, PCE, 10, 1)   /* Parity control enable */
+    FIELD(CR1, PS, 9, 1)     /* Parity selection */
+    FIELD(CR1, PEIE, 8, 1)   /* PE interrupt enable */
+    FIELD(CR1, TXEIE, 7, 1)  /* TXE interrupt enable */
+    FIELD(CR1, TCIE, 6, 1)   /* Transmission complete interrupt enable */
+    FIELD(CR1, RXNEIE, 5, 1) /* RXNE interrupt enable */
+    FIELD(CR1, IDLEIE, 4, 1) /* IDLE interrupt enable */
+    FIELD(CR1, TE, 3, 1)     /* Transmitter enable */
+    FIELD(CR1, RE, 2, 1)     /* Receiver enable */
+    FIELD(CR1, UESM, 1, 1)   /* USART enable in Stop mode */
+    FIELD(CR1, UE, 0, 1)     /* USART enable */
+REG32(CR2, 0x04)
+    FIELD(CR2, ADD_1, 28, 4)    /* ADD[7:4] */
+    FIELD(CR2, ADD_0, 24, 1)    /* ADD[3:0] */
+    FIELD(CR2, RTOEN, 23, 1)    /* Receiver timeout enable */
+    FIELD(CR2, ABRMOD, 21, 2)   /* Auto baud rate mode */
+    FIELD(CR2, ABREN, 20, 1)    /* Auto baud rate enable */
+    FIELD(CR2, MSBFIRST, 19, 1) /* Most significant bit first */
+    FIELD(CR2, DATAINV, 18, 1)  /* Binary data inversion */
+    FIELD(CR2, TXINV, 17, 1)    /* TX pin active level inversion */
+    FIELD(CR2, RXINV, 16, 1)    /* RX pin active level inversion */
+    FIELD(CR2, SWAP, 15, 1)     /* Swap RX/TX pins */
+    FIELD(CR2, LINEN, 14, 1)    /* LIN mode enable */
+    FIELD(CR2, STOP, 12, 2)     /* STOP bits */
+    FIELD(CR2, CLKEN, 11, 1)    /* Clock enable */
+    FIELD(CR2, CPOL, 10, 1)     /* Clock polarity */
+    FIELD(CR2, CPHA, 9, 1)      /* Clock phase */
+    FIELD(CR2, LBCL, 8, 1)      /* Last bit clock pulse */
+    FIELD(CR2, LBDIE, 6, 1)     /* LIN break detection interrupt enable */
+    FIELD(CR2, LBDL, 5, 1)      /* LIN break detection length */
+    FIELD(CR2, ADDM7, 4, 1)     /* 7-bit Address Detection /
+                                 * 4-bit Address Detection */
+REG32(CR3, 0x08)
+    /* TCBGTIE only on STM32L496xx/4A6xx devices */
+    FIELD(CR3, UCESM, 23, 1)   /* USART Clock Enable in Stop Mode */
+    FIELD(CR3, WUFIE, 22, 1)   /* Wakeup from Stop mode interrupt enable */
+    FIELD(CR3, WUS, 20, 2)     /* Wakeup from Stop mode interrupt flag selection */
+    FIELD(CR3, SCARCNT, 17, 3) /* Smartcard auto-retry count */
+    FIELD(CR3, DEP, 15, 1)     /* Driver enable polarity selection */
+    FIELD(CR3, DEM, 14, 1)     /* Driver enable mode */
+    FIELD(CR3, DDRE, 13, 1)    /* DMA Disable on Reception Error */
+    FIELD(CR3, OVRDIS, 12, 1)  /* Overrun Disable */
+    FIELD(CR3, ONEBIT, 11, 1)  /* One sample bit method enable */
+    FIELD(CR3, CTSIE, 10, 1)   /* CTS interrupt enable */
+    FIELD(CR3, CTSE, 9, 1)     /* CTS enable */
+    FIELD(CR3, RTSE, 8, 1)     /* RTS enable */
+    FIELD(CR3, DMAT, 7, 1)     /* DMA enable transmitter */
+    FIELD(CR3, DMAR, 6, 1)     /* DMA enable receiver */
+    FIELD(CR3, SCEN, 5, 1)     /* Smartcard mode enable */
+    FIELD(CR3, NACK, 4, 1)     /* Smartcard NACK enable */
+    FIELD(CR3, HDSEL, 3, 1)    /* Half-duplex selection */
+    FIELD(CR3, IRLP, 2, 1)     /* IrDA low-power */
+    FIELD(CR3, IREN, 1, 1)     /* IrDA mode enable */
+    FIELD(CR3, EIE, 0, 1)      /* Error interrupt enable */
+REG32(BRR, 0x0C)
+    FIELD(BRR, BRR, 0, 16)
+REG32(GTPR, 0x10)
+    FIELD(GTPR, GT, 8, 8)  /* Guard time value */
+    FIELD(GTPR, PSC, 0, 8) /* Prescaler value */
+REG32(RTOR, 0x14)
+    FIELD(RTOR, BLEN, 24, 8) /* Block Length */
+    FIELD(RTOR, RTO, 0, 24)  /* Receiver timeout value */
+REG32(RQR, 0x18)
+    FIELD(RQR, TXFRQ, 4, 1)  /* Transmit data flush request */
+    FIELD(RQR, RXFRQ, 3, 1)  /* Receive data flush request */
+    FIELD(RQR, MMRQ, 2, 1)   /* Mute mode request */
+    FIELD(RQR, SBKRQ, 1, 1)  /* Send break request */
+    FIELD(RQR, ABBRRQ, 0, 1) /* Auto baud rate request */
+REG32(ISR, 0x1C)
+    /* TCBGT only for STM32L475xx/476xx/486xx devices */
+    FIELD(ISR, REACK, 22, 1) /* Receive enable acknowledge flag */
+    FIELD(ISR, TEACK, 21, 1) /* Transmit enable acknowledge flag */
+    FIELD(ISR, WUF, 20, 1)   /* Wakeup from Stop mode flag */
+    FIELD(ISR, RWU, 19, 1)   /* Receiver wakeup from Mute mode */
+    FIELD(ISR, SBKF, 18, 1)  /* Send break flag */
+    FIELD(ISR, CMF, 17, 1)   /* Character match flag */
+    FIELD(ISR, BUSY, 16, 1)  /* Busy flag */
+    FIELD(ISR, ABRF, 15, 1)  /* Auto Baud rate flag */
+    FIELD(ISR, ABRE, 14, 1)  /* Auto Baud rate error */
+    FIELD(ISR, EOBF, 12, 1)  /* End of block flag */
+    FIELD(ISR, RTOF, 11, 1)  /* Receiver timeout */
+    FIELD(ISR, CTS, 10, 1)   /* CTS flag */
+    FIELD(ISR, CTSIF, 9, 1)  /* CTS interrupt flag */
+    FIELD(ISR, LBDF, 8, 1)   /* LIN break detection flag */
+    FIELD(ISR, TXE, 7, 1)    /* Transmit data register empty */
+    FIELD(ISR, TC, 6, 1)     /* Transmission complete */
+    FIELD(ISR, RXNE, 5, 1)   /* Read data register not empty */
+    FIELD(ISR, IDLE, 4, 1)   /* Idle line detected */
+    FIELD(ISR, ORE, 3, 1)    /* Overrun error */
+    FIELD(ISR, NF, 2, 1)     /* START bit Noise detection flag */
+    FIELD(ISR, FE, 1, 1)     /* Framing Error*/
+    FIELD(ISR, PE, 0, 1)     /* Parity Error*/
+REG32(ICR, 0x20)
+    FIELD(ICR, WUCF, 20, 1)   /* Wakeup from Stop mode clear flag */
+    FIELD(ICR, CMCF, 17, 1)   /* Character match clear flag */
+    FIELD(ICR, EOBCF, 12, 1)  /* End of block clear flag */
+    FIELD(ICR, RTOCF, 11, 1)  /* Receiver timeout clear flag */
+    FIELD(ICR, CTSCF, 9, 1)   /* CTS clear flag */
+    FIELD(ICR, LBDCF, 8, 1)   /* LIN break detection clear flag */
+    /* TCBGTCF only on STM32L496xx/4A6xx devices */
+    FIELD(ICR, TCCF, 6, 1)    /* Transmission complete clear flag */
+    FIELD(ICR, IDLECF, 4, 1)  /* Idle line detected clear flag */
+    FIELD(ICR, ORECF, 3, 1)   /* Overrun error clear flag */
+    FIELD(ICR, NCF, 2, 1)     /* Noise detected clear flag */
+    FIELD(ICR, FECF, 1, 1)    /* Framing error clear flag */
+    FIELD(ICR, PECF, 0, 1)    /* Parity error clear flag */
+REG32(RDR, 0x24)
+    FIELD(RDR, RDR, 0, 9)
+REG32(TDR, 0x28)
+    FIELD(TDR, TDR, 0, 9)
+
+static void stm32l4x5_usart_base_reset_hold(Object *obj)
+{
+    Stm32l4x5UsartBaseState *s = STM32L4X5_USART_BASE(obj);
+
+    s->cr1 = 0x00000000;
+    s->cr2 = 0x00000000;
+    s->cr3 = 0x00000000;
+    s->brr = 0x00000000;
+    s->gtpr = 0x00000000;
+    s->rtor = 0x00000000;
+    s->isr = 0x020000C0;
+    s->rdr = 0x00000000;
+    s->tdr = 0x00000000;
+}
+
+static uint64_t stm32l4x5_usart_base_read(void *opaque, hwaddr addr,
+                                     unsigned int size)
+{
+    Stm32l4x5UsartBaseState *s = opaque;
+    uint64_t retvalue = 0;
+
+    switch (addr) {
+    case A_CR1:
+        retvalue = s->cr1;
+        break;
+    case A_CR2:
+        retvalue = s->cr2;
+        break;
+    case A_CR3:
+        retvalue = s->cr3;
+        break;
+    case A_BRR:
+        retvalue = FIELD_EX32(s->brr, BRR, BRR);
+        break;
+    case A_GTPR:
+        retvalue = s->gtpr;
+        break;
+    case A_RTOR:
+        retvalue = s->rtor;
+        break;
+    case A_RQR:
+        /* RQR is a write only register */
+        retvalue = 0x00000000;
+        break;
+    case A_ISR:
+        retvalue = s->isr;
+        break;
+    case A_ICR:
+        /* ICR is a clear register */
+        retvalue = 0x00000000;
+        break;
+    case A_RDR:
+        retvalue = FIELD_EX32(s->rdr, RDR, RDR);
+        /* Reset RXNE flag */
+        s->isr &= ~R_ISR_RXNE_MASK;
+        break;
+    case A_TDR:
+        retvalue = FIELD_EX32(s->tdr, TDR, TDR);
+        break;
+    default:
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "%s: Bad offset 0x%"HWADDR_PRIx"\n", __func__, addr);
+        break;
+    }
+
+    trace_stm32l4x5_usart_read(addr, retvalue);
+
+    return retvalue;
+}
+
+static void stm32l4x5_usart_base_write(void *opaque, hwaddr addr,
+                                  uint64_t val64, unsigned int size)
+{
+    Stm32l4x5UsartBaseState *s = opaque;
+    const uint32_t value = val64;
+
+    trace_stm32l4x5_usart_write(addr, value);
+
+    switch (addr) {
+    case A_CR1:
+        s->cr1 = value;
+        return;
+    case A_CR2:
+        s->cr2 = value;
+        return;
+    case A_CR3:
+        s->cr3 = value;
+        return;
+    case A_BRR:
+        s->brr = value;
+        return;
+    case A_GTPR:
+        s->gtpr = value;
+        return;
+    case A_RTOR:
+        s->rtor = value;
+        return;
+    case A_RQR:
+        return;
+    case A_ISR:
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "%s: ISR is read only !\n", __func__);
+        return;
+    case A_ICR:
+        /* Clear the status flags */
+        s->isr &= ~value;
+        return;
+    case A_RDR:
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "%s: RDR is read only !\n", __func__);
+        return;
+    case A_TDR:
+        s->tdr = value;
+        return;
+    default:
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "%s: Bad offset 0x%"HWADDR_PRIx"\n", __func__, addr);
+    }
+}
+
+static const MemoryRegionOps stm32l4x5_usart_base_ops = {
+    .read = stm32l4x5_usart_base_read,
+    .write = stm32l4x5_usart_base_write,
+    .endianness = DEVICE_NATIVE_ENDIAN,
+    .valid = {
+        .max_access_size = 4,
+        .min_access_size = 4,
+        .unaligned = false
+    },
+    .impl = {
+        .max_access_size = 4,
+        .min_access_size = 4,
+        .unaligned = false
+    },
+};
+
+static Property stm32l4x5_usart_base_properties[] = {
+    DEFINE_PROP_CHR("chardev", Stm32l4x5UsartBaseState, chr),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void stm32l4x5_usart_base_init(Object *obj)
+{
+    Stm32l4x5UsartBaseState *s = STM32L4X5_USART_BASE(obj);
+
+    sysbus_init_irq(SYS_BUS_DEVICE(obj), &s->irq);
+
+    memory_region_init_io(&s->mmio, obj, &stm32l4x5_usart_base_ops, s,
+                          TYPE_STM32L4X5_USART_BASE, 0x400);
+    sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->mmio);
+
+    s->clk = qdev_init_clock_in(DEVICE(s), "clk", NULL, s, 0);
+}
+
+static const VMStateDescription vmstate_stm32l4x5_usart_base = {
+    .name = TYPE_STM32L4X5_USART_BASE,
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT32(cr1, Stm32l4x5UsartBaseState),
+        VMSTATE_UINT32(cr2, Stm32l4x5UsartBaseState),
+        VMSTATE_UINT32(cr3, Stm32l4x5UsartBaseState),
+        VMSTATE_UINT32(brr, Stm32l4x5UsartBaseState),
+        VMSTATE_UINT32(gtpr, Stm32l4x5UsartBaseState),
+        VMSTATE_UINT32(rtor, Stm32l4x5UsartBaseState),
+        VMSTATE_UINT32(isr, Stm32l4x5UsartBaseState),
+        VMSTATE_UINT32(rdr, Stm32l4x5UsartBaseState),
+        VMSTATE_UINT32(tdr, Stm32l4x5UsartBaseState),
+        VMSTATE_CLOCK(clk, Stm32l4x5UsartBaseState),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+
+static void stm32l4x5_usart_base_realize(DeviceState *dev, Error **errp)
+{
+    ERRP_GUARD();
+    Stm32l4x5UsartBaseState *s = STM32L4X5_USART_BASE(dev);
+    if (!clock_has_source(s->clk)) {
+        error_setg(errp, "USART clock must be wired up by SoC code");
+        return;
+    }
+}
+
+static void stm32l4x5_usart_base_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    ResettableClass *rc = RESETTABLE_CLASS(klass);
+
+    rc->phases.hold = stm32l4x5_usart_base_reset_hold;
+    device_class_set_props(dc, stm32l4x5_usart_base_properties);
+    dc->realize = stm32l4x5_usart_base_realize;
+    dc->vmsd = &vmstate_stm32l4x5_usart_base;
+}
+
+static void stm32l4x5_usart_class_init(ObjectClass *oc, void *data)
+{
+    Stm32l4x5UsartBaseClass *subc = STM32L4X5_USART_BASE_CLASS(oc);
+
+    subc->type = STM32L4x5_USART;
+}
+
+static void stm32l4x5_uart_class_init(ObjectClass *oc, void *data)
+{
+    Stm32l4x5UsartBaseClass *subc = STM32L4X5_USART_BASE_CLASS(oc);
+
+    subc->type = STM32L4x5_UART;
+}
+
+static void stm32l4x5_lpuart_class_init(ObjectClass *oc, void *data)
+{
+    Stm32l4x5UsartBaseClass *subc = STM32L4X5_USART_BASE_CLASS(oc);
+
+    subc->type = STM32L4x5_LPUART;
+}
+
+static const TypeInfo stm32l4x5_usart_types[] = {
+    {
+        .name           = TYPE_STM32L4X5_USART_BASE,
+        .parent         = TYPE_SYS_BUS_DEVICE,
+        .instance_size  = sizeof(Stm32l4x5UsartBaseState),
+        .instance_init  = stm32l4x5_usart_base_init,
+        .class_init     = stm32l4x5_usart_base_class_init,
+    }, {
+        .name           = TYPE_STM32L4X5_USART,
+        .parent         = TYPE_STM32L4X5_USART_BASE,
+        .class_init     = stm32l4x5_usart_class_init,
+    }, {
+        .name           = TYPE_STM32L4X5_UART,
+        .parent         = TYPE_STM32L4X5_USART_BASE,
+        .class_init     = stm32l4x5_uart_class_init,
+    }, {
+        .name           = TYPE_STM32L4X5_LPUART,
+        .parent         = TYPE_STM32L4X5_USART_BASE,
+        .class_init     = stm32l4x5_lpuart_class_init,
+    }
+};
+
+DEFINE_TYPES(stm32l4x5_usart_types)
diff --git a/hw/char/trace-events b/hw/char/trace-events
index 7a398c82a5..689bed9e02 100644
--- a/hw/char/trace-events
+++ b/hw/char/trace-events
@@ -106,6 +106,10 @@ cadence_uart_baudrate(unsigned baudrate) "baudrate %u"
 sh_serial_read(char *id, unsigned size, uint64_t offs, uint64_t val) " %s size %d offs 0x%02" PRIx64 " -> 0x%02" PRIx64
 sh_serial_write(char *id, unsigned size, uint64_t offs, uint64_t val) "%s size %d offs 0x%02" PRIx64 " <- 0x%02" PRIx64
 
+# stm32l4x5_usart.c
+stm32l4x5_usart_read(uint64_t addr, uint32_t data) "USART: Read <0x%" PRIx64 "> -> 0x%" PRIx32 ""
+stm32l4x5_usart_write(uint64_t addr, uint32_t data) "USART: Write <0x%" PRIx64 "> <- 0x%" PRIx32 ""
+
 # xen_console.c
 xen_console_connect(unsigned int idx, unsigned int ring_ref, unsigned int port, unsigned int limit) "idx %u ring_ref %u port %u limit %u"
 xen_console_disconnect(unsigned int idx) "idx %u"
diff --git a/include/hw/char/stm32l4x5_usart.h b/include/hw/char/stm32l4x5_usart.h
new file mode 100644
index 0000000000..8d38a85a6e
--- /dev/null
+++ b/include/hw/char/stm32l4x5_usart.h
@@ -0,0 +1,66 @@
+/*
+ * STM32L4X5 USART (Universal Synchronous Asynchronous Receiver Transmitter)
+ *
+ * Copyright (c) 2023 Arnaud Minier <arnaud.minier@telecom-paris.fr>
+ * Copyright (c) 2023 Inès Varhol <ines.varhol@telecom-paris.fr>
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ * The STM32L4X5 USART is heavily inspired by the stm32f2xx_usart
+ * by Alistair Francis.
+ * The reference used is the STMicroElectronics RM0351 Reference manual
+ * for STM32L4x5 and STM32L4x6 advanced Arm ® -based 32-bit MCUs.
+ */
+
+#ifndef HW_STM32L4X5_USART_H
+#define HW_STM32L4X5_USART_H
+
+#include "hw/sysbus.h"
+#include "chardev/char-fe.h"
+#include "qom/object.h"
+
+#define TYPE_STM32L4X5_USART_BASE "stm32l4x5-usart-base"
+#define TYPE_STM32L4X5_USART "stm32l4x5-usart"
+#define TYPE_STM32L4X5_UART "stm32l4x5-uart"
+#define TYPE_STM32L4X5_LPUART "stm32l4x5-lpuart"
+OBJECT_DECLARE_TYPE(Stm32l4x5UsartBaseState, Stm32l4x5UsartBaseClass,
+                    STM32L4X5_USART_BASE)
+
+typedef enum {
+    STM32L4x5_USART,
+    STM32L4x5_UART,
+    STM32L4x5_LPUART,
+} Stm32l4x5UsartType;
+
+struct Stm32l4x5UsartBaseState {
+    SysBusDevice parent_obj;
+
+    MemoryRegion mmio;
+
+    uint32_t cr1;
+    uint32_t cr2;
+    uint32_t cr3;
+    uint32_t brr;
+    uint32_t gtpr;
+    uint32_t rtor;
+    /* rqr is write-only */
+    uint32_t isr;
+    /* icr is a clear register */
+    uint32_t rdr;
+    uint32_t tdr;
+
+    Clock *clk;
+    CharBackend chr;
+    qemu_irq irq;
+};
+
+struct Stm32l4x5UsartBaseClass {
+    SysBusDeviceClass parent_class;
+
+    Stm32l4x5UsartType type;
+};
+
+#endif /* HW_STM32L4X5_USART_H */
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 3/6] hw/char/stm32l4x5_usart: Enable serial read and write
  2024-03-24 16:55 [PATCH v2 0/6] hw/char: Implement the STM32L4x5 USART, UART and LPUART Arnaud Minier
  2024-03-24 16:55 ` [PATCH v2 1/6] hw/misc/stm32l4x5_rcc: Propagate period when enabling a clock Arnaud Minier
  2024-03-24 16:55 ` [PATCH v2 2/6] hw/char: Implement STM32L4x5 USART skeleton Arnaud Minier
@ 2024-03-24 16:55 ` Arnaud Minier
  2024-03-28 15:59   ` Peter Maydell
  2024-03-24 16:55 ` [PATCH v2 4/6] hw/char/stm32l4x5_usart: Add options for serial parameters setting Arnaud Minier
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 14+ messages in thread
From: Arnaud Minier @ 2024-03-24 16:55 UTC (permalink / raw)
  To: qemu-devel
  Cc: Laurent Vivier, Marc-André Lureau, qemu-arm, Paolo Bonzini,
	Alistair Francis, Arnaud Minier, Thomas Huth, Inès Varhol,
	Samuel Tardieu, Peter Maydell

Implement the ability to read and write characters to the
usart using the serial port.

The character transmission is based on the
cmsdk-apb-uart implementation.

Signed-off-by: Arnaud Minier <arnaud.minier@telecom-paris.fr>
Signed-off-by: Inès Varhol <ines.varhol@telecom-paris.fr>
---
 hw/char/stm32l4x5_usart.c         | 139 ++++++++++++++++++++++++++++++
 hw/char/trace-events              |   7 ++
 include/hw/char/stm32l4x5_usart.h |   1 +
 3 files changed, 147 insertions(+)

diff --git a/hw/char/stm32l4x5_usart.c b/hw/char/stm32l4x5_usart.c
index 46e69bb096..ec8c2f6e63 100644
--- a/hw/char/stm32l4x5_usart.c
+++ b/hw/char/stm32l4x5_usart.c
@@ -154,6 +154,119 @@ REG32(RDR, 0x24)
 REG32(TDR, 0x28)
     FIELD(TDR, TDR, 0, 9)
 
+static void stm32l4x5_update_irq(Stm32l4x5UsartBaseState *s)
+{
+    if (((s->isr & R_ISR_WUF_MASK) && (s->cr3 & R_CR3_WUFIE_MASK))        ||
+        ((s->isr & R_ISR_CMF_MASK) && (s->cr1 & R_CR1_CMIE_MASK))         ||
+        ((s->isr & R_ISR_ABRF_MASK) && (s->cr1 & R_CR1_RXNEIE_MASK))      ||
+        ((s->isr & R_ISR_EOBF_MASK) && (s->cr1 & R_CR1_EOBIE_MASK))       ||
+        ((s->isr & R_ISR_RTOF_MASK) && (s->cr1 & R_CR1_RTOIE_MASK))       ||
+        ((s->isr & R_ISR_CTSIF_MASK) && (s->cr3 & R_CR3_CTSIE_MASK))      ||
+        ((s->isr & R_ISR_LBDF_MASK) && (s->cr2 & R_CR2_LBDIE_MASK))       ||
+        ((s->isr & R_ISR_TXE_MASK) && (s->cr1 & R_CR1_TXEIE_MASK))        ||
+        ((s->isr & R_ISR_TC_MASK) && (s->cr1 & R_CR1_TCIE_MASK))          ||
+        ((s->isr & R_ISR_RXNE_MASK) && (s->cr1 & R_CR1_RXNEIE_MASK))      ||
+        ((s->isr & R_ISR_IDLE_MASK) && (s->cr1 & R_CR1_IDLEIE_MASK))      ||
+        ((s->isr & R_ISR_ORE_MASK) &&
+            ((s->cr1 & R_CR1_RXNEIE_MASK) || (s->cr3 & R_CR3_EIE_MASK)))  ||
+        /* TODO: Handle NF ? */
+        ((s->isr & R_ISR_FE_MASK) && (s->cr3 & R_CR3_EIE_MASK))           ||
+        ((s->isr & R_ISR_PE_MASK) && (s->cr1 & R_CR1_PEIE_MASK))) {
+        qemu_irq_raise(s->irq);
+        trace_stm32l4x5_usart_irq_raised(s->isr);
+    } else {
+        qemu_irq_lower(s->irq);
+        trace_stm32l4x5_usart_irq_lowered();
+    }
+}
+
+static int stm32l4x5_usart_base_can_receive(void *opaque)
+{
+    Stm32l4x5UsartBaseState *s = opaque;
+
+    if (!(s->isr & R_ISR_RXNE_MASK)) {
+        return 1;
+    }
+
+    return 0;
+}
+
+static void stm32l4x5_usart_base_receive(void *opaque, const uint8_t *buf, int size)
+{
+    Stm32l4x5UsartBaseState *s = opaque;
+
+    if (!((s->cr1 & R_CR1_UE_MASK) && (s->cr1 & R_CR1_RE_MASK))) {
+        trace_stm32l4x5_usart_receiver_not_enabled(
+            FIELD_EX32(s->cr1, CR1, UE), FIELD_EX32(s->cr1, CR1, RE));
+        return;
+    }
+
+    /* Check if overrun detection is enabled and if there is an overrun */
+    if (!(s->cr3 & R_CR3_OVRDIS_MASK) && (s->isr & R_ISR_RXNE_MASK)) {
+        /*
+         * A character has been received while
+         * the previous has not been read = Overrun.
+         */
+        s->isr |= R_ISR_ORE_MASK;
+        trace_stm32l4x5_usart_overrun_detected(s->rdr, *buf);
+    } else {
+        /* No overrun */
+        s->rdr = *buf;
+        s->isr |= R_ISR_RXNE_MASK;
+        trace_stm32l4x5_usart_rx(s->rdr);
+    }
+
+    stm32l4x5_update_irq(s);
+}
+
+/* Try to send tx data, and arrange to be called back later if
+ * we can't (ie the char backend is busy/blocking).
+ */
+static gboolean usart_transmit(void *do_not_use, GIOCondition cond, void *opaque)
+{
+    Stm32l4x5UsartBaseState *s = STM32L4X5_USART_BASE(opaque);
+    int ret;
+    /* TODO: Handle 9 bits transmission */
+    uint8_t ch = s->tdr;
+
+    s->watch_tag = 0;
+
+    if (!(s->cr1 & R_CR1_TE_MASK) || (s->isr & R_ISR_TXE_MASK)) {
+        return G_SOURCE_REMOVE;
+    }
+
+    ret = qemu_chr_fe_write(&s->chr, &ch, 1);
+    if (ret <= 0) {
+        s->watch_tag = qemu_chr_fe_add_watch(&s->chr, G_IO_OUT | G_IO_HUP,
+                                             usart_transmit, s);
+        if (!s->watch_tag) {
+            /* Most common reason to be here is "no chardev backend":
+             * just insta-drain the buffer, so the serial output
+             * goes into a void, rather than blocking the guest.
+             */
+            goto buffer_drained;
+        }
+        /* Transmit pending */
+        trace_stm32l4x5_usart_tx_pending();
+        return G_SOURCE_REMOVE;
+    }
+
+buffer_drained:
+    /* Character successfully sent */
+    trace_stm32l4x5_usart_tx(ch);
+    s->isr |= R_ISR_TC_MASK | R_ISR_TXE_MASK;
+    stm32l4x5_update_irq(s);
+    return G_SOURCE_REMOVE;
+}
+
+static void usart_cancel_transmit(Stm32l4x5UsartBaseState *s)
+{
+    if (s->watch_tag) {
+        g_source_remove(s->watch_tag);
+        s->watch_tag = 0;
+    }
+}
+
 static void stm32l4x5_usart_base_reset_hold(Object *obj)
 {
     Stm32l4x5UsartBaseState *s = STM32L4X5_USART_BASE(obj);
@@ -167,6 +280,22 @@ static void stm32l4x5_usart_base_reset_hold(Object *obj)
     s->isr = 0x020000C0;
     s->rdr = 0x00000000;
     s->tdr = 0x00000000;
+
+    usart_cancel_transmit(s);
+    stm32l4x5_update_irq(s);
+}
+
+static void usart_update_rqr(Stm32l4x5UsartBaseState *s, uint32_t value)
+{
+    /* TXFRQ */
+    /* Reset RXNE flag */
+    if (value & R_RQR_RXFRQ_MASK) {
+        s->isr &= ~R_ISR_RXNE_MASK;
+    }
+    /* MMRQ */
+    /* SBKRQ */
+    /* ABRRQ */
+    stm32l4x5_update_irq(s);
 }
 
 static uint64_t stm32l4x5_usart_base_read(void *opaque, hwaddr addr,
@@ -209,6 +338,7 @@ static uint64_t stm32l4x5_usart_base_read(void *opaque, hwaddr addr,
         retvalue = FIELD_EX32(s->rdr, RDR, RDR);
         /* Reset RXNE flag */
         s->isr &= ~R_ISR_RXNE_MASK;
+        stm32l4x5_update_irq(s);
         break;
     case A_TDR:
         retvalue = FIELD_EX32(s->tdr, TDR, TDR);
@@ -235,6 +365,7 @@ static void stm32l4x5_usart_base_write(void *opaque, hwaddr addr,
     switch (addr) {
     case A_CR1:
         s->cr1 = value;
+        stm32l4x5_update_irq(s);
         return;
     case A_CR2:
         s->cr2 = value;
@@ -252,6 +383,7 @@ static void stm32l4x5_usart_base_write(void *opaque, hwaddr addr,
         s->rtor = value;
         return;
     case A_RQR:
+        usart_update_rqr(s, value);
         return;
     case A_ISR:
         qemu_log_mask(LOG_GUEST_ERROR,
@@ -260,6 +392,7 @@ static void stm32l4x5_usart_base_write(void *opaque, hwaddr addr,
     case A_ICR:
         /* Clear the status flags */
         s->isr &= ~value;
+        stm32l4x5_update_irq(s);
         return;
     case A_RDR:
         qemu_log_mask(LOG_GUEST_ERROR,
@@ -267,6 +400,8 @@ static void stm32l4x5_usart_base_write(void *opaque, hwaddr addr,
         return;
     case A_TDR:
         s->tdr = value;
+        s->isr &= ~R_ISR_TXE_MASK;
+        usart_transmit(NULL, G_IO_OUT, s);
         return;
     default:
         qemu_log_mask(LOG_GUEST_ERROR,
@@ -336,6 +471,10 @@ static void stm32l4x5_usart_base_realize(DeviceState *dev, Error **errp)
         error_setg(errp, "USART clock must be wired up by SoC code");
         return;
     }
+
+    qemu_chr_fe_set_handlers(&s->chr, stm32l4x5_usart_base_can_receive,
+                             stm32l4x5_usart_base_receive, NULL, NULL,
+                             s, NULL, true);
 }
 
 static void stm32l4x5_usart_base_class_init(ObjectClass *klass, void *data)
diff --git a/hw/char/trace-events b/hw/char/trace-events
index 689bed9e02..f22f0ee2bc 100644
--- a/hw/char/trace-events
+++ b/hw/char/trace-events
@@ -109,6 +109,13 @@ sh_serial_write(char *id, unsigned size, uint64_t offs, uint64_t val) "%s size %
 # stm32l4x5_usart.c
 stm32l4x5_usart_read(uint64_t addr, uint32_t data) "USART: Read <0x%" PRIx64 "> -> 0x%" PRIx32 ""
 stm32l4x5_usart_write(uint64_t addr, uint32_t data) "USART: Write <0x%" PRIx64 "> <- 0x%" PRIx32 ""
+stm32l4x5_usart_rx(uint8_t c) "USART: got character 0x%x from backend"
+stm32l4x5_usart_tx(uint8_t c) "USART: character 0x%x sent to backend"
+stm32l4x5_usart_tx_pending(void) "USART: character send to backend pending"
+stm32l4x5_usart_irq_raised(uint32_t reg) "USART: IRQ raised: 0x%08"PRIx32
+stm32l4x5_usart_irq_lowered(void) "USART: IRQ lowered"
+stm32l4x5_usart_overrun_detected(uint8_t current, uint8_t received) "USART: Overrun detected, RDR='0x%x', received 0x%x"
+stm32l4x5_usart_receiver_not_enabled(uint8_t ue_bit, uint8_t re_bit) "USART: Receiver not enabled, UE=0x%x, RE=0x%x"
 
 # xen_console.c
 xen_console_connect(unsigned int idx, unsigned int ring_ref, unsigned int port, unsigned int limit) "idx %u ring_ref %u port %u limit %u"
diff --git a/include/hw/char/stm32l4x5_usart.h b/include/hw/char/stm32l4x5_usart.h
index 8d38a85a6e..dd3866682a 100644
--- a/include/hw/char/stm32l4x5_usart.h
+++ b/include/hw/char/stm32l4x5_usart.h
@@ -55,6 +55,7 @@ struct Stm32l4x5UsartBaseState {
     Clock *clk;
     CharBackend chr;
     qemu_irq irq;
+    guint watch_tag;
 };
 
 struct Stm32l4x5UsartBaseClass {
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 4/6] hw/char/stm32l4x5_usart: Add options for serial parameters setting
  2024-03-24 16:55 [PATCH v2 0/6] hw/char: Implement the STM32L4x5 USART, UART and LPUART Arnaud Minier
                   ` (2 preceding siblings ...)
  2024-03-24 16:55 ` [PATCH v2 3/6] hw/char/stm32l4x5_usart: Enable serial read and write Arnaud Minier
@ 2024-03-24 16:55 ` Arnaud Minier
  2024-03-28 16:03   ` Peter Maydell
  2024-03-24 16:55 ` [PATCH v2 5/6] hw/arm: Add the USART to the stm32l4x5 SoC Arnaud Minier
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 14+ messages in thread
From: Arnaud Minier @ 2024-03-24 16:55 UTC (permalink / raw)
  To: qemu-devel
  Cc: Laurent Vivier, Marc-André Lureau, qemu-arm, Paolo Bonzini,
	Alistair Francis, Arnaud Minier, Thomas Huth, Inès Varhol,
	Samuel Tardieu, Peter Maydell

Add a function to change the settings of the
serial connection.

Signed-off-by: Arnaud Minier <arnaud.minier@telecom-paris.fr>
Signed-off-by: Inès Varhol <ines.varhol@telecom-paris.fr>
---
 hw/char/stm32l4x5_usart.c | 98 +++++++++++++++++++++++++++++++++++++++
 hw/char/trace-events      |  1 +
 2 files changed, 99 insertions(+)

diff --git a/hw/char/stm32l4x5_usart.c b/hw/char/stm32l4x5_usart.c
index ec8c2f6e63..b4d11dd826 100644
--- a/hw/char/stm32l4x5_usart.c
+++ b/hw/char/stm32l4x5_usart.c
@@ -267,6 +267,92 @@ static void usart_cancel_transmit(Stm32l4x5UsartBaseState *s)
     }
 }
 
+static void stm32l4x5_update_params(Stm32l4x5UsartBaseState *s)
+{
+    int speed, parity, data_bits, stop_bits;
+    uint32_t value, usart_div;
+    QEMUSerialSetParams ssp;
+
+    /* Select the parity type */
+    if (s->cr1 & R_CR1_PCE_MASK) {
+        if (s->cr1 & R_CR1_PS_MASK) {
+            parity = 'O';
+        } else {
+            parity = 'E';
+        }
+    } else {
+        parity = 'N';
+    }
+
+    /* Select the number of stop bits */
+    switch (FIELD_EX32(s->cr2, CR2, STOP)) {
+    case 0:
+        stop_bits = 1;
+        break;
+    case 2:
+        stop_bits = 2;
+        break;
+    default:
+        qemu_log_mask(LOG_UNIMP,
+            "UNIMPLEMENTED: fractionnal stop bits; CR2[13:12] = %x",
+            FIELD_EX32(s->cr2, CR2, STOP));
+        return;
+    }
+
+    /* Select the length of the word */
+    switch ((FIELD_EX32(s->cr1, CR1, M1) << 1) | FIELD_EX32(s->cr1, CR1, M0)) {
+    case 0:
+        data_bits = 8;
+        break;
+    case 1:
+        data_bits = 9;
+        break;
+    case 2:
+        data_bits = 7;
+        break;
+    default:
+        qemu_log_mask(LOG_GUEST_ERROR,
+            "UNDEFINED: invalid word length, CR1.M = 0b11");
+        return;
+    }
+
+    /* Select the baud rate */
+    value = FIELD_EX32(s->brr, BRR, BRR);
+    if (value < 16) {
+        qemu_log_mask(LOG_GUEST_ERROR,
+            "UNDEFINED: BRR lesser than 16: %u", value);
+        return;
+    }
+
+    if (FIELD_EX32(s->cr1, CR1, OVER8) == 0) {
+        /*
+         * Oversampling by 16
+         * BRR = USARTDIV
+         */
+        usart_div = value;
+    } else {
+        /*
+         * Oversampling by 8
+         * - BRR[2:0] = USARTDIV[3:0] shifted 1 bit to the right.
+         * - BRR[3] must be kept cleared.
+         * - BRR[15:4] = USARTDIV[15:4]
+         * - The frequency is multiplied by 2
+         */
+        usart_div = ((value & 0xFFF0) | ((value & 0x0007) << 1)) / 2;
+    }
+
+    speed = clock_get_hz(s->clk) / usart_div;
+
+    ssp.speed     = speed;
+    ssp.parity    = parity;
+    ssp.data_bits = data_bits;
+    ssp.stop_bits = stop_bits;
+
+    qemu_chr_fe_ioctl(&s->chr, CHR_IOCTL_SERIAL_SET_PARAMS, &ssp);
+
+    trace_stm32l4x5_usart_update_params(speed, parity, data_bits, stop_bits);
+}
+
 static void stm32l4x5_usart_base_reset_hold(Object *obj)
 {
     Stm32l4x5UsartBaseState *s = STM32L4X5_USART_BASE(obj);
@@ -365,16 +451,19 @@ static void stm32l4x5_usart_base_write(void *opaque, hwaddr addr,
     switch (addr) {
     case A_CR1:
         s->cr1 = value;
+        stm32l4x5_update_params(s);
         stm32l4x5_update_irq(s);
         return;
     case A_CR2:
         s->cr2 = value;
+        stm32l4x5_update_params(s);
         return;
     case A_CR3:
         s->cr3 = value;
         return;
     case A_BRR:
         s->brr = value;
+        stm32l4x5_update_params(s);
         return;
     case A_GTPR:
         s->gtpr = value;
@@ -443,10 +532,19 @@ static void stm32l4x5_usart_base_init(Object *obj)
     s->clk = qdev_init_clock_in(DEVICE(s), "clk", NULL, s, 0);
 }
 
+static int stm32l4x5_usart_base_post_load(void *opaque, int version_id)
+{
+    Stm32l4x5UsartBaseState *s = (Stm32l4x5UsartBaseState *)opaque;
+
+    stm32l4x5_update_params(s);
+    return 0;
+}
+
 static const VMStateDescription vmstate_stm32l4x5_usart_base = {
     .name = TYPE_STM32L4X5_USART_BASE,
     .version_id = 1,
     .minimum_version_id = 1,
+    .post_load = stm32l4x5_usart_base_post_load,
     .fields = (VMStateField[]) {
         VMSTATE_UINT32(cr1, Stm32l4x5UsartBaseState),
         VMSTATE_UINT32(cr2, Stm32l4x5UsartBaseState),
diff --git a/hw/char/trace-events b/hw/char/trace-events
index f22f0ee2bc..8875758076 100644
--- a/hw/char/trace-events
+++ b/hw/char/trace-events
@@ -116,6 +116,7 @@ stm32l4x5_usart_irq_raised(uint32_t reg) "USART: IRQ raised: 0x%08"PRIx32
 stm32l4x5_usart_irq_lowered(void) "USART: IRQ lowered"
 stm32l4x5_usart_overrun_detected(uint8_t current, uint8_t received) "USART: Overrun detected, RDR='0x%x', received 0x%x"
 stm32l4x5_usart_receiver_not_enabled(uint8_t ue_bit, uint8_t re_bit) "USART: Receiver not enabled, UE=0x%x, RE=0x%x"
+stm32l4x5_usart_update_params(int speed, uint8_t parity, int data, int stop) "USART: speed: %d, parity: %c, data bits: %d, stop bits: %d"
 
 # xen_console.c
 xen_console_connect(unsigned int idx, unsigned int ring_ref, unsigned int port, unsigned int limit) "idx %u ring_ref %u port %u limit %u"
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 5/6] hw/arm: Add the USART to the stm32l4x5 SoC
  2024-03-24 16:55 [PATCH v2 0/6] hw/char: Implement the STM32L4x5 USART, UART and LPUART Arnaud Minier
                   ` (3 preceding siblings ...)
  2024-03-24 16:55 ` [PATCH v2 4/6] hw/char/stm32l4x5_usart: Add options for serial parameters setting Arnaud Minier
@ 2024-03-24 16:55 ` Arnaud Minier
  2024-03-28 16:06   ` Peter Maydell
  2024-03-24 16:55 ` [PATCH v2 6/6] tests/qtest: Add tests for the STM32L4x5 USART Arnaud Minier
  2024-03-28 16:10 ` [PATCH v2 0/6] hw/char: Implement the STM32L4x5 USART, UART and LPUART Peter Maydell
  6 siblings, 1 reply; 14+ messages in thread
From: Arnaud Minier @ 2024-03-24 16:55 UTC (permalink / raw)
  To: qemu-devel
  Cc: Laurent Vivier, Marc-André Lureau, qemu-arm, Paolo Bonzini,
	Alistair Francis, Arnaud Minier, Thomas Huth, Inès Varhol,
	Samuel Tardieu, Peter Maydell

Add the USART to the SoC and connect it to the other implemented devices.

Signed-off-by: Arnaud Minier <arnaud.minier@telecom-paris.fr>
Signed-off-by: Inès Varhol <ines.varhol@telecom-paris.fr>
---
 docs/system/arm/b-l475e-iot01a.rst |  2 +-
 hw/arm/Kconfig                     |  1 +
 hw/arm/stm32l4x5_soc.c             | 82 +++++++++++++++++++++++++++---
 include/hw/arm/stm32l4x5_soc.h     | 13 +++++
 4 files changed, 91 insertions(+), 7 deletions(-)

diff --git a/docs/system/arm/b-l475e-iot01a.rst b/docs/system/arm/b-l475e-iot01a.rst
index 0afef8e4f4..a76c9976c5 100644
--- a/docs/system/arm/b-l475e-iot01a.rst
+++ b/docs/system/arm/b-l475e-iot01a.rst
@@ -19,13 +19,13 @@ Currently B-L475E-IOT01A machine's only supports the following devices:
 - STM32L4x5 SYSCFG (System configuration controller)
 - STM32L4x5 RCC (Reset and clock control)
 - STM32L4x5 GPIOs (General-purpose I/Os)
+- STM32L4x5 USARTs, UARTs and LPUART (Serial ports)
 
 Missing devices
 """""""""""""""
 
 The B-L475E-IOT01A does *not* support the following devices:
 
-- Serial ports (UART)
 - Analog to Digital Converter (ADC)
 - SPI controller
 - Timer controller (TIMER)
diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index 893a7bff66..098d043375 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -477,6 +477,7 @@ config STM32L4X5_SOC
     select STM32L4X5_SYSCFG
     select STM32L4X5_RCC
     select STM32L4X5_GPIO
+    select STM32L4X5_USART
 
 config XLNX_ZYNQMP_ARM
     bool
diff --git a/hw/arm/stm32l4x5_soc.c b/hw/arm/stm32l4x5_soc.c
index 40e294f838..ae0868dcab 100644
--- a/hw/arm/stm32l4x5_soc.c
+++ b/hw/arm/stm32l4x5_soc.c
@@ -28,6 +28,7 @@
 #include "sysemu/sysemu.h"
 #include "hw/or-irq.h"
 #include "hw/arm/stm32l4x5_soc.h"
+#include "hw/char/stm32l4x5_usart.h"
 #include "hw/gpio/stm32l4x5_gpio.h"
 #include "hw/qdev-clock.h"
 #include "hw/misc/unimp.h"
@@ -116,6 +117,22 @@ static const struct {
     { 0x48001C00, 0x0000000F, 0x00000000, 0x00000000 },
 };
 
+static const hwaddr usart_addr[] = {
+    0x40013800, /* "USART1", 0x400 */
+    0x40004400, /* "USART2", 0x400 */
+    0x40004800, /* "USART3", 0x400 */
+};
+static const hwaddr uart_addr[] = {
+    0x40004C00, /* "UART4" , 0x400 */
+    0x40005000  /* "UART5" , 0x400 */
+};
+
+#define LPUART_BASE_ADDRESS 0x40008000
+
+static const int usart_irq[] = { 37, 38, 39 };
+static const int uart_irq[] = { 52, 53 };
+#define LPUART_IRQ 70
+
 static void stm32l4x5_soc_initfn(Object *obj)
 {
     Stm32l4x5SocState *s = STM32L4X5_SOC(obj);
@@ -132,6 +149,18 @@ static void stm32l4x5_soc_initfn(Object *obj)
         g_autofree char *name = g_strdup_printf("gpio%c", 'a' + i);
         object_initialize_child(obj, name, &s->gpio[i], TYPE_STM32L4X5_GPIO);
     }
+    
+    for (int i = 0; i < STM_NUM_USARTS; i++) {
+        object_initialize_child(obj, "usart[*]", &s->usart[i],
+                                TYPE_STM32L4X5_USART);
+    }
+
+    for (int i = 0; i < STM_NUM_UARTS; i++) {
+        object_initialize_child(obj, "uart[*]", &s->uart[i],
+                                TYPE_STM32L4X5_UART);
+    }
+    object_initialize_child(obj, "lpuart1", &s->lpuart,
+                            TYPE_STM32L4X5_LPUART);
 }
 
 static void stm32l4x5_soc_realize(DeviceState *dev_soc, Error **errp)
@@ -279,6 +308,53 @@ static void stm32l4x5_soc_realize(DeviceState *dev_soc, Error **errp)
     sysbus_mmio_map(busdev, 0, RCC_BASE_ADDRESS);
     sysbus_connect_irq(busdev, 0, qdev_get_gpio_in(armv7m, RCC_IRQ));
 
+    /* USART devices */
+    for (int i = 0; i < STM_NUM_USARTS; i++) {
+        dev = DEVICE(&(s->usart[i]));
+        qdev_prop_set_chr(dev, "chardev", serial_hd(i));
+        g_autofree char *name = g_strdup_printf("usart%d-out", i + 1);
+        qdev_connect_clock_in(dev, "clk",
+            qdev_get_clock_out(DEVICE(&(s->rcc)), name));
+        busdev = SYS_BUS_DEVICE(dev);
+        if (!sysbus_realize(busdev, errp)) {
+            return;
+        }
+        sysbus_mmio_map(busdev, 0, usart_addr[i]);
+        sysbus_connect_irq(busdev, 0, qdev_get_gpio_in(armv7m, usart_irq[i]));
+    }
+
+    /*
+     * TODO: Connect the USARTs, UARTs and LPUART to the EXTI once the EXTI
+     * can handle other gpio-in than the gpios. (e.g. Direct Lines for the usarts)
+     */
+
+    /* UART devices */
+    for (int i = 0; i < STM_NUM_UARTS; i++) {
+        dev = DEVICE(&(s->uart[i]));
+        qdev_prop_set_chr(dev, "chardev", serial_hd(STM_NUM_USARTS + i));
+        g_autofree char *name = g_strdup_printf("uart%d-out", STM_NUM_USARTS + i + 1);
+        qdev_connect_clock_in(dev, "clk",
+            qdev_get_clock_out(DEVICE(&(s->rcc)), name));
+        busdev = SYS_BUS_DEVICE(dev);
+        if (!sysbus_realize(busdev, errp)) {
+            return;
+        }
+        sysbus_mmio_map(busdev, 0, uart_addr[i]);
+        sysbus_connect_irq(busdev, 0, qdev_get_gpio_in(armv7m, uart_irq[i]));
+    }
+
+    /* LPUART device*/
+    dev = DEVICE(&(s->lpuart));
+    qdev_prop_set_chr(dev, "chardev", serial_hd(STM_NUM_USARTS + STM_NUM_UARTS));
+    qdev_connect_clock_in(dev, "clk",
+        qdev_get_clock_out(DEVICE(&(s->rcc)), "lpuart1-out"));
+    busdev = SYS_BUS_DEVICE(dev);
+    if (!sysbus_realize(busdev, errp)) {
+        return;
+    }
+    sysbus_mmio_map(busdev, 0, LPUART_BASE_ADDRESS);
+    sysbus_connect_irq(busdev, 0, qdev_get_gpio_in(armv7m, LPUART_IRQ));
+
     /* APB1 BUS */
     create_unimplemented_device("TIM2",      0x40000000, 0x400);
     create_unimplemented_device("TIM3",      0x40000400, 0x400);
@@ -294,10 +370,6 @@ static void stm32l4x5_soc_realize(DeviceState *dev_soc, Error **errp)
     create_unimplemented_device("SPI2",      0x40003800, 0x400);
     create_unimplemented_device("SPI3",      0x40003C00, 0x400);
     /* RESERVED:    0x40004000, 0x400 */
-    create_unimplemented_device("USART2",    0x40004400, 0x400);
-    create_unimplemented_device("USART3",    0x40004800, 0x400);
-    create_unimplemented_device("UART4",     0x40004C00, 0x400);
-    create_unimplemented_device("UART5",     0x40005000, 0x400);
     create_unimplemented_device("I2C1",      0x40005400, 0x400);
     create_unimplemented_device("I2C2",      0x40005800, 0x400);
     create_unimplemented_device("I2C3",      0x40005C00, 0x400);
@@ -308,7 +380,6 @@ static void stm32l4x5_soc_realize(DeviceState *dev_soc, Error **errp)
     create_unimplemented_device("DAC1",      0x40007400, 0x400);
     create_unimplemented_device("OPAMP",     0x40007800, 0x400);
     create_unimplemented_device("LPTIM1",    0x40007C00, 0x400);
-    create_unimplemented_device("LPUART1",   0x40008000, 0x400);
     /* RESERVED:    0x40008400, 0x400 */
     create_unimplemented_device("SWPMI1",    0x40008800, 0x400);
     /* RESERVED:    0x40008C00, 0x800 */
@@ -325,7 +396,6 @@ static void stm32l4x5_soc_realize(DeviceState *dev_soc, Error **errp)
     create_unimplemented_device("TIM1",      0x40012C00, 0x400);
     create_unimplemented_device("SPI1",      0x40013000, 0x400);
     create_unimplemented_device("TIM8",      0x40013400, 0x400);
-    create_unimplemented_device("USART1",    0x40013800, 0x400);
     /* RESERVED:    0x40013C00, 0x400 */
     create_unimplemented_device("TIM15",     0x40014000, 0x400);
     create_unimplemented_device("TIM16",     0x40014400, 0x400);
diff --git a/include/hw/arm/stm32l4x5_soc.h b/include/hw/arm/stm32l4x5_soc.h
index ee5f362405..a94ddbd19c 100644
--- a/include/hw/arm/stm32l4x5_soc.h
+++ b/include/hw/arm/stm32l4x5_soc.h
@@ -21,6 +21,12 @@
  * https://www.st.com/en/microcontrollers-microprocessors/stm32l4x5/documentation.html
  */
 
+/*
+ * The STM32L4X5 is heavily inspired by the stm32f405 by Alistair Francis.
+ * The reference used is the STMicroElectronics RM0351 Reference manual
+ * for STM32L4x5 and STM32L4x6 advanced Arm ® -based 32-bit MCUs.
+ */
+
 #ifndef HW_ARM_STM32L4x5_SOC_H
 #define HW_ARM_STM32L4x5_SOC_H
 
@@ -31,6 +37,7 @@
 #include "hw/misc/stm32l4x5_exti.h"
 #include "hw/misc/stm32l4x5_rcc.h"
 #include "hw/gpio/stm32l4x5_gpio.h"
+#include "hw/char/stm32l4x5_usart.h"
 #include "qom/object.h"
 
 #define TYPE_STM32L4X5_SOC "stm32l4x5-soc"
@@ -41,6 +48,9 @@ OBJECT_DECLARE_TYPE(Stm32l4x5SocState, Stm32l4x5SocClass, STM32L4X5_SOC)
 
 #define NUM_EXTI_OR_GATES 4
 
+#define STM_NUM_USARTS 3
+#define STM_NUM_UARTS 2
+
 struct Stm32l4x5SocState {
     SysBusDevice parent_obj;
 
@@ -51,6 +61,9 @@ struct Stm32l4x5SocState {
     Stm32l4x5SyscfgState syscfg;
     Stm32l4x5RccState rcc;
     Stm32l4x5GpioState gpio[NUM_GPIOS];
+    Stm32l4x5UsartBaseState usart[STM_NUM_USARTS];
+    Stm32l4x5UsartBaseState uart[STM_NUM_UARTS];
+    Stm32l4x5UsartBaseState lpuart;
 
     MemoryRegion sram1;
     MemoryRegion sram2;
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 6/6] tests/qtest: Add tests for the STM32L4x5 USART
  2024-03-24 16:55 [PATCH v2 0/6] hw/char: Implement the STM32L4x5 USART, UART and LPUART Arnaud Minier
                   ` (4 preceding siblings ...)
  2024-03-24 16:55 ` [PATCH v2 5/6] hw/arm: Add the USART to the stm32l4x5 SoC Arnaud Minier
@ 2024-03-24 16:55 ` Arnaud Minier
  2024-03-25  6:19   ` Thomas Huth
  2024-03-28 16:10 ` [PATCH v2 0/6] hw/char: Implement the STM32L4x5 USART, UART and LPUART Peter Maydell
  6 siblings, 1 reply; 14+ messages in thread
From: Arnaud Minier @ 2024-03-24 16:55 UTC (permalink / raw)
  To: qemu-devel
  Cc: Laurent Vivier, Marc-André Lureau, qemu-arm, Paolo Bonzini,
	Alistair Francis, Arnaud Minier, Thomas Huth, Inès Varhol,
	Samuel Tardieu, Peter Maydell

Test:
- read/write from/to the usart registers
- send/receive a character/string over the serial port

Signed-off-by: Arnaud Minier <arnaud.minier@telecom-paris.fr>
Signed-off-by: Inès Varhol <ines.varhol@telecom-paris.fr>
---
 tests/qtest/meson.build            |   3 +-
 tests/qtest/stm32l4x5_usart-test.c | 326 +++++++++++++++++++++++++++++
 2 files changed, 328 insertions(+), 1 deletion(-)
 create mode 100644 tests/qtest/stm32l4x5_usart-test.c

diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index 36c5c13a7b..e0d72ee91e 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -205,7 +205,8 @@ qtests_stm32l4x5 = \
   ['stm32l4x5_exti-test',
    'stm32l4x5_syscfg-test',
    'stm32l4x5_rcc-test',
-   'stm32l4x5_gpio-test']
+   'stm32l4x5_gpio-test',
+   'stm32l4x5_usart-test']
 
 qtests_arm = \
   (config_all_devices.has_key('CONFIG_MPS2') ? ['sse-timer-test'] : []) + \
diff --git a/tests/qtest/stm32l4x5_usart-test.c b/tests/qtest/stm32l4x5_usart-test.c
new file mode 100644
index 0000000000..2d42f053f6
--- /dev/null
+++ b/tests/qtest/stm32l4x5_usart-test.c
@@ -0,0 +1,326 @@
+/*
+ * QTest testcase for STML4X5_USART
+ *
+ * Copyright (c) 2023 Arnaud Minier <arnaud.minier@telecom-paris.fr>
+ * Copyright (c) 2023 Inès Varhol <ines.varhol@telecom-paris.fr>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "libqtest.h"
+#include "hw/misc/stm32l4x5_rcc_internals.h"
+#include "hw/registerfields.h"
+
+#define RCC_BASE_ADDR 0x40021000
+/* Use USART 1 ADDR, assume the others work the same */
+#define USART1_BASE_ADDR 0x40013800
+
+/* See stm32l4x5_usart for definitions */
+REG32(CR1, 0x00)
+    FIELD(CR1, M1, 28, 1)
+    FIELD(CR1, OVER8, 15, 1)
+    FIELD(CR1, M0, 12, 1)
+    FIELD(CR1, PCE, 10, 1)
+    FIELD(CR1, TXEIE, 7, 1)
+    FIELD(CR1, RXNEIE, 5, 1)
+    FIELD(CR1, TE, 3, 1)
+    FIELD(CR1, RE, 2, 1)
+    FIELD(CR1, UE, 0, 1)
+REG32(CR2, 0x04)
+REG32(CR3, 0x08)
+    FIELD(CR3, OVRDIS, 12, 1)
+REG32(BRR, 0x0C)
+REG32(GTPR, 0x10)
+REG32(RTOR, 0x14)
+REG32(RQR, 0x18)
+REG32(ISR, 0x1C)
+    FIELD(ISR, TXE, 7, 1)
+    FIELD(ISR, RXNE, 5, 1)
+    FIELD(ISR, ORE, 3, 1)
+REG32(ICR, 0x20)
+REG32(RDR, 0x24)
+REG32(TDR, 0x28)
+
+#define NVIC_ISPR1 0XE000E204
+#define NVIC_ICPR1 0xE000E284
+#define USART1_IRQ 37
+
+static bool check_nvic_pending(QTestState *qts, unsigned int n)
+{
+    /* No USART interrupts are less than 32 */
+    assert(n > 32);
+    n -= 32;
+    return qtest_readl(qts, NVIC_ISPR1) & (1 << n);
+}
+
+static bool clear_nvic_pending(QTestState *qts, unsigned int n)
+{
+    /* No USART interrupts are less than 32 */
+    assert(n > 32);
+    n -= 32;
+    qtest_writel(qts, NVIC_ICPR1, (1 << n));
+    return true;
+}
+
+/*
+ Tests should never need to sleep(), because while it might be plenty of time on a
+ fast development machine, it can cause intermittent failures due
+ to timeouts if the test is on some heavily-loaded slow CI runner.
+ */
+static bool usart_wait_for_flag(QTestState *qts, uint32_t event_addr, uint32_t flag)
+{
+    /* Wait at most 10 minutes */
+    for (int i = 0; i < 600000; i++) {
+        if ((qtest_readl(qts, event_addr) & flag)) {
+            return true;
+        }
+        g_usleep(1000);
+    }
+
+    return false;
+}
+
+static void usart_receive_string(QTestState *qts, int sock_fd, const char *in,
+                           char *out)
+{
+    int i, in_len = strlen(in);
+
+    g_assert_true(send(sock_fd, in, in_len, 0) == in_len);
+    for (i = 0; i < in_len; i++) {
+        g_assert_true(usart_wait_for_flag(qts,
+            USART1_BASE_ADDR + A_ISR, R_ISR_RXNE_MASK));
+        out[i] = qtest_readl(qts, USART1_BASE_ADDR + A_RDR);
+    }
+    out[i] = '\0';
+}
+
+static void usart_send_string(QTestState *qts, const char *in)
+{
+    int i, in_len = strlen(in);
+
+    for (i = 0; i < in_len; i++) {
+        qtest_writel(qts, USART1_BASE_ADDR + A_TDR, in[i]);
+        g_assert_true(usart_wait_for_flag(qts,
+            USART1_BASE_ADDR + A_ISR, R_ISR_TXE_MASK));
+    }
+}
+
+/* Init the RCC clocks to run at 80 MHz */
+static void init_clocks(QTestState *qts)
+{
+    uint32_t value;
+
+    /* MSIRANGE can be set only when MSI is OFF or READY */
+    qtest_writel(qts, (RCC_BASE_ADDR + A_CR), R_CR_MSION_MASK);
+
+    /* Clocking from MSI, in case MSI was not the default source */
+    qtest_writel(qts, (RCC_BASE_ADDR + A_CFGR), 0);
+
+    /*
+     * Update PLL and set MSI as the source clock.
+     * PLLM = 1 --> 000
+     * PLLN = 40 --> 40
+     * PPLLR = 2 --> 00
+     * PLLDIV = unused, PLLP = unused (SAI3), PLLQ = unused (48M1)
+     * SRC = MSI --> 01
+     */
+    qtest_writel(qts, (RCC_BASE_ADDR + A_PLLCFGR), R_PLLCFGR_PLLREN_MASK |
+            (40 << R_PLLCFGR_PLLN_SHIFT) |
+            (0b01 << R_PLLCFGR_PLLSRC_SHIFT));
+
+    /* PLL activation */
+
+    value = qtest_readl(qts, (RCC_BASE_ADDR + A_CR));
+    qtest_writel(qts, (RCC_BASE_ADDR + A_CR), value | R_CR_PLLON_MASK);
+
+    /* RCC_CFGR is OK by defaut */
+    qtest_writel(qts, (RCC_BASE_ADDR + A_CFGR), 0);
+
+    /* CCIPR : no periph clock by default */
+    qtest_writel(qts, (RCC_BASE_ADDR + A_CCIPR), 0);
+
+    /* Switches on the PLL clock source */
+    value = qtest_readl(qts, (RCC_BASE_ADDR + A_CFGR));
+    qtest_writel(qts, (RCC_BASE_ADDR + A_CFGR), (value & ~R_CFGR_SW_MASK) |
+        (0b11 << R_CFGR_SW_SHIFT));
+
+    /* Enable SYSCFG clock enabled */
+    qtest_writel(qts, (RCC_BASE_ADDR + A_APB2ENR), R_APB2ENR_SYSCFGEN_MASK);
+
+    /* Enable the IO port B clock (See p.252) */
+    qtest_writel(qts, (RCC_BASE_ADDR + A_AHB2ENR), R_AHB2ENR_GPIOBEN_MASK);
+
+    /* Enable the clock for USART1 (cf p.259) */
+    /* We rewrite SYSCFGEN to not disable it */
+    qtest_writel(qts, (RCC_BASE_ADDR + A_APB2ENR),
+                 R_APB2ENR_SYSCFGEN_MASK | R_APB2ENR_USART1EN_MASK);
+
+    /* TODO: Enable usart via gpio */
+    /* Set Pin 6 & 7 of port B in Alternate Function mode */
+    /* GPIOB->MODER = (GPIOB->MODER & ~(GPIO_MODER_MODE6_Msk |
+                    GPIO_MODER_MODE7_Msk)) |
+                    (AF_MODE << GPIO_MODER_MODE6_Pos) |
+                    (AF_MODE << GPIO_MODER_MODE7_Pos); */
+
+    /* Specify Alternate Function mode n°7 for Pin 6 & 7 of port B */
+    /* GPIOB->AFR[0] = (GPIOB->AFR[0] & ~(GPIO_AFRL_AFSEL6_Msk |
+                        GPIO_AFRL_AFSEL7_Msk )) |
+                        (AF7 << GPIO_AFRL_AFSEL6_Pos) |
+                        (AF7 << GPIO_AFRL_AFSEL7_Pos); */
+
+    /* Set PCLK as the clock for USART1(cf p.272) i.e. reset both bits */
+    qtest_writel(qts, (RCC_BASE_ADDR + A_CCIPR), 0);
+
+    /* Reset USART1 (see p.249) */
+    qtest_writel(qts, (RCC_BASE_ADDR + A_APB2RSTR), 1 << 14);
+    qtest_writel(qts, (RCC_BASE_ADDR + A_APB2RSTR), 0);
+}
+
+static void init_uart(QTestState *qts)
+{
+    uint32_t cr1;
+
+    init_clocks(qts);
+
+    /*
+     * For 115200 bauds, see p.1349.
+     * The clock has a frequency of 80Mhz,
+     * for 115200, we have to put a divider of 695 = 0x2B7.
+     */
+    qtest_writel(qts, (USART1_BASE_ADDR + A_BRR), 0x2B7);
+
+    /*
+     * Set the oversampling by 16,
+     * disable the parity control and
+     * set the word length to 8. (cf p.1377)
+     */
+    cr1 = qtest_readl(qts, (USART1_BASE_ADDR + A_CR1));
+    cr1 &= ~(R_CR1_M1_MASK | R_CR1_M0_MASK | R_CR1_OVER8_MASK | R_CR1_PCE_MASK);
+    qtest_writel(qts, (USART1_BASE_ADDR + A_CR1), cr1);
+
+    /* Enable the transmitter, the receiver and the USART. */
+    qtest_writel(qts, (USART1_BASE_ADDR + A_CR1),
+        R_CR1_UE_MASK | R_CR1_RE_MASK | R_CR1_TE_MASK);
+}
+
+static void test_write_read(void)
+{
+    QTestState *qts = qtest_init("-M b-l475e-iot01a");
+
+    /* Test that we can write and retrieve a value from the device */
+    qtest_writel(qts, USART1_BASE_ADDR + A_TDR, 0xFFFFFFFF);
+    const uint32_t tdr = qtest_readl(qts, USART1_BASE_ADDR + A_TDR);
+    g_assert_cmpuint(tdr, ==, 0x000001FF);
+}
+
+static void test_receive_char(void)
+{
+    int sock_fd;
+    uint32_t cr1;
+    QTestState *qts = qtest_init_with_serial("-M b-l475e-iot01a", &sock_fd);
+
+    init_uart(qts);
+
+    /* Try without initializing IRQ */
+    g_assert_true(send(sock_fd, "a", 1, 0) == 1);
+    usart_wait_for_flag(qts, USART1_BASE_ADDR + A_ISR, R_ISR_RXNE_MASK);
+    g_assert_cmphex(qtest_readl(qts, USART1_BASE_ADDR + A_RDR), ==, 'a');
+    g_assert_false(check_nvic_pending(qts, USART1_IRQ));
+
+    /* Now with the IRQ */
+    cr1 = qtest_readl(qts, (USART1_BASE_ADDR + A_CR1));
+    cr1 |= R_CR1_RXNEIE_MASK;
+    qtest_writel(qts, USART1_BASE_ADDR + A_CR1, cr1);
+    g_assert_true(send(sock_fd, "b", 1, 0) == 1);
+    usart_wait_for_flag(qts, USART1_BASE_ADDR + A_ISR, R_ISR_RXNE_MASK);
+    g_assert_cmphex(qtest_readl(qts, USART1_BASE_ADDR + A_RDR), ==, 'b');
+    g_assert_true(check_nvic_pending(qts, USART1_IRQ));
+    clear_nvic_pending(qts, USART1_IRQ);
+
+    close(sock_fd);
+
+    qtest_quit(qts);
+}
+
+static void test_send_char(void)
+{
+    int sock_fd;
+    char s[1];
+    uint32_t cr1;
+    QTestState *qts = qtest_init_with_serial("-M b-l475e-iot01a", &sock_fd);
+
+    init_uart(qts);
+
+    /* Try without initializing IRQ */
+    qtest_writel(qts, USART1_BASE_ADDR + A_TDR, 'c');
+    g_assert_true(recv(sock_fd, s, 1, 0) == 1);
+    g_assert_cmphex(s[0], ==, 'c');
+    g_assert_false(check_nvic_pending(qts, USART1_IRQ));
+
+    /* Now with the IRQ */
+    cr1 = qtest_readl(qts, (USART1_BASE_ADDR + A_CR1));
+    cr1 |= R_CR1_TXEIE_MASK;
+    qtest_writel(qts, USART1_BASE_ADDR + A_CR1, cr1);
+    qtest_writel(qts, USART1_BASE_ADDR + A_TDR, 'd');
+    g_assert_true(recv(sock_fd, s, 1, 0) == 1);
+    g_assert_cmphex(s[0], ==, 'd');
+    g_assert_true(check_nvic_pending(qts, USART1_IRQ));
+    clear_nvic_pending(qts, USART1_IRQ);
+
+    close(sock_fd);
+
+    qtest_quit(qts);
+}
+
+static void test_receive_str(void)
+{
+    int sock_fd;
+    char s[10];
+    QTestState *qts = qtest_init_with_serial("-M b-l475e-iot01a", &sock_fd);
+
+    init_uart(qts);
+
+    usart_receive_string(qts, sock_fd, "hello", s);
+    g_assert_true(memcmp(s, "hello", 5) == 0);
+
+    close(sock_fd);
+
+    qtest_quit(qts);
+}
+
+static void test_send_str(void)
+{
+    int sock_fd;
+    char s[10];
+    QTestState *qts = qtest_init_with_serial("-M b-l475e-iot01a", &sock_fd);
+
+    init_uart(qts);
+
+    usart_send_string(qts, "world");
+    g_assert_true(recv(sock_fd, s, 10, 0) == 5);
+    g_assert_true(memcmp(s, "world", 5) == 0);
+
+    close(sock_fd);
+
+    qtest_quit(qts);
+}
+
+int main(int argc, char **argv)
+{
+    int ret;
+
+    g_test_init(&argc, &argv, NULL);
+    g_test_set_nonfatal_assertions();
+
+    qtest_add_func("stm32l4x5/usart/write_read", test_write_read);
+    qtest_add_func("stm32l4x5/usart/receive_char", test_receive_char);
+    qtest_add_func("stm32l4x5/usart/send_char", test_send_char);
+    qtest_add_func("stm32l4x5/usart/receive_str", test_receive_str);
+    qtest_add_func("stm32l4x5/usart/send_str", test_send_str);
+    ret = g_test_run();
+
+    return ret;
+}
+
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 6/6] tests/qtest: Add tests for the STM32L4x5 USART
  2024-03-24 16:55 ` [PATCH v2 6/6] tests/qtest: Add tests for the STM32L4x5 USART Arnaud Minier
@ 2024-03-25  6:19   ` Thomas Huth
  2024-03-28 16:14     ` Peter Maydell
  0 siblings, 1 reply; 14+ messages in thread
From: Thomas Huth @ 2024-03-25  6:19 UTC (permalink / raw)
  To: Arnaud Minier, qemu-devel
  Cc: Laurent Vivier, Marc-André Lureau, qemu-arm, Paolo Bonzini,
	Alistair Francis, Inès Varhol, Samuel Tardieu,
	Peter Maydell

  Hi!

On 24/03/2024 17.55, Arnaud Minier wrote:
> Test:
> - read/write from/to the usart registers
> - send/receive a character/string over the serial port
> 
> Signed-off-by: Arnaud Minier <arnaud.minier@telecom-paris.fr>
> Signed-off-by: Inès Varhol <ines.varhol@telecom-paris.fr>
> ---
>   tests/qtest/meson.build            |   3 +-
>   tests/qtest/stm32l4x5_usart-test.c | 326 +++++++++++++++++++++++++++++
>   2 files changed, 328 insertions(+), 1 deletion(-)
>   create mode 100644 tests/qtest/stm32l4x5_usart-test.c
> 
> diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
> index 36c5c13a7b..e0d72ee91e 100644
> --- a/tests/qtest/meson.build
> +++ b/tests/qtest/meson.build
> @@ -205,7 +205,8 @@ qtests_stm32l4x5 = \
>     ['stm32l4x5_exti-test',
>      'stm32l4x5_syscfg-test',
>      'stm32l4x5_rcc-test',
> -   'stm32l4x5_gpio-test']
> +   'stm32l4x5_gpio-test',
> +   'stm32l4x5_usart-test']

We are now using timeouts from the meson test harneess in meson.build, too, 
see the slow_qtests[] at the beginning of that file.
You seem to be using a 10 minutes timeout in your test below 
(usart_wait_for_flag() function), but you didn't adjust the meson timeout 
setting in meson.build, so this does not quite match...
How long does your test take on a very loaded machine (with --enable-debug 
used)? If it could take more than 30 seconds, you need to adjust the timeout 
in meson.build, too. If it is running very fast, you should likely adjust 
the 10 minutes timeout in usart_wait_for_flag() to < 30 seconds instead to 
match the meson timeout setting.

>   qtests_arm = \
>     (config_all_devices.has_key('CONFIG_MPS2') ? ['sse-timer-test'] : []) + \
> diff --git a/tests/qtest/stm32l4x5_usart-test.c b/tests/qtest/stm32l4x5_usart-test.c
> new file mode 100644
> index 0000000000..2d42f053f6
> --- /dev/null
> +++ b/tests/qtest/stm32l4x5_usart-test.c
> @@ -0,0 +1,326 @@
> +/*
> + * QTest testcase for STML4X5_USART
> + *
> + * Copyright (c) 2023 Arnaud Minier <arnaud.minier@telecom-paris.fr>
> + * Copyright (c) 2023 Inès Varhol <ines.varhol@telecom-paris.fr>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "libqtest.h"
> +#include "hw/misc/stm32l4x5_rcc_internals.h"
> +#include "hw/registerfields.h"
> +
> +#define RCC_BASE_ADDR 0x40021000
> +/* Use USART 1 ADDR, assume the others work the same */
> +#define USART1_BASE_ADDR 0x40013800
> +
> +/* See stm32l4x5_usart for definitions */
> +REG32(CR1, 0x00)
> +    FIELD(CR1, M1, 28, 1)
> +    FIELD(CR1, OVER8, 15, 1)
> +    FIELD(CR1, M0, 12, 1)
> +    FIELD(CR1, PCE, 10, 1)
> +    FIELD(CR1, TXEIE, 7, 1)
> +    FIELD(CR1, RXNEIE, 5, 1)
> +    FIELD(CR1, TE, 3, 1)
> +    FIELD(CR1, RE, 2, 1)
> +    FIELD(CR1, UE, 0, 1)
> +REG32(CR2, 0x04)
> +REG32(CR3, 0x08)
> +    FIELD(CR3, OVRDIS, 12, 1)
> +REG32(BRR, 0x0C)
> +REG32(GTPR, 0x10)
> +REG32(RTOR, 0x14)
> +REG32(RQR, 0x18)
> +REG32(ISR, 0x1C)
> +    FIELD(ISR, TXE, 7, 1)
> +    FIELD(ISR, RXNE, 5, 1)
> +    FIELD(ISR, ORE, 3, 1)
> +REG32(ICR, 0x20)
> +REG32(RDR, 0x24)
> +REG32(TDR, 0x28)
> +
> +#define NVIC_ISPR1 0XE000E204
> +#define NVIC_ICPR1 0xE000E284
> +#define USART1_IRQ 37
> +
> +static bool check_nvic_pending(QTestState *qts, unsigned int n)
> +{
> +    /* No USART interrupts are less than 32 */
> +    assert(n > 32);
> +    n -= 32;
> +    return qtest_readl(qts, NVIC_ISPR1) & (1 << n);
> +}
> +
> +static bool clear_nvic_pending(QTestState *qts, unsigned int n)
> +{
> +    /* No USART interrupts are less than 32 */
> +    assert(n > 32);
> +    n -= 32;
> +    qtest_writel(qts, NVIC_ICPR1, (1 << n));
> +    return true;

I'd suggest to change the return type to "void" and drop the "return true" here.

> +}
> +
> +/*
> + Tests should never need to sleep(), because while it might be plenty of time on a
> + fast development machine, it can cause intermittent failures due
> + to timeouts if the test is on some heavily-loaded slow CI runner.
> + */
> +static bool usart_wait_for_flag(QTestState *qts, uint32_t event_addr, uint32_t flag)
> +{
> +    /* Wait at most 10 minutes */
> +    for (int i = 0; i < 600000; i++) {
> +        if ((qtest_readl(qts, event_addr) & flag)) {
> +            return true;
> +        }
> +        g_usleep(1000);

As I recently learnt again, some systems (like some BSD kernels) still use a 
time slice resolution of 10 ms, so it might be better to g_usleep(10000) and 
adjust the loop counter to a value that is 10 times less instead, otherwise 
your loop might 100 minutes instead of 10 minutes in the worst case instead.

> +    }
> +
> +    return false;
> +}

  Thomas




^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 2/6] hw/char: Implement STM32L4x5 USART skeleton
  2024-03-24 16:55 ` [PATCH v2 2/6] hw/char: Implement STM32L4x5 USART skeleton Arnaud Minier
@ 2024-03-28 15:55   ` Peter Maydell
  0 siblings, 0 replies; 14+ messages in thread
From: Peter Maydell @ 2024-03-28 15:55 UTC (permalink / raw)
  To: Arnaud Minier
  Cc: qemu-devel, Laurent Vivier, Marc-André Lureau, qemu-arm,
	Paolo Bonzini, Alistair Francis, Thomas Huth, Inès Varhol,
	Samuel Tardieu

On Sun, 24 Mar 2024 at 16:56, Arnaud Minier
<arnaud.minier@telecom-paris.fr> wrote:
>
> Add the basic infrastructure (register read/write, type...)
> to implement the STM32L4x5 USART.
>
> Also create different types for the USART, UART and LPUART
> of the STM32L4x5 to deduplicate code and enable the
> implementation of different behaviors depending on the type.
>
> Signed-off-by: Arnaud Minier <arnaud.minier@telecom-paris.fr>
> Signed-off-by: Inès Varhol <ines.varhol@telecom-paris.fr>
> ---
>  MAINTAINERS                       |   1 +
>  hw/char/Kconfig                   |   3 +
>  hw/char/meson.build               |   1 +
>  hw/char/stm32l4x5_usart.c         | 395 ++++++++++++++++++++++++++++++
>  hw/char/trace-events              |   4 +
>  include/hw/char/stm32l4x5_usart.h |  66 +++++
>  6 files changed, 470 insertions(+)
>  create mode 100644 hw/char/stm32l4x5_usart.c
>  create mode 100644 include/hw/char/stm32l4x5_usart.h
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 409d7db4d4..deba4a54ce 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1128,6 +1128,7 @@ M: Inès Varhol <ines.varhol@telecom-paris.fr>
>  L: qemu-arm@nongnu.org
>  S: Maintained
>  F: hw/arm/stm32l4x5_soc.c
> +F: hw/char/stm32l4x5_usart.c
>  F: hw/misc/stm32l4x5_exti.c
>  F: hw/misc/stm32l4x5_syscfg.c
>  F: hw/misc/stm32l4x5_rcc.c
> diff --git a/hw/char/Kconfig b/hw/char/Kconfig
> index 6b6cf2fc1d..4fd74ea878 100644
> --- a/hw/char/Kconfig
> +++ b/hw/char/Kconfig
> @@ -41,6 +41,9 @@ config VIRTIO_SERIAL
>  config STM32F2XX_USART
>      bool
>
> +config STM32L4X5_USART
> +    bool
> +
>  config CMSDK_APB_UART
>      bool
>
> diff --git a/hw/char/meson.build b/hw/char/meson.build
> index 006d20f1e2..e5b13b6958 100644
> --- a/hw/char/meson.build
> +++ b/hw/char/meson.build
> @@ -31,6 +31,7 @@ system_ss.add(when: 'CONFIG_RENESAS_SCI', if_true: files('renesas_sci.c'))
>  system_ss.add(when: 'CONFIG_SIFIVE_UART', if_true: files('sifive_uart.c'))
>  system_ss.add(when: 'CONFIG_SH_SCI', if_true: files('sh_serial.c'))
>  system_ss.add(when: 'CONFIG_STM32F2XX_USART', if_true: files('stm32f2xx_usart.c'))
> +system_ss.add(when: 'CONFIG_STM32L4X5_USART', if_true: files('stm32l4x5_usart.c'))
>  system_ss.add(when: 'CONFIG_MCHP_PFSOC_MMUART', if_true: files('mchp_pfsoc_mmuart.c'))
>  system_ss.add(when: 'CONFIG_HTIF', if_true: files('riscv_htif.c'))
>  system_ss.add(when: 'CONFIG_GOLDFISH_TTY', if_true: files('goldfish_tty.c'))
> diff --git a/hw/char/stm32l4x5_usart.c b/hw/char/stm32l4x5_usart.c
> new file mode 100644
> index 0000000000..46e69bb096
> --- /dev/null
> +++ b/hw/char/stm32l4x5_usart.c
> @@ -0,0 +1,395 @@
> +/*
> + * STM32L4X5 USART (Universal Synchronous Asynchronous Receiver Transmitter)
> + *
> + * Copyright (c) 2023 Arnaud Minier <arnaud.minier@telecom-paris.fr>
> + * Copyright (c) 2023 Inès Varhol <ines.varhol@telecom-paris.fr>
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + *
> + * The STM32L4X5 USART is heavily inspired by the stm32f2xx_usart
> + * by Alistair Francis.
> + * The reference used is the STMicroElectronics RM0351 Reference manual
> + * for STM32L4x5 and STM32L4x6 advanced Arm ® -based 32-bit MCUs.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qemu/log.h"
> +#include "qemu/module.h"
> +#include "qapi/error.h"
> +#include "chardev/char-fe.h"
> +#include "chardev/char-serial.h"
> +#include "migration/vmstate.h"
> +#include "hw/char/stm32l4x5_usart.h"
> +#include "hw/clock.h"
> +#include "hw/irq.h"
> +#include "hw/qdev-clock.h"
> +#include "hw/qdev-properties.h"
> +#include "hw/qdev-properties-system.h"
> +#include "hw/registerfields.h"
> +#include "trace.h"
> +
> +
> +REG32(CR1, 0x00)
> +    FIELD(CR1, M1, 28, 1)    /* Word length (part 2, see M0)*/

Missing space before "*/"

> +static const TypeInfo stm32l4x5_usart_types[] = {
> +    {
> +        .name           = TYPE_STM32L4X5_USART_BASE,
> +        .parent         = TYPE_SYS_BUS_DEVICE,
> +        .instance_size  = sizeof(Stm32l4x5UsartBaseState),
> +        .instance_init  = stm32l4x5_usart_base_init,
> +        .class_init     = stm32l4x5_usart_base_class_init,

This should also have
    .abstract = true,

so you can't create an instance of this class, only of
the specific subclasses.

> +    }, {
> +        .name           = TYPE_STM32L4X5_USART,
> +        .parent         = TYPE_STM32L4X5_USART_BASE,
> +        .class_init     = stm32l4x5_usart_class_init,
> +    }, {
> +        .name           = TYPE_STM32L4X5_UART,
> +        .parent         = TYPE_STM32L4X5_USART_BASE,
> +        .class_init     = stm32l4x5_uart_class_init,
> +    }, {
> +        .name           = TYPE_STM32L4X5_LPUART,
> +        .parent         = TYPE_STM32L4X5_USART_BASE,
> +        .class_init     = stm32l4x5_lpuart_class_init,
> +    }
> +};
> +

Otherwise
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 3/6] hw/char/stm32l4x5_usart: Enable serial read and write
  2024-03-24 16:55 ` [PATCH v2 3/6] hw/char/stm32l4x5_usart: Enable serial read and write Arnaud Minier
@ 2024-03-28 15:59   ` Peter Maydell
  0 siblings, 0 replies; 14+ messages in thread
From: Peter Maydell @ 2024-03-28 15:59 UTC (permalink / raw)
  To: Arnaud Minier
  Cc: qemu-devel, Laurent Vivier, Marc-André Lureau, qemu-arm,
	Paolo Bonzini, Alistair Francis, Thomas Huth, Inès Varhol,
	Samuel Tardieu

On Sun, 24 Mar 2024 at 16:57, Arnaud Minier
<arnaud.minier@telecom-paris.fr> wrote:
>
> Implement the ability to read and write characters to the
> usart using the serial port.
>
> The character transmission is based on the
> cmsdk-apb-uart implementation.
>
> Signed-off-by: Arnaud Minier <arnaud.minier@telecom-paris.fr>
> Signed-off-by: Inès Varhol <ines.varhol@telecom-paris.fr>


> +/* Try to send tx data, and arrange to be called back later if
> + * we can't (ie the char backend is busy/blocking).
> + */

Coding style wants the opening "/*" on a line of its own.

Otherwise
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 4/6] hw/char/stm32l4x5_usart: Add options for serial parameters setting
  2024-03-24 16:55 ` [PATCH v2 4/6] hw/char/stm32l4x5_usart: Add options for serial parameters setting Arnaud Minier
@ 2024-03-28 16:03   ` Peter Maydell
  0 siblings, 0 replies; 14+ messages in thread
From: Peter Maydell @ 2024-03-28 16:03 UTC (permalink / raw)
  To: Arnaud Minier
  Cc: qemu-devel, Laurent Vivier, Marc-André Lureau, qemu-arm,
	Paolo Bonzini, Alistair Francis, Thomas Huth, Inès Varhol,
	Samuel Tardieu

On Sun, 24 Mar 2024 at 16:57, Arnaud Minier
<arnaud.minier@telecom-paris.fr> wrote:
>
> Add a function to change the settings of the
> serial connection.
>
> Signed-off-by: Arnaud Minier <arnaud.minier@telecom-paris.fr>
> Signed-off-by: Inès Varhol <ines.varhol@telecom-paris.fr>
> ---
>  hw/char/stm32l4x5_usart.c | 98 +++++++++++++++++++++++++++++++++++++++
>  hw/char/trace-events      |  1 +
>  2 files changed, 99 insertions(+)
>
> diff --git a/hw/char/stm32l4x5_usart.c b/hw/char/stm32l4x5_usart.c
> index ec8c2f6e63..b4d11dd826 100644
> --- a/hw/char/stm32l4x5_usart.c
> +++ b/hw/char/stm32l4x5_usart.c
> @@ -267,6 +267,92 @@ static void usart_cancel_transmit(Stm32l4x5UsartBaseState *s)
>      }
>  }
>
> +static void stm32l4x5_update_params(Stm32l4x5UsartBaseState *s)
> +{
> +    int speed, parity, data_bits, stop_bits;
> +    uint32_t value, usart_div;
> +    QEMUSerialSetParams ssp;
> +
> +    /* Select the parity type */
> +    if (s->cr1 & R_CR1_PCE_MASK) {
> +        if (s->cr1 & R_CR1_PS_MASK) {
> +            parity = 'O';
> +        } else {
> +            parity = 'E';
> +        }
> +    } else {
> +        parity = 'N';
> +    }
> +
> +    /* Select the number of stop bits */
> +    switch (FIELD_EX32(s->cr2, CR2, STOP)) {
> +    case 0:
> +        stop_bits = 1;
> +        break;
> +    case 2:
> +        stop_bits = 2;
> +        break;
> +    default:
> +        qemu_log_mask(LOG_UNIMP,
> +            "UNIMPLEMENTED: fractionnal stop bits; CR2[13:12] = %x",

%x without a leading 0x is a bit odd. In this case since
the possible values are 0-3 it doesn't make a difference,
but maybe better to use %u ?

> +            FIELD_EX32(s->cr2, CR2, STOP));
> +        return;
> +    }
> +
> +    /* Select the length of the word */
> +    switch ((FIELD_EX32(s->cr1, CR1, M1) << 1) | FIELD_EX32(s->cr1, CR1, M0)) {
> +    case 0:
> +        data_bits = 8;
> +        break;
> +    case 1:
> +        data_bits = 9;
> +        break;
> +    case 2:
> +        data_bits = 7;
> +        break;
> +    default:
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +            "UNDEFINED: invalid word length, CR1.M = 0b11");
> +        return;
> +    }
> +
> +    /* Select the baud rate */
> +    value = FIELD_EX32(s->brr, BRR, BRR);
> +    if (value < 16) {
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +            "UNDEFINED: BRR lesser than 16: %u", value);

"less than"

> +        return;
> +    }
> +
> +    if (FIELD_EX32(s->cr1, CR1, OVER8) == 0) {
> +        /*
> +         * Oversampling by 16
> +         * BRR = USARTDIV
> +         */
> +        usart_div = value;
> +    } else {
> +        /*
> +         * Oversampling by 8
> +         * - BRR[2:0] = USARTDIV[3:0] shifted 1 bit to the right.
> +         * - BRR[3] must be kept cleared.
> +         * - BRR[15:4] = USARTDIV[15:4]
> +         * - The frequency is multiplied by 2
> +         */
> +        usart_div = ((value & 0xFFF0) | ((value & 0x0007) << 1)) / 2;
> +    }
> +
> +    speed = clock_get_hz(s->clk) / usart_div;
> +
> +    ssp.speed     = speed;
> +    ssp.parity    = parity;
> +    ssp.data_bits = data_bits;
> +    ssp.stop_bits = stop_bits;
> +
> +    qemu_chr_fe_ioctl(&s->chr, CHR_IOCTL_SERIAL_SET_PARAMS, &ssp);
> +
> +    trace_stm32l4x5_usart_update_params(speed, parity, data_bits, stop_bits);
> +}

Otherwise
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 5/6] hw/arm: Add the USART to the stm32l4x5 SoC
  2024-03-24 16:55 ` [PATCH v2 5/6] hw/arm: Add the USART to the stm32l4x5 SoC Arnaud Minier
@ 2024-03-28 16:06   ` Peter Maydell
  0 siblings, 0 replies; 14+ messages in thread
From: Peter Maydell @ 2024-03-28 16:06 UTC (permalink / raw)
  To: Arnaud Minier
  Cc: qemu-devel, Laurent Vivier, Marc-André Lureau, qemu-arm,
	Paolo Bonzini, Alistair Francis, Thomas Huth, Inès Varhol,
	Samuel Tardieu

On Sun, 24 Mar 2024 at 16:57, Arnaud Minier
<arnaud.minier@telecom-paris.fr> wrote:
>
> Add the USART to the SoC and connect it to the other implemented devices.
>
> Signed-off-by: Arnaud Minier <arnaud.minier@telecom-paris.fr>
> Signed-off-by: Inès Varhol <ines.varhol@telecom-paris.fr>
> ---
>  docs/system/arm/b-l475e-iot01a.rst |  2 +-
>  hw/arm/Kconfig                     |  1 +
>  hw/arm/stm32l4x5_soc.c             | 82 +++++++++++++++++++++++++++---
>  include/hw/arm/stm32l4x5_soc.h     | 13 +++++
>  4 files changed, 91 insertions(+), 7 deletions(-)
>
> diff --git a/docs/system/arm/b-l475e-iot01a.rst b/docs/system/arm/b-l475e-iot01a.rst
> index 0afef8e4f4..a76c9976c5 100644
> --- a/docs/system/arm/b-l475e-iot01a.rst
> +++ b/docs/system/arm/b-l475e-iot01a.rst
> @@ -19,13 +19,13 @@ Currently B-L475E-IOT01A machine's only supports the following devices:
>  - STM32L4x5 SYSCFG (System configuration controller)
>  - STM32L4x5 RCC (Reset and clock control)
>  - STM32L4x5 GPIOs (General-purpose I/Os)
> +- STM32L4x5 USARTs, UARTs and LPUART (Serial ports)
>
>  Missing devices
>  """""""""""""""
>
>  The B-L475E-IOT01A does *not* support the following devices:
>
> -- Serial ports (UART)
>  - Analog to Digital Converter (ADC)
>  - SPI controller
>  - Timer controller (TIMER)
> diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
> index 893a7bff66..098d043375 100644
> --- a/hw/arm/Kconfig
> +++ b/hw/arm/Kconfig
> @@ -477,6 +477,7 @@ config STM32L4X5_SOC
>      select STM32L4X5_SYSCFG
>      select STM32L4X5_RCC
>      select STM32L4X5_GPIO
> +    select STM32L4X5_USART
>
>  config XLNX_ZYNQMP_ARM
>      bool
> diff --git a/hw/arm/stm32l4x5_soc.c b/hw/arm/stm32l4x5_soc.c
> index 40e294f838..ae0868dcab 100644
> --- a/hw/arm/stm32l4x5_soc.c
> +++ b/hw/arm/stm32l4x5_soc.c
> @@ -28,6 +28,7 @@
>  #include "sysemu/sysemu.h"
>  #include "hw/or-irq.h"
>  #include "hw/arm/stm32l4x5_soc.h"
> +#include "hw/char/stm32l4x5_usart.h"
>  #include "hw/gpio/stm32l4x5_gpio.h"
>  #include "hw/qdev-clock.h"
>  #include "hw/misc/unimp.h"
> @@ -116,6 +117,22 @@ static const struct {
>      { 0x48001C00, 0x0000000F, 0x00000000, 0x00000000 },
>  };
>
> +static const hwaddr usart_addr[] = {
> +    0x40013800, /* "USART1", 0x400 */
> +    0x40004400, /* "USART2", 0x400 */
> +    0x40004800, /* "USART3", 0x400 */
> +};
> +static const hwaddr uart_addr[] = {
> +    0x40004C00, /* "UART4" , 0x400 */
> +    0x40005000  /* "UART5" , 0x400 */
> +};
> +
> +#define LPUART_BASE_ADDRESS 0x40008000
> +
> +static const int usart_irq[] = { 37, 38, 39 };
> +static const int uart_irq[] = { 52, 53 };
> +#define LPUART_IRQ 70
> +
>  static void stm32l4x5_soc_initfn(Object *obj)
>  {
>      Stm32l4x5SocState *s = STM32L4X5_SOC(obj);
> @@ -132,6 +149,18 @@ static void stm32l4x5_soc_initfn(Object *obj)
>          g_autofree char *name = g_strdup_printf("gpio%c", 'a' + i);
>          object_initialize_child(obj, name, &s->gpio[i], TYPE_STM32L4X5_GPIO);
>      }
> +
> +    for (int i = 0; i < STM_NUM_USARTS; i++) {
> +        object_initialize_child(obj, "usart[*]", &s->usart[i],
> +                                TYPE_STM32L4X5_USART);
> +    }
> +
> +    for (int i = 0; i < STM_NUM_UARTS; i++) {
> +        object_initialize_child(obj, "uart[*]", &s->uart[i],
> +                                TYPE_STM32L4X5_UART);
> +    }
> +    object_initialize_child(obj, "lpuart1", &s->lpuart,
> +                            TYPE_STM32L4X5_LPUART);
>  }
>
>  static void stm32l4x5_soc_realize(DeviceState *dev_soc, Error **errp)
> @@ -279,6 +308,53 @@ static void stm32l4x5_soc_realize(DeviceState *dev_soc, Error **errp)
>      sysbus_mmio_map(busdev, 0, RCC_BASE_ADDRESS);
>      sysbus_connect_irq(busdev, 0, qdev_get_gpio_in(armv7m, RCC_IRQ));
>
> +    /* USART devices */
> +    for (int i = 0; i < STM_NUM_USARTS; i++) {
> +        dev = DEVICE(&(s->usart[i]));
> +        qdev_prop_set_chr(dev, "chardev", serial_hd(i));
> +        g_autofree char *name = g_strdup_printf("usart%d-out", i + 1);

Variable declarations should always be at the start of a code block.
Similarly below.

> +        qdev_connect_clock_in(dev, "clk",
> +            qdev_get_clock_out(DEVICE(&(s->rcc)), name));
> +        busdev = SYS_BUS_DEVICE(dev);
> +        if (!sysbus_realize(busdev, errp)) {
> +            return;
> +        }
> +        sysbus_mmio_map(busdev, 0, usart_addr[i]);
> +        sysbus_connect_irq(busdev, 0, qdev_get_gpio_in(armv7m, usart_irq[i]));
> +    }
> +
> +    /*
> +     * TODO: Connect the USARTs, UARTs and LPUART to the EXTI once the EXTI
> +     * can handle other gpio-in than the gpios. (e.g. Direct Lines for the usarts)
> +     */
> +
> +    /* UART devices */
> +    for (int i = 0; i < STM_NUM_UARTS; i++) {
> +        dev = DEVICE(&(s->uart[i]));
> +        qdev_prop_set_chr(dev, "chardev", serial_hd(STM_NUM_USARTS + i));
> +        g_autofree char *name = g_strdup_printf("uart%d-out", STM_NUM_USARTS + i + 1);
> +        qdev_connect_clock_in(dev, "clk",
> +            qdev_get_clock_out(DEVICE(&(s->rcc)), name));
> +        busdev = SYS_BUS_DEVICE(dev);
> +        if (!sysbus_realize(busdev, errp)) {
> +            return;
> +        }
> +        sysbus_mmio_map(busdev, 0, uart_addr[i]);
> +        sysbus_connect_irq(busdev, 0, qdev_get_gpio_in(armv7m, uart_irq[i]));
> +    }
> +
> +    /* LPUART device*/
> +    dev = DEVICE(&(s->lpuart));
> +    qdev_prop_set_chr(dev, "chardev", serial_hd(STM_NUM_USARTS + STM_NUM_UARTS));
> +    qdev_connect_clock_in(dev, "clk",
> +        qdev_get_clock_out(DEVICE(&(s->rcc)), "lpuart1-out"));
> +    busdev = SYS_BUS_DEVICE(dev);
> +    if (!sysbus_realize(busdev, errp)) {
> +        return;
> +    }
> +    sysbus_mmio_map(busdev, 0, LPUART_BASE_ADDRESS);
> +    sysbus_connect_irq(busdev, 0, qdev_get_gpio_in(armv7m, LPUART_IRQ));
> +
>      /* APB1 BUS */
>      create_unimplemented_device("TIM2",      0x40000000, 0x400);
>      create_unimplemented_device("TIM3",      0x40000400, 0x400);
> @@ -294,10 +370,6 @@ static void stm32l4x5_soc_realize(DeviceState *dev_soc, Error **errp)
>      create_unimplemented_device("SPI2",      0x40003800, 0x400);
>      create_unimplemented_device("SPI3",      0x40003C00, 0x400);
>      /* RESERVED:    0x40004000, 0x400 */
> -    create_unimplemented_device("USART2",    0x40004400, 0x400);
> -    create_unimplemented_device("USART3",    0x40004800, 0x400);
> -    create_unimplemented_device("UART4",     0x40004C00, 0x400);
> -    create_unimplemented_device("UART5",     0x40005000, 0x400);
>      create_unimplemented_device("I2C1",      0x40005400, 0x400);
>      create_unimplemented_device("I2C2",      0x40005800, 0x400);
>      create_unimplemented_device("I2C3",      0x40005C00, 0x400);
> @@ -308,7 +380,6 @@ static void stm32l4x5_soc_realize(DeviceState *dev_soc, Error **errp)
>      create_unimplemented_device("DAC1",      0x40007400, 0x400);
>      create_unimplemented_device("OPAMP",     0x40007800, 0x400);
>      create_unimplemented_device("LPTIM1",    0x40007C00, 0x400);
> -    create_unimplemented_device("LPUART1",   0x40008000, 0x400);
>      /* RESERVED:    0x40008400, 0x400 */
>      create_unimplemented_device("SWPMI1",    0x40008800, 0x400);
>      /* RESERVED:    0x40008C00, 0x800 */
> @@ -325,7 +396,6 @@ static void stm32l4x5_soc_realize(DeviceState *dev_soc, Error **errp)
>      create_unimplemented_device("TIM1",      0x40012C00, 0x400);
>      create_unimplemented_device("SPI1",      0x40013000, 0x400);
>      create_unimplemented_device("TIM8",      0x40013400, 0x400);
> -    create_unimplemented_device("USART1",    0x40013800, 0x400);
>      /* RESERVED:    0x40013C00, 0x400 */
>      create_unimplemented_device("TIM15",     0x40014000, 0x400);
>      create_unimplemented_device("TIM16",     0x40014400, 0x400);
> diff --git a/include/hw/arm/stm32l4x5_soc.h b/include/hw/arm/stm32l4x5_soc.h
> index ee5f362405..a94ddbd19c 100644
> --- a/include/hw/arm/stm32l4x5_soc.h
> +++ b/include/hw/arm/stm32l4x5_soc.h
> @@ -21,6 +21,12 @@
>   * https://www.st.com/en/microcontrollers-microprocessors/stm32l4x5/documentation.html
>   */
>
> +/*
> + * The STM32L4X5 is heavily inspired by the stm32f405 by Alistair Francis.
> + * The reference used is the STMicroElectronics RM0351 Reference manual
> + * for STM32L4x5 and STM32L4x6 advanced Arm ® -based 32-bit MCUs.
> + */
> +

Doesn't the comment above with the URL already have this
text about the RM0351 manual ?

>  #ifndef HW_ARM_STM32L4x5_SOC_H
>  #define HW_ARM_STM32L4x5_SOC_H

Otherwise
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 0/6] hw/char: Implement the STM32L4x5 USART, UART and LPUART
  2024-03-24 16:55 [PATCH v2 0/6] hw/char: Implement the STM32L4x5 USART, UART and LPUART Arnaud Minier
                   ` (5 preceding siblings ...)
  2024-03-24 16:55 ` [PATCH v2 6/6] tests/qtest: Add tests for the STM32L4x5 USART Arnaud Minier
@ 2024-03-28 16:10 ` Peter Maydell
  6 siblings, 0 replies; 14+ messages in thread
From: Peter Maydell @ 2024-03-28 16:10 UTC (permalink / raw)
  To: Arnaud Minier
  Cc: qemu-devel, Laurent Vivier, Marc-André Lureau, qemu-arm,
	Paolo Bonzini, Alistair Francis, Thomas Huth, Inès Varhol,
	Samuel Tardieu

On Sun, 24 Mar 2024 at 16:56, Arnaud Minier
<arnaud.minier@telecom-paris.fr> wrote:
>
> This patch adds the STM32L4x5 USART
> (Universal Synchronous/Asynchronous Receiver/Transmitter)
> device and is part of a series implementing the
> STM32L4x5 with a few peripherals.
>
> It implements the necessary functionalities to receive/send
> characters over the serial port, which are useful to
> communicate with the program currently running.
>
> Many thanks Peter for your review, I think I addressed almost
> everything.
> I'm just unsure about how to handle the waiting time in the tests.
> I understand your concerns about the unreliability of using the wallclock
> time but I don't understand how using clock_step() would make it
> more reliable. We will always be waiting on something
> that is out of our control (i.e. the OS).
> I increased the delay from 5s to 10min to match the microbit test
> and added a comment (I paraphrased your comment, is that okay ?).

I think I was slightly confused between two things. For
a lot of qtests we do want to use clock_step() and not
have wallclock-based delays, but we can only do that where
the thing we're waiting for is purely simulation time
(i.e. where we triggered a change via a qtest write and
then want to look for the result via a qtest read).
Where we're triggering something via a different OS
pathway (e.g. here where we write to the socket that's
backing the chardev connected to the UART and then look
at the UART registers) we do need a wallclock delay.

I recommend you follow Thomas's suggestions about timeouts
in his comments on patch 6; I'd forgotten we have a
meson timeout now too.

thanks
-- PMM


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 6/6] tests/qtest: Add tests for the STM32L4x5 USART
  2024-03-25  6:19   ` Thomas Huth
@ 2024-03-28 16:14     ` Peter Maydell
  0 siblings, 0 replies; 14+ messages in thread
From: Peter Maydell @ 2024-03-28 16:14 UTC (permalink / raw)
  To: Thomas Huth
  Cc: Arnaud Minier, qemu-devel, Laurent Vivier,
	Marc-André Lureau, qemu-arm, Paolo Bonzini,
	Alistair Francis, Inès Varhol, Samuel Tardieu

On Mon, 25 Mar 2024 at 06:19, Thomas Huth <thuth@redhat.com> wrote:
> We are now using timeouts from the meson test harneess in meson.build, too,
> see the slow_qtests[] at the beginning of that file.
> You seem to be using a 10 minutes timeout in your test below
> (usart_wait_for_flag() function), but you didn't adjust the meson timeout
> setting in meson.build, so this does not quite match...
> How long does your test take on a very loaded machine (with --enable-debug
> used)? If it could take more than 30 seconds, you need to adjust the timeout
> in meson.build, too. If it is running very fast, you should likely adjust
> the 10 minutes timeout in usart_wait_for_flag() to < 30 seconds instead to
> match the meson timeout setting.

I'd forgotten about the meson harness timeout.

tests/qtest/microbit-test.c also has a 10 minute timeout but
isn't listed as a "slow qtest" (that's the pattern I suggested
Arnaud follow for this test).

If the meson test harness now handles timeouts, should we write
this kind of test to not have a timeout at all, so it simply
waits indefinitely for the UART to become ready after writing
data to the socket connected to the chardev? Or are there
scenarios where the test gets run but not via the meson harness
and where we would want to still have a timeout?
(For running the test executable by hand for debugging I think
hanging indefinitely is fine and arguably more helpful than
timing out and stopping.)

thanks
-- PMM


^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2024-03-28 16:16 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-03-24 16:55 [PATCH v2 0/6] hw/char: Implement the STM32L4x5 USART, UART and LPUART Arnaud Minier
2024-03-24 16:55 ` [PATCH v2 1/6] hw/misc/stm32l4x5_rcc: Propagate period when enabling a clock Arnaud Minier
2024-03-24 16:55 ` [PATCH v2 2/6] hw/char: Implement STM32L4x5 USART skeleton Arnaud Minier
2024-03-28 15:55   ` Peter Maydell
2024-03-24 16:55 ` [PATCH v2 3/6] hw/char/stm32l4x5_usart: Enable serial read and write Arnaud Minier
2024-03-28 15:59   ` Peter Maydell
2024-03-24 16:55 ` [PATCH v2 4/6] hw/char/stm32l4x5_usart: Add options for serial parameters setting Arnaud Minier
2024-03-28 16:03   ` Peter Maydell
2024-03-24 16:55 ` [PATCH v2 5/6] hw/arm: Add the USART to the stm32l4x5 SoC Arnaud Minier
2024-03-28 16:06   ` Peter Maydell
2024-03-24 16:55 ` [PATCH v2 6/6] tests/qtest: Add tests for the STM32L4x5 USART Arnaud Minier
2024-03-25  6:19   ` Thomas Huth
2024-03-28 16:14     ` Peter Maydell
2024-03-28 16:10 ` [PATCH v2 0/6] hw/char: Implement the STM32L4x5 USART, UART and LPUART Peter Maydell

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.