Philippe Mathieu-Daudé <f4...@amsat.org> writes:
> Extract i2c_try_create_slave() and i2c_realize_and_unref()
> from i2c_create_slave().
> We can now set properties on a I2CSlave before it is realized.
>
> This is in line with the recent qdev/QOM changes merged
> in commit 6675a653d2e.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4...@amsat.org>
> ---
> Cc: Markus Armbruster <arm...@redhat.com>
> ---
> include/hw/i2c/i2c.h | 2 ++
> hw/i2c/core.c | 18 ++++++++++++++++--
> 2 files changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/include/hw/i2c/i2c.h b/include/hw/i2c/i2c.h
> index 4117211565..d6e3d85faf 100644
> --- a/include/hw/i2c/i2c.h
> +++ b/include/hw/i2c/i2c.h
> @@ -80,6 +80,8 @@ int i2c_send(I2CBus *bus, uint8_t data);
> uint8_t i2c_recv(I2CBus *bus);
>
> DeviceState *i2c_create_slave(I2CBus *bus, const char *name, uint8_t addr);
> +DeviceState *i2c_try_create_slave(const char *name, uint8_t addr);
> +bool i2c_realize_and_unref(DeviceState *dev, I2CBus *bus, Error **errp);
>
> /* lm832x.c */
> void lm832x_key_event(DeviceState *dev, int key, int state);
> diff --git a/hw/i2c/core.c b/hw/i2c/core.c
> index 1aac457a2a..acf34a12d6 100644
> --- a/hw/i2c/core.c
> +++ b/hw/i2c/core.c
> @@ -267,13 +267,27 @@ const VMStateDescription vmstate_i2c_slave = {
> }
> };
>
> -DeviceState *i2c_create_slave(I2CBus *bus, const char *name, uint8_t addr)
> +DeviceState *i2c_try_create_slave(const char *name, uint8_t addr)
> {
> DeviceState *dev;
>
> dev = qdev_new(name);
> qdev_prop_set_uint8(dev, "address", addr);
> - qdev_realize_and_unref(dev, &bus->qbus, &error_fatal);
> + return dev;
> +}
> +
> +bool i2c_realize_and_unref(DeviceState *dev, I2CBus *bus, Error **errp)
> +{
> + return qdev_realize_and_unref(dev, &bus->qbus, errp);
> +}
> +
> +DeviceState *i2c_create_slave(I2CBus *bus, const char *name, uint8_t addr)
> +{
> + DeviceState *dev;
> +
> + dev = i2c_try_create_slave(name, addr);
> + i2c_realize_and_unref(dev, bus, &error_fatal);
> +
> return dev;
> }
We use "create_simple" names for functions that allocate, initialize,
configure and realize device objects: pci_create_simple(),
isa_create_simple(), usb_create_simple(). Calling this one
i2c_create_slave() is okay with me. I'd prefer
i2c_slave_create_simple(), though.
We use "new" names for functions that allocate and initialize device
objects: pci_new(), isa_new(), usb_new(). Let's call this one
i2c_slave_new().
Your use of "realize_and_unref" matches existing names elsewhere:
pci_realize_and_unref(), isa_realize_and_unref(),
usb_realize_and_unref(). However, the other two i2c functions are
called i2c_slave_FOO(), not i2c_FOO(). You could name this one
i2c_slave_realize_and_unref(). Another path to consistency: drop the
slave_ from all three names.
Ideally with my naming suggestions considered:
Reviewed-by: Markus Armbruster <arm...@redhat.com>
