From: Pierre-Louis Bossart <pierre-louis.bossart@linux.dev>
To: Matthew Schwartz <matthew.schwartz@linux.dev>, broonie@kernel.org
Cc: Shenghao Ding <shenghao-ding@ti.com>, Kevin Lu <kevin-lu@ti.com>,
Baojun Xu <baojun.xu@ti.com>, Sen Wang <sen@ti.com>,
Liam Girdwood <lgirdwood@gmail.com>,
Jaroslav Kysela <perex@perex.cz>, Takashi Iwai <tiwai@suse.com>,
Niranjan H Y <niranjan.hy@ti.com>,
Bard Liao <yung-chuan.liao@linux.intel.com>,
linux-sound@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/3] ASoC: tas2783-sdw: load the firmware synchronously
Date: Mon, 19 Oct 2026 18:04:10 +0200 [thread overview]
Message-ID: <f51a62bc-18f9-4a82-aafa-c315941eb4d7@linux.dev> (raw)
In-Reply-To: <20261018200009.1493438-3-matthew.schwartz@linux.dev>
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?
next prev parent reply other threads:[~2026-10-19 16:20 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-10-18 20:00 [PATCH 0/3] ASoC: tas2783-sdw: volume and firmware load fixes Matthew Schwartz
2026-10-18 20:00 ` [PATCH 1/3] ASoC: tas2783-sdw: keep the volume settings across resets Matthew Schwartz
2026-10-19 15:56 ` Pierre-Louis Bossart
2026-10-19 17:53 ` Matthew Schwartz
2026-10-18 20:00 ` [PATCH 2/3] ASoC: tas2783-sdw: load the firmware synchronously Matthew Schwartz
2026-10-19 16:04 ` Pierre-Louis Bossart [this message]
2026-10-19 17:58 ` Matthew Schwartz
2026-10-18 20:00 ` [PATCH 3/3] ASoC: tas2783-sdw: refuse playback when the init writes failed Matthew Schwartz
2026-10-19 16:10 ` Pierre-Louis Bossart
2026-10-19 18:01 ` Matthew Schwartz
2026-10-18 23:03 ` [PATCH 0/3] ASoC: tas2783-sdw: volume and firmware load fixes Lee Mann
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=f51a62bc-18f9-4a82-aafa-c315941eb4d7@linux.dev \
--to=pierre-louis.bossart@linux.dev \
--cc=baojun.xu@ti.com \
--cc=broonie@kernel.org \
--cc=kevin-lu@ti.com \
--cc=lgirdwood@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-sound@vger.kernel.org \
--cc=matthew.schwartz@linux.dev \
--cc=niranjan.hy@ti.com \
--cc=perex@perex.cz \
--cc=sen@ti.com \
--cc=shenghao-ding@ti.com \
--cc=tiwai@suse.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.