sound/usb/mixer_scarlett_gen2.c:2000:5: warning: symbol 'snd_scarlett_gen2_controls_create' was not declared. Should it be static?
Fixes: 265d1a90e4 ("ALSA: usb-audio: scarlett2: Improve driver startup messages")
Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: kernel test robot <lkp@intel.com>
Link: https://lore.kernel.org/r/20210522180900.GA83915@f59a3af2f1d9
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Add separate init function to call the existing controls_create
function so a custom error can be displayed if initialisation fails.
Use info level instead of error for notifications.
Display the VID/PID so device_setup is targeted to the right device.
Display "enabled" message to easily confirm that the driver is loaded.
Signed-off-by: Geoffrey D. Bennett <g@b4.vu>
Cc: <stable@vger.kernel.org>
Link: https://lore.kernel.org/r/b5d140c65f640faf2427e085fbbc0297b32e5fce.1621584566.git.g@b4.vu
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Use usb_rcvctrlpipe() not usb_sndctrlpipe() for USB control input in
the Scarlett Gen 2 mixer driver. This fixes the device hang during
initialisation when used with the ehci-pci host driver.
Fixes: 9e4d5c1be2 ("ALSA: usb-audio: Scarlett Gen 2 mixer interface")
Signed-off-by: Geoffrey D. Bennett <g@b4.vu>
Cc: <stable@vger.kernel.org>
Link: https://lore.kernel.org/r/66a3d05dac325d5b53e4930578e143cef1f50dbe.1621584566.git.g@b4.vu
Signed-off-by: Takashi Iwai <tiwai@suse.de>
The direction of the pipe argument must match the request-type direction
bit or control requests may fail depending on the host-controller-driver
implementation.
Fix the UAC2_CS_CUR request which erroneously used usb_sndctrlpipe().
Fixes: 93db51d06b ("ALSA: usb-audio: Check valid altsetting at parsing rates for UAC2/3")
Cc: stable@vger.kernel.org # 5.10
Signed-off-by: Johan Hovold <johan@kernel.org>
Link: https://lore.kernel.org/r/20210521133742.18098-1-johan@kernel.org
Signed-off-by: Takashi Iwai <tiwai@suse.de>
The cur variable indicating the currently selected clock source can be
theoretically used as uninitialized after the recent commit
481f17c418 ("ALSA: usb-audio: Handle error for the current selector
gracefully"). For addressing it, initialize it before use.
Also, one place seems setting 0 to a wrong variable ret, instead of
cur; otherwise it makes little sense. Since the initialization is
done beforehand, we can get rid of this line, too.
Fixes: 481f17c418 ("ALSA: usb-audio: Handle error for the current selector gracefully")
Reported-by: Colin Ian King <colin.king@canonical.com>
Link: https://lore.kernel.org/r/4b261d68-f53f-240d-2d8a-2f88b337849d@canonical.com
Link: https://lore.kernel.org/r/s5hfsyhh97t.wl-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
The initialization of MIDI devices that are found on some LINE6
drivers are currently done in a racy way; namely, the MIDI buffer
instance is allocated and initialized in each private_init callback
while the communication with the interface is already started via
line6_init_cap_control() call before that point. This may lead to
Oops in line6_data_received() when a spurious event is received, as
reported by syzkaller.
This patch moves the MIDI initialization to line6_init_cap_control()
as well instead of the too-lately-called private_init for avoiding the
race. Also this reduces slightly more lines, so it's a win-win
change.
Reported-by: syzbot+0d2b3feb0a2887862e06@syzkallerlkml..appspotmail.com
Link: https://lore.kernel.org/r/000000000000a4be9405c28520de@google.com
Link: https://lore.kernel.org/r/20210517132725.GA50495@hyeyoo
Cc: Hyeonggon Yoo <42.hyeyoo@gmail.com>
Cc: <stable@vger.kernel.org>
Link: https://lore.kernel.org/r/20210518083939.1927-1-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Currently we bail out when the device returns an error or an invalid
value for the current clock selector value via
uac_clock_selector_get_val(). But it's possible that the device is
really uninitialized and waits for the setup of the proper route at
first.
For handling such a case, this patch lets the driver dealing with the
error or the invalid error more gracefully, choosing the clock source
automatically instead.
Link: https://lore.kernel.org/r/20210518152112.8016-3-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
This patch just does refactoring of the UAC2/3 clock setup code.
There should be no functional changes. The major changes are:
* Provide union objects for pointing both UAC2 and UAC3 objects
* Unify clock source, selector and multiplier helper functions
* Unify __uac_clock_find_source() to deal with both UAC2 and UAC3
equally
Link: https://lore.kernel.org/r/20210518152112.8016-2-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Minor code refactoring by merging the superfluous function calls.
The functions were split in the past for covering pre-history USB
driver code, but this is utterly useless.
Link: https://lore.kernel.org/r/20210517131545.27252-11-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Currently us428ctls_shmem pages are allocated dynamically upon the
mmap call, but this is quite racy. Since the shared memory itself is
mandatory for the mmap, let's allocate it at the beginning of the card
initialization. Also, fix the initialization of the wait queue, too.
Link: https://lore.kernel.org/r/20210517131545.27252-9-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
The PCM shmem pages are allocated in snd_usx2y_usbpcm_prepare().
Theoretically the prepare callback may be called simultaneously for
both playback and capture, hence this allocation can be racy.
Make sure that the allocation is performed exclusively by extending
the pcm_mutex lock to cover the allocation code, too.
Link: https://lore.kernel.org/r/20210517131545.27252-8-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Theoretically the initialization functions in usx2y drivers may be
called multiple times as the driver gets initialized via hwpdep
ioctl. Meanwhile, those functions including memory allocations don't
check whether they are called twice, and they forget the old
resources, which would lead to memory leaks.
This patch adds the sanity checks about the doubly initializations to
give kernel WARNING, and returns an error in such a case. Also, each
allocation assures to release the resources at its error path
properly.
Link: https://lore.kernel.org/r/20210517131545.27252-7-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
The initialization os usx2y driver is multi-staged, and the PCM and
other device creations are done after the DSP is loaded and
initialized. Upon the initialization, when an error happens, the
driver tries to call snd_card_free(). But this is dangerous, and in
general, the driver cannot kill itself during its operation.
Hence better to drop the snd_card_free() call from there.
Link: https://lore.kernel.org/r/20210517131545.27252-6-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
usx2y drivers may expose the allocated pages via mmap, but it performs
zero-clear only for the struct size, not aligned with the page size.
This leaves out some uninitialized trailing bytes.
This patch fixes the clearance to cover all memory that are exposed to
user-space.
Link: https://lore.kernel.org/r/20210517131545.27252-5-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
This patch fixes various trivial coding-style issues in usx2y code,
such as:
* the assginments in if condition
* comparison order with constants
* NULL / zero checks
* unsigned -> unsigned int
* addition of braces in control blocks
* debug print with function names
* move local variables in block into function head
* reduction of too nested indentations
No functional changes.
Link: https://lore.kernel.org/r/20210517131545.27252-4-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
This patch corrects merely the spaces in the usx2y code, including the
superfluous trailing space in the debug prints and a slight reformat
of some comment lines. Nothing really touches about the code itself.
Link: https://lore.kernel.org/r/20210517131545.27252-3-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
For improving readability, convert camelCase fields, variables and
functions to the plain names with underscore. Also align the macros
to be capital letters.
All done via sed, no functional changes.
Note that you'll still see many coding style issues even after this
patch; the fixes will follow.
Link: https://lore.kernel.org/r/20210517131545.27252-2-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
The recently introduced MIDI endpoint parser code has an access to the
field without the size validation, hence it might lead to
out-of-bounce access. Add the sanity checks for the descriptor
sizes.
Fixes: eb596e0fd1 ("ALSA: usb-audio: generate midi streaming substream names from jack names")
Link: https://lore.kernel.org/r/20210511090500.2637-1-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Variable len is set to zero but this value is never read as it is
overwritten with a new value later on, hence it is a redundant
assignment and can be removed.
Cleans up the following clang-analyzer warning:
sound/usb/mixer.c:2713:3: warning: Value stored to 'len' is never read
[clang-analyzer-deadcode.DeadStores].
Reported-by: Abaci Robot <abaci@linux.alibaba.com>
Signed-off-by: Jiapeng Chong <jiapeng.chong@linux.alibaba.com>
Link: https://lore.kernel.org/r/1619519194-57806-1-git-send-email-jiapeng.chong@linux.alibaba.com
Signed-off-by: Takashi Iwai <tiwai@suse.de>
The recent endpoint management change for implicit feedback mode added
a clearance of ep->sync_sink (formerly ep->sync_slave) pointer at
snd_usb_endpoint_stop() to assure no leftover for the feedback from
the already stopped capture stream. This turned out to cause a
regression, however, when full-duplex streams were running and only a
capture was stopped. Because of the above clearance of ep->sync_sink
pointer, no more feedback is done, hence the playback will stall.
This patch fixes the ep->sync_sink clearance to be done only after all
endpoints are released, for addressing the regression.
Reported-and-tested-by: Lucas Endres <jaffa225man@gmail.com>
Fixes: bf6313a0ff ("ALSA: usb-audio: Refactor endpoint management")
Cc: <stable@vger.kernel.org>
Link: https://lore.kernel.org/r/20210426063349.18601-1-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Through the examinations and experiments with lots of Roland and BOSS
USB-audio devices, we found out that the recently introduced
full-duplex operations with the implicit feedback mode work fine for
quite a few devices, while the others need only the capture-side quirk
to enforce the full-duplex mode. The recent commit d86f43b17e
("ALSA: usb-audio: Add support for many Roland devices' implicit
feedback quirks") tried to add such quirk entries manually in the
lists, but this turned out to be too many and error-prone, hence it
was reverted again.
This patch is another attempt to cover those missing Roland/BOSS
devices but in a more generic way. It matches the devices with the
vendor ID 0x0582, and checks whether they are with both ASYNC sync
types or ASYNC is only for capture device. In the former case, it's
the device with the implicit feedback mode, and applies accordingly.
In both cases, the capture stream requires always the full-duplex
mode, and we apply the known capture quirk for that, too.
Basically the already existing BOSS device quirk entries become
redundant after this generic matching, so those are removed. Although
the capture_implicit_fb_quirks[] table became empty and superfluous, I
keep it for now, so that people can put a special device easily at any
time later again.
Link: https://lore.kernel.org/r/CAOsVg8rA61B=005_VyUwpw3piVwA7Bo5fs1GYEB054efyzGjLw@mail.gmail.com
Link: https://lore.kernel.org/r/20210414083255.9527-1-tiwai@suse.de
BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=212519
Tested-by: Lucas Endres <jaffa225man@gmail.com>
Link: https://lore.kernel.org/r/20210422120413.457-2-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
This reverts commit d86f43b17e ("ALSA: usb-audio: Add support for
many Roland devices' feedback quirks").
It turned out that many quirk entries there don't contain the proper
EP values and/or the quirk types, which lead to the broken
operations.
As we're going to cover all Roland/BOSS devices in a more generic way
rather the explicit lists, let's revert the previous additions at
first.
Fixes: d86f43b17e ("ALSA: usb-audio: Add support for many Roland devices' implicit feedback quirks")
Link: https://lore.kernel.org/r/20210422120413.457-1-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Currently when the call to usb_urb_ep_type_check fails (returning -EINVAL)
the error return path returns -ENOMEM via the exit label "error". Other
uses of the same error exit label set the err variable to -ENOMEM but this
is not being used. I believe the original intent was for the error exit
path to return the value in err rather than the hard coded -ENOMEM, so
return this rather than the hard coded -ENOMEM.
Addresses-Coverity: ("Unused value")
Fixes: 738d9edcfd ("ALSA: usb-audio: Add sanity checks for invalid EPs")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
Link: https://lore.kernel.org/r/20210420134719.381409-1-colin.king@canonical.com
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Pioneer devices are supposed to be working with the implicit feedback
mode, but so far the attempt to apply the implicit feedback caused
issues, hence we explicitly skipped the implicit feedback mode for
them. Recently, Geraldo discovered that the device actually works if
you skip the generic matching of the sync EPs for the capture stream.
That is, we should apply the implicit feedback setup for the playback
like other similar devices, while we need to return 1 from
audioformat_capture_quirk() so that no further matching will be done.
And, later on, Olivia reported later that the fiddling with the
capture quirk alone doesn't suffice for the test with speaker-test
program. This seems to be a similar case like the recently fixed BOSS
devices. Indeed, the problem could be addressed by setting
playback_first flag, which indicates that the playback URBs have to be
sent out at first even in the implicit feedback mode.
This patch implements the application of the implicit feedback to
Pioneer devices as described in the above. The former
skip_pioneer_sync_ep() was dropped, and instead we provide
is_pioneer_implicit_fb() to check the Pioneer devices that need the
implicit feedback. In the audioformat_implicit_fb_quirk(), simply
apply the implicit fb for playback and set chip->playback_first flag
if matching, and in audioformat_capture_quirk()(), it returns 1 for
skipping the generic EP sync handling.
Reported-by: Geraldo <geraldogabriel@gmail.com>
Tested-by: Olivia Mackintosh <livvy@base.nu>
Link: https://lore.kernel.org/r/s5ha6pygqfz.wl-tiwai@suse.de
Link: https://lore.kernel.org/r/20210419153918.450-1-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Add case statement to set sample-rate for the DJM-750 Pioneer
mixer. This was included as part of another patch but I think it has
been archived on Patchwork and hasn't been merged.
Signed-off-by: Olivia Mackintosh <livvy@base.nu>
Cc: <stable@vger.kernel.org>
Link: https://lore.kernel.org/r/20210418165901.25776-1-livvy@base.nu
Signed-off-by: Takashi Iwai <tiwai@suse.de>
It makes USB audio capture and playback possible and pristine on my Roland
INTEGRA-7, Boutique D-05, and R-26, along with many more I've encountered
people having had issues with over the last decade or so.
Signed-off-by: Lucas Endres <jaffa225man@gmail.com>
Link: https://lore.kernel.org/r/CAOsVg8rA61B=005_VyUwpw3piVwA7Bo5fs1GYEB054efyzGjLw@mail.gmail.com
Signed-off-by: Takashi Iwai <tiwai@suse.de>
During the recent rewrite of the implicit feedback support, we've
tested to apply the implicit fb on BOSS devices, but it failed, as the
capture stream didn't start without the playback. As the end result,
it got another type of quirk for tying both streams but starts
playback always (commit 6234fdc1ce "ALSA: usb-audio: Quirk for BOSS
GT-001").
Meanwhile, Mike Oliphant has tested the real implicit feedback mode
for the playback again with the latest code, and found out that it
actually works if the initial feedback sync is skipped; that is, on
those BOSS devices, the playback stream has to be started at first
without waiting for the capture URB completions. Otherwise it gets
stuck. In the rest operations after the capture stream processed, we
can take them as the implicit feedback source.
This patch is an attempt to improve the support for BOSS devices with
the implicit feedback mode in the way described above. It adds a new
flag to snd_usb_audio, playback_first, indicating that the playback
stream starts without sync with the initial capture completion. This
flag is set in the quirk table with the new IMPLICIT_FB_BOTH type.
Reported-and-tested-by: Mike Oliphant <oliphant@nostatic.org>
Link: https://lore.kernel.org/r/20210414083255.9527-1-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
In the current code, we have some assumption that the audio clock
selector has been set up implicitly and don't want to touch it unless
it's really needed for the fallback autoclock setup. This works for
most devices but some seem having a problem. Partially this was
covered for the devices with a single connector at the initialization
phase (commit 086b957cc1 "ALSA: usb-audio: Skip the clock selector
inquiry for single connections"), but also there are cases where the
wrong clock set up is kept silently. The latter seems to be the cause
of the noises on Behringer devices.
In this patch, we explicitly set up the audio clock selector whenever
the appropriate node is found.
Reported-by: Geraldo Nascimento <geraldogabriel@gmail.com>
BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=199327
Link: https://lore.kernel.org/r/CAEsQvcvF7LnO8PxyyCxuRCx=7jNeSCvFAd-+dE0g_rd1rOxxdw@mail.gmail.com
Cc: <stable@vger.kernel.org>
Link: https://lore.kernel.org/r/20210413084152.32325-1-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
UA-101 device and co are supported by another driver, snd-ua101, but
the USB audio class driver (snd-usb-audio) catches all and this
resulted in the lack of functionality like missing MIDI devices.
This patch introduces a sort of deny-listing for those devices to just
return -ENODEV at probe in snd-usb-audio driver, so that it falls back
to the probe by snd-ua101.
BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=212477
Link: https://lore.kernel.org/r/20210408075656.30184-1-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
There are a few calls of usb_driver_claim_interface() but all of those
miss the proper error checks, as reported by Coverity. This patch
adds those missing checks.
Along with it, replace the magic pointer with -1 with a constant
USB_AUDIO_IFACE_UNUSED for better readability.
Reported-by: coverity-bot <keescook+coverity-bot@chromium.org>
Addresses-Coverity-ID: 1475943 ("Error handling issues")
Addresses-Coverity-ID: 1475944 ("Error handling issues")
Addresses-Coverity-ID: 1475945 ("Error handling issues")
Fixes: b1ce7ba619 ("ALSA: usb-audio: claim autodetected PCM interfaces all at once")
Fixes: e5779998bf ("ALSA: usb-audio: refactor code")
Link: https://lore.kernel.org/r/202104051059.FB7F3016@keescook
Link: https://lore.kernel.org/r/20210406113534.30455-1-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
This patchset tries to resolve the diversity in the audio LED
control among the ALSA drivers. A new control layer registration
is introduced which allows to run additional operations on
top of the elementary ALSA sound controls.
A new control access group (three bits in the access flags)
was introduced to carry the LED group information for
the sound controls. The low-level sound drivers can just
mark those controls using this access group. This information
is not exported to the user space, but user space can
manage the LED sound control associations through sysfs
(last patch) per Mark's request. It makes things fully
configurable in the kernel and user space (UCM).
The actual state ('route') evaluation is really easy
(the minimal value check for all channels / controls / cards).
If there's more complicated logic for a given hardware,
the card driver may eventually export a new read-only
sound control for the LED group and do the logic itself.
The new LED trigger control code is completely separated
and possibly optional (there's no symbol dependency).
The full code separation allows eventually to move this
LED trigger control to the user space in future.
Actually it replaces the already present functionality
in the kernel space (HDA drivers) and allows a quick adoption
for the recent hardware (ASoC codecs including SoundWire).
snd_ctl_led 24576 0
The sound driver implementation is really easy:
1) call snd_ctl_led_request() when control LED layer should be
automatically activated
/ it calls module_request("snd-ctl-led") on demand /
2) mark all related kcontrols with
SNDRV_CTL_ELEM_ACCESS_SPK_LED or
SNDRV_CTL_ELEM_ACCESS_MIC_LED
Link: https://lore.kernel.org/r/20210317172945.842280-1-perex@perex.cz
Signed-off-by: Takashi Iwai <tiwai@suse.de>
-----BEGIN PGP SIGNATURE-----
iQJCBAABCAAsFiEEIXTw5fNLNI7mMiVaLtJE4w1nLE8FAmBjRuQOHHRpd2FpQHN1
c2UuZGUACgkQLtJE4w1nLE9ZQw/6Ao2X1io4TVnyO/gO8HtwmnZ6TWcrLUlySaep
H6Suf0RHsOQO9VOaMcUarA3Wnz1vZ44qJ/fkdLTslnIPGSRJDUx15bbb+n2N6pQJ
gS7Umxy6n73rQyEoDjd3ZorvDGjFSVFpjM+RYjk/Ohq+yziz7nQ/SZuHPPeqm1GU
C5d9SxyXGdqlJJ6yFCHzbtjSmIey+l1TZ+j3rSSww/CzDKxB2l5J6JZAMUjVdL9b
J80Mcw0XLdG9iTtEnkUt3TwvLMcMl95UPeQHIkVQlwsRRb3BtHNIwJLPQ/n+Cou7
Hre3y2miUYHrNICEzMdMlpDzQBqu5wvpXgbgIV0CwAwCXPZBVWE1hVJ9gG0l+r1G
fy1a75OmEin4V9g8w+wNTWDEgjwAOkWhA67WVjpbvHtd6kEbISzt4JHFksG1rjU2
vXOIj2VBmQN6zHtxfcZqY8Ge4A7XGo7tAM/3NsUxin+2y7ZXI6sDvv+0esYmqrYr
9as/tD84L5LTrbUYewaUgEdauQXluQI1egRi7pSeg7hZyLeYYkmszk54Ra3zdlmM
m7Hr6u+Y/G7yeFdn/WdeG3VzdmxhC2ZFfk3gq0vneBS3WrATbf6nAORp2bwzGSz4
pUsVLSv+vhpZdSF+IxpUuMnsLkkbUCvFivXLjQ6irSWvp7uts1HWdRowdg7Pe2lC
FVbFRuA=
=1uM7
-----END PGP SIGNATURE-----
Merge tag 'tags/mute-led-rework' into for-next
ALSA: control - add generic LED API
This patchset tries to resolve the diversity in the audio LED
control among the ALSA drivers. A new control layer registration
is introduced which allows to run additional operations on
top of the elementary ALSA sound controls.
A new control access group (three bits in the access flags)
was introduced to carry the LED group information for
the sound controls. The low-level sound drivers can just
mark those controls using this access group. This information
is not exported to the user space, but user space can
manage the LED sound control associations through sysfs
(last patch) per Mark's request. It makes things fully
configurable in the kernel and user space (UCM).
The actual state ('route') evaluation is really easy
(the minimal value check for all channels / controls / cards).
If there's more complicated logic for a given hardware,
the card driver may eventually export a new read-only
sound control for the LED group and do the logic itself.
The new LED trigger control code is completely separated
and possibly optional (there's no symbol dependency).
The full code separation allows eventually to move this
LED trigger control to the user space in future.
Actually it replaces the already present functionality
in the kernel space (HDA drivers) and allows a quick adoption
for the recent hardware (ASoC codecs including SoundWire).
snd_ctl_led 24576 0
The sound driver implementation is really easy:
1) call snd_ctl_led_request() when control LED layer should be
automatically activated
/ it calls module_request("snd-ctl-led") on demand /
2) mark all related kcontrols with
SNDRV_CTL_ELEM_ACCESS_SPK_LED or
SNDRV_CTL_ELEM_ACCESS_MIC_LED
Link: https://lore.kernel.org/r/20210317172945.842280-1-perex@perex.cz
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Logitech ConferenceCam Connect is a compound USB device with UVC and
UAC. Not 100% reproducible but sometimes it keeps responding STALL to
every control transfer once it receives get_freq request.
This patch adds 046d:0x084c to a snd_usb_get_sample_rate_quirk list.
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=203419
Signed-off-by: Ikjoon Jang <ikjn@chromium.org>
Cc: <stable@vger.kernel.org>
Link: https://lore.kernel.org/r/20210324105153.2322881-1-ikjn@chromium.org
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Rear Mic on Lenovo P620 cannot record after S3, despite that there's no
error and the other two functions of the USB audio, Line In and Line
Out, work just fine.
The mic starts to work again after running userspace app like "alsactl
store". Following the lead, the evidence shows that as soon as connector
status is queried, the mic can work again.
So also check connector value on resume to "wake up" the USB audio to
make it functional.
This can be device specific, however I think this generic approach may
benefit more than one device.
Now the resume callback checks connector, and a new callback,
reset_resume, to also restore switches and volumes.
Suggested-by: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
Link: https://lore.kernel.org/r/20210325165918.22593-2-kai.heng.feng@canonical.com
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Majority of changes are various ASoC device/platform-specific small
fixes (including a removal of stale file) while the only common
change is a clk management fix in ASoC simple-card driver.
The rest are usual HD-audio quirks.
-----BEGIN PGP SIGNATURE-----
iQJCBAABCAAsFiEEIXTw5fNLNI7mMiVaLtJE4w1nLE8FAmBUbqgOHHRpd2FpQHN1
c2UuZGUACgkQLtJE4w1nLE+4yBAAkA6y2fNlWYsQzZeNeWNnZyRdeoi5M0Flqd0q
PRK6VynM/M0kn2U/PgwhlDgrcNPqmW6AgXAPEEnv5zfwPs9MPXM3B9uJiiIOW6t1
c0EfS2msnTXx6RAn62ecYJkp6BUVnejAYIgzKiLOBj5/0mNXgnbTfC7PSnfghWca
ajtq/+l3pr519C4ewJSKZ84ho3S7mEEZdBGkpvaYTVZY09mkoZj1eefuQBp8690d
n/jTl+gCk8Z3834bhBKgdax2huZmTaMQ2wXsoEHZNPpHr2NfmR0RvPgnNutDA6uJ
JVM4KBStTxIpgg84D0wXIOvUst4/FcPtaho5UCNqSJ1/xoom+BGV3hTXLSW1SVUT
/TUZje6hCfl9WyN5lR2S2JIYorTp6yqLQscXPmolkq+vtkLbQa83MybhyTK1w78q
xMLCezctc3cpFikydelWa39xMUyjd7/PkabhwWoFk50niBIVS119rhH/GqhGIusp
UU6JyujhoM8JWvywoyHsgV6c+2gDLQVU9k6mrAgMf9hgjRTz2ztCgK4fWccqEGvt
5retVRVdv0xUxBxLkD9AhZ47ym8udcOLP7NyBl70MHJSm4GuEPUdefPNOGOWqIUr
0XJfCmvipxJkA4/QwC5j7du8uILMXmjqzvf/LGfhg8Yo4YtQbJSv6mPmCSrtE1JE
2yz8zTA=
=kGbc
-----END PGP SIGNATURE-----
Merge tag 'sound-5.12-rc4' of git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound
Pull sound fixes from Takashi Iwai:
"The majority of changes are various ASoC device/platform-specific
small fixes (including a removal of stale file) while the only common
change is a clk management fix in ASoC simple-card driver.
The rest are the usual HD-audio quirks"
* tag 'sound-5.12-rc4' of git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound: (44 commits)
ALSA: usb-audio: Fix unintentional sign extension issue
ALSA: hda/realtek: fix mute/micmute LEDs for HP 850 G8
ASoC: dt-bindings: fsl_spdif: Add compatible string for new platforms
ASoC: rt711: add snd_soc_component remove callback
ASoC: rt5659: Update MCLK rate in set_sysclk()
ASoC: simple-card-utils: Do not handle device clock
ALSA: hda/realtek: fix mute/micmute LEDs for HP 440 G8
ALSA: hda/realtek: fix mute/micmute LEDs for HP 840 G8
ALSA: hda/realtek: apply pin quirk for XiaomiNotebook Pro
ALSA: hda/realtek: Apply headset-mic quirks for Xiaomi Redmibook Air
ASoC: mediatek: mt8192: fix tdm out data is valid on rising edge
ALSA: dice: fix null pointer dereference when node is disconnected
ALSA: hda: generic: Fix the micmute led init state
ASoC: qcom: lpass-cpu: Fix lpass dai ids parse
spi: cadence: set cqspi to the driver_data field of struct device
ASoC: SOF: intel: fix wrong poll bits in dsp power down
ASoC: codecs: wcd934x: add a sanity check in set channel map
ASoC: qcom: sdm845: Fix array out of range on rx slim channels
ASoC: qcom: sdm845: Fix array out of bounds access
ASoC: remove remnants of sirf prima/atlas audio codec
...
The shifting of the u8 integer device by 24 bits to the left will
be promoted to a 32 bit signed int and then sign-extended to a
64 bit unsigned long. In the event that the top bit of device is
set then all then all the upper 32 bits of the unsigned long will
end up as also being set because of the sign-extension. Fix this
by casting device to an unsigned long before the shift.
Addresses-Coverity: ("Unintended sign extension")
Fixes: a07df82c79 ("ALSA: usb-audio: Add DJM750 to Pioneer mixer quirk")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
Link: https://lore.kernel.org/r/20210318132008.15266-1-colin.king@canonical.com
Signed-off-by: Takashi Iwai <tiwai@suse.de>
MODULE_SUPPORTED_DEVICE was added in pre-git era and never was
implemented. We can safely remove it, because the kernel has grown
to have many more reliable mechanisms to determine if device is
supported or not.
Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
The problem was in wrong "if" placement. chip->quirk_type is freed
in snd_card_free_when_closed(), but inside if statement it's accesed.
Fixes: 9799110825 ("ALSA: usb-audio: Disable USB autosuspend properly in setup_disable_autosuspend()")
Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
Cc: <stable@vger.kernel.org>
Link: https://lore.kernel.org/r/16da19126ff461e5e64a9aec648cce28fb8ed73e.1615242183.git.paskripkin@gmail.com
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Other Plantronics headset models seem requiring the same workaround as
C320-M to add the 20ms delay for the control messages, too. Apply the
workaround generically for devices with the vendor ID 0x047f.
Note that the problem didn't surface before 5.11 just with luck.
Since 5.11 got a big code rewrite about the stream handling, the
parameter setup procedure has changed, and this seemed triggering the
problem more often.
BugLink: https://bugzilla.suse.com/show_bug.cgi?id=1182552
Cc: <stable@vger.kernel.org>
Link: https://lore.kernel.org/r/20210304085009.4770-1-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Rear audio on Lenovo ThinkStation P620 stops working after commit
1965c4364b ("ALSA: usb-audio: Disable autosuspend for Lenovo
ThinkStation P620"):
[ 6.013526] usbcore: registered new interface driver snd-usb-audio
[ 6.023064] usb 3-6: cannot get ctl value: req = 0x81, wValue = 0x100, wIndex = 0x0, type = 1
[ 6.023083] usb 3-6: cannot get ctl value: req = 0x81, wValue = 0x202, wIndex = 0x0, type = 4
[ 6.023090] usb 3-6: cannot get ctl value: req = 0x81, wValue = 0x100, wIndex = 0x0, type = 1
[ 6.023098] usb 3-6: cannot get ctl value: req = 0x81, wValue = 0x202, wIndex = 0x0, type = 4
[ 6.023103] usb 3-6: cannot get ctl value: req = 0x81, wValue = 0x100, wIndex = 0x0, type = 1
[ 6.023110] usb 3-6: cannot get ctl value: req = 0x81, wValue = 0x202, wIndex = 0x0, type = 4
[ 6.045846] usb 3-6: cannot get ctl value: req = 0x81, wValue = 0x100, wIndex = 0x0, type = 1
[ 6.045866] usb 3-6: cannot get ctl value: req = 0x81, wValue = 0x202, wIndex = 0x0, type = 4
[ 6.045877] usb 3-6: cannot get ctl value: req = 0x81, wValue = 0x100, wIndex = 0x0, type = 1
[ 6.045886] usb 3-6: cannot get ctl value: req = 0x81, wValue = 0x202, wIndex = 0x0, type = 4
[ 6.045894] usb 3-6: cannot get ctl value: req = 0x81, wValue = 0x100, wIndex = 0x0, type = 1
[ 6.045908] usb 3-6: cannot get ctl value: req = 0x81, wValue = 0x202, wIndex = 0x0, type = 4
I overlooked the issue because when I was working on the said commit,
only the front audio is tested. Apology for that.
Changing supports_autosuspend in driver is too late for disabling
autosuspend, because it was already used by USB probe routine, so it can
break the balance on the following code that depends on
supports_autosuspend.
Fix it by using usb_disable_autosuspend() helper, and balance the
suspend count in disconnect callback.
Fixes: 1965c4364b ("ALSA: usb-audio: Disable autosuspend for Lenovo ThinkStation P620")
Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
Cc: <stable@vger.kernel.org>
Link: https://lore.kernel.org/r/20210304043419.287191-1-kai.heng.feng@canonical.com
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Unlike the other DJM, the value to set the "CD/LINE" and "LINE" capture
control options are inverted. This fix makes sure that the displayed
info label while using `alsamixer` matches the input switches label
on the DJM-850 mixer.
Signed-off-by: Nicolas MURE <nicolas.mure2019@gmail.com>
Link: https://lore.kernel.org/r/20210301152729.18094-5-nicolas.mure2019@gmail.com
Signed-off-by: Takashi Iwai <tiwai@suse.de>
This commit only contains the fix about the `URB_CONTROL` request
direction to set the samplerate of Pioneer DJM devices (`URB_CONTROL out`).
Fixes: 3b85f5fc75 ("ALSA: usb-audio: Add DJM450 to Pioneer format quirk")
Signed-off-by: Nicolas MURE <nicolas.mure2019@gmail.com>
Link: https://lore.kernel.org/r/20210301142927.14552-1-nicolas.mure2019@gmail.com
Signed-off-by: Takashi Iwai <tiwai@suse.de>
The Corsair Virtuoso SE RGB Wireless is a USB headset with a mic and a
sidetone feature. Assign the Corsair Virtuoso name map to the SE product
ids as well, in order to label its mixer appropriately and allow
userspace to pick the correct volume controls.
Signed-off-by: Andrea Fagiani <andfagiani@gmail.com>
Cc: <stable@vger.kernel.org>
Link: https://lore.kernel.org/r/40bbdf55-f854-e2ee-87b4-183e6451352c@gmail.com
Signed-off-by: Takashi Iwai <tiwai@suse.de>
A number of devices have named substreams which are hard to remember /
decypher from <device> MIDI n names. Eg. Korg puts a pass through on
one substream and iConnectivity devices name the connections.
This makes it easier to connect to the correct device. Devices which
handle naming through quirks are unaffected by this change.
Addresses TODO comment in sound/usb/midi.c
Signed-off-by: George Harker <george@george-graphics.co.uk>
Link: https://lore.kernel.org/r/20210226212617.24616-1-george@george-graphics.co.uk
Signed-off-by: Takashi Iwai <tiwai@suse.de>
The recent fix for the hw constraints for implicit feedback streams
via commit e4ea77f8e5 ("ALSA: usb-audio: Always apply the hw
constraints for implicit fb sync") added the check of the matching
endpoints and whether those EPs are already opened. This is needed
and correct, per se, even for the normal streams without the implicit
feedback, as the endpoint setup is exclusive.
However, it's reported that there seem applications that behave in
unexpected ways to update the hw_params without clearing the previous
setup via hw_free, and those hit a problem now: then hw_params is
called with still the previous EP setup kept, hence it's restricted
with the previous own setup. Although the obvious fix is to call
snd_pcm_hw_free() API in the application side, it's a kind of
unwelcome change.
This patch tries to ease the situation: in the endpoint check, we add
a couple of more conditions and now skip the endpoint that is being
used only by the stream in question itself. That is, in addition to
the presence check of ep (ep->cur_audiofmt is non-NULL), when the
following conditions are met, we skip such an ep:
- ep->opened == 1, and
- ep->cur_audiofmt == subs->cur_audiofmt.
subs->cur_audiofmt is non-NULL only if it's a re-setup of hw_params,
and ep->cur_audiofmt points to the currently set up parameters. So if
those match, it must be this stream itself.
Fixes: e4ea77f8e5 ("ALSA: usb-audio: Always apply the hw constraints for implicit fb sync")
BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=211941
Cc: <stable@vger.kernel.org>
Link: https://lore.kernel.org/r/20210228080138.9936-1-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Some USB audio firmware seem to report broken dB values for the volume
controls, and this screws up applications like PulseAudio who blindly
trusts the given data. For example, Edifier G2000 reports a PCM
volume from -128dB to -127dB, and this results in barely inaudible
sound.
This patch adds a sort of sanity check at parsing the dB values in
USB-audio driver and disables the dB reporting if the range looks
bogus. Here, we assume -96dB as the bottom line of the max dB.
Note that, if one can figure out that proper dB range later, it can be
patched in the mixer maps.
BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=211929
Cc: <stable@vger.kernel.org>
Link: https://lore.kernel.org/r/20210227105737.3656-1-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
The commit 93db51d06b ("ALSA: usb-audio: Check valid altsetting at
parsing rates for UAC2/3") changed the behavior of the function
set_sample_rate_v2v3() slightly to treat the inconsistent sample rate
as an error. It was done by assumption that the sample rate
validation should have been done at the parser phase as implemented in
that patch. But the validation is later selectively enabled only for
certain devices as it causes a regression (the commit fe773b8711
"ALSA: usb-audio: workaround for iface reset issue"), and now the
inconsistency surfaced as a fatal error while it worked in the past as
is, as reported for FiiO M3K DAC.
For recovering from the regression, change set_sample_rate_v2v3()
again to ignore the sample rate difference as non-error.
BugLink: https://bugzilla.opensuse.org/show_bug.cgi?id=1182633
Fixes: 93db51d06b ("ALSA: usb-audio: Check valid altsetting at parsing rates for UAC2/3")
Cc: <stable@vger.kernel.org>
Link: https://lore.kernel.org/r/20210227082002.21185-1-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
BOSS GP-10 with 0582:0185 requires the similar quirk to make the
implicit feedback working like other BOSS devices.
Reported-by: Keith Milner <kamilner@superlative.org>
Cc: <stable@vger.kernel.org>
Link: https://lore.kernel.org/r/20210214154251.10750-1-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
In the later patch, we're going to issue the PCM sync_stop calls at
disconnection. But currently the USB-audio driver can't handle it
because it has a check of shutdown flag for stopping the URBs. This
is basically superfluous (the stopping URBs are safe at disconnection
state), so let's drop the check.
Fixes: dc5eafe778 ("ALSA: usb-audio: Support PCM sync_stop")
Cc: <stable@vger.kernel.org>
Link: https://lore.kernel.org/r/20210206203052.15606-4-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
The endpoint management has bit flags to indicate the current state,
and we're dealing two things: the running bit and the stopping bit.
There is a thin window in transition from the running to the stopping
in stop_urbs(), and as long as the bit flags are used, it's difficult
to plug.
This patch modifies the state management code to use the atomic int
and follow the explicit three states, STOPPED, RUNNING and STOPPING.
The state change is done via atomic_cmpxhg() for avoiding possible
races, and check the state change more strictly. The unexpected state
change is now handled as an error.
Fixes: d0f09d1e4a ("ALSA: usb-audio: Refactoring endpoint URB deactivation")
Cc: <stable@vger.kernel.org>
Link: https://lore.kernel.org/r/20210206203052.15606-3-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
When we stop an endpoint in release_urbs(), it ignores the
inconsistent endpoint state and tries to release the resources.
This shouldn't happen in theory, but it's still safer to abort the
release and let the caller proper error handling.
Also, stop_and_unlink_urbs() called from release_urbs() does two step
works, and it's more straightforward to split this to two functions
again, so that the call from the PCM trigger won't take the path with
sleeping.
This patch modifies the EP management code to adapt two points above.
Fixes: d0f09d1e4a ("ALSA: usb-audio: Refactoring endpoint URB deactivation")
Cc: <stable@vger.kernel.org>
Link: https://lore.kernel.org/r/20210206203052.15606-2-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
This commit adds mixer quirks for the Pioneer DJM-900NXS2 mixer. This
device has 6 capture channels, 5 of them allow setting the signal
source. This adds controls for these, similar to the DJM-250Mk2.
However, playpack channels are not controllable via software like on the
250Mk2, as they can only be set manually on the mixing console.
Read-only controls showing the currently selected playback channels are
omitted.
Signed-off-by: Fabian Lesniak <fabian@lesniak-it.de>
Link: https://lore.kernel.org/r/20210205215116.258724-2-fabian@lesniak-it.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
This allows for N different devices to use the pioneer mixer quirk for
setting capture/record type and recording level. The impementation has
not changed much with the exception of an additional mask on
private_value to allow storing of a device index:
DEVICE MASK 0xff000000
GROUP_MASK 0x00ff0000
VALUE_MASK 0x0000ffff
This could be improved by changing the arrays of wValues for each
channel to contain named definitions (e.g. SND_DJM_CAP_LINE). It would
improve readability and perhaps would allow using the same array for
multiple channels. The channel number can be specified on the control
next to the wIndex.
Feedback is very much appreciated as I'm not the most proficient C
programmer but am learning as I go.
Signed-off-by: Olivia Mackintosh <livvy@base.nu>
Link: https://lore.kernel.org/r/20210205184256.10201-2-livvy@base.nu
Signed-off-by: Takashi Iwai <tiwai@suse.de>
The commit f274baa49b ("ALSA: usb-audio: Allow non-vmalloc buffer
for PCM buffers") introduced the mode to allocate coherent pages for
PCM buffers, and it used bus->controller device as its DMA device.
It turned out, however, that bus->sysdev is a more appropriate device
to be used for DMA mapping in HCD code.
This patch corrects the device reference accordingly.
Note that, on most platforms, both point to the very same device,
hence this patch doesn't change anything practically. But on
platforms like xhcd-plat hcd, the change becomes effective.
Fixes: f274baa49b ("ALSA: usb-audio: Allow non-vmalloc buffer for PCM buffers")
Cc: <stable@vger.kernel.org>
Link: https://lore.kernel.org/r/20210205144559.29555-1-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
The kerndoc comment for the new function snd_usb_endpoint_free_all()
had a typo wrt the argument name. Fix it.
Fixes: 00272c6182 ("ALSA: usb-audio: Avoid unnecessary interface re-setup")
Reported-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Cc: <stable@vger.kernel.org>
Link: https://lore.kernel.org/r/20210205082837.6327-1-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
As with most Pioneer devices, the device descriptor is vendor specific
and as such, the number of channels, the PCM format, endpoints and
sample rate need to be specified. This device has 8 inputs and 8 outputs
and a sample rate of 48000 only. The PCM format is S24_3LE like other
devices.
There seems to be an appetite for reducing duplication amongs these
Pioneer patches but again, I feel this is a step to be taken after
support has been added as it's not completely clear where the
commonalities are.
Signed-off-by: Olivia Mackintosh <livvy@base.nu>
Link: https://lore.kernel.org/r/20210202134225.3217-3-livvy@base.nu
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Like the DJM-750, ensure that the format control message is passed to
the device when opening a stream. It seems as though fmt->sync_ep is not
always set when this function is called hence the passing of the value
at the call site. If this can be fixed, fmt->sync_up should be used as
the wvalue.
There doesn't seem to be a "cpu_to_le24" type function defined hence for
the open code but I did see a similar thing done in Bluez lib. Perhaps
we can get these definitions defined in byteorder.h. See hci_cpu_to_le24
in include/net/bluetooth/hci.h:2543 for similar usage.
Signed-off-by: Olivia Mackintosh <livvy@base.nu>
Link: https://lore.kernel.org/r/20210202134225.3217-2-livvy@base.nu
Signed-off-by: Takashi Iwai <tiwai@suse.de>
The recently introduced sample rate validation code seems causing a
problem on some devices; namely, after performing this, the bus gets
screwed and it influences even on other USB devices.
As a quick workaround, perform it only for the necessary devices;
currently MOTU devices are known to need the valid altset checks, so
filter out other devices.
Fixes: 93db51d06b ("ALSA: usb-audio: Check valid altsetting at parsing rates for UAC2/3")
Reported-by: Jamie Heilman <jamie@audible.transient.net>
BugLink: https://bugzilla.suse.com/show_bug.cgi?id=1178203
Link: https://lore.kernel.org/r/20210123155842.22652-1-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
At probing a UAC2/UAC3 device like NUX MG-300 USB interface, we get
error messages "RANGE setting not yet supported". It comes the place
where the driver tries to determine the resolution of mixer volumes
via SET_CUR_RES and GET_CUR_RES verbs. Those verbs aren't supported
on UAC2 and UAC3, hence the driver warns like the above. Although the
driver handles this error and works as expected, it's still ugly to
show such errors unnecessarily.
This patch papers over the errors by applying the resolution detection
only for UAC1 and skipping it for UAC2/UAC3.
Reported-by: Mike Oliphant <oliphant@nostatic.org>
Link: https://lore.kernel.org/r/20210120213932.1971-2-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
The current USB-audio driver gets an error at probing NUX MG-300 about
parsing the clocks. This is because the firmware doesn't return the
proper connection of the clock selector that is connected to a single
clock; it's likely that the firmware was lazy^w optimized and the
inquiry wasn't handled. Actually it makes little sense to inquire and
set up the single connection explicitly.
This patch fixes the issue by simply skipping the clock selector
inquiry if it's a single connection.
Reported-by: Mike Oliphant <oliphant@nostatic.org>
Link: https://lore.kernel.org/r/20210120213932.1971-1-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Since the recent refactoring, it's been reported that some USB-audio
devices (typically webcams) are no longer detected properly by
PulseAudio. The debug session revealed that it's failing at probing
by PA to try the sample rate 44.1kHz while the device has discrete
sample rates other than 44.1kHz. But the puzzle was that arecord
works as is, and some other devices with the discrete rates work,
either.
After all, this turned out to be the lack of the dependencies in a few
hw constraint rules: snd_pcm_hw_rule_add() has the (variable)
arguments specifying the dependent parameters, and some functions
didn't set the target parameter itself as the dependencies. This
resulted in an invalid parameter that could be generated only in a
certain call pattern. This bug itself has been present in the code,
but it didn't trigger errors just because the rules were casually
avoiding such a corner case. After the recent refactoring and
cleanup, however, the hw constraints work "as expected", and the
problem surfaced now.
For fixing the problem above, this patch adds the missing dependent
parameters to each snd_pcm_hw_rule() call.
Fixes: bc4e94aa8e ("ALSA: usb-audio: Handle discrete rates properly in hw constraints")
BugLink: http://bugzilla.opensuse.org/show_bug.cgi?id=1181014
Link: https://lore.kernel.org/r/20210120204554.30177-1-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
This adds the Pioneer DJ DJM-750 to the quirks table and ensures
skip_pioneer_sync_ep() is (also) called: this device uses the vendor
ID of 0x08e4 (I'm not sure why they use multiple vendor IDs but many
just like to be awkward it seems).
Playback on all 8 channels works. I'll likely keep this working in the
future and submit futher patches and improvements as necessary.
Signed-off-by: Olivia Mackintosh <livvy@base.nu>
Link: https://lore.kernel.org/r/20210118130621.77miiie47wp7mump@base.nu
Signed-off-by: Takashi Iwai <tiwai@suse.de>
For addressing the regression on Pioneer devices, we recently
corrected the quirk code to enable the implicit feedback mode on those
devices properly. However, the devices still showed problems with the
full duplex operations with JACK, and after debug sessions, we figured
out that the older kernels that had worked with JACK also didn't use
the implicit feedback mode at all although they had the quirk code to
enable it; instead, the old code worked just to skip the normal sync
endpoint setup that would have been detected without it. IOW, what
broke without the implicit-fb quirk in the past was the application of
the normal sync endpoint that is actually the capture data endpoint on
these devices.
This patch covers the overseen piece: it modifies the quirk code again
not to enable the implicit feedback mode but just to make the driver
skipping the sync endpoint detection. This made the driver working
with JACK full-duplex mode again.
Still it's not quite clear why the implicit feedback doesn't work on
those devices yet; maybe it's about some issues in the URB setup. But
at least, with this patch, the driver should work in the level of the
older kernels again.
Fixes: 167c9dc84e ("ALSA: usb-audio: Fix implicit feedback sync setup for Pioneer devices")
Link: https://lore.kernel.org/r/20210118075816.25068-4-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
The UAC2/3 sample rate setup is based on the clock node, which is
usually shared in the interface, and can't be re-setup without
deselecting the interface once, and that's how the current code
behaves. OTOH, the sample rate setup of UAC1 is per endpoint, hence
we basically need to call for each endpoint usage even if those share
the same interface.
This patch fixes the behavior of UAC1 to call always
snd_usb_init_sample_rate() in snd_usb_endpoint_configure().
Fixes: bf6313a0ff ("ALSA: usb-audio: Refactor endpoint management")
Link: https://lore.kernel.org/r/20210118075816.25068-3-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
The current sample rate setup function for UAC1 assumes only the first
endpoint retrieved from the interface:altset pair, but the rate set up
may be needed also for the secondary endpoint. Also, retrieving the
endpoint number from the interface descriptor is redundant; we have
already the target endpoint in the given audioformat object.
This patch simplifies the code and corrects the target endpoint as
described in the above. It simply refers to fmt->endpoint directly.
Also, this patch drops the pioneer_djm_set_format_quirk() that is
caleld from snd_usb_set_format_quirk(); this function does the sample
rate setup but for the capture endpoint (0x82), and that's exactly
what the change above fixes.
Link: https://lore.kernel.org/r/20210118075816.25068-2-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
The last remaining usage of strlcpy() in USB-audio driver is the setup
of the card longname string. Basically we need to know whether any
non-empty string is set or not, and no real length is needed.
Refactor the code and use strscpy() instead. After this change,
strlcpy() is gone from all sound/* code.
Link: https://lore.kernel.org/r/20210115100437.20906-1-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
USB-audio driver still contains two calls of strlcpy() because the
return size is evaluated. Basically it just checks whether the string
is copied or not, but since strcpy() may return a negative error code,
we should check the negative value and treat as filled.
Link: https://lore.kernel.org/r/20210115095758.19707-1-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Since the commit 5a6c3e11c9 ("ALSA: usb-audio: Add hw constraint for
implicit fb sync"), we apply the hw constraints for the implicit
feedback sync to make the secondary open aligned with the already
opened stream setup. This change assumed that the secondary open is
performed after the first stream has been already set up, and adds the
hw constraints to sync with the first stream's parameters only when
the EP setup for the first stream was confirmed at the open time.
However, most of applications handling the full-duplex operations do
open both playback and capture streams at first, then set up both
streams. This results in skipping the additional hw constraints since
the counter-part stream hasn't been set up yet at the open of the
second stream, and it eventually leads to "incompatible EP" error in
the end.
This patch corrects the behavior by always applying the hw constraints
for the implicit fb sync. The hw constraint rules are defined so that
they check the sync EP dynamically at each invocation, instead. This
covers the concurrent stream setups better and lets the hw refine
calls resolving to the right configuration.
Also this patch corrects a minor error that has existed in the debug
print that isn't built as default.
Fixes: 5a6c3e11c9 ("ALSA: usb-audio: Add hw constraint for implicit fb sync")
Link: https://lore.kernel.org/r/20210111081611.12790-1-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Pioneer devices have both playback and capture streams sharing the
same iface/altsetting, and those need to be paired as implicit
feedback. Instead of a half-baked (and broken) static quirk entry,
set up more generically for those devices by checking the number of
endpoints and the attribute of the secondary EP.
Fixes: bf6313a0ff ("ALSA: usb-audio: Refactor endpoint management")
Reported-by: František Kučera <konference@frantovo.cz>
Link: https://lore.kernel.org/r/20210108075219.21463-6-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
There are devices that have multiple endpoints sharing the same
iface/altset not only for sync but also for the actual streams, and
the audioformat for such an endpoint needs to be handled with the
proper endpoint index; otherwise it confuses the endpoint management.
This patch extends the audioformat to annotate the endpoint index, and
put the proper ep_idx=1 to Pioneer device quirk entries accordingly.
Fixes: bf6313a0ff ("ALSA: usb-audio: Refactor endpoint management")
Link: https://lore.kernel.org/r/20210108075219.21463-5-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
The current endpoint handling assumed (more or less) a unique 1:1
relation between the endpoint and the iface/altset. The exception was
the sync EP without the implicit feedback which has usually the
secondary EP of the same altset. This works fine for most devices,
but it turned out that some unusual devices like Pinoeer's ones have
both playback and capture endpoints in the same iface/altsetting and
use both for the implicit feedback mode. For handling such a case, we
need to extend the endpoint management to take the shared interface
into account.
This patch does that: it adds a new object snd_usb_iface_ref for
managing the reference counts of the each USB interface that is used
by each endpoint. The interface setup is performed only once for the
(sharing) endpoints, and the doubly initialization is avoided.
Along with this, the resource release of endpoints and interface
refcounts are put into a single function, snd_usb_endpoint_free_all()
instead of looping in the caller side.
Fixes: bf6313a0ff ("ALSA: usb-audio: Refactor endpoint management")
Link: https://lore.kernel.org/r/20210108075219.21463-4-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
The implicit feedback mode needs to handle two endpoints and the
choice of the audioformat object for the sync EP is important since
this determines the compatibility of the hw_params. The current code
uses the same audioformat object if both the main EP and the sync EP
point to the same iface/altsetting. This was done in consideration of
the non-implicit-fb sync EP handling, and it doesn't match well with
the cases where actually to endpoints are defined in the sameiface /
altsetting like a few Pioneer devices.
Modify snd_usb_find_implicit_fb_sync_format() to pick up the
audioformat that is assigned in the counter-part substreams primarily,
so that the actual capture stream can be opened properly. We keep the
same audioformat object only as a fallback in case nothing found,
though.
Fixes: 9fddc15e80 ("ALSA: usb-audio: Factor out the implicit feedback quirk code")
Link: https://lore.kernel.org/r/20210108075219.21463-3-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
The recent change in the endpoint management moved the endpoint object
creation from the stream open time to the parser of the audio
descriptor. It works fine for the standard audio, but it overlooked
the other places that create audio streams via quirks
(QUIRK_AUDIO_FIXED_ENDPOINT) like the reported a few Pioneer devices;
those call snd_usb_add_audio_stream() manually, hence they miss the
endpoints, eventually resulting in the error at opening streams.
Moreover, now the sync EP setup was moved to the explicit call of
snd_usb_audioformat_set_sync_ep(), and this needs to be added for
those places, too.
This patch addresses those regressions for quirks. It adds a local
helper function add_audio_stream_from_fixed_fmt(), which does the all
needed tasks, and replaces the calls of snd_usb_add_audio_stream()
with this new function.
Fixes: 54cb31901b ("ALSA: usb-audio: Create endpoint objects at parsing phase")
Reported-by: František Kučera <konference@frantovo.cz>
Link: https://lore.kernel.org/r/20210108075219.21463-2-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
strlcpy is deprecated. see: Documentation/process/deprecated.rst
Change the calls that do not use the strlcpy return value to the
preferred strscpy.
Done with cocci script:
@@
expression e1, e2, e3;
@@
- strlcpy(
+ strscpy(
e1, e2, e3);
This cocci script leaves the instances where the return value is
used unchanged.
After this patch, sound/ has 3 uses of strlcpy() that need to be
manually inspected for conversion and changed one day.
$ git grep -w strlcpy sound/
sound/usb/card.c: len = strlcpy(card->longname, s, sizeof(card->longname));
sound/usb/mixer.c: return strlcpy(buf, p->name, buflen);
sound/usb/mixer.c: return strlcpy(buf, p->names[index], buflen);
Miscellenea:
o Remove trailing whitespace in conversion of sound/core/hwdep.c
Link: https://lore.kernel.org/lkml/CAHk-=wgfRnXz0W3D37d01q3JFkr_i_uTL=V6A6G1oUZcprmknw@mail.gmail.com/
Signed-off-by: Joe Perches <joe@perches.com>
Acked-by: Mark Brown <broonie@kernel.org>
Link: https://lore.kernel.org/r/22b393d1790bb268769d0bab7bacf0866dcb0c14.camel@perches.com
Signed-off-by: Takashi Iwai <tiwai@suse.de>
BOSS RC-505 (shown by lsusb as "Roland Corp. RC-505") does require the
same quirk as these other BOSS devices.
Without this quirk it is neither possible to capture audio from nor to
write audio to the RC-505. Both just result in an empty audio
stream. With these changes both capture and playback seem to work
quite fine. MIDI funtionality was not tested.
Tested-by: Harry Reinold <harry.reinold@posteo.de>
Signed-off-by: Timon Reinold <tirei@agon.one>
Link: https://lore.kernel.org/r/20210102210835.21268-1-tirei@agon.one
Signed-off-by: Takashi Iwai <tiwai@suse.de>
BOSS AD-10 requires the very same quirk like other BOSS devices to
enable the special implicit feedback mode.
Reported-and-tested-by: Martin Passing <martin@passing.name>
Link: https://lore.kernel.org/r/20201229083428.20467-1-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Use DIV_ROUND_UP() instead of open-coding it. This documents intent
and makes it more clear what is going on for the casual reviewer.
Generated using the following the Coccinelle semantic patch.
// <smpl>
@@
expression x, y;
@@
-(((x) + (y) - 1) / (y))
+DIV_ROUND_UP(x, y)
// </smpl>
Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Link: https://lore.kernel.org/r/20201223172229.781-11-lars@metafoo.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
The calculation of in_cables and out_cables bitmaps are done with the
bit shift by the value from the descriptor, which is an arbitrary
value, and can lead to UBSAN shift-out-of-bounds warnings.
Fix it by filtering the bad descriptor values with the check of the
upper bound 0x10 (the cable bitmaps are 16 bits).
Reported-by: syzbot+92e45ae45543f89e8c88@syzkaller.appspotmail.com
Cc: <stable@vger.kernel.org>
Link: https://lore.kernel.org/r/20201223174557.10249-1-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
The BOSS GT-1 (USB ID 0582:01d6) requires implicit feedback
like other similar BOSS devices. This patch adds this support.
[ rearranged the table entry in the ID order -- tiwai ]
Signed-off-by: Mike Oliphant <oliphant@nostatic.org>
Link: https://lore.kernel.org/r/20201221215533.2511-1-oliphant@nostatic.org
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Some buggy firmware don't give the current sample rate but leaves
zero. Handle this case more gracefully without warning but just skip
the current rate verification from the next time.
Cc: <stable@vger.kernel.org>
Link: https://lore.kernel.org/r/20201218145858.2357-1-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
The current channel-map control implementation in USB-audio driver may
lead to an error message like
"control 3:0:0:Playback Channel Map:0: access overflow"
when CONFIG_SND_CTL_VALIDATION is set. It's because the chmap get
callback clears the whole array no matter which count is set, and
rather the false-positive detection.
This patch fixes the problem by clearing only the needed array range
at usb_chmap_ctl_get().
Cc: <stable@vger.kernel.org>
Link: https://lore.kernel.org/r/20201211130048.6358-1-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Steinberg UR22 (with USB ID 0499:1509) requires the implicit feedback
for the proper playback, otherwise it causes occasional cracks.
This patch adds the corresponding the quirk table entry with the
recently added generic implicit fb support.
Reported-and-tested-by: Kilian <meschi@posteo.de>
Link: https://lore.kernel.org/r/20201209161835.13625-1-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Yet another quirk for Pioneer DJ DDJ-SR2, which is quite similar like
other DJ DDJ models but with slightly different EPs or channels.
Reported-by: Geraldo <geraldogabriel@gmail.com>
Link: https://lore.kernel.org/r/20201130083714.10640-1-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
This patch allows the Pioneer DJ DDJ-RR to be seen as a USB audio
device under Linux and therefore usable in such applications as
Mixxx.
Tested Master Audio out, headphones (both output jacks) and microphone
input. All work perfectly.
Signed-off-by: Daniel Martin <dmanlfc@gmail.com>
Link: https://lore.kernel.org/r/20201128084035.2958-1-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Now that the usb audio driver correctly finds implicit feedback endpoints,
the implicit feedback quirk for the MOTU M-Series is no longer required.
This also removes some unnecessary vendor specific messages from the MOTU
M-Series boot quirk. The removed vendor specific messages turned on vendor
specific interrupts to the host every 32 samples. The only thing the boot
quirk needs to do is wait for 2 seconds.
Tested-by: Dylan Robinson <dylan_robinson@motu.com>
Signed-off-by: Dylan Robinson <dylan_robinson@motu.com>
Link: https://lore.kernel.org/r/20201123085347.19667-42-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
A few other BOSS devices (BR-80, GT-100v2, Katana) seem requiring the
same quirk as BOSS GT-001, i.e. no implicit feedback for playback but
tying with capture. Add and correct the corresponding quirk table
entries for them.
Reported-and-tested-by: Keith Milner <kamilner@superlative.org>
Link: https://lore.kernel.org/r/20201123085347.19667-41-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
A new module option, implicit_fb, is added to specify the driver
looking for the implicit feedback sync. This can be useful for a
device that could be working better in the implicit feed back mode and
user wants to test it quickly. When this works, we can add the quirk
entry easily.
Tested-by: Keith Milner <kamilner@superlative.org>
Tested-by: Dylan Robinson <dylan_robinson@motu.com>
Link: https://lore.kernel.org/r/20201123085347.19667-40-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
This patch extends the implicit feedback mode parser code to check the
description more generically, so that the quirk entries can be added
without the explicit EP and interface numbers. The search is done for
the next and the previous interface of the given altset, and if both
entries are ASYNC mode and the direction matches, it just takes as the
sync endpoint. The generic parser is applicable only for the playback
stream.
As of now, only a few M-Audio devices have been converted to use this
mode.
Tested-by: Keith Milner <kamilner@superlative.org>
Tested-by: Dylan Robinson <dylan_robinson@motu.com>
Link: https://lore.kernel.org/r/20201123085347.19667-39-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
The code dealing with the implicit feedback mode grew recently, and
it's becoming messy. As we receive more and more devices that need
the similar handling, it's better to be processed through a table
instead of the open code.
This patch moves the code that is relevant with parsing the implicit
feedback mode and some helpers into another file, implicit.c. The
detection and the setup of the implicit feedback sync EPs are
rewritten to use the ID/class matching table instead.
There should be no functional changes.
Tested-by: Keith Milner <kamilner@superlative.org>
Tested-by: Dylan Robinson <dylan_robinson@motu.com>
Link: https://lore.kernel.org/r/20201123085347.19667-38-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
The capture stream of BOSS GT-001 seems always requiring to be tied
with the playback stream. OTOH, the playback stream of this device
doesn't seem working in the implicit fb mode, per se, since the
playback must be running before the capture stream.
This patch tries to address the points above:
- Avoid the implicit fb mode for the playback
- Set up a fake sync EP for the capture stream with the hard-coded
playback stream using the implicit fb mode
Reported-by: Keith Milner <kamilner@superlative.org>
Tested-by: Keith Milner <kamilner@superlative.org>
Tested-by: Dylan Robinson <dylan_robinson@motu.com>
Link: https://lore.kernel.org/r/20201123085347.19667-37-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Now the sync endpoints have been parsed at the beginning and won't be
changed dynamically, let's show them in the proc outputs for helping
debugging.
Tested-by: Keith Milner <kamilner@superlative.org>
Tested-by: Dylan Robinson <dylan_robinson@motu.com>
Link: https://lore.kernel.org/r/20201123085347.19667-36-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Just for consistency, use unsigned char for iface and altsetting in
allover places. Also rearrange the field positions of
snd_usb_endpiont and tidy up with some comments.
Tested-by: Keith Milner <kamilner@superlative.org>
Tested-by: Dylan Robinson <dylan_robinson@motu.com>
Link: https://lore.kernel.org/r/20201123085347.19667-35-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Follow the inclusive terminology, just replace sync_master/sync_slave
with sync_source/sync_sink. It's also a bit clearer from its meaning,
too.
Tested-by: Keith Milner <kamilner@superlative.org>
Tested-by: Dylan Robinson <dylan_robinson@motu.com>
Link: https://lore.kernel.org/r/20201123085347.19667-34-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
There are multiple places in format.c performing the similar code for
setting the rate_min, rate_max and rates fields. This patch unifies
those in a helper function and calls it at the end of the parser phase
so that all rate_table entries have been already determined.
No functional changes, just a minor code refactoring.
Tested-by: Keith Milner <kamilner@superlative.org>
Tested-by: Dylan Robinson <dylan_robinson@motu.com>
Link: https://lore.kernel.org/r/20201123085347.19667-33-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
There are two places calculating the next packet size for the playback
stream in the exactly same way. Provide the single helper for this
purpose and use it from both places gracefully.
Tested-by: Keith Milner <kamilner@superlative.org>
Tested-by: Dylan Robinson <dylan_robinson@motu.com>
Link: https://lore.kernel.org/r/20201123085347.19667-32-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Some fields like interface and alt_idx in snd_usb_substream are mostly
useless now as they can be referred via either cur_audiofmt or
data_endpoint assigned to the substream. Drop those, and also assure
the concurrency about the access of cur_audiofmt field.
Tested-by: Keith Milner <kamilner@superlative.org>
Tested-by: Dylan Robinson <dylan_robinson@motu.com>
Link: https://lore.kernel.org/r/20201123085347.19667-31-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Minor code refactoring to consolidate the URB deactivation code in
endpoint.c. A slight behavior change is that the error handling in
snd_usb_endpoint_start() leaves EP_FLAG_STOPPING now. This should be
synced with the later PCM sync_stop callback.
Tested-by: Keith Milner <kamilner@superlative.org>
Tested-by: Dylan Robinson <dylan_robinson@motu.com>
Link: https://lore.kernel.org/r/20201123085347.19667-30-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
The endpoint objects may be started/stopped concurrently by different
substreams in the case of implicit feedback mode, while the current
code handles the reference counter without any protection.
This patch changes the refcount to atomic_t for avoiding the
inconsistency. We need no reference_t here as the refcount goes only
up to 2.
Also the name "use_count" is renamed to "running" since this is about
actually the running status, not the open refcount.
Tested-by: Keith Milner <kamilner@superlative.org>
Tested-by: Dylan Robinson <dylan_robinson@motu.com>
Link: https://lore.kernel.org/r/20201123085347.19667-29-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
The audioformat is referred in many places but most of usages are
read-only. Let's add const prefix in the possible places.
Tested-by: Keith Milner <kamilner@superlative.org>
Tested-by: Dylan Robinson <dylan_robinson@motu.com>
Link: https://lore.kernel.org/r/20201123085347.19667-28-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
The implicit feedback mode uses a ring buffer for storing the received
packet sizes from the feedback source, and the code has a slight flaw;
when a playback stream stalls by some reason and the URBs aren't
processed, the next_packet FIFO might become empty, but the driver
can't distinguish whether it's empty or full because it's managed with
read_poss and write_pos.
This patch addresses those by changing the next_packet array
management. Instead of keeping read and write positions, now the head
position and the queued amount are kept. It's easier to understand
about the emptiness. Also, the URB active flag is now cleared before
calling queue_pending_output_urbs() for avoiding (theoretically)
possible inconsistency.
Tested-by: Keith Milner <kamilner@superlative.org>
Tested-by: Dylan Robinson <dylan_robinson@motu.com>
Link: https://lore.kernel.org/r/20201123085347.19667-27-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
This is an intensive surgery for the endpoint and stream management
for achieving more robust and clean code.
The goals of this patch are:
- More clear endpoint resource changes
- The interface altsetting control in a single place
Below are brief description of the whole changes.
First off, most of the endpoint operations are moved into endpoint.c,
so that the snd_usb_endpoint object is only referred in other places.
The endpoint object is acquired and released via the new functions
snd_usb_endpoint_open() and snd_usb_endpoint_close() that are called
at PCM hw_params and hw_free callbacks, respectively. Those are
ref-counted and EPs can manage the multiple opens.
The open callback receives the audioformat and hw_params arguments,
and those are used for initializing the EP parameters; especially the
endpoint, interface and altset numbers are read from there, as well as
the PCM parameters like the format, rate and channels. Those are
stored in snd_usb_endpoint object. If it's the secondary open, the
function checks whether the given parameters are compatible with the
already opened EP setup, too.
The coupling with a sync EP (including an implicit feedback sync) is
done by the sole snd_usb_endpoint_set_sync() call.
The configuration of each endpoint is done in a single shot via
snd_usb_endpoint_configure() call. This is the place where most of
PCM configurations are done. A few flags and special handling in the
snd_usb_substream are dropped along with this change.
A significant difference wrt the configuration from the previous code
is the order of USB host interface setups. Now the interface is
always disabled at beginning and (re-)enabled at the last step of
snd_usb_endpoint_configure(), in order to be compliant with the
standard UAC2/3. For UAC1, the interface is set before the parameter
setups since there seem devices that require it (e.g. Yamaha THR10),
just like how it was done in the previous driver code.
The start/stop are almost same as before, also single-shots. The URB
callbacks need to be set via snd_usb_endpoint_set_callback() like the
previous code at the trigger phase, too.
Finally, the flag for the re-setup is set at the device suspend
through the full EP list, instead of PCM trigger. This catches the
overlooked cases where the PCM hasn't been running yet but the device
needs the full setup after resume.
Tested-by: Keith Milner <kamilner@superlative.org>
Tested-by: Dylan Robinson <dylan_robinson@motu.com>
Link: https://lore.kernel.org/r/20201123085347.19667-26-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
The function to evaluate the match of the parameters with an EP
assumes only the discrete rate tables and doesn't handle the
continuous rates properly.
This patch fixes match_endpoint_audioformats() to handle the
continuous rates. Also the almost useless debug prints there are
dropped.
Tested-by: Keith Milner <kamilner@superlative.org>
Tested-by: Dylan Robinson <dylan_robinson@motu.com>
Link: https://lore.kernel.org/r/20201123085347.19667-25-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
The commit 92adc96f8e ("ALSA: usb-audio: set the interface format
after resume on Dell WD19") introduced the workaround for the broken
setup after the resume specifically on a Dell dock model. However,
the full setup should have been performed after the resume on all
devices, as we can't guarantee the same state. So this patch removes
the conditional check and applies the workaround always.
Tested-by: Keith Milner <kamilner@superlative.org>
Tested-by: Dylan Robinson <dylan_robinson@motu.com>
Link: https://lore.kernel.org/r/20201123085347.19667-24-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
The prepare_data_urb and retire_data_urb fields of the endpoint object
are set dynamically at PCM trigger start/stop. Those are evaluated in
the endpoint handler, but there can be a race, especially if two
different PCM substreams are handling the same endpoint for the
implicit feedback case. Also, the data_subs field of the endpoint is
set and accessed dynamically, too, which has the same risk.
As a slight improvement for the concurrency, this patch introduces the
function to set the callbacks and the data in a shot with the memory
barrier. In the reader side, it's also fetched with the memory
barrier.
There is still a room of race if prepare and retire callbacks are set
during executing the URB completion. But such an inconsistency may
happen only for the implicit fb source, i.e. it's only about the
capture stream. And luckily, the capture stream never sets the
prepare callback, hence the problem doesn't happen practically.
Tested-by: Keith Milner <kamilner@superlative.org>
Tested-by: Dylan Robinson <dylan_robinson@motu.com>
Link: https://lore.kernel.org/r/20201123085347.19667-23-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
start_endpoints() may leave the data endpoint running if an error
happens at starting the sync endpoint. We should stop both streams
properly, instead.
While we're at it, move the debug prints into the endpoint.c that is a
more suitable place.
Tested-by: Keith Milner <kamilner@superlative.org>
Tested-by: Dylan Robinson <dylan_robinson@motu.com>
Link: https://lore.kernel.org/r/20201123085347.19667-22-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
A preliminary change for the later big changes. This is a minor code
refactoring to drop the unnecessary arguments that can be retrieved in
a different way.
Tested-by: Keith Milner <kamilner@superlative.org>
Tested-by: Dylan Robinson <dylan_robinson@motu.com>
Link: https://lore.kernel.org/r/20201123085347.19667-21-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
A preliminary change for the later big changes. This is a minor code
refactoring to drop the unnecessary arguments that can be retrieved in
a different way.
Tested-by: Keith Milner <kamilner@superlative.org>
Tested-by: Dylan Robinson <dylan_robinson@motu.com>
Link: https://lore.kernel.org/r/20201123085347.19667-20-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
A preliminary patch for the later big change. Just a minor code
refactoring.
Tested-by: Keith Milner <kamilner@superlative.org>
Tested-by: Dylan Robinson <dylan_robinson@motu.com>
Link: https://lore.kernel.org/r/20201123085347.19667-19-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Setting the active altsetting at changing sample rate seems
unrecommended. The host should deselect the altsetting at first
before that, then select it again.
Tested-by: Keith Milner <kamilner@superlative.org>
Tested-by: Dylan Robinson <dylan_robinson@motu.com>
Link: https://lore.kernel.org/r/20201123085347.19667-18-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Add a helper function to retrieve the usb_host_interface object from
the given interface and altsetting number pair, which is a commonly
used procedure in the driver code.
No functional changes, just minor code refactoring.
Tested-by: Keith Milner <kamilner@superlative.org>
Tested-by: Dylan Robinson <dylan_robinson@motu.com>
Link: https://lore.kernel.org/r/20201123085347.19667-17-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
This behavior turned out to be invalid from the USB spec POV and
shouldn't be applied. As it's an optional flag that is set only via
an card control element that must be hardly used, let's drop it
again.
Tested-by: Keith Milner <kamilner@superlative.org>
Tested-by: Dylan Robinson <dylan_robinson@motu.com>
Link: https://lore.kernel.org/r/20201123085347.19667-16-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Currently snd_usb_endpoint objects are created at first when the
substream is opened and tries to assign the endpoints corresponding to
the matching audioformat. But since basically the all endpoints have
been already parsed and the information have been obtained, we may
create the endpoint objects statically at the init phase. It's easier
to manage for the implicit fb case, for example.
This patch changes the endpoint object management and lets the parser
to create the all endpoint objects.
This change shouldn't bring any functional changes.
Tested-by: Keith Milner <kamilner@superlative.org>
Tested-by: Dylan Robinson <dylan_robinson@motu.com>
Link: https://lore.kernel.org/r/20201123085347.19667-15-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
The implicit feedback mode initializes both the main data stream and
the sync data stream. When a sync stream was already opened, this
would result in the doubly initialization and might screw up things.
Add the check of already opened sync streams and skip the unnecessary
initialization.
Tested-by: Keith Milner <kamilner@superlative.org>
Tested-by: Dylan Robinson <dylan_robinson@motu.com>
Link: https://lore.kernel.org/r/20201123085347.19667-14-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
The file debug.h contains a simple macro for debug prints, and it's
used only in two places, the format parser and the hw_params rules.
The former actually should print a more informative message instead,
so the only users are the hw_parmas rules.
This patch moves the contents of debug.h into the hw_params rules
local code and remove the unneeded includes. Also, the debug print in
the format parser is replaced with the information print with more
useful information, and the raw printk() call is replaced with
pr_debug().
Tested-by: Keith Milner <kamilner@superlative.org>
Tested-by: Dylan Robinson <dylan_robinson@motu.com>
Link: https://lore.kernel.org/r/20201123085347.19667-13-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Several hw_params functions narrows the interval via min/max rule in
the very similar way, so factor out those into a helper function and
use commonly.
No functional changes, just minor code refactoring.
Tested-by: Keith Milner <kamilner@superlative.org>
Tested-by: Dylan Robinson <dylan_robinson@motu.com>
Link: https://lore.kernel.org/r/20201123085347.19667-12-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
In the current code, there is no check at the stream open time whether
the endpoint is being already used by others. In the normal
operations, this shouldn't happen, but in the case of the implicit
feedback mode, it's a common problem with the full duplex operation,
because the capture stream is always opened by the playback stream as
an implicit sync source.
Although we recently introduced the check of such a conflict of
parameters at the PCM hw_params time, it doesn't give any hint at the
hw_params itself and just gives the error. This isn't quite
comfortable, and it caused problems on many applications.
This patch attempts to make the parameter handling easier by
introducing the strict hw constraint matching with the counterpart
stream that is being used. That said, when an implicit feedback
playback stream is running before a capture stream is opened, the
capture stream carries the PCM hw-constraint to allow only the same
sample rate, format, periods and period frames as the running playback
stream. If not opened or there is no conflict of endpoints, the
behavior remains as same as before.
Note that this kind of "weak link" should work for most cases, but
this is no concrete solution; e.g. if an application changes the hw
params multiple times while another stream is opened, this would lead
to inconsistencies.
Tested-by: Keith Milner <kamilner@superlative.org>
Tested-by: Dylan Robinson <dylan_robinson@motu.com>
Link: https://lore.kernel.org/r/20201123085347.19667-11-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
This is a preliminary work for the upcoming hw-constraint change for
the implicit feedback mode.
Currently snd_usb_autoresume() is called at the end of
setup_hwinfo(). It's a bit confusing; because of this implicit
refcount usage, the caller side needs to call snd_usb_autosuspend()
later in the error path although it's not seen inside the function.
Instead, it's clearer to call both snd_usb_autoresume() and suspend()
in the very same function.
It's only refactoring and no functional changes.
Tested-by: Keith Milner <kamilner@superlative.org>
Tested-by: Dylan Robinson <dylan_robinson@motu.com>
Link: https://lore.kernel.org/r/20201123085347.19667-10-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Instead of parsing and evaluating the sync endpoint and the implicit
feedback mode at each time the audio stream is opened, let's parse it
once at the probe time, as the all needed information can be obtained
statically from the descriptor or from the quirk.
This patch extends audioformat struct to record the sync endpoint,
interface and altsetting as well as the implicit feedback flag, which
are filled at parsing the streams. Then, set_sync_endpoint() is much
simplified just to follow the already parsed data.
Tested-by: Keith Milner <kamilner@superlative.org>
Tested-by: Dylan Robinson <dylan_robinson@motu.com>
Link: https://lore.kernel.org/r/20201123085347.19667-9-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
There are a few rooms for improvements wrt the debug prints:
- The EP debug print is shown only at starting, not at stopping
- The EP debug print contains useless object addresses
- Some helpers show the urb and the EP object addresses, too
This patch addresses those shortcomings.
Tested-by: Keith Milner <kamilner@superlative.org>
Tested-by: Dylan Robinson <dylan_robinson@motu.com>
Link: https://lore.kernel.org/r/20201123085347.19667-8-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
The sync EP setup isn't cleared at stopping the stream but expected to
be cleared at the next stream start. This may leave the sync link
setup stale and can spoof wrongly when full duplex streams were
running in the implicit fb sync. Let's initialize them properly at
start and end of the stream.
Tested-by: Keith Milner <kamilner@superlative.org>
Tested-by: Dylan Robinson <dylan_robinson@motu.com>
Link: https://lore.kernel.org/r/20201123085347.19667-7-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Factor out the code to obtain snd_usb_endpoint object matching with
the given endpoint. It'll be used in the later patch to add the
implicit feedback hw-constraint.
No functional change by this patch itself.
Tested-by: Keith Milner <kamilner@superlative.org>
Tested-by: Dylan Robinson <dylan_robinson@motu.com>
Link: https://lore.kernel.org/r/20201123085347.19667-6-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
It seems that many UAC2 devices are with the implicit feedback, but
they couldn't be probed properly because the assumption the driver
takes currently isn't applied: they have the single endpoint for both
data and implicit-fb streams, while we checked only the classical sync
endpoints assigned to the next altsetting in the same interface.
This patch extends the search to match with those typical cases where
the implicit fb stream is found in the next interface number.
While we're at it, slightly refactor the code, not returning 0/-ERROR
but use the standard bool to success/failur, which is more intuitive
in this particular case.
Reported-by: Dylan Robinson <dylan_robinson@motu.com>
Tested-by: Keith Milner <kamilner@superlative.org>
Tested-by: Dylan Robinson <dylan_robinson@motu.com>
Link: https://lore.kernel.org/r/20201123085347.19667-5-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
The current driver code assumes blindly that all found sample rates for
the same endpoint from the UAC2 and UAC3 descriptors can be used no
matter which altsetting, but actually this was wrong: some devices
accept only limited sample rates in each altsetting. For determining
which altsetting supports which rate, we need to verify each sample rate
and check the validity via UAC2_AS_VAL_ALT_SETTINGS. This control
reports back the available altsettings as a bitmap.
This patch implements the missing piece above, the verification and
reconstructs the sample rate tables based on the result.
An open question is how to deal with the altsettings that ended up
with no valid sample rates after verification. At least, there is a
device that showed this problem although the sample rates did work in
the later usage (see bug link). For now, we accept such an altset as
is, assuming that it's a firmware bug.
Reported-by: Dylan Robinson <dylan_robinson@motu.com>
Tested-by: Keith Milner <kamilner@superlative.org>
Tested-by: Dylan Robinson <dylan_robinson@motu.com>
BugLink: https://bugzilla.suse.com/show_bug.cgi?id=1178203
Link: https://lore.kernel.org/r/20201123085347.19667-4-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
The PCM trigger callback is atomic, hence we must not call a function
like usb_set_interface() there. Calling it from there would lead to a
kernel Oops.
Fix it by moving the usb_set_interface() call to set_sync_endpoint().
Also, apply the snd_usb_set_interface_quirk() for consistency, too.
Tested-by: Keith Milner <kamilner@superlative.org>
Tested-by: Dylan Robinson <dylan_robinson@motu.com>
Link: https://lore.kernel.org/r/20201123085347.19667-3-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
In the current code, when the device provides the discrete sample rate
tables with unusual sample rates, the driver tries to gather the whole
values from the audioformat entries and create a hw-constraint rule to
restrict with this single rate list. This is rather inefficient and
may overlook the rates that are associated only with the certain
audioformat entries.
This patch improves the hw constraint setup by rewriting the existing
hw_rule_rate(). The discrete sample rates (identified by rate_table
and nr_rates of format entry) are checked in the existing
hw_rule_rate() instead of extra rules; in the case of discrete rates,
the function compares with each rate table entry and calculates the
min/max values from there. For the contiguous rates, the behavior
doesn't change.
Along with it, snd_usb_pcm_check_knot() and snb_usb_substream
rate_list field become superfluous, thus those are dropped.
Tested-by: Keith Milner <kamilner@superlative.org>
Tested-by: Dylan Robinson <dylan_robinson@motu.com>
Link: https://lore.kernel.org/r/20201123085347.19667-2-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Found one more Logitech device, BCC950 ConferenceCam, which needs
the same delay here. This makes 3 out of 3 devices I have tried.
Therefore, add a delay for all Logitech devices as it does not hurt.
Signed-off-by: Joakim Tjernlund <joakim.tjernlund@infinera.com>
Cc: <stable@vger.kernel.org> # 4.19.y, 5.4.y
Link: https://lore.kernel.org/r/20201117122803.24310-1-joakim.tjernlund@infinera.com
Signed-off-by: Takashi Iwai <tiwai@suse.de>
kernel test robot
Geoffrey D. Bennett
Johan Hovold
Takashi Iwai
Timo Gurr
Jiapeng Chong
Colin Ian King
Olivia Mackintosh
Lucas Endres
Ikjoon Jang
Kai-Heng Feng
Linus Torvalds
Leon Romanovsky
Pavel Skripkin
John Ernberg
Nicolas MURE
Andrea Fagiani
George Harker
Fabian Lesniak
Joe Perches
Timon Reinold
Lars-Peter Clausen
Mike Oliphant
Amadej Kastelic
Daniel Martin
Hector Martin
Dylan Robinson
Joakim Tjernlund