diff mbox series

[v2,2/3] leon3: introduce the plug and play mecanism

Message ID 1556194715-24427-3-git-send-email-frederic.konrad@adacore.com (mailing list archive)
State New, archived
Headers show
Series Leon3 patches | expand

Commit Message

KONRAD Frederic April 25, 2019, 12:18 p.m. UTC
This adds the AHB and APB plug and play devices.
They are scanned during the linux boot to discover the various peripheral.

Reviewed-by: Fabien Chouteau <chouteau@adacore.com>
Signed-off-by: KONRAD Frederic <frederic.konrad@adacore.com>
---
 hw/misc/Makefile.objs               |   2 +
 hw/misc/grlib_ahb_apb_pnp.c         | 269 ++++++++++++++++++++++++++++++++++++
 hw/sparc/leon3.c                    |  34 ++++-
 include/hw/misc/grlib_ahb_apb_pnp.h |  60 ++++++++
 include/hw/sparc/grlib.h            |  35 +++--
 5 files changed, 382 insertions(+), 18 deletions(-)
 create mode 100644 hw/misc/grlib_ahb_apb_pnp.c
 create mode 100644 include/hw/misc/grlib_ahb_apb_pnp.h

Comments

Mark Cave-Ayland May 3, 2019, 8:09 a.m. UTC | #1
On 25/04/2019 13:18, KONRAD Frederic wrote:

> This adds the AHB and APB plug and play devices.
> They are scanned during the linux boot to discover the various peripheral.
> 
> Reviewed-by: Fabien Chouteau <chouteau@adacore.com>
> Signed-off-by: KONRAD Frederic <frederic.konrad@adacore.com>
> ---
>  hw/misc/Makefile.objs               |   2 +
>  hw/misc/grlib_ahb_apb_pnp.c         | 269 ++++++++++++++++++++++++++++++++++++
>  hw/sparc/leon3.c                    |  34 ++++-
>  include/hw/misc/grlib_ahb_apb_pnp.h |  60 ++++++++
>  include/hw/sparc/grlib.h            |  35 +++--
>  5 files changed, 382 insertions(+), 18 deletions(-)
>  create mode 100644 hw/misc/grlib_ahb_apb_pnp.c
>  create mode 100644 include/hw/misc/grlib_ahb_apb_pnp.h
> 
> diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
> index c71e07a..77b9df9 100644
> --- a/hw/misc/Makefile.objs
> +++ b/hw/misc/Makefile.objs
> @@ -77,3 +77,5 @@ obj-$(CONFIG_AUX) += auxbus.o
>  obj-$(CONFIG_ASPEED_SOC) += aspeed_scu.o aspeed_sdmc.o
>  obj-$(CONFIG_MSF2) += msf2-sysreg.o
>  obj-$(CONFIG_NRF51_SOC) += nrf51_rng.o
> +
> +obj-$(CONFIG_GRLIB) += grlib_ahb_apb_pnp.o
> diff --git a/hw/misc/grlib_ahb_apb_pnp.c b/hw/misc/grlib_ahb_apb_pnp.c
> new file mode 100644
> index 0000000..90d5f6e
> --- /dev/null
> +++ b/hw/misc/grlib_ahb_apb_pnp.c
> @@ -0,0 +1,269 @@
> +/*
> + * GRLIB AHB APB PNP
> + *
> + *  Copyright (C) 2019 AdaCore
> + *
> + *  Developed by :
> + *  Frederic Konrad   <frederic.konrad@adacore.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation, either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License along
> + * with this program; if not, see <http://www.gnu.org/licenses/>.
> + *
> + */
> +
> +#include "qemu/osdep.h"
> +#include "hw/sysbus.h"
> +#include "hw/misc/grlib_ahb_apb_pnp.h"
> +
> +#define GRLIB_PNP_VENDOR_SHIFT (24)
> +#define GRLIB_PNP_VENDOR_SIZE   (8)
> +#define GRLIB_PNP_DEV_SHIFT    (12)
> +#define GRLIB_PNP_DEV_SIZE     (12)
> +#define GRLIB_PNP_VER_SHIFT     (5)
> +#define GRLIB_PNP_VER_SIZE      (5)
> +#define GRLIB_PNP_IRQ_SHIFT     (0)
> +#define GRLIB_PNP_IRQ_SIZE      (5)
> +#define GRLIB_PNP_ADDR_SHIFT   (20)
> +#define GRLIB_PNP_ADDR_SIZE    (12)
> +#define GRLIB_PNP_MASK_SHIFT    (4)
> +#define GRLIB_PNP_MASK_SIZE    (12)
> +
> +#define GRLIB_AHB_DEV_ADDR_SHIFT   (20)
> +#define GRLIB_AHB_DEV_ADDR_SIZE    (12)
> +#define GRLIB_AHB_ENTRY_SIZE       (0x20)
> +#define GRLIB_AHB_MAX_DEV          (64)
> +#define GRLIB_AHB_SLAVE_OFFSET     (0x800)
> +
> +#define GRLIB_APB_DEV_ADDR_SHIFT   (8)
> +#define GRLIB_APB_DEV_ADDR_SIZE    (12)
> +#define GRLIB_APB_ENTRY_SIZE       (0x08)
> +#define GRLIB_APB_MAX_DEV          (512)
> +
> +#define GRLIB_PNP_MAX_REGS         (0x1000)
> +
> +typedef struct AHBPnp {
> +    SysBusDevice parent_obj;
> +    MemoryRegion iomem;
> +
> +    uint32_t regs[GRLIB_PNP_MAX_REGS >> 2];
> +    uint8_t master_count;
> +    uint8_t slave_count;
> +} AHBPnp;
> +
> +void grlib_ahb_pnp_add_entry(AHBPnp *dev, uint32_t address, uint32_t mask,
> +                             uint8_t vendor, uint16_t device, int slave,
> +                             int type)
> +{
> +    unsigned int reg_start;
> +
> +    /*
> +     * AHB entries look like this:
> +     *
> +     * 31 -------- 23 -------- 11 ----- 9 -------- 4 --- 0
> +     *  | VENDOR ID | DEVICE ID | IRQ ? | VERSION  | IRQ |
> +     *  --------------------------------------------------
> +     *  |                      USER                      |
> +     *  --------------------------------------------------
> +     *  |                      USER                      |
> +     *  --------------------------------------------------
> +     *  |                      USER                      |
> +     *  --------------------------------------------------
> +     *  |                      USER                      |
> +     *  --------------------------------------------------
> +     * 31 ----------- 20 --- 15 ----------------- 3 ---- 0
> +     *  | ADDR[31..12] | 00PC |        MASK       | TYPE |
> +     *  --------------------------------------------------
> +     * 31 ----------- 20 --- 15 ----------------- 3 ---- 0
> +     *  | ADDR[31..12] | 00PC |        MASK       | TYPE |
> +     *  --------------------------------------------------
> +     * 31 ----------- 20 --- 15 ----------------- 3 ---- 0
> +     *  | ADDR[31..12] | 00PC |        MASK       | TYPE |
> +     *  --------------------------------------------------
> +     * 31 ----------- 20 --- 15 ----------------- 3 ---- 0
> +     *  | ADDR[31..12] | 00PC |        MASK       | TYPE |
> +     *  --------------------------------------------------
> +     */
> +
> +    if (slave) {
> +        assert(dev->slave_count < GRLIB_AHB_MAX_DEV);
> +        reg_start = (GRLIB_AHB_SLAVE_OFFSET
> +                  + (dev->slave_count * GRLIB_AHB_ENTRY_SIZE)) >> 2;
> +        dev->slave_count++;
> +    } else {
> +        assert(dev->master_count < GRLIB_AHB_MAX_DEV);
> +        reg_start = (dev->master_count * GRLIB_AHB_ENTRY_SIZE) >> 2;
> +        dev->master_count++;
> +    }
> +
> +    dev->regs[reg_start] = deposit32(dev->regs[reg_start],
> +                                     GRLIB_PNP_VENDOR_SHIFT,
> +                                     GRLIB_PNP_VENDOR_SIZE,
> +                                     vendor);
> +    dev->regs[reg_start] = deposit32(dev->regs[reg_start],
> +                                     GRLIB_PNP_DEV_SHIFT,
> +                                     GRLIB_PNP_DEV_SIZE,
> +                                     device);
> +    reg_start += 4;
> +    /* AHB Memory Space */
> +    dev->regs[reg_start] = type;
> +    dev->regs[reg_start] = deposit32(dev->regs[reg_start],
> +                                     GRLIB_PNP_ADDR_SHIFT,
> +                                     GRLIB_PNP_ADDR_SIZE,
> +                                     extract32(address,
> +                                               GRLIB_AHB_DEV_ADDR_SHIFT,
> +                                               GRLIB_AHB_DEV_ADDR_SIZE));
> +    dev->regs[reg_start] = deposit32(dev->regs[reg_start],
> +                                     GRLIB_PNP_MASK_SHIFT,
> +                                     GRLIB_PNP_MASK_SIZE,
> +                                     mask);
> +}
> +
> +static uint64_t grlib_ahb_pnp_read(void *opaque, hwaddr offset, unsigned size)
> +{
> +    AHBPnp *ahb_pnp = GRLIB_AHB_PNP(opaque);
> +
> +    return ahb_pnp->regs[offset >> 2];
> +}
> +
> +static const MemoryRegionOps grlib_ahb_pnp_ops = {
> +    .read       = grlib_ahb_pnp_read,
> +    .endianness = DEVICE_NATIVE_ENDIAN,
> +};

Should be DEVICE_BIG_ENDIAN?

> +static void grlib_ahb_pnp_realize(DeviceState *dev, Error **errp)
> +{
> +    AHBPnp *ahb_pnp = GRLIB_AHB_PNP(dev);
> +    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
> +
> +    memory_region_init_io(&ahb_pnp->iomem, OBJECT(dev), &grlib_ahb_pnp_ops,
> +                          ahb_pnp, TYPE_GRLIB_AHB_PNP, GRLIB_PNP_MAX_REGS);
> +    sysbus_init_mmio(sbd, &ahb_pnp->iomem);
> +}
> +
> +static void grlib_ahb_pnp_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +
> +    dc->realize = grlib_ahb_pnp_realize;

Since this requires wiring up in code (i.e. can't be instantiated in the monitor via
device_add) you'll need to mark the device as not user creatable e.g.

    /* Reason: Must be wired up in code (see leon3_generic_hw_init() function) */
    dc->user_creatable = false;

> +}
> +
> +static const TypeInfo grlib_ahb_pnp_info = {
> +    .name          = TYPE_GRLIB_AHB_PNP,
> +    .parent        = TYPE_SYS_BUS_DEVICE,
> +    .instance_size = sizeof(AHBPnp),
> +    .class_init    = grlib_ahb_pnp_class_init,
> +};
> +
> +/* APBPnp */
> +
> +typedef struct APBPnp {
> +    SysBusDevice parent_obj;
> +    MemoryRegion iomem;
> +
> +    uint32_t regs[GRLIB_PNP_MAX_REGS >> 2];
> +    uint32_t entry_count;
> +} APBPnp;
> +
> +void grlib_apb_pnp_add_entry(APBPnp *dev, uint32_t address, uint32_t mask,
> +                             uint8_t vendor, uint16_t device, uint8_t version,
> +                             uint8_t irq, int type)
> +{
> +    unsigned int reg_start;
> +
> +    /*
> +     * APB entries look like this:
> +     *
> +     * 31 -------- 23 -------- 11 ----- 9 ------- 4 --- 0
> +     *  | VENDOR ID | DEVICE ID | IRQ ? | VERSION | IRQ |
> +     *
> +     * 31 ---------- 20 --- 15 ----------------- 3 ---- 0
> +     *  | ADDR[20..8] | 0000 |        MASK       | TYPE |
> +     */
> +
> +    assert(dev->entry_count < GRLIB_APB_MAX_DEV);
> +    reg_start = (dev->entry_count * GRLIB_APB_ENTRY_SIZE) >> 2;
> +    dev->entry_count++;
> +
> +    dev->regs[reg_start] = deposit32(dev->regs[reg_start],
> +                                     GRLIB_PNP_VENDOR_SHIFT,
> +                                     GRLIB_PNP_VENDOR_SIZE,
> +                                     vendor);
> +    dev->regs[reg_start] = deposit32(dev->regs[reg_start],
> +                                     GRLIB_PNP_DEV_SHIFT,
> +                                     GRLIB_PNP_DEV_SIZE,
> +                                     device);
> +    dev->regs[reg_start] = deposit32(dev->regs[reg_start],
> +                                     GRLIB_PNP_VER_SHIFT,
> +                                     GRLIB_PNP_VER_SIZE,
> +                                     version);
> +    dev->regs[reg_start] = deposit32(dev->regs[reg_start],
> +                                     GRLIB_PNP_IRQ_SHIFT,
> +                                     GRLIB_PNP_IRQ_SIZE,
> +                                     irq);
> +    reg_start += 1;
> +    dev->regs[reg_start] = type;
> +    dev->regs[reg_start] = deposit32(dev->regs[reg_start],
> +                                     GRLIB_PNP_ADDR_SHIFT,
> +                                     GRLIB_PNP_ADDR_SIZE,
> +                                     extract32(address,
> +                                               GRLIB_APB_DEV_ADDR_SHIFT,
> +                                               GRLIB_APB_DEV_ADDR_SIZE));
> +    dev->regs[reg_start] = deposit32(dev->regs[reg_start],
> +                                     GRLIB_PNP_MASK_SHIFT,
> +                                     GRLIB_PNP_MASK_SIZE,
> +                                     mask);
> +}
> +
> +static uint64_t grlib_apb_pnp_read(void *opaque, hwaddr offset, unsigned size)
> +{
> +    APBPnp *apb_pnp = GRLIB_APB_PNP(opaque);
> +
> +    return apb_pnp->regs[offset >> 2];
> +}
> +
> +static const MemoryRegionOps grlib_apb_pnp_ops = {
> +    .read       = grlib_apb_pnp_read,
> +    .endianness = DEVICE_NATIVE_ENDIAN,
> +};

