diff mbox series

[v4,03/11] hw: allwinner-r40: Complete uart devices

Message ID 20230510103004.30015-4-qianfanguijin@163.com
State New
Headers show
Series *** Add allwinner r40 support *** | expand

Commit Message

qianfan May 10, 2023, 10:29 a.m. UTC 
From: qianfan Zhao <qianfanguijin@163.com>

R40 has eight UARTs, support both 16450 and 16550 compatible modes.

Signed-off-by: qianfan Zhao <qianfanguijin@163.com>
---
 hw/arm/allwinner-r40.c         | 31 ++++++++++++++++++++++++++++---
 include/hw/arm/allwinner-r40.h |  8 ++++++++
 2 files changed, 36 insertions(+), 3 deletions(-)

Comments

Niek Linnenbank May 14, 2023, 6:55 p.m. UTC | #1 
Hi Qianfan,


On Wed, May 10, 2023 at 12:30 PM <qianfanguijin@163.com> wrote:

> From: qianfan Zhao <qianfanguijin@163.com>
>
> R40 has eight UARTs, support both 16450 and 16550 compatible modes.
>
> Signed-off-by: qianfan Zhao <qianfanguijin@163.com>
> ---
>  hw/arm/allwinner-r40.c         | 31 ++++++++++++++++++++++++++++---
>  include/hw/arm/allwinner-r40.h |  8 ++++++++
>  2 files changed, 36 insertions(+), 3 deletions(-)
>
> diff --git a/hw/arm/allwinner-r40.c b/hw/arm/allwinner-r40.c
> index 128c0ca470..537a90b23d 100644
> --- a/hw/arm/allwinner-r40.c
> +++ b/hw/arm/allwinner-r40.c
> @@ -45,6 +45,13 @@ const hwaddr allwinner_r40_memmap[] = {
>      [AW_R40_DEV_CCU]        = 0x01c20000,
>      [AW_R40_DEV_PIT]        = 0x01c20c00,
>      [AW_R40_DEV_UART0]      = 0x01c28000,
> +    [AW_R40_DEV_UART1]      = 0x01c28400,
> +    [AW_R40_DEV_UART2]      = 0x01c28800,
> +    [AW_R40_DEV_UART3]      = 0x01c28c00,
> +    [AW_R40_DEV_UART4]      = 0x01c29000,
> +    [AW_R40_DEV_UART5]      = 0x01c29400,
> +    [AW_R40_DEV_UART6]      = 0x01c29800,
> +    [AW_R40_DEV_UART7]      = 0x01c29c00,
>

After adding the uarts to the memory map here, you should remove them from
the unimplemented array.

     [AW_R40_DEV_GIC_DIST]   = 0x01c81000,
>      [AW_R40_DEV_GIC_CPU]    = 0x01c82000,
>      [AW_R40_DEV_GIC_HYP]    = 0x01c84000,
> @@ -160,6 +167,10 @@ enum {
>      AW_R40_GIC_SPI_UART1     =  2,
>      AW_R40_GIC_SPI_UART2     =  3,
>      AW_R40_GIC_SPI_UART3     =  4,
>

Since you put the addition of UART1-7 in this patch, probably it makes
sense to have adding the lines 'AW_R40_GIC_SPI_UART1/2/3' also part of this
patch.

With the two above remarks resolved, the patch looks good to me.

Reviewed-by: Niek Linnenbank <nieklinnenbank@gmail.com>

Regards,
Niek

> +    AW_R40_GIC_SPI_UART4     = 17,
> +    AW_R40_GIC_SPI_UART5     = 18,
> +    AW_R40_GIC_SPI_UART6     = 19,
> +    AW_R40_GIC_SPI_UART7     = 20,
>      AW_R40_GIC_SPI_TIMER0    = 22,
>      AW_R40_GIC_SPI_TIMER1    = 23,
>      AW_R40_GIC_SPI_MMC0      = 32,
> @@ -387,9 +398,23 @@ static void allwinner_r40_realize(DeviceState *dev,
> Error **errp)
>      }
>
>      /* UART0. For future clocktree API: All UARTS are connected to
> APB2_CLK. */
> -    serial_mm_init(get_system_memory(), s->memmap[AW_R40_DEV_UART0], 2,
> -                   qdev_get_gpio_in(DEVICE(&s->gic),
> AW_R40_GIC_SPI_UART0),
> -                   115200, serial_hd(0), DEVICE_NATIVE_ENDIAN);
> +    for (int i = 0; i < AW_R40_NUM_UARTS; i++) {
> +        static const int uart_irqs[AW_R40_NUM_UARTS] = {
> +            AW_R40_GIC_SPI_UART0,
> +            AW_R40_GIC_SPI_UART1,
> +            AW_R40_GIC_SPI_UART2,
> +            AW_R40_GIC_SPI_UART3,
> +            AW_R40_GIC_SPI_UART4,
> +            AW_R40_GIC_SPI_UART5,
> +            AW_R40_GIC_SPI_UART6,
> +            AW_R40_GIC_SPI_UART7,
> +        };
> +        const hwaddr addr = s->memmap[AW_R40_DEV_UART0 + i];
> +
> +        serial_mm_init(get_system_memory(), addr, 2,
> +                       qdev_get_gpio_in(DEVICE(&s->gic), uart_irqs[i]),
> +                       115200, serial_hd(i), DEVICE_NATIVE_ENDIAN);
> +    }
>
>      /* Unimplemented devices */
>      for (i = 0; i < ARRAY_SIZE(r40_unimplemented); i++) {
> diff --git a/include/hw/arm/allwinner-r40.h
> b/include/hw/arm/allwinner-r40.h
> index 3be9dc962b..959b5dc4e0 100644
> --- a/include/hw/arm/allwinner-r40.h
> +++ b/include/hw/arm/allwinner-r40.h
> @@ -41,6 +41,13 @@ enum {
>      AW_R40_DEV_CCU,
>      AW_R40_DEV_PIT,
>      AW_R40_DEV_UART0,
> +    AW_R40_DEV_UART1,
> +    AW_R40_DEV_UART2,
> +    AW_R40_DEV_UART3,
> +    AW_R40_DEV_UART4,
> +    AW_R40_DEV_UART5,
> +    AW_R40_DEV_UART6,
> +    AW_R40_DEV_UART7,
>      AW_R40_DEV_GIC_DIST,
>      AW_R40_DEV_GIC_CPU,
>      AW_R40_DEV_GIC_HYP,
> @@ -70,6 +77,7 @@ OBJECT_DECLARE_SIMPLE_TYPE(AwR40State, AW_R40)
>   * which are currently emulated by the R40 SoC code.
>   */
>  #define AW_R40_NUM_MMCS         4
> +#define AW_R40_NUM_UARTS        8
>
>  struct AwR40State {
>      /*< private >*/
> --
> 2.25.1
>
>
qianfan May 23, 2023, 1:24 a.m. UTC | #2 
在 2023/5/15 2:55, Niek Linnenbank 写道:
> Hi Qianfan,
>
>
> On Wed, May 10, 2023 at 12:30 PM <qianfanguijin@163.com> wrote:
>
>     From: qianfan Zhao <qianfanguijin@163.com>
>
>     R40 has eight UARTs, support both 16450 and 16550 compatible modes.
>
>     Signed-off-by: qianfan Zhao <qianfanguijin@163.com>
>     ---
>      hw/arm/allwinner-r40.c         | 31 ++++++++++++++++++++++++++++---
>      include/hw/arm/allwinner-r40.h |  8 ++++++++
>      2 files changed, 36 insertions(+), 3 deletions(-)
>
>     diff --git a/hw/arm/allwinner-r40.c b/hw/arm/allwinner-r40.c
>     index 128c0ca470..537a90b23d 100644
>     --- a/hw/arm/allwinner-r40.c
>     +++ b/hw/arm/allwinner-r40.c
>     @@ -45,6 +45,13 @@ const hwaddr allwinner_r40_memmap[] = {
>          [AW_R40_DEV_CCU]        = 0x01c20000,
>          [AW_R40_DEV_PIT]        = 0x01c20c00,
>          [AW_R40_DEV_UART0]      = 0x01c28000,
>     +    [AW_R40_DEV_UART1]      = 0x01c28400,
>     +    [AW_R40_DEV_UART2]      = 0x01c28800,
>     +    [AW_R40_DEV_UART3]      = 0x01c28c00,
>     +    [AW_R40_DEV_UART4]      = 0x01c29000,
>     +    [AW_R40_DEV_UART5]      = 0x01c29400,
>     +    [AW_R40_DEV_UART6]      = 0x01c29800,
>     +    [AW_R40_DEV_UART7]      = 0x01c29c00,
>
>
> After adding the uarts to the memory map here, you should remove them 
> from the unimplemented array.
OK.
>
>          [AW_R40_DEV_GIC_DIST]   = 0x01c81000,
>          [AW_R40_DEV_GIC_CPU]    = 0x01c82000,
>          [AW_R40_DEV_GIC_HYP]    = 0x01c84000,
>     @@ -160,6 +167,10 @@ enum {
>          AW_R40_GIC_SPI_UART1     =  2,
>          AW_R40_GIC_SPI_UART2     =  3,
>          AW_R40_GIC_SPI_UART3     =  4,
>
>
> Since you put the addition of UART1-7 in this patch, probably it makes 
> sense to have adding the lines 'AW_R40_GIC_SPI_UART1/2/3' also part of 
> this patch.
OK
>
> With the two above remarks resolved, the patch looks good to me.
>
> Reviewed-by: Niek Linnenbank <nieklinnenbank@gmail.com>
>
> Regards,
> Niek
>
>     +    AW_R40_GIC_SPI_UART4     = 17,
>     +    AW_R40_GIC_SPI_UART5     = 18,
>     +    AW_R40_GIC_SPI_UART6     = 19,
>     +    AW_R40_GIC_SPI_UART7     = 20,
>          AW_R40_GIC_SPI_TIMER0    = 22,
>          AW_R40_GIC_SPI_TIMER1    = 23,
>          AW_R40_GIC_SPI_MMC0      = 32,
>     @@ -387,9 +398,23 @@ static void allwinner_r40_realize(DeviceState
>     *dev, Error **errp)
>          }
>
>          /* UART0. For future clocktree API: All UARTS are connected
>     to APB2_CLK. */
>     -    serial_mm_init(get_system_memory(),
>     s->memmap[AW_R40_DEV_UART0], 2,
>     -                   qdev_get_gpio_in(DEVICE(&s->gic),
>     AW_R40_GIC_SPI_UART0),
>     -                   115200, serial_hd(0), DEVICE_NATIVE_ENDIAN);
>     +    for (int i = 0; i < AW_R40_NUM_UARTS; i++) {
>     +        static const int uart_irqs[AW_R40_NUM_UARTS] = {
>     +            AW_R40_GIC_SPI_UART0,
>     +            AW_R40_GIC_SPI_UART1,
>     +            AW_R40_GIC_SPI_UART2,
>     +            AW_R40_GIC_SPI_UART3,
>     +            AW_R40_GIC_SPI_UART4,
>     +            AW_R40_GIC_SPI_UART5,
>     +            AW_R40_GIC_SPI_UART6,
>     +            AW_R40_GIC_SPI_UART7,
>     +        };
>     +        const hwaddr addr = s->memmap[AW_R40_DEV_UART0 + i];
>     +
>     +        serial_mm_init(get_system_memory(), addr, 2,
>     +  qdev_get_gpio_in(DEVICE(&s->gic), uart_irqs[i]),
>     +                       115200, serial_hd(i), DEVICE_NATIVE_ENDIAN);
>     +    }
>
>          /* Unimplemented devices */
>          for (i = 0; i < ARRAY_SIZE(r40_unimplemented); i++) {
>     diff --git a/include/hw/arm/allwinner-r40.h
>     b/include/hw/arm/allwinner-r40.h
>     index 3be9dc962b..959b5dc4e0 100644
>     --- a/include/hw/arm/allwinner-r40.h
>     +++ b/include/hw/arm/allwinner-r40.h
>     @@ -41,6 +41,13 @@ enum {
>          AW_R40_DEV_CCU,
>          AW_R40_DEV_PIT,
>          AW_R40_DEV_UART0,
>     +    AW_R40_DEV_UART1,
>     +    AW_R40_DEV_UART2,
>     +    AW_R40_DEV_UART3,
>     +    AW_R40_DEV_UART4,
>     +    AW_R40_DEV_UART5,
>     +    AW_R40_DEV_UART6,
>     +    AW_R40_DEV_UART7,
>          AW_R40_DEV_GIC_DIST,
>          AW_R40_DEV_GIC_CPU,
>          AW_R40_DEV_GIC_HYP,
>     @@ -70,6 +77,7 @@ OBJECT_DECLARE_SIMPLE_TYPE(AwR40State, AW_R40)
>       * which are currently emulated by the R40 SoC code.
>       */
>      #define AW_R40_NUM_MMCS         4
>     +#define AW_R40_NUM_UARTS        8
>
>      struct AwR40State {
>          /*< private >*/
>     -- 
>     2.25.1
>
>
>
> -- 
> Niek Linnenbank
>
qianfan May 23, 2023, 3 a.m. UTC | #3 
在 2023/5/15 2:55, Niek Linnenbank 写道:
> Hi Qianfan,
>
>
> On Wed, May 10, 2023 at 12:30 PM <qianfanguijin@163.com> wrote:
>
>     From: qianfan Zhao <qianfanguijin@163.com>
>
>     R40 has eight UARTs, support both 16450 and 16550 compatible modes.
>
>     Signed-off-by: qianfan Zhao <qianfanguijin@163.com>
>     ---
>      hw/arm/allwinner-r40.c         | 31 ++++++++++++++++++++++++++++---
>      include/hw/arm/allwinner-r40.h |  8 ++++++++
>      2 files changed, 36 insertions(+), 3 deletions(-)
>
>     diff --git a/hw/arm/allwinner-r40.c b/hw/arm/allwinner-r40.c
>     index 128c0ca470..537a90b23d 100644
>     --- a/hw/arm/allwinner-r40.c
>     +++ b/hw/arm/allwinner-r40.c
>     @@ -45,6 +45,13 @@ const hwaddr allwinner_r40_memmap[] = {
>          [AW_R40_DEV_CCU]        = 0x01c20000,
>          [AW_R40_DEV_PIT]        = 0x01c20c00,
>          [AW_R40_DEV_UART0]      = 0x01c28000,
>     +    [AW_R40_DEV_UART1]      = 0x01c28400,
>     +    [AW_R40_DEV_UART2]      = 0x01c28800,
>     +    [AW_R40_DEV_UART3]      = 0x01c28c00,
>     +    [AW_R40_DEV_UART4]      = 0x01c29000,
>     +    [AW_R40_DEV_UART5]      = 0x01c29400,
>     +    [AW_R40_DEV_UART6]      = 0x01c29800,
>     +    [AW_R40_DEV_UART7]      = 0x01c29c00,
>
>
> After adding the uarts to the memory map here, you should remove them 
> from the unimplemented array.
Hi:

I had tried this including remove UART0 from allwinner_r40_memmap, but 
that will make qemu
for R40 doesn't work again. Only a few registers are implemented in 
hw/char/serial.c,
so we still need this.
>
>          [AW_R40_DEV_GIC_DIST]   = 0x01c81000,
>          [AW_R40_DEV_GIC_CPU]    = 0x01c82000,
>          [AW_R40_DEV_GIC_HYP]    = 0x01c84000,
>     @@ -160,6 +167,10 @@ enum {
>          AW_R40_GIC_SPI_UART1     =  2,
>          AW_R40_GIC_SPI_UART2     =  3,
>          AW_R40_GIC_SPI_UART3     =  4,
>
>
> Since you put the addition of UART1-7 in this patch, probably it makes 
> sense to have adding the lines 'AW_R40_GIC_SPI_UART1/2/3' also part of 
> this patch.
>
> With the two above remarks resolved, the patch looks good to me.
>
> Reviewed-by: Niek Linnenbank <nieklinnenbank@gmail.com>
>
> Regards,
> Niek
>
>     +    AW_R40_GIC_SPI_UART4     = 17,
>     +    AW_R40_GIC_SPI_UART5     = 18,
>     +    AW_R40_GIC_SPI_UART6     = 19,
>     +    AW_R40_GIC_SPI_UART7     = 20,
>          AW_R40_GIC_SPI_TIMER0    = 22,
>          AW_R40_GIC_SPI_TIMER1    = 23,
>          AW_R40_GIC_SPI_MMC0      = 32,
>     @@ -387,9 +398,23 @@ static void allwinner_r40_realize(DeviceState
>     *dev, Error **errp)
>          }
>
>          /* UART0. For future clocktree API: All UARTS are connected
>     to APB2_CLK. */
>     -    serial_mm_init(get_system_memory(),
>     s->memmap[AW_R40_DEV_UART0], 2,
>     -                   qdev_get_gpio_in(DEVICE(&s->gic),
>     AW_R40_GIC_SPI_UART0),
>     -                   115200, serial_hd(0), DEVICE_NATIVE_ENDIAN);
>     +    for (int i = 0; i < AW_R40_NUM_UARTS; i++) {
>     +        static const int uart_irqs[AW_R40_NUM_UARTS] = {
>     +            AW_R40_GIC_SPI_UART0,
>     +            AW_R40_GIC_SPI_UART1,
>     +            AW_R40_GIC_SPI_UART2,
>     +            AW_R40_GIC_SPI_UART3,
>     +            AW_R40_GIC_SPI_UART4,
>     +            AW_R40_GIC_SPI_UART5,
>     +            AW_R40_GIC_SPI_UART6,
>     +            AW_R40_GIC_SPI_UART7,
>     +        };
>     +        const hwaddr addr = s->memmap[AW_R40_DEV_UART0 + i];
>     +
>     +        serial_mm_init(get_system_memory(), addr, 2,
>     +  qdev_get_gpio_in(DEVICE(&s->gic), uart_irqs[i]),
>     +                       115200, serial_hd(i), DEVICE_NATIVE_ENDIAN);
>     +    }
>
>          /* Unimplemented devices */
>          for (i = 0; i < ARRAY_SIZE(r40_unimplemented); i++) {
>     diff --git a/include/hw/arm/allwinner-r40.h
>     b/include/hw/arm/allwinner-r40.h
>     index 3be9dc962b..959b5dc4e0 100644
>     --- a/include/hw/arm/allwinner-r40.h
>     +++ b/include/hw/arm/allwinner-r40.h
>     @@ -41,6 +41,13 @@ enum {
>          AW_R40_DEV_CCU,
>          AW_R40_DEV_PIT,
>          AW_R40_DEV_UART0,
>     +    AW_R40_DEV_UART1,
>     +    AW_R40_DEV_UART2,
>     +    AW_R40_DEV_UART3,
>     +    AW_R40_DEV_UART4,
>     +    AW_R40_DEV_UART5,
>     +    AW_R40_DEV_UART6,
>     +    AW_R40_DEV_UART7,
>          AW_R40_DEV_GIC_DIST,
>          AW_R40_DEV_GIC_CPU,
>          AW_R40_DEV_GIC_HYP,
>     @@ -70,6 +77,7 @@ OBJECT_DECLARE_SIMPLE_TYPE(AwR40State, AW_R40)
>       * which are currently emulated by the R40 SoC code.
>       */
>      #define AW_R40_NUM_MMCS         4
>     +#define AW_R40_NUM_UARTS        8
>
>      struct AwR40State {
>          /*< private >*/
>     -- 
>     2.25.1
>
>
>
> -- 
> Niek Linnenbank
>
diff mbox series

Patch

diff --git a/hw/arm/allwinner-r40.c b/hw/arm/allwinner-r40.c
index 128c0ca470..537a90b23d 100644
--- a/hw/arm/allwinner-r40.c
+++ b/hw/arm/allwinner-r40.c
@@ -45,6 +45,13 @@  const hwaddr allwinner_r40_memmap[] = {
     [AW_R40_DEV_CCU]        = 0x01c20000,
     [AW_R40_DEV_PIT]        = 0x01c20c00,
     [AW_R40_DEV_UART0]      = 0x01c28000,
+    [AW_R40_DEV_UART1]      = 0x01c28400,
+    [AW_R40_DEV_UART2]      = 0x01c28800,
+    [AW_R40_DEV_UART3]      = 0x01c28c00,
+    [AW_R40_DEV_UART4]      = 0x01c29000,
+    [AW_R40_DEV_UART5]      = 0x01c29400,
+    [AW_R40_DEV_UART6]      = 0x01c29800,
+    [AW_R40_DEV_UART7]      = 0x01c29c00,
     [AW_R40_DEV_GIC_DIST]   = 0x01c81000,
     [AW_R40_DEV_GIC_CPU]    = 0x01c82000,
     [AW_R40_DEV_GIC_HYP]    = 0x01c84000,
@@ -160,6 +167,10 @@  enum {
     AW_R40_GIC_SPI_UART1     =  2,
     AW_R40_GIC_SPI_UART2     =  3,
     AW_R40_GIC_SPI_UART3     =  4,
+    AW_R40_GIC_SPI_UART4     = 17,
+    AW_R40_GIC_SPI_UART5     = 18,
+    AW_R40_GIC_SPI_UART6     = 19,
+    AW_R40_GIC_SPI_UART7     = 20,
     AW_R40_GIC_SPI_TIMER0    = 22,
     AW_R40_GIC_SPI_TIMER1    = 23,
     AW_R40_GIC_SPI_MMC0      = 32,
@@ -387,9 +398,23 @@  static void allwinner_r40_realize(DeviceState *dev, Error **errp)
     }
 
     /* UART0. For future clocktree API: All UARTS are connected to APB2_CLK. */
-    serial_mm_init(get_system_memory(), s->memmap[AW_R40_DEV_UART0], 2,
-                   qdev_get_gpio_in(DEVICE(&s->gic), AW_R40_GIC_SPI_UART0),
-                   115200, serial_hd(0), DEVICE_NATIVE_ENDIAN);
+    for (int i = 0; i < AW_R40_NUM_UARTS; i++) {
+        static const int uart_irqs[AW_R40_NUM_UARTS] = {
+            AW_R40_GIC_SPI_UART0,
+            AW_R40_GIC_SPI_UART1,
+            AW_R40_GIC_SPI_UART2,
+            AW_R40_GIC_SPI_UART3,
+            AW_R40_GIC_SPI_UART4,
+            AW_R40_GIC_SPI_UART5,
+            AW_R40_GIC_SPI_UART6,
+            AW_R40_GIC_SPI_UART7,
+        };
+        const hwaddr addr = s->memmap[AW_R40_DEV_UART0 + i];
+
+        serial_mm_init(get_system_memory(), addr, 2,
+                       qdev_get_gpio_in(DEVICE(&s->gic), uart_irqs[i]),
+                       115200, serial_hd(i), DEVICE_NATIVE_ENDIAN);
+    }
 
     /* Unimplemented devices */
     for (i = 0; i < ARRAY_SIZE(r40_unimplemented); i++) {
diff --git a/include/hw/arm/allwinner-r40.h b/include/hw/arm/allwinner-r40.h
index 3be9dc962b..959b5dc4e0 100644
--- a/include/hw/arm/allwinner-r40.h
+++ b/include/hw/arm/allwinner-r40.h
@@ -41,6 +41,13 @@  enum {
     AW_R40_DEV_CCU,
     AW_R40_DEV_PIT,
     AW_R40_DEV_UART0,
+    AW_R40_DEV_UART1,
+    AW_R40_DEV_UART2,
+    AW_R40_DEV_UART3,
+    AW_R40_DEV_UART4,
+    AW_R40_DEV_UART5,
+    AW_R40_DEV_UART6,
+    AW_R40_DEV_UART7,
     AW_R40_DEV_GIC_DIST,
     AW_R40_DEV_GIC_CPU,
     AW_R40_DEV_GIC_HYP,
@@ -70,6 +77,7 @@  OBJECT_DECLARE_SIMPLE_TYPE(AwR40State, AW_R40)
  * which are currently emulated by the R40 SoC code.
  */
 #define AW_R40_NUM_MMCS         4
+#define AW_R40_NUM_UARTS        8
 
 struct AwR40State {
     /*< private >*/