* [PATCH 0/3] renesas_sci update
@ 2021-06-16 9:12 Yoshinori Sato
2021-06-16 9:12 ` [PATCH 1/3] hw/char: renesas_sci: Refactor for merge all SCI variant Yoshinori Sato
` (3 more replies)
0 siblings, 4 replies; 9+ messages in thread
From: Yoshinori Sato @ 2021-06-16 9:12 UTC (permalink / raw)
To: qemu-devel; +Cc: Yoshinori Sato
Renesas SH/RX have various SCI in serial interface.
The design of sh_serial is old, so I integrate these SCIs with renesas_sci.
Yoshinori Sato (3):
hw/char: renesas_sci: Refactor for merge all SCI variant..
hw/char: renesas_sci Add SCI and SCIF support.
hw/sh4: sh7750 using renesas_sci.
include/hw/char/renesas_sci.h | 117 +++-
include/hw/rx/rx62n.h | 2 +-
include/hw/sh4/sh.h | 8 -
hw/char/renesas_sci.c | 1053 +++++++++++++++++++++++++++------
hw/rx/rx62n.c | 2 +-
hw/sh4/sh7750.c | 41 ++
hw/sh4/Kconfig | 2 +-
7 files changed, 1025 insertions(+), 200 deletions(-)
--
2.20.1
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/3] hw/char: renesas_sci: Refactor for merge all SCI variant..
2021-06-16 9:12 [PATCH 0/3] renesas_sci update Yoshinori Sato
@ 2021-06-16 9:12 ` Yoshinori Sato
2021-06-29 12:33 ` Peter Maydell
2021-06-16 9:12 ` [PATCH 2/3] hw/char: renesas_sci Add SCI and SCIF support Yoshinori Sato
` (2 subsequent siblings)
3 siblings, 1 reply; 9+ messages in thread
From: Yoshinori Sato @ 2021-06-16 9:12 UTC (permalink / raw)
To: qemu-devel; +Cc: Yoshinori Sato
In order to handle unified all of the SCI, SCIa and SCIF in one part,
to separate the transmission and reception portion and a register portion.
RenesasSCIBase - common registers operation and event handling.
RenesasSCIA - SCIa specific reigisters / functions.
Signed-off-by: Yoshinori Sato <ysato@users.sourceforge.jp>
---
include/hw/char/renesas_sci.h | 80 ++++-
include/hw/rx/rx62n.h | 2 +-
hw/char/renesas_sci.c | 568 +++++++++++++++++++++++-----------
hw/rx/rx62n.c | 2 +-
4 files changed, 457 insertions(+), 195 deletions(-)
diff --git a/include/hw/char/renesas_sci.h b/include/hw/char/renesas_sci.h
index a4764e3eee..c666cf81d1 100644
--- a/include/hw/char/renesas_sci.h
+++ b/include/hw/char/renesas_sci.h
@@ -1,7 +1,7 @@
/*
* Renesas Serial Communication Interface
*
- * Copyright (c) 2018 Yoshinori Sato
+ * Copyright (c) 2020 Yoshinori Sato
*
* SPDX-License-Identifier: GPL-2.0-or-later
*/
@@ -10,45 +10,91 @@
#define HW_CHAR_RENESAS_SCI_H
#include "chardev/char-fe.h"
+#include "qemu/timer.h"
+#include "qemu/fifo8.h"
#include "hw/sysbus.h"
#include "qom/object.h"
-#define TYPE_RENESAS_SCI "renesas-sci"
-typedef struct RSCIState RSCIState;
-DECLARE_INSTANCE_CHECKER(RSCIState, RSCI,
- TYPE_RENESAS_SCI)
+#define TYPE_RENESAS_SCI_BASE "renesas-sci-base"
+#define TYPE_RENESAS_SCIA "renesas-scia"
+
+OBJECT_DECLARE_TYPE(RenesasSCIBaseState, RenesasSCIBaseClass,
+ RENESAS_SCI_BASE)
+OBJECT_DECLARE_TYPE(RenesasSCIAState, RenesasSCIAClass,
+ RENESAS_SCIA)
enum {
ERI = 0,
RXI = 1,
TXI = 2,
- TEI = 3,
- SCI_NR_IRQ = 4
+ BRI_TEI = 3,
+ SCI_NR_IRQ = 4,
+};
+
+enum {
+ RXNEXT,
+ TXEMPTY,
+ TXEND,
+ NR_SCI_EVENT,
};
-struct RSCIState {
+enum {
+ SCI_REGWIDTH_8 = 8,
+ SCI_REGWIDTH_16 = 16,
+ SCI_REGWIDTH_32 = 32,
+};
+
+typedef struct RenesasSCIBaseState {
/*< private >*/
SysBusDevice parent_obj;
- /*< public >*/
MemoryRegion memory;
- QEMUTimer timer;
+ QEMUTimer *event_timer;
+
+ /*< public >*/
+ uint64_t input_freq;
+ int64_t etu;
+ int64_t trtime;
+ int64_t tx_start_time;
+ Fifo8 rxfifo;
+ int regshift;
+ uint32_t unit;
CharBackend chr;
qemu_irq irq[SCI_NR_IRQ];
+ struct {
+ int64_t time;
+ int64_t (*handler)(struct RenesasSCIBaseState *sci);
+ } event[NR_SCI_EVENT];
+ /* common SCI register */
uint8_t smr;
uint8_t brr;
uint8_t scr;
uint8_t tdr;
- uint8_t ssr;
- uint8_t rdr;
+ uint16_t Xsr;
+} RenesasSCIBaseState;
+
+struct RenesasSCIAState {
+ RenesasSCIBaseState parent_obj;
+
+ /* SCIa specific register */
uint8_t scmr;
uint8_t semr;
-
- uint8_t read_ssr;
- int64_t trtime;
- int64_t rx_next;
- uint64_t input_freq;
};
+typedef struct RenesasSCIBaseClass {
+ SysBusDeviceClass parent;
+
+ const struct MemoryRegionOps *ops;
+ void (*irq_fn)(struct RenesasSCIBaseState *sci, int request);
+ int (*divrate)(struct RenesasSCIBaseState *sci);
+} RenesasSCIBaseClass;
+
+typedef struct RenesasSCIAClass {
+ RenesasSCIBaseClass parent;
+
+ void (*p_irq_fn)(struct RenesasSCIBaseState *sci, int request);
+ int (*p_divrate)(struct RenesasSCIBaseState *sci);
+} RenesasSCIAClass;
+
#endif
diff --git a/include/hw/rx/rx62n.h b/include/hw/rx/rx62n.h
index 3ed80dba0d..d6e6e168f9 100644
--- a/include/hw/rx/rx62n.h
+++ b/include/hw/rx/rx62n.h
@@ -57,7 +57,7 @@ struct RX62NState {
RXICUState icu;
RTMRState tmr[RX62N_NR_TMR];
RCMTState cmt[RX62N_NR_CMT];
- RSCIState sci[RX62N_NR_SCI];
+ RenesasSCIAState sci[RX62N_NR_SCI];
MemoryRegion *sysmem;
bool kernel;
diff --git a/hw/char/renesas_sci.c b/hw/char/renesas_sci.c
index 1c63467290..c1126b7817 100644
--- a/hw/char/renesas_sci.c
+++ b/hw/char/renesas_sci.c
@@ -4,7 +4,7 @@
* Datasheet: RX62N Group, RX621 Group User's Manual: Hardware
* (Rev.1.40 R01UH0033EJ0140)
*
- * Copyright (c) 2019 Yoshinori Sato
+ * Copyright (c) 2020 Yoshinori Sato
*
* SPDX-License-Identifier: GPL-2.0-or-later
*
@@ -23,15 +23,22 @@
#include "qemu/osdep.h"
#include "qemu/log.h"
+#include "qapi/error.h"
+#include "qemu-common.h"
+#include "hw/hw.h"
#include "hw/irq.h"
+#include "hw/sysbus.h"
#include "hw/registerfields.h"
-#include "hw/qdev-properties.h"
#include "hw/qdev-properties-system.h"
#include "hw/char/renesas_sci.h"
#include "migration/vmstate.h"
+#include "qemu/error-report.h"
-/* SCI register map */
-REG8(SMR, 0)
+/*
+ * SCI register map
+ * SCI(a) register size all 8bit.
+ */
+REG32(SMR, 0) /* 8bit */
FIELD(SMR, CKS, 0, 2)
FIELD(SMR, MP, 2, 1)
FIELD(SMR, STOP, 3, 1)
@@ -39,263 +46,447 @@ REG8(SMR, 0)
FIELD(SMR, PE, 5, 1)
FIELD(SMR, CHR, 6, 1)
FIELD(SMR, CM, 7, 1)
-REG8(BRR, 1)
-REG8(SCR, 2)
- FIELD(SCR, CKE, 0, 2)
+REG32(BRR, 4) /* 8bit */
+REG32(SCR, 8)
+ FIELD(SCR, CKE, 0, 2)
FIELD(SCR, TEIE, 2, 1)
FIELD(SCR, MPIE, 3, 1)
+ FIELD(SCR, REIE, 3, 1)
FIELD(SCR, RE, 4, 1)
FIELD(SCR, TE, 5, 1)
FIELD(SCR, RIE, 6, 1)
FIELD(SCR, TIE, 7, 1)
-REG8(TDR, 3)
-REG8(SSR, 4)
+REG32(TDR, 12) /* 8bit */
+REG32(SSR, 16) /* 8bit */
FIELD(SSR, MPBT, 0, 1)
FIELD(SSR, MPB, 1, 1)
FIELD(SSR, TEND, 2, 1)
- FIELD(SSR, ERR, 3, 3)
+ FIELD(SSR, ERR, 3, 3)
FIELD(SSR, PER, 3, 1)
FIELD(SSR, FER, 4, 1)
FIELD(SSR, ORER, 5, 1)
FIELD(SSR, RDRF, 6, 1)
FIELD(SSR, TDRE, 7, 1)
-REG8(RDR, 5)
-REG8(SCMR, 6)
+REG32(RDR, 20) /* 8bit */
+REG32(SCMR, 24) /* 8bit */
FIELD(SCMR, SMIF, 0, 1)
FIELD(SCMR, SINV, 2, 1)
FIELD(SCMR, SDIR, 3, 1)
FIELD(SCMR, BCP2, 7, 1)
-REG8(SEMR, 7)
+REG8(SEMR, 28)
FIELD(SEMR, ACS0, 0, 1)
FIELD(SEMR, ABCS, 4, 1)
-static int can_receive(void *opaque)
+#define SCIF_FIFO_DEPTH 16
+
+static int sci_can_receive(void *opaque)
{
- RSCIState *sci = RSCI(opaque);
- if (sci->rx_next > qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL)) {
- return 0;
+ RenesasSCIBaseState *sci = RENESAS_SCI_BASE(opaque);
+ if (FIELD_EX16(sci->scr, SCR, RE)) {
+ return fifo8_num_free(&sci->rxfifo);
} else {
- return FIELD_EX8(sci->scr, SCR, RE);
+ /* Receiver disabled. can't receive. */
+ return 0;
}
}
-static void receive(void *opaque, const uint8_t *buf, int size)
+static void update_expire_time(RenesasSCIBaseState *sci)
{
- RSCIState *sci = RSCI(opaque);
- sci->rx_next = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + sci->trtime;
- if (FIELD_EX8(sci->ssr, SSR, RDRF) || size > 1) {
- sci->ssr = FIELD_DP8(sci->ssr, SSR, ORER, 1);
- if (FIELD_EX8(sci->scr, SCR, RIE)) {
- qemu_set_irq(sci->irq[ERI], 1);
+ int64_t next;
+ int i;
+
+ next = INT64_MAX;
+ for (i = 0; i < NR_SCI_EVENT; i++) {
+ if (sci->event[i].time > 0) {
+ next = MIN(next, sci->event[i].time);
}
+ }
+ if (next < INT64_MAX) {
+ timer_mod(sci->event_timer, next);
} else {
- sci->rdr = buf[0];
- sci->ssr = FIELD_DP8(sci->ssr, SSR, RDRF, 1);
- if (FIELD_EX8(sci->scr, SCR, RIE)) {
- qemu_irq_pulse(sci->irq[RXI]);
+ timer_del(sci->event_timer);
+ }
+}
+
+static void update_event_time(RenesasSCIBaseState *sci, int evt, int64_t t)
+{
+ if (t > 0) {
+ t += qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
+ sci->event[evt].time = t;
+ } else {
+ sci->event[evt].time = 0;
+ }
+ update_expire_time(sci);
+}
+
+static void sci_receive(void *opaque, const uint8_t *buf, int size)
+{
+ RenesasSCIBaseState *sci = RENESAS_SCI_BASE(opaque);
+ RenesasSCIBaseClass *rc = RENESAS_SCI_BASE_GET_CLASS(sci);
+ fifo8_push_all(&sci->rxfifo, buf, size);
+ if (FIELD_EX16(sci->scr, SCR, RE)) {
+ if (sci->event[RXNEXT].time == 0) {
+ /* Receiver wake up */
+ sci->Xsr = FIELD_DP16(sci->Xsr, SSR, RDRF, 1);
+ rc->irq_fn(sci, RXI);
+ update_event_time(sci, RXNEXT, sci->trtime);
}
}
}
-static void send_byte(RSCIState *sci)
+static void scia_irq(RenesasSCIBaseState *sci, int req)
+{
+ int irq = 0;
+ int rie;
+ int tie;
+
+ rie = FIELD_EX16(sci->scr, SCR, RIE);
+ tie = FIELD_EX16(sci->scr, SCR, TIE);
+ switch (req) {
+ case ERI:
+ irq = (FIELD_EX16(sci->Xsr, SSR, ERR) != 0) && rie;
+ break;
+ case RXI:
+ irq = FIELD_EX16(sci->Xsr, SSR, RDRF) && rie;
+ break;
+ case TXI:
+ irq = FIELD_EX16(sci->Xsr, SSR, TDRE) && tie;
+ break;
+ case BRI_TEI:
+ irq = FIELD_EX16(sci->Xsr, SSR, TEND) &&
+ FIELD_EX16(sci->scr, SCR, TEIE);
+ break;
+ }
+ if (req == RXI || req == TXI) {
+ if (irq) {
+ qemu_irq_pulse(sci->irq[req]);
+ }
+ } else {
+ qemu_set_irq(sci->irq[req], irq);
+ }
+}
+
+static void sci_send_byte(RenesasSCIBaseState *sci)
{
if (qemu_chr_fe_backend_connected(&sci->chr)) {
qemu_chr_fe_write_all(&sci->chr, &sci->tdr, 1);
}
- timer_mod(&sci->timer, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + sci->trtime);
- sci->ssr = FIELD_DP8(sci->ssr, SSR, TEND, 0);
- sci->ssr = FIELD_DP8(sci->ssr, SSR, TDRE, 1);
- qemu_set_irq(sci->irq[TEI], 0);
- if (FIELD_EX8(sci->scr, SCR, TIE)) {
- qemu_irq_pulse(sci->irq[TXI]);
+ sci->Xsr = FIELD_DP16(sci->Xsr, SSR, TEND, 0);
+ sci->Xsr = FIELD_DP16(sci->Xsr, SSR, TDRE, 1);
+}
+
+static int64_t sci_rx_next(RenesasSCIBaseState *sci)
+{
+ RenesasSCIBaseClass *rc = RENESAS_SCI_BASE_GET_CLASS(sci);
+ if (!fifo8_is_empty(&sci->rxfifo)) {
+ /* have receive charactor */
+ if (FIELD_EX16(sci->Xsr, SSR, RDRF)) {
+ /* Receiver overrun */
+ sci->Xsr = FIELD_DP16(sci->Xsr, SSR, ORER, 1);
+ rc->irq_fn(sci, ERI);
+ return 0;
+ }
+ sci->Xsr = FIELD_DP16(sci->Xsr, SSR, RDRF, 1);
+ rc->irq_fn(sci, RXI);
+ /* next receive time */
+ return sci->trtime;
+ } else {
+ /* No receive charactor. move to idle state */
+ return 0;
}
}
-static void txend(void *opaque)
+static int64_t sci_tx_empty(RenesasSCIBaseState *sci)
{
- RSCIState *sci = RSCI(opaque);
- if (!FIELD_EX8(sci->ssr, SSR, TDRE)) {
- send_byte(sci);
+ int64_t ret = 0;
+ RenesasSCIBaseClass *rc = RENESAS_SCI_BASE_GET_CLASS(sci);
+ if (!FIELD_EX16(sci->Xsr, SSR, TDRE)) {
+ sci_send_byte(sci);
+ ret = sci->trtime;
+ rc->irq_fn(sci, TXI);
} else {
- sci->ssr = FIELD_DP8(sci->ssr, SSR, TEND, 1);
- if (FIELD_EX8(sci->scr, SCR, TEIE)) {
- qemu_set_irq(sci->irq[TEI], 1);
+ sci->Xsr = FIELD_DP16(sci->Xsr, SSR, TEND, 1);
+ rc->irq_fn(sci, BRI_TEI);
+ }
+ return ret;
+}
+
+static void sci_timer_event(void *opaque)
+{
+ RenesasSCIBaseState *sci = RENESAS_SCI_BASE(opaque);
+ int64_t now, t;
+ int i;
+
+ now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
+ for (i = 0; i < NR_SCI_EVENT; i++) {
+ if (sci->event[i].time > 0 && sci->event[i].time <= now) {
+ t = sci->event[i].handler(sci);
+ if (t > 0) {
+ sci->event[i].time = now + t;
+ } else {
+ /* No next event */
+ sci->event[i].time = 0;
+ }
}
}
+ update_expire_time(sci);
}
-static void update_trtime(RSCIState *sci)
+static int scia_divrate(RenesasSCIBaseState *sci)
{
- /* char per bits */
- sci->trtime = 8 - FIELD_EX8(sci->smr, SMR, CHR);
- sci->trtime += FIELD_EX8(sci->smr, SMR, PE);
- sci->trtime += FIELD_EX8(sci->smr, SMR, STOP) + 1;
- /* x bit transmit time (32 * divrate * brr) / base freq */
- sci->trtime *= 32 * sci->brr;
- sci->trtime *= 1 << (2 * FIELD_EX8(sci->smr, SMR, CKS));
- sci->trtime *= NANOSECONDS_PER_SECOND;
- sci->trtime /= sci->input_freq;
+ /*
+ * SEMR.ABCS = 0 -> 32
+ * SEMR.ABCS = 1 -> 16
+ */
+ RenesasSCIAState *scia = RENESAS_SCIA(sci);
+ return 16 * (2 - FIELD_EX8(scia->semr, SEMR, ABCS));
}
-static bool sci_is_tr_enabled(RSCIState *sci)
+static void update_trtime(RenesasSCIBaseState *sci)
{
- return FIELD_EX8(sci->scr, SCR, TE) || FIELD_EX8(sci->scr, SCR, RE);
+ RenesasSCIBaseClass *rc = RENESAS_SCI_BASE_GET_CLASS(sci);
+ int cks = 1 << (2 * FIELD_EX16(sci->smr, SMR, CKS));
+ if (sci->input_freq > 0) {
+ /* x bit transmit time (divrate * brr) / base freq */
+ sci->etu = rc->divrate(sci) * cks;
+ sci->etu *= sci->brr + 1;
+ sci->etu *= NANOSECONDS_PER_SECOND;
+ sci->etu /= sci->input_freq;
+
+ /* char per bits */
+ sci->trtime = 8 - FIELD_EX16(sci->smr, SMR, CHR);
+ sci->trtime += FIELD_EX16(sci->smr, SMR, PE);
+ sci->trtime += FIELD_EX16(sci->smr, SMR, STOP) + 1 + 1;
+ sci->trtime *= sci->etu;
+ }
}
-static void sci_write(void *opaque, hwaddr offset, uint64_t val, unsigned size)
+#define IS_TR_ENABLED(scr) \
+ (FIELD_EX16(scr, SCR, TE) || FIELD_EX16(scr, SCR, RE))
+
+static hwaddr map_address(RenesasSCIBaseState *sci, hwaddr addr)
{
- RSCIState *sci = RSCI(opaque);
+ return addr << (2 - sci->regshift);
+}
- switch (offset) {
- case A_SMR:
- if (!sci_is_tr_enabled(sci)) {
- sci->smr = val;
- update_trtime(sci);
+static void sci_common_write(void *opaque, hwaddr addr,
+ uint64_t val, unsigned size)
+{
+ RenesasSCIBaseState *sci = RENESAS_SCI_BASE(opaque);
+ RenesasSCIBaseClass *rc = RENESAS_SCI_BASE_GET_CLASS(opaque);
+ switch (addr) {
+ case A_SCR:
+ sci->scr = val;
+ if (FIELD_EX16(sci->scr, SCR, TE)) {
+ /* Transmitter enable */
+ sci->Xsr = FIELD_DP16(sci->Xsr, SSR, TDRE, 1);
+ sci->Xsr = FIELD_DP16(sci->Xsr, SSR, TEND, 1);
+ rc->irq_fn(sci, TXI);
+ rc->irq_fn(sci, BRI_TEI);
+ } else {
+ /* Transmitter disable */
+ update_event_time(sci, TXEND, 0);
+ update_event_time(sci, TXEMPTY, 0);
}
break;
+ case A_SMR:
+ sci->smr = val;
+ update_trtime(sci);
+ break;
case A_BRR:
- if (!sci_is_tr_enabled(sci)) {
- sci->brr = val;
- update_trtime(sci);
- }
+ sci->brr = val;
+ update_trtime(sci);
break;
- case A_SCR:
- sci->scr = val;
- if (FIELD_EX8(sci->scr, SCR, TE)) {
- sci->ssr = FIELD_DP8(sci->ssr, SSR, TDRE, 1);
- sci->ssr = FIELD_DP8(sci->ssr, SSR, TEND, 1);
- if (FIELD_EX8(sci->scr, SCR, TIE)) {
- qemu_irq_pulse(sci->irq[TXI]);
- }
- }
- if (!FIELD_EX8(sci->scr, SCR, TEIE)) {
- qemu_set_irq(sci->irq[TEI], 0);
+ default:
+ qemu_log_mask(LOG_UNIMP, "renesas_sci: Register 0x%" HWADDR_PRIX
+ " not implemented\n", addr);
+ }
+}
+
+static void scia_write(void *opaque, hwaddr addr, uint64_t val, unsigned size)
+{
+ RenesasSCIBaseState *sci = RENESAS_SCI_BASE(opaque);
+ RenesasSCIAState *scia = RENESAS_SCIA(opaque);
+
+ addr = map_address(sci, addr);
+ switch (addr) {
+ case A_SMR:
+ if (IS_TR_ENABLED(sci->scr)) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "reneas_sci: SMR write protected.\n");
+ } else {
+ sci_common_write(sci, addr, val, size);
}
- if (!FIELD_EX8(sci->scr, SCR, RIE)) {
- qemu_set_irq(sci->irq[ERI], 0);
+ break;
+ case A_BRR:
+ if (IS_TR_ENABLED(sci->scr)) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "reneas_sci: BRR write protected.\n");
+ break;
+ } else {
+ sci_common_write(sci, addr, val, size);
}
break;
case A_TDR:
sci->tdr = val;
- if (FIELD_EX8(sci->ssr, SSR, TEND)) {
- send_byte(sci);
+ if (FIELD_EX16(sci->Xsr, SSR, TEND)) {
+ /* Transmitter wakeup */
+ update_event_time(sci, TXEMPTY, sci->trtime);
+ sci_send_byte(sci);
} else {
- sci->ssr = FIELD_DP8(sci->ssr, SSR, TDRE, 0);
+ sci->Xsr = FIELD_DP16(sci->Xsr, SSR, TDRE, 0);
}
+ /* Clear TEI */
+ scia_irq(sci, BRI_TEI);
break;
case A_SSR:
- sci->ssr = FIELD_DP8(sci->ssr, SSR, MPBT,
- FIELD_EX8(val, SSR, MPBT));
- sci->ssr = FIELD_DP8(sci->ssr, SSR, ERR,
- FIELD_EX8(val, SSR, ERR) & 0x07);
- if (FIELD_EX8(sci->read_ssr, SSR, ERR) &&
- FIELD_EX8(sci->ssr, SSR, ERR) == 0) {
- qemu_set_irq(sci->irq[ERI], 0);
+ /* SSR.MBP and SSR.TEND is read only */
+ val = FIELD_DP16(val, SSR, MPB, 1);
+ val = FIELD_DP16(val, SSR, TEND, 1);
+ /* SSR.RDRF and SSR.TDRE can write 1 */
+ if (FIELD_EX16(val, SSR, RDRF) == 0 ||
+ FIELD_EX16(val, SSR, TDRE) == 0) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "reneas_sci: SSR invalid write value %02lux.\n", val);
}
- break;
- case A_RDR:
- qemu_log_mask(LOG_GUEST_ERROR, "reneas_sci: RDR is read only.\n");
+ val = FIELD_DP16(val, SSR, RDRF, 1);
+ val = FIELD_DP16(val, SSR, TDRE, 1);
+ /* SSR.MBP and SSR.TEND is read only */
+ val = FIELD_DP16(val, SSR, MPB, 1);
+ val = FIELD_DP16(val, SSR, TEND, 1);
+ /* SSR.PER, SSR.FER and SSR.ORER can write only 0 */
+ sci->Xsr &= val;
+ /* SSR.MPBT can write any value */
+ sci->Xsr = FIELD_DP16(RENESAS_SCI_BASE(sci)->Xsr, SSR, MPBT,
+ FIELD_EX16(val, SSR, MPBT));
+ /* Clear ERI */
+ scia_irq(sci, ERI);
break;
case A_SCMR:
- sci->scmr = val; break;
- case A_SEMR: /* SEMR */
- sci->semr = val; break;
+ scia->scmr = val;
+ break;
+ case A_SEMR:
+ scia->semr = val;
+ break;
default:
- qemu_log_mask(LOG_UNIMP, "renesas_sci: Register 0x%" HWADDR_PRIX " "
- "not implemented\n",
- offset);
+ sci_common_write(sci, addr, val, size);
+ break;
}
}
-static uint64_t sci_read(void *opaque, hwaddr offset, unsigned size)
+static uint64_t sci_common_read(void *opaque, hwaddr addr, unsigned size)
{
- RSCIState *sci = RSCI(opaque);
-
- switch (offset) {
+ RenesasSCIBaseState *sci = RENESAS_SCI_BASE(opaque);
+ switch (addr) {
case A_SMR:
return sci->smr;
case A_BRR:
return sci->brr;
case A_SCR:
return sci->scr;
+ case A_SSR:
+ return sci->Xsr;
case A_TDR:
return sci->tdr;
- case A_SSR:
- sci->read_ssr = sci->ssr;
- return sci->ssr;
case A_RDR:
- sci->ssr = FIELD_DP8(sci->ssr, SSR, RDRF, 0);
- return sci->rdr;
- case A_SCMR:
- return sci->scmr;
- case A_SEMR:
- return sci->semr;
+ if (fifo8_num_used(&sci->rxfifo) > 0) {
+ return fifo8_pop(&sci->rxfifo);
+ } else {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "renesas_sci: Receiver underrun.");
+ return 0xff;
+ }
default:
qemu_log_mask(LOG_UNIMP, "renesas_sci: Register 0x%" HWADDR_PRIX
- " not implemented.\n", offset);
+ " not implemented.\n", addr);
}
return UINT64_MAX;
}
-static const MemoryRegionOps sci_ops = {
- .write = sci_write,
- .read = sci_read,
- .endianness = DEVICE_NATIVE_ENDIAN,
- .impl.max_access_size = 1,
- .valid.max_access_size = 1,
-};
+static uint64_t scia_read(void *opaque, hwaddr addr, unsigned size)
+{
+ RenesasSCIBaseState *sci = RENESAS_SCI_BASE(opaque);
+ RenesasSCIAState *scia = RENESAS_SCIA(opaque);
+
+ addr = map_address(sci, addr);
+ switch (addr) {
+ case A_RDR:
+ sci->Xsr = FIELD_DP16(sci->Xsr, SSR, RDRF, 0);
+ return sci_common_read(sci, addr, size);
+ case A_SCMR:
+ return scia->scmr;
+ default:
+ return sci_common_read(sci, addr, size);
+ }
+ return UINT64_MAX;
+}
-static void rsci_reset(DeviceState *dev)
+static void rsci_common_init(Object *obj)
{
- RSCIState *sci = RSCI(dev);
- sci->smr = sci->scr = 0x00;
- sci->brr = 0xff;
- sci->tdr = 0xff;
- sci->rdr = 0x00;
- sci->ssr = 0x84;
- sci->scmr = 0x00;
- sci->semr = 0x00;
- sci->rx_next = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
+ RenesasSCIBaseState *sci = RENESAS_SCI_BASE(obj);
+ SysBusDevice *d = SYS_BUS_DEVICE(obj);
+ int i;
+
+ for (i = 0; i < SCI_NR_IRQ; i++) {
+ sysbus_init_irq(d, &sci->irq[i]);
+ }
+ fifo8_create(&sci->rxfifo, SCIF_FIFO_DEPTH);
+ sci->event_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, sci_timer_event, sci);
}
static void sci_event(void *opaque, QEMUChrEvent event)
{
- RSCIState *sci = RSCI(opaque);
+ RenesasSCIBaseState *sci = RENESAS_SCI_BASE(opaque);
+ RenesasSCIBaseClass *rc = RENESAS_SCI_BASE_GET_CLASS(sci);
if (event == CHR_EVENT_BREAK) {
- sci->ssr = FIELD_DP8(sci->ssr, SSR, FER, 1);
- if (FIELD_EX8(sci->scr, SCR, RIE)) {
- qemu_set_irq(sci->irq[ERI], 1);
- }
+ sci->Xsr = FIELD_DP16(sci->Xsr, SSR, FER, 1);
+ rc->irq_fn(sci, BRI_TEI);
}
}
-static void rsci_realize(DeviceState *dev, Error **errp)
+static void rsci_common_realize(DeviceState *dev, Error **errp)
{
- RSCIState *sci = RSCI(dev);
+ RenesasSCIBaseState *sci = RENESAS_SCI_BASE(dev);
+ int r;
- if (sci->input_freq == 0) {
+ r = sci->regshift;
+ if ((r % 8) != 0 || ((r / 8) >> 1) > 2) {
qemu_log_mask(LOG_GUEST_ERROR,
- "renesas_sci: input-freq property must be set.");
+ "renesas_sci: Invalid register size.");
return;
}
- qemu_chr_fe_set_handlers(&sci->chr, can_receive, receive,
- sci_event, NULL, sci, NULL, true);
+ sci->regshift = (r / 8) >> 1;
+ sci->smr = sci->scr = 0x00;
+ sci->brr = 0xff;
+ sci->tdr = 0xff;
+ sci->Xsr = 0x84;
+ update_trtime(sci);
+
}
-static void rsci_init(Object *obj)
+static void register_mmio(RenesasSCIBaseState *sci, int size)
{
- SysBusDevice *d = SYS_BUS_DEVICE(obj);
- RSCIState *sci = RSCI(obj);
- int i;
+ SysBusDevice *d = SYS_BUS_DEVICE(sci);
+ RenesasSCIBaseClass *rc = RENESAS_SCI_BASE_GET_CLASS(sci);
- memory_region_init_io(&sci->memory, OBJECT(sci), &sci_ops,
- sci, "renesas-sci", 0x8);
+ memory_region_init_io(&sci->memory, OBJECT(sci), rc->ops,
+ sci, "renesas-sci", size);
sysbus_init_mmio(d, &sci->memory);
+}
- for (i = 0; i < SCI_NR_IRQ; i++) {
- sysbus_init_irq(d, &sci->irq[i]);
- }
- timer_init_ns(&sci->timer, QEMU_CLOCK_VIRTUAL, txend, sci);
+static void rscia_realize(DeviceState *dev, Error **errp)
+{
+ RenesasSCIAState *sci = RENESAS_SCIA(dev);
+ RenesasSCIBaseState *common = RENESAS_SCI_BASE(dev);
+
+ rsci_common_realize(dev, errp);
+
+ register_mmio(common, 8 * (1 << common->regshift));
+ qemu_chr_fe_set_handlers(&common->chr, sci_can_receive, sci_receive,
+ sci_event, NULL, sci, NULL, true);
+
+ sci->scmr = 0x00;
+ sci->semr = 0x00;
}
static const VMStateDescription vmstate_rsci = {
@@ -303,49 +494,74 @@ static const VMStateDescription vmstate_rsci = {
.version_id = 1,
.minimum_version_id = 1,
.fields = (VMStateField[]) {
- VMSTATE_INT64(trtime, RSCIState),
- VMSTATE_INT64(rx_next, RSCIState),
- VMSTATE_UINT8(smr, RSCIState),
- VMSTATE_UINT8(brr, RSCIState),
- VMSTATE_UINT8(scr, RSCIState),
- VMSTATE_UINT8(tdr, RSCIState),
- VMSTATE_UINT8(ssr, RSCIState),
- VMSTATE_UINT8(rdr, RSCIState),
- VMSTATE_UINT8(scmr, RSCIState),
- VMSTATE_UINT8(semr, RSCIState),
- VMSTATE_UINT8(read_ssr, RSCIState),
- VMSTATE_TIMER(timer, RSCIState),
VMSTATE_END_OF_LIST()
}
};
static Property rsci_properties[] = {
- DEFINE_PROP_UINT64("input-freq", RSCIState, input_freq, 0),
- DEFINE_PROP_CHR("chardev", RSCIState, chr),
+ DEFINE_PROP_UINT64("input-freq", RenesasSCIBaseState, input_freq, 0),
+ DEFINE_PROP_INT32("register-size", RenesasSCIBaseState,
+ regshift, 0),
+ DEFINE_PROP_UINT32("unit", RenesasSCIBaseState, unit, 0),
+ DEFINE_PROP_CHR("chardev", RenesasSCIBaseState, chr),
DEFINE_PROP_END_OF_LIST(),
};
-static void rsci_class_init(ObjectClass *klass, void *data)
+static void rsci_init(Object *obj)
+{
+ RenesasSCIBaseState *sci = RENESAS_SCI_BASE(obj);
+ sci->event[RXNEXT].handler = sci_rx_next;
+ sci->event[TXEMPTY].handler = sci_tx_empty;
+}
+
+static void rsci_common_class_init(ObjectClass *klass, void *data)
{
DeviceClass *dc = DEVICE_CLASS(klass);
- dc->realize = rsci_realize;
dc->vmsd = &vmstate_rsci;
- dc->reset = rsci_reset;
device_class_set_props(dc, rsci_properties);
}
-static const TypeInfo rsci_info = {
- .name = TYPE_RENESAS_SCI,
- .parent = TYPE_SYS_BUS_DEVICE,
- .instance_size = sizeof(RSCIState),
- .instance_init = rsci_init,
- .class_init = rsci_class_init,
+static const MemoryRegionOps scia_ops = {
+ .read = scia_read,
+ .write = scia_write,
+ .endianness = DEVICE_NATIVE_ENDIAN,
+ .valid = {
+ .min_access_size = 1,
+ .max_access_size = 1,
+ },
};
-static void rsci_register_types(void)
+static void rscia_class_init(ObjectClass *klass, void *data)
{
- type_register_static(&rsci_info);
+ RenesasSCIBaseClass *comm_rc = RENESAS_SCI_BASE_CLASS(klass);
+ DeviceClass *dc = DEVICE_CLASS(klass);
+
+ comm_rc->ops = &scia_ops;
+ comm_rc->irq_fn = scia_irq;
+ comm_rc->divrate = scia_divrate;
+
+ dc->realize = rscia_realize;
}
-type_init(rsci_register_types)
+static const TypeInfo renesas_sci_info[] = {
+ {
+ .name = TYPE_RENESAS_SCI_BASE,
+ .parent = TYPE_SYS_BUS_DEVICE,
+ .instance_size = sizeof(RenesasSCIBaseState),
+ .instance_init = rsci_common_init,
+ .class_init = rsci_common_class_init,
+ .class_size = sizeof(RenesasSCIBaseClass),
+ .abstract = true,
+ },
+ {
+ .name = TYPE_RENESAS_SCIA,
+ .parent = TYPE_RENESAS_SCI_BASE,
+ .instance_size = sizeof(RenesasSCIAState),
+ .instance_init = rsci_init,
+ .class_init = rscia_class_init,
+ .class_size = sizeof(RenesasSCIAClass),
+ },
+};
+
+DEFINE_TYPES(renesas_sci_info)
diff --git a/hw/rx/rx62n.c b/hw/rx/rx62n.c
index fa5add9f9d..32c44ead1f 100644
--- a/hw/rx/rx62n.c
+++ b/hw/rx/rx62n.c
@@ -199,7 +199,7 @@ static void register_sci(RX62NState *s, int unit)
int i, irqbase;
object_initialize_child(OBJECT(s), "sci[*]",
- &s->sci[unit], TYPE_RENESAS_SCI);
+ &s->sci[unit], TYPE_RENESAS_SCIA);
sci = SYS_BUS_DEVICE(&s->sci[unit]);
qdev_prop_set_chr(DEVICE(sci), "chardev", serial_hd(unit));
qdev_prop_set_uint64(DEVICE(sci), "input-freq", s->pclk_freq_hz);
--
2.20.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/3] hw/char: renesas_sci Add SCI and SCIF support.
2021-06-16 9:12 [PATCH 0/3] renesas_sci update Yoshinori Sato
2021-06-16 9:12 ` [PATCH 1/3] hw/char: renesas_sci: Refactor for merge all SCI variant Yoshinori Sato
@ 2021-06-16 9:12 ` Yoshinori Sato
2021-06-29 12:37 ` Peter Maydell
2021-06-16 9:12 ` [PATCH 3/3] hw/sh4: sh7750 using renesas_sci Yoshinori Sato
2021-06-16 9:27 ` [PATCH 0/3] renesas_sci update no-reply
3 siblings, 1 reply; 9+ messages in thread
From: Yoshinori Sato @ 2021-06-16 9:12 UTC (permalink / raw)
To: qemu-devel; +Cc: Yoshinori Sato
This peripheral using SH7750.
Signed-off-by: Yoshinori Sato <ysato@users.sourceforge.jp>
---
include/hw/char/renesas_sci.h | 43 ++-
hw/char/renesas_sci.c | 489 ++++++++++++++++++++++++++++++++++
2 files changed, 531 insertions(+), 1 deletion(-)
diff --git a/include/hw/char/renesas_sci.h b/include/hw/char/renesas_sci.h
index c666cf81d1..aa6be53628 100644
--- a/include/hw/char/renesas_sci.h
+++ b/include/hw/char/renesas_sci.h
@@ -16,12 +16,18 @@
#include "qom/object.h"
#define TYPE_RENESAS_SCI_BASE "renesas-sci-base"
+#define TYPE_RENESAS_SCI "renesas-sci"
#define TYPE_RENESAS_SCIA "renesas-scia"
+#define TYPE_RENESAS_SCIF "renesas-scif"
OBJECT_DECLARE_TYPE(RenesasSCIBaseState, RenesasSCIBaseClass,
RENESAS_SCI_BASE)
+OBJECT_DECLARE_TYPE(RenesasSCIState, RenesasSCIClass,
+ RENESAS_SCI)
OBJECT_DECLARE_TYPE(RenesasSCIAState, RenesasSCIAClass,
RENESAS_SCIA)
+OBJECT_DECLARE_TYPE(RenesasSCIFState, RenesasSCIFClass,
+ RENESAS_SCIF)
enum {
ERI = 0,
@@ -32,6 +38,7 @@ enum {
};
enum {
+ RXTOUT,
RXNEXT,
TXEMPTY,
TXEND,
@@ -49,13 +56,14 @@ typedef struct RenesasSCIBaseState {
SysBusDevice parent_obj;
MemoryRegion memory;
+ MemoryRegion memory_p4;
+ MemoryRegion memory_a7;
QEMUTimer *event_timer;
/*< public >*/
uint64_t input_freq;
int64_t etu;
int64_t trtime;
- int64_t tx_start_time;
Fifo8 rxfifo;
int regshift;
uint32_t unit;
@@ -65,6 +73,7 @@ typedef struct RenesasSCIBaseState {
int64_t time;
int64_t (*handler)(struct RenesasSCIBaseState *sci);
} event[NR_SCI_EVENT];
+ uint16_t read_Xsr;
/* common SCI register */
uint8_t smr;
@@ -74,6 +83,13 @@ typedef struct RenesasSCIBaseState {
uint16_t Xsr;
} RenesasSCIBaseState;
+struct RenesasSCIState {
+ RenesasSCIBaseState parent_obj;
+
+ /* SCI specific register */
+ uint8_t sptr;
+};
+
struct RenesasSCIAState {
RenesasSCIBaseState parent_obj;
@@ -82,6 +98,19 @@ struct RenesasSCIAState {
uint8_t semr;
};
+struct RenesasSCIFState {
+ RenesasSCIBaseState parent_obj;
+
+ /* SCIF specific register */
+ uint16_t fcr;
+ uint16_t sptr;
+ uint16_t lsr;
+
+ /* private */
+ int64_t tx_fifo_top_t;
+ int txremain;
+};
+
typedef struct RenesasSCIBaseClass {
SysBusDeviceClass parent;
@@ -90,6 +119,12 @@ typedef struct RenesasSCIBaseClass {
int (*divrate)(struct RenesasSCIBaseState *sci);
} RenesasSCIBaseClass;
+typedef struct RenesasSCIClass {
+ RenesasSCIBaseClass parent;
+
+ void (*p_irq_fn)(struct RenesasSCIBaseState *sci, int request);
+} RenesasSCIClass;
+
typedef struct RenesasSCIAClass {
RenesasSCIBaseClass parent;
@@ -97,4 +132,10 @@ typedef struct RenesasSCIAClass {
int (*p_divrate)(struct RenesasSCIBaseState *sci);
} RenesasSCIAClass;
+typedef struct RenesasSCIFClass {
+ RenesasSCIBaseClass parent;
+
+ void (*p_irq_fn)(struct RenesasSCIBaseState *sci, int request);
+} RenesasSCIFClass;
+
#endif
diff --git a/hw/char/renesas_sci.c b/hw/char/renesas_sci.c
index c1126b7817..ba304fa1fa 100644
--- a/hw/char/renesas_sci.c
+++ b/hw/char/renesas_sci.c
@@ -3,6 +3,8 @@
*
* Datasheet: RX62N Group, RX621 Group User's Manual: Hardware
* (Rev.1.40 R01UH0033EJ0140)
+ * And SH7751 Group, SH7751R Group User's Manual: Hardware
+ * (Rev.4.01 R01UH0457EJ0401)
*
* Copyright (c) 2020 Yoshinori Sato
*
@@ -67,17 +69,58 @@ REG32(SSR, 16) /* 8bit */
FIELD(SSR, ORER, 5, 1)
FIELD(SSR, RDRF, 6, 1)
FIELD(SSR, TDRE, 7, 1)
+REG32(FSR, 16)
+ FIELD(FSR, DR, 0, 1)
+ FIELD(FSR, RDF, 1, 1)
+ FIELD(FSR, RDF_DR, 0, 2)
+ FIELD(FSR, PER, 2, 1)
+ FIELD(FSR, FER, 3, 1)
+ FIELD(FSR, BRK, 4, 1)
+ FIELD(FSR, TDFE, 5, 1)
+ FIELD(FSR, TEND, 6, 1)
+ FIELD(FSR, ER, 7, 1)
+ FIELD(FSR, FERn, 8, 4)
+ FIELD(FSR, PERn, 12, 4)
REG32(RDR, 20) /* 8bit */
REG32(SCMR, 24) /* 8bit */
FIELD(SCMR, SMIF, 0, 1)
FIELD(SCMR, SINV, 2, 1)
FIELD(SCMR, SDIR, 3, 1)
FIELD(SCMR, BCP2, 7, 1)
+REG32(FCR, 24)
+ FIELD(FCR, LOOP, 0, 1)
+ FIELD(FCR, RFRST, 1, 1)
+ FIELD(FCR, TFRST, 2, 1)
+ FIELD(FCR, MCE, 3, 1)
+ FIELD(FCR, TTRG, 4, 2)
+ FIELD(FCR, RTRG, 6, 2)
+ FIELD(FCR, RSTRG, 8, 3)
REG8(SEMR, 28)
FIELD(SEMR, ACS0, 0, 1)
FIELD(SEMR, ABCS, 4, 1)
+REG32(FDR, 28)
+ FIELD(FDR, Rn, 0, 4)
+ FIELD(FDR, Tn, 8, 4)
+REG32(SPTR, 32)
+ FIELD(SPTR, SPB2DT, 0, 1)
+ FIELD(SPTR, SPB2IO, 1, 1)
+ FIELD(SPTR, SCKDT, 2, 1)
+ FIELD(SPTR, SCKIO, 3, 1)
+ FIELD(SPTR, CTSDT, 4, 1)
+ FIELD(SPTR, CTSIO, 5, 1)
+ FIELD(SPTR, RTSDT, 6, 1)
+ FIELD(SPTR, RTSIO, 7, 1)
+ FIELD(SPTR, EIO, 7, 1)
+REG32(LSR, 36)
+ FIELD(LSR, ORER, 0, 1)
#define SCIF_FIFO_DEPTH 16
+static const int scif_rtrg[] = {1, 4, 8, 14};
+/* TTRG = 0 - 8byte */
+/* TTRG = 1 - 4byte */
+/* TTRG = 2 - 2byte */
+/* TTRG = 3 - 1byte */
+#define scif_ttrg(scif) (1 << (3 - FIELD_EX16(scif->fcr, FCR, TTRG)))
static int sci_can_receive(void *opaque)
{
@@ -134,6 +177,71 @@ static void sci_receive(void *opaque, const uint8_t *buf, int size)
}
}
+static int scif_can_receive(void *opaque)
+{
+ RenesasSCIFState *scif = RENESAS_SCIF(opaque);
+ RenesasSCIBaseState *sci = RENESAS_SCI_BASE(opaque);
+ RenesasSCIBaseClass *rc = RENESAS_SCI_BASE_GET_CLASS(sci);
+ int fifo_free = 0;
+ if (FIELD_EX16(sci->scr, SCR, RE)) {
+ /* Receiver enabled */
+ fifo_free = fifo8_num_free(&sci->rxfifo);
+ if (fifo_free == 0) {
+ /* FIFO overrun */
+ scif->lsr = FIELD_DP16(scif->lsr, LSR, ORER, 1);
+ rc->irq_fn(sci, ERI);
+ }
+ }
+ return fifo_free;
+}
+
+static void scif_receive(void *opaque, const uint8_t *buf, int size)
+{
+ RenesasSCIFState *scif = RENESAS_SCIF(opaque);
+ RenesasSCIBaseState *sci = RENESAS_SCI_BASE(opaque);
+ RenesasSCIBaseClass *rc = RENESAS_SCI_BASE_GET_CLASS(sci);
+ int rtrg;
+
+ fifo8_push_all(&sci->rxfifo, buf, size);
+ if (sci->event[RXNEXT].time == 0) {
+ rtrg = scif_rtrg[FIELD_EX16(scif->fcr, FCR, RTRG)];
+ if (fifo8_num_used(&sci->rxfifo) >= rtrg) {
+ sci->Xsr = FIELD_DP16(sci->Xsr, FSR, RDF, 1);
+ rc->irq_fn(sci, RXI);
+ } else {
+ update_event_time(sci, RXTOUT, 15 * sci->etu);
+ }
+ }
+}
+
+static void sci_irq(RenesasSCIBaseState *sci_common, int req)
+{
+ int irq = 0;
+ int rie;
+ int tie;
+ RenesasSCIState *sci = RENESAS_SCI(sci_common);
+
+ rie = FIELD_EX16(sci_common->scr, SCR, RIE);
+ tie = FIELD_EX16(sci_common->scr, SCR, TIE);
+ switch (req) {
+ case ERI:
+ irq = rie && (FIELD_EX16(sci_common->Xsr, SSR, ERR) != 0);
+ break;
+ case RXI:
+ irq = FIELD_EX16(sci_common->Xsr, SSR, RDRF) && rie &&
+ !FIELD_EX16(sci->sptr, SPTR, EIO);
+ break;
+ case TXI:
+ irq = FIELD_EX16(sci_common->Xsr, SSR, TDRE) && tie;
+ break;
+ case BRI_TEI:
+ irq = FIELD_EX16(sci_common->Xsr, SSR, TEND) &&
+ FIELD_EX16(sci_common->scr, SCR, TEIE);
+ break;
+ }
+ qemu_set_irq(sci_common->irq[req], irq);
+}
+
static void scia_irq(RenesasSCIBaseState *sci, int req)
{
int irq = 0;
@@ -166,6 +274,33 @@ static void scia_irq(RenesasSCIBaseState *sci, int req)
}
}
+static void scif_irq(RenesasSCIBaseState *sci, int req)
+{
+ int irq = 0;
+ int rie;
+ int reie;
+ int tie;
+
+ rie = FIELD_EX16(sci->scr, SCR, RIE);
+ reie = FIELD_EX16(sci->scr, SCR, REIE);
+ tie = FIELD_EX16(sci->scr, SCR, TIE);
+ switch (req) {
+ case ERI:
+ irq = (rie || reie) && FIELD_EX16(sci->Xsr, FSR, ER);
+ break;
+ case RXI:
+ irq = (FIELD_EX16(sci->Xsr, FSR, RDF_DR) != 0) && rie;
+ break;
+ case TXI:
+ irq = FIELD_EX16(sci->Xsr, FSR, TDFE) & tie;
+ break;
+ case BRI_TEI:
+ irq = (rie || reie) && FIELD_EX16(sci->Xsr, FSR, BRK);
+ break;
+ }
+ qemu_set_irq(sci->irq[req], irq);
+}
+
static void sci_send_byte(RenesasSCIBaseState *sci)
{
if (qemu_chr_fe_backend_connected(&sci->chr)) {
@@ -211,6 +346,46 @@ static int64_t sci_tx_empty(RenesasSCIBaseState *sci)
return ret;
}
+static int scif_txremain_byte(RenesasSCIFState *scif)
+{
+ RenesasSCIBaseState *sci = RENESAS_SCI_BASE(scif);
+ int64_t now, elapsed;
+ int byte = 0;
+ if (scif->tx_fifo_top_t > 0) {
+ now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
+ elapsed = now - scif->tx_fifo_top_t;
+ scif->tx_fifo_top_t = now;
+ byte = elapsed / sci->trtime + 1;
+ byte = MIN(scif->txremain, byte);
+ }
+ scif->txremain -= byte;
+ return scif->txremain;
+}
+
+static int64_t scif_rx_timeout(RenesasSCIBaseState *sci)
+{
+ sci->Xsr = FIELD_DP16(sci->Xsr, FSR, DR, 1);
+ scif_irq(sci, RXI);
+ return 0;
+}
+
+static int64_t scif_tx_empty(RenesasSCIBaseState *sci)
+{
+ RenesasSCIFState *scif = RENESAS_SCIF(sci);
+ scif_txremain_byte(scif);
+ sci->Xsr = FIELD_DP16(sci->Xsr, FSR, TDFE, 1);
+ scif_irq(sci, TXI);
+ return 0;
+}
+
+static int64_t scif_tx_end(RenesasSCIBaseState *sci)
+{
+ RenesasSCIFState *scif = RENESAS_SCIF(sci);
+ scif->txremain = 0;
+ sci->Xsr = FIELD_DP16(sci->Xsr, FSR, TEND, 1);
+ return 0;
+}
+
static void sci_timer_event(void *opaque)
{
RenesasSCIBaseState *sci = RENESAS_SCI_BASE(opaque);
@@ -232,6 +407,12 @@ static void sci_timer_event(void *opaque)
update_expire_time(sci);
}
+static int sci_divrate(RenesasSCIBaseState *sci)
+{
+ /* SCI / SCIF have static divide rate */
+ return 32;
+}
+
static int scia_divrate(RenesasSCIBaseState *sci)
{
/*
@@ -303,6 +484,45 @@ static void sci_common_write(void *opaque, hwaddr addr,
}
}
+static void sci_write(void *opaque, hwaddr addr, uint64_t val, unsigned size)
+{
+ RenesasSCIBaseState *sci = RENESAS_SCI_BASE(opaque);
+ RenesasSCIBaseClass *rc = RENESAS_SCI_BASE_GET_CLASS(sci);
+ bool tdre_reset;
+
+ addr = map_address(sci, addr);
+ switch (addr) {
+ case A_TDR:
+ sci->tdr = val;
+ break;
+ case A_SSR:
+ /* SSR.MBP and SSR.TEND is read only */
+ val = FIELD_DP16(val, SSR, MPB, 1);
+ val = FIELD_DP16(val, SSR, TEND, 1);
+ /* SSR can write only 0 */
+ sci->Xsr &= val;
+ /* SSR.MPBT can write any value */
+ sci->Xsr = FIELD_DP16(RENESAS_SCI_BASE(sci)->Xsr, SSR, MPBT,
+ FIELD_EX16(val, SSR, MPBT));
+ /* Clear ERI */
+ rc->irq_fn(sci, ERI);
+ /* Is TX start operation ? */
+ tdre_reset = FIELD_EX16(sci->read_Xsr, SSR, TDRE) &&
+ !FIELD_EX16(sci->Xsr, SSR, TDRE);
+ if (tdre_reset && FIELD_EX16(sci->Xsr, SSR, ERR) == 0) {
+ sci_send_byte(sci);
+ update_event_time(sci, TXEMPTY, sci->trtime);
+ rc->irq_fn(sci, TXI);
+ }
+ break;
+ case A_SPTR:
+ RENESAS_SCI(sci)->sptr = val;
+ break;
+ default:
+ sci_common_write(sci, addr, val, size);
+ }
+}
+
static void scia_write(void *opaque, hwaddr addr, uint64_t val, unsigned size)
{
RenesasSCIBaseState *sci = RENESAS_SCI_BASE(opaque);
@@ -374,6 +594,120 @@ static void scia_write(void *opaque, hwaddr addr, uint64_t val, unsigned size)
}
}
+static void scif_write(void *opaque, hwaddr addr, uint64_t val, unsigned size)
+{
+ RenesasSCIBaseState *sci = RENESAS_SCI_BASE(opaque);
+ RenesasSCIFState *scif = RENESAS_SCIF(opaque);
+ int txtrg;
+ int rxtrg;
+ uint8_t txd;
+
+ addr = map_address(sci, addr);
+ switch (addr) {
+ case A_SCR:
+ sci->scr = val;
+ if (FIELD_EX16(sci->scr, SCR, TE)) {
+ /* Transmiter enable */
+ sci->Xsr = FIELD_DP16(sci->Xsr, FSR, TEND, 1);
+ sci->Xsr = FIELD_DP16(sci->Xsr, FSR, TDFE, 1);
+ scif->tx_fifo_top_t = 0;
+ scif_irq(sci, TXI);
+ } else {
+ /* Transmiter disable */
+ update_event_time(sci, TXEND, 0);
+ update_event_time(sci, TXEMPTY, 0);
+ }
+ break;
+ case A_TDR:
+ if (scif->tx_fifo_top_t > 0) {
+ if (scif_txremain_byte(scif) >= SCIF_FIFO_DEPTH) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "reneas_sci: Tx FIFO is full.");
+ break;
+ }
+ } else {
+ scif->tx_fifo_top_t = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
+ }
+ txd = val;
+ if (qemu_chr_fe_backend_connected(&sci->chr)) {
+ qemu_chr_fe_write_all(&sci->chr, &txd, 1);
+ }
+ if (FIELD_EX16(scif->fcr, FCR, LOOP) && scif_can_receive(sci) > 0) {
+ /* Loopback mode */
+ scif_receive(sci, &txd, 1);
+ }
+ scif->txremain++;
+ sci->Xsr = FIELD_DP16(sci->Xsr, FSR, TEND, 0);
+ update_event_time(sci, TXEND, scif->txremain * sci->trtime);
+ txtrg = scif_ttrg(scif);
+ if (scif->txremain > txtrg) {
+ sci->Xsr = FIELD_DP16(sci->Xsr, FSR, TDFE, 0);
+ update_event_time(sci, TXEMPTY,
+ (scif->txremain - txtrg) * sci->trtime);
+ scif_irq(sci, TXI);
+ }
+ break;
+ case A_FSR:
+ rxtrg = scif_rtrg[FIELD_EX16(scif->fcr, FCR, RTRG)];
+ txtrg = scif_ttrg(scif);
+ /* FSR.FER and FSR.PER read only. Keep old value. */
+ val = FIELD_DP16(val, FSR, FER, 1);
+ val = FIELD_DP16(val, FSR, PER, 1);
+ val = FIELD_DP16(val, FSR, FERn, 15);
+ val = FIELD_DP16(val, FSR, PERn, 15);
+ if (scif_txremain_byte(scif) <= txtrg) {
+ /* It cannot be cleared when FIFO is free. */
+ val = FIELD_DP16(val, FSR, TDFE, 1);
+ }
+ if (fifo8_num_used(&sci->rxfifo) >= rxtrg) {
+ /* It cannot be cleared when FIFO is full. */
+ val = FIELD_DP16(val, FSR, TDFE, 1);
+ }
+ if (scif->txremain == 0) {
+ /* It cannot be cleared when FIFO is not empty. */
+ val = FIELD_DP16(val, FSR, TEND, 1);
+ }
+ sci->Xsr &= val;
+ scif_irq(sci, ERI);
+ scif_irq(sci, RXI);
+ scif_irq(sci, TXI);
+ break;
+ case A_FCR:
+ scif->fcr = val;
+ if (FIELD_EX16(scif->fcr, FCR, RFRST)) {
+ fifo8_reset(&sci->rxfifo);
+ update_event_time(sci, RXTOUT, 0);
+ update_event_time(sci, RXNEXT, 0);
+ sci->Xsr = FIELD_DP16(sci->Xsr, FSR, ER, 0);
+ sci->Xsr = FIELD_DP16(sci->Xsr, FSR, BRK, 0);
+ sci->Xsr = FIELD_DP16(sci->Xsr, FSR, FER, 0);
+ sci->Xsr = FIELD_DP16(sci->Xsr, FSR, PER, 0);
+ sci->Xsr = FIELD_DP16(sci->Xsr, FSR, RDF_DR, 0);
+ }
+ if (FIELD_EX16(scif->fcr, FCR, TFRST)) {
+ scif->txremain = 0;
+ update_event_time(sci, TXEMPTY, 0);
+ update_event_time(sci, TXEND, 0);
+ sci->Xsr = FIELD_DP16(sci->Xsr, FSR, TEND, 1);
+ sci->Xsr = FIELD_DP16(sci->Xsr, FSR, TDFE, 1);
+ }
+ break;
+ case A_FDR:
+ qemu_log_mask(LOG_GUEST_ERROR, "reneas_sci: FDR is read only.\n");
+ break;
+ case A_SPTR:
+ scif->sptr = val;
+ break;
+ case A_LSR:
+ scif->lsr &= val;
+ scif_irq(sci, ERI);
+ break;
+ default:
+ sci_common_write(sci, addr, val, size);
+ break;
+ }
+}
+
static uint64_t sci_common_read(void *opaque, hwaddr addr, unsigned size)
{
RenesasSCIBaseState *sci = RENESAS_SCI_BASE(opaque);
@@ -385,6 +719,7 @@ static uint64_t sci_common_read(void *opaque, hwaddr addr, unsigned size)
case A_SCR:
return sci->scr;
case A_SSR:
+ sci->read_Xsr = sci->Xsr;
return sci->Xsr;
case A_TDR:
return sci->tdr;
@@ -403,6 +738,20 @@ static uint64_t sci_common_read(void *opaque, hwaddr addr, unsigned size)
return UINT64_MAX;
}
+static uint64_t sci_read(void *opaque, hwaddr addr, unsigned size)
+{
+ RenesasSCIBaseState *sci = RENESAS_SCI_BASE(opaque);
+ addr = map_address(sci, addr);
+
+ switch (addr) {
+ case A_SPTR:
+ return RENESAS_SCI(sci)->sptr;
+ default:
+ return sci_common_read(sci, addr, size);
+ }
+ return UINT64_MAX;
+}
+
static uint64_t scia_read(void *opaque, hwaddr addr, unsigned size)
{
RenesasSCIBaseState *sci = RENESAS_SCI_BASE(opaque);
@@ -421,6 +770,34 @@ static uint64_t scia_read(void *opaque, hwaddr addr, unsigned size)
return UINT64_MAX;
}
+static uint64_t scif_read(void *opaque, hwaddr addr, unsigned size)
+{
+ RenesasSCIFState *scif = RENESAS_SCIF(opaque);
+ RenesasSCIBaseState *sci = RENESAS_SCI_BASE(opaque);
+ uint64_t ret;
+
+ addr = map_address(sci, addr);
+ switch (addr) {
+ case A_TDR:
+ qemu_log_mask(LOG_GUEST_ERROR, "reneas_sci: TDR is write only.\n");
+ return UINT64_MAX;
+ case A_FCR:
+ return scif->fcr & 0x7ff;
+ case A_FDR:
+ ret = 0;
+ ret = FIELD_DP16(ret, FDR, Rn, fifo8_num_used(&sci->rxfifo));
+ ret = FIELD_DP16(ret, FDR, Tn, scif_txremain_byte(scif));
+ return ret;
+ case A_SPTR:
+ return scif->sptr;
+ case A_LSR:
+ return scif->lsr;
+ default:
+ return sci_common_read(sci, addr, size);
+ }
+ return UINT64_MAX;
+}
+
static void rsci_common_init(Object *obj)
{
RenesasSCIBaseState *sci = RENESAS_SCI_BASE(obj);
@@ -444,6 +821,15 @@ static void sci_event(void *opaque, QEMUChrEvent event)
}
}
+static void scif_event(void *opaque, QEMUChrEvent event)
+{
+ RenesasSCIBaseState *sci = RENESAS_SCI_BASE(opaque);
+ if (event == CHR_EVENT_BREAK) {
+ sci->Xsr = FIELD_DP16(sci->Xsr, FSR, BRK, 1);
+ scif_irq(sci, BRI_TEI);
+ }
+}
+
static void rsci_common_realize(DeviceState *dev, Error **errp)
{
RenesasSCIBaseState *sci = RENESAS_SCI_BASE(dev);
@@ -472,6 +858,26 @@ static void register_mmio(RenesasSCIBaseState *sci, int size)
memory_region_init_io(&sci->memory, OBJECT(sci), rc->ops,
sci, "renesas-sci", size);
sysbus_init_mmio(d, &sci->memory);
+ memory_region_init_alias(&sci->memory_p4, NULL, "renesas-sci-p4",
+ &sci->memory, 0, size);
+ sysbus_init_mmio(d, &sci->memory_p4);
+ memory_region_init_alias(&sci->memory_a7, NULL, "renesas-sci-a7",
+ &sci->memory, 0, size);
+ sysbus_init_mmio(d, &sci->memory_a7);
+}
+
+static void rsci_realize(DeviceState *dev, Error **errp)
+{
+ RenesasSCIState *sci = RENESAS_SCI(dev);
+ RenesasSCIBaseState *common = RENESAS_SCI_BASE(dev);
+
+ rsci_common_realize(dev, errp);
+
+ register_mmio(common, 8 * (1 << common->regshift));
+ qemu_chr_fe_set_handlers(&common->chr, sci_can_receive, sci_receive,
+ sci_event, NULL, sci, NULL, true);
+
+ sci->sptr = 0x00;
}
static void rscia_realize(DeviceState *dev, Error **errp)
@@ -489,6 +895,22 @@ static void rscia_realize(DeviceState *dev, Error **errp)
sci->semr = 0x00;
}
+static void rscif_realize(DeviceState *dev, Error **errp)
+{
+ RenesasSCIFState *sci = RENESAS_SCIF(dev);
+ RenesasSCIBaseState *common = RENESAS_SCI_BASE(sci);
+
+ rsci_common_realize(dev, errp);
+
+ register_mmio(common, 10 * (1 << common->regshift));
+ qemu_chr_fe_set_handlers(&common->chr, scif_can_receive, scif_receive,
+ scif_event, NULL, sci, NULL, true);
+ common->Xsr = 0x0060;
+ sci->fcr = 0x0000;
+ sci->sptr = 0x0000;
+ sci->lsr = 0x0000;
+}
+
static const VMStateDescription vmstate_rsci = {
.name = "renesas-sci",
.version_id = 1,
@@ -514,6 +936,14 @@ static void rsci_init(Object *obj)
sci->event[TXEMPTY].handler = sci_tx_empty;
}
+static void rscif_init(Object *obj)
+{
+ RenesasSCIBaseState *sci = RENESAS_SCI_BASE(obj);
+ sci->event[RXTOUT].handler = scif_rx_timeout;
+ sci->event[TXEMPTY].handler = scif_tx_empty;
+ sci->event[TXEND].handler = scif_tx_end;
+}
+
static void rsci_common_class_init(ObjectClass *klass, void *data)
{
DeviceClass *dc = DEVICE_CLASS(klass);
@@ -522,6 +952,27 @@ static void rsci_common_class_init(ObjectClass *klass, void *data)
device_class_set_props(dc, rsci_properties);
}
+static const MemoryRegionOps sci_ops = {
+ .read = sci_read,
+ .write = sci_write,
+ .endianness = DEVICE_NATIVE_ENDIAN,
+ .valid = {
+ .min_access_size = 1,
+ .max_access_size = 4,
+ },
+};
+
+static void rsci_class_init(ObjectClass *klass, void *data)
+{
+ RenesasSCIBaseClass *comm_rc = RENESAS_SCI_BASE_CLASS(klass);
+ DeviceClass *dc = DEVICE_CLASS(klass);
+
+ comm_rc->ops = &sci_ops;
+ comm_rc->irq_fn = sci_irq;
+ comm_rc->divrate = sci_divrate;
+ dc->realize = rsci_realize;
+}
+
static const MemoryRegionOps scia_ops = {
.read = scia_read,
.write = scia_write,
@@ -544,6 +995,28 @@ static void rscia_class_init(ObjectClass *klass, void *data)
dc->realize = rscia_realize;
}
+static const MemoryRegionOps scif_ops = {
+ .read = scif_read,
+ .write = scif_write,
+ .endianness = DEVICE_NATIVE_ENDIAN,
+ .valid = {
+ .min_access_size = 1,
+ .max_access_size = 4,
+ },
+};
+
+static void rscif_class_init(ObjectClass *klass, void *data)
+{
+ RenesasSCIBaseClass *comm_rc = RENESAS_SCI_BASE_CLASS(klass);
+ DeviceClass *dc = DEVICE_CLASS(klass);
+
+ comm_rc->ops = &scif_ops;
+ comm_rc->irq_fn = scif_irq;
+ comm_rc->divrate = sci_divrate;
+
+ dc->realize = rscif_realize;
+}
+
static const TypeInfo renesas_sci_info[] = {
{
.name = TYPE_RENESAS_SCI_BASE,
@@ -554,6 +1027,14 @@ static const TypeInfo renesas_sci_info[] = {
.class_size = sizeof(RenesasSCIBaseClass),
.abstract = true,
},
+ {
+ .name = TYPE_RENESAS_SCI,
+ .parent = TYPE_RENESAS_SCI_BASE,
+ .instance_size = sizeof(RenesasSCIState),
+ .instance_init = rsci_init,
+ .class_init = rsci_class_init,
+ .class_size = sizeof(RenesasSCIClass),
+ },
{
.name = TYPE_RENESAS_SCIA,
.parent = TYPE_RENESAS_SCI_BASE,
@@ -562,6 +1043,14 @@ static const TypeInfo renesas_sci_info[] = {
.class_init = rscia_class_init,
.class_size = sizeof(RenesasSCIAClass),
},
+ {
+ .name = TYPE_RENESAS_SCIF,
+ .parent = TYPE_RENESAS_SCI_BASE,
+ .instance_size = sizeof(RenesasSCIFState),
+ .instance_init = rscif_init,
+ .class_init = rscif_class_init,
+ .class_size = sizeof(RenesasSCIFClass),
+ },
};
DEFINE_TYPES(renesas_sci_info)
--
2.20.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 3/3] hw/sh4: sh7750 using renesas_sci.
2021-06-16 9:12 [PATCH 0/3] renesas_sci update Yoshinori Sato
2021-06-16 9:12 ` [PATCH 1/3] hw/char: renesas_sci: Refactor for merge all SCI variant Yoshinori Sato
2021-06-16 9:12 ` [PATCH 2/3] hw/char: renesas_sci Add SCI and SCIF support Yoshinori Sato
@ 2021-06-16 9:12 ` Yoshinori Sato
2021-06-29 13:23 ` Peter Maydell
2021-06-16 9:27 ` [PATCH 0/3] renesas_sci update no-reply
3 siblings, 1 reply; 9+ messages in thread
From: Yoshinori Sato @ 2021-06-16 9:12 UTC (permalink / raw)
To: qemu-devel; +Cc: Yoshinori Sato
Signed-off-by: Yoshinori Sato <ysato@users.sourceforge.jp>
---
include/hw/sh4/sh.h | 8 --------
hw/sh4/sh7750.c | 41 +++++++++++++++++++++++++++++++++++++++++
hw/sh4/Kconfig | 2 +-
3 files changed, 42 insertions(+), 9 deletions(-)
diff --git a/include/hw/sh4/sh.h b/include/hw/sh4/sh.h
index becb596979..74e1ba59a8 100644
--- a/include/hw/sh4/sh.h
+++ b/include/hw/sh4/sh.h
@@ -55,14 +55,6 @@ int sh7750_register_io_device(struct SH7750State *s,
/* sh_serial.c */
#define SH_SERIAL_FEAT_SCIF (1 << 0)
-void sh_serial_init(MemoryRegion *sysmem,
- hwaddr base, int feat,
- uint32_t freq, Chardev *chr,
- qemu_irq eri_source,
- qemu_irq rxi_source,
- qemu_irq txi_source,
- qemu_irq tei_source,
- qemu_irq bri_source);
/* sh7750.c */
qemu_irq sh7750_irl(struct SH7750State *s);
diff --git a/hw/sh4/sh7750.c b/hw/sh4/sh7750.c
index d53a436d8c..1ef8b73c65 100644
--- a/hw/sh4/sh7750.c
+++ b/hw/sh4/sh7750.c
@@ -24,6 +24,7 @@
*/
#include "qemu/osdep.h"
+#include "qapi/error.h"
#include "hw/irq.h"
#include "hw/sh4/sh.h"
#include "sysemu/sysemu.h"
@@ -32,6 +33,8 @@
#include "hw/sh4/sh_intc.h"
#include "hw/timer/tmu012.h"
#include "exec/exec-all.h"
+#include "hw/char/renesas_sci.h"
+#include "hw/qdev-properties.h"
#define NB_DEVICES 4
@@ -752,6 +755,44 @@ static const MemoryRegionOps sh7750_mmct_ops = {
.endianness = DEVICE_NATIVE_ENDIAN,
};
+static void sh_serial_init(MemoryRegion *sysmem,
+ hwaddr base, int feat,
+ uint32_t freq, Chardev *chr,
+ qemu_irq eri_source,
+ qemu_irq rxi_source,
+ qemu_irq txi_source,
+ qemu_irq tei_source,
+ qemu_irq bri_source)
+{
+ RenesasSCIBaseState *sci;
+ char ckname[16];
+
+ switch(feat) {
+ case 0: /* SCI */
+ sci = RENESAS_SCI_BASE(qdev_new(TYPE_RENESAS_SCI));
+ snprintf(ckname, sizeof(ckname), "pck_sci");
+ break;
+ case SH_SERIAL_FEAT_SCIF:
+ sci = RENESAS_SCI_BASE(qdev_new(TYPE_RENESAS_SCIF));
+ snprintf(ckname, sizeof(ckname), "pck_scif");
+ break;
+ }
+ qdev_prop_set_chr(DEVICE(sci), "chardev", chr);
+ qdev_prop_set_uint32(DEVICE(sci), "register-size", SCI_REGWIDTH_32);
+ qdev_prop_set_uint64(DEVICE(sci), "input-freq", freq);
+ sysbus_connect_irq(SYS_BUS_DEVICE(sci), 0, eri_source);
+ sysbus_connect_irq(SYS_BUS_DEVICE(sci), 1, rxi_source);
+ sysbus_connect_irq(SYS_BUS_DEVICE(sci), 2, txi_source);
+ if (tei_source)
+ sysbus_connect_irq(SYS_BUS_DEVICE(sci), 3, tei_source);
+ if (bri_source)
+ sysbus_connect_irq(SYS_BUS_DEVICE(sci), 3, bri_source);
+ sysbus_realize(SYS_BUS_DEVICE(sci), &error_abort);
+ sysbus_mmio_map(SYS_BUS_DEVICE(sci), 0, base);
+ sysbus_mmio_map(SYS_BUS_DEVICE(sci), 1, P4ADDR(base));
+ sysbus_mmio_map(SYS_BUS_DEVICE(sci), 2, A7ADDR(base));
+}
+
SH7750State *sh7750_init(SuperHCPU *cpu, MemoryRegion *sysmem)
{
SH7750State *s;
diff --git a/hw/sh4/Kconfig b/hw/sh4/Kconfig
index ab733a3f76..d23d5f5b1c 100644
--- a/hw/sh4/Kconfig
+++ b/hw/sh4/Kconfig
@@ -20,5 +20,5 @@ config SHIX
config SH7750
bool
select SH_INTC
- select SH_SCI
+ select RENESAS_SCI
select SH_TIMER
--
2.20.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 0/3] renesas_sci update
2021-06-16 9:12 [PATCH 0/3] renesas_sci update Yoshinori Sato
` (2 preceding siblings ...)
2021-06-16 9:12 ` [PATCH 3/3] hw/sh4: sh7750 using renesas_sci Yoshinori Sato
@ 2021-06-16 9:27 ` no-reply
3 siblings, 0 replies; 9+ messages in thread
From: no-reply @ 2021-06-16 9:27 UTC (permalink / raw)
To: ysato; +Cc: qemu-devel, ysato
Patchew URL: https://patchew.org/QEMU/20210616091244.33049-1-ysato@users.sourceforge.jp/
Hi,
This series seems to have some coding style problems. See output below for
more information:
Type: series
Message-id: 20210616091244.33049-1-ysato@users.sourceforge.jp
Subject: [PATCH 0/3] renesas_sci update
=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===
Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
- [tag update] patchew/20210610133538.608390-1-pbonzini@redhat.com -> patchew/20210610133538.608390-1-pbonzini@redhat.com
- [tag update] patchew/20210615192848.1065297-1-venture@google.com -> patchew/20210615192848.1065297-1-venture@google.com
- [tag update] patchew/20210616064334.53398-1-lukas.juenger@greensocs.com -> patchew/20210616064334.53398-1-lukas.juenger@greensocs.com
- [tag update] patchew/20210616073358.750472-1-joel@jms.id.au -> patchew/20210616073358.750472-1-joel@jms.id.au
* [new tag] patchew/20210616091244.33049-1-ysato@users.sourceforge.jp -> patchew/20210616091244.33049-1-ysato@users.sourceforge.jp
Switched to a new branch 'test'
2aee00f hw/sh4: sh7750 using renesas_sci.
163b17f hw/char: renesas_sci Add SCI and SCIF support.
3a37650 hw/char: renesas_sci: Refactor for merge all SCI variant..
=== OUTPUT BEGIN ===
1/3 Checking commit 3a3765029d7c (hw/char: renesas_sci: Refactor for merge all SCI variant..)
2/3 Checking commit 163b17f2e78e (hw/char: renesas_sci Add SCI and SCIF support.)
ERROR: spaces required around that ':' (ctx:VxW)
#24: FILE: hw/char/renesas_sci.c:6:
+ * And SH7751 Group, SH7751R Group User's Manual: Hardware
^
ERROR: space prohibited after that '*' (ctx:ExW)
#25: FILE: hw/char/renesas_sci.c:7:
+ * (Rev.4.01 R01UH0457EJ0401)
^
total: 2 errors, 0 warnings, 715 lines checked
Patch 2/3 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
3/3 Checking commit 2aee00ffc830 (hw/sh4: sh7750 using renesas_sci.)
ERROR: space required before the open parenthesis '('
#63: FILE: hw/sh4/sh7750.c:770:
+ switch(feat) {
ERROR: braces {} are necessary for all arms of this statement
#79: FILE: hw/sh4/sh7750.c:786:
+ if (tei_source)
[...]
ERROR: braces {} are necessary for all arms of this statement
#81: FILE: hw/sh4/sh7750.c:788:
+ if (bri_source)
[...]
total: 3 errors, 0 warnings, 79 lines checked
Patch 3/3 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===
Test command exited with code: 1
The full log is available at
http://patchew.org/logs/20210616091244.33049-1-ysato@users.sourceforge.jp/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/3] hw/char: renesas_sci: Refactor for merge all SCI variant..
2021-06-16 9:12 ` [PATCH 1/3] hw/char: renesas_sci: Refactor for merge all SCI variant Yoshinori Sato
@ 2021-06-29 12:33 ` Peter Maydell
0 siblings, 0 replies; 9+ messages in thread
From: Peter Maydell @ 2021-06-29 12:33 UTC (permalink / raw)
To: Yoshinori Sato; +Cc: QEMU Developers
On Wed, 16 Jun 2021 at 10:22, Yoshinori Sato <ysato@users.sourceforge.jp> wrote:
>
> In order to handle unified all of the SCI, SCIa and SCIF in one part,
> to separate the transmission and reception portion and a register portion.
>
> RenesasSCIBase - common registers operation and event handling.
> RenesasSCIA - SCIa specific reigisters / functions.
>
> Signed-off-by: Yoshinori Sato <ysato@users.sourceforge.jp>
> ---
> include/hw/char/renesas_sci.h | 80 ++++-
> include/hw/rx/rx62n.h | 2 +-
> hw/char/renesas_sci.c | 568 +++++++++++++++++++++++-----------
> hw/rx/rx62n.c | 2 +-
> 4 files changed, 457 insertions(+), 195 deletions(-)
I'm not going to ask you to split it up any further, but this patch
was really painful to review because it's making so many changes
which are changing multiple different things at once (which is part
of why I've been putting it off for two weeks). It should probably
have been at least four or five different patches. For future
contributions, if you make them as more, smaller, self-contained
commits as you go along they will likely be much easier to review.
(Personally I find that writing them that way to start with is much
easier than trying to split up a larger commit afterwards.)
> +typedef struct RenesasSCIBaseState {
> /*< private >*/
> SysBusDevice parent_obj;
> - /*< public >*/
>
> MemoryRegion memory;
> - QEMUTimer timer;
> + QEMUTimer *event_timer;
> +
> + /*< public >*/
> + uint64_t input_freq;
> + int64_t etu;
> + int64_t trtime;
> + int64_t tx_start_time;
> + Fifo8 rxfifo;
> + int regshift;
> + uint32_t unit;
> CharBackend chr;
> qemu_irq irq[SCI_NR_IRQ];
> + struct {
> + int64_t time;
> + int64_t (*handler)(struct RenesasSCIBaseState *sci);
At least in this series, these handler function pointers seem
to be set in the device init function and never changed. Could
they be pointers in the class struct instead ?
> + } event[NR_SCI_EVENT];
>
> + /* common SCI register */
> uint8_t smr;
> uint8_t brr;
> uint8_t scr;
> uint8_t tdr;
> - uint8_t ssr;
> - uint8_t rdr;
> + uint16_t Xsr;
> +} RenesasSCIBaseState;
> +
> +struct RenesasSCIAState {
> + RenesasSCIBaseState parent_obj;
> +
> + /* SCIa specific register */
> uint8_t scmr;
> uint8_t semr;
> -
> - uint8_t read_ssr;
> - int64_t trtime;
> - int64_t rx_next;
> - uint64_t input_freq;
> };
>
> +typedef struct RenesasSCIBaseClass {
> + SysBusDeviceClass parent;
> +
> + const struct MemoryRegionOps *ops;
> + void (*irq_fn)(struct RenesasSCIBaseState *sci, int request);
> + int (*divrate)(struct RenesasSCIBaseState *sci);
> +} RenesasSCIBaseClass;
> +
> +typedef struct RenesasSCIAClass {
> + RenesasSCIBaseClass parent;
> +
> + void (*p_irq_fn)(struct RenesasSCIBaseState *sci, int request);
> + int (*p_divrate)(struct RenesasSCIBaseState *sci);
These two fields seem to never be used. (If they can be removed,
then you don't need a class struct at all for RenesasSCIA.)
> +} RenesasSCIAClass;
> +
> #endif
> diff --git a/include/hw/rx/rx62n.h b/include/hw/rx/rx62n.h
> index 3ed80dba0d..d6e6e168f9 100644
> --- a/include/hw/rx/rx62n.h
> +++ b/include/hw/rx/rx62n.h
> @@ -57,7 +57,7 @@ struct RX62NState {
> RXICUState icu;
> RTMRState tmr[RX62N_NR_TMR];
> RCMTState cmt[RX62N_NR_CMT];
> - RSCIState sci[RX62N_NR_SCI];
> + RenesasSCIAState sci[RX62N_NR_SCI];
>
> MemoryRegion *sysmem;
> bool kernel;
> diff --git a/hw/char/renesas_sci.c b/hw/char/renesas_sci.c
> index 1c63467290..c1126b7817 100644
> --- a/hw/char/renesas_sci.c
> +++ b/hw/char/renesas_sci.c
> @@ -4,7 +4,7 @@
> * Datasheet: RX62N Group, RX621 Group User's Manual: Hardware
> * (Rev.1.40 R01UH0033EJ0140)
> *
> - * Copyright (c) 2019 Yoshinori Sato
> + * Copyright (c) 2020 Yoshinori Sato
> *
> * SPDX-License-Identifier: GPL-2.0-or-later
> *
> @@ -23,15 +23,22 @@
>
> #include "qemu/osdep.h"
> #include "qemu/log.h"
> +#include "qapi/error.h"
> +#include "qemu-common.h"
> +#include "hw/hw.h"
> #include "hw/irq.h"
> +#include "hw/sysbus.h"
> #include "hw/registerfields.h"
> -#include "hw/qdev-properties.h"
> #include "hw/qdev-properties-system.h"
> #include "hw/char/renesas_sci.h"
> #include "migration/vmstate.h"
> +#include "qemu/error-report.h"
>
> -/* SCI register map */
> -REG8(SMR, 0)
> +/*
> + * SCI register map
> + * SCI(a) register size all 8bit.
> + */
> +REG32(SMR, 0) /* 8bit */
> FIELD(SMR, CKS, 0, 2)
> FIELD(SMR, MP, 2, 1)
> FIELD(SMR, STOP, 3, 1)
> @@ -39,263 +46,447 @@ REG8(SMR, 0)
> FIELD(SMR, PE, 5, 1)
> FIELD(SMR, CHR, 6, 1)
> FIELD(SMR, CM, 7, 1)
> -REG8(BRR, 1)
> -REG8(SCR, 2)
> - FIELD(SCR, CKE, 0, 2)
> +REG32(BRR, 4) /* 8bit */
> +REG32(SCR, 8)
> + FIELD(SCR, CKE, 0, 2)
This is an unnecessary whitespace change. In the old code, these FIELD()
macro lines have been set up so that the bitnumber arguments align.
I don't have a strong opinion on whether that's better or worse than
just having one space after every comma. But if you want to change it
then please:
(1) do that change in its own patch
(2) do it consistently -- here you have changed CKE but left RE, TE, etc
alone. Similarly below you change the line for SRR.ERR but leave
SRR.MPB alone.
> FIELD(SCR, TEIE, 2, 1)
> FIELD(SCR, MPIE, 3, 1)
> + FIELD(SCR, REIE, 3, 1)
> FIELD(SCR, RE, 4, 1)
> FIELD(SCR, TE, 5, 1)
> FIELD(SCR, RIE, 6, 1)
> FIELD(SCR, TIE, 7, 1)
> -REG8(TDR, 3)
> -REG8(SSR, 4)
> +REG32(TDR, 12) /* 8bit */
> +REG32(SSR, 16) /* 8bit */
> FIELD(SSR, MPBT, 0, 1)
> FIELD(SSR, MPB, 1, 1)
> FIELD(SSR, TEND, 2, 1)
> - FIELD(SSR, ERR, 3, 3)
> + FIELD(SSR, ERR, 3, 3)
> FIELD(SSR, PER, 3, 1)
> FIELD(SSR, FER, 4, 1)
> FIELD(SSR, ORER, 5, 1)
> FIELD(SSR, RDRF, 6, 1)
> FIELD(SSR, TDRE, 7, 1)
> -REG8(RDR, 5)
> -REG8(SCMR, 6)
> +REG32(RDR, 20) /* 8bit */
> +REG32(SCMR, 24) /* 8bit */
> FIELD(SCMR, SMIF, 0, 1)
> FIELD(SCMR, SINV, 2, 1)
> FIELD(SCMR, SDIR, 3, 1)
> FIELD(SCMR, BCP2, 7, 1)
> -REG8(SEMR, 7)
> +REG8(SEMR, 28)
> FIELD(SEMR, ACS0, 0, 1)
> FIELD(SEMR, ABCS, 4, 1)
>
> -static void rsci_realize(DeviceState *dev, Error **errp)
> +static void rsci_common_realize(DeviceState *dev, Error **errp)
> {
> - RSCIState *sci = RSCI(dev);
> + RenesasSCIBaseState *sci = RENESAS_SCI_BASE(dev);
> + int r;
>
> - if (sci->input_freq == 0) {
> + r = sci->regshift;
> + if ((r % 8) != 0 || ((r / 8) >> 1) > 2) {
> qemu_log_mask(LOG_GUEST_ERROR,
> - "renesas_sci: input-freq property must be set.");
> + "renesas_sci: Invalid register size.");
> return;
> }
> - qemu_chr_fe_set_handlers(&sci->chr, can_receive, receive,
> - sci_event, NULL, sci, NULL, true);
> + sci->regshift = (r / 8) >> 1;
> + sci->smr = sci->scr = 0x00;
> + sci->brr = 0xff;
> + sci->tdr = 0xff;
> + sci->Xsr = 0x84;
realize functions shouldn't generally set state fields like
this -- instead that should be done in the device's reset function.
(These devices seem to all be missing a reset function.)
> + update_trtime(sci);
> +
> }
> static const VMStateDescription vmstate_rsci = {
> @@ -303,49 +494,74 @@ static const VMStateDescription vmstate_rsci = {
> .version_id = 1,
> .minimum_version_id = 1,
> .fields = (VMStateField[]) {
> - VMSTATE_INT64(trtime, RSCIState),
> - VMSTATE_INT64(rx_next, RSCIState),
> - VMSTATE_UINT8(smr, RSCIState),
> - VMSTATE_UINT8(brr, RSCIState),
> - VMSTATE_UINT8(scr, RSCIState),
> - VMSTATE_UINT8(tdr, RSCIState),
> - VMSTATE_UINT8(ssr, RSCIState),
> - VMSTATE_UINT8(rdr, RSCIState),
> - VMSTATE_UINT8(scmr, RSCIState),
> - VMSTATE_UINT8(semr, RSCIState),
> - VMSTATE_UINT8(read_ssr, RSCIState),
> - VMSTATE_TIMER(timer, RSCIState),
> VMSTATE_END_OF_LIST()
This change seems to have lost all the migration support.
> }
> };
thanks
-- PMM
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/3] hw/char: renesas_sci Add SCI and SCIF support.
2021-06-16 9:12 ` [PATCH 2/3] hw/char: renesas_sci Add SCI and SCIF support Yoshinori Sato
@ 2021-06-29 12:37 ` Peter Maydell
0 siblings, 0 replies; 9+ messages in thread
From: Peter Maydell @ 2021-06-29 12:37 UTC (permalink / raw)
To: Yoshinori Sato; +Cc: QEMU Developers
On Wed, 16 Jun 2021 at 10:20, Yoshinori Sato <ysato@users.sourceforge.jp> wrote:
>
> This peripheral using SH7750.
>
> Signed-off-by: Yoshinori Sato <ysato@users.sourceforge.jp>
> ---
> include/hw/char/renesas_sci.h | 43 ++-
> hw/char/renesas_sci.c | 489 ++++++++++++++++++++++++++++++++++
> 2 files changed, 531 insertions(+), 1 deletion(-)
> enum {
> ERI = 0,
> @@ -32,6 +38,7 @@ enum {
> };
>
> enum {
> + RXTOUT,
> RXNEXT,
> TXEMPTY,
> TXEND,
> @@ -49,13 +56,14 @@ typedef struct RenesasSCIBaseState {
> SysBusDevice parent_obj;
>
> MemoryRegion memory;
> + MemoryRegion memory_p4;
> + MemoryRegion memory_a7;
> QEMUTimer *event_timer;
>
> /*< public >*/
> uint64_t input_freq;
> int64_t etu;
> int64_t trtime;
> - int64_t tx_start_time;
It looks like you added this field in patch 1, where it wasn't used,
and then deleted it again in patch 2...
thanks
-- PMM
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 3/3] hw/sh4: sh7750 using renesas_sci.
2021-06-16 9:12 ` [PATCH 3/3] hw/sh4: sh7750 using renesas_sci Yoshinori Sato
@ 2021-06-29 13:23 ` Peter Maydell
2021-06-30 15:15 ` Yoshinori Sato
0 siblings, 1 reply; 9+ messages in thread
From: Peter Maydell @ 2021-06-29 13:23 UTC (permalink / raw)
To: Yoshinori Sato; +Cc: QEMU Developers
On Wed, 16 Jun 2021 at 10:14, Yoshinori Sato <ysato@users.sourceforge.jp> wrote:
>
> Signed-off-by: Yoshinori Sato <ysato@users.sourceforge.jp>
> ---
> include/hw/sh4/sh.h | 8 --------
> hw/sh4/sh7750.c | 41 +++++++++++++++++++++++++++++++++++++++++
> hw/sh4/Kconfig | 2 +-
> 3 files changed, 42 insertions(+), 9 deletions(-)
>
> diff --git a/include/hw/sh4/sh.h b/include/hw/sh4/sh.h
> index becb596979..74e1ba59a8 100644
> --- a/include/hw/sh4/sh.h
> +++ b/include/hw/sh4/sh.h
> @@ -55,14 +55,6 @@ int sh7750_register_io_device(struct SH7750State *s,
>
> /* sh_serial.c */
> #define SH_SERIAL_FEAT_SCIF (1 << 0)
> -void sh_serial_init(MemoryRegion *sysmem,
> - hwaddr base, int feat,
> - uint32_t freq, Chardev *chr,
> - qemu_irq eri_source,
> - qemu_irq rxi_source,
> - qemu_irq txi_source,
> - qemu_irq tei_source,
> - qemu_irq bri_source);
This change means that the code in sh_serial.c will no longer compile,
because it has a non-static function with no previous prototype.
The patch as a whole compiles because the change to hw/sh4/Kconfig
file removes the selection of SH_SCI and so we never try to
compile sh_serial.c. But we shouldn't leave dead code around in
the tree.
Option A:
I guess the idea is to avoid having to rename sh_serial_init(),
which makes sense. If you want to take this route, then
in this patch we should mention that in the commit message,
something like:
===begin===
hw/sh4: Switch sh7750 to new renesas-sci devices
Switch the sh7750 away from the old serial device implementation
in sh_serial.c to use the new renesas-sci devices.
Note that deleting the prototype for sh_serial_init() means that
sh_serial.c will no longer be able to compile (it would hit a
warning about having a non-static function without a previous
prototype). This is OK because we remove the only place that
was selecting the SH_SCI Kconfig feature, so we won't attempt
to compile that source file. In the following commit, we will
delete the file entirely.
===end===
Then in a new patch to go after this one, remove:
* the file sh_serial.c itself
* the "config SH_SCI" section in hw/char/Kconfig
* the line for sh_serial.c in hw/char/meson.build
Option B: would be to give your new sh7750.c function a different
name than sh_serial_init() (eg sci_init()) and change the two
callsites in this patch. Then you could keep the old sh_serial_init()
prototype in this patch and delete it as part of the new "remove
sh_serial.c" patch (which is where removal of the prototype more
logically belongs.)
I don't mind which of the two you go for.
> /* sh7750.c */
> qemu_irq sh7750_irl(struct SH7750State *s);
> diff --git a/hw/sh4/sh7750.c b/hw/sh4/sh7750.c
> index d53a436d8c..1ef8b73c65 100644
> --- a/hw/sh4/sh7750.c
> +++ b/hw/sh4/sh7750.c
> @@ -24,6 +24,7 @@
> */
>
> #include "qemu/osdep.h"
> +#include "qapi/error.h"
> #include "hw/irq.h"
> #include "hw/sh4/sh.h"
> #include "sysemu/sysemu.h"
> @@ -32,6 +33,8 @@
> #include "hw/sh4/sh_intc.h"
> #include "hw/timer/tmu012.h"
> #include "exec/exec-all.h"
> +#include "hw/char/renesas_sci.h"
> +#include "hw/qdev-properties.h"
>
> #define NB_DEVICES 4
>
> @@ -752,6 +755,44 @@ static const MemoryRegionOps sh7750_mmct_ops = {
> .endianness = DEVICE_NATIVE_ENDIAN,
> };
>
> +static void sh_serial_init(MemoryRegion *sysmem,
> + hwaddr base, int feat,
> + uint32_t freq, Chardev *chr,
> + qemu_irq eri_source,
> + qemu_irq rxi_source,
> + qemu_irq txi_source,
> + qemu_irq tei_source,
> + qemu_irq bri_source)
> +{
> + RenesasSCIBaseState *sci;
> + char ckname[16];
> +
> + switch(feat) {
> + case 0: /* SCI */
> + sci = RENESAS_SCI_BASE(qdev_new(TYPE_RENESAS_SCI));
> + snprintf(ckname, sizeof(ckname), "pck_sci");
> + break;
> + case SH_SERIAL_FEAT_SCIF:
> + sci = RENESAS_SCI_BASE(qdev_new(TYPE_RENESAS_SCIF));
> + snprintf(ckname, sizeof(ckname), "pck_scif");
> + break;
> + }
The ckname[] array seems to be set but never used ?
Since you never use 'sci' as a RenesasSCIBaseState, you could
instead declare
Device *sci;
and then
sci = qdev_new(TYPE_RENESAS_whatever);
and avoid some of the DEVICE() casts below.
You might also prefer to have a SysBusDevice *sbd which you
can then set with
sbd = SYS_BUS_DEVICE(sci);
and use sbd instead of casting every time you need it.
> + qdev_prop_set_chr(DEVICE(sci), "chardev", chr);
> + qdev_prop_set_uint32(DEVICE(sci), "register-size", SCI_REGWIDTH_32);
> + qdev_prop_set_uint64(DEVICE(sci), "input-freq", freq);
> + sysbus_connect_irq(SYS_BUS_DEVICE(sci), 0, eri_source);
> + sysbus_connect_irq(SYS_BUS_DEVICE(sci), 1, rxi_source);
> + sysbus_connect_irq(SYS_BUS_DEVICE(sci), 2, txi_source);
> + if (tei_source)
> + sysbus_connect_irq(SYS_BUS_DEVICE(sci), 3, tei_source);
> + if (bri_source)
> + sysbus_connect_irq(SYS_BUS_DEVICE(sci), 3, bri_source);
QEMU coding style requires braces for all if statements, even when
they're only one line.
> + sysbus_realize(SYS_BUS_DEVICE(sci), &error_abort);
> + sysbus_mmio_map(SYS_BUS_DEVICE(sci), 0, base);
> + sysbus_mmio_map(SYS_BUS_DEVICE(sci), 1, P4ADDR(base));
> + sysbus_mmio_map(SYS_BUS_DEVICE(sci), 2, A7ADDR(base));
> +}
thanks
-- PMM
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 3/3] hw/sh4: sh7750 using renesas_sci.
2021-06-29 13:23 ` Peter Maydell
@ 2021-06-30 15:15 ` Yoshinori Sato
0 siblings, 0 replies; 9+ messages in thread
From: Yoshinori Sato @ 2021-06-30 15:15 UTC (permalink / raw)
To: Peter Maydell; +Cc: QEMU Developers
On Tue, 29 Jun 2021 22:23:01 +0900,
Peter Maydell wrote:
>
> On Wed, 16 Jun 2021 at 10:14, Yoshinori Sato <ysato@users.sourceforge.jp> wrote:
> >
> > Signed-off-by: Yoshinori Sato <ysato@users.sourceforge.jp>
> > ---
> > include/hw/sh4/sh.h | 8 --------
> > hw/sh4/sh7750.c | 41 +++++++++++++++++++++++++++++++++++++++++
> > hw/sh4/Kconfig | 2 +-
> > 3 files changed, 42 insertions(+), 9 deletions(-)
> >
> > diff --git a/include/hw/sh4/sh.h b/include/hw/sh4/sh.h
> > index becb596979..74e1ba59a8 100644
> > --- a/include/hw/sh4/sh.h
> > +++ b/include/hw/sh4/sh.h
> > @@ -55,14 +55,6 @@ int sh7750_register_io_device(struct SH7750State *s,
> >
> > /* sh_serial.c */
> > #define SH_SERIAL_FEAT_SCIF (1 << 0)
> > -void sh_serial_init(MemoryRegion *sysmem,
> > - hwaddr base, int feat,
> > - uint32_t freq, Chardev *chr,
> > - qemu_irq eri_source,
> > - qemu_irq rxi_source,
> > - qemu_irq txi_source,
> > - qemu_irq tei_source,
> > - qemu_irq bri_source);
>
> This change means that the code in sh_serial.c will no longer compile,
> because it has a non-static function with no previous prototype.
> The patch as a whole compiles because the change to hw/sh4/Kconfig
> file removes the selection of SH_SCI and so we never try to
> compile sh_serial.c. But we shouldn't leave dead code around in
> the tree.
>
> Option A:
> I guess the idea is to avoid having to rename sh_serial_init(),
> which makes sense. If you want to take this route, then
> in this patch we should mention that in the commit message,
> something like:
>
> ===begin===
> hw/sh4: Switch sh7750 to new renesas-sci devices
>
> Switch the sh7750 away from the old serial device implementation
> in sh_serial.c to use the new renesas-sci devices.
>
> Note that deleting the prototype for sh_serial_init() means that
> sh_serial.c will no longer be able to compile (it would hit a
> warning about having a non-static function without a previous
> prototype). This is OK because we remove the only place that
> was selecting the SH_SCI Kconfig feature, so we won't attempt
> to compile that source file. In the following commit, we will
> delete the file entirely.
>
> ===end===
>
> Then in a new patch to go after this one, remove:
> * the file sh_serial.c itself
> * the "config SH_SCI" section in hw/char/Kconfig
> * the line for sh_serial.c in hw/char/meson.build
>
>
> Option B: would be to give your new sh7750.c function a different
> name than sh_serial_init() (eg sci_init()) and change the two
> callsites in this patch. Then you could keep the old sh_serial_init()
> prototype in this patch and delete it as part of the new "remove
> sh_serial.c" patch (which is where removal of the prototype more
> logically belongs.)
>
> I don't mind which of the two you go for.
I think option B is fine.
I would like to change sh7750 to qom as the next step.
I don't want to make any major changes at this step.
> > /* sh7750.c */
> > qemu_irq sh7750_irl(struct SH7750State *s);
> > diff --git a/hw/sh4/sh7750.c b/hw/sh4/sh7750.c
> > index d53a436d8c..1ef8b73c65 100644
> > --- a/hw/sh4/sh7750.c
> > +++ b/hw/sh4/sh7750.c
> > @@ -24,6 +24,7 @@
> > */
> >
> > #include "qemu/osdep.h"
> > +#include "qapi/error.h"
> > #include "hw/irq.h"
> > #include "hw/sh4/sh.h"
> > #include "sysemu/sysemu.h"
> > @@ -32,6 +33,8 @@
> > #include "hw/sh4/sh_intc.h"
> > #include "hw/timer/tmu012.h"
> > #include "exec/exec-all.h"
> > +#include "hw/char/renesas_sci.h"
> > +#include "hw/qdev-properties.h"
> >
> > #define NB_DEVICES 4
> >
> > @@ -752,6 +755,44 @@ static const MemoryRegionOps sh7750_mmct_ops = {
> > .endianness = DEVICE_NATIVE_ENDIAN,
> > };
> >
> > +static void sh_serial_init(MemoryRegion *sysmem,
> > + hwaddr base, int feat,
> > + uint32_t freq, Chardev *chr,
> > + qemu_irq eri_source,
> > + qemu_irq rxi_source,
> > + qemu_irq txi_source,
> > + qemu_irq tei_source,
> > + qemu_irq bri_source)
> > +{
> > + RenesasSCIBaseState *sci;
> > + char ckname[16];
> > +
> > + switch(feat) {
> > + case 0: /* SCI */
> > + sci = RENESAS_SCI_BASE(qdev_new(TYPE_RENESAS_SCI));
> > + snprintf(ckname, sizeof(ckname), "pck_sci");
> > + break;
> > + case SH_SERIAL_FEAT_SCIF:
> > + sci = RENESAS_SCI_BASE(qdev_new(TYPE_RENESAS_SCIF));
> > + snprintf(ckname, sizeof(ckname), "pck_scif");
> > + break;
> > + }
>
> The ckname[] array seems to be set but never used ?
Yes.
This array used old changes. I forgot remove it.
>
> Since you never use 'sci' as a RenesasSCIBaseState, you could
> instead declare
>
> Device *sci;
>
> and then
> sci = qdev_new(TYPE_RENESAS_whatever);
>
> and avoid some of the DEVICE() casts below.
OK.
> You might also prefer to have a SysBusDevice *sbd which you
> can then set with
> sbd = SYS_BUS_DEVICE(sci);
> and use sbd instead of casting every time you need it.
OK.
> > + qdev_prop_set_chr(DEVICE(sci), "chardev", chr);
> > + qdev_prop_set_uint32(DEVICE(sci), "register-size", SCI_REGWIDTH_32);
> > + qdev_prop_set_uint64(DEVICE(sci), "input-freq", freq);
> > + sysbus_connect_irq(SYS_BUS_DEVICE(sci), 0, eri_source);
> > + sysbus_connect_irq(SYS_BUS_DEVICE(sci), 1, rxi_source);
> > + sysbus_connect_irq(SYS_BUS_DEVICE(sci), 2, txi_source);
> > + if (tei_source)
> > + sysbus_connect_irq(SYS_BUS_DEVICE(sci), 3, tei_source);
> > + if (bri_source)
> > + sysbus_connect_irq(SYS_BUS_DEVICE(sci), 3, bri_source);
>
> QEMU coding style requires braces for all if statements, even when
> they're only one line.
It was before checking with checkpatch.pl.
I will clean it in the next patch.
> > + sysbus_realize(SYS_BUS_DEVICE(sci), &error_abort);
> > + sysbus_mmio_map(SYS_BUS_DEVICE(sci), 0, base);
> > + sysbus_mmio_map(SYS_BUS_DEVICE(sci), 1, P4ADDR(base));
> > + sysbus_mmio_map(SYS_BUS_DEVICE(sci), 2, A7ADDR(base));
> > +}
>
> thanks
> -- PMM
--
Yoshinori Sato
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2021-06-30 15:16 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-16 9:12 [PATCH 0/3] renesas_sci update Yoshinori Sato
2021-06-16 9:12 ` [PATCH 1/3] hw/char: renesas_sci: Refactor for merge all SCI variant Yoshinori Sato
2021-06-29 12:33 ` Peter Maydell
2021-06-16 9:12 ` [PATCH 2/3] hw/char: renesas_sci Add SCI and SCIF support Yoshinori Sato
2021-06-29 12:37 ` Peter Maydell
2021-06-16 9:12 ` [PATCH 3/3] hw/sh4: sh7750 using renesas_sci Yoshinori Sato
2021-06-29 13:23 ` Peter Maydell
2021-06-30 15:15 ` Yoshinori Sato
2021-06-16 9:27 ` [PATCH 0/3] renesas_sci update no-reply
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.