[Qemu-devel] SH: support 7785 serial

[Qemu-devel] SH: support 7785 serial

[Vladimir Prus]

[-- Attachment #1: Type: Text/Plain, Size: 1016 bytes --]


These 3 patches make sh_serial.c support 7785 serial. The primary
difference is that on 7785, instead of single fifo count register
there are two separate ones -- for rx and tx. Patch 3 adds necessary
conditional logic. Patches 1 and 2 are cleanups that I've done alone
the way:

- Patch 1 makes it possible to change the size of RX fifo. It's 16
on 7751 and 64 on 7785. While I do not know of any breakage if we
use wrong size, it's still best to be correct.
- Patch 2 replaces a pile of hardcoded constants with macroses.
Also, the FSR register was handled strangely -- when written, we'd
look at the written value and set bits in the 'flags' field, and on
read, we'd reconstruct the value from the 'flags' field. There does
not seem to be any reason for such roundabout, so I've made the code
work with 'sr' directly.

This patch was tested both with r2d, using kernel and userland found
at:

	thttp://www.assembla.com/wiki/show/qemu-sh4/BuildingEnvironment

and with 7785, using a hand-made kernel.

- Volodya


[-- Attachment #2: 0001-Make-RX-fifo-size-configurable.patch --]
[-- Type: text/x-patch, Size: 2558 bytes --]

From 1d9dc814c6ad1e9e4c06e9b212c116716985b14e Mon Sep 17 00:00:00 2001
From: Vladimir Prus <vladimir@codesourcery.com>
Date: Thu, 2 Apr 2009 13:49:08 +0400
Subject: [PATCH 1/3] Make RX fifo size configurable.
To: qemu-devel@nongnu.org
X-KMail-Transport: CodeSourcery
X-KMail-Identity: 901867920

---
 hw/sh_serial.c |   15 ++++++++-------
 1 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/hw/sh_serial.c b/hw/sh_serial.c
index 4957c41..cb7c9d1 100644
--- a/hw/sh_serial.c
+++ b/hw/sh_serial.c
@@ -37,8 +37,6 @@
 #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;
@@ -48,10 +46,11 @@ typedef struct {
     uint16_t fcr;
     uint8_t sptr;
 
-    uint8_t rx_fifo[SH_RX_FIFO_LENGTH]; /* frdr / rdr */
+    uint8_t *rx_fifo; /* frdr / rdr */
     uint8_t rx_cnt;
     uint8_t rx_tail;
     uint8_t rx_head;
+    int rx_capacity;
 
     int freq;
     int feat;
@@ -69,7 +68,7 @@ typedef struct {
 
 static void sh_serial_clear_fifo(sh_serial_state * s)
 {
-    memset(s->rx_fifo, 0, SH_RX_FIFO_LENGTH);
+    memset(s->rx_fifo, 0, s->rx_capacity);
     s->rx_cnt = 0;
     s->rx_head = 0;
     s->rx_tail = 0;
@@ -236,7 +235,7 @@ static uint32_t sh_serial_ioport_read(void *opaque, uint32_t offs)
             if (s->rx_cnt > 0) {
                 ret = s->rx_fifo[s->rx_tail++];
                 s->rx_cnt--;
-                if (s->rx_tail == SH_RX_FIFO_LENGTH)
+                if (s->rx_tail == s->rx_capacity)
                     s->rx_tail = 0;
                 if (s->rx_cnt < s->rtrg)
                     s->flags &= ~SH_SERIAL_FLAG_RDF;
@@ -297,9 +296,9 @@ static int sh_serial_can_receive(sh_serial_state *s)
 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) {
+        if (s->rx_cnt < s->rx_capacity) {
             s->rx_fifo[s->rx_head++] = ch;
-            if (s->rx_head == SH_RX_FIFO_LENGTH)
+            if (s->rx_head == s->rx_capacity)
                 s->rx_head = 0;
             s->rx_cnt++;
             if (s->rx_cnt >= s->rtrg) {
@@ -393,6 +392,8 @@ void sh_serial_init (target_phys_addr_t base, int feat,
         s->dr = 0xff;
     }
 
+    s->rx_capacity = 16;
+    s->rx_fifo = (uint8_t *)malloc(s->rx_capacity);
     sh_serial_clear_fifo(s);
 
     s_io_memory = cpu_register_io_memory(0, sh_serial_readfn,
-- 
1.6.2.1


[-- Attachment #3: 0002-Use-symbolic-constants.patch --]
[-- Type: text/x-patch, Size: 5484 bytes --]

From a87364fd8cad59864417ec1eb87c02249f55d2fc Mon Sep 17 00:00:00 2001
From: Vladimir Prus <vladimir@codesourcery.com>
Date: Thu, 2 Apr 2009 17:20:09 +0400
Subject: [PATCH 2/3] Use symbolic constants.
To: qemu-devel@nongnu.org
X-KMail-Transport: CodeSourcery
X-KMail-Identity: 901867920

---
 hw/sh_serial.c |   72 ++++++++++++++++++++++---------------------------------
 1 files changed, 29 insertions(+), 43 deletions(-)

diff --git a/hw/sh_serial.c b/hw/sh_serial.c
index cb7c9d1..e004e28 100644
--- a/hw/sh_serial.c
+++ b/hw/sh_serial.c
@@ -31,11 +31,15 @@
 
 //#define DEBUG_SERIAL
 
-#define SH_SERIAL_FLAG_TEND (1 << 0)
-#define SH_SERIAL_FLAG_TDE  (1 << 1)
-#define SH_SERIAL_FLAG_RDF  (1 << 2)
-#define SH_SERIAL_FLAG_BRK  (1 << 3)
-#define SH_SERIAL_FLAG_DR   (1 << 4)
+#define SCR_RE   (1 << 4)
+#define SCR_TE   (1 << 5)
+#define SCR_RIE  (1 << 6)
+#define SCR_TIE  (1 << 7)
+
+#define FSR_DR   (1 << 0)
+#define FSR_RDF  (1 << 1)
+#define FSR_TDFE (1 << 5)
+#define FSR_TEND (1 << 6)
 
 typedef struct {
     uint8_t smr;
@@ -54,7 +58,6 @@ typedef struct {
 
     int freq;
     int feat;
-    int flags;
     int rtrg;
 
     CharDriverState *chr;
@@ -93,12 +96,12 @@ static void sh_serial_ioport_write(void *opaque, uint32_t offs, uint32_t val)
     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;
+        if (!(val & SCR_TE))
+            s->sr |= FSR_TEND;
         if ((s->feat & SH_SERIAL_FEAT_SCIF) && s->txi) {
-	    qemu_set_irq(s->txi, val & (1 << 7));
+	    qemu_set_irq(s->txi, val & SCR_TIE);
         }
-        if (!(val & (1 << 6))) {
+        if (!(val & SCR_RIE)) {
 	    qemu_set_irq(s->rxi, 0);
         }
         return;
@@ -108,7 +111,7 @@ static void sh_serial_ioport_write(void *opaque, uint32_t offs, uint32_t val)
             qemu_chr_write(s->chr, &ch, 1);
 	}
 	s->dr = val;
-	s->flags &= ~SH_SERIAL_FLAG_TDE;
+	s->sr &= ~FSR_TDFE;
         return;
 #if 0
     case 0x14: /* FRDR / RDR */
@@ -119,18 +122,11 @@ static void sh_serial_ioport_write(void *opaque, uint32_t offs, uint32_t val)
     if (s->feat & SH_SERIAL_FEAT_SCIF) {
         switch(offs) {
         case 0x10: /* FSR */
-            if (!(val & (1 << 6)))
-                s->flags &= ~SH_SERIAL_FLAG_TEND;
-            if (!(val & (1 << 5)))
-                s->flags &= ~SH_SERIAL_FLAG_TDE;
-            if (!(val & (1 << 4)))
-                s->flags &= ~SH_SERIAL_FLAG_BRK;
-            if (!(val & (1 << 1)))
-                s->flags &= ~SH_SERIAL_FLAG_RDF;
-            if (!(val & (1 << 0)))
-                s->flags &= ~SH_SERIAL_FLAG_DR;
-
-            if (!(val & (1 << 1)) || !(val & (1 << 0))) {
+            val &= 0xf3;
+            
+            s->sr = val;
+
+            if (!(val & FSR_RDF) || !(val & FSR_DR)) {
                 if (s->rxi) {
                     qemu_set_irq(s->rxi, 0);
                 }
@@ -214,21 +210,11 @@ static uint32_t sh_serial_ioport_read(void *opaque, uint32_t offs)
         case 0x08: /* SCR */
             ret = s->scr;
             break;
-        case 0x10: /* FSR */
-            ret = 0;
-            if (s->flags & SH_SERIAL_FLAG_TEND)
-                ret |= (1 << 6);
-            if (s->flags & SH_SERIAL_FLAG_TDE)
-                ret |= (1 << 5);
-            if (s->flags & SH_SERIAL_FLAG_BRK)
-                ret |= (1 << 4);
-            if (s->flags & SH_SERIAL_FLAG_RDF)
-                ret |= (1 << 1);
-            if (s->flags & SH_SERIAL_FLAG_DR)
-                ret |= (1 << 0);
-
-            if (s->scr & (1 << 5))
-                s->flags |= SH_SERIAL_FLAG_TDE | SH_SERIAL_FLAG_TEND;
+        case 0x10: /* FSR */            
+            ret = s->sr;
+
+            if (s->scr & SCR_TE)
+                s->sr |= FSR_TDFE | FSR_TEND;
 
             break;
         case 0x14:
@@ -238,7 +224,7 @@ static uint32_t sh_serial_ioport_read(void *opaque, uint32_t offs)
                 if (s->rx_tail == s->rx_capacity)
                     s->rx_tail = 0;
                 if (s->rx_cnt < s->rtrg)
-                    s->flags &= ~SH_SERIAL_FLAG_RDF;
+                    s->sr &= ~FSR_RDF;
             }
             break;
 #if 0
@@ -290,7 +276,7 @@ static uint32_t sh_serial_ioport_read(void *opaque, uint32_t offs)
 
 static int sh_serial_can_receive(sh_serial_state *s)
 {
-    return s->scr & (1 << 4);
+    return s->scr & SCR_RE;
 }
 
 static void sh_serial_receive_byte(sh_serial_state *s, int ch)
@@ -302,8 +288,8 @@ static void sh_serial_receive_byte(sh_serial_state *s, int ch)
                 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) {
+                s->sr |= FSR_RDF;
+                if (s->scr & SCR_RIE && s->rxi) {
                     qemu_set_irq(s->rxi, 1);
                 }
             }
@@ -377,7 +363,7 @@ void sh_serial_init (target_phys_addr_t base, int feat,
     s = qemu_mallocz(sizeof(sh_serial_state));
 
     s->feat = feat;
-    s->flags = SH_SERIAL_FLAG_TEND | SH_SERIAL_FLAG_TDE;
+    s->sr = FSR_TEND | FSR_TDFE;
     s->rtrg = 1;
 
     s->smr = 0;
-- 
1.6.2.1


[-- Attachment #4: 0003-Support-7785-s-serial.patch --]
[-- Type: text/x-patch, Size: 4585 bytes --]

From 4ae7e3fc941add72b2ff11ce95a579a09b79b4e6 Mon Sep 17 00:00:00 2001
From: Vladimir Prus <vladimir@codesourcery.com>
Date: Thu, 2 Apr 2009 20:44:01 +0400
Subject: [PATCH 3/3] Support 7785's serial.
To: qemu-devel@nongnu.org
X-KMail-Transport: CodeSourcery
X-KMail-Identity: 901867920

---
 hw/sh.h        |    1 +
 hw/sh_serial.c |   73 +++++++++++++++++++++++++++++++++++++++++++++++++------
 2 files changed, 66 insertions(+), 8 deletions(-)

diff --git a/hw/sh.h b/hw/sh.h
index 5e3c22b..da051f1 100644
--- a/hw/sh.h
+++ b/hw/sh.h
@@ -37,6 +37,7 @@ void tmu012_init(target_phys_addr_t base, int feat, uint32_t freq,
 
 /* sh_serial.c */
 #define SH_SERIAL_FEAT_SCIF (1 << 0)
+#define SH_SERIAL_FEAT_7785 (1 << 1)
 void sh_serial_init (target_phys_addr_t base, int feat,
 		     uint32_t freq, CharDriverState *chr,
 		     qemu_irq eri_source,
diff --git a/hw/sh_serial.c b/hw/sh_serial.c
index e004e28..4f606ff 100644
--- a/hw/sh_serial.c
+++ b/hw/sh_serial.c
@@ -154,10 +154,28 @@ static void sh_serial_ioport_write(void *opaque, uint32_t offs, uint32_t val)
             }
 
             return;
-        case 0x20: /* SPTR */
-            s->sptr = val & 0xf3;
+        case 0x20: 
+            if (s->feat & SH_SERIAL_FEAT_7785)
+            {
+                /* Recieve fifo data count register, not writable.  */
+            }
+            else
+            {
+                 /* SPTR */
+                s->sptr = val & 0xf3;
+            }
             return;
-        case 0x24: /* LSR */
+        case 0x24: 
+            if (s->feat & SH_SERIAL_FEAT_7785)
+            {
+                s->sptr = val & 0xf3;
+            }
+            else
+            {
+               /* LSR */
+            }
+            return;
+        case 0x28:
             return;
         }
     }
@@ -233,12 +251,47 @@ static uint32_t sh_serial_ioport_read(void *opaque, uint32_t offs)
             break;
 #endif
         case 0x1c:
-            ret = s->rx_cnt;
+            if (s->feat & SH_SERIAL_FEAT_7785)
+            {
+                /* Trasmit fifo data count.  Because we immediate send
+                   everything, we also claim the fifo is always empty.  */
+                ret = 0;
+            }
+            else
+            {
+                /* On 7751, this is unified receive/trasmit fifo data
+                   count register.  The received data count is in low
+                   8 bits. In QEMU, transmit fifo is always empty, so
+                   return just receive count.  */
+                ret = s->rx_cnt;
+            }
             break;
         case 0x20:
-            ret = s->sptr;
+            if (s->feat & SH_SERIAL_FEAT_7785)
+            {
+                /* On 7785, there's separate trasmit fifo data register.  */
+                ret = s->rx_cnt;
+            }
+            else
+            {
+                ret = s->sptr;
+            }
             break;
         case 0x24:
+            /* On 7785, this is serial port register.  On 7751, this is line
+               status register.  */
+            if (s->feat & SH_SERIAL_FEAT_7785)
+            {
+                ret = s->sptr;
+            }
+            else
+            {
+                ret = 0;
+            }
+            break;
+        case 0x28:
+            /* On 7785, this is line status register.  */
+            assert(s->feat & SH_SERIAL_FEAT_7785);
             ret = 0;
             break;
         }
@@ -359,6 +412,7 @@ void sh_serial_init (target_phys_addr_t base, int feat,
 {
     sh_serial_state *s;
     int s_io_memory;
+    int is_7785 = (feat & SH_SERIAL_FEAT_7785);
 
     s = qemu_mallocz(sizeof(sh_serial_state));
 
@@ -378,14 +432,17 @@ void sh_serial_init (target_phys_addr_t base, int feat,
         s->dr = 0xff;
     }
 
-    s->rx_capacity = 16;
+    s->rx_capacity = is_7785 ? 64 : 16;
     s->rx_fifo = (uint8_t *)malloc(s->rx_capacity);
     sh_serial_clear_fifo(s);
 
     s_io_memory = cpu_register_io_memory(0, sh_serial_readfn,
 					 sh_serial_writefn, s);
-    cpu_register_physical_memory(P4ADDR(base), 0x28, s_io_memory);
-    cpu_register_physical_memory(A7ADDR(base), 0x28, s_io_memory);
+    cpu_register_physical_memory(P4ADDR(base), 
+                                 is_7785 ? 0x2E : 0x28, s_io_memory);
+    cpu_register_physical_memory(A7ADDR(base), 
+                                 is_7785 ? 0x2E : 0x28, s_io_memory);
+
 
     s->chr = chr;
 
-- 
1.6.2.1

Re: [Qemu-devel] SH: support 7785 serial

[Shin-ichiro KAWASAKI]

Hi, Vladimir.
Thank you for your work.

Vladimir Prus wrote:
> These 3 patches make sh_serial.c support 7785 serial. The primary
> difference is that on 7785, instead of single fifo count register
> there are two separate ones -- for rx and tx. Patch 3 adds necessary
> conditional logic. Patches 1 and 2 are cleanups that I've done alone
> the way:
> 
> - Patch 1 makes it possible to change the size of RX fifo. It's 16
> on 7751 and 64 on 7785. While I do not know of any breakage if we
> use wrong size, it's still best to be correct.
> - Patch 2 replaces a pile of hardcoded constants with macroses.
> Also, the FSR register was handled strangely -- when written, we'd
> look at the written value and set bits in the 'flags' field, and on
> read, we'd reconstruct the value from the 'flags' field. There does
> not seem to be any reason for such roundabout, so I've made the code
> work with 'sr' directly.
> 
> This patch was tested both with r2d, using kernel and userland found
> at:
> 
> 	thttp://www.assembla.com/wiki/show/qemu-sh4/BuildingEnvironment
> 
> and with 7785, using a hand-made kernel.

Patch 2 produces a trouble in my environment.
For r2d, the output to SCIF from kernel is OK, but output from
shell is broken by inserted white space, like follows.

(before applying patch 2)
 # ls

(after applying patch2)
 #    l  s  

Do you have time to investigate it?


Regards,
Shin-ichiro KAWASAKI

[Qemu-devel] Re: SH: support 7785 serial

[Vladimir Prus]

Shin-ichiro KAWASAKI wrote:

> Hi, Vladimir.
> Thank you for your work.
> 
> Vladimir Prus wrote:
>> These 3 patches make sh_serial.c support 7785 serial. The primary
>> difference is that on 7785, instead of single fifo count register
>> there are two separate ones -- for rx and tx. Patch 3 adds necessary
>> conditional logic. Patches 1 and 2 are cleanups that I've done alone
>> the way:
>> 
>> - Patch 1 makes it possible to change the size of RX fifo. It's 16
>> on 7751 and 64 on 7785. While I do not know of any breakage if we
>> use wrong size, it's still best to be correct.
>> - Patch 2 replaces a pile of hardcoded constants with macroses.
>> Also, the FSR register was handled strangely -- when written, we'd
>> look at the written value and set bits in the 'flags' field, and on
>> read, we'd reconstruct the value from the 'flags' field. There does
>> not seem to be any reason for such roundabout, so I've made the code
>> work with 'sr' directly.
>> 
>> This patch was tested both with r2d, using kernel and userland found
>> at:
>> 
>> thttp://www.assembla.com/wiki/show/qemu-sh4/BuildingEnvironment
>> 
>> and with 7785, using a hand-made kernel.
> 
> Patch 2 produces a trouble in my environment.
> For r2d, the output to SCIF from kernel is OK, but output from
> shell is broken by inserted white space, like follows.
> 
> (before applying patch 2)
>  # ls
> 
> (after applying patch2)
>  #    l  s
> 
> Do you have time to investigate it?

Shin-ichiro,

I could not reproduce this locally. Did you tried typing this in virtual console
that QEMU pops up, or in the console where QEMU was started. For me, the former
does not accept any input at all.

Would it be possible for you to provide me with
- Source tree of qemu that you've tested this with? Pointer to some git commit
in some branch in some git repository will be OK.
- The kernel zImage you have used it
- The disk image you have used
- The command line for qemu

For the latter three, I have used what you have put at:

        http://www.assembla.com/wiki/show/qemu-sh4/BuildingEnvironment

but I want to be sure there's no accidental differences. And for qemu source tree,
I want to make sure I did not unintentially based by patch on some wrong state.

Thanks,
Volodya

Re: [Qemu-devel] Re: SH: support 7785 serial

[Shin-ichiro KAWASAKI]

Vladimir Prus wrote:
> Shin-ichiro KAWASAKI wrote:
> 
>> Hi, Vladimir.
>> Thank you for your work.
>>
>> Vladimir Prus wrote:
>>> These 3 patches make sh_serial.c support 7785 serial. The primary
>>> difference is that on 7785, instead of single fifo count register
>>> there are two separate ones -- for rx and tx. Patch 3 adds necessary
>>> conditional logic. Patches 1 and 2 are cleanups that I've done alone
>>> the way:
>>>
>>> - Patch 1 makes it possible to change the size of RX fifo. It's 16
>>> on 7751 and 64 on 7785. While I do not know of any breakage if we
>>> use wrong size, it's still best to be correct.
>>> - Patch 2 replaces a pile of hardcoded constants with macroses.
>>> Also, the FSR register was handled strangely -- when written, we'd
>>> look at the written value and set bits in the 'flags' field, and on
>>> read, we'd reconstruct the value from the 'flags' field. There does
>>> not seem to be any reason for such roundabout, so I've made the code
>>> work with 'sr' directly.
>>>
>>> This patch was tested both with r2d, using kernel and userland found
>>> at:
>>>
>>> thttp://www.assembla.com/wiki/show/qemu-sh4/BuildingEnvironment
>>>
>>> and with 7785, using a hand-made kernel.
>> Patch 2 produces a trouble in my environment.
>> For r2d, the output to SCIF from kernel is OK, but output from
>> shell is broken by inserted white space, like follows.
>>
>> (before applying patch 2)
>>  # ls
>>
>> (after applying patch2)
>>  #    l  s
>>
>> Do you have time to investigate it?
> 
> Shin-ichiro,
> 
> I could not reproduce this locally. Did you tried typing this in virtual console
> that QEMU pops up, or in the console where QEMU was started. For me, the former
> does not accept any input at all.

Thank you for your work.  I checked the defect again.
Please read following lines to make current status clear.

> Would it be possible for you to provide me with
> - Source tree of qemu that you've tested this with? Pointer to some git commit
> in some branch in some git repository will be OK.

For this case, I use the qemu trank svn tree, rev 7006.

> - The kernel zImage you have used it
> - The disk image you have used

Today, I updated the image package.  Before update, sh-test-0.1.tar.bz2
has been found at the URL, I guess you use it. Now, sh-test-0.2.tar.bz2
is there.

The trouble I reported happens, if following two conditions are both true.
 - your patch #2 is applied to qemu,
 - using zImage in 'sh-test-0.2.tar.bz2'

It does not happen for zImage in 'sh-test-0.1.tar.bz2' regardless of your
patch.  And it does not happen for qemu on which your patches are not applied,
regardless of the zImage difference.

> - The command line for qemu

The exact command line I used is as follows.

../savannah-current/trunk/sh4-softmmu/qemu-system-sh4 -M r2d -kernel ./sh-test-0.2/zImage -append "console=tty0 console=ttySC0,115200 earlyprintk=serial root=/dev/sda1 noiotrap" -hda ./sh-test-0.1/sh-linux-mini.img -serial null -serial stdio -usb -usbdevice keyboard -k ja -L ../savannah-current/trunk -monitor pty


> For the latter three, I have used what you have put at:
> 
>         http://www.assembla.com/wiki/show/qemu-sh4/BuildingEnvironment
> 
> but I want to be sure there's no accidental differences. And for qemu source tree,
> I want to make sure I did not unintentially based by patch on some wrong state.


zImage in sh-test-0.1.tar.bz2 is built from Linux 2.6.28 source tree.
On the other hand, that in sh-test-0.2.tar.bz2 is built from SH-Linux
branch, which is based on 2.6.29 and will be 2.6.30 rc.  I'm not yet
sure which modification against linux kernel causes this.  I'll investigate
it, and report it within several days.


Regards,
Shin-ichiro KAWASAKI

[Qemu-devel] Re: Re: SH: support 7785 serial

[Vladimir Prus]

Shin-ichiro KAWASAKI wrote:

> The trouble I reported happens, if following two conditions are both true.
>  - your patch #2 is applied to qemu,
>  - using zImage in 'sh-test-0.2.tar.bz2'
> 
> It does not happen for zImage in 'sh-test-0.1.tar.bz2' regardless of your
> patch.  And it does not happen for qemu on which your patches are not applied,
> regardless of the zImage difference.

Thank you, I can now see the problem. I will try to diagnose and fix it this week.

- Volodya

[Qemu-devel] Re: SH: support 7785 serial

[Vladimir Prus]

[-- Attachment #1: Type: text/plain, Size: 678 bytes --]

Shin-ichiro KAWASAKI wrote:

>> This patch was tested both with r2d, using kernel and userland found
>> at:
>> 
>> thttp://www.assembla.com/wiki/show/qemu-sh4/BuildingEnvironment
>> 
>> and with 7785, using a hand-made kernel.
> 
> Patch 2 produces a trouble in my environment.
> For r2d, the output to SCIF from kernel is OK, but output from
> shell is broken by inserted white space, like follows.
> 
> (before applying patch 2)
>  # ls
> 
> (after applying patch2)
>  #    l  s
> 
> Do you have time to investigate it?

The attached revision of the patch fixes the problem for me. I failed to
account for the fact that one cannot write '1' bit into FSR register.

- Volodya


[-- Attachment #2: 0002-Use-symbolic-constants.patch --]
[-- Type: application/mbox, Size: 5576 bytes --]

Re: [Qemu-devel] Re: SH: support 7785 serial

[Shin-ichiro KAWASAKI]

Vladimir Prus wrote:
> Shin-ichiro KAWASAKI wrote:
> 
>>> This patch was tested both with r2d, using kernel and userland found
>>> at:
>>>
>>> thttp://www.assembla.com/wiki/show/qemu-sh4/BuildingEnvironment
>>>
>>> and with 7785, using a hand-made kernel.
>> Patch 2 produces a trouble in my environment.
>> For r2d, the output to SCIF from kernel is OK, but output from
>> shell is broken by inserted white space, like follows.
>>
>> (before applying patch 2)
>>  # ls
>>
>> (after applying patch2)
>>  #    l  s
>>
>> Do you have time to investigate it?
> 
> The attached revision of the patch fixes the problem for me. I failed to
> account for the fact that one cannot write '1' bit into FSR register.

I've checked that the new patch avoids the problem.
Those 3 patches all seem OK for me.  Thank you!

Regards,
Shin-ichiro KAWASAKI

Re: [Qemu-devel] Re: SH: support 7785 serial

[Aurelien Jarno]

On Sat, Apr 18, 2009 at 08:47:11PM +0900, Shin-ichiro KAWASAKI wrote:
> Vladimir Prus wrote:
>> Shin-ichiro KAWASAKI wrote:
>>
>>>> This patch was tested both with r2d, using kernel and userland found
>>>> at:
>>>>
>>>> thttp://www.assembla.com/wiki/show/qemu-sh4/BuildingEnvironment
>>>>
>>>> and with 7785, using a hand-made kernel.
>>> Patch 2 produces a trouble in my environment.
>>> For r2d, the output to SCIF from kernel is OK, but output from
>>> shell is broken by inserted white space, like follows.
>>>
>>> (before applying patch 2)
>>>  # ls
>>>
>>> (after applying patch2)
>>>  #    l  s
>>>
>>> Do you have time to investigate it?
>>
>> The attached revision of the patch fixes the problem for me. I failed to
>> account for the fact that one cannot write '1' bit into FSR register.
>
> I've checked that the new patch avoids the problem.
> Those 3 patches all seem OK for me.  Thank you!
>

I have looked at the three patches, they all looks ok, except for coding
style issues of block structures. Vladimir, could you please fix that
(see file CODING_STYLE, 4. Block structure) and send the patches with a
Signed-off-by: ? I'll apply them.

-- 
Aurelien Jarno	                        GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net

Re: [Qemu-devel] Re: SH: support 7785 serial

[Vladimir Prus]

[-- Attachment #1: Type: Text/Plain, Size: 1449 bytes --]

On Saturday 18 April 2009 20:37:57 Aurelien Jarno wrote:
> On Sat, Apr 18, 2009 at 08:47:11PM +0900, Shin-ichiro KAWASAKI wrote:
> > Vladimir Prus wrote:
> >> Shin-ichiro KAWASAKI wrote:
> >>
> >>>> This patch was tested both with r2d, using kernel and userland found
> >>>> at:
> >>>>
> >>>> thttp://www.assembla.com/wiki/show/qemu-sh4/BuildingEnvironment
> >>>>
> >>>> and with 7785, using a hand-made kernel.
> >>> Patch 2 produces a trouble in my environment.
> >>> For r2d, the output to SCIF from kernel is OK, but output from
> >>> shell is broken by inserted white space, like follows.
> >>>
> >>> (before applying patch 2)
> >>>  # ls
> >>>
> >>> (after applying patch2)
> >>>  #    l  s
> >>>
> >>> Do you have time to investigate it?
> >>
> >> The attached revision of the patch fixes the problem for me. I failed to
> >> account for the fact that one cannot write '1' bit into FSR register.
> >
> > I've checked that the new patch avoids the problem.
> > Those 3 patches all seem OK for me.  Thank you!
> >
> 
> I have looked at the three patches, they all looks ok, except for coding
> style issues of block structures. Vladimir, could you please fix that
> (see file CODING_STYLE, 4. Block structure) and send the patches with a
> Signed-off-by: ? I'll apply them.

Here are the revised patches -- there were a few places where preexisting
code did not have {} around one-line statement, I've added braces there too.

Thanks,
Volodya


[-- Attachment #2: 0001-Make-RX-fifo-size-configurable.patch --]
[-- Type: text/x-patch, Size: 2739 bytes --]

From c9335797806ab006bca95b3749dfcb4ff0c1772c Mon Sep 17 00:00:00 2001
From: Vladimir Prus <vladimir@codesourcery.com>
Date: Thu, 2 Apr 2009 13:49:08 +0400
Subject: [PATCH 1/6] Make RX fifo size configurable.


Signed-off-by: Vladimir Prus <vladimir@codesourcery.com>
---
 hw/sh_serial.c |   20 ++++++++++++--------
 1 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/hw/sh_serial.c b/hw/sh_serial.c
index 4957c41..7fefaa6 100644
--- a/hw/sh_serial.c
+++ b/hw/sh_serial.c
@@ -37,8 +37,6 @@
 #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;
@@ -48,10 +46,11 @@ typedef struct {
     uint16_t fcr;
     uint8_t sptr;
 
-    uint8_t rx_fifo[SH_RX_FIFO_LENGTH]; /* frdr / rdr */
+    uint8_t *rx_fifo; /* frdr / rdr */
     uint8_t rx_cnt;
     uint8_t rx_tail;
     uint8_t rx_head;
+    int rx_capacity;
 
     int freq;
     int feat;
@@ -69,7 +68,7 @@ typedef struct {
 
 static void sh_serial_clear_fifo(sh_serial_state * s)
 {
-    memset(s->rx_fifo, 0, SH_RX_FIFO_LENGTH);
+    memset(s->rx_fifo, 0, s->rx_capacity);
     s->rx_cnt = 0;
     s->rx_head = 0;
     s->rx_tail = 0;
@@ -236,10 +235,12 @@ static uint32_t sh_serial_ioport_read(void *opaque, uint32_t offs)
             if (s->rx_cnt > 0) {
                 ret = s->rx_fifo[s->rx_tail++];
                 s->rx_cnt--;
-                if (s->rx_tail == SH_RX_FIFO_LENGTH)
+                if (s->rx_tail == s->rx_capacity) {
                     s->rx_tail = 0;
-                if (s->rx_cnt < s->rtrg)
+                }
+                if (s->rx_cnt < s->rtrg) {
                     s->flags &= ~SH_SERIAL_FLAG_RDF;
+                }
             }
             break;
 #if 0
@@ -297,10 +298,11 @@ static int sh_serial_can_receive(sh_serial_state *s)
 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) {
+        if (s->rx_cnt < s->rx_capacity) {
             s->rx_fifo[s->rx_head++] = ch;
-            if (s->rx_head == SH_RX_FIFO_LENGTH)
+            if (s->rx_head == s->rx_capacity) {
                 s->rx_head = 0;
+            }
             s->rx_cnt++;
             if (s->rx_cnt >= s->rtrg) {
                 s->flags |= SH_SERIAL_FLAG_RDF;
@@ -393,6 +395,8 @@ void sh_serial_init (target_phys_addr_t base, int feat,
         s->dr = 0xff;
     }
 
+    s->rx_capacity = 16;
+    s->rx_fifo = (uint8_t *)malloc(s->rx_capacity);
     sh_serial_clear_fifo(s);
 
     s_io_memory = cpu_register_io_memory(0, sh_serial_readfn,
-- 
1.6.2.1


[-- Attachment #3: 0003-Support-7785-s-serial.patch --]
[-- Type: text/x-patch, Size: 4347 bytes --]

From 4db2ac5c9f9b50dad488ba9d8c75d435ad30247f Mon Sep 17 00:00:00 2001
From: Vladimir Prus <vladimir@codesourcery.com>
Date: Thu, 2 Apr 2009 20:44:01 +0400
Subject: [PATCH 3/6] Support 7785's serial.


Signed-off-by: Vladimir Prus <vladimir@codesourcery.com>
---
 hw/sh.h        |    1 +
 hw/sh_serial.c |   58 ++++++++++++++++++++++++++++++++++++++++++++++++-------
 2 files changed, 51 insertions(+), 8 deletions(-)

diff --git a/hw/sh.h b/hw/sh.h
index 5e3c22b..da051f1 100644
--- a/hw/sh.h
+++ b/hw/sh.h
@@ -37,6 +37,7 @@ void tmu012_init(target_phys_addr_t base, int feat, uint32_t freq,
 
 /* sh_serial.c */
 #define SH_SERIAL_FEAT_SCIF (1 << 0)
+#define SH_SERIAL_FEAT_7785 (1 << 1)
 void sh_serial_init (target_phys_addr_t base, int feat,
 		     uint32_t freq, CharDriverState *chr,
 		     qemu_irq eri_source,
diff --git a/hw/sh_serial.c b/hw/sh_serial.c
index fc7de72..2881b5c 100644
--- a/hw/sh_serial.c
+++ b/hw/sh_serial.c
@@ -158,10 +158,22 @@ static void sh_serial_ioport_write(void *opaque, uint32_t offs, uint32_t val)
             }
 
             return;
-        case 0x20: /* SPTR */
-            s->sptr = val & 0xf3;
+        case 0x20: 
+            if (s->feat & SH_SERIAL_FEAT_7785)  {
+                /* Recieve fifo data count register, not writable.  */
+            } else {
+                 /* SPTR */
+                s->sptr = val & 0xf3;
+            }
+            return;
+        case 0x24: 
+            if (s->feat & SH_SERIAL_FEAT_7785) {
+                s->sptr = val & 0xf3;
+            } else {
+               /* LSR */
+            }
             return;
-        case 0x24: /* LSR */
+        case 0x28:
             return;
         }
     }
@@ -240,12 +252,38 @@ static uint32_t sh_serial_ioport_read(void *opaque, uint32_t offs)
             break;
 #endif
         case 0x1c:
-            ret = s->rx_cnt;
+            if (s->feat & SH_SERIAL_FEAT_7785) {
+                /* Trasmit fifo data count.  Because we immediate send
+                   everything, we also claim the fifo is always empty.  */
+                ret = 0;
+            } else {
+                /* On 7751, this is unified receive/trasmit fifo data
+                   count register.  The received data count is in low
+                   8 bits. In QEMU, transmit fifo is always empty, so
+                   return just receive count.  */
+                ret = s->rx_cnt;
+            }
             break;
         case 0x20:
-            ret = s->sptr;
+            if (s->feat & SH_SERIAL_FEAT_7785) {
+                /* On 7785, there's separate trasmit fifo data register.  */
+                ret = s->rx_cnt;
+            } else {
+                ret = s->sptr;
+            }
             break;
         case 0x24:
+            /* On 7785, this is serial port register.  On 7751, this is line
+               status register.  */
+            if (s->feat & SH_SERIAL_FEAT_7785) {
+                ret = s->sptr;
+            } else {
+                ret = 0;
+            }
+            break;
+        case 0x28:
+            /* On 7785, this is line status register.  */
+            assert(s->feat & SH_SERIAL_FEAT_7785);
             ret = 0;
             break;
         }
@@ -367,6 +405,7 @@ void sh_serial_init (target_phys_addr_t base, int feat,
 {
     sh_serial_state *s;
     int s_io_memory;
+    int is_7785 = (feat & SH_SERIAL_FEAT_7785);
 
     s = qemu_mallocz(sizeof(sh_serial_state));
 
@@ -386,14 +425,17 @@ void sh_serial_init (target_phys_addr_t base, int feat,
         s->dr = 0xff;
     }
 
-    s->rx_capacity = 16;
+    s->rx_capacity = is_7785 ? 64 : 16;
     s->rx_fifo = (uint8_t *)malloc(s->rx_capacity);
     sh_serial_clear_fifo(s);
 
     s_io_memory = cpu_register_io_memory(0, sh_serial_readfn,
 					 sh_serial_writefn, s);
-    cpu_register_physical_memory(P4ADDR(base), 0x28, s_io_memory);
-    cpu_register_physical_memory(A7ADDR(base), 0x28, s_io_memory);
+    cpu_register_physical_memory(P4ADDR(base), 
+                                 is_7785 ? 0x2E : 0x28, s_io_memory);
+    cpu_register_physical_memory(A7ADDR(base), 
+                                 is_7785 ? 0x2E : 0x28, s_io_memory);
+
 
     s->chr = chr;
 
-- 
1.6.2.1


[-- Attachment #4: 0002-Use-symbolic-constants.patch --]
[-- Type: text/x-patch, Size: 5708 bytes --]

From 9783a5ec82bcc6357f84633bacc1e9de3d055598 Mon Sep 17 00:00:00 2001
From: Vladimir Prus <vladimir@codesourcery.com>
Date: Thu, 2 Apr 2009 17:20:09 +0400
Subject: [PATCH 2/6] Use symbolic constants.


Signed-off-by: Vladimir Prus <vladimir@codesourcery.com>
---
 hw/sh_serial.c |   77 ++++++++++++++++++++++++-------------------------------
 1 files changed, 34 insertions(+), 43 deletions(-)

diff --git a/hw/sh_serial.c b/hw/sh_serial.c
index 7fefaa6..fc7de72 100644
--- a/hw/sh_serial.c
+++ b/hw/sh_serial.c
@@ -31,11 +31,15 @@
 
 //#define DEBUG_SERIAL
 
-#define SH_SERIAL_FLAG_TEND (1 << 0)
-#define SH_SERIAL_FLAG_TDE  (1 << 1)
-#define SH_SERIAL_FLAG_RDF  (1 << 2)
-#define SH_SERIAL_FLAG_BRK  (1 << 3)
-#define SH_SERIAL_FLAG_DR   (1 << 4)
+#define SCR_RE   (1 << 4)
+#define SCR_TE   (1 << 5)
+#define SCR_RIE  (1 << 6)
+#define SCR_TIE  (1 << 7)
+
+#define FSR_DR   (1 << 0)
+#define FSR_RDF  (1 << 1)
+#define FSR_TDFE (1 << 5)
+#define FSR_TEND (1 << 6)
 
 typedef struct {
     uint8_t smr;
@@ -54,7 +58,6 @@ typedef struct {
 
     int freq;
     int feat;
-    int flags;
     int rtrg;
 
     CharDriverState *chr;
@@ -93,12 +96,13 @@ static void sh_serial_ioport_write(void *opaque, uint32_t offs, uint32_t val)
     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;
+        if (!(val & SCR_TE)) {
+            s->sr |= FSR_TEND;
+        }
         if ((s->feat & SH_SERIAL_FEAT_SCIF) && s->txi) {
-	    qemu_set_irq(s->txi, val & (1 << 7));
+	    qemu_set_irq(s->txi, val & SCR_TIE);
         }
-        if (!(val & (1 << 6))) {
+        if (!(val & SCR_RIE)) {
 	    qemu_set_irq(s->rxi, 0);
         }
         return;
@@ -108,7 +112,7 @@ static void sh_serial_ioport_write(void *opaque, uint32_t offs, uint32_t val)
             qemu_chr_write(s->chr, &ch, 1);
 	}
 	s->dr = val;
-	s->flags &= ~SH_SERIAL_FLAG_TDE;
+	s->sr &= ~FSR_TDFE;
         return;
 #if 0
     case 0x14: /* FRDR / RDR */
@@ -119,18 +123,14 @@ static void sh_serial_ioport_write(void *opaque, uint32_t offs, uint32_t val)
     if (s->feat & SH_SERIAL_FEAT_SCIF) {
         switch(offs) {
         case 0x10: /* FSR */
-            if (!(val & (1 << 6)))
-                s->flags &= ~SH_SERIAL_FLAG_TEND;
-            if (!(val & (1 << 5)))
-                s->flags &= ~SH_SERIAL_FLAG_TDE;
-            if (!(val & (1 << 4)))
-                s->flags &= ~SH_SERIAL_FLAG_BRK;
-            if (!(val & (1 << 1)))
-                s->flags &= ~SH_SERIAL_FLAG_RDF;
-            if (!(val & (1 << 0)))
-                s->flags &= ~SH_SERIAL_FLAG_DR;
-
-            if (!(val & (1 << 1)) || !(val & (1 << 0))) {
+            /* Bits 2 and 3 cannot be written at all. */
+            val &= 0xf3; 
+            /* Other bits can only be cleared by writing 0
+               to them. In other words, a bit should remain set
+               only if it was set, and value 1 is written.  */            
+            s->sr = s->sr & val;
+
+            if (!(val & FSR_RDF) || !(val & FSR_DR)) {
                 if (s->rxi) {
                     qemu_set_irq(s->rxi, 0);
                 }
@@ -214,21 +214,12 @@ static uint32_t sh_serial_ioport_read(void *opaque, uint32_t offs)
         case 0x08: /* SCR */
             ret = s->scr;
             break;
-        case 0x10: /* FSR */
-            ret = 0;
-            if (s->flags & SH_SERIAL_FLAG_TEND)
-                ret |= (1 << 6);
-            if (s->flags & SH_SERIAL_FLAG_TDE)
-                ret |= (1 << 5);
-            if (s->flags & SH_SERIAL_FLAG_BRK)
-                ret |= (1 << 4);
-            if (s->flags & SH_SERIAL_FLAG_RDF)
-                ret |= (1 << 1);
-            if (s->flags & SH_SERIAL_FLAG_DR)
-                ret |= (1 << 0);
-
-            if (s->scr & (1 << 5))
-                s->flags |= SH_SERIAL_FLAG_TDE | SH_SERIAL_FLAG_TEND;
+        case 0x10: /* FSR */            
+            ret = s->sr;
+
+            if (s->scr & SCR_TE) {
+                s->sr |= FSR_TDFE | FSR_TEND;
+            }
 
             break;
         case 0x14:
@@ -239,7 +230,7 @@ static uint32_t sh_serial_ioport_read(void *opaque, uint32_t offs)
                     s->rx_tail = 0;
                 }
                 if (s->rx_cnt < s->rtrg) {
-                    s->flags &= ~SH_SERIAL_FLAG_RDF;
+                    s->sr &= ~FSR_RDF;
                 }
             }
             break;
@@ -292,7 +283,7 @@ static uint32_t sh_serial_ioport_read(void *opaque, uint32_t offs)
 
 static int sh_serial_can_receive(sh_serial_state *s)
 {
-    return s->scr & (1 << 4);
+    return s->scr & SCR_RE;
 }
 
 static void sh_serial_receive_byte(sh_serial_state *s, int ch)
@@ -305,8 +296,8 @@ static void sh_serial_receive_byte(sh_serial_state *s, int ch)
             }
             s->rx_cnt++;
             if (s->rx_cnt >= s->rtrg) {
-                s->flags |= SH_SERIAL_FLAG_RDF;
-                if (s->scr & (1 << 6) && s->rxi) {
+                s->sr |= FSR_RDF;
+                if (s->scr & SCR_RIE && s->rxi) {
                     qemu_set_irq(s->rxi, 1);
                 }
             }
@@ -380,7 +371,7 @@ void sh_serial_init (target_phys_addr_t base, int feat,
     s = qemu_mallocz(sizeof(sh_serial_state));
 
     s->feat = feat;
-    s->flags = SH_SERIAL_FLAG_TEND | SH_SERIAL_FLAG_TDE;
+    s->sr = FSR_TEND | FSR_TDFE;
     s->rtrg = 1;
 
     s->smr = 0;
-- 
1.6.2.1