Hi Takashi, It looks no issue now. Could I need to generate new patch for you? I will send this patch to Intel to do the test. BR, Kailang -----Original Message----- From: Takashi Iwai <tiwai@xxxxxxx> Sent: Thursday, November 29, 2018 3:45 PM To: Kailang <kailang@xxxxxxxxxxx> Cc: (alsa-devel@xxxxxxxxxxxxxxxx) <alsa-devel@xxxxxxxxxxxxxxxx> Subject: Re: HDA headset button support On Thu, 29 Nov 2018 08:22:39 +0100, Kailang wrote: > > Hi Takashi, > > I test the patch. > It will go to Max or Min volume when I press HS button up or down. > > + state = jack->button_state; > + if (get_jack_plug_state(jack->pin_sense)) > + state |= jack->type; > + snd_jack_report(jack->jack, state); > If (jack->nid == 0x55) > snd_jack_report(jack->jack, 0); ==> Add this will solve keep volume up and down. > > If the pin sense active, it will update Mic and Headphone state. > (Upper code) But it still had chance to update 0x55 during unplug and plug state. > So, Maybe need to clear the Button state. I guess. As we don't receive button-release events, it makes sense to send the button clear at each time, yes. For the one-shot button event, you can handle like below. --- a/sound/pci/hda/hda_jack.c +++ b/sound/pci/hda/hda_jack.c @@ -343,6 +343,11 @@ void snd_hda_jack_report_sync(struct hda_codec *codec) if (get_jack_plug_state(jack->pin_sense)) state |= jack->type; snd_jack_report(jack->jack, state); + if (jack->button_state) { + snd_jack_report(jack->jack, + state & ~jack->button_state); + jack->button_state = 0; /* button released */ + } } } EXPORT_SYMBOL_GPL(snd_hda_jack_report_sync); thanks, Takashi > > BR, > Kailang > > -----Original Message----- > From: Takashi Iwai <tiwai@xxxxxxx> > Sent: Wednesday, November 28, 2018 10:05 PM > To: Kailang <kailang@xxxxxxxxxxx> > Cc: (alsa-devel@xxxxxxxxxxxxxxxx) <alsa-devel@xxxxxxxxxxxxxxxx> > Subject: Re: HDA headset button support > > On Wed, 28 Nov 2018 13:03:58 +0100, > Kailang wrote: > > > > Hi Takashi, > > > > >I'd rather like to extend snd_hda_jack_add_kctl() with the key > > >support (maybe adding a new function). You can pass there the > > >combination of SND_JACK_BTN_X with KEY_XXX in an array. > > > > Do you mean create new function like below? > > snd_hda_jack_add_key_kctl() > > > > It couldn't use snd_hda_jack_add_kctl() directly. > > Because it had report pin state and get type from NID. > > > > type = get_input_jack_type(codec, nid); > > ..... > > ..... > > state = snd_hda_jack_detect(codec, nid); > > snd_jack_report(jack->jack, state ? jack->type : 0); > > > > So, to create a new function in hda_jack.c was better. > > Well, this seems more deeper than I thought. > > Below is my quick attempt to add the infrastructure. This adds more flexibility but still easiness for other possible button implementations, hopefully. > > Could you check the series below? > > > thanks, > > Takashi > > > ------Please consider the environment before printing this e-mail. > _______________________________________________ Alsa-devel mailing list Alsa-devel@xxxxxxxxxxxxxxxx http://mailman.alsa-project.org/mailman/listinfo/alsa-devel