and also here DEVICE_BIG_ENDIAN?

> +static void grlib_apb_pnp_realize(DeviceState *dev, Error **errp)
> +{
> +    APBPnp *apb_pnp = GRLIB_APB_PNP(dev);
> +    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
> +
> +    memory_region_init_io(&apb_pnp->iomem, OBJECT(dev), &grlib_apb_pnp_ops,
> +                          apb_pnp, TYPE_GRLIB_APB_PNP, GRLIB_PNP_MAX_REGS);
> +    sysbus_init_mmio(sbd, &apb_pnp->iomem);
> +}
> +
> +static void grlib_apb_pnp_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +
> +    dc->realize = grlib_apb_pnp_realize;

Same again here: this device should be marked as not user creatable.

> +}
> +
> +static const TypeInfo grlib_apb_pnp_info = {
> +    .name          = TYPE_GRLIB_APB_PNP,
> +    .parent        = TYPE_SYS_BUS_DEVICE,
> +    .instance_size = sizeof(APBPnp),
> +    .class_init    = grlib_apb_pnp_class_init,
> +};
> +
> +static void grlib_ahb_apb_pnp_register_types(void)
> +{
> +    type_register_static(&grlib_ahb_pnp_info);
> +    type_register_static(&grlib_apb_pnp_info);
> +}
> +
> +type_init(grlib_ahb_apb_pnp_register_types)
> diff --git a/hw/sparc/leon3.c b/hw/sparc/leon3.c
> index f25432c..03ff163 100644
> --- a/hw/sparc/leon3.c
> +++ b/hw/sparc/leon3.c
> @@ -46,6 +46,13 @@
>  #define PROM_FILENAME        "u-boot.bin"
>  #define LEON3_PROM_OFFSET    (0x00000000)
>  #define LEON3_RAM_OFFSET     (0x40000000)
> +#define LEON3_APBUART_OFFSET (0x80000100)
> +#define LEON3_APBUART_IRQ    (0x3)
> +#define LEON3_IRQMP_OFFSET   (0x80000200)
> +#define LEON3_GPTIMER_OFFSET (0x80000300)
> +#define LEON3_GPTIMER_IRQ    (0x6)
> +#define LEON3_APB_PNP_OFFSET (0x800FF000)
> +#define LEON3_AHB_PNP_OFFSET (0xFFFFF000)
>  
>  #define MAX_PILS 16
>  
> @@ -176,6 +183,8 @@ static void leon3_generic_hw_init(MachineState *machine)
>      int         bios_size;
>      int         prom_size;
>      ResetData  *reset_info;
> +    AHBPnp *ahb_pnp;
> +    APBPnp *apb_pnp;
>  
>      /* Init CPU */
>      cpu = SPARC_CPU(cpu_create(machine->cpu_type));
> @@ -189,9 +198,23 @@ static void leon3_generic_hw_init(MachineState *machine)
>      reset_info->sp    = LEON3_RAM_OFFSET + ram_size;
>      qemu_register_reset(main_cpu_reset, reset_info);
>  
> +    ahb_pnp = GRLIB_AHB_PNP(object_new(TYPE_GRLIB_AHB_PNP));
> +    object_property_set_bool(OBJECT(ahb_pnp), true, "realized", &error_fatal);
> +    sysbus_mmio_map(SYS_BUS_DEVICE(ahb_pnp), 0, LEON3_AHB_PNP_OFFSET);
> +    grlib_ahb_pnp_add_entry(ahb_pnp, 0, 0, GRLIB_VENDOR_GAISLER,
> +                            GRLIB_LEON3_DEV, GRLIB_AHB_MASTER,
> +                            GRLIB_CPU_AREA);
> +
> +    apb_pnp = GRLIB_APB_PNP(object_new(TYPE_GRLIB_APB_PNP));
> +    object_property_set_bool(OBJECT(apb_pnp), true, "realized", &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,
> +                            GRLIB_VENDOR_GAISLER, GRLIB_APBMST_DEV,
> +                            GRLIB_AHB_SLAVE, GRLIB_AHBMEM_AREA);
> +
>      /* Allocate IRQ manager */
> -    grlib_irqmp_create(0x80000200, env, &cpu_irqs, MAX_PILS,
> -                       &leon3_set_pil_in);
> +    grlib_irqmp_create(LEON3_IRQMP_OFFSET, env, &cpu_irqs, MAX_PILS,
> +                       &leon3_set_pil_in, apb_pnp);

Whilst we're here, is there any way we can get rid of these old-style create
functions? The general philosophy today is that rather than have multiple per-board
init functions, expose the required properties via qdev and then wire up the device
directly within the board itself.

A quick glance at the code suggests you can just move the contents of the relevant
functions from the header file directly inline with a few minor tweaks.

>      env->qemu_irq_ack = leon3_irq_manager;
>  
> @@ -270,11 +293,14 @@ static void leon3_generic_hw_init(MachineState *machine)
>      }
>  
>      /* Allocate timers */
> -    grlib_gptimer_create(0x80000300, 2, CPU_CLK, cpu_irqs, 6);
> +    grlib_gptimer_create(LEON3_GPTIMER_OFFSET, 2, CPU_CLK, cpu_irqs,
> +                         LEON3_GPTIMER_IRQ,
> +                         apb_pnp);

And here.

>      /* Allocate uart */
>      if (serial_hd(0)) {
> -        grlib_apbuart_create(0x80000100, serial_hd(0), cpu_irqs[3]);
> +        grlib_apbuart_create(LEON3_APBUART_OFFSET, serial_hd(0), cpu_irqs,
> +                             LEON3_APBUART_IRQ, apb_pnp);

And here too.

>      }
>  }
>  
> diff --git a/include/hw/misc/grlib_ahb_apb_pnp.h b/include/hw/misc/grlib_ahb_apb_pnp.h
> new file mode 100644
> index 0000000..a0f6dcf
> --- /dev/null
> +++ b/include/hw/misc/grlib_ahb_apb_pnp.h
> @@ -0,0 +1,60 @@
> +/*
> + * GRLIB AHB APB PNP
> + *
> + *  Copyright (C) 2019 AdaCore
> + *
> + *  Developed by :
> + *  Frederic Konrad   <frederic.konrad@adacore.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation, either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License along
> + * with this program; if not, see <http://www.gnu.org/licenses/>.
> + *
> + */
> +
> +#ifndef GRLIB_AHB_APB_PNP_H
> +#define GRLIB_AHB_APB_PNP_H
> +
> +#define TYPE_GRLIB_AHB_PNP "grlib,ahbpnp"
> +#define GRLIB_AHB_PNP(obj) \
> +    OBJECT_CHECK(AHBPnp, (obj), TYPE_GRLIB_AHB_PNP)
> +typedef struct AHBPnp AHBPnp;
> +
> +#define TYPE_GRLIB_APB_PNP "grlib,apbpnp"
> +#define GRLIB_APB_PNP(obj) \
> +    OBJECT_CHECK(APBPnp, (obj), TYPE_GRLIB_APB_PNP)
> +typedef struct APBPnp APBPnp;
> +
> +void grlib_ahb_pnp_add_entry(AHBPnp *dev, uint32_t address, uint32_t mask,
> +                             uint8_t vendor, uint16_t device, int slave,
> +                             int type);
> +void grlib_apb_pnp_add_entry(APBPnp *dev, uint32_t address, uint32_t mask,
> +                             uint8_t vendor, uint16_t device, uint8_t version,
> +                             uint8_t irq, int type);
> +
> +/* VENDORS */
> +#define GRLIB_VENDOR_GAISLER (0x01)
> +/* DEVICES */
> +#define GRLIB_LEON3_DEV      (0x03)
> +#define GRLIB_APBMST_DEV     (0x06)
> +#define GRLIB_APBUART_DEV    (0x0C)
> +#define GRLIB_IRQMP_DEV      (0x0D)
> +#define GRLIB_GPTIMER_DEV    (0x11)
> +/* TYPE */
> +#define GRLIB_CPU_AREA       (0x00)
> +#define GRLIB_APBIO_AREA     (0x01)
> +#define GRLIB_AHBMEM_AREA    (0x02)
> +
> +#define GRLIB_AHB_MASTER     (0x00)
> +#define GRLIB_AHB_SLAVE      (0x01)
> +
> +#endif /* GRLIB_AHB_APB_PNP_H */
> diff --git a/include/hw/sparc/grlib.h b/include/hw/sparc/grlib.h
> index 61a345c..28320ab 100644
> --- a/include/hw/sparc/grlib.h
> +++ b/include/hw/sparc/grlib.h
> @@ -27,6 +27,7 @@
>  
>  #include "hw/qdev.h"
>  #include "hw/sysbus.h"
> +#include "hw/misc/grlib_ahb_apb_pnp.h"
>  
>  /* Emulation of GrLib device is base on the GRLIB IP Core User's Manual:
>   * http://www.gaisler.com/products/grlib/grip.pdf
> @@ -41,11 +42,9 @@ void grlib_irqmp_set_irq(void *opaque, int irq, int level);
>  void grlib_irqmp_ack(DeviceState *dev, int intno);
>  
>  static inline
> -DeviceState *grlib_irqmp_create(hwaddr   base,
> -                                CPUSPARCState            *env,
> -                                qemu_irq           **cpu_irqs,
> -                                uint32_t             nr_irqs,
> -                                set_pil_in_fn        set_pil_in)
> +DeviceState *grlib_irqmp_create(hwaddr base, CPUSPARCState *env,
> +                                qemu_irq **cpu_irqs, uint32_t nr_irqs,
> +                                set_pil_in_fn set_pil_in, APBPnp *apb_pnp)
>  {
>      DeviceState *dev;
>  
> @@ -65,17 +64,18 @@ DeviceState *grlib_irqmp_create(hwaddr   base,
>                                     dev,
>                                     nr_irqs);
>  
> +    /* Register this device in the APB PNP device */
> +    grlib_apb_pnp_add_entry(apb_pnp, base, 0xFFF, GRLIB_VENDOR_GAISLER,
> +                            GRLIB_IRQMP_DEV, 2, 0, GRLIB_APBIO_AREA);
>      return dev;
>  }
>  
>  /* GPTimer */
>  
>  static inline
> -DeviceState *grlib_gptimer_create(hwaddr  base,
> -                                  uint32_t            nr_timers,
> -                                  uint32_t            freq,
> -                                  qemu_irq           *cpu_irqs,
> -                                  int                 base_irq)
> +DeviceState *grlib_gptimer_create(hwaddr base, uint32_t nr_timers,
> +                                  uint32_t freq, qemu_irq *cpu_irqs,
> +                                  int base_irq, APBPnp *apb_pnp)
>  {
>      DeviceState *dev;
>      int i;
> @@ -93,15 +93,18 @@ DeviceState *grlib_gptimer_create(hwaddr  base,
>          sysbus_connect_irq(SYS_BUS_DEVICE(dev), i, cpu_irqs[base_irq + i]);
>      }
>  
> +    /* Register this device in the APB PNP device */
> +    grlib_apb_pnp_add_entry(apb_pnp, base, 0xFFF, GRLIB_VENDOR_GAISLER,
> +                            GRLIB_GPTIMER_DEV, 0, base_irq, GRLIB_APBIO_AREA);
>      return dev;
>  }
>  
>  /* APB UART */
>  
>  static inline
> -DeviceState *grlib_apbuart_create(hwaddr  base,
> -                                  Chardev    *serial,
> -                                  qemu_irq            irq)
> +DeviceState *grlib_apbuart_create(hwaddr base, Chardev *serial,
> +                                  qemu_irq *cpu_irqs, int base_irq,
> +                                  APBPnp *apb_pnp)
>  {
>      DeviceState *dev;
>  
> @@ -112,7 +115,11 @@ DeviceState *grlib_apbuart_create(hwaddr  base,
>  
>      sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base);
>  
> -    sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, irq);
> +    sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, cpu_irqs[base_irq]);
> +
> +    /* Register this device in the APB PNP device */
> +    grlib_apb_pnp_add_entry(apb_pnp, base, 0xFFF, GRLIB_VENDOR_GAISLER,
> +                            GRLIB_APBUART_DEV, 1, base_irq, GRLIB_APBIO_AREA);
>  
>      return dev;
>  }
> 

Other than that, checkpatch gave a warning to check MAINTAINERS and it looks like
you're missing an entry for the new hw/misc/grlib_ahb_apb_pnp.h header file.

Finally there's a small typo in the commit message: mecanism -> mechanism.


ATB,

