Re: [PATCH RFC 0/2] Eliminate drive_get_next()

[Markus Armbruster]

Peter Maydell <peter.maydell@linaro.org> writes:

> On Mon, 15 Nov 2021 at 12:55, Markus Armbruster <armbru@redhat.com> wrote:
>>
>> This is RFC because I'm unsure about the removal of
>>
>>     /* Reason: init() method uses drive_get_next() */
>>     dc->user_creatable = false;
>>
>> in PATCH 1.  Both users appear to wire up some GPIO.  If that's
>> necessary for the thing to work, we should just replace the comment.
>
> Looking at the code, it sort of is and sort of isn't. The GPIO line
> is the chip-select line. If you don't connect it then (because the
> ssi-sd device configures cs_polarity to SSI_CS_LOW, requesting an
> active-low chip-select) the device will always be selected. If
> the machine created an SSI bus with no SSI device attached to it
> then in theory the user could create an ssi-sd device and connect
> it there and have it work. But in practice it's really unlikely:
> machines create SSI buses with specific wired-in devices on them,
> and the guest OS knows what it has to do to enable the chip select
> for the device it wants to talk to (often some known GPIO pin on
> a GPIO controller).
>
> So I would keep the user_creatable = false, with a reason of
> "user should wire up GPIO chip-select line". But see below for

I'll do it this way.

> a pile of contrary precedent.
>
> (The chip-select GPIO is created in the parent class, incidentally.)
>
>> Aside: there may be devices that need manual wiring to work, yet don't
>> have user_creatable unset.  Bugs if you ask me.  I don't have smart
>> ideas on how to track them down.
>
> Me neither. I notice that the TYPE_M25P80 is also an SSI peripheral
> with an active-low chipselect but that one doesn't set user_creatable
> to false. TYPE_SSD0323 also is user-creatable and that one has an
> active-high chipselect, which means the user can create a device but
> it will then never be usable because it will ignore all transactions.
> (More generally, looks like most subclasses of TYPE_SSI_PERIPHERAL
> don't set user_creatable = false.)

For sysbus devices, we clear user_creatable by default, because sysbus
devices pretty much always[*] need wiring.  Is this the case for SSI bus
devices, too?


[*] The most prominent exception is "dynamic sysbus", which I believe
was a mistake.