1. Patchew
  2. QEMU
  3. [PATCH 0/4] hw/dma: Align SiFive PDMA behavior with real hardware
  4. View patch

[PATCH 1/4] hw/dma: sifive_pdma: reset Next* registers when Control.claim is set

frank.chang@sifive.com posted 4 patches 4 years, 5 months ago
Maintainers: Alistair Francis <Alistair.Francis@wdc.com>, Palmer Dabbelt <palmer@dabbelt.com>, Bin Meng <bin.meng@windriver.com>
There is a newer version of this series
[PATCH 1/4] hw/dma: sifive_pdma: reset Next* registers when Control.claim is set
Posted by frank.chang@sifive.com 4 years, 5 months ago 
From: Frank Chang <frank.chang@sifive.com>

Setting Control.claim clears all of the chanel's Next registers.
This is effective only when Control.claim is set from 0 to 1.

Signed-off-by: Frank Chang <frank.chang@sifive.com>
Tested-by: Max Hsu <max.hsu@sifive.com>
---
 hw/dma/sifive_pdma.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/hw/dma/sifive_pdma.c b/hw/dma/sifive_pdma.c
index 9b2ac2017d9..e723db9d700 100644
--- a/hw/dma/sifive_pdma.c
+++ b/hw/dma/sifive_pdma.c
@@ -54,6 +54,9 @@
 #define DMA_EXEC_DST        0x110
 #define DMA_EXEC_SRC        0x118
 
