From: Charles Keepax <ckeepax@opensource.cirrus.com>
To: Pierre-Louis Bossart <pierre-louis.bossart@linux.dev>
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: Wed, 10 Dec 2025 09:55:45 +0000 [thread overview]
Message-ID: <aTlDoVxI0gJMQOuw@opensource.cirrus.com> (raw)
In-Reply-To: <5dcb51e4-e9e1-4b26-816a-90c92fbb200d@linux.dev>
On Tue, Dec 09, 2025 at 12:47:06PM +0000, Pierre-Louis Bossart wrote:
> > That is some scary stuff there, that is basically working around
> > the fact that with those drivers the soundcard is created before
> > the hardware is actually ready. But its one aspect of that and
> > there are likely many knock on effects/races hiding in there. At
> > some point we should probably revisit the whole enumeration
> > thing in soundwire it does cause a lot of scary code.
> I don't see how we can revisit this, the codec probe happens
> based on ACPI information even before the bus is started. The
> card driver is also probed when the PCI driver creates a platform
> device.
I mean in general the fact SoundWire probes devices before they
are available. It introduces a lot of scary things, as basically
the whole kernel is setup to believe that when a device is probed
it is ready. I mean I know why we did it, because we might need
to setup some things (resets/regulators) to cause the SoundWire
device to enumerate, but it does result in scaryness.
> > That said... the class driver doesn't have the same problem
> > however, because of the two layer nature of the auxiliary driver
> > stuff. The soundwire driver binds to the device and completes
> > probe, but it is the auxiliary drivers that are used in the
> > soundcard and those are only probed once the device itself has
> > enumerated in class_boot_work(). This means the sound card is
> > only created after the device has enumerated, so the same problem
> > isn't present and we can have a more normal PM runtime startup.
> Humm, that's an important detail indeed that I completely missed...
>
> You could also have registered the function subdevices based on
> ACPI information *before* the whole enumeration. I can see why
> you took that function-register-after-device-enumeration route,
> but I have mixed feelings about having separate mechanisms for
> vendor- and class-drivers.
At least currently you have separate mechanisms regardless
of the choice of probe time since all existing vendor drivers
register a single driver and the class driver registers many
through auxiliary. The difference is only in the secondary
drivers that the vendor drivers don't have anyway.
Personally I like the only registering the functions when the
device has enumerated approach as keeps interactions with other
kernel subsystems within the expectations those subsystems have
of the driver. There is a lot less to think about.
Ultimately, I think the long game solution here is probably that
we move SoundWire to internally have two devices. An initial
device the gets registered and does the probe setup and then a
child device that probes on enumeration. Basically, the same
setup as we have for the auxiliaries in SDCA but internal to
SoundWire. But in the mean time I think using MFD/auxiliary to
introduce a 2 step probe seems like a really neat solution.
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.
> 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.
Thanks,
Charles
next prev parent reply other threads:[~2025-12-10 9:56 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 [this message]
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
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=aTlDoVxI0gJMQOuw@opensource.cirrus.com \
--to=ckeepax@opensource.cirrus.com \
--cc=broonie@kernel.org \
--cc=lgirdwood@gmail.com \
--cc=linux-sound@vger.kernel.org \
--cc=patches@opensource.cirrus.com \
--cc=peter.ujfalusi@linux.intel.com \
--cc=pierre-louis.bossart@linux.dev \
--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