diff mbox series

[1/3] ALSA: bebob: expand sleep just after breaking connections for protocol version 1

Message ID 20191101131323.17300-2-o-takashi@sakamocchi.jp (mailing list archive)
State New, archived
Headers show
Series ALSA: bebob: misc improvements for starting/stopping packet streaming | expand

Commit Message

Takashi Sakamoto Nov. 1, 2019, 1:13 p.m. UTC 
As long as I investigated, some devices with BeBoB protocol version 1
can be freezed during several hundreds milliseconds after breaking
connections. When accessing during the freezed time, any transaction
is corrupted. In the worst case, the device is going to reboot.

I can see this issue in:
 * Roland FA-66
 * M-Audio FireWire Solo

This commit expands sleep just after breaking connections to avoid
the freezed time as much as possible. I note that the freeze/reboot
behaviour is similar to below models:
 * Focusrite Saffire Pro 10 I/O
 * Focusrite Saffire Pro 26 I/O

The above models certainly reboot after breaking connections.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/bebob/bebob_stream.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

Comments

Steve Morris March 26, 2020, 9:47 p.m. UTC | #1 
On Fri, Nov 01, 2019 at 10:13:21PM +0900, Takashi Sakamoto wrote:
> As long as I investigated, some devices with BeBoB protocol version 1
> can be freezed during several hundreds milliseconds after breaking
> connections. When accessing during the freezed time, any transaction
> is corrupted. In the worst case, the device is going to reboot.
> 
> I can see this issue in:
>  * Roland FA-66
>  * M-Audio FireWire Solo
> 
> This commit expands sleep just after breaking connections to avoid
> the freezed time as much as possible. I note that the freeze/reboot
> behaviour is similar to below models:
>  * Focusrite Saffire Pro 10 I/O
>  * Focusrite Saffire Pro 26 I/O
> 
> The above models certainly reboot after breaking connections.
> 
> Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
> ---
>  sound/firewire/bebob/bebob_stream.c | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/sound/firewire/bebob/bebob_stream.c b/sound/firewire/bebob/bebob_stream.c
> index f7f0db5aa811..1b264d0d63eb 100644
> --- a/sound/firewire/bebob/bebob_stream.c
> +++ b/sound/firewire/bebob/bebob_stream.c
> @@ -415,15 +415,16 @@ static int make_both_connections(struct snd_bebob *bebob)
>  	return 0;
>  }
>  
> -static void
> -break_both_connections(struct snd_bebob *bebob)
> +static void break_both_connections(struct snd_bebob *bebob)
>  {
>  	cmp_connection_break(&bebob->in_conn);
>  	cmp_connection_break(&bebob->out_conn);
>  
> -	/* These models seems to be in transition state for a longer time. */
> -	if (bebob->maudio_special_quirk != NULL)
> -		msleep(200);
> +	// These models seem to be in transition state for a longer time. When
> +	// accessing in the state, any transactions is corrupted. In the worst
> +	// case, the device is going to reboot.
> +	if (bebob->version < 2)
> +		msleep(600);
>  }
>  
>  static int
> -- 
> 2.20.1
> 
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

This patch causes a reboot when I power on my Edirol FA-101. This is
consistent behavior with kernels 5.5.1 - 5.5.11.

I've changed msleep(600) back to msleep(200) and rebuilt the module and
I am once again able to power on and use the interface.

I am running Arch linux with 5.5.11-arch1-1 #1 SMP PREEMPT Sun, 22 Mar 2020 16:33:15 +0000 x86_64 GNU/Linux.


I had previously posted to the list on Feb 19, 2020 with the subject:

sytem reboots when initializing Edirol FA-101 firewire audio interface

Please forgive any lapses in reporting protocol as I generally don't
work at the kernel level. Hopefully this more specific report will help
fix the issue.

Thanks, 

