Re: [PATCH] hda: add support for jack detection on IDT/Sigmatel

[Matthew Ranostay <mranostay@embeddedalley.com>]

Takashi Iwai wrote:
> At Wed, 15 Oct 2008 10:43:28 -0400,
> Matthew Ranostay wrote:
>> diff --git a/core/jack.c b/core/jack.c
>> index 8133a2b..7a5b07b 100644
>> --- a/core/jack.c
>> +++ b/core/jack.c
>> @@ -95,7 +95,7 @@ int snd_jack_new(struct snd_card *card, const char *id, int type,
>>  		goto fail_input;
>>  	}
>>
>> -	jack->input_dev->phys = "ALSA";
>> +	jack->input_dev->phys = "ALSA Jack Detection";
> 
> Any reason to change?
> 
> Well, I think even the original phys value "ALSA" is a bit strange in
> comparison with values of other input drivers, such as
> "isa0060/serio0/input0", "PNP0C0C/button/input0", too...
> Mark, what do you think?
> 
> 
>>  	jack->type = type;
>>
>> diff --git a/pci/Kconfig b/pci/Kconfig
>> index 7003711..7e40890 100644
>> --- a/pci/Kconfig
>> +++ b/pci/Kconfig
>> @@ -501,6 +501,7 @@ config SND_HDA_INTEL
>>  	tristate "Intel HD Audio"
>>  	select SND_PCM
>>  	select SND_VMASTER
>> +	select SND_JACK if INPUT=y || INPUT=SND
>>  	help
>>  	  Say Y here to include support for Intel "High Definition
>>  	  Audio" (Azalia) motherboard devices.
>> diff --git a/pci/hda/hda_codec.h b/pci/hda/hda_codec.h
>> index 60468f5..93dc961 100644
>> --- a/pci/hda/hda_codec.h
>> +++ b/pci/hda/hda_codec.h
>> @@ -729,6 +729,9 @@ struct hda_codec {
>>
>>  	struct snd_hwdep *hwdep;	/* assigned hwdep device */
>>
>> +	/* jack detection */
>> +	struct snd_jack *jack;
>> +
>>  	/* misc flags */
>>  	unsigned int spdif_status_reset :1; /* needs to toggle SPDIF for each
>>  					     * status change
> 
> Does this have to be in hda_codec core now?  Maybe later, but I feel
> it's too early to change the core side unless you really need it.
> Note that you may have multiple headphone, mic, line-in and whatever
> jacks.  So, if any, it should be either an array or a list as a more
> generic representation.
> 
> And, if you add struct snd_jack there, include <sound/jack.h> in that
> header, or add a forward declaration of the struct.
> 
> 
>> diff --git a/pci/hda/hda_proc.c b/pci/hda/hda_proc.c
>> index 743d779..e07652a 100644
>> --- a/pci/hda/hda_proc.c
>> +++ b/pci/hda/hda_proc.c
>> @@ -425,6 +425,22 @@ static void print_unsol_cap(struct snd_info_buffer *buffer,
>>  		    (unsol & AC_UNSOL_ENABLED) ? 1 : 0);
>>  }
>>
>> +static void print_presence_cap(struct snd_info_buffer *buffer,
>> +			      struct hda_codec *codec, hda_nid_t nid)
>> +{
>> +	int presence = snd_hda_codec_read(codec, nid, 0,
>> +			AC_VERB_GET_PIN_SENSE, 0);
>> +	int def_conf = snd_hda_codec_read(codec, nid, 0,
>> +			AC_VERB_GET_CONFIG_DEFAULT, 0x00);
>> +	def_conf = get_defcfg_connect(def_conf);
>> +	if (def_conf && def_conf != AC_JACK_PORT_FIXED)
>> +		return;
>> +	snd_iprintf(buffer,
>> +			"  Jack Presence: %s\n",
>> +			(presence & AC_PINSENSE_PRESENCE)
>> +				? "Plugged in" : "Unplugged");
>> +}
>> +
>>  static void print_proc_caps(struct snd_info_buffer *buffer,
>>  			    struct hda_codec *codec, hda_nid_t nid)
>>  {
>> @@ -552,6 +568,9 @@ static void print_codec_info(struct snd_info_entry *entry,
>>  					   AC_PAR_AUDIO_WIDGET_CAP);
>>  		unsigned int wid_type =
>>  			(wid_caps & AC_WCAP_TYPE) >> AC_WCAP_TYPE_SHIFT;
>> +		unsigned int pincap = snd_hda_param_read(codec, nid,
>> +					 AC_PAR_PIN_CAP);
>> +
>>  		hda_nid_t conn[HDA_MAX_CONNECTIONS];
>>  		int conn_len = 0;
>>
>> @@ -632,6 +651,9 @@ static void print_codec_info(struct snd_info_entry *entry,
>>  		if (wid_caps & AC_WCAP_UNSOL_CAP)
>>  			print_unsol_cap(buffer, codec, nid);
>>
>> +		if (pincap & AC_PINCAP_PRES_DETECT)
>> +			print_presence_cap(buffer, codec, nid);
>> +
>>  		if (wid_caps & AC_WCAP_POWER)
>>  			print_power_state(buffer, codec, nid);
> 
> Sorry, I don't want to put this into proc file.  In some obscure
> hardwares, you can't call jack-detect verb always even if the pin
> shows it's possible, and it breaks the later jack-sense reading
> (believe nor not :)  In some cases, it needs triggering.
> So, "just read" is dangerous. 
So what is the safest way to report the initial state of the jacks then?

> 
> BTW, this change has nothing to do with the addition of jack layer
> support.  If this addition is inevitably necessary, please split to an
> individual patch.
> 
> 
> Thanks,
> 
> Takashi
>