[PATCH] sound: usb: mixer: increase control transfer timeout

[PATCH] sound: usb: mixer: increase control transfer timeout

[Felipe Balbi]

100ms might not be enough for slower devices. Increasing
timeout to 1000ms doesn't seem like overkill.

Signed-off-by: Felipe Balbi <balbi@ti.com>
---

Not sure this information is valid on a commit log, but I
have a FPGA-platform which I use to test my USB controller
and when that's running on FS, this message fails because
control message times out before controller has the opportunity
to complete all three phases of the SETUP transaction.

 sound/usb/mixer.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c
index cdd19d7..a3cbc98 100644
--- a/sound/usb/mixer.c
+++ b/sound/usb/mixer.c
@@ -296,7 +296,7 @@ static int get_ctl_value_v1(struct usb_mixer_elem_info *cval, int request, int v
 		if (snd_usb_ctl_msg(chip->dev, usb_rcvctrlpipe(chip->dev, 0), request,
 				    USB_RECIP_INTERFACE | USB_TYPE_CLASS | USB_DIR_IN,
 				    validx, snd_usb_ctrl_intf(chip) | (cval->id << 8),
-				    buf, val_len, 100) >= val_len) {
+				    buf, val_len, 1000) >= val_len) {
 			*value_ret = convert_signed_value(cval, snd_usb_combine_bytes(buf, val_len));
 			snd_usb_autosuspend(cval->mixer->chip);
 			return 0;
-- 
1.7.6.396.ge0613

Re: [PATCH] sound: usb: mixer: increase control transfer timeout

[Sebastian Andrzej Siewior]

* Felipe Balbi | 2011-08-29 14:10:26 [+0300]:

>Not sure this information is valid on a commit log, but I
>have a FPGA-platform which I use to test my USB controller
>and when that's running on FS, this message fails because
>control message times out before controller has the opportunity
>to complete all three phases of the SETUP transaction.

Does it still happen if you remove all the debug output from your udc
driver?

Sebastian

Re: [PATCH] sound: usb: mixer: increase control transfer timeout

[Felipe Balbi]

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

Hi,

On Mon, Aug 29, 2011 at 01:16:07PM +0200, Sebastian Andrzej Siewior wrote:
> * Felipe Balbi | 2011-08-29 14:10:26 [+0300]:
> 
> >Not sure this information is valid on a commit log, but I
> >have a FPGA-platform which I use to test my USB controller
> >and when that's running on FS, this message fails because
> >control message times out before controller has the opportunity
> >to complete all three phases of the SETUP transaction.
> 
> Does it still happen if you remove all the debug output from your udc
> driver?

I can try it out, but what's the point ? If we don't increase the
timeout, then whenever we want to debug some other problem, we will have
to keep a local patch on that file ?

-- 
balbi

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

Re: [PATCH] sound: usb: mixer: increase control transfer timeout

[Clemens Ladisch]

Felipe Balbi wrote:
> 100ms might not be enough for slower devices. Increasing
> timeout to 1000ms doesn't seem like overkill.
> 
> --- a/sound/usb/mixer.c
> +++ b/sound/usb/mixer.c
> @@ -296,7 +296,7 @@ static int get_ctl_value_v1(struct usb_mixer_elem_info *cval, int request, int v
>  		if (snd_usb_ctl_msg(chip->dev, usb_rcvctrlpipe(chip->dev, 0), request,
>  				    USB_RECIP_INTERFACE | USB_TYPE_CLASS | USB_DIR_IN,
>  				    validx, snd_usb_ctrl_intf(chip) | (cval->id << 8),
> -				    buf, val_len, 100) >= val_len) {
> +				    buf, val_len, 1000) >= val_len) {

If the timeout for reading values needs to be increased, writing (in
snd_usb_mixer_set_ctl_value) probably needs some attention too.


Regards,
Clemens

Re: [PATCH] sound: usb: mixer: increase control transfer timeout

[Felipe Balbi]

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

Hi,

On Mon, Aug 29, 2011 at 01:58:31PM +0200, Clemens Ladisch wrote:
> Felipe Balbi wrote:
> > 100ms might not be enough for slower devices. Increasing
> > timeout to 1000ms doesn't seem like overkill.
> > 
> > --- a/sound/usb/mixer.c
> > +++ b/sound/usb/mixer.c
> > @@ -296,7 +296,7 @@ static int get_ctl_value_v1(struct usb_mixer_elem_info *cval, int request, int v
> >  		if (snd_usb_ctl_msg(chip->dev, usb_rcvctrlpipe(chip->dev, 0), request,
> >  				    USB_RECIP_INTERFACE | USB_TYPE_CLASS | USB_DIR_IN,
> >  				    validx, snd_usb_ctrl_intf(chip) | (cval->id << 8),
> > -				    buf, val_len, 100) >= val_len) {
> > +				    buf, val_len, 1000) >= val_len) {
> 
> If the timeout for reading values needs to be increased, writing (in
> snd_usb_mixer_set_ctl_value) probably needs some attention too.

Ok, I'll increase all 100ms delays to 1000. Is that ok ?

-- 
balbi

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

[PATCH v2] sound: usb: mixer: increase control transfer timeout

[Felipe Balbi]

100ms might not be enough for slower devices. Increasing
timeout to 1000ms doesn't seem like overkill.

Signed-off-by: Felipe Balbi <balbi@ti.com>
---

Changes since v1:
 . Increase all 100ms timeout to 1000ms

 sound/usb/mixer.c        |    4 ++--
 sound/usb/mixer_quirks.c |   10 +++++-----
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c
index cdd19d7..1c9d443 100644
--- a/sound/usb/mixer.c
+++ b/sound/usb/mixer.c
@@ -296,7 +296,7 @@ static int get_ctl_value_v1(struct usb_mixer_elem_info *cval, int request, int v
 		if (snd_usb_ctl_msg(chip->dev, usb_rcvctrlpipe(chip->dev, 0), request,
 				    USB_RECIP_INTERFACE | USB_TYPE_CLASS | USB_DIR_IN,
 				    validx, snd_usb_ctrl_intf(chip) | (cval->id << 8),
-				    buf, val_len, 100) >= val_len) {
+				    buf, val_len, 1000) >= val_len) {
 			*value_ret = convert_signed_value(cval, snd_usb_combine_bytes(buf, val_len));
 			snd_usb_autosuspend(cval->mixer->chip);
 			return 0;
@@ -445,7 +445,7 @@ int snd_usb_mixer_set_ctl_value(struct usb_mixer_elem_info *cval,
 				    usb_sndctrlpipe(chip->dev, 0), request,
 				    USB_RECIP_INTERFACE | USB_TYPE_CLASS | USB_DIR_OUT,
 				    validx, snd_usb_ctrl_intf(chip) | (cval->id << 8),
-				    buf, val_len, 100) >= 0) {
+				    buf, val_len, 1000) >= 0) {
 			snd_usb_autosuspend(chip);
 			return 0;
 		}
diff --git a/sound/usb/mixer_quirks.c b/sound/usb/mixer_quirks.c
index 3d0f487..7c41399 100644
--- a/sound/usb/mixer_quirks.c
+++ b/sound/usb/mixer_quirks.c
@@ -190,18 +190,18 @@ static int snd_audigy2nx_led_put(struct snd_kcontrol *kcontrol, struct snd_ctl_e
 		err = snd_usb_ctl_msg(mixer->chip->dev,
 			      usb_sndctrlpipe(mixer->chip->dev, 0), 0x24,
 			      USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_OTHER,
-			      !value, 0, NULL, 0, 100);
+			      !value, 0, NULL, 0, 1000);
 	/* USB X-Fi S51 Pro */
 	if (mixer->chip->usb_id == USB_ID(0x041e, 0x30df))
 		err = snd_usb_ctl_msg(mixer->chip->dev,
 			      usb_sndctrlpipe(mixer->chip->dev, 0), 0x24,
 			      USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_OTHER,
-			      !value, 0, NULL, 0, 100);
+			      !value, 0, NULL, 0, 1000);
 	else
 		err = snd_usb_ctl_msg(mixer->chip->dev,
 			      usb_sndctrlpipe(mixer->chip->dev, 0), 0x24,
 			      USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_OTHER,
-			      value, index + 2, NULL, 0, 100);
+			      value, index + 2, NULL, 0, 1000);
 	if (err < 0)
 		return err;
 	mixer->audigy2nx_leds[index] = value;
