From mboxrd@z Thu Jan  1 00:00:00 1970
From: Takashi Iwai <tiwai@suse.de>
Subject: Re: [PATCH v2 1/1] ALSA: usb-audio: Add custom mixer
	status quirks for RME CC devices
Date: Thu, 04 Oct 2018 23:19:08 +0200
Message-ID: <s5h36tll8ab.wl-tiwai@suse.de>
References: <20181004191743.20100-1-jussi@sonarnerd.net>
 <20181004191743.20100-2-jussi@sonarnerd.net>
 <s5hefd5lacy.wl-tiwai@suse.de>
 <99b2ea2f-00e6-52d5-ee39-1125736b0d7e@sonarnerd.net>
Mime-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka")
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <alsa-devel-bounces@alsa-project.org>
Received: from mx1.suse.de (mx2.suse.de [195.135.220.15])
 by alsa0.perex.cz (Postfix) with ESMTP id 5DDC7267737
 for <alsa-devel@alsa-project.org>; Thu,  4 Oct 2018 23:19:09 +0200 (CEST)
In-Reply-To: <99b2ea2f-00e6-52d5-ee39-1125736b0d7e@sonarnerd.net>
List-Unsubscribe: <http://mailman.alsa-project.org/mailman/options/alsa-devel>,
 <mailto:alsa-devel-request@alsa-project.org?subject=unsubscribe>
List-Archive: <http://mailman.alsa-project.org/pipermail/alsa-devel/>
List-Post: <mailto:alsa-devel@alsa-project.org>
List-Help: <mailto:alsa-devel-request@alsa-project.org?subject=help>
List-Subscribe: <http://mailman.alsa-project.org/mailman/listinfo/alsa-devel>,
 <mailto:alsa-devel-request@alsa-project.org?subject=subscribe>
Errors-To: alsa-devel-bounces@alsa-project.org
Sender: alsa-devel-bounces@alsa-project.org
To: Jussi Laako <jussi@sonarnerd.net>
Cc: alsa-devel@alsa-project.org
List-Id: alsa-devel@alsa-project.org

On Thu, 04 Oct 2018 23:00:31 +0200,
Jussi Laako wrote:
> 
> On 04.10.2018 23:34, Takashi Iwai wrote:
> > Rather include <linux/math64.h>, and put in the appropriate order.
> 
> Sorry, I had a bit of hard time finding the function in first place,
> but what is the appropriate order?

No strict rule here.  Often people put in the alphabetical order, or
some logic (e.g. header dependencies).  In this case, just append to
linux/*.h files.

> > Since you're calling in the same pattern, it's better to consider to
> > provide a helper to call snd_usb_ctl_msg() with the given type (and
> > shows the error there).  The helper can take snd_usb_lock_shutdown()
> > and unlock in it, so it'll simplify a lot.
> 
> In first case I'm calling GET_STATUS1 and the lock surrounds that. In
> the second case I'm calling both GET_STATUS1 and GET_CURRENT_FREQ and
> the lock surrounds both calls togher. I didn't completely understand
> how to achieve this with the helper, unless you meant through some
> extra argument and control path inside?

This lock/unlock is not about spinlock or such race protection, but
just for a protection for the disconnection check.  So it's fine to
take twice, once for each.  As it's already with the USB ctl msg, it's
no real-time task, after all.

That is, a picture I had in my mind is something like:

static int rme_vendor_verb(struct snd_kcontrol *kcontrol, u32 cmd,
			   u32 *result)
{
	struct usb_mixer_elem_list *list = snd_kcontrol_chip(kcontrol);
	struct snd_usb_audio *chip = list->mixer->chip;
	struct usb_device *dev = chip->dev;
	int err;

	err = snd_usb_lock_shutdown(chip);
	if (err < 0)
		return err;

	err = snd_usb_ctl_msg(dev, usb_rcvctrlpipe(dev, 0), cmd,
			      USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
			      0, 0, result, sizeof(*result));
	if (err < 0)
		dev_err(&dev->dev,
			"unable to issue vendor read request (ret = %d)", err);

	snd_usb_unlock_shutdown(chip);
	return err;
}

... and call it like
	err = rme_vendor_verb(kcontrol, RME_GET_STATUS1, &status1);


Takashi