On 6 June 2016 at 13:00, Paolo Bonzini <pbonz...@redhat.com> wrote: > > > On 31/05/2016 13:36, P J P wrote: >> + if ((s->rregs[ESP_RSTAT] & STAT_PIO_MASK) == 0) { >> + /* Data out. */ >> + qemu_log_mask(LOG_UNIMP, "esp: PIO data read not >> implemented\n"); >> + s->rregs[ESP_FIFO] = 0; >> + esp_raise_irq(s); >> + } else if (s->ti_rptr < s->ti_wptr) { >> s->ti_size--; >> - if ((s->rregs[ESP_RSTAT] & STAT_PIO_MASK) == 0) { >> - /* Data out. */ >> - qemu_log_mask(LOG_UNIMP, >> - "esp: PIO data read not implemented\n"); >> - s->rregs[ESP_FIFO] = 0; >> - } else { >> - s->rregs[ESP_FIFO] = s->ti_buf[s->ti_rptr++]; >> - } >> + s->rregs[ESP_FIFO] = s->ti_buf[s->ti_rptr++]; >> esp_raise_irq(s); >> } >> if (s->ti_size == 0) { > > Shouldn't this become a "s->ti_rptr == s->ti_wptr" too?
It would probably be helpful to document what the intended invariant relating ti_rptr, ti_wptr and ti_size is as well. thanks -- PMM