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

[Charles Keepax <ckeepax@opensource.cirrus.com>]

On Wed, Sep 10, 2025 at 04:29:40PM +0200, Pierre-Louis Bossart wrote:
> On 9/10/25 15:37, Charles Keepax wrote:
> > On Wed, Sep 10, 2025 at 02:09:14PM +0200, Pierre-Louis Bossart wrote:
> >> On 9/9/25 18:52, Charles Keepax wrote:
> >>> On Tue, Sep 09, 2025 at 03:14:00PM +0200, Pierre-Louis Bossart wrote:
> >>>> On 9/8/25 15:15, Charles Keepax wrote:
> >>>>> On Mon, Sep 08, 2025 at 02:02:09PM +0200, Pierre-Louis Bossart wrote:
> >>>>>> On 9/5/25 16:31, Charles Keepax wrote:
> > 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.

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.

Thanks,
Charles