Mark.
KONRAD Frederic May 3, 2019, 8:24 a.m. UTC | #2
Le 5/3/19 à 10:09 AM, Mark Cave-Ayland a écrit :
> On 25/04/2019 13:18, KONRAD Frederic wrote:
> 
>> This adds the AHB and APB plug and play devices.
>> They are scanned during the linux boot to discover the various peripheral.
>>
>> Reviewed-by: Fabien Chouteau <chouteau@adacore.com>
>> Signed-off-by: KONRAD Frederic <frederic.konrad@adacore.com>
>> ---
>>   hw/misc/Makefile.objs               |   2 +
>>   hw/misc/grlib_ahb_apb_pnp.c         | 269 ++++++++++++++++++++++++++++++++++++
>>   hw/sparc/leon3.c                    |  34 ++++-
>>   include/hw/misc/grlib_ahb_apb_pnp.h |  60 ++++++++
>>   include/hw/sparc/grlib.h            |  35 +++--
>>   5 files changed, 382 insertions(+), 18 deletions(-)
>>   create mode 100644 hw/misc/grlib_ahb_apb_pnp.c
>>   create mode 100644 include/hw/misc/grlib_ahb_apb_pnp.h
>>
>> diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
>> index c71e07a..77b9df9 100644
>> --- a/hw/misc/Makefile.objs
>> +++ b/hw/misc/Makefile.objs
>> @@ -77,3 +77,5 @@ obj-$(CONFIG_AUX) += auxbus.o
>>   obj-$(CONFIG_ASPEED_SOC) += aspeed_scu.o aspeed_sdmc.o
>>   obj-$(CONFIG_MSF2) += msf2-sysreg.o
>>   obj-$(CONFIG_NRF51_SOC) += nrf51_rng.o
>> +
>> +obj-$(CONFIG_GRLIB) += grlib_ahb_apb_pnp.o
>> diff --git a/hw/misc/grlib_ahb_apb_pnp.c b/hw/misc/grlib_ahb_apb_pnp.c
>> new file mode 100644
>> index 0000000..90d5f6e
>> --- /dev/null
>> +++ b/hw/misc/grlib_ahb_apb_pnp.c
>> @@ -0,0 +1,269 @@
>> +/*
>> + * GRLIB AHB APB PNP
>> + *
>> + *  Copyright (C) 2019 AdaCore
>> + *
>> + *  Developed by :
>> + *  Frederic Konrad   <frederic.konrad@adacore.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation, either version 2 of the License, or
>> + * (at your option) any later version.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + * GNU General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public License along
>> + * with this program; if not, see <http://www.gnu.org/licenses/>.
>> + *
>> + */
>> +
>> +#include "qemu/osdep.h"
>> +#include "hw/sysbus.h"
>> +#include "hw/misc/grlib_ahb_apb_pnp.h"
>> +
>> +#define GRLIB_PNP_VENDOR_SHIFT (24)
>> +#define GRLIB_PNP_VENDOR_SIZE   (8)
>> +#define GRLIB_PNP_DEV_SHIFT    (12)
>> +#define GRLIB_PNP_DEV_SIZE     (12)
>> +#define GRLIB_PNP_VER_SHIFT     (5)
>> +#define GRLIB_PNP_VER_SIZE      (5)
>> +#define GRLIB_PNP_IRQ_SHIFT     (0)
>> +#define GRLIB_PNP_IRQ_SIZE      (5)
>> +#define GRLIB_PNP_ADDR_SHIFT   (20)
>> +#define GRLIB_PNP_ADDR_SIZE    (12)
>> +#define GRLIB_PNP_MASK_SHIFT    (4)
>> +#define GRLIB_PNP_MASK_SIZE    (12)
>> +
>> +#define GRLIB_AHB_DEV_ADDR_SHIFT   (20)
>> +#define GRLIB_AHB_DEV_ADDR_SIZE    (12)
>> +#define GRLIB_AHB_ENTRY_SIZE       (0x20)
>> +#define GRLIB_AHB_MAX_DEV          (64)
>> +#define GRLIB_AHB_SLAVE_OFFSET     (0x800)
>> +
>> +#define GRLIB_APB_DEV_ADDR_SHIFT   (8)
>> +#define GRLIB_APB_DEV_ADDR_SIZE    (12)
>> +#define GRLIB_APB_ENTRY_SIZE       (0x08)
>> +#define GRLIB_APB_MAX_DEV          (512)
>> +
>> +#define GRLIB_PNP_MAX_REGS         (0x1000)
>> +
>> +typedef struct AHBPnp {
>> +    SysBusDevice parent_obj;
>> +    MemoryRegion iomem;
>> +
>> +    uint32_t regs[GRLIB_PNP_MAX_REGS >> 2];
>> +    uint8_t master_count;
>> +    uint8_t slave_count;
>> +} AHBPnp;
>> +
>> +void grlib_ahb_pnp_add_entry(AHBPnp *dev, uint32_t address, uint32_t mask,
>> +                             uint8_t vendor, uint16_t device, int slave,
>> +                             int type)
>> +{
>> +    unsigned int reg_start;
>> +
>> +    /*
>> +     * AHB entries look like this:
>> +     *
>> +     * 31 -------- 23 -------- 11 ----- 9 -------- 4 --- 0
>> +     *  | VENDOR ID | DEVICE ID | IRQ ? | VERSION  | IRQ |
>> +     *  --------------------------------------------------
>> +     *  |                      USER                      |
>> +     *  --------------------------------------------------
>> +     *  |                      USER                      |
>> +     *  --------------------------------------------------
>> +     *  |                      USER                      |
>> +     *  --------------------------------------------------
>> +     *  |                      USER                      |
>> +     *  --------------------------------------------------
>> +     * 31 ----------- 20 --- 15 ----------------- 3 ---- 0
>> +     *  | ADDR[31..12] | 00PC |        MASK       | TYPE |
>> +     *  --------------------------------------------------
>> +     * 31 ----------- 20 --- 15 ----------------- 3 ---- 0
>> +     *  | ADDR[31..12] | 00PC |        MASK       | TYPE |
>> +     *  --------------------------------------------------
>> +     * 31 ----------- 20 --- 15 ----------------- 3 ---- 0
>> +     *  | ADDR[31..12] | 00PC |        MASK       | TYPE |
>> +     *  --------------------------------------------------
>> +     * 31 ----------- 20 --- 15 ----------------- 3 ---- 0
>> +     *  | ADDR[31..12] | 00PC |        MASK       | TYPE |
>> +     *  --------------------------------------------------
>> +     */
>> +
>> +    if (slave) {
>> +        assert(dev->slave_count < GRLIB_AHB_MAX_DEV);
>> +        reg_start = (GRLIB_AHB_SLAVE_OFFSET
>> +                  + (dev->slave_count * GRLIB_AHB_ENTRY_SIZE)) >> 2;
>> +        dev->slave_count++;
>> +    } else {
>> +        assert(dev->master_count < GRLIB_AHB_MAX_DEV);
>> +        reg_start = (dev->master_count * GRLIB_AHB_ENTRY_SIZE) >> 2;
>> +        dev->master_count++;
>> +    }
>> +
>> +    dev->regs[reg_start] = deposit32(dev->regs[reg_start],
>> +                                     GRLIB_PNP_VENDOR_SHIFT,
>> +                                     GRLIB_PNP_VENDOR_SIZE,
>> +                                     vendor);
>> +    dev->regs[reg_start] = deposit32(dev->regs[reg_start],
>> +                                     GRLIB_PNP_DEV_SHIFT,
>> +                                     GRLIB_PNP_DEV_SIZE,
>> +                                     device);
>> +    reg_start += 4;
>> +    /* AHB Memory Space */
>> +    dev->regs[reg_start] = type;
>> +    dev->regs[reg_start] = deposit32(dev->regs[reg_start],
>> +                                     GRLIB_PNP_ADDR_SHIFT,
>> +                                     GRLIB_PNP_ADDR_SIZE,
>> +                                     extract32(address,
>> +                                               GRLIB_AHB_DEV_ADDR_SHIFT,
>> +                                               GRLIB_AHB_DEV_ADDR_SIZE));
>> +    dev->regs[reg_start] = deposit32(dev->regs[reg_start],
>> +                                     GRLIB_PNP_MASK_SHIFT,
>> +                                     GRLIB_PNP_MASK_SIZE,
>> +                                     mask);
>> +}
>> +
>> +static uint64_t grlib_ahb_pnp_read(void *opaque, hwaddr offset, unsigned size)
>> +{
>> +    AHBPnp *ahb_pnp = GRLIB_AHB_PNP(opaque);
>> +
>> +    return ahb_pnp->regs[offset >> 2];
>> +}
>> +
>> +static const MemoryRegionOps grlib_ahb_pnp_ops = {
>> +    .read       = grlib_ahb_pnp_read,
>> +    .endianness = DEVICE_NATIVE_ENDIAN,
>> +};
> 
> Should be DEVICE_BIG_ENDIAN?
> 
>> +static void grlib_ahb_pnp_realize(DeviceState *dev, Error **errp)
>> +{
>> +    AHBPnp *ahb_pnp = GRLIB_AHB_PNP(dev);
>> +    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
>> +
>> +    memory_region_init_io(&ahb_pnp->iomem, OBJECT(dev), &grlib_ahb_pnp_ops,
>> +                          ahb_pnp, TYPE_GRLIB_AHB_PNP, GRLIB_PNP_MAX_REGS);
>> +    sysbus_init_mmio(sbd, &ahb_pnp->iomem);
>> +}
>> +
>> +static void grlib_ahb_pnp_class_init(ObjectClass *klass, void *data)
>> +{
>> +    DeviceClass *dc = DEVICE_CLASS(klass);
>> +
>> +    dc->realize = grlib_ahb_pnp_realize;
> 
> Since this requires wiring up in code (i.e. can't be instantiated in the monitor via
> device_add) you'll need to mark the device as not user creatable e.g.
> 
>      /* Reason: Must be wired up in code (see leon3_generic_hw_init() function) */
>      dc->user_creatable = false;
>

I think that's actually the default for SYS_BUS_DEVICE:

static void sysbus_device_class_init(ObjectClass *klass, void *data)
{
     DeviceClass *k = DEVICE_CLASS(klass);
     k->realize = sysbus_realize;
     k->bus_type = TYPE_SYSTEM_BUS;
     /*
      * device_add plugs devices into a suitable bus.  For "real" buses,
      * that actually connects the device.  For sysbus, the connections
      * need to be made separately, and device_add can't do that.  The
      * device would be left unconnected, and will probably not work
      *
      * However, a few machines can handle device_add/-device with
      * a few specific sysbus devices. In those cases, the device
      * subclass needs to override it and set user_creatable=true.
      */
     k->user_creatable = false;
}

But I can change that for clarity if you want.

>> +}
>> +
>> +static const TypeInfo grlib_ahb_pnp_info = {
>> +    .name          = TYPE_GRLIB_AHB_PNP,
>> +    .parent        = TYPE_SYS_BUS_DEVICE,
>> +    .instance_size = sizeof(AHBPnp),
>> +    .class_init    = grlib_ahb_pnp_class_init,
>> +};
>> +
>> +/* APBPnp */
>> +
>> +typedef struct APBPnp {
>> +    SysBusDevice parent_obj;
>> +    MemoryRegion iomem;
>> +
>> +    uint32_t regs[GRLIB_PNP_MAX_REGS >> 2];
>> +    uint32_t entry_count;
>> +} APBPnp;
>> +
>> +void grlib_apb_pnp_add_entry(APBPnp *dev, uint32_t address, uint32_t mask,
>> +                             uint8_t vendor, uint16_t device, uint8_t version,
>> +                             uint8_t irq, int type)
>> +{
>> +    unsigned int reg_start;
>> +
>> +    /*
>> +     * APB entries look like this:
>> +     *
>> +     * 31 -------- 23 -------- 11 ----- 9 ------- 4 --- 0
>> +     *  | VENDOR ID | DEVICE ID | IRQ ? | VERSION | IRQ |
>> +     *
>> +     * 31 ---------- 20 --- 15 ----------------- 3 ---- 0
>> +     *  | ADDR[20..8] | 0000 |        MASK       | TYPE |
>> +     */
>> +
>> +    assert(dev->entry_count < GRLIB_APB_MAX_DEV);
>> +    reg_start = (dev->entry_count * GRLIB_APB_ENTRY_SIZE) >> 2;
>> +    dev->entry_count++;
>> +
>> +    dev->regs[reg_start] = deposit32(dev->regs[reg_start],
>> +                                     GRLIB_PNP_VENDOR_SHIFT,
>> +                                     GRLIB_PNP_VENDOR_SIZE,
>> +                                     vendor);
>> +    dev->regs[reg_start] = deposit32(dev->regs[reg_start],
>> +                                     GRLIB_PNP_DEV_SHIFT,
>> +                                     GRLIB_PNP_DEV_SIZE,
>> +                                     device);
>> +    dev->regs[reg_start] = deposit32(dev->regs[reg_start],
>> +                                     GRLIB_PNP_VER_SHIFT,
>> +                                     GRLIB_PNP_VER_SIZE,
>> +                                     version);
>> +    dev->regs[reg_start] = deposit32(dev->regs[reg_start],
>> +                                     GRLIB_PNP_IRQ_SHIFT,
>> +                                     GRLIB_PNP_IRQ_SIZE,
>> +                                     irq);
>> +    reg_start += 1;
>> +    dev->regs[reg_start] = type;
>> +    dev->regs[reg_start] = deposit32(dev->regs[reg_start],
>> +                                     GRLIB_PNP_ADDR_SHIFT,
>> +                                     GRLIB_PNP_ADDR_SIZE,
>> +                                     extract32(address,
>> +                                               GRLIB_APB_DEV_ADDR_SHIFT,
>> +                                               GRLIB_APB_DEV_ADDR_SIZE));
>> +    dev->regs[reg_start] = deposit32(dev->regs[reg_start],
>> +                                     GRLIB_PNP_MASK_SHIFT,
>> +                                     GRLIB_PNP_MASK_SIZE,
>> +                                     mask);
>> +}
>> +
>> +static uint64_t grlib_apb_pnp_read(void *opaque, hwaddr offset, unsigned size)
>> +{
>> +    APBPnp *apb_pnp = GRLIB_APB_PNP(opaque);
>> +
>> +    return apb_pnp->regs[offset >> 2];
>> +}
>> +
>> +static const MemoryRegionOps grlib_apb_pnp_ops = {
>> +    .read       = grlib_apb_pnp_read,
>> +    .endianness = DEVICE_NATIVE_ENDIAN,
>> +};
> 
> and also here DEVICE_BIG_ENDIAN?
> 
>> +static void grlib_apb_pnp_realize(DeviceState *dev, Error **errp)
>> +{
>> +    APBPnp *apb_pnp = GRLIB_APB_PNP(dev);
>> +    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
>> +
>> +    memory_region_init_io(&apb_pnp->iomem, OBJECT(dev), &grlib_apb_pnp_ops,
>> +                          apb_pnp, TYPE_GRLIB_APB_PNP, GRLIB_PNP_MAX_REGS);
>> +    sysbus_init_mmio(sbd, &apb_pnp->iomem);
>> +}
>> +
>> +static void grlib_apb_pnp_class_init(ObjectClass *klass, void *data)
>> +{
>> +    DeviceClass *dc = DEVICE_CLASS(klass);
>> +
>> +    dc->realize = grlib_apb_pnp_realize;
> 
> Same again here: this device should be marked as not user creatable.
> 
>> +}
>> +
>> +static const TypeInfo grlib_apb_pnp_info = {
>> +    .name          = TYPE_GRLIB_APB_PNP,
>> +    .parent        = TYPE_SYS_BUS_DEVICE,
>> +    .instance_size = sizeof(APBPnp),
>> +    .class_init    = grlib_apb_pnp_class_init,
>> +};
>> +
>> +static void grlib_ahb_apb_pnp_register_types(void)
>> +{
>> +    type_register_static(&grlib_ahb_pnp_info);
>> +    type_register_static(&grlib_apb_pnp_info);
>> +}
>> +
>> +type_init(grlib_ahb_apb_pnp_register_types)
>> diff --git a/hw/sparc/leon3.c b/hw/sparc/leon3.c
>> index f25432c..03ff163 100644
>> --- a/hw/sparc/leon3.c
>> +++ b/hw/sparc/leon3.c
>> @@ -46,6 +46,13 @@
>>   #define PROM_FILENAME        "u-boot.bin"
>>   #define LEON3_PROM_OFFSET    (0x00000000)
>>   #define LEON3_RAM_OFFSET     (0x40000000)
>> +#define LEON3_APBUART_OFFSET (0x80000100)
>> +#define LEON3_APBUART_IRQ    (0x3)
>> +#define LEON3_IRQMP_OFFSET   (0x80000200)
>> +#define LEON3_GPTIMER_OFFSET (0x80000300)
>> +#define LEON3_GPTIMER_IRQ    (0x6)
>> +#define LEON3_APB_PNP_OFFSET (0x800FF000)
>> +#define LEON3_AHB_PNP_OFFSET (0xFFFFF000)
>>   
>>   #define MAX_PILS 16
>>   
>> @@ -176,6 +183,8 @@ static void leon3_generic_hw_init(MachineState *machine)
>>       int         bios_size;
>>       int         prom_size;
>>       ResetData  *reset_info;
>> +    AHBPnp *ahb_pnp;
>> +    APBPnp *apb_pnp;
>>   
>>       /* Init CPU */
>>       cpu = SPARC_CPU(cpu_create(machine->cpu_type));
>> @@ -189,9 +198,23 @@ static void leon3_generic_hw_init(MachineState *machine)
>>       reset_info->sp    = LEON3_RAM_OFFSET + ram_size;
>>       qemu_register_reset(main_cpu_reset, reset_info);
>>   
>> +    ahb_pnp = GRLIB_AHB_PNP(object_new(TYPE_GRLIB_AHB_PNP));
>> +    object_property_set_bool(OBJECT(ahb_pnp), true, "realized", &error_fatal);
>> +    sysbus_mmio_map(SYS_BUS_DEVICE(ahb_pnp), 0, LEON3_AHB_PNP_OFFSET);
>> +    grlib_ahb_pnp_add_entry(ahb_pnp, 0, 0, GRLIB_VENDOR_GAISLER,
>> +                            GRLIB_LEON3_DEV, GRLIB_AHB_MASTER,
>> +                            GRLIB_CPU_AREA);
>> +
>> +    apb_pnp = GRLIB_APB_PNP(object_new(TYPE_GRLIB_APB_PNP));
>> +    object_property_set_bool(OBJECT(apb_pnp), true, "realized", &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,
>> +                            GRLIB_VENDOR_GAISLER, GRLIB_APBMST_DEV,
>> +                            GRLIB_AHB_SLAVE, GRLIB_AHBMEM_AREA);
>> +
>>       /* Allocate IRQ manager */
>> -    grlib_irqmp_create(0x80000200, env, &cpu_irqs, MAX_PILS,
>> -                       &leon3_set_pil_in);
>> +    grlib_irqmp_create(LEON3_IRQMP_OFFSET, env, &cpu_irqs, MAX_PILS,
>> +                       &leon3_set_pil_in, apb_pnp);
> 
> Whilst we're here, is there any way we can get rid of these old-style create
> functions? The general philosophy today is that rather than have multiple per-board
> init functions, expose the required properties via qdev and then wire up the device
> directly within the board itself.
> 
> A quick glance at the code suggests you can just move the contents of the relevant
> functions from the header file directly inline with a few minor tweaks.

