All of lore.kernel.org
 help / color / mirror / Atom feed
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?


  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.