diff mbox

[RESEND,1/1] Allow any MIDI endpoint to drive use of interrupt transfer on newer Roland devices

Message ID 561a700d.04dfc20a.4d7e9.5ab3@mx.google.com (mailing list archive)
State New, archived
Headers show

Commit Message

Keith Milner Oct. 11, 2015, 2:19 p.m. UTC
From: "Keith A. Milner" <maillist@superlative.org>

This patch enables interrupt transfer mode for MIDI ports on newer
Boss/Roland devices such as the GT-100/001 which support interrupt
transfer on both IN and OUT MIDI endpoints. Previously this wasn't being
enabled for these devices as the code was specifically looking for the
scenario where the IN endpoint supported interrupt transfer and the OUT
endpoint was bulk transfer. Newer devices support interrupt transfer for
both endpoints.

This has been tested on Boss devices GT-001, BR-80 and JS-8 and Roland
VS-20.

It would benefit from some regresison testing with other devices if
possible.

Signed-off-by: Keith A. Milner <maillist@superlative.org>

Comments

Takashi Iwai Oct. 11, 2015, 4:21 p.m. UTC | #1
On Sun, 11 Oct 2015 16:19:48 +0200,
maillist@superlative.org wrote:
> 
> From: "Keith A. Milner" <maillist@superlative.org>
> 
> This patch enables interrupt transfer mode for MIDI ports on newer
> Boss/Roland devices such as the GT-100/001 which support interrupt
> transfer on both IN and OUT MIDI endpoints. Previously this wasn't being
> enabled for these devices as the code was specifically looking for the
> scenario where the IN endpoint supported interrupt transfer and the OUT
> endpoint was bulk transfer. Newer devices support interrupt transfer for
> both endpoints.
> 
> This has been tested on Boss devices GT-001, BR-80 and JS-8 and Roland
> VS-20.
> 
> It would benefit from some regresison testing with other devices if
> possible.
> 
> Signed-off-by: Keith A. Milner <maillist@superlative.org>

Thanks, now it looks much better.

But the patch didn't apply cleanly to the latest kernel, so I had to
adjust manually.  At the next time, please try to align with the
latest kernel tree.


Takashi

