All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] hw/avr: Add limited support for avr gpio registers
@ 2020-10-03 12:38 Heecheol Yang
  2020-10-03 17:08 ` Michael Rolnik
  2020-10-03 18:03 ` Philippe Mathieu-Daudé
  0 siblings, 2 replies; 5+ messages in thread
From: Heecheol Yang @ 2020-10-03 12:38 UTC (permalink / raw)
  To: qemu-devel; +Cc: Sarah Harris, Michael Rolnik, Heecheol Yang

Add some of these features for avr gpio:

  - GPIO I/O : PORTx registers
  - Data Direction : DDRx registers

Following things are not supported yet:
  - PINx registers
  - MCUR registers
  - Even though read/write for DDRx registers are
    implemented, actual direction controls are not
    supported yet.

Signed-off-by: Heecheol Yang <heecheol.yang@outlook.com>
---
 hw/avr/Kconfig             |   1 +
 hw/avr/atmega.c            |   7 ++-
 hw/avr/atmega.h            |   2 +
 hw/gpio/Kconfig            |   3 +
 hw/gpio/avr_gpio.c         | 112 +++++++++++++++++++++++++++++++++++++
 hw/gpio/meson.build        |   2 +
 include/hw/gpio/avr_gpio.h |  46 +++++++++++++++
 7 files changed, 171 insertions(+), 2 deletions(-)
 create mode 100644 hw/gpio/avr_gpio.c
 create mode 100644 include/hw/gpio/avr_gpio.h

diff --git a/hw/avr/Kconfig b/hw/avr/Kconfig
index d31298c3cc..16a57ced11 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/avr/atmega.c b/hw/avr/atmega.c
index 44c6afebbb..ad942028fd 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/avr/atmega.h b/hw/avr/atmega.h
index a99ee15c7e..e2289d5744 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/hw/gpio/Kconfig b/hw/gpio/Kconfig
index b6fdaa2586..1752d0ce56 100644
--- a/hw/gpio/Kconfig
+++ b/hw/gpio/Kconfig
@@ -10,3 +10,6 @@ config GPIO_KEY
 
 config SIFIVE_GPIO
     bool
+
+config AVR_GPIO
+    bool
diff --git a/hw/gpio/avr_gpio.c b/hw/gpio/avr_gpio.c
new file mode 100644
index 0000000000..6ca8e8703a
--- /dev/null
+++ b/hw/gpio/avr_gpio.c
@@ -0,0 +1,112 @@
+/*
+ * 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 "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->ddr_val = 0u;
+    gpio->port_val = 0u;
+}
+static uint64_t avr_gpio_read(void *opaque, hwaddr offset, unsigned int size)
+{
+    AVRGPIOState *s = (AVRGPIOState *)opaque;
+    switch (offset) {
+    case GPIO_PIN:
+        /* Not implemented yet */
+        break;
+    case GPIO_DDR:
+        return s->ddr_val;
+        break;
+    case GPIO_PORT:
+        return s->port_val;
+    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;
+    switch (offset) {
+    case GPIO_PIN:
+        /* Not implemented yet */
+        break;
+    case GPIO_DDR:
+        s->ddr_val = value & 0xF;
+        break;
+    case GPIO_PORT:
+        s->port_val = value & 0xF;
+        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);
+    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)
+{
+    avr_gpio_reset(dev);
+}
+
+
+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/gpio/meson.build b/hw/gpio/meson.build
index 86cae9a0f3..258bd5dcfc 100644
--- a/hw/gpio/meson.build
+++ b/hw/gpio/meson.build
@@ -11,3 +11,5 @@ softmmu_ss.add(when: 'CONFIG_OMAP', if_true: files('omap_gpio.c'))
 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'))
diff --git a/include/hw/gpio/avr_gpio.h b/include/hw/gpio/avr_gpio.h
new file mode 100644
index 0000000000..84d783f8fc
--- /dev/null
+++ b/include/hw/gpio/avr_gpio.h
@@ -0,0 +1,46 @@
+/*
+ * 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)
+
+struct AVRGPIOState {
+    /*< private >*/
+    SysBusDevice parent_obj;
+
+    /*< public >*/
+    MemoryRegion mmio;
+
+    uint8_t ddr_val;
+    uint8_t port_val;
+
+};
+
+#endif /* AVR_GPIO_H */
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] hw/avr: Add limited support for avr gpio registers
  2020-10-03 12:38 [PATCH v2] hw/avr: Add limited support for avr gpio registers Heecheol Yang