Ok I'll give it a try.

> 
>>       env->qemu_irq_ack = leon3_irq_manager;
>>   
>> @@ -270,11 +293,14 @@ static void leon3_generic_hw_init(MachineState *machine)
>>       }
>>   
>>       /* Allocate timers */
>> -    grlib_gptimer_create(0x80000300, 2, CPU_CLK, cpu_irqs, 6);
>> +    grlib_gptimer_create(LEON3_GPTIMER_OFFSET, 2, CPU_CLK, cpu_irqs,
>> +                         LEON3_GPTIMER_IRQ,
>> +                         apb_pnp);
> 
> And here.
> 
>>       /* Allocate uart */
>>       if (serial_hd(0)) {
>> -        grlib_apbuart_create(0x80000100, serial_hd(0), cpu_irqs[3]);
>> +        grlib_apbuart_create(LEON3_APBUART_OFFSET, serial_hd(0), cpu_irqs,
>> +                             LEON3_APBUART_IRQ, apb_pnp);
> 
> And here too.
> 
>>       }
>>   }
>>   
>> diff --git a/include/hw/misc/grlib_ahb_apb_pnp.h b/include/hw/misc/grlib_ahb_apb_pnp.h
>> new file mode 100644
>> index 0000000..a0f6dcf
>> --- /dev/null
>> +++ b/include/hw/misc/grlib_ahb_apb_pnp.h
>> @@ -0,0 +1,60 @@
>> +/*
>> + * GRLIB AHB APB PNP
>> + *
>> + *  Copyright (C) 2019 AdaCore
>> + *
>> + *  Developed by :
>> + *  Frederic Konrad   <frederic.konrad@adacore.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation, either version 2 of the License, or
>> + * (at your option) any later version.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + * GNU General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public License along
>> + * with this program; if not, see <http://www.gnu.org/licenses/>.
>> + *
>> + */
>> +
>> +#ifndef GRLIB_AHB_APB_PNP_H
>> +#define GRLIB_AHB_APB_PNP_H
>> +
>> +#define TYPE_GRLIB_AHB_PNP "grlib,ahbpnp"
>> +#define GRLIB_AHB_PNP(obj) \
>> +    OBJECT_CHECK(AHBPnp, (obj), TYPE_GRLIB_AHB_PNP)
>> +typedef struct AHBPnp AHBPnp;
>> +
>> +#define TYPE_GRLIB_APB_PNP "grlib,apbpnp"
>> +#define GRLIB_APB_PNP(obj) \
>> +    OBJECT_CHECK(APBPnp, (obj), TYPE_GRLIB_APB_PNP)
>> +typedef struct APBPnp APBPnp;
>> +
>> +void grlib_ahb_pnp_add_entry(AHBPnp *dev, uint32_t address, uint32_t mask,
>> +                             uint8_t vendor, uint16_t device, int slave,
>> +                             int type);
>> +void grlib_apb_pnp_add_entry(APBPnp *dev, uint32_t address, uint32_t mask,
>> +                             uint8_t vendor, uint16_t device, uint8_t version,
>> +                             uint8_t irq, int type);
>> +
>> +/* VENDORS */
>> +#define GRLIB_VENDOR_GAISLER (0x01)
>> +/* DEVICES */
>> +#define GRLIB_LEON3_DEV      (0x03)
>> +#define GRLIB_APBMST_DEV     (0x06)
>> +#define GRLIB_APBUART_DEV    (0x0C)
>> +#define GRLIB_IRQMP_DEV      (0x0D)
>> +#define GRLIB_GPTIMER_DEV    (0x11)
>> +/* TYPE */
>> +#define GRLIB_CPU_AREA       (0x00)
>> +#define GRLIB_APBIO_AREA     (0x01)
>> +#define GRLIB_AHBMEM_AREA    (0x02)
>> +
>> +#define GRLIB_AHB_MASTER     (0x00)
>> +#define GRLIB_AHB_SLAVE      (0x01)
>> +
>> +#endif /* GRLIB_AHB_APB_PNP_H */
>> diff --git a/include/hw/sparc/grlib.h b/include/hw/sparc/grlib.h
>> index 61a345c..28320ab 100644
>> --- a/include/hw/sparc/grlib.h
>> +++ b/include/hw/sparc/grlib.h
>> @@ -27,6 +27,7 @@
>>   
>>   #include "hw/qdev.h"
>>   #include "hw/sysbus.h"
>> +#include "hw/misc/grlib_ahb_apb_pnp.h"
>>   
>>   /* Emulation of GrLib device is base on the GRLIB IP Core User's Manual:
>>    * http://www.gaisler.com/products/grlib/grip.pdf
>> @@ -41,11 +42,9 @@ void grlib_irqmp_set_irq(void *opaque, int irq, int level);
>>   void grlib_irqmp_ack(DeviceState *dev, int intno);
>>   
>>   static inline
>> -DeviceState *grlib_irqmp_create(hwaddr   base,
>> -                                CPUSPARCState            *env,
>> -                                qemu_irq           **cpu_irqs,
>> -                                uint32_t             nr_irqs,
>> -                                set_pil_in_fn        set_pil_in)
>> +DeviceState *grlib_irqmp_create(hwaddr base, CPUSPARCState *env,
>> +                                qemu_irq **cpu_irqs, uint32_t nr_irqs,
>> +                                set_pil_in_fn set_pil_in, APBPnp *apb_pnp)
>>   {
>>       DeviceState *dev;
>>   
>> @@ -65,17 +64,18 @@ DeviceState *grlib_irqmp_create(hwaddr   base,
>>                                      dev,
>>                                      nr_irqs);
>>   
>> +    /* Register this device in the APB PNP device */
>> +    grlib_apb_pnp_add_entry(apb_pnp, base, 0xFFF, GRLIB_VENDOR_GAISLER,
>> +                            GRLIB_IRQMP_DEV, 2, 0, GRLIB_APBIO_AREA);
>>       return dev;
>>   }
>>   
>>   /* GPTimer */
>>   
>>   static inline
>> -DeviceState *grlib_gptimer_create(hwaddr  base,
>> -                                  uint32_t            nr_timers,
>> -                                  uint32_t            freq,
>> -                                  qemu_irq           *cpu_irqs,
>> -                                  int                 base_irq)
>> +DeviceState *grlib_gptimer_create(hwaddr base, uint32_t nr_timers,
>> +                                  uint32_t freq, qemu_irq *cpu_irqs,
>> +                                  int base_irq, APBPnp *apb_pnp)
>>   {
>>       DeviceState *dev;
>>       int i;
>> @@ -93,15 +93,18 @@ DeviceState *grlib_gptimer_create(hwaddr  base,
>>           sysbus_connect_irq(SYS_BUS_DEVICE(dev), i, cpu_irqs[base_irq + i]);
>>       }
>>   
>> +    /* Register this device in the APB PNP device */
>> +    grlib_apb_pnp_add_entry(apb_pnp, base, 0xFFF, GRLIB_VENDOR_GAISLER,
>> +                            GRLIB_GPTIMER_DEV, 0, base_irq, GRLIB_APBIO_AREA);
>>       return dev;
>>   }
>>   
>>   /* APB UART */
>>   
>>   static inline
>> -DeviceState *grlib_apbuart_create(hwaddr  base,
>> -                                  Chardev    *serial,
>> -                                  qemu_irq            irq)
>> +DeviceState *grlib_apbuart_create(hwaddr base, Chardev *serial,
>> +                                  qemu_irq *cpu_irqs, int base_irq,
>> +                                  APBPnp *apb_pnp)
>>   {
>>       DeviceState *dev;
>>   
>> @@ -112,7 +115,11 @@ DeviceState *grlib_apbuart_create(hwaddr  base,
>>   
>>       sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base);
>>   
>> -    sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, irq);
>> +    sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, cpu_irqs[base_irq]);
>> +
>> +    /* Register this device in the APB PNP device */
>> +    grlib_apb_pnp_add_entry(apb_pnp, base, 0xFFF, GRLIB_VENDOR_GAISLER,
>> +                            GRLIB_APBUART_DEV, 1, base_irq, GRLIB_APBIO_AREA);
>>   
>>       return dev;
>>   }
>>
> 
> Other than that, checkpatch gave a warning to check MAINTAINERS and it looks like
> you're missing an entry for the new hw/misc/grlib_ahb_apb_pnp.h header file.

