* [PATCH] hw/timer/renesas_tmr.c cleanup read operation.
@ 2020-07-11 15:42 Yoshinori Sato
2020-10-28 20:57 ` Thomas Huth
0 siblings, 1 reply; 2+ messages in thread
From: Yoshinori Sato @ 2020-07-11 15:42 UTC (permalink / raw)
To: qemu-devel; +Cc: Yoshinori Sato
Cleanup read operation.
This module different return of access size.
Signed-off-by: Yoshinori Sato <ysato@users.sourceforge.jp>
---
hw/timer/renesas_tmr.c | 106 ++++++++++++++++++++++-------------------
1 file changed, 57 insertions(+), 49 deletions(-)
diff --git a/hw/timer/renesas_tmr.c b/hw/timer/renesas_tmr.c
index 446f2eacdd..d7b21edf39 100644
--- a/hw/timer/renesas_tmr.c
+++ b/hw/timer/renesas_tmr.c
@@ -187,59 +187,67 @@ static uint64_t tmr_read(void *opaque, hwaddr addr, unsigned size)
addr);
return UINT64_MAX;
}
- switch (addr & 0x0e) {
- case A_TCR:
- ret = 0;
- ret = FIELD_DP8(ret, TCR, CCLR,
- FIELD_EX8(tmr->tcr[ch], TCR, CCLR));
- ret = FIELD_DP8(ret, TCR, OVIE,
- FIELD_EX8(tmr->tcr[ch], TCR, OVIE));
- ret = FIELD_DP8(ret, TCR, CMIEA,
- FIELD_EX8(tmr->tcr[ch], TCR, CMIEA));
- ret = FIELD_DP8(ret, TCR, CMIEB,
- FIELD_EX8(tmr->tcr[ch], TCR, CMIEB));
- return ret;
- case A_TCSR:
- ret = 0;
- ret = FIELD_DP8(ret, TCSR, OSA,
- FIELD_EX8(tmr->tcsr[ch], TCSR, OSA));
- ret = FIELD_DP8(ret, TCSR, OSB,
- FIELD_EX8(tmr->tcsr[ch], TCSR, OSB));
- switch (ch) {
- case 0:
- ret = FIELD_DP8(ret, TCSR, ADTE,
- FIELD_EX8(tmr->tcsr[ch], TCSR, ADTE));
- break;
- case 1: /* CH1 ADTE unimplement always 1 */
- ret = FIELD_DP8(ret, TCSR, ADTE, 1);
- break;
- }
- return ret;
- case A_TCORA:
- if (size == 1) {
+ switch (size) {
+ case 1:
+ switch (addr & 0x0e) {
+ case A_TCR:
+ ret = 0;
+ ret = FIELD_DP8(ret, TCR, CCLR,
+ FIELD_EX8(tmr->tcr[ch], TCR, CCLR));
+ ret = FIELD_DP8(ret, TCR, OVIE,
+ FIELD_EX8(tmr->tcr[ch], TCR, OVIE));
+ ret = FIELD_DP8(ret, TCR, CMIEA,
+ FIELD_EX8(tmr->tcr[ch], TCR, CMIEA));
+ ret = FIELD_DP8(ret, TCR, CMIEB,
+ FIELD_EX8(tmr->tcr[ch], TCR, CMIEB));
+ return ret;
+ case A_TCSR:
+ ret = 0;
+ ret = FIELD_DP8(ret, TCSR, OSA,
+ FIELD_EX8(tmr->tcsr[ch], TCSR, OSA));
+ ret = FIELD_DP8(ret, TCSR, OSB,
+ FIELD_EX8(tmr->tcsr[ch], TCSR, OSB));
+ switch (ch) {
+ case 0:
+ ret = FIELD_DP8(ret, TCSR, ADTE,
+ FIELD_EX8(tmr->tcsr[ch], TCSR, ADTE));
+ break;
+ case 1: /* CH1 ADTE unimplement always 1 */
+ ret = FIELD_DP8(ret, TCSR, ADTE, 1);
+ break;
+ }
+ return ret;
+ case A_TCORA:
return tmr->tcora[ch];
- } else if (ch == 0) {
- return concat_reg(tmr->tcora);
- }
- case A_TCORB:
- if (size == 1) {
+ case A_TCORB:
return tmr->tcorb[ch];
- } else {
- return concat_reg(tmr->tcorb);
- }
- case A_TCNT:
- return read_tcnt(tmr, size, ch);
- case A_TCCR:
- if (size == 1) {
+ case A_TCNT:
+ return read_tcnt(tmr, size, ch);
+ case A_TCCR:
return read_tccr(tmr->tccr[ch]);
- } else {
- return read_tccr(tmr->tccr[0]) << 8 | read_tccr(tmr->tccr[1]);
+ default:
+ qemu_log_mask(LOG_UNIMP, "renesas_tmr: Register 0x%" HWADDR_PRIX
+ " not implemented\n",
+ addr);
+ break;
+ }
+ case 2:
+ switch (addr) {
+ case A_TCORA:
+ return concat_reg(tmr->tcora);
+ case A_TCORB:
+ return concat_reg(tmr->tcora);
+ case A_TCNT:
+ return read_tcnt(tmr, size, ch);
+ case A_TCCR:
+ return read_tccr(tmr->tccr[ch]) << 8 | read_tccr(tmr->tccr[1]);
+ default:
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "renesas_tmr: Register 0x%" HWADDR_PRIX
+ " invalid access size\n",
+ addr);
+ break;
}
- default:
- qemu_log_mask(LOG_UNIMP, "renesas_tmr: Register 0x%" HWADDR_PRIX
- " not implemented\n",
- addr);
- break;
}
return UINT64_MAX;
}
--
2.20.1
^ permalink raw reply related [flat|nested] 2+ messages in thread* Re: [PATCH] hw/timer/renesas_tmr.c cleanup read operation.
2020-07-11 15:42 [PATCH] hw/timer/renesas_tmr.c cleanup read operation Yoshinori Sato
@ 2020-10-28 20:57 ` Thomas Huth
0 siblings, 0 replies; 2+ messages in thread
From: Thomas Huth @ 2020-10-28 20:57 UTC (permalink / raw)
To: Yoshinori Sato, qemu-devel, Philippe Mathieu-Daudé
On 11/07/2020 17.42, Yoshinori Sato wrote:
> Cleanup read operation.
> This module different return of access size.
The last sentence is hard to read, could you maybe rephrase it?
> - case A_TCORB:
> - if (size == 1) {
> + case A_TCORB:
> return tmr->tcorb[ch];
> - } else {
> - return concat_reg(tmr->tcorb);
So with size == 2 and addr == TCORB you were returning tmr->tcorb here...
> - }
> - case A_TCNT:
> - return read_tcnt(tmr, size, ch);
> - case A_TCCR:
> - if (size == 1) {
> + case A_TCNT:
> + return read_tcnt(tmr, size, ch);
> + case A_TCCR:
> return read_tccr(tmr->tccr[ch]);
> - } else {
> - return read_tccr(tmr->tccr[0]) << 8 | read_tccr(tmr->tccr[1]);
> + default:
> + qemu_log_mask(LOG_UNIMP, "renesas_tmr: Register 0x%" HWADDR_PRIX
> + " not implemented\n",
> + addr);
> + break;
> + }
> + case 2:
> + switch (addr) {
> + case A_TCORA:
> + return concat_reg(tmr->tcora);
> + case A_TCORB:
> + return concat_reg(tmr->tcora);
... but in the new code, you are now returning tmr->tcora ... copy-n-paste
error?
> + case A_TCNT:
> + return read_tcnt(tmr, size, ch);
> + case A_TCCR:
> + return read_tccr(tmr->tccr[ch]) << 8 | read_tccr(tmr->tccr[1]);
> + default:
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "renesas_tmr: Register 0x%" HWADDR_PRIX
> + " invalid access size\n",
> + addr);
> + break;
> }
> - default:
> - qemu_log_mask(LOG_UNIMP, "renesas_tmr: Register 0x%" HWADDR_PRIX
> - " not implemented\n",
> - addr);
> - break;
> }
> return UINT64_MAX;
> }
>
Thomas
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2020-10-28 20:59 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-07-11 15:42 [PATCH] hw/timer/renesas_tmr.c cleanup read operation Yoshinori Sato
2020-10-28 20:57 ` Thomas Huth
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).