[alsa-devel] [PATCHv2 00/10] UAC2: Automatic clock switching
This is version 2 of the automatic clock switching. v1: http://mailman.alsa-project.org/pipermail/alsa-devel/2013-March/060734.html
No patches added or removed. Rebased patches with "ALSA: usb: Work around CM6631 sample rate change bug", commit 690a863ff03d9a29ace2b752b8f802fba78a842f .
Clemens, Takashi and Torstein, thank you very much for reviewing and providing comments!
Changes: * Fixed formatting * (#5) Fixed type cast: u32 -> __l32 * (#6) Added rational to commit message * (#7) Fixed assignment of int to unsigned char, return error code if encountered * (#8) Fixed the parameter name, annotated the default value * (#10) Avoid second read of frequency for read-only clocks
Tested with Takashi's for-next. Applies against Takashi's for-next HEAD efc33ce197e4b6aaddf1eb2ba6293f51daf3c283 .
Cheers,
Eldad Zack (10): ALSA: usb-audio: convert list_for_each to entry variant ALSA: usb-audio: neaten MODULE_DEVICE_TABLE placement ALSA: usb-audio: neaten EXPORT_SYMBOLS placement ALSA: usb-audio: spelling correction ALSA: usb-audio: use endianness macros ALSA: usb-audio: UAC2: do clock validity check earlier ALSA: usb-audio: UAC2: try to find and switch to valid clock ALSA: usb-audio: UAC2: auto clock selection module param ALSA: usb-audio: show err in set_sample_rate_v2 debug ALSA: usb-audio: UAC2: support read-only freq control
sound/usb/card.c | 11 ++-- sound/usb/clock.c | 153 ++++++++++++++++++++++++++++++++++++++------------- sound/usb/clock.h | 3 +- sound/usb/endpoint.c | 18 +++--- sound/usb/endpoint.h | 2 +- sound/usb/format.c | 2 +- sound/usb/midi.c | 12 ++-- sound/usb/pcm.c | 32 ++++------- sound/usb/proc.c | 7 +-- sound/usb/stream.c | 12 ++-- sound/usb/usbaudio.h | 1 + 11 files changed, 157 insertions(+), 96 deletions(-)
Change occurances of list_for_each into list_for_each_entry where applicable.
Signed-off-by: Eldad Zack eldad@fogrefinery.com --- sound/usb/card.c | 4 +--- sound/usb/endpoint.c | 4 +--- sound/usb/midi.c | 5 ++--- sound/usb/pcm.c | 30 ++++++++++-------------------- sound/usb/proc.c | 7 +++---- sound/usb/stream.c | 12 ++++-------- 6 files changed, 21 insertions(+), 41 deletions(-)
diff --git a/sound/usb/card.c b/sound/usb/card.c index 2da8ad7..8bab36c 100644 --- a/sound/usb/card.c +++ b/sound/usb/card.c @@ -645,7 +645,6 @@ void snd_usb_autosuspend(struct snd_usb_audio *chip) static int usb_audio_suspend(struct usb_interface *intf, pm_message_t message) { struct snd_usb_audio *chip = usb_get_intfdata(intf); - struct list_head *p; struct snd_usb_stream *as; struct usb_mixer_interface *mixer;
@@ -655,8 +654,7 @@ static int usb_audio_suspend(struct usb_interface *intf, pm_message_t message) if (!PMSG_IS_AUTO(message)) { snd_power_change_state(chip->card, SNDRV_CTL_POWER_D3hot); if (!chip->num_suspended_intf++) { - list_for_each(p, &chip->pcm_list) { - as = list_entry(p, struct snd_usb_stream, list); + list_for_each_entry(as, &chip->pcm_list, list) { snd_pcm_suspend_all(as->pcm); as->substream[0].need_setup_ep = as->substream[1].need_setup_ep = true; diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c index 21049b8..b1f687f 100644 --- a/sound/usb/endpoint.c +++ b/sound/usb/endpoint.c @@ -415,14 +415,12 @@ struct snd_usb_endpoint *snd_usb_add_endpoint(struct snd_usb_audio *chip, struct usb_host_interface *alts, int ep_num, int direction, int type) { - struct list_head *p; struct snd_usb_endpoint *ep; int is_playback = direction == SNDRV_PCM_STREAM_PLAYBACK;
mutex_lock(&chip->mutex);
- list_for_each(p, &chip->ep_list) { - ep = list_entry(p, struct snd_usb_endpoint, list); + list_for_each_entry(ep, &chip->ep_list, list) { if (ep->ep_num == ep_num && ep->iface == alts->desc.bInterfaceNumber && ep->alt_idx == alts->desc.bAlternateSetting) { diff --git a/sound/usb/midi.c b/sound/usb/midi.c index 34b9bb7..1cf943d 100644 --- a/sound/usb/midi.c +++ b/sound/usb/midi.c @@ -1465,10 +1465,9 @@ static void snd_usbmidi_rawmidi_free(struct snd_rawmidi *rmidi) static struct snd_rawmidi_substream *snd_usbmidi_find_substream(struct snd_usb_midi* umidi, int stream, int number) { - struct list_head* list; + struct snd_rawmidi_substream *substream;
- list_for_each(list, &umidi->rmidi->streams[stream].substreams) { - struct snd_rawmidi_substream *substream = list_entry(list, struct snd_rawmidi_substream, list); + list_for_each_entry(substream, &umidi->rmidi->streams[stream].substreams, list) { if (substream->number == number) return substream; } diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c index c263991..652724c 100644 --- a/sound/usb/pcm.c +++ b/sound/usb/pcm.c @@ -94,13 +94,11 @@ static snd_pcm_uframes_t snd_usb_pcm_pointer(struct snd_pcm_substream *substream */ static struct audioformat *find_format(struct snd_usb_substream *subs) { - struct list_head *p; + struct audioformat *fp; struct audioformat *found = NULL; int cur_attr = 0, attr;
- list_for_each(p, &subs->fmt_list) { - struct audioformat *fp; - fp = list_entry(p, struct audioformat, list); + list_for_each_entry(fp, &subs->fmt_list, list) { if (!(fp->formats & (1uLL << subs->pcm_format))) continue; if (fp->channels != subs->channels) @@ -809,7 +807,7 @@ static int hw_rule_rate(struct snd_pcm_hw_params *params, struct snd_pcm_hw_rule *rule) { struct snd_usb_substream *subs = rule->private; - struct list_head *p; + struct audioformat *fp; struct snd_interval *it = hw_param_interval(params, SNDRV_PCM_HW_PARAM_RATE); unsigned int rmin, rmax; int changed; @@ -817,9 +815,7 @@ static int hw_rule_rate(struct snd_pcm_hw_params *params, hwc_debug("hw_rule_rate: (%d,%d)\n", it->min, it->max); changed = 0; rmin = rmax = 0; - list_for_each(p, &subs->fmt_list) { - struct audioformat *fp; - fp = list_entry(p, struct audioformat, list); + list_for_each_entry(fp, &subs->fmt_list, list) { if (!hw_check_valid_format(subs, params, fp)) continue; if (changed++) { @@ -863,7 +859,7 @@ static int hw_rule_channels(struct snd_pcm_hw_params *params, struct snd_pcm_hw_rule *rule) { struct snd_usb_substream *subs = rule->private; - struct list_head *p; + struct audioformat *fp; struct snd_interval *it = hw_param_interval(params, SNDRV_PCM_HW_PARAM_CHANNELS); unsigned int rmin, rmax; int changed; @@ -871,9 +867,7 @@ static int hw_rule_channels(struct snd_pcm_hw_params *params, hwc_debug("hw_rule_channels: (%d,%d)\n", it->min, it->max); changed = 0; rmin = rmax = 0; - list_for_each(p, &subs->fmt_list) { - struct audioformat *fp; - fp = list_entry(p, struct audioformat, list); + list_for_each_entry(fp, &subs->fmt_list, list) { if (!hw_check_valid_format(subs, params, fp)) continue; if (changed++) { @@ -916,7 +910,7 @@ static int hw_rule_format(struct snd_pcm_hw_params *params, struct snd_pcm_hw_rule *rule) { struct snd_usb_substream *subs = rule->private; - struct list_head *p; + struct audioformat *fp; struct snd_mask *fmt = hw_param_mask(params, SNDRV_PCM_HW_PARAM_FORMAT); u64 fbits; u32 oldbits[2]; @@ -924,9 +918,7 @@ static int hw_rule_format(struct snd_pcm_hw_params *params,
hwc_debug("hw_rule_format: %x:%x\n", fmt->bits[0], fmt->bits[1]); fbits = 0; - list_for_each(p, &subs->fmt_list) { - struct audioformat *fp; - fp = list_entry(p, struct audioformat, list); + list_for_each_entry(fp, &subs->fmt_list, list) { if (!hw_check_valid_format(subs, params, fp)) continue; fbits |= fp->formats; @@ -1034,7 +1026,7 @@ static int snd_usb_pcm_check_knot(struct snd_pcm_runtime *runtime,
static int setup_hw_info(struct snd_pcm_runtime *runtime, struct snd_usb_substream *subs) { - struct list_head *p; + struct audioformat *fp; unsigned int pt, ptmin; int param_period_time_if_needed; int err; @@ -1048,9 +1040,7 @@ static int setup_hw_info(struct snd_pcm_runtime *runtime, struct snd_usb_substre runtime->hw.rates = 0; ptmin = UINT_MAX; /* check min/max rates and channels */ - list_for_each(p, &subs->fmt_list) { - struct audioformat *fp; - fp = list_entry(p, struct audioformat, list); + list_for_each_entry(fp, &subs->fmt_list, list) { runtime->hw.rates |= fp->rates; if (runtime->hw.rate_min > fp->rate_min) runtime->hw.rate_min = fp->rate_min; diff --git a/sound/usb/proc.c b/sound/usb/proc.c index d218f76..0182ef6 100644 --- a/sound/usb/proc.c +++ b/sound/usb/proc.c @@ -73,15 +73,14 @@ void snd_usb_audio_create_proc(struct snd_usb_audio *chip) */ static void proc_dump_substream_formats(struct snd_usb_substream *subs, struct snd_info_buffer *buffer) { - struct list_head *p; + struct audioformat *fp; static char *sync_types[4] = { "NONE", "ASYNC", "ADAPTIVE", "SYNC" };
- list_for_each(p, &subs->fmt_list) { - struct audioformat *fp; + list_for_each_entry(fp, &subs->fmt_list, list) { snd_pcm_format_t fmt; - fp = list_entry(p, struct audioformat, list); + snd_iprintf(buffer, " Interface %d\n", fp->iface); snd_iprintf(buffer, " Altset %d\n", fp->altsetting); snd_iprintf(buffer, " Format:"); diff --git a/sound/usb/stream.c b/sound/usb/stream.c index ad07046..74beea237 100644 --- a/sound/usb/stream.c +++ b/sound/usb/stream.c @@ -42,12 +42,11 @@ */ static void free_substream(struct snd_usb_substream *subs) { - struct list_head *p, *n; + struct audioformat *fp, *n;
if (!subs->num_formats) return; /* not initialized */ - list_for_each_safe(p, n, &subs->fmt_list) { - struct audioformat *fp = list_entry(p, struct audioformat, list); + list_for_each_entry_safe(fp, n, &subs->fmt_list, list) { kfree(fp->rate_table); kfree(fp->chmap); kfree(fp); @@ -313,14 +312,12 @@ int snd_usb_add_audio_stream(struct snd_usb_audio *chip, int stream, struct audioformat *fp) { - struct list_head *p; struct snd_usb_stream *as; struct snd_usb_substream *subs; struct snd_pcm *pcm; int err;
- list_for_each(p, &chip->pcm_list) { - as = list_entry(p, struct snd_usb_stream, list); + list_for_each_entry(as, &chip->pcm_list, list) { if (as->fmt_type != fp->fmt_type) continue; subs = &as->substream[stream]; @@ -332,8 +329,7 @@ int snd_usb_add_audio_stream(struct snd_usb_audio *chip, } } /* look for an empty stream */ - list_for_each(p, &chip->pcm_list) { - as = list_entry(p, struct snd_usb_stream, list); + list_for_each_entry(as, &chip->pcm_list, list) { if (as->fmt_type != fp->fmt_type) continue; subs = &as->substream[stream];
Minor style fix, following a general code style in the kernel.
Signed-off-by: Eldad Zack eldad@fogrefinery.com --- sound/usb/card.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/sound/usb/card.c b/sound/usb/card.c index 8bab36c..314e8a2 100644 --- a/sound/usb/card.c +++ b/sound/usb/card.c @@ -714,8 +714,7 @@ static struct usb_device_id usb_audio_ids [] = { .bInterfaceSubClass = USB_SUBCLASS_AUDIOCONTROL }, { } /* Terminating entry */ }; - -MODULE_DEVICE_TABLE (usb, usb_audio_ids); +MODULE_DEVICE_TABLE(usb, usb_audio_ids);
/* * entry point for linux usb interface
Put EXPORT_SYMBOLS directly under the exported function.
Signed-off-by: Eldad Zack eldad@fogrefinery.com --- sound/usb/midi.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/sound/usb/midi.c b/sound/usb/midi.c index 1cf943d..60c7aa7 100644 --- a/sound/usb/midi.c +++ b/sound/usb/midi.c @@ -1455,6 +1455,7 @@ void snd_usbmidi_disconnect(struct list_head* p) } del_timer_sync(&umidi->error_timer); } +EXPORT_SYMBOL(snd_usbmidi_disconnect);
static void snd_usbmidi_rawmidi_free(struct snd_rawmidi *rmidi) { @@ -2090,6 +2091,7 @@ void snd_usbmidi_input_stop(struct list_head* p) } umidi->input_running = 0; } +EXPORT_SYMBOL(snd_usbmidi_input_stop);
static void snd_usbmidi_input_start_ep(struct snd_usb_midi_in_endpoint* ep) { @@ -2119,6 +2121,7 @@ void snd_usbmidi_input_start(struct list_head* p) snd_usbmidi_input_start_ep(umidi->endpoints[i].in); umidi->input_running = 1; } +EXPORT_SYMBOL(snd_usbmidi_input_start);
/* * Creates and registers everything needed for a MIDI streaming interface. @@ -2258,8 +2261,4 @@ int snd_usbmidi_create(struct snd_card *card, list_add_tail(&umidi->list, midi_list); return 0; } - EXPORT_SYMBOL(snd_usbmidi_create); -EXPORT_SYMBOL(snd_usbmidi_input_stop); -EXPORT_SYMBOL(snd_usbmidi_input_start); -EXPORT_SYMBOL(snd_usbmidi_disconnect);
Correct spelling of snd_usb_endpoint_implict_feedback_sink in all occurances.
Signed-off-by: Eldad Zack eldad@fogrefinery.com --- sound/usb/endpoint.c | 14 +++++++------- sound/usb/endpoint.h | 2 +- sound/usb/pcm.c | 2 +- 3 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c index b1f687f..7e9c55a 100644 --- a/sound/usb/endpoint.c +++ b/sound/usb/endpoint.c @@ -128,7 +128,7 @@ static const char *usb_error_string(int err) * Determine whether an endpoint is driven by an implicit feedback * data endpoint source. */ -int snd_usb_endpoint_implict_feedback_sink(struct snd_usb_endpoint *ep) +int snd_usb_endpoint_implicit_feedback_sink(struct snd_usb_endpoint *ep) { return ep->sync_master && ep->sync_master->type == SND_USB_ENDPOINT_TYPE_DATA && @@ -363,7 +363,7 @@ static void snd_complete_urb(struct urb *urb) if (unlikely(!test_bit(EP_FLAG_RUNNING, &ep->flags))) goto exit_clear;
- if (snd_usb_endpoint_implict_feedback_sink(ep)) { + if (snd_usb_endpoint_implicit_feedback_sink(ep)) { unsigned long flags;
spin_lock_irqsave(&ep->lock, flags); @@ -605,7 +605,7 @@ static int data_ep_set_params(struct snd_usb_endpoint *ep, else packs_per_ms = 1;
- if (is_playback && !snd_usb_endpoint_implict_feedback_sink(ep)) { + if (is_playback && !snd_usb_endpoint_implicit_feedback_sink(ep)) { urb_packs = max(ep->chip->nrpacks, 1); urb_packs = min(urb_packs, (unsigned int) MAX_PACKS); } else { @@ -614,11 +614,11 @@ static int data_ep_set_params(struct snd_usb_endpoint *ep,
urb_packs *= packs_per_ms;
- if (sync_ep && !snd_usb_endpoint_implict_feedback_sink(ep)) + if (sync_ep && !snd_usb_endpoint_implicit_feedback_sink(ep)) urb_packs = min(urb_packs, 1U << sync_ep->syncinterval);
/* decide how many packets to be used */ - if (is_playback && !snd_usb_endpoint_implict_feedback_sink(ep)) { + if (is_playback && !snd_usb_endpoint_implicit_feedback_sink(ep)) { unsigned int minsize, maxpacks; /* determine how small a packet can be */ minsize = (ep->freqn >> (16 - ep->datainterval)) @@ -845,7 +845,7 @@ int snd_usb_endpoint_start(struct snd_usb_endpoint *ep, bool can_sleep)
set_bit(EP_FLAG_RUNNING, &ep->flags);
- if (snd_usb_endpoint_implict_feedback_sink(ep)) { + if (snd_usb_endpoint_implicit_feedback_sink(ep)) { for (i = 0; i < ep->nurbs; i++) { struct snd_urb_ctx *ctx = ep->urb + i; list_add_tail(&ctx->ready_list, &ep->ready_playback_urbs); @@ -988,7 +988,7 @@ void snd_usb_handle_sync_urb(struct snd_usb_endpoint *ep, * and add it to the list of pending urbs. queue_pending_output_urbs() * will take care of them later. */ - if (snd_usb_endpoint_implict_feedback_sink(ep) && + if (snd_usb_endpoint_implicit_feedback_sink(ep) && ep->use_count != 0) {
/* implicit feedback case */ diff --git a/sound/usb/endpoint.h b/sound/usb/endpoint.h index 447902d..2287adf 100644 --- a/sound/usb/endpoint.h +++ b/sound/usb/endpoint.h @@ -23,7 +23,7 @@ int snd_usb_endpoint_activate(struct snd_usb_endpoint *ep); int snd_usb_endpoint_deactivate(struct snd_usb_endpoint *ep); void snd_usb_endpoint_free(struct list_head *head);
-int snd_usb_endpoint_implict_feedback_sink(struct snd_usb_endpoint *ep); +int snd_usb_endpoint_implicit_feedback_sink(struct snd_usb_endpoint *ep); int snd_usb_endpoint_next_packet_size(struct snd_usb_endpoint *ep);
void snd_usb_handle_sync_urb(struct snd_usb_endpoint *ep, diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c index 652724c..754cb5b 100644 --- a/sound/usb/pcm.c +++ b/sound/usb/pcm.c @@ -1271,7 +1271,7 @@ static void prepare_playback_urb(struct snd_usb_substream *subs, } } if (period_elapsed && - !snd_usb_endpoint_implict_feedback_sink(subs->data_endpoint)) /* finish at the period boundary */ + !snd_usb_endpoint_implicit_feedback_sink(subs->data_endpoint)) /* finish at the period boundary */ break; } bytes = frames * stride;
Replace the endianness conversions with the kernel-wide swabbing macros in get/set_sample_rate_v2.
Signed-off-by: Eldad Zack eldad@fogrefinery.com --- sound/usb/clock.c | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-)
diff --git a/sound/usb/clock.c b/sound/usb/clock.c index e3ccf3d..013b346 100644 --- a/sound/usb/clock.c +++ b/sound/usb/clock.c @@ -251,21 +251,21 @@ static int get_sample_rate_v2(struct snd_usb_audio *chip, int iface, int altsetting, int clock) { struct usb_device *dev = chip->dev; - unsigned char data[4]; + __le32 data; int err;
err = snd_usb_ctl_msg(dev, usb_rcvctrlpipe(dev, 0), UAC2_CS_CUR, USB_TYPE_CLASS | USB_RECIP_INTERFACE | USB_DIR_IN, UAC2_CS_CONTROL_SAM_FREQ << 8, snd_usb_ctrl_intf(chip) | (clock << 8), - data, sizeof(data)); + &data, sizeof(data)); if (err < 0) { snd_printk(KERN_WARNING "%d:%d:%d: cannot get freq (v2)\n", dev->devnum, iface, altsetting); return 0; }
- return data[0] | (data[1] << 8) | (data[2] << 16) | (data[3] << 24); + return le32_to_cpu(data); }
static int set_sample_rate_v2(struct snd_usb_audio *chip, int iface, @@ -273,7 +273,7 @@ static int set_sample_rate_v2(struct snd_usb_audio *chip, int iface, struct audioformat *fmt, int rate) { struct usb_device *dev = chip->dev; - unsigned char data[4]; + __le32 data; int err, cur_rate, prev_rate; int clock = snd_usb_clock_find_source(chip, fmt->clock);
@@ -289,15 +289,12 @@ static int set_sample_rate_v2(struct snd_usb_audio *chip, int iface,
prev_rate = get_sample_rate_v2(chip, iface, fmt->altsetting, clock);
- data[0] = rate; - data[1] = rate >> 8; - data[2] = rate >> 16; - data[3] = rate >> 24; + data = cpu_to_le32(rate); if ((err = snd_usb_ctl_msg(dev, usb_sndctrlpipe(dev, 0), UAC2_CS_CUR, USB_TYPE_CLASS | USB_RECIP_INTERFACE | USB_DIR_OUT, UAC2_CS_CONTROL_SAM_FREQ << 8, snd_usb_ctrl_intf(chip) | (clock << 8), - data, sizeof(data))) < 0) { + &data, sizeof(data))) < 0) { snd_printk(KERN_ERR "%d:%d:%d: cannot set freq %d (v2)\n", dev->devnum, iface, fmt->altsetting, rate); return err;
Move the check that parse_audio_format_rates_v2() do after receiving the clock source entity ID directly into the find function and add a validation flag to the function.
This patch does not introduce any logic flow change.
It is provided to allow introducing automatic clock switching easier later. By moving this uac_clock_source_is_valid callsite, 2 additional callsites can be avoided.
Signed-off-by: Eldad Zack eldad@fogrefinery.com --- sound/usb/clock.c | 34 ++++++++++++++++++---------------- sound/usb/clock.h | 3 ++- sound/usb/format.c | 2 +- 3 files changed, 21 insertions(+), 18 deletions(-)
diff --git a/sound/usb/clock.c b/sound/usb/clock.c index 013b346..0c98f52 100644 --- a/sound/usb/clock.c +++ b/sound/usb/clock.c @@ -131,7 +131,8 @@ static bool uac_clock_source_is_valid(struct snd_usb_audio *chip, int source_id) }
static int __uac_clock_find_source(struct snd_usb_audio *chip, - int entity_id, unsigned long *visited) + int entity_id, unsigned long *visited, + bool validate) { struct uac_clock_source_descriptor *source; struct uac_clock_selector_descriptor *selector; @@ -148,8 +149,15 @@ static int __uac_clock_find_source(struct snd_usb_audio *chip,
/* first, see if the ID we're looking for is a clock source already */ source = snd_usb_find_clock_source(chip->ctrl_intf, entity_id); - if (source) - return source->bClockID; + if (source) { + entity_id = source->bClockID; + if (validate && !uac_clock_source_is_valid(chip, entity_id)) { + snd_printk(KERN_ERR "usb-audio:%d: clock source %d is not valid, cannot use\n", + chip->dev->devnum, entity_id); + return -ENXIO; + } + return entity_id; + }
selector = snd_usb_find_clock_selector(chip->ctrl_intf, entity_id); if (selector) { @@ -164,7 +172,7 @@ static int __uac_clock_find_source(struct snd_usb_audio *chip, /* Selector values are one-based */
if (ret > selector->bNrInPins || ret < 1) { - printk(KERN_ERR + snd_printk(KERN_ERR "%s(): selector reported illegal value, id %d, ret %d\n", __func__, selector->bClockID, ret);
@@ -172,14 +180,14 @@ static int __uac_clock_find_source(struct snd_usb_audio *chip, }
return __uac_clock_find_source(chip, selector->baCSourceID[ret-1], - visited); + visited, validate); }
/* FIXME: multipliers only act as pass-thru element for now */ multiplier = snd_usb_find_clock_multiplier(chip->ctrl_intf, entity_id); if (multiplier) return __uac_clock_find_source(chip, multiplier->bCSourceID, - visited); + visited, validate);
return -EINVAL; } @@ -195,11 +203,12 @@ static int __uac_clock_find_source(struct snd_usb_audio *chip, * * Returns the clock source UnitID (>=0) on success, or an error. */ -int snd_usb_clock_find_source(struct snd_usb_audio *chip, int entity_id) +int snd_usb_clock_find_source(struct snd_usb_audio *chip, int entity_id, + bool validate) { DECLARE_BITMAP(visited, 256); memset(visited, 0, sizeof(visited)); - return __uac_clock_find_source(chip, entity_id, visited); + return __uac_clock_find_source(chip, entity_id, visited, validate); }
static int set_sample_rate_v1(struct snd_usb_audio *chip, int iface, @@ -275,18 +284,11 @@ static int set_sample_rate_v2(struct snd_usb_audio *chip, int iface, struct usb_device *dev = chip->dev; __le32 data; int err, cur_rate, prev_rate; - int clock = snd_usb_clock_find_source(chip, fmt->clock); + int clock = snd_usb_clock_find_source(chip, fmt->clock, true);
if (clock < 0) return clock;
- if (!uac_clock_source_is_valid(chip, clock)) { - /* TODO: should we try to find valid clock setups by ourself? */ - snd_printk(KERN_ERR "%d:%d:%d: clock source %d is not valid, cannot use\n", - dev->devnum, iface, fmt->altsetting, clock); - return -ENXIO; - } - prev_rate = get_sample_rate_v2(chip, iface, fmt->altsetting, clock);
data = cpu_to_le32(rate); diff --git a/sound/usb/clock.h b/sound/usb/clock.h index 4663093..d592e4a 100644 --- a/sound/usb/clock.h +++ b/sound/usb/clock.h @@ -5,6 +5,7 @@ int snd_usb_init_sample_rate(struct snd_usb_audio *chip, int iface, struct usb_host_interface *alts, struct audioformat *fmt, int rate);
-int snd_usb_clock_find_source(struct snd_usb_audio *chip, int entity_id); +int snd_usb_clock_find_source(struct snd_usb_audio *chip, int entity_id, + bool validate);
#endif /* __USBAUDIO_CLOCK_H */ diff --git a/sound/usb/format.c b/sound/usb/format.c index a695caf..20c7751 100644 --- a/sound/usb/format.c +++ b/sound/usb/format.c @@ -280,7 +280,7 @@ static int parse_audio_format_rates_v2(struct snd_usb_audio *chip, struct usb_device *dev = chip->dev; unsigned char tmp[2], *data; int nr_triplets, data_size, ret = 0; - int clock = snd_usb_clock_find_source(chip, fp->clock); + int clock = snd_usb_clock_find_source(chip, fp->clock, false);
if (clock < 0) { snd_printk(KERN_ERR "%s(): unable to find clock source (clock %d)\n",
If a selector is available on a device, it may be pointing to a clock source which is currently invalid. If there is a valid clock source which can be selected, switch to it.
Signed-off-by: Eldad Zack eldad@fogrefinery.com --- sound/usb/clock.c | 69 ++++++++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 66 insertions(+), 3 deletions(-)
diff --git a/sound/usb/clock.c b/sound/usb/clock.c index 0c98f52..d7ab2d7 100644 --- a/sound/usb/clock.c +++ b/sound/usb/clock.c @@ -99,6 +99,41 @@ static int uac_clock_selector_get_val(struct snd_usb_audio *chip, int selector_i return buf; }
+static int uac_clock_selector_set_val(struct snd_usb_audio *chip, int selector_id, + unsigned char pin) +{ + int ret; + + ret = snd_usb_ctl_msg(chip->dev, usb_sndctrlpipe(chip->dev, 0), + UAC2_CS_CUR, + USB_RECIP_INTERFACE | USB_TYPE_CLASS | USB_DIR_OUT, + UAC2_CX_CLOCK_SELECTOR << 8, + snd_usb_ctrl_intf(chip) | (selector_id << 8), + &pin, sizeof(pin)); + if (ret < 0) + return ret; + + if (ret != sizeof(pin)) { + snd_printk(KERN_ERR + "usb-audio:%d: setting selector (id %d) unexpected length %d\n", + chip->dev->devnum, selector_id, ret); + return -EINVAL; + } + + ret = uac_clock_selector_get_val(chip, selector_id); + if (ret < 0) + return ret; + + if (ret != pin) { + snd_printk(KERN_ERR + "usb-audio:%d: setting selector (id %d) to %x failed (current: %d)\n", + chip->dev->devnum, selector_id, pin, ret); + return -EINVAL; + } + + return ret; +} + static bool uac_clock_source_is_valid(struct snd_usb_audio *chip, int source_id) { int err; @@ -161,7 +196,7 @@ static int __uac_clock_find_source(struct snd_usb_audio *chip,
selector = snd_usb_find_clock_selector(chip->ctrl_intf, entity_id); if (selector) { - int ret; + int ret, i, cur;
/* the entity ID we are looking for is a selector. * find out what it currently selects */ @@ -179,8 +214,35 @@ static int __uac_clock_find_source(struct snd_usb_audio *chip, return -EINVAL; }
- return __uac_clock_find_source(chip, selector->baCSourceID[ret-1], + cur = ret; + ret = __uac_clock_find_source(chip, selector->baCSourceID[ret - 1], visited, validate); + if (!validate || ret > 0) + return ret; + + /* The current clock source is invalid, try others. */ + for (i = 1; i <= selector->bNrInPins; i++) { + int err; + + if (i == cur) + continue; + + ret = __uac_clock_find_source(chip, selector->baCSourceID[i - 1], + visited, true); + if (ret < 0) + continue; + + err = uac_clock_selector_set_val(chip, entity_id, i); + if (err < 0) + continue; + + snd_printk(KERN_INFO + "usb-audio:%d: found and selected valid clock source %d\n", + chip->dev->devnum, ret); + return ret; + } + + return -ENXIO; }
/* FIXME: multipliers only act as pass-thru element for now */ @@ -284,8 +346,9 @@ static int set_sample_rate_v2(struct snd_usb_audio *chip, int iface, struct usb_device *dev = chip->dev; __le32 data; int err, cur_rate, prev_rate; - int clock = snd_usb_clock_find_source(chip, fmt->clock, true); + int clock;
+ clock = snd_usb_clock_find_source(chip, fmt->clock, true); if (clock < 0) return clock;
Add a module param to disable auto clock selection. This is provided for users that expect the audio stream to fail when the clock source is invalid (e.g., the word clock was unintentionally disconnected).
Signed-off-by: Eldad Zack eldad@fogrefinery.com --- sound/usb/card.c | 4 ++++ sound/usb/clock.c | 2 +- sound/usb/usbaudio.h | 1 + 3 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/sound/usb/card.c b/sound/usb/card.c index 314e8a2..5254b18 100644 --- a/sound/usb/card.c +++ b/sound/usb/card.c @@ -82,6 +82,7 @@ static int pid[SNDRV_CARDS] = { [0 ... (SNDRV_CARDS-1)] = -1 }; static int nrpacks = 8; /* max. number of packets per urb */ static int device_setup[SNDRV_CARDS]; /* device parameter for this card */ static bool ignore_ctl_error; +static bool autoclock = true;
module_param_array(index, int, NULL, 0444); MODULE_PARM_DESC(index, "Index value for the USB audio adapter."); @@ -100,6 +101,8 @@ MODULE_PARM_DESC(device_setup, "Specific device setup (if needed)."); module_param(ignore_ctl_error, bool, 0444); MODULE_PARM_DESC(ignore_ctl_error, "Ignore errors from USB controller for mixer interfaces."); +module_param(autoclock, bool, 0444); +MODULE_PARM_DESC(autoclock, "Enable auto-clock selection for UAC2 devices (default: yes).");
/* * we keep the snd_usb_audio_t instances by ourselves for merging @@ -354,6 +357,7 @@ static int snd_usb_audio_create(struct usb_device *dev, int idx, chip->card = card; chip->setup = device_setup[idx]; chip->nrpacks = nrpacks; + chip->autoclock = autoclock; chip->probing = 1;
chip->usb_id = USB_ID(le16_to_cpu(dev->descriptor.idVendor), diff --git a/sound/usb/clock.c b/sound/usb/clock.c index d7ab2d7..e59d359 100644 --- a/sound/usb/clock.c +++ b/sound/usb/clock.c @@ -217,7 +217,7 @@ static int __uac_clock_find_source(struct snd_usb_audio *chip, cur = ret; ret = __uac_clock_find_source(chip, selector->baCSourceID[ret - 1], visited, validate); - if (!validate || ret > 0) + if (!validate || ret > 0 || !chip->autoclock) return ret;
/* The current clock source is invalid, try others. */ diff --git a/sound/usb/usbaudio.h b/sound/usb/usbaudio.h index 1ac3fd9..bc43bca 100644 --- a/sound/usb/usbaudio.h +++ b/sound/usb/usbaudio.h @@ -56,6 +56,7 @@ struct snd_usb_audio {
int setup; /* from the 'device_setup' module param */ int nrpacks; /* from the 'nrpacks' module param */ + bool autoclock; /* from the 'autoclock' module param */
struct usb_host_interface *ctrl_intf; /* the audio control interface */ };
Show the error code returned from the USB subsystem in the debug messages.
Signed-off-by: Eldad Zack eldad@fogrefinery.com --- sound/usb/clock.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/sound/usb/clock.c b/sound/usb/clock.c index e59d359..a694e1c 100644 --- a/sound/usb/clock.c +++ b/sound/usb/clock.c @@ -331,8 +331,8 @@ static int get_sample_rate_v2(struct snd_usb_audio *chip, int iface, snd_usb_ctrl_intf(chip) | (clock << 8), &data, sizeof(data)); if (err < 0) { - snd_printk(KERN_WARNING "%d:%d:%d: cannot get freq (v2)\n", - dev->devnum, iface, altsetting); + snd_printk(KERN_WARNING "%d:%d:%d: cannot get freq (v2): err %d\n", + dev->devnum, iface, altsetting, err); return 0; }
@@ -360,8 +360,8 @@ static int set_sample_rate_v2(struct snd_usb_audio *chip, int iface, UAC2_CS_CONTROL_SAM_FREQ << 8, snd_usb_ctrl_intf(chip) | (clock << 8), &data, sizeof(data))) < 0) { - snd_printk(KERN_ERR "%d:%d:%d: cannot set freq %d (v2)\n", - dev->devnum, iface, fmt->altsetting, rate); + snd_printk(KERN_ERR "%d:%d:%d: cannot set freq %d (v2): err %d\n", + dev->devnum, iface, fmt->altsetting, rate, err); return err; }
Some clocks might be read-only, e.g., external clocks (see also UAC2 4.7.2.1).
In this case, setting the sample frequency will always fail (even if the rate is equal to the current clock rate), therefore do not write, but read the value and compare to the requested rate. If the clock is read only, avoid reading it twice.
If it doesn't match, return -ENXIO since the clock is invalid for this configuration.
Signed-off-by: Eldad Zack eldad@fogrefinery.com --- sound/usb/clock.c | 37 ++++++++++++++++++++++++++----------- 1 file changed, 26 insertions(+), 11 deletions(-)
diff --git a/sound/usb/clock.c b/sound/usb/clock.c index a694e1c..ae35e7d 100644 --- a/sound/usb/clock.c +++ b/sound/usb/clock.c @@ -347,6 +347,8 @@ static int set_sample_rate_v2(struct snd_usb_audio *chip, int iface, __le32 data; int err, cur_rate, prev_rate; int clock; + bool writeable; + struct uac_clock_source_descriptor *cs_desc;
clock = snd_usb_clock_find_source(chip, fmt->clock, true); if (clock < 0) @@ -354,20 +356,33 @@ static int set_sample_rate_v2(struct snd_usb_audio *chip, int iface,
prev_rate = get_sample_rate_v2(chip, iface, fmt->altsetting, clock);
- data = cpu_to_le32(rate); - if ((err = snd_usb_ctl_msg(dev, usb_sndctrlpipe(dev, 0), UAC2_CS_CUR, - USB_TYPE_CLASS | USB_RECIP_INTERFACE | USB_DIR_OUT, - UAC2_CS_CONTROL_SAM_FREQ << 8, - snd_usb_ctrl_intf(chip) | (clock << 8), - &data, sizeof(data))) < 0) { - snd_printk(KERN_ERR "%d:%d:%d: cannot set freq %d (v2): err %d\n", - dev->devnum, iface, fmt->altsetting, rate, err); - return err; - } + cs_desc = snd_usb_find_clock_source(chip->ctrl_intf, clock); + writeable = uac2_control_is_writeable(cs_desc->bmControls, UAC2_CS_CONTROL_SAM_FREQ - 1); + if (writeable) { + data = cpu_to_le32(rate); + err = snd_usb_ctl_msg(dev, usb_sndctrlpipe(dev, 0), UAC2_CS_CUR, + USB_TYPE_CLASS | USB_RECIP_INTERFACE | USB_DIR_OUT, + UAC2_CS_CONTROL_SAM_FREQ << 8, + snd_usb_ctrl_intf(chip) | (clock << 8), + &data, sizeof(data)); + if (err < 0) { + snd_printk(KERN_ERR "%d:%d:%d: cannot set freq %d (v2): err %d\n", + dev->devnum, iface, fmt->altsetting, rate, err); + return err; + }
- cur_rate = get_sample_rate_v2(chip, iface, fmt->altsetting, clock); + cur_rate = get_sample_rate_v2(chip, iface, fmt->altsetting, clock); + } else { + cur_rate = prev_rate; + }
if (cur_rate != rate) { + if (!writeable) { + snd_printk(KERN_WARNING + "%d:%d:%d: freq mismatch (RO clock): req %d, clock runs @%d\n", + dev->devnum, iface, fmt->altsetting, rate, cur_rate); + return -ENXIO; + } snd_printd(KERN_WARNING "current rate %d is different from the runtime rate %d\n", cur_rate, rate);
At Wed, 3 Apr 2013 23:18:48 +0200, Eldad Zack wrote:
This is version 2 of the automatic clock switching. v1: http://mailman.alsa-project.org/pipermail/alsa-devel/2013-March/060734.html
No patches added or removed. Rebased patches with "ALSA: usb: Work around CM6631 sample rate change bug", commit 690a863ff03d9a29ace2b752b8f802fba78a842f .
Clemens, Takashi and Torstein, thank you very much for reviewing and providing comments!
Changes:
- Fixed formatting
- (#5) Fixed type cast: u32 -> __l32
- (#6) Added rational to commit message
- (#7) Fixed assignment of int to unsigned char, return error code if encountered
- (#8) Fixed the parameter name, annotated the default value
- (#10) Avoid second read of frequency for read-only clocks
Tested with Takashi's for-next. Applies against Takashi's for-next HEAD efc33ce197e4b6aaddf1eb2ba6293f51daf3c283 .
Applied all 10 patches now. Thanks.
Takashi
Cheers,
Eldad Zack (10): ALSA: usb-audio: convert list_for_each to entry variant ALSA: usb-audio: neaten MODULE_DEVICE_TABLE placement ALSA: usb-audio: neaten EXPORT_SYMBOLS placement ALSA: usb-audio: spelling correction ALSA: usb-audio: use endianness macros ALSA: usb-audio: UAC2: do clock validity check earlier ALSA: usb-audio: UAC2: try to find and switch to valid clock ALSA: usb-audio: UAC2: auto clock selection module param ALSA: usb-audio: show err in set_sample_rate_v2 debug ALSA: usb-audio: UAC2: support read-only freq control
sound/usb/card.c | 11 ++-- sound/usb/clock.c | 153 ++++++++++++++++++++++++++++++++++++++------------- sound/usb/clock.h | 3 +- sound/usb/endpoint.c | 18 +++--- sound/usb/endpoint.h | 2 +- sound/usb/format.c | 2 +- sound/usb/midi.c | 12 ++-- sound/usb/pcm.c | 32 ++++------- sound/usb/proc.c | 7 +-- sound/usb/stream.c | 12 ++-- sound/usb/usbaudio.h | 1 + 11 files changed, 157 insertions(+), 96 deletions(-)
-- 1.8.1.5
Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
participants (2)
-
Eldad Zack -
Takashi Iwai