On Sat, Jan 18, 2020 at 10:06 AM Philippe Mathieu-Daudé <phi...@redhat.com>
wrote:

> On 1/15/20 12:04 AM, Niek Linnenbank wrote:
> > On Tue, Jan 14, 2020 at 12:14 AM Philippe Mathieu-Daudé
> > <phi...@redhat.com <mailto:phi...@redhat.com>> wrote:
> >
> >     On 1/8/20 9:00 PM, Niek Linnenbank wrote:
> >      > Various Allwinner System on Chip designs contain multiple
> processors
> >      > that can be configured and reset using the generic CPU
> Configuration
> >      > module interface. This commit adds support for the Allwinner CPU
> >      > configuration interface which emulates the following features:
> >      >
> >      >   * CPU reset
> >      >   * CPU status
> >      >   * Shared 64-bit timer
> >      >
> [...]
> >      > +    case REG_CPU0_CTRL:         /* CPU#0 Control */
> >      > +    case REG_CPU1_CTRL:         /* CPU#1 Control */
> >      > +    case REG_CPU2_CTRL:         /* CPU#2 Control */
> >      > +    case REG_CPU3_CTRL:         /* CPU#3 Control */
> >      > +    case REG_CPU0_STATUS:       /* CPU#0 Status */
> >      > +    case REG_CPU1_STATUS:       /* CPU#1 Status */
> >      > +    case REG_CPU2_STATUS:       /* CPU#2 Status */
> >      > +    case REG_CPU3_STATUS:       /* CPU#3 Status */
> >      > +    case REG_CLK_GATING:        /* CPU Clock Gating */
> >      > +    case REG_GEN_CTRL:          /* General Control */
> >      > +        s->gen_ctrl = val;
> >      > +        break;
> >      > +    case REG_SUPER_STANDBY:     /* Super Standby Flag */
> >      > +        s->super_standby = val;
> >      > +        break;
> >      > +    case REG_ENTRY_ADDR:        /* Reset Entry Address */
> >      > +        s->entry_addr = val;
> >      > +        break;
> >      > +    case REG_DBG_EXTERN:        /* Debug External */
> >      > +        break;
> >      > +    case REG_CNT64_CTRL:        /* 64-bit Counter Control */
> >      > +        s->counter_ctrl = val;
> >      > +        break;
> >      > +    case REG_CNT64_LOW:         /* 64-bit Counter Low */
> >      > +    case REG_CNT64_HIGH:        /* 64-bit Counter High */
> >
> >     You forgot to set these. Maybe you can add a int64_t cnt64_diff, set
> it
> >     here to the difference with qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL),
> and
> >     in the read() function return cnt64_diff +
> >     qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL).
> >
> > OK I'll need to look into that. Currently this timer is not used by
> > Linux, NetBSD or U-Boot as far
> > as I know. But since it is there, it should be correct indeed.
>
> You might reduce this patch by simply using LOG_UNIMP for these
> registers. Than add the patch when you find some use.
>
> We are more confident when reviewing code when we have a way to test it :)
>
>
True indeed. I'll just remove the 64-bit counter from this patch. Thanks!

Regards,
Niek


-- 
Niek Linnenbank

Reply via email to