[PATCH] hw/dma/soc_dma: Replace if-else chain with array lookup

[PATCH] hw/dma/soc_dma: Replace if-else chain with array lookup

[Evgeny Kolmakov]

Refactor transfer_fn assignment in soc_dma_ch_update()
via array lookup as suggested by the TODO comment.

Signed-off-by: Evgeny Kolmakov <randomjack94dev@gmail.com>
---
 hw/dma/soc_dma.c | 29 +++++++++++++++++------------
 1 file changed, 17 insertions(+), 12 deletions(-)

diff --git a/hw/dma/soc_dma.c b/hw/dma/soc_dma.c
index d5c52b804f..a5b1ef9f61 100644
--- a/hw/dma/soc_dma.c
+++ b/hw/dma/soc_dma.c
@@ -163,6 +163,16 @@ static inline enum soc_dma_port_type soc_dma_ch_update_type(
 
 void soc_dma_ch_update(struct soc_dma_ch_s *ch)
 {
+    static const soc_dma_transfer_t transfer_fn[2][2] = {
+        [soc_dma_port_mem] = {
+            [soc_dma_port_mem] = transfer_mem2mem,
+            [soc_dma_port_fifo] = transfer_mem2fifo,
+        },
+        [soc_dma_port_fifo] = {
+            [soc_dma_port_mem] = transfer_fifo2mem,
+            [soc_dma_port_fifo] = transfer_fifo2fifo,
+        },
+    };
     enum soc_dma_port_type src, dst;
 
     src = soc_dma_ch_update_type(ch, 0);
@@ -171,21 +181,16 @@ void soc_dma_ch_update(struct soc_dma_ch_s *ch)
         ch->transfer_fn = ch->dma->transfer_fn;
         return;
     }
-    dst = soc_dma_ch_update_type(ch, 1);
 
-    /* TODO: use src and dst as array indices.  */
-    if (src == soc_dma_port_mem && dst == soc_dma_port_mem)
-        ch->transfer_fn = transfer_mem2mem;
-    else if (src == soc_dma_port_mem && dst == soc_dma_port_fifo)
-        ch->transfer_fn = transfer_mem2fifo;
-    else if (src == soc_dma_port_fifo && dst == soc_dma_port_mem)
-        ch->transfer_fn = transfer_fifo2mem;
-    else if (src == soc_dma_port_fifo && dst == soc_dma_port_fifo)
-        ch->transfer_fn = transfer_fifo2fifo;
-    else
+    dst = soc_dma_ch_update_type(ch, 1);
+    if (dst == soc_dma_port_other) {
+        ch->update = 0;
         ch->transfer_fn = ch->dma->transfer_fn;
+        return;
+    }
 
-    ch->update = (dst != soc_dma_port_other);
+    ch->update = 1;
+    ch->transfer_fn = transfer_fn[src][dst];
 }
 
 static void soc_dma_ch_freq_update(struct dma_s *s)
-- 
2.43.0

Re: [PATCH] hw/dma/soc_dma: Replace if-else chain with array lookup

[Peter Maydell]

On Sat, 20 Jun 2026 at 18:20, Evgeny Kolmakov <randomjack94dev@gmail.com> wrote:
>
> Refactor transfer_fn assignment in soc_dma_ch_update()
> via array lookup as suggested by the TODO comment.
>
> Signed-off-by: Evgeny Kolmakov <randomjack94dev@gmail.com>

Any particular reason why you're looking at this ancient
code? It's used only by the omap SoC -- if we care enough
to do anything to it then converting it to qdev is fairly
high up the list...

thanks
-- PMM

Re: [PATCH] hw/dma/soc_dma: Replace if-else chain with array lookup

[Evgeny]

[-- Attachment #1: Type: text/plain, Size: 825 bytes --]

Dear Peter,

No particular reason.

Just found a small refactor-like TODO comment.
Did not think about this code being ancient.
Feel free to discard the patch then.

Thank you for your time!

Best regards,
Evgeny

On Sat, Jun 20, 2026 at 11:23 PM Peter Maydell <peter.maydell@linaro.org>
wrote:

> On Sat, 20 Jun 2026 at 18:20, Evgeny Kolmakov <randomjack94dev@gmail.com>
> wrote:
> >
> > Refactor transfer_fn assignment in soc_dma_ch_update()
> > via array lookup as suggested by the TODO comment.
> >
> > Signed-off-by: Evgeny Kolmakov <randomjack94dev@gmail.com>
>
> Any particular reason why you're looking at this ancient
> code? It's used only by the omap SoC -- if we care enough
> to do anything to it then converting it to qdev is fairly
> high up the list...
>
> thanks
> -- PMM
>

[-- Attachment #2: Type: text/html, Size: 1403 bytes --]

Re: [PATCH] hw/dma/soc_dma: Replace if-else chain with array lookup

[Peter Maydell]

On Sat, 20 Jun 2026 at 21:33, Evgeny <randomjack94dev@gmail.com> wrote:
>
> Dear Peter,
>
> No particular reason.
>
> Just found a small refactor-like TODO comment.
> Did not think about this code being ancient.
> Feel free to discard the patch then.

No worries. Unfortunately QEMU has a fair bit of
old code, so TODO comments are often a bit misleading
about how worthwhile making some change is. Some of
them effectively degrade into "we could do this but
it isn't worth the time" :-/

-- PMM