Re: [PATCH v2 2/2] hw/dma: sifive_pdma: Don't run DMA when channel is disclaimed

2021-09-28 Thread Alistair Francis
On Mon, Sep 27, 2021 at 5:22 PM Bin Meng  wrote:
>
> If Control.run bit is set while not preserving the Control.claim
> bit, the DMA transfer shall not be started.
>
> The following result is PDMA tested in U-Boot on Unleashed board:
>
> => mw.l 0x300 0x0  <= Disclaim channel 0
> => mw.l 0x300 0x1  <= Claim channel 0
> => mw.l 0x304 0x5500   <= wsize = rsize = 5 (2^5 = 32 
> bytes)
> => mw.q 0x308 0x2  <= NextBytes = 2
> => mw.q 0x310 0x8400   <= NextDestination = 0x8400
> => mw.q 0x318 0x84001000   <= NextSource = 0x84001000
> => mw.l 0x8400 0x87654321  <= Fill test data to dst
> => mw.l 0x84001000 0x12345678  <= Fill test data to src
> => md.l 0x8400 1; md.l 0x84001000 1<= Dump src/dst memory contents
> 8400: 87654321   !Ce.
> 84001000: 12345678   xV4.
> => md.l 0x300 8<= Dump PDMA status
> 0300: 0001 5500 0002 ...U
> 0310: 8400  84001000 
> => mw.l 0x300 0x2  <= Set channel 0 run bit only
> => md.l 0x300 8<= Dump PDMA status
> 0300:  5500 0002 ...U
> 0310: 8400  84001000 
> => md.l 0x8400 1; md.l 0x84001000 1<= Dump src/dst memory contents
> 8400: 87654321   !Ce.
> 84001000: 12345678   xV4.
>
> Signed-off-by: Bin Meng 

Thanks!

Applied to riscv-to-apply.next

Alistair

