[alsa-devel] [PATCH 0/3] ALSA: usb-audio: trivial cleanups
Hi,
this is a series of trivial cleanups in USB-audio codes, as a wrap up for the usb-audio code changes for 4.18. No functional change by the series.
Takashi
===
Takashi Iwai (3): ALSA: usb-audio: Avoid lowlevel device object ALSA: usb-audio: Move autoresume call at the end of open ALSA: usb-audio: Follow standard coding style
sound/usb/card.c | 28 +++------------ sound/usb/clock.c | 18 +++++----- sound/usb/mixer.c | 25 ++++++++----- sound/usb/mixer_quirks.c | 3 +- sound/usb/pcm.c | 91 +++++++++++++++++++++++++----------------------- 5 files changed, 81 insertions(+), 84 deletions(-)
Simplify the device management by replacing the lowlevel device object allocation with the card->private_data. Nowadays there is almost no advantage by the lowlevel device, and with card->private_data, the code becomes cleaner.
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/usb/card.c | 28 +++++----------------------- 1 file changed, 5 insertions(+), 23 deletions(-)
diff --git a/sound/usb/card.c b/sound/usb/card.c index f6c3c1cd591e..615ee1e92de9 100644 --- a/sound/usb/card.c +++ b/sound/usb/card.c @@ -342,8 +342,9 @@ static int snd_usb_create_streams(struct snd_usb_audio *chip, int ctrlif) * */
-static int snd_usb_audio_free(struct snd_usb_audio *chip) +static void snd_usb_audio_free(struct snd_card *card) { + struct snd_usb_audio *chip = card->private_data; struct snd_usb_endpoint *ep, *n;
list_for_each_entry_safe(ep, n, &chip->ep_list, list) @@ -353,13 +354,6 @@ static int snd_usb_audio_free(struct snd_usb_audio *chip) if (!atomic_read(&chip->shutdown)) dev_set_drvdata(&chip->dev->dev, NULL); kfree(chip); - return 0; -} - -static int snd_usb_audio_dev_free(struct snd_device *device) -{ - struct snd_usb_audio *chip = device->device_data; - return snd_usb_audio_free(chip); }
static void usb_audio_make_shortname(struct usb_device *dev, @@ -459,9 +453,6 @@ static int snd_usb_audio_create(struct usb_interface *intf, struct snd_usb_audio *chip; int err; char component[14]; - static struct snd_device_ops ops = { - .dev_free = snd_usb_audio_dev_free, - };
*rchip = NULL;
@@ -479,18 +470,13 @@ static int snd_usb_audio_create(struct usb_interface *intf, }
err = snd_card_new(&intf->dev, index[idx], id[idx], THIS_MODULE, - 0, &card); + sizeof(*chip), &card); if (err < 0) { dev_err(&dev->dev, "cannot create card instance %d\n", idx); return err; }
- chip = kzalloc(sizeof(*chip), GFP_KERNEL); - if (! chip) { - snd_card_free(card); - return -ENOMEM; - } - + chip = card->private_data; mutex_init(&chip->mutex); init_waitqueue_head(&chip->shutdown_wait); chip->index = idx; @@ -508,11 +494,7 @@ static int snd_usb_audio_create(struct usb_interface *intf, INIT_LIST_HEAD(&chip->midi_list); INIT_LIST_HEAD(&chip->mixer_list);
- if ((err = snd_device_new(card, SNDRV_DEV_LOWLEVEL, chip, &ops)) < 0) { - snd_usb_audio_free(chip); - snd_card_free(card); - return err; - } + card->private_free = snd_usb_audio_free;
strcpy(card->driver, "USB-Audio"); sprintf(component, "USB%04x:%04x",
On Mon, 28 May 2018 11:58:19 +0200, Takashi Iwai wrote:
Simplify the device management by replacing the lowlevel device object allocation with the card->private_data. Nowadays there is almost no advantage by the lowlevel device, and with card->private_data, the code becomes cleaner.
Scratch this, it's no trivial change, and even slightly buggy... The other twos are really trivial, so they can be merged without problems.
Will refresh this one later as an individual patch.
Takashi
... so that we can avoid the extra goto lines. Also beautify the code to follow the standard codex.
No functional changes.
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/usb/pcm.c | 73 +++++++++++++++++++++++++++++---------------------------- 1 file changed, 37 insertions(+), 36 deletions(-)
diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c index d5b9c30d3bb1..a66bc717b952 100644 --- a/sound/usb/pcm.c +++ b/sound/usb/pcm.c @@ -76,10 +76,9 @@ snd_pcm_uframes_t snd_usb_pcm_delay(struct snd_usb_substream *subs, */ static snd_pcm_uframes_t snd_usb_pcm_pointer(struct snd_pcm_substream *substream) { - struct snd_usb_substream *subs; + struct snd_usb_substream *subs = substream->runtime->private_data; unsigned int hwptr_done;
- subs = (struct snd_usb_substream *)substream->runtime->private_data; if (atomic_read(&subs->stream->chip->shutdown)) return SNDRV_PCM_POS_XRUN; spin_lock(&subs->lock); @@ -1172,9 +1171,6 @@ static int setup_hw_info(struct snd_pcm_runtime *runtime, struct snd_usb_substre pt = 125 * (1 << fp->datainterval); ptmin = min(ptmin, pt); } - err = snd_usb_autoresume(subs->stream->chip); - if (err < 0) - return err;
param_period_time_if_needed = SNDRV_PCM_HW_PARAM_PERIOD_TIME; if (subs->speed == USB_SPEED_FULL) @@ -1183,30 +1179,37 @@ static int setup_hw_info(struct snd_pcm_runtime *runtime, struct snd_usb_substre if (ptmin == 1000) /* if period time doesn't go below 1 ms, no rules needed */ param_period_time_if_needed = -1; - snd_pcm_hw_constraint_minmax(runtime, SNDRV_PCM_HW_PARAM_PERIOD_TIME, - ptmin, UINT_MAX); - - if ((err = snd_pcm_hw_rule_add(runtime, 0, SNDRV_PCM_HW_PARAM_RATE, - hw_rule_rate, subs, - SNDRV_PCM_HW_PARAM_FORMAT, - SNDRV_PCM_HW_PARAM_CHANNELS, - param_period_time_if_needed, - -1)) < 0) - goto rep_err; - if ((err = snd_pcm_hw_rule_add(runtime, 0, SNDRV_PCM_HW_PARAM_CHANNELS, - hw_rule_channels, subs, - SNDRV_PCM_HW_PARAM_FORMAT, - SNDRV_PCM_HW_PARAM_RATE, - param_period_time_if_needed, - -1)) < 0) - goto rep_err; - if ((err = snd_pcm_hw_rule_add(runtime, 0, SNDRV_PCM_HW_PARAM_FORMAT, - hw_rule_format, subs, - SNDRV_PCM_HW_PARAM_RATE, - SNDRV_PCM_HW_PARAM_CHANNELS, - param_period_time_if_needed, - -1)) < 0) - goto rep_err; + + err = snd_pcm_hw_constraint_minmax(runtime, + SNDRV_PCM_HW_PARAM_PERIOD_TIME, + ptmin, UINT_MAX); + if (err < 0) + return err; + + err = snd_pcm_hw_rule_add(runtime, 0, SNDRV_PCM_HW_PARAM_RATE, + hw_rule_rate, subs, + SNDRV_PCM_HW_PARAM_FORMAT, + SNDRV_PCM_HW_PARAM_CHANNELS, + param_period_time_if_needed, + -1); + if (err < 0) + return err; + err = snd_pcm_hw_rule_add(runtime, 0, SNDRV_PCM_HW_PARAM_CHANNELS, + hw_rule_channels, subs, + SNDRV_PCM_HW_PARAM_FORMAT, + SNDRV_PCM_HW_PARAM_RATE, + param_period_time_if_needed, + -1); + if (err < 0) + return err; + err = snd_pcm_hw_rule_add(runtime, 0, SNDRV_PCM_HW_PARAM_FORMAT, + hw_rule_format, subs, + SNDRV_PCM_HW_PARAM_RATE, + SNDRV_PCM_HW_PARAM_CHANNELS, + param_period_time_if_needed, + -1); + if (err < 0) + return err; if (param_period_time_if_needed >= 0) { err = snd_pcm_hw_rule_add(runtime, 0, SNDRV_PCM_HW_PARAM_PERIOD_TIME, @@ -1216,15 +1219,13 @@ static int setup_hw_info(struct snd_pcm_runtime *runtime, struct snd_usb_substre SNDRV_PCM_HW_PARAM_RATE, -1); if (err < 0) - goto rep_err; + return err; } - if ((err = snd_usb_pcm_check_knot(runtime, subs)) < 0) - goto rep_err; - return 0; + err = snd_usb_pcm_check_knot(runtime, subs); + if (err < 0) + return err;
-rep_err: - snd_usb_autosuspend(subs->stream->chip); - return err; + return snd_usb_autoresume(subs->stream->chip); }
static int snd_usb_pcm_open(struct snd_pcm_substream *substream)
Avoid if ((err = ...) style and expand to multiple lines instead. No change in the end result, but just the beautification.
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/usb/clock.c | 18 ++++++++++-------- sound/usb/mixer.c | 25 +++++++++++++++++-------- sound/usb/mixer_quirks.c | 3 ++- sound/usb/pcm.c | 18 ++++++++++-------- 4 files changed, 39 insertions(+), 25 deletions(-)
diff --git a/sound/usb/clock.c b/sound/usb/clock.c index 17673f37fcc8..c79749613fa6 100644 --- a/sound/usb/clock.c +++ b/sound/usb/clock.c @@ -443,10 +443,11 @@ static int set_sample_rate_v1(struct snd_usb_audio *chip, int iface, data[0] = rate; data[1] = rate >> 8; data[2] = rate >> 16; - if ((err = snd_usb_ctl_msg(dev, usb_sndctrlpipe(dev, 0), UAC_SET_CUR, - USB_TYPE_CLASS | USB_RECIP_ENDPOINT | USB_DIR_OUT, - UAC_EP_CS_ATTR_SAMPLE_RATE << 8, ep, - data, sizeof(data))) < 0) { + err = snd_usb_ctl_msg(dev, usb_sndctrlpipe(dev, 0), UAC_SET_CUR, + USB_TYPE_CLASS | USB_RECIP_ENDPOINT | USB_DIR_OUT, + UAC_EP_CS_ATTR_SAMPLE_RATE << 8, ep, + data, sizeof(data)); + if (err < 0) { dev_err(&dev->dev, "%d:%d: cannot set freq %d to ep %#x\n", iface, fmt->altsetting, rate, ep); return err; @@ -460,10 +461,11 @@ static int set_sample_rate_v1(struct snd_usb_audio *chip, int iface, if (chip->sample_rate_read_error > 2) return 0;
- if ((err = snd_usb_ctl_msg(dev, usb_rcvctrlpipe(dev, 0), UAC_GET_CUR, - USB_TYPE_CLASS | USB_RECIP_ENDPOINT | USB_DIR_IN, - UAC_EP_CS_ATTR_SAMPLE_RATE << 8, ep, - data, sizeof(data))) < 0) { + err = snd_usb_ctl_msg(dev, usb_rcvctrlpipe(dev, 0), UAC_GET_CUR, + USB_TYPE_CLASS | USB_RECIP_ENDPOINT | USB_DIR_IN, + UAC_EP_CS_ATTR_SAMPLE_RATE << 8, ep, + data, sizeof(data)); + if (err < 0) { dev_err(&dev->dev, "%d:%d: cannot get freq at ep %#x\n", iface, fmt->altsetting, ep); chip->sample_rate_read_error++; diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c index bf74e7edc92b..898afd3001ea 100644 --- a/sound/usb/mixer.c +++ b/sound/usb/mixer.c @@ -598,7 +598,8 @@ int snd_usb_mixer_add_control(struct usb_mixer_elem_list *list,
while (snd_ctl_find_id(mixer->chip->card, &kctl->id)) kctl->id.index++; - if ((err = snd_ctl_add(mixer->chip->card, kctl)) < 0) { + err = snd_ctl_add(mixer->chip->card, kctl); + if (err < 0) { usb_audio_dbg(mixer->chip, "cannot add control (err = %d)\n", err); return err; @@ -1850,7 +1851,8 @@ static int parse_audio_feature_unit(struct mixer_build *state, int unitid, }
/* parse the source unit */ - if ((err = parse_audio_unit(state, hdr->bSourceID)) < 0) + err = parse_audio_unit(state, hdr->bSourceID); + if (err < 0) return err;
/* determine the input source type and name */ @@ -2270,7 +2272,8 @@ static int build_audio_procunit(struct mixer_build *state, int unitid, }
for (i = 0; i < num_ins; i++) { - if ((err = parse_audio_unit(state, desc->baSourceID[i])) < 0) + err = parse_audio_unit(state, desc->baSourceID[i]); + if (err < 0) return err; }
@@ -2483,7 +2486,8 @@ static int parse_audio_selector_unit(struct mixer_build *state, int unitid, }
for (i = 0; i < desc->bNrInPins; i++) { - if ((err = parse_audio_unit(state, desc->baSourceID[i])) < 0) + err = parse_audio_unit(state, desc->baSourceID[i]); + if (err < 0) return err; }
@@ -3310,11 +3314,16 @@ int snd_usb_create_mixer(struct snd_usb_audio *chip, int ctrlif,
if (mixer->protocol == UAC_VERSION_3 && chip->badd_profile >= UAC3_FUNCTION_SUBCLASS_GENERIC_IO) { - if ((err = snd_usb_mixer_controls_badd(mixer, ctrlif)) < 0) + err = snd_usb_mixer_controls_badd(mixer, ctrlif); + if (err < 0) + goto _error; + } else { + err = snd_usb_mixer_controls(mixer); + if (err < 0) + goto _error; + err = snd_usb_mixer_status_create(mixer); + if (err < 0) goto _error; - } else if ((err = snd_usb_mixer_controls(mixer)) < 0 || - (err = snd_usb_mixer_status_create(mixer)) < 0) { - goto _error; } err = create_keep_iface_ctl(mixer); if (err < 0) diff --git a/sound/usb/mixer_quirks.c b/sound/usb/mixer_quirks.c index 1b94387e18b6..4149543f613e 100644 --- a/sound/usb/mixer_quirks.c +++ b/sound/usb/mixer_quirks.c @@ -1824,7 +1824,8 @@ int snd_usb_mixer_apply_create_quirk(struct usb_mixer_interface *mixer) int err = 0; struct snd_info_entry *entry;
- if ((err = snd_usb_soundblaster_remote_init(mixer)) < 0) + err = snd_usb_soundblaster_remote_init(mixer); + if (err < 0) return err;
switch (mixer->chip->usb_id) { diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c index a66bc717b952..897a2cbef6de 100644 --- a/sound/usb/pcm.c +++ b/sound/usb/pcm.c @@ -163,10 +163,11 @@ static int init_pitch_v1(struct snd_usb_audio *chip, int iface, ep = get_endpoint(alts, 0)->bEndpointAddress;
data[0] = 1; - if ((err = snd_usb_ctl_msg(dev, usb_sndctrlpipe(dev, 0), UAC_SET_CUR, - USB_TYPE_CLASS|USB_RECIP_ENDPOINT|USB_DIR_OUT, - UAC_EP_CS_ATTR_PITCH_CONTROL << 8, ep, - data, sizeof(data))) < 0) { + err = snd_usb_ctl_msg(dev, usb_sndctrlpipe(dev, 0), UAC_SET_CUR, + USB_TYPE_CLASS|USB_RECIP_ENDPOINT|USB_DIR_OUT, + UAC_EP_CS_ATTR_PITCH_CONTROL << 8, ep, + data, sizeof(data)); + if (err < 0) { usb_audio_err(chip, "%d:%d: cannot set enable PITCH\n", iface, ep); return err; @@ -184,10 +185,11 @@ static int init_pitch_v2(struct snd_usb_audio *chip, int iface, int err;
data[0] = 1; - if ((err = snd_usb_ctl_msg(dev, usb_sndctrlpipe(dev, 0), UAC2_CS_CUR, - USB_TYPE_CLASS | USB_RECIP_ENDPOINT | USB_DIR_OUT, - UAC2_EP_CS_PITCH << 8, 0, - data, sizeof(data))) < 0) { + err = snd_usb_ctl_msg(dev, usb_sndctrlpipe(dev, 0), UAC2_CS_CUR, + USB_TYPE_CLASS | USB_RECIP_ENDPOINT | USB_DIR_OUT, + UAC2_EP_CS_PITCH << 8, 0, + data, sizeof(data)); + if (err < 0) { usb_audio_err(chip, "%d:%d: cannot set enable PITCH (v2)\n", iface, fmt->altsetting); return err;
participants (1)
-
Takashi Iwai