Hi, thanks for your patience. If some things are still unclear I'll be glad to clarify.
Christopher
On 2017-02-02 Martin Pieuchot <m...@openbsd.org> wrote:
> On 31/01/17(Tue) 14:05, Christopher Zimmermann wrote:
> > On 2017-01-29 Martin Pieuchot <m...@openbsd.org> 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
