Hi Peter,
Except DeviceState::reset(), stream_reset() is only used in
axidma_write() and axidma_write() has qemu_set_irq() at the end of
function.
I think this commit could be dropped. I will remove it in the v2 patchset.
Thanks,
Jim Shu
On Mon, Jul 29, 2024 at 11:23 PM Peter Maydell <peter.mayd...@linaro.org> wrote:
>
> On Fri, 26 Jul 2024 at 06:59, Jim Shu <jim....@sifive.com> wrote:
> >
> > Current DMA/Stream reset will clear interrupt pending bit of DMA device.
> > The qemu_irq of device should be updated at the same time.
> >
> > Signed-off-by: Jim Shu <jim....@sifive.com>
> > ---
> > hw/dma/xilinx_axidma.c | 25 +++++++++++++------------
> > 1 file changed, 13 insertions(+), 12 deletions(-)
> >
> > diff --git a/hw/dma/xilinx_axidma.c b/hw/dma/xilinx_axidma.c
> > index 728246f925..beb3fbf1d5 100644
> > --- a/hw/dma/xilinx_axidma.c
> > +++ b/hw/dma/xilinx_axidma.c
> > @@ -177,11 +177,24 @@ static inline int stream_halted(struct Stream *s)
> > return !!(s->regs[R_DMASR] & DMASR_HALTED);
> > }
> >
> > +static void stream_update_irq(struct Stream *s)
> > +{
> > + unsigned int pending, mask, irq;
> > +
> > + pending = s->regs[R_DMASR] & DMASR_IRQ_MASK;
> > + mask = s->regs[R_DMACR] & DMASR_IRQ_MASK;
> > +
> > + irq = pending & mask;
> > +
> > + qemu_set_irq(s->irq, !!irq);
> > +}
> > +
> > static void stream_reset(struct Stream *s)
> > {
> > s->regs[R_DMASR] = DMASR_HALTED; /* starts up halted. */
> > s->regs[R_DMACR] = 1 << 16; /* Starts with one in compl threshold. */
> > s->sof = true;
> > + stream_update_irq(s); /* Clear interrupt */
> > }
>
> The general rule of thumb in QEMU is not to call
> qemu_set_irq() from a DeviceState::reset function,
> and xilinx_axidma_reset calls stream_reset. I think
> probably the best thing is to separate out whether
> this is a DeviceState::reset or a software-triggered
> reset, and only call qemu_set_irq() in the latter case.
>
> thanks
> -- PMM