@@ -299,7 +299,7 @@ static void snd_audigy2nx_proc_read(struct snd_info_entry *entry,
 				      usb_rcvctrlpipe(mixer->chip->dev, 0),
 				      UAC_GET_MEM, USB_DIR_IN | USB_TYPE_CLASS |
 				      USB_RECIP_INTERFACE, 0,
-				      jacks[i].unitid << 8, buf, 3, 100);
+				      jacks[i].unitid << 8, buf, 3, 1000);
 		if (err == 3 && (buf[0] == 3 || buf[0] == 6))
 			snd_iprintf(buffer, "%02x %02x\n", buf[1], buf[2]);
 		else
@@ -332,7 +332,7 @@ static int snd_xonar_u1_switch_put(struct snd_kcontrol *kcontrol,
 	err = snd_usb_ctl_msg(mixer->chip->dev,
 			      usb_sndctrlpipe(mixer->chip->dev, 0), 0x08,
 			      USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_OTHER,
-			      50, 0, &new_status, 1, 100);
+			      50, 0, &new_status, 1, 1000);
 	if (err < 0)
 		return err;
 	mixer->xonar_u1_status = new_status;
-- 
1.7.6.396.ge0613

Re: [PATCH v2] sound: usb: mixer: increase control transfer timeout

[Jassi Brar]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=UTF-8, Size: 5261 bytes --]

