Hi Stefano, On 15/06/2023 03:36, Stefano Stabellini wrote: > > > On Wed, 7 Jun 2023, Michal Orzel wrote: >> Although most PL011 UARTs can cope with 32-bit accesses, some of the old >> legacy ones might not. PL011 registers are 8/16-bit wide and this shall >> be perceived as the normal behavior. >> >> Modify early printk pl011 code for arm32/arm64 to use the correct >> accessors depending on the register size (refer ARM DDI 0183G, Table 3.1). >> >> Signed-off-by: Michal Orzel <michal.or...@amd.com> >> --- >> Next patch will override strX,ldrX with macros but I prefer to keep the >> history clean (+ possibiltity for a backport if needed). >> --- >> xen/arch/arm/arm32/debug-pl011.inc | 12 ++++++------ >> xen/arch/arm/arm64/debug-pl011.inc | 6 +++--- >> 2 files changed, 9 insertions(+), 9 deletions(-) >> >> diff --git a/xen/arch/arm/arm32/debug-pl011.inc >> b/xen/arch/arm/arm32/debug-pl011.inc >> index c527f1d4424d..9fe0c2503831 100644 >> --- a/xen/arch/arm/arm32/debug-pl011.inc >> +++ b/xen/arch/arm/arm32/debug-pl011.inc >> @@ -26,13 +26,13 @@ >> */ >> .macro early_uart_init rb, rc, rd >> mov \rc, #(7372800 / CONFIG_EARLY_UART_PL011_BAUD_RATE % 16) >> - str \rc, [\rb, #FBRD] /* -> UARTFBRD (Baud divisor fraction) >> */ >> + strb \rc, [\rb, #FBRD] /* -> UARTFBRD (Baud divisor fraction) >> */ >> mov \rc, #(7372800 / CONFIG_EARLY_UART_PL011_BAUD_RATE / 16) >> - str \rc, [\rb, #IBRD] /* -> UARTIBRD (Baud divisor integer) */ >> + strh \rc, [\rb, #IBRD] /* -> UARTIBRD (Baud divisor integer) */ >> mov \rc, #WLEN_8 /* 8n1 */ >> - str \rc, [\rb, #LCR_H] /* -> UARTLCR_H (Line control) */ >> + strb \rc, [\rb, #LCR_H] /* -> UARTLCR_H (Line control) */ >> ldr \rc, =(RXE | TXE | UARTEN) /* RXE | TXE | UARTEN */ >> - str \rc, [\rb, #CR] /* -> UARTCR (Control Register) */ >> + strh \rc, [\rb, #CR] /* -> UARTCR (Control Register) */ >> .endm >> >> /* >> @@ -42,7 +42,7 @@ >> */ >> .macro early_uart_ready rb, rc >> 1: >> - ldr \rc, [\rb, #FR] /* <- UARTFR (Flag register) */ >> + ldrh \rc, [\rb, #FR] /* <- UARTFR (Flag register) */ >> tst \rc, #BUSY /* Check BUSY bit */ >> bne 1b /* Wait for the UART to be ready */ >> .endm >> @@ -53,7 +53,7 @@ >> * rt: register which contains the character to transmit >> */ >> .macro early_uart_transmit rb, rt >> - str \rt, [\rb, #DR] /* -> UARTDR (Data Register) */ >> + strb \rt, [\rb, #DR] /* -> UARTDR (Data Register) */ > > Isn't UARTDR potentially 12-bit? I am not sure if we should use strb or > strh here... UARTDR is 16-bit register where bits 15:12 are reserved and 11:8 are for indicating errors during READ. Here, we perform WRITE meaning the actual register width is 8 bytes. This is also indicated by the PL011 TRM which specifies width as "12/8" meaning 12 for READ and 8 for WRITE.
~Michal