Steve
Takashi Iwai March 27, 2020, 8:40 a.m. UTC | #2 
On Thu, 26 Mar 2020 22:47:22 +0100,
Steve Morris wrote:
> 
> On Fri, Nov 01, 2019 at 10:13:21PM +0900, Takashi Sakamoto wrote:
> > As long as I investigated, some devices with BeBoB protocol version 1
> > can be freezed during several hundreds milliseconds after breaking
> > connections. When accessing during the freezed time, any transaction
> > is corrupted. In the worst case, the device is going to reboot.
> > 
> > I can see this issue in:
> >  * Roland FA-66
> >  * M-Audio FireWire Solo
> > 
> > This commit expands sleep just after breaking connections to avoid
> > the freezed time as much as possible. I note that the freeze/reboot
> > behaviour is similar to below models:
> >  * Focusrite Saffire Pro 10 I/O
> >  * Focusrite Saffire Pro 26 I/O
> > 
> > The above models certainly reboot after breaking connections.
> > 
> > Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
> > ---
> >  sound/firewire/bebob/bebob_stream.c | 11 ++++++-----
> >  1 file changed, 6 insertions(+), 5 deletions(-)
> > 
> > diff --git a/sound/firewire/bebob/bebob_stream.c b/sound/firewire/bebob/bebob_stream.c
> > index f7f0db5aa811..1b264d0d63eb 100644
> > --- a/sound/firewire/bebob/bebob_stream.c
> > +++ b/sound/firewire/bebob/bebob_stream.c
> > @@ -415,15 +415,16 @@ static int make_both_connections(struct snd_bebob *bebob)
> >  	return 0;
> >  }
> >  
> > -static void
> > -break_both_connections(struct snd_bebob *bebob)
> > +static void break_both_connections(struct snd_bebob *bebob)
> >  {
> >  	cmp_connection_break(&bebob->in_conn);
> >  	cmp_connection_break(&bebob->out_conn);
> >  
> > -	/* These models seems to be in transition state for a longer time. */
> > -	if (bebob->maudio_special_quirk != NULL)
> > -		msleep(200);
> > +	// These models seem to be in transition state for a longer time. When
> > +	// accessing in the state, any transactions is corrupted. In the worst
> > +	// case, the device is going to reboot.
> > +	if (bebob->version < 2)
> > +		msleep(600);
> >  }
> >  
> >  static int
> > -- 
> > 2.20.1
> > 
> > _______________________________________________
> > Alsa-devel mailing list
> > Alsa-devel@alsa-project.org
> > https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
> 
> This patch causes a reboot when I power on my Edirol FA-101. This is
> consistent behavior with kernels 5.5.1 - 5.5.11.
> 
> I've changed msleep(600) back to msleep(200) and rebuilt the module and
> I am once again able to power on and use the interface.
> 
> I am running Arch linux with 5.5.11-arch1-1 #1 SMP PREEMPT Sun, 22 Mar 2020 16:33:15 +0000 x86_64 GNU/Linux.
> 
> 
> I had previously posted to the list on Feb 19, 2020 with the subject:
> 
> sytem reboots when initializing Edirol FA-101 firewire audio interface
> 
> Please forgive any lapses in reporting protocol as I generally don't
> work at the kernel level. Hopefully this more specific report will help
> fix the issue.

This is weird.  If just changing the msleep value from 600 to 200
"fixes" the problem, it means that something is already fundamentally
broken...


