[PATCH 0/3] hw/misc/mac_via: Factor generic via_irq_request() out

[PATCH 0/3] hw/misc/mac_via: Factor generic via_irq_request() out

[Philippe Mathieu-Daudé]

The same logic is used in 4 different places:
- via1_irq_request()
- via2_irq_request()
- via1_VBL()
- via1_one_second()

Extract the common function and reuse it.

Philippe Mathieu-Daudé (3):
  hw/misc/mac_via: Make generic via_irq_request() from
    via1_irq_request()
  hw/misc/mac_via: Replace via2_irq_request() with via_irq_request()
  hw/misc/mac_via: Use via_irq_request() in via1_VBL(),
    via1_one_second()

 hw/misc/mac_via.c | 59 +++++++++++++++--------------------------------
 1 file changed, 18 insertions(+), 41 deletions(-)

-- 
2.26.2

[PATCH 1/3] hw/misc/mac_via: Make generic via_irq_request() from via1_irq_request()

[Philippe Mathieu-Daudé]

Rewrite via1_irq_request() as generic via_irq_request().

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/misc/mac_via.c | 31 +++++++++++++++----------------
 1 file changed, 15 insertions(+), 16 deletions(-)

diff --git a/hw/misc/mac_via.c b/hw/misc/mac_via.c
index 6db62dab7db..9e64c2521fc 100644
--- a/hw/misc/mac_via.c
+++ b/hw/misc/mac_via.c
@@ -324,6 +324,20 @@ static void via1_one_second_update(MOS6522Q800VIA1State *v1s)
     }
 }
 
+static void via_irq_request(void *opaque, int irq, int level)
+{
+    MOS6522State *s = MOS6522(opaque);
+    MOS6522DeviceClass *mdc = MOS6522_GET_CLASS(s);
+
+    if (level) {
+        s->ifr |= 1 << irq;
+    } else {
+        s->ifr &= ~(1 << irq);
+    }
+
+    mdc->update_irq(s);
+}
+
 static void via1_VBL(void *opaque)
 {
     MOS6522Q800VIA1State *v1s = opaque;
@@ -348,21 +362,6 @@ static void via1_one_second(void *opaque)
     via1_one_second_update(v1s);
 }
 
