All of lore.kernel.org
 help / color / mirror / Atom feed
From: Stefano Stabellini <sstabellini@kernel.org>
To: Michal Orzel <michal.orzel@amd.com>
Cc: xen-devel@lists.xenproject.org,
	 Stefano Stabellini <sstabellini@kernel.org>,
	Julien Grall <julien@xen.org>,
	 Bertrand Marquis <bertrand.marquis@arm.com>,
	 Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>
Subject: Re: [PATCH 1/4] xen/arm: debug-pl011: Use correct accessors
Date: Wed, 14 Jun 2023 18:36:37 -0700 (PDT)	[thread overview]
Message-ID: <alpine.DEB.2.22.394.2306141836050.897208@ubuntu-linux-20-04-desktop> (raw)
In-Reply-To: <20230607092727.19654-2-michal.orzel@amd.com>

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.orzel@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...

Everything else checks out.


>  .endm
>  
>  /*
> diff --git a/xen/arch/arm/arm64/debug-pl011.inc b/xen/arch/arm/arm64/debug-pl011.inc
> index 6d60e78c8ba3..df713eff4922 100644
> --- a/xen/arch/arm/arm64/debug-pl011.inc
> +++ b/xen/arch/arm/arm64/debug-pl011.inc
> @@ -25,13 +25,13 @@
>   */
>  .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) */
> +        strb  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) */
>          mov   x\c, #WLEN_8           /* 8n1 */
> -        str   w\c, [\xb, #LCR_H]     /* -> UARTLCR_H (Line control) */
> +        strb  w\c, [\xb, #LCR_H]     /* -> UARTLCR_H (Line control) */
>          ldr   x\c, =(RXE | TXE | UARTEN)
> -        str   w\c, [\xb, #CR]        /* -> UARTCR (Control Register) */
> +        strh  w\c, [\xb, #CR]        /* -> UARTCR (Control Register) */
>  .endm
>  
>  /*
> -- 
> 2.25.1
> 


  parent reply	other threads:[~2023-06-15  1:37 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-07  9:27 [PATCH 0/4] xen/arm: pl011: Use correct accessors Michal Orzel
2023-06-07  9:27 ` [PATCH 1/4] xen/arm: debug-pl011: " Michal Orzel
2023-06-13  2:59   ` Henry Wang
2023-06-15  1:36   ` Stefano Stabellini [this message]
2023-06-15  6:41     ` Michal Orzel
2023-06-15 21:03       ` Stefano Stabellini
2023-06-07  9:27 ` [PATCH 2/4] xen/arm: debug-pl011: Add support for 32-bit only MMIO Michal Orzel
2023-06-13  2:59   ` Henry Wang
2023-06-15  1:42   ` Stefano Stabellini
2023-06-07  9:27 ` [PATCH 3/4] xen/arm: pl011: Use correct accessors Michal Orzel
2023-06-13  2:59   ` Henry Wang
2023-06-15  1:49   ` Stefano Stabellini
2023-06-07  9:27 ` [PATCH 4/4] xen/arm: pl011: Add SBSA UART device-tree support Michal Orzel
2023-06-13  3:00   ` Henry Wang
2023-06-15  1:50   ` Stefano Stabellini

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alpine.DEB.2.22.394.2306141836050.897208@ubuntu-linux-20-04-desktop \
    --to=sstabellini@kernel.org \
    --cc=Volodymyr_Babchuk@epam.com \
    --cc=bertrand.marquis@arm.com \
    --cc=julien@xen.org \
    --cc=michal.orzel@amd.com \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.