| Message ID | 1399574818-19349-8-git-send-email-peter.maydell@linaro.org |
|---|---|
| State | Accepted |
| Headers | show |
On Fri, May 9, 2014 at 4:46 AM, Peter Maydell <peter.maydell@linaro.org> 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 <peter.maydell@linaro.org> > --- > 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 <peter.crosthwaite@xilinx.com> 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 > >
On 10 May 2014 13:33, Peter Crosthwaite <peter.crosthwaite@xilinx.com> wrote: > On Fri, May 9, 2014 at 4:46 AM, Peter Maydell <peter.maydell@linaro.org> 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 <peter.maydell@linaro.org> >> --- >> 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? Good idea. thanks -- PMM
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"); + 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; } }
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 <peter.maydell@linaro.org> --- hw/arm/stellaris.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-)