diff mbox series

ALSA: usb-audio: Fix "cannot set freq 48000 to ep 0x3" msgs

Message ID 20230601131116.1014250-1-joakim.tjernlund@infinera.com
State New
Headers
Series ALSA: usb-audio: Fix "cannot set freq 48000 to ep 0x3" msgs |

Commit Message

Joakim Tjernlund June 1, 2023, 1:11 p.m. UTC 
  On some USB speaker devices(Jabra/Logitech) we see above error
msg when connecting device to computer and kernel is probing the device.

Moving the snd_usb_init_sample_rate() to after usb_set_interface() makes
the error go away.

Signed-off-by: Joakim Tjernlund <joakim.tjernlund@infinera.com>
Cc: stable@vger.kernel.org
---
 sound/usb/stream.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Joakim Tjernlund June 1, 2023, 2:28 p.m. UTC | #1 
Adding Michael Ralston <michael@ralston.id.au>

He did have problems with his behringer UMC404HD device when this whole seq. was removed.
Ralston, can you try if the below change affects your device?

 Jocke

On Thu, 2023-06-01 at 15:11 +0200, Joakim Tjernlund wrote:
> On some USB speaker devices(Jabra/Logitech) we see above error
> msg when connecting device to computer and kernel is probing the device.
> 
> Moving the snd_usb_init_sample_rate() to after usb_set_interface() makes
> the error go away.
> 
> Signed-off-by: Joakim Tjernlund <joakim.tjernlund@infinera.com>
> Cc: stable@vger.kernel.org
> ---
>  sound/usb/stream.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/sound/usb/stream.c b/sound/usb/stream.c
> index f10f4e6d3fb8..d9ac8663a48b 100644
> --- a/sound/usb/stream.c
> +++ b/sound/usb/stream.c
> @@ -1226,8 +1226,8 @@ static int __snd_usb_parse_audio_interface(struct snd_usb_audio *chip,
>  		/* try to set the interface... */
>  		usb_set_interface(chip->dev, iface_no, 0);
>  		snd_usb_init_pitch(chip, fp);
> -		snd_usb_init_sample_rate(chip, fp, fp->rate_max);
>  		usb_set_interface(chip->dev, iface_no, altno);
> +		snd_usb_init_sample_rate(chip, fp, fp->rate_max);
>  	}
>  	return 0;
>  }
Takashi Iwai June 5, 2023, 7:39 a.m. UTC | #2 
On Thu, 01 Jun 2023 16:28:05 +0200,
Joakim Tjernlund wrote:
> 
> Adding Michael Ralston <michael@ralston.id.au>
> 
> He did have problems with his behringer UMC404HD device when this whole seq. was removed.
> Ralston, can you try if the below change affects your device?

Sorry for the late reaction, as I've been off in the last weeks.

The code sequence there seems pretty sensitive, and swapping or
dropping the call might break things easily on certain devices,
unfortunately.  So, I guess we can't take the patch as is.  If any, we
need to fiddle the call order depending on the device quirk or such.
I guess we may try to fit with the existing quirk flag.  Let me check
it later.


thanks,

Takashi


>  Jocke
> 
> On Thu, 2023-06-01 at 15:11 +0200, Joakim Tjernlund wrote:
> > On some USB speaker devices(Jabra/Logitech) we see above error
> > msg when connecting device to computer and kernel is probing the device.
> > 
> > Moving the snd_usb_init_sample_rate() to after usb_set_interface() makes
> > the error go away.
> > 
> > Signed-off-by: Joakim Tjernlund <joakim.tjernlund@infinera.com>
> > Cc: stable@vger.kernel.org
> > ---
> >  sound/usb/stream.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/sound/usb/stream.c b/sound/usb/stream.c
> > index f10f4e6d3fb8..d9ac8663a48b 100644
> > --- a/sound/usb/stream.c
> > +++ b/sound/usb/stream.c
> > @@ -1226,8 +1226,8 @@ static int __snd_usb_parse_audio_interface(struct snd_usb_audio *chip,
> >  		/* try to set the interface... */
> >  		usb_set_interface(chip->dev, iface_no, 0);
> >  		snd_usb_init_pitch(chip, fp);
> > -		snd_usb_init_sample_rate(chip, fp, fp->rate_max);
> >  		usb_set_interface(chip->dev, iface_no, altno);
> > +		snd_usb_init_sample_rate(chip, fp, fp->rate_max);
> >  	}
> >  	return 0;
> >  }
>
Joakim Tjernlund June 5, 2023, 1:21 p.m. UTC | #3 
On Mon, 2023-06-05 at 09:39 +0200, Takashi Iwai wrote:
> On Thu, 01 Jun 2023 16:28:05 +0200,
> Joakim Tjernlund wrote:
> > 
> > Adding Michael Ralston <michael@ralston.id.au>
> > 
> > He did have problems with his behringer UMC404HD device when this whole seq. was removed.
> > Ralston, can you try if the below change affects your device?
> 
> Sorry for the late reaction, as I've been off in the last weeks.

NP.

> 
> The code sequence there seems pretty sensitive, and swapping or
> dropping the call might break things easily on certain devices,
> unfortunately.  So, I guess we can't take the patch as is.  If any, we
> need to fiddle the call order depending on the device quirk or such.
> I guess we may try to fit with the existing quirk flag.  Let me check
> it later.

yes, this is tricky. To me it look like this code will always set 48000 sample rate and
not all devices support that on all EPs.
Maybe one can get away with not setting sample rate at all? Just guessing though.

 Jocke

