Re: [PATCH 30/55] auxbus: New aux_realize_bus(), pairing with aux_init_bus()

[Philippe Mathieu-Daudé]

On 5/19/20 4:55 PM, Markus Armbruster wrote:
> aux_init_bus() encapsulates the creation of an aux-bus and its
> aux-to-i2c-bridge device.
> 
> Create aux_realize_bus() to similarly encapsulate their realization.
> 
> Signed-off-by: Markus Armbruster <address@hidden>
> ---
>  include/hw/misc/auxbus.h | 7 +++++++
>  hw/display/xlnx_dp.c     | 2 +-
>  hw/misc/auxbus.c         | 5 +++++
>  3 files changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/include/hw/misc/auxbus.h b/include/hw/misc/auxbus.h
> index a539a98c4b..6ab6cf5bb6 100644
> --- a/include/hw/misc/auxbus.h
> +++ b/include/hw/misc/auxbus.h
> @@ -93,6 +93,13 @@ struct AUXSlave {
>   */
>  AUXBus *aux_init_bus(DeviceState *parent, const char *name);

Previous to your patch, but aux_init_bus() is misnamed.

I'd rather rename this one aux_bus_init() ...

>  
> +/**
> + * aux_realize_bus: Realize an AUX bus.
> + *
> + * @bus: The AUX bus.
> + */
> +void aux_realize_bus(AUXBus *bus);

... and this one aux_bus_realize().

If you mind :)

Reviewed-by: Philippe Mathieu-Daudé <address@hidden>

> +
>  /*
>   * aux_request: Make a request on the bus.
>   *
> diff --git a/hw/display/xlnx_dp.c b/hw/display/xlnx_dp.c
> index dd6aa172f3..b1d90cab1d 100644
> --- a/hw/display/xlnx_dp.c
> +++ b/hw/display/xlnx_dp.c
> @@ -1264,7 +1264,7 @@ static void xlnx_dp_realize(DeviceState *dev, Error 
> **errp)
>      DisplaySurface *surface;
>      struct audsettings as;
>  
> -    qdev_init_nofail(DEVICE(s->aux_bus->bridge));
> +    aux_realize_bus(s->aux_bus);
>  
>      qdev_init_nofail(DEVICE(s->dpcd));
>      aux_map_slave(AUX_SLAVE(s->dpcd), 0x0000);
> diff --git a/hw/misc/auxbus.c b/hw/misc/auxbus.c
> index 7fb020086f..9b4ccfa39c 100644
> --- a/hw/misc/auxbus.c
> +++ b/hw/misc/auxbus.c
> @@ -81,6 +81,11 @@ AUXBus *aux_init_bus(DeviceState *parent, const char *name)
>      return bus;
>  }
>  
> +void aux_realize_bus(AUXBus *bus)
> +{
> +    qdev_init_nofail(DEVICE(bus->bridge));
> +}
> +
>  void aux_map_slave(AUXSlave *aux_dev, hwaddr addr)
>  {
>      DeviceState *dev = DEVICE(aux_dev);
>