On 2 November 2018 at 17:07, Steffen Görtz <cont...@steffen-goertz.de> wrote: > This adds a model of the nRF51 GPIO peripheral. > > Reference Manual: http://infocenter.nordicsemi.com/pdf/nRF51_RM_v3.0.pdf > > The nRF51 series microcontrollers support up to 32 GPIO pins in various > configurations. > The pins can be used as input pins with pull-ups or pull-down. > Furthermore, three different output driver modes per level are > available (disconnected, standard, high-current). > > The GPIO-Peripheral has a mechanism for detecting level changes which is > not featured in this model. > > Signed-off-by: Steffen Görtz <cont...@steffen-goertz.de> > Reviewed-by: Stefan Hajnoczi <stefa...@redhat.com>
Hi; I just had a few minor nits here... > --- > Makefile.objs | 1 + > hw/gpio/Makefile.objs | 1 + > hw/gpio/nrf51_gpio.c | 300 +++++++++++++++++++++++++++++++++++ > hw/gpio/trace-events | 7 + > include/hw/gpio/nrf51_gpio.h | 69 ++++++++ > 5 files changed, 378 insertions(+) > create mode 100644 hw/gpio/nrf51_gpio.c > create mode 100644 hw/gpio/trace-events > create mode 100644 include/hw/gpio/nrf51_gpio.h > > diff --git a/Makefile.objs b/Makefile.objs > index 1e1ff387d7..fbc3bad1e1 100644 > --- a/Makefile.objs > +++ b/Makefile.objs > @@ -243,6 +243,7 @@ trace-events-subdirs += hw/vfio > trace-events-subdirs += hw/virtio > trace-events-subdirs += hw/watchdog > trace-events-subdirs += hw/xen > +trace-events-subdirs += hw/gpio > trace-events-subdirs += io > trace-events-subdirs += linux-user > trace-events-subdirs += migration > diff --git a/hw/gpio/Makefile.objs b/hw/gpio/Makefile.objs > index fa0a72e6d0..e5da0cb54f 100644 > --- a/hw/gpio/Makefile.objs > +++ b/hw/gpio/Makefile.objs > @@ -8,3 +8,4 @@ common-obj-$(CONFIG_GPIO_KEY) += gpio_key.o > obj-$(CONFIG_OMAP) += omap_gpio.o > obj-$(CONFIG_IMX) += imx_gpio.o > obj-$(CONFIG_RASPI) += bcm2835_gpio.o > +obj-$(CONFIG_NRF51_SOC) += nrf51_gpio.o > diff --git a/hw/gpio/nrf51_gpio.c b/hw/gpio/nrf51_gpio.c > new file mode 100644 > index 0000000000..0a378e03ab > --- /dev/null > +++ b/hw/gpio/nrf51_gpio.c > @@ -0,0 +1,300 @@ > +/* > + * nRF51 System-on-Chip general purpose input/output register definition > + * > + * Reference Manual: http://infocenter.nordicsemi.com/pdf/nRF51_RM_v3.0.pdf > + * Product Spec: http://infocenter.nordicsemi.com/pdf/nRF51822_PS_v3.1.pdf > + * > + * Copyright 2018 Steffen Görtz <cont...@steffen-goertz.de> > + * > + * This code is licensed under the GPL version 2 or later. See > + * the COPYING file in the top-level directory. > + */ > + > +#include "qemu/osdep.h" > +#include "qemu/log.h" > +#include "hw/gpio/nrf51_gpio.h" > +#include "trace.h" > + > +/* > + * Check if the output driver is connected to the direction switch > + * given the current configuration and logic level. > + * It is not differentiated between standard and "high"(-power) drive modes. > + */ > +static bool is_connected(uint32_t config, uint32_t level) > +{ > + bool state; > + uint32_t drive_config = extract32(config, 8, 3); > + > + switch (drive_config) { > + case 0 ... 3: > + state = true; > + break; > + case 4 ... 5: > + state = level != 0; > + break; > + case 6 ... 7: > + state = level == 0; > + break; > + default: > + /* Some compilers can not infer the value range of extract32(.., 3) > */ Usually we handle this with g_assert_not_reached(). > + state = false; > + break; > + } > + > + return state; > +} > +static void nrf51_gpio_write(void *opaque, hwaddr offset, > + uint64_t value, unsigned int size) > +{ > + NRF51GPIOState *s = NRF51_GPIO(opaque); > + size_t idx; > + > + trace_nrf51_gpio_write(offset, value); > + > + switch (offset) { > + case NRF51_GPIO_REG_OUT: > + s->out = value; > + break; > + > + case NRF51_GPIO_REG_OUTSET: > + s->out |= value; > + break; > + > + case NRF51_GPIO_REG_OUTCLR: > + s->out &= ~value; > + break; > + > + case NRF51_GPIO_REG_DIR: > + s->dir = value; > + reflect_dir_bit_in_cnf(s); > + break; > + > + case NRF51_GPIO_REG_DIRSET: > + s->dir |= value; > + reflect_dir_bit_in_cnf(s); > + break; > + > + case NRF51_GPIO_REG_DIRCLR: > + s->dir &= ~value; > + reflect_dir_bit_in_cnf(s); > + break; > + > + case NRF51_GPIO_REG_CNF_START ... NRF51_GPIO_REG_CNF_END: > + idx = (offset - NRF51_GPIO_REG_CNF_START) / 4; > + s->cnf[idx] = value; > + /* direction is exposed in both the DIR register and the DIR bit > + * of each PINs CNF configuration register. */ Nonstandard multiline comment format; see CODING_STYLE for the preferred form. > + s->dir = (s->dir & ~(1UL << idx)) | ((value & 0x01) << idx); > + break; > + > + default: > + qemu_log_mask(LOG_GUEST_ERROR, > + "%s: bad write offset 0x%" HWADDR_PRIx "\n", > + __func__, offset); > + } > + > + update_state(s); > +} > + > +static const VMStateDescription vmstate_nrf51_gpio = { > + .name = TYPE_NRF51_GPIO, > + .version_id = 1, > + .minimum_version_id = 1, > + .minimum_version_id_old = 1, You don't need to specify minimum_version_id_old unless you provide a load_state_old handler. > + .fields = (VMStateField[]) { > + VMSTATE_UINT32(out, NRF51GPIOState), > + VMSTATE_UINT32(in, NRF51GPIOState), > + VMSTATE_UINT32(in_mask, NRF51GPIOState), > + VMSTATE_UINT32(dir, NRF51GPIOState), > + VMSTATE_UINT32_ARRAY(cnf, NRF51GPIOState, NRF51_GPIO_PINS), > + VMSTATE_UINT32(old_out, NRF51GPIOState), > + VMSTATE_UINT32(old_out_connected, NRF51GPIOState), > + VMSTATE_END_OF_LIST() > + } > +}; Otherwise Reviewed-by: Peter Maydell <peter.mayd...@linaro.org> thanks -- PMM