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, 6 Jan 2026 18:10:06 +0100	[thread overview]
Message-ID: <030c6d7f-cccd-414f-a2b7-51df88c7991f@linux.dev> (raw)
In-Reply-To: <aV0HDh57C2gdtYc5@opensource.cirrus.com>

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:
>>> Additionally, there arn't that many SDCA vendor drivers at the
>>> moment, a handful of Realtek ones and a TI one. If we wanted
>>> to standardise I would suggest standardising on the better
>>> solution, which to my mind is clearly this approach.
>>
>> I am not against having a single mechanism, and that change
>> would be relatively minimal.
>>
>> 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.

My take is that it's better to 'hide' some of the low-level detail and keep the card visible to apps, always. 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.

One way to test the recovery capabilities is to inject a bad sync pattern. The Cadence IP can do this manually, I vaguely remember adding a debugfs entry to test this but I don't recall if it ever made it in the mainline.

>>>> Note that things will be interesting when we use the new
>>>> ACPI aggregation information to create the card, we're missing
>>>> a means to re-trigger deferred probe checks as devices become
>>>> functional. I had a patch on this a couple of years ago, look
>>>> for "driver core: export driver_deferred_probe_trigger()",
>>>> we probably need to revisit the entire scheme...
>>>
>>> Apologies if I am misunderstanding but is this not another example
>>> of the advantages of probing when the device enumerates? If I
>>> understand correctly the problem those patches are solving is
>>> resources that become available outside of a probe break deferred
>>> probe. By tying the probe of the auxiliaries to the enumeration
>>> of the device, we ensure that link between probe and resource
>>> availability. I think the point Greg is making in that thread
>>> is that the kernel expects resources are available once probed,
>>> so if one is going to break that assumption it should really
>>> be handled exclusively in the driver that does that. But the
>>> simplest solution to my mind is just to not break that assumption
>>> then everything works as expected.
>>
>> 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.

That b) point isn't an issue at the moment because the PCI driver creates a platform devices which results in the card creation by the platform driver. But if that sequential part is broken then the deferred probe mechanism will fail by construction - it assumes that all resources are available when the probe returns.
 
>> 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...

>> Assuming a perfect behavior where all resources are available
>> at probe time is naive IMHO. In the specific case of the
>> machine driver probed with ACPI information that isn't tied
>> to the bus status, it's a guaranteed fail.
> 
> Well I do love a good bit of being naive until it bites me :-)
> Ultimately the machine driver has to call snd_soc_register_card
> which can probe defer if the card components aren't ready. What
> parts cause the issue here?

The issue is that the deferred probe mechanism revisits the list of needed resources when a new device is added or when a probe completes. When the probe completes 'later', then we are missing a mechanism to signal that a missing resource is now available.

  reply	other threads:[~2026-01-06 17:10 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 [this message]
2026-01-13 17:27                       ` Charles Keepax
2026-01-13 22:05                         ` Pierre-Louis Bossart
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=030c6d7f-cccd-414f-a2b7-51df88c7991f@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