Ah yes good point.. I missed that.. I took that as a false positive because
hw/*/grlib* was in MAINTAINERS.

Regards,
Fred

> 
> Finally there's a small typo in the commit message: mecanism -> mechanism.
> 
> 
> ATB,
> 
> Mark.
>
Mark Cave-Ayland May 3, 2019, 8:37 a.m. UTC | #3
On 03/05/2019 09:24, KONRAD Frederic wrote:

> Le 5/3/19 à 10:09 AM, Mark Cave-Ayland a écrit :
>> On 25/04/2019 13:18, KONRAD Frederic wrote:
>>
>>> This adds the AHB and APB plug and play devices.
>>> They are scanned during the linux boot to discover the various peripheral.
>>>
>>> Reviewed-by: Fabien Chouteau <chouteau@adacore.com>
>>> Signed-off-by: KONRAD Frederic <frederic.konrad@adacore.com>
>>> ---
>>>   hw/misc/Makefile.objs               |   2 +
>>>   hw/misc/grlib_ahb_apb_pnp.c         | 269 ++++++++++++++++++++++++++++++++++++
>>>   hw/sparc/leon3.c                    |  34 ++++-
>>>   include/hw/misc/grlib_ahb_apb_pnp.h |  60 ++++++++
>>>   include/hw/sparc/grlib.h            |  35 +++--
>>>   5 files changed, 382 insertions(+), 18 deletions(-)
>>>   create mode 100644 hw/misc/grlib_ahb_apb_pnp.c
>>>   create mode 100644 include/hw/misc/grlib_ahb_apb_pnp.h
>>>
>>> diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
>>> index c71e07a..77b9df9 100644
>>> --- a/hw/misc/Makefile.objs
>>> +++ b/hw/misc/Makefile.objs
>>> @@ -77,3 +77,5 @@ obj-$(CONFIG_AUX) += auxbus.o
>>>   obj-$(CONFIG_ASPEED_SOC) += aspeed_scu.o aspeed_sdmc.o
>>>   obj-$(CONFIG_MSF2) += msf2-sysreg.o
>>>   obj-$(CONFIG_NRF51_SOC) += nrf51_rng.o
>>> +
>>> +obj-$(CONFIG_GRLIB) += grlib_ahb_apb_pnp.o
>>> diff --git a/hw/misc/grlib_ahb_apb_pnp.c b/hw/misc/grlib_ahb_apb_pnp.c
>>> new file mode 100644
>>> index 0000000..90d5f6e
>>> --- /dev/null
>>> +++ b/hw/misc/grlib_ahb_apb_pnp.c
>>> @@ -0,0 +1,269 @@
>>> +/*
>>> + * GRLIB AHB APB PNP
>>> + *
>>> + *  Copyright (C) 2019 AdaCore
>>> + *
>>> + *  Developed by :
>>> + *  Frederic Konrad   <frederic.konrad@adacore.com>
>>> + *
>>> + * This program is free software; you can redistribute it and/or modify
>>> + * it under the terms of the GNU General Public License as published by
>>> + * the Free Software Foundation, either version 2 of the License, or
>>> + * (at your option) any later version.
>>> + *
>>> + * This program is distributed in the hope that it will be useful,
>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>>> + * GNU General Public License for more details.
>>> + *
>>> + * You should have received a copy of the GNU General Public License along
>>> + * with this program; if not, see <http://www.gnu.org/licenses/>.
>>> + *
>>> + */
>>> +
>>> +#include "qemu/osdep.h"
>>> +#include "hw/sysbus.h"
>>> +#include "hw/misc/grlib_ahb_apb_pnp.h"
>>> +
>>> +#define GRLIB_PNP_VENDOR_SHIFT (24)
>>> +#define GRLIB_PNP_VENDOR_SIZE   (8)
>>> +#define GRLIB_PNP_DEV_SHIFT    (12)
>>> +#define GRLIB_PNP_DEV_SIZE     (12)
>>> +#define GRLIB_PNP_VER_SHIFT     (5)
>>> +#define GRLIB_PNP_VER_SIZE      (5)
>>> +#define GRLIB_PNP_IRQ_SHIFT     (0)
>>> +#define GRLIB_PNP_IRQ_SIZE      (5)
>>> +#define GRLIB_PNP_ADDR_SHIFT   (20)
>>> +#define GRLIB_PNP_ADDR_SIZE    (12)
>>> +#define GRLIB_PNP_MASK_SHIFT    (4)
>>> +#define GRLIB_PNP_MASK_SIZE    (12)
>>> +
>>> +#define GRLIB_AHB_DEV_ADDR_SHIFT   (20)
>>> +#define GRLIB_AHB_DEV_ADDR_SIZE    (12)
>>> +#define GRLIB_AHB_ENTRY_SIZE       (0x20)
>>> +#define GRLIB_AHB_MAX_DEV          (64)
>>> +#define GRLIB_AHB_SLAVE_OFFSET     (0x800)
>>> +
>>> +#define GRLIB_APB_DEV_ADDR_SHIFT   (8)
>>> +#define GRLIB_APB_DEV_ADDR_SIZE    (12)
>>> +#define GRLIB_APB_ENTRY_SIZE       (0x08)
>>> +#define GRLIB_APB_MAX_DEV          (512)
>>> +
>>> +#define GRLIB_PNP_MAX_REGS         (0x1000)
>>> +
>>> +typedef struct AHBPnp {
>>> +    SysBusDevice parent_obj;
>>> +    MemoryRegion iomem;
>>> +
>>> +    uint32_t regs[GRLIB_PNP_MAX_REGS >> 2];
>>> +    uint8_t master_count;
>>> +    uint8_t slave_count;
>>> +} AHBPnp;
>>> +
>>> +void grlib_ahb_pnp_add_entry(AHBPnp *dev, uint32_t address, uint32_t mask,
>>> +                             uint8_t vendor, uint16_t device, int slave,
>>> +                             int type)
>>> +{
>>> +    unsigned int reg_start;
>>> +
>>> +    /*
>>> +     * AHB entries look like this:
>>> +     *
>>> +     * 31 -------- 23 -------- 11 ----- 9 -------- 4 --- 0
>>> +     *  | VENDOR ID | DEVICE ID | IRQ ? | VERSION  | IRQ |
>>> +     *  --------------------------------------------------
>>> +     *  |                      USER                      |
>>> +     *  --------------------------------------------------
>>> +     *  |                      USER                      |
>>> +     *  --------------------------------------------------
>>> +     *  |                      USER                      |
>>> +     *  --------------------------------------------------
>>> +     *  |                      USER                      |
>>> +     *  --------------------------------------------------
>>> +     * 31 ----------- 20 --- 15 ----------------- 3 ---- 0
>>> +     *  | ADDR[31..12] | 00PC |        MASK       | TYPE |
>>> +     *  --------------------------------------------------
>>> +     * 31 ----------- 20 --- 15 ----------------- 3 ---- 0
>>> +     *  | ADDR[31..12] | 00PC |        MASK       | TYPE |
>>> +     *  --------------------------------------------------
>>> +     * 31 ----------- 20 --- 15 ----------------- 3 ---- 0
>>> +     *  | ADDR[31..12] | 00PC |        MASK       | TYPE |
>>> +     *  --------------------------------------------------
>>> +     * 31 ----------- 20 --- 15 ----------------- 3 ---- 0
>>> +     *  | ADDR[31..12] | 00PC |        MASK       | TYPE |
>>> +     *  --------------------------------------------------
>>> +     */
>>> +
>>> +    if (slave) {
>>> +        assert(dev->slave_count < GRLIB_AHB_MAX_DEV);
>>> +        reg_start = (GRLIB_AHB_SLAVE_OFFSET
>>> +                  + (dev->slave_count * GRLIB_AHB_ENTRY_SIZE)) >> 2;
>>> +        dev->slave_count++;
>>> +    } else {
>>> +        assert(dev->master_count < GRLIB_AHB_MAX_DEV);
>>> +        reg_start = (dev->master_count * GRLIB_AHB_ENTRY_SIZE) >> 2;
>>> +        dev->master_count++;
>>> +    }
>>> +
>>> +    dev->regs[reg_start] = deposit32(dev->regs[reg_start],
>>> +                                     GRLIB_PNP_VENDOR_SHIFT,
>>> +                                     GRLIB_PNP_VENDOR_SIZE,
>>> +                                     vendor);
>>> +    dev->regs[reg_start] = deposit32(dev->regs[reg_start],
>>> +                                     GRLIB_PNP_DEV_SHIFT,
>>> +                                     GRLIB_PNP_DEV_SIZE,
>>> +                                     device);
>>> +    reg_start += 4;
>>> +    /* AHB Memory Space */
>>> +    dev->regs[reg_start] = type;
>>> +    dev->regs[reg_start] = deposit32(dev->regs[reg_start],
>>> +                                     GRLIB_PNP_ADDR_SHIFT,
>>> +                                     GRLIB_PNP_ADDR_SIZE,
>>> +                                     extract32(address,
>>> +                                               GRLIB_AHB_DEV_ADDR_SHIFT,
>>> +                                               GRLIB_AHB_DEV_ADDR_SIZE));
>>> +    dev->regs[reg_start] = deposit32(dev->regs[reg_start],
>>> +                                     GRLIB_PNP_MASK_SHIFT,
>>> +                                     GRLIB_PNP_MASK_SIZE,
>>> +                                     mask);
>>> +}
>>> +
>>> +static uint64_t grlib_ahb_pnp_read(void *opaque, hwaddr offset, unsigned size)
>>> +{
>>> +    AHBPnp *ahb_pnp = GRLIB_AHB_PNP(opaque);
>>> +
>>> +    return ahb_pnp->regs[offset >> 2];
>>> +}
>>> +
>>> +static const MemoryRegionOps grlib_ahb_pnp_ops = {
>>> +    .read       = grlib_ahb_pnp_read,
>>> +    .endianness = DEVICE_NATIVE_ENDIAN,
>>> +};
>>
>> Should be DEVICE_BIG_ENDIAN?
>>
>>> +static void grlib_ahb_pnp_realize(DeviceState *dev, Error **errp)
>>> +{
>>> +    AHBPnp *ahb_pnp = GRLIB_AHB_PNP(dev);
>>> +    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
>>> +
>>> +    memory_region_init_io(&ahb_pnp->iomem, OBJECT(dev), &grlib_ahb_pnp_ops,
>>> +                          ahb_pnp, TYPE_GRLIB_AHB_PNP, GRLIB_PNP_MAX_REGS);
>>> +    sysbus_init_mmio(sbd, &ahb_pnp->iomem);
>>> +}
>>> +
>>> +static void grlib_ahb_pnp_class_init(ObjectClass *klass, void *data)
>>> +{
>>> +    DeviceClass *dc = DEVICE_CLASS(klass);
>>> +
>>> +    dc->realize = grlib_ahb_pnp_realize;
>>
>> Since this requires wiring up in code (i.e. can't be instantiated in the monitor via
>> device_add) you'll need to mark the device as not user creatable e.g.
>>
>>      /* Reason: Must be wired up in code (see leon3_generic_hw_init() function) */
>>      dc->user_creatable = false;
>>
> 
> I think that's actually the default for SYS_BUS_DEVICE:
> 
> static void sysbus_device_class_init(ObjectClass *klass, void *data)
> {
>     DeviceClass *k = DEVICE_CLASS(klass);
>     k->realize = sysbus_realize;
>     k->bus_type = TYPE_SYSTEM_BUS;
>     /*
>      * device_add plugs devices into a suitable bus.  For "real" buses,
>      * that actually connects the device.  For sysbus, the connections
>      * need to be made separately, and device_add can't do that.  The
>      * device would be left unconnected, and will probably not work
>      *
>      * However, a few machines can handle device_add/-device with
>      * a few specific sysbus devices. In those cases, the device
>      * subclass needs to override it and set user_creatable=true.
>      */
>     k->user_creatable = false;
> }
> 
> But I can change that for clarity if you want.

Ah good spot - in that case don't worry about adding this, since I can't see that
default changing anytime.

>>> +}
>>> +
>>> +static const TypeInfo grlib_ahb_pnp_info = {
>>> +    .name          = TYPE_GRLIB_AHB_PNP,
>>> +    .parent        = TYPE_SYS_BUS_DEVICE,
>>> +    .instance_size = sizeof(AHBPnp),
>>> +    .class_init    = grlib_ahb_pnp_class_init,
>>> +};
>>> +
>>> +/* APBPnp */
>>> +
>>> +typedef struct APBPnp {
>>> +    SysBusDevice parent_obj;
>>> +    MemoryRegion iomem;
>>> +
>>> +    uint32_t regs[GRLIB_PNP_MAX_REGS >> 2];
>>> +    uint32_t entry_count;
>>> +} APBPnp;
>>> +
>>> +void grlib_apb_pnp_add_entry(APBPnp *dev, uint32_t address, uint32_t mask,
>>> +                             uint8_t vendor, uint16_t device, uint8_t version,
>>> +                             uint8_t irq, int type)
>>> +{
>>> +    unsigned int reg_start;
>>> +
>>> +    /*
>>> +     * APB entries look like this:
>>> +     *
>>> +     * 31 -------- 23 -------- 11 ----- 9 ------- 4 --- 0
>>> +     *  | VENDOR ID | DEVICE ID | IRQ ? | VERSION | IRQ |
>>> +     *
>>> +     * 31 ---------- 20 --- 15 ----------------- 3 ---- 0
>>> +     *  | ADDR[20..8] | 0000 |        MASK       | TYPE |
>>> +     */
>>> +
>>> +    assert(dev->entry_count < GRLIB_APB_MAX_DEV);
>>> +    reg_start = (dev->entry_count * GRLIB_APB_ENTRY_SIZE) >> 2;
>>> +    dev->entry_count++;
>>> +
>>> +    dev->regs[reg_start] = deposit32(dev->regs[reg_start],
>>> +                                     GRLIB_PNP_VENDOR_SHIFT,
>>> +                                     GRLIB_PNP_VENDOR_SIZE,
>>> +                                     vendor);
>>> +    dev->regs[reg_start] = deposit32(dev->regs[reg_start],
>>> +                                     GRLIB_PNP_DEV_SHIFT,
>>> +                                     GRLIB_PNP_DEV_SIZE,
>>> +                                     device);
>>> +    dev->regs[reg_start] = deposit32(dev->regs[reg_start],
>>> +                                     GRLIB_PNP_VER_SHIFT,
>>> +                                     GRLIB_PNP_VER_SIZE,
>>> +                                     version);
>>> +    dev->regs[reg_start] = deposit32(dev->regs[reg_start],
>>> +                                     GRLIB_PNP_IRQ_SHIFT,
>>> +                                     GRLIB_PNP_IRQ_SIZE,
>>> +                                     irq);
>>> +    reg_start += 1;
>>> +    dev->regs[reg_start] = type;
>>> +    dev->regs[reg_start] = deposit32(dev->regs[reg_start],
>>> +                                     GRLIB_PNP_ADDR_SHIFT,
>>> +                                     GRLIB_PNP_ADDR_SIZE,
>>> +                                     extract32(address,
>>> +                                               GRLIB_APB_DEV_ADDR_SHIFT,
>>> +                                               GRLIB_APB_DEV_ADDR_SIZE));
>>> +    dev->regs[reg_start] = deposit32(dev->regs[reg_start],
>>> +                                     GRLIB_PNP_MASK_SHIFT,
>>> +                                     GRLIB_PNP_MASK_SIZE,
>>> +                                     mask);
>>> +}
>>> +
>>> +static uint64_t grlib_apb_pnp_read(void *opaque, hwaddr offset, unsigned size)
>>> +{
>>> +    APBPnp *apb_pnp = GRLIB_APB_PNP(opaque);
>>> +
>>> +    return apb_pnp->regs[offset >> 2];
>>> +}
>>> +
>>> +static const MemoryRegionOps grlib_apb_pnp_ops = {
>>> +    .read       = grlib_apb_pnp_read,
>>> +    .endianness = DEVICE_NATIVE_ENDIAN,
>>> +};
>>
>> and also here DEVICE_BIG_ENDIAN?
>>
>>> +static void grlib_apb_pnp_realize(DeviceState *dev, Error **errp)
>>> +{
>>> +    APBPnp *apb_pnp = GRLIB_APB_PNP(dev);
>>> +    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
>>> +
>>> +    memory_region_init_io(&apb_pnp->iomem, OBJECT(dev), &grlib_apb_pnp_ops,
>>> +                          apb_pnp, TYPE_GRLIB_APB_PNP, GRLIB_PNP_MAX_REGS);
>>> +    sysbus_init_mmio(sbd, &apb_pnp->iomem);
>>> +}
>>> +
>>> +static void grlib_apb_pnp_class_init(ObjectClass *klass, void *data)
>>> +{
>>> +    DeviceClass *dc = DEVICE_CLASS(klass);
>>> +
>>> +    dc->realize = grlib_apb_pnp_realize;
>>
>> Same again here: this device should be marked as not user creatable.
>>
>>> +}
>>> +
>>> +static const TypeInfo grlib_apb_pnp_info = {
>>> +    .name          = TYPE_GRLIB_APB_PNP,
>>> +    .parent        = TYPE_SYS_BUS_DEVICE,
>>> +    .instance_size = sizeof(APBPnp),
>>> +    .class_init    = grlib_apb_pnp_class_init,
>>> +};
>>> +
>>> +static void grlib_ahb_apb_pnp_register_types(void)
>>> +{
>>> +    type_register_static(&grlib_ahb_pnp_info);
>>> +    type_register_static(&grlib_apb_pnp_info);
>>> +}
>>> +
>>> +type_init(grlib_ahb_apb_pnp_register_types)
>>> diff --git a/hw/sparc/leon3.c b/hw/sparc/leon3.c
>>> index f25432c..03ff163 100644
>>> --- a/hw/sparc/leon3.c
>>> +++ b/hw/sparc/leon3.c
>>> @@ -46,6 +46,13 @@
>>>   #define PROM_FILENAME        "u-boot.bin"
>>>   #define LEON3_PROM_OFFSET    (0x00000000)
>>>   #define LEON3_RAM_OFFSET     (0x40000000)
>>> +#define LEON3_APBUART_OFFSET (0x80000100)
>>> +#define LEON3_APBUART_IRQ    (0x3)
>>> +#define LEON3_IRQMP_OFFSET   (0x80000200)
>>> +#define LEON3_GPTIMER_OFFSET (0x80000300)
>>> +#define LEON3_GPTIMER_IRQ    (0x6)
>>> +#define LEON3_APB_PNP_OFFSET (0x800FF000)
>>> +#define LEON3_AHB_PNP_OFFSET (0xFFFFF000)
>>>     #define MAX_PILS 16
>>>   @@ -176,6 +183,8 @@ static void leon3_generic_hw_init(MachineState *machine)
>>>       int         bios_size;
>>>       int         prom_size;
>>>       ResetData  *reset_info;
>>> +    AHBPnp *ahb_pnp;
>>> +    APBPnp *apb_pnp;
>>>         /* Init CPU */
>>>       cpu = SPARC_CPU(cpu_create(machine->cpu_type));
>>> @@ -189,9 +198,23 @@ static void leon3_generic_hw_init(MachineState *machine)
>>>       reset_info->sp    = LEON3_RAM_OFFSET + ram_size;
>>>       qemu_register_reset(main_cpu_reset, reset_info);
>>>   +    ahb_pnp = GRLIB_AHB_PNP(object_new(TYPE_GRLIB_AHB_PNP));
>>> +    object_property_set_bool(OBJECT(ahb_pnp), true, "realized", &error_fatal);
>>> +    sysbus_mmio_map(SYS_BUS_DEVICE(ahb_pnp), 0, LEON3_AHB_PNP_OFFSET);
>>> +    grlib_ahb_pnp_add_entry(ahb_pnp, 0, 0, GRLIB_VENDOR_GAISLER,
>>> +                            GRLIB_LEON3_DEV, GRLIB_AHB_MASTER,
>>> +                            GRLIB_CPU_AREA);
>>> +
>>> +    apb_pnp = GRLIB_APB_PNP(object_new(TYPE_GRLIB_APB_PNP));
>>> +    object_property_set_bool(OBJECT(apb_pnp), true, "realized", &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,
>>> +                            GRLIB_VENDOR_GAISLER, GRLIB_APBMST_DEV,
>>> +                            GRLIB_AHB_SLAVE, GRLIB_AHBMEM_AREA);
>>> +
>>>       /* Allocate IRQ manager */
>>> -    grlib_irqmp_create(0x80000200, env, &cpu_irqs, MAX_PILS,
>>> -                       &leon3_set_pil_in);
>>> +    grlib_irqmp_create(LEON3_IRQMP_OFFSET, env, &cpu_irqs, MAX_PILS,
>>> +                       &leon3_set_pil_in, apb_pnp);
>>
>> Whilst we're here, is there any way we can get rid of these old-style create
>> functions? The general philosophy today is that rather than have multiple per-board
>> init functions, expose the required properties via qdev and then wire up the device
>> directly within the board itself.
>>
>> A quick glance at the code suggests you can just move the contents of the relevant
>> functions from the header file directly inline with a few minor tweaks.
> 
> Ok I'll give it a try.

