mbox series

[RESEND,0/9] hw/riscv: microchip_pfsoc: Support factory HSS boot out of the box

Message ID 20201027141740.18336-1-bmeng.cn@gmail.com
Headers show
Series hw/riscv: microchip_pfsoc: Support factory HSS boot out of the box | expand

Message

Bin Meng Oct. 27, 2020, 2:17 p.m. UTC
From: Bin Meng <bin.meng@windriver.com>

At present the DDR memory controller is not modeled, hence the factory
HSS firmware does not boot out of the box on QEMU. A modified HSS is
required per the instructions on [1].

This series adds the missing DDR memory controller support to PolarFire
SoC, as well as adding various misc models to support the DDR memory
initialization done by HSS.

With this series, the unmodified HSS image can boot on QEMU out of the
box. The latest SD card image [2] released by Microchip was used for
testing which includes the pre-built U-Boot, device tree blob and Linux
kernel for the Icicle Kit. The instructions on [1] have been updated to
include the information on HSS and SD card image.

[1] https://wiki.qemu.org/Documentation/Platforms/RISCV#Microchip_PolarFire_SoC_Icicle_Kit
[2] ftp://ftpsoc.microsemi.com/outgoing/core-image-minimal-dev-icicle-kit-es-sd-20201009141623.rootfs.wic.gz


Bin Meng (9):
  hw/misc: Add Microchip PolarFire SoC DDR Memory Controller support
  hw/riscv: microchip_pfsoc: Connect DDR memory controller modules
  hw/misc: Add Microchip PolarFire SoC IOSCB module support
  hw/riscv: microchip_pfsoc: Connect the IOSCB module
  hw/misc: Add Microchip PolarFire SoC SYSREG module support
  hw/riscv: microchip_pfsoc: Connect the SYSREG module
  hw/riscv: microchip_pfsoc: Map debug memory
  hw/riscv: microchip_pfsoc: Correct DDR memory map
  hw/riscv: microchip_pfsoc: Hook the I2C1 controller

 MAINTAINERS                         |   6 +
 hw/misc/Kconfig                     |   9 ++
 hw/misc/mchp_pfsoc_dmc.c            | 216 +++++++++++++++++++++++++
 hw/misc/mchp_pfsoc_ioscb.c          | 242 ++++++++++++++++++++++++++++
 hw/misc/mchp_pfsoc_sysreg.c         |  99 ++++++++++++
 hw/misc/meson.build                 |   3 +
 hw/riscv/Kconfig                    |   3 +
 hw/riscv/microchip_pfsoc.c          | 103 ++++++++++--
 include/hw/misc/mchp_pfsoc_dmc.h    |  56 +++++++
 include/hw/misc/mchp_pfsoc_ioscb.h  |  50 ++++++
 include/hw/misc/mchp_pfsoc_sysreg.h |  39 +++++
 include/hw/riscv/microchip_pfsoc.h  |  17 +-
 12 files changed, 828 insertions(+), 15 deletions(-)
 create mode 100644 hw/misc/mchp_pfsoc_dmc.c
 create mode 100644 hw/misc/mchp_pfsoc_ioscb.c
 create mode 100644 hw/misc/mchp_pfsoc_sysreg.c
 create mode 100644 include/hw/misc/mchp_pfsoc_dmc.h
 create mode 100644 include/hw/misc/mchp_pfsoc_ioscb.h
 create mode 100644 include/hw/misc/mchp_pfsoc_sysreg.h

Comments

Alistair Francis Oct. 27, 2020, 5:30 p.m. UTC | #1
On Tue, Oct 27, 2020 at 7:53 AM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> From: Bin Meng <bin.meng@windriver.com>
>
> Somehow HSS needs to access address 0 [1] for the DDR calibration data
> which is in the chipset's debug memory. Let's map the debug memory.
>
> [1] See the config_copy() calls in various places in ddr_setup() in
>     the HSS source codes.

Really? This is reserved memory that they just read and write to? That's crazy.

If we really need this can you add a comment saying that the
documentation is wrong (again) and that this needs to be here.

>
> Signed-off-by: Bin Meng <bin.meng@windriver.com>
> ---
>
>  hw/riscv/microchip_pfsoc.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/hw/riscv/microchip_pfsoc.c b/hw/riscv/microchip_pfsoc.c
> index 69117c6000..b9c2f73e7c 100644
> --- a/hw/riscv/microchip_pfsoc.c
> +++ b/hw/riscv/microchip_pfsoc.c
> @@ -158,6 +158,7 @@ static void microchip_pfsoc_soc_realize(DeviceState *dev, Error **errp)
>      MicrochipPFSoCState *s = MICROCHIP_PFSOC(dev);
>      const struct MemmapEntry *memmap = microchip_pfsoc_memmap;
>      MemoryRegion *system_memory = get_system_memory();
> +    MemoryRegion *debug_mem = g_new(MemoryRegion, 1);
>      MemoryRegion *e51_dtim_mem = g_new(MemoryRegion, 1);
>      MemoryRegion *l2lim_mem = g_new(MemoryRegion, 1);
>      MemoryRegion *envm_data = g_new(MemoryRegion, 1);
> @@ -177,6 +178,13 @@ static void microchip_pfsoc_soc_realize(DeviceState *dev, Error **errp)
>      qdev_realize(DEVICE(&s->e_cluster), NULL, &error_abort);
>      qdev_realize(DEVICE(&s->u_cluster), NULL, &error_abort);
>
> +    /* Debug */

This doesn't seem right.

Debug should start at 0x0000_0100 (it seems like what is currently in
tree is wrong).

Address 0x00 is marked as reserved in the documentation.

Do you mind updating the memory map to fix the debug address? Then
just add a reserved region as well.

Alistair

