Replace the use of __free(kfree) in _cs_amp_set_efi_calibration_data()
with normal kfree() at the end of the function.
Krzysztof Kozlowski pointed out that __free() can be dangerous.
It can introduce new cleanup bugs. These are more subtle and difficult to
spot than a missing goto in traditional cleanup, because they are triggered
by writing regular idiomatic C code instead of using C++ conventions. As
it's regular C style it's more likely to be missed because the code is as
would be expected for C. The traditional goto also more obviously flags
to anyone changing the code in the future that they must be careful about
the cleanup.
Arguably the traditional approach is more readable, and it avoids the
manipulation of __free() pointers when the buffer is reallocated for
resizing.
Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
Link: https://patch.msgid.link/20251201123906.86876-1-rf@opensource.cirrus.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Replace manual "request_threaded_irq()" with the device managed
"devm_request_threaded_irq" to manage the IRQ lifetime and also
it removes the smatch reported warning.
Remove the manual "free_irq()" in the "remove" function as free_irq
is tied to device teardown.
Signed-off-by: HariKrishna Sagala <hariconscious@gmail.com>
Link: https://patch.msgid.link/20251217104356.60839-3-hariconscious@gmail.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Merge series from Peter Ujfalusi <peter.ujfalusi@linux.intel.com>:
On system suspend / resume we always power up the DSP and boot the
firmware, which is not strictly needed as right after the firmware booted
up we power the DSP down again on suspend and we also power it down after
resume after some inactivity.
Similarly, on jack insert/removal we needlesly boot up the firmware to check
the jack status, which needs no DSP/firmware communication.
The on-demand DSP boot will make sure that we boot the DSP firmware up only
when it is needed - for audio activity, in other cases the firmware will be
not booted up, which saves time.
Out of caution, add a new platform descriptor flag to enable on-demand
DSP boot since this might not work without changes to platform code on
certain platforms.
With the on-demand dsp boot enabled we will not boot the DSP and firmware
up on system or rpm resume, just enable audio subsystem since audio IPs,
like HDA and SoundWire might be needed (codecs suspend/resume operation).
Only boot up the DSP during the first hw_params() call when the DSP is
really going to be needed.
In this way we can handle the audio related use cases:
normal audio use (rpm suspend/resume)
system suspend/resume without active audio
system suspend/resume with active audio
system suspend/resume without active audio, and audio start before the rpm
suspend timeout
Add module option to force the on-demand DSP boot to allow it to be
disabled or enabled without kernel change for testing.
The on-demand boot has been tested in our CI for more than half a year
and so far no issues have been seen on supported platforms since it's
introduction to our development tree (sof-dev).
On system suspend / resume we always power up the DSP and boot the
firmware, which is not strictly needed as right after the firmware booted
up we power the DSP down again on suspend and we also power it down after
resume after some inactivity.
Out of caution, add a new platform descriptor flag to enable on-demand
DSP boot since this might not work without changes to platform code on
certain platforms.
With the on-demand dsp boot enabled we will not boot the DSP and firmware
up on system or rpm resume, just enable audio subsystem since audio IPs,
like HDA and SoundWire might be needed (codecs suspend/resume operation).
Only boot up the DSP during the first hw_params() call when the DSP is
really going to be needed.
In this way we can handle the audio related use cases:
normal audio use (rpm suspend/resume)
system suspend/resume without active audio
system suspend/resume with active audio
system suspend/resume without active audio, and audio start before the rpm
suspend timeout
Add module option to force the on-demand DSP boot to allow it to be
disabled or enabled without kernel change for testing.
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Reviewed-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Reviewed-by: Liam Girdwood <liam.r.girdwood@intel.com>
Link: https://patch.msgid.link/20251215132946.2155-4-peter.ujfalusi@linux.intel.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Merge series from Cezary Rojewski <cezary.rojewski@intel.com>:
Entire patchset provides no new features and does not alter the code
from functional (user) perspective.
The first two improve IPC-error logging 'mechanism' and align the
catpt-driver with what's done in another Intel's driver: the avs-driver.
In essence, no need to log the error in every function, let the common
handler do so instead.
The last three simplify the code, and fix some spacing issues. All in
all, we get better readability with lower LOC.
Merge series from Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>:
cleanup.h coding style asks to avoid having constructors with redundant
values (= NULL). On purpose, because it is just not making the code
simpler. The constructor should be meaningful not just NULL.
If you do not agree in declaration-in-place-of-use (fair!), then do not
use cleanup.h. If you want to use cleanup.h, then please read cleanup.h
before.
Merge series from Biju <biju.das.au@gmail.com>:
Add support for 24 and 32-bit sample format width for RZ/G2L SoCs. Apart
from this, the patch series includes some code cleanups.
Merge series from Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>:
Please, please stop ending cleanup.h patches for very simple code like:
foo = kzalloc();
kfree(foo);
return;
... *if you do not intend to read cleanup.h*. These changes are making
simple code not necessarily simpler. But worse, if you do not read
cleanup.h then you introduce actually undesired, error-prone and wrong
style of having constructors with redundant values (= NULL).
This is actually worse code.
If you do not agree in declaration-in-place-of-use (fair!), then do not
use cleanup.h. If you want to use cleanup.h, then please read cleanup.h
before.
This is second mixup I see recently around Qualcomm files.
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>
A recent change switched to using guards for the port list lock but only
modified two out of three functions where the lock is held.
Convert also the third function for consistency while switching to a
scoped guard in q6afe_port_get_from_id() for clarity.
Signed-off-by: Johan Hovold <johan@kernel.org>
Link: https://patch.msgid.link/20251203105542.24765-3-johan@kernel.org
Signed-off-by: Mark Brown <broonie@kernel.org>
Replace snprintf() with scnprintf() when constructing the property
and remove negative return error handling as scnprintf() returns the
actual number of bytes written to buffer.
snprintf() as defined by the C99 standard,returns the number of
characters that *would have been* written if enough space were
available.Use scnprintf() that returns the actual number of
characters written.
Link: https://github.com/KSPP/linux/issues/105
Signed-off-by: HariKrishna Sagala <hariconscious@gmail.com>
Link: https://patch.msgid.link/20251212044408.1286-2-hariconscious@gmail.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>
Replace manual "request_threaded_irq()" with the device managed
"devm_request_threaded_irq" to manage the IRQ lifetime and also
it removes the smatch reported warning.
Remove the manual "free_irq()" in the "remove" function as free_irq
is tied to device teardown.
Signed-off-by: HariKrishna Sagala <hariconscious@gmail.com>
Link: https://patch.msgid.link/20251121140940.40678-4-hariconscious@gmail.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Initializing automatic __free variables to NULL without need (e.g.
branches with different allocations), followed by actual allocation is
in contrary to explicit coding rules guiding cleanup.h:
"Given that the "__free(...) = NULL" pattern for variables defined at
the top of the function poses this potential interdependency problem the
recommendation is to always define and assign variables in one statement
and not group variable definitions at the top of the function when
__free() is used."
Code does not have a bug, but is less readable and uses discouraged
coding practice, so fix that by moving declaration to the place of
assignment.
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Reviewed-by: Charles Keepax <ckeepax@opensource.cirrus.com>
Link: https://patch.msgid.link/20251203-asoc-wrong-cleanup-h-continued-v1-3-5142be4874fb@oss.qualcomm.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Initializing automatic __free variables to NULL without need (e.g.
branches with different allocations), followed by actual allocation is
in contrary to explicit coding rules guiding cleanup.h:
"Given that the "__free(...) = NULL" pattern for variables defined at
the top of the function poses this potential interdependency problem the
recommendation is to always define and assign variables in one statement
and not group variable definitions at the top of the function when
__free() is used."
Code does not have a bug, but is less readable and uses discouraged
coding practice, so fix that by moving declaration to the place of
assignment.
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Link: https://patch.msgid.link/20251203-asoc-wrong-cleanup-h-continued-v1-2-5142be4874fb@oss.qualcomm.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Initializing automatic __free variables to NULL without need (e.g.
branches with different allocations), followed by actual allocation is
in contrary to explicit coding rules guiding cleanup.h:
"Given that the "__free(...) = NULL" pattern for variables defined at
the top of the function poses this potential interdependency problem the
recommendation is to always define and assign variables in one statement
and not group variable definitions at the top of the function when
__free() is used."
Code does not have a bug, but is less readable and uses discouraged
coding practice, so fix that by moving declaration to the place of
assignment.
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Link: https://patch.msgid.link/20251203-asoc-wrong-cleanup-h-continued-v1-1-5142be4874fb@oss.qualcomm.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Commit 88a5f8e628 ("ASoc: qcom: audioreach: Use automatic cleanup of
kfree()") did not make the code simpler but more complicated. Already
simple code of allocation and free, without any error paths, got now
declaration with one constructor followed by another allocation, which
is in contrary to explicit coding rules guiding cleanup.h:
"Given that the "__free(...) = NULL" pattern for variables defined at
the top of the function poses this potential interdependency problem the
recommendation is to always define and assign variables in one statement
and not group variable definitions at the top of the function when
__free() is used."
Code does not have a bug, but is less readable and uses discouraged
coding practice, so fix that by moving declaration to the place of
assignment.
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Reviewed-by: Srinivas Kandagatla <srinivas.kandagatla@oss.qualcomm.com>
Link: https://patch.msgid.link/20251129-asoc-wrong-cleanup-h-can-people-stop-sending-this-without-reading-docs-v1-5-c38b06884e39@oss.qualcomm.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Commit 55094e55ae ("ASoc: qcom: q6afe: Use automatic cleanup of
kfree()") did not make the code simpler but more complicated. Already
simple code of allocation and free, without any error paths, got now
declaration with one constructor followed by another allocation, which
is in contrary to explicit coding rules guiding cleanup.h:
"Given that the "__free(...) = NULL" pattern for variables defined at
the top of the function poses this potential interdependency problem the
recommendation is to always define and assign variables in one statement
and not group variable definitions at the top of the function when
__free() is used."
Code does not have a bug, but is less readable and uses discouraged
coding practice, so fix that by moving declaration to the place of
assignment.
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Reviewed-by: Srinivas Kandagatla <srinivas.kandagatla@oss.qualcomm.com>
Link: https://patch.msgid.link/20251129-asoc-wrong-cleanup-h-can-people-stop-sending-this-without-reading-docs-v1-4-c38b06884e39@oss.qualcomm.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Commit 89cf2223ee ("ASoc: qcom: q6apm: Use automatic cleanup of
kfree()") did not make the code simpler but more complicated. Already
simple code of allocation and free, without any error paths, got now
declaration with one constructor followed by another allocation, which
is in contrary to explicit coding rules guiding cleanup.h:
"Given that the "__free(...) = NULL" pattern for variables defined at
the top of the function poses this potential interdependency problem the
recommendation is to always define and assign variables in one statement
and not group variable definitions at the top of the function when
__free() is used."
Code does not have a bug, but is less readable and uses discouraged
coding practice, so fix that by moving declaration to the place of
assignment.
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Reviewed-by: Srinivas Kandagatla <srinivas.kandagatla@oss.qualcomm.com>
Link: https://patch.msgid.link/20251129-asoc-wrong-cleanup-h-can-people-stop-sending-this-without-reading-docs-v1-3-c38b06884e39@oss.qualcomm.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Commit 6e00112d31 ("ASoc: qcom: q6asm: Use automatic cleanup of
kfree()") did not make the code simpler but more complicated. Already
simple code of allocation and free, without any error paths, got now
declaration with one constructor followed by another allocation, which
is in contrary to explicit coding rules guiding cleanup.h:
"Given that the "__free(...) = NULL" pattern for variables defined at
the top of the function poses this potential interdependency problem the
recommendation is to always define and assign variables in one statement
and not group variable definitions at the top of the function when
__free() is used."
Code does not have a bug, but is less readable and uses discouraged
coding practice, so fix that by moving declaration to the place of
assignment.
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Reviewed-by: Srinivas Kandagatla <srinivas.kandagatla@oss.qualcomm.com>
Link: https://patch.msgid.link/20251129-asoc-wrong-cleanup-h-can-people-stop-sending-this-without-reading-docs-v1-2-c38b06884e39@oss.qualcomm.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Niranjan H Y
Kuninori Morimoto
Krzysztof Kozlowski
Richard Fitzgerald
Shuming Fan
Simon Trimmer
HariKrishna Sagala
Mark Brown
Peter Ujfalusi
Andy Shevchenko
Uwe Kleine-König
Thorsten Blum
Johan Hovold
Bartosz Golaszewski
Cezary Rojewski