-static void via1_irq_request(void *opaque, int irq, int level)
-{
-    MOS6522Q800VIA1State *v1s = opaque;
-    MOS6522State *s = MOS6522(v1s);
-    MOS6522DeviceClass *mdc = MOS6522_GET_CLASS(s);
-
-    if (level) {
-        s->ifr |= 1 << irq;
-    } else {
-        s->ifr &= ~(1 << irq);
-    }
-
-    mdc->update_irq(s);
-}
-
 static void via2_irq_request(void *opaque, int irq, int level)
 {
     MOS6522Q800VIA2State *v2s = opaque;
@@ -1195,7 +1194,7 @@ static void mos6522_q800_via1_reset(DeviceState *dev)
 
 static void mos6522_q800_via1_init(Object *obj)
 {
-    qdev_init_gpio_in_named(DEVICE(obj), via1_irq_request, "via1-irq",
+    qdev_init_gpio_in_named(DEVICE(obj), via_irq_request, "via1-irq",
                             VIA1_IRQ_NB);
 }
 
-- 
2.26.2

[PATCH 2/3] hw/misc/mac_via: Replace via2_irq_request() with via_irq_request()

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/misc/mac_via.c | 18 +-----------------
 1 file changed, 1 insertion(+), 17 deletions(-)

diff --git a/hw/misc/mac_via.c b/hw/misc/mac_via.c
index 9e64c2521fc..54088b6625a 100644
--- a/hw/misc/mac_via.c
+++ b/hw/misc/mac_via.c
@@ -362,22 +362,6 @@ static void via1_one_second(void *opaque)
     via1_one_second_update(v1s);
 }
 
-static void via2_irq_request(void *opaque, int irq, int level)
-{
-    MOS6522Q800VIA2State *v2s = opaque;
-    MOS6522State *s = MOS6522(v2s);
-    MOS6522DeviceClass *mdc = MOS6522_GET_CLASS(s);
-
-    if (level) {
-        s->ifr |= 1 << irq;
-    } else {
-        s->ifr &= ~(1 << irq);
-    }
-
-    mdc->update_irq(s);
-}
-
-
 static void pram_update(MacVIAState *m)
 {
     if (m->blk) {
@@ -1238,7 +1222,7 @@ static void mos6522_q800_via2_reset(DeviceState *dev)
 
 static void mos6522_q800_via2_init(Object *obj)
 {
-    qdev_init_gpio_in_named(DEVICE(obj), via2_irq_request, "via2-irq",
+    qdev_init_gpio_in_named(DEVICE(obj), via_irq_request, "via2-irq",
                             VIA2_IRQ_NB);
 }
 
-- 
2.26.2

[PATCH 3/3] hw/misc/mac_via: Use via_irq_request() in via1_VBL(), via1_one_second()

[Philippe Mathieu-Daudé]

via1_VBL() and via1_one_second() just call the generic
via_irq_request() handler raising a specific IRQ.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/misc/mac_via.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/hw/misc/mac_via.c b/hw/misc/mac_via.c
index 54088b6625a..00877a42447 100644
--- a/hw/misc/mac_via.c
+++ b/hw/misc/mac_via.c
@@ -341,11 +341,8 @@ static void via_irq_request(void *opaque, int irq, int level)
 static void via1_VBL(void *opaque)
 {
     MOS6522Q800VIA1State *v1s = opaque;
-    MOS6522State *s = MOS6522(v1s);
-    MOS6522DeviceClass *mdc = MOS6522_GET_CLASS(s);
 
-    s->ifr |= VIA1_IRQ_VBLANK;
-    mdc->update_irq(s);
+    via_irq_request(v1s, VIA1_IRQ_VBLANK, 1);
 
     via1_VBL_update(v1s);
 }
@@ -353,11 +350,8 @@ static void via1_VBL(void *opaque)
 static void via1_one_second(void *opaque)
 {
     MOS6522Q800VIA1State *v1s = opaque;
-    MOS6522State *s = MOS6522(v1s);
-    MOS6522DeviceClass *mdc = MOS6522_GET_CLASS(s);
 
-    s->ifr |= VIA1_IRQ_ONE_SECOND;
-    mdc->update_irq(s);
+    via_irq_request(v1s, VIA1_IRQ_ONE_SECOND, 1);
 
     via1_one_second_update(v1s);
 }
-- 
2.26.2

Re: [PATCH 0/3] hw/misc/mac_via: Factor generic via_irq_request() out

[Mark Cave-Ayland]

On 13/10/2020 21:49, Philippe Mathieu-Daudé wrote:

> The same logic is used in 4 different places:
> - via1_irq_request()
> - via2_irq_request()
> - via1_VBL()
> - via1_one_second()
> 
> Extract the common function and reuse it.
> 
> Philippe Mathieu-Daudé (3):
>    hw/misc/mac_via: Make generic via_irq_request() from
>      via1_irq_request()
>    hw/misc/mac_via: Replace via2_irq_request() with via_irq_request()
>    hw/misc/mac_via: Use via_irq_request() in via1_VBL(),
>      via1_one_second()
> 
>   hw/misc/mac_via.c | 59 +++++++++++++++--------------------------------
>   1 file changed, 18 insertions(+), 41 deletions(-)

Whilst I can see the advantage of consolidating the logic in via_irq_request(), I'd 
still like to keep the above 4 functions as wrappers for now since they are a great 
aid with current work debugging Linux and MacOS. Perhaps for now the functions above 
could act as thin wrappers on your version of via_irq_request(), or alternatively 
this could be something to revisit once the m68k/q800 emulation has matured further?


ATB,

Mark.