public inbox for linux-sound@vger.kernel.org
 help / color / mirror / Atom feed
From: Pierre-Louis Bossart <pierre-louis.bossart@linux.dev>
To: Charles Keepax <ckeepax@opensource.cirrus.com>
Cc: Richard Fitzgerald <rf@opensource.cirrus.com>,
	broonie@kernel.org, vkoul@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 3/4] ASoC: SDCA: Add basic SDCA class driver
Date: Tue, 13 Jan 2026 23:05:31 +0100	[thread overview]
Message-ID: <2ba6e4e1-1040-4eaf-ac2d-3a308866097c@linux.dev> (raw)
In-Reply-To: <aWaAbaDiuJlCnL1Q@opensource.cirrus.com>

On 1/13/26 18:27, Charles Keepax wrote:
> On Tue, Jan 06, 2026 at 06:10:06PM +0100, Pierre-Louis Bossart wrote:
>> On 1/6/26 13:58, Charles Keepax wrote:
>>> On Sat, Dec 20, 2025 at 12:04:47PM +0100, Pierre-Louis Bossart wrote:
>>>> On 12/10/25 10:55, Charles Keepax wrote:
>>>>> On Tue, Dec 09, 2025 at 12:47:06PM +0000, Pierre-Louis Bossart wrote:
>>>> The only issue I have with it is whether one would deregister
>>>> the child device on a soft reset or whenever the device
>>>> loses sync? It'd be somewhat ugly to do so, but then again
>>>> we have the issue that for the second enumeration we already
>>>> have a probed child driver, which would bring us back to an
>>>> async model really. The 'beauty' of the current model is
>>>> that the probe does nothing really, everything happens at
>>>> enumeration/sync loss. If we dynamically add/remove child
>>>> devices it'll be 'fun' really quickly.
>>>
>>> Yeah that is one of the questions I have pondered a few times.
>>> Were I usually get to is separating out expected cases of
>>> something dropping off the bus and unexpected cases. For say
>>> suspend/resume it is quite likely the device will drop off the
>>> bus and that is expected and probably shouldn't result in the
>>> child drivers being removed, as it will as you say become 'fun'
>>> very quickly. However, an unexpected drop off the bus during
>>> normal operation is really a pretty fatal error. In many ways the
>>> best thing to do is probably to remove the child drivers in this
>>> case, since that will tear down the sound card, and allow
>>> everything to be rebuilt. But these are far from fully formed
>>> ideas, for the most part at the moment we just report things went
>>> bad.
>>
>> A device falling off the bus is not that rare in early
>> bring-up or experimental platforms.  IIRC some devices will
>> require a full reset after firmware download.  Not to mention
>> the glitches that can be seen consistently after clock stop
>> on some platforms.
> 
> So doing a reset after firmware download should be fine, I would
> consider than an expected drop off.
> 
>> My take is that it's better to 'hide' some of the low-level
>> detail and keep the card visible to apps, always.
> 
> I would agree here, it is better if the card stays present and
> hides non-error states.

Sorry, I am not following what you meant by 'hide non-error states'

If the card stays present, that would require the child device to remain registered.
Otherwise if the child device is unregistered, the card would be torn down due to missing resources, no?
Or maybe we end-up with a case where the resource is already in-use so the child device cannot be unregistered...

>> That means the drivers need to deal with cases where accesses
>> to devices might be delayed due to slow sync or sync loss. As
>> long as everything recovers we are good to go.
> 
> Yeah that is a point we slightly disagree on, I really am not
> a huge fan of auto-recovery stuff. Been involved in too many
> situations where the auto-recovery recovers *most* of the time
> and takes a 1/10 fail to a 1/10000 fail.
> 
> Taking this case, if the device loses sync during audio that
> is gonna glitch the audio, which makes it a problem I will be
> expected to fix. I would rather get a report the audio stopped
> with a log up to that point, than get reports of audio glitches.

I don't disagree about audio glitches, my main problem is with sync loss during initialization or suspend-resume.

Maybe a tangent but if a device loses sync while audio is playing, there would be no xrun signaled to the upper layers. The bus allocation reserves specific slots for specific streams, and the data will be written/read whether there's a device or not. That's how we tested the 'virtual' devices to create pretend master-only audio cards with no actual codecs attached.
If we really want to report glitches due to sync loss during audio playback, we have some plumbing to do. I tested with a couple of years ago by forcing a device to reset with a debugfs command, it wasn't detected by any ALSA layers.

