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.shu@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 */
}
/* Map an offset addr into a channel index. */
@@ -249,18 +262,6 @@ static MemTxResult stream_desc_store(struct Stream *s, hwaddr addr)
return result;
}
-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_reload_complete_cnt(struct Stream *s)
{
unsigned int comp_th;
--
2.17.1
On Fri, 26 Jul 2024 at 06:59, Jim Shu <jim.shu@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.shu@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
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.maydell@linaro.org> wrote: > > On Fri, 26 Jul 2024 at 06:59, Jim Shu <jim.shu@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.shu@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
© 2016 - 2024 Red Hat, Inc.