* [PATCH 01/11] hw/misc/led: Add yellow LED
2021-03-13 16:54 [PATCH 00/11] AVR patch queue for QEMU 6.0 Philippe Mathieu-Daudé
@ 2021-03-13 16:54 ` Philippe Mathieu-Daudé
2021-03-13 19:51 ` Richard Henderson
2021-03-13 16:54 ` [PATCH 02/11] hw/avr/arduino: List board schematic links Philippe Mathieu-Daudé
` (11 subsequent siblings)
12 siblings, 1 reply; 30+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-03-13 16:54 UTC (permalink / raw)
To: qemu-devel
Cc: Thomas Huth, Sarah Harris, Michael Rolnik, Philippe Mathieu-Daudé
Add the yellow "lime" LED.
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
include/hw/misc/led.h | 1 +
hw/misc/led.c | 1 +
2 files changed, 2 insertions(+)
diff --git a/include/hw/misc/led.h b/include/hw/misc/led.h
index aa359b87c20..29c08795708 100644
--- a/include/hw/misc/led.h
+++ b/include/hw/misc/led.h
@@ -27,6 +27,7 @@ typedef enum { /* Coarse wavelength range */
LED_COLOR_BLUE, /* 475 nm */
LED_COLOR_CYAN, /* 500 nm */
LED_COLOR_GREEN, /* 535 nm */
+ LED_COLOR_YELLOW, /* 567 nm */
LED_COLOR_AMBER, /* 590 nm */
LED_COLOR_ORANGE, /* 615 nm */
LED_COLOR_RED, /* 630 nm */
diff --git a/hw/misc/led.c b/hw/misc/led.c
index 5266d026d0b..f552b8b6483 100644
--- a/hw/misc/led.c
+++ b/hw/misc/led.c
@@ -20,6 +20,7 @@ static const char * const led_color_name[] = {
[LED_COLOR_BLUE] = "blue",
[LED_COLOR_CYAN] = "cyan",
[LED_COLOR_GREEN] = "green",
+ [LED_COLOR_YELLOW] = "yellow",
[LED_COLOR_AMBER] = "amber",
[LED_COLOR_ORANGE] = "orange",
[LED_COLOR_RED] = "red",
--
2.26.2
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH 02/11] hw/avr/arduino: List board schematic links
2021-03-13 16:54 [PATCH 00/11] AVR patch queue for QEMU 6.0 Philippe Mathieu-Daudé
2021-03-13 16:54 ` [PATCH 01/11] hw/misc/led: Add yellow LED Philippe Mathieu-Daudé
@ 2021-03-13 16:54 ` Philippe Mathieu-Daudé
2021-03-13 19:51 ` Richard Henderson
2021-03-13 16:54 ` [PATCH 03/11] hw/avr: Add limited support for avr gpio registers Philippe Mathieu-Daudé
` (10 subsequent siblings)
12 siblings, 1 reply; 30+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-03-13 16:54 UTC (permalink / raw)
To: qemu-devel
Cc: Thomas Huth, Sarah Harris, Michael Rolnik, Philippe Mathieu-Daudé
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
hw/avr/arduino.c | 20 ++++++++++++++++----
1 file changed, 16 insertions(+), 4 deletions(-)
diff --git a/hw/avr/arduino.c b/hw/avr/arduino.c
index 3c8388490d6..3ff31492fa6 100644
--- a/hw/avr/arduino.c
+++ b/hw/avr/arduino.c
@@ -75,7 +75,10 @@ static void arduino_duemilanove_class_init(ObjectClass *oc, void *data)
MachineClass *mc = MACHINE_CLASS(oc);
ArduinoMachineClass *amc = ARDUINO_MACHINE_CLASS(oc);
- /* https://www.arduino.cc/en/Main/ArduinoBoardDuemilanove */
+ /*
+ * https://www.arduino.cc/en/Main/ArduinoBoardDuemilanove
+ * https://www.arduino.cc/en/uploads/Main/arduino-duemilanove-schematic.pdf
+ */
mc->desc = "Arduino Duemilanove (ATmega168)",
mc->alias = "2009";
amc->mcu_type = TYPE_ATMEGA168_MCU;
@@ -87,7 +90,10 @@ static void arduino_uno_class_init(ObjectClass *oc, void *data)
MachineClass *mc = MACHINE_CLASS(oc);
ArduinoMachineClass *amc = ARDUINO_MACHINE_CLASS(oc);
- /* https://store.arduino.cc/arduino-uno-rev3 */
+ /*
+ * https://store.arduino.cc/arduino-uno-rev3
+ * https://www.arduino.cc/en/uploads/Main/arduino-uno-schematic.pdf
+ */
mc->desc = "Arduino UNO (ATmega328P)";
mc->alias = "uno";
amc->mcu_type = TYPE_ATMEGA328_MCU;
@@ -99,7 +105,10 @@ static void arduino_mega_class_init(ObjectClass *oc, void *data)
MachineClass *mc = MACHINE_CLASS(oc);
ArduinoMachineClass *amc = ARDUINO_MACHINE_CLASS(oc);
- /* https://www.arduino.cc/en/Main/ArduinoBoardMega */
+ /*
+ * https://www.arduino.cc/en/Main/ArduinoBoardMega
+ * https://www.arduino.cc/en/uploads/Main/arduino-mega2560-schematic.pdf
+ */
mc->desc = "Arduino Mega (ATmega1280)";
mc->alias = "mega";
amc->mcu_type = TYPE_ATMEGA1280_MCU;
@@ -111,7 +120,10 @@ static void arduino_mega2560_class_init(ObjectClass *oc, void *data)
MachineClass *mc = MACHINE_CLASS(oc);
ArduinoMachineClass *amc = ARDUINO_MACHINE_CLASS(oc);
- /* https://store.arduino.cc/arduino-mega-2560-rev3 */
+ /*
+ * https://store.arduino.cc/arduino-mega-2560-rev3
+ * https://www.arduino.cc/en/uploads/Main/arduino-mega2560_R3-sch.pdf
+ */
mc->desc = "Arduino Mega 2560 (ATmega2560)";
mc->alias = "mega2560";
amc->mcu_type = TYPE_ATMEGA2560_MCU;
--
2.26.2
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH 03/11] hw/avr: Add limited support for avr gpio registers
2021-03-13 16:54 [PATCH 00/11] AVR patch queue for QEMU 6.0 Philippe Mathieu-Daudé
2021-03-13 16:54 ` [PATCH 01/11] hw/misc/led: Add yellow LED Philippe Mathieu-Daudé
2021-03-13 16:54 ` [PATCH 02/11] hw/avr/arduino: List board schematic links Philippe Mathieu-Daudé
@ 2021-03-13 16:54 ` Philippe Mathieu-Daudé
2021-03-13 19:55 ` Richard Henderson
` (2 more replies)
2021-03-13 16:54 ` [PATCH 04/11] hw/gpio/avr_gpio: Add migration VMstate Philippe Mathieu-Daudé
` (9 subsequent siblings)
12 siblings, 3 replies; 30+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-03-13 16:54 UTC (permalink / raw)
To: qemu-devel
Cc: Thomas Huth, Sarah Harris, Michael Rolnik,
Philippe Mathieu-Daudé,
Heecheol Yang
From: Heecheol Yang <heecheol.yang@outlook.com>
Add some of these features for AVR GPIO:
- GPIO I/O : PORTx registers
- Data Direction : DDRx registers
- DDRx toggling : PINx registers
Following things are not supported yet:
- MCUR registers
Signed-off-by: Heecheol Yang <heecheol.yang@outlook.com>
Reviewed-by: Michael Rolnik <mrolnik@gmail.com>
Message-Id: <DM6PR16MB247368DBD3447ABECDD795D7E6090@DM6PR16MB2473.namprd16.prod.outlook.com>
[PMD: Use AVR_GPIO_COUNT]
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
hw/avr/atmega.h | 2 +
include/hw/gpio/avr_gpio.h | 53 ++++++++++++++
hw/avr/atmega.c | 7 +-
hw/gpio/avr_gpio.c | 138 +++++++++++++++++++++++++++++++++++++
hw/avr/Kconfig | 1 +
hw/gpio/Kconfig | 3 +
hw/gpio/meson.build | 1 +
7 files changed, 203 insertions(+), 2 deletions(-)
create mode 100644 include/hw/gpio/avr_gpio.h
create mode 100644 hw/gpio/avr_gpio.c
diff --git a/hw/avr/atmega.h b/hw/avr/atmega.h
index a99ee15c7e1..e2289d5744e 100644
--- a/hw/avr/atmega.h
+++ b/hw/avr/atmega.h
@@ -13,6 +13,7 @@
#include "hw/char/avr_usart.h"
#include "hw/timer/avr_timer16.h"
+#include "hw/gpio/avr_gpio.h"
#include "hw/misc/avr_power.h"
#include "target/avr/cpu.h"
#include "qom/object.h"
@@ -44,6 +45,7 @@ struct AtmegaMcuState {
DeviceState *io;
AVRMaskState pwr[POWER_MAX];
AVRUsartState usart[USART_MAX];
+ AVRGPIOState gpio[GPIO_MAX];
AVRTimer16State timer[TIMER_MAX];
uint64_t xtal_freq_hz;
};
diff --git a/include/hw/gpio/avr_gpio.h b/include/hw/gpio/avr_gpio.h
new file mode 100644
index 00000000000..498a7275f05
--- /dev/null
+++ b/include/hw/gpio/avr_gpio.h
@@ -0,0 +1,53 @@
+/*
+ * AVR processors GPIO registers definition.
+ *
+ * Copyright (C) 2020 Heecheol Yang <heecheol.yang@outlook.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 or
+ * (at your option) version 3 of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef AVR_GPIO_H
+#define AVR_GPIO_H
+
+#include "hw/sysbus.h"
+#include "qom/object.h"
+
+/* Offsets of registers. */
+#define GPIO_PIN 0x00
+#define GPIO_DDR 0x01
+#define GPIO_PORT 0x02
+
+#define TYPE_AVR_GPIO "avr-gpio"
+OBJECT_DECLARE_SIMPLE_TYPE(AVRGPIOState, AVR_GPIO)
+#define AVR_GPIO_COUNT 8
+
+struct AVRGPIOState {
+ /*< private >*/
+ SysBusDevice parent_obj;
+
+ /*< public >*/
+ MemoryRegion mmio;
+
+ struct {
+ uint8_t pin;
+ uint8_t ddr;
+ uint8_t port;
+ } reg;
+
+ /* PORTx data changed IRQs */
+ qemu_irq out[8u];
+
+};
+
+#endif /* AVR_GPIO_H */
diff --git a/hw/avr/atmega.c b/hw/avr/atmega.c
index 44c6afebbb6..19c3122189f 100644
--- a/hw/avr/atmega.c
+++ b/hw/avr/atmega.c
@@ -283,8 +283,11 @@ static void atmega_realize(DeviceState *dev, Error **errp)
continue;
}
devname = g_strdup_printf("atmega-gpio-%c", 'a' + (char)i);
- create_unimplemented_device(devname,
- OFFSET_DATA + mc->dev[idx].addr, 3);
+ object_initialize_child(OBJECT(dev), devname, &s->gpio[i],
+ TYPE_AVR_GPIO);
+ sysbus_realize(SYS_BUS_DEVICE(&s->gpio[i]), &error_abort);
+ sysbus_mmio_map(SYS_BUS_DEVICE(&s->gpio[i]), 0,
+ OFFSET_DATA + mc->dev[idx].addr);
g_free(devname);
}
diff --git a/hw/gpio/avr_gpio.c b/hw/gpio/avr_gpio.c
new file mode 100644
index 00000000000..cdb574ef0d8
--- /dev/null
+++ b/hw/gpio/avr_gpio.c
@@ -0,0 +1,138 @@
+/*
+ * AVR processors GPIO registers emulation.
+ *
+ * Copyright (C) 2020 Heecheol Yang <heecheol.yang@outlook.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 or
+ * (at your option) version 3 of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+#include "qemu/osdep.h"
+#include "qemu/log.h"
+#include "qemu/module.h"
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "hw/sysbus.h"
+#include "hw/irq.h"
+#include "hw/gpio/avr_gpio.h"
+#include "hw/qdev-properties.h"
+
+static void avr_gpio_reset(DeviceState *dev)
+{
+ AVRGPIOState *gpio = AVR_GPIO(dev);
+
+ gpio->reg.pin = 0u;
+ gpio->reg.ddr = 0u;
+ gpio->reg.port = 0u;
+}
+
+static void avr_gpio_write_port(AVRGPIOState *s, uint64_t value)
+{
+ uint8_t pin;
+ uint8_t cur_port_val = s->reg.port;
+ uint8_t cur_ddr_val = s->reg.ddr;
+
+ for (pin = 0u; pin < AVR_GPIO_COUNT ; pin++) {
+ uint8_t cur_port_pin_val = cur_port_val & 0x01u;
+ uint8_t cur_ddr_pin_val = cur_ddr_val & 0x01u;
+ uint8_t new_port_pin_val = value & 0x01u;
+
+ if (cur_ddr_pin_val && (cur_port_pin_val != new_port_pin_val)) {
+ qemu_set_irq(s->out[pin], new_port_pin_val);
+ }
+ cur_port_val >>= 1u;
+ cur_ddr_val >>= 1u;
+ value >>= 1u;
+ }
+ s->reg.port = value & s->reg.ddr;
+}
+static uint64_t avr_gpio_read(void *opaque, hwaddr offset, unsigned int size)
+{
+ AVRGPIOState *s = (AVRGPIOState *)opaque;
+ switch (offset) {
+ case GPIO_PIN:
+ return s->reg.pin;
+ case GPIO_DDR:
+ return s->reg.ddr;
+ case GPIO_PORT:
+ return s->reg.port;
+ default:
+ g_assert_not_reached();
+ break;
+ }
+ return 0;
+}
+
+static void avr_gpio_write(void *opaque, hwaddr offset, uint64_t value,
+ unsigned int size)
+{
+ AVRGPIOState *s = (AVRGPIOState *)opaque;
+ value = value & 0xF;
+ switch (offset) {
+ case GPIO_PIN:
+ s->reg.pin = value;
+ s->reg.port ^= s->reg.pin;
+ break;
+ case GPIO_DDR:
+ s->reg.ddr = value;
+ break;
+ case GPIO_PORT:
+ avr_gpio_write_port(s, value);
+ break;
+ default:
+ g_assert_not_reached();
+ break;
+ }
+}
+
+static const MemoryRegionOps avr_gpio_ops = {
+ .read = avr_gpio_read,
+ .write = avr_gpio_write,
+ .endianness = DEVICE_NATIVE_ENDIAN,
+};
+
+static void avr_gpio_init(Object *obj)
+{
+ AVRGPIOState *s = AVR_GPIO(obj);
+
+ qdev_init_gpio_out(DEVICE(obj), s->out, ARRAY_SIZE(s->out));
+ memory_region_init_io(&s->mmio, obj, &avr_gpio_ops, s, TYPE_AVR_GPIO, 3);
+ sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->mmio);
+}
+static void avr_gpio_realize(DeviceState *dev, Error **errp)
+{
+ /* Do nothing currently */
+}
+
+
+static void avr_gpio_class_init(ObjectClass *klass, void *data)
+{
+ DeviceClass *dc = DEVICE_CLASS(klass);
+
+ dc->reset = avr_gpio_reset;
+ dc->realize = avr_gpio_realize;
+}
+
+static const TypeInfo avr_gpio_info = {
+ .name = TYPE_AVR_GPIO,
+ .parent = TYPE_SYS_BUS_DEVICE,
+ .instance_size = sizeof(AVRGPIOState),
+ .instance_init = avr_gpio_init,
+ .class_init = avr_gpio_class_init,
+};
+
+static void avr_gpio_register_types(void)
+{
+ type_register_static(&avr_gpio_info);
+}
+
+type_init(avr_gpio_register_types)
diff --git a/hw/avr/Kconfig b/hw/avr/Kconfig
index d31298c3cce..16a57ced11f 100644
--- a/hw/avr/Kconfig
+++ b/hw/avr/Kconfig
@@ -3,6 +3,7 @@ config AVR_ATMEGA_MCU
select AVR_TIMER16
select AVR_USART
select AVR_POWER
+ select AVR_GPIO
config ARDUINO
select AVR_ATMEGA_MCU
diff --git a/hw/gpio/Kconfig b/hw/gpio/Kconfig
index f0e7405f6e6..fde7019b2ba 100644
--- a/hw/gpio/Kconfig
+++ b/hw/gpio/Kconfig
@@ -13,3 +13,6 @@ config GPIO_PWR
config SIFIVE_GPIO
bool
+
+config AVR_GPIO
+ bool
diff --git a/hw/gpio/meson.build b/hw/gpio/meson.build
index 79568f00ce3..366aca52ca2 100644
--- a/hw/gpio/meson.build
+++ b/hw/gpio/meson.build
@@ -13,3 +13,4 @@
softmmu_ss.add(when: 'CONFIG_RASPI', if_true: files('bcm2835_gpio.c'))
softmmu_ss.add(when: 'CONFIG_ASPEED_SOC', if_true: files('aspeed_gpio.c'))
softmmu_ss.add(when: 'CONFIG_SIFIVE_GPIO', if_true: files('sifive_gpio.c'))
+softmmu_ss.add(when: 'CONFIG_AVR_GPIO', if_true: files('avr_gpio.c'))
--
2.26.2
^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH 03/11] hw/avr: Add limited support for avr gpio registers
2021-03-13 16:54 ` [PATCH 03/11] hw/avr: Add limited support for avr gpio registers Philippe Mathieu-Daudé
@ 2021-03-13 19:55 ` Richard Henderson
2021-03-14 10:26 ` Mark Cave-Ayland
2021-03-15 13:15 ` Sarah Harris
2 siblings, 0 replies; 30+ messages in thread
From: Richard Henderson @ 2021-03-13 19:55 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Thomas Huth, Sarah Harris, Michael Rolnik, Heecheol Yang
On 3/13/21 10:54 AM, Philippe Mathieu-Daudé wrote:
> +#define AVR_GPIO_COUNT 8
> +
> +struct AVRGPIOState {
> + /*< private >*/
> + SysBusDevice parent_obj;
> +
> + /*< public >*/
> + MemoryRegion mmio;
> +
> + struct {
> + uint8_t pin;
> + uint8_t ddr;
> + uint8_t port;
> + } reg;
> +
> + /* PORTx data changed IRQs */
> + qemu_irq out[8u];
AVR_GPIO_COUNT?
Can we drop all the useless 'u' suffixes from all over?
> + gpio->reg.pin = 0u;
> + gpio->reg.ddr = 0u;
> + gpio->reg.port = 0u;
...
> + uint8_t cur_port_pin_val = cur_port_val & 0x01u;
> + uint8_t cur_ddr_pin_val = cur_ddr_val & 0x01u;
> + uint8_t new_port_pin_val = value & 0x01u;
...
> + cur_port_val >>= 1u;
> + cur_ddr_val >>= 1u;
> + value >>= 1u;
etc.
Otherwise,
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
r~
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 03/11] hw/avr: Add limited support for avr gpio registers
2021-03-13 16:54 ` [PATCH 03/11] hw/avr: Add limited support for avr gpio registers Philippe Mathieu-Daudé
2021-03-13 19:55 ` Richard Henderson
@ 2021-03-14 10:26 ` Mark Cave-Ayland
2021-03-14 23:41 ` Philippe Mathieu-Daudé
2021-03-15 13:15 ` Sarah Harris
2 siblings, 1 reply; 30+ messages in thread
From: Mark Cave-Ayland @ 2021-03-14 10:26 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Thomas Huth, Sarah Harris, Michael Rolnik, Heecheol Yang
On 13/03/2021 16:54, Philippe Mathieu-Daudé wrote:
> From: Heecheol Yang <heecheol.yang@outlook.com>
>
> Add some of these features for AVR GPIO:
>
> - GPIO I/O : PORTx registers
> - Data Direction : DDRx registers
> - DDRx toggling : PINx registers
>
> Following things are not supported yet:
> - MCUR registers
>
> Signed-off-by: Heecheol Yang <heecheol.yang@outlook.com>
> Reviewed-by: Michael Rolnik <mrolnik@gmail.com>
> Message-Id: <DM6PR16MB247368DBD3447ABECDD795D7E6090@DM6PR16MB2473.namprd16.prod.outlook.com>
> [PMD: Use AVR_GPIO_COUNT]
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> hw/avr/atmega.h | 2 +
> include/hw/gpio/avr_gpio.h | 53 ++++++++++++++
> hw/avr/atmega.c | 7 +-
> hw/gpio/avr_gpio.c | 138 +++++++++++++++++++++++++++++++++++++
> hw/avr/Kconfig | 1 +
> hw/gpio/Kconfig | 3 +
> hw/gpio/meson.build | 1 +
> 7 files changed, 203 insertions(+), 2 deletions(-)
> create mode 100644 include/hw/gpio/avr_gpio.h
> create mode 100644 hw/gpio/avr_gpio.c
>
> diff --git a/hw/avr/atmega.h b/hw/avr/atmega.h
> index a99ee15c7e1..e2289d5744e 100644
> --- a/hw/avr/atmega.h
> +++ b/hw/avr/atmega.h
> @@ -13,6 +13,7 @@
>
> #include "hw/char/avr_usart.h"
> #include "hw/timer/avr_timer16.h"
> +#include "hw/gpio/avr_gpio.h"
> #include "hw/misc/avr_power.h"
> #include "target/avr/cpu.h"
> #include "qom/object.h"
> @@ -44,6 +45,7 @@ struct AtmegaMcuState {
> DeviceState *io;
> AVRMaskState pwr[POWER_MAX];
> AVRUsartState usart[USART_MAX];
> + AVRGPIOState gpio[GPIO_MAX];
> AVRTimer16State timer[TIMER_MAX];
> uint64_t xtal_freq_hz;
> };
> diff --git a/include/hw/gpio/avr_gpio.h b/include/hw/gpio/avr_gpio.h
> new file mode 100644
> index 00000000000..498a7275f05
> --- /dev/null
> +++ b/include/hw/gpio/avr_gpio.h
> @@ -0,0 +1,53 @@
> +/*
> + * AVR processors GPIO registers definition.
> + *
> + * Copyright (C) 2020 Heecheol Yang <heecheol.yang@outlook.com>
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation; either version 2 or
> + * (at your option) version 3 of the License.
Minor nit: isn't the wording of the existing QEMU license "licensed under the terms
of the GNU GPL, version 2 or later" rather than explicitly stating versions 2 and 3?
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License along
> + * with this program; if not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#ifndef AVR_GPIO_H
> +#define AVR_GPIO_H
> +
> +#include "hw/sysbus.h"
> +#include "qom/object.h"
> +
> +/* Offsets of registers. */
> +#define GPIO_PIN 0x00
> +#define GPIO_DDR 0x01
> +#define GPIO_PORT 0x02
> +
> +#define TYPE_AVR_GPIO "avr-gpio"
> +OBJECT_DECLARE_SIMPLE_TYPE(AVRGPIOState, AVR_GPIO)
> +#define AVR_GPIO_COUNT 8
> +
> +struct AVRGPIOState {
> + /*< private >*/
> + SysBusDevice parent_obj;
> +
> + /*< public >*/
> + MemoryRegion mmio;
> +
> + struct {
> + uint8_t pin;
> + uint8_t ddr;
> + uint8_t port;
> + } reg;
> +
> + /* PORTx data changed IRQs */
> + qemu_irq out[8u];
> +
> +};
> +
> +#endif /* AVR_GPIO_H */
> diff --git a/hw/avr/atmega.c b/hw/avr/atmega.c
> index 44c6afebbb6..19c3122189f 100644
> --- a/hw/avr/atmega.c
> +++ b/hw/avr/atmega.c
> @@ -283,8 +283,11 @@ static void atmega_realize(DeviceState *dev, Error **errp)
> continue;
> }
> devname = g_strdup_printf("atmega-gpio-%c", 'a' + (char)i);
> - create_unimplemented_device(devname,
> - OFFSET_DATA + mc->dev[idx].addr, 3);
> + object_initialize_child(OBJECT(dev), devname, &s->gpio[i],
> + TYPE_AVR_GPIO);
> + sysbus_realize(SYS_BUS_DEVICE(&s->gpio[i]), &error_abort);
> + sysbus_mmio_map(SYS_BUS_DEVICE(&s->gpio[i]), 0,
> + OFFSET_DATA + mc->dev[idx].addr);
> g_free(devname);
> }
>
> diff --git a/hw/gpio/avr_gpio.c b/hw/gpio/avr_gpio.c
> new file mode 100644
> index 00000000000..cdb574ef0d8
> --- /dev/null
> +++ b/hw/gpio/avr_gpio.c
> @@ -0,0 +1,138 @@
> +/*
> + * AVR processors GPIO registers emulation.
> + *
> + * Copyright (C) 2020 Heecheol Yang <heecheol.yang@outlook.com>
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation; either version 2 or
> + * (at your option) version 3 of the License.
And here too.
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License along
> + * with this program; if not, see <http://www.gnu.org/licenses/>.
> + */
> +#include "qemu/osdep.h"
> +#include "qemu/log.h"
> +#include "qemu/module.h"
> +#include "qemu/osdep.h"
> +#include "qapi/error.h"
> +#include "hw/sysbus.h"
> +#include "hw/irq.h"
> +#include "hw/gpio/avr_gpio.h"
> +#include "hw/qdev-properties.h"
> +
> +static void avr_gpio_reset(DeviceState *dev)
> +{
> + AVRGPIOState *gpio = AVR_GPIO(dev);
> +
> + gpio->reg.pin = 0u;
> + gpio->reg.ddr = 0u;
> + gpio->reg.port = 0u;
> +}
> +
> +static void avr_gpio_write_port(AVRGPIOState *s, uint64_t value)
> +{
> + uint8_t pin;
> + uint8_t cur_port_val = s->reg.port;
> + uint8_t cur_ddr_val = s->reg.ddr;
> +
> + for (pin = 0u; pin < AVR_GPIO_COUNT ; pin++) {
> + uint8_t cur_port_pin_val = cur_port_val & 0x01u;
> + uint8_t cur_ddr_pin_val = cur_ddr_val & 0x01u;
> + uint8_t new_port_pin_val = value & 0x01u;
> +
> + if (cur_ddr_pin_val && (cur_port_pin_val != new_port_pin_val)) {
> + qemu_set_irq(s->out[pin], new_port_pin_val);
> + }
> + cur_port_val >>= 1u;
> + cur_ddr_val >>= 1u;
> + value >>= 1u;
> + }
> + s->reg.port = value & s->reg.ddr;
> +}
> +static uint64_t avr_gpio_read(void *opaque, hwaddr offset, unsigned int size)
> +{
> + AVRGPIOState *s = (AVRGPIOState *)opaque;
> + switch (offset) {
> + case GPIO_PIN:
> + return s->reg.pin;
> + case GPIO_DDR:
> + return s->reg.ddr;
> + case GPIO_PORT:
> + return s->reg.port;
> + default:
> + g_assert_not_reached();
> + break;
> + }
> + return 0;
> +}
> +
> +static void avr_gpio_write(void *opaque, hwaddr offset, uint64_t value,
> + unsigned int size)
> +{
> + AVRGPIOState *s = (AVRGPIOState *)opaque;
> + value = value & 0xF;
> + switch (offset) {
> + case GPIO_PIN:
> + s->reg.pin = value;
> + s->reg.port ^= s->reg.pin;
> + break;
> + case GPIO_DDR:
> + s->reg.ddr = value;
> + break;
> + case GPIO_PORT:
> + avr_gpio_write_port(s, value);
> + break;
> + default:
> + g_assert_not_reached();
> + break;
> + }
> +}
> +
> +static const MemoryRegionOps avr_gpio_ops = {
> + .read = avr_gpio_read,
> + .write = avr_gpio_write,
> + .endianness = DEVICE_NATIVE_ENDIAN,
> +};
> +
> +static void avr_gpio_init(Object *obj)
> +{
> + AVRGPIOState *s = AVR_GPIO(obj);
> +
> + qdev_init_gpio_out(DEVICE(obj), s->out, ARRAY_SIZE(s->out));
> + memory_region_init_io(&s->mmio, obj, &avr_gpio_ops, s, TYPE_AVR_GPIO, 3);
> + sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->mmio);
> +}
> +static void avr_gpio_realize(DeviceState *dev, Error **errp)
> +{
> + /* Do nothing currently */
> +}
> +
> +
> +static void avr_gpio_class_init(ObjectClass *klass, void *data)
> +{
> + DeviceClass *dc = DEVICE_CLASS(klass);
> +
> + dc->reset = avr_gpio_reset;
> + dc->realize = avr_gpio_realize;
> +}
> +
> +static const TypeInfo avr_gpio_info = {
> + .name = TYPE_AVR_GPIO,
> + .parent = TYPE_SYS_BUS_DEVICE,
> + .instance_size = sizeof(AVRGPIOState),
> + .instance_init = avr_gpio_init,
> + .class_init = avr_gpio_class_init,
> +};
> +
> +static void avr_gpio_register_types(void)
> +{
> + type_register_static(&avr_gpio_info);
> +}
> +
> +type_init(avr_gpio_register_types)
> diff --git a/hw/avr/Kconfig b/hw/avr/Kconfig
> index d31298c3cce..16a57ced11f 100644
> --- a/hw/avr/Kconfig
> +++ b/hw/avr/Kconfig
> @@ -3,6 +3,7 @@ config AVR_ATMEGA_MCU
> select AVR_TIMER16
> select AVR_USART
> select AVR_POWER
> + select AVR_GPIO
>
> config ARDUINO
> select AVR_ATMEGA_MCU
> diff --git a/hw/gpio/Kconfig b/hw/gpio/Kconfig
> index f0e7405f6e6..fde7019b2ba 100644
> --- a/hw/gpio/Kconfig
> +++ b/hw/gpio/Kconfig
> @@ -13,3 +13,6 @@ config GPIO_PWR
>
> config SIFIVE_GPIO
> bool
> +
> +config AVR_GPIO
> + bool
> diff --git a/hw/gpio/meson.build b/hw/gpio/meson.build
> index 79568f00ce3..366aca52ca2 100644
> --- a/hw/gpio/meson.build
> +++ b/hw/gpio/meson.build
> @@ -13,3 +13,4 @@
> softmmu_ss.add(when: 'CONFIG_RASPI', if_true: files('bcm2835_gpio.c'))
> softmmu_ss.add(when: 'CONFIG_ASPEED_SOC', if_true: files('aspeed_gpio.c'))
> softmmu_ss.add(when: 'CONFIG_SIFIVE_GPIO', if_true: files('sifive_gpio.c'))
> +softmmu_ss.add(when: 'CONFIG_AVR_GPIO', if_true: files('avr_gpio.c'))
Not that I can see GPL 4 coming on the horizon, but given there was a large exercise
a while back to clarify the licensing of all the files in the QEMU tree it is worth
someone who knows more about licensing to check this.
ATB,
Mark.
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 03/11] hw/avr: Add limited support for avr gpio registers
2021-03-14 10:26 ` Mark Cave-Ayland
@ 2021-03-14 23:41 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 30+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-03-14 23:41 UTC (permalink / raw)
To: Mark Cave-Ayland, qemu-devel, Heecheol Yang
Cc: Thomas Huth, Sarah Harris, Michael Rolnik
On 3/14/21 11:26 AM, Mark Cave-Ayland wrote:
> On 13/03/2021 16:54, Philippe Mathieu-Daudé wrote:
>
>> From: Heecheol Yang <heecheol.yang@outlook.com>
>>
>> Add some of these features for AVR GPIO:
>>
>> - GPIO I/O : PORTx registers
>> - Data Direction : DDRx registers
>> - DDRx toggling : PINx registers
>>
>> Following things are not supported yet:
>> - MCUR registers
>>
>> Signed-off-by: Heecheol Yang <heecheol.yang@outlook.com>
>> Reviewed-by: Michael Rolnik <mrolnik@gmail.com>
>> Message-Id:
>> <DM6PR16MB247368DBD3447ABECDD795D7E6090@DM6PR16MB2473.namprd16.prod.outlook.com>
>>
>> [PMD: Use AVR_GPIO_COUNT]
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>> hw/avr/atmega.h | 2 +
>> include/hw/gpio/avr_gpio.h | 53 ++++++++++++++
>> hw/avr/atmega.c | 7 +-
>> hw/gpio/avr_gpio.c | 138 +++++++++++++++++++++++++++++++++++++
>> hw/avr/Kconfig | 1 +
>> hw/gpio/Kconfig | 3 +
>> hw/gpio/meson.build | 1 +
>> 7 files changed, 203 insertions(+), 2 deletions(-)
>> create mode 100644 include/hw/gpio/avr_gpio.h
>> create mode 100644 hw/gpio/avr_gpio.c
>>
>> diff --git a/hw/avr/atmega.h b/hw/avr/atmega.h
>> index a99ee15c7e1..e2289d5744e 100644
>> --- a/hw/avr/atmega.h
>> +++ b/hw/avr/atmega.h
>> @@ -13,6 +13,7 @@
>> #include "hw/char/avr_usart.h"
>> #include "hw/timer/avr_timer16.h"
>> +#include "hw/gpio/avr_gpio.h"
>> #include "hw/misc/avr_power.h"
>> #include "target/avr/cpu.h"
>> #include "qom/object.h"
>> @@ -44,6 +45,7 @@ struct AtmegaMcuState {
>> DeviceState *io;
>> AVRMaskState pwr[POWER_MAX];
>> AVRUsartState usart[USART_MAX];
>> + AVRGPIOState gpio[GPIO_MAX];
>> AVRTimer16State timer[TIMER_MAX];
>> uint64_t xtal_freq_hz;
>> };
>> diff --git a/include/hw/gpio/avr_gpio.h b/include/hw/gpio/avr_gpio.h
>> new file mode 100644
>> index 00000000000..498a7275f05
>> --- /dev/null
>> +++ b/include/hw/gpio/avr_gpio.h
>> @@ -0,0 +1,53 @@
>> +/*
>> + * AVR processors GPIO registers definition.
>> + *
>> + * Copyright (C) 2020 Heecheol Yang <heecheol.yang@outlook.com>
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public License as
>> + * published by the Free Software Foundation; either version 2 or
>> + * (at your option) version 3 of the License.
>
> Minor nit: isn't the wording of the existing QEMU license "licensed
> under the terms of the GNU GPL, version 2 or later" rather than
> explicitly stating versions 2 and 3?
Good point.
Heecheol Yang, do you mind resending your patch with the correct
license please?
>
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> along
>> + * with this program; if not, see <http://www.gnu.org/licenses/>.
>> + */
>> +
>> +#ifndef AVR_GPIO_H
>> +#define AVR_GPIO_H
>> +
>> +#include "hw/sysbus.h"
>> +#include "qom/object.h"
>> +
>> +/* Offsets of registers. */
>> +#define GPIO_PIN 0x00
>> +#define GPIO_DDR 0x01
>> +#define GPIO_PORT 0x02
>> +
>> +#define TYPE_AVR_GPIO "avr-gpio"
>> +OBJECT_DECLARE_SIMPLE_TYPE(AVRGPIOState, AVR_GPIO)
>> +#define AVR_GPIO_COUNT 8
>> +
>> +struct AVRGPIOState {
>> + /*< private >*/
>> + SysBusDevice parent_obj;
>> +
>> + /*< public >*/
>> + MemoryRegion mmio;
>> +
>> + struct {
>> + uint8_t pin;
>> + uint8_t ddr;
>> + uint8_t port;
>> + } reg;
>> +
>> + /* PORTx data changed IRQs */
>> + qemu_irq out[8u];
>> +
>> +};
>> +
>> +#endif /* AVR_GPIO_H */
>> diff --git a/hw/avr/atmega.c b/hw/avr/atmega.c
>> index 44c6afebbb6..19c3122189f 100644
>> --- a/hw/avr/atmega.c
>> +++ b/hw/avr/atmega.c
>> @@ -283,8 +283,11 @@ static void atmega_realize(DeviceState *dev,
>> Error **errp)
>> continue;
>> }
>> devname = g_strdup_printf("atmega-gpio-%c", 'a' + (char)i);
>> - create_unimplemented_device(devname,
>> - OFFSET_DATA + mc->dev[idx].addr, 3);
>> + object_initialize_child(OBJECT(dev), devname, &s->gpio[i],
>> + TYPE_AVR_GPIO);
>> + sysbus_realize(SYS_BUS_DEVICE(&s->gpio[i]), &error_abort);
>> + sysbus_mmio_map(SYS_BUS_DEVICE(&s->gpio[i]), 0,
>> + OFFSET_DATA + mc->dev[idx].addr);
>> g_free(devname);
>> }
>> diff --git a/hw/gpio/avr_gpio.c b/hw/gpio/avr_gpio.c
>> new file mode 100644
>> index 00000000000..cdb574ef0d8
>> --- /dev/null
>> +++ b/hw/gpio/avr_gpio.c
>> @@ -0,0 +1,138 @@
>> +/*
>> + * AVR processors GPIO registers emulation.
>> + *
>> + * Copyright (C) 2020 Heecheol Yang <heecheol.yang@outlook.com>
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public License as
>> + * published by the Free Software Foundation; either version 2 or
>> + * (at your option) version 3 of the License.
>
> And here too.
>
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> along
>> + * with this program; if not, see <http://www.gnu.org/licenses/>.
>> + */
>> +#include "qemu/osdep.h"
>> +#include "qemu/log.h"
>> +#include "qemu/module.h"
>> +#include "qemu/osdep.h"
>> +#include "qapi/error.h"
>> +#include "hw/sysbus.h"
>> +#include "hw/irq.h"
>> +#include "hw/gpio/avr_gpio.h"
>> +#include "hw/qdev-properties.h"
>> +
>> +static void avr_gpio_reset(DeviceState *dev)
>> +{
>> + AVRGPIOState *gpio = AVR_GPIO(dev);
>> +
>> + gpio->reg.pin = 0u;
>> + gpio->reg.ddr = 0u;
>> + gpio->reg.port = 0u;
>> +}
>> +
>> +static void avr_gpio_write_port(AVRGPIOState *s, uint64_t value)
>> +{
>> + uint8_t pin;
>> + uint8_t cur_port_val = s->reg.port;
>> + uint8_t cur_ddr_val = s->reg.ddr;
>> +
>> + for (pin = 0u; pin < AVR_GPIO_COUNT ; pin++) {
>> + uint8_t cur_port_pin_val = cur_port_val & 0x01u;
>> + uint8_t cur_ddr_pin_val = cur_ddr_val & 0x01u;
>> + uint8_t new_port_pin_val = value & 0x01u;
>> +
>> + if (cur_ddr_pin_val && (cur_port_pin_val != new_port_pin_val)) {
>> + qemu_set_irq(s->out[pin], new_port_pin_val);
>> + }
>> + cur_port_val >>= 1u;
>> + cur_ddr_val >>= 1u;
>> + value >>= 1u;
>> + }
>> + s->reg.port = value & s->reg.ddr;
>> +}
>> +static uint64_t avr_gpio_read(void *opaque, hwaddr offset, unsigned
>> int size)
>> +{
>> + AVRGPIOState *s = (AVRGPIOState *)opaque;
>> + switch (offset) {
>> + case GPIO_PIN:
>> + return s->reg.pin;
>> + case GPIO_DDR:
>> + return s->reg.ddr;
>> + case GPIO_PORT:
>> + return s->reg.port;
>> + default:
>> + g_assert_not_reached();
>> + break;
>> + }
>> + return 0;
>> +}
>> +
>> +static void avr_gpio_write(void *opaque, hwaddr offset, uint64_t value,
>> + unsigned int size)
>> +{
>> + AVRGPIOState *s = (AVRGPIOState *)opaque;
>> + value = value & 0xF;
>> + switch (offset) {
>> + case GPIO_PIN:
>> + s->reg.pin = value;
>> + s->reg.port ^= s->reg.pin;
>> + break;
>> + case GPIO_DDR:
>> + s->reg.ddr = value;
>> + break;
>> + case GPIO_PORT:
>> + avr_gpio_write_port(s, value);
>> + break;
>> + default:
>> + g_assert_not_reached();
>> + break;
>> + }
>> +}
>> +
>> +static const MemoryRegionOps avr_gpio_ops = {
>> + .read = avr_gpio_read,
>> + .write = avr_gpio_write,
>> + .endianness = DEVICE_NATIVE_ENDIAN,
>> +};
>> +
>> +static void avr_gpio_init(Object *obj)
>> +{
>> + AVRGPIOState *s = AVR_GPIO(obj);
>> +
>> + qdev_init_gpio_out(DEVICE(obj), s->out, ARRAY_SIZE(s->out));
>> + memory_region_init_io(&s->mmio, obj, &avr_gpio_ops, s,
>> TYPE_AVR_GPIO, 3);
>> + sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->mmio);
>> +}
>> +static void avr_gpio_realize(DeviceState *dev, Error **errp)
>> +{
>> + /* Do nothing currently */
>> +}
>> +
>> +
>> +static void avr_gpio_class_init(ObjectClass *klass, void *data)
>> +{
>> + DeviceClass *dc = DEVICE_CLASS(klass);
>> +
>> + dc->reset = avr_gpio_reset;
>> + dc->realize = avr_gpio_realize;
>> +}
>> +
>> +static const TypeInfo avr_gpio_info = {
>> + .name = TYPE_AVR_GPIO,
>> + .parent = TYPE_SYS_BUS_DEVICE,
>> + .instance_size = sizeof(AVRGPIOState),
>> + .instance_init = avr_gpio_init,
>> + .class_init = avr_gpio_class_init,
>> +};
>> +
>> +static void avr_gpio_register_types(void)
>> +{
>> + type_register_static(&avr_gpio_info);
>> +}
>> +
>> +type_init(avr_gpio_register_types)
>> diff --git a/hw/avr/Kconfig b/hw/avr/Kconfig
>> index d31298c3cce..16a57ced11f 100644
>> --- a/hw/avr/Kconfig
>> +++ b/hw/avr/Kconfig
>> @@ -3,6 +3,7 @@ config AVR_ATMEGA_MCU
>> select AVR_TIMER16
>> select AVR_USART
>> select AVR_POWER
>> + select AVR_GPIO
>> config ARDUINO
>> select AVR_ATMEGA_MCU
>> diff --git a/hw/gpio/Kconfig b/hw/gpio/Kconfig
>> index f0e7405f6e6..fde7019b2ba 100644
>> --- a/hw/gpio/Kconfig
>> +++ b/hw/gpio/Kconfig
>> @@ -13,3 +13,6 @@ config GPIO_PWR
>> config SIFIVE_GPIO
>> bool
>> +
>> +config AVR_GPIO
>> + bool
>> diff --git a/hw/gpio/meson.build b/hw/gpio/meson.build
>> index 79568f00ce3..366aca52ca2 100644
>> --- a/hw/gpio/meson.build
>> +++ b/hw/gpio/meson.build
>> @@ -13,3 +13,4 @@
>> softmmu_ss.add(when: 'CONFIG_RASPI', if_true: files('bcm2835_gpio.c'))
>> softmmu_ss.add(when: 'CONFIG_ASPEED_SOC', if_true:
>> files('aspeed_gpio.c'))
>> softmmu_ss.add(when: 'CONFIG_SIFIVE_GPIO', if_true:
>> files('sifive_gpio.c'))
>> +softmmu_ss.add(when: 'CONFIG_AVR_GPIO', if_true: files('avr_gpio.c'))
>
> Not that I can see GPL 4 coming on the horizon, but given there was a
> large exercise a while back to clarify the licensing of all the files in
> the QEMU tree it is worth someone who knows more about licensing to
> check this.
>
>
> ATB,
>
> Mark.
>
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 03/11] hw/avr: Add limited support for avr gpio registers
2021-03-13 16:54 ` [PATCH 03/11] hw/avr: Add limited support for avr gpio registers Philippe Mathieu-Daudé
2021-03-13 19:55 ` Richard Henderson
2021-03-14 10:26 ` Mark Cave-Ayland
@ 2021-03-15 13:15 ` Sarah Harris
2 siblings, 0 replies; 30+ messages in thread
From: Sarah Harris @ 2021-03-15 13:15 UTC (permalink / raw)
To: qemu-devel
Cc: Thomas Huth, Sarah Harris, Michael Rolnik,
Philippe Mathieu-Daudé,
Heecheol Yang
On Sat, 13 Mar 2021 17:54:37 +0100
Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> From: Heecheol Yang <heecheol.yang@outlook.com>
>
> Add some of these features for AVR GPIO:
>
> - GPIO I/O : PORTx registers
> - Data Direction : DDRx registers
> - DDRx toggling : PINx registers
>
> Following things are not supported yet:
> - MCUR registers
>
> Signed-off-by: Heecheol Yang <heecheol.yang@outlook.com>
> Reviewed-by: Michael Rolnik <mrolnik@gmail.com>
> Message-Id: <DM6PR16MB247368DBD3447ABECDD795D7E6090@DM6PR16MB2473.namprd16.prod.outlook.com>
> [PMD: Use AVR_GPIO_COUNT]
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> hw/avr/atmega.h | 2 +
> include/hw/gpio/avr_gpio.h | 53 ++++++++++++++
> hw/avr/atmega.c | 7 +-
> hw/gpio/avr_gpio.c | 138 +++++++++++++++++++++++++++++++++++++
> hw/avr/Kconfig | 1 +
> hw/gpio/Kconfig | 3 +
> hw/gpio/meson.build | 1 +
> 7 files changed, 203 insertions(+), 2 deletions(-)
> create mode 100644 include/hw/gpio/avr_gpio.h
> create mode 100644 hw/gpio/avr_gpio.c
>
> diff --git a/hw/avr/atmega.h b/hw/avr/atmega.h
> index a99ee15c7e1..e2289d5744e 100644
> --- a/hw/avr/atmega.h
> +++ b/hw/avr/atmega.h
> @@ -13,6 +13,7 @@
>
> #include "hw/char/avr_usart.h"
> #include "hw/timer/avr_timer16.h"
> +#include "hw/gpio/avr_gpio.h"
> #include "hw/misc/avr_power.h"
> #include "target/avr/cpu.h"
> #include "qom/object.h"
> @@ -44,6 +45,7 @@ struct AtmegaMcuState {
> DeviceState *io;
> AVRMaskState pwr[POWER_MAX];
> AVRUsartState usart[USART_MAX];
> + AVRGPIOState gpio[GPIO_MAX];
> AVRTimer16State timer[TIMER_MAX];
> uint64_t xtal_freq_hz;
> };
> diff --git a/include/hw/gpio/avr_gpio.h b/include/hw/gpio/avr_gpio.h
> new file mode 100644
> index 00000000000..498a7275f05
> --- /dev/null
> +++ b/include/hw/gpio/avr_gpio.h
> @@ -0,0 +1,53 @@
> +/*
> + * AVR processors GPIO registers definition.
> + *
> + * Copyright (C) 2020 Heecheol Yang <heecheol.yang@outlook.com>
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation; either version 2 or
> + * (at your option) version 3 of the License.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License along
> + * with this program; if not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#ifndef AVR_GPIO_H
> +#define AVR_GPIO_H
> +
> +#include "hw/sysbus.h"
> +#include "qom/object.h"
> +
> +/* Offsets of registers. */
> +#define GPIO_PIN 0x00
> +#define GPIO_DDR 0x01
> +#define GPIO_PORT 0x02
> +
> +#define TYPE_AVR_GPIO "avr-gpio"
> +OBJECT_DECLARE_SIMPLE_TYPE(AVRGPIOState, AVR_GPIO)
> +#define AVR_GPIO_COUNT 8
> +
> +struct AVRGPIOState {
> + /*< private >*/
> + SysBusDevice parent_obj;
> +
> + /*< public >*/
> + MemoryRegion mmio;
> +
> + struct {
> + uint8_t pin;
> + uint8_t ddr;
> + uint8_t port;
> + } reg;
> +
> + /* PORTx data changed IRQs */
> + qemu_irq out[8u];
> +
> +};
> +
> +#endif /* AVR_GPIO_H */
> diff --git a/hw/avr/atmega.c b/hw/avr/atmega.c
> index 44c6afebbb6..19c3122189f 100644
> --- a/hw/avr/atmega.c
> +++ b/hw/avr/atmega.c
> @@ -283,8 +283,11 @@ static void atmega_realize(DeviceState *dev, Error **errp)
> continue;
> }
> devname = g_strdup_printf("atmega-gpio-%c", 'a' + (char)i);
> - create_unimplemented_device(devname,
> - OFFSET_DATA + mc->dev[idx].addr, 3);
> + object_initialize_child(OBJECT(dev), devname, &s->gpio[i],
> + TYPE_AVR_GPIO);
> + sysbus_realize(SYS_BUS_DEVICE(&s->gpio[i]), &error_abort);
> + sysbus_mmio_map(SYS_BUS_DEVICE(&s->gpio[i]), 0,
> + OFFSET_DATA + mc->dev[idx].addr);
> g_free(devname);
> }
>
> diff --git a/hw/gpio/avr_gpio.c b/hw/gpio/avr_gpio.c
> new file mode 100644
> index 00000000000..cdb574ef0d8
> --- /dev/null
> +++ b/hw/gpio/avr_gpio.c
> @@ -0,0 +1,138 @@
> +/*
> + * AVR processors GPIO registers emulation.
> + *
> + * Copyright (C) 2020 Heecheol Yang <heecheol.yang@outlook.com>
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation; either version 2 or
> + * (at your option) version 3 of the License.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License along
> + * with this program; if not, see <http://www.gnu.org/licenses/>.
> + */
> +#include "qemu/osdep.h"
> +#include "qemu/log.h"
> +#include "qemu/module.h"
> +#include "qemu/osdep.h"
> +#include "qapi/error.h"
> +#include "hw/sysbus.h"
> +#include "hw/irq.h"
> +#include "hw/gpio/avr_gpio.h"
> +#include "hw/qdev-properties.h"
> +
> +static void avr_gpio_reset(DeviceState *dev)
> +{
> + AVRGPIOState *gpio = AVR_GPIO(dev);
> +
> + gpio->reg.pin = 0u;
> + gpio->reg.ddr = 0u;
> + gpio->reg.port = 0u;
> +}
> +
> +static void avr_gpio_write_port(AVRGPIOState *s, uint64_t value)
> +{
> + uint8_t pin;
> + uint8_t cur_port_val = s->reg.port;
> + uint8_t cur_ddr_val = s->reg.ddr;
> +
> + for (pin = 0u; pin < AVR_GPIO_COUNT ; pin++) {
> + uint8_t cur_port_pin_val = cur_port_val & 0x01u;
> + uint8_t cur_ddr_pin_val = cur_ddr_val & 0x01u;
> + uint8_t new_port_pin_val = value & 0x01u;
> +
> + if (cur_ddr_pin_val && (cur_port_pin_val != new_port_pin_val)) {
> + qemu_set_irq(s->out[pin], new_port_pin_val);
> + }
> + cur_port_val >>= 1u;
> + cur_ddr_val >>= 1u;
> + value >>= 1u;
> + }
> + s->reg.port = value & s->reg.ddr;
> +}
Unless I've misunderstood something about how this is designed to be used, I think the logic here and below is slightly wrong.
PORT should be set to the value written, regardless of the state of DDR.
From re-reading the ATmega2560 datasheet (http://ww1.microchip.com/downloads/en/DeviceDoc/Atmel-2549-8-bit-AVR-Microcontroller-ATmega640-1280-1281-2560-2561_datasheet.pdf) description of the GPIOs (section 13):
PORT is defined to be read/write in 13.1 and the diagram in 13.2 (by my understanding) indicates that reading PORT gives the state last written.
Reinforcing this, zero bits in DDR indicate a pin in input mode (13.2.3), in which case the corresponding bit in PORT controls the pull up resistor and shouldn't be discarded.
This can be fixed by: s->reg.port = value;
> +static uint64_t avr_gpio_read(void *opaque, hwaddr offset, unsigned int size)
> +{
> + AVRGPIOState *s = (AVRGPIOState *)opaque;
> + switch (offset) {
> + case GPIO_PIN:
> + return s->reg.pin;
> + case GPIO_DDR:
> + return s->reg.ddr;
> + case GPIO_PORT:
> + return s->reg.port;
> + default:
> + g_assert_not_reached();
> + break;
> + }
> + return 0;
> +}
> +
> +static void avr_gpio_write(void *opaque, hwaddr offset, uint64_t value,
> + unsigned int size)
> +{
> + AVRGPIOState *s = (AVRGPIOState *)opaque;
> + value = value & 0xF;
> + switch (offset) {
> + case GPIO_PIN:
> + s->reg.pin = value;
> + s->reg.port ^= s->reg.pin;
> + break;
> + case GPIO_DDR:
> + s->reg.ddr = value;
> + break;
> + case GPIO_PORT:
> + avr_gpio_write_port(s, value);
> + break;
> + default:
> + g_assert_not_reached();
> + break;
> + }
> +}
Again, I think the logic here isn't quite right.
Writing ones to PIN toggles the corresponding bits in PORT (section 13.2.2), but shouldn't change the state of PIN itself because it's read only (section 13.1)
Judging by the diagram in 13.2, PIN is controlled only by the voltage on the pin itself (and synchronisation logic), confirming that it shouldn't be modified here when writes to PORT don't change it.
This code also won't generate pin change interrupts for pins toggled via PIN that would generate interrupts if toggled via PORT.
This can be fixed by:
case GPIO_PIN:
avr_gpio_write_port(s, s->reg.port^value);
break;
Also, note that I don't think masking the value with 0xF is right (should be 0xFF), though this is already fixed by a later patch in this series.
Given that this patch generates pin change interrupts when pins states are written, which I assume only happens in hardware due to the voltage on the pin changing, it might actually be arguable that the value read from PIN should always be equal to PORT (i.e. pins set output-high or input-pull-up read high).
> +
> +static const MemoryRegionOps avr_gpio_ops = {
> + .read = avr_gpio_read,
> + .write = avr_gpio_write,
> + .endianness = DEVICE_NATIVE_ENDIAN,
> +};
> +
> +static void avr_gpio_init(Object *obj)
> +{
> + AVRGPIOState *s = AVR_GPIO(obj);
> +
> + qdev_init_gpio_out(DEVICE(obj), s->out, ARRAY_SIZE(s->out));
> + memory_region_init_io(&s->mmio, obj, &avr_gpio_ops, s, TYPE_AVR_GPIO, 3);
> + sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->mmio);
> +}
> +static void avr_gpio_realize(DeviceState *dev, Error **errp)
> +{
> + /* Do nothing currently */
> +}
> +
> +
> +static void avr_gpio_class_init(ObjectClass *klass, void *data)
> +{
> + DeviceClass *dc = DEVICE_CLASS(klass);
> +
> + dc->reset = avr_gpio_reset;
> + dc->realize = avr_gpio_realize;
> +}
> +
> +static const TypeInfo avr_gpio_info = {
> + .name = TYPE_AVR_GPIO,
> + .parent = TYPE_SYS_BUS_DEVICE,
> + .instance_size = sizeof(AVRGPIOState),
> + .instance_init = avr_gpio_init,
> + .class_init = avr_gpio_class_init,
> +};
> +
> +static void avr_gpio_register_types(void)
> +{
> + type_register_static(&avr_gpio_info);
> +}
> +
> +type_init(avr_gpio_register_types)
> diff --git a/hw/avr/Kconfig b/hw/avr/Kconfig
> index d31298c3cce..16a57ced11f 100644
> --- a/hw/avr/Kconfig
> +++ b/hw/avr/Kconfig
> @@ -3,6 +3,7 @@ config AVR_ATMEGA_MCU
> select AVR_TIMER16
> select AVR_USART
> select AVR_POWER
> + select AVR_GPIO
>
> config ARDUINO
> select AVR_ATMEGA_MCU
> diff --git a/hw/gpio/Kconfig b/hw/gpio/Kconfig
> index f0e7405f6e6..fde7019b2ba 100644
> --- a/hw/gpio/Kconfig
> +++ b/hw/gpio/Kconfig
> @@ -13,3 +13,6 @@ config GPIO_PWR
>
> config SIFIVE_GPIO
> bool
> +
> +config AVR_GPIO
> + bool
> diff --git a/hw/gpio/meson.build b/hw/gpio/meson.build
> index 79568f00ce3..366aca52ca2 100644
> --- a/hw/gpio/meson.build
> +++ b/hw/gpio/meson.build
> @@ -13,3 +13,4 @@
> softmmu_ss.add(when: 'CONFIG_RASPI', if_true: files('bcm2835_gpio.c'))
> softmmu_ss.add(when: 'CONFIG_ASPEED_SOC', if_true: files('aspeed_gpio.c'))
> softmmu_ss.add(when: 'CONFIG_SIFIVE_GPIO', if_true: files('sifive_gpio.c'))
> +softmmu_ss.add(when: 'CONFIG_AVR_GPIO', if_true: files('avr_gpio.c'))
> --
> 2.26.2
Regards,
Sarah Harris
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH 04/11] hw/gpio/avr_gpio: Add migration VMstate
2021-03-13 16:54 [PATCH 00/11] AVR patch queue for QEMU 6.0 Philippe Mathieu-Daudé
` (2 preceding siblings ...)
2021-03-13 16:54 ` [PATCH 03/11] hw/avr: Add limited support for avr gpio registers Philippe Mathieu-Daudé
@ 2021-03-13 16:54 ` Philippe Mathieu-Daudé
2021-03-13 19:57 ` Richard Henderson
2021-03-13 16:54 ` [PATCH 05/11] hw/gpio/avr_gpio: Add 'id' field in AVRGPIOState Philippe Mathieu-Daudé
` (8 subsequent siblings)
12 siblings, 1 reply; 30+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-03-13 16:54 UTC (permalink / raw)
To: qemu-devel
Cc: Thomas Huth, Sarah Harris, Michael Rolnik, Philippe Mathieu-Daudé
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
hw/gpio/avr_gpio.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/hw/gpio/avr_gpio.c b/hw/gpio/avr_gpio.c
index cdb574ef0d8..da34009daea 100644
--- a/hw/gpio/avr_gpio.c
+++ b/hw/gpio/avr_gpio.c
@@ -25,6 +25,7 @@
#include "hw/irq.h"
#include "hw/gpio/avr_gpio.h"
#include "hw/qdev-properties.h"
+#include "migration/vmstate.h"
static void avr_gpio_reset(DeviceState *dev)
{
@@ -100,6 +101,18 @@ static const MemoryRegionOps avr_gpio_ops = {
.endianness = DEVICE_NATIVE_ENDIAN,
};
+static const VMStateDescription avr_gpio_vmstate = {
+ .name = "avr-gpio",
+ .version_id = 0,
+ .minimum_version_id = 0,
+ .fields = (VMStateField[]) {
+ VMSTATE_UINT8(reg.pin, AVRGPIOState),
+ VMSTATE_UINT8(reg.ddr, AVRGPIOState),
+ VMSTATE_UINT8(reg.port, AVRGPIOState),
+ VMSTATE_END_OF_LIST(),
+ },
+};
+
static void avr_gpio_init(Object *obj)
{
AVRGPIOState *s = AVR_GPIO(obj);
@@ -120,6 +133,7 @@ static void avr_gpio_class_init(ObjectClass *klass, void *data)
dc->reset = avr_gpio_reset;
dc->realize = avr_gpio_realize;
+ dc->vmsd = &avr_gpio_vmstate;
}
static const TypeInfo avr_gpio_info = {
--
2.26.2
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH 05/11] hw/gpio/avr_gpio: Add 'id' field in AVRGPIOState
2021-03-13 16:54 [PATCH 00/11] AVR patch queue for QEMU 6.0 Philippe Mathieu-Daudé
` (3 preceding siblings ...)
2021-03-13 16:54 ` [PATCH 04/11] hw/gpio/avr_gpio: Add migration VMstate Philippe Mathieu-Daudé
@ 2021-03-13 16:54 ` Philippe Mathieu-Daudé
2021-03-13 19:58 ` Richard Henderson
2021-03-13 16:54 ` [PATCH 06/11] hw/gpio/avr_gpio: Simplify avr_gpio_write_port using extract32() Philippe Mathieu-Daudé
` (7 subsequent siblings)
12 siblings, 1 reply; 30+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-03-13 16:54 UTC (permalink / raw)
To: qemu-devel
Cc: Thomas Huth, Sarah Harris, Michael Rolnik, Philippe Mathieu-Daudé
AVR MCU have various GPIO ports. Add an 'id' property to distinct
all instances.
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
include/hw/gpio/avr_gpio.h | 1 +
hw/avr/atmega.c | 1 +
hw/gpio/avr_gpio.c | 14 ++++++++++++--
3 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/include/hw/gpio/avr_gpio.h b/include/hw/gpio/avr_gpio.h
index 498a7275f05..e982f627ea3 100644
--- a/include/hw/gpio/avr_gpio.h
+++ b/include/hw/gpio/avr_gpio.h
@@ -48,6 +48,7 @@ struct AVRGPIOState {
/* PORTx data changed IRQs */
qemu_irq out[8u];
+ uint8_t id;
};
#endif /* AVR_GPIO_H */
diff --git a/hw/avr/atmega.c b/hw/avr/atmega.c
index 19c3122189f..a19e3035022 100644
--- a/hw/avr/atmega.c
+++ b/hw/avr/atmega.c
@@ -285,6 +285,7 @@ static void atmega_realize(DeviceState *dev, Error **errp)
devname = g_strdup_printf("atmega-gpio-%c", 'a' + (char)i);
object_initialize_child(OBJECT(dev), devname, &s->gpio[i],
TYPE_AVR_GPIO);
+ qdev_prop_set_uint8(DEVICE(&s->gpio[i]), "id", i);
sysbus_realize(SYS_BUS_DEVICE(&s->gpio[i]), &error_abort);
sysbus_mmio_map(SYS_BUS_DEVICE(&s->gpio[i]), 0,
OFFSET_DATA + mc->dev[idx].addr);
diff --git a/hw/gpio/avr_gpio.c b/hw/gpio/avr_gpio.c
index da34009daea..3db55bfa77f 100644
--- a/hw/gpio/avr_gpio.c
+++ b/hw/gpio/avr_gpio.c
@@ -113,6 +113,11 @@ static const VMStateDescription avr_gpio_vmstate = {
},
};
+static Property avr_gpio_properties[] = {
+ DEFINE_PROP_UINT8("id", AVRGPIOState, id, UINT8_MAX),
+ DEFINE_PROP_END_OF_LIST(),
+};
+
static void avr_gpio_init(Object *obj)
{
AVRGPIOState *s = AVR_GPIO(obj);
@@ -123,9 +128,13 @@ static void avr_gpio_init(Object *obj)
}
static void avr_gpio_realize(DeviceState *dev, Error **errp)
{
- /* Do nothing currently */
-}
+ AVRGPIOState *s = AVR_GPIO(dev);
+ if (s->id == UINT8_MAX) {
+ error_setg(errp, "property 'id' not set");
+ return;
+ }
+}
static void avr_gpio_class_init(ObjectClass *klass, void *data)
{
@@ -134,6 +143,7 @@ static void avr_gpio_class_init(ObjectClass *klass, void *data)
dc->reset = avr_gpio_reset;
dc->realize = avr_gpio_realize;
dc->vmsd = &avr_gpio_vmstate;
+ device_class_set_props(dc, avr_gpio_properties);
}
static const TypeInfo avr_gpio_info = {
--
2.26.2
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH 06/11] hw/gpio/avr_gpio: Simplify avr_gpio_write_port using extract32()
2021-03-13 16:54 [PATCH 00/11] AVR patch queue for QEMU 6.0 Philippe Mathieu-Daudé
` (4 preceding siblings ...)
2021-03-13 16:54 ` [PATCH 05/11] hw/gpio/avr_gpio: Add 'id' field in AVRGPIOState Philippe Mathieu-Daudé
@ 2021-03-13 16:54 ` Philippe Mathieu-Daudé
2021-03-13 19:59 ` Richard Henderson
2021-03-13 16:54 ` [PATCH 07/11] hw/gpio/avr_gpio: Add tracing for reads and writes Philippe Mathieu-Daudé
` (6 subsequent siblings)
12 siblings, 1 reply; 30+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-03-13 16:54 UTC (permalink / raw)
To: qemu-devel
Cc: Thomas Huth, Sarah Harris, Michael Rolnik, Philippe Mathieu-Daudé
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
hw/gpio/avr_gpio.c | 11 +++--------
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/hw/gpio/avr_gpio.c b/hw/gpio/avr_gpio.c
index 3db55bfa77f..e4c7122e62c 100644
--- a/hw/gpio/avr_gpio.c
+++ b/hw/gpio/avr_gpio.c
@@ -39,20 +39,15 @@ static void avr_gpio_reset(DeviceState *dev)
static void avr_gpio_write_port(AVRGPIOState *s, uint64_t value)
{
uint8_t pin;
- uint8_t cur_port_val = s->reg.port;
- uint8_t cur_ddr_val = s->reg.ddr;
for (pin = 0u; pin < AVR_GPIO_COUNT ; pin++) {
- uint8_t cur_port_pin_val = cur_port_val & 0x01u;
- uint8_t cur_ddr_pin_val = cur_ddr_val & 0x01u;
- uint8_t new_port_pin_val = value & 0x01u;
+ uint8_t cur_port_pin_val = extract32(s->reg.port, pin, 1);
+ uint8_t cur_ddr_pin_val = extract32(s->reg.ddr, pin, 1);
+ uint8_t new_port_pin_val = extract32(value, pin, 1);
if (cur_ddr_pin_val && (cur_port_pin_val != new_port_pin_val)) {
qemu_set_irq(s->out[pin], new_port_pin_val);
}
- cur_port_val >>= 1u;
- cur_ddr_val >>= 1u;
- value >>= 1u;
}
s->reg.port = value & s->reg.ddr;
}
--
2.26.2
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH 07/11] hw/gpio/avr_gpio: Add tracing for reads and writes
2021-03-13 16:54 [PATCH 00/11] AVR patch queue for QEMU 6.0 Philippe Mathieu-Daudé
` (5 preceding siblings ...)
2021-03-13 16:54 ` [PATCH 06/11] hw/gpio/avr_gpio: Simplify avr_gpio_write_port using extract32() Philippe Mathieu-Daudé
@ 2021-03-13 16:54 ` Philippe Mathieu-Daudé
2021-03-13 17:21 ` Niteesh G. S.
2021-03-13 16:54 ` [PATCH 08/11] hw/avr/arduino: Add D13 LED Philippe Mathieu-Daudé
` (5 subsequent siblings)
12 siblings, 1 reply; 30+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-03-13 16:54 UTC (permalink / raw)
To: qemu-devel
Cc: Thomas Huth, Sarah Harris, Michael Rolnik,
Philippe Mathieu-Daudé,
G S Niteesh Babu
From: G S Niteesh Babu <niteesh.gs@gmail.com>
Added tracing for gpio read, write, and update output irq.
1) trace_avr_gpio_update_ouput_irq
2) trace_avr_gpio_read
3) trace_avr_gpio_write
Signed-off-by: G S Niteesh Babu <niteesh.gs@gmail.com>
Reviewed-by: Michael Rolnik <mrolnik@gmail.com>
Message-Id: <20210311135539.10206-3-niteesh.gs@gmail.com>
[PMD: Added port_name(), display port name in trace events]
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
hw/gpio/avr_gpio.c | 26 +++++++++++++++++++++-----
hw/gpio/trace-events | 5 +++++
2 files changed, 26 insertions(+), 5 deletions(-)
diff --git a/hw/gpio/avr_gpio.c b/hw/gpio/avr_gpio.c
index e4c7122e62c..29252d6ccfe 100644
--- a/hw/gpio/avr_gpio.c
+++ b/hw/gpio/avr_gpio.c
@@ -2,6 +2,7 @@
* AVR processors GPIO registers emulation.
*
* Copyright (C) 2020 Heecheol Yang <heecheol.yang@outlook.com>
+ * Copyright (C) 2021 Niteesh Babu G S <niteesh.gs@gmail.com>
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU General Public License as
@@ -26,6 +27,12 @@
#include "hw/gpio/avr_gpio.h"
#include "hw/qdev-properties.h"
#include "migration/vmstate.h"
+#include "trace.h"
+
+static char port_name(AVRGPIOState *s)
+{
+ return 'A' + s->id;
+}
static void avr_gpio_reset(DeviceState *dev)
{
@@ -47,32 +54,41 @@ static void avr_gpio_write_port(AVRGPIOState *s, uint64_t value)
if (cur_ddr_pin_val && (cur_port_pin_val != new_port_pin_val)) {
qemu_set_irq(s->out[pin], new_port_pin_val);
+ trace_avr_gpio_update_output_irq(port_name(s), pin, new_port_pin_val);
}
}
s->reg.port = value & s->reg.ddr;
}
static uint64_t avr_gpio_read(void *opaque, hwaddr offset, unsigned int size)
{
+ uint8_t val = 0;
AVRGPIOState *s = (AVRGPIOState *)opaque;
switch (offset) {
case GPIO_PIN:
- return s->reg.pin;
+ val = s->reg.pin;
+ break;
case GPIO_DDR:
- return s->reg.ddr;
+ val = s->reg.ddr;
+ break;
case GPIO_PORT:
- return s->reg.port;
+ val = s->reg.port;
+ break;
default:
g_assert_not_reached();
break;
}
- return 0;
+
+ trace_avr_gpio_read(port_name(s), offset, val);
+ return val;
}
static void avr_gpio_write(void *opaque, hwaddr offset, uint64_t value,
unsigned int size)
{
AVRGPIOState *s = (AVRGPIOState *)opaque;
- value = value & 0xF;
+ value = value & 0xFF;
+
+ trace_avr_gpio_write(port_name(s), offset, value);
switch (offset) {
case GPIO_PIN:
s->reg.pin = value;
diff --git a/hw/gpio/trace-events b/hw/gpio/trace-events
index 46ab9323bd0..640834597a8 100644
--- a/hw/gpio/trace-events
+++ b/hw/gpio/trace-events
@@ -18,3 +18,8 @@ sifive_gpio_read(uint64_t offset, uint64_t r) "offset 0x%" PRIx64 " value 0x%" P
sifive_gpio_write(uint64_t offset, uint64_t value) "offset 0x%" PRIx64 " value 0x%" PRIx64
sifive_gpio_set(int64_t line, int64_t value) "line %" PRIi64 " value %" PRIi64
sifive_gpio_update_output_irq(int64_t line, int64_t value) "line %" PRIi64 " value %" PRIi64
+
+# avr_gpio.c
+avr_gpio_read(unsigned id, uint64_t offset, uint64_t r) "port %c offset 0x%" PRIx64 " value 0x%" PRIx64
+avr_gpio_write(unsigned id, uint64_t offset, uint64_t value) "port %c offset 0x%" PRIx64 " value 0x%" PRIx64
+avr_gpio_update_output_irq(unsigned id, int64_t line, int64_t value) "port %c pin %" PRIi64 " value %" PRIi64
--
2.26.2
^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH 07/11] hw/gpio/avr_gpio: Add tracing for reads and writes
2021-03-13 16:54 ` [PATCH 07/11] hw/gpio/avr_gpio: Add tracing for reads and writes Philippe Mathieu-Daudé
@ 2021-03-13 17:21 ` Niteesh G. S.
2021-03-23 2:42 ` Niteesh G. S.
0 siblings, 1 reply; 30+ messages in thread
From: Niteesh G. S. @ 2021-03-13 17:21 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Thomas Huth, Sarah Harris, Michael Rolnik, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 4018 bytes --]
Reviewed-by: Niteesh G S <niteesh.gs@gmail.com>
On Sat, Mar 13, 2021 at 10:25 PM Philippe Mathieu-Daudé <f4bug@amsat.org>
wrote:
> From: G S Niteesh Babu <niteesh.gs@gmail.com>
>
> Added tracing for gpio read, write, and update output irq.
>
> 1) trace_avr_gpio_update_ouput_irq
> 2) trace_avr_gpio_read
> 3) trace_avr_gpio_write
>
> Signed-off-by: G S Niteesh Babu <niteesh.gs@gmail.com>
> Reviewed-by: Michael Rolnik <mrolnik@gmail.com>
> Message-Id: <20210311135539.10206-3-niteesh.gs@gmail.com>
> [PMD: Added port_name(), display port name in trace events]
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> hw/gpio/avr_gpio.c | 26 +++++++++++++++++++++-----
> hw/gpio/trace-events | 5 +++++
> 2 files changed, 26 insertions(+), 5 deletions(-)
>
> diff --git a/hw/gpio/avr_gpio.c b/hw/gpio/avr_gpio.c
> index e4c7122e62c..29252d6ccfe 100644
> --- a/hw/gpio/avr_gpio.c
> +++ b/hw/gpio/avr_gpio.c
> @@ -2,6 +2,7 @@
> * AVR processors GPIO registers emulation.
> *
> * Copyright (C) 2020 Heecheol Yang <heecheol.yang@outlook.com>
> + * Copyright (C) 2021 Niteesh Babu G S <niteesh.gs@gmail.com>
> *
> * This program is free software; you can redistribute it and/or
> * modify it under the terms of the GNU General Public License as
> @@ -26,6 +27,12 @@
> #include "hw/gpio/avr_gpio.h"
> #include "hw/qdev-properties.h"
> #include "migration/vmstate.h"
> +#include "trace.h"
> +
> +static char port_name(AVRGPIOState *s)
> +{
> + return 'A' + s->id;
> +}
>
> static void avr_gpio_reset(DeviceState *dev)
> {
> @@ -47,32 +54,41 @@ static void avr_gpio_write_port(AVRGPIOState *s,
> uint64_t value)
>
> if (cur_ddr_pin_val && (cur_port_pin_val != new_port_pin_val)) {
> qemu_set_irq(s->out[pin], new_port_pin_val);
> + trace_avr_gpio_update_output_irq(port_name(s), pin,
> new_port_pin_val);
> }
> }
> s->reg.port = value & s->reg.ddr;
> }
> static uint64_t avr_gpio_read(void *opaque, hwaddr offset, unsigned int
> size)
> {
> + uint8_t val = 0;
> AVRGPIOState *s = (AVRGPIOState *)opaque;
> switch (offset) {
> case GPIO_PIN:
> - return s->reg.pin;
> + val = s->reg.pin;
> + break;
> case GPIO_DDR:
> - return s->reg.ddr;
> + val = s->reg.ddr;
> + break;
> case GPIO_PORT:
> - return s->reg.port;
> + val = s->reg.port;
> + break;
> default:
> g_assert_not_reached();
> break;
> }
> - return 0;
> +
> + trace_avr_gpio_read(port_name(s), offset, val);
> + return val;
> }
>
> static void avr_gpio_write(void *opaque, hwaddr offset, uint64_t value,
> unsigned int size)
> {
> AVRGPIOState *s = (AVRGPIOState *)opaque;
> - value = value & 0xF;
> + value = value & 0xFF;
> +
> + trace_avr_gpio_write(port_name(s), offset, value);
> switch (offset) {
> case GPIO_PIN:
> s->reg.pin = value;
> diff --git a/hw/gpio/trace-events b/hw/gpio/trace-events
> index 46ab9323bd0..640834597a8 100644
> --- a/hw/gpio/trace-events
> +++ b/hw/gpio/trace-events
> @@ -18,3 +18,8 @@ sifive_gpio_read(uint64_t offset, uint64_t r) "offset
> 0x%" PRIx64 " value 0x%" P
> sifive_gpio_write(uint64_t offset, uint64_t value) "offset 0x%" PRIx64 "
> value 0x%" PRIx64
> sifive_gpio_set(int64_t line, int64_t value) "line %" PRIi64 " value %"
> PRIi64
> sifive_gpio_update_output_irq(int64_t line, int64_t value) "line %"
> PRIi64 " value %" PRIi64
> +
> +# avr_gpio.c
> +avr_gpio_read(unsigned id, uint64_t offset, uint64_t r) "port %c offset
> 0x%" PRIx64 " value 0x%" PRIx64
> +avr_gpio_write(unsigned id, uint64_t offset, uint64_t value) "port %c
> offset 0x%" PRIx64 " value 0x%" PRIx64
> +avr_gpio_update_output_irq(unsigned id, int64_t line, int64_t value)
> "port %c pin %" PRIi64 " value %" PRIi64
> --
> 2.26.2
>
>
[-- Attachment #2: Type: text/html, Size: 5463 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 07/11] hw/gpio/avr_gpio: Add tracing for reads and writes
2021-03-13 17:21 ` Niteesh G. S.
@ 2021-03-23 2:42 ` Niteesh G. S.
2021-03-23 8:39 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 30+ messages in thread
From: Niteesh G. S. @ 2021-03-23 2:42 UTC (permalink / raw)
To: Philippe Mathieu-Daudé; +Cc: qemu-devel
[-- Attachment #1: Type: text/plain, Size: 4301 bytes --]
Hii Phil,
A gentle reminder to push these patches.
Thanks,
Niteesh.
On Sat, Mar 13, 2021 at 10:51 PM Niteesh G. S. <niteesh.gs@gmail.com> wrote:
> Reviewed-by: Niteesh G S <niteesh.gs@gmail.com>
>
> On Sat, Mar 13, 2021 at 10:25 PM Philippe Mathieu-Daudé <f4bug@amsat.org>
> wrote:
>
>> From: G S Niteesh Babu <niteesh.gs@gmail.com>
>>
>> Added tracing for gpio read, write, and update output irq.
>>
>> 1) trace_avr_gpio_update_ouput_irq
>> 2) trace_avr_gpio_read
>> 3) trace_avr_gpio_write
>>
>> Signed-off-by: G S Niteesh Babu <niteesh.gs@gmail.com>
>> Reviewed-by: Michael Rolnik <mrolnik@gmail.com>
>> Message-Id: <20210311135539.10206-3-niteesh.gs@gmail.com>
>> [PMD: Added port_name(), display port name in trace events]
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>> hw/gpio/avr_gpio.c | 26 +++++++++++++++++++++-----
>> hw/gpio/trace-events | 5 +++++
>> 2 files changed, 26 insertions(+), 5 deletions(-)
>>
>> diff --git a/hw/gpio/avr_gpio.c b/hw/gpio/avr_gpio.c
>> index e4c7122e62c..29252d6ccfe 100644
>> --- a/hw/gpio/avr_gpio.c
>> +++ b/hw/gpio/avr_gpio.c
>> @@ -2,6 +2,7 @@
>> * AVR processors GPIO registers emulation.
>> *
>> * Copyright (C) 2020 Heecheol Yang <heecheol.yang@outlook.com>
>> + * Copyright (C) 2021 Niteesh Babu G S <niteesh.gs@gmail.com>
>> *
>> * This program is free software; you can redistribute it and/or
>> * modify it under the terms of the GNU General Public License as
>> @@ -26,6 +27,12 @@
>> #include "hw/gpio/avr_gpio.h"
>> #include "hw/qdev-properties.h"
>> #include "migration/vmstate.h"
>> +#include "trace.h"
>> +
>> +static char port_name(AVRGPIOState *s)
>> +{
>> + return 'A' + s->id;
>> +}
>>
>> static void avr_gpio_reset(DeviceState *dev)
>> {
>> @@ -47,32 +54,41 @@ static void avr_gpio_write_port(AVRGPIOState *s,
>> uint64_t value)
>>
>> if (cur_ddr_pin_val && (cur_port_pin_val != new_port_pin_val)) {
>> qemu_set_irq(s->out[pin], new_port_pin_val);
>> + trace_avr_gpio_update_output_irq(port_name(s), pin,
>> new_port_pin_val);
>> }
>> }
>> s->reg.port = value & s->reg.ddr;
>> }
>> static uint64_t avr_gpio_read(void *opaque, hwaddr offset, unsigned int
>> size)
>> {
>> + uint8_t val = 0;
>> AVRGPIOState *s = (AVRGPIOState *)opaque;
>> switch (offset) {
>> case GPIO_PIN:
>> - return s->reg.pin;
>> + val = s->reg.pin;
>> + break;
>> case GPIO_DDR:
>> - return s->reg.ddr;
>> + val = s->reg.ddr;
>> + break;
>> case GPIO_PORT:
>> - return s->reg.port;
>> + val = s->reg.port;
>> + break;
>> default:
>> g_assert_not_reached();
>> break;
>> }
>> - return 0;
>> +
>> + trace_avr_gpio_read(port_name(s), offset, val);
>> + return val;
>> }
>>
>> static void avr_gpio_write(void *opaque, hwaddr offset, uint64_t value,
>> unsigned int size)
>> {
>> AVRGPIOState *s = (AVRGPIOState *)opaque;
>> - value = value & 0xF;
>> + value = value & 0xFF;
>> +
>> + trace_avr_gpio_write(port_name(s), offset, value);
>> switch (offset) {
>> case GPIO_PIN:
>> s->reg.pin = value;
>> diff --git a/hw/gpio/trace-events b/hw/gpio/trace-events
>> index 46ab9323bd0..640834597a8 100644
>> --- a/hw/gpio/trace-events
>> +++ b/hw/gpio/trace-events
>> @@ -18,3 +18,8 @@ sifive_gpio_read(uint64_t offset, uint64_t r) "offset
>> 0x%" PRIx64 " value 0x%" P
>> sifive_gpio_write(uint64_t offset, uint64_t value) "offset 0x%" PRIx64 "
>> value 0x%" PRIx64
>> sifive_gpio_set(int64_t line, int64_t value) "line %" PRIi64 " value %"
>> PRIi64
>> sifive_gpio_update_output_irq(int64_t line, int64_t value) "line %"
>> PRIi64 " value %" PRIi64
>> +
>> +# avr_gpio.c
>> +avr_gpio_read(unsigned id, uint64_t offset, uint64_t r) "port %c offset
>> 0x%" PRIx64 " value 0x%" PRIx64
>> +avr_gpio_write(unsigned id, uint64_t offset, uint64_t value) "port %c
>> offset 0x%" PRIx64 " value 0x%" PRIx64
>> +avr_gpio_update_output_irq(unsigned id, int64_t line, int64_t value)
>> "port %c pin %" PRIi64 " value %" PRIi64
>> --
>> 2.26.2
>>
>>
[-- Attachment #2: Type: text/html, Size: 6269 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH 08/11] hw/avr/arduino: Add D13 LED
2021-03-13 16:54 [PATCH 00/11] AVR patch queue for QEMU 6.0 Philippe Mathieu-Daudé
` (6 preceding siblings ...)
2021-03-13 16:54 ` [PATCH 07/11] hw/gpio/avr_gpio: Add tracing for reads and writes Philippe Mathieu-Daudé
@ 2021-03-13 16:54 ` Philippe Mathieu-Daudé
2021-03-13 17:02 ` Niteesh G. S.
2021-03-13 16:54 ` [PATCH 09/11] hw/avr/arduino: Replace magic number by gpio_port_index() call Philippe Mathieu-Daudé
` (4 subsequent siblings)
12 siblings, 1 reply; 30+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-03-13 16:54 UTC (permalink / raw)
To: qemu-devel
Cc: Thomas Huth, Sarah Harris, Michael Rolnik,
Philippe Mathieu-Daudé,
G S Niteesh Babu
From: G S Niteesh Babu <niteesh.gs@gmail.com>
Signed-off-by: G S Niteesh Babu <niteesh.gs@gmail.com>
Reviewed-by: Michael Rolnik <mrolnik@gmail.com>
Message-Id: <20210311135539.10206-4-niteesh.gs@gmail.com>
[PMD: Added ArduinoMachineClass::d13_led_portb_bit]
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
hw/avr/arduino.c | 18 ++++++++++++++++++
hw/avr/Kconfig | 1 +
2 files changed, 19 insertions(+)
diff --git a/hw/avr/arduino.c b/hw/avr/arduino.c
index 3ff31492fa6..73563a35d0d 100644
--- a/hw/avr/arduino.c
+++ b/hw/avr/arduino.c
@@ -13,6 +13,7 @@
#include "qemu/osdep.h"
#include "qapi/error.h"
#include "hw/boards.h"
+#include "hw/misc/led.h"
#include "atmega.h"
#include "boot.h"
#include "qom/object.h"
@@ -22,6 +23,8 @@ struct ArduinoMachineState {
MachineState parent_obj;
/*< public >*/
AtmegaMcuState mcu;
+
+ LEDState *onboard_led;
};
typedef struct ArduinoMachineState ArduinoMachineState;
@@ -31,6 +34,7 @@ struct ArduinoMachineClass {
/*< public >*/
const char *mcu_type;
uint64_t xtal_hz;
+ unsigned d13_led_portb_bit; /* PORTB GPIO for D13 yellow LED */
};
typedef struct ArduinoMachineClass ArduinoMachineClass;
@@ -49,6 +53,16 @@ static void arduino_machine_init(MachineState *machine)
amc->xtal_hz, &error_abort);
sysbus_realize(SYS_BUS_DEVICE(&ams->mcu), &error_abort);
+ /* Onboard led connected to digital header PIN 13 */
+ ams->onboard_led = led_create_simple(OBJECT(ams),
+ GPIO_POLARITY_ACTIVE_HIGH,
+ LED_COLOR_YELLOW,
+ "D13 LED");
+
+ qdev_connect_gpio_out(DEVICE(&ams->mcu.gpio[1]),
+ amc->d13_led_portb_bit,
+ qdev_get_gpio_in(DEVICE(ams->onboard_led), 0));
+
if (machine->firmware) {
if (!avr_load_firmware(&ams->mcu.cpu, machine,
&ams->mcu.flash, machine->firmware)) {
@@ -83,6 +97,7 @@ static void arduino_duemilanove_class_init(ObjectClass *oc, void *data)
mc->alias = "2009";
amc->mcu_type = TYPE_ATMEGA168_MCU;
amc->xtal_hz = 16 * 1000 * 1000;
+ amc->d13_led_portb_bit = 5;
};
static void arduino_uno_class_init(ObjectClass *oc, void *data)
@@ -98,6 +113,7 @@ static void arduino_uno_class_init(ObjectClass *oc, void *data)
mc->alias = "uno";
amc->mcu_type = TYPE_ATMEGA328_MCU;
amc->xtal_hz = 16 * 1000 * 1000;
+ amc->d13_led_portb_bit = 5;
};
static void arduino_mega_class_init(ObjectClass *oc, void *data)
@@ -113,6 +129,7 @@ static void arduino_mega_class_init(ObjectClass *oc, void *data)
mc->alias = "mega";
amc->mcu_type = TYPE_ATMEGA1280_MCU;
amc->xtal_hz = 16 * 1000 * 1000;
+ amc->d13_led_portb_bit = 7;
};
static void arduino_mega2560_class_init(ObjectClass *oc, void *data)
@@ -128,6 +145,7 @@ static void arduino_mega2560_class_init(ObjectClass *oc, void *data)
mc->alias = "mega2560";
amc->mcu_type = TYPE_ATMEGA2560_MCU;
amc->xtal_hz = 16 * 1000 * 1000; /* CSTCE16M0V53-R0 */
+ amc->d13_led_portb_bit = 7;
};
static const TypeInfo arduino_machine_types[] = {
diff --git a/hw/avr/Kconfig b/hw/avr/Kconfig
index 16a57ced11f..e0d4fc5537a 100644
--- a/hw/avr/Kconfig
+++ b/hw/avr/Kconfig
@@ -8,3 +8,4 @@ config AVR_ATMEGA_MCU
config ARDUINO
select AVR_ATMEGA_MCU
select UNIMP
+ select LED
--
2.26.2
^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH 08/11] hw/avr/arduino: Add D13 LED
2021-03-13 16:54 ` [PATCH 08/11] hw/avr/arduino: Add D13 LED Philippe Mathieu-Daudé
@ 2021-03-13 17:02 ` Niteesh G. S.
2021-03-13 17:21 ` Niteesh G. S.
0 siblings, 1 reply; 30+ messages in thread
From: Niteesh G. S. @ 2021-03-13 17:02 UTC (permalink / raw)
To: Philippe Mathieu-Daudé; +Cc: Michael Rolnik, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 3993 bytes --]
Hii Phil,
Just a few mins earlier than me :)
Thanks,
Niteesh
On Sat, Mar 13, 2021 at 10:25 PM Philippe Mathieu-Daudé <f4bug@amsat.org>
wrote:
> From: G S Niteesh Babu <niteesh.gs@gmail.com>
>
> Signed-off-by: G S Niteesh Babu <niteesh.gs@gmail.com>
> Reviewed-by: Michael Rolnik <mrolnik@gmail.com>
> Message-Id: <20210311135539.10206-4-niteesh.gs@gmail.com>
> [PMD: Added ArduinoMachineClass::d13_led_portb_bit]
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> hw/avr/arduino.c | 18 ++++++++++++++++++
> hw/avr/Kconfig | 1 +
> 2 files changed, 19 insertions(+)
>
> diff --git a/hw/avr/arduino.c b/hw/avr/arduino.c
> index 3ff31492fa6..73563a35d0d 100644
> --- a/hw/avr/arduino.c
> +++ b/hw/avr/arduino.c
> @@ -13,6 +13,7 @@
> #include "qemu/osdep.h"
> #include "qapi/error.h"
> #include "hw/boards.h"
> +#include "hw/misc/led.h"
> #include "atmega.h"
> #include "boot.h"
> #include "qom/object.h"
> @@ -22,6 +23,8 @@ struct ArduinoMachineState {
> MachineState parent_obj;
> /*< public >*/
> AtmegaMcuState mcu;
> +
> + LEDState *onboard_led;
> };
> typedef struct ArduinoMachineState ArduinoMachineState;
>
> @@ -31,6 +34,7 @@ struct ArduinoMachineClass {
> /*< public >*/
> const char *mcu_type;
> uint64_t xtal_hz;
> + unsigned d13_led_portb_bit; /* PORTB GPIO for D13 yellow LED */
> };
> typedef struct ArduinoMachineClass ArduinoMachineClass;
>
> @@ -49,6 +53,16 @@ static void arduino_machine_init(MachineState *machine)
> amc->xtal_hz, &error_abort);
> sysbus_realize(SYS_BUS_DEVICE(&ams->mcu), &error_abort);
>
> + /* Onboard led connected to digital header PIN 13 */
> + ams->onboard_led = led_create_simple(OBJECT(ams),
> + GPIO_POLARITY_ACTIVE_HIGH,
> + LED_COLOR_YELLOW,
> + "D13 LED");
> +
> + qdev_connect_gpio_out(DEVICE(&ams->mcu.gpio[1]),
> + amc->d13_led_portb_bit,
> + qdev_get_gpio_in(DEVICE(ams->onboard_led), 0));
> +
> if (machine->firmware) {
> if (!avr_load_firmware(&ams->mcu.cpu, machine,
> &ams->mcu.flash, machine->firmware)) {
> @@ -83,6 +97,7 @@ static void arduino_duemilanove_class_init(ObjectClass
> *oc, void *data)
> mc->alias = "2009";
> amc->mcu_type = TYPE_ATMEGA168_MCU;
> amc->xtal_hz = 16 * 1000 * 1000;
> + amc->d13_led_portb_bit = 5;
> };
>
> static void arduino_uno_class_init(ObjectClass *oc, void *data)
> @@ -98,6 +113,7 @@ static void arduino_uno_class_init(ObjectClass *oc,
> void *data)
> mc->alias = "uno";
> amc->mcu_type = TYPE_ATMEGA328_MCU;
> amc->xtal_hz = 16 * 1000 * 1000;
> + amc->d13_led_portb_bit = 5;
> };
>
> static void arduino_mega_class_init(ObjectClass *oc, void *data)
> @@ -113,6 +129,7 @@ static void arduino_mega_class_init(ObjectClass *oc,
> void *data)
> mc->alias = "mega";
> amc->mcu_type = TYPE_ATMEGA1280_MCU;
> amc->xtal_hz = 16 * 1000 * 1000;
> + amc->d13_led_portb_bit = 7;
> };
>
> static void arduino_mega2560_class_init(ObjectClass *oc, void *data)
> @@ -128,6 +145,7 @@ static void arduino_mega2560_class_init(ObjectClass
> *oc, void *data)
> mc->alias = "mega2560";
> amc->mcu_type = TYPE_ATMEGA2560_MCU;
> amc->xtal_hz = 16 * 1000 * 1000; /* CSTCE16M0V53-R0 */
> + amc->d13_led_portb_bit = 7;
> };
>
> static const TypeInfo arduino_machine_types[] = {
> diff --git a/hw/avr/Kconfig b/hw/avr/Kconfig
> index 16a57ced11f..e0d4fc5537a 100644
> --- a/hw/avr/Kconfig
> +++ b/hw/avr/Kconfig
> @@ -8,3 +8,4 @@ config AVR_ATMEGA_MCU
> config ARDUINO
> select AVR_ATMEGA_MCU
> select UNIMP
> + select LED
> --
> 2.26.2
>
>
[-- Attachment #2: Type: text/html, Size: 5640 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 08/11] hw/avr/arduino: Add D13 LED
2021-03-13 17:02 ` Niteesh G. S.
@ 2021-03-13 17:21 ` Niteesh G. S.
0 siblings, 0 replies; 30+ messages in thread
From: Niteesh G. S. @ 2021-03-13 17:21 UTC (permalink / raw)
To: Philippe Mathieu-Daudé; +Cc: Michael Rolnik, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 4247 bytes --]
Reviewed-by: Niteesh G S <niteesh.gs@gmail.com>
On Sat, Mar 13, 2021 at 10:32 PM Niteesh G. S. <niteesh.gs@gmail.com> wrote:
> Hii Phil,
>
> Just a few mins earlier than me :)
>
> Thanks,
> Niteesh
>
> On Sat, Mar 13, 2021 at 10:25 PM Philippe Mathieu-Daudé <f4bug@amsat.org>
> wrote:
>
>> From: G S Niteesh Babu <niteesh.gs@gmail.com>
>>
>> Signed-off-by: G S Niteesh Babu <niteesh.gs@gmail.com>
>> Reviewed-by: Michael Rolnik <mrolnik@gmail.com>
>> Message-Id: <20210311135539.10206-4-niteesh.gs@gmail.com>
>> [PMD: Added ArduinoMachineClass::d13_led_portb_bit]
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>> hw/avr/arduino.c | 18 ++++++++++++++++++
>> hw/avr/Kconfig | 1 +
>> 2 files changed, 19 insertions(+)
>>
>> diff --git a/hw/avr/arduino.c b/hw/avr/arduino.c
>> index 3ff31492fa6..73563a35d0d 100644
>> --- a/hw/avr/arduino.c
>> +++ b/hw/avr/arduino.c
>> @@ -13,6 +13,7 @@
>> #include "qemu/osdep.h"
>> #include "qapi/error.h"
>> #include "hw/boards.h"
>> +#include "hw/misc/led.h"
>> #include "atmega.h"
>> #include "boot.h"
>> #include "qom/object.h"
>> @@ -22,6 +23,8 @@ struct ArduinoMachineState {
>> MachineState parent_obj;
>> /*< public >*/
>> AtmegaMcuState mcu;
>> +
>> + LEDState *onboard_led;
>> };
>> typedef struct ArduinoMachineState ArduinoMachineState;
>>
>> @@ -31,6 +34,7 @@ struct ArduinoMachineClass {
>> /*< public >*/
>> const char *mcu_type;
>> uint64_t xtal_hz;
>> + unsigned d13_led_portb_bit; /* PORTB GPIO for D13 yellow LED */
>> };
>> typedef struct ArduinoMachineClass ArduinoMachineClass;
>>
>> @@ -49,6 +53,16 @@ static void arduino_machine_init(MachineState *machine)
>> amc->xtal_hz, &error_abort);
>> sysbus_realize(SYS_BUS_DEVICE(&ams->mcu), &error_abort);
>>
>> + /* Onboard led connected to digital header PIN 13 */
>> + ams->onboard_led = led_create_simple(OBJECT(ams),
>> + GPIO_POLARITY_ACTIVE_HIGH,
>> + LED_COLOR_YELLOW,
>> + "D13 LED");
>> +
>> + qdev_connect_gpio_out(DEVICE(&ams->mcu.gpio[1]),
>> + amc->d13_led_portb_bit,
>> + qdev_get_gpio_in(DEVICE(ams->onboard_led), 0));
>> +
>> if (machine->firmware) {
>> if (!avr_load_firmware(&ams->mcu.cpu, machine,
>> &ams->mcu.flash, machine->firmware)) {
>> @@ -83,6 +97,7 @@ static void arduino_duemilanove_class_init(ObjectClass
>> *oc, void *data)
>> mc->alias = "2009";
>> amc->mcu_type = TYPE_ATMEGA168_MCU;
>> amc->xtal_hz = 16 * 1000 * 1000;
>> + amc->d13_led_portb_bit = 5;
>> };
>>
>> static void arduino_uno_class_init(ObjectClass *oc, void *data)
>> @@ -98,6 +113,7 @@ static void arduino_uno_class_init(ObjectClass *oc,
>> void *data)
>> mc->alias = "uno";
>> amc->mcu_type = TYPE_ATMEGA328_MCU;
>> amc->xtal_hz = 16 * 1000 * 1000;
>> + amc->d13_led_portb_bit = 5;
>> };
>>
>> static void arduino_mega_class_init(ObjectClass *oc, void *data)
>> @@ -113,6 +129,7 @@ static void arduino_mega_class_init(ObjectClass *oc,
>> void *data)
>> mc->alias = "mega";
>> amc->mcu_type = TYPE_ATMEGA1280_MCU;
>> amc->xtal_hz = 16 * 1000 * 1000;
>> + amc->d13_led_portb_bit = 7;
>> };
>>
>> static void arduino_mega2560_class_init(ObjectClass *oc, void *data)
>> @@ -128,6 +145,7 @@ static void arduino_mega2560_class_init(ObjectClass
>> *oc, void *data)
>> mc->alias = "mega2560";
>> amc->mcu_type = TYPE_ATMEGA2560_MCU;
>> amc->xtal_hz = 16 * 1000 * 1000; /* CSTCE16M0V53-R0 */
>> + amc->d13_led_portb_bit = 7;
>> };
>>
>> static const TypeInfo arduino_machine_types[] = {
>> diff --git a/hw/avr/Kconfig b/hw/avr/Kconfig
>> index 16a57ced11f..e0d4fc5537a 100644
>> --- a/hw/avr/Kconfig
>> +++ b/hw/avr/Kconfig
>> @@ -8,3 +8,4 @@ config AVR_ATMEGA_MCU
>> config ARDUINO
>> select AVR_ATMEGA_MCU
>> select UNIMP
>> + select LED
>> --
>> 2.26.2
>>
>>
[-- Attachment #2: Type: text/html, Size: 6172 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH 09/11] hw/avr/arduino: Replace magic number by gpio_port_index() call
2021-03-13 16:54 [PATCH 00/11] AVR patch queue for QEMU 6.0 Philippe Mathieu-Daudé
` (7 preceding siblings ...)
2021-03-13 16:54 ` [PATCH 08/11] hw/avr/arduino: Add D13 LED Philippe Mathieu-Daudé
@ 2021-03-13 16:54 ` Philippe Mathieu-Daudé
2021-03-13 20:02 ` Richard Henderson
2021-03-13 16:54 ` [PATCH 10/11] target/avr: Fix some comment spelling errors Philippe Mathieu-Daudé
` (3 subsequent siblings)
12 siblings, 1 reply; 30+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-03-13 16:54 UTC (permalink / raw)
To: qemu-devel
Cc: Thomas Huth, Sarah Harris, Michael Rolnik, Philippe Mathieu-Daudé
The '1' magic value means 'Port B'. Introduce and use the
gpio_port_index() helper to explicit the port name.
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
hw/avr/arduino.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/hw/avr/arduino.c b/hw/avr/arduino.c
index 73563a35d0d..87124d17f18 100644
--- a/hw/avr/arduino.c
+++ b/hw/avr/arduino.c
@@ -43,6 +43,12 @@ typedef struct ArduinoMachineClass ArduinoMachineClass;
DECLARE_OBJ_CHECKERS(ArduinoMachineState, ArduinoMachineClass,
ARDUINO_MACHINE, TYPE_ARDUINO_MACHINE)
+static unsigned gpio_port_index(char c)
+{
+ assert(c >= 'A' && c < 'A' + GPIO_MAX);
+ return c - 'A';
+}
+
static void arduino_machine_init(MachineState *machine)
{
ArduinoMachineClass *amc = ARDUINO_MACHINE_GET_CLASS(machine);
@@ -59,7 +65,7 @@ static void arduino_machine_init(MachineState *machine)
LED_COLOR_YELLOW,
"D13 LED");
- qdev_connect_gpio_out(DEVICE(&ams->mcu.gpio[1]),
+ qdev_connect_gpio_out(DEVICE(&ams->mcu.gpio[gpio_port_index('B')]),
amc->d13_led_portb_bit,
qdev_get_gpio_in(DEVICE(ams->onboard_led), 0));
--
2.26.2
^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH 09/11] hw/avr/arduino: Replace magic number by gpio_port_index() call
2021-03-13 16:54 ` [PATCH 09/11] hw/avr/arduino: Replace magic number by gpio_port_index() call Philippe Mathieu-Daudé
@ 2021-03-13 20:02 ` Richard Henderson
2021-03-13 22:20 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 30+ messages in thread
From: Richard Henderson @ 2021-03-13 20:02 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Thomas Huth, Sarah Harris, Michael Rolnik
On 3/13/21 10:54 AM, Philippe Mathieu-Daudé wrote:
> +static unsigned gpio_port_index(char c)
> +{
> + assert(c >= 'A' && c < 'A' + GPIO_MAX);
> + return c - 'A';
> +}
If you're not going to use this for anything else, isn't
#define PORT_B 1
enough?
r~
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 09/11] hw/avr/arduino: Replace magic number by gpio_port_index() call
2021-03-13 20:02 ` Richard Henderson
@ 2021-03-13 22:20 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 30+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-03-13 22:20 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: Thomas Huth, Sarah Harris, Michael Rolnik
On 3/13/21 9:02 PM, Richard Henderson wrote:
> On 3/13/21 10:54 AM, Philippe Mathieu-Daudé wrote:
>> +static unsigned gpio_port_index(char c)
>> +{
>> + assert(c >= 'A' && c < 'A' + GPIO_MAX);
>> + return c - 'A';
>> +}
>
> If you're not going to use this for anything else, isn't
>
> #define PORT_B 1
>
> enough?
Surely :) I'll squash that in previous patch.
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH 10/11] target/avr: Fix some comment spelling errors
2021-03-13 16:54 [PATCH 00/11] AVR patch queue for QEMU 6.0 Philippe Mathieu-Daudé
` (8 preceding siblings ...)
2021-03-13 16:54 ` [PATCH 09/11] hw/avr/arduino: Replace magic number by gpio_port_index() call Philippe Mathieu-Daudé
@ 2021-03-13 16:54 ` Philippe Mathieu-Daudé
2021-03-13 16:54 ` [PATCH 11/11] target/avr: Fix interrupt execution Philippe Mathieu-Daudé
` (2 subsequent siblings)
12 siblings, 0 replies; 30+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-03-13 16:54 UTC (permalink / raw)
To: qemu-devel
Cc: Sarah Harris, Thomas Huth, Lichang Zhao,
Philippe Mathieu-Daudé,
David Edmondson, Michael Rolnik
From: Lichang Zhao <zhaolichang@huawei.com>
I found that there are many spelling errors in the comments of qemu/target/avr.
I used spellcheck to check the spelling errors and found some errors in the folder.
Signed-off-by: Lichang Zhao <zhaolichang@huawei.com>
Reviewed-by: David Edmondson <david.edmondson@oracle.com>
Reviewed-by: Philippe Mathieu-Daude<f4bug@amsat.org>
Message-Id: <20201009064449.2336-12-zhaolichang@huawei.com>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
target/avr/helper.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/target/avr/helper.c b/target/avr/helper.c
index 65880b9928c..b4532de2523 100644
--- a/target/avr/helper.c
+++ b/target/avr/helper.c
@@ -98,7 +98,7 @@ int avr_cpu_memory_rw_debug(CPUState *cs, vaddr addr, uint8_t *buf,
hwaddr avr_cpu_get_phys_page_debug(CPUState *cs, vaddr addr)
{
- return addr; /* I assume 1:1 address correspondance */
+ return addr; /* I assume 1:1 address correspondence */
}
bool avr_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
@@ -299,7 +299,7 @@ void helper_outb(CPUAVRState *env, uint32_t port, uint32_t data)
}
/*
- * this function implements LD instruction when there is a posibility to read
+ * this function implements LD instruction when there is a possibility to read
* from a CPU register
*/
target_ulong helper_fullrd(CPUAVRState *env, uint32_t addr)
@@ -323,7 +323,7 @@ target_ulong helper_fullrd(CPUAVRState *env, uint32_t addr)
}
/*
- * this function implements ST instruction when there is a posibility to write
+ * this function implements ST instruction when there is a possibility to write
* into a CPU register
*/
void helper_fullwr(CPUAVRState *env, uint32_t data, uint32_t addr)
--
2.26.2
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH 11/11] target/avr: Fix interrupt execution
2021-03-13 16:54 [PATCH 00/11] AVR patch queue for QEMU 6.0 Philippe Mathieu-Daudé
` (9 preceding siblings ...)
2021-03-13 16:54 ` [PATCH 10/11] target/avr: Fix some comment spelling errors Philippe Mathieu-Daudé
@ 2021-03-13 16:54 ` Philippe Mathieu-Daudé
2021-03-13 22:16 ` [PATCH 00/11] AVR patch queue for QEMU 6.0 Michael Rolnik
2021-03-14 23:42 ` Philippe Mathieu-Daudé
12 siblings, 0 replies; 30+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-03-13 16:54 UTC (permalink / raw)
To: qemu-devel
Cc: Thomas Huth, Sarah Harris, Ivanov Arkasha, Michael Rolnik,
Philippe Mathieu-Daudé
From: Ivanov Arkasha <ivanovrkasha@gmail.com>
Only one interrupt is in progress at the moment.
It is only necessary to set to reset interrupt_request
after all interrupts have been executed.
Signed-off-by: Ivanov Arkasha <ivanovrkasha@gmail.com>
Message-Id: <20210312164754.18437-1-arkaisp2021@gmail.com>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
target/avr/helper.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/target/avr/helper.c b/target/avr/helper.c
index b4532de2523..35e10195940 100644
--- a/target/avr/helper.c
+++ b/target/avr/helper.c
@@ -49,7 +49,9 @@ bool avr_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
cc->tcg_ops->do_interrupt(cs);
env->intsrc &= env->intsrc - 1; /* clear the interrupt */
- cs->interrupt_request &= ~CPU_INTERRUPT_HARD;
+ if (!env->intsrc) {
+ cs->interrupt_request &= ~CPU_INTERRUPT_HARD;
+ }
ret = true;
}
--
2.26.2
^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH 00/11] AVR patch queue for QEMU 6.0
2021-03-13 16:54 [PATCH 00/11] AVR patch queue for QEMU 6.0 Philippe Mathieu-Daudé
` (10 preceding siblings ...)
2021-03-13 16:54 ` [PATCH 11/11] target/avr: Fix interrupt execution Philippe Mathieu-Daudé
@ 2021-03-13 22:16 ` Michael Rolnik
2021-03-14 23:42 ` Philippe Mathieu-Daudé
12 siblings, 0 replies; 30+ messages in thread
From: Michael Rolnik @ 2021-03-13 22:16 UTC (permalink / raw)
To: Philippe Mathieu-Daudé; +Cc: Thomas Huth, Sarah Harris, QEMU Developers
[-- Attachment #1: Type: text/plain, Size: 1804 bytes --]
Reviewed-by: Michael Rolnik <mrolnik@gmail.com>
On Sat, Mar 13, 2021 at 6:54 PM Philippe Mathieu-Daudé <f4bug@amsat.org>
wrote:
> Hi,
>
> This series contains all the AVR patches I could find on the list.
>
> Niteesh, I fixed minor issues. Do you mind reviewing on top?
>
> Pull request planned for Monday if no problem arises.
>
> Thanks,
>
> Phil.
>
> G S Niteesh Babu (2):
> hw/gpio/avr_gpio: Add tracing for reads and writes
> hw/avr/arduino: Add D13 LED
>
> Heecheol Yang (1):
> hw/avr: Add limited support for avr gpio registers
>
> Ivanov Arkasha (1):
> target/avr: Fix interrupt execution
>
> Lichang Zhao (1):
> target/avr: Fix some comment spelling errors
>
> Philippe Mathieu-Daudé (6):
> hw/misc/led: Add yellow LED
> hw/avr/arduino: List board schematic links
> hw/gpio/avr_gpio: Add migration VMstate
> hw/gpio/avr_gpio: Add 'id' field in AVRGPIOState
> hw/gpio/avr_gpio: Simplify avr_gpio_write_port using extract32()
> hw/avr/arduino: Replace magic number by gpio_port_index() call
>
> hw/avr/atmega.h | 2 +
> include/hw/gpio/avr_gpio.h | 54 ++++++++++++
> include/hw/misc/led.h | 1 +
> hw/avr/arduino.c | 44 +++++++++-
> hw/avr/atmega.c | 8 +-
> hw/gpio/avr_gpio.c | 173 +++++++++++++++++++++++++++++++++++++
> hw/misc/led.c | 1 +
> target/avr/helper.c | 10 ++-
> hw/avr/Kconfig | 2 +
> hw/gpio/Kconfig | 3 +
> hw/gpio/meson.build | 1 +
> hw/gpio/trace-events | 5 ++
> 12 files changed, 294 insertions(+), 10 deletions(-)
> create mode 100644 include/hw/gpio/avr_gpio.h
> create mode 100644 hw/gpio/avr_gpio.c
>
> --
> 2.26.2
>
>
--
Best Regards,
Michael Rolnik
[-- Attachment #2: Type: text/html, Size: 2421 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 00/11] AVR patch queue for QEMU 6.0
2021-03-13 16:54 [PATCH 00/11] AVR patch queue for QEMU 6.0 Philippe Mathieu-Daudé
` (11 preceding siblings ...)
2021-03-13 22:16 ` [PATCH 00/11] AVR patch queue for QEMU 6.0 Michael Rolnik
@ 2021-03-14 23:42 ` Philippe Mathieu-Daudé
12 siblings, 0 replies; 30+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-03-14 23:42 UTC (permalink / raw)
To: qemu-devel; +Cc: Thomas Huth, Sarah Harris, Michael Rolnik
On 3/13/21 5:54 PM, Philippe Mathieu-Daudé wrote:
> Hi,
>
> This series contains all the AVR patches I could find on the list.
>
> Niteesh, I fixed minor issues. Do you mind reviewing on top?
>
> Pull request planned for Monday if no problem arises.
>
> Thanks,
>
> Phil.
>
> G S Niteesh Babu (2):
> hw/gpio/avr_gpio: Add tracing for reads and writes
> hw/avr/arduino: Add D13 LED
>
> Heecheol Yang (1):
> hw/avr: Add limited support for avr gpio registers
>
> Ivanov Arkasha (1):
> target/avr: Fix interrupt execution
>
> Lichang Zhao (1):
> target/avr: Fix some comment spelling errors
>
> Philippe Mathieu-Daudé (6):
> hw/misc/led: Add yellow LED
> hw/avr/arduino: List board schematic links
> hw/gpio/avr_gpio: Add migration VMstate
> hw/gpio/avr_gpio: Add 'id' field in AVRGPIOState
> hw/gpio/avr_gpio: Simplify avr_gpio_write_port using extract32()
> hw/avr/arduino: Replace magic number by gpio_port_index() call
>
> hw/avr/atmega.h | 2 +
> include/hw/gpio/avr_gpio.h | 54 ++++++++++++
> include/hw/misc/led.h | 1 +
> hw/avr/arduino.c | 44 +++++++++-
> hw/avr/atmega.c | 8 +-
> hw/gpio/avr_gpio.c | 173 +++++++++++++++++++++++++++++++++++++
> hw/misc/led.c | 1 +
> target/avr/helper.c | 10 ++-
> hw/avr/Kconfig | 2 +
> hw/gpio/Kconfig | 3 +
> hw/gpio/meson.build | 1 +
> hw/gpio/trace-events | 5 ++
> 12 files changed, 294 insertions(+), 10 deletions(-)
> create mode 100644 include/hw/gpio/avr_gpio.h
> create mode 100644 hw/gpio/avr_gpio.c
Patches 1, 2, 10, 11 queued to avr-next.
^ permalink raw reply [flat|nested] 30+ messages in thread