On Thu, 1 Jun 2023 at 13:48, Michal Orzel <michal.or...@amd.com
<mailto:michal.or...@amd.com>> wrote:
Hi Julien,
On 01/06/2023 13:12, Julien Grall wrote:
>
>
>
> Hi,
>
> Sorry for the formatting.
>
> On Thu, 1 Jun 2023 at 12:31, Michal Orzel <michal.or...@amd.com
<mailto:michal.or...@amd.com> <mailto:michal.or...@amd.com
<mailto:michal.or...@amd.com>>> wrote:
>
> Hi Bertrand,
>
> On 01/06/2023 12:19, Bertrand Marquis wrote:
> >
> >
> > Hi Michal,
> >
> >> On 1 Jun 2023, at 10:50, Michal Orzel <michal.or...@amd.com
<mailto:michal.or...@amd.com> <mailto:michal.or...@amd.com <mailto:michal.or...@amd.com>>>
wrote:
> >>
> >> There are implementations of the PL011 that can only handle 32-bit
> >> accesses (i.e. no 16-bit or 8-bit), usually advertised by
'reg-io-width'
> >> dt property set to 4. On such UARTs, the current early printk
code for
> >> arm64 does not work. To fix this issue, make all the accesses to
be 32-bit
> >> by using ldr, str without a size field. This makes it possible to
use
> >> early printk on such platforms, while all the other
implementations should
> >> generally cope with 32-bit accesses. In case they do not, they
would
> >> already fail as we explicitly use writel/readl in the runtime
driver to
> >> maintain broader compatibility and to be SBSAv2 compliant.
Therefore, this
> >> change makes the runtime/early handling consistent (also it
matches the
> >> arm32 debug-pl011 code).
> >>
> >> Signed-off-by: Michal Orzel <michal.or...@amd.com <mailto:michal.or...@amd.com>
<mailto:michal.or...@amd.com <mailto:michal.or...@amd.com>>>
> >> ---
> >> xen/arch/arm/arm64/debug-pl011.inc | 8 ++++----
> >> 1 file changed, 4 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/xen/arch/arm/arm64/debug-pl011.inc
b/xen/arch/arm/arm64/debug-pl011.inc
> >> index 6d60e78c8ba3..80eb8fdc1ec7 100644
> >> --- a/xen/arch/arm/arm64/debug-pl011.inc
> >> +++ b/xen/arch/arm/arm64/debug-pl011.inc
> >> @@ -25,9 +25,9 @@
> >> */
> >> .macro early_uart_init xb, c
> >> mov x\c, #(7372800 / CONFIG_EARLY_UART_PL011_BAUD_RATE
% 16)
> >> - strh w\c, [\xb, #FBRD] /* -> UARTFBRD (Baud
divisor fraction) */
> >> + str w\c, [\xb, #FBRD] /* -> UARTFBRD (Baud
divisor fraction) */
> >> mov x\c, #(7372800 / CONFIG_EARLY_UART_PL011_BAUD_RATE
/ 16)
> >> - strh w\c, [\xb, #IBRD] /* -> UARTIBRD (Baud
divisor integer) */
> >> + str w\c, [\xb, #IBRD] /* -> UARTIBRD (Baud
divisor integer) */
> >> mov x\c, #WLEN_8 /* 8n1 */
> >> str w\c, [\xb, #LCR_H] /* -> UARTLCR_H (Line
control) */
> >> ldr x\c, =(RXE | TXE | UARTEN)
> >> @@ -41,7 +41,7 @@
> >> */
> >> .macro early_uart_ready xb, c
> >> 1:
> >> - ldrh w\c, [\xb, #FR] /* <- UARTFR (Flag
register) */
> >> + ldr w\c, [\xb, #FR] /* <- UARTFR (Flag
register) */
> >> tst w\c, #BUSY /* Check BUSY bit */
> >> b.ne <http://b.ne> <http://b.ne <http://b.ne>> 1b
/* Wait for the UART to be ready */
> >> .endm
> >> @@ -52,7 +52,7 @@
> >> * wt: register which contains the character to transmit
> >> */
> >> .macro early_uart_transmit xb, wt
> >> - strb \wt, [\xb, #DR] /* -> UARTDR (Data
Register) */
> >> + str \wt, [\xb, #DR] /* -> UARTDR (Data
Register) */
> >
> > Is it really ok to drop the 8bit access here ?
> It is not only ok, it is necessary. Otherwise it won't work on the
above mentioned UARTs (they can only perform 32-bit access).
>
>
> IIRC some compilers will complain because you use wN with “str”.
Hmm, I would expect it to be totally ok as the size is determined by the
reg name. Any reference?
I don’t have the spec with me. I will have a look on Monday and reply back here.
