diff mbox series

[26/36] next-cube: don't use rtc phase value of -1

Message ID 20241023085852.1061031-27-mark.cave-ayland@ilande.co.uk
State New
Headers show
Series next-cube: more tidy-ups and improvements | expand

Commit Message

Mark Cave-Ayland Oct. 23, 2024, 8:58 a.m. UTC
The rtc phase value of -1 is directly equivalent to using a phase value of 0 so
simplify the logic to use an initial rtc phase of 0.

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

Comments

BALATON Zoltan Oct. 23, 2024, 10:37 a.m. UTC | #1
On Wed, 23 Oct 2024, Mark Cave-Ayland wrote:
> The rtc phase value of -1 is directly equivalent to using a phase value of 0 so
> simplify the logic to use an initial rtc phase of 0.
>
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
> hw/m68k/next-cube.c | 5 +----
> 1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/hw/m68k/next-cube.c b/hw/m68k/next-cube.c
> index 43b2c775c0..e4d0083eb0 100644
> --- a/hw/m68k/next-cube.c
> +++ b/hw/m68k/next-cube.c
> @@ -265,9 +265,6 @@ static void next_scr2_rtc_update(NeXTPC *s)
>
>     if (scr2_2 & 0x1) {
>         /* DPRINTF("RTC %x phase %i\n", scr2_2, rtc->phase); */
> -        if (rtc->phase == -1) {
> -            rtc->phase = 0;
> -        }
>         /* If we are in going down clock... do something */
>         if (((old_scr2 & SCR2_RTCLK) != (scr2_2 & SCR2_RTCLK)) &&
>                 ((scr2_2 & SCR2_RTCLK) == 0)) {
> @@ -282,7 +279,7 @@ static void next_scr2_rtc_update(NeXTPC *s)
>         }
>     } else {
>         /* else end or abort */
> -        rtc->phase = -1;
> +        rtc->phase = 0;
>         rtc->command = 0;
>         rtc->value = 0;
>     }

Additionally, maybe it would be simpler to invert the if condition and 
move this else branch up there to the beginning so you can return early 
after this reset (the deposit at the end does nothing after the else case 
as it's just storing back the unmodified value) and then you can deindent 
the big if where most of the functionality is now.

Regards,
BALATON Zoltan
Mark Cave-Ayland Oct. 24, 2024, 8:28 a.m. UTC | #2
On 23/10/2024 11:37, BALATON Zoltan wrote:

> On Wed, 23 Oct 2024, Mark Cave-Ayland wrote:
>> The rtc phase value of -1 is directly equivalent to using a phase value of 0 so
>> simplify the logic to use an initial rtc phase of 0.
>>
>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>> ---
>> hw/m68k/next-cube.c | 5 +----
>> 1 file changed, 1 insertion(+), 4 deletions(-)
>>
>> diff --git a/hw/m68k/next-cube.c b/hw/m68k/next-cube.c
>> index 43b2c775c0..e4d0083eb0 100644
>> --- a/hw/m68k/next-cube.c
>> +++ b/hw/m68k/next-cube.c
>> @@ -265,9 +265,6 @@ static void next_scr2_rtc_update(NeXTPC *s)
>>
>>     if (scr2_2 & 0x1) {
>>         /* DPRINTF("RTC %x phase %i\n", scr2_2, rtc->phase); */
>> -        if (rtc->phase == -1) {
>> -            rtc->phase = 0;
>> -        }
>>         /* If we are in going down clock... do something */
>>         if (((old_scr2 & SCR2_RTCLK) != (scr2_2 & SCR2_RTCLK)) &&
>>                 ((scr2_2 & SCR2_RTCLK) == 0)) {
>> @@ -282,7 +279,7 @@ static void next_scr2_rtc_update(NeXTPC *s)
>>         }
>>     } else {
>>         /* else end or abort */
>> -        rtc->phase = -1;
>> +        rtc->phase = 0;
>>         rtc->command = 0;
>>         rtc->value = 0;
>>     }
> 
> Additionally, maybe it would be simpler to invert the if condition and move this else 
> branch up there to the beginning so you can return early after this reset (the 
> deposit at the end does nothing after the else case as it's just storing back the 
> unmodified value) and then you can deindent the big if where most of the 
> functionality is now.

I didn't change the layout of the outer if() mainly because Bryce reversed engineered 
a lot of this from Mach/NetBSD, and wasn't 100% confident that there was any other 
logic that needed to be applied to bit 0. It's also worth pointing out that by the 
end of the series all of the functionality has been moved to gpios, so all the 
complexity within the big if() disappears anyway.


ATB,

Mark.
diff mbox series

Patch

diff --git a/hw/m68k/next-cube.c b/hw/m68k/next-cube.c
index 43b2c775c0..e4d0083eb0 100644
--- a/hw/m68k/next-cube.c
+++ b/hw/m68k/next-cube.c
@@ -265,9 +265,6 @@  static void next_scr2_rtc_update(NeXTPC *s)
 
     if (scr2_2 & 0x1) {
         /* DPRINTF("RTC %x phase %i\n", scr2_2, rtc->phase); */
-        if (rtc->phase == -1) {
-            rtc->phase = 0;
-        }
         /* If we are in going down clock... do something */
         if (((old_scr2 & SCR2_RTCLK) != (scr2_2 & SCR2_RTCLK)) &&
                 ((scr2_2 & SCR2_RTCLK) == 0)) {
@@ -282,7 +279,7 @@  static void next_scr2_rtc_update(NeXTPC *s)
         }
     } else {
         /* else end or abort */
-        rtc->phase = -1;
+        rtc->phase = 0;
         rtc->command = 0;
         rtc->value = 0;
     }