Index | Thread | Search

From:
Pascal Stumpf <pascal@stumpf.co>
Subject:
Re: uaudio: Handle certain devices with multiple clock domains, v2
To:
Alexandre Ratchov <alex@caoua.org>
Cc:
tech@openbsd.org
Date:
Fri, 14 Nov 2025 17:53:46 +0100

Download raw body.

Thread
  • Alexandre Ratchov:

    uaudio: Handle certain devices with multiple clock domains, v2

    • Pascal Stumpf:

      uaudio: Handle certain devices with multiple clock domains, v2

  • On Fri, 14 Nov 2025 17:20:25 +0100, Alexandre Ratchov wrote:
    > On Thu, Nov 13, 2025 at 02:11:18PM +0100, Alexandre Ratchov wrote:
    > > uaudio(4) doesn't support devices with multiple "clock domains". They
    > > would be almost multiple independent devices in a single piece of
    > > hardware.
    > > 
    > > Physical clocks cost money, so I suspect that most devices have one
    > > physical clock only, i.e. that the different clocks the device exposes
    > > are synchronous. If so, uaudio(4) could handle them with the diff
    > > below.
    > > 
    > > To test this, first use a know working uaudio(4) device to validate
    > > your setup (usb host controller, hub etc). Once you feel audio is
    > > stable, try the device with the multiple clock domains on the same
    > > port.
    > > 
    > > I'm insisting on this because there are still usb-related bugs, that
    > > would prevent understanding the cause of a possible failure.
    > > 
    > 
    > Thanks for all the feedback.
    > 
    > Here's a better version of the diff. It assumes that the clocks are
    > equivalent if they have the same attributes and at least one common
    > rate.
    > 
    > This version addresses devices where certain rates are disabled in one
    > direction i.e. ones that failed with "clock rates differ" with the
    > previous version of the diff. If your devices just worked with the
    > previous diff, no need to test this one.
    
    Thanks!  This now attaches the Lenovo Thunderbolt 3 Dock integrated
    audio which did not work before:
    
    uaudio0 at uhub5 port 2 configuration 1 interface 1 "Lenovo ThinkPad
    Thunderbolt 3 Dock USB Audio" rev 2.01/0.95 addr 5
    uaudio0: warning: assuming common clock
    uaudio0: class v2, full-speed, sync, channels: 2 play, 1 rec, 3 ctls
    audio1 at uaudio0
    
    Both playback and recording are working fine.
    
    > Index: uaudio.c
    > ===================================================================
    > RCS file: /cvs/src/sys/dev/usb/uaudio.c,v
    > diff -u -p -u -p -r1.180 uaudio.c
    > --- uaudio.c	2 Nov 2025 14:35:20 -0000	1.180
    > +++ uaudio.c	14 Nov 2025 15:54:47 -0000
    > @@ -263,7 +263,7 @@ struct uaudio_softc {
    >  	/*
    >  	 * Current clock, UAC v2.0 only
    >  	 */
    > -	struct uaudio_unit *clock;
    > +	struct uaudio_unit *pclock, *rclock;
    >  
    >  	/*
    >  	 * Number of input and output terminals
    > @@ -1023,7 +1023,7 @@ uaudio_alt_getrates(struct uaudio_softc 
    >  	case UAUDIO_V1:
    >  		return p->v1_rates;
    >  	case UAUDIO_V2:
    > -		u = sc->clock;
    > +		u = p->mode == AUMODE_PLAY ? sc->pclock : sc->rclock;
    >  		while (1) {
    >  			switch (u->type) {
    >  			case UAUDIO_AC_CLKSRC:
    > @@ -1050,11 +1050,8 @@ uaudio_alt_getrates(struct uaudio_softc 
    >   * return the clock source unit
    >   */
    >  struct uaudio_unit *
    > -uaudio_clock(struct uaudio_softc *sc)
    > +uaudio_clock(struct uaudio_unit *u)
    >  {
    > -	struct uaudio_unit *u;
    > -
    > -	u = sc->clock;
    >  	while (1) {
    >  		if (u == NULL) {
    >  			DPRINTF("%s: NULL clock pointer\n", __func__);
    > @@ -1081,13 +1078,20 @@ uaudio_clock(struct uaudio_softc *sc)
    >   * Return the rates bitmap of the given parameters setting
    >   */
    >  int
    > -uaudio_getrates(struct uaudio_softc *sc, struct uaudio_params *p)
    > +uaudio_getrates(struct uaudio_softc *sc, int mode, struct uaudio_params *p)
    >  {
    > +	int rates;
    > +
    >  	switch (sc->version) {
    >  	case UAUDIO_V1:
    >  		return p->v1_rates;
    >  	case UAUDIO_V2:
    > -		return uaudio_alt_getrates(sc, p->palt ? p->palt : p->ralt);
    > +		rates = ~0;
    > +		if (p->palt != NULL && (mode & AUMODE_PLAY))
    > +			rates &= uaudio_alt_getrates(sc, p->palt);
    > +		if (p->ralt != NULL && (mode & AUMODE_RECORD))
    > +			rates &= uaudio_alt_getrates(sc, p->ralt);
    > +		return rates;
    >  	}
    >  	return 0;
    >  }
    > @@ -1181,6 +1185,38 @@ uaudio_feature_addent(struct uaudio_soft
    >  }
    >  
    >  /*
    > + * Compare two clock source units. According to UAC2 each clock
    > + * source defines its own clock domain. However there is hardware
    > + * exposing multiple clock units that are clocked by the same source
    > + * and consequently are equivalent to single domain.
    > + */
    > +int
    > +uaudio_clock_equiv(struct uaudio_softc *sc,
    > +	struct uaudio_unit *u, struct uaudio_unit *v)
    > +{
    > +	if (u == NULL || v == NULL)
    > +		return 1;
    > +
    > +	u = uaudio_clock(u);
    > +	if (u == NULL)
    > +		return 0;
    > +
    > +	v = uaudio_clock(v);
    > +	if (v == NULL)
    > +		return 0;
    > +
    > +	if (u->term != v->term) {
    > +		printf("%s: clock attributes differ\n", DEVNAME(sc));
    > +		return 0;
    > +	}
    > +
    > +	if (u != v)
    > +		printf("%s: warning: assuming common clock\n", DEVNAME(sc));
    > +
    > +	return 1;
    > +}
    > +
    > +/*
    >   * For the given unit, parse the list of its sources and recursively
    >   * call uaudio_process_unit() for each.
    >   */
    > @@ -1887,7 +1923,7 @@ uaudio_conf_print(struct uaudio_softc *s
    >  			rates = p->v1_rates;
    >  			break;
    >  		case UAUDIO_V2:
    > -			rates = uaudio_getrates(sc, p);
    > +			rates = uaudio_getrates(sc, AUMODE_PLAY | AUMODE_RECORD, p);
    >  			break;
    >  		}
    >  		printf("pchan = %d, s%dle%d, rchan = %d, s%dle%d, rates:",
    > @@ -2264,34 +2300,39 @@ uaudio_process_ac(struct uaudio_softc *s
    >  	}
    >  
    >  	if (sc->version == UAUDIO_V2) {
    > +
    >  		/*
    > -		 * Find common clock unit. We assume all terminals
    > -		 * belong to the same clock domain (ie are connected
    > -		 * to the same source)
    > +		 * Find the clocks for the usb-streaming terminal units
    >  		 */
    > -		sc->clock = NULL;
    > +
    >  		for (u = sc->unit_list; u != NULL; u = u->unit_next) {
    > -			if (u->type != UAUDIO_AC_INPUT &&
    > -			    u->type != UAUDIO_AC_OUTPUT)
    > -				continue;
    > -			if (sc->clock == NULL) {
    > +			if (u->type == UAUDIO_AC_INPUT && (u->term >> 8) == 1) {
    >  				if (u->clock == NULL) {
    > -					printf("%s: terminal with no clock\n",
    > -					    DEVNAME(sc));
    > +					printf("%s: no play clock\n", DEVNAME(sc));
    >  					return 0;
    >  				}
    > -				sc->clock = u->clock;
    > -			} else if (u->clock != sc->clock) {
    > -				printf("%s: only one clock domain supported\n",
    > -				    DEVNAME(sc));
    > -				return 0;
    > +				sc->pclock = u->clock;
    > +				break;
    >  			}
    >  		}
    >  
    > -		if (sc->clock == NULL) {
    > -			printf("%s: no clock found\n", DEVNAME(sc));
    > -			return 0;
    > +		for (u = sc->unit_list; u != NULL; u = u->unit_next) {
    > +			if (u->type == UAUDIO_AC_OUTPUT && (u->term >> 8) == 1) {
    > +				if (u->clock == NULL) {
    > +					printf("%s: no rec clock\n", DEVNAME(sc));
    > +					return 0;
    > +				}
    > +				sc->rclock = u->clock;
    > +				break;
    > +			}
    >  		}
    > +
    > +		DPRINTF("%s: pclock = %d, rclock = %d\n", __func__,
    > +		    sc->pclock ? sc->pclock->id : -1,
    > +		    sc->rclock ? sc->rclock->id : -1);
    > +
    > +		if (!uaudio_clock_equiv(sc, sc->pclock, sc->rclock))
    > +			return 0;
    >  	}
    >  	return 1;
    >  }
    > @@ -3118,7 +3159,7 @@ uaudio_stream_open(struct uaudio_softc *
    >  		req_buf[1] = sc->rate >> 8;
    >  		req_buf[2] = sc->rate >> 16;
    >  		req_buf[3] = sc->rate >> 24;
    > -		clock = uaudio_clock(sc);
    > +		clock = uaudio_clock(dir == AUMODE_PLAY ? sc->pclock : sc->rclock);
    >  		if (clock == NULL) {
    >  			printf("%s: can't get clock\n", DEVNAME(sc));
    >  			goto failed;
    > @@ -3879,7 +3920,8 @@ uaudio_attach(struct device *parent, str
    >  	sc->names = NULL;
    >  	sc->alts = NULL;
    >  	sc->params_list = NULL;
    > -	sc->clock = NULL;
    > +	sc->rclock = NULL;
    > +	sc->pclock = NULL;
    >  	sc->params = NULL;
    >  	sc->rate = 0;
    >  	sc->mode = 0;
    > @@ -4059,7 +4101,7 @@ uaudio_set_params(void *self, int setmod
    >  			best_mode = p;
    >  
    >  		/* test if params match the requested rate */
    > -		if ((uaudio_getrates(sc, p) & (1 << rateindex)) == 0)
    > +		if ((uaudio_getrates(sc, setmode, p) & (1 << rateindex)) == 0)
    >  			continue;
    >  		if (best_rate == NULL)
    >  			best_rate = p;
    > @@ -4105,7 +4147,7 @@ uaudio_set_params(void *self, int setmod
    >  	 * Recalculate rate index, because the chosen parameters
    >  	 * may not support the requested one
    >  	 */
    > -	rateindex = uaudio_rates_indexof(uaudio_getrates(sc, p), rate);
    > +	rateindex = uaudio_rates_indexof(uaudio_getrates(sc, setmode, p), rate);
    >  	if (rateindex < 0)
    >  		return ENOTTY;
    >  
    > 
    
    
  • Alexandre Ratchov:

    uaudio: Handle certain devices with multiple clock domains, v2