[PATCH0/2] jack: Add support for SND_JACK_LINEOUT

[PATCH0/2] jack: Add support for SND_JACK_LINEOUT

[Matthew Ranostay]

Add support to the jack abstraction layer to report 'Line Out' presence insertions.

Signed-off-by: Matthew Ranostay <mranostay@embeddedalley.com>
---

diff --git a/include/linux/input.h b/include/linux/input.h
index a5802c9..7323d2f 100644
--- a/include/linux/input.h
+++ b/include/linux/input.h
@@ -644,6 +644,7 @@ struct input_absinfo {
 #define SW_RADIO		SW_RFKILL_ALL	/* deprecated */
 #define SW_MICROPHONE_INSERT	0x04  /* set = inserted */
 #define SW_DOCK			0x05  /* set = plugged into dock */
+#define SW_LINEOUT_INSERT	0x06  /* set = inserted */
 #define SW_MAX			0x0f
 #define SW_CNT			(SW_MAX+1)

diff --git a/include/sound/jack.h b/include/sound/jack.h
index b1b2b8b..67270ee 100644
--- a/include/sound/jack.h
+++ b/include/sound/jack.h
@@ -35,6 +35,8 @@ enum snd_jack_types {
 	SND_JACK_HEADPHONE	= 0x0001,
 	SND_JACK_MICROPHONE	= 0x0002,
 	SND_JACK_HEADSET	= SND_JACK_HEADPHONE | SND_JACK_MICROPHONE,
+	SND_JACK_LINEOUT	= 0x0004,
+	SND_JACK_SWITCH		= SND_JACK_HEADPHONE | SND_JACK_LINEOUT,
 };

 struct snd_jack {
diff --git a/sound/core/jack.c b/sound/core/jack.c
index bd2d9e6..284432f 100644
--- a/sound/core/jack.c
+++ b/sound/core/jack.c
@@ -34,6 +34,7 @@ static int snd_jack_dev_free(struct snd_device *device)
 	else
 		input_free_device(jack->input_dev);

+	kfree(jack->id);
 	kfree(jack);

 	return 0;
@@ -87,7 +88,7 @@ int snd_jack_new(struct snd_card *card, const char *id, int type,
 	if (jack == NULL)
 		return -ENOMEM;

-	jack->id = id;
+	jack->id = kstrdup(id, GFP_KERNEL);

 	jack->input_dev = input_allocate_device();
 	if (jack->input_dev == NULL) {
@@ -102,6 +103,9 @@ int snd_jack_new(struct snd_card *card, const char *id, int
type,
 	if (type & SND_JACK_HEADPHONE)
 		input_set_capability(jack->input_dev, EV_SW,
 				     SW_HEADPHONE_INSERT);
+	if (type & SND_JACK_LINEOUT)
+		input_set_capability(jack->input_dev, EV_SW,
+				     SW_LINEOUT_INSERT);
 	if (type & SND_JACK_MICROPHONE)
 		input_set_capability(jack->input_dev, EV_SW,
 				     SW_MICROPHONE_INSERT);
@@ -153,6 +157,9 @@ void snd_jack_report(struct snd_jack *jack, int status)
 	if (jack->type & SND_JACK_HEADPHONE)
 		input_report_switch(jack->input_dev, SW_HEADPHONE_INSERT,
 				    status & SND_JACK_HEADPHONE);
+	if (jack->type & SND_JACK_LINEOUT)
+		input_report_switch(jack->input_dev, SW_LINEOUT_INSERT,
+				    status & SND_JACK_LINEOUT);
 	if (jack->type & SND_JACK_MICROPHONE)
 		input_report_switch(jack->input_dev, SW_MICROPHONE_INSERT,
 				    status & SND_JACK_MICROPHONE);

Re: [PATCH0/2] jack: Add support for SND_JACK_LINEOUT

[Takashi Iwai]

At Mon, 20 Oct 2008 17:39:52 -0400,
Matthew Ranostay wrote:
> 
> Add support to the jack abstraction layer to report 'Line Out' presence insertions.
> 
> Signed-off-by: Matthew Ranostay <mranostay@embeddedalley.com>
> ---
> 
> diff --git a/include/linux/input.h b/include/linux/input.h
> index a5802c9..7323d2f 100644
> --- a/include/linux/input.h
> +++ b/include/linux/input.h
> @@ -644,6 +644,7 @@ struct input_absinfo {
>  #define SW_RADIO		SW_RFKILL_ALL	/* deprecated */
>  #define SW_MICROPHONE_INSERT	0x04  /* set = inserted */
>  #define SW_DOCK			0x05  /* set = plugged into dock */
> +#define SW_LINEOUT_INSERT	0x06  /* set = inserted */
>  #define SW_MAX			0x0f
>  #define SW_CNT			(SW_MAX+1)
> 
> diff --git a/include/sound/jack.h b/include/sound/jack.h
> index b1b2b8b..67270ee 100644
> --- a/include/sound/jack.h
> +++ b/include/sound/jack.h
> @@ -35,6 +35,8 @@ enum snd_jack_types {
>  	SND_JACK_HEADPHONE	= 0x0001,
>  	SND_JACK_MICROPHONE	= 0x0002,
>  	SND_JACK_HEADSET	= SND_JACK_HEADPHONE | SND_JACK_MICROPHONE,
> +	SND_JACK_LINEOUT	= 0x0004,
> +	SND_JACK_SWITCH		= SND_JACK_HEADPHONE | SND_JACK_LINEOUT,

Please describe what these two items are supposed to be used.
Especially, the use case of SND_JACK_SWITCH is unclear.


>  };
> 
>  struct snd_jack {
> diff --git a/sound/core/jack.c b/sound/core/jack.c
> index bd2d9e6..284432f 100644
> --- a/sound/core/jack.c
> +++ b/sound/core/jack.c
> @@ -34,6 +34,7 @@ static int snd_jack_dev_free(struct snd_device *device)
>  	else
>  		input_free_device(jack->input_dev);
> 
> +	kfree(jack->id);
>  	kfree(jack);
> 
>  	return 0;
> @@ -87,7 +88,7 @@ int snd_jack_new(struct snd_card *card, const char *id, int type,
>  	if (jack == NULL)
>  		return -ENOMEM;
> 
> -	jack->id = id;
> +	jack->id = kstrdup(id, GFP_KERNEL);
> 
>  	jack->input_dev = input_allocate_device();
>  	if (jack->input_dev == NULL) {

These two chunks are basically independent from the other change.
Please split to another patch with a proper changelog.


Takashi

Re: [PATCH0/2] jack: Add support for SND_JACK_LINEOUT

[Takashi Iwai]

BTW, [PATCH 0/x] is used usually for a description of the following
patch series, and contains no patch by itself.  The real patches are
posted in [PATCH 1/x] until [PATCH x/x].


HTH,

Takashi

Re: [PATCH0/2] jack: Add support for SND_JACK_LINEOUT

[Mark Brown]

On Mon, Oct 20, 2008 at 05:39:52PM -0400, Matthew Ranostay wrote:
> Add support to the jack abstraction layer to report 'Line Out' presence insertions.

The line output parts of this look good.  However, there look to be some
other things in here as well...

Please word-wrap your changelogs, also - they should have lines no
longer than 80 columns.

> --- a/include/sound/jack.h
> +++ b/include/sound/jack.h
> @@ -35,6 +35,8 @@ enum snd_jack_types {
>  	SND_JACK_HEADPHONE	= 0x0001,
>  	SND_JACK_MICROPHONE	= 0x0002,
>  	SND_JACK_HEADSET	= SND_JACK_HEADPHONE | SND_JACK_MICROPHONE,
> +	SND_JACK_LINEOUT	= 0x0004,
> +	SND_JACK_SWITCH		= SND_JACK_HEADPHONE | SND_JACK_LINEOUT,
>  };

Like Takashi said, SND_JACK_LINEOUT I understand but SND_JACK_SWITCH is
rather abstruse.  If it's just that the jack can be either a headphone
or a microphone then there's no need for it - HEADSET is only provided
to make it more obvious how to implement one since people often don't
think of a headset as being its components.

> index bd2d9e6..284432f 100644
> --- a/sound/core/jack.c
> +++ b/sound/core/jack.c
> @@ -34,6 +34,7 @@ static int snd_jack_dev_free(struct snd_device *device)
>  	else
>  		input_free_device(jack->input_dev);
> 
> +	kfree(jack->id);
>  	kfree(jack);
> 
>  	return 0;
> @@ -87,7 +88,7 @@ int snd_jack_new(struct snd_card *card, const char *id, int type,
>  	if (jack == NULL)
>  		return -ENOMEM;
> 
> -	jack->id = id;
> +	jack->id = kstrdup(id, GFP_KERNEL);

These two changes look entirely unrelated to adding line output support
and aren't mentioned in the changelog.  Please split them into a
separate patch with a changelog entry.

Re: [PATCH0/2] jack: Add support for SND_JACK_LINEOUT

[Matthew Ranostay]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Mark Brown wrote:
> On Mon, Oct 20, 2008 at 05:39:52PM -0400, Matthew Ranostay wrote:
>> Add support to the jack abstraction layer to report 'Line Out' presence insertions.
> 
> The line output parts of this look good.  However, there look to be some
> other things in here as well...
> 
> Please word-wrap your changelogs, also - they should have lines no
> longer than 80 columns.
> 
>> --- a/include/sound/jack.h
>> +++ b/include/sound/jack.h
>> @@ -35,6 +35,8 @@ enum snd_jack_types {
>>  	SND_JACK_HEADPHONE	= 0x0001,
>>  	SND_JACK_MICROPHONE	= 0x0002,
>>  	SND_JACK_HEADSET	= SND_JACK_HEADPHONE | SND_JACK_MICROPHONE,
>> +	SND_JACK_LINEOUT	= 0x0004,
>> +	SND_JACK_SWITCH		= SND_JACK_HEADPHONE | SND_JACK_LINEOUT,
>>  };
> 
> Like Takashi said, SND_JACK_LINEOUT I understand but SND_JACK_SWITCH is
> rather abstruse.  If it's just that the jack can be either a headphone
> or a microphone then there's no need for it - HEADSET is only provided
> to make it more obvious how to implement one since people often don't
> think of a headset as being its components.
> 
Actaully it's for the jack that has a mixer switch, in which the headphone
support turned off and becoming a line out. I can understand not adding this,
since only one codec patchset will probably use it.


>> index bd2d9e6..284432f 100644
>> --- a/sound/core/jack.c
>> +++ b/sound/core/jack.c
>> @@ -34,6 +34,7 @@ static int snd_jack_dev_free(struct snd_device *device)
>>  	else
>>  		input_free_device(jack->input_dev);
>>
>> +	kfree(jack->id);
>>  	kfree(jack);
>>
>>  	return 0;
>> @@ -87,7 +88,7 @@ int snd_jack_new(struct snd_card *card, const char *id, int type,
>>  	if (jack == NULL)
>>  		return -ENOMEM;
>>
>> -	jack->id = id;
>> +	jack->id = kstrdup(id, GFP_KERNEL);
> 
> These two changes look entirely unrelated to adding line output support
> and aren't mentioned in the changelog.  Please split them into a
> separate patch with a changelog entry.
> 

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Darwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkj98xgACgkQ7s2wy7nhBHUqOwCdFs0U7v6/bPvsmiZ/2qiysz3G
E3QAnjdLoByV3M3s4m8JpjKRJFgfVpXz
=hB+0
-----END PGP SIGNATURE-----

Re: [PATCH0/2] jack: Add support for SND_JACK_LINEOUT

[Mark Brown]

On Tue, Oct 21, 2008 at 11:19:52AM -0400, Matthew Ranostay wrote:
> Mark Brown wrote:

> > or a microphone then there's no need for it - HEADSET is only provided
> > to make it more obvious how to implement one since people often don't
> > think of a headset as being its components.

> Actaully it's for the jack that has a mixer switch, in which the headphone
> support turned off and becoming a line out. I can understand not adding this,
> since only one codec patchset will probably use it.

What do you mean by a "mixer switch"?  Do you just mean a software
control that can select between line and headphone signals?  Is the
hardware able to determine which kind of device is connected to the
jack?

Re: [PATCH0/2] jack: Add support for SND_JACK_LINEOUT

[Matthew Ranostay]

Mark Brown wrote:
> On Tue, Oct 21, 2008 at 11:19:52AM -0400, Matthew Ranostay wrote:
>> Mark Brown wrote:
> 
>>> or a microphone then there's no need for it - HEADSET is only provided
>>> to make it more obvious how to implement one since people often don't
>>> think of a headset as being its components.
> 
>> Actaully it's for the jack that has a mixer switch, in which the headphone
>> support turned off and becoming a line out. I can understand not adding this,
>> since only one codec patchset will probably use it.
> 
> What do you mean by a "mixer switch"?  Do you just mean a software
> control that can select between line and headphone signals?
Yeah, when it's a line out it doesn't mute the other lineouts.
Is the
> hardware able to determine which kind of device is connected to the
> jack?
> 

Yes with the AC_VERB_GET_PIN_WIDGET_CONTROL verb being read when reporting the
jack status.


-Matt

Re: [PATCH0/2] jack: Add support for SND_JACK_LINEOUT

[Mark Brown]

On Tue, Oct 21, 2008 at 03:42:19PM -0400, Matthew Ranostay wrote:
> Mark Brown wrote:

> > What do you mean by a "mixer switch"?  Do you just mean a software
> > control that can select between line and headphone signals?

> Yeah, when it's a line out it doesn't mute the other lineouts.

> > Is the
> > hardware able to determine which kind of device is connected to the
> > jack?

> Yes with the AC_VERB_GET_PIN_WIDGET_CONTROL verb being read when reporting the
> jack status.

OK, that's exactly the case I was describing before: it's a jack with
independant line and headphone functions that is able to distinguish
between the two.  The switching isn't particularly relevant to the
function of the jack here, it's a natural feature of the jack API - if
the hardware could also determine if a microphone were connected to the
jack and be configured appropriately then it could also report that.