We already have all the bits necessary in PL011 driver to support SBSA
UART thanks to commit 032ea8c736d10f02672863c6e369338f948f7ed8 that
enabled it for ACPI. Plumb in the remaining part for device-tree boot:
 - add arm,sbsa-uart compatible to pl011_dt_match (no need for a separate
   struct and DT_DEVICE_START as SBSA is a subset of PL011),
 - from pl011_dt_uart_init(), check for SBSA UART compatible to determine
   the UART type in use.

Signed-off-by: Michal Orzel <michal.orzel@amd.com>
---
After this series the last thing not to be in spec for newer UARTs (well,
for rev1.5 introduced in 2007 I believe) is incorrect FIFO size. We hardcode it
to 16 but in r1.5 it is 32. This requires checking the peripheral ID register
or using arm,primecell-periphid dt property for overriding HW. Something to
be done in the future (at least 16 is not harmful).
---
 xen/drivers/char/pl011.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/xen/drivers/char/pl011.c b/xen/drivers/char/pl011.c
index 403b1ac06551..f7bf3ad117af 100644
--- a/xen/drivers/char/pl011.c
+++ b/xen/drivers/char/pl011.c
@@ -286,7 +286,7 @@ static int __init pl011_dt_uart_init(struct dt_device_node *dev,
     int res;
     paddr_t addr, size;
     uint32_t io_width;
-    bool mmio32 = false;
+    bool mmio32 = false, sbsa;
 
     if ( strcmp(config, "") )
     {
@@ -320,7 +320,9 @@ static int __init pl011_dt_uart_init(struct dt_device_node *dev,
         }
     }
 
-    res = pl011_uart_init(res, addr, size, false, mmio32);
+    sbsa = dt_device_is_compatible(dev, "arm,sbsa-uart");
+
+    res = pl011_uart_init(res, addr, size, sbsa, mmio32);
     if ( res < 0 )
     {
         printk("pl011: Unable to initialize\n");
@@ -335,6 +337,8 @@ static int __init pl011_dt_uart_init(struct dt_device_node *dev,
 static const struct dt_device_match pl011_dt_match[] __initconst =
 {
     DT_MATCH_COMPATIBLE("arm,pl011"),
+    /* No need for a separate struct as SBSA UART is a subset of PL011 */
+    DT_MATCH_COMPATIBLE("arm,sbsa-uart"),
     { /* sentinel */ },
 };
 
-- 
2.25.1

On Wed, 7 Jun 2023, Michal Orzel wrote:
> We already have all the bits necessary in PL011 driver to support SBSA
> UART thanks to commit 032ea8c736d10f02672863c6e369338f948f7ed8 that
> enabled it for ACPI. Plumb in the remaining part for device-tree boot:
>  - add arm,sbsa-uart compatible to pl011_dt_match (no need for a separate
>    struct and DT_DEVICE_START as SBSA is a subset of PL011),
>  - from pl011_dt_uart_init(), check for SBSA UART compatible to determine
>    the UART type in use.
> 
> Signed-off-by: Michal Orzel <michal.orzel@amd.com>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>


> ---
> After this series the last thing not to be in spec for newer UARTs (well,
> for rev1.5 introduced in 2007 I believe) is incorrect FIFO size. We hardcode it
> to 16 but in r1.5 it is 32. This requires checking the peripheral ID register
> or using arm,primecell-periphid dt property for overriding HW. Something to
> be done in the future (at least 16 is not harmful).
> ---
>  xen/drivers/char/pl011.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/drivers/char/pl011.c b/xen/drivers/char/pl011.c
> index 403b1ac06551..f7bf3ad117af 100644
> --- a/xen/drivers/char/pl011.c
> +++ b/xen/drivers/char/pl011.c
> @@ -286,7 +286,7 @@ static int __init pl011_dt_uart_init(struct dt_device_node *dev,
>      int res;
>      paddr_t addr, size;
>      uint32_t io_width;
> -    bool mmio32 = false;
> +    bool mmio32 = false, sbsa;
>  
>      if ( strcmp(config, "") )
>      {
> @@ -320,7 +320,9 @@ static int __init pl011_dt_uart_init(struct dt_device_node *dev,
>          }
>      }
>  
> -    res = pl011_uart_init(res, addr, size, false, mmio32);
> +    sbsa = dt_device_is_compatible(dev, "arm,sbsa-uart");
> +
> +    res = pl011_uart_init(res, addr, size, sbsa, mmio32);
>      if ( res < 0 )
>      {
>          printk("pl011: Unable to initialize\n");
> @@ -335,6 +337,8 @@ static int __init pl011_dt_uart_init(struct dt_device_node *dev,
>  static const struct dt_device_match pl011_dt_match[] __initconst =
>  {
>      DT_MATCH_COMPATIBLE("arm,pl011"),
> +    /* No need for a separate struct as SBSA UART is a subset of PL011 */
> +    DT_MATCH_COMPATIBLE("arm,sbsa-uart"),
>      { /* sentinel */ },
>  };
>  
> -- 
> 2.25.1
>

Hi Michal,

> -----Original Message-----
> Subject: [PATCH 4/4] xen/arm: pl011: Add SBSA UART device-tree support
> 
> We already have all the bits necessary in PL011 driver to support SBSA
> UART thanks to commit 032ea8c736d10f02672863c6e369338f948f7ed8 that
> enabled it for ACPI. Plumb in the remaining part for device-tree boot:
>  - add arm,sbsa-uart compatible to pl011_dt_match (no need for a separate
>    struct and DT_DEVICE_START as SBSA is a subset of PL011),
>  - from pl011_dt_uart_init(), check for SBSA UART compatible to determine
>    the UART type in use.
> 
> Signed-off-by: Michal Orzel <michal.orzel@amd.com>

Reviewed-by: Henry Wang <Henry.Wang@arm.com>

I've also tested this patch on top of today's staging on FVP arm32 and arm64
and confirm this patch will not break existing functionality. So:

Tested-by: Henry Wang <Henry.Wang@arm.com>

Kind regards,
Henry