From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.2 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id DFC3BC73C7C for ; Wed, 10 Jul 2019 10:07:58 +0000 (UTC) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 964BF20838 for ; Wed, 10 Jul 2019 10:07:58 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 964BF20838 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Received: from localhost ([::1]:59662 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.86_2) (envelope-from ) id 1hl9W1-0002DR-Sv for qemu-devel@archiver.kernel.org; Wed, 10 Jul 2019 06:07:57 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:34919) by lists.gnu.org with esmtp (Exim 4.86_2) (envelope-from ) id 1hl9UH-0001Lg-L4 for qemu-devel@nongnu.org; Wed, 10 Jul 2019 06:06:16 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1hl9UD-0000sK-SB for qemu-devel@nongnu.org; Wed, 10 Jul 2019 06:06:09 -0400 Received: from mail-wr1-f67.google.com ([209.85.221.67]:39191) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1hl9UD-0000ql-Gc for qemu-devel@nongnu.org; Wed, 10 Jul 2019 06:06:05 -0400 Received: by mail-wr1-f67.google.com with SMTP id x4so1779305wrt.6 for ; Wed, 10 Jul 2019 03:06:05 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:openpgp:message-id :date:user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=iz6zlZTqNWXU0I16vuRYLL8veqa/ROUscseuKW9P5+U=; b=U6Da9Gsq2OpXKKkO4vbGL50m+k66Ld1zv2kRiCBhV+U2QPn2Z5sUruvlSGKItWIEa0 XYL+oybUrPaO6BgJrY2h5qtIX30j0AObSzwuJTzcwF4VLyscW0+HFeRnoGm/4tELBpps wnD7Gpmf1TiXPXds6eFJmQAMRewZLkFoOaA6uI4sQge7lrP8046cybfTIacHe8ITQQmo NqFH2irg7LeBjLfSdXPc3fHT6124aT8AR1p4+kBMgKM6zOcFht54kDQ1gbn41azS+zqy S56iIR/1vjTdTS/4XARmOhlEGYmnLztdHyM162bL7Ecj18l4yvvdykSfFcT4bXLupuky bFUg== X-Gm-Message-State: APjAAAVkEsIpVUCLzQAsns1USbKNnm5n8JsOStTmT6WBm8NsdNy4KCLw 2nqnKWylcawVWfsRXXgo9jlMKA== X-Google-Smtp-Source: APXvYqw4o8i7gw0mfRust9aiCVY8NgLrb26ThGv/QyY7/Gm1P1eMf07ttuBtm4W276oBPegrO1J0NQ== X-Received: by 2002:adf:e947:: with SMTP id m7mr31303883wrn.123.1562753163835; Wed, 10 Jul 2019 03:06:03 -0700 (PDT) Received: from [10.32.224.100] (red-hat-inc.vlan560.asr1.mad1.gblx.net. [159.63.51.90]) by smtp.gmail.com with ESMTPSA id s3sm1821877wmh.27.2019.07.10.03.06.02 (version=TLS1_3 cipher=AEAD-AES128-GCM-SHA256 bits=128/128); Wed, 10 Jul 2019 03:06:02 -0700 (PDT) To: Michael Rolnik , qemu-devel@nongnu.org References: <20190709213651.77315-1-mrolnik@gmail.com> <20190709213651.77315-6-mrolnik@gmail.com> From: =?UTF-8?Q?Philippe_Mathieu-Daud=c3=a9?= Openpgp: id=89C1E78F601EE86C867495CBA2A3FD6EDEADC0DE; url=http://pgp.mit.edu/pks/lookup?op=get&search=0xA2A3FD6EDEADC0DE Message-ID: <0c7166ff-616b-574d-428c-6463e34aca0e@redhat.com> Date: Wed, 10 Jul 2019 12:06:01 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.7.0 MIME-Version: 1.0 In-Reply-To: <20190709213651.77315-6-mrolnik@gmail.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 209.85.221.67 Subject: Re: [Qemu-devel] [PATCH v25 5/7] target/avr: Add limited support for USART and 16 bit timer peripherals X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: imammedo@redhat.com, Sarah Harris , richard.henderson@linaro.org Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" Hi Michael, Sarah. Sorry for reviewing this series that late. On 7/9/19 11:36 PM, Michael Rolnik wrote: > From: Sarah Harris > > 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: Michael Rolnik > --- > hw/char/Kconfig | 3 + > hw/char/Makefile.objs | 1 + > hw/char/avr_usart.c | 316 ++++++++++++++++++ > hw/timer/Kconfig | 3 + > hw/timer/Makefile.objs | 1 + > hw/timer/avr_timer16.c | 587 +++++++++++++++++++++++++++++++++ > include/hw/char/avr_usart.h | 99 ++++++ > include/hw/timer/avr_timer16.h | 99 ++++++ > 8 files changed, 1109 insertions(+) > create mode 100644 hw/char/avr_usart.c > create mode 100644 hw/timer/avr_timer16.c > create mode 100644 include/hw/char/avr_usart.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..09ed50f1d0 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 > +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..26c711336b > --- /dev/null > +++ b/hw/char/avr_usart.c > @@ -0,0 +1,316 @@ > +/* > + * 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" > + > +static int avr_usart_can_receive(void *opaque) > +{ > + AVRUsartState *usart = opaque; > + > + if (usart->data_valid || !(usart->csrb & USART_CSRB_RXEN)) { > + return 0; > + } > + return 1; > +} > + > +static void avr_usart_receive(void *opaque, const uint8_t *buffer, int size) > +{ > + AVRUsartState *usart = opaque; > + assert(size == 1); > + assert(!usart->data_valid); > + usart->data = buffer[0]; > + usart->data_valid = true; > + usart->csra |= USART_CSRA_RXC; > + if (usart->csrb & USART_CSRB_RXCIE) { > + qemu_set_irq(usart->rxc_irq, 1); > + } > +} > + > +static void update_char_mask(AVRUsartState *usart) > +{ > + uint8_t mode = ((usart->csrc & USART_CSRC_CSZ0) ? 1 : 0) | > + ((usart->csrc & USART_CSRC_CSZ1) ? 2 : 0) | > + ((usart->csrb & USART_CSRB_CSZ2) ? 4 : 0); > + switch (mode) { > + case 0: > + usart->char_mask = 0b11111; > + break; > + case 1: > + usart->char_mask = 0b111111; > + break; > + case 2: > + usart->char_mask = 0b1111111; > + break; > + case 3: > + usart->char_mask = 0b11111111; > + break; > + case 4: > + /* Fallthrough. */ > + case 5: > + /* Fallthrough. */ > + case 6: > + qemu_log_mask( > + LOG_GUEST_ERROR, > + "%s: Reserved character size 0x%x\n", > + __func__, > + mode); > + break; > + case 7: > + qemu_log_mask( > + LOG_GUEST_ERROR, > + "%s: Nine bit character size not supported (forcing eight)\n", > + __func__); > + usart->char_mask = 0b11111111; > + break; > + default: > + assert(0); > + } > +} > + > +static void avr_usart_reset(DeviceState *dev) > +{ > + AVRUsartState *usart = AVR_USART(dev); > + usart->data_valid = false; > + usart->csra = 0b00100000; > + usart->csrb = 0b00000000; > + usart->csrc = 0b00000110; > + usart->brrl = 0; > + usart->brrh = 0; > + update_char_mask(usart); > + qemu_set_irq(usart->rxc_irq, 0); > + qemu_set_irq(usart->txc_irq, 0); > + qemu_set_irq(usart->dre_irq, 0); > +} > + > +static uint64_t avr_usart_read(void *opaque, hwaddr addr, unsigned int size) > +{ > + AVRUsartState *usart = opaque; > + uint8_t prr; > + uint8_t data; > + assert(size == 1); > + > + cpu_physical_memory_read(usart->prr_address, &prr, 1); This looks wrong... You should implement the PowerManagement block as a device, map it, link it to this device, then access directly usart->prr_link->regs[PRR0]. > + if (prr & usart->prr_mask) { > + /* USART disabled, ignore. */ > + avr_usart_reset(DEVICE(usart)); > + return 0; > + } > + > + switch (addr) { > + case USART_DR: > + if (!(usart->csrb & USART_CSRB_RXEN)) { > + /* Receiver disabled, ignore. */ > + return 0; > + } > + if (usart->data_valid) { > + data = usart->data & usart->char_mask; > + usart->data_valid = false; > + } else { > + data = 0; > + } > + usart->csra &= 0xff ^ USART_CSRA_RXC; > + qemu_set_irq(usart->rxc_irq, 0); > + qemu_chr_fe_accept_input(&usart->chr); > + return data; > + case USART_CSRA: > + return usart->csra; > + case USART_CSRB: > + return usart->csrb; > + case USART_CSRC: > + return usart->csrc; > + case USART_BRRL: > + return usart->brrl; > + case USART_BRRH: > + return usart->brrh; > + default: > + qemu_log_mask( > + LOG_GUEST_ERROR, > + "%s: Bad offset 0x%"HWADDR_PRIx"\n", > + __func__, > + addr); > + } > + return 0; > +} > + > +static void avr_usart_write(void *opaque, hwaddr addr, uint64_t value, > + unsigned int size) > +{ > + AVRUsartState *usart = opaque; > + uint8_t mask; > + uint8_t data; > + assert((value & 0xff) == value); > + assert(size == 1); > + > + uint8_t prr; > + cpu_physical_memory_read(usart->prr_address, &prr, 1); > + if (prr & usart->prr_mask) { > + /* USART disabled, ignore. */ > + avr_usart_reset(DEVICE(usart)); > + return; > + } > + > + switch (addr) { > + case USART_DR: > + if (!(usart->csrb & USART_CSRB_TXEN)) { > + /* Transmitter disabled, ignore. */ > + return; > + } > + usart->csra |= USART_CSRA_TXC; > + usart->csra |= USART_CSRA_DRE; > + if (usart->csrb & USART_CSRB_TXCIE) { > + qemu_set_irq(usart->txc_irq, 1); > + usart->csra &= 0xff ^ USART_CSRA_TXC; > + } > + if (usart->csrb & USART_CSRB_DREIE) { > + qemu_set_irq(usart->dre_irq, 1); > + } > + data = value; > + qemu_chr_fe_write_all(&usart->chr, &data, 1); > + break; > + case USART_CSRA: > + mask = 0b01000011; > + /* Mask read-only bits. */ > + value = (value & mask) | (usart->csra & (0xff ^ mask)); > + usart->csra = value; > + if (value & USART_CSRA_TXC) { > + usart->csra ^= USART_CSRA_TXC; > + qemu_set_irq(usart->txc_irq, 0); > + } > + if (value & USART_CSRA_MPCM) { > + qemu_log_mask( > + LOG_GUEST_ERROR, > + "%s: MPCM not supported by USART\n", > + __func__); > + } > + break; > + case USART_CSRB: > + mask = 0b11111101; > + /* Mask read-only bits. */ > + value = (value & mask) | (usart->csrb & (0xff ^ mask)); > + usart->csrb = value; > + if (!(value & USART_CSRB_RXEN)) { > + /* Receiver disabled, flush input buffer. */ > + usart->data_valid = false; > + } > + qemu_set_irq(usart->rxc_irq, > + ((value & USART_CSRB_RXCIE) && > + (usart->csra & USART_CSRA_RXC)) ? 1 : 0); > + qemu_set_irq(usart->txc_irq, > + ((value & USART_CSRB_TXCIE) && > + (usart->csra & USART_CSRA_TXC)) ? 1 : 0); > + qemu_set_irq(usart->dre_irq, > + ((value & USART_CSRB_DREIE) && > + (usart->csra & USART_CSRA_DRE)) ? 1 : 0); > + update_char_mask(usart); > + break; > + case USART_CSRC: > + usart->csrc = value; > + if ((value & USART_CSRC_MSEL1) && (value & USART_CSRC_MSEL0)) { > + qemu_log_mask( > + LOG_GUEST_ERROR, > + "%s: SPI mode not supported by USART\n", > + __func__); > + } > + if ((value & USART_CSRC_MSEL1) && !(value & USART_CSRC_MSEL0)) { > + qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad USART mode\n", __func__); > + } > + if (!(value & USART_CSRC_PM1) && (value & USART_CSRC_PM0)) { > + qemu_log_mask( > + LOG_GUEST_ERROR, > + "%s: Bad USART parity mode\n", > + __func__); > + } > + update_char_mask(usart); > + break; > + case USART_BRRL: > + usart->brrl = value; > + break; > + case USART_BRRH: > + usart->brrh = value & 0b00001111; > + break; > + default: > + qemu_log_mask( > + LOG_GUEST_ERROR, > + "%s: Bad offset 0x%"HWADDR_PRIx"\n", > + __func__, > + addr); > + } > +} > + > +static const MemoryRegionOps avr_usart_ops = { > + .read = avr_usart_read, > + .write = avr_usart_write, This misses: .impl.max = 1, > + .endianness = DEVICE_NATIVE_ENDIAN, > +}; > + > +static Property avr_usart_properties[] = { > + DEFINE_PROP_CHR("chardev", AVRUsartState, chr), > + DEFINE_PROP_END_OF_LIST(), > +}; > + > +static void avr_usart_init(Object *obj) > +{ > + AVRUsartState *s = AVR_USART(obj); > + sysbus_init_irq(SYS_BUS_DEVICE(obj), &s->rxc_irq); > + sysbus_init_irq(SYS_BUS_DEVICE(obj), &s->dre_irq); > + sysbus_init_irq(SYS_BUS_DEVICE(obj), &s->txc_irq); > + memory_region_init_io(&s->mmio, obj, &avr_usart_ops, s, TYPE_AVR_USART, 7); USART is 8 byte wide. > + sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->mmio); > +} > + > +static void avr_usart_realize(DeviceState *dev, Error **errp) > +{ > + AVRUsartState *s = AVR_USART(dev); > + qemu_chr_fe_set_handlers(&s->chr, avr_usart_can_receive, > + avr_usart_receive, NULL, NULL, > + s, NULL, true); > + avr_usart_reset(dev); > +} > + > +static void avr_usart_class_init(ObjectClass *klass, void *data) > +{ > + DeviceClass *dc = DEVICE_CLASS(klass); > + > + dc->reset = avr_usart_reset; > + dc->props = avr_usart_properties; > + dc->realize = avr_usart_realize; > +} > + > +static const TypeInfo avr_usart_info = { > + .name = TYPE_AVR_USART, > + .parent = TYPE_SYS_BUS_DEVICE, > + .instance_size = sizeof(AVRUsartState), > + .instance_init = avr_usart_init, > + .class_init = avr_usart_class_init, > +}; > + > +static void avr_usart_register_types(void) > +{ > + type_register_static(&avr_usart_info); > +} > + > +type_init(avr_usart_register_types) > diff --git a/hw/timer/Kconfig b/hw/timer/Kconfig > index 51921eb63f..ad754df750 100644 > --- a/hw/timer/Kconfig > +++ b/hw/timer/Kconfig > @@ -61,3 +61,6 @@ config CMSDK_APB_TIMER > config CMSDK_APB_DUALTIMER > bool > select PTIMER > + > +config AVR_TIMER16 > + bool > diff --git a/hw/timer/Makefile.objs b/hw/timer/Makefile.objs > index 123d92c969..4f853e265d 100644 > --- a/hw/timer/Makefile.objs > +++ b/hw/timer/Makefile.objs > @@ -35,6 +35,7 @@ obj-$(CONFIG_PXA2XX) += pxa2xx_timer.o > obj-$(CONFIG_SH4) += sh_timer.o > obj-$(CONFIG_DIGIC) += digic-timer.o > obj-$(CONFIG_MIPS_CPS) += mips_gictimer.o > +obj-$(CONFIG_AVR_TIMER16) += avr_timer16.o > > obj-$(CONFIG_MC146818RTC) += mc146818rtc.o > > diff --git a/hw/timer/avr_timer16.c b/hw/timer/avr_timer16.c > new file mode 100644 > index 0000000000..89f8f55eaa > --- /dev/null > +++ b/hw/timer/avr_timer16.c > @@ -0,0 +1,587 @@ > +/* > + * AVR 16 bit timer > + * > + * Copyright (c) 2018 University of Kent > + * Author: Ed Robbins > + * > + * 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. > + */ > + > +/* > + * Driver for 16 bit timers on 8 bit AVR devices. > + * Note: > + * ATmega640/V-1280/V-1281/V-2560/V-2561/V timers 1, 3, 4 and 5 are 16 bit > + */ > + > +/* > + * XXX TODO: Power Reduction Register support > + * prescaler pause support > + * PWM modes, GPIO, output capture pins, input compare pin > + */ > + > +#include "qemu/osdep.h" > +#include "hw/timer/avr_timer16.h" > +#include "qemu/log.h" > + > +/* Register offsets */ > +#define T16_CRA 0x0 > +#define T16_CRB 0x1 > +#define T16_CRC 0x2 > +#define T16_CNTL 0x4 > +#define T16_CNTH 0x5 > +#define T16_ICRL 0x6 > +#define T16_ICRH 0x7 > +#define T16_OCRAL 0x8 > +#define T16_OCRAH 0x9 > +#define T16_OCRBL 0xa > +#define T16_OCRBH 0xb > +#define T16_OCRCL 0xc > +#define T16_OCRCH 0xd > + > +/* Field masks */ > +#define T16_CRA_WGM01 0x3 > +#define T16_CRA_COMC 0xc > +#define T16_CRA_COMB 0x30 > +#define T16_CRA_COMA 0xc0 > +#define T16_CRA_OC_CONF \ > + (T16_CRA_COMA | T16_CRA_COMB | T16_CRA_COMC) > + > +#define T16_CRB_CS 0x7 > +#define T16_CRB_WGM23 0x18 > +#define T16_CRB_ICES 0x40 > +#define T16_CRB_ICNC 0x80 > + > +#define T16_CRC_FOCC 0x20 > +#define T16_CRC_FOCB 0x40 > +#define T16_CRC_FOCA 0x80 > + > +/* Fields masks both TIMSK and TIFR (interrupt mask/flag registers) */ > +#define T16_INT_TOV 0x1 /* Timer overflow */ > +#define T16_INT_OCA 0x2 /* Output compare A */ > +#define T16_INT_OCB 0x4 /* Output compare B */ > +#define T16_INT_OCC 0x8 /* Output compare C */ > +#define T16_INT_IC 0x20 /* Input capture */ > + > +/* Clock source values */ > +#define T16_CLKSRC_STOPPED 0 > +#define T16_CLKSRC_DIV1 1 > +#define T16_CLKSRC_DIV8 2 > +#define T16_CLKSRC_DIV64 3 > +#define T16_CLKSRC_DIV256 4 > +#define T16_CLKSRC_DIV1024 5 > +#define T16_CLKSRC_EXT_FALLING 6 > +#define T16_CLKSRC_EXT_RISING 7 > + > +/* Timer mode values (not including PWM modes) */ > +#define T16_MODE_NORMAL 0 > +#define T16_MODE_CTC_OCRA 4 > +#define T16_MODE_CTC_ICR 12 > + > +/* Accessors */ > +#define CLKSRC(t16) (t16->crb & T16_CRB_CS) > +#define MODE(t16) (((t16->crb & T16_CRB_WGM23) >> 1) | \ > + (t16->cra & T16_CRA_WGM01)) > +#define CNT(t16) VAL16(t16->cntl, t16->cnth) > +#define OCRA(t16) VAL16(t16->ocral, t16->ocrah) > +#define OCRB(t16) VAL16(t16->ocrbl, t16->ocrbh) > +#define OCRC(t16) VAL16(t16->ocrcl, t16->ocrch) > +#define ICR(t16) VAL16(t16->icrl, t16->icrh) > + > +/* Helper macros */ > +#define VAL16(l, h) ((h << 8) | l) > +#define ERROR(fmt, args...) \ > + qemu_log_mask(LOG_GUEST_ERROR, "%s: " fmt "\n", __func__, ## args) > +#define DB_PRINT(fmt, args...) /* Nothing */ > +/*#define DB_PRINT(fmt, args...) printf("%s: " fmt "\n", __func__, ## args)*/ > + > +static inline int64_t avr_timer16_ns_to_ticks(AVRTimer16State *t16, int64_t t) > +{ > + if (t16->period_ns == 0) { > + return 0; > + } > + return t / t16->period_ns; > +} > + > +static void avr_timer16_update_cnt(AVRTimer16State *t16) > +{ > + uint16_t cnt; > + cnt = avr_timer16_ns_to_ticks(t16, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) - > + t16->reset_time_ns); > + t16->cntl = (uint8_t)(cnt & 0xff); > + t16->cnth = (uint8_t)((cnt & 0xff00) >> 8); > +} > + > +static inline void avr_timer16_recalc_reset_time(AVRTimer16State *t16) > +{ > + t16->reset_time_ns = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) - > + CNT(t16) * t16->period_ns; > +} > + > +static void avr_timer16_clock_reset(AVRTimer16State *t16) > +{ > + t16->cntl = 0; > + t16->cnth = 0; > + t16->reset_time_ns = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL); > +} > + > +static void avr_timer16_clksrc_update(AVRTimer16State *t16) > +{ > + uint16_t divider = 0; > + switch (CLKSRC(t16)) { > + case T16_CLKSRC_EXT_FALLING: > + case T16_CLKSRC_EXT_RISING: > + ERROR("external clock source unsupported"); > + goto end; > + case T16_CLKSRC_STOPPED: > + goto end; > + case T16_CLKSRC_DIV1: > + divider = 1; > + break; > + case T16_CLKSRC_DIV8: > + divider = 8; > + break; > + case T16_CLKSRC_DIV64: > + divider = 64; > + break; > + case T16_CLKSRC_DIV256: > + divider = 256; > + break; > + case T16_CLKSRC_DIV1024: > + divider = 1024; > + break; > + default: > + goto end; > + } > + t16->freq_hz = t16->cpu_freq_hz / divider; > + t16->period_ns = 1000000000ULL / t16->freq_hz; > + DB_PRINT("Timer frequency %" PRIu64 " hz, period %" PRIu64 " ns (%f s)", > + t16->freq_hz, t16->period_ns, 1 / (double)t16->freq_hz); > +end: > + return; > +} > + > +static void avr_timer16_set_alarm(AVRTimer16State *t16) > +{ > + if (CLKSRC(t16) == T16_CLKSRC_EXT_FALLING || > + CLKSRC(t16) == T16_CLKSRC_EXT_RISING || > + CLKSRC(t16) == T16_CLKSRC_STOPPED) { > + /* Timer is disabled or set to external clock source (unsupported) */ > + goto end; > + } > + > + uint64_t alarm_offset = 0xffff; > + enum NextInterrupt next_interrupt = OVERFLOW; > + > + switch (MODE(t16)) { > + case T16_MODE_NORMAL: > + /* Normal mode */ > + if (OCRA(t16) < alarm_offset && OCRA(t16) > CNT(t16) && > + (t16->imsk & T16_INT_OCA)) { > + alarm_offset = OCRA(t16); > + next_interrupt = COMPA; > + } > + break; > + case T16_MODE_CTC_OCRA: > + /* CTC mode, top = ocra */ > + if (OCRA(t16) < alarm_offset && OCRA(t16) > CNT(t16)) { > + alarm_offset = OCRA(t16); > + next_interrupt = COMPA; > + } > + break; > + case T16_MODE_CTC_ICR: > + /* CTC mode, top = icr */ > + if (ICR(t16) < alarm_offset && ICR(t16) > CNT(t16)) { > + alarm_offset = ICR(t16); > + next_interrupt = CAPT; > + } > + if (OCRA(t16) < alarm_offset && OCRA(t16) > CNT(t16) && > + (t16->imsk & T16_INT_OCA)) { > + alarm_offset = OCRA(t16); > + next_interrupt = COMPA; > + } > + break; > + default: > + ERROR("pwm modes are unsupported"); > + goto end; > + } > + if (OCRB(t16) < alarm_offset && OCRB(t16) > CNT(t16) && > + (t16->imsk & T16_INT_OCB)) { > + alarm_offset = OCRB(t16); > + next_interrupt = COMPB; > + } > + if (OCRC(t16) < alarm_offset && OCRB(t16) > CNT(t16) && > + (t16->imsk & T16_INT_OCC)) { > + alarm_offset = OCRB(t16); > + next_interrupt = COMPC; > + } > + alarm_offset -= CNT(t16); > + > + t16->next_interrupt = next_interrupt; > + uint64_t alarm_ns = > + t16->reset_time_ns + ((CNT(t16) + alarm_offset) * t16->period_ns); > + timer_mod(t16->timer, alarm_ns); > + > + DB_PRINT("next alarm %" PRIu64 " ns from now", > + alarm_offset * t16->period_ns); > + > +end: > + return; > +} > + > +static void avr_timer16_interrupt(void *opaque) > +{ > + AVRTimer16State *t16 = opaque; > + uint8_t mode = MODE(t16); > + > + avr_timer16_update_cnt(t16); > + > + if (CLKSRC(t16) == T16_CLKSRC_EXT_FALLING || > + CLKSRC(t16) == T16_CLKSRC_EXT_RISING || > + CLKSRC(t16) == T16_CLKSRC_STOPPED) { > + /* Timer is disabled or set to external clock source (unsupported) */ > + return; > + } > + > + DB_PRINT("interrupt, cnt = %d", CNT(t16)); > + > + /* Counter overflow */ > + if (t16->next_interrupt == OVERFLOW) { > + DB_PRINT("0xffff overflow"); > + avr_timer16_clock_reset(t16); > + if (t16->imsk & T16_INT_TOV) { > + t16->ifr |= T16_INT_TOV; > + qemu_set_irq(t16->ovf_irq, 1); > + } > + } > + /* Check for ocra overflow in CTC mode */ > + if (mode == T16_MODE_CTC_OCRA && t16->next_interrupt == COMPA) { > + DB_PRINT("CTC OCRA overflow"); > + avr_timer16_clock_reset(t16); > + } > + /* Check for icr overflow in CTC mode */ > + if (mode == T16_MODE_CTC_ICR && t16->next_interrupt == CAPT) { > + DB_PRINT("CTC ICR overflow"); > + avr_timer16_clock_reset(t16); > + if (t16->imsk & T16_INT_IC) { > + t16->ifr |= T16_INT_IC; > + qemu_set_irq(t16->capt_irq, 1); > + } > + } > + /* Check for output compare interrupts */ > + if (t16->imsk & T16_INT_OCA && t16->next_interrupt == COMPA) { > + t16->ifr |= T16_INT_OCA; > + qemu_set_irq(t16->compa_irq, 1); > + } > + if (t16->imsk & T16_INT_OCB && t16->next_interrupt == COMPB) { > + t16->ifr |= T16_INT_OCB; > + qemu_set_irq(t16->compb_irq, 1); > + } > + if (t16->imsk & T16_INT_OCC && t16->next_interrupt == COMPC) { > + t16->ifr |= T16_INT_OCC; > + qemu_set_irq(t16->compc_irq, 1); > + } > + avr_timer16_set_alarm(t16); > +} > + > +static void avr_timer16_reset(DeviceState *dev) > +{ > + AVRTimer16State *t16 = AVR_TIMER16(dev); > + > + avr_timer16_clock_reset(t16); > + avr_timer16_clksrc_update(t16); > + avr_timer16_set_alarm(t16); > + > + qemu_set_irq(t16->capt_irq, 0); > + qemu_set_irq(t16->compa_irq, 0); > + qemu_set_irq(t16->compb_irq, 0); > + qemu_set_irq(t16->compc_irq, 0); > + qemu_set_irq(t16->ovf_irq, 0); > +} > + > +static uint64_t avr_timer16_read(void *opaque, hwaddr offset, unsigned size) > +{ > + assert(size == 1); > + AVRTimer16State *t16 = opaque; > + uint8_t retval = 0; > + > + switch (offset) { > + case T16_CRA: > + retval = t16->cra; > + break; > + case T16_CRB: > + retval = t16->crb; > + break; > + case T16_CRC: > + retval = t16->crc; > + break; > + case T16_CNTL: > + avr_timer16_update_cnt(t16); > + t16->rtmp = t16->cnth; > + retval = t16->cntl; > + break; > + case T16_CNTH: > + retval = t16->rtmp; > + break; > + case T16_ICRL: > + /* > + * The timer copies cnt to icr when the input capture pin changes > + * state or when the analog comparator has a match. We don't > + * emulate this behaviour. We do support it's use for defining a > + * TOP value in T16_MODE_CTC_ICR > + */ > + t16->rtmp = t16->icrh; > + retval = t16->icrl; > + break; > + case T16_ICRH: > + retval = t16->rtmp; > + break; > + case T16_OCRAL: > + retval = t16->ocral; > + break; > + case T16_OCRAH: > + retval = t16->ocrah; > + break; > + case T16_OCRBL: > + retval = t16->ocrbl; > + break; > + case T16_OCRBH: > + retval = t16->ocrbh; > + break; > + case T16_OCRCL: > + retval = t16->ocrcl; > + break; > + case T16_OCRCH: > + retval = t16->ocrch; > + break; > + default: > + break; > + } > + return (uint64_t)retval; > +} > + > +static void avr_timer16_write(void *opaque, hwaddr offset, > + uint64_t val64, unsigned size) > +{ > + assert(size == 1); > + AVRTimer16State *t16 = opaque; > + uint8_t val8 = (uint8_t)val64; > + uint8_t prev_clk_src = CLKSRC(t16); > + > + DB_PRINT("write %d to offset %d", val8, (uint8_t)offset); > + > + switch (offset) { > + case T16_CRA: > + t16->cra = val8; > + if (t16->cra & T16_CRA_OC_CONF) { > + ERROR("output compare pins unsupported"); > + } > + break; > + case T16_CRB: > + t16->crb = val8; > + if (t16->crb & T16_CRB_ICNC) { > + ERROR("input capture noise canceller unsupported"); > + } > + if (t16->crb & T16_CRB_ICES) { > + ERROR("input capture unsupported"); > + } > + if (CLKSRC(t16) != prev_clk_src) { > + avr_timer16_clksrc_update(t16); > + if (prev_clk_src == T16_CLKSRC_STOPPED) { > + t16->reset_time_ns = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL); > + } > + } > + break; > + case T16_CRC: > + t16->crc = val8; > + ERROR("output compare pins unsupported"); > + break; > + case T16_CNTL: > + /* > + * CNT is the 16-bit counter value, it must be read/written via > + * a temporary register (rtmp) to make the read/write atomic. > + */ > + /* ICR also has this behaviour, and shares rtmp */ > + /* > + * Writing CNT blocks compare matches for one clock cycle. > + * Writing CNT to TOP or to an OCR value (if in use) will > + * skip the relevant interrupt > + */ > + t16->cntl = val8; > + t16->cnth = t16->rtmp; > + avr_timer16_recalc_reset_time(t16); > + break; > + case T16_CNTH: > + t16->rtmp = val8; > + break; > + case T16_ICRL: > + /* ICR can only be written in mode T16_MODE_CTC_ICR */ > + if (MODE(t16) == T16_MODE_CTC_ICR) { > + t16->icrl = val8; > + t16->icrh = t16->rtmp; > + } > + break; > + case T16_ICRH: > + if (MODE(t16) == T16_MODE_CTC_ICR) { > + t16->rtmp = val8; > + } > + break; > + case T16_OCRAL: > + /* > + * OCRn cause the relevant output compare flag to be raised, and > + * trigger an interrupt, when CNT is equal to the value here > + */ > + t16->ocral = val8; > + break; > + case T16_OCRAH: > + t16->ocrah = val8; > + break; > + case T16_OCRBL: > + t16->ocrbl = val8; > + break; > + case T16_OCRBH: > + t16->ocrbh = val8; > + break; > + case T16_OCRCL: > + t16->ocrcl = val8; > + break; > + case T16_OCRCH: > + t16->ocrch = val8; > + break; > + default: > + break; > + } > + avr_timer16_set_alarm(t16); > +} > + > +static uint64_t avr_timer16_imsk_read(void *opaque, > + hwaddr offset, > + unsigned size) > +{ > + assert(size == 1); > + AVRTimer16State *t16 = opaque; > + if (offset != 0) { > + return 0; > + } > + return t16->imsk; > +} > + > +static void avr_timer16_imsk_write(void *opaque, hwaddr offset, > + uint64_t val64, unsigned size) > +{ > + assert(size == 1); > + AVRTimer16State *t16 = opaque; > + if (offset != 0) { > + return; > + } > + t16->imsk = (uint8_t)val64; > +} > + > +static uint64_t avr_timer16_ifr_read(void *opaque, > + hwaddr offset, > + unsigned size) > +{ > + assert(size == 1); > + AVRTimer16State *t16 = opaque; > + if (offset != 0) { > + return 0; > + } > + return t16->ifr; > +} > + > +static void avr_timer16_ifr_write(void *opaque, hwaddr offset, > + uint64_t val64, unsigned size) > +{ > + assert(size == 1); > + AVRTimer16State *t16 = opaque; > + if (offset != 0) { > + return; > + } > + t16->ifr = (uint8_t)val64; > +} > + > +static const MemoryRegionOps avr_timer16_ops = { > + .read = avr_timer16_read, > + .write = avr_timer16_write if you use .impl { .min = 2 } you simplify your design. > + .endianness = DEVICE_NATIVE_ENDIAN > +}; > + > +static const MemoryRegionOps avr_timer16_imsk_ops = { > + .read = avr_timer16_imsk_read, > + .write = avr_timer16_imsk_write, > + .endianness = DEVICE_NATIVE_ENDIAN > +}; > + > +static const MemoryRegionOps avr_timer16_ifr_ops = { > + .read = avr_timer16_ifr_read, > + .write = avr_timer16_ifr_write, > + .endianness = DEVICE_NATIVE_ENDIAN > +}; > + > +static Property avr_timer16_properties[] = { > + DEFINE_PROP_UINT64("cpu-frequency-hz", struct AVRTimer16State, > + cpu_freq_hz, 20000000), > + DEFINE_PROP_END_OF_LIST(), > +}; > + > +static void avr_timer16_init(Object *obj) > +{ > + AVRTimer16State *s = AVR_TIMER16(obj); > + > + sysbus_init_irq(SYS_BUS_DEVICE(obj), &s->capt_irq); > + sysbus_init_irq(SYS_BUS_DEVICE(obj), &s->compa_irq); > + sysbus_init_irq(SYS_BUS_DEVICE(obj), &s->compb_irq); > + sysbus_init_irq(SYS_BUS_DEVICE(obj), &s->compc_irq); > + sysbus_init_irq(SYS_BUS_DEVICE(obj), &s->ovf_irq); > + > + memory_region_init_io(&s->iomem, obj, &avr_timer16_ops, > + s, TYPE_AVR_TIMER16, 0xe); A timer region is 0x10 wide, Ok. > + memory_region_init_io(&s->imsk_iomem, obj, &avr_timer16_imsk_ops, > + s, TYPE_AVR_TIMER16, 0x1); > + memory_region_init_io(&s->ifr_iomem, obj, &avr_timer16_ifr_ops, > + s, TYPE_AVR_TIMER16, 0x1); These 2 registers seem part of the Interrupt Controller, and don't belong here. You might want to use a LINK_PROPERTY to link a timer device to the INTC. > + > + sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->iomem); > + sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->imsk_iomem); > + sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->ifr_iomem); > + > + s->timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, avr_timer16_interrupt, s); > +} > + > +static void avr_timer16_class_init(ObjectClass *klass, void *data) > +{ > + DeviceClass *dc = DEVICE_CLASS(klass); > + > + dc->reset = avr_timer16_reset; > + dc->props = avr_timer16_properties; > +} > + > +static const TypeInfo avr_timer16_info = { > + .name = TYPE_AVR_TIMER16, > + .parent = TYPE_SYS_BUS_DEVICE, > + .instance_size = sizeof(AVRTimer16State), > + .instance_init = avr_timer16_init, > + .class_init = avr_timer16_class_init, > +}; > + > +static void avr_timer16_register_types(void) > +{ > + type_register_static(&avr_timer16_info); > +} > + > +type_init(avr_timer16_register_types) > diff --git a/include/hw/char/avr_usart.h b/include/hw/char/avr_usart.h > new file mode 100644 > index 0000000000..ba28fbe5d1 > --- /dev/null > +++ b/include/hw/char/avr_usart.h > @@ -0,0 +1,99 @@ > +/* > + * 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. > + */ > + > +#ifndef HW_AVR_USART_H > +#define HW_AVR_USART_H > + > +#include "hw/sysbus.h" > +#include "chardev/char-fe.h" > +#include "hw/hw.h" > + > +/* Offsets of registers. */ > +#define USART_DR 0x06 > +#define USART_CSRA 0x00 > +#define USART_CSRB 0x01 > +#define USART_CSRC 0x02 > +#define USART_BRRH 0x05 > +#define USART_BRRL 0x04 > + > +/* Relevant bits in regiters. */ > +#define USART_CSRA_RXC (1 << 7) > +#define USART_CSRA_TXC (1 << 6) > +#define USART_CSRA_DRE (1 << 5) > +#define USART_CSRA_MPCM (1 << 0) > + > +#define USART_CSRB_RXCIE (1 << 7) > +#define USART_CSRB_TXCIE (1 << 6) > +#define USART_CSRB_DREIE (1 << 5) > +#define USART_CSRB_RXEN (1 << 4) > +#define USART_CSRB_TXEN (1 << 3) > +#define USART_CSRB_CSZ2 (1 << 2) > +#define USART_CSRB_RXB8 (1 << 1) > +#define USART_CSRB_TXB8 (1 << 0) > + > +#define USART_CSRC_MSEL1 (1 << 7) > +#define USART_CSRC_MSEL0 (1 << 6) > +#define USART_CSRC_PM1 (1 << 5) > +#define USART_CSRC_PM0 (1 << 4) > +#define USART_CSRC_CSZ1 (1 << 2) > +#define USART_CSRC_CSZ0 (1 << 1) > + > +#define TYPE_AVR_USART "avr-usart" > +#define AVR_USART(obj) \ > + OBJECT_CHECK(AVRUsartState, (obj), TYPE_AVR_USART) > + > +typedef struct { > + /* */ > + SysBusDevice parent_obj; > + > + /* */ > + MemoryRegion mmio; > + > + CharBackend chr; > + > + /* Address of Power Reduction Register and bit that controls this UART */ > + hwaddr prr_address; > + uint8_t prr_mask; So ideally you'd use a link to the PowerManagement block here. > + > + uint8_t data; > + bool data_valid; > + uint8_t char_mask; > + /* Control and Status Registers */ > + uint8_t csra; > + uint8_t csrb; > + uint8_t csrc; > + /* Baud Rate Registers (low/high byte) */ > + uint8_t brrh; > + uint8_t brrl; > + > + /* Receive Complete */ > + qemu_irq rxc_irq; > + /* Transmit Complete */ > + qemu_irq txc_irq; > + /* Data Register Empty */ > + qemu_irq dre_irq; > +} AVRUsartState; > + > +#endif /* HW_AVR_USART_H */ > diff --git a/include/hw/timer/avr_timer16.h b/include/hw/timer/avr_timer16.h > new file mode 100644 > index 0000000000..301e36a154 > --- /dev/null > +++ b/include/hw/timer/avr_timer16.h > @@ -0,0 +1,99 @@ > +/* > + * AVR 16 bit timer > + * > + * Copyright (c) 2018 University of Kent > + * Author: Ed Robbins > + * > + * 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. > + */ > + > +/* > + * Driver for 16 bit timers on 8 bit AVR devices. > + * Note: > + * On ATmega640/V-1280/V-1281/V-2560/V-2561/V timers 1, 3, 4 and 5 are 16 bit > + */ > + > +#ifndef AVR_TIMER16_H > +#define AVR_TIMER16_H > + > +#include "hw/sysbus.h" > +#include "qemu/timer.h" > +#include "hw/hw.h" > + > +enum NextInterrupt { > + OVERFLOW, > + COMPA, > + COMPB, > + COMPC, > + CAPT > +}; > + > +#define TYPE_AVR_TIMER16 "avr-timer16" > +#define AVR_TIMER16(obj) \ > + OBJECT_CHECK(AVRTimer16State, (obj), TYPE_AVR_TIMER16) > + > +typedef struct AVRTimer16State { > + /* */ > + SysBusDevice parent_obj; > + > + /* */ > + MemoryRegion iomem; > + MemoryRegion imsk_iomem; > + MemoryRegion ifr_iomem; > + QEMUTimer *timer; > + qemu_irq capt_irq; > + qemu_irq compa_irq; > + qemu_irq compb_irq; > + qemu_irq compc_irq; > + qemu_irq ovf_irq; > + > + /* Address of Power Reduction Register and bit that controls this timer */ > + hwaddr prr_address; > + uint8_t prr_mask; Again, you should use a link to the PowerManagement block here. > + > + /* registers */ > + uint8_t cra; > + uint8_t crb; > + uint8_t crc; Using 16-bit for following... > + uint8_t cntl; > + uint8_t cnth; > + uint8_t icrl; > + uint8_t icrh; > + uint8_t ocral; > + uint8_t ocrah; > + uint8_t ocrbl; > + uint8_t ocrbh; > + uint8_t ocrcl; > + uint8_t ocrch; ... until here would simplify your model. > + /* > + * Reads and writes to CNT and ICR utilise a bizarre temporary > + * register, which we emulate > + */ > + uint8_t rtmp; > + uint8_t imsk; > + uint8_t ifr; > + > + uint64_t cpu_freq_hz; > + uint64_t freq_hz; > + uint64_t period_ns; > + uint64_t reset_time_ns; > + enum NextInterrupt next_interrupt; > +} AVRTimer16State; > + > +#endif /* AVR_TIMER16_H */ > If the maintainer taking your series don't think my comments are blockers, you might consider them as later cleanup. Regards, Phil.