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

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

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:
> >> 4. host detects ownership change or times out with a failed transfer error message.
> > 
> > by receiving an IRQ on the owner control.
> 
> but that IRQ may never come, in which case the entire protocol would fail.
> > 
> > You might need to elaborate a little more on what you think is
> > missing. The four functions implement the four operations that
> > power a UMP buffer, checking you have ownership, passing
> > ownership back to the device, reading data, and writing data.
> > 
> > One could combine more of the functionality into a single helper,
> > but then you start to hit problems where you need to do extra
> > bits.
> > 
> > For example HID, very simple just: check you own the buffer,
> > read the message, and pass the buffer back.
> 
> I remember one of the early implementations of HID support
> in a Realtek driver needed some sort of timeout to reset the
> communication in case the ownership change interrupt never
> came. It's reasonable to expect that things will go wrong at
> some point in the communication between host and device, isn't it?

I will have a look see if I can find that driver and see what is
going on there. But HID seems like a good example (or at least the
event part of HID) of why the wait isn't in the UMP code to me.

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.

Thanks,
Charles