[PATCH] hw/dma/etraxfs_dma: Simplify using MemoryRegionOps::impl access_size

[PATCH] hw/dma/etraxfs_dma: Simplify using MemoryRegionOps::impl access_size

[Philippe Mathieu-Daudé]

This device implementation is clearly restricted to 32-bit
accesses. Set the MemoryRegionOps::impl min/max access_size
fields to simplify the code, and remove the hw_error() call.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/dma/etraxfs_dma.c | 25 ++++---------------------
 1 file changed, 4 insertions(+), 21 deletions(-)

diff --git a/hw/dma/etraxfs_dma.c b/hw/dma/etraxfs_dma.c
index 47e1c6df12..dcb2286bd9 100644
--- a/hw/dma/etraxfs_dma.c
+++ b/hw/dma/etraxfs_dma.c
@@ -23,7 +23,6 @@
  */
 
 #include "qemu/osdep.h"
-#include "hw/hw.h"
 #include "hw/irq.h"
 #include "qemu/main-loop.h"
 #include "sysemu/runstate.h"
@@ -577,12 +576,6 @@ static inline int channel_in_run(struct fs_dma_ctrl *ctrl, int c)
 		return 0;
 }
 
-static uint32_t dma_rinvalid (void *opaque, hwaddr addr)
-{
-        hw_error("Unsupported short raccess. reg=" TARGET_FMT_plx "\n", addr);
-        return 0;
-}
-
 static uint64_t
 dma_read(void *opaque, hwaddr addr, unsigned int size)
 {
@@ -590,10 +583,6 @@ dma_read(void *opaque, hwaddr addr, unsigned int size)
 	int c;
 	uint32_t r = 0;
 
-	if (size != 4) {
-		dma_rinvalid(opaque, addr);
-	}
-
 	/* Make addr relative to this channel and bounded to nr regs.  */
 	c = fs_channel(addr);
 	addr &= 0xff;
@@ -615,12 +604,6 @@ dma_read(void *opaque, hwaddr addr, unsigned int size)
 	return r;
 }
 
-static void
-dma_winvalid (void *opaque, hwaddr addr, uint32_t value)
-{
-        hw_error("Unsupported short waccess. reg=" TARGET_FMT_plx "\n", addr);
-}
-
 static void
 dma_update_state(struct fs_dma_ctrl *ctrl, int c)
 {
@@ -638,10 +621,6 @@ dma_write(void *opaque, hwaddr addr,
 	uint32_t value = val64;
 	int c;
 
-	if (size != 4) {
-		dma_winvalid(opaque, addr, value);
-	}
-
         /* Make addr relative to this channel and bounded to nr regs.  */
 	c = fs_channel(addr);
         addr &= 0xff;
@@ -701,6 +680,10 @@ static const MemoryRegionOps dma_ops = {
 	.read = dma_read,
 	.write = dma_write,
 	.endianness = DEVICE_NATIVE_ENDIAN,
+    .impl = {
+        .min_access_size = 4,
+        .max_access_size = 4,
+    },
 	.valid = {
 		.min_access_size = 1,
 		.max_access_size = 4
-- 
2.20.1

Re: [PATCH] hw/dma/etraxfs_dma: Simplify using MemoryRegionOps::impl access_size

[Peter Maydell]

On Sat, 5 Oct 2019 at 16:04, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> This device implementation is clearly restricted to 32-bit
> accesses. Set the MemoryRegionOps::impl min/max access_size
> fields to simplify the code, and remove the hw_error() call.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  hw/dma/etraxfs_dma.c | 25 ++++---------------------
>  1 file changed, 4 insertions(+), 21 deletions(-)
>
> @@ -701,6 +680,10 @@ static const MemoryRegionOps dma_ops = {
>         .read = dma_read,
>         .write = dma_write,
>         .endianness = DEVICE_NATIVE_ENDIAN,
> +    .impl = {
> +        .min_access_size = 4,
> +        .max_access_size = 4,
> +    },
>         .valid = {
>                 .min_access_size = 1,
>                 .max_access_size = 4

Unless I've forgotten how the memory layer works, doesn't this
mean we'll now try to synthesize 1 and 2 byte accesses by
making 4 byte accesses ? Would it be better to just set the
.valid.min_access_size to 4 ?

(The indent on the change looks a bit suspect but that's
because the whole file is tab-indent.)

thanks
-- PMM