Re: [PULL 08/21] hw/misc/zynq_slcr: Add logic for DCI configuration

["Corvin Köhne" <C.Koehne@beckhoff.com>]

[-- Attachment #1: Type: text/plain, Size: 3739 bytes --]

Hi Peter,

sry, for the late response. I've checked the manual [1]. 0xb70 is the right
offset for R_DDRIOB_DCI_CTRL. Additionally, the correct reset value is 0x20 not
0x21. The commit also requires 0x20 as reset value because it tries to detect a
toggle of the reset bit (bit 0). Do you fix the commit or should we do it?
Thanks for catching!

[1]
https://docs.amd.com/r/en-US/ug585-zynq-7000-SoC-TRM/Register-slcr-DDRIOB_DCI_CTRL


-- 
Best regards,
Corvin

On Tue, 2026-07-07 at 11:58 +0100, Peter Maydell wrote:
> Ping -- Corvin, Yannick, please could you suggest the right resolution
> for this bug?
> 
> thanks
> -- PMM
> 
> 
> On Mon, 22 Jun 2026 at 15:49, Peter Maydell <peter.maydell@linaro.org> wrote:
> > 
> > On Fri, 29 May 2026 at 12:47, Peter Maydell <peter.maydell@linaro.org>
> > 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://urldefense.com/v3/__https://docs.amd.com/r/en-US/ug585-zynq-7000-SoC-TRM/DDR-IOB-Impedance-Calibration__;!!PlYMe_wG1g!lthDLWtCQd7a8JMN8G4iIPO8hmKqHg5ON9qsiI2YQGx5fTuThS3u5WwV6IeiZCeJf2gNSKMWSZ2wKu91Kb6gVFdpPQ$
> > > 
> > > 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



This email contains confidential information. If you have received it in error, you must not read, use, copy or pass on this e-mail or its attachments.
If you have received the e-mail in error, please inform me immediately by reply e-mail and then delete this e-mail from your system. Thank you

Diese E-Mail enthält vertrauliche Informationen. Sollten Sie sie irrtümlich erhalten haben, dürfen Sie diese E-Mail oder ihre Anhänge nicht lesen, verwenden, kopieren oder weitergeben.
Sollten Sie die Mail versehentlich erhalten haben, teilen Sie mir dies bitte umgehend per Antwort-E-Mail mit und löschen Sie diese E-Mail dann aus Ihrem System. Vielen Dank

Beckhoff Automation GmbH & Co. KG 
Managing Director: Dipl. Phys. Hans Beckhoff
Registered office: Verl, Germany 
Register court: Guetersloh HRA 7075

DisclaimerID:x9d5u140


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]