qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Vladimir Prus <vladimir@codesourcery.com>
To: Aurelien Jarno <aurelien@aurel32.net>
Cc: qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] Re: SH: support 7785 serial
Date: Mon, 11 May 2009 19:44:46 +0400	[thread overview]
Message-ID: <200905111944.47469.vladimir@codesourcery.com> (raw)
In-Reply-To: <20090418163757.GB29955@volta.aurel32.net>

[-- 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


      reply	other threads:[~2009-05-11 15:44 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-04-02 17:29 [Qemu-devel] SH: support 7785 serial Vladimir Prus
2009-04-06 13:15 ` Shin-ichiro KAWASAKI
2009-04-06 15:23   ` [Qemu-devel] " Vladimir Prus
2009-04-06 16:49     ` Shin-ichiro KAWASAKI
2009-04-06 18:04       ` [Qemu-devel] " Vladimir Prus
2009-04-15 13:18   ` [Qemu-devel] " Vladimir Prus
2009-04-18 11:47     ` Shin-ichiro KAWASAKI
2009-04-18 16:37       ` Aurelien Jarno
2009-05-11 15:44         ` Vladimir Prus [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=200905111944.47469.vladimir@codesourcery.com \
    --to=vladimir@codesourcery.com \
    --cc=aurelien@aurel32.net \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).