| Message ID | 20241023085852.1061031-27-mark.cave-ayland@ilande.co.uk (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | next-cube: more tidy-ups and improvements | expand |
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
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.
Am Wed, 23 Oct 2024 09:58:42 +0100 schrieb Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>: > 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(-) Reviewed-by: Thomas Huth <huth@tuxfamily.org>
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; }
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(-)