If you remove the _create() functions in a separate patch before this one, then it
helps simplify this patch as it removes the need to change grlib.h (I think you can
move the #include for hw/misc/grlib_ahb_apb_pnp.h directly into hw/sparc/leon3.c).

>>
>>>       env->qemu_irq_ack = leon3_irq_manager;
>>>   @@ -270,11 +293,14 @@ static void leon3_generic_hw_init(MachineState *machine)
>>>       }
>>>         /* Allocate timers */
>>> -    grlib_gptimer_create(0x80000300, 2, CPU_CLK, cpu_irqs, 6);
>>> +    grlib_gptimer_create(LEON3_GPTIMER_OFFSET, 2, CPU_CLK, cpu_irqs,
>>> +                         LEON3_GPTIMER_IRQ,
>>> +                         apb_pnp);
>>
>> And here.
>>
>>>       /* Allocate uart */
>>>       if (serial_hd(0)) {
>>> -        grlib_apbuart_create(0x80000100, serial_hd(0), cpu_irqs[3]);
>>> +        grlib_apbuart_create(LEON3_APBUART_OFFSET, serial_hd(0), cpu_irqs,
>>> +                             LEON3_APBUART_IRQ, apb_pnp);
>>
>> And here too.
>>
>>>       }
>>>   }
>>>   diff --git a/include/hw/misc/grlib_ahb_apb_pnp.h
>>> b/include/hw/misc/grlib_ahb_apb_pnp.h
>>> new file mode 100644
>>> index 0000000..a0f6dcf
>>> --- /dev/null
>>> +++ b/include/hw/misc/grlib_ahb_apb_pnp.h
>>> @@ -0,0 +1,60 @@
>>> +/*
>>> + * GRLIB AHB APB PNP
>>> + *
>>> + *  Copyright (C) 2019 AdaCore
>>> + *
>>> + *  Developed by :
>>> + *  Frederic Konrad   <frederic.konrad@adacore.com>
>>> + *
>>> + * This program is free software; you can redistribute it and/or modify
>>> + * it under the terms of the GNU General Public License as published by
>>> + * the Free Software Foundation, either version 2 of the License, or
>>> + * (at your option) any later version.
>>> + *
>>> + * This program is distributed in the hope that it will be useful,
>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>>> + * GNU General Public License for more details.
>>> + *
>>> + * You should have received a copy of the GNU General Public License along
>>> + * with this program; if not, see <http://www.gnu.org/licenses/>.
>>> + *
>>> + */
>>> +
>>> +#ifndef GRLIB_AHB_APB_PNP_H
>>> +#define GRLIB_AHB_APB_PNP_H
>>> +
>>> +#define TYPE_GRLIB_AHB_PNP "grlib,ahbpnp"
>>> +#define GRLIB_AHB_PNP(obj) \
>>> +    OBJECT_CHECK(AHBPnp, (obj), TYPE_GRLIB_AHB_PNP)
>>> +typedef struct AHBPnp AHBPnp;
>>> +
>>> +#define TYPE_GRLIB_APB_PNP "grlib,apbpnp"
>>> +#define GRLIB_APB_PNP(obj) \
>>> +    OBJECT_CHECK(APBPnp, (obj), TYPE_GRLIB_APB_PNP)
>>> +typedef struct APBPnp APBPnp;
>>> +
>>> +void grlib_ahb_pnp_add_entry(AHBPnp *dev, uint32_t address, uint32_t mask,
>>> +                             uint8_t vendor, uint16_t device, int slave,
>>> +                             int type);
>>> +void grlib_apb_pnp_add_entry(APBPnp *dev, uint32_t address, uint32_t mask,
>>> +                             uint8_t vendor, uint16_t device, uint8_t version,
>>> +                             uint8_t irq, int type);
>>> +
>>> +/* VENDORS */
>>> +#define GRLIB_VENDOR_GAISLER (0x01)
>>> +/* DEVICES */
>>> +#define GRLIB_LEON3_DEV      (0x03)
>>> +#define GRLIB_APBMST_DEV     (0x06)
>>> +#define GRLIB_APBUART_DEV    (0x0C)
>>> +#define GRLIB_IRQMP_DEV      (0x0D)
>>> +#define GRLIB_GPTIMER_DEV    (0x11)
>>> +/* TYPE */
>>> +#define GRLIB_CPU_AREA       (0x00)
>>> +#define GRLIB_APBIO_AREA     (0x01)
>>> +#define GRLIB_AHBMEM_AREA    (0x02)
>>> +
>>> +#define GRLIB_AHB_MASTER     (0x00)
>>> +#define GRLIB_AHB_SLAVE      (0x01)
>>> +
>>> +#endif /* GRLIB_AHB_APB_PNP_H */
>>> diff --git a/include/hw/sparc/grlib.h b/include/hw/sparc/grlib.h
>>> index 61a345c..28320ab 100644
>>> --- a/include/hw/sparc/grlib.h
>>> +++ b/include/hw/sparc/grlib.h
>>> @@ -27,6 +27,7 @@
>>>     #include "hw/qdev.h"
>>>   #include "hw/sysbus.h"
>>> +#include "hw/misc/grlib_ahb_apb_pnp.h"
>>>     /* Emulation of GrLib device is base on the GRLIB IP Core User's Manual:
>>>    * http://www.gaisler.com/products/grlib/grip.pdf
>>> @@ -41,11 +42,9 @@ void grlib_irqmp_set_irq(void *opaque, int irq, int level);
>>>   void grlib_irqmp_ack(DeviceState *dev, int intno);
>>>     static inline
>>> -DeviceState *grlib_irqmp_create(hwaddr   base,
>>> -                                CPUSPARCState            *env,
>>> -                                qemu_irq           **cpu_irqs,
>>> -                                uint32_t             nr_irqs,
>>> -                                set_pil_in_fn        set_pil_in)
>>> +DeviceState *grlib_irqmp_create(hwaddr base, CPUSPARCState *env,
>>> +                                qemu_irq **cpu_irqs, uint32_t nr_irqs,
>>> +                                set_pil_in_fn set_pil_in, APBPnp *apb_pnp)
>>>   {
>>>       DeviceState *dev;
>>>   @@ -65,17 +64,18 @@ DeviceState *grlib_irqmp_create(hwaddr   base,
>>>                                      dev,
>>>                                      nr_irqs);
>>>   +    /* Register this device in the APB PNP device */
>>> +    grlib_apb_pnp_add_entry(apb_pnp, base, 0xFFF, GRLIB_VENDOR_GAISLER,
>>> +                            GRLIB_IRQMP_DEV, 2, 0, GRLIB_APBIO_AREA);
>>>       return dev;
>>>   }
>>>     /* GPTimer */
>>>     static inline
>>> -DeviceState *grlib_gptimer_create(hwaddr  base,
>>> -                                  uint32_t            nr_timers,
>>> -                                  uint32_t            freq,
>>> -                                  qemu_irq           *cpu_irqs,
>>> -                                  int                 base_irq)
>>> +DeviceState *grlib_gptimer_create(hwaddr base, uint32_t nr_timers,
>>> +                                  uint32_t freq, qemu_irq *cpu_irqs,
>>> +                                  int base_irq, APBPnp *apb_pnp)
>>>   {
>>>       DeviceState *dev;
>>>       int i;
>>> @@ -93,15 +93,18 @@ DeviceState *grlib_gptimer_create(hwaddr  base,
>>>           sysbus_connect_irq(SYS_BUS_DEVICE(dev), i, cpu_irqs[base_irq + i]);
>>>       }
>>>   +    /* Register this device in the APB PNP device */
>>> +    grlib_apb_pnp_add_entry(apb_pnp, base, 0xFFF, GRLIB_VENDOR_GAISLER,
>>> +                            GRLIB_GPTIMER_DEV, 0, base_irq, GRLIB_APBIO_AREA);
>>>       return dev;
>>>   }
>>>     /* APB UART */
>>>     static inline
>>> -DeviceState *grlib_apbuart_create(hwaddr  base,
>>> -                                  Chardev    *serial,
>>> -                                  qemu_irq            irq)
>>> +DeviceState *grlib_apbuart_create(hwaddr base, Chardev *serial,
>>> +                                  qemu_irq *cpu_irqs, int base_irq,
>>> +                                  APBPnp *apb_pnp)
>>>   {
>>>       DeviceState *dev;
>>>   @@ -112,7 +115,11 @@ DeviceState *grlib_apbuart_create(hwaddr  base,
>>>         sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base);
>>>   -    sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, irq);
>>> +    sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, cpu_irqs[base_irq]);
>>> +
>>> +    /* Register this device in the APB PNP device */
>>> +    grlib_apb_pnp_add_entry(apb_pnp, base, 0xFFF, GRLIB_VENDOR_GAISLER,
>>> +                            GRLIB_APBUART_DEV, 1, base_irq, GRLIB_APBIO_AREA);
>>>         return dev;
>>>   }
>>>
>>
>> Other than that, checkpatch gave a warning to check MAINTAINERS and it looks like
>> you're missing an entry for the new hw/misc/grlib_ahb_apb_pnp.h header file.
> 
> Ah yes good point.. I missed that.. I took that as a false positive because
> hw/*/grlib* was in MAINTAINERS.
> 
> Regards,
> Fred
> 
>>
>> Finally there's a small typo in the commit message: mecanism -> mechanism.


ATB,

Mark.
diff mbox series

Patch

diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
index c71e07a..77b9df9 100644
--- a/hw/misc/Makefile.objs
+++ b/hw/misc/Makefile.objs
@@ -77,3 +77,5 @@  obj-$(CONFIG_AUX) += auxbus.o
 obj-$(CONFIG_ASPEED_SOC) += aspeed_scu.o aspeed_sdmc.o
 obj-$(CONFIG_MSF2) += msf2-sysreg.o
 obj-$(CONFIG_NRF51_SOC) += nrf51_rng.o
+
+obj-$(CONFIG_GRLIB) += grlib_ahb_apb_pnp.o
diff --git a/hw/misc/grlib_ahb_apb_pnp.c b/hw/misc/grlib_ahb_apb_pnp.c
new file mode 100644
index 0000000..90d5f6e
--- /dev/null
+++ b/hw/misc/grlib_ahb_apb_pnp.c
@@ -0,0 +1,269 @@ 
+/*
+ * GRLIB AHB APB PNP
+ *
+ *  Copyright (C) 2019 AdaCore
+ *
+ *  Developed by :
+ *  Frederic Konrad   <frederic.konrad@adacore.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#include "qemu/osdep.h"
+#include "hw/sysbus.h"
+#include "hw/misc/grlib_ahb_apb_pnp.h"
+
+#define GRLIB_PNP_VENDOR_SHIFT (24)
+#define GRLIB_PNP_VENDOR_SIZE   (8)
+#define GRLIB_PNP_DEV_SHIFT    (12)
+#define GRLIB_PNP_DEV_SIZE     (12)
+#define GRLIB_PNP_VER_SHIFT     (5)
+#define GRLIB_PNP_VER_SIZE      (5)
+#define GRLIB_PNP_IRQ_SHIFT     (0)
+#define GRLIB_PNP_IRQ_SIZE      (5)
+#define GRLIB_PNP_ADDR_SHIFT   (20)
+#define GRLIB_PNP_ADDR_SIZE    (12)
+#define GRLIB_PNP_MASK_SHIFT    (4)
+#define GRLIB_PNP_MASK_SIZE    (12)
+
+#define GRLIB_AHB_DEV_ADDR_SHIFT   (20)
+#define GRLIB_AHB_DEV_ADDR_SIZE    (12)
+#define GRLIB_AHB_ENTRY_SIZE       (0x20)
+#define GRLIB_AHB_MAX_DEV          (64)
+#define GRLIB_AHB_SLAVE_OFFSET     (0x800)
+
+#define GRLIB_APB_DEV_ADDR_SHIFT   (8)
+#define GRLIB_APB_DEV_ADDR_SIZE    (12)
+#define GRLIB_APB_ENTRY_SIZE       (0x08)
+#define GRLIB_APB_MAX_DEV          (512)
+
+#define GRLIB_PNP_MAX_REGS         (0x1000)
+
+typedef struct AHBPnp {
+    SysBusDevice parent_obj;
+    MemoryRegion iomem;
+
+    uint32_t regs[GRLIB_PNP_MAX_REGS >> 2];
+    uint8_t master_count;
+    uint8_t slave_count;
+} AHBPnp;
+
+void grlib_ahb_pnp_add_entry(AHBPnp *dev, uint32_t address, uint32_t mask,
+                             uint8_t vendor, uint16_t device, int slave,
+                             int type)
+{
+    unsigned int reg_start;
+
+    /*
+     * AHB entries look like this:
+     *
+     * 31 -------- 23 -------- 11 ----- 9 -------- 4 --- 0
+     *  | VENDOR ID | DEVICE ID | IRQ ? | VERSION  | IRQ |
+     *  --------------------------------------------------
+     *  |                      USER                      |
+     *  --------------------------------------------------
+     *  |                      USER                      |
+     *  --------------------------------------------------
+     *  |                      USER                      |
+     *  --------------------------------------------------
+     *  |                      USER                      |
+     *  --------------------------------------------------
+     * 31 ----------- 20 --- 15 ----------------- 3 ---- 0
+     *  | ADDR[31..12] | 00PC |        MASK       | TYPE |
+     *  --------------------------------------------------
+     * 31 ----------- 20 --- 15 ----------------- 3 ---- 0
+     *  | ADDR[31..12] | 00PC |        MASK       | TYPE |
+     *  --------------------------------------------------
+     * 31 ----------- 20 --- 15 ----------------- 3 ---- 0
+     *  | ADDR[31..12] | 00PC |        MASK       | TYPE |
+     *  --------------------------------------------------
+     * 31 ----------- 20 --- 15 ----------------- 3 ---- 0
+     *  | ADDR[31..12] | 00PC |        MASK       | TYPE |
+     *  --------------------------------------------------
+     */
+
+    if (slave) {
+        assert(dev->slave_count < GRLIB_AHB_MAX_DEV);
+        reg_start = (GRLIB_AHB_SLAVE_OFFSET
+                  + (dev->slave_count * GRLIB_AHB_ENTRY_SIZE)) >> 2;
+        dev->slave_count++;
+    } else {
+        assert(dev->master_count < GRLIB_AHB_MAX_DEV);
+        reg_start = (dev->master_count * GRLIB_AHB_ENTRY_SIZE) >> 2;
+        dev->master_count++;
+    }
+
+    dev->regs[reg_start] = deposit32(dev->regs[reg_start],
+                                     GRLIB_PNP_VENDOR_SHIFT,
+                                     GRLIB_PNP_VENDOR_SIZE,
+                                     vendor);
+    dev->regs[reg_start] = deposit32(dev->regs[reg_start],
+                                     GRLIB_PNP_DEV_SHIFT,
+                                     GRLIB_PNP_DEV_SIZE,
+                                     device);
+    reg_start += 4;
+    /* AHB Memory Space */
+    dev->regs[reg_start] = type;
+    dev->regs[reg_start] = deposit32(dev->regs[reg_start],
+                                     GRLIB_PNP_ADDR_SHIFT,
+                                     GRLIB_PNP_ADDR_SIZE,
+                                     extract32(address,
+                                               GRLIB_AHB_DEV_ADDR_SHIFT,
+                                               GRLIB_AHB_DEV_ADDR_SIZE));
+    dev->regs[reg_start] = deposit32(dev->regs[reg_start],
+                                     GRLIB_PNP_MASK_SHIFT,
+                                     GRLIB_PNP_MASK_SIZE,
+                                     mask);
+}
+
+static uint64_t grlib_ahb_pnp_read(void *opaque, hwaddr offset, unsigned size)
+{
+    AHBPnp *ahb_pnp = GRLIB_AHB_PNP(opaque);
+
+    return ahb_pnp->regs[offset >> 2];
+}
+
+static const MemoryRegionOps grlib_ahb_pnp_ops = {
+    .read       = grlib_ahb_pnp_read,
+    .endianness = DEVICE_NATIVE_ENDIAN,
+};
+
+static void grlib_ahb_pnp_realize(DeviceState *dev, Error **errp)
+{
+    AHBPnp *ahb_pnp = GRLIB_AHB_PNP(dev);
+    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
+
+    memory_region_init_io(&ahb_pnp->iomem, OBJECT(dev), &grlib_ahb_pnp_ops,
+                          ahb_pnp, TYPE_GRLIB_AHB_PNP, GRLIB_PNP_MAX_REGS);
+    sysbus_init_mmio(sbd, &ahb_pnp->iomem);
+}
+
+static void grlib_ahb_pnp_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    dc->realize = grlib_ahb_pnp_realize;
+}
+
+static const TypeInfo grlib_ahb_pnp_info = {
+    .name          = TYPE_GRLIB_AHB_PNP,
+    .parent        = TYPE_SYS_BUS_DEVICE,
+    .instance_size = sizeof(AHBPnp),
+    .class_init    = grlib_ahb_pnp_class_init,
+};
+
+/* APBPnp */
+
+typedef struct APBPnp {
+    SysBusDevice parent_obj;
+    MemoryRegion iomem;
+
+    uint32_t regs[GRLIB_PNP_MAX_REGS >> 2];
+    uint32_t entry_count;
+} APBPnp;
+
+void grlib_apb_pnp_add_entry(APBPnp *dev, uint32_t address, uint32_t mask,
+                             uint8_t vendor, uint16_t device, uint8_t version,
+                             uint8_t irq, int type)
+{
+    unsigned int reg_start;
+
+    /*
+     * APB entries look like this:
+     *
+     * 31 -------- 23 -------- 11 ----- 9 ------- 4 --- 0
+     *  | VENDOR ID | DEVICE ID | IRQ ? | VERSION | IRQ |
+     *
+     * 31 ---------- 20 --- 15 ----------------- 3 ---- 0
+     *  | ADDR[20..8] | 0000 |        MASK       | TYPE |
+     */
+
+    assert(dev->entry_count < GRLIB_APB_MAX_DEV);
+    reg_start = (dev->entry_count * GRLIB_APB_ENTRY_SIZE) >> 2;
+    dev->entry_count++;
+
+    dev->regs[reg_start] = deposit32(dev->regs[reg_start],
+                                     GRLIB_PNP_VENDOR_SHIFT,
+                                     GRLIB_PNP_VENDOR_SIZE,
+                                     vendor);
+    dev->regs[reg_start] = deposit32(dev->regs[reg_start],
+                                     GRLIB_PNP_DEV_SHIFT,
+                                     GRLIB_PNP_DEV_SIZE,
+                                     device);
+    dev->regs[reg_start] = deposit32(dev->regs[reg_start],
+                                     GRLIB_PNP_VER_SHIFT,
+                                     GRLIB_PNP_VER_SIZE,
+                                     version);
+    dev->regs[reg_start] = deposit32(dev->regs[reg_start],
+                                     GRLIB_PNP_IRQ_SHIFT,
+                                     GRLIB_PNP_IRQ_SIZE,
+                                     irq);
+    reg_start += 1;
+    dev->regs[reg_start] = type;
+    dev->regs[reg_start] = deposit32(dev->regs[reg_start],
+                                     GRLIB_PNP_ADDR_SHIFT,
+                                     GRLIB_PNP_ADDR_SIZE,
+                                     extract32(address,
+                                               GRLIB_APB_DEV_ADDR_SHIFT,
+                                               GRLIB_APB_DEV_ADDR_SIZE));
+    dev->regs[reg_start] = deposit32(dev->regs[reg_start],
+                                     GRLIB_PNP_MASK_SHIFT,
+                                     GRLIB_PNP_MASK_SIZE,
+                                     mask);
+}
+
+static uint64_t grlib_apb_pnp_read(void *opaque, hwaddr offset, unsigned size)
+{
+    APBPnp *apb_pnp = GRLIB_APB_PNP(opaque);
+
+    return apb_pnp->regs[offset >> 2];
+}
+
+static const MemoryRegionOps grlib_apb_pnp_ops = {
+    .read       = grlib_apb_pnp_read,
+    .endianness = DEVICE_NATIVE_ENDIAN,
+};
+
+static void grlib_apb_pnp_realize(DeviceState *dev, Error **errp)
+{
+    APBPnp *apb_pnp = GRLIB_APB_PNP(dev);
+    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
+
+    memory_region_init_io(&apb_pnp->iomem, OBJECT(dev), &grlib_apb_pnp_ops,
+                          apb_pnp, TYPE_GRLIB_APB_PNP, GRLIB_PNP_MAX_REGS);
+    sysbus_init_mmio(sbd, &apb_pnp->iomem);
+}
+
+static void grlib_apb_pnp_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    dc->realize = grlib_apb_pnp_realize;
+}
+
+static const TypeInfo grlib_apb_pnp_info = {
+    .name          = TYPE_GRLIB_APB_PNP,
+    .parent        = TYPE_SYS_BUS_DEVICE,
+    .instance_size = sizeof(APBPnp),
+    .class_init    = grlib_apb_pnp_class_init,
+};
+
+static void grlib_ahb_apb_pnp_register_types(void)
+{
+    type_register_static(&grlib_ahb_pnp_info);
+    type_register_static(&grlib_apb_pnp_info);
+}
+
+type_init(grlib_ahb_apb_pnp_register_types)
diff --git a/hw/sparc/leon3.c b/hw/sparc/leon3.c
index f25432c..03ff163 100644
--- a/hw/sparc/leon3.c
+++ b/hw/sparc/leon3.c
@@ -46,6 +46,13 @@ 
 #define PROM_FILENAME        "u-boot.bin"
 #define LEON3_PROM_OFFSET    (0x00000000)
 #define LEON3_RAM_OFFSET     (0x40000000)
