At Sat, 18 Apr 2015 18:04:15 +0800, Jie Yang wrote:
Currently the ALSA jack core registers only input devices for each jack registered. These jack input devices are not readable by userspace devices that run as non root.
This patch adds support for additionally registering jack kcontrol devices for every input jack registered. This allows non root userspace to read jack status.
Please give more description what you really changed. It's not only doing the additional kctl creation. Also, a brief introduction about what will follow in the patchset would be nice for readers.
Signed-off-by: Liam Girdwood liam.r.girdwood@linux.intel.com Modified-by: Jie Yang yang.jie@intel.com Signed-off-by: Jie Yang yang.jie@intel.com Reveiwed-by: Mark Brown broonie@kernel.org
include/sound/jack.h | 17 +++++- sound/core/Kconfig | 3 -- sound/core/Makefile | 3 +- sound/core/jack.c | 141 +++++++++++++++++++++++++++++++++++++++++++++++++- sound/pci/hda/Kconfig | 1 - 5 files changed, 157 insertions(+), 8 deletions(-)
diff --git a/include/sound/jack.h b/include/sound/jack.h index 2182350..8337000 100644 --- a/include/sound/jack.h +++ b/include/sound/jack.h @@ -73,6 +73,8 @@ enum snd_jack_types {
struct snd_jack { struct input_dev *input_dev;
- struct list_head kctl_list;
- struct snd_card *card; int registered; int type; const char *id;
@@ -82,10 +84,19 @@ struct snd_jack { void (*private_free)(struct snd_jack *); };
+struct snd_jack_kctl {
- struct snd_kcontrol *kctl;
- struct list_head list; /* list of controls belong to the same jack */
- unsigned int mask_bits; /* one of the corresponding bits of status change will report to this kctl */
- void *private_data;
- void (*private_free)(struct snd_jack_kctl *jack_kctl);
+};
You don't have to expose this. The hda_jack.c doesn't actually need these whole bits, as it supposes 1:1 as jack:kctl mapping. You can get rid of hda_jack->kctl since snd_jack_report() does it already, thus the private_data and the destructor can be dropped, too.
That said, the kctl management internal isn't the thing the driver needs to care.
#ifdef CONFIG_SND_JACK
int snd_jack_new(struct snd_card *card, const char *id, int type, struct snd_jack **jack); +int snd_jack_add_new_kctls(struct snd_jack *jack, const char * name, int mask);
It adds a single kctl. Using a plural (kctl"s") is confusing.
+static int snd_jack_get_available_index(struct snd_card *card, const char *name) +{
- struct snd_ctl_elem_id sid;
- memset(&sid, 0, sizeof(sid));
- sid.index = 0;
- sid.iface = SNDRV_CTL_ELEM_IFACE_CARD;
- strlcpy(sid.name, name, sizeof(sid.name));
- while (snd_ctl_find_id(card, &sid)) {
sid.index++;- }
Drop braces for a single line loop.
- return sid.index;
+}
IMO, this function should be rather put to ctljack.c. It's kctl specific, and not really defining the jack implementation itself.
+static void snd_jack_kctl_private_free(struct snd_kcontrol *kctl) +{
- struct snd_jack_kctl *jack_kctl;
- if (kctl) {
This NULL check is superfluous.
jack_kctl = kctl->private_data;if (jack_kctl) {if (jack_kctl->private_free)jack_kctl->private_free(jack_kctl);list_del(&jack_kctl->list);kfree(jack_kctl);}- }
+}
+static void snd_jack_kctl_name_gen(char *name, const char *jack_id, int size) +{
- size_t count = strlen(jack_id);
- /* remove redundant " Jack" from jack_id */
- if (count >= 5)
count = strncmp(&jack_id[count - 5], " Jack", 5) ? count : count - 5;- count = (size > count) ? count + 1 : size;
- /* name[count] will be filled to '\0' */
- strlcpy(name, jack_id, count);
+}
This function should be moved to ctljack.c.
That is, split the code finding a unique index and the removal of superfluous jack suffix as another patch. There fold that code into snd_kctl_jack_new() itself. At the same time, the code doing the same thing in hda_jack.c can be removed. Then the index argument of snd_kctl_jack_new() can be dropped.
As a bonus, you can save the stack size, too, by using a single string buffer for the name string. "Jack" suffix manipulation can be simplified there.
+static void snd_jack_kctl_add(struct snd_jack *jack, struct snd_jack_kctl *jack_kctl) +{
- list_add_tail(&jack_kctl->list, &jack->kctl_list);
+}
+/**
- snd_jack_kctl_new - Create a new snd_jack_kctl and return it
- @card: the card instance
- @kctl_name: the name for the snd_kcontrol object
- @mask: a bitmask of enum snd_jack_type values that can be detected
by this snd_jack_kctl object.
- Creates a new snd_kcontrol object, and assign it to the new created
snd_jack_kctl object.
- Return: The new created snd_jack_kctl object, or NULL if failed.
- */
+static struct snd_jack_kctl * snd_jack_kctl_new(struct snd_card *card, const char *name, unsigned int mask) +{
- struct snd_kcontrol *kctl;
- struct snd_jack_kctl *jack_kctl;
- int index, err;
- char jack_name[SNDRV_CTL_ELEM_ID_NAME_MAXLEN];
- index = snd_jack_get_available_index(card, name);
- snd_jack_kctl_name_gen(jack_name, name, SNDRV_CTL_ELEM_ID_NAME_MAXLEN);
- kctl = snd_kctl_jack_new(jack_name, index, card);
- if (!kctl)
return NULL;- jack_kctl = kzalloc(sizeof(*jack_kctl), GFP_KERNEL);
- if (!jack_kctl)
goto error;- jack_kctl->kctl = kctl;
- jack_kctl->mask_bits = mask;
- kctl->private_data = jack_kctl;
- kctl->private_free = snd_jack_kctl_private_free;
- err = snd_ctl_add(card, jack_kctl->kctl);
- if (err < 0)
goto error;- return jack_kctl;
+error:
- snd_ctl_free_one(kctl);
This will be a double-free. snd_ctl_add() itself frees the object at error path.
Takashi