linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Mark Brown <broonie@kernel.org>
To: Richard Fitzgerald <rf@opensource.cirrus.com>
Cc: pierre-louis.bossart@linux.intel.com,
	rander.wang@linux.intel.com, shumingf@realtek.com,
	patches@opensource.cirrus.com, alsa-devel@alsa-project.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] ASoC: dapm: Revert "use component prefix when checking widget names"
Date: Mon, 5 Jul 2021 17:50:41 +0100	[thread overview]
Message-ID: <20210705165041.GC4574@sirena.org.uk> (raw)
In-Reply-To: <20210703125034.24655-1-rf@opensource.cirrus.com>

[-- Attachment #1: Type: text/plain, Size: 1786 bytes --]

On Sat, Jul 03, 2021 at 01:50:34PM +0100, Richard Fitzgerald wrote:

> That commit breaks all users of the snd_soc_component_*_pin() functions
> because it results in the prefix being added twice. It also breaks code
> that correctly uses the snd_soc_dapm_*_pin() functions.

> Use the snd_soc_component_*_pin() functions if you want the component
> prefix to be prepended automatically.

> Use the raw snd_soc_dapm_*_pin() functions if the caller has the full
> name that should be matched exactly.

I'm not sure the analysis of which function to use when is correct or
what we want here (though it will work ATM), though looking again more
closely at the patch it doesn't look entirely right either.  The way
this used to be done, and the way that older code will most likely
assume things work, was that the DAPM functions would first try to match
on the local DAPM context before falling back to doing a global match.
This is what the fallback loop is intended to do, and the dapm functions
are passing the "search other contexts" flag into dapm_find_widget().

I'd not expect the distinction you seem to expect between component and
DAPM and we probably have a bunch of older drivers that aren't working
correctly like the Realtek driver mentioned in the original fix.  I
think what needs to happen is that dapm_find_widget() needs to be
checking both the prefixed and non-prefixed names, and that the
component stuff shouldn't need to bother and just be a convenience
wrapper for users that happene to have a component to hand.
Alternatively we need to do an audit of all the non-machine drivers to
switch them to use the component functions exclusively (and possibly
some of the machine drivers as well), most of the CODEC users look to be
a small number of Wolfson/Cirrus ones.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

  reply	other threads:[~2021-07-05 16:51 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-03 12:50 [PATCH] ASoC: dapm: Revert "use component prefix when checking widget names" Richard Fitzgerald
2021-07-05 16:50 ` Mark Brown [this message]
2021-07-22  9:55   ` Richard Fitzgerald
2021-07-23 15:17     ` Richard Fitzgerald
2021-07-23 15:24       ` Mark Brown
2021-07-28 16:09     ` Mark Brown
2021-07-28 18:29       ` Richard Fitzgerald
2021-07-28 23:40         ` Mark Brown

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=20210705165041.GC4574@sirena.org.uk \
    --to=broonie@kernel.org \
    --cc=alsa-devel@alsa-project.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=patches@opensource.cirrus.com \
    --cc=pierre-louis.bossart@linux.intel.com \
    --cc=rander.wang@linux.intel.com \
    --cc=rf@opensource.cirrus.com \
    --cc=shumingf@realtek.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;
as well as URLs for NNTP newsgroup(s).