Re: [Qemu-devel] [PATCH v3 1/4] SSI: Built in multiple device support

[Peter Maydell]

On 20 April 2012 03:12, Peter A. G. Crosthwaite
<address@hidden> wrote:
> Added support for multiple devices attached to a single SSI bus (Previously
> SSI masters with multiple slaves were emulated as multiple point to point SSI
> busses)

>  static struct BusInfo ssi_bus_info = {
>     .name = "SSI",
>     .size = sizeof(SSIBus),
> +    .props = (Property[]) {
> +        DEFINE_PROP_INT32("ss", struct SSISlave, ss, 0),

"ss" is a terrible name for a property. Can we have something
a little less abbreviated, please?

>  SSIBus *ssi_create_bus(DeviceState *parent, const char *name)
>  {
> -    BusState *bus;
> -    bus = qbus_create(&ssi_bus_info, parent, name);
> -    return FROM_QBUS(SSIBus, bus);
> +    SSIBus *bus;
> +
> +    bus = FROM_QBUS(SSIBus, qbus_create(&ssi_bus_info, parent, name));
> +    vmstate_register(NULL, -1, &vmstate_ssi_bus, bus);
> +    return  bus;

Stray double space.

> +void ssi_select_slave(SSIBus *bus, int32_t ss)
> +{
> +    SSISlave *slave;
> +    SSISlaveClass *ssc;
> +
> +    if (bus->ss == ss) {
> +        return;
> +    }
> +
> +    slave = get_current_slave(bus);
> +    if (slave) {
> +        ssc = SSI_SLAVE_GET_CLASS(slave);
> +        if (ssc->set_cs) {
> +            ssc->set_cs(slave, 0);
> +        }
> +    }
> +    bus->ss = ss;

Something wrong here. If bus->ss is a property you can't modify
it randomly at runtime. (It won't get put back to the right
value at reset, for instance.)

-- PMM