On 1/28/21 3:22 PM, Peter Maydell wrote:
> On Thu, 28 Jan 2021 at 14:18, Philippe Mathieu-Daudé <[email protected]> wrote:
>>
>> On 1/28/21 2:54 PM, Peter Maydell wrote:
>>> On Thu, 28 Jan 2021 at 13:47, Bin Meng <[email protected]> wrote:
>>>>
>>>> On Thu, Jan 28, 2021 at 9:43 PM Peter Maydell <[email protected]>
>>>> wrote:
>>>>> 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."
>
> Yes, but QOM device reset does not happen just once at startup and
> not thereafter. Consider:
>
> * user starts QEMU
> * QOM devices are created and realized
> * QOM device reset happens
> -- CONREG is zero here because QOM structs are zero-initialized
> * guest runs
> * guest modifies CONREG from its initial value
> * system reset is requested (perhaps by user, perhaps by
> guest writing some register or another)
> * QOM device reset happens
> -- CONREG is not zero here, so reset must clear it
Oh I totally missed that :S
Bin, I'd correct this as:
- extract imx_spi_soft_reset(IMXSPIState *s) from imx_spi_reset()
- zero-initialize CONREG in imx_spi_reset().
static void imx_spi_soft_reset(IMXSPIState *s)
{
...
}
static void imx_spi_reset(DeviceState *dev)
{
IMXSPIState *s = IMX_SPI(dev);
s->regs[ECSPI_CONREG] = 0;
imx_spi_soft_reset(s);
}
What do you think?
Phil.
