Skip to contentSkip to conversation

Serial cleanup

PATCH Applied [email protected] 3 participants 13 messages Jun 3, 2013 – Jun 11, 2013

lore
Share & tools
lore

[email protected]

author
#0 lore raw

cover Serial cleanup

From: Peter Crosthwaite <[email protected]>

Some cosmetics, refactored to use util/fifo8 for the FIFO8, then factored out some common code.

Tested as working on petalogix-ml605 machine model + Linux (has coverage of serial fifo usage).

Peter Crosthwaite (3):
char/serial: cosmetic fixes.
char/serial: Use generic Fifo8
char/serial: serial_ioport_write: Factor out common code

 hw/char/serial.c         | 128 +++++++++++++++++++----------------------------
 include/hw/char/serial.h |  15 ++----
 2 files changed, 56 insertions(+), 87 deletions(-)
Signature · 1 line
1.8.3.rc1.44.gb387c77.dirty
author
#1 lore raw

PATCH 1/3 char/serial: cosmetic fixes.

From: Peter Crosthwaite <[email protected]>

Some cosmetic fixes to char/serial fixing some checkpatch errors.

Cc [email protected]
Signed-off-by Peter Crosthwaite <[email protected]>

---
Needed for the next patch to pass checkpatch. Done as sep patch to not obscure that patch.

 hw/char/serial.c | 30 +++++++++++++++++++-----------
 1 file changed, 19 insertions(+), 11 deletions(-)
