Skip to content

Commit 9be71d4

Browse files
rfvirgilbroonie
authored andcommitted
firmware: cs_dsp: Simplify suppressing log messages during KUnit testing
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 10db9f6 ("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>
1 parent 207fd1f commit 9be71d4

6 files changed

Lines changed: 79 additions & 81 deletions

File tree

drivers/firmware/cirrus/cs_dsp.c

Lines changed: 36 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
* Cirrus Logic International Semiconductor Ltd.
1010
*/
1111

12+
#include <kunit/static_stub.h>
1213
#include <kunit/visibility.h>
1314
#include <linux/cleanup.h>
1415
#include <linux/ctype.h>
@@ -18,6 +19,7 @@
1819
#include <linux/minmax.h>
1920
#include <linux/module.h>
2021
#include <linux/moduleparam.h>
22+
#include <linux/ratelimit.h>
2123
#include <linux/seq_file.h>
2224
#include <linux/slab.h>
2325
#include <linux/vmalloc.h>
@@ -30,45 +32,47 @@
3032
/*
3133
* When the KUnit test is running the error-case tests will cause a lot
3234
* of messages. Rate-limit to prevent overflowing the kernel log buffer
33-
* during KUnit test runs.
35+
* during KUnit test runs and allow the test to redirect this function.
36+
* In normal (not KUnit) builds this collapses to only return true.
3437
*/
35-
#if IS_ENABLED(CONFIG_FW_CS_DSP_KUNIT_TEST)
36-
bool cs_dsp_suppress_err_messages;
37-
EXPORT_SYMBOL_IF_KUNIT(cs_dsp_suppress_err_messages);
38+
VISIBLE_IF_KUNIT bool cs_dsp_can_emit_message(void)
39+
{
40+
KUNIT_STATIC_STUB_REDIRECT(cs_dsp_can_emit_message);
3841

39-
bool cs_dsp_suppress_warn_messages;
40-
EXPORT_SYMBOL_IF_KUNIT(cs_dsp_suppress_warn_messages);
42+
if (IS_ENABLED(CONFIG_FW_CS_DSP_KUNIT_TEST)) {
43+
static DEFINE_RATELIMIT_STATE(_rs,
44+
DEFAULT_RATELIMIT_INTERVAL,
45+
DEFAULT_RATELIMIT_BURST);
46+
return __ratelimit(&_rs);
47+
}
4148

42-
bool cs_dsp_suppress_info_messages;
43-
EXPORT_SYMBOL_IF_KUNIT(cs_dsp_suppress_info_messages);
49+
return true;
50+
}
51+
EXPORT_SYMBOL_IF_KUNIT(cs_dsp_can_emit_message);
4452

45-
#define cs_dsp_err(_dsp, fmt, ...) \
46-
do { \
47-
if (!cs_dsp_suppress_err_messages) \
48-
dev_err_ratelimited(_dsp->dev, "%s: " fmt, _dsp->name, ##__VA_ARGS__); \
53+
#define cs_dsp_err(_dsp, fmt, ...) \
54+
do { \
55+
if (cs_dsp_can_emit_message()) \
56+
dev_err(_dsp->dev, "%s: " fmt, _dsp->name, ##__VA_ARGS__); \
4957
} while (false)
50-
#define cs_dsp_warn(_dsp, fmt, ...) \
51-
do { \
52-
if (!cs_dsp_suppress_warn_messages) \
53-
dev_warn_ratelimited(_dsp->dev, "%s: " fmt, _dsp->name, ##__VA_ARGS__); \
58+
59+
#define cs_dsp_warn(_dsp, fmt, ...) \
60+
do { \
61+
if (cs_dsp_can_emit_message()) \
62+
dev_warn(_dsp->dev, "%s: " fmt, _dsp->name, ##__VA_ARGS__); \
5463
} while (false)
55-
#define cs_dsp_info(_dsp, fmt, ...) \
56-
do { \
57-
if (!cs_dsp_suppress_info_messages) \
58-
dev_info_ratelimited(_dsp->dev, "%s: " fmt, _dsp->name, ##__VA_ARGS__); \
64+
65+
#define cs_dsp_info(_dsp, fmt, ...) \
66+
do { \
67+
if (cs_dsp_can_emit_message()) \
68+
dev_info(_dsp->dev, "%s: " fmt, _dsp->name, ##__VA_ARGS__); \
69+
} while (false)
70+
71+
#define cs_dsp_dbg(_dsp, fmt, ...) \
72+
do { \
73+
if (cs_dsp_can_emit_message()) \
74+
dev_dbg(_dsp->dev, "%s: " fmt, _dsp->name, ##__VA_ARGS__); \
5975
} while (false)
60-
#define cs_dsp_dbg(_dsp, fmt, ...) \
61-
dev_dbg_ratelimited(_dsp->dev, "%s: " fmt, _dsp->name, ##__VA_ARGS__)
62-
#else
63-
#define cs_dsp_err(_dsp, fmt, ...) \
64-
dev_err(_dsp->dev, "%s: " fmt, _dsp->name, ##__VA_ARGS__)
65-
#define cs_dsp_warn(_dsp, fmt, ...) \
66-
dev_warn(_dsp->dev, "%s: " fmt, _dsp->name, ##__VA_ARGS__)
67-
#define cs_dsp_info(_dsp, fmt, ...) \
68-
dev_info(_dsp->dev, "%s: " fmt, _dsp->name, ##__VA_ARGS__)
69-
#define cs_dsp_dbg(_dsp, fmt, ...) \
70-
dev_dbg(_dsp->dev, "%s: " fmt, _dsp->name, ##__VA_ARGS__)
71-
#endif
7276

7377
#define ADSP1_CONTROL_1 0x00
7478
#define ADSP1_CONTROL_2 0x02

drivers/firmware/cirrus/cs_dsp.h

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,7 @@
1010
#define FW_CS_DSP_H
1111

1212
#if IS_ENABLED(CONFIG_KUNIT)
13-
extern bool cs_dsp_suppress_err_messages;
14-
extern bool cs_dsp_suppress_warn_messages;
15-
extern bool cs_dsp_suppress_info_messages;
13+
bool cs_dsp_can_emit_message(void);
1614
#endif
1715

1816
#endif /* ifndef FW_CS_DSP_H */

drivers/firmware/cirrus/test/cs_dsp_test_bin.c

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
#include <kunit/device.h>
99
#include <kunit/resource.h>
10+
#include <kunit/static_stub.h>
1011
#include <kunit/test.h>
1112
#include <linux/build_bug.h>
1213
#include <linux/firmware/cirrus/cs_dsp.h>
@@ -2155,6 +2156,15 @@ static void bin_patch_name_and_info(struct kunit *test)
21552156
KUNIT_EXPECT_EQ(test, reg_val, payload_data);
21562157
}
21572158

2159+
static bool cs_dsp_bin_test_can_emit_message_hook(void)
2160+
{
2161+
#if defined(DEBUG)
2162+
return true;
2163+
#else
2164+
return false;
2165+
#endif
2166+
}
2167+
21582168
static int cs_dsp_bin_test_common_init(struct kunit *test, struct cs_dsp *dsp,
21592169
int wmdr_ver)
21602170
{
@@ -2239,16 +2249,12 @@ static int cs_dsp_bin_test_common_init(struct kunit *test, struct cs_dsp *dsp,
22392249
* The large number of test cases will cause an unusually large amount
22402250
* of dev_info() messages from cs_dsp, so suppress these.
22412251
*/
2242-
cs_dsp_suppress_info_messages = true;
2252+
kunit_activate_static_stub(test, cs_dsp_can_emit_message,
2253+
cs_dsp_bin_test_can_emit_message_hook);
22432254

22442255
return 0;
22452256
}
22462257

2247-
static void cs_dsp_bin_test_exit(struct kunit *test)
2248-
{
2249-
cs_dsp_suppress_info_messages = false;
2250-
}
2251-
22522258
static int cs_dsp_bin_test_halo_init_common(struct kunit *test, int wmdr_ver)
22532259
{
22542260
struct cs_dsp *dsp;
@@ -2833,7 +2839,6 @@ static struct kunit_case cs_dsp_bin_test_cases_adsp2[] = {
28332839
static struct kunit_suite cs_dsp_bin_test_halo = {
28342840
.name = "cs_dsp_bin_halo",
28352841
.init = cs_dsp_bin_test_halo_init,
2836-
.exit = cs_dsp_bin_test_exit,
28372842
.test_cases = cs_dsp_bin_test_cases_halo,
28382843
.attr.speed = KUNIT_SPEED_SLOW,
28392844
};
@@ -2848,15 +2853,13 @@ static struct kunit_suite cs_dsp_bin_test_halo_wmdr3 = {
28482853
static struct kunit_suite cs_dsp_bin_test_adsp2_32bit = {
28492854
.name = "cs_dsp_bin_adsp2_32bit",
28502855
.init = cs_dsp_bin_test_adsp2_32bit_init,
2851-
.exit = cs_dsp_bin_test_exit,
28522856
.test_cases = cs_dsp_bin_test_cases_adsp2,
28532857
.attr.speed = KUNIT_SPEED_SLOW,
28542858
};
28552859

28562860
static struct kunit_suite cs_dsp_bin_test_adsp2_16bit = {
28572861
.name = "cs_dsp_bin_adsp2_16bit",
28582862
.init = cs_dsp_bin_test_adsp2_16bit_init,
2859-
.exit = cs_dsp_bin_test_exit,
28602863
.test_cases = cs_dsp_bin_test_cases_adsp2,
28612864
.attr.speed = KUNIT_SPEED_SLOW,
28622865
};

drivers/firmware/cirrus/test/cs_dsp_test_bin_error.c

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88

99
#include <kunit/device.h>
1010
#include <kunit/resource.h>
11+
#include <kunit/static_stub.h>
1112
#include <kunit/test.h>
1213
#include <linux/build_bug.h>
1314
#include <linux/firmware/cirrus/cs_dsp.h>
@@ -380,11 +381,13 @@ static void bin_block_payload_len_garbage(struct kunit *test)
380381
0);
381382
}
382383

383-
static void cs_dsp_bin_err_test_exit(struct kunit *test)
384+
static bool cs_dsp_bin_err_test_can_emit_message_hook(void)
384385
{
385-
cs_dsp_suppress_err_messages = false;
386-
cs_dsp_suppress_warn_messages = false;
387-
cs_dsp_suppress_info_messages = false;
386+
#if defined(DEBUG)
387+
return true;
388+
#else
389+
return false;
390+
#endif
388391
}
389392

390393
static int cs_dsp_bin_err_test_common_init(struct kunit *test, struct cs_dsp *dsp,
@@ -482,9 +485,8 @@ static int cs_dsp_bin_err_test_common_init(struct kunit *test, struct cs_dsp *ds
482485
* Testing error conditions can produce a lot of log output
483486
* from cs_dsp error messages, so suppress messages.
484487
*/
485-
cs_dsp_suppress_err_messages = true;
486-
cs_dsp_suppress_warn_messages = true;
487-
cs_dsp_suppress_info_messages = true;
488+
kunit_activate_static_stub(test, cs_dsp_can_emit_message,
489+
cs_dsp_bin_err_test_can_emit_message_hook);
488490

489491
return 0;
490492
}
@@ -584,23 +586,20 @@ static struct kunit_case cs_dsp_bin_err_test_cases[] = {
584586
static struct kunit_suite cs_dsp_bin_err_test_halo = {
585587
.name = "cs_dsp_bin_err_halo",
586588
.init = cs_dsp_bin_err_test_halo_init,
587-
.exit = cs_dsp_bin_err_test_exit,
588589
.test_cases = cs_dsp_bin_err_test_cases,
589590
.attr.speed = KUNIT_SPEED_SLOW,
590591
};
591592

592593
static struct kunit_suite cs_dsp_bin_err_test_adsp2_32bit = {
593594
.name = "cs_dsp_bin_err_adsp2_32bit",
594595
.init = cs_dsp_bin_err_test_adsp2_32bit_init,
595-
.exit = cs_dsp_bin_err_test_exit,
596596
.test_cases = cs_dsp_bin_err_test_cases,
597597
.attr.speed = KUNIT_SPEED_SLOW,
598598
};
599599

600600
static struct kunit_suite cs_dsp_bin_err_test_adsp2_16bit = {
601601
.name = "cs_dsp_bin_err_adsp2_16bit",
602602
.init = cs_dsp_bin_err_test_adsp2_16bit_init,
603-
.exit = cs_dsp_bin_err_test_exit,
604603
.test_cases = cs_dsp_bin_err_test_cases,
605604
.attr.speed = KUNIT_SPEED_SLOW,
606605
};

drivers/firmware/cirrus/test/cs_dsp_test_wmfw.c

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88

99
#include <kunit/device.h>
1010
#include <kunit/resource.h>
11+
#include <kunit/static_stub.h>
1112
#include <kunit/test.h>
1213
#include <linux/build_bug.h>
1314
#include <linux/firmware/cirrus/cs_dsp.h>
@@ -1775,6 +1776,15 @@ static void wmfw_load_with_info(struct kunit *test)
17751776
KUNIT_EXPECT_MEMEQ(test, readback, payload_data, payload_size_bytes);
17761777
}
17771778

1779+
static bool cs_dsp_wmfw_test_can_emit_message_hook(void)
1780+
{
1781+
#if defined(DEBUG)
1782+
return true;
1783+
#else
1784+
return false;
1785+
#endif
1786+
}
1787+
17781788
static int cs_dsp_wmfw_test_common_init(struct kunit *test, struct cs_dsp *dsp,
17791789
int wmfw_version)
17801790
{
@@ -1863,16 +1873,12 @@ static int cs_dsp_wmfw_test_common_init(struct kunit *test, struct cs_dsp *dsp,
18631873
* The large number of test cases will cause an unusually large amount
18641874
* of dev_info() messages from cs_dsp, so suppress these.
18651875
*/
1866-
cs_dsp_suppress_info_messages = true;
1876+
kunit_activate_static_stub(test, cs_dsp_can_emit_message,
1877+
cs_dsp_wmfw_test_can_emit_message_hook);
18671878

18681879
return 0;
18691880
}
18701881

1871-
static void cs_dsp_wmfw_test_exit(struct kunit *test)
1872-
{
1873-
cs_dsp_suppress_info_messages = false;
1874-
}
1875-
18761882
static int cs_dsp_wmfw_test_halo_init(struct kunit *test)
18771883
{
18781884
struct cs_dsp *dsp;
@@ -2180,55 +2186,48 @@ static struct kunit_case cs_dsp_wmfw_test_cases_adsp2[] = {
21802186
static struct kunit_suite cs_dsp_wmfw_test_halo = {
21812187
.name = "cs_dsp_wmfwV3_halo",
21822188
.init = cs_dsp_wmfw_test_halo_init,
2183-
.exit = cs_dsp_wmfw_test_exit,
21842189
.test_cases = cs_dsp_wmfw_test_cases_halo,
21852190
.attr.speed = KUNIT_SPEED_SLOW,
21862191
};
21872192

21882193
static struct kunit_suite cs_dsp_wmfw_test_adsp2_32bit_wmfw0 = {
21892194
.name = "cs_dsp_wmfwV0_adsp2_32bit",
21902195
.init = cs_dsp_wmfw_test_adsp2_32bit_wmfw0_init,
2191-
.exit = cs_dsp_wmfw_test_exit,
21922196
.test_cases = cs_dsp_wmfw_test_cases_adsp2,
21932197
.attr.speed = KUNIT_SPEED_SLOW,
21942198
};
21952199

21962200
static struct kunit_suite cs_dsp_wmfw_test_adsp2_32bit_wmfw1 = {
21972201
.name = "cs_dsp_wmfwV1_adsp2_32bit",
21982202
.init = cs_dsp_wmfw_test_adsp2_32bit_wmfw1_init,
2199-
.exit = cs_dsp_wmfw_test_exit,
22002203
.test_cases = cs_dsp_wmfw_test_cases_adsp2,
22012204
.attr.speed = KUNIT_SPEED_SLOW,
22022205
};
22032206

22042207
static struct kunit_suite cs_dsp_wmfw_test_adsp2_32bit_wmfw2 = {
22052208
.name = "cs_dsp_wmfwV2_adsp2_32bit",
22062209
.init = cs_dsp_wmfw_test_adsp2_32bit_wmfw2_init,
2207-
.exit = cs_dsp_wmfw_test_exit,
22082210
.test_cases = cs_dsp_wmfw_test_cases_adsp2,
22092211
.attr.speed = KUNIT_SPEED_SLOW,
22102212
};
22112213

22122214
static struct kunit_suite cs_dsp_wmfw_test_adsp2_16bit_wmfw0 = {
22132215
.name = "cs_dsp_wmfwV0_adsp2_16bit",
22142216
.init = cs_dsp_wmfw_test_adsp2_16bit_wmfw0_init,
2215-
.exit = cs_dsp_wmfw_test_exit,
22162217
.test_cases = cs_dsp_wmfw_test_cases_adsp2,
22172218
.attr.speed = KUNIT_SPEED_SLOW,
22182219
};
22192220

22202221
static struct kunit_suite cs_dsp_wmfw_test_adsp2_16bit_wmfw1 = {
22212222
.name = "cs_dsp_wmfwV1_adsp2_16bit",
22222223
.init = cs_dsp_wmfw_test_adsp2_16bit_wmfw1_init,
2223-
.exit = cs_dsp_wmfw_test_exit,
22242224
.test_cases = cs_dsp_wmfw_test_cases_adsp2,
22252225
.attr.speed = KUNIT_SPEED_SLOW,
22262226
};
22272227

22282228
static struct kunit_suite cs_dsp_wmfw_test_adsp2_16bit_wmfw2 = {
22292229
.name = "cs_dsp_wmfwV2_adsp2_16bit",
22302230
.init = cs_dsp_wmfw_test_adsp2_16bit_wmfw2_init,
2231-
.exit = cs_dsp_wmfw_test_exit,
22322231
.test_cases = cs_dsp_wmfw_test_cases_adsp2,
22332232
.attr.speed = KUNIT_SPEED_SLOW,
22342233
};

0 commit comments

Comments
 (0)