Re: [PATCH 03/26] hw/misc: pfsoc: Support PolarFire SoC DDR training with newer version HSS

On Thu, 2026-07-23 at 23:18 +0800, Bin Meng wrote:
> HSS v2024.06 performs DDR and SGMII initialization through the DMC
> register window before it can complete platform startup. The existing
> model, which was validated against older version HSS in the early
> days,
> exposes only a few fixed training results, so newer version HSS
> stalls
> while waiting for PVT, PLL, DLL, ADDCMD, and verification status.
> 
> Update the DMC model by adding the minimum state and status behavior
> needed by that firmware flow. Model the SGMII control registers,
> main DDR PLL lock, deterministic ADDCMD transitions, and successful
> gate and DQ/DQS verification. Keep training progress and writable
> controls per device and clear them on reset.
> 
> These values are compatibility responses for HSS v2024.06 rather than
> a cycle-accurate description of the DDR PHY. Document that limitation
> in the source.
> 
> While we are here, update the license text to use the SPDX format.
> 
> Note at the time being there is an HSS bug [1] that consistenly
> blocks
> the DDR traning from succeed when using an HSS image built from
> certain
> version compiler like GCC 8.3.0. A patch [2] was proposed and
> verified
> against HSS v2024.06.
> 
> [1] https://github.com/polarfire-soc/platform/issues/36
> [2] https://github.com/processmission/qemu-machine-images/blob/main/
>     machine/riscv64/microchip-icicle-kit/
>     0001-mpfs_hal-preserve-DDR-pattern-test-parameters.patch
> 
> Signed-off-by: Bin Meng <[email protected]>
> ---
> 
>  include/hw/misc/mchp_pfsoc_dmc.h |   6 +
>  hw/misc/mchp_pfsoc_dmc.c         | 272 ++++++++++++++++++++++++++++-
> --
>  2 files changed, 254 insertions(+), 24 deletions(-)
> 
> diff --git a/include/hw/misc/mchp_pfsoc_dmc.h
> b/include/hw/misc/mchp_pfsoc_dmc.h
> index 2ed582fb8c..6a1986c2c2 100644
> --- a/include/hw/misc/mchp_pfsoc_dmc.h
> +++ b/include/hw/misc/mchp_pfsoc_dmc.h
> @@ -28,10 +28,16 @@
>  /* DDR SGMII PHY module */
>  
>  #define MCHP_PFSOC_DDR_SGMII_PHY_REG_SIZE   0x1000
> +#define MCHP_PFSOC_DDR_SGMII_PHY_REG_NUM \
> +    (MCHP_PFSOC_DDR_SGMII_PHY_REG_SIZE / sizeof(uint32_t))
>  
>  typedef struct MchpPfSoCDdrSgmiiPhyState {
>      SysBusDevice parent;
>      MemoryRegion sgmii_phy;
> +    uint32_t regs[MCHP_PFSOC_DDR_SGMII_PHY_REG_NUM];
> +    uint8_t training_status_bit;
> +    uint8_t addcmd_tap;
> +    bool addcmd_move_active;
>  } MchpPfSoCDdrSgmiiPhyState;
>  
>  #define TYPE_MCHP_PFSOC_DDR_SGMII_PHY "mchp.pfsoc.ddr_sgmii_phy"
> diff --git a/hw/misc/mchp_pfsoc_dmc.c b/hw/misc/mchp_pfsoc_dmc.c
> index 1fe4535464..443de5ddd1 100644
> --- a/hw/misc/mchp_pfsoc_dmc.c
> +++ b/hw/misc/mchp_pfsoc_dmc.c
> @@ -2,22 +2,20 @@
>   * Microchip PolarFire SoC DDR Memory Controller module emulation
>   *
>   * Copyright (c) 2020 Wind River Systems, Inc.
> + * Copyright (c) 2026 Process Mission
>   *
>   * Author:
>   *   Bin Meng <[email protected]>
>   *
> - * 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.
> + * Updated by Bin Meng <[email protected]> to support DDR
> + * training emulation for with newer version Hart Software Services,
> + * aka HSS.
>   *
> - * 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.
> + * The DMC register model is based on HSS v2024.06. Its read/write
> + * behavior is the minimum needed for DDR training to complete and
> + * may not reflect actual hardware register behavior.
>   *
> - * You should have received a copy of the GNU General Public License
> along
> - * with this program; if not, see <http://www.gnu.org/licenses/>.
> + * SPDX-License-Identifier: GPL-2.0-or-later
>   */
>  
>  #include "qemu/osdep.h"
> @@ -29,17 +27,56 @@
>  
>  /* DDR SGMII PHY module */
>  
> -#define SGMII_PHY_IOC_REG1              0x208
> -#define SGMII_PHY_TRAINING_STATUS       0x814
> -#define SGMII_PHY_DQ_DQS_ERR_DONE       0x834
> -#define SGMII_PHY_DQDQS_STATUS1         0x84c
> -#define SGMII_PHY_PVT_STAT              0xc20
> +#define PHY_ADDCMD_CK_TRANSITION_TAP              12
> +#define PHY_ADDCMD_A5_TRANSITION_TAP              20
> +#define PHY_ADDCMD_CK_READBACK                    5
> +#define PHY_ADDCMD_A5_READBACK                    (3 << 8)
> +#define PHY_GT_TXDLY_VALUE                        0x01010101
> +#define PHY_DQDQS_STATUS2_VALUE                   5
> +
> +#define SGMII_PHY_PLL_CTRL_MAIN                   0x084
> +#define SGMII_PHY_PLL_CTRL_MAIN_CONTROL_LO_MASK   0x0000007f
> +#define SGMII_PHY_PLL_CTRL_MAIN_LP_REQUIRES_LOCK  BIT(24)
> +#define SGMII_PHY_PLL_CTRL_MAIN_LOCK              BIT(25)
> +#define SGMII_PHY_IOC_REG1                        0x208
> +#define SGMII_PHY_TRAINING_STATUS                 0x814
> +#define SGMII_PHY_GT_ERR_COMB                     0x81c
> +#define SGMII_PHY_GT_CLK_SEL                      0x820
> +#define SGMII_PHY_GT_TXDLY                        0x824
> +#define SGMII_PHY_DQ_DQS_ERR_DONE                 0x834
> +#define SGMII_PHY_DQDQS_STATUS1                   0x84c
> +#define SGMII_PHY_DQDQS_STATUS2                   0x850
> +#define SGMII_PHY_EXPERT_DLYCNT_MOVE_REG1         0x880
> +#define SGMII_PHY_EXPERT_DLYCNT_MOVE_CONTROL_MASK 0x001fffff
> +#define SGMII_PHY_EXPERT_DLYCNT_MOVE_ADDCMD_MASK  0x00180000
> +#define SGMII_PHY_EXPERT_DLYCNT_LOAD_REG1         0x890
> +#define SGMII_PHY_EXPERT_DLYCNT_LOAD_CONTROL_MASK 0x001fffff
> +#define SGMII_PHY_EXPERT_DLYCNT_LOAD_ADDCMD_MASK  0x00180000
> +#define SGMII_PHY_EXPERT_ADDCMD_LN_READBACK       0x8ac
> +#define SGMII_PHY_SOFT_RESET_SGMII                0xc00
> +#define SGMII_PHY_SGMII_MODE                      0xc04
> +#define SGMII_PHY_PLL_CNTL                        0xc08
> +#define SGMII_PHY_PLL_CNTL_LOCK                   BIT(7)
> +#define SGMII_PHY_CH0_CNTL                        0xc0c
> +#define SGMII_PHY_CH1_CNTL                        0xc10
> +#define SGMII_PHY_RECAL_CNTL                      0xc14
> +#define SGMII_PHY_RECAL_CNTL_STATUS_MASK          0xffff0000
> +#define SGMII_PHY_RECAL_CNTL_LOCK                 BIT(23)
> +#define SGMII_PHY_CLK_CNTL                        0xc18
> +#define SGMII_PHY_DYN_CNTL                        0xc1c
> +#define SGMII_PHY_PVT_STAT                        0xc20
> +#define SGMII_PHY_PVT_STAT_IO_ENABLE              BIT(6)
> +#define SGMII_PHY_PVT_STAT_CALIBRATED             BIT(14)
> +#define SGMII_PHY_PVT_STAT_GUEST_CTRL_MASK        0xc0000000
> +#define SGMII_PHY_SPARE_CNTL                      0xc24
> +#define SGMII_PHY_SPARE_STAT                      0xc28
>  
>  static uint64_t mchp_pfsoc_ddr_sgmii_phy_read(void *opaque, hwaddr
> offset,
>                                                unsigned size)
>  {
>      uint32_t val = 0;
> -    static int training_status_bit;
> +    MchpPfSoCDdrSgmiiPhyState *s = opaque;
> +    uint32_t index = offset / sizeof(uint32_t);
>  
>      switch (offset) {
>      case SGMII_PHY_IOC_REG1:
> @@ -53,8 +90,8 @@ static uint64_t mchp_pfsoc_ddr_sgmii_phy_read(void
> *opaque, hwaddr offset,
>           *
>           * See ddr_setup() in mss_ddr.c in the HSS source codes.
>           */
> -        val = 1 << training_status_bit;
> -        training_status_bit = (training_status_bit + 1) % 5;
> +        val = BIT(s->training_status_bit);
> +        s->training_status_bit = (s->training_status_bit + 1) % 5;
>          break;
>      case SGMII_PHY_DQ_DQS_ERR_DONE:
>          /*
> @@ -71,8 +108,106 @@ static uint64_t
> mchp_pfsoc_ddr_sgmii_phy_read(void *opaque, hwaddr offset,
>          val = 0xff;
>          break;
>      case SGMII_PHY_PVT_STAT:
> -        /* See sgmii_channel_setup() in HSS */
> -        val = BIT(14) | BIT(6);
> +        /*
> +         * HSS polls IO enable and calibration status, then writes
> the
> +         * calibration lock. The HSS register definitions mark bits
> 31:30
> +         * as writable controls.
> +         *
> +         * See sgmii_channel_setup() in mss_sgmii.c in HSS.
> +         */
> +        val = s->regs[index] | SGMII_PHY_PVT_STAT_IO_ENABLE |
> +              SGMII_PHY_PVT_STAT_CALIBRATED;
> +        break;
> +    case SGMII_PHY_PLL_CTRL_MAIN:
> +        /*
> +         * HSS programs the main DDR PLL controls and polls LOCK
> after
> +         * changing the PLL dividers during LPDDR4 manual training.
> The
> +         * HSS register definitions mark bits 6:0 and 24 as
> writable.
> +         *
> +         * See ddr_pll_config() in mss_pll.c and
> +         * lpddr4_manual_training() in mss_ddr.c in HSS.
> +         */
> +        val = s->regs[index] | SGMII_PHY_PLL_CTRL_MAIN_LOCK;
> +        break;
> +    case SGMII_PHY_PLL_CNTL:
> +        /*
> +         * HSS programs the SGMII PLL controls and polls
> aro_pll0_lock.
> +         *
> +         * See setup_sgmii_rpc_per_config() and
> sgmii_channel_setup()
> +         * in mss_sgmii.c in HSS.
> +         */
> +        val = s->regs[index] | SGMII_PHY_PLL_CNTL_LOCK;
> +        break;
> +    case SGMII_PHY_RECAL_CNTL:
> +        /*
> +         * HSS programs the low control half, polls sro_dll_lock,
> and
> +         * reads sro_dll_90_code from the read-only status half.
> +         *
> +         * See setup_sgmii_rpc_per_config() and
> sgmii_channel_setup()
> +         * in mss_sgmii.c in HSS.
> +         */
> +        val = s->regs[index] | SGMII_PHY_RECAL_CNTL_LOCK;
> +        break;
> +    case SGMII_PHY_EXPERT_ADDCMD_LN_READBACK:
> +        /*
> +         * HSS pulses the ADDCMD move controls while sampling CK
> from
> +         * bits 3:0 and A5 from bits 9:8. The transition tap
> positions
> +         * below are deterministic QEMU model choices.
> +         *
> +         * See lpddr4_manual_training() in mss_ddr.c in HSS.
> +         */
> +        if (s->addcmd_tap >= PHY_ADDCMD_CK_TRANSITION_TAP) {
> +            val |= PHY_ADDCMD_CK_READBACK;
> +        }
> +        if (s->addcmd_tap >= PHY_ADDCMD_A5_TRANSITION_TAP) {
> +            val |= PHY_ADDCMD_A5_READBACK;
> +        }
> +        break;
> +    case SGMII_PHY_GT_ERR_COMB:
> +    case SGMII_PHY_GT_CLK_SEL:
> +        /*
> +         * HSS fails DDR verification on any gate error, a zero
> delay
> +         * selected by gt_clk_sel, or more than one zero delay byte.
> +         * Model no gate errors; the all-nonzero delay below makes
> the
> +         * deterministic clock selection immaterial.
> +         *
> +         * See DDR_TRAINING_IP_SM_VERIFY in ddr_setup() in mss_ddr.c
> +         * in HSS.
> +         */
> +        val = 0;
> +        break;
> +    case SGMII_PHY_GT_TXDLY:
> +        val = PHY_GT_TXDLY_VALUE;
> +        break;
> +    case SGMII_PHY_DQDQS_STATUS2:
> +        /*
> +         * HSS requires the calculated DQ/DQS window to be at least
> +         * DQ_DQS_NUM_TAPS, which is 5. Return the minimum pass
> value.
> +         *
> +         * See DDR_TRAINING_IP_SM_VERIFY in ddr_setup() and the
> +         * DQ_DQS_NUM_TAPS definition in mss_ddr.h in HSS.
> +         */
> +        val = PHY_DQDQS_STATUS2_VALUE;
> +        break;
> +    case SGMII_PHY_EXPERT_DLYCNT_MOVE_REG1:
> +    case SGMII_PHY_EXPERT_DLYCNT_LOAD_REG1:
> +    case SGMII_PHY_SOFT_RESET_SGMII:
> +    case SGMII_PHY_SGMII_MODE:
> +    case SGMII_PHY_CH0_CNTL:
> +    case SGMII_PHY_CH1_CNTL:
> +    case SGMII_PHY_CLK_CNTL:
> +    case SGMII_PHY_DYN_CNTL:
> +    case SGMII_PHY_SPARE_CNTL:
> +    case SGMII_PHY_SPARE_STAT:
> +        /*
> +         * HSS programs the ADDCMD and SGMII controls and reads
> +         * SPARE_STAT to classify the silicon and select eye-width
> +         * thresholds.
> +         *
> +         * See lpddr4_manual_training() in mss_ddr.c and the SGMII
> setup
> +         * functions in mss_sgmii.c in HSS.
> +         */
> +        val = s->regs[index];
>          break;
>      default:
>          qemu_log_mask(LOG_UNIMP, "%s: unimplemented device read "
> @@ -87,10 +222,88 @@ static uint64_t
> mchp_pfsoc_ddr_sgmii_phy_read(void *opaque, hwaddr offset,
>  static void mchp_pfsoc_ddr_sgmii_phy_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);
> +    MchpPfSoCDdrSgmiiPhyState *s = opaque;
> +    uint32_t index = offset / sizeof(uint32_t);
> +
> +    switch (offset) {
> +    case SGMII_PHY_IOC_REG1:
> +    case SGMII_PHY_TRAINING_STATUS:
> +    case SGMII_PHY_GT_ERR_COMB:
> +    case SGMII_PHY_GT_CLK_SEL:
> +    case SGMII_PHY_GT_TXDLY:
> +    case SGMII_PHY_DQ_DQS_ERR_DONE:
> +    case SGMII_PHY_DQDQS_STATUS1:
> +    case SGMII_PHY_DQDQS_STATUS2:
> +    case SGMII_PHY_EXPERT_ADDCMD_LN_READBACK:
> +    case SGMII_PHY_SPARE_STAT:
> +        /*
> +         * The HSS register definitions declare these status and
> +         * readback registers read-only.
> +         *
> +         * See mss_ddr_sgmii_phy_defs.h in HSS.
> +         */
> +        return;
> +    case SGMII_PHY_PLL_CTRL_MAIN:
> +        value &= SGMII_PHY_PLL_CTRL_MAIN_CONTROL_LO_MASK |
> +                 SGMII_PHY_PLL_CTRL_MAIN_LP_REQUIRES_LOCK;
> +        break;
> +    case SGMII_PHY_PLL_CNTL:
> +        value &= ~SGMII_PHY_PLL_CNTL_LOCK;
> +        break;
> +    case SGMII_PHY_RECAL_CNTL:
> +        value &= ~SGMII_PHY_RECAL_CNTL_STATUS_MASK;
> +        break;
> +    case SGMII_PHY_PVT_STAT:
> +        value &= SGMII_PHY_PVT_STAT_GUEST_CTRL_MASK;
> +        break;
> +    case SGMII_PHY_EXPERT_DLYCNT_MOVE_REG1: {
> +        bool active;
> +
> +        /*
> +         * HSS pulses 0 -> 0x180000 -> 0 while scanning the ADDCMD
> eye.
> +         * Count each rising pulse as one abstract delay tap.
> +         *
> +         * See lpddr4_manual_training() in mss_ddr.c in HSS.
> +         */
> +        value &= SGMII_PHY_EXPERT_DLYCNT_MOVE_CONTROL_MASK;
> +        active = value & SGMII_PHY_EXPERT_DLYCNT_MOVE_ADDCMD_MASK;
> +        if (active && !s->addcmd_move_active &&
> +            s->addcmd_tap != UINT8_MAX) {
> +            s->addcmd_tap++;
> +        }
> +        s->addcmd_move_active = active;
> +        break;
> +    }
> +    case SGMII_PHY_EXPERT_DLYCNT_LOAD_REG1:
> +        /*
> +         * HSS pulses 0x180000 before each ADDCMD scan. Restart the
> +         * abstract QEMU tap position when that load pulse is
> asserted.
> +         *
> +         * See lpddr4_manual_training() in mss_ddr.c in HSS.
> +         */
> +        value &= SGMII_PHY_EXPERT_DLYCNT_LOAD_CONTROL_MASK;
> +        if (value & SGMII_PHY_EXPERT_DLYCNT_LOAD_ADDCMD_MASK) {
> +            s->addcmd_tap = 0;
> +            s->addcmd_move_active = false;
> +        }
> +        break;
> +    case SGMII_PHY_SOFT_RESET_SGMII:
> +    case SGMII_PHY_SGMII_MODE:
> +    case SGMII_PHY_CH0_CNTL:
> +    case SGMII_PHY_CH1_CNTL:
> +    case SGMII_PHY_CLK_CNTL:
> +    case SGMII_PHY_DYN_CNTL:
> +    case SGMII_PHY_SPARE_CNTL:
> +        break;
> +    default:
> +        qemu_log_mask(LOG_UNIMP, "%s: unimplemented device write "
> +                      "(size %d, value 0x%" PRIx64
> +                      ", offset 0x%" HWADDR_PRIx ")\n",
> +                      __func__, size, value, offset);
> +        return;
> +    }
> +
> +    s->regs[index] = value;
>  }
>  
>  static const MemoryRegionOps mchp_pfsoc_ddr_sgmii_phy_ops = {
> @@ -99,6 +312,16 @@ static const MemoryRegionOps
> mchp_pfsoc_ddr_sgmii_phy_ops = {
>      .endianness = DEVICE_LITTLE_ENDIAN,
>  };
>  
> +static void mchp_pfsoc_ddr_sgmii_phy_reset(DeviceState *dev)
> +{
> +    MchpPfSoCDdrSgmiiPhyState *s = MCHP_PFSOC_DDR_SGMII_PHY(dev);
> +
> +    memset(s->regs, 0, sizeof(s->regs));
> +    s->training_status_bit = 0;
> +    s->addcmd_tap = 0;
> +    s->addcmd_move_active = false;
> +}
> +
>  static void mchp_pfsoc_ddr_sgmii_phy_realize(DeviceState *dev, Error
> **errp)
>  {
>      MchpPfSoCDdrSgmiiPhyState *s = MCHP_PFSOC_DDR_SGMII_PHY(dev);
> @@ -117,6 +340,7 @@ static void
> mchp_pfsoc_ddr_sgmii_phy_class_init(ObjectClass *klass,
>  
>      dc->desc = "Microchip PolarFire SoC DDR SGMII PHY module";
>      dc->realize = mchp_pfsoc_ddr_sgmii_phy_realize;
> +    device_class_set_legacy_reset(dc,
> mchp_pfsoc_ddr_sgmii_phy_reset);

Can we use the non-legacy reset instead

Alistair

>  }
>  
>  static const TypeInfo mchp_pfsoc_ddr_sgmii_phy_info = {