All of lore.kernel.org
 help / color / mirror / Atom feed
From: Hans de Goede <hdegoede@redhat.com>
To: Charles Keepax <ckeepax@opensource.cirrus.com>
Cc: Lee Jones <lee.jones@linaro.org>,
	Cezary Rojewski <cezary.rojewski@intel.com>,
	Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>,
	Liam Girdwood <liam.r.girdwood@linux.intel.com>,
	Jie Yang <yang.jie@linux.intel.com>,
	Mark Brown <broonie@kernel.org>,
	patches@opensource.cirrus.com, linux-kernel@vger.kernel.org,
	Andy Shevchenko <andy.shevchenko@gmail.com>,
	alsa-devel@alsa-project.org
Subject: Re: [PATCH v4 13/13] ASoC: Intel: bytcr_wm5102: Add jack detect support
Date: Sat, 30 Jan 2021 19:26:32 +0100	[thread overview]
Message-ID: <9492d03c-2198-1298-b15c-944b2cdd7876@redhat.com> (raw)
In-Reply-To: <20210130154035.GX106851@ediswmail.ad.cirrus.com>

Hi,

On 1/30/21 4:40 PM, Charles Keepax wrote:
> On Sat, Jan 23, 2021 at 01:17:20PM +0100, Hans de Goede wrote:
>> Add jack detect support by creating a jack and calling
>> snd_soc_component_set_jack to register the created jack
>> with the codec.
>>
>> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>> +static struct snd_soc_jack_pin byt_wm5102_pins[] = {
>> +	{
>> +		.pin	= "Headphone",
>> +		.mask	= SND_JACK_HEADPHONE,
>> +	},
>> +	{
>> +		.pin	= "Headset Mic",
>> +		.mask	= SND_JACK_MICROPHONE,
>> +	},
>> +};
>> +
> 
> This patch looks fine to me, but I did have one small question.
> What is the thinking behind punting this to the machine driver?
> 
> I guess you can not register it if there is no jack present
> on the board, or if you have multiple jacks name them
> meaningfully. Although I sort of feel like those applied to
> the old extcon approach that just internally registered all
> the interfaces.

To be honest I'm not 100% sure why this is done this way,
this is how *all* ASoC drivers do it (AFAICT).

I think it is done this way because of 2 reasons:

1. The pins controlled by the jack are what for lack of
a better word I call "end-point" pins. And these get
registered by the machine-driver, so to make sure that
the names match it makes sense to also declare the
snd_soc_jack_pin array in the machine-driver.
For example the "Headphone" pin is a widget registered
by the machine driver as:

        SND_SOC_DAPM_HP("Headphone", NULL),

2. Probe ordering, the jack gets attached to the card and
when the coded driver's probe function runs the card does
not exist yet. But I think that could be worked around by
doing things in the component-probe function.

Regards,

Hans


WARNING: multiple messages have this Message-ID (diff)
From: Hans de Goede <hdegoede@redhat.com>
To: Charles Keepax <ckeepax@opensource.cirrus.com>
Cc: Cezary Rojewski <cezary.rojewski@intel.com>,
	alsa-devel@alsa-project.org, patches@opensource.cirrus.com,
	Jie Yang <yang.jie@linux.intel.com>,
	Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>,
	linux-kernel@vger.kernel.org,
	Liam Girdwood <liam.r.girdwood@linux.intel.com>,
	Andy Shevchenko <andy.shevchenko@gmail.com>,
	Mark Brown <broonie@kernel.org>, Lee Jones <lee.jones@linaro.org>
Subject: Re: [PATCH v4 13/13] ASoC: Intel: bytcr_wm5102: Add jack detect support
Date: Sat, 30 Jan 2021 19:26:32 +0100	[thread overview]
Message-ID: <9492d03c-2198-1298-b15c-944b2cdd7876@redhat.com> (raw)
In-Reply-To: <20210130154035.GX106851@ediswmail.ad.cirrus.com>

Hi,

On 1/30/21 4:40 PM, Charles Keepax wrote:
> On Sat, Jan 23, 2021 at 01:17:20PM +0100, Hans de Goede wrote:
>> Add jack detect support by creating a jack and calling
>> snd_soc_component_set_jack to register the created jack
>> with the codec.
>>
>> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>> +static struct snd_soc_jack_pin byt_wm5102_pins[] = {
>> +	{
>> +		.pin	= "Headphone",
>> +		.mask	= SND_JACK_HEADPHONE,
>> +	},
>> +	{
>> +		.pin	= "Headset Mic",
>> +		.mask	= SND_JACK_MICROPHONE,
>> +	},
>> +};
>> +
> 
> This patch looks fine to me, but I did have one small question.
> What is the thinking behind punting this to the machine driver?
> 
> I guess you can not register it if there is no jack present
> on the board, or if you have multiple jacks name them
> meaningfully. Although I sort of feel like those applied to
> the old extcon approach that just internally registered all
> the interfaces.

To be honest I'm not 100% sure why this is done this way,
this is how *all* ASoC drivers do it (AFAICT).

I think it is done this way because of 2 reasons:

1. The pins controlled by the jack are what for lack of
a better word I call "end-point" pins. And these get
registered by the machine-driver, so to make sure that
the names match it makes sense to also declare the
snd_soc_jack_pin array in the machine-driver.
For example the "Headphone" pin is a widget registered
by the machine driver as:

        SND_SOC_DAPM_HP("Headphone", NULL),

2. Probe ordering, the jack gets attached to the card and
when the coded driver's probe function runs the card does
not exist yet. But I think that could be worked around by
doing things in the component-probe function.

Regards,

Hans


  reply	other threads:[~2021-01-30 18:28 UTC|newest]

