| Message ID | 20210123121720.79863-4-hdegoede@redhat.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | MFD/extcon/ASoC: Rework arizona codec jack-detect support | expand |
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. But to be clear not asking for any changes just more about trying to refine my understanding of things. Thanks, Charles
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
diff --git a/sound/soc/intel/boards/bytcr_wm5102.c b/sound/soc/intel/boards/bytcr_wm5102.c index f38850eb2eaf..cdfe203ed9fa 100644 --- a/sound/soc/intel/boards/bytcr_wm5102.c +++ b/sound/soc/intel/boards/bytcr_wm5102.c @@ -18,6 +18,7 @@ #include <linux/platform_device.h> #include <linux/slab.h> #include <linux/spi/spi.h> +#include <sound/jack.h> #include <sound/pcm.h> #include <sound/pcm_params.h> #include <sound/soc.h> @@ -31,6 +32,7 @@ #define WM5102_MAX_SYSCLK_11025 45158400 /* max sysclk for 11.025K family */ struct byt_wm5102_private { + struct snd_soc_jack jack; struct clk *mclk; struct gpio_desc *spkvdd_en_gpio; }; @@ -177,11 +179,23 @@ static const struct snd_kcontrol_new byt_wm5102_controls[] = { SOC_DAPM_PIN_SWITCH("Speaker"), }; +static struct snd_soc_jack_pin byt_wm5102_pins[] = { + { + .pin = "Headphone", + .mask = SND_JACK_HEADPHONE, + }, + { + .pin = "Headset Mic", + .mask = SND_JACK_MICROPHONE, + }, +}; + static int byt_wm5102_init(struct snd_soc_pcm_runtime *runtime) { struct snd_soc_card *card = runtime->card; struct byt_wm5102_private *priv = snd_soc_card_get_drvdata(card); - int ret; + struct snd_soc_component *component = asoc_rtd_to_codec(runtime, 0)->component; + int ret, jack_type; card->dapm.idle_bias_off = true; @@ -210,6 +224,18 @@ static int byt_wm5102_init(struct snd_soc_pcm_runtime *runtime) return ret; } + jack_type = ARIZONA_JACK_MASK | SND_JACK_BTN_0 | SND_JACK_BTN_1 | + SND_JACK_BTN_2 | SND_JACK_BTN_3; + ret = snd_soc_card_jack_new(card, "Headset", jack_type, + &priv->jack, byt_wm5102_pins, + ARRAY_SIZE(byt_wm5102_pins)); + if (ret) { + dev_err(card->dev, "Error creating jack: %d\n", ret); + return ret; + } + + snd_soc_component_set_jack(component, &priv->jack, NULL); + return 0; }