+#define LEON3_APBUART_OFFSET (0x80000100)
+#define LEON3_APBUART_IRQ    (0x3)
+#define LEON3_IRQMP_OFFSET   (0x80000200)
+#define LEON3_GPTIMER_OFFSET (0x80000300)
+#define LEON3_GPTIMER_IRQ    (0x6)
+#define LEON3_APB_PNP_OFFSET (0x800FF000)
+#define LEON3_AHB_PNP_OFFSET (0xFFFFF000)
 
 #define MAX_PILS 16
 
@@ -176,6 +183,8 @@  static void leon3_generic_hw_init(MachineState *machine)
     int         bios_size;
     int         prom_size;
     ResetData  *reset_info;
+    AHBPnp *ahb_pnp;
+    APBPnp *apb_pnp;
 
     /* Init CPU */
     cpu = SPARC_CPU(cpu_create(machine->cpu_type));
@@ -189,9 +198,23 @@  static void leon3_generic_hw_init(MachineState *machine)
     reset_info->sp    = LEON3_RAM_OFFSET + ram_size;
     qemu_register_reset(main_cpu_reset, reset_info);
 
+    ahb_pnp = GRLIB_AHB_PNP(object_new(TYPE_GRLIB_AHB_PNP));
+    object_property_set_bool(OBJECT(ahb_pnp), true, "realized", &error_fatal);
+    sysbus_mmio_map(SYS_BUS_DEVICE(ahb_pnp), 0, LEON3_AHB_PNP_OFFSET);
+    grlib_ahb_pnp_add_entry(ahb_pnp, 0, 0, GRLIB_VENDOR_GAISLER,
+                            GRLIB_LEON3_DEV, GRLIB_AHB_MASTER,
+                            GRLIB_CPU_AREA);
+
+    apb_pnp = GRLIB_APB_PNP(object_new(TYPE_GRLIB_APB_PNP));
+    object_property_set_bool(OBJECT(apb_pnp), true, "realized", &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,
+                            GRLIB_VENDOR_GAISLER, GRLIB_APBMST_DEV,
+                            GRLIB_AHB_SLAVE, GRLIB_AHBMEM_AREA);
+
     /* Allocate IRQ manager */
