* [Qemu-devel] [PATCH V2] GRLIB UART: Add RX channel
@ 2012-01-26 14:12 Fabien Chouteau
0 siblings, 0 replies; 6+ messages in thread
From: Fabien Chouteau @ 2012-01-26 14:12 UTC (permalink / raw)
To: qemu-devel; +Cc: blauwirbel, avi, atar4qemu
This patch implements the RX channel of GRLIB UART with a FIFO to
improve data rate.
Signed-off-by: Fabien Chouteau <chouteau@adacore.com>
---
hw/grlib_apbuart.c | 106 +++++++++++++++++++++++++++++++++++++++++++--------
trace-events | 1 +
2 files changed, 90 insertions(+), 17 deletions(-)
diff --git a/hw/grlib_apbuart.c b/hw/grlib_apbuart.c
index f8a64e1..51406c6 100644
--- a/hw/grlib_apbuart.c
+++ b/hw/grlib_apbuart.c
@@ -24,7 +24,6 @@
#include "sysbus.h"
#include "qemu-char.h"
-#include "ptimer.h"
#include "trace.h"
@@ -66,6 +65,8 @@
#define SCALER_OFFSET 0x0C /* not supported */
#define FIFO_DEBUG_OFFSET 0x10 /* not supported */
+#define FIFO_LENGTH 1024
+
typedef struct UART {
SysBusDevice busdev;
MemoryRegion iomem;
@@ -77,21 +78,67 @@ typedef struct UART {
uint32_t receive;
uint32_t status;
uint32_t control;
+
+ /* FIFO */
+ char buffer[FIFO_LENGTH];
+ int len;
+ int current;
} UART;
+static int uart_data_to_read(UART *uart)
+{
+ return uart->current < uart->len;
+}
+
+static char uart_pop(UART *uart)
+{
+ char ret;
+
+ if (uart->len == 0) {
+ uart->status &= ~UART_DATA_READY;
+ return 0;
+ }
+
+ ret = uart->buffer[uart->current++];
+
+ if (uart->current >= uart->len) {
+ /* Flush */
+ uart->len = 0;
+ uart->current = 0;
+ }
+
+ if (!uart_data_to_read(uart)) {
+ uart->status &= ~UART_DATA_READY;
+ }
+
+ return ret;
+}
+
+static void uart_add_to_fifo(UART *uart,
+ const uint8_t *buffer,
+ int length)
+{
+ if (uart->len + length > FIFO_LENGTH) {
+ abort();
+ }
+ memcpy(uart->buffer + uart->len, buffer, length);
+ uart->len += length;
+}
+
static int grlib_apbuart_can_receive(void *opaque)
{
UART *uart = opaque;
- return !!(uart->status & UART_DATA_READY);
+ return FIFO_LENGTH - uart->len;
}
static void grlib_apbuart_receive(void *opaque, const uint8_t *buf, int size)
{
UART *uart = opaque;
- uart->receive = *buf;
- uart->status |= UART_DATA_READY;
+ uart_add_to_fifo(uart, buf, size);
+
+ uart->status |= UART_DATA_READY;
if (uart->control & UART_RECEIVE_INTERRUPT) {
qemu_irq_pulse(uart->irq);
@@ -103,9 +150,39 @@ static void grlib_apbuart_event(void *opaque, int event)
trace_grlib_apbuart_event(event);
}
-static void
-grlib_apbuart_write(void *opaque, target_phys_addr_t addr,
- uint64_t value, unsigned size)
+
+static uint64_t grlib_apbuart_read(void *opaque, target_phys_addr_t addr,
+ unsigned size)
+{
+ UART *uart = opaque;
+
+ addr &= 0xff;
+
+ /* Unit registers */
+ switch (addr) {
+ case DATA_OFFSET:
+ case DATA_OFFSET + 3: /* when only one byte read */
+ return uart_pop(uart);
+
+ case STATUS_OFFSET:
+ /* Read Only */
+ return uart->status;
+
+ case CONTROL_OFFSET:
+ return uart->control;
+
+ case SCALER_OFFSET:
+ /* Not supported */
+ return 0;
+
+ default:
+ trace_grlib_apbuart_readl_unknown(addr);
+ return 0;
+ }
+}
+
+static void grlib_apbuart_write(void *opaque, target_phys_addr_t addr,
+ uint64_t value, unsigned size)
{
UART *uart = opaque;
unsigned char c = 0;
@@ -115,6 +192,7 @@ grlib_apbuart_write(void *opaque, target_phys_addr_t addr,
/* Unit registers */
switch (addr) {
case DATA_OFFSET:
+ case DATA_OFFSET + 3: /* When only one byte write */
c = value & 0xFF;
qemu_chr_fe_write(uart->chr, &c, 1);
return;
@@ -124,7 +202,7 @@ grlib_apbuart_write(void *opaque, target_phys_addr_t addr,
return;
case CONTROL_OFFSET:
- /* Not supported */
+ uart->control = value;
return;
case SCALER_OFFSET:
@@ -138,21 +216,15 @@ grlib_apbuart_write(void *opaque, target_phys_addr_t addr,
trace_grlib_apbuart_writel_unknown(addr, value);
}
-static bool grlib_apbuart_accepts(void *opaque, target_phys_addr_t addr,
- unsigned size, bool is_write)
-{
- return is_write && size == 4;
-}
-
static const MemoryRegionOps grlib_apbuart_ops = {
- .write = grlib_apbuart_write,
- .valid.accepts = grlib_apbuart_accepts,
+ .write = grlib_apbuart_write,
+ .read = grlib_apbuart_read,
.endianness = DEVICE_NATIVE_ENDIAN,
};
static int grlib_apbuart_init(SysBusDevice *dev)
{
- UART *uart = FROM_SYSBUS(typeof(*uart), dev);
+ UART *uart = FROM_SYSBUS(typeof(*uart), dev);
qemu_chr_add_handlers(uart->chr,
grlib_apbuart_can_receive,
diff --git a/trace-events b/trace-events
index d2b0c61..5bb8bfb 100644
--- a/trace-events
+++ b/trace-events
@@ -340,6 +340,7 @@ grlib_irqmp_writel_unknown(uint64_t addr, uint32_t value) "addr 0x%"PRIx64" valu
# hw/grlib_apbuart.c
grlib_apbuart_event(int event) "event:%d"
grlib_apbuart_writel_unknown(uint64_t addr, uint32_t value) "addr 0x%"PRIx64" value 0x%x"
+grlib_apbuart_readl_unknown(uint64_t addr) "addr 0x%"PRIx64""
# hw/leon3.c
leon3_set_irq(int intno) "Set CPU IRQ %d"
--
1.7.4.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [Qemu-devel] [PATCH V2] GRLIB UART: Add RX channel
@ 2012-01-26 14:54 Fabien Chouteau
0 siblings, 0 replies; 6+ messages in thread
From: Fabien Chouteau @ 2012-01-26 14:54 UTC (permalink / raw)
To: qemu-devel; +Cc: blauwirbel, avi, atar4qemu
This patch implements the RX channel of GRLIB UART with a FIFO to
improve data rate.
Signed-off-by: Fabien Chouteau <chouteau@adacore.com>
---
hw/grlib_apbuart.c | 106 +++++++++++++++++++++++++++++++++++++++++++--------
trace-events | 1 +
2 files changed, 90 insertions(+), 17 deletions(-)
diff --git a/hw/grlib_apbuart.c b/hw/grlib_apbuart.c
index f8a64e1..51406c6 100644
--- a/hw/grlib_apbuart.c
+++ b/hw/grlib_apbuart.c
@@ -24,7 +24,6 @@
#include "sysbus.h"
#include "qemu-char.h"
-#include "ptimer.h"
#include "trace.h"
@@ -66,6 +65,8 @@
#define SCALER_OFFSET 0x0C /* not supported */
#define FIFO_DEBUG_OFFSET 0x10 /* not supported */
+#define FIFO_LENGTH 1024
+
typedef struct UART {
SysBusDevice busdev;
MemoryRegion iomem;
@@ -77,21 +78,67 @@ typedef struct UART {
uint32_t receive;
uint32_t status;
uint32_t control;
+
+ /* FIFO */
+ char buffer[FIFO_LENGTH];
+ int len;
+ int current;
} UART;
+static int uart_data_to_read(UART *uart)
+{
+ return uart->current < uart->len;
+}
+
+static char uart_pop(UART *uart)
+{
+ char ret;
+
+ if (uart->len == 0) {
+ uart->status &= ~UART_DATA_READY;
+ return 0;
+ }
+
+ ret = uart->buffer[uart->current++];
+
+ if (uart->current >= uart->len) {
+ /* Flush */
+ uart->len = 0;
+ uart->current = 0;
+ }
+
+ if (!uart_data_to_read(uart)) {
+ uart->status &= ~UART_DATA_READY;
+ }
+
+ return ret;
+}
+
+static void uart_add_to_fifo(UART *uart,
+ const uint8_t *buffer,
+ int length)
+{
+ if (uart->len + length > FIFO_LENGTH) {
+ abort();
+ }
+ memcpy(uart->buffer + uart->len, buffer, length);
+ uart->len += length;
+}
+
static int grlib_apbuart_can_receive(void *opaque)
{
UART *uart = opaque;
- return !!(uart->status & UART_DATA_READY);
+ return FIFO_LENGTH - uart->len;
}
static void grlib_apbuart_receive(void *opaque, const uint8_t *buf, int size)
{
UART *uart = opaque;
- uart->receive = *buf;
- uart->status |= UART_DATA_READY;
+ uart_add_to_fifo(uart, buf, size);
+
+ uart->status |= UART_DATA_READY;
if (uart->control & UART_RECEIVE_INTERRUPT) {
qemu_irq_pulse(uart->irq);
@@ -103,9 +150,39 @@ static void grlib_apbuart_event(void *opaque, int event)
trace_grlib_apbuart_event(event);
}
-static void
-grlib_apbuart_write(void *opaque, target_phys_addr_t addr,
- uint64_t value, unsigned size)
+
+static uint64_t grlib_apbuart_read(void *opaque, target_phys_addr_t addr,
+ unsigned size)
+{
+ UART *uart = opaque;
+
+ addr &= 0xff;
+
+ /* Unit registers */
+ switch (addr) {
+ case DATA_OFFSET:
+ case DATA_OFFSET + 3: /* when only one byte read */
+ return uart_pop(uart);
+
+ case STATUS_OFFSET:
+ /* Read Only */
+ return uart->status;
+
+ case CONTROL_OFFSET:
+ return uart->control;
+
+ case SCALER_OFFSET:
+ /* Not supported */
+ return 0;
+
+ default:
+ trace_grlib_apbuart_readl_unknown(addr);
+ return 0;
+ }
+}
+
+static void grlib_apbuart_write(void *opaque, target_phys_addr_t addr,
+ uint64_t value, unsigned size)
{
UART *uart = opaque;
unsigned char c = 0;
@@ -115,6 +192,7 @@ grlib_apbuart_write(void *opaque, target_phys_addr_t addr,
/* Unit registers */
switch (addr) {
case DATA_OFFSET:
+ case DATA_OFFSET + 3: /* When only one byte write */
c = value & 0xFF;
qemu_chr_fe_write(uart->chr, &c, 1);
return;
@@ -124,7 +202,7 @@ grlib_apbuart_write(void *opaque, target_phys_addr_t addr,
return;
case CONTROL_OFFSET:
- /* Not supported */
+ uart->control = value;
return;
case SCALER_OFFSET:
@@ -138,21 +216,15 @@ grlib_apbuart_write(void *opaque, target_phys_addr_t addr,
trace_grlib_apbuart_writel_unknown(addr, value);
}
-static bool grlib_apbuart_accepts(void *opaque, target_phys_addr_t addr,
- unsigned size, bool is_write)
-{
- return is_write && size == 4;
-}
-
static const MemoryRegionOps grlib_apbuart_ops = {
- .write = grlib_apbuart_write,
- .valid.accepts = grlib_apbuart_accepts,
+ .write = grlib_apbuart_write,
+ .read = grlib_apbuart_read,
.endianness = DEVICE_NATIVE_ENDIAN,
};
static int grlib_apbuart_init(SysBusDevice *dev)
{
- UART *uart = FROM_SYSBUS(typeof(*uart), dev);
+ UART *uart = FROM_SYSBUS(typeof(*uart), dev);
qemu_chr_add_handlers(uart->chr,
grlib_apbuart_can_receive,
diff --git a/trace-events b/trace-events
index d2b0c61..5bb8bfb 100644
--- a/trace-events
+++ b/trace-events
@@ -340,6 +340,7 @@ grlib_irqmp_writel_unknown(uint64_t addr, uint32_t value) "addr 0x%"PRIx64" valu
# hw/grlib_apbuart.c
grlib_apbuart_event(int event) "event:%d"
grlib_apbuart_writel_unknown(uint64_t addr, uint32_t value) "addr 0x%"PRIx64" value 0x%x"
+grlib_apbuart_readl_unknown(uint64_t addr) "addr 0x%"PRIx64""
# hw/leon3.c
leon3_set_irq(int intno) "Set CPU IRQ %d"
--
1.7.4.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [Qemu-devel] [PATCH V2] GRLIB UART: Add RX channel
@ 2012-01-26 17:03 Fabien Chouteau
2012-01-28 12:20 ` Blue Swirl
0 siblings, 1 reply; 6+ messages in thread
From: Fabien Chouteau @ 2012-01-26 17:03 UTC (permalink / raw)
To: qemu-devel; +Cc: blauwirbel, avi, atar4qemu
This patch implements the RX channel of GRLIB UART with a FIFO to
improve data rate.
Signed-off-by: Fabien Chouteau <chouteau@adacore.com>
---
hw/grlib_apbuart.c | 106 +++++++++++++++++++++++++++++++++++++++++++--------
trace-events | 1 +
2 files changed, 90 insertions(+), 17 deletions(-)
diff --git a/hw/grlib_apbuart.c b/hw/grlib_apbuart.c
index f8a64e1..51406c6 100644
--- a/hw/grlib_apbuart.c
+++ b/hw/grlib_apbuart.c
@@ -24,7 +24,6 @@
#include "sysbus.h"
#include "qemu-char.h"
-#include "ptimer.h"
#include "trace.h"
@@ -66,6 +65,8 @@
#define SCALER_OFFSET 0x0C /* not supported */
#define FIFO_DEBUG_OFFSET 0x10 /* not supported */
+#define FIFO_LENGTH 1024
+
typedef struct UART {
SysBusDevice busdev;
MemoryRegion iomem;
@@ -77,21 +78,67 @@ typedef struct UART {
uint32_t receive;
uint32_t status;
uint32_t control;
+
+ /* FIFO */
+ char buffer[FIFO_LENGTH];
+ int len;
+ int current;
} UART;
+static int uart_data_to_read(UART *uart)
+{
+ return uart->current < uart->len;
+}
+
+static char uart_pop(UART *uart)
+{
+ char ret;
+
+ if (uart->len == 0) {
+ uart->status &= ~UART_DATA_READY;
+ return 0;
+ }
+
+ ret = uart->buffer[uart->current++];
+
+ if (uart->current >= uart->len) {
+ /* Flush */
+ uart->len = 0;
+ uart->current = 0;
+ }
+
+ if (!uart_data_to_read(uart)) {
+ uart->status &= ~UART_DATA_READY;
+ }
+
+ return ret;
+}
+
+static void uart_add_to_fifo(UART *uart,
+ const uint8_t *buffer,
+ int length)
+{
+ if (uart->len + length > FIFO_LENGTH) {
+ abort();
+ }
+ memcpy(uart->buffer + uart->len, buffer, length);
+ uart->len += length;
+}
+
static int grlib_apbuart_can_receive(void *opaque)
{
UART *uart = opaque;
- return !!(uart->status & UART_DATA_READY);
+ return FIFO_LENGTH - uart->len;
}
static void grlib_apbuart_receive(void *opaque, const uint8_t *buf, int size)
{
UART *uart = opaque;
- uart->receive = *buf;
- uart->status |= UART_DATA_READY;
+ uart_add_to_fifo(uart, buf, size);
+
+ uart->status |= UART_DATA_READY;
if (uart->control & UART_RECEIVE_INTERRUPT) {
qemu_irq_pulse(uart->irq);
@@ -103,9 +150,39 @@ static void grlib_apbuart_event(void *opaque, int event)
trace_grlib_apbuart_event(event);
}
-static void
-grlib_apbuart_write(void *opaque, target_phys_addr_t addr,
- uint64_t value, unsigned size)
+
+static uint64_t grlib_apbuart_read(void *opaque, target_phys_addr_t addr,
+ unsigned size)
+{
+ UART *uart = opaque;
+
+ addr &= 0xff;
+
+ /* Unit registers */
+ switch (addr) {
+ case DATA_OFFSET:
+ case DATA_OFFSET + 3: /* when only one byte read */
+ return uart_pop(uart);
+
+ case STATUS_OFFSET:
+ /* Read Only */
+ return uart->status;
+
+ case CONTROL_OFFSET:
+ return uart->control;
+
+ case SCALER_OFFSET:
+ /* Not supported */
+ return 0;
+
+ default:
+ trace_grlib_apbuart_readl_unknown(addr);
+ return 0;
+ }
+}
+
+static void grlib_apbuart_write(void *opaque, target_phys_addr_t addr,
+ uint64_t value, unsigned size)
{
UART *uart = opaque;
unsigned char c = 0;
@@ -115,6 +192,7 @@ grlib_apbuart_write(void *opaque, target_phys_addr_t addr,
/* Unit registers */
switch (addr) {
case DATA_OFFSET:
+ case DATA_OFFSET + 3: /* When only one byte write */
c = value & 0xFF;
qemu_chr_fe_write(uart->chr, &c, 1);
return;
@@ -124,7 +202,7 @@ grlib_apbuart_write(void *opaque, target_phys_addr_t addr,
return;
case CONTROL_OFFSET:
- /* Not supported */
+ uart->control = value;
return;
case SCALER_OFFSET:
@@ -138,21 +216,15 @@ grlib_apbuart_write(void *opaque, target_phys_addr_t addr,
trace_grlib_apbuart_writel_unknown(addr, value);
}
-static bool grlib_apbuart_accepts(void *opaque, target_phys_addr_t addr,
- unsigned size, bool is_write)
-{
- return is_write && size == 4;
-}
-
static const MemoryRegionOps grlib_apbuart_ops = {
- .write = grlib_apbuart_write,
- .valid.accepts = grlib_apbuart_accepts,
+ .write = grlib_apbuart_write,
+ .read = grlib_apbuart_read,
.endianness = DEVICE_NATIVE_ENDIAN,
};
static int grlib_apbuart_init(SysBusDevice *dev)
{
- UART *uart = FROM_SYSBUS(typeof(*uart), dev);
+ UART *uart = FROM_SYSBUS(typeof(*uart), dev);
qemu_chr_add_handlers(uart->chr,
grlib_apbuart_can_receive,
diff --git a/trace-events b/trace-events
index d2b0c61..5bb8bfb 100644
--- a/trace-events
+++ b/trace-events
@@ -340,6 +340,7 @@ grlib_irqmp_writel_unknown(uint64_t addr, uint32_t value) "addr 0x%"PRIx64" valu
# hw/grlib_apbuart.c
grlib_apbuart_event(int event) "event:%d"
grlib_apbuart_writel_unknown(uint64_t addr, uint32_t value) "addr 0x%"PRIx64" value 0x%x"
+grlib_apbuart_readl_unknown(uint64_t addr) "addr 0x%"PRIx64""
# hw/leon3.c
leon3_set_irq(int intno) "Set CPU IRQ %d"
--
1.7.4.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH V2] GRLIB UART: Add RX channel
2012-01-26 17:03 [Qemu-devel] [PATCH V2] GRLIB UART: Add RX channel Fabien Chouteau
@ 2012-01-28 12:20 ` Blue Swirl
2012-01-30 9:22 ` Fabien Chouteau
0 siblings, 1 reply; 6+ messages in thread
From: Blue Swirl @ 2012-01-28 12:20 UTC (permalink / raw)
To: Fabien Chouteau; +Cc: qemu-devel, atar4qemu, avi
On Thu, Jan 26, 2012 at 17:03, Fabien Chouteau <chouteau@adacore.com> wrote:
> This patch implements the RX channel of GRLIB UART with a FIFO to
> improve data rate.
>
> Signed-off-by: Fabien Chouteau <chouteau@adacore.com>
> ---
> hw/grlib_apbuart.c | 106 +++++++++++++++++++++++++++++++++++++++++++--------
> trace-events | 1 +
> 2 files changed, 90 insertions(+), 17 deletions(-)
>
> diff --git a/hw/grlib_apbuart.c b/hw/grlib_apbuart.c
> index f8a64e1..51406c6 100644
> --- a/hw/grlib_apbuart.c
> +++ b/hw/grlib_apbuart.c
> @@ -24,7 +24,6 @@
>
> #include "sysbus.h"
> #include "qemu-char.h"
> -#include "ptimer.h"
>
> #include "trace.h"
>
> @@ -66,6 +65,8 @@
> #define SCALER_OFFSET 0x0C /* not supported */
> #define FIFO_DEBUG_OFFSET 0x10 /* not supported */
>
> +#define FIFO_LENGTH 1024
> +
> typedef struct UART {
> SysBusDevice busdev;
> MemoryRegion iomem;
> @@ -77,21 +78,67 @@ typedef struct UART {
> uint32_t receive;
> uint32_t status;
> uint32_t control;
> +
> + /* FIFO */
> + char buffer[FIFO_LENGTH];
> + int len;
> + int current;
> } UART;
>
> +static int uart_data_to_read(UART *uart)
> +{
> + return uart->current < uart->len;
> +}
> +
> +static char uart_pop(UART *uart)
> +{
> + char ret;
> +
> + if (uart->len == 0) {
> + uart->status &= ~UART_DATA_READY;
> + return 0;
> + }
> +
> + ret = uart->buffer[uart->current++];
> +
> + if (uart->current >= uart->len) {
> + /* Flush */
> + uart->len = 0;
> + uart->current = 0;
> + }
> +
> + if (!uart_data_to_read(uart)) {
> + uart->status &= ~UART_DATA_READY;
> + }
> +
> + return ret;
> +}
> +
> +static void uart_add_to_fifo(UART *uart,
> + const uint8_t *buffer,
> + int length)
> +{
> + if (uart->len + length > FIFO_LENGTH) {
> + abort();
A guest could trigger this abort(), which is not OK. I think you can
just return.
> + }
> + memcpy(uart->buffer + uart->len, buffer, length);
> + uart->len += length;
> +}
> +
> static int grlib_apbuart_can_receive(void *opaque)
> {
> UART *uart = opaque;
>
> - return !!(uart->status & UART_DATA_READY);
> + return FIFO_LENGTH - uart->len;
> }
>
> static void grlib_apbuart_receive(void *opaque, const uint8_t *buf, int size)
> {
> UART *uart = opaque;
>
> - uart->receive = *buf;
> - uart->status |= UART_DATA_READY;
> + uart_add_to_fifo(uart, buf, size);
> +
> + uart->status |= UART_DATA_READY;
>
> if (uart->control & UART_RECEIVE_INTERRUPT) {
> qemu_irq_pulse(uart->irq);
> @@ -103,9 +150,39 @@ static void grlib_apbuart_event(void *opaque, int event)
> trace_grlib_apbuart_event(event);
> }
>
> -static void
> -grlib_apbuart_write(void *opaque, target_phys_addr_t addr,
> - uint64_t value, unsigned size)
> +
> +static uint64_t grlib_apbuart_read(void *opaque, target_phys_addr_t addr,
> + unsigned size)
> +{
> + UART *uart = opaque;
> +
> + addr &= 0xff;
> +
> + /* Unit registers */
> + switch (addr) {
> + case DATA_OFFSET:
> + case DATA_OFFSET + 3: /* when only one byte read */
> + return uart_pop(uart);
> +
> + case STATUS_OFFSET:
> + /* Read Only */
> + return uart->status;
> +
> + case CONTROL_OFFSET:
> + return uart->control;
> +
> + case SCALER_OFFSET:
> + /* Not supported */
> + return 0;
> +
> + default:
> + trace_grlib_apbuart_readl_unknown(addr);
> + return 0;
> + }
> +}
> +
> +static void grlib_apbuart_write(void *opaque, target_phys_addr_t addr,
> + uint64_t value, unsigned size)
> {
> UART *uart = opaque;
> unsigned char c = 0;
> @@ -115,6 +192,7 @@ grlib_apbuart_write(void *opaque, target_phys_addr_t addr,
> /* Unit registers */
> switch (addr) {
> case DATA_OFFSET:
> + case DATA_OFFSET + 3: /* When only one byte write */
> c = value & 0xFF;
> qemu_chr_fe_write(uart->chr, &c, 1);
> return;
> @@ -124,7 +202,7 @@ grlib_apbuart_write(void *opaque, target_phys_addr_t addr,
> return;
>
> case CONTROL_OFFSET:
> - /* Not supported */
> + uart->control = value;
> return;
>
> case SCALER_OFFSET:
> @@ -138,21 +216,15 @@ grlib_apbuart_write(void *opaque, target_phys_addr_t addr,
> trace_grlib_apbuart_writel_unknown(addr, value);
> }
>
> -static bool grlib_apbuart_accepts(void *opaque, target_phys_addr_t addr,
> - unsigned size, bool is_write)
> -{
> - return is_write && size == 4;
> -}
> -
> static const MemoryRegionOps grlib_apbuart_ops = {
> - .write = grlib_apbuart_write,
> - .valid.accepts = grlib_apbuart_accepts,
> + .write = grlib_apbuart_write,
> + .read = grlib_apbuart_read,
> .endianness = DEVICE_NATIVE_ENDIAN,
> };
>
> static int grlib_apbuart_init(SysBusDevice *dev)
> {
> - UART *uart = FROM_SYSBUS(typeof(*uart), dev);
> + UART *uart = FROM_SYSBUS(typeof(*uart), dev);
>
> qemu_chr_add_handlers(uart->chr,
> grlib_apbuart_can_receive,
> diff --git a/trace-events b/trace-events
> index d2b0c61..5bb8bfb 100644
> --- a/trace-events
> +++ b/trace-events
> @@ -340,6 +340,7 @@ grlib_irqmp_writel_unknown(uint64_t addr, uint32_t value) "addr 0x%"PRIx64" valu
> # hw/grlib_apbuart.c
> grlib_apbuart_event(int event) "event:%d"
> grlib_apbuart_writel_unknown(uint64_t addr, uint32_t value) "addr 0x%"PRIx64" value 0x%x"
> +grlib_apbuart_readl_unknown(uint64_t addr) "addr 0x%"PRIx64""
>
> # hw/leon3.c
> leon3_set_irq(int intno) "Set CPU IRQ %d"
> --
> 1.7.4.1
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH V2] GRLIB UART: Add RX channel
2012-01-28 12:20 ` Blue Swirl
@ 2012-01-30 9:22 ` Fabien Chouteau
2012-01-30 19:28 ` Blue Swirl
0 siblings, 1 reply; 6+ messages in thread
From: Fabien Chouteau @ 2012-01-30 9:22 UTC (permalink / raw)
To: Blue Swirl; +Cc: qemu-devel, atar4qemu, avi
On 28/01/2012 13:20, Blue Swirl wrote:
> On Thu, Jan 26, 2012 at 17:03, Fabien Chouteau <chouteau@adacore.com> wrote:
>> This patch implements the RX channel of GRLIB UART with a FIFO to
>> improve data rate.
>>
>> Signed-off-by: Fabien Chouteau <chouteau@adacore.com>
>> ---
>> hw/grlib_apbuart.c | 106 +++++++++++++++++++++++++++++++++++++++++++--------
>> trace-events | 1 +
>> 2 files changed, 90 insertions(+), 17 deletions(-)
>>
>> diff --git a/hw/grlib_apbuart.c b/hw/grlib_apbuart.c
>> index f8a64e1..51406c6 100644
>> --- a/hw/grlib_apbuart.c
>> +++ b/hw/grlib_apbuart.c
>> @@ -24,7 +24,6 @@
>>
>> #include "sysbus.h"
>> #include "qemu-char.h"
>> -#include "ptimer.h"
>>
>> #include "trace.h"
>>
>> @@ -66,6 +65,8 @@
>> #define SCALER_OFFSET 0x0C /* not supported */
>> #define FIFO_DEBUG_OFFSET 0x10 /* not supported */
>>
>> +#define FIFO_LENGTH 1024
>> +
>> typedef struct UART {
>> SysBusDevice busdev;
>> MemoryRegion iomem;
>> @@ -77,21 +78,67 @@ typedef struct UART {
>> uint32_t receive;
>> uint32_t status;
>> uint32_t control;
>> +
>> + /* FIFO */
>> + char buffer[FIFO_LENGTH];
>> + int len;
>> + int current;
>> } UART;
>>
>> +static int uart_data_to_read(UART *uart)
>> +{
>> + return uart->current < uart->len;
>> +}
>> +
>> +static char uart_pop(UART *uart)
>> +{
>> + char ret;
>> +
>> + if (uart->len == 0) {
>> + uart->status &= ~UART_DATA_READY;
>> + return 0;
>> + }
>> +
>> + ret = uart->buffer[uart->current++];
>> +
>> + if (uart->current >= uart->len) {
>> + /* Flush */
>> + uart->len = 0;
>> + uart->current = 0;
>> + }
>> +
>> + if (!uart_data_to_read(uart)) {
>> + uart->status &= ~UART_DATA_READY;
>> + }
>> +
>> + return ret;
>> +}
>> +
>> +static void uart_add_to_fifo(UART *uart,
>> + const uint8_t *buffer,
>> + int length)
>> +{
>> + if (uart->len + length > FIFO_LENGTH) {
>> + abort();
>
> A guest could trigger this abort(), which is not OK. I think you can
> just return.
>
This will abort if Qemu sends more bytes than the number requested in
grlib_apbuart_can_receive, so this would be a failure from Qemu not the
guest.
Regards,
--
Fabien Chouteau
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH V2] GRLIB UART: Add RX channel
2012-01-30 9:22 ` Fabien Chouteau
@ 2012-01-30 19:28 ` Blue Swirl
0 siblings, 0 replies; 6+ messages in thread
From: Blue Swirl @ 2012-01-30 19:28 UTC (permalink / raw)
To: Fabien Chouteau; +Cc: qemu-devel, atar4qemu, avi
On Mon, Jan 30, 2012 at 09:22, Fabien Chouteau <chouteau@adacore.com> wrote:
> On 28/01/2012 13:20, Blue Swirl wrote:
>> On Thu, Jan 26, 2012 at 17:03, Fabien Chouteau <chouteau@adacore.com> wrote:
>>> This patch implements the RX channel of GRLIB UART with a FIFO to
>>> improve data rate.
>>>
>>> Signed-off-by: Fabien Chouteau <chouteau@adacore.com>
>>> ---
>>> hw/grlib_apbuart.c | 106 +++++++++++++++++++++++++++++++++++++++++++--------
>>> trace-events | 1 +
>>> 2 files changed, 90 insertions(+), 17 deletions(-)
>>>
>>> diff --git a/hw/grlib_apbuart.c b/hw/grlib_apbuart.c
>>> index f8a64e1..51406c6 100644
>>> --- a/hw/grlib_apbuart.c
>>> +++ b/hw/grlib_apbuart.c
>>> @@ -24,7 +24,6 @@
>>>
>>> #include "sysbus.h"
>>> #include "qemu-char.h"
>>> -#include "ptimer.h"
>>>
>>> #include "trace.h"
>>>
>>> @@ -66,6 +65,8 @@
>>> #define SCALER_OFFSET 0x0C /* not supported */
>>> #define FIFO_DEBUG_OFFSET 0x10 /* not supported */
>>>
>>> +#define FIFO_LENGTH 1024
>>> +
>>> typedef struct UART {
>>> SysBusDevice busdev;
>>> MemoryRegion iomem;
>>> @@ -77,21 +78,67 @@ typedef struct UART {
>>> uint32_t receive;
>>> uint32_t status;
>>> uint32_t control;
>>> +
>>> + /* FIFO */
>>> + char buffer[FIFO_LENGTH];
>>> + int len;
>>> + int current;
>>> } UART;
>>>
>>> +static int uart_data_to_read(UART *uart)
>>> +{
>>> + return uart->current < uart->len;
>>> +}
>>> +
>>> +static char uart_pop(UART *uart)
>>> +{
>>> + char ret;
>>> +
>>> + if (uart->len == 0) {
>>> + uart->status &= ~UART_DATA_READY;
>>> + return 0;
>>> + }
>>> +
>>> + ret = uart->buffer[uart->current++];
>>> +
>>> + if (uart->current >= uart->len) {
>>> + /* Flush */
>>> + uart->len = 0;
>>> + uart->current = 0;
>>> + }
>>> +
>>> + if (!uart_data_to_read(uart)) {
>>> + uart->status &= ~UART_DATA_READY;
>>> + }
>>> +
>>> + return ret;
>>> +}
>>> +
>>> +static void uart_add_to_fifo(UART *uart,
>>> + const uint8_t *buffer,
>>> + int length)
>>> +{
>>> + if (uart->len + length > FIFO_LENGTH) {
>>> + abort();
>>
>> A guest could trigger this abort(), which is not OK. I think you can
>> just return.
>>
>
> This will abort if Qemu sends more bytes than the number requested in
> grlib_apbuart_can_receive, so this would be a failure from Qemu not the
> guest.
OK. Thanks, applied.
> Regards,
>
> --
> Fabien Chouteau
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-01-30 19:29 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-01-26 17:03 [Qemu-devel] [PATCH V2] GRLIB UART: Add RX channel Fabien Chouteau
2012-01-28 12:20 ` Blue Swirl
2012-01-30 9:22 ` Fabien Chouteau
2012-01-30 19:28 ` Blue Swirl
-- strict thread matches above, loose matches on Subject: below --
2012-01-26 14:54 Fabien Chouteau
2012-01-26 14:12 Fabien Chouteau
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).