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

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

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:
> > The only notification that comes from the device is the the
> > UMP buffer is being returned to the host, which indicates a HID
> > event has happened. But there is no time out on HID events, it
> > could be months between HID events. Like one could set a timeout
> > and at each timeout just verify the buffer is still set to the
> > device but that seems very cautious. Or one could check the buffer
> > is set as expected on boot, but I would really expect the SDCA
> > reset/boot logic to ensure that is good.
> > 
> > If it is a UMP transfer where the host expects the device to do
> > something and return that may need a timeout but as that is a
> > subset of UMP transfers it feels like it makes more sense to
> > implement the timeouts in the places that need them.
> 
> You have a point that in the case where the host waits for an
> event, then the notion of timeout is irrelevant.
>
> What I was referring to was the case where the host writes
> a message and waits for a notification that the message has
> been handled.
> This could be a write to a buffer or a read from a buffer.
> That's standard IPC stuff, and all existing cases of such
> message-passing IPC do have a timeout built-in. The communication
> is serial by nature since there is a single signaling mechanism,
> so the host has to wait before sending another message.
>
> I am not sure it's a great idea to leave the timeout handling
> at a higher level, because you will have to deal with other
> sources of information. Once a message has been sent with UMP,
> there will be additional signaling that the action requested by
> the host was completed on the peripheral side. In the case of
> FDL, once the firmware buffers have been transferred, possibly
> in different chunks, then the peripheral might have to perform
> an internal reboot/reset. That takes time and it's a different
> level of timeout.
>
> My take is that the UMP should implement basic timeout
> capabilities for just 'buffer passed/ownership changed'
> transition. Things will go wrong and you want information on
> where they went wrong. Implementing timeouts at a higher level
> makes it harder to debug/root cause.

I guess my slight concern here is that SDCA is a huge fan
of corner cases and everything being slightly inconsistent
with itself. So for example we add two things in this patch
chain HID and FDL. HID very clearly wants no timeouts, there
is no waiting for responses, its just get event, do stuff.
And for FDL the vast majority of the signalling is outside of
the actual UMP buffer through the FDL state control. I am sure
once we get around to Algorithm Extension, History Buffers and
Device-to-Device messaging they will have their own set of odd
requirements. So I liked the APIs being fairly low level and
implementing the basic operations, as it leaves space for
implementing the weirdness.

I guess perhaps the first question here, is this something we can
add incrementally or are you seeing this as a bit more fundamental
and needs done before we progress this chain?  (we are getting
a little nervous that devices are going to be shipping in the
not too distant future that are going to require this stuff).

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.

Thanks,
Charles