Re: [PATCH v4 16/45] Enable BCM2838 PCIE

[Peter Maydell]

On Fri, 8 Dec 2023 at 02:33, Sergey Kambalin <serg.oker@gmail.com> wrote:
>
> Signed-off-by: Sergey Kambalin <sergey.kambalin@auriga.com>
> ---
>  hw/arm/bcm2838_pcie.c                |  3 +++
>  hw/arm/bcm2838_peripherals.c         | 26 ++++++++++++++++++++++++++
>  hw/arm/meson.build                   |  7 ++++---
>  hw/arm/raspi4b.c                     |  1 -
>  include/hw/arm/bcm2838_peripherals.h |  3 +++
>  5 files changed, 36 insertions(+), 4 deletions(-)
>
> diff --git a/hw/arm/bcm2838_pcie.c b/hw/arm/bcm2838_pcie.c
> index 75146d6c2e..8e046d0ac7 100644
> --- a/hw/arm/bcm2838_pcie.c
> +++ b/hw/arm/bcm2838_pcie.c
> @@ -239,6 +239,9 @@ static void bcm2838_pcie_root_reg_reset(PCIDevice *dev)
>  }
>
>  static void bcm2838_pcie_root_realize(PCIDevice *dev, Error **errp) {
> +    BCM2838PcieRootClass *brpc = BCM2838_PCIE_ROOT_GET_CLASS(dev);
> +
> +    brpc->parent_realize(dev, errp);
>      bcm2838_pcie_root_reg_reset(dev);
>  }
>

I guess this change should have been in the previous patch ?

> diff --git a/hw/arm/bcm2838_peripherals.c b/hw/arm/bcm2838_peripherals.c
> index d3b42cf25b..ee06214715 100644
> --- a/hw/arm/bcm2838_peripherals.c
> +++ b/hw/arm/bcm2838_peripherals.c
> @@ -12,6 +12,11 @@
>  #include "hw/arm/raspi_platform.h"
>  #include "hw/arm/bcm2838_peripherals.h"
>
> +#define PCIE_RC_OFFSET          0x1500000
> +#define PCIE_MMIO_OFFSET        0xc0000000
> +#define PCIE_MMIO_ARM_OFFSET    0x600000000
> +#define PCIE_MMIO_SIZE          0x40000000
> +
>  /* Lower peripheral base address on the VC (GPU) system bus */
>  #define BCM2838_VC_PERI_LOW_BASE 0x7c000000
>
> @@ -29,6 +34,10 @@ static void bcm2838_peripherals_init(Object *obj)
>                         bc->peri_low_size);
>      sysbus_init_mmio(SYS_BUS_DEVICE(s), &s->peri_low_mr);
>
> +    /* PCIe Host Bridge */
> +    object_initialize_child(obj, "pcie-host", &s->pcie_host,
> +                            TYPE_BCM2838_PCIE_HOST);
> +
>      /* Extended Mass Media Controller 2 */
>      object_initialize_child(obj, "emmc2", &s->emmc2, TYPE_SYSBUS_SDHCI);
>
> @@ -61,6 +70,9 @@ static void bcm2838_peripherals_realize(DeviceState *dev, 
> Error **errp)
>      MemoryRegion *mphi_mr;
>      BCM2838PeripheralState *s = BCM2838_PERIPHERALS(dev);
>      BCMSocPeripheralBaseState *s_base = BCM_SOC_PERIPHERALS_BASE(dev);
> +    MemoryRegion *regs_mr;
> +    MemoryRegion *mmio_mr;
> +
>      int n;
>
>      bcm_soc_peripherals_common_realize(dev, errp);
> @@ -172,6 +184,20 @@ static void bcm2838_peripherals_realize(DeviceState 
> *dev, Error **errp)
>                               BCM2838_MPHI_SIZE);
>      memory_region_add_subregion(&s_base->peri_mr, BCM2838_MPHI_OFFSET,
>                                  &s->mphi_mr_alias);
> +    /* PCIe Root Complex */
> +    if (!sysbus_realize(SYS_BUS_DEVICE(&s->pcie_host), errp)) {
> +        return;
> +    }
> +    /* RC registers region */
> +    regs_mr = sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->pcie_host), 0);
> +    memory_region_add_subregion(&s->peri_low_mr, PCIE_RC_OFFSET, regs_mr);
> +    /* MMIO region */
> +    mmio_mr = sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->pcie_host), 1);
> +    memory_region_init_alias(&s->pcie_mmio_alias, OBJECT(&s->pcie_host),
> +                             "pcie-mmio", mmio_mr, PCIE_MMIO_OFFSET,
> +                             PCIE_MMIO_SIZE);
> +    memory_region_add_subregion(get_system_memory(), PCIE_MMIO_ARM_OFFSET,
> +                                &s->pcie_mmio_alias);

Why do we need an alias region here rather than just mapping
the region from the pcie host device directly ?

>
>      /* GPIO */
>      if (!sysbus_realize(SYS_BUS_DEVICE(&s->gpio), errp)) {

thanks
-- PMM