Move comments to better locations, de-duplicate, fix/remove incorrect/
outdated ones, add new ones, and unify spacing somewhat.
While at it, also add testing credits for Jonathan Dowland (SB Live!
Platinum) and myself (E-MU 0404b).
Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Link: https://lore.kernel.org/r/20230422161021.1143903-2-oswald.buddenhagen@gmx.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Unlike the Alice2 chips used on 1st generation E-MU cards, the
Tina/Tina2 chips used on the 2nd gen cards have only six GPIN pins,
which means that we need to use a smaller mask. Failure to do so would
falsify the read data if the FPGA tried to raise an IRQ right at that
moment. This wasn't a problem so far, as we didn't actually enable FPGA
IRQs, but that's going to change soon.
Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Link: https://lore.kernel.org/r/20230422132430.1057490-1-oswald.buddenhagen@gmx.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
- In snd_emu10k1_look_for_ctl(), evaluate the cheap condition first
- In _snd_emu10k1_{audigy_,}init_efx(), don't use expensive bit setting
loops to fill arrays
- In snd_emu_proc_ptr_reg_read(), remove useless condition - iobase can
be only 0 or 0x20
Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Link: https://lore.kernel.org/r/20230422121519.1052813-1-oswald.buddenhagen@gmx.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
These seem to be another instance of cargo-culting from the Windows
driver. It presumably queries the register to decide about the followup
actions, but we don't do that.
Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Link: https://lore.kernel.org/r/20230421141006.1005539-6-oswald.buddenhagen@gmx.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
These seem to be simply cargo-culted from the Windows driver's behavior.
However, the original reason were presumably read-modify-write cycles,
which we don't do.
Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Link: https://lore.kernel.org/r/20230421141006.1005539-5-oswald.buddenhagen@gmx.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
This doesn't do anything snd_emu10k1_init() wouldn't do later, and none
of the things it does seem relevant for the function itself (which is
pretty much about setting up the FPGA). It was probably a Windows
driver behavior cargo-culting artifact.
Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Link: https://lore.kernel.org/r/20230421141006.1005539-4-oswald.buddenhagen@gmx.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
As the register definition clearly states, this is a 16-bit register,
yet we did all accesses as 32-bit. The writes in particular would have
the potential to clear the TIMER register (depending on how the bus/card
actually handles the too long writes).
This commit also introduces a separate define A_GPIO which aliases
A_IOCFG, which better reflects the distinct usage on E-MU cards.
This is done in the same commit to keep the churn down, as we're
touching all involved lines anyway.
Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Link: https://lore.kernel.org/r/20230421141006.1005539-2-oswald.buddenhagen@gmx.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Assert the validity of the registers and values, as them being out of
range would indicate an error in the driver. Consequently, don't bother
returning error codes; they were ignored everywhere anyway.
Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Link: https://lore.kernel.org/r/20230421141006.1005539-1-oswald.buddenhagen@gmx.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Detach it better from the main PCM driver, which it really doesn't have
much in common with.
In particular, this moves the interrupt handler implementation into
p16v.c, and makes it access the substream runtime status more directly,
so it doesn't need to abuse structs snd_emu10k1_pcm and
snd_emu10k1_voice any more.
We don't need private pcm runtime data at all, as the only thing it was
used for (except the back-link to the substream) was the `running` flag.
So store that directly in runtime->private_data.
This somewhat radical strip-down shows that this driver contains some
complexity that was never actually utilized. I suppose the right way to
fully utilize the hardware in a simple way would be introducing more
substreams. This wouldn't require any of the removed code.
Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Link: https://lore.kernel.org/r/20230421141006.1005452-7-oswald.buddenhagen@gmx.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
The use of the variable was removed in commit 2b637da5a1 ("clean up
card features"). That commit also broke user space (the ioctl
structure), at which point the defines became meaningless, so I don't
think purging them is a problem.
Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Link: https://lore.kernel.org/r/20230421141006.1005452-3-oswald.buddenhagen@gmx.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
extin_mask and extout_mask are used only by the SbLive! microcode, so
they have no effect on Audigy.
Eliminate fxbus_mask entirely, as it wasn't actually used for anything.
As a drive-by, remove the pointless pad1 field from struct
snd_emu10k1_fx8010 - it is not visible to user space, so it has no
binary compatibility constraints.
Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Link: https://lore.kernel.org/r/20230421141006.1005509-1-oswald.buddenhagen@gmx.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
When an error occurs during USB disconnection sometimes things can go
wrong as endpoint_set_interface may end up being called repeatedly. For
example:
% dmesg --notime | grep 'usb 3-7.1.4' | sort | uniq -c | head -2
3069 usb 3-7.1.4: 1:1: usb_set_interface failed (-19)
908 usb 3-7.1.4: 1:1: usb_set_interface failed (-71)
In my case, there sometimes are hundreds of these usb_set_interface
failure messages a second when I disconnect the hub that has my USB
audio device.
These messages can take a huge amount of the kmsg ringbuffer and don't
provide any extra information over the previous ones, so ratelimit them.
Signed-off-by: Chris Down <chris@chrisdown.name>
Link: https://lore.kernel.org/r/ZEKf8UYBYa1h4JWR@chrisdown.name
Signed-off-by: Takashi Iwai <tiwai@suse.de>
- Update some outdated info
- Language fixes
- Whitespace/formatting fixes
- Prefer attached over stand-alone '::'
[ dropped a trailing white space in the patch -- tiwai ]
Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Link: https://lore.kernel.org/r/20230421112751.990244-1-oswald.buddenhagen@gmx.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
The auto-silencer supports two modes: "thresholded" to fill up "just
enough", and "top-up" to fill up "as much as possible". The two modes
used rather distinct code paths, which this patch unifies. The only
remaining distinction is how much we actually want to fill.
This fixes a bug in thresholded mode, where we failed to use new_hw_ptr,
resulting in under-fill.
Top-up mode is now more well-behaved and much easier to understand in
corner cases.
This also updates comments in the proximity of silencing-related data
structures.
Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Reviewed-by: Jaroslav Kysela <perex@perex.cz>
Link: https://lore.kernel.org/r/20230420113324.877164-1-oswald.buddenhagen@gmx.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
It is preferred to use typed property access functions (i.e.
of_property_read_<type> functions) rather than low-level
of_get_property/of_find_property functions for reading properties.
Convert reading boolean properties to to of_property_read_bool().
Signed-off-by: Rob Herring <robh@kernel.org>
Link: https://lore.kernel.org/r/20230310144734.1546587-1-robh@kernel.org
Signed-off-by: Takashi Iwai <tiwai@suse.de>
It is preferred to use typed property access functions (i.e.
of_property_read_<type> functions) rather than low-level
of_get_property/of_find_property functions for reading properties. As
part of this, convert of_get_property/of_find_property calls to the
recently added of_property_present() helper when we just want to test
for presence of a property and nothing more.
Signed-off-by: Rob Herring <robh@kernel.org>
Link: https://lore.kernel.org/r/20230310144733.1546500-1-robh@kernel.org
Signed-off-by: Takashi Iwai <tiwai@suse.de>
... in wait_for_avail() and snd_pcm_drain().
t was calculated in seconds, so it would be pretty much always zero, to
be subsequently de-facto ignored due to being max(t, 10)'d. And then it
(i.e., 10) would be treated as secs, which doesn't seem right.
However, fixing it to properly calculate msecs would potentially cause
timeouts when using twice the period size for the default timeout (which
seems reasonable to me), so instead use the buffer size plus 10 percent
to be on the safe side ... but that still seems insufficient, presumably
because the hardware typically needs a moment to fire up. To compensate
for this, we up the minimal timeout to 100ms, which is still two orders
of magnitude less than the bogus minimum.
substream->wait_time was also misinterpreted as jiffies, despite being
documented as being in msecs. Only the soc/sof driver sets it - to 500,
which looks very much like msecs were intended.
Speaking of which, shouldn't snd_pcm_drain() also use substream->
wait_time?
As a drive-by, make the debug messages on timeout less confusing.
Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Link: https://lore.kernel.org/r/20230405201219.2197774-1-oswald.buddenhagen@gmx.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
The newer E-MU cards weren't mentioned at all.
The "partially supported" is removed ahead of it becoming mostly untrue.
Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Link: https://lore.kernel.org/r/20230405201220.2197908-1-oswald.buddenhagen@gmx.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
For allowing the build without CONFIG_PM, add definitions of dummy
functions for snd_ac97_suspend() and snd_ac97_resume() without
CONFIG_PM, too.
Link: https://lore.kernel.org/r/20230330132847.12882-1-tiwai@suse.de
Reviewed-by: Jaroslav Kysela <perex@perex.cz>
Reviewed-by: Tasos Sahanidis <tasos@tasossah.com>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
clang with W=1 reports
sound/pci/asihpi/hpi6000.c:1256:6: error: variable
'loop_count' set but not used [-Werror,-Wunused-but-set-variable]
u32 loop_count = 0;
^
This variable is not used so remove it.
Signed-off-by: Tom Rix <trix@redhat.com>
Link: https://lore.kernel.org/r/20230326205712.1358918-1-trix@redhat.com
Signed-off-by: Takashi Iwai <tiwai@suse.de>
There's no need to switch between unsigned short and u16, especially since
all the functions that end up using old_legacy_ctrl specify u16 anyway.
Signed-off-by: Tasos Sahanidis <tasos@tasossah.com>
Link: https://lore.kernel.org/r/20230329043627.178899-1-tasos@tasossah.com
Signed-off-by: Takashi Iwai <tiwai@suse.de>
YMF744 and newer store the base IO ports in separate PCI config registers.
Since these registers were not restored, when set to a non-default value,
features that rely on them (FM, MPU401, gameport) were not functional
after restore, as their respective IO ports were reset to their defaults.
Signed-off-by: Tasos Sahanidis <tasos@tasossah.com>
Link: https://lore.kernel.org/r/20230329041440.177363-5-tasos@tasossah.com
Signed-off-by: Takashi Iwai <tiwai@suse.de>
In preparation for storing more than two legacy PCI registers, the
existing ones are moved into a new array.
Signed-off-by: Tasos Sahanidis <tasos@tasossah.com>
Link: https://lore.kernel.org/r/20230329041440.177363-4-tasos@tasossah.com
Signed-off-by: Takashi Iwai <tiwai@suse.de>
The registers were previously allocated when CONFIG_PM_SLEEP was set,
however this only saved an insignificant amount of memory otherwise.
Signed-off-by: Tasos Sahanidis <tasos@tasossah.com>
Link: https://lore.kernel.org/r/20230329041440.177363-3-tasos@tasossah.com
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Since commit 1a3c7bb088 ("PM: core: Add new *_PM_OPS macros, deprecate
old ones") SIMPLE_DEV_PM_OPS has been marked deprecated.
The intent is to remove CONFIG_PM_SLEEP guards for PM callbacks. As such
the ifdefs are now removed.
Signed-off-by: Tasos Sahanidis <tasos@tasossah.com>
Link: https://lore.kernel.org/r/20230329041440.177363-2-tasos@tasossah.com
Signed-off-by: Takashi Iwai <tiwai@suse.de>
As an end user, it can be confusing to request an arbitrary IO port be
used only to find out that it doesn't work without an obvious reason,
especially since /sys/module/snd_ymfpci/parameters/{fm,joystick,mpu}_port
indicate 0 after the module has been loaded.
In my case, I was unaware that the YMF724 did not support such usage, and
thus ended up spending time attempting to debug the issue.
Now, when a user attempts to request an IO port that isn't supported by
the hardware, the following message is printed:
[ 25.549530] snd_ymfpci 0000:06:05.0: The Yamaha DS-1 (YMF724F) does not support arbitrary IO ports for FM (requested 0x1234)
Signed-off-by: Tasos Sahanidis <tasos@tasossah.com>
Link: https://lore.kernel.org/r/20230329034204.171901-1-tasos@tasossah.com
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Enhance the documents about the PCM, missing descriptions for a couple
of helpers like snd_pcm_period_elapsed_under_stream_lock() and
snd_pcm_stop_xrun().
Link: https://lore.kernel.org/r/20230323065237.5062-4-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
As PCM ack callback may handle the XRUN situation gracefully now,
change the indirect PCM helpers to give a proper error (-EPIPE).
Also, change the pointer callback helpers to deal with the XRUN error
properly, too.
This requires the PCM core change by the commit 8c721c53dd
("ALSA: usb-audio: Fix recursive locking at XRUN during syncing").
Link: https://lore.kernel.org/r/20230323065237.5062-2-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
It's been reported that the recent kernel can't probe the PCM devices
on Roland VS-100 properly, and it turned out to be a regression by the
recent addition of the bit shift range check for the format bits.
In the old code, we just did bit-shift and it resulted in zero, which
is then corrected to the standard PCM format, while the new code
explicitly returns an error in such a case.
For addressing the regression, relax the check and fallback to the
standard PCM type (with the info output).
Fixes: 43d5ca88df ("ALSA: usb-audio: Fix potential out-of-bounds shift")
Cc: <stable@vger.kernel.org>
Link: https://bugzilla.kernel.org/show_bug.cgi?id=217084
Link: https://lore.kernel.org/r/20230324075005.19403-1-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
clang with W=1 reports
sound/pci/rme9652/hdspm.c:6149:19: error: unused function
'copy_u32_le' [-Werror,-Wunused-function]
static inline int copy_u32_le(void __user *dest, void __iomem *src)
^
This function is not used so remove it.
Signed-off-by: Tom Rix <trix@redhat.com>
Link: https://lore.kernel.org/r/20230323202713.2637150-1-trix@redhat.com
Signed-off-by: Takashi Iwai <tiwai@suse.de>
When reporting errors or skips we currently include the diagnostic message
indicating why we're failing or skipping. This isn't ideal since KTAP
defines the entire print as the test name, so if there's an error then test
systems won't detect the test as being the same one as a passing test. Move
the diagnostic to a separate ksft_print_msg() to avoid this issue, the test
name part will always be the same for passes, fails and skips and the
diagnostic information is still displayed.
Signed-off-by: Mark Brown <broonie@kernel.org>
Link: https://lore.kernel.org/r/20230323-alsa-pcm-test-names-v1-1-8be67a8885ff@kernel.org
Signed-off-by: Takashi Iwai <tiwai@suse.de>
While it is common for driver bugs with events to apply to all events there
are some issues which only trigger for specific values. Understanding these
is easier if we know what we were trying to do when configuring the control
so add logging for the specific values involved in the spurious event.
Signed-off-by: Mark Brown <broonie@kernel.org>
Link: https://lore.kernel.org/r/20230322-alsa-mixer-event-values-v1-1-78189fcf6655@kernel.org
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Unfortunately, in commit 5911d78fab a wrong codec patch was selected.
The model=alc283-dac-wcaps is equivalent to ALC283_FIXUP_CHROME_BOOK not
ALC295_FIXUP_CHROME_BOOK.
Fixes: 5911d78fab ("ALSA: hda/realtek: Improve support for Dell Precision 3260")
Signed-off-by: Jaroslav Kysela <perex@perex.cz>
Link: https://lore.kernel.org/r/20230322153404.386473-1-perex@perex.cz
Signed-off-by: Takashi Iwai <tiwai@suse.de>
The recent support of low latency playback in USB-audio driver made
the snd_usb_queue_pending_output_urbs() function to be called via PCM
ack ops. In the new code path, the function is performed already in
the PCM stream lock. The problem is that, when an XRUN is detected,
the function calls snd_pcm_xrun() to notify, but snd_pcm_xrun() is
supposed to be called only outside the stream lock. As a result, it
leads to a deadlock of PCM stream locking.
For avoiding such a recursive locking, this patch adds an additional
check to the code paths in PCM core that call the ack callback; now it
checks the error code from the callback, and if it's -EPIPE, the XRUN
is handled in the PCM core side gracefully. Along with it, the
USB-audio driver code is changed to follow that, i.e. -EPIPE is
returned instead of the explicit snd_pcm_xrun() call when the function
is performed already in the stream lock.
Fixes: d5f871f89e ("ALSA: usb-audio: Improved lowlatency playback support")
Reported-and-tested-by: John Keeping <john@metanate.com>
Link: https://lore.kernel.org/r/20230317195128.3911155-1-john@metanate.com
Reviewed-by: Jaroslav Kysela <perex@perex.cz>
Reviewed-by; Takashi Sakamoto <o-takashi@sakamocchi.jp>
Link: https://lore.kernel.org/r/20230320142838.494-1-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
The recent commit f83bb25924 ("ALSA: hda/conexant: Add quirk for
LENOVO 20149 Notebook model") introduced a quirk for the device with
17aa:3977, but this caused a regression on another model (Lenovo
Ideadpad U31) with the very same PCI SSID. And, through skimming over
the net, it seems that this PCI SSID is used for multiple different
models, so it's no good idea to apply the quirk with the SSID.
Although we may take a different ID check (e.g. the codec SSID instead
of the PCI SSID), unfortunately, the original patch author couldn't
identify the hardware details any longer as the machine was returned,
and we can't develop the further proper fix.
In this patch, instead, we partially revert the change so that the
quirk won't be applied as default for addressing the regression.
Meanwhile, the quirk function itself is kept, and it's now made to be
applicable via the explicit model=lenovo-20149 option.
Fixes: f83bb25924 ("ALSA: hda/conexant: Add quirk for LENOVO 20149 Notebook model")
Reported-by: Jetro Jormalainen <jje-lxkl@jetro.fi>
Link: https://lore.kernel.org/r/20230308215009.4d3e58a6@mopti
Cc: <stable@vger.kernel.org>
Link: https://lore.kernel.org/r/20230320140954.31154-1-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
