Re: Add quirks to support M-Audio FastTrack Pro (uaudio)

2017-02-07 Thread Martin Pieuchot
On 03/02/17(Fri) 12:06, Christopher Zimmermann wrote:
> [...]
> > > > > @@ -444,6 +447,11 @@ uaudio_match(struct device *parent, void
> > > > >   if (uaa->iface == NULL || uaa->device == NULL)
> > > > >   return (UMATCH_NONE);
> > > > >  
> > > > > + if (uaa->vendor == USB_VENDOR_MAUDIO &&
> > > > > + uaa->product == USB_PRODUCT_MAUDIO_FASTTRACKPRO &&
> > > > > + uaa->configno != 2)
> > > > > + return UMATCH_NONE;
> > > > 
> > > > Why to you need this?  
> > > 
> > > This device exposes only a very limited set of features in
> > > configuration 1.  
> > 
> > But configuration 1 is midi not audio right?  So it won't match, so
> > why do you need that?
> 
> In config 1 the device exposes these ifaces:
> 0 audio control
> 1 midi
> 2 audio play
> 3 audio rec
> 
> in config 2 the device exposes these ifaces:
> 0 audio control
> 1 midi
> 2 audio analog out
> 3 audio analog / digital out
> 4 audio analog in (with mic preamps)
> 5 audio digital in
> 
> if any iface in config 1 is claimed the device cannot switch to config 2.

But do you still want umidi(4) to attach or not at all?

> > > > > + if (uaa->vendor == USB_VENDOR_MAUDIO &&
> > > > > + uaa->product == USB_PRODUCT_MAUDIO_FASTTRACKPRO &&
> > > > > + uaa->configno != 2)
> > > > >   return UMATCH_NONE;
> > > > 
> > > > I'd leave the configno check out and add a comment explaining why
> > > > we want to force this driver to attach to uaudio(4).  
> > > 
> > > See my comment above about the same piece of code in uaudio.c.
> > > If I don't add this condition umidi will attach to configuration 1
> > > and configuration 2 wouldn't be tried, uaudio wouldn't attach.  
> > 
> > Please re-read my comment.  I'm talking about the "configno" check.
> > Think about somebody reading your code in 5 years, it will just looks
> > like black magic.
> 
> What about adding this comment:
> 
> /*
>  * This combined audio / midi device exposes its
>  * full set of features only in configuration 2.
>  */

Or something along the way of:

 /*
  * As long as the USB stack does not support switching between multiple
  * configurations, only match the second one, it offers more features.
  */

> > What you're doing is working around our stupid USB detection mechanism
> > to use a specific configuration.  So you don't want your device to
> > attach to umidi(4) no matter with which configuration.
> > 
> > > + if (p->precision > 8)
> > > + /*
> > > +  * Don't search for matching encoding.
> > > +  * Just tell the upper layers which one we support.
> > > +  */
> > > + p->encoding = sc->sc_alts[i].encoding;  
> > 
> > Why do you need that?
> 
> - The audio device is big endian.
> - The uaudio driver was written for little endian devices.
> - The audio driver assumes _host_ endianness.
> - The uaudio driver will reject AUDIO_SETPAR (audio(4)) requests which don't
>   exactly match the device encoding. 
> - I change uaudio to communicate the supported endianness back to audio(4), so
>   that userspace will receive the supported parameters via AUDIO_GETPAR and
>   will use a supported encoding.

Ok I don't understand this magic, maybe ratchov@ can comment on it?  

> > > + else if (p->encoding != sc->sc_alts[i].encoding)
> > > + continue;
> > > +
> > >   alts_eh |= 1 << i;
> > >   DPRINTFN(6,("%s: matched %s alt %d for enc/pre\n",
> > > __func__, mode == AUMODE_RECORD ? "rec" : "play", i));  
> 
> 
> 
> -- 
> http://gmerlin.de
> OpenPGP: http://gmerlin.de/christopher.pub
> 2779 7F73 44FD 0736 B67A  C410 69EC 7922 34B4 2566




Re: Add quirks to support M-Audio FastTrack Pro (uaudio)

2017-02-03 Thread Christopher Zimmermann
Hi, 

thanks for your patience.
If some things are still unclear I'll be glad to clarify.


Christopher


On 2017-02-02 Martin Pieuchot  wrote:
> On 31/01/17(Tue) 14:05, Christopher Zimmermann wrote:
> > On 2017-01-29 Martin Pieuchot  wrote:  
> > > On 29/01/17(Sun) 19:33, Christopher Zimmermann wrote:  
> > > > [...]
> > > > @@ -444,6 +447,11 @@ uaudio_match(struct device *parent, void
> > > > if (uaa->iface == NULL || uaa->device == NULL)
> > > > return (UMATCH_NONE);
> > > >  
> > > > +   if (uaa->vendor == USB_VENDOR_MAUDIO &&
> > > > +   uaa->product == USB_PRODUCT_MAUDIO_FASTTRACKPRO &&
> > > > +   uaa->configno != 2)
> > > > +   return UMATCH_NONE;
> > > 
> > > Why to you need this?  
> > 
> > This device exposes only a very limited set of features in
> > configuration 1.  
> 
> But configuration 1 is midi not audio right?  So it won't match, so
> why do you need that?

In config 1 the device exposes these ifaces:
0 audio control
1 midi
2 audio play
3 audio rec

in config 2 the device exposes these ifaces:
0 audio control
1 midi
2 audio analog out
3 audio analog / digital out
4 audio analog in (with mic preamps)
5 audio digital in

if any iface in config 1 is claimed the device cannot switch to config 2.

> > > > @@ -3312,6 +3340,9 @@ uaudio_set_params(void *addr, int setmod
> > > > }
> > > > break;
> > > > }
> > > > +
> > > > +   if (sc->sc_quirks & UAUDIO_FLAG_BE)
> > > > +   p->encoding =
> > > > AUDIO_ENCODING_SLINEAR_BE;
> > > 
> > > Why do you need this chunk and we don't need to set
> > > AUDIO_ENCODING_SLINEAR_LE in the other case?  
> > 
> > We probably should set it to _LE in the other case. I moved this
> > piece of code into uaudio_match_alt() and made it agnostic about
> > the specific endianness. 
> > > > @@ -152,6 +153,11 @@ umidi_match(struct device *parent, void 
> > > > DPRINTFN(1,("umidi_match\n"));
> > > >  
> > > > if (uaa->iface == NULL)
> > > > +   return UMATCH_NONE;
> > > > +
> > > > +   if (uaa->vendor == USB_VENDOR_MAUDIO &&
> > > > +   uaa->product == USB_PRODUCT_MAUDIO_FASTTRACKPRO &&
> > > > +   uaa->configno != 2)
> > > > return UMATCH_NONE;
> > > 
> > > I'd leave the configno check out and add a comment explaining why
> > > we want to force this driver to attach to uaudio(4).  
> > 
> > See my comment above about the same piece of code in uaudio.c.
> > If I don't add this condition umidi will attach to configuration 1
> > and configuration 2 wouldn't be tried, uaudio wouldn't attach.  
> 
> Please re-read my comment.  I'm talking about the "configno" check.
> Think about somebody reading your code in 5 years, it will just looks
> like black magic.

What about adding this comment:

/*
 * This combined audio / midi device exposes its
 * full set of features only in configuration 2.
 */

> What you're doing is working around our stupid USB detection mechanism
> to use a specific configuration.  So you don't want your device to
> attach to umidi(4) no matter with which configuration.
> 
> > +   if (p->precision > 8)
> > +   /*
> > +* Don't search for matching encoding.
> > +* Just tell the upper layers which one we support.
> > +*/
> > +   p->encoding = sc->sc_alts[i].encoding;  
> 
> Why do you need that?

- The audio device is big endian.
- The uaudio driver was written for little endian devices.
- The audio driver assumes _host_ endianness.
- The uaudio driver will reject AUDIO_SETPAR (audio(4)) requests which don't
  exactly match the device encoding. 
- I change uaudio to communicate the supported endianness back to audio(4), so
  that userspace will receive the supported parameters via AUDIO_GETPAR and
  will use a supported encoding.

> > +   else if (p->encoding != sc->sc_alts[i].encoding)
> > +   continue;
> > +
> > alts_eh |= 1 << i;
> > DPRINTFN(6,("%s: matched %s alt %d for enc/pre\n",
> > __func__, mode == AUMODE_RECORD ? "rec" : "play", i));  



-- 
http://gmerlin.de
OpenPGP: http://gmerlin.de/christopher.pub
2779 7F73 44FD 0736 B67A  C410 69EC 7922 34B4 2566


pgpwepgVnSMc9.pgp
Description: OpenPGP digital signature


Re: Add quirks to support M-Audio FastTrack Pro (uaudio)

2017-02-02 Thread Martin Pieuchot
On 31/01/17(Tue) 14:05, Christopher Zimmermann wrote:
> On 2017-01-29 Martin Pieuchot  wrote:
> > On 29/01/17(Sun) 19:33, Christopher Zimmermann wrote:
> > > [...]
> > > @@ -444,6 +447,11 @@ uaudio_match(struct device *parent, void
> > >   if (uaa->iface == NULL || uaa->device == NULL)
> > >   return (UMATCH_NONE);
> > >  
> > > + if (uaa->vendor == USB_VENDOR_MAUDIO &&
> > > + uaa->product == USB_PRODUCT_MAUDIO_FASTTRACKPRO &&
> > > + uaa->configno != 2)
> > > + return UMATCH_NONE;  
> > 
> > Why to you need this?
> 
> This device exposes only a very limited set of features in
> configuration 1.

But configuration 1 is midi not audio right?  So it won't match, so why
do you need that?

> > > @@ -3312,6 +3340,9 @@ uaudio_set_params(void *addr, int setmod
> > >   }
> > >   break;
> > >   }
> > > +
> > > + if (sc->sc_quirks & UAUDIO_FLAG_BE)
> > > + p->encoding = AUDIO_ENCODING_SLINEAR_BE;  
> > 
> > Why do you need this chunk and we don't need to set
> > AUDIO_ENCODING_SLINEAR_LE in the other case?
> 
> We probably should set it to _LE in the other case. I moved this piece
> of code into uaudio_match_alt() and made it agnostic about the specific 
> endianness.
> 
> > > @@ -152,6 +153,11 @@ umidi_match(struct device *parent, void 
> > >   DPRINTFN(1,("umidi_match\n"));
> > >  
> > >   if (uaa->iface == NULL)
> > > + return UMATCH_NONE;
> > > +
> > > + if (uaa->vendor == USB_VENDOR_MAUDIO &&
> > > + uaa->product == USB_PRODUCT_MAUDIO_FASTTRACKPRO &&
> > > + uaa->configno != 2)
> > >   return UMATCH_NONE;  
> > 
> > I'd leave the configno check out and add a comment explaining why we
> > want to force this driver to attach to uaudio(4).
> 
> See my comment above about the same piece of code in uaudio.c.
> If I don't add this condition umidi will attach to configuration 1 and
> configuration 2 wouldn't be tried, uaudio wouldn't attach.

Please re-read my comment.  I'm talking about the "configno" check.
Think about somebody reading your code in 5 years, it will just looks
like black magic.

What you're doing is working around our stupid USB detection mechanism
to use a specific configuration.  So you don't want your device to
attach to umidi(4) no matter with which configuration.

> + if (p->precision > 8)
> + /*
> +  * Don't search for matching encoding.
> +  * Just tell the upper layers which one we support.
> +  */
> + p->encoding = sc->sc_alts[i].encoding;

Why do you need that?

> + else if (p->encoding != sc->sc_alts[i].encoding)
> + continue;
> +
>   alts_eh |= 1 << i;
>   DPRINTFN(6,("%s: matched %s alt %d for enc/pre\n", __func__,
>   mode == AUMODE_RECORD ? "rec" : "play", i));



Re: Add quirks to support M-Audio FastTrack Pro (uaudio)

2017-01-31 Thread Christopher Zimmermann
Hi,

below you find an updated diff. OK?


On 2017-01-29 Martin Pieuchot  wrote:
> On 29/01/17(Sun) 19:33, Christopher Zimmermann wrote:
> > [...]
> > @@ -444,6 +447,11 @@ uaudio_match(struct device *parent, void
> > if (uaa->iface == NULL || uaa->device == NULL)
> > return (UMATCH_NONE);
> >  
> > +   if (uaa->vendor == USB_VENDOR_MAUDIO &&
> > +   uaa->product == USB_PRODUCT_MAUDIO_FASTTRACKPRO &&
> > +   uaa->configno != 2)
> > +   return UMATCH_NONE;  
> 
> Why to you need this?

This device exposes only a very limited set of features in
configuration 1.
To force usb_subr.c:usbd_probe_and_attach() to try configuration 2
both uaudio and umidi must attach to the device only configuration 2.
Otherwise one will claim the device and other configurations won't be
tried.

> > @@ -531,8 +539,21 @@ uaudio_attach(struct device *parent, str
> > found = 1;
> > }
> > }
> > -   if (found)
> > +   if (found) {
> > usbd_claim_iface(sc->sc_udev, i);
> > +   if (uaa->vendor == USB_VENDOR_MAUDIO &&
> > +   uaa->product == USB_PRODUCT_MAUDIO_FASTTRACKPRO) {
> > +   /*
> > +* temporarily switch every iface to 24bit.
> > +* Causes the device to be big endian even
> > +* for 16bit samples.
> > +* using 16bits first will cause the device
> > +* to break when we later switch to 24bit.
> > +*/
> > +   usbd_set_interface(sc->sc_alts[i].ifaceh, 2);
> > +   usbd_set_interface(sc->sc_alts[i].ifaceh, 0);  
> 
> How did you figure that out?  Is this behavior documented somehwere?

It's not documented. I did not find a similar hack in the linux driver.
Still its a reproducable behaviour.

> > @@ -3312,6 +3340,9 @@ uaudio_set_params(void *addr, int setmod
> > }
> > break;
> > }
> > +
> > +   if (sc->sc_quirks & UAUDIO_FLAG_BE)
> > +   p->encoding = AUDIO_ENCODING_SLINEAR_BE;  
> 
> Why do you need this chunk and we don't need to set
> AUDIO_ENCODING_SLINEAR_LE in the other case?

We probably should set it to _LE in the other case. I moved this piece
of code into uaudio_match_alt() and made it agnostic about the specific 
endianness.

> > @@ -152,6 +153,11 @@ umidi_match(struct device *parent, void 
> > DPRINTFN(1,("umidi_match\n"));
> >  
> > if (uaa->iface == NULL)
> > +   return UMATCH_NONE;
> > +
> > +   if (uaa->vendor == USB_VENDOR_MAUDIO &&
> > +   uaa->product == USB_PRODUCT_MAUDIO_FASTTRACKPRO &&
> > +   uaa->configno != 2)
> > return UMATCH_NONE;  
> 
> I'd leave the configno check out and add a comment explaining why we
> want to force this driver to attach to uaudio(4).

See my comment above about the same piece of code in uaudio.c.
If I don't add this condition umidi will attach to configuration 1 and
configuration 2 wouldn't be tried, uaudio wouldn't attach.


Here's the updated diff:



Index: uaudio.c
===
RCS file: /cvs/src/sys/dev/usb/uaudio.c,v
retrieving revision 1.122
diff -u -p -r1.122 uaudio.c
--- uaudio.c3 Jan 2017 06:45:58 -   1.122
+++ uaudio.c31 Jan 2017 12:37:46 -
@@ -172,6 +172,7 @@ struct chan {
 #define UAUDIO_FLAG_VENDOR_CLASS 0x0010/* claims vendor class but 
works */
 #define UAUDIO_FLAG_DEPENDENT   0x0020 /* play and record params must equal */
 #define UAUDIO_FLAG_EMU0202 0x0040
+#define UAUDIO_FLAG_BE  0x0080
 
 struct uaudio_devs {
struct usb_devno uv_dev;
@@ -223,7 +224,9 @@ struct uaudio_devs {
{ { USB_VENDOR_LOGITECH, USB_PRODUCT_LOGITECH_QUICKCAMZOOM },
UAUDIO_FLAG_BAD_AUDIO },
{ { USB_VENDOR_TELEX, USB_PRODUCT_TELEX_MIC1 },
-   UAUDIO_FLAG_NO_FRAC }
+   UAUDIO_FLAG_NO_FRAC },
+   { { USB_VENDOR_MIDIMAN, USB_PRODUCT_MIDIMAN_FASTTRACKPRO },
+   UAUDIO_FLAG_BE | UAUDIO_FLAG_DEPENDENT }
 };
 #define uaudio_lookup(v, p) \
((struct uaudio_devs *)usb_lookup(uaudio_devs, v, p))
@@ -444,6 +447,11 @@ uaudio_match(struct device *parent, void
if (uaa->iface == NULL || uaa->device == NULL)
return (UMATCH_NONE);
 
+   if (uaa->vendor == USB_VENDOR_MIDIMAN &&
+   uaa->product == USB_PRODUCT_MIDIMAN_FASTTRACKPRO &&
+   uaa->configno != 2)
+   return UMATCH_NONE;
+
quirk = uaudio_lookup(uaa->vendor, uaa->product);
if (quirk)
flags = quirk->flags;
@@ -531,8 +539,21 @@ uaudio_attach(struct device *parent, str
found = 1;
}
}
-

Re: Add quirks to support M-Audio FastTrack Pro (uaudio)

2017-01-29 Thread Martin Pieuchot
On 29/01/17(Sun) 19:33, Christopher Zimmermann wrote:
> [...]
> Index: uaudio.c
> ===
> RCS file: /cvs/src/sys/dev/usb/uaudio.c,v
> retrieving revision 1.122
> diff -u -p -r1.122 uaudio.c
> --- uaudio.c  3 Jan 2017 06:45:58 -   1.122
> +++ uaudio.c  29 Jan 2017 17:13:28 -
> @@ -172,6 +172,7 @@ struct chan {
>  #define UAUDIO_FLAG_VENDOR_CLASS 0x0010  /* claims vendor class but 
> works */
>  #define UAUDIO_FLAG_DEPENDENT 0x0020 /* play and record params must 
> equal */
>  #define UAUDIO_FLAG_EMU0202   0x0040
> +#define UAUDIO_FLAG_BE0x0080
>  
>  struct uaudio_devs {
>   struct usb_devno uv_dev;
> @@ -223,7 +224,9 @@ struct uaudio_devs {
>   { { USB_VENDOR_LOGITECH, USB_PRODUCT_LOGITECH_QUICKCAMZOOM },
>   UAUDIO_FLAG_BAD_AUDIO },
>   { { USB_VENDOR_TELEX, USB_PRODUCT_TELEX_MIC1 },
> - UAUDIO_FLAG_NO_FRAC }
> + UAUDIO_FLAG_NO_FRAC },
> + { { USB_VENDOR_MAUDIO, USB_PRODUCT_MAUDIO_FASTTRACKPRO },
> + UAUDIO_FLAG_BE | UAUDIO_FLAG_DEPENDENT }
>  };
>  #define uaudio_lookup(v, p) \
>   ((struct uaudio_devs *)usb_lookup(uaudio_devs, v, p))
> @@ -444,6 +447,11 @@ uaudio_match(struct device *parent, void
>   if (uaa->iface == NULL || uaa->device == NULL)
>   return (UMATCH_NONE);
>  
> + if (uaa->vendor == USB_VENDOR_MAUDIO &&
> + uaa->product == USB_PRODUCT_MAUDIO_FASTTRACKPRO &&
> + uaa->configno != 2)
> + return UMATCH_NONE;

Why to you need this?

> +
>   quirk = uaudio_lookup(uaa->vendor, uaa->product);
>   if (quirk)
>   flags = quirk->flags;
> @@ -531,8 +539,21 @@ uaudio_attach(struct device *parent, str
>   found = 1;
>   }
>   }
> - if (found)
> + if (found) {
>   usbd_claim_iface(sc->sc_udev, i);
> + if (uaa->vendor == USB_VENDOR_MAUDIO &&
> + uaa->product == USB_PRODUCT_MAUDIO_FASTTRACKPRO) {
> + /*
> +  * temporarily switch every iface to 24bit.
> +  * Causes the device to be big endian even
> +  * for 16bit samples.
> +  * using 16bits first will cause the device
> +  * to break when we later switch to 24bit.
> +  */
> + usbd_set_interface(sc->sc_alts[i].ifaceh, 2);
> + usbd_set_interface(sc->sc_alts[i].ifaceh, 0);

How did you figure that out?  Is this behavior documented somehwere?

> + }
> + }
>   }
>  
>   for (j = 0; j < sc->sc_nalts; j++) {
> @@ -1662,7 +1683,10 @@ uaudio_process_as(struct uaudio_softc *s
>   } else if (prec == 24) {
>   sc->sc_altflags |= HAS_24;
>   }
> - enc = AUDIO_ENCODING_SLINEAR_LE;
> + if (sc->sc_quirks & UAUDIO_FLAG_BE)
> + enc = AUDIO_ENCODING_SLINEAR_BE;
> + else
> + enc = AUDIO_ENCODING_SLINEAR_LE;
>   format_str = "pcm";
>   break;
>   case UA_FMT_PCM8:
> @@ -1687,9 +1711,13 @@ uaudio_process_as(struct uaudio_softc *s
>   return (USBD_NORMAL_COMPLETION);
>   }
>  #ifdef UAUDIO_DEBUG
> - printf("%s: %s: %d-ch %d-bit %d-byte %s,", sc->sc_dev.dv_xname,
> + printf("%s: %s: alt %d(%d) for interface %d %d-ch %d-bit %d-byte %s enc 
> %d,",

This line is > 80 chars.

> +sc->sc_dev.dv_xname,
>  dir == UE_DIR_IN ? "recording" : "playback",
> -chan, prec, bps, format_str);
> +id->bAlternateSetting,
> +sc->sc_nalts,
> +id->bInterfaceNumber,
> +chan, prec, bps, format_str, enc);
>   if (asf1d->bSamFreqType == UA_SAMP_CONTNUOUS) {
>   printf(" %d-%dHz\n", UA_SAMP_LO(asf1d), UA_SAMP_HI(asf1d));
>   } else {
> @@ -3312,6 +3340,9 @@ uaudio_set_params(void *addr, int setmod
>   }
>   break;
>   }
> +
> + if (sc->sc_quirks & UAUDIO_FLAG_BE)
> + p->encoding = AUDIO_ENCODING_SLINEAR_BE;

Why do you need this chunk and we don't need to set AUDIO_ENCODING_SLINEAR_SE
in the other case?

>  
>   i = uaudio_match_alt(sc, p, mode);
>   if (i < 0) {
> Index: umidi.c
> ===
> RCS file: /cvs/src/sys/dev/usb/umidi.c,v
> retrieving revision 1.43
> diff -u -p -r1.43 umidi.c
> --- umidi.c   7 Jan 2017 06:10:40 -   1.43
> +++ umidi.c   29 Jan 2017 17:13:29 -
> @@ -41,6 +41,7 @@
>  #include 
>  
>  #include 
> +#include 
>  #include 
>  #include 
>  
> @@ -152,6 +153,11 @@ 

Re: Add quirks to support M-Audio FastTrack Pro (uaudio)

2017-01-29 Thread Christopher Zimmermann
On 2017-01-30 Martin Pieuchot  wrote:
> On 29/01/17(Sun) 14:45, Christopher Zimmermann wrote:
> 
> > You are talking about usb_subr.c? Done.  
> 
> No I'm not :)

> Yes please, do not touch usb_subr.c.  

> Also make sure your comment below respect style(9).

ok. I got it. I added conditions to uaudio.c and umidi.c to accept the
device only in configuration 2.
The quirks in uaudio.c stay the same.

Christopher


Index: uaudio.c
===
RCS file: /cvs/src/sys/dev/usb/uaudio.c,v
retrieving revision 1.122
diff -u -p -r1.122 uaudio.c
--- uaudio.c3 Jan 2017 06:45:58 -   1.122
+++ uaudio.c29 Jan 2017 17:13:28 -
@@ -172,6 +172,7 @@ struct chan {
 #define UAUDIO_FLAG_VENDOR_CLASS 0x0010/* claims vendor class but 
works */
 #define UAUDIO_FLAG_DEPENDENT   0x0020 /* play and record params must equal */
 #define UAUDIO_FLAG_EMU0202 0x0040
+#define UAUDIO_FLAG_BE  0x0080
 
 struct uaudio_devs {
struct usb_devno uv_dev;
@@ -223,7 +224,9 @@ struct uaudio_devs {
{ { USB_VENDOR_LOGITECH, USB_PRODUCT_LOGITECH_QUICKCAMZOOM },
UAUDIO_FLAG_BAD_AUDIO },
{ { USB_VENDOR_TELEX, USB_PRODUCT_TELEX_MIC1 },
-   UAUDIO_FLAG_NO_FRAC }
+   UAUDIO_FLAG_NO_FRAC },
+   { { USB_VENDOR_MAUDIO, USB_PRODUCT_MAUDIO_FASTTRACKPRO },
+   UAUDIO_FLAG_BE | UAUDIO_FLAG_DEPENDENT }
 };
 #define uaudio_lookup(v, p) \
((struct uaudio_devs *)usb_lookup(uaudio_devs, v, p))
@@ -444,6 +447,11 @@ uaudio_match(struct device *parent, void
if (uaa->iface == NULL || uaa->device == NULL)
return (UMATCH_NONE);
 
+   if (uaa->vendor == USB_VENDOR_MAUDIO &&
+   uaa->product == USB_PRODUCT_MAUDIO_FASTTRACKPRO &&
+   uaa->configno != 2)
+   return UMATCH_NONE;
+
quirk = uaudio_lookup(uaa->vendor, uaa->product);
if (quirk)
flags = quirk->flags;
@@ -531,8 +539,21 @@ uaudio_attach(struct device *parent, str
found = 1;
}
}
-   if (found)
+   if (found) {
usbd_claim_iface(sc->sc_udev, i);
+   if (uaa->vendor == USB_VENDOR_MAUDIO &&
+   uaa->product == USB_PRODUCT_MAUDIO_FASTTRACKPRO) {
+   /*
+* temporarily switch every iface to 24bit.
+* Causes the device to be big endian even
+* for 16bit samples.
+* using 16bits first will cause the device
+* to break when we later switch to 24bit.
+*/
+   usbd_set_interface(sc->sc_alts[i].ifaceh, 2);
+   usbd_set_interface(sc->sc_alts[i].ifaceh, 0);
+   }
+   }
}
 
for (j = 0; j < sc->sc_nalts; j++) {
@@ -1662,7 +1683,10 @@ uaudio_process_as(struct uaudio_softc *s
} else if (prec == 24) {
sc->sc_altflags |= HAS_24;
}
-   enc = AUDIO_ENCODING_SLINEAR_LE;
+   if (sc->sc_quirks & UAUDIO_FLAG_BE)
+   enc = AUDIO_ENCODING_SLINEAR_BE;
+   else
+   enc = AUDIO_ENCODING_SLINEAR_LE;
format_str = "pcm";
break;
case UA_FMT_PCM8:
@@ -1687,9 +1711,13 @@ uaudio_process_as(struct uaudio_softc *s
return (USBD_NORMAL_COMPLETION);
}
 #ifdef UAUDIO_DEBUG
-   printf("%s: %s: %d-ch %d-bit %d-byte %s,", sc->sc_dev.dv_xname,
+   printf("%s: %s: alt %d(%d) for interface %d %d-ch %d-bit %d-byte %s enc 
%d,",
+  sc->sc_dev.dv_xname,
   dir == UE_DIR_IN ? "recording" : "playback",
-  chan, prec, bps, format_str);
+  id->bAlternateSetting,
+  sc->sc_nalts,
+  id->bInterfaceNumber,
+  chan, prec, bps, format_str, enc);
if (asf1d->bSamFreqType == UA_SAMP_CONTNUOUS) {
printf(" %d-%dHz\n", UA_SAMP_LO(asf1d), UA_SAMP_HI(asf1d));
} else {
@@ -3312,6 +3340,9 @@ uaudio_set_params(void *addr, int setmod
}
break;
}
+
+   if (sc->sc_quirks & UAUDIO_FLAG_BE)
+   p->encoding = AUDIO_ENCODING_SLINEAR_BE;
 
i = uaudio_match_alt(sc, p, mode);
if (i < 0) {
Index: umidi.c
===
RCS file: /cvs/src/sys/dev/usb/umidi.c,v
retrieving revision 1.43
diff -u -p -r1.43 umidi.c
--- umidi.c 7 Jan 2017 06:10:40 -   1.43
+++ umidi.c 29 Jan 2017 17:13:29 -
@@ -41,6 +41,7 @@
 #include 
 
 #include 

Re: Add quirks to support M-Audio FastTrack Pro (uaudio)

2017-01-29 Thread Martin Pieuchot
On 29/01/17(Sun) 14:45, Christopher Zimmermann wrote:
> On 2017-01-29 Martin Pieuchot  wrote:
> > On 28/01/17(Sat) 13:02, Christopher Zimmermann wrote:
> > > Hi,
> > > 
> > > I needed to add some quirks to support the M-Audio FastTrack Pro USB
> > > audio interface.  
> > 
> > Since this is just for one device there's no need for a quirk.  Simply
> > check for the device ID.
> 
> You are talking about usb_subr.c? Done.

No I'm not :)

> > > * The device needs to be switched to the configuration 2 to show all
> > >   its capabilities. This is done by a new quirk in usb_subr.c  
> > 
> > Then do not match the first configuration in uaudio(4).
> 
> I already tried that. It can be done, but then the umidi driver needs
> changes, too, because it would otherwise claim the device in
> configuration 0.
> Should I do it this way without touching usb_subr.c?

Yes please, do not touch usb_subr.c.  

> Here's an updated diff which changes configuration explicitely in
> usb_subr.c and doesn't touch umidi.c.

Also make sure your comment below respect style(9).

> 
> 
> Christopher
> 
> 
> Index: uaudio.c
> ===
> RCS file: /cvs/src/sys/dev/usb/uaudio.c,v
> retrieving revision 1.122
> diff -u -p -r1.122 uaudio.c
> --- uaudio.c  3 Jan 2017 06:45:58 -   1.122
> +++ uaudio.c  29 Jan 2017 13:40:36 -
> @@ -172,6 +172,7 @@ struct chan {
>  #define UAUDIO_FLAG_VENDOR_CLASS 0x0010  /* claims vendor class but 
> works */
>  #define UAUDIO_FLAG_DEPENDENT 0x0020 /* play and record params must 
> equal */
>  #define UAUDIO_FLAG_EMU0202   0x0040
> +#define UAUDIO_FLAG_BE0x0080
>  
>  struct uaudio_devs {
>   struct usb_devno uv_dev;
> @@ -223,7 +224,9 @@ struct uaudio_devs {
>   { { USB_VENDOR_LOGITECH, USB_PRODUCT_LOGITECH_QUICKCAMZOOM },
>   UAUDIO_FLAG_BAD_AUDIO },
>   { { USB_VENDOR_TELEX, USB_PRODUCT_TELEX_MIC1 },
> - UAUDIO_FLAG_NO_FRAC }
> + UAUDIO_FLAG_NO_FRAC },
> + { { USB_VENDOR_MAUDIO, USB_PRODUCT_MAUDIO_FASTTRACKPRO },
> + UAUDIO_FLAG_BE | UAUDIO_FLAG_DEPENDENT }
>  };
>  #define uaudio_lookup(v, p) \
>   ((struct uaudio_devs *)usb_lookup(uaudio_devs, v, p))
> @@ -531,8 +534,19 @@ uaudio_attach(struct device *parent, str
>   found = 1;
>   }
>   }
> - if (found)
> + if (found) {
>   usbd_claim_iface(sc->sc_udev, i);
> + if (uaa->vendor == USB_VENDOR_MAUDIO &&
> + uaa->product == USB_PRODUCT_MAUDIO_FASTTRACKPRO) {
> + /* temporarily switch every iface to 24bit.
> +  * Causes the device to be big endian even
> +  * for 16bit samples.
> +  * using 16bits first will cause the device
> +  * to break when we later switch to 24bit. */
> + usbd_set_interface(sc->sc_alts[i].ifaceh, 2);
> + usbd_set_interface(sc->sc_alts[i].ifaceh, 0);
> + }
> + }
>   }
>  
>   for (j = 0; j < sc->sc_nalts; j++) {
> @@ -1662,7 +1676,10 @@ uaudio_process_as(struct uaudio_softc *s
>   } else if (prec == 24) {
>   sc->sc_altflags |= HAS_24;
>   }
> - enc = AUDIO_ENCODING_SLINEAR_LE;
> + if (sc->sc_quirks & UAUDIO_FLAG_BE)
> + enc = AUDIO_ENCODING_SLINEAR_BE;
> + else
> + enc = AUDIO_ENCODING_SLINEAR_LE;
>   format_str = "pcm";
>   break;
>   case UA_FMT_PCM8:
> @@ -1687,9 +1704,13 @@ uaudio_process_as(struct uaudio_softc *s
>   return (USBD_NORMAL_COMPLETION);
>   }
>  #ifdef UAUDIO_DEBUG
> - printf("%s: %s: %d-ch %d-bit %d-byte %s,", sc->sc_dev.dv_xname,
> + printf("%s: %s: alt %d(%d) for interface %d %d-ch %d-bit %d-byte %s enc 
> %d,",
> +sc->sc_dev.dv_xname,
>  dir == UE_DIR_IN ? "recording" : "playback",
> -chan, prec, bps, format_str);
> +id->bAlternateSetting,
> +sc->sc_nalts,
> +id->bInterfaceNumber,
> +chan, prec, bps, format_str, enc);
>   if (asf1d->bSamFreqType == UA_SAMP_CONTNUOUS) {
>   printf(" %d-%dHz\n", UA_SAMP_LO(asf1d), UA_SAMP_HI(asf1d));
>   } else {
> @@ -3312,6 +,9 @@ uaudio_set_params(void *addr, int setmod
>   }
>   break;
>   }
> +
> + if (sc->sc_quirks & UAUDIO_FLAG_BE)
> + p->encoding = AUDIO_ENCODING_SLINEAR_BE;
>  
>   i = uaudio_match_alt(sc, p, mode);
>   if (i < 0) {
> Index: usb_subr.c
> ===
> RCS 

Re: Add quirks to support M-Audio FastTrack Pro (uaudio)

2017-01-29 Thread Martin Pieuchot
On 28/01/17(Sat) 13:02, Christopher Zimmermann wrote:
> Hi,
> 
> I needed to add some quirks to support the M-Audio FastTrack Pro USB
> audio interface.

Since this is just for one device there's no need for a quirk.  Simply
check for the device ID.

> 
> * The device needs to be switched to the configuration 2 to show all
>   its capabilities. This is done by a new quirk in usb_subr.c

Then do not match the first configuration in uaudio(4).

> * It claims to consume little-endian samples, but actually expects
>   big-endian samples.
> 
> 
> OK?
> 
> Christopher
> 
> 
> Index: uaudio.c
> ===
> RCS file: /cvs/src/sys/dev/usb/uaudio.c,v
> retrieving revision 1.122
> diff -u -p -r1.122 uaudio.c
> --- uaudio.c  3 Jan 2017 06:45:58 -   1.122
> +++ uaudio.c  28 Jan 2017 11:53:17 -
> @@ -172,6 +172,7 @@ struct chan {
>  #define UAUDIO_FLAG_VENDOR_CLASS 0x0010  /* claims vendor class but 
> works */
>  #define UAUDIO_FLAG_DEPENDENT 0x0020 /* play and record params must 
> equal */
>  #define UAUDIO_FLAG_EMU0202   0x0040
> +#define UAUDIO_FLAG_FASTTRACKPRO 0x0080
>  
>  struct uaudio_devs {
>   struct usb_devno uv_dev;
> @@ -223,7 +224,9 @@ struct uaudio_devs {
>   { { USB_VENDOR_LOGITECH, USB_PRODUCT_LOGITECH_QUICKCAMZOOM },
>   UAUDIO_FLAG_BAD_AUDIO },
>   { { USB_VENDOR_TELEX, USB_PRODUCT_TELEX_MIC1 },
> - UAUDIO_FLAG_NO_FRAC }
> + UAUDIO_FLAG_NO_FRAC },
> + { { USB_VENDOR_MIDIMAN, USB_PRODUCT_MIDIMAN_FASTTRACKPRO },
> + UAUDIO_FLAG_FASTTRACKPRO | UAUDIO_FLAG_DEPENDENT }
>  };
>  #define uaudio_lookup(v, p) \
>   ((struct uaudio_devs *)usb_lookup(uaudio_devs, v, p))
> @@ -531,8 +534,18 @@ uaudio_attach(struct device *parent, str
>   found = 1;
>   }
>   }
> - if (found)
> + if (found) {
>   usbd_claim_iface(sc->sc_udev, i);
> + if (sc->sc_quirks & UAUDIO_FLAG_FASTTRACKPRO) {
> + /* temporarily switch every iface to 24bit.
> +  * Causes the device to be big endian even
> +  * for 16bit samples.
> +  * using 16bits first will cause the device
> +  * to break when we later switch to 24bit. */
> + usbd_set_interface(sc->sc_alts[i].ifaceh, 2);
> + usbd_set_interface(sc->sc_alts[i].ifaceh, 0);
> + }
> + }
>   }
>  
>   for (j = 0; j < sc->sc_nalts; j++) {
> @@ -1662,7 +1675,10 @@ uaudio_process_as(struct uaudio_softc *s
>   } else if (prec == 24) {
>   sc->sc_altflags |= HAS_24;
>   }
> - enc = AUDIO_ENCODING_SLINEAR_LE;
> + if (sc->sc_quirks & UAUDIO_FLAG_FASTTRACKPRO)
> + enc = AUDIO_ENCODING_SLINEAR_BE;
> + else
> + enc = AUDIO_ENCODING_SLINEAR_LE;
>   format_str = "pcm";
>   break;
>   case UA_FMT_PCM8:
> @@ -1687,9 +1703,13 @@ uaudio_process_as(struct uaudio_softc *s
>   return (USBD_NORMAL_COMPLETION);
>   }
>  #ifdef UAUDIO_DEBUG
> - printf("%s: %s: %d-ch %d-bit %d-byte %s,", sc->sc_dev.dv_xname,
> + printf("%s: %s: alt %d(%d) for interface %d %d-ch %d-bit %d-byte %s enc 
> %d,",
> +sc->sc_dev.dv_xname,
>  dir == UE_DIR_IN ? "recording" : "playback",
> -chan, prec, bps, format_str);
> +id->bAlternateSetting,
> +sc->sc_nalts,
> +id->bInterfaceNumber,
> +chan, prec, bps, format_str, enc);
>   if (asf1d->bSamFreqType == UA_SAMP_CONTNUOUS) {
>   printf(" %d-%dHz\n", UA_SAMP_LO(asf1d), UA_SAMP_HI(asf1d));
>   } else {
> @@ -3312,6 +3332,9 @@ uaudio_set_params(void *addr, int setmod
>   }
>   break;
>   }
> +
> + if (sc->sc_quirks & UAUDIO_FLAG_FASTTRACKPRO)
> + p->encoding = AUDIO_ENCODING_SLINEAR_BE;
>  
>   i = uaudio_match_alt(sc, p, mode);
>   if (i < 0) {
> Index: usb_quirks.c
> ===
> RCS file: /cvs/src/sys/dev/usb/usb_quirks.c,v
> retrieving revision 1.74
> diff -u -p -r1.74 usb_quirks.c
> --- usb_quirks.c  27 Nov 2015 10:59:32 -  1.74
> +++ usb_quirks.c  28 Jan 2017 11:53:17 -
> @@ -67,6 +67,8 @@ const struct usbd_quirk_entry {
>   0x001, { UQ_ASSUME_CM_OVER_DATA }},
>   { USB_VENDOR_EICON, USB_PRODUCT_EICON_DIVA852,
>   0x100, { UQ_ASSUME_CM_OVER_DATA }},
> + { USB_VENDOR_MIDIMAN, USB_PRODUCT_MIDIMAN_FASTTRACKPRO,
> + 0x100, { 1 << UQ_CONFIG_SHIFT }},
>   /* YAMAHA router's ucdDevice is the version of firmware and often 

Add quirks to support M-Audio FastTrack Pro (uaudio)

2017-01-28 Thread Christopher Zimmermann
Hi,

I needed to add some quirks to support the M-Audio FastTrack Pro USB
audio interface.

* The device needs to be switched to the configuration 2 to show all
  its capabilities. This is done by a new quirk in usb_subr.c
* It claims to consume little-endian samples, but actually expects
  big-endian samples.


OK?

Christopher


Index: uaudio.c
===
RCS file: /cvs/src/sys/dev/usb/uaudio.c,v
retrieving revision 1.122
diff -u -p -r1.122 uaudio.c
--- uaudio.c3 Jan 2017 06:45:58 -   1.122
+++ uaudio.c28 Jan 2017 11:53:17 -
@@ -172,6 +172,7 @@ struct chan {
 #define UAUDIO_FLAG_VENDOR_CLASS 0x0010/* claims vendor class but 
works */
 #define UAUDIO_FLAG_DEPENDENT   0x0020 /* play and record params must equal */
 #define UAUDIO_FLAG_EMU0202 0x0040
+#define UAUDIO_FLAG_FASTTRACKPRO 0x0080
 
 struct uaudio_devs {
struct usb_devno uv_dev;
@@ -223,7 +224,9 @@ struct uaudio_devs {
{ { USB_VENDOR_LOGITECH, USB_PRODUCT_LOGITECH_QUICKCAMZOOM },
UAUDIO_FLAG_BAD_AUDIO },
{ { USB_VENDOR_TELEX, USB_PRODUCT_TELEX_MIC1 },
-   UAUDIO_FLAG_NO_FRAC }
+   UAUDIO_FLAG_NO_FRAC },
+   { { USB_VENDOR_MIDIMAN, USB_PRODUCT_MIDIMAN_FASTTRACKPRO },
+   UAUDIO_FLAG_FASTTRACKPRO | UAUDIO_FLAG_DEPENDENT }
 };
 #define uaudio_lookup(v, p) \
((struct uaudio_devs *)usb_lookup(uaudio_devs, v, p))
@@ -531,8 +534,18 @@ uaudio_attach(struct device *parent, str
found = 1;
}
}
-   if (found)
+   if (found) {
usbd_claim_iface(sc->sc_udev, i);
+   if (sc->sc_quirks & UAUDIO_FLAG_FASTTRACKPRO) {
+   /* temporarily switch every iface to 24bit.
+* Causes the device to be big endian even
+* for 16bit samples.
+* using 16bits first will cause the device
+* to break when we later switch to 24bit. */
+   usbd_set_interface(sc->sc_alts[i].ifaceh, 2);
+   usbd_set_interface(sc->sc_alts[i].ifaceh, 0);
+   }
+   }
}
 
for (j = 0; j < sc->sc_nalts; j++) {
@@ -1662,7 +1675,10 @@ uaudio_process_as(struct uaudio_softc *s
} else if (prec == 24) {
sc->sc_altflags |= HAS_24;
}
-   enc = AUDIO_ENCODING_SLINEAR_LE;
+   if (sc->sc_quirks & UAUDIO_FLAG_FASTTRACKPRO)
+   enc = AUDIO_ENCODING_SLINEAR_BE;
+   else
+   enc = AUDIO_ENCODING_SLINEAR_LE;
format_str = "pcm";
break;
case UA_FMT_PCM8:
@@ -1687,9 +1703,13 @@ uaudio_process_as(struct uaudio_softc *s
return (USBD_NORMAL_COMPLETION);
}
 #ifdef UAUDIO_DEBUG
-   printf("%s: %s: %d-ch %d-bit %d-byte %s,", sc->sc_dev.dv_xname,
+   printf("%s: %s: alt %d(%d) for interface %d %d-ch %d-bit %d-byte %s enc 
%d,",
+  sc->sc_dev.dv_xname,
   dir == UE_DIR_IN ? "recording" : "playback",
-  chan, prec, bps, format_str);
+  id->bAlternateSetting,
+  sc->sc_nalts,
+  id->bInterfaceNumber,
+  chan, prec, bps, format_str, enc);
if (asf1d->bSamFreqType == UA_SAMP_CONTNUOUS) {
printf(" %d-%dHz\n", UA_SAMP_LO(asf1d), UA_SAMP_HI(asf1d));
} else {
@@ -3312,6 +3332,9 @@ uaudio_set_params(void *addr, int setmod
}
break;
}
+
+   if (sc->sc_quirks & UAUDIO_FLAG_FASTTRACKPRO)
+   p->encoding = AUDIO_ENCODING_SLINEAR_BE;
 
i = uaudio_match_alt(sc, p, mode);
if (i < 0) {
Index: usb_quirks.c
===
RCS file: /cvs/src/sys/dev/usb/usb_quirks.c,v
retrieving revision 1.74
diff -u -p -r1.74 usb_quirks.c
--- usb_quirks.c27 Nov 2015 10:59:32 -  1.74
+++ usb_quirks.c28 Jan 2017 11:53:17 -
@@ -67,6 +67,8 @@ const struct usbd_quirk_entry {
0x001, { UQ_ASSUME_CM_OVER_DATA }},
  { USB_VENDOR_EICON, USB_PRODUCT_EICON_DIVA852,
0x100, { UQ_ASSUME_CM_OVER_DATA }},
+ { USB_VENDOR_MIDIMAN, USB_PRODUCT_MIDIMAN_FASTTRACKPRO,
+   0x100, { 1 << UQ_CONFIG_SHIFT }},
  /* YAMAHA router's ucdDevice is the version of firmware and often changes. */
  { USB_VENDOR_YAMAHA, USB_PRODUCT_YAMAHA_RTA54I,
ANY, { UQ_ASSUME_CM_OVER_DATA }},
Index: usb_quirks.h
===
RCS file: /cvs/src/sys/dev/usb/usb_quirks.h,v
retrieving revision 1.16
diff -u -p -r1.16 usb_quirks.h
--- usb_quirks.h