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@xxxxxxxxx> >> Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx> >> --- >> +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