UCM updates and fixes

UCM updates and fixes

[Liam Girdwood]

Jarolsav,

This fixes the remaining issues I can currently see in the alsa-lib ucm branch. It also includes a patch to execute commands, although I dont know if this is what you had in mind for exec.

I've also done quite a bit of testing now. What is your merge plan for UCM ? We really need UCM merged soon as there are many users who depend on it.

Thanks

Liam

[PATCH 01/11] ucm: removed unused 'active' member in struct use_case_device

[Liam Girdwood]

Signed-off-by: Liam Girdwood <lrg@slimlogic.co.uk>
---
 src/ucm/ucm_local.h |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/src/ucm/ucm_local.h b/src/ucm/ucm_local.h
index 9f71d42..13e82da 100644
--- a/src/ucm/ucm_local.h
+++ b/src/ucm/ucm_local.h
@@ -115,8 +115,6 @@ struct use_case_device {
 	struct list_head list;
 	struct list_head active_list;
 
-	unsigned int active: 1;
-
 	char *name;
 	char *comment;
 
-- 
1.7.1

[PATCH 02/11] ucm: modifiers names must be compound parse

[Liam Girdwood]

Modifier names must end in a .index to ensure we can support
same named modifiers. However the modifier index will be for internal
use only and the client will not use the index when setting modifier.

The modifier selection for same name modifiers will be based upon supported
device.

Signed-off-by: Liam Girdwood <lrg@slimlogic.co.uk>
---
 src/ucm/parser.c |   27 +++++++++++++++++++++++----
 1 files changed, 23 insertions(+), 4 deletions(-)

diff --git a/src/ucm/parser.c b/src/ucm/parser.c
index 3d2fd71..7d547f4 100644
--- a/src/ucm/parser.c
+++ b/src/ucm/parser.c
@@ -401,10 +401,11 @@ static int parse_value(snd_use_case_mgr_t *uc_mgr ATTRIBUTE_UNUSED,
 static int parse_modifier(snd_use_case_mgr_t *uc_mgr,
 		snd_config_t *cfg,
 		void *data1,
-		void *data2 ATTRIBUTE_UNUSED)
+		void *data2)
 {
 	struct use_case_verb *verb = data1;
 	struct use_case_modifier *modifier;
+	char *name = data2;
 	const char *id;
 	snd_config_iterator_t i, next;
 	snd_config_t *n;
@@ -423,9 +424,12 @@ static int parse_modifier(snd_use_case_mgr_t *uc_mgr,
 	err = snd_config_get_id(cfg, &id);
 	if (err < 0)
 		return err;
-	modifier->name = strdup(id);
+	modifier->name = malloc(strlen(name) + strlen(id) + 2);
 	if (modifier->name == NULL)
-		return -EINVAL;
+		return -ENOMEM;
+	strcpy(modifier->name, name);
+	strcat(modifier->name, ".");
+	strcat(modifier->name, id);
 
 	snd_config_for_each(i, next, cfg) {
 		const char *id;
@@ -636,6 +640,21 @@ static int parse_device(snd_use_case_mgr_t *uc_mgr,
 	return parse_compound(uc_mgr, cfg, parse_device_name, verb, NULL);
 }
 
+static int parse_modifier_name(snd_use_case_mgr_t *uc_mgr,
+			     snd_config_t *cfg,
+			     void *data1,
+			     void *data2 ATTRIBUTE_UNUSED)
+{
+	const char *id;
+	int err;
+
+	err = snd_config_get_id(cfg, &id);
+	if (err < 0)
+		return err;
+	return parse_compound(uc_mgr, cfg, parse_modifier,
+			      data1, (void *)id);
+}
+
 /*
  * Parse Verb Section
  *
@@ -817,7 +836,7 @@ static int parse_verb_file(snd_use_case_mgr_t *uc_mgr,
 		/* find modifier sections and parse them */
 		if (strcmp(id, "SectionModifier") == 0) {
 			err = parse_compound(uc_mgr, n,
-					     parse_modifier, verb, NULL);
+					     parse_modifier_name, verb, NULL);
 			if (err < 0) {
 				uc_error("error: %s failed to parse modifier",
 						file);
-- 
1.7.1

[PATCH 03/11] ucm: remove parse_device() and call parse_compound() directly

[Liam Girdwood]

Signed-off-by: Liam Girdwood <lrg@slimlogic.co.uk>
---
 src/ucm/parser.c |   10 ++--------
 1 files changed, 2 insertions(+), 8 deletions(-)

diff --git a/src/ucm/parser.c b/src/ucm/parser.c
index 7d547f4..f3a75e6 100644
--- a/src/ucm/parser.c
+++ b/src/ucm/parser.c
@@ -633,13 +633,6 @@ static int parse_device_name(snd_use_case_mgr_t *uc_mgr,
 			      data1, (void *)id);
 }
 
-static int parse_device(snd_use_case_mgr_t *uc_mgr,
-			struct use_case_verb *verb,
-			snd_config_t *cfg)
-{
-	return parse_compound(uc_mgr, cfg, parse_device_name, verb, NULL);
-}
-
 static int parse_modifier_name(snd_use_case_mgr_t *uc_mgr,
 			     snd_config_t *cfg,
 			     void *data1,
@@ -824,7 +817,8 @@ static int parse_verb_file(snd_use_case_mgr_t *uc_mgr,
 
 		/* find device sections and parse them */
 		if (strcmp(id, "SectionDevice") == 0) {
-			err = parse_device(uc_mgr, verb, n);
+			err = parse_compound(uc_mgr, n,
+						parse_device_name, verb, NULL);
 			if (err < 0) {
 				uc_error("error: %s failed to parse device",
 						file);
-- 
1.7.1

[PATCH 04/11] ucm: implement basic script exec functionality

[Liam Girdwood]

Signed-off-by: Liam Girdwood <lrg@slimlogic.co.uk>
---
 src/ucm/main.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/src/ucm/main.c b/src/ucm/main.c
index ecf07f9..64e4279 100644
--- a/src/ucm/main.c
+++ b/src/ucm/main.c
@@ -265,7 +265,9 @@ static int execute_sequence(snd_use_case_mgr_t *uc_mgr,
 			usleep(s->data.sleep);
 			break;
 		case SEQUENCE_ELEMENT_TYPE_EXEC:
-			uc_error("exec not yet implemented: '%s'", s->data.exec);
+			err = system(s->data.exec);
+			if (err < 0)
+				goto __fail;
 			break;
 		default:
 			uc_error("unknown sequence command %i", s->type);
-- 
1.7.1

[PATCH 05/11] ucm: make sure active modifier and device lists are initialised

[Liam Girdwood]

Signed-off-by: Liam Girdwood <lrg@slimlogic.co.uk>
---
 src/ucm/main.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/src/ucm/main.c b/src/ucm/main.c
index 64e4279..fae89a2 100644
--- a/src/ucm/main.c
+++ b/src/ucm/main.c
@@ -587,6 +587,8 @@ int snd_use_case_mgr_open(snd_use_case_mgr_t **mgr,
 	INIT_LIST_HEAD(&uc_mgr->verb_list);
 	INIT_LIST_HEAD(&uc_mgr->default_list);
 	INIT_LIST_HEAD(&uc_mgr->value_list);
+	INIT_LIST_HEAD(&uc_mgr->active_modifiers);
+	INIT_LIST_HEAD(&uc_mgr->active_devices);
 	pthread_mutex_init(&uc_mgr->mutex, NULL);
 
 	uc_mgr->card_name = strdup(card_name);
-- 
1.7.1

[PATCH 06/11] ucm: remove unused code.

[Liam Girdwood]

Signed-off-by: Liam Girdwood <lrg@slimlogic.co.uk>
---
 src/ucm/main.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/src/ucm/main.c b/src/ucm/main.c
index fae89a2..403f5c5 100644
--- a/src/ucm/main.c
+++ b/src/ucm/main.c
@@ -744,7 +744,6 @@ static int get_device_list(snd_use_case_mgr_t *uc_mgr, const char **list[],
         return get_list2(&verb->device_list, list,
                          struct use_case_device, list,
                          name, comment);
-        return 0;
 }
 
 /**
@@ -768,7 +767,6 @@ static int get_modifier_list(snd_use_case_mgr_t *uc_mgr, const char **list[],
         return get_list2(&verb->modifier_list, list,
                          struct use_case_modifier, list,
                          name, comment);
-        return 0;
 }
 
 struct myvalue {
-- 
1.7.1

[PATCH 07/11] ucm: check for valid value list before dereference.

[Liam Girdwood]

Signed-off-by: Liam Girdwood <lrg@slimlogic.co.uk>
---
 src/ucm/main.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/src/ucm/main.c b/src/ucm/main.c
index 403f5c5..3a61679 100644
--- a/src/ucm/main.c
+++ b/src/ucm/main.c
@@ -959,6 +959,9 @@ static int get_value1(const char **value, struct list_head *value_list,
         struct ucm_value *val;
         struct list_head *pos;
         
+	if (!value_list)
+		return -ENOENT;
+
         list_for_each(pos, value_list) {
               val = list_entry(pos, struct ucm_value, list);
               if (check_identifier(identifier, val->name)) {
-- 
1.7.1

[PATCH 08/11] ucm: check the correct return value for modifier

[Liam Girdwood]

Signed-off-by: Liam Girdwood <lrg@slimlogic.co.uk>
---
 src/ucm/main.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/src/ucm/main.c b/src/ucm/main.c
index 3a61679..4c13547 100644
--- a/src/ucm/main.c
+++ b/src/ucm/main.c
@@ -1323,7 +1323,7 @@ static int switch_modifier(snd_use_case_mgr_t *uc_mgr,
         if (xold == NULL)
                 return -ENOENT;
         xnew = find_modifier(uc_mgr->active_verb, new_modifier);
-        if (xold == NULL)
+        if (xnew == NULL)
                 return -ENOENT;
         err = 0;
         list_for_each(pos, &xold->transition_list) {
-- 
1.7.1

[PATCH 09/11] ucm: only select modifier when supported device is enabled

[Liam Girdwood]

Make sure the supported device of a modifier is enabled before
we enable the modifier.

Signed-off-by: Liam Girdwood <lrg@slimlogic.co.uk>
---
 src/ucm/main.c |   54 ++++++++++++++++++++++++++++++++++++++++++++----------
 1 files changed, 44 insertions(+), 10 deletions(-)

diff --git a/src/ucm/main.c b/src/ucm/main.c
index 4c13547..5f44461 100644
--- a/src/ucm/main.c
+++ b/src/ucm/main.c
@@ -463,19 +463,52 @@ static inline struct use_case_device *
 		    device_name);
 }
 
+static int is_modifier_supported(snd_use_case_mgr_t *uc_mgr, 
+	struct use_case_modifier *modifier)
+{
+	struct dev_list *device;
+	struct list_head *pos;
+
+	list_for_each(pos, &modifier->dev_list) {
+		device = list_entry(pos, struct dev_list, list);
+		if (find(&uc_mgr->active_devices,
+		    struct use_case_device, active_list, name, device->name))
+			return 1;
+		
+	}
+	return 0;
+}
+
 /**
  * \brief Find modifier
  * \param verb Use case verb
  * \param modifier_name modifier to find
  * \return structure on success, otherwise a NULL (not found)
  */
-static inline struct use_case_modifier *
-        find_modifier(struct use_case_verb *verb,
-                      const char *modifier_name)
+static struct use_case_modifier *
+        find_modifier(snd_use_case_mgr_t *uc_mgr, const char *modifier_name)
 {
-	return find(&verb->modifier_list,
-		    struct use_case_modifier, list, name,
-		    modifier_name);
+	struct use_case_modifier *modifier;
+	struct use_case_verb *verb = uc_mgr->active_verb;
+	struct list_head *pos;
+	char name[64], *cpos;
+
+	list_for_each(pos, &verb->modifier_list) {
+		modifier = list_entry(pos, struct use_case_modifier, list);
+
+		strncpy(name, modifier->name, sizeof(name));
+		cpos = strchr(name, '.');
+		if (!cpos)
+			continue;
+		*cpos= '\0';
+
+		if (strcmp(name, modifier_name))
+			continue;
+
+		if (is_modifier_supported(uc_mgr, modifier))
+			return modifier;
+	}
+	return NULL;
 }
 
 /**
@@ -1011,7 +1044,7 @@ static int get_value(snd_use_case_mgr_t *uc_mgr,
 	int err;
 
 	if (modifier != NULL) {
-	        mod = find_modifier(uc_mgr->active_verb, modifier);
+	        mod = find_modifier(uc_mgr, modifier);
 		if (mod != NULL) {
 			err = get_value1(value, &mod->value_list, identifier);
 			if (err >= 0 || err != -ENOENT)
@@ -1242,7 +1275,8 @@ static int set_modifier_user(snd_use_case_mgr_t *uc_mgr,
 
         if (uc_mgr->active_verb == NULL)
                 return -ENOENT;
-        modifier = find_modifier(uc_mgr->active_verb, modifier_name);
+
+        modifier = find_modifier(uc_mgr, modifier_name);
         if (modifier == NULL)
                 return -ENOENT;
         return set_modifier(uc_mgr, modifier, enable);
@@ -1319,10 +1353,10 @@ static int switch_modifier(snd_use_case_mgr_t *uc_mgr,
                 uc_error("error: modifier %s already enabled", new_modifier);
                 return -EINVAL;
         }
-        xold = find_modifier(uc_mgr->active_verb, old_modifier);
+        xold = find_modifier(uc_mgr, old_modifier);
         if (xold == NULL)
                 return -ENOENT;
-        xnew = find_modifier(uc_mgr->active_verb, new_modifier);
+        xnew = find_modifier(uc_mgr, new_modifier);
         if (xnew == NULL)
                 return -ENOENT;
         err = 0;
-- 
1.7.1

[PATCH 10/11] ucm: fix handling of cset with spaces in the control name

[Liam Girdwood]

Always terminate the cset command based on the last space found within the
cset command since the control name may contain spaces.
---
 src/ucm/main.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/src/ucm/main.c b/src/ucm/main.c
index 5f44461..7305042 100644
--- a/src/ucm/main.c
+++ b/src/ucm/main.c
@@ -168,7 +168,7 @@ static int execute_cset(snd_ctl_t *ctl, char *cset)
 	snd_ctl_elem_value_malloc(&value);
 	snd_ctl_elem_info_malloc(&info);
 
-	pos = strchr(cset, ' ');
+	pos = strrchr(cset, ' ');
 	if (pos == NULL) {
 		uc_error("undefined value for cset >%s<", cset);
 		return -EINVAL;
-- 
1.7.1

[PATCH 11/11] ucm: ctlparse - always terminate cset buffer

[Liam Girdwood]

Signed-off-by: Liam Girdwood <lrg@slimlogic.co.uk>
---
 src/control/ctlparse.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/src/control/ctlparse.c b/src/control/ctlparse.c
index c8300f3..a929816 100644
--- a/src/control/ctlparse.c
+++ b/src/control/ctlparse.c
@@ -208,8 +208,8 @@ int snd_ctl_ascii_elem_id_parse(snd_ctl_elem_id_t *dst, const char *str)
 					}
 					str++;
 				}
-				*ptr = '\0';
 			}
+			*ptr = '\0';
 			snd_ctl_elem_id_set_name(dst, buf);
 		} else if (!strncasecmp(str, "index=", 6)) {
 			str += 6;
-- 
1.7.1

Re: UCM updates and fixes

[Mark Brown]

On Tue, Dec 21, 2010 at 10:11:49PM +0000, Liam Girdwood wrote:

> This fixes the remaining issues I can currently see in the alsa-lib
> ucm branch. It also includes a patch to execute commands, although I
> dont know if this is what you had in mind for exec.

All looks good to me.

Re: UCM updates and fixes

[Liam Girdwood]

On Tue, 2010-12-21 at 22:11 +0000, Liam Girdwood wrote:
> Jarolsav,
> 
> This fixes the remaining issues I can currently see in the alsa-lib
> ucm branch. It also includes a patch to execute commands, although I
> dont know if this is what you had in mind for exec.
> 
> I've also done quite a bit of testing now. What is your merge plan for
> UCM ? We really need UCM merged soon as there are many users who
> depend on it.
> 

Have you had a chance to look over this patch series yet ?

Thanks

Liam

-- 
Freelance Developer, SlimLogic Ltd
ASoC and Voltage Regulator Maintainer.
http://www.slimlogic.co.uk

Re: UCM updates and fixes

[Jaroslav Kysela]

On Tue, 11 Jan 2011, Liam Girdwood wrote:

> On Tue, 2010-12-21 at 22:11 +0000, Liam Girdwood wrote:
>> Jarolsav,
>>
>> This fixes the remaining issues I can currently see in the alsa-lib
>> ucm branch. It also includes a patch to execute commands, although I
>> dont know if this is what you had in mind for exec.
>>
>> I've also done quite a bit of testing now. What is your merge plan for
>> UCM ? We really need UCM merged soon as there are many users who
>> depend on it.
>>
>
> Have you had a chance to look over this patch series yet ?

Your patches are in my branch. Thank you. I'm actually thinking about more 
inline functions in use-case.h header file like snd_use_case_card_list() to help 
application developers. But I'm having mixed feelings about this right 
now. What do you think?

 					Jaroslav

-----
Jaroslav Kysela <perex@perex.cz>
Linux Kernel Sound Maintainer
ALSA Project, Red Hat, Inc.

Re: UCM updates and fixes

[Liam Girdwood]

On Tue, 2011-01-11 at 17:46 +0100, Jaroslav Kysela wrote:
> On Tue, 11 Jan 2011, Liam Girdwood wrote:
> 
> > On Tue, 2010-12-21 at 22:11 +0000, Liam Girdwood wrote:
> >> Jarolsav,
> >>
> >> This fixes the remaining issues I can currently see in the alsa-lib
> >> ucm branch. It also includes a patch to execute commands, although I
> >> dont know if this is what you had in mind for exec.
> >>
> >> I've also done quite a bit of testing now. What is your merge plan for
> >> UCM ? We really need UCM merged soon as there are many users who
> >> depend on it.
> >>
> >
> > Have you had a chance to look over this patch series yet ?
> 
> Your patches are in my branch. Thank you. I'm actually thinking about more 
> inline functions in use-case.h header file like snd_use_case_card_list() to help 
> application developers. But I'm having mixed feelings about this right 
> now. What do you think?

My feelings are we go for a release now and make the relevant functions
inline later. We currently have a lot of folks needing UCM upstream now
to delay much longer.

I have one more small fix pending from Samsung, will send shortly. 

Thanks

Liam

-- 
Freelance Developer, SlimLogic Ltd
ASoC and Voltage Regulator Maintainer.
http://www.slimlogic.co.uk

Re: UCM updates and fixes

[Jaroslav Kysela]

On Tue, 11 Jan 2011, Liam Girdwood wrote:

> On Tue, 2011-01-11 at 17:46 +0100, Jaroslav Kysela wrote:
>> On Tue, 11 Jan 2011, Liam Girdwood wrote:
>>
>>> On Tue, 2010-12-21 at 22:11 +0000, Liam Girdwood wrote:
>>>> Jarolsav,
>>>>
>>>> This fixes the remaining issues I can currently see in the alsa-lib
>>>> ucm branch. It also includes a patch to execute commands, although I
>>>> dont know if this is what you had in mind for exec.
>>>>
>>>> I've also done quite a bit of testing now. What is your merge plan for
>>>> UCM ? We really need UCM merged soon as there are many users who
>>>> depend on it.
>>>>
>>>
>>> Have you had a chance to look over this patch series yet ?
>>
>> Your patches are in my branch. Thank you. I'm actually thinking about more
>> inline functions in use-case.h header file like snd_use_case_card_list() to help
>> application developers. But I'm having mixed feelings about this right
>> now. What do you think?
>
> My feelings are we go for a release now and make the relevant functions
> inline later. We currently have a lot of folks needing UCM upstream now
> to delay much longer.

Okay. I will concentrate on the next ALSA release in next days.

 						Jaroslav

> I have one more small fix pending from Samsung, will send shortly.

Go ahead. Thanks.

-----
Jaroslav Kysela <perex@perex.cz>
Linux Kernel Sound Maintainer
ALSA Project, Red Hat, Inc.