Hi Jacob,
On 10/9/24 06:34, Jacob Abrams wrote:
On 9/9/24 10:40, Philippe Mathieu-Daudé wrote:
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 <satur9n...@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 <satur9n...@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.
Hmm it does appear to be a documentation error in RM0351 section 40.8.
I have an STM32L476 board (chip is nearly identical to the STM32L475 minus the
display module) which is functioning with the USART driver code provided by STM
in their examples that uses 16-bit access for several of the USART registers,
see:
https://github.com/STMicroelectronics/cmsis_device_l4/blob/a2530753e86dd326a75467d28feb92e2ba7d0df2/Include/stm32l475xx.h#L950
OK, we need to mention that in a comment around, something like:
/*
* While the reference manual states "the peripheral registers
* have to be accessed by words (32 bits)", code -- like the
* STM32CubeL4 CMSIS Device MCU Component -- ran on real
* hardware shows some registers can be accessed by 16 bits.
*/
.valid.min_access_size = 2,
I have a contact at STM through my work that I can mention this to just to
double check.
Thanks, that is great.
Ultimately my goal is to get an STM32CubeL4 example such as UART_Printf running
as-is in Qemu and it was only while attempting to run such a project in Qemu
that I ran into this problem, see:
https://github.com/STMicroelectronics/STM32CubeL4/tree/master/Projects/STM32L476G-EVAL/Examples/UART/UART_Printf
It would be nice to add a test using this file in our functional
test suite, see for example tests/functional/test_avr_mega2560.py.
This project works on my HW dev board just fine.
Of course if you still desire I don't mind splitting this change into 3 parts.
Well, you clearly list 3 steps in the commit description.
Besides, the discussions whether .imp.min_access_size = 2
is valid still stands.
Regards,
Phil.
