Index | Thread | Search

From:
Mikolaj Kucharski <mikolaj@kucharski.name>
Subject:
Re: uaudio: Handle certain devices with multiple clock domains
To:
Alexandre Ratchov <alex@caoua.org>
Cc:
tech@openbsd.org
Date:
Thu, 13 Nov 2025 17:44:14 +0000

Download raw body.

Thread
Hi Alexandre.

Thank you for the diff. All three USB-C devices works here now.

I tested two devices which didn't work before this diff as reported here:

	https://marc.info/?t=176298272900002&r=1&w=2

With your diff, they work. Device number one, which didn't work:

uaudio0 at uhub1 port 5 configuration 1 interface 1 "Apple, Inc. USB-C to 3.5mm Headphone Jack Adapter" rev 2.01/26.50 addr 5
uaudio0: class v2, full-speed, sync, channels: 2 play, 1 rec, 7 ctls
audio1 at uaudio0
uhidev0 at uhub1 port 5 configuration 1 interface 3 "Apple, Inc. USB-C to 3.5mm Headphone Jack Adapter" rev 2.01/26.50 addr 5
uhidev0: iclass 3/0, 1 report id
ucc0 at uhidev0 reportid 1: 4 usages, 3 keys, enum
wskbd1 at ucc0 mux 1
wskbd1: connecting to wsdisplay0
uhidev1 at uhub1 port 5 configuration 1 interface 4 "Apple, Inc. USB-C to 3.5mm Headphone Jack Adapter" rev 2.01/26.50 addr 5
uhidev1: iclass 3/0, 2 report ids
uhid0 at uhidev1 reportid 1: input=0, output=63, feature=0
uhid1 at uhidev1 reportid 2: input=63, output=0, feature=0
uaudio0: 0: bad rec intr count


Device number two, which didn't work:

uaudio0 at uhub1 port 5 configuration 1 interface 1 "Apple, Inc. EarPods" rev 2.01/26.81 addr 5
uaudio0: class v2, full-speed, sync, channels: 2 play, 1 rec, 7 ctls
audio1 at uaudio0
uhidev0 at uhub1 port 5 configuration 1 interface 3 "Apple, Inc. EarPods" rev 2.01/26.81 addr 5
uhidev0: iclass 3/0, 1 report id
ucc0 at uhidev0 reportid 1: 4 usages, 3 keys, enum
wskbd1 at ucc0 mux 1
wskbd1: connecting to wsdisplay0
uhidev1 at uhub1 port 5 configuration 1 interface 4 "Apple, Inc. EarPods" rev 2.01/26.81 addr 5
uhidev1: iclass 3/0, 2 report ids
uhid0 at uhidev1 reportid 1: input=0, output=63, feature=0
uhid1 at uhidev1 reportid 2: input=63, output=0, feature=0
uaudio0: 0: bad rec intr count


And device which previously worked, still works:

uaudio0 at uhub1 port 5 configuration 1 interface 1 "Google Pixel USB-C earbuds" rev 2.00/0.20 addr 5
uaudio0: class v1, full-speed, sync, channels: 2 play, 1 rec, 7 ctls
audio1 at uaudio0
uhidev0 at uhub1 port 5 configuration 1 interface 3 "Google Pixel USB-C earbuds" rev 2.00/0.20 addr 5
uhidev0: iclass 3/0, 5 report ids
ucc0 at uhidev0 reportid 1: 3 usages, 3 keys, enum
wskbd1 at ucc0 mux 1
wskbd1: connecting to wsdisplay0
uhid0 at uhidev0 reportid 4: input=0, output=38, feature=0
uhid1 at uhidev0 reportid 5: input=34, output=0, feature=0


Thank you!

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.
> 
> 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	13 Nov 2025 13:03:24 -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__);
> @@ -1181,6 +1178,54 @@ 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)
> +{
> +	struct uaudio_ranges_el *ur, *vr;
> +
> +	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;
> +	}
> +
> +	ur = u->rates.el;
> +	vr = v->rates.el;
> +	while (1) {
> +		if (ur == NULL && vr == NULL)
> +			break;
> +		if (ur == NULL ||
> +		    vr == NULL ||
> +		    ur->min != vr->min ||
> +		    ur->max != vr->max ||
> +		    ur->res != vr->res) {
> +			printf("%s: clock rates differ\n", DEVNAME(sc));
> +			return 0;
> +		}
> +		ur = ur->next;
> +		vr = vr->next;
> +	}
> +
> +	DPRINTF("%s: %d, %d: equivalent\n", __func__, u->id, v->id);
> +	return 1;
> +}
> +/*
>   * For the given unit, parse the list of its sources and recursively
>   * call uaudio_process_unit() for each.
>   */
> @@ -2264,34 +2309,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 +3168,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 +3929,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;
> 

-- 
Regards,
 Mikolaj