diff mbox series

[08/12] next-cube.c: move LED logic to new next_scr2_led_update() function

Message ID 20231215200009.346212-9-mark.cave-ayland@ilande.co.uk (mailing list archive)
State New, archived
Headers show
Series next-cube: various tidy-ups and improvements | expand

Commit Message

Mark Cave-Ayland Dec. 15, 2023, 8 p.m. UTC
Ensure that the LED status is updated by calling next_scr2_led_update() whenever
the SC2 register is written.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/m68k/next-cube.c | 22 +++++++++++++---------
 1 file changed, 13 insertions(+), 9 deletions(-)

Comments

Thomas Huth Dec. 16, 2023, 8:13 p.m. UTC | #1
Am Fri, 15 Dec 2023 20:00:05 +0000
schrieb Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>:

> Ensure that the LED status is updated by calling next_scr2_led_update() whenever
> the SC2 register is written.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
>  hw/m68k/next-cube.c | 22 +++++++++++++---------
>  1 file changed, 13 insertions(+), 9 deletions(-)
> 
> diff --git a/hw/m68k/next-cube.c b/hw/m68k/next-cube.c
> index f2222554fa..7ffd1c412e 100644
> --- a/hw/m68k/next-cube.c
> +++ b/hw/m68k/next-cube.c
> @@ -123,6 +123,18 @@ static const uint8_t rtc_ram2[32] = {
>  #define SCR2_RTDATA 0x4
>  #define SCR2_TOBCD(x) (((x / 10) << 4) + (x % 10))
>  
> +static void next_scr2_led_update(NeXTPC *s)
> +{
> +    if (s->scr2 & 0x1) {
> +        DPRINTF("fault!\n");
> +        s->led++;
> +        if (s->led == 10) {
> +            DPRINTF("LED flashing, possible fault!\n");
> +            s->led = 0;
> +        }
> +    }
> +}

This will now operate on the old value of scr2 ...

>  static void nextscr2_write(NeXTPC *s, uint32_t val, int size)
>  {
>      static uint8_t old_scr2;
> @@ -135,15 +147,6 @@ static void nextscr2_write(NeXTPC *s, uint32_t val, int size)
>          scr2_2 = val & 0xFF;
>      }
>  
> -    if (val & 0x1) {
> -        DPRINTF("fault!\n");
> -        s->led++;
> -        if (s->led == 10) {
> -            DPRINTF("LED flashing, possible fault!\n");
> -            s->led = 0;
> -        }
> -    }

.. while this was using the new value that was just written.
So this looks wrong to me ... or do I miss something?

 Thomas


>      if (scr2_2 & 0x1) {
>          /* DPRINTF("RTC %x phase %i\n", scr2_2, rtc->phase); */
>          if (rtc->phase == -1) {
> @@ -318,6 +321,7 @@ static void next_mmio_write(void *opaque, hwaddr addr, uint64_t val,
>          break;
>  
>      case 0xd000 ... 0xd003:
> +        next_scr2_led_update(s);
>          nextscr2_write(s, val, size);
>          break;
>
Mark Cave-Ayland Dec. 19, 2023, 9:40 p.m. UTC | #2
On 16/12/2023 20:13, Thomas Huth wrote:

> Am Fri, 15 Dec 2023 20:00:05 +0000
> schrieb Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>:
> 
>> Ensure that the LED status is updated by calling next_scr2_led_update() whenever
>> the SC2 register is written.
>>
>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>> ---
>>   hw/m68k/next-cube.c | 22 +++++++++++++---------
>>   1 file changed, 13 insertions(+), 9 deletions(-)
>>
>> diff --git a/hw/m68k/next-cube.c b/hw/m68k/next-cube.c
>> index f2222554fa..7ffd1c412e 100644
>> --- a/hw/m68k/next-cube.c
>> +++ b/hw/m68k/next-cube.c
>> @@ -123,6 +123,18 @@ static const uint8_t rtc_ram2[32] = {
>>   #define SCR2_RTDATA 0x4
>>   #define SCR2_TOBCD(x) (((x / 10) << 4) + (x % 10))
>>   
>> +static void next_scr2_led_update(NeXTPC *s)
>> +{
>> +    if (s->scr2 & 0x1) {
>> +        DPRINTF("fault!\n");
>> +        s->led++;
>> +        if (s->led == 10) {
>> +            DPRINTF("LED flashing, possible fault!\n");
>> +            s->led = 0;
>> +        }
>> +    }
>> +}
> 
> This will now operate on the old value of scr2 ...
> 
>>   static void nextscr2_write(NeXTPC *s, uint32_t val, int size)
>>   {
>>       static uint8_t old_scr2;
>> @@ -135,15 +147,6 @@ static void nextscr2_write(NeXTPC *s, uint32_t val, int size)
>>           scr2_2 = val & 0xFF;
>>       }
>>   
>> -    if (val & 0x1) {
>> -        DPRINTF("fault!\n");
>> -        s->led++;
>> -        if (s->led == 10) {
>> -            DPRINTF("LED flashing, possible fault!\n");
>> -            s->led = 0;
>> -        }
>> -    }
> 
> .. while this was using the new value that was just written.
> So this looks wrong to me ... or do I miss something?

Oops no, I think you're right and it's an unintended change. I'll fix this before 
sending a v2 series.

>   Thomas
> 
> 
>>       if (scr2_2 & 0x1) {
>>           /* DPRINTF("RTC %x phase %i\n", scr2_2, rtc->phase); */
>>           if (rtc->phase == -1) {
>> @@ -318,6 +321,7 @@ static void next_mmio_write(void *opaque, hwaddr addr, uint64_t val,
>>           break;
>>   
>>       case 0xd000 ... 0xd003:
>> +        next_scr2_led_update(s);
>>           nextscr2_write(s, val, size);
>>           break;


ATB,

Mark.
diff mbox series

Patch

diff --git a/hw/m68k/next-cube.c b/hw/m68k/next-cube.c
index f2222554fa..7ffd1c412e 100644
--- a/hw/m68k/next-cube.c
+++ b/hw/m68k/next-cube.c
@@ -123,6 +123,18 @@  static const uint8_t rtc_ram2[32] = {
 #define SCR2_RTDATA 0x4
 #define SCR2_TOBCD(x) (((x / 10) << 4) + (x % 10))
 
+static void next_scr2_led_update(NeXTPC *s)
+{
+    if (s->scr2 & 0x1) {
+        DPRINTF("fault!\n");
+        s->led++;
+        if (s->led == 10) {
+            DPRINTF("LED flashing, possible fault!\n");
+            s->led = 0;
+        }
+    }
+}
+
 static void nextscr2_write(NeXTPC *s, uint32_t val, int size)
 {
     static uint8_t old_scr2;
@@ -135,15 +147,6 @@  static void nextscr2_write(NeXTPC *s, uint32_t val, int size)
         scr2_2 = val & 0xFF;
     }
 
-    if (val & 0x1) {
-        DPRINTF("fault!\n");
-        s->led++;
-        if (s->led == 10) {
-            DPRINTF("LED flashing, possible fault!\n");
-            s->led = 0;
-        }
-    }
-
     if (scr2_2 & 0x1) {
         /* DPRINTF("RTC %x phase %i\n", scr2_2, rtc->phase); */
         if (rtc->phase == -1) {
@@ -318,6 +321,7 @@  static void next_mmio_write(void *opaque, hwaddr addr, uint64_t val,
         break;
 
     case 0xd000 ... 0xd003:
+        next_scr2_led_update(s);
         nextscr2_write(s, val, size);
         break;