On Mon, Aug 29, 2011 at 6:01 PM, Felipe Balbi <balbi@ti.com> wrote:
> 100ms might not be enough for slower devices. Increasing
> timeout to 1000ms doesn't seem like overkill.
>
> Signed-off-by: Felipe Balbi <balbi@ti.com>
> ---
>
> Changes since v1:
>  . Increase all 100ms timeout to 1000ms
>
>  sound/usb/mixer.c        |    4 ++--
>  sound/usb/mixer_quirks.c |   10 +++++-----
>  2 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c
> index cdd19d7..1c9d443 100644
> --- a/sound/usb/mixer.c
> +++ b/sound/usb/mixer.c
> @@ -296,7 +296,7 @@ static int get_ctl_value_v1(struct usb_mixer_elem_info *cval, int request, int v
>                if (snd_usb_ctl_msg(chip->dev, usb_rcvctrlpipe(chip->dev, 0), request,
>                                    USB_RECIP_INTERFACE | USB_TYPE_CLASS | USB_DIR_IN,
>                                    validx, snd_usb_ctrl_intf(chip) | (cval->id << 8),
> -                                   buf, val_len, 100) >= val_len) {
> +                                   buf, val_len, 1000) >= val_len) {
>                        *value_ret = convert_signed_value(cval, snd_usb_combine_bytes(buf, val_len));
>                        snd_usb_autosuspend(cval->mixer->chip);
>                        return 0;
> @@ -445,7 +445,7 @@ int snd_usb_mixer_set_ctl_value(struct usb_mixer_elem_info *cval,
>                                    usb_sndctrlpipe(chip->dev, 0), request,
>                                    USB_RECIP_INTERFACE | USB_TYPE_CLASS | USB_DIR_OUT,
>                                    validx, snd_usb_ctrl_intf(chip) | (cval->id << 8),
> -                                   buf, val_len, 100) >= 0) {
> +                                   buf, val_len, 1000) >= 0) {
>                        snd_usb_autosuspend(chip);
>                        return 0;
>                }
> diff --git a/sound/usb/mixer_quirks.c b/sound/usb/mixer_quirks.c
> index 3d0f487..7c41399 100644
> --- a/sound/usb/mixer_quirks.c
> +++ b/sound/usb/mixer_quirks.c
> @@ -190,18 +190,18 @@ static int snd_audigy2nx_led_put(struct snd_kcontrol *kcontrol, struct snd_ctl_e
>                err = snd_usb_ctl_msg(mixer->chip->dev,
>                              usb_sndctrlpipe(mixer->chip->dev, 0), 0x24,
>                              USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_OTHER,
> -                             !value, 0, NULL, 0, 100);
> +                             !value, 0, NULL, 0, 1000);
>        /* USB X-Fi S51 Pro */
>        if (mixer->chip->usb_id == USB_ID(0x041e, 0x30df))
>                err = snd_usb_ctl_msg(mixer->chip->dev,
>                              usb_sndctrlpipe(mixer->chip->dev, 0), 0x24,
>                              USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_OTHER,
> -                             !value, 0, NULL, 0, 100);
> +                             !value, 0, NULL, 0, 1000);
>        else
>                err = snd_usb_ctl_msg(mixer->chip->dev,
>                              usb_sndctrlpipe(mixer->chip->dev, 0), 0x24,
>                              USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_OTHER,
> -                             value, index + 2, NULL, 0, 100);
> +                             value, index + 2, NULL, 0, 1000);
>        if (err < 0)
>                return err;
>        mixer->audigy2nx_leds[index] = value;
> @@ -299,7 +299,7 @@ static void snd_audigy2nx_proc_read(struct snd_info_entry *entry,
>                                      usb_rcvctrlpipe(mixer->chip->dev, 0),
>                                      UAC_GET_MEM, USB_DIR_IN | USB_TYPE_CLASS |
>                                      USB_RECIP_INTERFACE, 0,
> -                                     jacks[i].unitid << 8, buf, 3, 100);
> +                                     jacks[i].unitid << 8, buf, 3, 1000);
>                if (err == 3 && (buf[0] == 3 || buf[0] == 6))
>                        snd_iprintf(buffer, "%02x %02x\n", buf[1], buf[2]);
>                else
> @@ -332,7 +332,7 @@ static int snd_xonar_u1_switch_put(struct snd_kcontrol *kcontrol,
>        err = snd_usb_ctl_msg(mixer->chip->dev,
>                              usb_sndctrlpipe(mixer->chip->dev, 0), 0x08,
>                              USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_OTHER,
> -                             50, 0, &new_status, 1, 100);
> +                             50, 0, &new_status, 1, 1000);

