Re: [PATCH-for-5.1 v2] hw/core/register: Add register_init_block8 helper.

[Alistair Francis]

On Thu, Apr 2, 2020 at 3:45 PM Alistair Francis <address@hidden> wrote:
>
> On Thu, Apr 2, 2020 at 9:29 AM Joaquin de Andres <address@hidden> wrote:
> >
> > There was no support for 8 bits block registers. Changed
> > register_init_block32 to be generic and static, adding register
> > size in bits as parameter. Created one helper for each size.
> >
> > Signed-off-by: Joaquin de Andres <address@hidden>
>
> Reviewed-by: Alistair Francis <address@hidden>

Thanks for the patch. I have applied this to the device tree pull
request for 5.1.

Alistair

>
> Alistair
>
> > ---
> > This patch is small and I could see that there is not much movement with
> > the release, so, I let my self send this. Also this is my first patch :)
> > Reviews are welcome.
> > ---
> >  hw/core/register.c    | 46 +++++++++++++++++++++++++++++++++----------
> >  include/hw/register.h |  8 ++++++++
> >  2 files changed, 44 insertions(+), 10 deletions(-)
> >
> > diff --git a/hw/core/register.c b/hw/core/register.c
> > index 3c77396587..ddf91eb445 100644
> > --- a/hw/core/register.c
> > +++ b/hw/core/register.c
> > @@ -246,16 +246,18 @@ uint64_t register_read_memory(void *opaque, hwaddr 
> > addr,
> >      return extract64(read_val, 0, size * 8);
> >  }
> >
> > -RegisterInfoArray *register_init_block32(DeviceState *owner,
> > -                                         const RegisterAccessInfo *rae,
> > -                                         int num, RegisterInfo *ri,
> > -                                         uint32_t *data,
> > -                                         const MemoryRegionOps *ops,
> > -                                         bool debug_enabled,
> > -                                         uint64_t memory_size)
> > +static RegisterInfoArray *register_init_block(DeviceState *owner,
> > +                                              const RegisterAccessInfo 
> > *rae,
> > +                                              int num, RegisterInfo *ri,
> > +                                              void *data,
> > +                                              const MemoryRegionOps *ops,
> > +                                              bool debug_enabled,
> > +                                              uint64_t memory_size,
> > +                                              size_t data_size_bits)
> >  {
> >      const char *device_prefix = object_get_typename(OBJECT(owner));
> >      RegisterInfoArray *r_array = g_new0(RegisterInfoArray, 1);
> > +    int data_size = data_size_bits >> 3;
> >      int i;
> >
> >      r_array->r = g_new0(RegisterInfo *, num);
> > @@ -264,12 +266,12 @@ RegisterInfoArray *register_init_block32(DeviceState 
> > *owner,
> >      r_array->prefix = device_prefix;
> >
> >      for (i = 0; i < num; i++) {
> > -        int index = rae[i].addr / 4;
> > +        int index = rae[i].addr / data_size;
> >          RegisterInfo *r = &ri[index];
> >
> >          *r = (RegisterInfo) {
> > -            .data = &data[index],
> > -            .data_size = sizeof(uint32_t),
> > +            .data = data + data_size * index,
> > +            .data_size = data_size,
> >              .access = &rae[i],
> >              .opaque = owner,
> >          };
> > @@ -284,6 +286,30 @@ RegisterInfoArray *register_init_block32(DeviceState 
> > *owner,
> >      return r_array;
> >  }
> >
> > +RegisterInfoArray *register_init_block8(DeviceState *owner,
> > +                                        const RegisterAccessInfo *rae,
> > +                                        int num, RegisterInfo *ri,
> > +                                        uint8_t *data,
> > +                                        const MemoryRegionOps *ops,
> > +                                        bool debug_enabled,
> > +                                        uint64_t memory_size)
> > +{
> > +    return register_init_block(owner, rae, num, ri, (void *)
> > +                               data, ops, debug_enabled, memory_size, 8);
> > +}
> > +
> > +RegisterInfoArray *register_init_block32(DeviceState *owner,
> > +                                         const RegisterAccessInfo *rae,
> > +                                         int num, RegisterInfo *ri,
> > +                                         uint32_t *data,
> > +                                         const MemoryRegionOps *ops,
> > +                                         bool debug_enabled,
> > +                                         uint64_t memory_size)
> > +{
> > +    return register_init_block(owner, rae, num, ri, (void *)
> > +                               data, ops, debug_enabled, memory_size, 32);
> > +}
> > +
> >  void register_finalize_block(RegisterInfoArray *r_array)
> >  {
> >      object_unparent(OBJECT(&r_array->mem));
> > diff --git a/include/hw/register.h b/include/hw/register.h
> > index 5796584588..5d2c565ae0 100644
> > --- a/include/hw/register.h
> > +++ b/include/hw/register.h
> > @@ -185,6 +185,14 @@ uint64_t register_read_memory(void *opaque, hwaddr 
> > addr, unsigned size);
> >   *          memory region (r_array->mem) the caller should add to a 
> > container.
> >   */
> >
> > +RegisterInfoArray *register_init_block8(DeviceState *owner,
> > +                                        const RegisterAccessInfo *rae,
> > +                                        int num, RegisterInfo *ri,
> > +                                        uint8_t *data,
> > +                                        const MemoryRegionOps *ops,
> > +                                        bool debug_enabled,
> > +                                        uint64_t memory_size);
> > +
> >  RegisterInfoArray *register_init_block32(DeviceState *owner,
> >                                           const RegisterAccessInfo *rae,
> >                                           int num, RegisterInfo *ri,
> > --
> > 2.26.0
> >
> >