hw/char/serial.c +19 -11
index 66b6348..bd6813e 100644
@@ -263,8 +263,9 @@ static gboolean serial_xmit(GIOChannel *chan, GIOCondition cond, void *opaque)
     if (s->tsr_retry <= 0) {
         if (s->fcr & UART_FCR_FE) {
             s->tsr = fifo_get(s,XMIT_FIFO);
-            if (!s->xmit_fifo.count)
+            if (!s->xmit_fifo.count) {
                 s->lsr |= UART_LSR_THRE;
+            }
         } else if ((s->lsr & UART_LSR_THRE)) {
             return FALSE;
         } else {

@@ -461,10 +462,11 @@ static uint64_t serial_ioport_read(void *opaque, hwaddr addr, unsigned size)
         } else {
             if(s->fcr & UART_FCR_FE) {
                 ret = fifo_get(s,RECV_FIFO);
-                if (s->recv_fifo.count == 0)
+                if (s->recv_fifo.count == 0) {
                     s->lsr &= ~(UART_LSR_DR | UART_LSR_BI);
-                else
+                } else {
                     qemu_mod_timer(s->fifo_timeout_timer, qemu_get_clock_ns (vm_clock) + s->char_transmit_time * 4);
+                }
                 s->timeout_ipending = 0;
             } else {
                 ret = s->rbr;

@@ -534,15 +536,21 @@ static uint64_t serial_ioport_read(void *opaque, hwaddr addr, unsigned size)
 static int serial_can_receive(SerialState *s)
 {
     if(s->fcr & UART_FCR_FE) {
-        if(s->recv_fifo.count < UART_FIFO_LENGTH)
-        /* Advertise (fifo.itl - fifo.count) bytes when count < ITL, and 1 if above. If UART_FIFO_LENGTH - fifo.count is
-        advertised the effect will be to almost always fill the fifo completely before the guest has a chance to respond,
-        effectively overriding the ITL that the guest has set. */
-             return (s->recv_fifo.count <= s->recv_fifo.itl) ? s->recv_fifo.itl - s->recv_fifo.count : 1;
-        else
-             return 0;
+        if (s->recv_fifo.count < UART_FIFO_LENGTH) {
+            /*
+             * Advertise (fifo.itl - fifo.count) bytes when count < ITL, and 1
+             * if above. If UART_FIFO_LENGTH - fifo.count is advertised the
+             * effect will be to almost always fill the fifo completely before
+             * the guest has a chance to respond, effectively overriding the ITL
+             * that the guest has set.
+             */
+            return (s->recv_fifo.count <= s->recv_fifo.itl) ?
+                        s->recv_fifo.itl - s->recv_fifo.count : 1;
+        } else {
+            return 0;
+        }
     } else {
-    return !(s->lsr & UART_LSR_DR);
+        return !(s->lsr & UART_LSR_DR);
     }
 }
Signature · 1 line
1.8.3.rc1.44.gb387c77.dirty
#2 lore raw

Re: [Qemu-devel] [PATCH v1 1/3] char/serial: cosmetic fixes.

Hi Peter,

Am 03.06.2013 07:12, schrieb [email protected]:

Quoted 30 lines

From: Peter Crosthwaite <[email protected]>

Some cosmetic fixes to char/serial fixing some checkpatch errors.

Cc [email protected]
Signed-off-by Peter Crosthwaite <[email protected]>

---
Needed for the next patch to pass checkpatch. Done as sep patch to not obscure that patch.

 hw/char/serial.c | 30 +++++++++++++++++++-----------
 1 file changed, 19 insertions(+), 11 deletions(-)
hw/char/serial.c +2 -1
index 66b6348..bd6813e 100644
@@ -263,8 +263,9 @@ static gboolean serial_xmit(GIOChannel *chan, GIOCondition cond, void *opaque)
     if (s->tsr_retry <= 0) {
         if (s->fcr & UART_FCR_FE) {
             s->tsr = fifo_get(s,XMIT_FIFO);
-            if (!s->xmit_fifo.count)
+            if (!s->xmit_fifo.count) {
                 s->lsr |= UART_LSR_THRE;
+            }
         } else if ((s->lsr & UART_LSR_THRE)) {
             return FALSE;
         } else {

hw/char/serial.c static uint64_t serial_ioport_read(void *opaque, hwaddr addr, unsigned size) · line 468

Quoted 9 lines
hw/char/serial.c +2 -2
@@ -461,10 +462,11 @@ static uint64_t serial_ioport_read(void *opaque, hwaddr addr, unsigned size)
         } else {
             if(s->fcr & UART_FCR_FE) {
                 ret = fifo_get(s,RECV_FIFO);
-                if (s->recv_fifo.count == 0)
+                if (s->recv_fifo.count == 0) {
                     s->lsr &= ~(UART_LSR_DR | UART_LSR_BI);
-                else
+                } else {
                     qemu_mod_timer(s->fifo_timeout_timer, qemu_get_clock_ns (vm_clock) + s->char_transmit_time * 4);

Wanna rebreak this one too in case you respin/pull?

Quoted 15 lines
hw/char/serial.c +1 -7
+                }
                 s->timeout_ipending = 0;
             } else {
                 ret = s->rbr;

@@ -534,15 +536,21 @@ static uint64_t serial_ioport_read(void *opaque, hwaddr addr, unsigned size)
 static int serial_can_receive(SerialState *s)
 {
     if(s->fcr & UART_FCR_FE) {
-        if(s->recv_fifo.count < UART_FIFO_LENGTH)
-        /* Advertise (fifo.itl - fifo.count) bytes when count < ITL, and 1 if above. If UART_FIFO_LENGTH - fifo.count is
-        advertised the effect will be to almost always fill the fifo completely before the guest has a chance to respond,
-        effectively overriding the ITL that the guest has set. */
-             return (s->recv_fifo.count <= s->recv_fifo.itl) ? s->recv_fifo.itl - s->recv_fifo.count : 1;
-        else
-             return 0;

hw/char/serial.c static uint64_t serial_ioport_read(void *opaque, hwaddr addr, unsigned size) · line 548

Quoted 10 lines
hw/char/serial.c +10 -0
+        if (s->recv_fifo.count < UART_FIFO_LENGTH) {
+            /*
+             * Advertise (fifo.itl - fifo.count) bytes when count < ITL, and 1
+             * if above. If UART_FIFO_LENGTH - fifo.count is advertised the
+             * effect will be to almost always fill the fifo completely before
+             * the guest has a chance to respond, effectively overriding the ITL
+             * that the guest has set.
+             */
+            return (s->recv_fifo.count <= s->recv_fifo.itl) ?
+                        s->recv_fifo.itl - s->recv_fifo.count : 1;

Here I stumbled over the indentation being 5 chars from '(' or 4 chars within, but the latter doesn't make sense since it's terminated before. I would've expected 4 chars from block or aligned below '(' or 4-char-indented from there. But I'm not sure if there are any clear recommendations, so since it's apparently not using tabs (my initial suspicion), no objection.

Cheers,
Andreas

Quoted 10 lines
hw/char/serial.c +4 -1
+        } else {
+            return 0;
+        }
     } else {
-    return !(s->lsr & UART_LSR_DR);
+        return !(s->lsr & UART_LSR_DR);
     }
 }
Signature · 2 lines
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Applied
#3 lore raw

Re: [Qemu-devel] [Qemu-trivial] [PATCH v1 1/3] char/serial: cosmetic fixes.

In reply to [email protected]
Quoted 3 lines · 03.06.2013 09:12, [email protected] wrote:

From: Peter Crosthwaite <[email protected]>

Some cosmetic fixes to char/serial fixing some checkpatch errors.

These are all cosmetic fixes indeed. I weren't sure we're applying stylistic changes by its own. But apparently people don't object so I don't see why not, either, except that I don't really want to open a can of worms with other stylistic patches to come ;)

Applied all 3 to the trivial patches queue.

Thank you!

/mjt

#4 lore raw

Re: [Qemu-devel] [Qemu-trivial] [PATCH v1 1/3] char/serial: cosmetic fixes.

10.06.2013 19:17, Michael Tokarev пишет:

Quoted 5 lines
Quoted 3 lines · 03.06.2013 09:12, [email protected] wrote:

From: Peter Crosthwaite <[email protected]>

Some cosmetic fixes to char/serial fixing some checkpatch errors.

Quoted 7 lines

These are all cosmetic fixes indeed. I weren't sure we're applying stylistic changes by its own. But apparently people don't object so I don't see why not, either, except that I don't really want to open a can of worms with other stylistic patches to come ;)

Applied all 3 to the trivial patches queue.

Actually I didn't apply these. Since patch 2/3 weren't sent to -trivial, and 3/3 does not apply without it, I'm not sure I want to apply either of these. Maybe all 3 should be sent to the same tree? Should I pick the 2/3 too, which wasn't submitted to -trivial?

Thank you!

/mjt

author
#5 lore raw

Re: [Qemu-devel] [Qemu-trivial] [PATCH v1 1/3] char/serial: cosmetic fixes.

Hi Michael,

Quoted 12 lines · Michael Tokarev

10.06.2013 19:17, Michael Tokarev пишет:

Quoted 11 lines
Quoted 3 lines · 03.06.2013 09:12, [email protected] wrote:

From: Peter Crosthwaite <[email protected]>

Some cosmetic fixes to char/serial fixing some checkpatch errors.

These are all cosmetic fixes indeed. I weren't sure we're applying stylistic changes by its own. But apparently people don't object so I don't see why not, either, except that I don't really want to open a can of worms with other stylistic patches to come ;)

Quoted 8 lines
Quoted 1 line

Applied all 3 to the trivial patches queue.

Actually I didn't apply these. Since patch 2/3 weren't sent to -trivial, and 3/3 does not apply without it, I'm not sure I want to apply either of these. Maybe all 3 should be sent to the same tree? Should I pick the 2/3 too, which wasn't submitted to -trivial?

Im happy for you to take all three as they have had a week of list time and 2/3 is cleanup only with no functional change. It's borderline as to whether its trivial.

Regards,
Peter

Quoted 5 lines

Thank you!

/mjt

author
#6 lore raw

PATCH 2/3 char/serial: Use generic Fifo8

In reply to [email protected]

From: Peter Crosthwaite <[email protected]>

Use the generic Fifo8 helper provided by QEMU, rather than re-implement privately.

Signed-off-by Peter Crosthwaite <[email protected]>

---

 hw/char/serial.c         | 98 +++++++++++++++++-------------------------------
 include/hw/char/serial.h | 15 +++-----
 2 files changed, 39 insertions(+), 74 deletions(-)
hw/char/serial.c +34 -64
index bd6813e..0a2b6c9 100644
@@ -92,8 +92,6 @@
 #define UART_FCR_RFR        0x02    /* RCVR Fifo Reset */
 #define UART_FCR_FE         0x01    /* FIFO Enable */
 
-#define XMIT_FIFO           0
-#define RECV_FIFO           1
 #define MAX_XMIT_RETRY      4
 
 #ifdef DEBUG_SERIAL

@@ -106,50 +104,14 @@ do {} while (0)
 
 static void serial_receive1(void *opaque, const uint8_t *buf, int size);
 
-static void fifo_clear(SerialState *s, int fifo)
+static inline void recv_fifo_put(SerialState *s, uint8_t chr)
 {
-    SerialFIFO *f = (fifo) ? &s->recv_fifo : &s->xmit_fifo;
-    memset(f->data, 0, UART_FIFO_LENGTH);
-    f->count = 0;
-    f->head = 0;
-    f->tail = 0;
-}
-
-static int fifo_put(SerialState *s, int fifo, uint8_t chr)
-{
-    SerialFIFO *f = (fifo) ? &s->recv_fifo : &s->xmit_fifo;
-
     /* Receive overruns do not overwrite FIFO contents. */
-    if (fifo == XMIT_FIFO || f->count < UART_FIFO_LENGTH) {
-
-        f->data[f->head++] = chr;
-
-        if (f->head == UART_FIFO_LENGTH)
-            f->head = 0;
-    }
-
-    if (f->count < UART_FIFO_LENGTH)
-        f->count++;
-    else if (fifo == RECV_FIFO)
+    if (!fifo8_is_full(&s->recv_fifo)) {
+        fifo8_push(&s->recv_fifo, chr);
+    } else {
         s->lsr |= UART_LSR_OE;
-
-    return 1;
-}
-
-static uint8_t fifo_get(SerialState *s, int fifo)
-{
-    SerialFIFO *f = (fifo) ? &s->recv_fifo : &s->xmit_fifo;
-    uint8_t c;
-
-    if(f->count == 0)
-        return 0;
-
-    c = f->data[f->tail++];
-    if (f->tail == UART_FIFO_LENGTH)
-        f->tail = 0;
-    f->count--;
-
-    return c;
+    }
 }
 
 static void serial_update_irq(SerialState *s)

@@ -165,7 +127,7 @@ static void serial_update_irq(SerialState *s)
         tmp_iir = UART_IIR_CTI;
     } else if ((s->ier & UART_IER_RDI) && (s->lsr & UART_LSR_DR) &&
                (!(s->fcr & UART_FCR_FE) ||
-                s->recv_fifo.count >= s->recv_fifo.itl)) {
+                s->recv_fifo.num >= s->recv_fifo_itl)) {
         tmp_iir = UART_IIR_RDI;
     } else if ((s->ier & UART_IER_THRI) && s->thr_ipending) {
         tmp_iir = UART_IIR_THRI;

@@ -262,8 +224,9 @@ static gboolean serial_xmit(GIOChannel *chan, GIOCondition cond, void *opaque)
 
     if (s->tsr_retry <= 0) {
         if (s->fcr & UART_FCR_FE) {
-            s->tsr = fifo_get(s,XMIT_FIFO);
-            if (!s->xmit_fifo.count) {
+            s->tsr = fifo8_is_full(&s->xmit_fifo) ?
+                        0 : fifo8_pop(&s->xmit_fifo);
+            if (!s->xmit_fifo.num) {
                 s->lsr |= UART_LSR_THRE;
             }
         } else if ((s->lsr & UART_LSR_THRE)) {

@@ -317,7 +280,11 @@ static void serial_ioport_write(void *opaque, hwaddr addr, uint64_t val,
         } else {
             s->thr = (uint8_t) val;
             if(s->fcr & UART_FCR_FE) {
-                fifo_put(s, XMIT_FIFO, s->thr);
+                /* xmit overruns overwrite data, so make space if needed */
+                if (fifo8_is_full(&s->xmit_fifo)) {
+                    fifo8_pop(&s->xmit_fifo);
+                }
+                fifo8_push(&s->xmit_fifo, s->thr);
                 s->thr_ipending = 0;
                 s->lsr &= ~UART_LSR_TEMT;
                 s->lsr &= ~UART_LSR_THRE;

@@ -368,28 +335,28 @@ static void serial_ioport_write(void *opaque, hwaddr addr, uint64_t val,
         if (val & UART_FCR_RFR) {
             qemu_del_timer(s->fifo_timeout_timer);
             s->timeout_ipending=0;
-            fifo_clear(s,RECV_FIFO);
+            fifo8_reset(&s->recv_fifo);
         }
 
         if (val & UART_FCR_XFR) {
-            fifo_clear(s,XMIT_FIFO);
+            fifo8_reset(&s->xmit_fifo);
         }
 
         if (val & UART_FCR_FE) {
             s->iir |= UART_IIR_FE;
-            /* Set RECV_FIFO trigger Level */
+            /* Set recv_fifo trigger Level */
             switch (val & 0xC0) {
             case UART_FCR_ITL_1:
-                s->recv_fifo.itl = 1;
+                s->recv_fifo_itl = 1;
                 break;
             case UART_FCR_ITL_2:
-                s->recv_fifo.itl = 4;
+                s->recv_fifo_itl = 4;
                 break;
             case UART_FCR_ITL_3:
-                s->recv_fifo.itl = 8;
+                s->recv_fifo_itl = 8;
                 break;
             case UART_FCR_ITL_4:
-                s->recv_fifo.itl = 14;
+                s->recv_fifo_itl = 14;
                 break;
             }
         } else

@@ -461,8 +428,9 @@ static uint64_t serial_ioport_read(void *opaque, hwaddr addr, unsigned size)
             ret = s->divider & 0xff;
         } else {
             if(s->fcr & UART_FCR_FE) {
-                ret = fifo_get(s,RECV_FIFO);
-                if (s->recv_fifo.count == 0) {
+                ret = fifo8_is_full(&s->recv_fifo) ?
+                            0 : fifo8_pop(&s->recv_fifo);
+                if (s->recv_fifo.num == 0) {
                     s->lsr &= ~(UART_LSR_DR | UART_LSR_BI);
                 } else {
                     qemu_mod_timer(s->fifo_timeout_timer, qemu_get_clock_ns (vm_clock) + s->char_transmit_time * 4);

@@ -536,7 +504,7 @@ static uint64_t serial_ioport_read(void *opaque, hwaddr addr, unsigned size)
 static int serial_can_receive(SerialState *s)
 {
     if(s->fcr & UART_FCR_FE) {
-        if (s->recv_fifo.count < UART_FIFO_LENGTH) {
+        if (s->recv_fifo.num < UART_FIFO_LENGTH) {
             /*
              * Advertise (fifo.itl - fifo.count) bytes when count < ITL, and 1
              * if above. If UART_FIFO_LENGTH - fifo.count is advertised the

@@ -544,8 +512,8 @@ static int serial_can_receive(SerialState *s)
              * the guest has a chance to respond, effectively overriding the ITL
              * that the guest has set.
              */
-            return (s->recv_fifo.count <= s->recv_fifo.itl) ?
-                        s->recv_fifo.itl - s->recv_fifo.count : 1;
+            return (s->recv_fifo.num <= s->recv_fifo_itl) ?
+                        s->recv_fifo_itl - s->recv_fifo.num : 1;
         } else {
             return 0;
         }

@@ -558,7 +526,7 @@ static void serial_receive_break(SerialState *s)
 {
     s->rbr = 0;
     /* When the LSR_DR is set a null byte is pushed into the fifo */
-    fifo_put(s, RECV_FIFO, '\0');
+    recv_fifo_put(s, '\0');
     s->lsr |= UART_LSR_BI | UART_LSR_DR;
     serial_update_irq(s);
 }

@@ -566,7 +534,7 @@ static void serial_receive_break(SerialState *s)
 /* There's data in recv_fifo and s->rbr has not been read for 4 char transmit times */
 static void fifo_timeout_int (void *opaque) {
     SerialState *s = opaque;
-    if (s->recv_fifo.count) {
+    if (s->recv_fifo.num) {
         s->timeout_ipending = 1;
         serial_update_irq(s);
     }

@@ -588,7 +556,7 @@ static void serial_receive1(void *opaque, const uint8_t *buf, int size)
     if(s->fcr & UART_FCR_FE) {
         int i;
         for (i = 0; i < size; i++) {
-            fifo_put(s, RECV_FIFO, buf[i]);
+            recv_fifo_put(s, buf[i]);
         }
         s->lsr |= UART_LSR_DR;
         /* call the timeout receive callback in 4 char transmit time */

@@ -668,8 +636,8 @@ static void serial_reset(void *opaque)
     s->char_transmit_time = (get_ticks_per_sec() / 9600) * 10;
     s->poll_msl = 0;
 
-    fifo_clear(s,RECV_FIFO);
-    fifo_clear(s,XMIT_FIFO);
+    fifo8_reset(&s->recv_fifo);
+    fifo8_reset(&s->xmit_fifo);
 
     s->last_xmit_ts = qemu_get_clock_ns(vm_clock);
 

@@ -692,6 +660,8 @@ void serial_init_core(SerialState *s)
 
     qemu_chr_add_handlers(s->chr, serial_can_receive1, serial_receive1,
                           serial_event, s);
+    fifo8_create(&s->recv_fifo, UART_FIFO_LENGTH);
+    fifo8_create(&s->xmit_fifo, UART_FIFO_LENGTH);
 }
 
 void serial_exit_core(SerialState *s)
include/hw/char/serial.h +5 -10
index bca79f1..9ab81f6 100644
@@ -28,17 +28,10 @@
 #include "hw/hw.h"
 #include "sysemu/sysemu.h"
 #include "exec/memory.h"
+#include "qemu/fifo8.h"
 
 #define UART_FIFO_LENGTH    16      /* 16550A Fifo Length */
 
-typedef struct SerialFIFO {
-    uint8_t data[UART_FIFO_LENGTH];
-    uint8_t count;
-    uint8_t itl;                        /* Interrupt Trigger Level */
-    uint8_t tail;
-    uint8_t head;
-} SerialFIFO;
-
 struct SerialState {
     uint16_t divider;
     uint8_t rbr; /* receive register */

@@ -67,8 +60,10 @@ struct SerialState {
 
     /* Time when the last byte was successfully sent out of the tsr */
     uint64_t last_xmit_ts;
-    SerialFIFO recv_fifo;
-    SerialFIFO xmit_fifo;
+    Fifo8 recv_fifo;
+    Fifo8 xmit_fifo;
+    /* Interrupt trigger level for recv_fifo */
+    uint8_t recv_fifo_itl;
 
     struct QEMUTimer *fifo_timeout_timer;
     int timeout_ipending;           /* timeout interrupt pending state */
Signature · 1 line
1.8.3.rc1.44.gb387c77.dirty
author
#7 lore raw

PATCH 3/3 char/serial: serial_ioport_write: Factor out common code

In reply to [email protected]

From: Peter Crosthwaite <[email protected]>

These three lines are common to both FIFO and regular mode. Just factor them out to outside the if rather than replicate the same lines inside both if and else.

Cc [email protected]
Signed-off-by Peter Crosthwaite <[email protected]>

---

 hw/char/serial.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)
hw/char/serial.c +3 -7
index 0a2b6c9..017610e 100644
@@ -285,15 +285,11 @@ static void serial_ioport_write(void *opaque, hwaddr addr, uint64_t val,
                     fifo8_pop(&s->xmit_fifo);
                 }
                 fifo8_push(&s->xmit_fifo, s->thr);
-                s->thr_ipending = 0;
                 s->lsr &= ~UART_LSR_TEMT;
-                s->lsr &= ~UART_LSR_THRE;
-                serial_update_irq(s);
-            } else {
-                s->thr_ipending = 0;
-                s->lsr &= ~UART_LSR_THRE;
-                serial_update_irq(s);
             }
+            s->thr_ipending = 0;
+            s->lsr &= ~UART_LSR_THRE;
+            serial_update_irq(s);
             serial_xmit(NULL, G_IO_OUT, s);
         }
         break;
Signature · 1 line
1.8.3.rc1.44.gb387c77.dirty
Reviewed-by on v1 3/3
#8 lore raw

Re: [Qemu-devel] [PATCH v1 3/3] char/serial: serial_ioport_write: Factor out common code

Am 03.06.2013 07:14, schrieb [email protected]:

Quoted 9 lines

From: Peter Crosthwaite <[email protected]>

These three lines are common to both FIFO and regular mode. Just factor them out to outside the if rather than replicate the same lines inside both if and else.

Cc [email protected]
Signed-off-by Peter Crosthwaite <[email protected]>
Reviewed-by Andreas Färber <[email protected]>

Andreas

Signature · 2 lines
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
author
#9 lore raw
In reply to [email protected]

Ping!

Any objections to this one going in? perhaps even via trivial queue?

Regards,
Peter

Quoted 21 lines · On Mon, Jun 3, 2013 at 3:11 PM, <[email protected]> wrote:

From: Peter Crosthwaite <[email protected]>

Some cosmetics, refactored to use util/fifo8 for the FIFO8, then factored out some common code.

Tested as working on petalogix-ml605 machine model + Linux (has coverage of serial fifo usage).

Peter Crosthwaite (3):
char/serial: cosmetic fixes.
char/serial: Use generic Fifo8
char/serial: serial_ioport_write: Factor out common code

 hw/char/serial.c         | 128 +++++++++++++++++++----------------------------
 include/hw/char/serial.h |  15 ++----
 2 files changed, 56 insertions(+), 87 deletions(-)

--
1.8.3.rc1.44.gb387c77.dirty

#10 lore raw

Am 10.06.2013 12:23, schrieb Peter Crosthwaite:

Quoted 3 lines

Ping!

Any objections to this one going in? perhaps even via trivial queue?

No strong objection, but you are using an unusual 12-char indentation in some places that you may want to check.

Otherwise the cosmetic cleanup looks fine to me.

Cheers,
Andreas

Quoted 23 lines · On Mon, Jun 3, 2013 at 3:11 PM, <[email protected]> wrote:
Quoted 21 lines

From: Peter Crosthwaite <[email protected]>

Some cosmetics, refactored to use util/fifo8 for the FIFO8, then factored out some common code.

Tested as working on petalogix-ml605 machine model + Linux (has coverage of serial fifo usage).

Peter Crosthwaite (3):
char/serial: cosmetic fixes.
char/serial: Use generic Fifo8
char/serial: serial_ioport_write: Factor out common code

 hw/char/serial.c         | 128 +++++++++++++++++++----------------------------
 include/hw/char/serial.h |  15 ++----
 2 files changed, 56 insertions(+), 87 deletions(-)

--
1.8.3.rc1.44.gb387c77.dirty

Signature · 2 lines
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
author
#11 lore raw

Hi Andreas,

Quoted 8 lines · Andreas Färber

Am 10.06.2013 12:23, schrieb Peter Crosthwaite:

Quoted 3 lines

Ping!

Any objections to this one going in? perhaps even via trivial queue?

No strong objection, but you are using an unusual 12-char indentation in some places that you may want to check.

I use that indentation when a ? : operator continues to the next line. What indentation scheme should be used in this instance?

Regards,
Peter

Quoted 34 lines

Otherwise the cosmetic cleanup looks fine to me.

Cheers,
Andreas

Quoted 15 lines · On Mon, Jun 3, 2013 at 3:11 PM, <[email protected]> wrote:
Quoted 14 lines

From: Peter Crosthwaite <[email protected]>

Some cosmetics, refactored to use util/fifo8 for the FIFO8, then factored out some common code.

Tested as working on petalogix-ml605 machine model + Linux (has coverage of serial fifo usage).

Peter Crosthwaite (3):
char/serial: cosmetic fixes.
char/serial: Use generic Fifo8
char/serial: serial_ioport_write: Factor out common code

Quoted 8 lines
Quoted 7 lines
 hw/char/serial.c         | 128 +++++++++++++++++++----------------------------
 include/hw/char/serial.h |  15 ++----
 2 files changed, 56 insertions(+), 87 deletions(-)

--
1.8.3.rc1.44.gb387c77.dirty

--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

#12 lore raw
In reply to Peter Crosthwaite
Quoted 3 lines · Peter Crosthwaite

Ping!

Any objections to this one going in? perhaps even via trivial queue?

Actually applied all 3 (including 2/3 which weren't submitted to -trivial -- I had some fun verifying it :) to the trivial-patches queue.

Thanks!

/mjt