>
> ---
>
> (no changes since v1)
>
>  hw/dma/sifive_pdma.c | 11 +--
>  1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/hw/dma/sifive_pdma.c b/hw/dma/sifive_pdma.c
> index b8ec7621f3..85fe34f5f3 100644
> --- a/hw/dma/sifive_pdma.c
> +++ b/hw/dma/sifive_pdma.c
> @@ -232,7 +232,7 @@ static void sifive_pdma_write(void *opaque, hwaddr offset,
>  {
>  SiFivePDMAState *s = opaque;
>  int ch = SIFIVE_PDMA_CHAN_NO(offset);
> -bool claimed;
> +bool claimed, run;
>
>  if (ch >= SIFIVE_PDMA_CHANS) {
>  qemu_log_mask(LOG_GUEST_ERROR, "%s: Invalid channel no %d\n",
> @@ -244,6 +244,7 @@ static void sifive_pdma_write(void *opaque, hwaddr offset,
>  switch (offset) {
>  case DMA_CONTROL:
>  claimed = !!(s->chan[ch].control & CONTROL_CLAIM);
> +run = !!(s->chan[ch].control & CONTROL_RUN);
>
>  if (!claimed && (value & CONTROL_CLAIM)) {
>  /* reset Next* registers */
> @@ -254,13 +255,19 @@ static void sifive_pdma_write(void *opaque, hwaddr 
> offset,
>  s->chan[ch].next_src = 0;
>  }
>
> +/* claim bit can only be cleared when run is low */
> +if (run && !(value & CONTROL_CLAIM)) {
> +value |= CONTROL_CLAIM;
> +}
> +
>  s->chan[ch].control = value;
>
>  /*
>   * If channel was not claimed before run bit is set,
> + * or if the channel is disclaimed when run was low,
>   * DMA won't run.
>   */
> -if (!claimed) {
> +if (!claimed || (!run && !(value & CONTROL_CLAIM))) {
>  s->chan[ch].control &= ~CONTROL_RUN;
>  return;
>  }
> --
> 2.25.1
>
>



Re: [PATCH v2 2/2] hw/dma: sifive_pdma: Don't run DMA when channel is disclaimed

2021-09-28 Thread Alistair Francis
On Mon, Sep 27, 2021 at 5:22 PM Bin Meng  wrote:
>
> If Control.run bit is set while not preserving the Control.claim
> bit, the DMA transfer shall not be started.
>
> The following result is PDMA tested in U-Boot on Unleashed board:
>
> => mw.l 0x300 0x0  <= Disclaim channel 0
> => mw.l 0x300 0x1  <= Claim channel 0
> => mw.l 0x304 0x5500   <= wsize = rsize = 5 (2^5 = 32 
> bytes)
> => mw.q 0x308 0x2  <= NextBytes = 2
> => mw.q 0x310 0x8400   <= NextDestination = 0x8400
> => mw.q 0x318 0x84001000   <= NextSource = 0x84001000
> => mw.l 0x8400 0x87654321  <= Fill test data to dst
> => mw.l 0x84001000 0x12345678  <= Fill test data to src
> => md.l 0x8400 1; md.l 0x84001000 1<= Dump src/dst memory contents
> 8400: 87654321   !Ce.
> 84001000: 12345678   xV4.
> => md.l 0x300 8<= Dump PDMA status
> 0300: 0001 5500 0002 ...U
> 0310: 8400  84001000 
> => mw.l 0x300 0x2  <= Set channel 0 run bit only
> => md.l 0x300 8<= Dump PDMA status
> 0300:  5500 0002 ...U
> 0310: 8400  84001000 
> => md.l 0x8400 1; md.l 0x84001000 1<= Dump src/dst memory contents
> 8400: 87654321   !Ce.
> 84001000: 12345678   xV4.
>
> Signed-off-by: Bin Meng 

Acked-by: Alistair Francis 

Alistair

>
> ---
>
> (no changes since v1)
>
>  hw/dma/sifive_pdma.c | 11 +--
>  1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/hw/dma/sifive_pdma.c b/hw/dma/sifive_pdma.c
> index b8ec7621f3..85fe34f5f3 100644
> --- a/hw/dma/sifive_pdma.c
> +++ b/hw/dma/sifive_pdma.c
> @@ -232,7 +232,7 @@ static void sifive_pdma_write(void *opaque, hwaddr offset,
>  {
>  SiFivePDMAState *s = opaque;
>  int ch = SIFIVE_PDMA_CHAN_NO(offset);
> -bool claimed;
> +bool claimed, run;
>
>  if (ch >= SIFIVE_PDMA_CHANS) {
>  qemu_log_mask(LOG_GUEST_ERROR, "%s: Invalid channel no %d\n",
> @@ -244,6 +244,7 @@ static void sifive_pdma_write(void *opaque, hwaddr offset,
>  switch (offset) {
>  case DMA_CONTROL:
>  claimed = !!(s->chan[ch].control & CONTROL_CLAIM);
> +run = !!(s->chan[ch].control & CONTROL_RUN);
>
>  if (!claimed && (value & CONTROL_CLAIM)) {
>  /* reset Next* registers */
> @@ -254,13 +255,19 @@ static void sifive_pdma_write(void *opaque, hwaddr 
> offset,
>  s->chan[ch].next_src = 0;
>  }
>
> +/* claim bit can only be cleared when run is low */
> +if (run && !(value & CONTROL_CLAIM)) {
> +value |= CONTROL_CLAIM;
> +}
> +
>  s->chan[ch].control = value;
>
>  /*
>   * If channel was not claimed before run bit is set,
> + * or if the channel is disclaimed when run was low,
>   * DMA won't run.
>   */
> -if (!claimed) {
> +if (!claimed || (!run && !(value & CONTROL_CLAIM))) {
>  s->chan[ch].control &= ~CONTROL_RUN;
>  return;
>  }
> --
> 2.25.1
>
>



[PATCH v2 2/2] hw/dma: sifive_pdma: Don't run DMA when channel is disclaimed

2021-09-27 Thread Bin Meng
If Control.run bit is set while not preserving the Control.claim
bit, the DMA transfer shall not be started.

The following result is PDMA tested in U-Boot on Unleashed board:

=> mw.l 0x300 0x0  <= Disclaim channel 0
=> mw.l 0x300 0x1  <= Claim channel 0
=> mw.l 0x304 0x5500   <= wsize = rsize = 5 (2^5 = 32 bytes)
=> mw.q 0x308 0x2  <= NextBytes = 2
=> mw.q 0x310 0x8400   <= NextDestination = 0x8400
=> mw.q 0x318 0x84001000   <= NextSource = 0x84001000
=> mw.l 0x8400 0x87654321  <= Fill test data to dst
=> mw.l 0x84001000 0x12345678  <= Fill test data to src
=> md.l 0x8400 1; md.l 0x84001000 1<= Dump src/dst memory contents
8400: 87654321   !Ce.
84001000: 12345678   xV4.
=> md.l 0x300 8<= Dump PDMA status
0300: 0001 5500 0002 ...U
0310: 8400  84001000 
=> mw.l 0x300 0x2  <= Set channel 0 run bit only
=> md.l 0x300 8<= Dump PDMA status
0300:  5500 0002 ...U
0310: 8400  84001000 
=> md.l 0x8400 1; md.l 0x84001000 1<= Dump src/dst memory contents
8400: 87654321   !Ce.
84001000: 12345678   xV4.

Signed-off-by: Bin Meng 

---

(no changes since v1)

 hw/dma/sifive_pdma.c | 11 +--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/hw/dma/sifive_pdma.c b/hw/dma/sifive_pdma.c
index b8ec7621f3..85fe34f5f3 100644
--- a/hw/dma/sifive_pdma.c
+++ b/hw/dma/sifive_pdma.c
@@ -232,7 +232,7 @@ static void sifive_pdma_write(void *opaque, hwaddr offset,
 {
 SiFivePDMAState *s = opaque;
 int ch = SIFIVE_PDMA_CHAN_NO(offset);
-bool claimed;
+bool claimed, run;
 
 if (ch >= SIFIVE_PDMA_CHANS) {
 qemu_log_mask(LOG_GUEST_ERROR, "%s: Invalid channel no %d\n",
@@ -244,6 +244,7 @@ static void sifive_pdma_write(void *opaque, hwaddr offset,
 switch (offset) {
 case DMA_CONTROL:
 claimed = !!(s->chan[ch].control & CONTROL_CLAIM);
+run = !!(s->chan[ch].control & CONTROL_RUN);
 
 if (!claimed && (value & CONTROL_CLAIM)) {
 /* reset Next* registers */
@@ -254,13 +255,19 @@ static void sifive_pdma_write(void *opaque, hwaddr offset,
 s->chan[ch].next_src = 0;
 }
 
+/* claim bit can only be cleared when run is low */
+if (run && !(value & CONTROL_CLAIM)) {
+value |= CONTROL_CLAIM;
+}
+
 s->chan[ch].control = value;
 
 /*
  * If channel was not claimed before run bit is set,
+ * or if the channel is disclaimed when run was low,
  * DMA won't run.
  */
-if (!claimed) {
+if (!claimed || (!run && !(value & CONTROL_CLAIM))) {
 s->chan[ch].control &= ~CONTROL_RUN;
 return;
 }
-- 
2.25.1