From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38197) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ad5E0-0003IX-3w for qemu-devel@nongnu.org; Mon, 07 Mar 2016 19:10:09 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ad5Dy-0000mv-U5 for qemu-devel@nongnu.org; Mon, 07 Mar 2016 19:10:08 -0500 Received: from mail-ob0-x244.google.com ([2607:f8b0:4003:c01::244]:35223) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ad5Dy-0000mU-N7 for qemu-devel@nongnu.org; Mon, 07 Mar 2016 19:10:06 -0500 Received: by mail-ob0-x244.google.com with SMTP id e7so1168569obv.2 for ; Mon, 07 Mar 2016 16:10:06 -0800 (PST) MIME-Version: 1.0 Sender: alistair23@gmail.com In-Reply-To: <87io17kb6b.fsf@linaro.org> References: <32e6f2f10d2394ce59543e0fc5f5ef63203cfede.1455055858.git.alistair.francis@xilinx.com> <87io17kb6b.fsf@linaro.org> From: Alistair Francis Date: Mon, 7 Mar 2016 16:09:36 -0800 Message-ID: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v4 07/16] register: Add block initialise helper List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?UTF-8?B?QWxleCBCZW5uw6ll?= Cc: Edgar Iglesias , Peter Maydell , "qemu-devel@nongnu.org Developers" , Alistair Francis , Peter Crosthwaite , Edgar Iglesias , =?UTF-8?Q?Andreas_F=C3=A4rber?= , =?UTF-8?B?S09OUkFEIEZyw6lkw6lyaWM=?= On Mon, Feb 29, 2016 at 3:28 AM, Alex Benn=C3=A9e = wrote: > > Alistair Francis writes: > >> From: Peter Crosthwaite >> >> Add a helper that will scan a static RegisterAccessInfo Array >> and populate a container MemoryRegion with registers as defined. >> >> Signed-off-by: Peter Crosthwaite >> Signed-off-by: Alistair Francis >> --- >> V3: >> - Fix typo >> V2: >> - Use memory_region_add_subregion_no_print() >> >> hw/core/register.c | 29 +++++++++++++++++++++++++++++ >> include/hw/register.h | 20 ++++++++++++++++++++ >> 2 files changed, 49 insertions(+) >> >> diff --git a/hw/core/register.c b/hw/core/register.c >> index d766517..ac866f6 100644 >> --- a/hw/core/register.c >> +++ b/hw/core/register.c >> @@ -210,6 +210,35 @@ uint64_t register_read_memory_le(void *opaque, hwad= dr addr, unsigned size) >> return register_read_memory(opaque, addr, size, false); >> } >> >> +void register_init_block32(DeviceState *owner, const RegisterAccessInfo= *rae, >> + int num, RegisterInfo *ri, uint32_t *data, >> + MemoryRegion *container, const MemoryRegionO= ps *ops, >> + bool debug_enabled) >> +{ >> + const char *debug_prefix =3D object_get_typename(OBJECT(owner)); >> + int i; >> + >> + for (i =3D 0; i < num; i++) { >> + int index =3D rae[i].decode.addr / 4; >> + RegisterInfo *r =3D &ri[index]; >> + >> + *r =3D (RegisterInfo) { >> + .data =3D &data[index], >> + .data_size =3D sizeof(uint32_t), >> + .access =3D &rae[i], >> + .debug =3D debug_enabled, >> + .prefix =3D debug_prefix, >> + .opaque =3D owner, >> + }; >> + register_init(r); >> + >> + memory_region_init_io(&r->mem, OBJECT(owner), ops, r, r->access= ->name, >> + sizeof(uint32_t)); >> + memory_region_add_subregion_no_print(container, >> + r->access->decode.addr, >> &r->mem); > > As I mentioned in the previous patch I think having a subregion > per-register is excessive. I think you need single io region with the > private data giving a structure with the list of registers in the block. > You can then have a general purpose set of register ops to lookup the > eventual register and dispatch to the handler. Ok, I figured out a pretty straightforward way to do that. Keep an eye out for the next version. Thanks, Alistair > >> + } >> +} >> + >> static const TypeInfo register_info =3D { >> .name =3D TYPE_REGISTER, >> .parent =3D TYPE_DEVICE, >> diff --git a/include/hw/register.h b/include/hw/register.h >> index d3469c6..6ac005c 100644 >> --- a/include/hw/register.h >> +++ b/include/hw/register.h >> @@ -162,6 +162,26 @@ void register_write_memory_le(void *opaque, hwaddr = addr, uint64_t value, >> uint64_t register_read_memory_be(void *opaque, hwaddr addr, unsigned si= ze); >> uint64_t register_read_memory_le(void *opaque, hwaddr addr, unsigned si= ze); >> >> +/** >> + * Init a block of consecutive registers into a container MemoryRegion.= A >> + * number of constant register definitions are parsed to create a corre= sponding >> + * array of RegisterInfo's. >> + * >> + * @owner: device owning the registers >> + * @rae: Register definitions to init >> + * @num: number of registers to init (length of @rae) >> + * @ri: Register array to init >> + * @data: Array to use for register data >> + * @container: Memory region to contain new registers >> + * @ops: Memory region ops to access registers. >> + * @debug enabled: turn on/off verbose debug information >> + */ >> + >> +void register_init_block32(DeviceState *owner, const RegisterAccessInfo= *rae, >> + int num, RegisterInfo *ri, uint32_t *data, >> + MemoryRegion *container, const MemoryRegionO= ps *ops, >> + bool debug_enabled); >> + >> /* Define constants for a 32 bit register */ >> #define REG32(reg, addr) = \ >> enum { A_ ## reg =3D (addr) }; = \ > > > -- > Alex Benn=C3=A9e >