How about also converting to use a local define for timeout value ?
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: [PATCH v2] sound: usb: mixer: increase control transfer timeout

[Felipe Balbi]

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

Hi,

On Mon, Aug 29, 2011 at 06:07:30PM +0530, Jassi Brar wrote:
> On Mon, Aug 29, 2011 at 6:01 PM, Felipe Balbi <balbi@ti.com> wrote:
> > 100ms might not be enough for slower devices. Increasing
> > timeout to 1000ms doesn't seem like overkill.
> >
> > Signed-off-by: Felipe Balbi <balbi@ti.com>
> > ---
> >
> > Changes since v1:
> >  . Increase all 100ms timeout to 1000ms
> >
> >  sound/usb/mixer.c        |    4 ++--
> >  sound/usb/mixer_quirks.c |   10 +++++-----
> >  2 files changed, 7 insertions(+), 7 deletions(-)
> >
> > diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c
> > index cdd19d7..1c9d443 100644
> > --- a/sound/usb/mixer.c
> > +++ b/sound/usb/mixer.c
> > @@ -296,7 +296,7 @@ static int get_ctl_value_v1(struct usb_mixer_elem_info *cval, int request, int v
> >                if (snd_usb_ctl_msg(chip->dev, usb_rcvctrlpipe(chip->dev, 0), request,
> >                                    USB_RECIP_INTERFACE | USB_TYPE_CLASS | USB_DIR_IN,
> >                                    validx, snd_usb_ctrl_intf(chip) | (cval->id << 8),
> > -                                   buf, val_len, 100) >= val_len) {
> > +                                   buf, val_len, 1000) >= val_len) {
> >                        *value_ret = convert_signed_value(cval, snd_usb_combine_bytes(buf, val_len));
> >                        snd_usb_autosuspend(cval->mixer->chip);
> >                        return 0;
> > @@ -445,7 +445,7 @@ int snd_usb_mixer_set_ctl_value(struct usb_mixer_elem_info *cval,
> >                                    usb_sndctrlpipe(chip->dev, 0), request,
> >                                    USB_RECIP_INTERFACE | USB_TYPE_CLASS | USB_DIR_OUT,
> >                                    validx, snd_usb_ctrl_intf(chip) | (cval->id << 8),
> > -                                   buf, val_len, 100) >= 0) {
> > +                                   buf, val_len, 1000) >= 0) {
> >                        snd_usb_autosuspend(chip);
> >                        return 0;
> >                }
> > diff --git a/sound/usb/mixer_quirks.c b/sound/usb/mixer_quirks.c
> > index 3d0f487..7c41399 100644
> > --- a/sound/usb/mixer_quirks.c
> > +++ b/sound/usb/mixer_quirks.c
> > @@ -190,18 +190,18 @@ static int snd_audigy2nx_led_put(struct snd_kcontrol *kcontrol, struct snd_ctl_e
> >                err = snd_usb_ctl_msg(mixer->chip->dev,
> >                              usb_sndctrlpipe(mixer->chip->dev, 0), 0x24,
> >                              USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_OTHER,
> > -                             !value, 0, NULL, 0, 100);
> > +                             !value, 0, NULL, 0, 1000);
> >        /* USB X-Fi S51 Pro */
> >        if (mixer->chip->usb_id == USB_ID(0x041e, 0x30df))
> >                err = snd_usb_ctl_msg(mixer->chip->dev,
> >                              usb_sndctrlpipe(mixer->chip->dev, 0), 0x24,
> >                              USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_OTHER,
> > -                             !value, 0, NULL, 0, 100);
> > +                             !value, 0, NULL, 0, 1000);
> >        else
> >                err = snd_usb_ctl_msg(mixer->chip->dev,
> >                              usb_sndctrlpipe(mixer->chip->dev, 0), 0x24,
> >                              USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_OTHER,
> > -                             value, index + 2, NULL, 0, 100);
> > +                             value, index + 2, NULL, 0, 1000);
> >        if (err < 0)
> >                return err;
> >        mixer->audigy2nx_leds[index] = value;
> > @@ -299,7 +299,7 @@ static void snd_audigy2nx_proc_read(struct snd_info_entry *entry,
> >                                      usb_rcvctrlpipe(mixer->chip->dev, 0),
> >                                      UAC_GET_MEM, USB_DIR_IN | USB_TYPE_CLASS |
> >                                      USB_RECIP_INTERFACE, 0,
> > -                                     jacks[i].unitid << 8, buf, 3, 100);
> > +                                     jacks[i].unitid << 8, buf, 3, 1000);
> >                if (err == 3 && (buf[0] == 3 || buf[0] == 6))
> >                        snd_iprintf(buffer, "%02x %02x\n", buf[1], buf[2]);
> >                else
> > @@ -332,7 +332,7 @@ static int snd_xonar_u1_switch_put(struct snd_kcontrol *kcontrol,
> >        err = snd_usb_ctl_msg(mixer->chip->dev,
> >                              usb_sndctrlpipe(mixer->chip->dev, 0), 0x08,
> >                              USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_OTHER,
> > -                             50, 0, &new_status, 1, 100);
> > +                             50, 0, &new_status, 1, 1000);
> 
> How about also converting to use a local define for timeout value ?

