[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 2/2] hw/sparc/leon3: add second uart with extended interru
|
From: |
Clément Chigot |
|
Subject: |
Re: [PATCH v2 2/2] hw/sparc/leon3: add second uart with extended interrupt usage |
|
Date: |
Mon, 23 Sep 2024 13:21:41 +0200 |
On Sat, Sep 21, 2024 at 6:43 PM Nikita Shushura <me@nikitashushura.com> wrote:
>
> Signed-off-by: Nikita Shushura <me@nikitashushura.com>
Mostly OK just a few comments.
> ---
> hw/sparc/leon3.c | 63 +++++++++++++++++++++++++++++++++++-------------
> 1 file changed, 46 insertions(+), 17 deletions(-)
>
> diff --git a/hw/sparc/leon3.c b/hw/sparc/leon3.c
> index 6aaa04cb19..c559854e5e 100644
> --- a/hw/sparc/leon3.c
> +++ b/hw/sparc/leon3.c
> @@ -54,10 +54,14 @@
> #define LEON3_PROM_OFFSET (0x00000000)
> #define LEON3_RAM_OFFSET (0x40000000)
>
> -#define MAX_CPUS 4
> +#define MAX_CPUS (4)
> +#define LEON3_EIRQ (12)
>
> -#define LEON3_UART_OFFSET (0x80000100)
> -#define LEON3_UART_IRQ (3)
> +#define LEON3_UART0_OFFSET (0x80000100)
> +#define LEON3_UART0_IRQ (2)
> +
> +#define LEON3_UART1_OFFSET (0x80100100)
> +#define LEON3_UART1_IRQ (17)
>
> #define LEON3_IRQMP_OFFSET (0x80000200)
>
> @@ -65,7 +69,8 @@
> #define LEON3_TIMER_IRQ (6)
> #define LEON3_TIMER_COUNT (2)
>
> -#define LEON3_APB_PNP_OFFSET (0x800FF000)
> +#define LEON3_APB1_PNP_OFFSET (0x800FF000)
> +#define LEON3_APB2_PNP_OFFSET (0x801FF000)
> #define LEON3_AHB_PNP_OFFSET (0xFFFFF000)
>
> typedef struct ResetData {
> @@ -122,7 +127,8 @@ static void write_bootloader(void *ptr, hwaddr
> kernel_addr)
>
> /* Initialize the UARTs */
> /* *UART_CONTROL = UART_RECEIVE_ENABLE | UART_TRANSMIT_ENABLE; */
> - p = gen_store_u32(p, 0x80000108, 3);
> + p = gen_store_u32(p, LEON3_UART0_OFFSET + 0x8, 3);
> + p = gen_store_u32(p, LEON3_UART1_OFFSET + 0x8, 3);
>
> /* Initialize the TIMER 0 */
> /* *GPTIMER_SCALER_RELOAD = 40 - 1; */
> @@ -271,7 +277,8 @@ static void leon3_generic_hw_init(MachineState *machine)
> DeviceState *dev, *irqmpdev;
> int i;
> AHBPnp *ahb_pnp;
> - APBPnp *apb_pnp;
> + APBPnp *apb1_pnp;
> + APBPnp *apb2_pnp;
>
> reset_info = g_malloc0(sizeof(ResetData));
>
> @@ -298,10 +305,19 @@ static void leon3_generic_hw_init(MachineState *machine)
> GRLIB_LEON3_DEV, GRLIB_AHB_MASTER,
> GRLIB_CPU_AREA);
>
> - apb_pnp = GRLIB_APB_PNP(qdev_new(TYPE_GRLIB_APB_PNP));
> - sysbus_realize_and_unref(SYS_BUS_DEVICE(apb_pnp), &error_fatal);
> - sysbus_mmio_map(SYS_BUS_DEVICE(apb_pnp), 0, LEON3_APB_PNP_OFFSET);
> - grlib_ahb_pnp_add_entry(ahb_pnp, LEON3_APB_PNP_OFFSET, 0xFFF,
> + /* Initialize APB1 */
> + apb1_pnp = GRLIB_APB_PNP(qdev_new(TYPE_GRLIB_APB_PNP));
> + sysbus_realize_and_unref(SYS_BUS_DEVICE(apb1_pnp), &error_fatal);
> + sysbus_mmio_map(SYS_BUS_DEVICE(apb1_pnp), 0, LEON3_APB1_PNP_OFFSET);
> + grlib_ahb_pnp_add_entry(ahb_pnp, LEON3_APB1_PNP_OFFSET, 0xFFF,
> + GRLIB_VENDOR_GAISLER, GRLIB_APBMST_DEV,
> + GRLIB_AHB_SLAVE, GRLIB_AHBMEM_AREA);
> +
> + /* Initialize APB2 */
> + apb2_pnp = GRLIB_APB_PNP(qdev_new(TYPE_GRLIB_APB_PNP));
> + sysbus_realize_and_unref(SYS_BUS_DEVICE(apb2_pnp), &error_fatal);
> + sysbus_mmio_map(SYS_BUS_DEVICE(apb2_pnp), 0, LEON3_APB2_PNP_OFFSET);
> + grlib_ahb_pnp_add_entry(ahb_pnp, LEON3_APB2_PNP_OFFSET, 0xFFF,
> GRLIB_VENDOR_GAISLER, GRLIB_APBMST_DEV,
> GRLIB_AHB_SLAVE, GRLIB_AHBMEM_AREA);
You could embed that within a loop.
> @@ -309,6 +325,8 @@ static void leon3_generic_hw_init(MachineState *machine)
> irqmpdev = qdev_new(TYPE_GRLIB_IRQMP);
> object_property_set_int(OBJECT(irqmpdev), "ncpus", machine->smp.cpus,
> &error_fatal);
> + /*object_property_set_int(OBJECT(irqmpdev), "eirq", LEON3_EIRQ,*/
> + /* &error_fatal);*/
Why is this commented ?
> sysbus_realize_and_unref(SYS_BUS_DEVICE(irqmpdev), &error_fatal);
>
> for (i = 0; i < machine->smp.cpus; i++) {
> @@ -325,7 +343,7 @@ static void leon3_generic_hw_init(MachineState *machine)
> }
>
> sysbus_mmio_map(SYS_BUS_DEVICE(irqmpdev), 0, LEON3_IRQMP_OFFSET);
> - grlib_apb_pnp_add_entry(apb_pnp, LEON3_IRQMP_OFFSET, 0xFFF,
> + grlib_apb_pnp_add_entry(apb1_pnp, LEON3_IRQMP_OFFSET, 0xFFF,
> GRLIB_VENDOR_GAISLER, GRLIB_IRQMP_DEV,
> 2, 0, GRLIB_APBIO_AREA);
>
> @@ -417,20 +435,31 @@ static void leon3_generic_hw_init(MachineState *machine)
> qdev_get_gpio_in(irqmpdev, LEON3_TIMER_IRQ + i));
> }
>
> - grlib_apb_pnp_add_entry(apb_pnp, LEON3_TIMER_OFFSET, 0xFFF,
> + grlib_apb_pnp_add_entry(apb1_pnp, LEON3_TIMER_OFFSET, 0xFFF,
> GRLIB_VENDOR_GAISLER, GRLIB_GPTIMER_DEV,
> 0, LEON3_TIMER_IRQ, GRLIB_APBIO_AREA);
>
> - /* Allocate uart */
> + /* Allocate UART0 */
> dev = qdev_new(TYPE_GRLIB_APB_UART);
> qdev_prop_set_chr(dev, "chrdev", serial_hd(0));
> sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
> - sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, LEON3_UART_OFFSET);
> + sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, LEON3_UART0_OFFSET);
> + sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0,
> + qdev_get_gpio_in(irqmpdev, LEON3_UART0_IRQ));
> + grlib_apb_pnp_add_entry(apb1_pnp, LEON3_UART0_OFFSET, 0xFFF,
> + GRLIB_VENDOR_GAISLER, GRLIB_APBUART_DEV, 1,
> + LEON3_UART0_IRQ, GRLIB_APBIO_AREA);
> +
> + /* Allocate UART1 */
> + dev = qdev_new(TYPE_GRLIB_APB_UART);
> + qdev_prop_set_chr(dev, "chrdev", serial_hd(1));
> + sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
> + sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, LEON3_UART1_OFFSET);
> sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0,
> - qdev_get_gpio_in(irqmpdev, LEON3_UART_IRQ));
> - grlib_apb_pnp_add_entry(apb_pnp, LEON3_UART_OFFSET, 0xFFF,
> + qdev_get_gpio_in(irqmpdev, LEON3_UART1_IRQ));
> + grlib_apb_pnp_add_entry(apb1_pnp, LEON3_UART1_OFFSET, 0xFFF,
Should this be apb2 here ?
Moreover, this is pretty similar to UART0. Thus, this would be better
to include within a loop with just a few adjustments for UART0. This
would also have the benefits to ease adding other UARTs, at some
point.
> GRLIB_VENDOR_GAISLER, GRLIB_APBUART_DEV, 1,
> - LEON3_UART_IRQ, GRLIB_APBIO_AREA);
> + LEON3_UART1_IRQ, GRLIB_APBIO_AREA);
> }
>
> static void leon3_generic_machine_init(MachineClass *mc)
> --
> 2.46.1
>
>