Re: [PATCH 10/21] hw/misc/zynq_slcr: Add logic for DCI configuration

["Edgar E. Iglesias" <edgar.iglesias@gmail.com>]

On Tue, Mar 18, 2025 at 02:08:01PM +0100, Corvin Köhne wrote:
> From: YannickV <Y.Vossen@beckhoff.com>
> 
> The registers for the digitally controlled impedance (DCI) clock are
> part of the system level control registers (SLCR). The DONE bit in
> the status register indicates a successfull DCI calibration. An
> description of the calibration process can be found here:
> https://docs.amd.com/r/en-US/ug585-zynq-7000-SoC-TRM/DDR-IOB-Impedance-Calibration
> 
> The DCI control register and status register have been added. As soon
> as the ENABLE and RESET bit are set, the RESET bit has also been toggled
> to 0 before and the UPDATE_CONTROL is not set, the DONE bit in the status
> register is set. If these bits change the DONE bit is reset. Note that the
> option bits are not taken into consideration.
> 
> Signed-off-by: Yannick Voßen <y.vossen@beckhoff.com>

LGTM:
Reviewed-by: Edgar E. Iglesias <edgar.iglesias@amd.com>




> ---
>  hw/misc/zynq_slcr.c | 31 +++++++++++++++++++++++++++++++
>  1 file changed, 31 insertions(+)
> 
> diff --git a/hw/misc/zynq_slcr.c b/hw/misc/zynq_slcr.c
> index 9b3220f354..10ef8ecee8 100644
> --- a/hw/misc/zynq_slcr.c
> +++ b/hw/misc/zynq_slcr.c
> @@ -181,6 +181,12 @@ REG32(GPIOB_CFG_HSTL, 0xb14)
>  REG32(GPIOB_DRVR_BIAS_CTRL, 0xb18)
>  
>  REG32(DDRIOB, 0xb40)
> +REG32(DDRIOB_DCI_CTRL, 0xb70)
> +    FIELD(DDRIOB_DCI_CTRL, RESET, 0, 1)
> +    FIELD(DDRIOB_DCI_CTRL, ENABLE, 1, 1)
> +    FIELD(DDRIOB_DCI_CTRL, UPDATE_CONTROL, 20, 1)
> +REG32(DDRIOB_DCI_STATUS, 0xb74)
> +    FIELD(DDRIOB_DCI_STATUS, DONE, 13, 1)
>  #define DDRIOB_LENGTH 14
>  
>  #define ZYNQ_SLCR_MMIO_SIZE     0x1000
> @@ -194,6 +200,8 @@ struct ZynqSLCRState {
>  
>      MemoryRegion iomem;
>  
> +    bool ddriob_dci_ctrl_reset_toggled;
> +
>      uint32_t regs[ZYNQ_SLCR_NUM_REGS];
>  
>      Clock *ps_clk;
> @@ -332,6 +340,8 @@ static void zynq_slcr_reset_init(Object *obj, ResetType type)
>  
>      DB_PRINT("RESET\n");
>  
> +    s->ddriob_dci_ctrl_reset_toggled = false;
> +
>      s->regs[R_LOCKSTA] = 1;
>      /* 0x100 - 0x11C */
>      s->regs[R_ARM_PLL_CTRL]   = 0x0001A008;
> @@ -419,6 +429,8 @@ static void zynq_slcr_reset_init(Object *obj, ResetType type)
>      s->regs[R_DDRIOB + 4] = s->regs[R_DDRIOB + 5] = s->regs[R_DDRIOB + 6]
>                            = 0x00000e00;
>      s->regs[R_DDRIOB + 12] = 0x00000021;
> +
> +    s->regs[R_DDRIOB_DCI_CTRL] = 0x00000020;
>  }
>  
>  static void zynq_slcr_reset_hold(Object *obj, ResetType type)
> @@ -555,6 +567,25 @@ static void zynq_slcr_write(void *opaque, hwaddr offset,
>                  (int)offset, (unsigned)val & 0xFFFF);
>          }
>          return;
> +
> +    case R_DDRIOB_DCI_CTRL:
> +        if (!FIELD_EX32(val, DDRIOB_DCI_CTRL, RESET) && 
> +            FIELD_EX32(s->regs[R_DDRIOB_DCI_CTRL], DDRIOB_DCI_CTRL, RESET)) {
> +
> +            s->ddriob_dci_ctrl_reset_toggled = true;
> +            DB_PRINT("DDRIOB DCI CTRL RESET was toggled\n");
> +        }
> +
> +        if (FIELD_EX32(val, DDRIOB_DCI_CTRL, ENABLE) &&
> +            FIELD_EX32(val, DDRIOB_DCI_CTRL, RESET) &&
> +            !FIELD_EX32(val, DDRIOB_DCI_CTRL, UPDATE_CONTROL) &&
> +            s->ddriob_dci_ctrl_reset_toggled) {
> +
> +            s->regs[R_DDRIOB_DCI_STATUS] |= R_DDRIOB_DCI_STATUS_DONE_MASK;
> +        } else {
> +            s->regs[R_DDRIOB_DCI_STATUS] &= ~R_DDRIOB_DCI_STATUS_DONE_MASK;
> +        }
> +        break;
>      }
>  
>      if (s->regs[R_LOCKSTA]) {
> -- 
> 2.49.0
>