should it be part of another patch ? The current code has no defines at
all and $SUBJECT is just changing what's currently there to another
value.

-- 
balbi

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

Re: [PATCH v2] sound: usb: mixer: increase control transfer timeout

[Jassi Brar]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=UTF-8, Size: 5921 bytes --]

On Mon, Aug 29, 2011 at 6:30 PM, Felipe Balbi <balbi@ti.com> wrote:
> Hi,
>
> On Mon, Aug 29, 2011 at 06:07:30PM +0530, Jassi Brar wrote:
>> On Mon, Aug 29, 2011 at 6:01 PM, Felipe Balbi <balbi@ti.com> wrote:
>> > 100ms might not be enough for slower devices. Increasing
>> > timeout to 1000ms doesn't seem like overkill.
>> >
>> > Signed-off-by: Felipe Balbi <balbi@ti.com>
>> > ---
>> >
>> > Changes since v1:
>> >  . Increase all 100ms timeout to 1000ms
>> >
>> >  sound/usb/mixer.c        |    4 ++--
>> >  sound/usb/mixer_quirks.c |   10 +++++-----
>> >  2 files changed, 7 insertions(+), 7 deletions(-)
>> >
>> > diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c
>> > index cdd19d7..1c9d443 100644
>> > --- a/sound/usb/mixer.c
>> > +++ b/sound/usb/mixer.c
>> > @@ -296,7 +296,7 @@ static int get_ctl_value_v1(struct usb_mixer_elem_info *cval, int request, int v
>> >                if (snd_usb_ctl_msg(chip->dev, usb_rcvctrlpipe(chip->dev, 0), request,
>> >                                    USB_RECIP_INTERFACE | USB_TYPE_CLASS | USB_DIR_IN,
>> >                                    validx, snd_usb_ctrl_intf(chip) | (cval->id << 8),
>> > -                                   buf, val_len, 100) >= val_len) {
>> > +                                   buf, val_len, 1000) >= val_len) {
>> >                        *value_ret = convert_signed_value(cval, snd_usb_combine_bytes(buf, val_len));
>> >                        snd_usb_autosuspend(cval->mixer->chip);
>> >                        return 0;
>> > @@ -445,7 +445,7 @@ int snd_usb_mixer_set_ctl_value(struct usb_mixer_elem_info *cval,
>> >                                    usb_sndctrlpipe(chip->dev, 0), request,
>> >                                    USB_RECIP_INTERFACE | USB_TYPE_CLASS | USB_DIR_OUT,
>> >                                    validx, snd_usb_ctrl_intf(chip) | (cval->id << 8),
>> > -                                   buf, val_len, 100) >= 0) {
>> > +                                   buf, val_len, 1000) >= 0) {
>> >                        snd_usb_autosuspend(chip);
>> >                        return 0;
>> >                }
>> > diff --git a/sound/usb/mixer_quirks.c b/sound/usb/mixer_quirks.c
>> > index 3d0f487..7c41399 100644
>> > --- a/sound/usb/mixer_quirks.c
>> > +++ b/sound/usb/mixer_quirks.c
>> > @@ -190,18 +190,18 @@ static int snd_audigy2nx_led_put(struct snd_kcontrol *kcontrol, struct snd_ctl_e
>> >                err = snd_usb_ctl_msg(mixer->chip->dev,
>> >                              usb_sndctrlpipe(mixer->chip->dev, 0), 0x24,
>> >                              USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_OTHER,
>> > -                             !value, 0, NULL, 0, 100);
>> > +                             !value, 0, NULL, 0, 1000);
>> >        /* USB X-Fi S51 Pro */
>> >        if (mixer->chip->usb_id == USB_ID(0x041e, 0x30df))
>> >                err = snd_usb_ctl_msg(mixer->chip->dev,
>> >                              usb_sndctrlpipe(mixer->chip->dev, 0), 0x24,
>> >                              USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_OTHER,
>> > -                             !value, 0, NULL, 0, 100);
>> > +                             !value, 0, NULL, 0, 1000);
>> >        else
>> >                err = snd_usb_ctl_msg(mixer->chip->dev,
>> >                              usb_sndctrlpipe(mixer->chip->dev, 0), 0x24,
>> >                              USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_OTHER,
>> > -                             value, index + 2, NULL, 0, 100);
>> > +                             value, index + 2, NULL, 0, 1000);
>> >        if (err < 0)
>> >                return err;
>> >        mixer->audigy2nx_leds[index] = value;
>> > @@ -299,7 +299,7 @@ static void snd_audigy2nx_proc_read(struct snd_info_entry *entry,
>> >                                      usb_rcvctrlpipe(mixer->chip->dev, 0),
>> >                                      UAC_GET_MEM, USB_DIR_IN | USB_TYPE_CLASS |
>> >                                      USB_RECIP_INTERFACE, 0,
>> > -                                     jacks[i].unitid << 8, buf, 3, 100);
>> > +                                     jacks[i].unitid << 8, buf, 3, 1000);
>> >                if (err == 3 && (buf[0] == 3 || buf[0] == 6))
>> >                        snd_iprintf(buffer, "%02x %02x\n", buf[1], buf[2]);
>> >                else
>> > @@ -332,7 +332,7 @@ static int snd_xonar_u1_switch_put(struct snd_kcontrol *kcontrol,
>> >        err = snd_usb_ctl_msg(mixer->chip->dev,
>> >                              usb_sndctrlpipe(mixer->chip->dev, 0), 0x08,
>> >                              USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_OTHER,
>> > -                             50, 0, &new_status, 1, 100);
>> > +                             50, 0, &new_status, 1, 1000);
>>
>> How about also converting to use a local define for timeout value ?
>
> should it be part of another patch ? The current code has no defines at
> all and $SUBJECT is just changing what's currently there to another
> value.