Takashi
Steve Morris March 27, 2020, 7:13 p.m. UTC | #3 
On Fri, Mar 27, 2020 at 09:40:34AM +0100, Takashi Iwai wrote:
> On Thu, 26 Mar 2020 22:47:22 +0100,
> Steve Morris wrote:
> > 
> > On Fri, Nov 01, 2019 at 10:13:21PM +0900, Takashi Sakamoto wrote:
> > > As long as I investigated, some devices with BeBoB protocol version 1
> > > can be freezed during several hundreds milliseconds after breaking
> > > connections. When accessing during the freezed time, any transaction
> > > is corrupted. In the worst case, the device is going to reboot.
> > > 
> > > I can see this issue in:
> > >  * Roland FA-66
> > >  * M-Audio FireWire Solo
> > > 
> > > This commit expands sleep just after breaking connections to avoid
> > > the freezed time as much as possible. I note that the freeze/reboot
> > > behaviour is similar to below models:
> > >  * Focusrite Saffire Pro 10 I/O
> > >  * Focusrite Saffire Pro 26 I/O
> > > 
> > > The above models certainly reboot after breaking connections.
> > > 
> > > Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
> > > ---
> > >  sound/firewire/bebob/bebob_stream.c | 11 ++++++-----
> > >  1 file changed, 6 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/sound/firewire/bebob/bebob_stream.c b/sound/firewire/bebob/bebob_stream.c
> > > index f7f0db5aa811..1b264d0d63eb 100644
> > > --- a/sound/firewire/bebob/bebob_stream.c
> > > +++ b/sound/firewire/bebob/bebob_stream.c
> > > @@ -415,15 +415,16 @@ static int make_both_connections(struct snd_bebob *bebob)
> > >  	return 0;
> > >  }
> > >  
> > > -static void
> > > -break_both_connections(struct snd_bebob *bebob)
> > > +static void break_both_connections(struct snd_bebob *bebob)
> > >  {
> > >  	cmp_connection_break(&bebob->in_conn);
> > >  	cmp_connection_break(&bebob->out_conn);
> > >  
> > > -	/* These models seems to be in transition state for a longer time. */
> > > -	if (bebob->maudio_special_quirk != NULL)
> > > -		msleep(200);
> > > +	// These models seem to be in transition state for a longer time. When
> > > +	// accessing in the state, any transactions is corrupted. In the worst
> > > +	// case, the device is going to reboot.
> > > +	if (bebob->version < 2)
> > > +		msleep(600);
> > >  }
> > >  
> > >  static int
> > > -- 
> > > 2.20.1
> > > 
> > > _______________________________________________
> > > Alsa-devel mailing list
> > > Alsa-devel@alsa-project.org
> > > https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
> > 
> > This patch causes a reboot when I power on my Edirol FA-101. This is
> > consistent behavior with kernels 5.5.1 - 5.5.11.
> > 
> > I've changed msleep(600) back to msleep(200) and rebuilt the module and
> > I am once again able to power on and use the interface.
> > 
> > I am running Arch linux with 5.5.11-arch1-1 #1 SMP PREEMPT Sun, 22 Mar 2020 16:33:15 +0000 x86_64 GNU/Linux.
> > 
> > 
> > I had previously posted to the list on Feb 19, 2020 with the subject:
> > 
> > sytem reboots when initializing Edirol FA-101 firewire audio interface
> > 
> > Please forgive any lapses in reporting protocol as I generally don't
> > work at the kernel level. Hopefully this more specific report will help
> > fix the issue.
> 
> This is weird.  If just changing the msleep value from 600 to 200
> "fixes" the problem, it means that something is already fundamentally
> broken...
> 
> 
> Takashi

It appears that 'fixed' was too strong a word. While I have managed to
get it to work a couple of times, generally it is still broken.

I'm happy to help debug, but this is outside my scope.

Back to the LTS kernel for the moment.

Steve
diff mbox series

Patch

diff --git a/sound/firewire/bebob/bebob_stream.c b/sound/firewire/bebob/bebob_stream.c
index f7f0db5aa811..1b264d0d63eb 100644
--- a/sound/firewire/bebob/bebob_stream.c
+++ b/sound/firewire/bebob/bebob_stream.c
@@ -415,15 +415,16 @@  static int make_both_connections(struct snd_bebob *bebob)
 	return 0;
 }
 
-static void
-break_both_connections(struct snd_bebob *bebob)
+static void break_both_connections(struct snd_bebob *bebob)
 {
 	cmp_connection_break(&bebob->in_conn);
 	cmp_connection_break(&bebob->out_conn);
 
-	/* These models seems to be in transition state for a longer time. */
-	if (bebob->maudio_special_quirk != NULL)
-		msleep(200);
+	// These models seem to be in transition state for a longer time. When
+	// accessing in the state, any transactions is corrupted. In the worst
+	// case, the device is going to reboot.
+	if (bebob->version < 2)
+		msleep(600);
 }
 
 static int