>>>> That's precisely the problem, the model assumes something
>>>> that is broken left and right in practice.
>>>> Exhibit A is the PCI/HDaudio parts where we rely on an async
>>>> probe due to the module handling.
>>>
>>> I think I am perhaps not totally familiar with the issues on the
>>> PCI/HDaudio side. What is the primary issue here?
>>
>> The HDAudio side loads different modules, which can take a lot
>> of time. For that reason, the driver probe returns immediately
>> but most of the processing is done in a workqueue. This leads
>> to two problems
>> a) if the processing fails for some reason, we have no way
>>    of signaling any error.
>> b) if the card is created independently from the PCI driver,
>>    e.g. with ACPI information, then we have no way of telling
>>    that the resources needed by the card are available.
> 
> I mean sure it does make the boot look faster, but is it really
> faster as the devices weren't actually ready when we told
> user-space they were? And now we have to deal with the problems
> outlined here, as well as what happens if user-space/another
> driver tries to use the device whilst it is in this
> not-quite-ready state. It feels like we sacrifice quite a lot
> for appearing faster.
> 
>>>> You also have devices that require firmware download to be
>>>> functional, or some sort of internal ramp-up needed.
>>>
>>> Yeah these can be fairly annoying. But both of these can be
>>> tackled with either handling the situation in probe, or if that
>>> makes boot too slow, moving to a similar child driver approach.
>>
>> Both options are not so good IMHO...
> 
> What is it you dislike about them? Just the slower boot, or
> something more fundamental?

The slower boot isn't going to be accepted by anyone, and in the specific case of HDaudio I don't think anyone has an appetite for changing the probe/workqueue approach that's been around for many many years.
I don't think creating a child device is much better than using a workqueue, in both cases errors during the two-step probe can't be handled, you end-up with zoombie devices that are probed but not functional.

The only benefit you get with the child device is that if it provides the resources required by the deferred probe you have nothing to do. the open is what happens when those resources are not present due to sync loss.

  reply	other threads:[~2026-01-13 22:06 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-09-25 13:33 [PATCH 0/4] Add SDCA class driver Charles Keepax
2025-09-25 13:33 ` [PATCH 1/4] ASoC: SDCA: Add helper to write initialization writes Charles Keepax
2025-10-13  5:43   ` Vinod Koul
2025-09-25 13:33 ` [PATCH 2/4] ASoC: SDCA: add function devices Charles Keepax
2025-09-25 13:33 ` [PATCH 3/4] ASoC: SDCA: Add basic SDCA class driver Charles Keepax
2025-10-27 15:02   ` Pierre-Louis Bossart
2025-10-30 15:29     ` Charles Keepax
2025-10-30 15:36       ` Richard Fitzgerald
2025-11-04 16:13         ` Pierre-Louis Bossart
2025-11-04 17:14           ` Charles Keepax
2025-12-09 12:47             ` Pierre-Louis Bossart
2025-12-10  9:55               ` Charles Keepax
2025-12-20 11:04                 ` Pierre-Louis Bossart
2026-01-06 12:58                   ` Charles Keepax
2026-01-06 17:10                     ` Pierre-Louis Bossart
2026-01-13 17:27                       ` Charles Keepax
2026-01-13 22:05                         ` Pierre-Louis Bossart [this message]
2026-01-14  9:58                           ` Charles Keepax
2025-09-25 13:33 ` [PATCH 4/4] ASoC: SDCA: Add basic SDCA function driver Charles Keepax
2025-10-27 15:24   ` Pierre-Louis Bossart
2025-10-30 15:44     ` 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=2ba6e4e1-1040-4eaf-ac2d-3a308866097c@linux.dev \
    --to=pierre-louis.bossart@linux.dev \
    --cc=broonie@kernel.org \
    --cc=ckeepax@opensource.cirrus.com \
    --cc=lgirdwood@gmail.com \
    --cc=linux-sound@vger.kernel.org \
    --cc=patches@opensource.cirrus.com \
    --cc=peter.ujfalusi@linux.intel.com \
    --cc=rf@opensource.cirrus.com \
    --cc=shumingf@realtek.com \
    --cc=vkoul@kernel.org \
    --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