On Tue, Jun 1, 2010 at 7:56 PM, Artyom Tarasenko
<atar4q...@googlemail.com> wrote:
> 2010/6/1 Blue Swirl <blauwir...@gmail.com>:
>> On Sun, May 30, 2010 at 10:35 PM, Artyom Tarasenko
>> <atar4q...@googlemail.com> wrote:
>>> lower interrupt during chip reset. Otherwise the ESP_RSTAT register
>>> may get out of sync with the IRQ line status. This effect became
>>> visible after commit 65899fe3
>>
>> Hard reset handlers should not touch qemu_irqs, because on cold start,
>> the receiving end may be unprepared to handle the signal.
>
> Wouldn't the real hardware lower irq on the hardware reset?
Yes, but since qemu_irqs have no state, and on a cold start or system
reset all other devices are guaranteed to be reset, the callback would
be useless.
> And if it would not, would it still clear the corresponding bit in
> the ESP_RSTAT register?
All registers are set to zero in the lines below.
>
>> See
>> 0d0a7e69e853639b123798877e019c3c7ee6634a,
>> bc26e55a6615dc594be425d293db40d5cdcdb84b and
>> 42f1ced228c9b616cfa2b69846025271618e4ef5.
>>
>> For ESP there are two other sources of reset: signal from DMA and chip
>> reset command. On those cases, lowering IRQ makes sense.
>>
>> So the correct fix is to refactor the reset handling a bit. Does this
>> patch also fix your test case?
>
> It does, but
>
> +static void esp_soft_reset(DeviceState *d)
> +{
> + ESPState *s = container_of(d, ESPState, busdev.qdev);
> +
> + qemu_irq_lower(s->irq);
>
> Shouldn't it be esp_lower_irq(s)? What's going to happen to the
> DMA_INTR bit if dma was the source of the irq?
Again, the registers are zeroed in esp_hard_reset().
Thanks for testing.
