From: Charles Keepax <ckeepax@opensource.cirrus.com>
To: Pierre-Louis Bossart <pierre-louis.bossart@linux.dev>
Cc: broonie@kernel.org, rafael@kernel.org,
yung-chuan.liao@linux.intel.com, peter.ujfalusi@linux.intel.com,
shumingf@realtek.com, lgirdwood@gmail.com,
linux-sound@vger.kernel.org, patches@opensource.cirrus.com
Subject: Re: [PATCH 09/15] ASoC: SDCA: Add UMP buffer helper functions
Date: Fri, 12 Sep 2025 11:33:11 +0100 [thread overview]
Message-ID: <aMP25yy/pxSlshf/@opensource.cirrus.com> (raw)
In-Reply-To: <384bd185-9aad-4590-9251-41c572dde3c2@linux.dev>
On Fri, Sep 12, 2025 at 12:15:45PM +0200, Pierre-Louis Bossart wrote:
> On 9/11/25 15:07, Charles Keepax wrote:
> > On Thu, Sep 11, 2025 at 02:33:46PM +0200, Pierre-Louis Bossart wrote:
> >>> 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...
> >
> > No, I had missed this property! That is excellent. Don't suppose
> > you have a similar magic touch with the "Wait until there are
> > no new requests for File Download" in the Class Software Load
> > Sequence?
>
> Glad I was able to help!
>
> The FDL state machine is rather complicated, I can't pretend
> understanding it fully :-(
Ha, you and me both.
> My limited understanding is that the Host doesn't control which
> firmware files need to be downloaded. The Device will provide
> detailed information back to the host. That's how I interpret
> the vague statement in Figure 213:
>
> "Device interprets
> FDL_Host_request and perhaps
> requests one or more new
> FDL_Set_Index(es)."
>
> IOW the number of sets to download is indicated in the
> FDLD_Needs_set control. Based on the bit pattern in Table 240,
> there can be up to 8 sets (3 bits).
>
> The main issue I have is that the files will be transmitted
> by chunks. I am not quite sure who defines the size of
> the chunks, but each chunk should be handled by the same
> "ownership-transition-max-delay" property. It's rather odd that
> the timeout doesn't depend on the size of the chunk, but assuming
> this is correct the time to download should be bound by number of
> sets x number of chunks per set x ownership-transition-max-delay.
Our devices only download a since chunk so we haven't added
support yet for the indirect FDL yet. Which keeps things a
little simpler and means we can file work out the chunk size as a
problem for later. Although my rough interpretation of that bit
is that the chunk size is defined by the UMP buffer itself, so
basically the host fills the whole buffer each time, but if the
file is larger than the buffer it takes a few goes to send it all.
> That's the limited extended of my FDL "magic touch" I am afraid...
Yeah mainly my question was that it says "wait until" but I have
never been able to define what that wait is. One possible
interpretation is as soon as you see the device not requesting
FDL you can progress, but perhaps a safer interpretation is if
you see X milliseconds without an FDL request you can progress
which is what we have gone with in the code for now. Was really
just hoping you were going to be like "you also missed this other
useful DisCo property" :-)
But I am just about to push up a v2 so we can carry on the chats
on those.
Thanks,
Charles
next prev parent reply other threads:[~2025-09-12 10:33 UTC|newest]
Thread overview: 50+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-09-05 14:31 [PATCH 00/15] Add SDCA UMP/FDL support Charles Keepax
2025-09-05 14:31 ` [PATCH 01/15] regmap: sdw-mbq: Don't assume the regmap device is the SoundWire slave Charles Keepax
2025-09-08 11:35 ` Pierre-Louis Bossart
2025-09-08 12:38 ` Charles Keepax
2025-09-05 14:31 ` [PATCH 02/15] ASoC: SDCA: Add manual PM runtime gets to IRQ handlers Charles Keepax
2025-09-08 11:42 ` Pierre-Louis Bossart
2025-09-08 13:31 ` Charles Keepax
2025-09-08 14:15 ` Charles Keepax
2025-09-09 12:56 ` Pierre-Louis Bossart
2025-09-05 14:31 ` [PATCH 03/15] ASoC: SDCA: Pass SoundWire slave to HID Charles Keepax
2025-09-05 14:31 ` [PATCH 04/15] ASoC: SDCA: Pass device register map from IRQ alloc to handlers Charles Keepax
2025-09-08 11:49 ` Pierre-Louis Bossart
2025-09-08 12:56 ` Charles Keepax
2025-09-09 13:05 ` Pierre-Louis Bossart
2025-09-09 16:43 ` Charles Keepax
2025-09-05 14:31 ` [PATCH 05/15] ASoC: SDCA: Update externally_requested flag to cover all requests Charles Keepax
2025-09-05 14:31 ` [PATCH 06/15] ASoC: SDCA: Factor out a helper to find SDCA IRQ data Charles Keepax
2025-09-05 14:31 ` [PATCH 07/15] ASoC: SDCA: Rely less on the ASoC component in IRQ handling Charles Keepax
2025-09-08 10:27 ` Liao, Bard
2025-09-08 12:56 ` Charles Keepax
2025-09-08 14:43 ` Charles Keepax
2025-09-09 1:42 ` Liao, Bard
2025-09-09 8:56 ` Charles Keepax
2025-09-10 1:33 ` Liao, Bard
2025-09-08 11:55 ` Pierre-Louis Bossart
2025-09-08 12:58 ` Charles Keepax
2025-09-05 14:31 ` [PATCH 08/15] ASoC: SDCA: Force some SDCA Controls to be volatile Charles Keepax
2025-09-05 14:31 ` [PATCH 09/15] ASoC: SDCA: Add UMP buffer helper functions Charles Keepax
2025-09-08 12:02 ` Pierre-Louis Bossart
2025-09-08 13:15 ` Charles Keepax
2025-09-09 13:14 ` Pierre-Louis Bossart
2025-09-09 16:52 ` Charles Keepax
2025-09-10 12:09 ` Pierre-Louis Bossart
2025-09-10 13:37 ` Charles Keepax
2025-09-10 14:29 ` Pierre-Louis Bossart
2025-09-10 15:39 ` Charles Keepax
2025-09-11 12:33 ` Pierre-Louis Bossart
2025-09-11 13:07 ` Charles Keepax
2025-09-12 10:15 ` Pierre-Louis Bossart
2025-09-12 10:33 ` Charles Keepax [this message]
2025-09-12 11:30 ` Pierre-Louis Bossart
2025-09-05 14:31 ` [PATCH 10/15] ASoC: SDCA: Add SDCA FDL data parsing Charles Keepax
2025-09-05 14:31 ` [PATCH 11/15] ASoC: SDCA: Add FDL library for XU entities Charles Keepax
2025-09-08 12:14 ` Pierre-Louis Bossart
2025-09-08 13:16 ` Charles Keepax
2025-09-09 13:14 ` Pierre-Louis Bossart
2025-09-05 14:31 ` [PATCH 12/15] ASoC: SDCA: Add XU-specific IRQ processing Charles Keepax
2025-09-05 14:31 ` [PATCH 13/15] ASoC: SDCA: Add completion for FDL start and stop Charles Keepax
2025-09-05 14:31 ` [PATCH 14/15] ASoC: SDCA: Add early IRQ handling Charles Keepax
2025-09-05 14:31 ` [PATCH 15/15] ASoC: SDCA: Add HID button IRQ Charles Keepax
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=aMP25yy/pxSlshf/@opensource.cirrus.com \
--to=ckeepax@opensource.cirrus.com \
--cc=broonie@kernel.org \
--cc=lgirdwood@gmail.com \
--cc=linux-sound@vger.kernel.org \
--cc=patches@opensource.cirrus.com \
--cc=peter.ujfalusi@linux.intel.com \
--cc=pierre-louis.bossart@linux.dev \
--cc=rafael@kernel.org \
--cc=shumingf@realtek.com \
--cc=yung-chuan.liao@linux.intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox