[PATCH 1/4] alsa: Add extcon (Android switch) jack detection

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

 



On Wed, 2013-09-18 at 16:01 +0200, David Henningsson wrote:
> This is for headphone/headset only, so far. It currently works for
> Android switches, but should be easy to convert to extcon when there
> is a need, hence the name.
> 
> Meanwhile, if you're the 99% that does not run an Android kernel with
> this switch, this patch should be harmless - it would just notice there
> is no switch and don't do anything else.
> ---
>  src/Makefile.am                     |    1 +
>  src/modules/alsa/alsa-extcon.c      |  272 +++++++++++++++++++++++++++++++++++
>  src/modules/alsa/alsa-extcon.h      |   33 +++++
>  src/modules/alsa/alsa-ucm.c         |    4 +-
>  src/modules/alsa/alsa-ucm.h         |    1 +
>  src/modules/alsa/module-alsa-card.c |    5 +
>  6 files changed, 314 insertions(+), 2 deletions(-)
>  create mode 100644 src/modules/alsa/alsa-extcon.c
>  create mode 100644 src/modules/alsa/alsa-extcon.h
> 
> diff --git a/src/Makefile.am b/src/Makefile.am
> index 8392953..7caa1db 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -1729,6 +1729,7 @@ module_coreaudio_device_la_LIBADD = $(MODULE_LIBADD)
>  libalsa_util_la_SOURCES = \
>  		modules/alsa/alsa-util.c modules/alsa/alsa-util.h \
>  		modules/alsa/alsa-ucm.c modules/alsa/alsa-ucm.h \
> +		modules/alsa/alsa-extcon.c modules/alsa/alsa-extcon.h \

Apparently the code depends on UCM and udev. I think these files should
be added to SOURCES only if both UCM and udev are available. The two
calls in alsa-card.c need to be then protected with #ifdefs too.

Another alternative is to do the ifdeffery in alsa-extcon.h and
alsa-extcon.c only, similar to how alsa-ucm.h and .c do it. I'd be fine
with that too.

>  		modules/alsa/alsa-mixer.c modules/alsa/alsa-mixer.h \
>  		modules/alsa/alsa-sink.c modules/alsa/alsa-sink.h \
>  		modules/alsa/alsa-source.c modules/alsa/alsa-source.h \
> diff --git a/src/modules/alsa/alsa-extcon.c b/src/modules/alsa/alsa-extcon.c
> new file mode 100644
> index 0000000..fb21fdb
> --- /dev/null
> +++ b/src/modules/alsa/alsa-extcon.c
> @@ -0,0 +1,272 @@
> +/***
> +  This file is part of PulseAudio.
> +
> +  Copyright 2013 David Henningsson, Canonical Ltd.
> +
> +  PulseAudio is free software; you can redistribute it and/or modify
> +  it under the terms of the GNU Lesser General Public License as published
> +  by the Free Software Foundation; either version 2.1 of the License,
> +  or (at your option) any later version.
> +
> +  PulseAudio is distributed in the hope that it will be useful, but
> +  WITHOUT ANY WARRANTY; without even the implied warranty of
> +  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> +  General Public License for more details.
> +
> +  You should have received a copy of the GNU Lesser General Public License
> +  along with PulseAudio; if not, write to the Free Software
> +  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307
> +  USA.
> +***/
> +
> +#ifdef HAVE_CONFIG_H
> +#include <config.h>
> +#endif
> +
> +#include <pulsecore/core-util.h>
> +#include <pulsecore/device-port.h>
> +#include <pulsecore/i18n.h>
> +#include <libudev.h>
> +
> +#include "alsa-util.h"
> +#include "alsa-extcon.h"
> +
> +/* IFDEF HAVE_UCM ? */
> +#include <use-case.h>
> +#include "alsa-ucm.h"
> +/* ENDIF */
> +
> +/* For android */
> +#define EXTCON_NAME "switch"
> +/* For extcon */
> +/* #define EXTCON_NAME "extcon" */
> +
> +static pa_available_t hp_avail(int state)
> +{
> +    return ((state & 3) != 0) ? PA_AVAILABLE_YES : PA_AVAILABLE_NO;

Some commenting would be nice: what do the first and second bits mean
that you check here?

> +}
> +
> +static pa_available_t hsmic_avail(int state)
> +{
> +    return (state & 1) ? PA_AVAILABLE_YES : PA_AVAILABLE_NO;
> +}
> +
> +struct android_switch {
> +    char *name;
> +    uint32_t current_value;

I suggest storing the decoded pa_available_t value here instead of the
raw integer.

> +};
> +
> +static void android_switch_free(struct android_switch *as) {
> +    if (!as)
> +        return;

Style: don't accept NULL in free().

> +    pa_xfree(as->name);
> +    pa_xfree(as);
> +}
> +
> +static struct android_switch *android_switch_new(const char *name) {
> +
> +    struct android_switch *as = NULL;
> +    char *filename = pa_sprintf_malloc("/sys/class/%s/%s/state", EXTCON_NAME, name);
> +    char *state = pa_read_line_from_file(filename);
> +
> +    if (state == NULL) {
> +        pa_log_debug("Cannot open '%s'. Skipping.", filename);
> +        pa_xfree(filename);
> +        return NULL;
> +    }
> +    pa_xfree(filename);
> +
> +    as = pa_xnew0(struct android_switch, 1);
> +    as->name = pa_xstrdup(name);
> +
> +    if (pa_atou(state, &as->current_value) < 0) {
> +        pa_log_warn("Switch '%s' has invalid value '%s'", name, state);
> +        pa_xfree(state);
> +        android_switch_free(as);
> +        return NULL;
> +    }
> +
> +    return as;
> +}
> +
> +struct udev_data {
> +    struct udev *udev;
> +    struct udev_monitor *monitor;
> +    pa_io_event *event;
> +};