@ 2020-10-03 17:08 ` Michael Rolnik
  2020-10-03 18:03 ` Philippe Mathieu-Daudé
  1 sibling, 0 replies; 5+ messages in thread
From: Michael Rolnik @ 2020-10-03 17:08 UTC (permalink / raw)
  To: Heecheol Yang; +Cc: Sarah Harris, QEMU Developers

[-- Attachment #1: Type: text/plain, Size: 8577 bytes --]

On Sat, Oct 3, 2020 at 3:39 PM Heecheol Yang <heecheol.yang@outlook.com>
wrote:

> Add some of these features for avr gpio:
>
>   - GPIO I/O : PORTx registers
>   - Data Direction : DDRx registers
>
> Following things are not supported yet:
>   - PINx registers
>   - MCUR registers
>   - Even though read/write for DDRx registers are
>     implemented, actual direction controls are not
>     supported yet.
>
> Signed-off-by: Heecheol Yang <heecheol.yang@outlook.com>
> ---
>  hw/avr/Kconfig             |   1 +
>  hw/avr/atmega.c            |   7 ++-
>  hw/avr/atmega.h            |   2 +
>  hw/gpio/Kconfig            |   3 +
>  hw/gpio/avr_gpio.c         | 112 +++++++++++++++++++++++++++++++++++++
>  hw/gpio/meson.build        |   2 +
>  include/hw/gpio/avr_gpio.h |  46 +++++++++++++++
>  7 files changed, 171 insertions(+), 2 deletions(-)
>  create mode 100644 hw/gpio/avr_gpio.c
>  create mode 100644 include/hw/gpio/avr_gpio.h
>
> diff --git a/hw/avr/Kconfig b/hw/avr/Kconfig
> index d31298c3cc..16a57ced11 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/avr/atmega.c b/hw/avr/atmega.c
> index 44c6afebbb..ad942028fd 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/avr/atmega.h b/hw/avr/atmega.h
> index a99ee15c7e..e2289d5744 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/hw/gpio/Kconfig b/hw/gpio/Kconfig
> index b6fdaa2586..1752d0ce56 100644
> --- a/hw/gpio/Kconfig
> +++ b/hw/gpio/Kconfig
> @@ -10,3 +10,6 @@ config GPIO_KEY
>
>  config SIFIVE_GPIO
>      bool
> +
> +config AVR_GPIO
> +    bool
> diff --git a/hw/gpio/avr_gpio.c b/hw/gpio/avr_gpio.c
> new file mode 100644
> index 0000000000..6ca8e8703a
> --- /dev/null
> +++ b/hw/gpio/avr_gpio.c
> @@ -0,0 +1,112 @@
> +/*
> + * 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 "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->ddr_val = 0u;
> +    gpio->port_val = 0u;
> +}
> +static uint64_t avr_gpio_read(void *opaque, hwaddr offset, unsigned int
> size)
> +{
> +    AVRGPIOState *s = (AVRGPIOState *)opaque;
> +    switch (offset) {
> +    case GPIO_PIN:
> +        /* Not implemented yet */
> +        break;
> +    case GPIO_DDR:
> +        return s->ddr_val;
>
Why do you need `break` after `return` ?

> +        break;
> +    case GPIO_PORT:
> +        return s->port_val;
> +    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;
> +    switch (offset) {
> +    case GPIO_PIN:
> +        /* Not implemented yet */
> +        break;
> +    case GPIO_DDR:
> +        s->ddr_val = value & 0xF;
> +        break;
> +    case GPIO_PORT:
> +        s->port_val = value & 0xF;
> +        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);
> +    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)
> +{
> +    avr_gpio_reset(dev);
> +}
> +
> +
> +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/gpio/meson.build b/hw/gpio/meson.build
> index 86cae9a0f3..258bd5dcfc 100644
> --- a/hw/gpio/meson.build
> +++ b/hw/gpio/meson.build
> @@ -11,3 +11,5 @@ softmmu_ss.add(when: 'CONFIG_OMAP', if_true:
> files('omap_gpio.c'))
>  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'))
> diff --git a/include/hw/gpio/avr_gpio.h b/include/hw/gpio/avr_gpio.h
> new file mode 100644
> index 0000000000..84d783f8fc
> --- /dev/null
> +++ b/include/hw/gpio/avr_gpio.h
> @@ -0,0 +1,46 @@
> +/*
> + * 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)
> +
> +struct AVRGPIOState {
> +    /*< private >*/
> +    SysBusDevice parent_obj;
> +
> +    /*< public >*/
> +    MemoryRegion mmio;
> +
> +    uint8_t ddr_val;
> +    uint8_t port_val;
> +
> +};
> +
> +#endif /* AVR_GPIO_H */
> --
> 2.17.1
>
>

