Re: [PATCH] ALSA: SoC: Added jack detection kcontrol support

Re: [PATCH] ALSA: SoC: Added jack detection kcontrol support

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 889 bytes --]

On Fri, Jul 19, 2013 at 05:59:22PM -0700, Felipe F. Tonello wrote:
> From: "Felipe F. Tonello" <ftonello@cercacor.com>
> 
> It adds soc jack support for alsa kcontrol.
> 
> This support is necessary since the new kcontrol is used by user-space
> daemons, such as PulseAudio(>=2.0), to do jack detection.
> 
> Signed-off-by: Felipe F. Tonello <ftonello@cercacor.com>
> ---
>  include/sound/soc.h  |  1 +
>  sound/soc/Kconfig    |  1 +
>  sound/soc/soc-jack.c | 22 ++++++++++++++++++++++
>  3 files changed, 24 insertions(+)

No, this makes no sense at all as an ASoC specific feature - it should
be a standard feature of ALSA jacks.  The whole point of having ALSA
core infrastructure for jacks is to avoid having to go round doing make
work stuff like this.

The fact that the kcontrols were introduced as a HDA-specific feature at
all is of course a problem here...

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH] ALSA: SoC: Added jack detection kcontrol support

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 889 bytes --]

On Fri, Jul 19, 2013 at 05:59:22PM -0700, Felipe F. Tonello wrote:
> From: "Felipe F. Tonello" <ftonello@cercacor.com>
> 
> It adds soc jack support for alsa kcontrol.
> 
> This support is necessary since the new kcontrol is used by user-space
> daemons, such as PulseAudio(>=2.0), to do jack detection.
> 
> Signed-off-by: Felipe F. Tonello <ftonello@cercacor.com>
> ---
>  include/sound/soc.h  |  1 +
>  sound/soc/Kconfig    |  1 +
>  sound/soc/soc-jack.c | 22 ++++++++++++++++++++++
>  3 files changed, 24 insertions(+)

No, this makes no sense at all as an ASoC specific feature - it should
be a standard feature of ALSA jacks.  The whole point of having ALSA
core infrastructure for jacks is to avoid having to go round doing make
work stuff like this.

The fact that the kcontrols were introduced as a HDA-specific feature at
all is of course a problem here...

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] ALSA: SoC: Added jack detection kcontrol support

[Felipe Tonello]

Hi Mark,

On Sun, Jul 21, 2013 at 5:01 PM, Mark Brown <broonie@kernel.org> wrote:
> On Fri, Jul 19, 2013 at 05:59:22PM -0700, Felipe F. Tonello wrote:
>> From: "Felipe F. Tonello" <ftonello@cercacor.com>
>>
>> It adds soc jack support for alsa kcontrol.
>>
>> This support is necessary since the new kcontrol is used by user-space
>> daemons, such as PulseAudio(>=2.0), to do jack detection.
>>
>> Signed-off-by: Felipe F. Tonello <ftonello@cercacor.com>
>> ---
>>  include/sound/soc.h  |  1 +
>>  sound/soc/Kconfig    |  1 +
>>  sound/soc/soc-jack.c | 22 ++++++++++++++++++++++
>>  3 files changed, 24 insertions(+)
>
> No, this makes no sense at all as an ASoC specific feature - it should
> be a standard feature of ALSA jacks.  The whole point of having ALSA
> core infrastructure for jacks is to avoid having to go round doing make
> work stuff like this.

I agree. I added as a ASoC feature because I followed the example of
HDA implementation.

My question is: Why ALSA Jack support uses evdev input events to
notify user-space? Is there any user-space relying on that? If no, I
would like to change ALSA Jack implementation to only use KControl and
remove specific implementations.

At least the only daemon that I know that supports jack detection,
which is PulseAudio, uses this new ALSA KControl for Jacks.

Regards,
Felipe Tonello

Re: [PATCH] ALSA: SoC: Added jack detection kcontrol support

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 594 bytes --]

On Mon, Jul 22, 2013 at 02:11:44PM -0700, Felipe Tonello wrote:

> My question is: Why ALSA Jack support uses evdev input events to
> notify user-space? Is there any user-space relying on that? If no, I
> would like to change ALSA Jack implementation to only use KControl and
> remove specific implementations.

This was the existing kernel API at the time that the code was added -
I beleive people had been doing that for simple switches that detect the
physical presence of a jack since it looks exactly like a button from a
hardware point of view.  See the original commit adding the code.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH] ALSA: SoC: Added jack detection kcontrol support

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 594 bytes --]

On Mon, Jul 22, 2013 at 02:11:44PM -0700, Felipe Tonello wrote:

> My question is: Why ALSA Jack support uses evdev input events to
> notify user-space? Is there any user-space relying on that? If no, I
> would like to change ALSA Jack implementation to only use KControl and
> remove specific implementations.

This was the existing kernel API at the time that the code was added -
I beleive people had been doing that for simple switches that detect the
physical presence of a jack since it looks exactly like a button from a
hardware point of view.  See the original commit adding the code.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] ALSA: SoC: Added jack detection kcontrol support

[Felipe Tonello]

Hi Mark,

