On Thu, Jan 28, 2021 at 9:54 PM Peter Maydell <peter.mayd...@linaro.org> wrote:
>
> On Thu, 28 Jan 2021 at 13:47, Bin Meng <bmeng...@gmail.com> wrote:
> >
> > On Thu, Jan 28, 2021 at 9:43 PM Peter Maydell <peter.mayd...@linaro.org>
> > wrote:
> > >
> > > On Tue, 19 Jan 2021 at 13:40, Bin Meng <bmeng...@gmail.com> wrote:
> > > >
> > > > From: Philippe Mathieu-Daudé <f4...@amsat.org>
> > > >
> > > > When the block is disabled, all registers are reset with the
> > > > exception of the ECSPI_CONREG. It is initialized to zero
> > > > when the instance is created.
> > > >
> > > > Ref: i.MX 6DQ Applications Processor Reference Manual (IMX6DQRM),
> > > > chapter 21.7.3: Control Register (ECSPIx_CONREG)
> > >
> > > > diff --git a/hw/ssi/imx_spi.c b/hw/ssi/imx_spi.c
> > > > index 8fb3c9b..c952a3d 100644
> > > > --- a/hw/ssi/imx_spi.c
> > > > +++ b/hw/ssi/imx_spi.c
> > > > @@ -231,12 +231,23 @@ static void imx_spi_flush_txfifo(IMXSPIState *s)
> > > > static void imx_spi_reset(DeviceState *dev)
> > > > {
> > > > IMXSPIState *s = IMX_SPI(dev);
> > > > + int i;
> > > >
> > > > DPRINTF("\n");
> > > >
> > > > - memset(s->regs, 0, sizeof(s->regs));
> > > > -
> > > > - s->regs[ECSPI_STATREG] = 0x00000003;
> > > > + for (i = 0; i < ARRAY_SIZE(s->regs); i++) {
> > > > + switch (i) {
> > > > + case ECSPI_CONREG:
> > > > + /* CONREG is not updated on reset */
> > > > + break;
> > > > + case ECSPI_STATREG:
> > > > + s->regs[i] = 0x00000003;
> > > > + break;
> > > > + default:
> > > > + s->regs[i] = 0;
> > > > + break;
> > > > + }
> > > > + }
> > >
> > > This retains the CONREG register value for both:
> > > * 'soft' reset caused by write to device register to disable
> > > the block
> > > -- this is corrcet as per the datasheet quote
> > > * 'power on' reset via TYPE_DEVICE's reset method
> > > -- but in this case we should reset CONREG, because the Device
> > > reset method is like a complete device powercycle and should
> > > return the device state to what it was when QEMU was first
> > > started.
> >
> > The POR value of CONREG is zero, which should be the default value, no?
>
> But you're not setting it to zero here, you're leaving it with
> whatever value it had before. (That's correct for soft reset,
> but wrong for power-on.)
I think that's ensured by object_initialize_with_type().
Regards,
Bin