-- 
Best Regards,
Michael Rolnik

[-- Attachment #2: Type: text/html, Size: 11019 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] hw/avr: Add limited support for avr gpio registers
  2020-10-03 12:38 [PATCH v2] hw/avr: Add limited support for avr gpio registers Heecheol Yang
  2020-10-03 17:08 ` Michael Rolnik
@ 2020-10-03 18:03 ` Philippe Mathieu-Daudé
  2020-10-03 22:02   ` Hee-cheol Yang
  1 sibling, 1 reply; 5+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-10-03 18:03 UTC (permalink / raw)
  To: Heecheol Yang, qemu-devel; +Cc: Sarah Harris, Michael Rolnik

On 10/3/20 2:38 PM, Heecheol Yang wrote:
> Add some of these features for avr gpio:
> 
>   - GPIO I/O : PORTx registers
>   - Data Direction : DDRx registers
> 
> Following things are not supported yet:
>   - PINx registers
>   - MCUR registers
>   - Even though read/write for DDRx registers are
>     implemented, actual direction controls are not
>     supported yet.
> 
> Signed-off-by: Heecheol Yang <heecheol.yang@outlook.com>
> ---
>  hw/avr/Kconfig             |   1 +
>  hw/avr/atmega.c            |   7 ++-
>  hw/avr/atmega.h            |   2 +
>  hw/gpio/Kconfig            |   3 +
>  hw/gpio/avr_gpio.c         | 112 +++++++++++++++++++++++++++++++++++++
>  hw/gpio/meson.build        |   2 +
>  include/hw/gpio/avr_gpio.h |  46 +++++++++++++++
>  7 files changed, 171 insertions(+), 2 deletions(-)
>  create mode 100644 hw/gpio/avr_gpio.c
>  create mode 100644 include/hw/gpio/avr_gpio.h

FYI this one is posted correctly, I can read it and patchew
successfully applied it:
https://patchew.org/QEMU/DM6PR16MB2473C5A77E009CA2FEF3D8ECE60E0@DM6PR16MB2473.namprd16.prod.outlook.com/

But v3 is broken again...


^ permalink raw reply	[flat|nested] 5+ messages in thread

* RE: [PATCH v2] hw/avr: Add limited support for avr gpio registers
  2020-10-03 18:03 ` Philippe Mathieu-Daudé
@ 2020-10-03 22:02   ` Hee-cheol Yang
  0 siblings, 0 replies; 5+ messages in thread
From: Hee-cheol Yang @ 2020-10-03 22:02 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel, Hee-cheol Yang
  Cc: Sarah Harris, Michael Rolnik

[-- Attachment #1: Type: text/plain, Size: 1813 bytes --]

Thanks for your review.
Actually, the very recent v2 one is v4. There was an typo while I was writing multiple mails for successful patchew builds(v2 and v3 has the same contents) .. Sorry.

If needed, let me know to write the new titled version of mail which contains the same contents.



-------- 원본 이메일 --------
발신: Philippe Mathieu-Daudé <f4bug@amsat.org>
날짜: 20/10/4 오전 3:03 (GMT+09:00)
받은 사람: Heecheol Yang <heecheol.yang@outlook.com>, qemu-devel@nongnu.org
참조: Sarah Harris <S.E.Harris@kent.ac.uk>, Michael Rolnik <mrolnik@gmail.com>
제목: Re: [PATCH v2] hw/avr: Add limited support for avr gpio registers

On 10/3/20 2:38 PM, Heecheol Yang wrote:
> Add some of these features for avr gpio:
>
>   - GPIO I/O : PORTx registers
>   - Data Direction : DDRx registers
>
> Following things are not supported yet:
>   - PINx registers
>   - MCUR registers
>   - Even though read/write for DDRx registers are
>     implemented, actual direction controls are not
>     supported yet.
>
> Signed-off-by: Heecheol Yang <heecheol.yang@outlook.com>
> ---
>  hw/avr/Kconfig             |   1 +
>  hw/avr/atmega.c            |   7 ++-
>  hw/avr/atmega.h            |   2 +
>  hw/gpio/Kconfig            |   3 +
>  hw/gpio/avr_gpio.c         | 112 +++++++++++++++++++++++++++++++++++++
>  hw/gpio/meson.build        |   2 +
>  include/hw/gpio/avr_gpio.h |  46 +++++++++++++++
>  7 files changed, 171 insertions(+), 2 deletions(-)
>  create mode 100644 hw/gpio/avr_gpio.c
>  create mode 100644 include/hw/gpio/avr_gpio.h

FYI this one is posted correctly, I can read it and patchew
successfully applied it:
https://patchew.org/QEMU/DM6PR16MB2473C5A77E009CA2FEF3D8ECE60E0@DM6PR16MB2473.namprd16.prod.outlook.com/

But v3 is broken again...

[-- Attachment #2: Type: text/html, Size: 3352 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v2] hw/avr: Add limited support for avr gpio registers
  2020-10-02 15:48 ` Philippe Mathieu-Daudé
@ 2020-10-03 11:25   ` Hee-cheol Yang
  0 siblings, 0 replies; 5+ messages in thread
From: Hee-cheol Yang @ 2020-10-03 11:25 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: S.E.Harris, mrolnik, Hee-cheol Yang

[-- Attachment #1: Type: text/plain, Size: 19398 bytes --]

Hello, Thank you very much for your prompt and kind review.
This is my next version of the patch. Here are what I changed from the previous one:


  *   Remove unnecessary header inclusions
  *   Replace codes for unreachable conditions with g_assert_not_reached() function
  *   Remove 'enable' field from AVRGPIOState structure: It is actually unnecessary. I copied this field from AVRUSARTState structure.

I am afraid that there is an encoding issue even though I tried to do it correctly.
Sorry in advance for this issue again.

With best regards
Heecheol Yang.

diff --git a/hw/avr/Kconfig b/hw/avr/Kconfig
index d31298c3cc..16a57ced11 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/avr/atmega.c b/hw/avr/atmega.c
index 44c6afebbb..ad942028fd 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/avr/atmega.h b/hw/avr/atmega.h
index a99ee15c7e..e2289d5744 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/hw/gpio/Kconfig b/hw/gpio/Kconfig
index b6fdaa2586..1752d0ce56 100644
--- a/hw/gpio/Kconfig
+++ b/hw/gpio/Kconfig
@@ -10,3 +10,6 @@ config GPIO_KEY

 config SIFIVE_GPIO
     bool
+
+config AVR_GPIO
+    bool
diff --git a/hw/gpio/avr_gpio.c b/hw/gpio/avr_gpio.c
new file mode 100644
index 0000000000..6ca8e8703a
--- /dev/null
+++ b/hw/gpio/avr_gpio.c
@@ -0,0 +1,112 @@
+/*
+ * 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 "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->ddr_val = 0u;
+    gpio->port_val = 0u;
+}
+static uint64_t avr_gpio_read(void *opaque, hwaddr offset, unsigned int size)
+{
+    AVRGPIOState *s = (AVRGPIOState *)opaque;
+    switch (offset) {
+    case GPIO_PIN:
+        /* Not implemented yet */
+        break;
+    case GPIO_DDR:
+        return s->ddr_val;
+        break;
+    case GPIO_PORT:
+        return s->port_val;
+    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;
+    switch (offset) {
+    case GPIO_PIN:
+        /* Not implemented yet */
+        break;
+    case GPIO_DDR:
+        s->ddr_val = value & 0xF;
+        break;
+    case GPIO_PORT:
+        s->port_val = value & 0xF;
+        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);
+    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)
+{
+    avr_gpio_reset(dev);
+}
+
+
+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/gpio/meson.build b/hw/gpio/meson.build
index 86cae9a0f3..258bd5dcfc 100644
--- a/hw/gpio/meson.build
+++ b/hw/gpio/meson.build
@@ -11,3 +11,5 @@ softmmu_ss.add(when: 'CONFIG_OMAP', if_true: files('omap_gpio.c'))
 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'))
diff --git a/include/hw/gpio/avr_gpio.h b/include/hw/gpio/avr_gpio.h
new file mode 100644
index 0000000000..84d783f8fc
--- /dev/null
+++ b/include/hw/gpio/avr_gpio.h
@@ -0,0 +1,46 @@
+/*
+ * 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)
+
+struct AVRGPIOState {
+    /*< private >*/
+    SysBusDevice parent_obj;
+
+    /*< public >*/
+    MemoryRegion mmio;
+
+    uint8_t ddr_val;
+    uint8_t port_val;
+
+};
+
+#endif /* AVR_GPIO_H */

________________________________
보낸 사람: Philippe Mathieu-Daudé <f4bug@amsat.org> 대신 Philippe Mathieu-Daudé <philippe.mathieu.daude@gmail.com>
보낸 날짜: 2020년 10월 3일 토요일 오전 12:48
받는 사람: Heecheol Yang <heecheol.yang@outlook.com>; qemu-devel@nongnu.org <qemu-devel@nongnu.org>
참조: S.E.Harris@kent.ac.uk <S.E.Harris@kent.ac.uk>; mrolnik@gmail.com <mrolnik@gmail.com>
제목: Re: [PATCH] hw/avr: Add limited support for avr gpio registers

Hi Yang,

On 10/2/20 5:24 PM, Heecheol Yang wrote:
> Add some of these features for avr gpio:
>
>  Â  - GPIO I/O : PORTx registers
>  Â  - Data Direction : DDRx registers
>
> Following things are not supported yet:
>  Â  - PINx registers
>  Â  - MCUR registers
>  Â  - Even though read/write for DDRx registers are
>  Â Â Â  implemented, actual direction controls are not
>  Â Â Â  supported yet.

Thanks for your patch, however its encoding seems
completely broken, and I can not test it. Similar
problem occurred to the patchew automatic script:

https://patchew.org/QEMU/DM6PR16MB24737F911BD260F1FA8EBC37E6310@DM6PR16MB2473.namprd16.prod.outlook.com/

This might be useful:
https://wiki.qemu.org/Contribute/SubmitAPatch#Use_git_format-patch

Anyway I'll try to review what I can follow.

>
> Signed-off-by: Heecheol Yang <heecheol.yang@outlook.com>
> ---
>   hw/avr/Kconfig             |   1 +
>   hw/avr/atmega.c            |   7 ++-
>   hw/avr/atmega.h            |   2 +
>   hw/gpio/Kconfig            |   3 +
>   hw/gpio/avr_gpio.c         | 117
+++++++++++++++++++++++++++++++++++++
>   hw/gpio/meson.build        |   2 +
>  Â include/hw/gpio/avr_gpio.h |Â  48 +++++++++++++++
>  Â 7 files changed, 178 insertions(+), 2 deletions(-)
>  Â create mode 100644 hw/gpio/avr_gpio.c
>  Â create mode 100644 include/hw/gpio/avr_gpio.h
>
> diff --git a/hw/avr/Kconfig b/hw/avr/Kconfig
> index d31298c3cc..16a57ced11 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/avr/atmega.c b/hw/avr/atmega.c
> index 44c6afebbb..ad942028fd 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/avr/atmega.h b/hw/avr/atmega.h
> index a99ee15c7e..e2289d5744 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/hw/gpio/Kconfig b/hw/gpio/Kconfig
> index b6fdaa2586..1752d0ce56 100644
> --- a/hw/gpio/Kconfig
> +++ b/hw/gpio/Kconfig
> @@ -10,3 +10,6 @@ config GPIO_KEY
>
>  Â config SIFIVE_GPIO
>  Â Â Â Â  bool
> +
> +config AVR_GPIO
> +Â Â Â  bool
> diff --git a/hw/gpio/avr_gpio.c b/hw/gpio/avr_gpio.c
> new file mode 100644
> index 0000000000..7114216847
> --- /dev/null
> +++ b/hw/gpio/avr_gpio.c
> @@ -0,0 +1,117 @@
> +/*
> + * 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/timer.h"

"qemu/timer.h" not needed.

> +#include "qapi/error.h"
> +#include "hw/sysbus.h"
> +#include "hw/irq.h"
> +#include "hw/gpio/avr_gpio.h"
> +#include "hw/qdev-properties.h"
> +#include "chardev/char-fe.h"

"chardev/char-fe.h" not needed.

> +
> +static void avr_gpio_reset(DeviceState *dev)
> +{
> +Â Â Â  AVRGPIOState *gpio = AVR_GPIO(dev);
> +Â Â Â  gpio->ddr_val = 0u;
> +Â Â Â  gpio->port_val = 0u;
> +}
> +static uint64_t avr_gpio_read(void *opaque, hwaddr offset, unsigned int
> size)
> +{
> +Â Â Â  AVRGPIOState *s = (AVRGPIOState *)opaque;
> +Â Â Â  switch (offset) {
> +Â Â Â  case GPIO_PIN:
> +Â Â Â Â Â Â Â  /* Not implemented yet */

Please use:

   qemu_log_mask(LOG_UNIMP, ...

> +Â Â Â Â Â Â Â  break;
> +Â Â Â  case GPIO_DDR:
> +Â Â Â Â Â Â Â  return s->ddr_val;
> +Â Â Â Â Â Â Â  break;
> +Â Â Â  case GPIO_PORT:
> +Â Â Â Â Â Â Â  return s->port_val;
> +Â Â Â  default:
> +Â Â Â Â Â Â Â  qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad offset %lx\n",
> +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â  __func__, offset);
> +Â Â Â Â Â Â Â  break;

This can not happen, so I'd simply use:

     g_assert_not_reached();

> +Â Â Â  }
> +Â Â Â  return 0;
> +}
> +
> +static void avr_gpio_write(void *opaque, hwaddr offset, uint64_t value,
> +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â
unsigned int size)
> +{
> +Â Â Â  AVRGPIOState *s = (AVRGPIOState *)opaque;
> +Â Â Â  switch (offset) {
> +Â Â Â  case GPIO_PIN:
> +Â Â Â Â Â Â Â  /* Not implemented yet */
> +Â Â Â Â Â Â Â  break;
> +Â Â Â  case GPIO_DDR:
> +Â Â Â Â Â Â Â  s->ddr_val = value & 0xF;
> +Â Â Â Â Â Â Â  break;
> +Â Â Â  case GPIO_PORT:
> +Â Â Â Â Â Â Â  s->port_val = value & 0xF;
> +Â Â Â Â Â Â Â  break;
> +Â Â Â  default:
> +Â Â Â Â Â Â Â  qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad offset %lx\n",
> +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â  __func__, offset);
> +Â Â Â Â Â Â Â  break;
> +Â Â Â  }

Similar comments than avr_gpio_read().

> +}
> +
> +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);
> +Â Â Â  memory_region_init_io(&s->mmio, obj, &avr_gpio_ops, s,
> TYPE_AVR_GPIO, 3);
> +Â Â Â  sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->mmio);
> +Â Â Â  s->enabled = true;
> +}
> +static void avr_gpio_realize(DeviceState *dev, Error **errp)
> +{
> +Â Â Â  avr_gpio_reset(dev);
> +}
> +
> +
> +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/gpio/meson.build b/hw/gpio/meson.build
> index 86cae9a0f3..258bd5dcfc 100644
> --- a/hw/gpio/meson.build
> +++ b/hw/gpio/meson.build
> @@ -11,3 +11,5 @@ softmmu_ss.add(when: 'CONFIG_OMAP', if_true:
> files('omap_gpio.c'))
>  Â 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'))
> diff --git a/include/hw/gpio/avr_gpio.h b/include/hw/gpio/avr_gpio.h
> new file mode 100644
> index 0000000000..45d42753c8
> --- /dev/null
> +++ b/include/hw/gpio/avr_gpio.h
> @@ -0,0 +1,48 @@
> +/*
> + * 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"
> +#include "chardev/char-fe.h"

"chardev/char-fe.h" not needed.

> +
> +/* 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)
> +
> +struct AVRGPIOState {
> +Â Â Â  /*< private >*/
> +Â Â Â  SysBusDevice parent_obj;
> +
> +Â Â Â  /*< public >*/
> +Â Â Â  MemoryRegion mmio;
> +
> +Â Â Â  uint8_t ddr_val;
> +Â Â Â  uint8_t port_val;
> +Â Â Â  bool enabled;

What is 'enabled' for?

> +
> +};
> +
> +#endif /* AVR_GPIO_H */
> --
> 2.17.1

Regards,

Phil.

[-- Attachment #2: Type: text/html, Size: 28891 bytes --]

^ permalink raw reply related	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2020-10-04 19:42 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-03 12:38 [PATCH v2] hw/avr: Add limited support for avr gpio registers Heecheol Yang
2020-10-03 17:08 ` Michael Rolnik
2020-10-03 18:03 ` Philippe Mathieu-Daudé
2020-10-03 22:02   ` Hee-cheol Yang
  -- strict thread matches above, loose matches on Subject: below --
2020-10-02 15:24 [PATCH] " Heecheol Yang
2020-10-02 15:48 ` Philippe Mathieu-Daudé
2020-10-03 11:25   ` [PATCH v2] " Hee-cheol Yang

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.