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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[Index of Archives]     [ALSA User]     [Linux Audio Users]     [Kernel Archive]     [Asterisk PBX]     [Photo Sharing]     [Linux Sound]     [Video 4 Linux]     [Gimp]     [Yosemite News]

  Powered by Linux