+#define CONFIG_WRSZ_DEFAULT 6
+#define CONFIG_RDSZ_DEFAULT 6
+
 enum dma_chan_state {
     DMA_CHAN_STATE_IDLE,
     DMA_CHAN_STATE_STARTED,
@@ -221,6 +224,7 @@ static void sifive_pdma_write(void *opaque, hwaddr offset,
 {
     SiFivePDMAState *s = opaque;
     int ch = SIFIVE_PDMA_CHAN_NO(offset);
+    bool claimed;
 
     if (ch >= SIFIVE_PDMA_CHANS) {
         qemu_log_mask(LOG_GUEST_ERROR, "%s: Invalid channel no %d\n",
@@ -231,6 +235,17 @@ static void sifive_pdma_write(void *opaque, hwaddr offset,
     offset &= 0xfff;
     switch (offset) {
     case DMA_CONTROL:
+        claimed = !!s->chan[ch].control & CONTROL_CLAIM;
+
+        if (!claimed && (value & CONTROL_CLAIM)) {
+            /* reset Next* registers */
+            s->chan[ch].next_config = (CONFIG_RDSZ_DEFAULT << CONFIG_RDSZ_SHIFT) |
+                                      (CONFIG_WRSZ_DEFAULT << CONFIG_WRSZ_SHIFT);
+            s->chan[ch].next_bytes = 0;
+            s->chan[ch].next_dst = 0;
+            s->chan[ch].next_src = 0;
+        }
+
         s->chan[ch].control = value;
 
         if (value & CONTROL_RUN) {
-- 
2.25.1
Re: [PATCH 1/4] hw/dma: sifive_pdma: reset Next* registers when Control.claim is set
Posted by Bin Meng 4 years, 5 months ago 
On Fri, Sep 10, 2021 at 1:56 PM <frank.chang@sifive.com> wrote:
>
> From: Frank Chang <frank.chang@sifive.com>
>
> Setting Control.claim clears all of the chanel's Next registers.
> This is effective only when Control.claim is set from 0 to 1.
>
> Signed-off-by: Frank Chang <frank.chang@sifive.com>
> Tested-by: Max Hsu <max.hsu@sifive.com>
> ---
>  hw/dma/sifive_pdma.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
>
> diff --git a/hw/dma/sifive_pdma.c b/hw/dma/sifive_pdma.c
> index 9b2ac2017d9..e723db9d700 100644
> --- a/hw/dma/sifive_pdma.c
> +++ b/hw/dma/sifive_pdma.c
> @@ -54,6 +54,9 @@
>  #define DMA_EXEC_DST        0x110
>  #define DMA_EXEC_SRC        0x118
>
> +#define CONFIG_WRSZ_DEFAULT 6
> +#define CONFIG_RDSZ_DEFAULT 6

The FU540 manual says the next config reset value is 0, not 6.

> +
>  enum dma_chan_state {
>      DMA_CHAN_STATE_IDLE,
>      DMA_CHAN_STATE_STARTED,
> @@ -221,6 +224,7 @@ static void sifive_pdma_write(void *opaque, hwaddr offset,
>  {
>      SiFivePDMAState *s = opaque;
>      int ch = SIFIVE_PDMA_CHAN_NO(offset);
> +    bool claimed;
>
>      if (ch >= SIFIVE_PDMA_CHANS) {
>          qemu_log_mask(LOG_GUEST_ERROR, "%s: Invalid channel no %d\n",
> @@ -231,6 +235,17 @@ static void sifive_pdma_write(void *opaque, hwaddr offset,
>      offset &= 0xfff;
>      switch (offset) {
>      case DMA_CONTROL:
> +        claimed = !!s->chan[ch].control & CONTROL_CLAIM;
> +
> +        if (!claimed && (value & CONTROL_CLAIM)) {
> +            /* reset Next* registers */
> +            s->chan[ch].next_config = (CONFIG_RDSZ_DEFAULT << CONFIG_RDSZ_SHIFT) |
> +                                      (CONFIG_WRSZ_DEFAULT << CONFIG_WRSZ_SHIFT);
> +            s->chan[ch].next_bytes = 0;
> +            s->chan[ch].next_dst = 0;
> +            s->chan[ch].next_src = 0;
> +        }
> +
>          s->chan[ch].control = value;
>
>          if (value & CONTROL_RUN) {

Regards,
Bin
Re: [PATCH 1/4] hw/dma: sifive_pdma: reset Next* registers when Control.claim is set
Posted by Bin Meng 4 years, 5 months ago 
On Sat, Sep 11, 2021 at 8:37 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> On Fri, Sep 10, 2021 at 1:56 PM <frank.chang@sifive.com> wrote:
> >
> > From: Frank Chang <frank.chang@sifive.com>
> >
> > Setting Control.claim clears all of the chanel's Next registers.
> > This is effective only when Control.claim is set from 0 to 1.
> >
> > Signed-off-by: Frank Chang <frank.chang@sifive.com>
> > Tested-by: Max Hsu <max.hsu@sifive.com>
> > ---
> >  hw/dma/sifive_pdma.c | 15 +++++++++++++++
> >  1 file changed, 15 insertions(+)
> >
> > diff --git a/hw/dma/sifive_pdma.c b/hw/dma/sifive_pdma.c
> > index 9b2ac2017d9..e723db9d700 100644
> > --- a/hw/dma/sifive_pdma.c
> > +++ b/hw/dma/sifive_pdma.c
> > @@ -54,6 +54,9 @@
> >  #define DMA_EXEC_DST        0x110
> >  #define DMA_EXEC_SRC        0x118
> >
> > +#define CONFIG_WRSZ_DEFAULT 6
> > +#define CONFIG_RDSZ_DEFAULT 6
>
> The FU540 manual says the next config reset value is 0, not 6.
>

From patch#2 's log on Unmatched, I see where the number 6 is coming.
I also validated on Unleashed and observed the same. So there is a
documentation error.

Please add a comment to explain that.

Otherwise,
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
Re: [PATCH 1/4] hw/dma: sifive_pdma: reset Next* registers when Control.claim is set
Posted by Bin Meng 4 years, 5 months ago 
On Sat, Sep 11, 2021 at 9:12 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> On Sat, Sep 11, 2021 at 8:37 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> >
> > On Fri, Sep 10, 2021 at 1:56 PM <frank.chang@sifive.com> wrote:
> > >
> > > From: Frank Chang <frank.chang@sifive.com>
> > >
> > > Setting Control.claim clears all of the chanel's Next registers.
> > > This is effective only when Control.claim is set from 0 to 1.
> > >
> > > Signed-off-by: Frank Chang <frank.chang@sifive.com>
> > > Tested-by: Max Hsu <max.hsu@sifive.com>
> > > ---
> > >  hw/dma/sifive_pdma.c | 15 +++++++++++++++
> > >  1 file changed, 15 insertions(+)
> > >
> > > diff --git a/hw/dma/sifive_pdma.c b/hw/dma/sifive_pdma.c
> > > index 9b2ac2017d9..e723db9d700 100644
> > > --- a/hw/dma/sifive_pdma.c
> > > +++ b/hw/dma/sifive_pdma.c
> > > @@ -54,6 +54,9 @@
> > >  #define DMA_EXEC_DST        0x110
> > >  #define DMA_EXEC_SRC        0x118
> > >
> > > +#define CONFIG_WRSZ_DEFAULT 6
> > > +#define CONFIG_RDSZ_DEFAULT 6
> >
> > The FU540 manual says the next config reset value is 0, not 6.
> >
>
> From patch#2 's log on Unmatched, I see where the number 6 is coming.
> I also validated on Unleashed and observed the same. So there is a
> documentation error.
>
> Please add a comment to explain that.
>
> Otherwise,
> Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

Tested-by: Bin Meng <bmeng.cn@gmail.com>
Re: [PATCH 1/4] hw/dma: sifive_pdma: reset Next* registers when Control.claim is set
Posted by Frank Chang 4 years, 5 months ago 
On Sat, Sep 11, 2021 at 9:12 PM Bin Meng <bmeng.cn@gmail.com> wrote:

> On Sat, Sep 11, 2021 at 8:37 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> >
> > On Fri, Sep 10, 2021 at 1:56 PM <frank.chang@sifive.com> wrote:
> > >
> > > From: Frank Chang <frank.chang@sifive.com>
> > >
> > > Setting Control.claim clears all of the chanel's Next registers.
> > > This is effective only when Control.claim is set from 0 to 1.
> > >
> > > Signed-off-by: Frank Chang <frank.chang@sifive.com>
> > > Tested-by: Max Hsu <max.hsu@sifive.com>
> > > ---
> > >  hw/dma/sifive_pdma.c | 15 +++++++++++++++
> > >  1 file changed, 15 insertions(+)
> > >
> > > diff --git a/hw/dma/sifive_pdma.c b/hw/dma/sifive_pdma.c
> > > index 9b2ac2017d9..e723db9d700 100644
> > > --- a/hw/dma/sifive_pdma.c
> > > +++ b/hw/dma/sifive_pdma.c
> > > @@ -54,6 +54,9 @@
> > >  #define DMA_EXEC_DST        0x110
> > >  #define DMA_EXEC_SRC        0x118
> > >
> > > +#define CONFIG_WRSZ_DEFAULT 6
> > > +#define CONFIG_RDSZ_DEFAULT 6
> >
> > The FU540 manual says the next config reset value is 0, not 6.
> >
>
> From patch#2 's log on Unmatched, I see where the number 6 is coming.
> I also validated on Unleashed and observed the same. So there is a
> documentation error.
>
> Please add a comment to explain that.
>

Thanks for the review.
Will update it.

Regards,
Frank Chang


>
> Otherwise,
> Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.