From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38393) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aaMqh-0006UP-Lw for qemu-devel@nongnu.org; Mon, 29 Feb 2016 07:22:53 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aaMqb-0004e2-HQ for qemu-devel@nongnu.org; Mon, 29 Feb 2016 07:22:51 -0500 Received: from mail-wm0-x22b.google.com ([2a00:1450:400c:c09::22b]:37444) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aaMqb-0004dx-1X for qemu-devel@nongnu.org; Mon, 29 Feb 2016 07:22:45 -0500 Received: by mail-wm0-x22b.google.com with SMTP id p65so42926857wmp.0 for ; Mon, 29 Feb 2016 04:22:44 -0800 (PST) References: From: Alex =?utf-8?Q?Benn=C3=A9e?= In-reply-to: Date: Mon, 29 Feb 2016 12:22:42 +0000 Message-ID: <87egbvk8od.fsf@linaro.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH v4 14/16] register: Add GPIO API List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alistair Francis Cc: edgar.iglesias@xilinx.com, peter.maydell@linaro.org, qemu-devel@nongnu.org, crosthwaitepeter@gmail.com, edgar.iglesias@gmail.com, afaerber@suse.de, fred.konrad@greensocs.com Alistair Francis writes: > From: Peter Crosthwaite > > Add GPIO functionality to the register API. This allows association > and automatic connection of GPIOs to bits in registers. GPIO inputs > will attach to handlers that automatically set read-only bits in > registers. GPIO outputs will be updated to reflect their field value > when their respective registers are written (or reset). Supports > active low GPIOs. > > This is particularly effective for implementing system level > controllers, where heterogenous collections of control signals are > placed is a SoC specific peripheral then propagated all over the > system. > > Signed-off-by: Peter Crosthwaite > [ EI Changes: > * register: Add a polarity field to GPIO connections > Makes it possible to directly connect active low signals > to generic interrupt pins. > ] > Signed-off-by: Edgar E. Iglesias > Signed-off-by: Alistair Francis > --- > > hw/core/register.c | 96 +++++++++++++++++++++++++++++++++++++++++++++++++++ > include/hw/register.h | 37 ++++++++++++++++++++ > 2 files changed, 133 insertions(+) > > diff --git a/hw/core/register.c b/hw/core/register.c > index ac866f6..1dc7ccc 100644 > --- a/hw/core/register.c > +++ b/hw/core/register.c > @@ -106,6 +106,7 @@ void register_write(RegisterInfo *reg, uint64_t val, uint64_t we) > } > > register_write_val(reg, new_val); > + register_refresh_gpios(reg, old_val); > > if (ac->post_write) { > ac->post_write(reg, new_val); > @@ -147,19 +148,113 @@ void register_reset(RegisterInfo *reg) > g_assert(reg); > g_assert(reg->data); > g_assert(reg->access); > + uint64_t old_val; > + > + old_val = register_read_val(reg); > > register_write_val(reg, reg->access->reset); > + register_refresh_gpios(reg, old_val); > +} > + > +void register_refresh_gpios(RegisterInfo *reg, uint64_t old_value) > +{ > + const RegisterAccessInfo *ac; > + const RegisterGPIOMapping *gpio; > + > + ac = reg->access; > + for (gpio = ac->gpios; gpio && gpio->name; gpio++) { > + int i; > + > + if (gpio->input) { > + continue; > + } > + > + for (i = 0; i < gpio->num; ++i) { > + uint64_t gpio_value, gpio_value_old; > + > + qemu_irq gpo = qdev_get_gpio_out_named(DEVICE(reg), gpio->name, i); > + gpio_value_old = extract64(old_value, > + gpio->bit_pos + i * gpio->width, > + gpio->width) ^ gpio->polarity; > + gpio_value = extract64(register_read_val(reg), > + gpio->bit_pos + i * gpio->width, > + gpio->width) ^ gpio->polarity; > + if (!(gpio_value_old ^ gpio_value)) { > + continue; > + } > + if (reg->debug && gpo) { > + qemu_log("refreshing gpio out %s to %" PRIx64 "\n", > + gpio->name, gpio_value); > + } > + qemu_set_irq(gpo, gpio_value); > + } > + } > +} > + > +typedef struct DeviceNamedGPIOHandlerOpaque { > + DeviceState *dev; > + const char *name; > +} DeviceNamedGPIOHandlerOpaque; > + > +static void register_gpio_handler(void *opaque, int n, int level) > +{ > + DeviceNamedGPIOHandlerOpaque *gho = opaque; > + RegisterInfo *reg = REGISTER(gho->dev); > + > + const RegisterAccessInfo *ac; > + const RegisterGPIOMapping *gpio; > + > + ac = reg->access; > + for (gpio = ac->gpios; gpio && gpio->name; gpio++) { > + if (gpio->input && !strcmp(gho->name, gpio->name)) { > + register_write_val(reg, deposit64(register_read_val(reg), > + gpio->bit_pos + n * gpio->width, > + gpio->width, > + level ^ gpio->polarity)); > + return; > + } > + } > + > + abort(); > } > > void register_init(RegisterInfo *reg) > { > assert(reg); > + const RegisterAccessInfo *ac; > + const RegisterGPIOMapping *gpio; > > if (!reg->data || !reg->access) { > return; > } > > object_initialize((void *)reg, sizeof(*reg), TYPE_REGISTER); > + > + ac = reg->access; > + for (gpio = ac->gpios; gpio && gpio->name; gpio++) { > + if (!gpio->num) { > + ((RegisterGPIOMapping *)gpio)->num = 1; > + } > + if (!gpio->width) { > + ((RegisterGPIOMapping *)gpio)->width = 1; > + } > + if (gpio->input) { > + DeviceNamedGPIOHandlerOpaque gho = { > + .name = gpio->name, > + .dev = DEVICE(reg), > + }; > + qemu_irq irq; > + > + qdev_init_gpio_in_named(DEVICE(reg), register_gpio_handler, > + gpio->name, gpio->num); > + irq = qdev_get_gpio_in_named(DEVICE(reg), gpio->name, gpio->num); > + qemu_irq_set_opaque(irq, g_memdup(&gho, sizeof(gho))); > + } else { > + qemu_irq *gpos = g_new0(qemu_irq, gpio->num); > + > + qdev_init_gpio_out_named(DEVICE(reg), gpos, gpio->name, gpio->num); > + } > + } > } > > static inline void register_write_memory(void *opaque, hwaddr addr, > @@ -231,6 +326,7 @@ void register_init_block32(DeviceState *owner, const RegisterAccessInfo *rae, > .opaque = owner, > }; > register_init(r); > + qdev_pass_all_gpios(DEVICE(r), owner); > > memory_region_init_io(&r->mem, OBJECT(owner), ops, r, r->access->name, > sizeof(uint32_t)); > diff --git a/include/hw/register.h b/include/hw/register.h > index 6ac005c..4d284a9 100644 > --- a/include/hw/register.h > +++ b/include/hw/register.h > @@ -13,11 +13,35 @@ > > #include "hw/qdev-core.h" > #include "exec/memory.h" > +#include "hw/irq.h" > > typedef struct RegisterInfo RegisterInfo; > typedef struct RegisterAccessInfo RegisterAccessInfo; > > /** > + * A register access error message > + * @mask: Bits in the register the error applies to > + * @reason: Reason why this access is an error > + */ > + > +typedef struct RegisterAccessError { > + uint64_t mask; > + const char *reason; > +} RegisterAccessError; This seems out of place here. It's not used by any of the code. > + > +#define REG_GPIO_POL_HIGH 0 > +#define REG_GPIO_POL_LOW 1 > + > +typedef struct RegisterGPIOMapping { > + const char *name; > + uint8_t bit_pos; > + bool input; > + bool polarity; > + uint8_t num; > + uint8_t width; > +} RegisterGPIOMapping; > + > +/** > * Access description for a register that is part of guest accessible device > * state. > * > @@ -61,6 +85,8 @@ struct RegisterAccessInfo { > > uint64_t (*post_read)(RegisterInfo *reg, uint64_t val); > > + const RegisterGPIOMapping *gpios; > + > struct { > hwaddr addr; > } decode; > @@ -137,6 +163,17 @@ void register_reset(RegisterInfo *reg); > void register_init(RegisterInfo *reg); > > /** > + * Refresh GPIO outputs based on diff between old value register current value. > + * GPIOs are refreshed for fields where the old value differs to the current > + * value. > + * > + * @reg: Register to refresh GPIO outs > + * @old_value: previous value of register > + */ > + > +void register_refresh_gpios(RegisterInfo *reg, uint64_t old_value); > + > +/** > * Memory API MMIO write handler that will write to a Register API register. > * _be for big endian variant and _le for little endian. > * @opaque: RegisterInfo to write to -- Alex Bennée