Message ID | 8eb128d39abd89ae6a0d74e965c262650d3bf863.1463093051.git.alistair.francis@xilinx.com |
---|---|
State | New |
Headers | show |
On 12 May 2016 at 23:46, Alistair Francis <alistair.francis@xilinx.com> wrote: > From: Peter Crosthwaite <peter.crosthwaite@xilinx.com> > > QOMify registers as a child of TYPE_DEVICE. This allows registers to > define GPIOs. > > Define an init helper that will do QOM initialisation. You should just squash this down into patch 2. > Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com> > Signed-off-by: Alistair Francis <alistair.francis@xilinx.com> > Reviewed-by: KONRAD Frederic <fred.konrad@greensocs.com> > --- > /** > + * Initialize a register. GPIO's are setup as IOs to the specified device. > + * Fast paths for eligible registers are enabled. > + * @reg: Register to initialize > + */ I can't work out what this documentation comment is trying to say. How can a register have a GPIO? What does a fast path do, what registers are elegible, why do I care whether they're enabled or not? > + > +void register_init(RegisterInfo *reg); > + > +/** > * 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 > -- > 2.7.4 thanks -- PMM
On Fri, Jun 10, 2016 at 3:55 AM, Peter Maydell <peter.maydell@linaro.org> wrote: > On 12 May 2016 at 23:46, Alistair Francis <alistair.francis@xilinx.com> wrote: >> From: Peter Crosthwaite <peter.crosthwaite@xilinx.com> >> >> QOMify registers as a child of TYPE_DEVICE. This allows registers to >> define GPIOs. >> >> Define an init helper that will do QOM initialisation. > > You should just squash this down into patch 2. It relies on some work from the previous two patches so it becomes a pretty big patch if they all get squashed together. I'd rather leave it separate like this. > >> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com> >> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com> >> Reviewed-by: KONRAD Frederic <fred.konrad@greensocs.com> >> --- > >> /** >> + * Initialize a register. GPIO's are setup as IOs to the specified device. >> + * Fast paths for eligible registers are enabled. >> + * @reg: Register to initialize >> + */ > > I can't work out what this documentation comment is trying to say. > How can a register have a GPIO? What does a fast path do, what > registers are elegible, why do I care whether they're enabled or not? I have updated it to this, which removes the fast path (that was left over) and explains more about GPIOs. /** * Initialize a register. This will also setup any GPIO links which are used * to connect register updates in one device to other devices. Generally this * is useful for interrupt propagation. * @reg: Register to initialize */ Thanks, Alistair > >> + >> +void register_init(RegisterInfo *reg); >> + >> +/** >> * 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 >> -- >> 2.7.4 > > thanks > -- PMM >
diff --git a/hw/core/register.c b/hw/core/register.c index 25196e6..c5a2c78 100644 --- a/hw/core/register.c +++ b/hw/core/register.c @@ -148,6 +148,17 @@ void register_reset(RegisterInfo *reg) register_write_val(reg, reg->access->reset); } +void register_init(RegisterInfo *reg) +{ + assert(reg); + + if (!reg->data || !reg->access) { + return; + } + + object_initialize((void *)reg, sizeof(*reg), TYPE_REGISTER); +} + static inline void register_write_memory(void *opaque, hwaddr addr, uint64_t value, unsigned size, bool be) { @@ -219,3 +230,15 @@ uint64_t register_read_memory_le(void *opaque, hwaddr addr, unsigned size) { return register_read_memory(opaque, addr, size, false); } + +static const TypeInfo register_info = { + .name = TYPE_REGISTER, + .parent = TYPE_DEVICE, +}; + +static void register_register_types(void) +{ + type_register_static(®ister_info); +} + +type_init(register_register_types) diff --git a/include/hw/register.h b/include/hw/register.h index e0aac91..eedd578 100644 --- a/include/hw/register.h +++ b/include/hw/register.h @@ -11,6 +11,7 @@ #ifndef REGISTER_H #define REGISTER_H +#include "hw/qdev-core.h" #include "exec/memory.h" typedef struct RegisterInfo RegisterInfo; @@ -74,6 +75,9 @@ struct RegisterAccessInfo { */ struct RegisterInfo { + /* <private> */ + DeviceState parent_obj; + /* <public> */ void *data; int data_size; @@ -83,6 +87,9 @@ struct RegisterInfo { void *opaque; }; +#define TYPE_REGISTER "qemu,register" +#define REGISTER(obj) OBJECT_CHECK(RegisterInfo, (obj), TYPE_REGISTER) + /** * This structure is used to group all of the individual registers which are * modeled using the RegisterInfo strucutre. @@ -132,6 +139,14 @@ uint64_t register_read(RegisterInfo *reg, const char* prefix, bool debug); void register_reset(RegisterInfo *reg); /** + * Initialize a register. GPIO's are setup as IOs to the specified device. + * Fast paths for eligible registers are enabled. + * @reg: Register to initialize + */ + +void register_init(RegisterInfo *reg); + +/** * 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