Re: [PATCH 2/3] ASoC: tas2783-sdw: load the firmware synchronously

[Matthew Schwartz <matthew.schwartz@linux.dev>]

On 10/19/26 9:04 AM, Pierre-Louis Bossart wrote:
> On 10/18/26 22:00, Matthew Schwartz wrote:
>> tas_fw_load() requests the firmware asynchronously and gives up waiting
>> for the callback after three seconds. tas_io_init() then returns without
>> the SDCA function initialization writes, while the callback carries on
>> downloading, sets fw_dl_success and so allows playback on an amp that
>> never got them. Nothing cancels the callback when the driver is unbound
>> either, so it can run on freed driver data. The download takes about
>> 1.2 s per amp on the ASUS ROG Xbox Ally X20, so it fits there, but
>> nothing bounds it.
>>
>> Request the firmware with request_firmware_direct() and download it in
>> the caller, as rt1320 does from its io_init, which removes the wait, the
>> timeout and the callback. The download now returns its result, so a
>> missing or bad firmware file fails tas_io_init() before the init writes
>> instead of running them on an amp that refuses playback anyway.
> 
> The changes in this patch look ok, but this commit description doesn't
> really say *why* or *how often* the firmware download fails.
> Is this really a problem on actual devices?
> Is this defensive programming?

This one is defensive, yeah. I haven't seen the timeout fire. On the
X20 the download takes about 1.2 s per amp against the 3 s limit, and
every TAS2783 file in linux-firmware is the same size. But if it does
fire, tas_io_init() returns without the init writes while the callback
can still go on to set fw_dl_success, so playback is allowed, and
nothing cancels the callback on unbind. tas_fw_load() waits for the
callback anyway, so the asynchronous request gains nothing. I'll
clarify that in v2 and drop the Fixes tag, or I can drop the patch from
this series if we'd rather wait to see if this actually causes issues in
practice.