Re: [PATCH 1/2] ASoC: rt711: remap buttons

[Curtis Malainey <cujomalainey@google.com>]

On Thu, Jun 17, 2021 at 7:23 AM Pierre-Louis Bossart
<pierre-louis.bossart@linux.intel.com> wrote:
>
>
>
> On 6/17/21 8:27 AM, Mark Brown wrote:
> > On Thu, Jun 17, 2021 at 01:19:05PM +0000, Shuming [范書銘] wrote:
> >
> >>>> This patch uses the same mapping as the machine driver:
> >>>> BTN_0 : KEY_PLAYPAUSE
> >>>> BTN_1 : KEY_VOICECOMMAND
> >>>> BTN_2 : KEY_VOLUMEUP
> >>>> BTN_3 : KEY_VOLUMEDOWN
> >
> >>> Which machine driver?  Can't there be multiple machine drivers, and if
> >>> they're already overriding things why do this?
> >
> >> The rt711 codec is designed in the Intel platform only for now.
> >
> > I'm sure your sales team would be happy to change that!
> >
> >> The machine driver is 'sof_sdw.c' that resides under sound/soc/intel/boards.
> >> It seems Intel uses the same mapping in all other Intel machine drivers.
> >> Please check the commit as below.
> >> https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git/commit/?id=1f64a08bd5a9a218deb37c03c1e98e9567379698
> >
> > Sure, but if the machine drivers for these platforms already do the
> > right thing why change the CODEC driver?
> >
> >> Hi Sathya,
> >> Do you know why the m/c driver uses this mapping?
> >
> > I'd guess it's just because that's the standard set of headphone buttons
> > that Intel uses for their platforms.
>
> Intel has no specific requirement, we just follow what the Google 4-button description says [1]
>
> That said, I can't recall why half of the machine drivers follow one pattern and the other half a different one.
>
> Same as spec:
> sof_da7219_max98373.c:  snd_jack_set_key(jack->jack, SND_JACK_BTN_0, KEY_PLAYPAUSE);
> sof_da7219_max98373.c-  snd_jack_set_key(jack->jack, SND_JACK_BTN_1, KEY_VOLUMEUP);
> sof_da7219_max98373.c-  snd_jack_set_key(jack->jack, SND_JACK_BTN_2, KEY_VOLUMEDOWN);
> sof_da7219_max98373.c-  snd_jack_set_key(jack->jack, SND_JACK_BTN_3, KEY_VOICECOMMAND);
>
> sof_cs42l42.c:  snd_jack_set_key(jack->jack, SND_JACK_BTN_0, KEY_PLAYPAUSE);
> sof_cs42l42.c-  snd_jack_set_key(jack->jack, SND_JACK_BTN_1, KEY_VOLUMEUP);
> sof_cs42l42.c-  snd_jack_set_key(jack->jack, SND_JACK_BTN_2, KEY_VOLUMEDOWN);
> sof_cs42l42.c-  snd_jack_set_key(jack->jack, SND_JACK_BTN_3, KEY_VOICECOMMAND);
>
> swap wrt. spec:
> sof_sdw_rt711.c:        snd_jack_set_key(jack->jack, SND_JACK_BTN_0, KEY_PLAYPAUSE);
> sof_sdw_rt711.c-        snd_jack_set_key(jack->jack, SND_JACK_BTN_1, KEY_VOICECOMMAND);
> sof_sdw_rt711.c-        snd_jack_set_key(jack->jack, SND_JACK_BTN_2, KEY_VOLUMEUP);
> sof_sdw_rt711.c-        snd_jack_set_key(jack->jack, SND_JACK_BTN_3, KEY_VOLUMEDOWN);
>
> kbl_rt5663_max98927.c:  snd_jack_set_key(jack->jack, SND_JACK_BTN_0, KEY_PLAYPAUSE);
> kbl_rt5663_max98927.c-  snd_jack_set_key(jack->jack, SND_JACK_BTN_1, KEY_VOICECOMMAND);
> kbl_rt5663_max98927.c-  snd_jack_set_key(jack->jack, SND_JACK_BTN_2, KEY_VOLUMEUP);
> kbl_rt5663_max98927.c-  snd_jack_set_key(jack->jack, SND_JACK_BTN_3, KEY_VOLUMEDOWN);
>
> Brent, Curtis, Jimmy, can you comment on the order?

Sorry for the late reply, I couldn't figure out why this slipped by my
email filter, I guess I should look closer next time. Soraka has been
checked (kbl_rt5663_max98927) and yes the mapping does appear to be
incorrect, volume up key returns voice command in evtest. Sathya will
have to check the headset button mapping, on rt711 but my guess is it
is also incorrect.

>
>
> [1] https://source.android.com/devices/accessories/headset/plug-headset-spec#control-function_mapping
>
>
>
>
>