-    grlib_irqmp_create(0x80000200, env, &cpu_irqs, MAX_PILS,
-                       &leon3_set_pil_in);
+    grlib_irqmp_create(LEON3_IRQMP_OFFSET, env, &cpu_irqs, MAX_PILS,
+                       &leon3_set_pil_in, apb_pnp);
 
     env->qemu_irq_ack = leon3_irq_manager;
 
@@ -270,11 +293,14 @@  static void leon3_generic_hw_init(MachineState *machine)
     }
 
     /* Allocate timers */
-    grlib_gptimer_create(0x80000300, 2, CPU_CLK, cpu_irqs, 6);
+    grlib_gptimer_create(LEON3_GPTIMER_OFFSET, 2, CPU_CLK, cpu_irqs,
+                         LEON3_GPTIMER_IRQ,
+                         apb_pnp);
 
     /* Allocate uart */
     if (serial_hd(0)) {
-        grlib_apbuart_create(0x80000100, serial_hd(0), cpu_irqs[3]);
+        grlib_apbuart_create(LEON3_APBUART_OFFSET, serial_hd(0), cpu_irqs,
+                             LEON3_APBUART_IRQ, apb_pnp);
     }
 }
 
diff --git a/include/hw/misc/grlib_ahb_apb_pnp.h b/include/hw/misc/grlib_ahb_apb_pnp.h
new file mode 100644
index 0000000..a0f6dcf
--- /dev/null
+++ b/include/hw/misc/grlib_ahb_apb_pnp.h
@@ -0,0 +1,60 @@ 
+/*
+ * GRLIB AHB APB PNP
+ *
+ *  Copyright (C) 2019 AdaCore
+ *
+ *  Developed by :
+ *  Frederic Konrad   <frederic.konrad@adacore.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#ifndef GRLIB_AHB_APB_PNP_H
+#define GRLIB_AHB_APB_PNP_H
+
+#define TYPE_GRLIB_AHB_PNP "grlib,ahbpnp"
+#define GRLIB_AHB_PNP(obj) \
+    OBJECT_CHECK(AHBPnp, (obj), TYPE_GRLIB_AHB_PNP)
+typedef struct AHBPnp AHBPnp;
+
+#define TYPE_GRLIB_APB_PNP "grlib,apbpnp"
+#define GRLIB_APB_PNP(obj) \
+    OBJECT_CHECK(APBPnp, (obj), TYPE_GRLIB_APB_PNP)
+typedef struct APBPnp APBPnp;
+
+void grlib_ahb_pnp_add_entry(AHBPnp *dev, uint32_t address, uint32_t mask,
+                             uint8_t vendor, uint16_t device, int slave,
+                             int type);
+void grlib_apb_pnp_add_entry(APBPnp *dev, uint32_t address, uint32_t mask,
+                             uint8_t vendor, uint16_t device, uint8_t version,
+                             uint8_t irq, int type);
+
+/* VENDORS */
+#define GRLIB_VENDOR_GAISLER (0x01)
+/* DEVICES */
+#define GRLIB_LEON3_DEV      (0x03)
+#define GRLIB_APBMST_DEV     (0x06)
+#define GRLIB_APBUART_DEV    (0x0C)
+#define GRLIB_IRQMP_DEV      (0x0D)
+#define GRLIB_GPTIMER_DEV    (0x11)
+/* TYPE */
+#define GRLIB_CPU_AREA       (0x00)
+#define GRLIB_APBIO_AREA     (0x01)
+#define GRLIB_AHBMEM_AREA    (0x02)
+
+#define GRLIB_AHB_MASTER     (0x00)
+#define GRLIB_AHB_SLAVE      (0x01)
+
+#endif /* GRLIB_AHB_APB_PNP_H */
diff --git a/include/hw/sparc/grlib.h b/include/hw/sparc/grlib.h
index 61a345c..28320ab 100644
--- a/include/hw/sparc/grlib.h
+++ b/include/hw/sparc/grlib.h
@@ -27,6 +27,7 @@ 
 
 #include "hw/qdev.h"
 #include "hw/sysbus.h"
+#include "hw/misc/grlib_ahb_apb_pnp.h"
 
 /* Emulation of GrLib device is base on the GRLIB IP Core User's Manual:
  * http://www.gaisler.com/products/grlib/grip.pdf
@@ -41,11 +42,9 @@  void grlib_irqmp_set_irq(void *opaque, int irq, int level);
 void grlib_irqmp_ack(DeviceState *dev, int intno);
 
 static inline
-DeviceState *grlib_irqmp_create(hwaddr   base,
-                                CPUSPARCState            *env,
-                                qemu_irq           **cpu_irqs,
-                                uint32_t             nr_irqs,
-                                set_pil_in_fn        set_pil_in)
+DeviceState *grlib_irqmp_create(hwaddr base, CPUSPARCState *env,
+                                qemu_irq **cpu_irqs, uint32_t nr_irqs,
+                                set_pil_in_fn set_pil_in, APBPnp *apb_pnp)
 {
     DeviceState *dev;
 
@@ -65,17 +64,18 @@  DeviceState *grlib_irqmp_create(hwaddr   base,
                                    dev,
                                    nr_irqs);
 
+    /* Register this device in the APB PNP device */
+    grlib_apb_pnp_add_entry(apb_pnp, base, 0xFFF, GRLIB_VENDOR_GAISLER,
+                            GRLIB_IRQMP_DEV, 2, 0, GRLIB_APBIO_AREA);
     return dev;
 }
 
 /* GPTimer */
 
 static inline
-DeviceState *grlib_gptimer_create(hwaddr  base,
-                                  uint32_t            nr_timers,
-                                  uint32_t            freq,
-                                  qemu_irq           *cpu_irqs,
-                                  int                 base_irq)
+DeviceState *grlib_gptimer_create(hwaddr base, uint32_t nr_timers,
+                                  uint32_t freq, qemu_irq *cpu_irqs,
+                                  int base_irq, APBPnp *apb_pnp)
 {
     DeviceState *dev;
     int i;
@@ -93,15 +93,18 @@  DeviceState *grlib_gptimer_create(hwaddr  base,
         sysbus_connect_irq(SYS_BUS_DEVICE(dev), i, cpu_irqs[base_irq + i]);
     }
 
+    /* Register this device in the APB PNP device */
+    grlib_apb_pnp_add_entry(apb_pnp, base, 0xFFF, GRLIB_VENDOR_GAISLER,
+                            GRLIB_GPTIMER_DEV, 0, base_irq, GRLIB_APBIO_AREA);
     return dev;
 }
 
 /* APB UART */
 
 static inline
-DeviceState *grlib_apbuart_create(hwaddr  base,
-                                  Chardev    *serial,
-                                  qemu_irq            irq)
+DeviceState *grlib_apbuart_create(hwaddr base, Chardev *serial,
+                                  qemu_irq *cpu_irqs, int base_irq,
+                                  APBPnp *apb_pnp)
 {
     DeviceState *dev;
 
@@ -112,7 +115,11 @@  DeviceState *grlib_apbuart_create(hwaddr  base,
 
     sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base);
 
-    sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, irq);
+    sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, cpu_irqs[base_irq]);
+
+    /* Register this device in the APB PNP device */
+    grlib_apb_pnp_add_entry(apb_pnp, base, 0xFFF, GRLIB_VENDOR_GAISLER,
+                            GRLIB_APBUART_DEV, 1, base_irq, GRLIB_APBIO_AREA);
 
     return dev;
 }