Re: [Qemu-devel] [PATCH 1/2] serial: reset state at startup

2014-09-21 Thread Chen, Tiejun 

On 2014/9/19 20:57, Paolo Bonzini wrote:

Il 19/09/2014 11:17, Chen, Tiejun ha scritto:

On 2014/9/19 16:54, Paolo Bonzini wrote:

When a serial port is started, its initial state is all zero.  Make
it consistent with reset state instead.

Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
   hw/char/serial.c | 1 +
   1 file changed, 1 insertion(+)

diff --git a/hw/char/serial.c b/hw/char/serial.c
index 764e184..4523ccb 100644
--- a/hw/char/serial.c
+++ b/hw/char/serial.c
@@ -668,6 +668,7 @@ void serial_realize_core(SerialState *s, Error
**errp)
 serial_event, s);


It should just follow qemu_register_reset(serial_reset, s).


   fifo8_create(s-recv_fifo, UART_FIFO_LENGTH);
   fifo8_create(s-xmit_fifo, UART_FIFO_LENGTH);
+serial_reset(s);


Or at least we should push this before this pair of fifo8_create() since


No, it should be _after_ the fifo8_create() pair.  With the current
implementation it doesn't matter, but first you create something and


Yes, I took a look at this pair,

void fifo8_create(Fifo8 *fifo, uint32_t capacity)
{
fifo-data = g_new(uint8_t, capacity);
fifo-capacity = capacity;
fifo-head = 0;
fifo-num = 0;
}

and

void fifo8_reset(Fifo8 *fifo)
{
fifo-num = 0;
fifo-head = 0;
}


then you initialize it, not the other way round.



Thanks for your explanation in this case.

Thanks
Tiejun


Paolo


static void serial_reset(void *opaque)
{
 ...
 fifo8_reset(s-recv_fifo);
 fifo8_reset(s-xmit_fifo);


Thanks
Tiejun


   }

   void serial_exit_core(SerialState *s)

[Qemu-devel] [PATCH 1/2] serial: reset state at startup

2014-09-19 Thread Paolo Bonzini 
When a serial port is started, its initial state is all zero.  Make
it consistent with reset state instead.

Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
 hw/char/serial.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/char/serial.c b/hw/char/serial.c
index 764e184..4523ccb 100644
--- a/hw/char/serial.c
+++ b/hw/char/serial.c
@@ -668,6 +668,7 @@ void serial_realize_core(SerialState *s, Error **errp)
   serial_event, s);
 fifo8_create(s-recv_fifo, UART_FIFO_LENGTH);
 fifo8_create(s-xmit_fifo, UART_FIFO_LENGTH);
+serial_reset(s);
 }
 
 void serial_exit_core(SerialState *s)
-- 
2.1.0

Re: [Qemu-devel] [PATCH 1/2] serial: reset state at startup

2014-09-19 Thread Chen, Tiejun 

On 2014/9/19 16:54, Paolo Bonzini wrote:

When a serial port is started, its initial state is all zero.  Make
it consistent with reset state instead.

Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
  hw/char/serial.c | 1 +
  1 file changed, 1 insertion(+)

diff --git a/hw/char/serial.c b/hw/char/serial.c
index 764e184..4523ccb 100644
--- a/hw/char/serial.c
+++ b/hw/char/serial.c
@@ -668,6 +668,7 @@ void serial_realize_core(SerialState *s, Error **errp)
serial_event, s);


It should just follow qemu_register_reset(serial_reset, s).


  fifo8_create(s-recv_fifo, UART_FIFO_LENGTH);
  fifo8_create(s-xmit_fifo, UART_FIFO_LENGTH);
+serial_reset(s);


Or at least we should push this before this pair of fifo8_create() since

static void serial_reset(void *opaque)
{
...
fifo8_reset(s-recv_fifo);
fifo8_reset(s-xmit_fifo);


Thanks
Tiejun


  }

  void serial_exit_core(SerialState *s)

Re: [Qemu-devel] [PATCH 1/2] serial: reset state at startup

2014-09-19 Thread Paolo Bonzini 
Il 19/09/2014 11:17, Chen, Tiejun ha scritto:
 On 2014/9/19 16:54, Paolo Bonzini wrote:
 When a serial port is started, its initial state is all zero.  Make
 it consistent with reset state instead.

 Signed-off-by: Paolo Bonzini pbonz...@redhat.com
 ---
   hw/char/serial.c | 1 +
   1 file changed, 1 insertion(+)

 diff --git a/hw/char/serial.c b/hw/char/serial.c
 index 764e184..4523ccb 100644
 --- a/hw/char/serial.c
 +++ b/hw/char/serial.c
 @@ -668,6 +668,7 @@ void serial_realize_core(SerialState *s, Error
 **errp)
 serial_event, s);
 
 It should just follow qemu_register_reset(serial_reset, s).
 
   fifo8_create(s-recv_fifo, UART_FIFO_LENGTH);
   fifo8_create(s-xmit_fifo, UART_FIFO_LENGTH);
 +serial_reset(s);
 
 Or at least we should push this before this pair of fifo8_create() since

No, it should be _after_ the fifo8_create() pair.  With the current
implementation it doesn't matter, but first you create something and
then you initialize it, not the other way round.

Paolo

 static void serial_reset(void *opaque)
 {
 ...
 fifo8_reset(s-recv_fifo);
 fifo8_reset(s-xmit_fifo);
 
 
 Thanks
 Tiejun
 
   }

   void serial_exit_core(SerialState *s)