On Tue, Jul 23, 2013 at 5:40 AM, Mark Brown <broonie@kernel.org> wrote:
> On Mon, Jul 22, 2013 at 02:11:44PM -0700, Felipe Tonello wrote:
>
>> My question is: Why ALSA Jack support uses evdev input events to
>> notify user-space? Is there any user-space relying on that? If no, I
>> would like to change ALSA Jack implementation to only use KControl and
>> remove specific implementations.
>
> This was the existing kernel API at the time that the code was added -
> I beleive people had been doing that for simple switches that detect the
> physical presence of a jack since it looks exactly like a button from a
> hardware point of view.  See the original commit adding the code.

I saw it. I don't know what you think is the best way to go. I'm not
sure if someone is using this input event API, since even PulseAudio
even declined that implementation due KControl implementation.

I'm thinking in re-implement core/jack.c using only kcontrol for
reporting status and leaving input events only for keys (which is
already implemented). This will break compatibility if any user-space
checks for jack status using input events, but again, is anyone using
this?

Regards,

Felipe Tonello

Re: [PATCH] ALSA: SoC: Added jack detection kcontrol support

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 1287 bytes --]

On Tue, Jul 23, 2013 at 05:10:29PM -0700, Felipe Tonello wrote:
> On Tue, Jul 23, 2013 at 5:40 AM, Mark Brown <broonie@kernel.org> wrote:

> > This was the existing kernel API at the time that the code was added -
> > I beleive people had been doing that for simple switches that detect the
> > physical presence of a jack since it looks exactly like a button from a
> > hardware point of view.  See the original commit adding the code.

> I saw it. I don't know what you think is the best way to go. I'm not
> sure if someone is using this input event API, since even PulseAudio
> even declined that implementation due KControl implementation.

It's used quite a bit in embedded systems, there are actually patches
for PulseAudio which were deployed for a while - I guess they might have
been removed or something.  The kcontrol stuff was actually implemented
after that was already in place.

> I'm thinking in re-implement core/jack.c using only kcontrol for
> reporting status and leaving input events only for keys (which is
> already implemented). This will break compatibility if any user-space
> checks for jack status using input events, but again, is anyone using
> this?

The jack core should be implementing all three methods of reporting
jacks (input, controls and extcon).

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH] ALSA: SoC: Added jack detection kcontrol support

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1287 bytes --]

On Tue, Jul 23, 2013 at 05:10:29PM -0700, Felipe Tonello wrote:
> On Tue, Jul 23, 2013 at 5:40 AM, Mark Brown <broonie@kernel.org> wrote:

> > This was the existing kernel API at the time that the code was added -
> > I beleive people had been doing that for simple switches that detect the
> > physical presence of a jack since it looks exactly like a button from a
> > hardware point of view.  See the original commit adding the code.

> I saw it. I don't know what you think is the best way to go. I'm not
> sure if someone is using this input event API, since even PulseAudio
> even declined that implementation due KControl implementation.

It's used quite a bit in embedded systems, there are actually patches
for PulseAudio which were deployed for a while - I guess they might have
been removed or something.  The kcontrol stuff was actually implemented
after that was already in place.

> I'm thinking in re-implement core/jack.c using only kcontrol for
> reporting status and leaving input events only for keys (which is
> already implemented). This will break compatibility if any user-space
> checks for jack status using input events, but again, is anyone using
> this?

The jack core should be implementing all three methods of reporting
jacks (input, controls and extcon).

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] ALSA: SoC: Added jack detection kcontrol support

[David Henningsson]

On 07/24/2013 02:13 AM, Mark Brown wrote:
> On Tue, Jul 23, 2013 at 05:10:29PM -0700, Felipe Tonello wrote:
>> On Tue, Jul 23, 2013 at 5:40 AM, Mark Brown <broonie@kernel.org> wrote:
>
>>> This was the existing kernel API at the time that the code was added -
>>> I beleive people had been doing that for simple switches that detect the
>>> physical presence of a jack since it looks exactly like a button from a
>>> hardware point of view.  See the original commit adding the code.
>
>> I saw it. I don't know what you think is the best way to go. I'm not
>> sure if someone is using this input event API, since even PulseAudio
>> even declined that implementation due KControl implementation.
>
> It's used quite a bit in embedded systems, there are actually patches
> for PulseAudio which were deployed for a while- I guess they might have
> been removed or something.

The input event implementation was only in Ubuntu (for a release or 
two), and was never merged in upstream PA. (I guess one could find them 
by looking in the Ubuntu archives.)

-- 
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic

Re: [alsa-devel] [PATCH] ALSA: SoC: Added jack detection kcontrol support

[David Henningsson]

On 07/24/2013 02:13 AM, Mark Brown wrote:
> On Tue, Jul 23, 2013 at 05:10:29PM -0700, Felipe Tonello wrote:
>> On Tue, Jul 23, 2013 at 5:40 AM, Mark Brown <broonie@kernel.org> wrote:
>
>>> This was the existing kernel API at the time that the code was added -
>>> I beleive people had been doing that for simple switches that detect the
>>> physical presence of a jack since it looks exactly like a button from a
>>> hardware point of view.  See the original commit adding the code.
>
>> I saw it. I don't know what you think is the best way to go. I'm not
>> sure if someone is using this input event API, since even PulseAudio
>> even declined that implementation due KControl implementation.
>
> It's used quite a bit in embedded systems, there are actually patches
> for PulseAudio which were deployed for a while- I guess they might have
> been removed or something.

The input event implementation was only in Ubuntu (for a release or 
two), and was never merged in upstream PA. (I guess one could find them 
by looking in the Ubuntu archives.)

-- 
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic