Re: [PATCH 5/5] hw/i2c: Document the I2C qdev helpers

[Peter Maydell]

On Tue, 30 Jun 2020 at 11:15, Markus Armbruster <armbru@redhat.com> wrote:
>
> Philippe Mathieu-Daudé <f4bug@amsat.org> writes:
>
> > In commit d88c42ff2c we added new prototype but neglected to
> > add their documentation. Fix that.
> >
> > Reported-by: Peter Maydell <peter.maydell@linaro.org>
> > Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> > + * This function is useful if you have created @dev via qdev_new(),
> > + * i2c_slave_new() or i2c_slave_try_new() (which take a reference to
> > + * the device it returns to you), so that you can set properties on it
> > + * before realizing it. If you don't need to set properties then
> > + * i2c_slave_create_simple() is probably better (as it does the create,
> > + * init and realize in one step).
> > + *
> > + * If you are embedding the I2C slave into another QOM device and
> > + * initialized it via some variant on object_initialize_child() then
> > + * do not use this function, because that family of functions arrange
> > + * for the only reference to the child device to be held by the parent
> > + * via the child<> property, and so the reference-count-drop done here
> > + * would be incorrect.  (Instead you would want i2c_slave_realize(),
> > + * which doesn't currently exist but would be trivial to create if we
> > + * had any code that wanted it.)
> > + */
>
> The advice on use is more elaborate qdev_realize_and_unref()'s.  That
> one simply shows intended use.  I doubt we need more.  But as the person
> who wrote qdev_realize_and_unref(), I'm singularly unqualified judging
> the need ;)

If qdev_realize_and_unref() has documentation which gives
the use-cases similar to the text above, then we could make
this text say "This function follows the patterns and
intended usecases for qdev_realize_and_unref(); see the
documentation for that function for whether you would be
better off using i2c_realize() or (the not-yet-existing)
i2c_slave_realize()" or similar. I originally wrote the
version of the above text for ssi_realize_and_unref()
as essentially the documentation I would have liked
qdev_realize_and_unref() to have, ie including the nuances
which I had to figure out for myself.

thanks
-- PMM