> +    memory_region_init_ram(debug_mem, NULL, "microchip.pfsoc.debug_mem",
> +                           memmap[MICROCHIP_PFSOC_DEBUG].size, &error_fatal);
> +    memory_region_add_subregion(system_memory,
> +                                memmap[MICROCHIP_PFSOC_DEBUG].base,
> +                                debug_mem);
> +
>      /* E51 DTIM */
>      memory_region_init_ram(e51_dtim_mem, NULL, "microchip.pfsoc.e51_dtim_mem",
>                             memmap[MICROCHIP_PFSOC_E51_DTIM].size, &error_fatal);
> --
> 2.25.1
>
>
Alistair Francis Oct. 27, 2020, 5:37 p.m. UTC | #2
On Tue, Oct 27, 2020 at 7:46 AM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> From: Bin Meng <bin.meng@windriver.com>
>
> Connect DDR SGMII PHY module and CFG module to the PolarFire SoC.
>
> Signed-off-by: Bin Meng <bin.meng@windriver.com>
> ---
>
>  hw/riscv/Kconfig                   |  1 +
>  hw/riscv/microchip_pfsoc.c         | 18 ++++++++++++++++++
>  include/hw/riscv/microchip_pfsoc.h |  5 +++++
>  3 files changed, 24 insertions(+)
>
> diff --git a/hw/riscv/Kconfig b/hw/riscv/Kconfig
> index 2df978fe8d..c8e50bde99 100644
> --- a/hw/riscv/Kconfig
> +++ b/hw/riscv/Kconfig
> @@ -4,6 +4,7 @@ config IBEX
>  config MICROCHIP_PFSOC
>      bool
>      select CADENCE_SDHCI
> +    select MCHP_PFSOC_DMC
>      select MCHP_PFSOC_MMUART
>      select MSI_NONBROKEN
>      select SIFIVE_CLINT
> diff --git a/hw/riscv/microchip_pfsoc.c b/hw/riscv/microchip_pfsoc.c
> index 4627179cd3..85be2bcde8 100644
> --- a/hw/riscv/microchip_pfsoc.c
> +++ b/hw/riscv/microchip_pfsoc.c
> @@ -15,6 +15,7 @@
>   * 4) Cadence eMMC/SDHC controller and an SD card connected to it
>   * 5) SiFive Platform DMA (Direct Memory Access Controller)
>   * 6) GEM (Gigabit Ethernet MAC Controller)
> + * 7) DMC (DDR Memory Controller)
>   *
>   * This board currently generates devicetree dynamically that indicates at least
>   * two harts and up to five harts.
> @@ -85,7 +86,9 @@ static const struct MemmapEntry {
>      [MICROCHIP_PFSOC_MMUART0] =         { 0x20000000,     0x1000 },
>      [MICROCHIP_PFSOC_SYSREG] =          { 0x20002000,     0x2000 },
>      [MICROCHIP_PFSOC_MPUCFG] =          { 0x20005000,     0x1000 },
> +    [MICROCHIP_PFSOC_DDR_SGMII_PHY] =   { 0x20007000,     0x1000 },
>      [MICROCHIP_PFSOC_EMMC_SD] =         { 0x20008000,     0x1000 },
> +    [MICROCHIP_PFSOC_DDR_CFG] =         { 0x20080000,    0x40000 },

Neither of these are documented....

Maybe just add a single comment above the memory layout clarifying
that this is not what is documented from the SoC but is instead based
on what guests do?

It seems to be a constant problem with this board, unless I am really
misreading the memory map.

Alistair

>      [MICROCHIP_PFSOC_MMUART1] =         { 0x20100000,     0x1000 },
>      [MICROCHIP_PFSOC_MMUART2] =         { 0x20102000,     0x1000 },
>      [MICROCHIP_PFSOC_MMUART3] =         { 0x20104000,     0x1000 },
> @@ -131,6 +134,11 @@ static void microchip_pfsoc_soc_instance_init(Object *obj)
>      object_initialize_child(obj, "dma-controller", &s->dma,
>                              TYPE_SIFIVE_PDMA);
>
> +    object_initialize_child(obj, "ddr-sgmii-phy", &s->ddr_sgmii_phy,
> +                            TYPE_MCHP_PFSOC_DDR_SGMII_PHY);
> +    object_initialize_child(obj, "ddr-cfg", &s->ddr_cfg,
> +                            TYPE_MCHP_PFSOC_DDR_CFG);
> +
>      object_initialize_child(obj, "gem0", &s->gem0, TYPE_CADENCE_GEM);
>      object_initialize_child(obj, "gem1", &s->gem1, TYPE_CADENCE_GEM);
>
> @@ -260,6 +268,16 @@ static void microchip_pfsoc_soc_realize(DeviceState *dev, Error **errp)
>          memmap[MICROCHIP_PFSOC_MPUCFG].base,
>          memmap[MICROCHIP_PFSOC_MPUCFG].size);
>
> +    /* DDR SGMII PHY */
> +    sysbus_realize(SYS_BUS_DEVICE(&s->ddr_sgmii_phy), errp);
> +    sysbus_mmio_map(SYS_BUS_DEVICE(&s->ddr_sgmii_phy), 0,
> +                    memmap[MICROCHIP_PFSOC_DDR_SGMII_PHY].base);
> +
> +    /* DDR CFG */
> +    sysbus_realize(SYS_BUS_DEVICE(&s->ddr_cfg), errp);
> +    sysbus_mmio_map(SYS_BUS_DEVICE(&s->ddr_cfg), 0,
> +                    memmap[MICROCHIP_PFSOC_DDR_CFG].base);
> +
>      /* SDHCI */
>      sysbus_realize(SYS_BUS_DEVICE(&s->sdhci), errp);
>      sysbus_mmio_map(SYS_BUS_DEVICE(&s->sdhci), 0,
> diff --git a/include/hw/riscv/microchip_pfsoc.h b/include/hw/riscv/microchip_pfsoc.h
> index 8bfc7e1a85..5b81e26241 100644
> --- a/include/hw/riscv/microchip_pfsoc.h
> +++ b/include/hw/riscv/microchip_pfsoc.h
> @@ -24,6 +24,7 @@
>
>  #include "hw/char/mchp_pfsoc_mmuart.h"
>  #include "hw/dma/sifive_pdma.h"
> +#include "hw/misc/mchp_pfsoc_dmc.h"
>  #include "hw/net/cadence_gem.h"
>  #include "hw/sd/cadence_sdhci.h"
>
> @@ -37,6 +38,8 @@ typedef struct MicrochipPFSoCState {
>      RISCVHartArrayState e_cpus;
>      RISCVHartArrayState u_cpus;
>      DeviceState *plic;
> +    MchpPfSoCDdrSgmiiPhyState ddr_sgmii_phy;
> +    MchpPfSoCDdrCfgState ddr_cfg;
>      MchpPfSoCMMUartState *serial0;
>      MchpPfSoCMMUartState *serial1;
>      MchpPfSoCMMUartState *serial2;
> @@ -82,7 +85,9 @@ enum {
>      MICROCHIP_PFSOC_MMUART0,
>      MICROCHIP_PFSOC_SYSREG,
>      MICROCHIP_PFSOC_MPUCFG,
> +    MICROCHIP_PFSOC_DDR_SGMII_PHY,
>      MICROCHIP_PFSOC_EMMC_SD,
> +    MICROCHIP_PFSOC_DDR_CFG,
>      MICROCHIP_PFSOC_MMUART1,
>      MICROCHIP_PFSOC_MMUART2,
>      MICROCHIP_PFSOC_MMUART3,
> --
> 2.25.1
>
>
Alistair Francis Oct. 27, 2020, 8:48 p.m. UTC | #3
On Tue, Oct 27, 2020 at 7:50 AM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> From: Bin Meng <bin.meng@windriver.com>
>
> This creates a model for PolarFire SoC IOSCB [1] module. It actually
> contains lots of sub-modules like various PLLs to control different
> peripherals. Only the mininum capabilities are emulated to make the
> HSS DDR memory initialization codes happy. Lots of sub-modules are
> created as an unimplemented devices.
>
> [1] PF_SoC_RegMap_V1_1/MPFS250T/mpfs250t_ioscb_memmap_dri.htm in
>     https://www.microsemi.com/document-portal/doc_download/1244581-polarfire-soc-register-map
>
> Signed-off-by: Bin Meng <bin.meng@windriver.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>
>  MAINTAINERS                        |   2 +
>  hw/misc/Kconfig                    |   3 +
>  hw/misc/mchp_pfsoc_ioscb.c         | 242 +++++++++++++++++++++++++++++
>  hw/misc/meson.build                |   1 +
>  include/hw/misc/mchp_pfsoc_ioscb.h |  50 ++++++
>  5 files changed, 298 insertions(+)
>  create mode 100644 hw/misc/mchp_pfsoc_ioscb.c
>  create mode 100644 include/hw/misc/mchp_pfsoc_ioscb.h
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index caacec401c..ebbc62a62f 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1327,9 +1327,11 @@ S: Supported
>  F: hw/riscv/microchip_pfsoc.c
>  F: hw/char/mchp_pfsoc_mmuart.c
>  F: hw/misc/mchp_pfsoc_dmc.c
> +F: hw/misc/mchp_pfsoc_ioscb.c
>  F: include/hw/riscv/microchip_pfsoc.h
>  F: include/hw/char/mchp_pfsoc_mmuart.h
>  F: include/hw/misc/mchp_pfsoc_dmc.h
> +F: include/hw/misc/mchp_pfsoc_ioscb.h
>
>  RX Machines
>  -----------
> diff --git a/hw/misc/Kconfig b/hw/misc/Kconfig
> index 198bb1c6ba..3db15e06b4 100644
> --- a/hw/misc/Kconfig
> +++ b/hw/misc/Kconfig
> @@ -137,6 +137,9 @@ config AVR_POWER
>  config MCHP_PFSOC_DMC
>      bool
>
> +config MCHP_PFSOC_IOSCB
> +    bool
> +
>  config SIFIVE_TEST
>      bool
>
> diff --git a/hw/misc/mchp_pfsoc_ioscb.c b/hw/misc/mchp_pfsoc_ioscb.c
> new file mode 100644
> index 0000000000..32b88f53c1
> --- /dev/null
> +++ b/hw/misc/mchp_pfsoc_ioscb.c
> @@ -0,0 +1,242 @@
> +/*
> + * Microchip PolarFire SoC IOSCB module emulation
> + *
> + * Copyright (c) 2020 Wind River Systems, Inc.
> + *
> + * Author:
> + *   Bin Meng <bin.meng@windriver.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 or
> + * (at your option) version 3 of the License.
> + *
> + * 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 "qemu/bitops.h"
> +#include "qemu/log.h"
> +#include "qapi/error.h"
> +#include "hw/hw.h"
> +#include "hw/sysbus.h"
> +#include "hw/misc/mchp_pfsoc_ioscb.h"
> +
> +/*
> + * The whole IOSCB module registers map into the system address at 0x3000_0000,
> + * named as "System Port 0 (AXI-D0)".
> + */
> +#define IOSCB_WHOLE_REG_SIZE        0x10000000
> +#define IOSCB_SUBMOD_REG_SIZE       0x1000
> +
> +/*
> + * There are many sub-modules in the IOSCB module.
> + * See Microchip PolarFire SoC documentation (Register_Map.zip),
> + * Register Map/PF_SoC_RegMap_V1_1/MPFS250T/mpfs250t_ioscb_memmap_dri.htm
> + *
> + * The following are sub-modules offsets that are of concern.
> + */
> +#define IOSCB_LANE01_BASE           0x06500000
> +#define IOSCB_LANE23_BASE           0x06510000
> +#define IOSCB_CTRL_BASE             0x07020000
> +#define IOSCB_CFG_BASE              0x07080000
> +#define IOSCB_PLL_MSS_BASE          0x0E001000
> +#define IOSCB_CFM_MSS_BASE          0x0E002000
> +#define IOSCB_PLL_DDR_BASE          0x0E010000
> +#define IOSCB_BC_DDR_BASE           0x0E020000
> +#define IOSCB_IO_CALIB_DDR_BASE     0x0E040000
> +#define IOSCB_PLL_SGMII_BASE        0x0E080000
> +#define IOSCB_DLL_SGMII_BASE        0x0E100000
> +#define IOSCB_CFM_SGMII_BASE        0x0E200000
> +#define IOSCB_BC_SGMII_BASE         0x0E400000
> +#define IOSCB_IO_CALIB_SGMII_BASE   0x0E800000
> +
> +static uint64_t mchp_pfsoc_dummy_read(void *opaque, hwaddr offset,
> +                                      unsigned size)
> +{
> +    qemu_log_mask(LOG_UNIMP, "%s: unimplemented device read "
> +                  "(size %d, offset 0x%" HWADDR_PRIx ")\n",
> +                  __func__, size, offset);
> +
> +    return 0;
> +}
> +
> +static void mchp_pfsoc_dummy_write(void *opaque, hwaddr offset,
> +                                   uint64_t value, unsigned size)
> +{
> +    qemu_log_mask(LOG_UNIMP, "%s: unimplemented device write "
> +                  "(size %d, value 0x%" PRIx64
> +                  ", offset 0x%" HWADDR_PRIx ")\n",
> +                  __func__, size, value, offset);
> +}
> +
> +static const MemoryRegionOps mchp_pfsoc_dummy_ops = {
> +    .read = mchp_pfsoc_dummy_read,
> +    .write = mchp_pfsoc_dummy_write,
> +    .endianness = DEVICE_LITTLE_ENDIAN,
> +};
> +
> +/* All PLL modules in IOSCB have the same register layout */
> +
> +#define PLL_CTRL    0x04
> +
> +static uint64_t mchp_pfsoc_pll_read(void *opaque, hwaddr offset,
> +                                    unsigned size)
> +{
> +    uint32_t val = 0;
> +
> +    switch (offset) {
> +    case PLL_CTRL:
> +        /* PLL is locked */
> +        val = BIT(25);
> +        break;
> +    default:
> +        qemu_log_mask(LOG_UNIMP, "%s: unimplemented device read "
> +                      "(size %d, offset 0x%" HWADDR_PRIx ")\n",
> +                      __func__, size, offset);
> +        break;
> +    }
> +
> +    return val;
> +}
> +
> +static const MemoryRegionOps mchp_pfsoc_pll_ops = {
> +    .read = mchp_pfsoc_pll_read,
> +    .write = mchp_pfsoc_dummy_write,
> +    .endianness = DEVICE_LITTLE_ENDIAN,
> +};
> +
> +/* IO_CALIB_DDR submodule */
> +
> +#define IO_CALIB_DDR_IOC_REG1   0x08
> +
> +static uint64_t mchp_pfsoc_io_calib_ddr_read(void *opaque, hwaddr offset,
> +                                             unsigned size)
> +{
> +    uint32_t val = 0;
> +
> +    switch (offset) {
> +    case IO_CALIB_DDR_IOC_REG1:
> +        /* calibration completed */
> +        val = BIT(2);
> +        break;
> +    default:
> +        qemu_log_mask(LOG_UNIMP, "%s: unimplemented device read "
> +                      "(size %d, offset 0x%" HWADDR_PRIx ")\n",
> +                      __func__, size, offset);
> +        break;
> +    }
> +
> +    return val;
> +}
> +
> +static const MemoryRegionOps mchp_pfsoc_io_calib_ddr_ops = {
> +    .read = mchp_pfsoc_io_calib_ddr_read,
> +    .write = mchp_pfsoc_dummy_write,
> +    .endianness = DEVICE_LITTLE_ENDIAN,
> +};
> +
> +static void mchp_pfsoc_ioscb_realize(DeviceState *dev, Error **errp)
> +{
> +    MchpPfSoCIoscbState *s = MCHP_PFSOC_IOSCB(dev);
> +    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
> +
> +    memory_region_init(&s->container, OBJECT(s),
> +                       "mchp.pfsoc.ioscb", IOSCB_WHOLE_REG_SIZE);
> +    sysbus_init_mmio(sbd, &s->container);
> +
> +    /* add subregions for all sub-modules in IOSCB */
> +
> +    memory_region_init_io(&s->lane01, OBJECT(s), &mchp_pfsoc_dummy_ops, s,
> +                          "mchp.pfsoc.ioscb.lane01", IOSCB_SUBMOD_REG_SIZE);
> +    memory_region_add_subregion(&s->container, IOSCB_LANE01_BASE, &s->lane01);
> +
> +    memory_region_init_io(&s->lane23, OBJECT(s), &mchp_pfsoc_dummy_ops, s,
> +                          "mchp.pfsoc.ioscb.lane23", IOSCB_SUBMOD_REG_SIZE);
> +    memory_region_add_subregion(&s->container, IOSCB_LANE23_BASE, &s->lane23);
> +
> +    memory_region_init_io(&s->ctrl, OBJECT(s), &mchp_pfsoc_dummy_ops, s,
> +                          "mchp.pfsoc.ioscb.ctrl", IOSCB_SUBMOD_REG_SIZE);
> +    memory_region_add_subregion(&s->container, IOSCB_CTRL_BASE, &s->ctrl);
> +
> +    memory_region_init_io(&s->cfg, OBJECT(s), &mchp_pfsoc_dummy_ops, s,
> +                          "mchp.pfsoc.ioscb.cfg", IOSCB_SUBMOD_REG_SIZE);
> +    memory_region_add_subregion(&s->container, IOSCB_CFG_BASE, &s->cfg);
> +
> +    memory_region_init_io(&s->pll_mss, OBJECT(s), &mchp_pfsoc_pll_ops, s,
> +                          "mchp.pfsoc.ioscb.pll_mss", IOSCB_SUBMOD_REG_SIZE);
> +    memory_region_add_subregion(&s->container, IOSCB_PLL_MSS_BASE, &s->pll_mss);
> +
> +    memory_region_init_io(&s->cfm_mss, OBJECT(s), &mchp_pfsoc_dummy_ops, s,
> +                          "mchp.pfsoc.ioscb.cfm_mss", IOSCB_SUBMOD_REG_SIZE);
> +    memory_region_add_subregion(&s->container, IOSCB_CFM_MSS_BASE, &s->cfm_mss);
> +
> +    memory_region_init_io(&s->pll_ddr, OBJECT(s), &mchp_pfsoc_pll_ops, s,
> +                          "mchp.pfsoc.ioscb.pll_ddr", IOSCB_SUBMOD_REG_SIZE);
> +    memory_region_add_subregion(&s->container, IOSCB_PLL_DDR_BASE, &s->pll_ddr);
> +
> +    memory_region_init_io(&s->bc_ddr, OBJECT(s), &mchp_pfsoc_dummy_ops, s,
> +                          "mchp.pfsoc.ioscb.bc_ddr", IOSCB_SUBMOD_REG_SIZE);
> +    memory_region_add_subregion(&s->container, IOSCB_BC_DDR_BASE, &s->bc_ddr);
> +
> +    memory_region_init_io(&s->io_calib_ddr, OBJECT(s),
> +                          &mchp_pfsoc_io_calib_ddr_ops, s,
> +                          "mchp.pfsoc.ioscb.io_calib_ddr",
> +                          IOSCB_SUBMOD_REG_SIZE);
> +    memory_region_add_subregion(&s->container, IOSCB_IO_CALIB_DDR_BASE,
> +                                &s->io_calib_ddr);
> +
> +    memory_region_init_io(&s->pll_sgmii, OBJECT(s), &mchp_pfsoc_pll_ops, s,
> +                          "mchp.pfsoc.ioscb.pll_sgmii", IOSCB_SUBMOD_REG_SIZE);
> +    memory_region_add_subregion(&s->container, IOSCB_PLL_SGMII_BASE,
> +                                &s->pll_sgmii);
> +
> +    memory_region_init_io(&s->dll_sgmii, OBJECT(s), &mchp_pfsoc_dummy_ops, s,
> +                          "mchp.pfsoc.ioscb.dll_sgmii", IOSCB_SUBMOD_REG_SIZE);
> +    memory_region_add_subregion(&s->container, IOSCB_DLL_SGMII_BASE,
> +                                &s->dll_sgmii);
> +
> +    memory_region_init_io(&s->cfm_sgmii, OBJECT(s), &mchp_pfsoc_dummy_ops, s,
> +                          "mchp.pfsoc.ioscb.cfm_sgmii", IOSCB_SUBMOD_REG_SIZE);
> +    memory_region_add_subregion(&s->container, IOSCB_CFM_SGMII_BASE,
> +                                &s->cfm_sgmii);
> +
> +    memory_region_init_io(&s->bc_sgmii, OBJECT(s), &mchp_pfsoc_dummy_ops, s,
> +                          "mchp.pfsoc.ioscb.bc_sgmii", IOSCB_SUBMOD_REG_SIZE);
> +    memory_region_add_subregion(&s->container, IOSCB_BC_SGMII_BASE,
> +                                &s->bc_sgmii);
> +
> +    memory_region_init_io(&s->io_calib_sgmii, OBJECT(s), &mchp_pfsoc_dummy_ops,
> +                          s, "mchp.pfsoc.ioscb.io_calib_sgmii",
> +                          IOSCB_SUBMOD_REG_SIZE);
> +    memory_region_add_subregion(&s->container, IOSCB_IO_CALIB_SGMII_BASE,
> +                                &s->io_calib_sgmii);
> +}
> +
> +static void mchp_pfsoc_ioscb_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +
> +    dc->desc = "Microchip PolarFire SoC IOSCB";
> +    dc->realize = mchp_pfsoc_ioscb_realize;
> +}
> +
> +static const TypeInfo mchp_pfsoc_ioscb_info = {
> +    .name          = TYPE_MCHP_PFSOC_IOSCB,
> +    .parent        = TYPE_SYS_BUS_DEVICE,
> +    .instance_size = sizeof(MchpPfSoCIoscbState),
> +    .class_init    = mchp_pfsoc_ioscb_class_init,
> +};
> +
> +static void mchp_pfsoc_ioscb_register_types(void)
> +{
> +    type_register_static(&mchp_pfsoc_ioscb_info);
> +}
> +
> +type_init(mchp_pfsoc_ioscb_register_types)
> diff --git a/hw/misc/meson.build b/hw/misc/meson.build
> index 2d79a657e0..6d3c1a3455 100644
> --- a/hw/misc/meson.build
> +++ b/hw/misc/meson.build
> @@ -23,6 +23,7 @@ softmmu_ss.add(when: 'CONFIG_MOS6522', if_true: files('mos6522.c'))
>
>  # RISC-V devices
>  softmmu_ss.add(when: 'CONFIG_MCHP_PFSOC_DMC', if_true: files('mchp_pfsoc_dmc.c'))
> +softmmu_ss.add(when: 'CONFIG_MCHP_PFSOC_IOSCB', if_true: files('mchp_pfsoc_ioscb.c'))
>  softmmu_ss.add(when: 'CONFIG_SIFIVE_TEST', if_true: files('sifive_test.c'))
>  softmmu_ss.add(when: 'CONFIG_SIFIVE_E_PRCI', if_true: files('sifive_e_prci.c'))
>  softmmu_ss.add(when: 'CONFIG_SIFIVE_U_OTP', if_true: files('sifive_u_otp.c'))
> diff --git a/include/hw/misc/mchp_pfsoc_ioscb.h b/include/hw/misc/mchp_pfsoc_ioscb.h
> new file mode 100644
> index 0000000000..9235523e33
> --- /dev/null
> +++ b/include/hw/misc/mchp_pfsoc_ioscb.h
> @@ -0,0 +1,50 @@
> +/*
> + * Microchip PolarFire SoC IOSCB module emulation
> + *
> + * Copyright (c) 2020 Wind River Systems, Inc.
> + *
> + * Author:
> + *   Bin Meng <bin.meng@windriver.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 or
> + * (at your option) version 3 of the License.
> + *
> + * 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 MCHP_PFSOC_IOSCB_H
> +#define MCHP_PFSOC_IOSCB_H
> +
> +typedef struct MchpPfSoCIoscbState {
> +    SysBusDevice parent;
> +    MemoryRegion container;
> +    MemoryRegion lane01;
> +    MemoryRegion lane23;
> +    MemoryRegion ctrl;
> +    MemoryRegion cfg;
> +    MemoryRegion pll_mss;
> +    MemoryRegion cfm_mss;
> +    MemoryRegion pll_ddr;
> +    MemoryRegion bc_ddr;
> +    MemoryRegion io_calib_ddr;
> +    MemoryRegion pll_sgmii;
> +    MemoryRegion dll_sgmii;
> +    MemoryRegion cfm_sgmii;
> +    MemoryRegion bc_sgmii;
> +    MemoryRegion io_calib_sgmii;
> +} MchpPfSoCIoscbState;
> +
> +#define TYPE_MCHP_PFSOC_IOSCB "mchp.pfsoc.ioscb"
> +
> +#define MCHP_PFSOC_IOSCB(obj) \
> +    OBJECT_CHECK(MchpPfSoCIoscbState, (obj), TYPE_MCHP_PFSOC_IOSCB)
> +
> +#endif /* MCHP_PFSOC_IOSCB_H */
> --
> 2.25.1
>
>
Alistair Francis Oct. 27, 2020, 8:55 p.m. UTC | #4
On Tue, Oct 27, 2020 at 7:48 AM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> From: Bin Meng <bin.meng@windriver.com>
>
> When system memory is larger than 1 GiB (high memory), PolarFire SoC
> maps it at address 0x10_0000_0000. Address 0xC000_0000 and above is
> aliased to the same 1 GiB low memory with different cache attributes.
>
> At present QEMU maps the system memory contiguously from 0x8000_0000.
> This corrects the wrong QEMU logic. Note address 0x14_0000_0000 is
> the alias to the high memory, and even physical memory is only 1 GiB,
> the HSS codes still tries to probe the high memory alias address.
> It seems there is no issue on the real hardware, so we will have to
> take that into the consideration in our emulation.
>
> Signed-off-by: Bin Meng <bin.meng@windriver.com>
> ---
>
>  hw/riscv/microchip_pfsoc.c         | 49 +++++++++++++++++++++++++++---
>  include/hw/riscv/microchip_pfsoc.h |  5 ++-
>  2 files changed, 48 insertions(+), 6 deletions(-)
>
> diff --git a/hw/riscv/microchip_pfsoc.c b/hw/riscv/microchip_pfsoc.c
> index b9c2f73e7c..c595c9c967 100644
> --- a/hw/riscv/microchip_pfsoc.c
> +++ b/hw/riscv/microchip_pfsoc.c
> @@ -102,7 +102,10 @@ static const struct MemmapEntry {
>      [MICROCHIP_PFSOC_ENVM_CFG] =        { 0x20200000,     0x1000 },
>      [MICROCHIP_PFSOC_ENVM_DATA] =       { 0x20220000,    0x20000 },
>      [MICROCHIP_PFSOC_IOSCB] =           { 0x30000000, 0x10000000 },
> -    [MICROCHIP_PFSOC_DRAM] =            { 0x80000000,        0x0 },
> +    [MICROCHIP_PFSOC_DRAM_LO] =         { 0x80000000, 0x40000000 },
> +    [MICROCHIP_PFSOC_DRAM_LO_ALIAS] =   { 0xc0000000, 0x40000000 },
> +    [MICROCHIP_PFSOC_DRAM_HI] =       { 0x1000000000,        0x0 },
> +    [MICROCHIP_PFSOC_DRAM_HI_ALIAS] = { 0x1400000000,        0x0 },
>  };
>
>  static void microchip_pfsoc_soc_instance_init(Object *obj)
> @@ -405,7 +408,11 @@ static void microchip_icicle_kit_machine_init(MachineState *machine)
>      const struct MemmapEntry *memmap = microchip_pfsoc_memmap;
>      MicrochipIcicleKitState *s = MICROCHIP_ICICLE_KIT_MACHINE(machine);
>      MemoryRegion *system_memory = get_system_memory();
> -    MemoryRegion *main_mem = g_new(MemoryRegion, 1);
> +    MemoryRegion *mem_low = g_new(MemoryRegion, 1);
> +    MemoryRegion *mem_low_alias = g_new(MemoryRegion, 1);
> +    MemoryRegion *mem_high = g_new(MemoryRegion, 1);
> +    MemoryRegion *mem_high_alias = g_new(MemoryRegion, 1);
> +    uint64_t mem_high_size;
>      DriveInfo *dinfo = drive_get_next(IF_SD);
>
>      /* Sanity check on RAM size */
> @@ -422,10 +429,42 @@ static void microchip_icicle_kit_machine_init(MachineState *machine)
>      qdev_realize(DEVICE(&s->soc), NULL, &error_abort);
>
>      /* Register RAM */
> -    memory_region_init_ram(main_mem, NULL, "microchip.icicle.kit.ram",
> -                           machine->ram_size, &error_fatal);
> +    memory_region_init_ram(mem_low, NULL, "microchip.icicle.kit.ram_low",
> +                           memmap[MICROCHIP_PFSOC_DRAM_LO].size,
> +                           &error_fatal);
> +    memory_region_init_alias(mem_low_alias, NULL,
> +                             "microchip.icicle.kit.ram_low.alias",
> +                             mem_low, 0,
> +                             memmap[MICROCHIP_PFSOC_DRAM_LO_ALIAS].size);
> +    memory_region_add_subregion(system_memory,
> +                                memmap[MICROCHIP_PFSOC_DRAM_LO].base,
> +                                mem_low);
> +    memory_region_add_subregion(system_memory,
> +                                memmap[MICROCHIP_PFSOC_DRAM_LO_ALIAS].base,
> +                                mem_low_alias);
> +
> +    /*
> +     * Map 1 GiB high memory because HSS will do memory test against the high
> +     * memory address range regardless of physical memory installed.
> +     *
> +     * See memory_tests() in mss_ddr.c in the HSS source code.
> +     */
> +    mem_high_size = machine->ram_size - 1 * GiB;
> +    if (mem_high_size < 1 * GiB) {
> +        mem_high_size = 1 * GiB;

This now means that the machine requires at least 2GB of memory!

Can you increase the default_ram_size so we will return an error if
the user specified less than 2GB instead of just increasing it without
their knowledge?

Alistair

> +    }
> +
> +    memory_region_init_ram(mem_high, NULL, "microchip.icicle.kit.ram_high",
> +                           mem_high_size, &error_fatal);
> +    memory_region_init_alias(mem_high_alias, NULL,
> +                             "microchip.icicle.kit.ram_high.alias",
> +                             mem_high, 0, mem_high_size);
> +    memory_region_add_subregion(system_memory,
> +                                memmap[MICROCHIP_PFSOC_DRAM_HI].base,
> +                                mem_high);
>      memory_region_add_subregion(system_memory,
> -                                memmap[MICROCHIP_PFSOC_DRAM].base, main_mem);
> +                                memmap[MICROCHIP_PFSOC_DRAM_HI_ALIAS].base,
> +                                mem_high_alias);
>
>      /* Load the firmware */
>      riscv_find_and_load_firmware(machine, BIOS_FILENAME, RESET_VECTOR, NULL);
> diff --git a/include/hw/riscv/microchip_pfsoc.h b/include/hw/riscv/microchip_pfsoc.h
> index 245c82db61..dc05688d94 100644
> --- a/include/hw/riscv/microchip_pfsoc.h
> +++ b/include/hw/riscv/microchip_pfsoc.h
> @@ -104,7 +104,10 @@ enum {
>      MICROCHIP_PFSOC_ENVM_CFG,
>      MICROCHIP_PFSOC_ENVM_DATA,
>      MICROCHIP_PFSOC_IOSCB,
> -    MICROCHIP_PFSOC_DRAM,
> +    MICROCHIP_PFSOC_DRAM_LO,
> +    MICROCHIP_PFSOC_DRAM_LO_ALIAS,
> +    MICROCHIP_PFSOC_DRAM_HI,
> +    MICROCHIP_PFSOC_DRAM_HI_ALIAS
>  };
>
>  enum {
> --
> 2.25.1
>
>
Bin Meng Oct. 28, 2020, 1:43 a.m. UTC | #5
Hi Alistair,

On Wed, Oct 28, 2020 at 1:49 AM Alistair Francis <alistair23@gmail.com> wrote:
>
> On Tue, Oct 27, 2020 at 7:46 AM Bin Meng <bmeng.cn@gmail.com> wrote:
> >
> > From: Bin Meng <bin.meng@windriver.com>
> >
> > Connect DDR SGMII PHY module and CFG module to the PolarFire SoC.
> >
> > Signed-off-by: Bin Meng <bin.meng@windriver.com>
> > ---
> >
> >  hw/riscv/Kconfig                   |  1 +
> >  hw/riscv/microchip_pfsoc.c         | 18 ++++++++++++++++++
> >  include/hw/riscv/microchip_pfsoc.h |  5 +++++
> >  3 files changed, 24 insertions(+)
> >
> > diff --git a/hw/riscv/Kconfig b/hw/riscv/Kconfig
> > index 2df978fe8d..c8e50bde99 100644
> > --- a/hw/riscv/Kconfig
> > +++ b/hw/riscv/Kconfig
> > @@ -4,6 +4,7 @@ config IBEX
> >  config MICROCHIP_PFSOC
> >      bool
> >      select CADENCE_SDHCI
> > +    select MCHP_PFSOC_DMC
> >      select MCHP_PFSOC_MMUART
> >      select MSI_NONBROKEN
> >      select SIFIVE_CLINT
> > diff --git a/hw/riscv/microchip_pfsoc.c b/hw/riscv/microchip_pfsoc.c
> > index 4627179cd3..85be2bcde8 100644
> > --- a/hw/riscv/microchip_pfsoc.c
> > +++ b/hw/riscv/microchip_pfsoc.c
> > @@ -15,6 +15,7 @@
> >   * 4) Cadence eMMC/SDHC controller and an SD card connected to it
> >   * 5) SiFive Platform DMA (Direct Memory Access Controller)
> >   * 6) GEM (Gigabit Ethernet MAC Controller)
> > + * 7) DMC (DDR Memory Controller)
> >   *
> >   * This board currently generates devicetree dynamically that indicates at least
> >   * two harts and up to five harts.
> > @@ -85,7 +86,9 @@ static const struct MemmapEntry {
> >      [MICROCHIP_PFSOC_MMUART0] =         { 0x20000000,     0x1000 },
> >      [MICROCHIP_PFSOC_SYSREG] =          { 0x20002000,     0x2000 },
> >      [MICROCHIP_PFSOC_MPUCFG] =          { 0x20005000,     0x1000 },
> > +    [MICROCHIP_PFSOC_DDR_SGMII_PHY] =   { 0x20007000,     0x1000 },
> >      [MICROCHIP_PFSOC_EMMC_SD] =         { 0x20008000,     0x1000 },
> > +    [MICROCHIP_PFSOC_DDR_CFG] =         { 0x20080000,    0x40000 },
>
> Neither of these are documented....

It's documented in the "Register
Map/PF_SoC_RegMap_V1_1/pfsoc_regmap.htm" in
https://www.microsemi.com/document-portal/doc_download/1244581-polarfire-soc-register-map

>
> Maybe just add a single comment above the memory layout clarifying
> that this is not what is documented from the SoC but is instead based
> on what guests do?
>

I can add a link to the Microchip website that documents the memory
map above the memory layout.

> It seems to be a constant problem with this board, unless I am really
> misreading the memory map.
>

Regards,
Bin
Bin Meng Oct. 28, 2020, 2:06 a.m. UTC | #6
Hi Alistair,

On Wed, Oct 28, 2020 at 5:07 AM Alistair Francis <alistair23@gmail.com> wrote:
>
> On Tue, Oct 27, 2020 at 7:48 AM Bin Meng <bmeng.cn@gmail.com> wrote:
> >
> > From: Bin Meng <bin.meng@windriver.com>
> >
> > When system memory is larger than 1 GiB (high memory), PolarFire SoC
> > maps it at address 0x10_0000_0000. Address 0xC000_0000 and above is
> > aliased to the same 1 GiB low memory with different cache attributes.
> >
> > At present QEMU maps the system memory contiguously from 0x8000_0000.
> > This corrects the wrong QEMU logic. Note address 0x14_0000_0000 is
> > the alias to the high memory, and even physical memory is only 1 GiB,
> > the HSS codes still tries to probe the high memory alias address.
> > It seems there is no issue on the real hardware, so we will have to
> > take that into the consideration in our emulation.
> >
> > Signed-off-by: Bin Meng <bin.meng@windriver.com>
> > ---
> >
> >  hw/riscv/microchip_pfsoc.c         | 49 +++++++++++++++++++++++++++---
> >  include/hw/riscv/microchip_pfsoc.h |  5 ++-
> >  2 files changed, 48 insertions(+), 6 deletions(-)
> >
> > diff --git a/hw/riscv/microchip_pfsoc.c b/hw/riscv/microchip_pfsoc.c
> > index b9c2f73e7c..c595c9c967 100644
> > --- a/hw/riscv/microchip_pfsoc.c
> > +++ b/hw/riscv/microchip_pfsoc.c
> > @@ -102,7 +102,10 @@ static const struct MemmapEntry {
> >      [MICROCHIP_PFSOC_ENVM_CFG] =        { 0x20200000,     0x1000 },
> >      [MICROCHIP_PFSOC_ENVM_DATA] =       { 0x20220000,    0x20000 },
> >      [MICROCHIP_PFSOC_IOSCB] =           { 0x30000000, 0x10000000 },
> > -    [MICROCHIP_PFSOC_DRAM] =            { 0x80000000,        0x0 },
> > +    [MICROCHIP_PFSOC_DRAM_LO] =         { 0x80000000, 0x40000000 },
> > +    [MICROCHIP_PFSOC_DRAM_LO_ALIAS] =   { 0xc0000000, 0x40000000 },
> > +    [MICROCHIP_PFSOC_DRAM_HI] =       { 0x1000000000,        0x0 },
> > +    [MICROCHIP_PFSOC_DRAM_HI_ALIAS] = { 0x1400000000,        0x0 },
> >  };
> >
> >  static void microchip_pfsoc_soc_instance_init(Object *obj)
> > @@ -405,7 +408,11 @@ static void microchip_icicle_kit_machine_init(MachineState *machine)
> >      const struct MemmapEntry *memmap = microchip_pfsoc_memmap;
> >      MicrochipIcicleKitState *s = MICROCHIP_ICICLE_KIT_MACHINE(machine);
> >      MemoryRegion *system_memory = get_system_memory();
> > -    MemoryRegion *main_mem = g_new(MemoryRegion, 1);
> > +    MemoryRegion *mem_low = g_new(MemoryRegion, 1);
> > +    MemoryRegion *mem_low_alias = g_new(MemoryRegion, 1);
> > +    MemoryRegion *mem_high = g_new(MemoryRegion, 1);
> > +    MemoryRegion *mem_high_alias = g_new(MemoryRegion, 1);
> > +    uint64_t mem_high_size;
> >      DriveInfo *dinfo = drive_get_next(IF_SD);
> >
> >      /* Sanity check on RAM size */
> > @@ -422,10 +429,42 @@ static void microchip_icicle_kit_machine_init(MachineState *machine)
> >      qdev_realize(DEVICE(&s->soc), NULL, &error_abort);
> >
> >      /* Register RAM */
> > -    memory_region_init_ram(main_mem, NULL, "microchip.icicle.kit.ram",
> > -                           machine->ram_size, &error_fatal);
> > +    memory_region_init_ram(mem_low, NULL, "microchip.icicle.kit.ram_low",
> > +                           memmap[MICROCHIP_PFSOC_DRAM_LO].size,
> > +                           &error_fatal);
> > +    memory_region_init_alias(mem_low_alias, NULL,
> > +                             "microchip.icicle.kit.ram_low.alias",
> > +                             mem_low, 0,
> > +                             memmap[MICROCHIP_PFSOC_DRAM_LO_ALIAS].size);
> > +    memory_region_add_subregion(system_memory,
> > +                                memmap[MICROCHIP_PFSOC_DRAM_LO].base,
> > +                                mem_low);
> > +    memory_region_add_subregion(system_memory,
> > +                                memmap[MICROCHIP_PFSOC_DRAM_LO_ALIAS].base,
> > +                                mem_low_alias);
> > +
> > +    /*
> > +     * Map 1 GiB high memory because HSS will do memory test against the high
> > +     * memory address range regardless of physical memory installed.
> > +     *
> > +     * See memory_tests() in mss_ddr.c in the HSS source code.
> > +     */
> > +    mem_high_size = machine->ram_size - 1 * GiB;
> > +    if (mem_high_size < 1 * GiB) {
> > +        mem_high_size = 1 * GiB;
>
> This now means that the machine requires at least 2GB of memory!

Yes, unfortunately :(

>
> Can you increase the default_ram_size so we will return an error if
> the user specified less than 2GB instead of just increasing it without
> their knowledge?
>

Sure.

Regards,
Bin
Bin Meng Oct. 28, 2020, 2:08 a.m. UTC | #7
Hi Alistair,

On Wed, Oct 28, 2020 at 1:42 AM Alistair Francis <alistair23@gmail.com> wrote:
>
> On Tue, Oct 27, 2020 at 7:53 AM Bin Meng <bmeng.cn@gmail.com> wrote:
> >
> > From: Bin Meng <bin.meng@windriver.com>
> >
> > Somehow HSS needs to access address 0 [1] for the DDR calibration data
> > which is in the chipset's debug memory. Let's map the debug memory.
> >
> > [1] See the config_copy() calls in various places in ddr_setup() in
> >     the HSS source codes.
>
> Really? This is reserved memory that they just read and write to? That's crazy.

Yes, that's crazy.

>
> If we really need this can you add a comment saying that the
> documentation is wrong (again) and that this needs to be here.
>

I will try to only map 256 bytes to see how that goes.

> >
> > Signed-off-by: Bin Meng <bin.meng@windriver.com>
> > ---
> >
> >  hw/riscv/microchip_pfsoc.c | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> >

Regards,
Bin
Alistair Francis Oct. 28, 2020, 2:13 p.m. UTC | #8
On Tue, Oct 27, 2020 at 6:43 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> Hi Alistair,
>
> On Wed, Oct 28, 2020 at 1:49 AM Alistair Francis <alistair23@gmail.com> wrote:
> >
> > On Tue, Oct 27, 2020 at 7:46 AM Bin Meng <bmeng.cn@gmail.com> wrote:
> > >
> > > From: Bin Meng <bin.meng@windriver.com>
> > >
> > > Connect DDR SGMII PHY module and CFG module to the PolarFire SoC.
> > >
> > > Signed-off-by: Bin Meng <bin.meng@windriver.com>
> > > ---
> > >
> > >  hw/riscv/Kconfig                   |  1 +
> > >  hw/riscv/microchip_pfsoc.c         | 18 ++++++++++++++++++
> > >  include/hw/riscv/microchip_pfsoc.h |  5 +++++
> > >  3 files changed, 24 insertions(+)
> > >
> > > diff --git a/hw/riscv/Kconfig b/hw/riscv/Kconfig
> > > index 2df978fe8d..c8e50bde99 100644
> > > --- a/hw/riscv/Kconfig
> > > +++ b/hw/riscv/Kconfig
> > > @@ -4,6 +4,7 @@ config IBEX
> > >  config MICROCHIP_PFSOC
> > >      bool
> > >      select CADENCE_SDHCI
> > > +    select MCHP_PFSOC_DMC
> > >      select MCHP_PFSOC_MMUART
> > >      select MSI_NONBROKEN
> > >      select SIFIVE_CLINT
> > > diff --git a/hw/riscv/microchip_pfsoc.c b/hw/riscv/microchip_pfsoc.c
> > > index 4627179cd3..85be2bcde8 100644
> > > --- a/hw/riscv/microchip_pfsoc.c
> > > +++ b/hw/riscv/microchip_pfsoc.c
> > > @@ -15,6 +15,7 @@
> > >   * 4) Cadence eMMC/SDHC controller and an SD card connected to it
> > >   * 5) SiFive Platform DMA (Direct Memory Access Controller)
> > >   * 6) GEM (Gigabit Ethernet MAC Controller)
> > > + * 7) DMC (DDR Memory Controller)
> > >   *
> > >   * This board currently generates devicetree dynamically that indicates at least
> > >   * two harts and up to five harts.
> > > @@ -85,7 +86,9 @@ static const struct MemmapEntry {
> > >      [MICROCHIP_PFSOC_MMUART0] =         { 0x20000000,     0x1000 },
> > >      [MICROCHIP_PFSOC_SYSREG] =          { 0x20002000,     0x2000 },
> > >      [MICROCHIP_PFSOC_MPUCFG] =          { 0x20005000,     0x1000 },
> > > +    [MICROCHIP_PFSOC_DDR_SGMII_PHY] =   { 0x20007000,     0x1000 },
> > >      [MICROCHIP_PFSOC_EMMC_SD] =         { 0x20008000,     0x1000 },
> > > +    [MICROCHIP_PFSOC_DDR_CFG] =         { 0x20080000,    0x40000 },
> >
> > Neither of these are documented....
>
> It's documented in the "Register
> Map/PF_SoC_RegMap_V1_1/pfsoc_regmap.htm" in
> https://www.microsemi.com/document-portal/doc_download/1244581-polarfire-soc-register-map

That doesn't seem to be an official version controled doc though and
it seems to conflict with the other UG document.

>
> >
> > Maybe just add a single comment above the memory layout clarifying
> > that this is not what is documented from the SoC but is instead based
> > on what guests do?
> >
>
> I can add a link to the Microchip website that documents the memory
> map above the memory layout.

Thanks, that's at least something.

Alistair

>
> > It seems to be a constant problem with this board, unless I am really
> > misreading the memory map.
> >
>
> Regards,
> Bin