* [PATCH 01/10] ALSA: usb-audio: convert list_for_each to entry variant
2013-03-31 15:52 [PATCH 00/10] UAC2: Automatic clock switching Eldad Zack
@ 2013-03-31 15:52 ` Eldad Zack
2013-03-31 15:52 ` [PATCH 02/10] ALSA: usb-audio: neaten MODULE_DEVICE_TABLE placement Eldad Zack
` (8 subsequent siblings)
9 siblings, 0 replies; 24+ messages in thread
From: Eldad Zack @ 2013-03-31 15:52 UTC (permalink / raw)
To: Jaroslav Kysela, Takashi Iwai, Clemens Ladisch, Daniel Mack
Cc: alsa-devel, Eldad Zack
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 f94397b..e42fc19 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)
@@ -802,7 +800,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;
@@ -810,9 +808,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++) {
@@ -856,7 +852,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;
@@ -864,9 +860,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++) {
@@ -909,7 +903,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];
@@ -917,9 +911,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;
@@ -1027,7 +1019,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;
@@ -1041,9 +1033,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 ad181d5..42b7e0a 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];
--
1.8.1.5
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH 02/10] ALSA: usb-audio: neaten MODULE_DEVICE_TABLE placement
2013-03-31 15:52 [PATCH 00/10] UAC2: Automatic clock switching Eldad Zack
2013-03-31 15:52 ` [PATCH 01/10] ALSA: usb-audio: convert list_for_each to entry variant Eldad Zack
@ 2013-03-31 15:52 ` Eldad Zack
2013-03-31 15:52 ` [PATCH 03/10] ALSA: usb-audio: neaten EXPORT_SYMBOLS placement Eldad Zack
` (7 subsequent siblings)
9 siblings, 0 replies; 24+ messages in thread
From: Eldad Zack @ 2013-03-31 15:52 UTC (permalink / raw)
To: Jaroslav Kysela, Takashi Iwai, Clemens Ladisch, Daniel Mack
Cc: alsa-devel, Eldad Zack
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
--
1.8.1.5
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH 03/10] ALSA: usb-audio: neaten EXPORT_SYMBOLS placement
2013-03-31 15:52 [PATCH 00/10] UAC2: Automatic clock switching Eldad Zack
2013-03-31 15:52 ` [PATCH 01/10] ALSA: usb-audio: convert list_for_each to entry variant Eldad Zack
2013-03-31 15:52 ` [PATCH 02/10] ALSA: usb-audio: neaten MODULE_DEVICE_TABLE placement Eldad Zack
@ 2013-03-31 15:52 ` Eldad Zack
2013-03-31 15:52 ` [PATCH 04/10] ALSA: usb-audio: spelling correction Eldad Zack
` (6 subsequent siblings)
9 siblings, 0 replies; 24+ messages in thread
From: Eldad Zack @ 2013-03-31 15:52 UTC (permalink / raw)
To: Jaroslav Kysela, Takashi Iwai, Clemens Ladisch, Daniel Mack
Cc: alsa-devel, Eldad Zack
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);
--
1.8.1.5
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH 04/10] ALSA: usb-audio: spelling correction
2013-03-31 15:52 [PATCH 00/10] UAC2: Automatic clock switching Eldad Zack
` (2 preceding siblings ...)
2013-03-31 15:52 ` [PATCH 03/10] ALSA: usb-audio: neaten EXPORT_SYMBOLS placement Eldad Zack
@ 2013-03-31 15:52 ` Eldad Zack
2013-03-31 15:52 ` [PATCH 05/10] ALSA: usb-audio: use endianness macros Eldad Zack
` (5 subsequent siblings)
9 siblings, 0 replies; 24+ messages in thread
From: Eldad Zack @ 2013-03-31 15:52 UTC (permalink / raw)
To: Jaroslav Kysela, Takashi Iwai, Clemens Ladisch, Daniel Mack
Cc: alsa-devel, Eldad Zack
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 e42fc19..9531355 100644
--- a/sound/usb/pcm.c
+++ b/sound/usb/pcm.c
@@ -1264,7 +1264,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;
--
1.8.1.5
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH 05/10] ALSA: usb-audio: use endianness macros
2013-03-31 15:52 [PATCH 00/10] UAC2: Automatic clock switching Eldad Zack
` (3 preceding siblings ...)
2013-03-31 15:52 ` [PATCH 04/10] ALSA: usb-audio: spelling correction Eldad Zack
@ 2013-03-31 15:52 ` Eldad Zack
2013-03-31 16:06 ` Clemens Ladisch
2013-03-31 15:52 ` [PATCH 06/10] ALSA: usb-audio: UAC2: do clock validity check earlier Eldad Zack
` (4 subsequent siblings)
9 siblings, 1 reply; 24+ messages in thread
From: Eldad Zack @ 2013-03-31 15:52 UTC (permalink / raw)
To: Jaroslav Kysela, Takashi Iwai, Clemens Ladisch, Daniel Mack
Cc: alsa-devel, Eldad Zack
Replace the endianness conversions with the kernel-wide swabbing macros.
Signed-off-by: Eldad Zack <eldad@fogrefinery.com>
---
sound/usb/clock.c | 13 +++++--------
1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/sound/usb/clock.c b/sound/usb/clock.c
index 5e634a2..47b601d 100644
--- a/sound/usb/clock.c
+++ b/sound/usb/clock.c
@@ -252,7 +252,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];
+ u32 data;
int err, crate;
int clock = snd_usb_clock_find_source(chip, fmt->clock);
@@ -266,15 +266,12 @@ static int set_sample_rate_v2(struct snd_usb_audio *chip, int iface,
return -ENXIO;
}
- 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;
@@ -284,13 +281,13 @@ static int set_sample_rate_v2(struct snd_usb_audio *chip, int iface,
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))) < 0) {
+ &data, sizeof(rate))) < 0) {
snd_printk(KERN_WARNING "%d:%d:%d: cannot get freq (v2)\n",
dev->devnum, iface, fmt->altsetting);
return err;
}
- crate = data[0] | (data[1] << 8) | (data[2] << 16) | (data[3] << 24);
+ crate = le32_to_cpu(data);
if (crate != rate)
snd_printd(KERN_WARNING "current rate %d is different from the runtime rate %d\n", crate, rate);
--
1.8.1.5
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH 05/10] ALSA: usb-audio: use endianness macros
2013-03-31 15:52 ` [PATCH 05/10] ALSA: usb-audio: use endianness macros Eldad Zack
@ 2013-03-31 16:06 ` Clemens Ladisch
2013-03-31 17:15 ` Eldad Zack
0 siblings, 1 reply; 24+ messages in thread
From: Clemens Ladisch @ 2013-03-31 16:06 UTC (permalink / raw)
To: Eldad Zack; +Cc: Takashi Iwai, alsa-devel, Daniel Mack
Eldad Zack wrote:
> Replace the endianness conversions with the kernel-wide swabbing macros.
> + u32 data;
> + data = cpu_to_le32(rate);
cpu_to_le32() returns not u32 but __le32.
Please use that to make sparse happy.
Regards,
Clemens
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 05/10] ALSA: usb-audio: use endianness macros
2013-03-31 16:06 ` Clemens Ladisch
@ 2013-03-31 17:15 ` Eldad Zack
2013-03-31 17:30 ` Clemens Ladisch
0 siblings, 1 reply; 24+ messages in thread
From: Eldad Zack @ 2013-03-31 17:15 UTC (permalink / raw)
To: Clemens Ladisch; +Cc: Takashi Iwai, alsa-devel, Daniel Mack
Hi Clemens,
On Sun, 31 Mar 2013, Clemens Ladisch wrote:
> Eldad Zack wrote:
> > Replace the endianness conversions with the kernel-wide swabbing macros.
>
> > + u32 data;
> > + data = cpu_to_le32(rate);
>
> cpu_to_le32() returns not u32 but __le32.
> Please use that to make sparse happy.
Thanks! Fixed locally. This propogates along the series, so I'll wait
for other comments to repost.
BTW, sparse (0.4.3) didn't complain on my box. Maybe I'm doing it wrong?
I'm just calling make C=1.
Cheers,
Eldad
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 05/10] ALSA: usb-audio: use endianness macros
2013-03-31 17:15 ` Eldad Zack
@ 2013-03-31 17:30 ` Clemens Ladisch
2013-04-05 18:36 ` Eldad Zack
0 siblings, 1 reply; 24+ messages in thread
From: Clemens Ladisch @ 2013-03-31 17:30 UTC (permalink / raw)
To: Eldad Zack; +Cc: Takashi Iwai, alsa-devel, Daniel Mack
Eldad Zack wrote:
> On Sun, 31 Mar 2013, Clemens Ladisch wrote:
>> cpu_to_le32() returns not u32 but __le32.
>> Please use that to make sparse happy.
>
> BTW, sparse (0.4.3) didn't complain on my box.
> I'm just calling make C=1.
make C=1 CF=-D__CHECK_ENDIAN__
Regards,
Clemens
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 05/10] ALSA: usb-audio: use endianness macros
2013-03-31 17:30 ` Clemens Ladisch
@ 2013-04-05 18:36 ` Eldad Zack
0 siblings, 0 replies; 24+ messages in thread
From: Eldad Zack @ 2013-04-05 18:36 UTC (permalink / raw)
To: Clemens Ladisch; +Cc: Takashi Iwai, alsa-devel, Daniel Mack
On Sun, 31 Mar 2013, Clemens Ladisch wrote:
> Eldad Zack wrote:
> > On Sun, 31 Mar 2013, Clemens Ladisch wrote:
> >> cpu_to_le32() returns not u32 but __le32.
> >> Please use that to make sparse happy.
> >
> > BTW, sparse (0.4.3) didn't complain on my box.
> > I'm just calling make C=1.
>
> make C=1 CF=-D__CHECK_ENDIAN__
Heh, I felt quite silly about this question now that I read
Documentation/sparse.txt to the end :)
I see why you care so much about it too. I just found 3 places
with endian bugs thanks to sparse, where usb_control_msg() is
handed a LE instead of CPU native.
snd_nativeinstruments_control_get()/put() and
snd_usb_nativeinstruments_boot_quirk do this (I'll post a patch to
fix this soon).
Cheers,
Eldad
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 06/10] ALSA: usb-audio: UAC2: do clock validity check earlier
2013-03-31 15:52 [PATCH 00/10] UAC2: Automatic clock switching Eldad Zack
` (4 preceding siblings ...)
2013-03-31 15:52 ` [PATCH 05/10] ALSA: usb-audio: use endianness macros Eldad Zack
@ 2013-03-31 15:52 ` Eldad Zack
2013-04-01 8:16 ` Torstein Hegge
2013-04-02 8:38 ` Takashi Iwai
2013-03-31 15:52 ` [PATCH 07/10] ALSA: usb-audio: UAC2: try to find and switch to valid clock Eldad Zack
` (3 subsequent siblings)
9 siblings, 2 replies; 24+ messages in thread
From: Eldad Zack @ 2013-03-31 15:52 UTC (permalink / raw)
To: Jaroslav Kysela, Takashi Iwai, Clemens Ladisch, Daniel Mack
Cc: alsa-devel, Eldad Zack
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.
Signed-off-by: Eldad Zack <eldad@fogrefinery.com>
---
sound/usb/clock.c | 35 +++++++++++++++++++----------------
sound/usb/clock.h | 3 ++-
sound/usb/format.c | 2 +-
3 files changed, 22 insertions(+), 18 deletions(-)
diff --git a/sound/usb/clock.c b/sound/usb/clock.c
index 47b601d..08fa345 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,
@@ -254,18 +263,12 @@ static int set_sample_rate_v2(struct snd_usb_audio *chip, int iface,
struct usb_device *dev = chip->dev;
u32 data;
int err, crate;
- int clock = snd_usb_clock_find_source(chip, fmt->clock);
+ int clock;
+ 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;
- }
-
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,
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 e831ee4..1086b87 100644
--- a/sound/usb/format.c
+++ b/sound/usb/format.c
@@ -277,7 +277,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",
--
1.8.1.5
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH 06/10] ALSA: usb-audio: UAC2: do clock validity check earlier
2013-03-31 15:52 ` [PATCH 06/10] ALSA: usb-audio: UAC2: do clock validity check earlier Eldad Zack
@ 2013-04-01 8:16 ` Torstein Hegge
2013-04-01 22:36 ` Eldad Zack
2013-04-02 8:38 ` Takashi Iwai
1 sibling, 1 reply; 24+ messages in thread
From: Torstein Hegge @ 2013-04-01 8:16 UTC (permalink / raw)
To: Eldad Zack; +Cc: Takashi Iwai, alsa-devel, Daniel Mack, Clemens Ladisch
On Sun, Mar 31, 2013 at 17:52:28 +0200, Eldad Zack wrote:
> 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.
What would be lost by letting __uac_clock_find_source() always check
that the clock source is valid? It would avoid having to pass the
validate parameter, at the cost of having parse_audio_format_rates_v2()
validate clock source.
Torstein
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 06/10] ALSA: usb-audio: UAC2: do clock validity check earlier
2013-04-01 8:16 ` Torstein Hegge
@ 2013-04-01 22:36 ` Eldad Zack
0 siblings, 0 replies; 24+ messages in thread
From: Eldad Zack @ 2013-04-01 22:36 UTC (permalink / raw)
To: Torstein Hegge; +Cc: Takashi Iwai, alsa-devel, Daniel Mack, Clemens Ladisch
Hi Torstein,
On Mon, 1 Apr 2013, Torstein Hegge wrote:
> On Sun, Mar 31, 2013 at 17:52:28 +0200, Eldad Zack wrote:
> > 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.
>
> What would be lost by letting __uac_clock_find_source() always check
> that the clock source is valid? It would avoid having to pass the
> validate parameter, at the cost of having parse_audio_format_rates_v2()
> validate clock source.
I didn't want to change that logic since (1) I can't test it, my device
doesn't go through that code and (2) failing there on account of that
check would error out and not support the device (as far as I
understand from the code). So if a (theoretical) device would only
return valid one minute after it is connected, this will cause a
regression. I might be wrong, but I figured it's better to do one thing
at a time.
Cheers,
Eldad
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 06/10] ALSA: usb-audio: UAC2: do clock validity check earlier
2013-03-31 15:52 ` [PATCH 06/10] ALSA: usb-audio: UAC2: do clock validity check earlier Eldad Zack
2013-04-01 8:16 ` Torstein Hegge
@ 2013-04-02 8:38 ` Takashi Iwai
1 sibling, 0 replies; 24+ messages in thread
From: Takashi Iwai @ 2013-04-02 8:38 UTC (permalink / raw)
To: Eldad Zack; +Cc: alsa-devel, Daniel Mack, Clemens Ladisch
At Sun, 31 Mar 2013 17:52:28 +0200,
Eldad Zack wrote:
>
> 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.
Please give a few more words why this change is introduced.
thanks,
Takashi
>
> Signed-off-by: Eldad Zack <eldad@fogrefinery.com>
> ---
> sound/usb/clock.c | 35 +++++++++++++++++++----------------
> sound/usb/clock.h | 3 ++-
> sound/usb/format.c | 2 +-
> 3 files changed, 22 insertions(+), 18 deletions(-)
>
> diff --git a/sound/usb/clock.c b/sound/usb/clock.c
> index 47b601d..08fa345 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,
> @@ -254,18 +263,12 @@ static int set_sample_rate_v2(struct snd_usb_audio *chip, int iface,
> struct usb_device *dev = chip->dev;
> u32 data;
> int err, crate;
> - int clock = snd_usb_clock_find_source(chip, fmt->clock);
> + int clock;
>
> + 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;
> - }
> -
> 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,
> 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 e831ee4..1086b87 100644
> --- a/sound/usb/format.c
> +++ b/sound/usb/format.c
> @@ -277,7 +277,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",
> --
> 1.8.1.5
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 07/10] ALSA: usb-audio: UAC2: try to find and switch to valid clock
2013-03-31 15:52 [PATCH 00/10] UAC2: Automatic clock switching Eldad Zack
` (5 preceding siblings ...)
2013-03-31 15:52 ` [PATCH 06/10] ALSA: usb-audio: UAC2: do clock validity check earlier Eldad Zack
@ 2013-03-31 15:52 ` Eldad Zack
2013-04-02 8:46 ` Takashi Iwai
2013-03-31 15:52 ` [PATCH 08/10] ALSA: usb-audio: UAC2: auto clock selection module param Eldad Zack
` (2 subsequent siblings)
9 siblings, 1 reply; 24+ messages in thread
From: Eldad Zack @ 2013-03-31 15:52 UTC (permalink / raw)
To: Jaroslav Kysela, Takashi Iwai, Clemens Ladisch, Daniel Mack
Cc: alsa-devel, Eldad Zack
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 | 65 +++++++++++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 63 insertions(+), 2 deletions(-)
diff --git a/sound/usb/clock.c b/sound/usb/clock.c
index 08fa345..6b79e25 100644
--- a/sound/usb/clock.c
+++ b/sound/usb/clock.c
@@ -99,6 +99,40 @@ 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)
+{
+ unsigned char buf;
+ 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;
+ }
+
+ buf = uac_clock_selector_get_val(chip, selector_id);
+ if (buf != pin) {
+ snd_printk(KERN_ERR
+ "usb-audio:%d: setting selector (id %d) failed (wanted %x, got %x)\n",
+ chip->dev->devnum, selector_id, pin, buf);
+ return -EINVAL;
+ }
+
+ return buf;
+}
+
static bool uac_clock_source_is_valid(struct snd_usb_audio *chip, int source_id)
{
int err;
@@ -161,7 +195,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 +213,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 */
--
1.8.1.5
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH 07/10] ALSA: usb-audio: UAC2: try to find and switch to valid clock
2013-03-31 15:52 ` [PATCH 07/10] ALSA: usb-audio: UAC2: try to find and switch to valid clock Eldad Zack
@ 2013-04-02 8:46 ` Takashi Iwai
2013-04-02 20:20 ` Eldad Zack
0 siblings, 1 reply; 24+ messages in thread
From: Takashi Iwai @ 2013-04-02 8:46 UTC (permalink / raw)
To: Eldad Zack; +Cc: alsa-devel, Daniel Mack, Clemens Ladisch
At Sun, 31 Mar 2013 17:52:29 +0200,
Eldad Zack wrote:
>
> 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 | 65 +++++++++++++++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 63 insertions(+), 2 deletions(-)
>
> diff --git a/sound/usb/clock.c b/sound/usb/clock.c
> index 08fa345..6b79e25 100644
> --- a/sound/usb/clock.c
> +++ b/sound/usb/clock.c
> @@ -99,6 +99,40 @@ 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)
> +{
> + unsigned char buf;
> + 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;
> + }
> +
> + buf = uac_clock_selector_get_val(chip, selector_id);
> + if (buf != pin) {
uac_clock_selector_get_val() returns an int with a negative value,
so it's safer to compare it as an int.
Takashi
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH 07/10] ALSA: usb-audio: UAC2: try to find and switch to valid clock
2013-04-02 8:46 ` Takashi Iwai
@ 2013-04-02 20:20 ` Eldad Zack
0 siblings, 0 replies; 24+ messages in thread
From: Eldad Zack @ 2013-04-02 20:20 UTC (permalink / raw)
To: Takashi Iwai; +Cc: alsa-devel, Daniel Mack, Clemens Ladisch
On Tue, 2 Apr 2013, Takashi Iwai wrote:
> At Sun, 31 Mar 2013 17:52:29 +0200,
> Eldad Zack wrote:
> >
> > 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 | 65 +++++++++++++++++++++++++++++++++++++++++++++++++++++--
> > 1 file changed, 63 insertions(+), 2 deletions(-)
> >
> > diff --git a/sound/usb/clock.c b/sound/usb/clock.c
> > index 08fa345..6b79e25 100644
> > --- a/sound/usb/clock.c
> > +++ b/sound/usb/clock.c
> > @@ -99,6 +99,40 @@ static int uac_clock_selector_get_val(struct snd_usb_audio *chip, int selector_i
> > + buf = uac_clock_selector_get_val(chip, selector_id);
> > + if (buf != pin) {
>
> uac_clock_selector_get_val() returns an int with a negative value,
> so it's safer to compare it as an int.
Right, and I need to return the error code, too. Thanks!
Cheers,
Eldad
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 08/10] ALSA: usb-audio: UAC2: auto clock selection module param
2013-03-31 15:52 [PATCH 00/10] UAC2: Automatic clock switching Eldad Zack
` (6 preceding siblings ...)
2013-03-31 15:52 ` [PATCH 07/10] ALSA: usb-audio: UAC2: try to find and switch to valid clock Eldad Zack
@ 2013-03-31 15:52 ` Eldad Zack
2013-04-02 8:51 ` Takashi Iwai
2013-03-31 15:52 ` [PATCH 09/10] ALSA: usb-audio: show err in set_sample_rate_v2 debug Eldad Zack
2013-03-31 15:52 ` [PATCH 10/10] ALSA: usb-audio: UAC2: support read-only freq control Eldad Zack
9 siblings, 1 reply; 24+ messages in thread
From: Eldad Zack @ 2013-03-31 15:52 UTC (permalink / raw)
To: Jaroslav Kysela, Takashi Iwai, Clemens Ladisch, Daniel Mack
Cc: alsa-devel, Eldad Zack
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..dfbf317 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 noautoclk;
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(noautoclk, bool, 0444);
+MODULE_PARM_DESC(noautoclk, "Disables auto-clock selection for UAC2 devices.");
/*
* 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->noautoclk = noautoclk;
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 6b79e25..e3075ca 100644
--- a/sound/usb/clock.c
+++ b/sound/usb/clock.c
@@ -216,7 +216,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->noautoclk)
return ret;
/* The current clock source is invalid, try others. */
diff --git a/sound/usb/usbaudio.h b/sound/usb/usbaudio.h
index 1ac3fd9..4852eb0 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 noautoclk; /* from the 'noautoclk' module param */
struct usb_host_interface *ctrl_intf; /* the audio control interface */
};
--
1.8.1.5
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH 08/10] ALSA: usb-audio: UAC2: auto clock selection module param
2013-03-31 15:52 ` [PATCH 08/10] ALSA: usb-audio: UAC2: auto clock selection module param Eldad Zack
@ 2013-04-02 8:51 ` Takashi Iwai
2013-04-02 20:27 ` Eldad Zack
0 siblings, 1 reply; 24+ messages in thread
From: Takashi Iwai @ 2013-04-02 8:51 UTC (permalink / raw)
To: Eldad Zack; +Cc: alsa-devel, Daniel Mack, Clemens Ladisch
At Sun, 31 Mar 2013 17:52:30 +0200,
Eldad Zack wrote:
>
> 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..dfbf317 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 noautoclk;
>
> 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(noautoclk, bool, 0444);
> +MODULE_PARM_DESC(noautoclk, "Disables auto-clock selection for UAC2 devices.");
Don't use "no" for a boolean option. You can pass "yes" or "no" as an
option value, so it can confuse easily by passing noautoclk=yes. See?
It's fine to use internally an inverted boolean, of course, though.
But the exposed option should be less confusing.
In addition, you don't have to use abbreviation.
The option autoclock or auto_clock should be better than autoclk.
Takashi
>
> /*
> * 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->noautoclk = noautoclk;
> 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 6b79e25..e3075ca 100644
> --- a/sound/usb/clock.c
> +++ b/sound/usb/clock.c
> @@ -216,7 +216,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->noautoclk)
> return ret;
>
> /* The current clock source is invalid, try others. */
> diff --git a/sound/usb/usbaudio.h b/sound/usb/usbaudio.h
> index 1ac3fd9..4852eb0 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 noautoclk; /* from the 'noautoclk' module param */
>
> struct usb_host_interface *ctrl_intf; /* the audio control interface */
> };
> --
> 1.8.1.5
>
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH 08/10] ALSA: usb-audio: UAC2: auto clock selection module param
2013-04-02 8:51 ` Takashi Iwai
@ 2013-04-02 20:27 ` Eldad Zack
0 siblings, 0 replies; 24+ messages in thread
From: Eldad Zack @ 2013-04-02 20:27 UTC (permalink / raw)
To: Takashi Iwai; +Cc: alsa-devel, Daniel Mack, Clemens Ladisch
On Tue, 2 Apr 2013, Takashi Iwai wrote:
> At Sun, 31 Mar 2013 17:52:30 +0200,
> Eldad Zack wrote:
> >
> > +module_param(noautoclk, bool, 0444);
> > +MODULE_PARM_DESC(noautoclk, "Disables auto-clock selection for UAC2 devices.");
>
> Don't use "no" for a boolean option. You can pass "yes" or "no" as an
> option value, so it can confuse easily by passing noautoclk=yes. See?
Ah, I understand. Good point. I'll change it to autoclock like you
suggested (also internally).
Thanks!
I'll add some text to the other patch and repost.
Cheers,
Eldad
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 09/10] ALSA: usb-audio: show err in set_sample_rate_v2 debug
2013-03-31 15:52 [PATCH 00/10] UAC2: Automatic clock switching Eldad Zack
` (7 preceding siblings ...)
2013-03-31 15:52 ` [PATCH 08/10] ALSA: usb-audio: UAC2: auto clock selection module param Eldad Zack
@ 2013-03-31 15:52 ` Eldad Zack
2013-03-31 15:52 ` [PATCH 10/10] ALSA: usb-audio: UAC2: support read-only freq control Eldad Zack
9 siblings, 0 replies; 24+ messages in thread
From: Eldad Zack @ 2013-03-31 15:52 UTC (permalink / raw)
To: Jaroslav Kysela, Takashi Iwai, Clemens Ladisch, Daniel Mack
Cc: alsa-devel, Eldad Zack
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 e3075ca..1c0ec82 100644
--- a/sound/usb/clock.c
+++ b/sound/usb/clock.c
@@ -336,8 +336,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;
}
@@ -346,8 +346,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(rate))) < 0) {
- snd_printk(KERN_WARNING "%d:%d:%d: cannot get freq (v2)\n",
- dev->devnum, iface, fmt->altsetting);
+ snd_printk(KERN_WARNING "%d:%d:%d: cannot get freq (v2): err %d\n",
+ dev->devnum, iface, fmt->altsetting, err);
return err;
}
--
1.8.1.5
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH 10/10] ALSA: usb-audio: UAC2: support read-only freq control
2013-03-31 15:52 [PATCH 00/10] UAC2: Automatic clock switching Eldad Zack
` (8 preceding siblings ...)
2013-03-31 15:52 ` [PATCH 09/10] ALSA: usb-audio: show err in set_sample_rate_v2 debug Eldad Zack
@ 2013-03-31 15:52 ` Eldad Zack
2013-04-01 8:17 ` Torstein Hegge
9 siblings, 1 reply; 24+ messages in thread
From: Eldad Zack @ 2013-03-31 15:52 UTC (permalink / raw)
To: Jaroslav Kysela, Takashi Iwai, Clemens Ladisch, Daniel Mack
Cc: alsa-devel, Eldad Zack
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 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 | 39 +++++++++++++++++++++++++++------------
1 file changed, 27 insertions(+), 12 deletions(-)
diff --git a/sound/usb/clock.c b/sound/usb/clock.c
index 1c0ec82..6ad9951 100644
--- a/sound/usb/clock.c
+++ b/sound/usb/clock.c
@@ -325,35 +325,50 @@ static int set_sample_rate_v2(struct snd_usb_audio *chip, int iface,
u32 data;
int err, crate;
int clock;
+ bool writeable;
+ struct uac_clock_source_descriptor *cs_desc;
clock = snd_usb_clock_find_source(chip, fmt->clock, true);
if (clock < 0)
return 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;
+ }
}
- if ((err = snd_usb_ctl_msg(dev, usb_rcvctrlpipe(dev, 0), UAC2_CS_CUR,
+ 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(rate))) < 0) {
+ &data, sizeof(rate));
+ if (err < 0) {
snd_printk(KERN_WARNING "%d:%d:%d: cannot get freq (v2): err %d\n",
dev->devnum, iface, fmt->altsetting, err);
return err;
}
crate = le32_to_cpu(data);
- if (crate != rate)
+ if (crate != 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, crate);
+ return -ENXIO;
+ }
snd_printd(KERN_WARNING "current rate %d is different from the runtime rate %d\n", crate, rate);
+ }
return 0;
}
--
1.8.1.5
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH 10/10] ALSA: usb-audio: UAC2: support read-only freq control
2013-03-31 15:52 ` [PATCH 10/10] ALSA: usb-audio: UAC2: support read-only freq control Eldad Zack
@ 2013-04-01 8:17 ` Torstein Hegge
2013-04-01 22:45 ` Eldad Zack
0 siblings, 1 reply; 24+ messages in thread
From: Torstein Hegge @ 2013-04-01 8:17 UTC (permalink / raw)
To: Eldad Zack; +Cc: Takashi Iwai, alsa-devel, Daniel Mack, Clemens Ladisch
On Sun, Mar 31, 2013 at 17:52:32 +0200, Eldad Zack wrote:
> 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 it doesn't match, return -ENXIO since the clock is invalid for
> this configuration.
I think could be more readable if it was built on top of [1]. Then it
could check the target rate against the prev_rate reported by the device
and return before the sample rate set, something like:
diff --git a/sound/usb/clock.c b/sound/usb/clock.c
index 9e2703a..395327c 100644
--- a/sound/usb/clock.c
+++ b/sound/usb/clock.c
@@ -254,18 +254,14 @@ static int set_sample_rate_v2(struct snd_usb_audio *chip, int iface,
struct usb_device *dev = chip->dev;
unsigned char data[4];
int err, cur_rate, prev_rate;
- int clock = snd_usb_clock_find_source(chip, fmt->clock);
+ int clock;
+ bool writeable;
+ struct uac_clock_source_descriptor *cs_desc;
+ 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;
- }
-
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,
@@ -279,6 +275,20 @@ static int set_sample_rate_v2(struct snd_usb_audio *chip, int iface,
prev_rate = data[0] | (data[1] << 8) | (data[2] << 16) | (data[3] << 24);
}
+ 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) {
+ if (prev_rate != rate) {
+ snd_printk(KERN_WARNING
+ "%d:%d:%d: freq mismatch (RO clock): req %d, clock runs @%d\n",
+ dev->devnum, iface, fmt->altsetting, rate, crate);
+ return -ENXIO;
+ }
+ else {
+ return 0;
+ }
+ }
+
data[0] = rate;
data[1] = rate >> 8;
data[2] = rate >> 16;
+ the other changes from the rest of your patch series.
[1] http://thread.gmane.org/gmane.linux.alsa.devel/106773
> + 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));
Is the indentation here intentional?
> - if ((err = snd_usb_ctl_msg(dev, usb_rcvctrlpipe(dev, 0), UAC2_CS_CUR,
> + 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(rate))) < 0) {
> + &data, sizeof(rate));
Same formatting comment as above.
> + if (crate != 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, crate);
> + return -ENXIO;
> + }
> snd_printd(KERN_WARNING "current rate %d is different from the runtime rate %d\n", crate, rate);
> + }
This bit didn't read very well on a narrow terminal.
Torstein
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH 10/10] ALSA: usb-audio: UAC2: support read-only freq control
2013-04-01 8:17 ` Torstein Hegge
@ 2013-04-01 22:45 ` Eldad Zack
0 siblings, 0 replies; 24+ messages in thread
From: Eldad Zack @ 2013-04-01 22:45 UTC (permalink / raw)
To: Torstein Hegge; +Cc: Takashi Iwai, alsa-devel, Daniel Mack, Clemens Ladisch
Hi Torstein,
On Mon, 1 Apr 2013, Torstein Hegge wrote:
> On Sun, Mar 31, 2013 at 17:52:32 +0200, Eldad Zack wrote:
> > 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 it doesn't match, return -ENXIO since the clock is invalid for
> > this configuration.
>
> I think could be more readable if it was built on top of [1]. Then it
> could check the target rate against the prev_rate reported by the device
> and return before the sample rate set, something like:
Thanks, I think it's a good idea. I'll wait with this patch until you
get your change to Takashi's tree to save some work for everyone.
> @@ -279,6 +275,20 @@ static int set_sample_rate_v2(struct snd_usb_audio *chip, int iface,
> prev_rate = data[0] | (data[1] << 8) | (data[2] << 16) | (data[3] << 24);
You might also want to convert this into le32_to_cpu, etc. like in
patch #5 of this series -- note that as Clemens said, the type should
should be __le32 (and not u32).
http://mailman.alsa-project.org/pipermail/alsa-devel/2013-March/060738.html
Thanks for pointing out the formatting issues, I'll fix these before
reposting.
Cheers,
Eldad
^ permalink raw reply [flat|nested] 24+ messages in thread