* [Qemu-devel] [PATCH] SH4: Serial controller improvement and sleep op bug fix
@ 2008-09-07 14:34 Shin-ichiro KAWASAKI
2008-09-14 16:56 ` Aurelien Jarno
0 siblings, 1 reply; 6+ messages in thread
From: Shin-ichiro KAWASAKI @ 2008-09-07 14:34 UTC (permalink / raw)
To: qemu-devel
Hi, all.
This patch adds some improvements for SH4 system emulation.
It enables serial controller to receive characters, so the
shell can receive command inputs. Now we can input 'ls'
and see it works, at last!
It does:
- adds receive character feature to SH4 SCIF.
SH4-SCI feature implementation work is left.
- fixed a bug on 'sleep' instruction, which have caused halt of idle task.
As i386 'hlt' instruction does, it should save PC before sleep.
I checked the patch with the kernel at the following URL,
which I mentioned before.
> > http://www.assembla.com/file/qemu-sh4/3_zImage
> > The command line is like this.
> > % ./qemu/sh4-softmmu/qemu-system-sh4 -M r2d -serial vc -serial stdio -m 1024M -kernel zImage
Comments on it and commit to the trunk will be appreciated.
Regards,
Shin-ichiro KAWASAKI
Index: target-sh4/helper.h
===================================================================
--- target-sh4/helper.h (revision 5132)
+++ target-sh4/helper.h (working copy)
@@ -6,7 +6,7 @@
DEF_HELPER(void, helper_raise_illegal_instruction, (void))
DEF_HELPER(void, helper_raise_slot_illegal_instruction, (void))
DEF_HELPER(void, helper_debug, (void))
-DEF_HELPER(void, helper_sleep, (void))
+DEF_HELPER(void, helper_sleep, (int32_t))
DEF_HELPER(void, helper_trapa, (uint32_t))
DEF_HELPER(uint32_t, helper_addv, (uint32_t, uint32_t))
Index: target-sh4/op_helper.c
===================================================================
--- target-sh4/op_helper.c (revision 5132)
+++ target-sh4/op_helper.c (working copy)
@@ -94,10 +94,11 @@
cpu_loop_exit();
}
-void helper_sleep(void)
+void helper_sleep(int32_t next_pc)
{
env->halted = 1;
env->exception_index = EXCP_HLT;
+ env-> pc = next_pc;
cpu_loop_exit();
}
Index: target-sh4/translate.c
===================================================================
--- target-sh4/translate.c (revision 5132)
+++ target-sh4/translate.c (working copy)
@@ -446,7 +446,7 @@
return;
case 0x001b: /* sleep */
if (ctx->memidx) {
- tcg_gen_helper_0_0(helper_sleep);
+ tcg_gen_helper_0_1(helper_sleep, tcg_const_i32(ctx->pc + 2));
} else {
tcg_gen_helper_0_0(helper_raise_illegal_instruction);
ctx->bstate = BS_EXCP;
Index: hw/sh_serial.c
===================================================================
--- hw/sh_serial.c (revision 5132)
+++ hw/sh_serial.c (working copy)
@@ -53,6 +53,7 @@
int freq;
int feat;
int flags;
+ int rtrg;
CharDriverState *chr;
@@ -80,6 +81,7 @@
s->brr = val;
return;
case 0x08: /* SCR */
+ /* TODO : For SH7751, SCIF mask should be 0xfb. */
s->scr = val & ((s->feat & SH_SERIAL_FEAT_SCIF) ? 0xfa : 0xff);
if (!(val & (1 << 5)))
s->flags |= SH_SERIAL_FLAG_TEND;
@@ -89,6 +91,9 @@
else if (!(val & (1 << 7)) && s->txi->asserted)
sh_intc_toggle_source(s->txi, 0, -1);
}
+ if (!(val & (1 << 6)) && s->rxi->asserted) {
+ sh_intc_toggle_source(s->rxi, 0, -1);
+ }
return;
case 0x0c: /* FTDR / TDR */
if (s->chr) {
@@ -117,12 +122,37 @@
s->flags &= ~SH_SERIAL_FLAG_RDF;
if (!(val & (1 << 0)))
s->flags &= ~SH_SERIAL_FLAG_DR;
+
+ if (!(val & (1 << 1)) || !(val & (1 << 0))) {
+ if (s->rxi && s->rxi->asserted) {
+ sh_intc_toggle_source(s->rxi, 0, -1);
+ }
+ }
return;
case 0x18: /* FCR */
s->fcr = val;
+ switch ((val >> 6) & 3) {
+ case 0:
+ s->rtrg = 1;
+ break;
+ case 1:
+ s->rtrg = 4;
+ break;
+ case 2:
+ s->rtrg = 8;
+ break;
+ case 3:
+ s->rtrg = 14;
+ break;
+ }
+ if (val & (1 << 1)) {
+ s->rx_cnt = 0;
+ s->sr &= ~(1 << 1);
+ }
+
return;
case 0x20: /* SPTR */
- s->sptr = val;
+ s->sptr = val & 0xf3;
return;
case 0x24: /* LSR */
return;
@@ -194,6 +224,15 @@
s->flags |= SH_SERIAL_FLAG_TDE | SH_SERIAL_FLAG_TEND;
break;
+ case 0x14:
+ if (s->rx_cnt > 0) {
+ ret = s->rx_fifo[0];
+ s->rx_cnt--;
+ memmove(&s->rx_fifo[0], &s->rx_fifo[1], s->rx_cnt);
+ if (s->rx_cnt < s->rtrg)
+ s->flags &= ~SH_SERIAL_FLAG_RDF;
+ }
+ break;
#if 0
case 0x18:
ret = s->fcr;
@@ -219,6 +258,9 @@
case 0x10:
ret = 0;
break;
+ case 0x14:
+ ret = s->rx_fifo[0];
+ break;
case 0x1c:
ret = s->sptr;
break;
@@ -240,15 +282,30 @@
static int sh_serial_can_receive(sh_serial_state *s)
{
- return 0;
+ return s->scr & (1 << 4);
}
static void sh_serial_receive_byte(sh_serial_state *s, int ch)
{
+ if (s->feat & SH_SERIAL_FEAT_SCIF) {
+ if (s->rx_cnt < sizeof(s->rx_fifo)) {
+ s->rx_fifo[s->rx_cnt++] = ch;
+ if (s->rx_cnt >= s->rtrg) {
+ s->flags |= SH_SERIAL_FLAG_RDF;
+ if (s->scr & (1 << 6) && s->rxi) {
+ sh_intc_toggle_source(s->rxi, 0, 1);
+ }
+ }
+ }
+ } else {
+ s->rx_fifo[0] = ch;
+ }
}
static void sh_serial_receive_break(sh_serial_state *s)
{
+ if (s->feat & SH_SERIAL_FEAT_SCIF)
+ s->sr |= (1 << 4);
}
static int sh_serial_can_receive1(void *opaque)
@@ -313,6 +370,7 @@
s->base = base;
s->feat = feat;
s->flags = SH_SERIAL_FLAG_TEND | SH_SERIAL_FLAG_TDE;
+ s->rtrg = 1;
s->smr = 0;
s->brr = 0xff;
@@ -323,6 +381,7 @@
s->fcr = 0;
}
else {
+ memset(s->rx_fifo, 0, sizeof(s->rx_fifo));
s->dr = 0xff;
}
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] SH4: Serial controller improvement and sleep op bug fix
2008-09-07 14:34 [Qemu-devel] [PATCH] SH4: Serial controller improvement and sleep op bug fix Shin-ichiro KAWASAKI
@ 2008-09-14 16:56 ` Aurelien Jarno
2008-09-15 5:28 ` [PATCH] [RESEND] SH4 : sleep instruction bug fix (was Re: [Qemu-devel] [PATCH] SH4: Serial controller improvement and sleep op bug fix) Shin-ichiro KAWASAKI
2008-09-15 5:42 ` [Qemu-devel] [PATCH] [RESEND] SH4 : Serial controller improvement Shin-ichiro KAWASAKI
0 siblings, 2 replies; 6+ messages in thread
From: Aurelien Jarno @ 2008-09-14 16:56 UTC (permalink / raw)
To: Shin-ichiro KAWASAKI; +Cc: qemu-devel
On Sun, Sep 07, 2008 at 11:34:10PM +0900, Shin-ichiro KAWASAKI wrote:
> Hi, all.
Hi!
> This patch adds some improvements for SH4 system emulation.
> It enables serial controller to receive characters, so the
> shell can receive command inputs. Now we can input 'ls'
> and see it works, at last!
>
> It does:
> - adds receive character feature to SH4 SCIF.
> SH4-SCI feature implementation work is left.
> - fixed a bug on 'sleep' instruction, which have caused halt of idle task.
> As i386 'hlt' instruction does, it should save PC before sleep.
Next time could you please send two different patches, so that one patch
doesn't block the other.
> I checked the patch with the kernel at the following URL,
> which I mentioned before.
>
> > > http://www.assembla.com/file/qemu-sh4/3_zImage
> > > The command line is like this.
> > > % ./qemu/sh4-softmmu/qemu-system-sh4 -M r2d -serial vc -serial stdio -m 1024M -kernel zImage
>
> Comments on it and commit to the trunk will be appreciated.
Please find my comments below.
> Regards,
> Shin-ichiro KAWASAKI
>
>
> Index: target-sh4/helper.h
> ===================================================================
> --- target-sh4/helper.h (revision 5132)
> +++ target-sh4/helper.h (working copy)
> @@ -6,7 +6,7 @@
> DEF_HELPER(void, helper_raise_illegal_instruction, (void))
> DEF_HELPER(void, helper_raise_slot_illegal_instruction, (void))
> DEF_HELPER(void, helper_debug, (void))
> -DEF_HELPER(void, helper_sleep, (void))
> +DEF_HELPER(void, helper_sleep, (int32_t))
> DEF_HELPER(void, helper_trapa, (uint32_t))
>
> DEF_HELPER(uint32_t, helper_addv, (uint32_t, uint32_t))
> Index: target-sh4/op_helper.c
> ===================================================================
> --- target-sh4/op_helper.c (revision 5132)
> +++ target-sh4/op_helper.c (working copy)
> @@ -94,10 +94,11 @@
> cpu_loop_exit();
> }
>
> -void helper_sleep(void)
> +void helper_sleep(int32_t next_pc)
Given then env->pc is unsigned, I think it should be uint32_t instead of
int32_t here. Same in helper.h
> {
> env->halted = 1;
> env->exception_index = EXCP_HLT;
> + env-> pc = next_pc;
> cpu_loop_exit();
> }
>
> Index: target-sh4/translate.c
> ===================================================================
> --- target-sh4/translate.c (revision 5132)
> +++ target-sh4/translate.c (working copy)
> @@ -446,7 +446,7 @@
> return;
> case 0x001b: /* sleep */
> if (ctx->memidx) {
> - tcg_gen_helper_0_0(helper_sleep);
> + tcg_gen_helper_0_1(helper_sleep, tcg_const_i32(ctx->pc + 2));
> } else {
> tcg_gen_helper_0_0(helper_raise_illegal_instruction);
> ctx->bstate = BS_EXCP;
> Index: hw/sh_serial.c
> ===================================================================
> --- hw/sh_serial.c (revision 5132)
> +++ hw/sh_serial.c (working copy)
> @@ -53,6 +53,7 @@
> int freq;
> int feat;
> int flags;
> + int rtrg;
>
> CharDriverState *chr;
>
> @@ -80,6 +81,7 @@
> s->brr = val;
> return;
> case 0x08: /* SCR */
> + /* TODO : For SH7751, SCIF mask should be 0xfb. */
> s->scr = val & ((s->feat & SH_SERIAL_FEAT_SCIF) ? 0xfa : 0xff);
> if (!(val & (1 << 5)))
> s->flags |= SH_SERIAL_FLAG_TEND;
> @@ -89,6 +91,9 @@
> else if (!(val & (1 << 7)) && s->txi->asserted)
> sh_intc_toggle_source(s->txi, 0, -1);
> }
> + if (!(val & (1 << 6)) && s->rxi->asserted) {
> + sh_intc_toggle_source(s->rxi, 0, -1);
> + }
> return;
> case 0x0c: /* FTDR / TDR */
> if (s->chr) {
> @@ -117,12 +122,37 @@
> s->flags &= ~SH_SERIAL_FLAG_RDF;
> if (!(val & (1 << 0)))
> s->flags &= ~SH_SERIAL_FLAG_DR;
> +
> + if (!(val & (1 << 1)) || !(val & (1 << 0))) {
> + if (s->rxi && s->rxi->asserted) {
> + sh_intc_toggle_source(s->rxi, 0, -1);
> + }
> + }
> return;
> case 0x18: /* FCR */
> s->fcr = val;
> + switch ((val >> 6) & 3) {
> + case 0:
> + s->rtrg = 1;
> + break;
> + case 1:
> + s->rtrg = 4;
> + break;
> + case 2:
> + s->rtrg = 8;
> + break;
> + case 3:
> + s->rtrg = 14;
> + break;
> + }
> + if (val & (1 << 1)) {
> + s->rx_cnt = 0;
> + s->sr &= ~(1 << 1);
> + }
> +
> return;
> case 0x20: /* SPTR */
> - s->sptr = val;
> + s->sptr = val & 0xf3;
> return;
> case 0x24: /* LSR */
> return;
> @@ -194,6 +224,15 @@
> s->flags |= SH_SERIAL_FLAG_TDE | SH_SERIAL_FLAG_TEND;
>
> break;
> + case 0x14:
> + if (s->rx_cnt > 0) {
> + ret = s->rx_fifo[0];
> + s->rx_cnt--;
> + memmove(&s->rx_fifo[0], &s->rx_fifo[1], s->rx_cnt);
I don't think moving the whole buffer each time a character is read is a
good idea. I would suggest to use a circular buffer instead. Have a look
at hw/serial.c how it is done.
> + if (s->rx_cnt < s->rtrg)
> + s->flags &= ~SH_SERIAL_FLAG_RDF;
> + }
> + break;
> #if 0
> case 0x18:
> ret = s->fcr;
> @@ -219,6 +258,9 @@
> case 0x10:
> ret = 0;
> break;
> + case 0x14:
> + ret = s->rx_fifo[0];
> + break;
> case 0x1c:
> ret = s->sptr;
> break;
> @@ -240,15 +282,30 @@
>
> static int sh_serial_can_receive(sh_serial_state *s)
> {
> - return 0;
> + return s->scr & (1 << 4);
> }
>
> static void sh_serial_receive_byte(sh_serial_state *s, int ch)
> {
> + if (s->feat & SH_SERIAL_FEAT_SCIF) {
> + if (s->rx_cnt < sizeof(s->rx_fifo)) {
> + s->rx_fifo[s->rx_cnt++] = ch;
> + if (s->rx_cnt >= s->rtrg) {
> + s->flags |= SH_SERIAL_FLAG_RDF;
> + if (s->scr & (1 << 6) && s->rxi) {
> + sh_intc_toggle_source(s->rxi, 0, 1);
> + }
> + }
> + }
> + } else {
> + s->rx_fifo[0] = ch;
> + }
> }
>
> static void sh_serial_receive_break(sh_serial_state *s)
> {
> + if (s->feat & SH_SERIAL_FEAT_SCIF)
> + s->sr |= (1 << 4);
> }
>
> static int sh_serial_can_receive1(void *opaque)
> @@ -313,6 +370,7 @@
> s->base = base;
> s->feat = feat;
> s->flags = SH_SERIAL_FLAG_TEND | SH_SERIAL_FLAG_TDE;
> + s->rtrg = 1;
>
> s->smr = 0;
> s->brr = 0xff;
> @@ -323,6 +381,7 @@
> s->fcr = 0;
> }
> else {
> + memset(s->rx_fifo, 0, sizeof(s->rx_fifo));
> s->dr = 0xff;
> }
>
>
>
>
--
.''`. Aurelien Jarno | GPG: 1024D/F1BCDB73
: :' : Debian developer | Electrical Engineer
`. `' aurel32@debian.org | aurelien@aurel32.net
`- people.debian.org/~aurel32 | www.aurel32.net
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] [RESEND] SH4 : sleep instruction bug fix (was Re: [Qemu-devel] [PATCH] SH4: Serial controller improvement and sleep op bug fix)
2008-09-14 16:56 ` Aurelien Jarno
@ 2008-09-15 5:28 ` Shin-ichiro KAWASAKI
2008-09-15 6:42 ` Aurelien Jarno
2008-09-15 5:42 ` [Qemu-devel] [PATCH] [RESEND] SH4 : Serial controller improvement Shin-ichiro KAWASAKI
1 sibling, 1 reply; 6+ messages in thread
From: Shin-ichiro KAWASAKI @ 2008-09-15 5:28 UTC (permalink / raw)
To: qemu-devel
Thank you for your response!
Aurelien Jarno wrote:
> On Sun, Sep 07, 2008 at 11:34:10PM +0900, Shin-ichiro KAWASAKI wrote:
>
> Next time could you please send two different patches, so that one patch
> doesn't block the other.
I see. I'll do so next time. Working with this patch, I'll resend twice to
divide into sleep instruction part and serial part.
>> Index: target-sh4/helper.h
>> ===================================================================
>> --- target-sh4/helper.h (revision 5132)
>> +++ target-sh4/helper.h (working copy)
>> @@ -6,7 +6,7 @@
>> DEF_HELPER(void, helper_raise_illegal_instruction, (void))
>> DEF_HELPER(void, helper_raise_slot_illegal_instruction, (void))
>> DEF_HELPER(void, helper_debug, (void))
>> -DEF_HELPER(void, helper_sleep, (void))
>> +DEF_HELPER(void, helper_sleep, (int32_t))
>> DEF_HELPER(void, helper_trapa, (uint32_t))
>>
>> DEF_HELPER(uint32_t, helper_addv, (uint32_t, uint32_t))
>> Index: target-sh4/op_helper.c
>> ===================================================================
>> --- target-sh4/op_helper.c (revision 5132)
>> +++ target-sh4/op_helper.c (working copy)
>> @@ -94,10 +94,11 @@
>> cpu_loop_exit();
>> }
>>
>> -void helper_sleep(void)
>> +void helper_sleep(int32_t next_pc)
>
> Given then env->pc is unsigned, I think it should be uint32_t instead of
> int32_t here. Same in helper.h
This is my mistake. Thanks for pointing it out.
This mail is attached with new patch of sleep instruction part.
Regards,
Shin-ichiro KAWASAKI
Index: trunk/target-sh4/helper.h
===================================================================
--- trunk/target-sh4/helper.h (revision 5219)
+++ trunk/target-sh4/helper.h (working copy)
@@ -6,7 +6,7 @@
DEF_HELPER(void, helper_raise_illegal_instruction, (void))
DEF_HELPER(void, helper_raise_slot_illegal_instruction, (void))
DEF_HELPER(void, helper_debug, (void))
-DEF_HELPER(void, helper_sleep, (void))
+DEF_HELPER(void, helper_sleep, (uint32_t))
DEF_HELPER(void, helper_trapa, (uint32_t))
DEF_HELPER(uint32_t, helper_addv, (uint32_t, uint32_t))
Index: trunk/target-sh4/op_helper.c
===================================================================
--- trunk/target-sh4/op_helper.c (revision 5219)
+++ trunk/target-sh4/op_helper.c (working copy)
@@ -94,10 +94,11 @@
cpu_loop_exit();
}
-void helper_sleep(void)
+void helper_sleep(uint32_t next_pc)
{
env->halted = 1;
env->exception_index = EXCP_HLT;
+ env->pc = next_pc;
cpu_loop_exit();
}
Index: trunk/target-sh4/translate.c
===================================================================
--- trunk/target-sh4/translate.c (revision 5219)
+++ trunk/target-sh4/translate.c (working copy)
@@ -505,7 +505,7 @@
return;
case 0x001b: /* sleep */
if (ctx->memidx) {
- tcg_gen_helper_0_0(helper_sleep);
+ tcg_gen_helper_0_1(helper_sleep, tcg_const_i32(ctx->pc + 2));
} else {
tcg_gen_helper_0_0(helper_raise_illegal_instruction);
ctx->bstate = BS_EXCP;
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Qemu-devel] [PATCH] [RESEND] SH4 : Serial controller improvement
2008-09-14 16:56 ` Aurelien Jarno
2008-09-15 5:28 ` [PATCH] [RESEND] SH4 : sleep instruction bug fix (was Re: [Qemu-devel] [PATCH] SH4: Serial controller improvement and sleep op bug fix) Shin-ichiro KAWASAKI
@ 2008-09-15 5:42 ` Shin-ichiro KAWASAKI
2008-09-15 7:06 ` Aurelien Jarno
1 sibling, 1 reply; 6+ messages in thread
From: Shin-ichiro KAWASAKI @ 2008-09-15 5:42 UTC (permalink / raw)
To: qemu-devel
Aurelien Jarno wrote:
>> @@ -194,6 +224,15 @@
>> s->flags |= SH_SERIAL_FLAG_TDE | SH_SERIAL_FLAG_TEND;
>>
>> break;
>> + case 0x14:
>> + if (s->rx_cnt > 0) {
>> + ret = s->rx_fifo[0];
>> + s->rx_cnt--;
>> + memmove(&s->rx_fifo[0], &s->rx_fifo[1], s->rx_cnt);
>
> I don't think moving the whole buffer each time a character is read is a
> good idea. I would suggest to use a circular buffer instead. Have a look
> at hw/serial.c how it is done.
I see. I'm sending the patch with a circular buffer.
Thanks for your comments!
Regards,
Shin-ichiro KAWASAKI
Index: trunk/hw/sh_serial.c
===================================================================
--- trunk/hw/sh_serial.c (revision 5219)
+++ trunk/hw/sh_serial.c (working copy)
@@ -37,6 +37,8 @@
#define SH_SERIAL_FLAG_BRK (1 << 3)
#define SH_SERIAL_FLAG_DR (1 << 4)
+#define SH_RX_FIFO_LENGTH (16)
+
typedef struct {
uint8_t smr;
uint8_t brr;
@@ -46,13 +48,16 @@
uint16_t fcr;
uint8_t sptr;
- uint8_t rx_fifo[16]; /* frdr / rdr */
+ uint8_t rx_fifo[SH_RX_FIFO_LENGTH]; /* frdr / rdr */
uint8_t rx_cnt;
+ uint8_t rx_tail;
+ uint8_t rx_head;
target_phys_addr_t base;
int freq;
int feat;
int flags;
+ int rtrg;
CharDriverState *chr;
@@ -63,6 +68,14 @@
struct intc_source *bri;
} sh_serial_state;
+static void sh_serial_clear_fifo(sh_serial_state * s)
+{
+ memset(s->rx_fifo, 0, SH_RX_FIFO_LENGTH);
+ s->rx_cnt = 0;
+ s->rx_head = 0;
+ s->rx_tail = 0;
+}
+
static void sh_serial_ioport_write(void *opaque, uint32_t offs, uint32_t val)
{
sh_serial_state *s = opaque;
@@ -80,6 +93,7 @@
s->brr = val;
return;
case 0x08: /* SCR */
+ /* TODO : For SH7751, SCIF mask should be 0xfb. */
s->scr = val & ((s->feat & SH_SERIAL_FEAT_SCIF) ? 0xfa : 0xff);
if (!(val & (1 << 5)))
s->flags |= SH_SERIAL_FLAG_TEND;
@@ -89,6 +103,9 @@
else if (!(val & (1 << 7)) && s->txi->asserted)
sh_intc_toggle_source(s->txi, 0, -1);
}
+ if (!(val & (1 << 6)) && s->rxi->asserted) {
+ sh_intc_toggle_source(s->rxi, 0, -1);
+ }
return;
case 0x0c: /* FTDR / TDR */
if (s->chr) {
@@ -117,12 +134,37 @@
s->flags &= ~SH_SERIAL_FLAG_RDF;
if (!(val & (1 << 0)))
s->flags &= ~SH_SERIAL_FLAG_DR;
+
+ if (!(val & (1 << 1)) || !(val & (1 << 0))) {
+ if (s->rxi && s->rxi->asserted) {
+ sh_intc_toggle_source(s->rxi, 0, -1);
+ }
+ }
return;
case 0x18: /* FCR */
s->fcr = val;
+ switch ((val >> 6) & 3) {
+ case 0:
+ s->rtrg = 1;
+ break;
+ case 1:
+ s->rtrg = 4;
+ break;
+ case 2:
+ s->rtrg = 8;
+ break;
+ case 3:
+ s->rtrg = 14;
+ break;
+ }
+ if (val & (1 << 1)) {
+ sh_serial_clear_fifo(s);
+ s->sr &= ~(1 << 1);
+ }
+
return;
case 0x20: /* SPTR */
- s->sptr = val;
+ s->sptr = val & 0xf3;
return;
case 0x24: /* LSR */
return;
@@ -194,6 +236,16 @@
s->flags |= SH_SERIAL_FLAG_TDE | SH_SERIAL_FLAG_TEND;
break;
+ case 0x14:
+ if (s->rx_cnt > 0) {
+ ret = s->rx_fifo[s->rx_tail++];
+ s->rx_cnt--;
+ if (s->rx_tail == SH_RX_FIFO_LENGTH)
+ s->rx_tail = 0;
+ if (s->rx_cnt < s->rtrg)
+ s->flags &= ~SH_SERIAL_FLAG_RDF;
+ }
+ break;
#if 0
case 0x18:
ret = s->fcr;
@@ -219,6 +271,9 @@
case 0x10:
ret = 0;
break;
+ case 0x14:
+ ret = s->rx_fifo[0];
+ break;
case 0x1c:
ret = s->sptr;
break;
@@ -240,15 +295,33 @@
static int sh_serial_can_receive(sh_serial_state *s)
{
- return 0;
+ return s->scr & (1 << 4);
}
static void sh_serial_receive_byte(sh_serial_state *s, int ch)
{
+ if (s->feat & SH_SERIAL_FEAT_SCIF) {
+ if (s->rx_cnt < SH_RX_FIFO_LENGTH) {
+ s->rx_fifo[s->rx_head++] = ch;
+ if (s->rx_head == SH_RX_FIFO_LENGTH)
+ s->rx_head = 0;
+ s->rx_cnt++;
+ if (s->rx_cnt >= s->rtrg) {
+ s->flags |= SH_SERIAL_FLAG_RDF;
+ if (s->scr & (1 << 6) && s->rxi) {
+ sh_intc_toggle_source(s->rxi, 0, 1);
+ }
+ }
+ }
+ } else {
+ s->rx_fifo[0] = ch;
+ }
}
static void sh_serial_receive_break(sh_serial_state *s)
{
+ if (s->feat & SH_SERIAL_FEAT_SCIF)
+ s->sr |= (1 << 4);
}
static int sh_serial_can_receive1(void *opaque)
@@ -313,6 +386,7 @@
s->base = base;
s->feat = feat;
s->flags = SH_SERIAL_FLAG_TEND | SH_SERIAL_FLAG_TDE;
+ s->rtrg = 1;
s->smr = 0;
s->brr = 0xff;
@@ -326,7 +400,7 @@
s->dr = 0xff;
}
- s->rx_cnt = 0;
+ sh_serial_clear_fifo(s);
s_io_memory = cpu_register_io_memory(0, sh_serial_readfn,
sh_serial_writefn, s);
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] [RESEND] SH4 : sleep instruction bug fix (was Re: [Qemu-devel] [PATCH] SH4: Serial controller improvement and sleep op bug fix)
2008-09-15 5:28 ` [PATCH] [RESEND] SH4 : sleep instruction bug fix (was Re: [Qemu-devel] [PATCH] SH4: Serial controller improvement and sleep op bug fix) Shin-ichiro KAWASAKI
@ 2008-09-15 6:42 ` Aurelien Jarno
0 siblings, 0 replies; 6+ messages in thread
From: Aurelien Jarno @ 2008-09-15 6:42 UTC (permalink / raw)
To: qemu-devel
On Mon, Sep 15, 2008 at 02:28:53PM +0900, Shin-ichiro KAWASAKI wrote:
> Thank you for your response!
>
> Aurelien Jarno wrote:
>> On Sun, Sep 07, 2008 at 11:34:10PM +0900, Shin-ichiro KAWASAKI wrote:
>>
>> Next time could you please send two different patches, so that one patch
>> doesn't block the other.
>
> I see. I'll do so next time. Working with this patch, I'll resend twice to
> divide into sleep instruction part and serial part.
>
>
>>> Index: target-sh4/helper.h
>>> ===================================================================
>>> --- target-sh4/helper.h (revision 5132)
>>> +++ target-sh4/helper.h (working copy)
>>> @@ -6,7 +6,7 @@
>>> DEF_HELPER(void, helper_raise_illegal_instruction, (void))
>>> DEF_HELPER(void, helper_raise_slot_illegal_instruction, (void))
>>> DEF_HELPER(void, helper_debug, (void))
>>> -DEF_HELPER(void, helper_sleep, (void))
>>> +DEF_HELPER(void, helper_sleep, (int32_t))
>>> DEF_HELPER(void, helper_trapa, (uint32_t))
>>> DEF_HELPER(uint32_t, helper_addv, (uint32_t, uint32_t))
>>> Index: target-sh4/op_helper.c
>>> ===================================================================
>>> --- target-sh4/op_helper.c (revision 5132)
>>> +++ target-sh4/op_helper.c (working copy)
>>> @@ -94,10 +94,11 @@
>>> cpu_loop_exit();
>>> }
>>> -void helper_sleep(void)
>>> +void helper_sleep(int32_t next_pc)
>>
>> Given then env->pc is unsigned, I think it should be uint32_t instead of
>> int32_t here. Same in helper.h
>
> This is my mistake. Thanks for pointing it out.
> This mail is attached with new patch of sleep instruction part.
Thanks, applied.
> Regards,
> Shin-ichiro KAWASAKI
>
>
> Index: trunk/target-sh4/helper.h
> ===================================================================
> --- trunk/target-sh4/helper.h (revision 5219)
> +++ trunk/target-sh4/helper.h (working copy)
> @@ -6,7 +6,7 @@
> DEF_HELPER(void, helper_raise_illegal_instruction, (void))
> DEF_HELPER(void, helper_raise_slot_illegal_instruction, (void))
> DEF_HELPER(void, helper_debug, (void))
> -DEF_HELPER(void, helper_sleep, (void))
> +DEF_HELPER(void, helper_sleep, (uint32_t))
> DEF_HELPER(void, helper_trapa, (uint32_t))
>
> DEF_HELPER(uint32_t, helper_addv, (uint32_t, uint32_t))
> Index: trunk/target-sh4/op_helper.c
> ===================================================================
> --- trunk/target-sh4/op_helper.c (revision 5219)
> +++ trunk/target-sh4/op_helper.c (working copy)
> @@ -94,10 +94,11 @@
> cpu_loop_exit();
> }
>
> -void helper_sleep(void)
> +void helper_sleep(uint32_t next_pc)
> {
> env->halted = 1;
> env->exception_index = EXCP_HLT;
> + env->pc = next_pc;
> cpu_loop_exit();
> }
>
> Index: trunk/target-sh4/translate.c
> ===================================================================
> --- trunk/target-sh4/translate.c (revision 5219)
> +++ trunk/target-sh4/translate.c (working copy)
> @@ -505,7 +505,7 @@
> return;
> case 0x001b: /* sleep */
> if (ctx->memidx) {
> - tcg_gen_helper_0_0(helper_sleep);
> + tcg_gen_helper_0_1(helper_sleep, tcg_const_i32(ctx->pc + 2));
> } else {
> tcg_gen_helper_0_0(helper_raise_illegal_instruction);
> ctx->bstate = BS_EXCP;
>
>
>
--
.''`. Aurelien Jarno | GPG: 1024D/F1BCDB73
: :' : Debian developer | Electrical Engineer
`. `' aurel32@debian.org | aurelien@aurel32.net
`- people.debian.org/~aurel32 | www.aurel32.net
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] [RESEND] SH4 : Serial controller improvement
2008-09-15 5:42 ` [Qemu-devel] [PATCH] [RESEND] SH4 : Serial controller improvement Shin-ichiro KAWASAKI
@ 2008-09-15 7:06 ` Aurelien Jarno
0 siblings, 0 replies; 6+ messages in thread
From: Aurelien Jarno @ 2008-09-15 7:06 UTC (permalink / raw)
To: qemu-devel
On Mon, Sep 15, 2008 at 02:42:45PM +0900, Shin-ichiro KAWASAKI wrote:
> Aurelien Jarno wrote:
>>> @@ -194,6 +224,15 @@
>>> s->flags |= SH_SERIAL_FLAG_TDE | SH_SERIAL_FLAG_TEND;
>>> break;
>>> + case 0x14:
>>> + if (s->rx_cnt > 0) {
>>> + ret = s->rx_fifo[0];
>>> + s->rx_cnt--;
>>> + memmove(&s->rx_fifo[0], &s->rx_fifo[1], s->rx_cnt);
>>
>> I don't think moving the whole buffer each time a character is read is a
>> good idea. I would suggest to use a circular buffer instead. Have a look
>> at hw/serial.c how it is done.
>
> I see. I'm sending the patch with a circular buffer.
> Thanks for your comments!
Thanks, applied.
> Regards,
> Shin-ichiro KAWASAKI
>
>
> Index: trunk/hw/sh_serial.c
> ===================================================================
> --- trunk/hw/sh_serial.c (revision 5219)
> +++ trunk/hw/sh_serial.c (working copy)
> @@ -37,6 +37,8 @@
> #define SH_SERIAL_FLAG_BRK (1 << 3)
> #define SH_SERIAL_FLAG_DR (1 << 4)
>
> +#define SH_RX_FIFO_LENGTH (16)
> +
> typedef struct {
> uint8_t smr;
> uint8_t brr;
> @@ -46,13 +48,16 @@
> uint16_t fcr;
> uint8_t sptr;
>
> - uint8_t rx_fifo[16]; /* frdr / rdr */
> + uint8_t rx_fifo[SH_RX_FIFO_LENGTH]; /* frdr / rdr */
> uint8_t rx_cnt;
> + uint8_t rx_tail;
> + uint8_t rx_head;
>
> target_phys_addr_t base;
> int freq;
> int feat;
> int flags;
> + int rtrg;
>
> CharDriverState *chr;
>
> @@ -63,6 +68,14 @@
> struct intc_source *bri;
> } sh_serial_state;
>
> +static void sh_serial_clear_fifo(sh_serial_state * s)
> +{
> + memset(s->rx_fifo, 0, SH_RX_FIFO_LENGTH);
> + s->rx_cnt = 0;
> + s->rx_head = 0;
> + s->rx_tail = 0;
> +}
> +
> static void sh_serial_ioport_write(void *opaque, uint32_t offs, uint32_t val)
> {
> sh_serial_state *s = opaque;
> @@ -80,6 +93,7 @@
> s->brr = val;
> return;
> case 0x08: /* SCR */
> + /* TODO : For SH7751, SCIF mask should be 0xfb. */
> s->scr = val & ((s->feat & SH_SERIAL_FEAT_SCIF) ? 0xfa : 0xff);
> if (!(val & (1 << 5)))
> s->flags |= SH_SERIAL_FLAG_TEND;
> @@ -89,6 +103,9 @@
> else if (!(val & (1 << 7)) && s->txi->asserted)
> sh_intc_toggle_source(s->txi, 0, -1);
> }
> + if (!(val & (1 << 6)) && s->rxi->asserted) {
> + sh_intc_toggle_source(s->rxi, 0, -1);
> + }
> return;
> case 0x0c: /* FTDR / TDR */
> if (s->chr) {
> @@ -117,12 +134,37 @@
> s->flags &= ~SH_SERIAL_FLAG_RDF;
> if (!(val & (1 << 0)))
> s->flags &= ~SH_SERIAL_FLAG_DR;
> +
> + if (!(val & (1 << 1)) || !(val & (1 << 0))) {
> + if (s->rxi && s->rxi->asserted) {
> + sh_intc_toggle_source(s->rxi, 0, -1);
> + }
> + }
> return;
> case 0x18: /* FCR */
> s->fcr = val;
> + switch ((val >> 6) & 3) {
> + case 0:
> + s->rtrg = 1;
> + break;
> + case 1:
> + s->rtrg = 4;
> + break;
> + case 2:
> + s->rtrg = 8;
> + break;
> + case 3:
> + s->rtrg = 14;
> + break;
> + }
> + if (val & (1 << 1)) {
> + sh_serial_clear_fifo(s);
> + s->sr &= ~(1 << 1);
> + }
> +
> return;
> case 0x20: /* SPTR */
> - s->sptr = val;
> + s->sptr = val & 0xf3;
> return;
> case 0x24: /* LSR */
> return;
> @@ -194,6 +236,16 @@
> s->flags |= SH_SERIAL_FLAG_TDE | SH_SERIAL_FLAG_TEND;
>
> break;
> + case 0x14:
> + if (s->rx_cnt > 0) {
> + ret = s->rx_fifo[s->rx_tail++];
> + s->rx_cnt--;
> + if (s->rx_tail == SH_RX_FIFO_LENGTH)
> + s->rx_tail = 0;
> + if (s->rx_cnt < s->rtrg)
> + s->flags &= ~SH_SERIAL_FLAG_RDF;
> + }
> + break;
> #if 0
> case 0x18:
> ret = s->fcr;
> @@ -219,6 +271,9 @@
> case 0x10:
> ret = 0;
> break;
> + case 0x14:
> + ret = s->rx_fifo[0];
> + break;
> case 0x1c:
> ret = s->sptr;
> break;
> @@ -240,15 +295,33 @@
>
> static int sh_serial_can_receive(sh_serial_state *s)
> {
> - return 0;
> + return s->scr & (1 << 4);
> }
>
> static void sh_serial_receive_byte(sh_serial_state *s, int ch)
> {
> + if (s->feat & SH_SERIAL_FEAT_SCIF) {
> + if (s->rx_cnt < SH_RX_FIFO_LENGTH) {
> + s->rx_fifo[s->rx_head++] = ch;
> + if (s->rx_head == SH_RX_FIFO_LENGTH)
> + s->rx_head = 0;
> + s->rx_cnt++;
> + if (s->rx_cnt >= s->rtrg) {
> + s->flags |= SH_SERIAL_FLAG_RDF;
> + if (s->scr & (1 << 6) && s->rxi) {
> + sh_intc_toggle_source(s->rxi, 0, 1);
> + }
> + }
> + }
> + } else {
> + s->rx_fifo[0] = ch;
> + }
> }
>
> static void sh_serial_receive_break(sh_serial_state *s)
> {
> + if (s->feat & SH_SERIAL_FEAT_SCIF)
> + s->sr |= (1 << 4);
> }
>
> static int sh_serial_can_receive1(void *opaque)
> @@ -313,6 +386,7 @@
> s->base = base;
> s->feat = feat;
> s->flags = SH_SERIAL_FLAG_TEND | SH_SERIAL_FLAG_TDE;
> + s->rtrg = 1;
>
> s->smr = 0;
> s->brr = 0xff;
> @@ -326,7 +400,7 @@
> s->dr = 0xff;
> }
>
> - s->rx_cnt = 0;
> + sh_serial_clear_fifo(s);
>
> s_io_memory = cpu_register_io_memory(0, sh_serial_readfn,
> sh_serial_writefn, s);
>
>
>
--
.''`. Aurelien Jarno | GPG: 1024D/F1BCDB73
: :' : Debian developer | Electrical Engineer
`. `' aurel32@debian.org | aurelien@aurel32.net
`- people.debian.org/~aurel32 | www.aurel32.net
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2008-09-15 7:06 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-09-07 14:34 [Qemu-devel] [PATCH] SH4: Serial controller improvement and sleep op bug fix Shin-ichiro KAWASAKI
2008-09-14 16:56 ` Aurelien Jarno
2008-09-15 5:28 ` [PATCH] [RESEND] SH4 : sleep instruction bug fix (was Re: [Qemu-devel] [PATCH] SH4: Serial controller improvement and sleep op bug fix) Shin-ichiro KAWASAKI
2008-09-15 6:42 ` Aurelien Jarno
2008-09-15 5:42 ` [Qemu-devel] [PATCH] [RESEND] SH4 : Serial controller improvement Shin-ichiro KAWASAKI
2008-09-15 7:06 ` Aurelien Jarno
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).