Can't speak for maintainers, but personally I'd prefer a single patch with two
points in changelog. The change is trivial enough.
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: [PATCH v2] sound: usb: mixer: increase control transfer timeout

[Clemens Ladisch]

Jassi Brar wrote:
> On Mon, Aug 29, 2011 at 6:01 PM, Felipe Balbi <balbi@ti.com> wrote:
> > 100ms might not be enough for slower devices. Increasing
> > timeout to 1000ms doesn't seem like overkill.
> >
> > Signed-off-by: Felipe Balbi <balbi@ti.com>
> > ---
> >
> > Changes since v1:
> >  . Increase all 100ms timeout to 1000ms
> >
> >  sound/usb/mixer.c        |    4 ++--
> >  sound/usb/mixer_quirks.c |   10 +++++-----
> >  2 files changed, 7 insertions(+), 7 deletions(-)

The calls in mixer_quirks.c are device-specific and known to work, so
none of them actually require a longer timeout. However, increasing
their timeout doesn't hurt either, and ...

> > +                                   buf, val_len, 1000) >= val_len) {
> > +                                   buf, val_len, 1000) >= 0) {
> > +                             !value, 0, NULL, 0, 1000);
> > +                             !value, 0, NULL, 0, 1000);
> > +                             value, index + 2, NULL, 0, 1000);
> > +                                     jacks[i].unitid << 8, buf, 3, 1000);
> > +                             50, 0, &new_status, 1, 1000);
> 
> How about also converting to use a local define for timeout value ?

