Hi,
(Cc'ing Arnaud & Inès who are listed as maintainers)
On 6/9/24 18:12, Peter Maydell wrote:
On Mon, 2 Sept 2024 at 14:38, Jacob Abrams <satur9nine@gmail.com> wrote:
These changes allow the official STM32L4xx HAL UART driver to function
properly with the b-l475e-iot01a machine.
Modifying USART_CR1 TE bit should alter USART_ISR TEACK bit, and
likewise for RE and REACK bit.
USART registers may be accessed via 16-bit instructions.
Reseting USART_CR1 UE bit should restore ISR to default value.
Fixes: 87b77e6e01ca ("hw/char/stm32l4x5_usart: Enable serial read and write")
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2540
Signed-off-by: Jacob Abrams <satur9nine@gmail.com>
Thanks for this patch. I have one question below, and one
minor nit.
---
hw/char/stm32l4x5_usart.c | 29 +++++++++++++++++++---
tests/qtest/stm32l4x5_usart-test.c | 39 +++++++++++++++++++++++++++++-
2 files changed, 64 insertions(+), 4 deletions(-)
static void stm32l4x5_update_irq(Stm32l4x5UsartBaseState *s)
{
if (((s->isr & R_ISR_WUF_MASK) && (s->cr3 & R_CR3_WUFIE_MASK)) ||
@@ -367,7 +389,7 @@ static void stm32l4x5_usart_base_reset_hold(Object *obj,
ResetType type)
s->brr = 0x00000000;
s->gtpr = 0x00000000;
s->rtor = 0x00000000;
- s->isr = 0x020000C0;
+ s->isr = ISR_RESET_VALUE;
s->rdr = 0x00000000;
s->tdr = 0x00000000;
@@ -456,6 +478,7 @@ static void stm32l4x5_usart_base_write(void *opaque, hwaddr
addr,
case A_CR1:
s->cr1 = value;
stm32l4x5_update_params(s);
+ stm32l4x5_update_isr(s);
stm32l4x5_update_irq(s);
return;
case A_CR2:
@@ -508,12 +531,12 @@ static const MemoryRegionOps stm32l4x5_usart_base_ops = {
.endianness = DEVICE_NATIVE_ENDIAN,
.valid = {
.max_access_size = 4,
- .min_access_size = 4,
+ .min_access_size = 2,
.unaligned = false
},
.impl = {
.max_access_size = 4,
- .min_access_size = 4,
+ .min_access_size = 2,
.unaligned = false
},
The effect of these is that a 16-bit write not aligned
to a (4-aligned) register offset will generate a GUEST_ERROR
logged message, and a 16-bit write aligned to a 4-aligned
register offset will write the value zero-extended to 32 bits.
That seems reasonable to me.
Peter, are you describing the .valid.min_access_size 4 -> 2 change
or the .impl.min_access_size one?
My understanding of the implementation is a 32-bit one:
REG32(CR1, 0x00)
struct Stm32l4x5UsartBaseState {
...
uint32_t cr1;
static void stm32l4x5_usart_base_write(void *opaque, hwaddr addr,
uint64_t val64, unsigned int size)
{
...
switch (addr) {
case A_CR1:
s->cr1 = value;
Am I missing something?
Now, back to .valid.min_access_size, per the section "40.8 USART
registers" of the reference manual:
The peripheral registers have to be accessed by words (32 bits).
So I don't get the "USART registers may be accessed via 16-bit
instructions." part of this patch.
Jacob, for clarity, can you split this patch in 3 distinct parts
(TE bit, UE bit, unaligned access) so this discussion doesn't delay
the part which are OK?
Thanks,
Phil.