Thread overview: 66+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-23 12:13 [PATCH v4 00/13] MFD/extcon/ASoC: Rework arizona codec jack-detect support Hans de Goede
2021-01-23 12:13 ` Hans de Goede
2021-01-23 12:13 ` [PATCH v4 01/13] mfd: arizona: Drop arizona-extcon cells Hans de Goede
2021-01-23 12:13   ` Hans de Goede
2021-01-30 14:41   ` Charles Keepax
2021-01-30 14:41     ` Charles Keepax
2021-01-23 12:13 ` [PATCH v4 02/13] extcon: arizona: Fix some issues when HPDET IRQ fires after the jack has been unplugged Hans de Goede
2021-01-23 12:13   ` Hans de Goede
2021-01-30 14:43   ` Charles Keepax
2021-01-30 14:43     ` Charles Keepax
2021-01-23 12:13 ` [PATCH v4 03/13] extcon: arizona: Fix various races on driver unbind Hans de Goede
2021-01-23 12:13   ` Hans de Goede
2021-01-30 14:56   ` Charles Keepax
2021-01-30 14:56     ` Charles Keepax
2021-01-23 12:13 ` [PATCH v4 04/13] extcon: arizona: Fix flags parameter to the gpiod_get("wlf,micd-pol") call Hans de Goede
2021-01-23 12:13   ` [PATCH v4 04/13] extcon: arizona: Fix flags parameter to the gpiod_get("wlf, micd-pol") call Hans de Goede
2021-01-23 12:13 ` [PATCH v4 05/13] extcon: arizona: Always use pm_runtime_get_sync() when we need the device to be awake Hans de Goede
2021-01-23 12:13   ` Hans de Goede
2021-01-23 16:41   ` Charles Keepax
2021-01-23 16:41     ` Charles Keepax
2021-01-30 14:57   ` Charles Keepax
2021-01-30 14:57     ` Charles Keepax
2021-01-23 12:13 ` [PATCH v4 06/13] ASoC/extcon: arizona: Move arizona jack code to sound/soc/codecs/arizona-jack.c Hans de Goede
2021-01-23 12:13   ` Hans de Goede
2021-01-30 14:58   ` Charles Keepax
2021-01-30 14:58     ` Charles Keepax
2021-01-23 12:13 ` [PATCH v4 07/13] ASoC: arizona-jack: Move jack-detect variables to struct arizona_priv Hans de Goede
2021-01-23 12:13   ` Hans de Goede
2021-01-30 15:15   ` Charles Keepax
2021-01-30 15:15     ` Charles Keepax
2021-01-23 12:13 ` [PATCH v4 08/13] ASoC: arizona-jack: Use arizona->dev for runtime-pm Hans de Goede
2021-01-23 12:13   ` Hans de Goede
2021-01-30 15:16   ` Charles Keepax
2021-01-30 15:16     ` Charles Keepax
2021-01-23 12:13 ` [PATCH v4 09/13] ASoC: arizona-jack: convert into a helper library for codec drivers Hans de Goede
2021-01-23 12:13   ` Hans de Goede
2021-01-30 15:25   ` Charles Keepax
2021-01-30 15:25     ` Charles Keepax
2021-01-23 12:17 ` [PATCH v4 10/13] ASoC: arizona-jack: Use snd_soc_jack to report jack events Hans de Goede
2021-01-23 12:17   ` Hans de Goede
2021-01-23 12:17   ` [PATCH v4 11/13] ASoC: arizona-jack: Cleanup logging Hans de Goede
2021-01-23 12:17     ` Hans de Goede
2021-01-24 19:53     ` Andy Shevchenko
2021-01-24 19:53       ` Andy Shevchenko
2021-01-24 21:13       ` Hans de Goede
2021-01-24 21:13         ` Hans de Goede
2021-01-30 15:30     ` Charles Keepax
2021-01-30 15:30       ` Charles Keepax
2021-01-23 12:17   ` [PATCH v4 12/13] ASoC: arizona: Make the wm5102, wm5110, wm8997 and wm8998 drivers use the new jack library Hans de Goede
2021-01-23 12:17     ` Hans de Goede
2021-01-30 15:32     ` Charles Keepax
2021-01-30 15:32       ` Charles Keepax
2021-01-23 12:17   ` [PATCH v4 13/13] ASoC: Intel: bytcr_wm5102: Add jack detect support Hans de Goede
2021-01-23 12:17     ` Hans de Goede
2021-01-30 15:40     ` Charles Keepax
2021-01-30 15:40       ` Charles Keepax
2021-01-30 18:26       ` Hans de Goede [this message]
2021-01-30 18:26         ` Hans de Goede
2021-01-30 15:28   ` [PATCH v4 10/13] ASoC: arizona-jack: Use snd_soc_jack to report jack events Charles Keepax
2021-01-30 15:28     ` Charles Keepax
2021-02-04 11:05 ` [PATCH v4 00/13] MFD/extcon/ASoC: Rework arizona codec jack-detect support Lee Jones
2021-02-04 11:05   ` Lee Jones
2021-02-04 11:09   ` Hans de Goede
2021-02-04 11:09     ` Hans de Goede
2021-02-04 11:13     ` Lee Jones
2021-02-04 11:13       ` Lee Jones

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=9492d03c-2198-1298-b15c-944b2cdd7876@redhat.com \
    --to=hdegoede@redhat.com \
    --cc=alsa-devel@alsa-project.org \
    --cc=andy.shevchenko@gmail.com \
    --cc=broonie@kernel.org \
    --cc=cezary.rojewski@intel.com \
    --cc=ckeepax@opensource.cirrus.com \
    --cc=lee.jones@linaro.org \
    --cc=liam.r.girdwood@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=patches@opensource.cirrus.com \
    --cc=pierre-louis.bossart@linux.intel.com \
    --cc=yang.jie@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.