Re: [Xen-devel] [PATCH V4 4/5] xen/arm: Extend SCIF early prink code to handle other interfaces

[Julien Grall <julien.grall@xxxxxxx>]

Hi Oleksandr,

On 17/04/2019 15:59, Oleksandr Tyshchenko wrote:
> From: Oleksandr Tyshchenko <oleksandr_tyshchenko@xxxxxxxx>
>
> Extend early prink code to be able to handle other SCIF(X)
> compatible interfaces as well. These interfaces have lot in common,
> but mostly differ in offsets and bits for some registers.
>
> Introduce "EARLY_PRINTK_VERSION" config option to choose which
> interface version should be used (to properly apply register offsets).
>
> Please note, nothing has been technically changed for Renesas "Lager"
> and other supported boards (SCIF).
>
> The "EARLY_PRINTK_VERSION" option for that board should be empty:
> CONFIG_EARLY_PRINTK=scif,0xe6e60000
>
> Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@xxxxxxxx>
> CC: Julien Grall <julien.grall@xxxxxxx>
>
> ---
>      Changes in v3:
>          - It was decided not to introduce new debug-scifa.inc
>            for handling SCIFA interface, but to extend existing
>            debug-scif.inc for handling both interfaces.
>            This patch is a result of splitting an initial patch
>            "xen/arm: Add SCIFA UART support for early printk"
>            and only reworks a code
>
>          Changes in v4:
>          - Update docs/misc/arm/early-printk.txt with the new option
> ---
>   docs/misc/arm/early-printk.txt    |  6 ++++++
>   xen/arch/arm/Rules.mk             |  7 +++++++
>   xen/arch/arm/arm32/debug-scif.inc | 13 +++++++++----
>   3 files changed, 22 insertions(+), 4 deletions(-)
>
> diff --git a/docs/misc/arm/early-printk.txt b/docs/misc/arm/early-printk.txt
> index b23c54f..c054674 100644
> --- a/docs/misc/arm/early-printk.txt
> +++ b/docs/misc/arm/early-printk.txt
> @@ -27,6 +27,12 @@ CONFIG_EARLY_PRINTK=<INC>,<BASE_ADDRESS>,<OTHER_OPTIONS>
>         If <BAUD_RATE> is not given then the code will not try to
>         initialize the UART, so that bootloader or firmware settings can
>        be used for maximum compatibility.
> +  - scif,<BASE_ADDRESS>,<VERSION>
> +    - <VERSION> is a option to choose which interface version should be used
By "a option" did you intend to mean it is an optional parameter?

If so, how about:

"SCIF<VERSION> is, optionally, the interface version of the UART."

So you would also provide a way for the user to know how to specify it. Stefano 
any opinion?
> +      If <VERSION> is not given then a default interface version (SCIF)
s/a/the/

> +      will be used.
>     - For all other uarts there are no additional options.
>   
>   As a convenience it is also possible to select from a list of
> diff --git a/xen/arch/arm/Rules.mk b/xen/arch/arm/Rules.mk
> index f264592..3d9a0ed 100644
> --- a/xen/arch/arm/Rules.mk
> +++ b/xen/arch/arm/Rules.mk
> @@ -68,6 +68,13 @@ EARLY_PRINTK_INIT_UART := y
>   EARLY_PRINTK_BAUD := $(word 3,$(EARLY_PRINTK_CFG))
>   endif
>   endif
> +ifeq ($(EARLY_PRINTK_INC),scif)
> +ifneq ($(word 3,$(EARLY_PRINTK_CFG)),)
> +CFLAGS-y += -DEARLY_PRINTK_VERSION_$(word 3,$(EARLY_PRINTK_CFG))
> +else
> +CFLAGS-y += -DEARLY_PRINTK_VERSION_NONE
> +endif
> +endif
>   
>   ifneq ($(EARLY_PRINTK_INC),)
>   EARLY_PRINTK := y
> diff --git a/xen/arch/arm/arm32/debug-scif.inc 
> b/xen/arch/arm/arm32/debug-scif.inc
> index 143f05d..a8d2eae 100644
> --- a/xen/arch/arm/arm32/debug-scif.inc
> +++ b/xen/arch/arm/arm32/debug-scif.inc
> @@ -19,6 +19,11 @@
>   
>   #include <asm/scif-uart.h>
>   
> +#ifdef EARLY_PRINTK_VERSION_NONE
> +#define STATUS_REG    SCIF_SCFSR
> +#define TX_FIFO_REG   SCIF_SCFTDR
> +#endif
> +
>   /*
>    * SCIF UART wait UART to be ready to transmit
>    * rb: register which contains the UART base address
> @@ -26,7 +31,7 @@
>    */
>   .macro early_uart_ready rb rc
>   1:
> -        ldrh   \rc, [\rb, #SCIF_SCFSR]   /* <- SCFSR (status register) */
> +        ldrh   \rc, [\rb, #STATUS_REG]   /* Read status register */
>           tst    \rc, #SCFSR_TDFE          /* Check TDFE bit */
>           beq    1b                        /* Wait for the UART to be ready */
>   .endm
> @@ -37,10 +42,10 @@
>    * rt: register which contains the character to transmit
>    */
>   .macro early_uart_transmit rb rt
> -        strb   \rt, [\rb, #SCIF_SCFTDR]                  /* -> SCFTDR (data 
> register) */
> -        ldrh   \rt, [\rb, #SCIF_SCFSR]                   /* <- SCFSR (status 
> register) */
> +        strb   \rt, [\rb, #TX_FIFO_REG]                  /* Write data 
> register */
> +        ldrh   \rt, [\rb, #STATUS_REG]                   /* Read status 
> register */
>           and    \rt, \rt, #(~(SCFSR_TEND | SCFSR_TDFE))   /* Clear TEND and 
> TDFE bits */
> -        strh   \rt, [\rb, #SCIF_SCFSR]                   /* -> SCFSR (status 
> register) */
> +        strh   \rt, [\rb, #STATUS_REG]                   /* Write status 
> register */
>   .endm
>   
>   /*
CHeers,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel