qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v35 10/13] target/avr: Add limited support for USART and 16 b


From: Philippe Mathieu-Daudé
Subject: Re: [PATCH v35 10/13] target/avr: Add limited support for USART and 16 bit timer peripherals
Date: Fri, 22 Nov 2019 16:41:16 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.1.1

On 11/22/19 3:41 PM, Aleksandar Markovic wrote:
On Tue, Oct 29, 2019 at 10:25 PM Michael Rolnik <address@hidden> wrote:

From: Sarah Harris <address@hidden>

These were designed to facilitate testing but should provide enough function to 
be useful in other contexts.
Only a subset of the functions of each peripheral is implemented, mainly due to 
the lack of a standard way to handle electrical connections (like GPIO pins).

Signed-off-by: Sarah Harris <address@hidden>
---
  hw/char/Kconfig                |   3 +
  hw/char/Makefile.objs          |   1 +
  hw/char/avr_usart.c            | 324 ++++++++++++++++++
  hw/misc/Kconfig                |   3 +
  hw/misc/Makefile.objs          |   2 +
  hw/misc/avr_mask.c             | 112 ++++++
  hw/timer/Kconfig               |   3 +
  hw/timer/Makefile.objs         |   2 +
  hw/timer/avr_timer16.c         | 605 +++++++++++++++++++++++++++++++++
  include/hw/char/avr_usart.h    |  97 ++++++
  include/hw/misc/avr_mask.h     |  47 +++
  include/hw/timer/avr_timer16.h |  97 ++++++
  12 files changed, 1296 insertions(+)
  create mode 100644 hw/char/avr_usart.c
  create mode 100644 hw/misc/avr_mask.c
  create mode 100644 hw/timer/avr_timer16.c
  create mode 100644 include/hw/char/avr_usart.h
  create mode 100644 include/hw/misc/avr_mask.h
  create mode 100644 include/hw/timer/avr_timer16.h

diff --git a/hw/char/Kconfig b/hw/char/Kconfig
index 40e7a8b8bb..331b20983f 100644
--- a/hw/char/Kconfig
+++ b/hw/char/Kconfig
@@ -46,3 +46,6 @@ config SCLPCONSOLE

  config TERMINAL3270
      bool
+
+config AVR_USART
+    bool
diff --git a/hw/char/Makefile.objs b/hw/char/Makefile.objs
index 02d8a66925..f05c1f5667 100644
--- a/hw/char/Makefile.objs
+++ b/hw/char/Makefile.objs
@@ -21,6 +21,7 @@ obj-$(CONFIG_PSERIES) += spapr_vty.o
  obj-$(CONFIG_DIGIC) += digic-uart.o
  obj-$(CONFIG_STM32F2XX_USART) += stm32f2xx_usart.o
  obj-$(CONFIG_RASPI) += bcm2835_aux.o
+common-obj-$(CONFIG_AVR_USART) += avr_usart.o

  common-obj-$(CONFIG_CMSDK_APB_UART) += cmsdk-apb-uart.o
  common-obj-$(CONFIG_ETRAXFS) += etraxfs_ser.o
diff --git a/hw/char/avr_usart.c b/hw/char/avr_usart.c
new file mode 100644
index 0000000000..9ca3c2a1cd
--- /dev/null
+++ b/hw/char/avr_usart.c
@@ -0,0 +1,324 @@
+/*
+ * AVR USART
+ *
+ * Copyright (c) 2018 University of Kent
+ * Author: Sarah Harris
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to 
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "qemu/osdep.h"
+#include "hw/char/avr_usart.h"
+#include "qemu/log.h"
+#include "hw/irq.h"
+#include "hw/qdev-properties.h"
+
+static int avr_usart_can_receive(void *opaque)
+{
+    AVRUsartState *usart = opaque;
+
+    if (usart->data_valid || !(usart->csrb & USART_CSRB_RXEN)) {
+        return 0;
+    }
+    return 1;

Here we tell the chardev frontend that we can receive at most 1 byte, ...

+}
+
+static void avr_usart_receive(void *opaque, const uint8_t *buffer, int size)
+{
+    AVRUsartState *usart = opaque;
+    assert(size == 1);

... so this condition is true, the frontend will never provide us more than 1 byte.


Hello, Michael.

I see the line "assert(size == 1);" is used here, and in really numerous
places in USART emulation (as a rule, at the very beginnings of function
bodies). Could you explain to me the justification for that line? Is there
a place in documentation that would expain the need for it? If this is
justified, why is there the need for argument "int size" in corresponding
functions? If some external rule/API forces you to have that argument for
all such functions, can you tell me what rule/API is that?

Some backends have FIFO queues, so can process more chars at once.


Yours,
Aleksandar

+    assert(!usart->data_valid);
+    usart->data = buffer[0];

Here the model consumes the 1st char of an array of at most 1 byte.

I suppose Sarah wanted to be sure we are not dropping characters.

+    usart->data_valid = true;
+    usart->csra |= USART_CSRA_RXC;
+    if (usart->csrb & USART_CSRB_RXCIE) {
+        qemu_set_irq(usart->rxc_irq, 1);
+    }
+}
[...]




reply via email to

[Prev in Thread] Current Thread [Next in Thread]