Minor bike-shedding, feel free to ignore:

The type "struct udev_data" is only used as a member of pa_alsa_extcon,
so having a stand-alone type is unnecessary. I think it one of these
would be neater:

struct pa_alsa_extcon {
    pa_card *card;
    struct android_switch *h2w;
    struct {
        struct udev *udev;
        struct udev_monitor *monitor;
        pa_io_event *event;
    } udev;
};

or

struct pa_alsa_extcon {
    pa_card *card;
    struct android_switch *h2w;
    struct udev *udev;
    struct udev_monitor *udev_monitor;
    pa_io_event *udev_event;
};

> +
> +struct pa_alsa_extcon {
> +    pa_card *card;
> +    struct android_switch *h2w;
> +    struct udev_data udev;
> +};
> +
> +static struct android_switch *find_matching_switch(pa_alsa_extcon *u,
> +                                                   const char *devpath) {
> +
> +    if (pa_streq(devpath, "/devices/virtual/" EXTCON_NAME "/h2w"))
> +        return u->h2w;  /* To be extended if we ever support more switches */
> +    return NULL;
> +}
> +
> +static void notify_ports(pa_alsa_extcon *u, struct android_switch *as) {
> +
> +    pa_device_port *p;
> +    void *state;
> +
> +    pa_assert(as == u->h2w); /* To be extended if we ever support more switches */
> +
> +    pa_log_debug("Value of switch %s is now %d.", as->name, as->current_value);
> +
> +    PA_HASHMAP_FOREACH(p, u->card->ports, state) {
> +        if (p->direction == PA_DIRECTION_OUTPUT) {
> +            if (!strcmp(p->name, "analog-output-headphones"))

Use pa_streq().

> +                 pa_device_port_set_available(p, hp_avail(as->current_value));
> +/* IFDEF HAVE_UCM ? */
> +            else if (pa_alsa_ucm_port_contains(p->name, SND_USE_CASE_DEV_HEADSET, true) ||
> +                     pa_alsa_ucm_port_contains(p->name, SND_USE_CASE_DEV_HEADPHONES, true))

pa_alsa_ucm_port_contains() assumes that the port name follows the
format that alsa-ucm.c uses when generating port names. I think it would
be good to check if the card is actually using UCM before calling
pa_alsa_ucm_port_contains(). This is not a big issue in practice (the
only "problem" is accidental match in a non-UCM port name), but still
I'd prefer not feeding "garbage" to pa_alsa_ucm_port_contains().

> +                pa_device_port_set_available(p, hp_avail(as->current_value));
> +/* ENDIF */
> +        }
> +        else if (p->direction == PA_DIRECTION_INPUT) {
> +            if (!strcmp(p->name, "analog-input-headset-mic"))

Use pa_streq().

> +                pa_device_port_set_available(p, hsmic_avail(as->current_value));
> +/* IFDEF HAVE_UCM ? */
> +            else if (pa_alsa_ucm_port_contains(p->name, SND_USE_CASE_DEV_HEADSET, false))
> +                pa_device_port_set_available(p, hsmic_avail(as->current_value));
> +/* ENDIF */
> +        }
> +    }
> +}
> +
> +static void udev_cb(pa_mainloop_api *a, pa_io_event *e, int fd,
> +                    pa_io_event_flags_t events, void *userdata) {
> +
> +    pa_alsa_extcon *u = userdata;
> +    struct udev_device *d = udev_monitor_receive_device(u->udev.monitor);
> +    struct udev_list_entry *entry;
> +    struct android_switch *as;
> +    const char *devpath, *state;
> +
> +    if (!d) {
> +        pa_log("udev_monitor_receive_device failed.");
> +        pa_assert(a);
> +        a->io_free(u->udev.event);
> +        u->udev.event = NULL;
> +        return;
> +    }
> +
> +    devpath = udev_device_get_devpath(d);
> +    if (!devpath) {
> +        pa_log("udev_device_get_devpath failed.");
> +        goto out;
> +    }
> +    pa_log_debug("Got uevent with devpath=%s", devpath);
> +
> +    as = find_matching_switch(u, devpath);
> +    if (!as)
> +        goto out;
> +
> +    entry = udev_list_entry_get_by_name(
> +            udev_device_get_properties_list_entry(d), "SWITCH_STATE");
> +    if (!entry) {
> +        pa_log("udev_list_entry_get_by_name failed to find 'SWITCH_STATE' entry.");
> +        goto out;
> +    }
> +
> +    state = udev_list_entry_get_value(entry);
> +    if (!state) {
> +        pa_log("udev_list_entry_get_by_name failed.");

Copy-paste error in log message.

> +        goto out;
> +    }
> +
> +    if (pa_atou(state, &as->current_value) < 0) {
> +        pa_log_warn("Switch '%s' has invalid value '%s'", as->name, state);
> +        goto out;
> +    }
> +
> +    notify_ports(u, as);
> +
> +out:
> +    udev_device_unref(d);
> +}
> +
> +static bool init_udev(pa_alsa_extcon *u, pa_core *core) {

Use int for success/failure reporting.

> +
> +    int fd;
> +
> +    u->udev.udev = udev_new();
> +    if (!u->udev.udev) {
> +        pa_log("udev_new failed.");
> +        return false;
> +    }
> +
> +    u->udev.monitor = udev_monitor_new_from_netlink(u->udev.udev, "udev");
> +    if (!u->udev.monitor) {
> +        pa_log("udev_monitor_new_from_netlink failed.");
> +        return false;
> +    }
> +/* FIXME: Research why this filter did not work */

Do you plan to do the research?

-- 
Tanu



[Index of Archives]     [Linux Audio Users]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux