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

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

On Fri, Aug 15, 2025 at 11:01:08AM +0200, 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>
> Reviewed-by: Edgar E. Iglesias <edgar.iglesias@amd.com>


BTW, I just noticed that this patch has style problems:

e8874ea66c (HEAD, beckhoff) hw/misc/zynq_slcr: Add logic for DCI configuration

ERROR: trailing whitespace
#72: FILE: hw/misc/zynq_slcr.c:571:
+        if (!FIELD_EX32(val, DDRIOB_DCI_CTRL, RESET) && $

total: 1 errors, 0 warnings, 61 lines checked



> ---
>  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 a766bab182..8d15f0cc66 100644
> --- a/hw/misc/zynq_slcr.c
> +++ b/hw/misc/zynq_slcr.c
> @@ -180,6 +180,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
> @@ -193,6 +199,8 @@ struct ZynqSLCRState {
>  
>      MemoryRegion iomem;
>  
> +    bool ddriob_dci_ctrl_reset_toggled;
> +
>      uint32_t regs[ZYNQ_SLCR_NUM_REGS];
>  
>      Clock *ps_clk;
> @@ -331,6 +339,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;
> @@ -418,6 +428,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)
> @@ -554,6 +566,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.50.1
>