On Wed, Apr 25, 2012 at 2:41 AM, Peter Maydell <peter.mayd...@linaro.org> wrote:
> On 20 April 2012 03:12, Peter A. G. Crosthwaite
> <peter.crosthwa...@petalogix.com> 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.
>
ack
>> +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.)
>
Hi Peter,
Thanks for your review.
I think there is confusion here in that I have named both the bus and
slave properties ss. Each slave has a index which ive called ss and
the bus has a property ss. The property defintion above applies the
slave device ss property which is constant. I think this will go away
when I clear up the name confusion as you have suggested.
Regards,
Peter
> -- PMM
