On Fri, 29 May 2026 at 12:47, Peter Maydell <[email protected]> wrote: > > From: YannickV <[email protected]> > > 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.
Hi; we've just had a Coverity Scan run that went over this code, and it points out an issue: > REG32(DDRIOB, 0xb40) > +REG32(DDRIOB_DCI_CTRL, 0xb70) These REG32() macros define R_DDRIOB as 0xb40 / 4 == 0x2d0, and R_DDRIOB_DCI_CTRL as 0xb70 / 4 == 0x2dc... > @@ -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; ...and here in the reset function we are now initializing both s->regs[R_DDRIOB + 12] and s->regs[R_DDRIOB_DCI_CTRL], but those are the same array element (since 0x2d0 + 12 == 0x2dc). What was the intention here? Which reset value is correct? Should we be resetting some other register where we currently are resetting s->regs[R_DDRIOB + 12] ? thanks -- PMM
