Re: [PATCH 09/15] ASoC: SDCA: Add UMP buffer helper functions

[Pierre-Louis Bossart <pierre-louis.bossart@linux.dev>]

>>> Second question would probably be could we get a little more of a
>>> concrete example of what you are looking for. Options I can see
>>> would be to add the options of using a blocking type API, maybe
>>> something like:
>>>
>>> sdca_ump_notify_owner() - this could probably be part of
>>>                           sdca_ump_get_owner_host()
>>> sdca_ump_wait_for_owner()
>>> sdca_ump_send_message_and_wait() - this would like be a wrapper
>>>                                    for a write and the first two.
>>>
>>> But I am a little hesitant to switch the FDL process over this,
>>> has to start firing off work queues etc. and everything gets a
>>> little more complex.
>>>
>>> Or one could add something like:
>>>
>>> sdca_ump_cancel_timeout()
>>> sdca_ump_schedule_timeout(callback, timeout)
>>>
>>> This probably integrates nicer with the IRQ driven way the FDL
>>> is implemented at the moment.
>>>
>>> Would something like one of those be what you are looking for? Or
>>> are you really looking for a much higher level API with some sort
>>> of message queue? Although if going in that way I get very very
>>> nervous about all the SDCA weirdness.
>>
>> Oh I wasn't thinking at all of a message queue with a
>> higher-level API, more something along the lines of the SOF/cAVS
>> IPC based on doorbells.
> 
> I am not super familiar with the inner workings of SOF but at
> first glance it does look like exactly the thing I was hoping to
> avoid :-)
> 
>> You probably want an async mechanism where the set_owner()
>> returns immediately and is followed by a wait with a configurable
>> timeout. I don't really get the nuance between your two proposals
>> but they go in the direction I had in mind.
> 
> The difference was intended to be the first is a blocking API
> (ie. function returns on either owner being transferred back to
> you or the timeout expires). The second was intended to be async
> and based on a callback ie. returns immediately and the callback
> is called when the timeout expires so you can run whatever error
> handling you want.
> 
>> That said, I do think you'll have to deal with workqueues,
>> not sure how else to deal with the protocol. If you have an
>> interrupt that requires a follow-up UMP-based check, things
>> could get messy with an interrupt blocking another one. We had
>> similar problems with the SOF IPC and for the standard SoundWire
>> interrupts. Best to keep interrupt handlers simple and signal
>> stuff to do for workqueues.
> 
> Definitely agree that blocking in the IRQs is not what we want,
> hence my previous comments about preferring the callback based
> option.

ok

> For the most part HID and FDL are extremely device led
> processes. We don't really need to keep super complex state on the
> host side as we don't initiate anything, we are just responding
> to what the device wants to do. To my mind keeping complex state
> on the host side likely increases the chance something goes wrong.
> 
> I will have a think see if I can come up with a nice way to
> integrate a timeout to individual UMP owner transfers, but I am
> not exactly sure how much value this adds for the currently
> implemented features. HID won't use this, and FDL already has a
> timeout for the FDL process, so one still sees if it failed and
> as you can see the status and response pairs debugging should be
> fairly easy.
> 
> Once these timeouts exist we have to pick values for them,
> because SDCA is a huge huge fan of asking you to wait for
> something but not saying for how long. If you have any thoughts
> on this that would be appreciated too, but mostly I will just
> pluck some vaguely reasonable sounding values as I did for the
> existing FDL timeouts.

The SDCA 1.0 spec defines DisCo properties for UMP timeouts, just do a search for "ownership-transition-max-delay". Table 139 page 236 defines this property specifically for XUs and hence the low-level protocol FDL is based on.

The description reads:

"
The maximum time allowed for Device to change the
ownership from Device to Host in an Rx UMP when Host
Software is waiting for the owner to change back to Host.
"

As usual it's quite possible that platform firmware is broken with bad values, but the design intent was to provide a timeout value for software to use. Ignoring these properties doesn't seem quite right to me...