Re: [Qemu-devel] [RFC v2 1/2] arm: Add nRF51 GPIO peripheral

[Stefan Hajnoczi]

On Mon, Jul 16, 2018 at 12:47:43PM +0200, Steffen Görtz wrote:
> Signed-off-by: Steffen Görtz <address@hidden>
> ---
>  Changes in v2:
>    - Only call QEMU GPIO update handlers if value changes
>    - Code style changes
>    - Removed unused includes
> 
>  hw/gpio/Makefile.objs        |   1 +
>  hw/gpio/nrf51_gpio.c         | 320 +++++++++++++++++++++++++++++++++++
>  include/hw/gpio/nrf51_gpio.h |  57 +++++++
>  3 files changed, 378 insertions(+)
>  create mode 100644 hw/gpio/nrf51_gpio.c
>  create mode 100644 include/hw/gpio/nrf51_gpio.h

Hi Steffen,
Sorry I didn't see this patch series.  Please email address@hidden
in the future.  The way I have GMail set up means qemu-devel threads
with address@hidden CCed are not brought to my attention immediately.

I have checked the nRF51 GPIO documentation and your implementation
looks good.

> +static void gpio_write(void *opaque, hwaddr offset,
> +                       uint64_t value, unsigned int size)
> +{
> +    Nrf51GPIOState *s = NRF51_GPIO(opaque);
> +    size_t idx;
> +
> +    switch (offset) {
> +    case NRF51_GPIO_REG_OUT:
> +        DPRINTF("write out=0x%" PRIx32 "\n", (uint32_t)value);
> +        s->out = value;
> +        gpio_update_state(s);
> +        break;
> +    case NRF51_GPIO_REG_OUTSET:
> +        DPRINTF("set out=0x%" PRIx32 "\n", (uint32_t)value);
> +        s->out |= value;
> +        gpio_update_state(s);
> +        break;
> +    case NRF51_GPIO_REG_OUTCLR:
> +        DPRINTF("clr out=0x%" PRIx32 "\n", (uint32_t)value);
> +        s->out &= ~value;
> +        gpio_update_state(s);
> +        break;
> +    case NRF51_GPIO_REG_DIR:
> +        DPRINTF("write dir=0x%" PRIx32 "\n", (uint32_t)value);
> +        s->dir = value;
> +        /* direction is exposed in both the DIR register and the DIR bit
> +         * of each PINs CNF configuration register. */
> +        for (size_t i = 0; i < NRF51_GPIO_PINS; i++) {
> +            s->cnf[i] = (s->cnf[i] & ~(1UL)) | ((value >> i) & 0x01);
> +        }
> +        gpio_update_state(s);
> +        break;

Missing DIRSET and DIRCLR?

signature.asc
Description: PGP signature