Re: [Qemu-devel] [RFC v4 1/5] hw/arm: add very initial support for Canon DIGIC SoC

[Antony Pavlov]

On Thu, 05 Sep 2013 23:38:26 +0200
Andreas Färber <address@hidden> wrote:

> Am 05.09.2013 23:23, schrieb Antony Pavlov:
> > On Thu, 05 Sep 2013 20:08:34 +0200
> > Andreas Färber <address@hidden> wrote:
> >> Am 05.09.2013 09:52, schrieb Antony Pavlov:
> >>> diff --git a/hw/arm/digic.c b/hw/arm/digic.c
> >>> new file mode 100644
> >>> index 0000000..95a9fcd
> >>> --- /dev/null
> >>> +++ b/hw/arm/digic.c
> [...]
> >>> +static const TypeInfo digic_type_info = {
> >>> +    .name = TYPE_DIGIC,
> >>> +    .parent = TYPE_DEVICE,
> >>> +    .instance_size = sizeof(DigicState),
> >>> +    .instance_init = digic_init,
> >>> +    .class_init = digic_class_init,
> >>> +};
> >>> +
> >>> +static void digic_register_types(void)
> >>> +{
> >>> +    type_register_static(&digic_type_info);
> >>> +}
> >>> +
> >>> +type_init(digic_register_types)
> >>> diff --git a/include/hw/arm/digic.h b/include/hw/arm/digic.h
> >>> new file mode 100644
> >>> index 0000000..0ef4723
> >>> --- /dev/null
> >>> +++ b/include/hw/arm/digic.h
> [...]
> >>> +typedef struct DigicState {
> >>
> >> Please add
> >>     /*< private >*/
> >>
> >>> +    Object parent_obj;
> >>
> >>     /*< private >*/
> > 
> > /*< public >*/ ?
> 
> Yes, sorry, copy&paste and then noticing Object. ;)
> Or just leave the latter out so that all fields are undocumented.

I have grepped the qemu code and found that the "private" and "public" labels
usage is:

=== quote start ===
    typedef struct SomethingState {
       /*< private >*/
       DeviceState parent_obj;
       /*< public >*/

       some_type public_field1;
=== quote end ===

Is there any intention in empty line after the "public" label and no empty line 
befor it?
Can I wrote something like this:

=== quote start ===
    typedef struct SomethingState {
       /*< private >*/
       DeviceState parent_obj;

       /*< public >*/
       some_type public_field1;
=== quote end ===

?


> >> markers for documentation.
> >>
> >> It needs to be DeviceState parent_obj though.
> > 
> > In your tegra2 support 'Object parent_obj' is used in a similar situation.
> > 
> > http://repo.or.cz/w/qemu/afaerber.git/blob/refs/heads/tegra:/include/hw/arm/tegra2.h#l42
> 
> Thanks for spotting, fixed. (It used to be derived from TYPE_OBJECT, but
> we decided to provide QOM realize support only for devices.)
> 
> Unfortunately Tegra kernel is still stuck after USB init either way...
> 
> Cheers,
> Andreas
> 
> -- 
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg


-- 
-- 
Best regards,
  Antony Pavlov