> 
> 
> thanks,
> 
> Takashi
> 
> 
> >  Jocke
> > 
> > On Thu, 2023-06-01 at 15:11 +0200, Joakim Tjernlund wrote:
> > > On some USB speaker devices(Jabra/Logitech) we see above error
> > > msg when connecting device to computer and kernel is probing the device.
> > > 
> > > Moving the snd_usb_init_sample_rate() to after usb_set_interface() makes
> > > the error go away.
> > > 
> > > Signed-off-by: Joakim Tjernlund <joakim.tjernlund@infinera.com>
> > > Cc: stable@vger.kernel.org
> > > ---
> > >  sound/usb/stream.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/sound/usb/stream.c b/sound/usb/stream.c
> > > index f10f4e6d3fb8..d9ac8663a48b 100644
> > > --- a/sound/usb/stream.c
> > > +++ b/sound/usb/stream.c
> > > @@ -1226,8 +1226,8 @@ static int __snd_usb_parse_audio_interface(struct snd_usb_audio *chip,
> > >  		/* try to set the interface... */
> > >  		usb_set_interface(chip->dev, iface_no, 0);
> > >  		snd_usb_init_pitch(chip, fp);
> > > -		snd_usb_init_sample_rate(chip, fp, fp->rate_max);
> > >  		usb_set_interface(chip->dev, iface_no, altno);
> > > +		snd_usb_init_sample_rate(chip, fp, fp->rate_max);
> > >  	}
> > >  	return 0;
> > >  }
> >
Takashi Iwai June 5, 2023, 1:43 p.m. UTC | #4 
On Mon, 05 Jun 2023 15:21:51 +0200,
Joakim Tjernlund wrote:
> 
> On Mon, 2023-06-05 at 09:39 +0200, Takashi Iwai wrote:
> > On Thu, 01 Jun 2023 16:28:05 +0200,
> > Joakim Tjernlund wrote:
> > > 
> > > Adding Michael Ralston <michael@ralston.id.au>
> > > 
> > > He did have problems with his behringer UMC404HD device when this whole seq. was removed.
> > > Ralston, can you try if the below change affects your device?
> > 
> > Sorry for the late reaction, as I've been off in the last weeks.
> 
> NP.
> 
> > 
> > The code sequence there seems pretty sensitive, and swapping or
> > dropping the call might break things easily on certain devices,
> > unfortunately.  So, I guess we can't take the patch as is.  If any, we
> > need to fiddle the call order depending on the device quirk or such.
> > I guess we may try to fit with the existing quirk flag.  Let me check
> > it later.
> 
> yes, this is tricky. To me it look like this code will always set 48000 sample rate and
> not all devices support that on all EPs.
> Maybe one can get away with not setting sample rate at all? Just guessing though.

I guess it should work without setting the rate explicitly, but as
already mentioned, some devices (firmware) might need it.  Who knows.

The vendors apparently test only with Windows and/or MacOS, and
issuing unexpected commands (or missing the expected command) can
screw up firmware...


Takashi

> 
>  Jocke
> 
> > 
> > 
> > thanks,
> > 
> > Takashi
> > 
> > 
> > >  Jocke
> > > 
> > > On Thu, 2023-06-01 at 15:11 +0200, Joakim Tjernlund wrote:
> > > > On some USB speaker devices(Jabra/Logitech) we see above error
> > > > msg when connecting device to computer and kernel is probing the device.
> > > > 
> > > > Moving the snd_usb_init_sample_rate() to after usb_set_interface() makes
> > > > the error go away.
> > > > 
> > > > Signed-off-by: Joakim Tjernlund <joakim.tjernlund@infinera.com>
> > > > Cc: stable@vger.kernel.org
> > > > ---
> > > >  sound/usb/stream.c | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > 
> > > > diff --git a/sound/usb/stream.c b/sound/usb/stream.c
> > > > index f10f4e6d3fb8..d9ac8663a48b 100644
> > > > --- a/sound/usb/stream.c
> > > > +++ b/sound/usb/stream.c
> > > > @@ -1226,8 +1226,8 @@ static int __snd_usb_parse_audio_interface(struct snd_usb_audio *chip,
> > > >  		/* try to set the interface... */
> > > >  		usb_set_interface(chip->dev, iface_no, 0);
> > > >  		snd_usb_init_pitch(chip, fp);
> > > > -		snd_usb_init_sample_rate(chip, fp, fp->rate_max);
> > > >  		usb_set_interface(chip->dev, iface_no, altno);
> > > > +		snd_usb_init_sample_rate(chip, fp, fp->rate_max);
> > > >  	}
> > > >  	return 0;
> > > >  }
> > > 
>
diff mbox series

Patch

diff --git a/sound/usb/stream.c b/sound/usb/stream.c
index f10f4e6d3fb8..d9ac8663a48b 100644
--- a/sound/usb/stream.c
+++ b/sound/usb/stream.c
@@ -1226,8 +1226,8 @@  static int __snd_usb_parse_audio_interface(struct snd_usb_audio *chip,
 		/* try to set the interface... */
 		usb_set_interface(chip->dev, iface_no, 0);
 		snd_usb_init_pitch(chip, fp);
-		snd_usb_init_sample_rate(chip, fp, fp->rate_max);
 		usb_set_interface(chip->dev, iface_no, altno);
+		snd_usb_init_sample_rate(chip, fp, fp->rate_max);
 	}
 	return 0;
 }