* [PATCH] ucm: change msleep to usleep
@ 2011-08-19 7:22 Lu Guanqun
2011-08-19 7:40 ` Takashi Iwai
0 siblings, 1 reply; 9+ messages in thread
From: Lu Guanqun @ 2011-08-19 7:22 UTC (permalink / raw)
To: Takashi Iwai, ALSA, Jaroslav Kysela
In file src/ucm/parser.c:
if (strcmp(cmd, "usleep") == 0) {
string `usleep' is compared, however, in the comment and example conf file,
`msleep' is used, it's better to unify them all.
Signed-off-by: Lu Guanqun <guanqun.lu@intel.com>
---
src/ucm/parser.c | 8 ++++----
test/ucm/TestHDA/TestHDA.conf | 2 +-
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/src/ucm/parser.c b/src/ucm/parser.c
index 23b67bc..a56f1e6 100644
--- a/src/ucm/parser.c
+++ b/src/ucm/parser.c
@@ -792,7 +792,7 @@ static int parse_modifier_name(snd_use_case_mgr_t *uc_mgr,
* EnableSequence [
* cset "name='Master Playback Switch',index=2 0,0"
* cset "name='Master Playback Volume',index=2 25,25"
- * msleep 50
+ * usleep 50
* cset "name='Master Playback Switch',index=2 1,1"
* cset "name='Master Playback Volume',index=2 50,50"
* ]
@@ -800,14 +800,14 @@ static int parse_modifier_name(snd_use_case_mgr_t *uc_mgr,
* DisableSequence [
* cset "name='Master Playback Switch',index=2 0,0"
* cset "name='Master Playback Volume',index=2 25,25"
- * msleep 50
+ * usleep 50
* cset "name='Master Playback Switch',index=2 1,1"
* cset "name='Master Playback Volume',index=2 50,50"
* ]
*
* # Optional transition verb
* TransitionSequence."ToCaseName" [
- * msleep 1
+ * usleep 1
* ]
*
* # Optional TQ and ALSA PCMs
@@ -1106,7 +1106,7 @@ static int parse_controls(snd_use_case_mgr_t *uc_mgr, snd_config_t *cfg)
* cset "name='Master Mono Playback Volume',index=1 0"
* cset "name='PCM Switch',index=2 1,1"
* exec "some binary here"
- * msleep 50
+ * usleep 50
* ........
* ]
*
diff --git a/test/ucm/TestHDA/TestHDA.conf b/test/ucm/TestHDA/TestHDA.conf
index 41dd74c..0a75e21 100644
--- a/test/ucm/TestHDA/TestHDA.conf
+++ b/test/ucm/TestHDA/TestHDA.conf
@@ -7,7 +7,7 @@ SectionUseCase."Case1" {
SectionDefaults [
exec "my prg"
- msleep 1
+ usleep 1
cdev "hw:0"
cset "name='PCM Playback Volume' 50%"
]
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH] ucm: change msleep to usleep
2011-08-19 7:22 [PATCH] ucm: change msleep to usleep Lu Guanqun
@ 2011-08-19 7:40 ` Takashi Iwai
2011-08-19 8:13 ` Lu Guanqun
2011-08-19 16:12 ` Liam Girdwood
0 siblings, 2 replies; 9+ messages in thread
From: Takashi Iwai @ 2011-08-19 7:40 UTC (permalink / raw)
To: Lu Guanqun; +Cc: ALSA
At Fri, 19 Aug 2011 15:22:17 +0800,
Lu Guanqun wrote:
>
> In file src/ucm/parser.c:
> if (strcmp(cmd, "usleep") == 0) {
> string `usleep' is compared, however, in the comment and example conf file,
> `msleep' is used, it's better to unify them all.
Don't we scale the value appropriately (although it's just a demo)?
Takashi
>
> Signed-off-by: Lu Guanqun <guanqun.lu@intel.com>
> ---
> src/ucm/parser.c | 8 ++++----
> test/ucm/TestHDA/TestHDA.conf | 2 +-
> 2 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/src/ucm/parser.c b/src/ucm/parser.c
> index 23b67bc..a56f1e6 100644
> --- a/src/ucm/parser.c
> +++ b/src/ucm/parser.c
> @@ -792,7 +792,7 @@ static int parse_modifier_name(snd_use_case_mgr_t *uc_mgr,
> * EnableSequence [
> * cset "name='Master Playback Switch',index=2 0,0"
> * cset "name='Master Playback Volume',index=2 25,25"
> - * msleep 50
> + * usleep 50
> * cset "name='Master Playback Switch',index=2 1,1"
> * cset "name='Master Playback Volume',index=2 50,50"
> * ]
> @@ -800,14 +800,14 @@ static int parse_modifier_name(snd_use_case_mgr_t *uc_mgr,
> * DisableSequence [
> * cset "name='Master Playback Switch',index=2 0,0"
> * cset "name='Master Playback Volume',index=2 25,25"
> - * msleep 50
> + * usleep 50
> * cset "name='Master Playback Switch',index=2 1,1"
> * cset "name='Master Playback Volume',index=2 50,50"
> * ]
> *
> * # Optional transition verb
> * TransitionSequence."ToCaseName" [
> - * msleep 1
> + * usleep 1
> * ]
> *
> * # Optional TQ and ALSA PCMs
> @@ -1106,7 +1106,7 @@ static int parse_controls(snd_use_case_mgr_t *uc_mgr, snd_config_t *cfg)
> * cset "name='Master Mono Playback Volume',index=1 0"
> * cset "name='PCM Switch',index=2 1,1"
> * exec "some binary here"
> - * msleep 50
> + * usleep 50
> * ........
> * ]
> *
> diff --git a/test/ucm/TestHDA/TestHDA.conf b/test/ucm/TestHDA/TestHDA.conf
> index 41dd74c..0a75e21 100644
> --- a/test/ucm/TestHDA/TestHDA.conf
> +++ b/test/ucm/TestHDA/TestHDA.conf
> @@ -7,7 +7,7 @@ SectionUseCase."Case1" {
>
> SectionDefaults [
> exec "my prg"
> - msleep 1
> + usleep 1
> cdev "hw:0"
> cset "name='PCM Playback Volume' 50%"
> ]
>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] ucm: change msleep to usleep
2011-08-19 7:40 ` Takashi Iwai
@ 2011-08-19 8:13 ` Lu Guanqun
2011-08-19 8:21 ` Takashi Iwai
2011-08-19 16:12 ` Liam Girdwood
1 sibling, 1 reply; 9+ messages in thread
From: Lu Guanqun @ 2011-08-19 8:13 UTC (permalink / raw)
To: Takashi Iwai; +Cc: ALSA
Hi Takashi,
On Fri, Aug 19, 2011 at 03:40:37PM +0800, Takashi Iwai wrote:
> At Fri, 19 Aug 2011 15:22:17 +0800,
> Lu Guanqun wrote:
> >
> > In file src/ucm/parser.c:
> > if (strcmp(cmd, "usleep") == 0) {
> > string `usleep' is compared, however, in the comment and example conf file,
> > `msleep' is used, it's better to unify them all.
>
> Don't we scale the value appropriately (although it's just a demo)?
No.
This is where the value gets set in src/ucm/parser.c:
if (strcmp(cmd, "usleep") == 0) {
curr->type = SEQUENCE_ELEMENT_TYPE_SLEEP;
err = snd_config_get_integer(n, &curr->data.sleep);
if (err < 0) {
uc_error("error: usleep requires integer!");
return err;
}
continue;
}
This is where it gets used in src/ucm/main.c:
case SEQUENCE_ELEMENT_TYPE_SLEEP:
usleep(s->data.sleep);
break;
I don't see it gets scaled somewhere...
I agree it's a demo, but as the documentation for UCM is very limited,
the wrong variable will get users into confusion.
>
>
> Takashi
>
> >
> > Signed-off-by: Lu Guanqun <guanqun.lu@intel.com>
> > ---
> > src/ucm/parser.c | 8 ++++----
> > test/ucm/TestHDA/TestHDA.conf | 2 +-
> > 2 files changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/src/ucm/parser.c b/src/ucm/parser.c
> > index 23b67bc..a56f1e6 100644
> > --- a/src/ucm/parser.c
> > +++ b/src/ucm/parser.c
> > @@ -792,7 +792,7 @@ static int parse_modifier_name(snd_use_case_mgr_t *uc_mgr,
> > * EnableSequence [
> > * cset "name='Master Playback Switch',index=2 0,0"
> > * cset "name='Master Playback Volume',index=2 25,25"
> > - * msleep 50
> > + * usleep 50
> > * cset "name='Master Playback Switch',index=2 1,1"
> > * cset "name='Master Playback Volume',index=2 50,50"
> > * ]
> > @@ -800,14 +800,14 @@ static int parse_modifier_name(snd_use_case_mgr_t *uc_mgr,
> > * DisableSequence [
> > * cset "name='Master Playback Switch',index=2 0,0"
> > * cset "name='Master Playback Volume',index=2 25,25"
> > - * msleep 50
> > + * usleep 50
> > * cset "name='Master Playback Switch',index=2 1,1"
> > * cset "name='Master Playback Volume',index=2 50,50"
> > * ]
> > *
> > * # Optional transition verb
> > * TransitionSequence."ToCaseName" [
> > - * msleep 1
> > + * usleep 1
> > * ]
> > *
> > * # Optional TQ and ALSA PCMs
> > @@ -1106,7 +1106,7 @@ static int parse_controls(snd_use_case_mgr_t *uc_mgr, snd_config_t *cfg)
> > * cset "name='Master Mono Playback Volume',index=1 0"
> > * cset "name='PCM Switch',index=2 1,1"
> > * exec "some binary here"
> > - * msleep 50
> > + * usleep 50
> > * ........
> > * ]
> > *
> > diff --git a/test/ucm/TestHDA/TestHDA.conf b/test/ucm/TestHDA/TestHDA.conf
> > index 41dd74c..0a75e21 100644
> > --- a/test/ucm/TestHDA/TestHDA.conf
> > +++ b/test/ucm/TestHDA/TestHDA.conf
> > @@ -7,7 +7,7 @@ SectionUseCase."Case1" {
> >
> > SectionDefaults [
> > exec "my prg"
> > - msleep 1
> > + usleep 1
> > cdev "hw:0"
> > cset "name='PCM Playback Volume' 50%"
> > ]
> >
--
guanqun
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] ucm: change msleep to usleep
2011-08-19 8:13 ` Lu Guanqun
@ 2011-08-19 8:21 ` Takashi Iwai
0 siblings, 0 replies; 9+ messages in thread
From: Takashi Iwai @ 2011-08-19 8:21 UTC (permalink / raw)
To: Lu Guanqun; +Cc: ALSA
At Fri, 19 Aug 2011 16:13:06 +0800,
Lu Guanqun wrote:
>
> Hi Takashi,
>
> On Fri, Aug 19, 2011 at 03:40:37PM +0800, Takashi Iwai wrote:
> > At Fri, 19 Aug 2011 15:22:17 +0800,
> > Lu Guanqun wrote:
> > >
> > > In file src/ucm/parser.c:
> > > if (strcmp(cmd, "usleep") == 0) {
> > > string `usleep' is compared, however, in the comment and example conf file,
> > > `msleep' is used, it's better to unify them all.
> >
> > Don't we scale the value appropriately (although it's just a demo)?
>
> No.
Then taking over the value as is doesn't look correct.
Takashi
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] ucm: change msleep to usleep
2011-08-19 7:40 ` Takashi Iwai
2011-08-19 8:13 ` Lu Guanqun
@ 2011-08-19 16:12 ` Liam Girdwood
2011-08-22 1:25 ` [PATCH] ucm: add another sequence 'msleep' Lu Guanqun
1 sibling, 1 reply; 9+ messages in thread
From: Liam Girdwood @ 2011-08-19 16:12 UTC (permalink / raw)
To: Lu Guanqun; +Cc: Takashi Iwai, ALSA
On 19/08/11 08:40, Takashi Iwai wrote:
> At Fri, 19 Aug 2011 15:22:17 +0800,
> Lu Guanqun wrote:
>>
>> In file src/ucm/parser.c:
>> if (strcmp(cmd, "usleep") == 0) {
>> string `usleep' is compared, however, in the comment and example conf file,
>> `msleep' is used, it's better to unify them all.
>
> Don't we scale the value appropriately (although it's just a demo)?
>
msleep is more useful here for pop prevention, but I suppose usleep could be useful too.
Could you redo to support both usleep and msleep ?
Thanks
Liam
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH] ucm: add another sequence 'msleep'
2011-08-19 16:12 ` Liam Girdwood
@ 2011-08-22 1:25 ` Lu Guanqun
2011-08-22 5:35 ` [PATCH v2] " Lu Guanqun
0 siblings, 1 reply; 9+ messages in thread
From: Lu Guanqun @ 2011-08-22 1:25 UTC (permalink / raw)
To: Takashi Iwai, ALSA, Liam Girdwood
Thus, we have two sleep statements:
msleep <milliseconds>
usleep <microseconds>
Signed-off-by: Lu Guanqun <guanqun.lu@intel.com>
---
src/ucm/parser.c | 11 +++++++++++
src/ucm/ucm_local.h | 2 +-
2 files changed, 12 insertions(+), 1 deletions(-)
diff --git a/src/ucm/parser.c b/src/ucm/parser.c
index 23b67bc..b93d832 100644
--- a/src/ucm/parser.c
+++ b/src/ucm/parser.c
@@ -316,6 +316,17 @@ static int parse_sequence(snd_use_case_mgr_t *uc_mgr ATTRIBUTE_UNUSED,
continue;
}
+ if (strcmp(cmd, "msleep") == 0) {
+ curr->type = SEQUENCE_ELEMENT_TYPE_SLEEP;
+ err = snd_config_get_integer(n, &curr->data.sleep);
+ if (err < 0) {
+ uc_error("error: msleep requires integer!");
+ return err;
+ }
+ curr->data.sleep *= 1000L;
+ continue;
+ }
+
if (strcmp(cmd, "exec") == 0) {
curr->type = SEQUENCE_ELEMENT_TYPE_EXEC;
err = parse_string(n, &curr->data.exec);
diff --git a/src/ucm/ucm_local.h b/src/ucm/ucm_local.h
index 0522bf5..aa4d493 100644
--- a/src/ucm/ucm_local.h
+++ b/src/ucm/ucm_local.h
@@ -57,7 +57,7 @@ struct sequence_element {
struct list_head list;
unsigned int type;
union {
- long sleep; /* Sleep time in msecs if sleep element, else 0 */
+ long sleep; /* Sleep time in macroseconds if sleep element, else 0 */
char *cdev;
char *cset;
char *exec;
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH v2] ucm: add another sequence 'msleep'
2011-08-22 1:25 ` [PATCH] ucm: add another sequence 'msleep' Lu Guanqun
@ 2011-08-22 5:35 ` Lu Guanqun
2011-08-22 10:39 ` Liam Girdwood
0 siblings, 1 reply; 9+ messages in thread
From: Lu Guanqun @ 2011-08-22 5:35 UTC (permalink / raw)
To: Takashi Iwai, ALSA, Liam Girdwood
Thus, we have two sleep statements:
msleep <milliseconds>
usleep <microseconds>
Signed-off-by: Lu Guanqun <guanqun.lu@intel.com>
---
src/ucm/parser.c | 11 +++++++++++
src/ucm/ucm_local.h | 2 +-
2 files changed, 12 insertions(+), 1 deletions(-)
diff --git a/src/ucm/parser.c b/src/ucm/parser.c
index 23b67bc..b93d832 100644
--- a/src/ucm/parser.c
+++ b/src/ucm/parser.c
@@ -316,6 +316,17 @@ static int parse_sequence(snd_use_case_mgr_t *uc_mgr ATTRIBUTE_UNUSED,
continue;
}
+ if (strcmp(cmd, "msleep") == 0) {
+ curr->type = SEQUENCE_ELEMENT_TYPE_SLEEP;
+ err = snd_config_get_integer(n, &curr->data.sleep);
+ if (err < 0) {
+ uc_error("error: msleep requires integer!");
+ return err;
+ }
+ curr->data.sleep *= 1000L;
+ continue;
+ }
+
if (strcmp(cmd, "exec") == 0) {
curr->type = SEQUENCE_ELEMENT_TYPE_EXEC;
err = parse_string(n, &curr->data.exec);
diff --git a/src/ucm/ucm_local.h b/src/ucm/ucm_local.h
index 0522bf5..03d3ace 100644
--- a/src/ucm/ucm_local.h
+++ b/src/ucm/ucm_local.h
@@ -57,7 +57,7 @@ struct sequence_element {
struct list_head list;
unsigned int type;
union {
- long sleep; /* Sleep time in msecs if sleep element, else 0 */
+ long sleep; /* Sleep time in microseconds if sleep element, else 0 */
char *cdev;
char *cset;
char *exec;
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH v2] ucm: add another sequence 'msleep'
2011-08-22 5:35 ` [PATCH v2] " Lu Guanqun
@ 2011-08-22 10:39 ` Liam Girdwood
2011-08-22 10:42 ` Takashi Iwai
0 siblings, 1 reply; 9+ messages in thread
From: Liam Girdwood @ 2011-08-22 10:39 UTC (permalink / raw)
To: Lu Guanqun; +Cc: Takashi Iwai, ALSA
On 22/08/11 06:35, Lu Guanqun wrote:
> Thus, we have two sleep statements:
> msleep <milliseconds>
> usleep <microseconds>
>
> Signed-off-by: Lu Guanqun <guanqun.lu@intel.com>
> ---
> src/ucm/parser.c | 11 +++++++++++
> src/ucm/ucm_local.h | 2 +-
> 2 files changed, 12 insertions(+), 1 deletions(-)
>
> diff --git a/src/ucm/parser.c b/src/ucm/parser.c
> index 23b67bc..b93d832 100644
> --- a/src/ucm/parser.c
> +++ b/src/ucm/parser.c
> @@ -316,6 +316,17 @@ static int parse_sequence(snd_use_case_mgr_t *uc_mgr ATTRIBUTE_UNUSED,
> continue;
> }
>
> + if (strcmp(cmd, "msleep") == 0) {
> + curr->type = SEQUENCE_ELEMENT_TYPE_SLEEP;
> + err = snd_config_get_integer(n, &curr->data.sleep);
> + if (err < 0) {
> + uc_error("error: msleep requires integer!");
> + return err;
> + }
> + curr->data.sleep *= 1000L;
> + continue;
> + }
> +
> if (strcmp(cmd, "exec") == 0) {
> curr->type = SEQUENCE_ELEMENT_TYPE_EXEC;
> err = parse_string(n, &curr->data.exec);
> diff --git a/src/ucm/ucm_local.h b/src/ucm/ucm_local.h
> index 0522bf5..03d3ace 100644
> --- a/src/ucm/ucm_local.h
> +++ b/src/ucm/ucm_local.h
> @@ -57,7 +57,7 @@ struct sequence_element {
> struct list_head list;
> unsigned int type;
> union {
> - long sleep; /* Sleep time in msecs if sleep element, else 0 */
> + long sleep; /* Sleep time in microseconds if sleep element, else 0 */
> char *cdev;
> char *cset;
> char *exec;
>
Acked-by: Liam Girdwood <lrg@ti.com>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v2] ucm: add another sequence 'msleep'
2011-08-22 10:39 ` Liam Girdwood
@ 2011-08-22 10:42 ` Takashi Iwai
0 siblings, 0 replies; 9+ messages in thread
From: Takashi Iwai @ 2011-08-22 10:42 UTC (permalink / raw)
To: Lu Guanqun; +Cc: ALSA, Liam Girdwood
At Mon, 22 Aug 2011 11:39:28 +0100,
Liam Girdwood wrote:
>
> On 22/08/11 06:35, Lu Guanqun wrote:
> > Thus, we have two sleep statements:
> > msleep <milliseconds>
> > usleep <microseconds>
> >
> > Signed-off-by: Lu Guanqun <guanqun.lu@intel.com>
> > ---
> > src/ucm/parser.c | 11 +++++++++++
> > src/ucm/ucm_local.h | 2 +-
> > 2 files changed, 12 insertions(+), 1 deletions(-)
> >
> > diff --git a/src/ucm/parser.c b/src/ucm/parser.c
> > index 23b67bc..b93d832 100644
> > --- a/src/ucm/parser.c
> > +++ b/src/ucm/parser.c
> > @@ -316,6 +316,17 @@ static int parse_sequence(snd_use_case_mgr_t *uc_mgr ATTRIBUTE_UNUSED,
> > continue;
> > }
> >
> > + if (strcmp(cmd, "msleep") == 0) {
> > + curr->type = SEQUENCE_ELEMENT_TYPE_SLEEP;
> > + err = snd_config_get_integer(n, &curr->data.sleep);
> > + if (err < 0) {
> > + uc_error("error: msleep requires integer!");
> > + return err;
> > + }
> > + curr->data.sleep *= 1000L;
> > + continue;
> > + }
> > +
> > if (strcmp(cmd, "exec") == 0) {
> > curr->type = SEQUENCE_ELEMENT_TYPE_EXEC;
> > err = parse_string(n, &curr->data.exec);
> > diff --git a/src/ucm/ucm_local.h b/src/ucm/ucm_local.h
> > index 0522bf5..03d3ace 100644
> > --- a/src/ucm/ucm_local.h
> > +++ b/src/ucm/ucm_local.h
> > @@ -57,7 +57,7 @@ struct sequence_element {
> > struct list_head list;
> > unsigned int type;
> > union {
> > - long sleep; /* Sleep time in msecs if sleep element, else 0 */
> > + long sleep; /* Sleep time in microseconds if sleep element, else 0 */
> > char *cdev;
> > char *cset;
> > char *exec;
> >
>
> Acked-by: Liam Girdwood <lrg@ti.com>
Applied now. Thanks.
Takashi
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2011-08-22 10:42 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-08-19 7:22 [PATCH] ucm: change msleep to usleep Lu Guanqun
2011-08-19 7:40 ` Takashi Iwai
2011-08-19 8:13 ` Lu Guanqun
2011-08-19 8:21 ` Takashi Iwai
2011-08-19 16:12 ` Liam Girdwood
2011-08-22 1:25 ` [PATCH] ucm: add another sequence 'msleep' Lu Guanqun
2011-08-22 5:35 ` [PATCH v2] " Lu Guanqun
2011-08-22 10:39 ` Liam Girdwood
2011-08-22 10:42 ` Takashi Iwai
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.