... now that _all_ calls to snd_usb_ctl_msg use the same timeout value,
we could just as well drop this parameter.


Regards,
Clemens

Re: [PATCH v2] sound: usb: mixer: increase control transfer timeout

[Jassi Brar]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=UTF-8, Size: 1087 bytes --]

On Mon, Aug 29, 2011 at 7:35 PM, Clemens Ladisch <clemens@ladisch.de> wrote:

>> > +                                   buf, val_len, 1000) >= val_len) {
>> > +                                   buf, val_len, 1000) >= 0) {
>> > +                             !value, 0, NULL, 0, 1000);
>> > +                             !value, 0, NULL, 0, 1000);
>> > +                             value, index + 2, NULL, 0, 1000);
>> > +                                     jacks[i].unitid << 8, buf, 3, 1000);
>> > +                             50, 0, &new_status, 1, 1000);
>>
>> How about also converting to use a local define for timeout value ?
>
> ... now that _all_ calls to snd_usb_ctl_msg use the same timeout value,
> we could just as well drop this parameter.
Sounds better, esp when we have 9 arguments to snd_usb_ctl_msg!
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: [PATCH v2] sound: usb: mixer: increase control transfer timeout

[Takashi Iwai]

At Mon, 29 Aug 2011 19:52:09 +0530,
Jassi Brar wrote:
> 
> On Mon, Aug 29, 2011 at 7:35 PM, Clemens Ladisch <clemens@ladisch.de> wrote:
> 
> >> > +                                   buf, val_len, 1000) >= val_len) {
> >> > +                                   buf, val_len, 1000) >= 0) {
> >> > +                             !value, 0, NULL, 0, 1000);
> >> > +                             !value, 0, NULL, 0, 1000);
> >> > +                             value, index + 2, NULL, 0, 1000);
> >> > +                                     jacks[i].unitid << 8, buf, 3, 1000);
> >> > +                             50, 0, &new_status, 1, 1000);
> >>
> >> How about also converting to use a local define for timeout value ?
> >
> > ... now that _all_ calls to snd_usb_ctl_msg use the same timeout value,
> > we could just as well drop this parameter.
> Sounds better, esp when we have 9 arguments to snd_usb_ctl_msg!

