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 900baa6e7b
("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 cdcd7f7287 ("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: 900baa6e7b ("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>
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>
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: cd8c058499 ("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>
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: 900baa6e7b ("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: 3045e29d24 ("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>
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 fe08b7d508 ("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>
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>
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 fe08b7d508 ("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: fe08b7d508 ("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>
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: 8a731fd37f ("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>
Pull sound updates from Takashi Iwai:
"Lots of changes in this cycle, but mostly for cleanups and
refactoring.
Significant amount of changes are about DT schema conversions for ASoC
at this time while we see other usual suspects, too.
Some highlights below:
Core:
- Re-introduction of PCM sync ID support API
- MIDI2 time-base extension in ALSA sequencer API
ASoC:
- Syncing of features between simple-audio-card and the two
audio-graph cards
- Support for specifying the order of operations for components
within cards to allow quirking for unusual systems
- Lots of DT schema conversions
- Continued SOF/Intel updates for topology, SoundWire, IPC3/4
- New support for Asahi Kasei AK4619, Cirrus Logic CS530x, Everest
Semiconductors ES8311, NXP i.MX95 and LPC32xx, Qualcomm LPASS v2.5
and WCD937x, Realtek RT1318 and RT1320 and Texas Instruments
PCM5242
HD-audio:
- More quirks, Intel PantherLake support, senarytech codec support
- Refactoring of Cirrus codec component-binding
Others:
- ALSA control kselftest improvements, and fixes for input value
checks in various drivers"
* tag 'sound-6.11-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound: (349 commits)
kselftest/alsa: Log the PCM ID in pcm-test
kselftest/alsa: Use card name rather than number in test names
ALSA: hda/realtek: Fix the speaker output on Samsung Galaxy Book Pro 360
ALSA: hda/tas2781: Add new quirk for Lenovo Hera2 Laptop
ALSA: seq: ump: Skip useless ports for static blocks
ALSA: pcm_dmaengine: Don't synchronize DMA channel when DMA is paused
ALSA: usb: Use BIT() for bit values
ALSA: usb: Fix UBSAN warning in parse_audio_unit()
ALSA: hda/realtek: Enable headset mic on Positivo SU C1400
ASoC: tas2781: Add new Kontrol to set tas2563 digital Volume
ASoC: codecs: wcd937x: Remove separate handling for vdd-buck supply
ASoC: codecs: wcd937x: Remove the string compare in MIC BIAS widget settings
ASoC: codecs: wcd937x-sdw: Fix Unbalanced pm_runtime_enable
ASoC: dt-bindings: cirrus,cs42xx8: Convert to dtschema
ASoC: cs530x: Remove bclk from private structure
ASoC: cs530x: Calculate proper bclk rate using TDM
ASoC: dt-bindings: cirrus,cs4270: Convert to dtschema
firmware: cs_dsp: Rename fw_ver to wmfw_ver
firmware: cs_dsp: Clarify wmfw format version log message
firmware: cs_dsp: Make wmfw and bin filename arguments const char *
...
Pull MFD updates from Lee Jones:
"New Drivers:
- ROHM BD96801 Power Management IC
- Cirrus Logic CS40L50 Haptic Driver with Waveform Memory
- Marvell 88PM886 Power Management IC
New Device Support:
- Keyboard Backlight to ChromeOS Embedded Controller
- LEDs to ChromeOS Embedded Controller
- Charge Control to ChromeOS Embedded Controller
- HW Monitoring Service to ChromeOS Embedded Controller
- AUXADCs to MediaTek MT635{7,8,9} Power Management ICs
New Functionality:
- Allow Syscon consumers to supply their own Regmaps on registration
Fix-ups:
- Constify/staticise applicable data structures
- Remove superfluous/duplicated/unused sections
- Device Tree binding adaptions/conversions/creation
- Trivial; spelling, whitespace, coding-style adaptions
- Utilise centrally provided helpers and macros to aid
simplicity/duplication
- Drop i2c_device_id::driver_data where the value is unused
- Replace ACPI/DT firmware helpers with agnostic variants
- Move over to GPIOD (descriptor-based) APIs
- Annotate a bunch of __counted_by() cases
- Straighten out some includes
Bug Fixes:
- Ensure potentially asserted recent lines are deasserted during
initialisation
- Avoid "<module>.ko is added to multiple modules" warnings
- Supply a bunch of MODULE_DESCRIPTIONs to silence modpost warnings
- Fix Wvoid-pointer-to-enum-cast warnings"
* tag 'mfd-next-6.11' of git://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd: (87 commits)
mfd: timberdale: Attach device properties to TSC2007 board info
mfd: tmio: Move header to platform_data
mfd: tmio: Sanitize comments
mfd: tmio: Update include files
mmc: tmio/sdhi: Fix includes
mfd: tmio: Remove obsolete io accessors
mfd: tmio: Remove obsolete platform_data
watchdog: bd96801_wdt: Add missing include for FIELD_*()
dt-bindings: mfd: syscon: Add APM poweroff mailbox
dt-bindings: mfd: syscon: Split and enforce documenting MFD children
dt-bindings: mfd: rk817: Merge support for RK809
dt-bindings: mfd: rk817: Fixup clocks and reference dai-common
dt-bindings: mfd: syscon: Add TI's opp table compatible
mfd: omap-usb-tll: Use struct_size to allocate tll
dt-bindings: mfd: Explain lack of child dependency in simple-mfd
dt-bindings: mfd: Dual licensing for st,stpmic1 bindings
mfd: omap-usb-tll: Annotate struct usbtll_omap with __counted_by
mfd: tps6594-core: Remove unneeded semicolon in tps6594_check_crc_mode()
mfd: lm3533: Move to new GPIO descriptor-based APIs
mfd: tps65912: Use devm helper functions to simplify probe
...
Change the log message of the wmfw format version to include
the file name, and change the message to say "format" instead
of "Firmware version". Merge this with the message that logs
the timestamp.
The wmfw format version is information that is useful to have
logged because the behaviour of firmware controls depends on
the wmfw format. So "unexpected" behaviour could be caused by
having expectations based on one format of wmfw when a
different format has been loaded.
But the original message was confusing. It reported the file
format version but didn't actually log the name of the file it
referred to. It also called it "Firmware version", which is
confusing when a later message also logs a firmware version
that is the version of the actual firmware within the wmfw.
The logging of the firmware timestamp has been merged into this.
That was originally a dbg-only message, but as we are already
logging a line of info, we might as well add a few extra
characters to log the timestamp. The timestamp is now logged
in hexadecimal - it's not particularly useful as a decimal
value.
Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
Reviewed-by: Charles Keepax <ckeepax@opensource.cirrus.com>
Link: https://lore.kernel.org/r/20240710103640.78197-4-rf@opensource.cirrus.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Don't allocate a temporary buffer to hold a NUL-terminated copy
of the NAME/INFO string from the wmfw/bin. It can be printed
directly to the log. Also limit the maximum number of characters
that will be logged from this string.
The NAME/INFO blocks in the firmware files are an array of
characters with a length, not a NUL-terminated C string. The
original code allocated a temporary buffer to make a
NUL-terminated copy of the string and then passed that to
dev_info(). There's no need for this: printf formatting can
use "%.*s" to print a character array of a given length.
Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
Reviewed-by: Charles Keepax <ckeepax@opensource.cirrus.com>
Link: https://lore.kernel.org/r/20240710103640.78197-2-rf@opensource.cirrus.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Use strnlen() instead of strlen() on the algorithm and coefficient name
string arrays in V1 wmfw files.
In V1 wmfw files the name is a NUL-terminated string in a fixed-size
array. cs_dsp should protect against overrunning the array if the NUL
terminator is missing.
Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
Fixes: f6bc909e76 ("firmware: cs_dsp: add driver to support firmware loading on Cirrus Logic DSPs")
Link: https://patch.msgid.link/20240708144855.385332-1-rf@opensource.cirrus.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Add a check to cs_dsp_coeff_write_ctrl() to abort if the control
is not writeable.
The cs_dsp code originated as an ASoC driver (wm_adsp) where all
controls were exported as ALSA controls. It relied on ALSA to
enforce the read-only permission. Now that the code has been
separated from ALSA/ASoC it must perform its own permission check.
This isn't currently causing any problems so there shouldn't be any
need to backport this. If the client of cs_dsp exposes the control as
an ALSA control, it should set permissions on that ALSA control to
protect it. The few uses of cs_dsp_coeff_write_ctrl() inside drivers
are for writable controls.
Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
Link: https://patch.msgid.link/20240702110809.16836-1-rf@opensource.cirrus.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Check that all fields of a V2 algorithm header fit into the available
firmware data buffer.
The wmfw V2 format introduced variable-length strings in the algorithm
block header. This means the overall header length is variable, and the
position of most fields varies depending on the length of the string
fields. Each field must be checked to ensure that it does not overflow
the firmware data buffer.
As this ia bugfix patch, the fixes avoid making any significant change to
the existing code. This makes it easier to review and less likely to
introduce new bugs.
Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
Fixes: f6bc909e76 ("firmware: cs_dsp: add driver to support firmware loading on Cirrus Logic DSPs")
Link: https://patch.msgid.link/20240627141432.93056-5-rf@opensource.cirrus.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Move the payload length check in cs_dsp_load() and cs_dsp_coeff_load()
to be done before the block is processed.
The check that the length of a block payload does not exceed the number
of remaining bytes in the firwmware file buffer was being done near the
end of the loop iteration. However, some code before that check used the
length field without validating it.
Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
Fixes: f6bc909e76 ("firmware: cs_dsp: add driver to support firmware loading on Cirrus Logic DSPs")
Link: https://patch.msgid.link/20240627141432.93056-4-rf@opensource.cirrus.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Return an error from cs_dsp_power_up() if a block header is longer
than the amount of data left in the file.
The previous code in cs_dsp_load() and cs_dsp_load_coeff() would loop
while there was enough data left in the file for a valid region. This
protected against overrunning the end of the file data, but it didn't
abort the file processing with an error.
Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
Fixes: f6bc909e76 ("firmware: cs_dsp: add driver to support firmware loading on Cirrus Logic DSPs")
Link: https://patch.msgid.link/20240627141432.93056-3-rf@opensource.cirrus.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Fix the checking that firmware file buffer is large enough for the
wmfw header, to prevent overrunning the buffer.
The original code tested that the firmware data buffer contained
enough bytes for the sums of the size of the structs
wmfw_header + wmfw_adsp1_sizes + wmfw_footer
But wmfw_adsp1_sizes is only used on ADSP1 firmware. For ADSP2 and
Halo Core the equivalent struct is wmfw_adsp2_sizes, which is
4 bytes longer. So the length check didn't guarantee that there
are enough bytes in the firmware buffer for a header with
wmfw_adsp2_sizes.
This patch splits the length check into three separate parts. Each
of the wmfw_header, wmfw_adsp?_sizes and wmfw_footer are checked
separately before they are used.
Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
Fixes: f6bc909e76 ("firmware: cs_dsp: add driver to support firmware loading on Cirrus Logic DSPs")
Link: https://patch.msgid.link/20240627141432.93056-2-rf@opensource.cirrus.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Merge series from Richard Fitzgerald <rf@opensource.cirrus.com>:
This set of patches factors out some repeated code to clean up
firmware control read/write functions, and removes some redundant
control notification code.
base-commit: f193957b0f
Initialize debugfs_root to -ENODEV so that if the client never sets a
valid debugfs root the debugfs files will not be created.
A NULL pointer passed to any of the debugfs_create_*() functions means
"create in the root of debugfs". It doesn't mean "ignore".
Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
Link: https://msgid.link/r/20240307105353.40067-1-rf@opensource.cirrus.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Change the logging of each algorithm from info level to debug level.
On the original devices supported by this code there were typically only
one or two algorithms in a firmware and one or two DSPs so this logging
only used a small number of log lines.
However, for the latest devices there could be 30-40 algorithms in a
firmware and 8 DSPs being loaded in parallel, so using 300+ lines of log
for information that isn't particularly important to have logged.
Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
Link: https://lore.kernel.org/r/20230913160523.3701189-1-rf@opensource.cirrus.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Before adding a new FW control, its name is checked against
existing controls list. But the string length in strncmp used
to compare controls names is taken from the list, so if beginnings
of the controls are matching, then the new control is not created.
For example, if CAL_R control already exists, CAL_R_SELECTED
is not created.
The fix is to compare string lengths as well.
Fixes: 6477960755 ("ASoC: wm_adsp: Move check for control existence")
Signed-off-by: Vlad Karpovich <vkarpovi@opensource.cirrus.com>
Link: https://lore.kernel.org/r/20230815172908.3454056-1-vkarpovi@opensource.cirrus.com
Signed-off-by: Mark Brown <broonie@kernel.org>
In cs_dsp_load_coeff() region_name should be set in the XM/YM/ZM
cases otherwise any errors will log the region as "Unknown".
While doing this also change one error message that logged the
region type ID to log the region_name instead. This makes it
consistent with other messages in the same function.
Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
Link: https://lore.kernel.org/r/20230605143238.4001982-1-rf@opensource.cirrus.com
Signed-off-by: Mark Brown <broonie@kernel.org>
The file named 'controls' in the DSP's debugfs root contains a
formatted table describing the controls defined within the loaded DSP
firmware, it is of the form
name: len region:offset addr fwname algid ctltype flags en dirty
Where flags is represented as a character for each flag if set, or '-',
enabled is whether the control is enabled or disabled and dirty is
whether the control value is set in the cache but not the hardware.
Signed-off-by: Simon Trimmer <simont@opensource.cirrus.com>
Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
Link: https://lore.kernel.org/r/20230328131018.6820-1-rf@opensource.cirrus.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Merge series from Richard Fitzgerald <rf@opensource.cirrus.com>:
Use EXPORT_SYMBOL_NS_GPL() instead of EXPORT_SYMBOL_GPL() and patch the
three drivers that use cs_dsp to add the MODULE_IMPORT_NS().
To make the namespace more specific the KConfig symbol for cs_dsp is
changed from CS_DSP to FW_CS_DSP.
For wmfw format v2 and later the coefficient name strings have a length
field and are NOT null-terminated. Use kasprintf() to convert the
unterminated string into a null-terminated string in an allocated buffer.
The previous code handled this duplication incorrectly using kmemdup()
and getting the length from a strlen() of the (unterminated) source string.
This resulted in creating a string that continued up to the next byte in
the firmware file that just happened to be 0x00.
Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
Fixes: f6bc909e76 ("firmware: cs_dsp: add driver to support firmware loading on Cirrus Logic DSPs")
Link: https://lore.kernel.org/r/20220412163927.1303470-1-rf@opensource.cirrus.com
Signed-off-by: Mark Brown <broonie@kernel.org>
