Re: [PATCH v4 00/17] ALSA: hda: cirrus: Add initial DSP support and firmware loading

[Takashi Iwai <tiwai@suse.de>]

On Mon, 30 May 2022 12:53:29 +0200,
Charles Keepax wrote:
> 
> On Mon, May 30, 2022 at 12:45:08PM +0200, Takashi Iwai wrote:
> > On Mon, 30 May 2022 12:34:15 +0200,
> > Charles Keepax wrote:
> > Well, if an ALSA control can trigger the firmware loading, that's
> > already fragile.  A firmware loading is a heavy task, which should
> > happen only at probing and/or resuming in general.  Do we have other
> > drivers doing the f/w loading triggered by a kctl...?
> > 
> > > I guess we could look at adding locked versions of the add
> > > control functions as well if that might be preferred?
> > 
> > If the patterns of additional kctls (specific for firmware?) are
> > fixed, we may create all such kctls beforehand and let them inactive
> > unless the corresponding firmware is really loaded, too.
> > 
> 
> I am afraid we do, basically all the Wolfson/Cirrus audio devices
> allow you to select the firmware through a kctl. The patterns of
> controls are specific to the firmwares, so we can't really create
> them ahead of time. One could maybe look at changing when the
> firmwares are loaded, such as attempting to load all possible
> firmwares on boot or something but its a fairly sizable change
> that isn't without some side effects.

The call of request_firmawre() itself can be pretty lengthy (e.g. it
may hold until user-space process uploads the firmware if the fallback
mode is enabled), and it implies that the request_firmware() call
doesn't fit well as the operation to be done in a kctl put callback.
So, even if we accept the firmware loading behavior via kctl as-is,
the whole procedure should be async in work instead; namely, not only
kctl creation/deletion but both request_firmware() + post-process
should be done in the work.

And yet moreover, we'll need to consider some way for protecting
against DoS-like behavior by frequent kctl changes.


thanks,

Takashi