On 1/28/21 2:54 PM, Peter Maydell 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.)
zero value on power-on is what I tried to describe as
"It is initialized to zero when the instance is created."
Most of the codebase assumes QOM provides a zero-initialized
instance state. Do you think it should be explicit?
Regards,
Phil.
