[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3 1/4] SSI: Built in multiple device support
From: |
Peter Crosthwaite |
Subject: |
Re: [Qemu-devel] [PATCH v3 1/4] SSI: Built in multiple device support |
Date: |
Thu, 26 Apr 2012 12:28:53 +1000 |
On Wed, Apr 25, 2012 at 2:41 AM, Peter Maydell <address@hidden> wrote:
> 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.
>
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
[Qemu-devel] [PATCH v3 2/4] m25p80: initial verion, Peter A. G. Crosthwaite, 2012/04/19
[Qemu-devel] [PATCH v3 3/4] xilinx_spi: initial version, Peter A. G. Crosthwaite, 2012/04/19
[Qemu-devel] [PATCH v3 4/4] petalogix-ml605: added spi controller with m25p80, Peter A. G. Crosthwaite, 2012/04/19