| Age | Commit message (Collapse) | Author |
|---|
|
Linus Walleij <linusw@kernel.org> says:
After a quick look and test-compile I can determine that
all of these drivers include <linux/gpio.h> for no reason
whatsoever, so fixing it is low hanging fruit.
Link: https://patch.msgid.link/20260314-asoc-amd-v1-0-31afed06e022@kernel.org
|
|
Rework the way that kernel log messages are rate-limited or suppressed
while running the cs_dsp KUnit tests.
Under normal conditions cs_dsp doesn't produce an unreasonable number of
log messages, and state changes are relatively infrequent. But the KUnit
tests run through a very large number of test cases, especially error
cases, and this produces an unusually large amount of log output from
cs_dsp.
The original fix for this in commit 10db9f6899dd ("firmware: cs_dsp:
rate-limit log messages in KUnit builds") was effective but not pretty.
It involved different definitions of the log macros for KUnit and
not-KUnit builds, and exported variables for the KUnit tests to disable
log messages. I would have preferred to turn the log macros into real
functions that can contain a KUNIT_STATIC_STUB_REDIRECT(), but the
dev_xxx() macros don't have a version that take va_args, so they can't
be wrapped by a function.
This patch enables the use of a KUNIT_STATIC_STUB_REDIRECT() instead
of exported variables, and avoids the need for different definitions of
the debug macros in KUnit and not-KUnit builds.
- A new function cs_dsp_can_emit_message() returns true if the
messages can be emitted to the kernel log. In a normal not-KUnit build
this function collapses to simply returning true. In KUnit builds it
will rate-limit output, and this uses a single static rate limiter so
it limits the overall rate across all cs_dsp log messages. The KUnit
test can redirect it to change the suppression behavior.
- The cs_dsp debug message macros are changed to only call the dev_xxx()
if cs_dsp_can_emit_message() returns true. These are still macros so
there is no problem wrapping the dev_xxx(). For a normal not-KUnit
build cs_dsp_can_emit_message() always returns true so these macros
simplify down to being identical to calling dev_xxx() directly.
- The KUnit tests that cause a lot of cs_dsp messages now redirect
cs_dsp_can_emit_message() to a local function. This returns false
to suppress cs_dsp messages, unless DEBUG is defined for that test.
I have checked that for a x86_64 production (non-KUnit) build the
disassembled cs_dsp.o is identical to what was generated from the
original code. So the complier is correctly simplifying the
cs_dsp_can_emit_message() and macros down to only the call to dev_xxx().
Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
Link: https://patch.msgid.link/20260310130343.1791951-1-rf@opensource.cirrus.com
Signed-off-by: Mark Brown <broonie@kernel.org>
|
|
Use vmalloc() instead of kmalloc(..., GFP_DMA) to alloc the temporary
buffer for firmware download blobs. This avoids the problem that a
heavily fragmented system cannot allocate enough physically-contiguous
memory for a large blob.
The redundant alloc buffer mechanism was removed in commit 900baa6e7bb0
("firmware: cs_dsp: Remove redundant download buffer allocator").
While doing that I was overly focused on the possibility of the
underlying bus requiring DMA-safe memory. So I used GFP_DMA kmalloc()s.
I failed to notice that the code I was removing used vmalloc().
This creates a regression.
Way back in 2014 the problem of fragmentation with kmalloc()s was fixed
by commit cdcd7f728753 ("ASoC: wm_adsp: Use vmalloc to allocate firmware
download buffer").
Although we don't need physically-contiguous memory, we don't know if the
bus needs some particular alignment of the buffers. Since the change in
2014, the firmware download has always used whatever alignment vmalloc()
returns. To avoid introducing a new problem, the temporary buffer is still
used, to keep the same alignment of pointers passed to regmap_raw_write().
Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
Fixes: 900baa6e7bb0 ("firmware: cs_dsp: Remove redundant download buffer allocator")
Link: https://patch.msgid.link/20260304141250.1578597-1-rf@opensource.cirrus.com
Signed-off-by: Mark Brown <broonie@kernel.org>
|
|
Merge series from Stefan Binding <sbinding@opensource.cirrus.com>:
When the CS35L41 and CS35L45 drivers suspend, they are put into
hibernation, and the regmap goes into cache_only, but the firmware is
still running, and wm_adsp is not stopped. If userspace attempts to
read a firmware control, it will perform a regmap_raw_read() and this
will produce an error in the kernel log.
To prevent these spurious errors, add an apis into cs_dsp and wm_adsp
to allow wm_adsp to hibernate. In this hibernation mode, reads or
writes to the dsp controls would be rejected with -EPERM rather than
-EBUSY, and no error will be printed to the kernel log.
|
|
For some parts, the DSP is kept running when in low power mode
(hibernation), leaving the firmware ALSA controls enabled, but the
registers are inaccessible. Attempts to access volatile firmware
controls whilst in this state would produce errors in the kernel log
due to a regmap_raw_read() into DSP registers whilst the regmap is in
cache_only.
To remove this error log, add a hibernating flag to indicate that the
controls are inaccessible, so we no longer try to read or write to the
registers whilst the regmap is in cache_only.
This would still produce an error when trying to read or write to these
controls, but this would be a different error (-EPERM instead of
-EBUSY), and would not produce a spurious error log in the kernel.
Upon wake from hibernation, the control caches are re-synced to the
hardware, if the DSP is running.
Signed-off-by: Stefan Binding <sbinding@opensource.cirrus.com>
Link: https://patch.msgid.link/20260224161821.93365-2-sbinding@opensource.cirrus.com
Reviewed-by: Richard Fitzgerald <rf@opensource.cirrus.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
|
|
Most of the test suites perform a large number of test cases, so
mark the suites as KUNIT_SPEED_SLOW for anyone who finds that useful.
The callback test suites haven't been marked because they don't
perform many test cases, and the tests are quick.
Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
Link: https://patch.msgid.link/20260225125644.1248826-1-rf@opensource.cirrus.com
Signed-off-by: Mark Brown <broonie@kernel.org>
|
|
This was done entirely with mindless brute force, using
git grep -l '\<k[vmz]*alloc_objs*(.*, GFP_KERNEL)' |
xargs sed -i 's/\(alloc_objs*(.*\), GFP_KERNEL)/\1)/'
to convert the new alloc_obj() users that had a simple GFP_KERNEL
argument to just drop that argument.
Note that due to the extreme simplicity of the scripting, any slightly
more complex cases spread over multiple lines would not be triggered:
they definitely exist, but this covers the vast bulk of the cases, and
the resulting diff is also then easier to check automatically.
For the same reason the 'flex' versions will be done as a separate
conversion.
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
|
|
This is the result of running the Coccinelle script from
scripts/coccinelle/api/kmalloc_objs.cocci. The script is designed to
avoid scalar types (which need careful case-by-case checking), and
instead replace kmalloc-family calls that allocate struct or union
object instances:
Single allocations: kmalloc(sizeof(TYPE), ...)
are replaced with: kmalloc_obj(TYPE, ...)
Array allocations: kmalloc_array(COUNT, sizeof(TYPE), ...)
are replaced with: kmalloc_objs(TYPE, COUNT, ...)
Flex array allocations: kmalloc(struct_size(PTR, FAM, COUNT), ...)
are replaced with: kmalloc_flex(*PTR, FAM, COUNT, ...)
(where TYPE may also be *VAR)
The resulting allocations no longer return "void *", instead returning
"TYPE *".
Signed-off-by: Kees Cook <kees@kernel.org>
|
|
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound into for-linus
ASoC: Updates for v7.0
This release is almost all abut driers, there's very little core work
here, although some of that driver work is in more generic areas like
SDCA and SOF:
- Generic SDCA support for reporting jack events.
- Continuing platform support, cleanup and feature improements for the
AMD, Intel, Qualcomm and SOF code.
- Platform description improvements for the Cirrus drivers.
- Support for NXP i.MX952, Realtek RT1320 and RT5575, and Sophogo
CV1800B.
We also pulled in one small SPI API update and some more substantial
regmap work (cache description improvements) for use in drivers.
|
|
Use the dev_*_ratelimit() macros if the cs_dsp KUnit tests are enabled
in the build, and allow the KUnit tests to disable message output.
Some of the KUnit tests cause a very large number of log messages from
cs_dsp, because the tests perform many different test cases. This could
cause some lines to be dropped from the kernel log. Dropped lines can
prevent the KUnit wrappers from parsing the ktap output in the dmesg log.
The KUnit builds of cs_dsp export three bools that the KUnit tests can
use to entirely disable log output of err, warn and info messages. Some
tests have been updated to use this, replacing the previous fudge of a
usleep() in the exit handler of each test. We don't necessarily want to
disable all log messages if they aren't expected to be excessive,
so the rate-limiting allows leaving some logging enabled.
The rate-limited macros are not used in normal builds because it is not
appropriate to rate-limit every message. That could cause important
messages to be dropped, and there wouldn't be such a high rate of
messages in normal operation.
Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
Reported-by: Mark Brown <broonie@kernel.org>
Closes: https://lore.kernel.org/linux-sound/af393f08-facb-4c44-a054-1f61254803ec@opensource.cirrus.com/T/#t
Fixes: cd8c058499b6 ("firmware: cs_dsp: Add KUnit testing of bin error cases")
Link: https://patch.msgid.link/20260130171256.863152-1-rf@opensource.cirrus.com
Signed-off-by: Mark Brown <broonie@kernel.org>
|
|
Add test cases for using the new long-offset block types to patch
memory that is >0xffff from the algorithm base.
This is just adding entries to the parameter data that have larger
offset values.
Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
Link: https://patch.msgid.link/20251231172711.450024-9-rf@opensource.cirrus.com
Signed-off-by: Mark Brown <broonie@kernel.org>
|
|
Run the patch block test cases using the new long-offset block type.
This adds a new set of parameterization that uses
cs_dsp_mock_bin_add_patch_off32() to create the patch blocks in the
test bin file.
The test cases for Halo Core with V3 file format include another
run of the tests with the new parameterization, so that the tests
are run on the standard blocks and the long-offset blocks.
This commit does not add any new cases for offsets > 0xffff.
Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
Link: https://patch.msgid.link/20251231172711.450024-8-rf@opensource.cirrus.com
Signed-off-by: Mark Brown <broonie@kernel.org>
|
|
Increase the size of the XM and YM regions in the mock Halo Core
register map for testing patch blocks with 32-bit offsets.
Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
Link: https://patch.msgid.link/20251231172711.450024-7-rf@opensource.cirrus.com
Signed-off-by: Mark Brown <broonie@kernel.org>
|
|
Add cs_dsp_mock_bin_add_patch_off32(). This is the same as
cs_dsp_mock_bin_add_patch() except that it puts the offset in the
new 32-bit offset field and modifies the block type to indicate
that it uses the long offset.
Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
Link: https://patch.msgid.link/20251231172711.450024-6-rf@opensource.cirrus.com
Signed-off-by: Mark Brown <broonie@kernel.org>
|
|
Add an argument to cs_dsp_mock_bin_add_raw_block() to pass a 32-bit
offset, and change the type of the existing offset argument to u16.
The cs_dsp_test_bin_error.c test uses cs_dsp_mock_bin_add_raw_block()
so it needs corresponding updates to pass 0 as the 32-bit offset.
Version 1 and 2 of the bin file format had a 16-bit offset on blocks
and the sample rate field of the blocks was not used. Version 3 adds
new block types that change the old sample rate field to be a 32-bit
offset with the old offset currently unused.
cs_dsp_mock_bin_add_raw_block() doesn't attempt to do any magic - its
purpose is to create a raw block exactly as specified by the calling
test code. So the test case can pass a value for both offset fields.
Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
Link: https://patch.msgid.link/20251231172711.450024-5-rf@opensource.cirrus.com
Signed-off-by: Mark Brown <broonie@kernel.org>
|
|
Make the call to cs_dsp_mock_bin_add_patch() a function pointer in
the test case parameters, instead of calling it directly.
This is to allow for future parameterization by which function to
call to add a patch.
Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
Link: https://patch.msgid.link/20251231172711.450024-4-rf@opensource.cirrus.com
Signed-off-by: Mark Brown <broonie@kernel.org>
|
|
The new v3 file format has all the same functionality as the earlier
formats, so run all the existing test cases with a file type of 3.
This is only done for Halo Core because v3 files are not used with the
older ADSP cores.
Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
Link: https://patch.msgid.link/20251231172711.450024-3-rf@opensource.cirrus.com
Signed-off-by: Mark Brown <broonie@kernel.org>
|
|
Handle a new type of data block that has a 32-bit offset. These are
identical to the normal blocks except that the offset is now in the
32-bit field that was previously 'sr'.
A new file version of 3 indicates that it is mandatory to process
the long-offset blocks, so that older code without that support will
reject the file.
The original 'sr' field was never used by the driver so it has been
renamed offset32.
Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
Link: https://patch.msgid.link/20251231172711.450024-2-rf@opensource.cirrus.com
Signed-off-by: Mark Brown <broonie@kernel.org>
|
|
Replace the __free(kfree) in cs_dsp_load() and cs_dsp_load_coeff() with
a kfree(buf) at the end of the function.
The use of __free() can create new cleanup bugs that are difficult to spot
because the defective code is idiomatically correct regular C. In these two
functions the __free() was mixed with gotos, and also used the suspect
declaration __free(kfree) = NULL;.
The __free() did not do anything to simplify the code. There aren't any
early returns after the pointer is set, and the __free() can be replaced by
a kfree() at the end of the function.
Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
Fixes: 900baa6e7bb0 ("firmware: cs_dsp: Remove redundant download buffer allocator")
Link: https://patch.msgid.link/20251201160729.231867-1-rf@opensource.cirrus.com
Signed-off-by: Mark Brown <broonie@kernel.org>
|
|
Don't use __free(kfree) in cs_dsp_debugfs_string_read. Instead use
normal kfree() to cleanup.
The use of __free() can create new cleanup bugs that are difficult to spot
because the defective code is idiomatically correct regular C. This
function used the suspect declaration __free(kfree) = NULL;.
The __free(kfree) didn't really do anything here. The function can be
rearranged to avoid any need to return or goto within the code.
Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
Fixes: 3045e29d248b ("firmware: cs_dsp: Append \n to debugfs string during read")
Link: https://patch.msgid.link/20251202113425.413700-1-rf@opensource.cirrus.com
Signed-off-by: Mark Brown <broonie@kernel.org>
|
|
Add test cases for the client not providing a pointer to a
struct cs_dsp_client_ops. This proves that it is safe for the client
to leave the client_ops pointer at NULL if it does not need to
implement any of the callbacks.
Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
Link: https://patch.msgid.link/20251128102132.1575177-3-rf@opensource.cirrus.com
Signed-off-by: Mark Brown <broonie@kernel.org>
|
|
A client of cs_dsp does not necessarily need to implement any of the
optional callbacks in struct cs_dsp_client_ops, so allow the client_ops
pointer to be NULL.
This has been done by pointing client_ops at a default empty
cs_dsp_client_ops. It keeps the code cleaner by avoiding having to add
double nested NULL checks everywhere one of these callbacks is called.
Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
Link: https://patch.msgid.link/20251128102132.1575177-2-rf@opensource.cirrus.com
Signed-off-by: Mark Brown <broonie@kernel.org>
|
|
Use kvzalloc() instead of kzalloc() to allocate memory to cache the
content of a firmware control.
Most firmware controls are only small, typically a few bytes. But on
some firmware there can be much larger controls for coefficient or
model data.
The overhead of kvzalloc() is negligible because most control allocs
can be satisfied by the normal kmalloc() that kvzalloc() will try first.
Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
Link: https://patch.msgid.link/20251127103947.1094934-1-rf@opensource.cirrus.com
Signed-off-by: Mark Brown <broonie@kernel.org>
|
|
In cs_dsp_debugfs_read_controls_show() take the pwr_lock mutex
around the list walk. This protects against debugfs returning
a partial set of new controls if those controls are being added
to the list while it is being walked.
Controls are never deleted from this list, and are only added to
the end of the list. So there was never a danger of following a
stale pointer to garbage.
The worst case was that the printed list is truncated if it saw an
entry that was the list end just before a new entry was appended to
the list.
With the original code, the truncated list from the debugfs could
show only _some_ of the new entries. This could be confusing because
it appears that some new entries are missing.
Adding the mutex means that the debugfs read provides an atomic view.
Either it shows the old content before any of the new controls were
added; or it shows the new content after all the new controls are
added.
Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
Link: https://patch.msgid.link/20251127113238.1251352-1-rf@opensource.cirrus.com
Signed-off-by: Mark Brown <broonie@kernel.org>
|
|
Now that cs_dsp uses regmap_raw_write() instead of regmap_raw_write_async()
it doesn't need to keep multiple DMA-safe buffers of every chunk of data
it wrote.
See commit fe08b7d5085a ("firmware: cs_dsp: Remove async regmap writes")
Only one write can be in progress at a time, so one DMA-safe buffer can be
re-used. The single DMA-safe buffer is reallocated if the next write chunk
is larger. Reallocation size is rounded up to reduce the amount of churn.
PAGE_SIZE is used as a convenient size multiple. Typically for .wmfw files
the first chunk is the largest.
A DMA-safe intermediate buffer is used because we can't assume that the
bus underlying regmap can accept non-DMA-safe buffers.
Note that a chunk from the firmware file cannot simply be split into
arbitrarily-sized chunks to avoid buffer reallocation. The data in the
firmware file is preformatted to be written directly to the device as one
block. It already takes account of alignment, write-bursts and write
length requirements of the target hardware, so that the cs_dsp driver can
treat it as an opaque blob.
Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
Link: https://patch.msgid.link/20251126131501.884188-1-rf@opensource.cirrus.com
Signed-off-by: Mark Brown <broonie@kernel.org>
|
|
The architectures supported by this driver have a maximum of 32-bits
of address, so we don't need more than 32-bits to store the length of
control data. Change the length in struct cs_dsp_coeff_ctl to an
unsigned int instead of a size_t. Also make a corresponding trivial
change to wm_adsp.c to prevent a compiler warning.
Tested on x86_64 builds this saves at least 4 bytes per control
(another 4 bytes might be saved if the compiler was inserting padding
to align the size_t).
Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
Link: https://patch.msgid.link/20251124171536.78962-1-rf@opensource.cirrus.com
Signed-off-by: Mark Brown <broonie@kernel.org>
|
|
Append the terminating \n to the string during the debugfs file
read instead of creating a string that already has a trailing \n.
This avoids the ugly behaviour of having to include a trailing \n
in the filenames stored to wmfw_file_name and bin_file_name in
struct cs_dsp.
Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
Link: https://patch.msgid.link/20251120130640.1169780-3-rf@opensource.cirrus.com
Signed-off-by: Mark Brown <broonie@kernel.org>
|
|
cs_dsp_debugfs_wmfw_read() and cs_dsp_debugfs_bin_read() were identical
except for which struct member they printed. Move all this duplicated
code into a common function cs_dsp_debugfs_string_read().
The check for dsp->booted has been removed because this is redundant.
The two strings are set when the DSP is booted and cleared when the
DSP is powered-down.
Access to the string char * must be protected by the pwr_lock mutex. The
string is passed into cs_dsp_debugfs_string_read() as a pointer to the
char * so that the mutex lock can also be factored out into
cs_dsp_debugfs_string_read().
wmfw_file_name and bin_file_name members of struct cs_dsp have been
changed to const char *. It makes for a better API to pass a const
pointer into cs_dsp_debugfs_string_read().
Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
Link: https://patch.msgid.link/20251120130640.1169780-2-rf@opensource.cirrus.com
Signed-off-by: Mark Brown <broonie@kernel.org>
|
|
Remove two unused pointers from struct cs_dsp_coeff_ctl by taking the
struct list_head out of struct cs_dsp_alg_region. On a x86_64 build
this saves 16 bytes per control.
Each cs_dsp_coeff_ctl instance needs to keep information about the
algorithm region it refers to. This is done by embedding an instance
of struct cs_dsp_alg_region. But cs_dsp_alg_region was also used to
store entries in a list of algorithm regions, and so had a struct
list_head object for that purpose. This list_head object is not used
with the embedded object in struct cs_dsp_alg_region so was just
wasted bytes.
A new struct cs_dsp_alg_region_list_item has been defined for creating
the list of algorithm regions. It contains a struct cs_dsp_alg_region
and a struct list_head.
Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
Link: https://patch.msgid.link/20250616103052.66537-1-rf@opensource.cirrus.com
Signed-off-by: Mark Brown <broonie@kernel.org>
|
|
KASAN reported out of bounds access - cs_dsp_ctl_cache_init_multiple_offsets().
The code uses mock_coeff_template.length_bytes (4 bytes) for register value
allocations. But later, this length is set to 8 bytes which causes
test code failures.
As fix, just remove the lenght override, keeping the original value 4
for all operations.
Cc: Simon Trimmer <simont@opensource.cirrus.com>
Cc: Charles Keepax <ckeepax@opensource.cirrus.com>
Cc: Richard Fitzgerald <rf@opensource.cirrus.com>
Cc: patches@opensource.cirrus.com
Cc: stable@vger.kernel.org
Signed-off-by: Jaroslav Kysela <perex@perex.cz>
Reviewed-by: Richard Fitzgerald <rf@opensource.cirrus.com>
Link: https://patch.msgid.link/20250523154151.1252585-1-perex@perex.cz
Signed-off-by: Mark Brown <broonie@kernel.org>
|
|
KASAN reported out of bounds access - cs_dsp_mock_wmfw_add_info(),
because the source string length was rounded up to the allocation size.
Cc: Simon Trimmer <simont@opensource.cirrus.com>
Cc: Charles Keepax <ckeepax@opensource.cirrus.com>
Cc: Richard Fitzgerald <rf@opensource.cirrus.com>
Cc: patches@opensource.cirrus.com
Cc: stable@vger.kernel.org
Signed-off-by: Jaroslav Kysela <perex@perex.cz>
Reviewed-by: Richard Fitzgerald <rf@opensource.cirrus.com>
Link: https://patch.msgid.link/20250523155814.1256762-1-perex@perex.cz
Signed-off-by: Mark Brown <broonie@kernel.org>
|
|
KASAN reported out of bounds access - cs_dsp_mock_bin_add_name_or_info(),
because the source string length was rounded up to the allocation size.
Cc: Simon Trimmer <simont@opensource.cirrus.com>
Cc: Charles Keepax <ckeepax@opensource.cirrus.com>
Cc: Richard Fitzgerald <rf@opensource.cirrus.com>
Cc: patches@opensource.cirrus.com
Cc: stable@vger.kernel.org
Signed-off-by: Jaroslav Kysela <perex@perex.cz>
Link: https://patch.msgid.link/20250523102102.1177151-1-perex@perex.cz
Reviewed-by: Richard Fitzgerald <rf@opensource.cirrus.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
|
|
They have build/application dependencies for some new changes coming in.
|
|
Add sanity checking to some test harness functions to help catch bugs
in the test code. This consists of checking the range of some arguments
and checking that reads from the dummy regmap succeed.
Most of the harness code already had sanity-checking but there were a
few places where it was missing or was assumed that the test could be
trusted to pass valid values.
Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
Link: https://patch.msgid.link/20250416122422.783215-1-rf@opensource.cirrus.com
Signed-off-by: Mark Brown <broonie@kernel.org>
|
|
Merge series from Richard Fitzgerald <rf@opensource.cirrus.com>:
This series fixes the KConfig for cs_dsp and cs-amp-lib tests so that
CONFIG_KUNIT_ALL_TESTS doesn't cause them to add modules to the build.
|
|
FW_CS_DSP gets enabled if KUNIT is enabled. The test should rather
depend on if the feature is enabled. Fix this by moving FW_CS_DSP to the
depends on clause.
Fixes: dd0b6b1f29b9 ("firmware: cs_dsp: Add KUnit testing of bin file download")
Signed-off-by: Nico Pache <npache@redhat.com>
Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
Link: https://patch.msgid.link/20250411123608.1676462-4-rf@opensource.cirrus.com
Reviewed-by: David Gow <davidgow@google.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
|
|
Call cs_dsp_mock_xm_header_get_fw_version() to get the firmware version
from the dummy XM header data in cs_dsp_bin_err_test_common_init().
Make the same change to cs_dsp_bin_test_common_init() and remove the
cs_dsp_mock_xm_header_get_fw_version_from_regmap() function.
The code in cs_dsp_test_bin.c was correctly calling
cs_dsp_mock_xm_header_get_fw_version_from_regmap() to fetch the fw version
from a dummy header it wrote to XM registers. However in
cs_dsp_test_bin_error.c the test doesn't stuff a dummy header into XM, it
populates it the normal way using a wmfw file. It should have called
cs_dsp_mock_xm_header_get_fw_version() to get the data from its blob
buffer, but was calling cs_dsp_mock_xm_header_get_fw_version_from_regmap().
As nothing had been written to the registers this returned the value of
uninitialized data.
The only other use of cs_dsp_mock_xm_header_get_fw_version_from_regmap()
was cs_dsp_test_bin.c, but it doesn't need to use it. It already has a
blob buffer containing the dummy XM header so it can use
cs_dsp_mock_xm_header_get_fw_version() to read from that.
Fixes: cd8c058499b6 ("firmware: cs_dsp: Add KUnit testing of bin error cases")
Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
Link: https://patch.msgid.link/20250410132129.1312541-1-rf@opensource.cirrus.com
Signed-off-by: Mark Brown <broonie@kernel.org>
|
|
Set ret = 0 on successful completion of the processing loop in
cs_dsp_load() and cs_dsp_load_coeff() to ensure that the function
returns 0 on success.
All normal firmware files will have at least one data block, and
processing this block will set ret == 0, from the result of either
regmap_raw_write() or cs_dsp_parse_coeff().
The kunit tests create a dummy firmware file that contains only the
header, without any data blocks. This gives cs_dsp a file to "load"
that will not cause any side-effects. As there aren't any data blocks,
the processing loop will not set ret == 0.
Originally there was a line after the processing loop:
ret = regmap_async_complete(regmap);
which would set ret == 0 before the function returned.
Commit fe08b7d5085a ("firmware: cs_dsp: Remove async regmap writes")
changed the regmap write to a normal sync write, so the call to
regmap_async_complete() wasn't necessary and was removed. It was
overlooked that the ret here wasn't only to check the result of
regmap_async_complete(), it also set the final return value of the
function.
Fixes: fe08b7d5085a ("firmware: cs_dsp: Remove async regmap writes")
Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
Link: https://patch.msgid.link/20250323170529.197205-1-rf@opensource.cirrus.com
Signed-off-by: Mark Brown <broonie@kernel.org>
|
|
Merge branch 'for-6.14' of
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound into
asoc-6.15 to avoid a bunch of add/add conflicts.
|
|
Change calls to async regmap write functions to use the normal
blocking writes so that the cs35l56 driver can use spi_bus_lock() to
gain exclusive access to the SPI bus.
As this is part of a fix, it makes only the minimal change to swap the
functions to the blocking equivalents. There's no need to risk
reworking the buffer allocation logic that is now partially redundant.
The async writes are a 12-year-old workaround for inefficiency of
synchronous writes in the SPI subsystem. The SPI subsystem has since
been changed to avoid the overheads, so this workaround should not be
necessary.
The cs35l56 driver needs to use spi_bus_lock() prevent bus activity
while it is soft-resetting the cs35l56. But spi_bus_lock() is
incompatible with spi_async() calls, which will fail with -EBUSY.
Fixes: 8a731fd37f8b ("ASoC: cs35l56: Move utility functions to shared file")
Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
Link: https://patch.msgid.link/20250225131843.113752-2-rf@opensource.cirrus.com
Signed-off-by: Mark Brown <broonie@kernel.org>
|
|
The char pointers in 'struct cs_dsp_mock_coeff_def' are expected to
point to C strings. They need to be terminated by a null byte.
However the code does not allocate that trailing null byte and only
works if by chance the allocation is followed by such a null byte.
Refactor the repeated string allocation logic into a new helper which
makes sure the terminating null is always present.
It also makes the code more readable.
Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
Fixes: 83baecd92e7c ("firmware: cs_dsp: Add KUnit testing of control parsing")
Cc: stable@vger.kernel.org
Reviewed-by: Richard Fitzgerald <rf@opensource.cirrus.com>
Tested-by: Richard Fitzgerald <rf@opensource.cirrus.com>
Link: https://patch.msgid.link/20250211-cs_dsp-kunit-strings-v1-1-d9bc2035d154@linutronix.de
Signed-off-by: Mark Brown <broonie@kernel.org>
|
|
Re-use the adsp2 test cases for the Halo Core test run. Before this the
Halo Core kunit_case array was an empty placeholder.
Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
Link: https://patch.msgid.link/20250219152132.1285941-1-rf@opensource.cirrus.com
Signed-off-by: Mark Brown <broonie@kernel.org>
|
|
Enabling a (modular) test should not silently enable additional kernel
functionality, as that may increase the attack vector of a product.
Fix this by making FW_CS_DSP_KUNIT_TEST (and FW_CS_DSP_KUNIT_TEST_UTILS)
depend on REGMAP instead of selecting it.
After this, one can safely enable CONFIG_KUNIT_ALL_TESTS=m to build
modules for all appropriate tests for ones system, without pulling in
extra unwanted functionality, while still allowing a tester to manually
enable REGMAP_BUILD and this test suite on a system where REGMAP is not
enabled by default.
Fixes: dd0b6b1f29b92202 ("firmware: cs_dsp: Add KUnit testing of bin file download")
Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
Link: https://patch.msgid.link/73c81ac85e21f1c5a75b7628d90cbb0e1b4ed0fa.1737833376.git.geert@linux-m68k.org
Signed-off-by: Mark Brown <broonie@kernel.org>
|
|
Delete two redundant assignments in cs_dsp_test_bin.c.
Unfortunately none of W=1 or the static analysis tools I ran
flagged these.
Fixes: dd0b6b1f29b9 ("firmware: cs_dsp: Add KUnit testing of bin file download")
Reported-by: Dheeraj Reddy Jonnalagadda <dheeraj.linuxdev@gmail.com>
Closes: https://scan7.scan.coverity.com/#/project-view/52337/11354?selectedIssue=1602511
Closes: https://scan7.scan.coverity.com/#/project-view/52337/11354?selectedIssue=1602490
Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
Link: https://patch.msgid.link/20241219155719.84276-1-rf@opensource.cirrus.com
Signed-off-by: Mark Brown <broonie@kernel.org>
|
|
In cs_dsp_mock_wmfw_add_coeff_desc() the value stored in longstring->len
needs a cpu_to_le16() conversion.
Fixes: 5cf1b7b47180 ("firmware: cs_dsp: Add mock wmfw file generator for KUnit testing")
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202412170233.8DnsdtY6-lkp@intel.com/
Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
Link: https://patch.msgid.link/20241217105624.139479-1-rf@opensource.cirrus.com
Signed-off-by: Mark Brown <broonie@kernel.org>
|
|
In cs_dsp_mock_xm_header_drop_from_regmap_cache() for the ADSP2 case read
the big-endian firmware word into a dedicated __be32 variable instead of
using the same u32 for both the big-endian and cpu-endian value.
Fixes: 41e78c0f44f9 ("firmware: cs_dsp: Add mock DSP memory map for KUnit testing")
Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
Link: https://patch.msgid.link/20241217113127.186736-1-rf@opensource.cirrus.com
Signed-off-by: Mark Brown <broonie@kernel.org>
|
|
Having 1280 bytes of local variables on the stack exceeds the limit
on 32-bit architectures:
drivers/firmware/cirrus/test/cs_dsp_test_bin.c: In function 'bin_patch_mixed_packed_unpacked_random':
drivers/firmware/cirrus/test/cs_dsp_test_bin.c:2097:1: error: the frame size of 1784 bytes is larger than 1024 bytes [-Werror=frame-larger-than=]
Use dynamic allocation for the largest two here.
Fixes: dd0b6b1f29b9 ("firmware: cs_dsp: Add KUnit testing of bin file download")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Reviewed-by: Richard Fitzgerald <rf@opensource.cirrus.com>
Link: https://patch.msgid.link/20241216121541.3455880-1-arnd@kernel.org
Signed-off-by: Mark Brown <broonie@kernel.org>
|
|
Fix two places in kerneldoc where alg_id had been mistyped as alg_ig.
Fixes: 7c052c661529 ("firmware: cs_dsp: Add mock bin file generator for KUnit testing")
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202412142205.HHHcousT-lkp@intel.com/
Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
Link: https://patch.msgid.link/20241216105520.22135-1-rf@opensource.cirrus.com
Signed-off-by: Mark Brown <broonie@kernel.org>
|
|
Test that the cs_dsp_client_ops callbacks are called when expected.
pre_run, post_run - when cs_dsp_run() is called.
pre_stop, post_stop - when cs_dsp_stop() is called
control_add - when a WMFW is loaded
control_remove - when cs_dsp_remove() is called
Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
Link: https://patch.msgid.link/20241212143725.1381013-13-rf@opensource.cirrus.com
Signed-off-by: Mark Brown <broonie@kernel.org>
|
|
Add tests for various types of errors and illegal values in
wmfw files. This covers buffer overflows as well as general
unsupported field values.
There are several sets of test cases to cover various different
versions of the wmfw file format.
V0 format was only used on the earlier ADSP2 devices. It does
not have algorithm blocks.
V1 format is used on all ADSP2 versions. It added algorithm
blocks and firmware coefficient descriptor blocks. Strings
are stored in fixed-length arrays.
V2 format is used on all ADSP2 versions. It is similar to V1
but space for strings is variable-length with either an 8-bit
or 16-bit length field.
V3 format is used on Halo Core DSPs and is mostly identical to
the V3 format.
Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
Link: https://patch.msgid.link/20241212143725.1381013-12-rf@opensource.cirrus.com
Signed-off-by: Mark Brown <broonie@kernel.org>
|
