Skip to content

Refine logger #1065

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
keqiaozhang wants to merge 3 commits into from
Closed

Refine logger #1065

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
844ceeb
lib.sh: refine the logger_disabled function
keqiaozhang Jul 4, 2023
7960aef
check-sof-logger.sh: remove the ldc dependency for IPC4 Zephyr platforms
keqiaozhang Jul 4, 2023
8adb3ce
check-sof-logger.sh: cavstool.py clean up
keqiaozhang Jul 4, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 12 additions & 14 deletions case-lib/lib.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -752,16 +752,6 @@ is_ipc4()
logger_disabled()
{
local ldcFile
# Some firmware/OS configurations do not support logging.
ldcFile=$(find_ldc_file) || {
dlogi '.ldc dictionary file not found, SOF logs collection disabled'
return 0 # 0 is 'true'
}

# Disable logging when available...
if [ ${OPT_VAL['s']} -eq 0 ]; then
return 0
fi

# ... across all tests at once.
# In the future we should support SOF_LOGGING=etrace (only), see
Expand All @@ -771,12 +761,20 @@ logger_disabled()
return 0
fi

if is_ipc4 && ! is_firmware_file_zephyr; then
dlogi 'IPC4 FW logging only support with SOF Zephyr build'
dlogi 'SOF logs collection is globally disabled.'
return 0
if is_ipc4; then
is_firmware_file_zephyr || {
dlogi 'IPC4 FW logging only support with SOF Zephyr build'
dlogi 'SOF logs collection is globally disabled.'
return 0
}
else
Copy link
Contributor

@lyakh lyakh Jun 21, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no need for this else

Copy link
Collaborator

@marc-hb marc-hb Jun 21, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1. Less indentation and less nesting: easier to read.

https://medium.com/swlh/return-early-pattern-3d18a41bba8

ldcFile=$(find_ldc_file) || {
dlogi '.ldc dictionary file not found, SOF logs collection disabled'
return 0 # 0 is 'true'
}
fi

# return 1 means run tests with logging.
return 1
Copy link
Contributor

@greg-intel greg-intel Jun 21, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An error condition should have a message stating what the error is before returning.

}

Expand Down
44 changes: 9 additions & 35 deletions test-case/check-sof-logger.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
##
## Case Name: check-sof-logger
## Preconditions:
## sof-logger, cavstool.py and mtrace-reader.py installed in system path
## sof-logger and mtrace-reader.py installed in system path
## dictionary (ldc) file is in /etc/sof/ or /lib/firmware
##
## Description:
Expand Down Expand Up @@ -76,10 +76,10 @@ run_loggers()
# the same time, so $data_file will hopefully not be long.
local collect_secs=2

if is_zephyr; then
if is_ipc4; then
# Collect logs from Zephyr logging backends

if is_ipc4 ; then
if is_firmware_file_zephyr; then
# Get logs via SOF kernel IPC4 SRAM logging interfaces (mtrace)

if [ -z "$MTRACE" ]; then
Expand All @@ -95,23 +95,10 @@ run_loggers()
sudo timeout -k 3 "$collect_secs" \
"$mtracetool" > "$etrace_file" 2> "$etrace_stderr_file" & mtracetoolPID=$!
else
# SOF kernel IPC3 SRAM logging interface (etrace)

local cavstool
cavstool=$(type -p cavstool.py)

dlogi "Trying to get Zephyr logs from etrace with background $cavstool ..."
dlogc \
"sudo $cavstool --log-only > $etrace_file 2>&1"
# Firmware messages are on stdout and local messages on
# stderr. Merge them and then grep ERROR below.
# shellcheck disable=SC2024
sudo timeout -k 3 "$collect_secs" \
"$cavstool" --log-only > "$etrace_file" 2>&1 & cavstoolPID=$!
dlogi 'IPC4 FW logging only support with SOF Zephyr build'
return 0
fi
fi

if ! is_ipc4 ; then
else
# Sof-logger DMA logging (IPC3 only)

dlogi "Trying to get the DMA .ldc trace log with background sof-logger ..."
Expand All @@ -136,7 +123,7 @@ run_loggers()
}
fi

if is_zephyr; then
if is_firmware_file_zephyr; then
# Zephyr logging backends

if is_ipc4 ; then
Expand All @@ -149,15 +136,8 @@ run_loggers()
}
else
# SOF kernel IPC3 SRAM logging interface (etrace)

loggerStatus=0; wait "$cavstoolPID" || loggerStatus=$?
test "$loggerStatus" -eq 124 || {
cat "$error_file"
die "timeout $cavstool returned unexpected: $loggerStatus"
}
( set -x
grep -i ERROR "$etrace_file" > "$etrace_stderr_file"
) || true
dlogi 'IPC3 Zephyr built FW is no longer supported, EOL.'
return 0
fi

# All Zephyr backends checked at this point, we can return
Expand All @@ -180,7 +160,6 @@ run_loggers()

return $etrace_exit
fi

# XTOS IPC4 case
return 0
}
Expand Down Expand Up @@ -255,7 +234,6 @@ main()
fi

reload_drivers
# cavstool is now racing against D3
run_loggers

local f
Expand Down Expand Up @@ -286,10 +264,6 @@ main()
# No specific tool banner, just check some logs are visible.
tool_banner=' .*<.*>'
boot_banner='FW ABI.*tag.*zephyr'
elif is_zephyr && [ "$f" = 'etrace' ]; then
# cavstool
tool_banner=':cavs-fw:'
boot_banner='FW ABI.*tag.*zephyr'
else
# sof-logger
# Other columns besides COMPONENT and CONTENT are optional
Expand Down