On Fri, May 05, 2017 at 01:54:19PM -0300, Eduardo Habkost wrote:
> On Fri, May 05, 2017 at 09:36:22AM +0200, Cornelia Huck wrote:
> [...]
> > > diff --git a/hw/core/sysbus.c b/hw/core/sysbus.c
> > > index c0f560b289..6a2eec8dd0 100644
> > > --- a/hw/core/sysbus.c
> > > +++ b/hw/core/sysbus.c
> > > @@ -326,6 +326,17 @@ static void sysbus_device_class_init(ObjectClass *klass, void *data)
> > >      DeviceClass *k = DEVICE_CLASS(klass);
> > >      k->init = sysbus_device_init;
> > >      k->bus_type = TYPE_SYSTEM_BUS;
> > > +    /*
> > > +     * device_add plugs devices into suitable bus.  For "real" buses,
> > 
> > s/suitable/a suitable/
> 
> Thanks, I will fix it.
> 
> > 
> > > +     * that actually connects the device.  For sysbus, the connections
> > > +     * need to be made separately, and device_add can't do that.  The
> > > +     * device would be left unconnected, and will probably not work
> > > +     *
> > > +     * However, a few machines and a few devices can handle a few sysbus
> > > +     * devices. In this case, the device subclass needs to override
> > 
> > Should that rather be "a few machines can handle a few specific sysbus
> > devices"?
> 
> I will extend it to "a few machines can handle device_add for a
> few specific sysbus devices". Thanks for spotting it.

Fixup will be included in v3, or can be applied by maintainer:

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 hw/core/sysbus.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/core/sysbus.c b/hw/core/sysbus.c
index 6a2eec8dd0..5d0887f499 100644
--- a/hw/core/sysbus.c
+++ b/hw/core/sysbus.c
@@ -327,14 +327,14 @@ static void sysbus_device_class_init(ObjectClass *klass, void *data)
     k->init = sysbus_device_init;
     k->bus_type = TYPE_SYSTEM_BUS;
     /*
-     * device_add plugs devices into suitable bus.  For "real" buses,
+     * device_add plugs devices into a suitable bus.  For "real" buses,
      * that actually connects the device.  For sysbus, the connections
      * need to be made separately, and device_add can't do that.  The
      * device would be left unconnected, and will probably not work
      *
-     * However, a few machines and a few devices can handle a few sysbus
-     * devices. In this case, the device subclass needs to override
-     * it and set user_creatable=true.
+     * However, a few machines can handle device_add/-device with
+     * a few specific sysbus devices. In those cases, the device
+     * subclass needs to override it and set user_creatable=true.
      */
     k->user_creatable = false;
 }
-- 
2.11.0.259.g40922b1

On Fri, 5 May 2017 17:36:20 -0300
Eduardo Habkost <ehabkost@redhat.com> wrote:

> Fixup will be included in v3, or can be applied by maintainer:
> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
>  hw/core/sysbus.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/core/sysbus.c b/hw/core/sysbus.c
> index 6a2eec8dd0..5d0887f499 100644
> --- a/hw/core/sysbus.c
> +++ b/hw/core/sysbus.c
> @@ -327,14 +327,14 @@ static void sysbus_device_class_init(ObjectClass *klass, void *data)
>      k->init = sysbus_device_init;
>      k->bus_type = TYPE_SYSTEM_BUS;
>      /*
> -     * device_add plugs devices into suitable bus.  For "real" buses,
> +     * device_add plugs devices into a suitable bus.  For "real" buses,
>       * that actually connects the device.  For sysbus, the connections
>       * need to be made separately, and device_add can't do that.  The
>       * device would be left unconnected, and will probably not work
>       *
> -     * However, a few machines and a few devices can handle a few sysbus
> -     * devices. In this case, the device subclass needs to override
> -     * it and set user_creatable=true.
> +     * However, a few machines can handle device_add/-device with
> +     * a few specific sysbus devices. In those cases, the device
> +     * subclass needs to override it and set user_creatable=true.
>       */
>      k->user_creatable = false;
>  }

FWIW: LGTM