> 
> diff --git a/sound/usb/midi.c b/sound/usb/midi.c
> index b901f46..d50c649 100644
> --- a/sound/usb/midi.c
> +++ b/sound/usb/midi.c
> @@ -1860,9 +1860,14 @@ static void snd_usbmidi_switch_roland_altsetting(struct snd_usb_midi* umidi)
>  
>  	hostif = &intf->altsetting[1];
>  	intfd = get_iface_desc(hostif);
> +       /* If either or both of the endpoints support interrupt transfer,
> +        * then use the alternate setting
> +        */
>  	if (intfd->bNumEndpoints != 2 ||
> -	    (get_endpoint(hostif, 0)->bmAttributes & USB_ENDPOINT_XFERTYPE_MASK) != USB_ENDPOINT_XFER_BULK ||
> -	    (get_endpoint(hostif, 1)->bmAttributes & USB_ENDPOINT_XFERTYPE_MASK) != USB_ENDPOINT_XFER_INT)
> +	     !((get_endpoint(hostif, 0)->bmAttributes
> +			& USB_ENDPOINT_XFERTYPE_MASK) == USB_ENDPOINT_XFER_INT ||
> +	      (get_endpoint(hostif, 1)->bmAttributes
> +			& USB_ENDPOINT_XFERTYPE_MASK) == USB_ENDPOINT_XFER_INT))
>  		return;
>  
>  	snd_printdd(KERN_INFO "switching to altsetting %d with int ep\n",
> -- 
> 1.9.1
>
Keith Milner Oct. 11, 2015, 5:23 p.m. UTC | #2
On Sunday 11 Oct 2015 18:21:48 Takashi Iwai wrote:

> Thanks, now it looks much better.
> 
> But the patch didn't apply cleanly to the latest kernel, so I had to
> adjust manually.  At the next time, please try to align with the
> latest kernel tree.

Thanks.

Which tree should I be using for this? I rebased against the git repository 
described in http://www.alsa-project.org/main/index.php/GIT_Server#Occasional_Developers

Is this not the correct on?

Cheers,

Keith
Takashi Iwai Oct. 11, 2015, 5:44 p.m. UTC | #3
On Sun, 11 Oct 2015 19:23:29 +0200,
Keith A. Milner wrote:
> 
> On Sunday 11 Oct 2015 18:21:48 Takashi Iwai wrote:
> 
> > Thanks, now it looks much better.
> > 
> > But the patch didn't apply cleanly to the latest kernel, so I had to
> > adjust manually.  At the next time, please try to align with the
> > latest kernel tree.
> 
> Thanks.
> 
> Which tree should I be using for this? I rebased against the git repository 
> described in http://www.alsa-project.org/main/index.php/GIT_Server#Occasional_Developers
> 
> Is this not the correct on?

The information looks obsoleted...  For sending a patch, please base
either on the latest Linus tree or my sound git tree for-next branch:
  git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git

In anyway, I already applied your patch, so you don't have to rebase
the patch again.


Takashi
Keith Milner Oct. 11, 2015, 5:48 p.m. UTC | #4
On Sunday 11 Oct 2015 19:44:53 Takashi Iwai wrote:
> 
> The information looks obsoleted...  For sending a patch, please base
> either on the latest Linus tree or my sound git tree for-next branch:
>   git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git

Thanks. This may explain some of the issues I had.

Is there a way to get this documentation updated to reflect the current 
situation?

> In anyway, I already applied your patch, so you don't have to rebase
> the patch again.

OK, I will start with a fresh tree. I have some other small patches I'm 
looking to contribute.

Cheers,

Keith
Takashi Iwai Oct. 11, 2015, 5:50 p.m. UTC | #5
On Sun, 11 Oct 2015 19:48:52 +0200,
Keith A. Milner wrote:
> 
> On Sunday 11 Oct 2015 19:44:53 Takashi Iwai wrote:
> > 
> > The information looks obsoleted...  For sending a patch, please base
> > either on the latest Linus tree or my sound git tree for-next branch:
> >   git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git
> 
> Thanks. This may explain some of the issues I had.
> 
> Is there a way to get this documentation updated to reflect the current 
> situation?

It's a Wiki, so everyone should be able to edit :)

> > In anyway, I already applied your patch, so you don't have to rebase
> > the patch again.
> 
> OK, I will start with a fresh tree. I have some other small patches I'm 
> looking to contribute.

Great, I'm looking forward to seeing them.


Takashi
diff mbox

Patch

diff --git a/sound/usb/midi.c b/sound/usb/midi.c
index b901f46..d50c649 100644
--- a/sound/usb/midi.c
+++ b/sound/usb/midi.c
@@ -1860,9 +1860,14 @@  static void snd_usbmidi_switch_roland_altsetting(struct snd_usb_midi* umidi)
 
 	hostif = &intf->altsetting[1];
 	intfd = get_iface_desc(hostif);
+       /* If either or both of the endpoints support interrupt transfer,
+        * then use the alternate setting
+        */
 	if (intfd->bNumEndpoints != 2 ||
-	    (get_endpoint(hostif, 0)->bmAttributes & USB_ENDPOINT_XFERTYPE_MASK) != USB_ENDPOINT_XFER_BULK ||
-	    (get_endpoint(hostif, 1)->bmAttributes & USB_ENDPOINT_XFERTYPE_MASK) != USB_ENDPOINT_XFER_INT)
+	     !((get_endpoint(hostif, 0)->bmAttributes
+			& USB_ENDPOINT_XFERTYPE_MASK) == USB_ENDPOINT_XFER_INT ||
+	      (get_endpoint(hostif, 1)->bmAttributes
+			& USB_ENDPOINT_XFERTYPE_MASK) == USB_ENDPOINT_XFER_INT))
 		return;
 
 	snd_printdd(KERN_INFO "switching to altsetting %d with int ep\n",