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