Indeed, I'd like to see the reduced arguments, too.


thanks,

Takashi

Re: [PATCH] sound: usb: mixer: increase control transfer timeout

[Takashi Iwai]

At Mon, 29 Aug 2011 14:10:26 +0300,
Felipe Balbi wrote:
> 
> 100ms might not be enough for slower devices. Increasing
> timeout to 1000ms doesn't seem like overkill.
> 
> Signed-off-by: Felipe Balbi <balbi@ti.com>

Felipe, what is the status of this patch?
Could you resend a revised patch?


thanks,

Takashi

Re: [PATCH] sound: usb: mixer: increase control transfer timeout

[Felipe Balbi]

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

Hi,

On Fri, Sep 16, 2011 at 08:25:47AM +0200, Takashi Iwai wrote:
> At Mon, 29 Aug 2011 14:10:26 +0300,
> Felipe Balbi wrote:
> > 
> > 100ms might not be enough for slower devices. Increasing
> > timeout to 1000ms doesn't seem like overkill.
> > 
> > Signed-off-by: Felipe Balbi <balbi@ti.com>
> 
> Felipe, what is the status of this patch?
> Could you resend a revised patch?

Someone else can take care of it. It was supposed to be a very simple
patch and became something larger which I don't have enough time to
handle right now.

-- 
balbi

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

Re: [PATCH] sound: usb: mixer: increase control transfer timeout

[Clemens Ladisch]

Felipe Balbi wrote:
> On Fri, Sep 16, 2011 at 08:25:47AM +0200, Takashi Iwai wrote:
> > Felipe Balbi wrote:
> > > 100ms might not be enough for slower devices. Increasing
> > > timeout to 1000ms doesn't seem like overkill.
> > > 
> > > Signed-off-by: Felipe Balbi <balbi@ti.com>
> > 
> > Felipe, what is the status of this patch?
> > Could you resend a revised patch?
> 
> Someone else can take care of it. It was supposed to be a very simple
> patch and became something larger which I don't have enough time to
> handle right now.

Done:
http://git.alsa-project.org/?p=alsa-kernel.git;a=commit;h=14d1242f90e0


Regards,
Clemens