Re: [Qemu-devel] [PATCH 7/8] hw/arm/stellaris: Correct handling of GPTM TAR register

[Peter Crosthwaite]

On Fri, May 9, 2014 at 4:46 AM, Peter Maydell <address@hidden> wrote:
> We don't implement very much of the GPTM TAR register, and what we
> do is wrong. The "are we in RT mode?" field is in s->config, not
> s->control. Correct this, use LOG_UNIMP rather than hw_error()
> for the cases we don't support, and avoid an unlabelled fallthrough
> that makes Coverity complain.
>
> Signed-off-by: Peter Maydell <address@hidden>
> ---
>  hw/arm/stellaris.c | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/hw/arm/stellaris.c b/hw/arm/stellaris.c
> index d6cc77b..487ee72 100644
> --- a/hw/arm/stellaris.c
> +++ b/hw/arm/stellaris.c
> @@ -185,12 +185,19 @@ static uint64_t gptm_read(void *opaque, hwaddr offset,
>      case 0x44: /* TBPMR */
>          return s->match_prescale[1];
>      case 0x48: /* TAR */
> -        if (s->control == 1)
> +        if (s->config == 1) {
>              return s->rtc;
> +        }
> +        qemu_log_mask(LOG_UNIMP,
> +                      "gptm_read of TAR but timer read not supported");

Should it perhaps be "GPTM read" to be more human?

gptm_read is the qemu fn name and more developer centric, but if thats
what your really going for the perhaps it is better done with %s
__func__ ?

Otherwise:

Reviewed-by: Peter Crosthwaite <address@hidden>

Regards,
Peter

> +        return 0;
>      case 0x4c: /* TBR */
> -        hw_error("TODO: Timer value read\n");
> +        qemu_log_mask(LOG_UNIMP,
> +                      "gptm_read of TBR but timer read not supported");
> +        return 0;
>      default:
> -        hw_error("gptm_read: Bad offset 0x%x\n", (int)offset);
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "gptm_read: Bad offset 0x%x\n", (int)offset);
>          return 0;
>      }
>  }
> --
> 1.9.2
>
>