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

[Mark Cave-Ayland]

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.