doc: Updated 54H logging documentation #17331
Conversation
github-actions
bot
added
doc-required
CI InformationTo view the history of this post, clich the 'edited' button above Inputs:Sources:sdk-nrf: PR head: d0cf7150d107c4401f693287070a29951a9b17a4 more detailssdk-nrf:
Github labels
List of changed files detected by CI (3)Outputs:ToolchainVersion: Test Spec & Results: ✅ Success; ❌ Failure; 🟠 Queued; 🟡 Progress; ◻️ Skipped;
|
There was a problem hiding this comment.
I cannot leave comments inline because those lines are not changed but following things changed:
- proxy core is changed. ETR is handled by cpuapp (it used to be SDFW). SDFW knows if STM shall be enabled based on UICR entry
- enabling logging is done using a snippet
nordic-log-stmornordic-log-stm-dict. That is because not only Kconfig is needed but also devicetree entry (to get info to UICR). - There is an upstream documenation which might be referenced here: https://docs.zephyrproject.org/latest/services/logging/cs_stm.html
doc/nrf/app_dev/device_guides/working_with_nrf/nrf54h/ug_nrf54h20_logging.rst
Outdated
Show resolved
Hide resolved
FrancescoSer
force-pushed
the
54hloggingupd
branch
2 times, most recently
from
01cbb3c
to
7b6ec8f
Compare
|
You can find the documentation preview for this PR at this link. It will be updated about 10 minutes after the documentation build succeeds. Note: This comment is automatically posted by the Documentation Publishing GitHub Action. |
There was a problem hiding this comment.
I got the bundle that supports STM over UART and was able to run it and went through this page. Here are my comments.
-
I can't comment on technical details as I don't know how this is actually implemented, so I trust what is written here. But I find the figure (https://docs.zephyrproject.org/latest/services/logging/cs_stm.html#logging-using-stm) from zephyr doc very useful. It helped me wade through a bunch of acronyms. Duplicating it is probably not a good idea, but a link to that page or even figure could be very useful.
-
Regarding both configuration sections, they work with 2 snippets:
nordic-log-stm(for standalone logging) andnordic-log-stm-dict(dictionary logging/assisted multicore logging) as @nordic-krch said. The Kconfig option in the Standalone logging section can be replaced with-S nordic-log-stmaddition to west build command and in the assisted multicore logging sections with-S nordic-log-stm-dict. -
This is a nit-picking comment but referring a user to a devicetree doc here
for just enabling UART is way to confusing... Aren't there any samples that explain this already in a less intimidating manner? -
This (":file:
build/zephyr") is not fully correct:
It will be underbuild/<app_name>/zephyr/folder where<app_name>is the name of the application built for a specific core. And there will be as many<app_name>folders as apps built for each core. So for example, in my case when I builtsamples/bluetooth/peripheral_uartsample, I got 2 files:build/peripheral_uart/zephyr/log_dictionary.jsonandbuild/ipc_radio/zephyr/log_dictionary.jsonbecause it built 2 images:peripheral_uartfor the application core andipc_radiofor the radio core. -
This line has few issues:
- It can't be copy-pasted it assumes that secdom fw is also built which is not the case when building samples. Usually it is application core and radio core that are built more often.
- It uses decimals as domain IDs, but in the table below (
nRF54H20 log prefixes) they are provided in heximals. Passing heximals to a terminal will most likely won't work (can't say for all terminals though, but for bash it won't work). So these IDs should be in decimals for a user to easily copying them. - Might be more convenient to use
--stdout asciiinstead of--output-ascii out.txtoption just to see the output in a real-time (like a user would see if used serial terminal with standalone logging or normal logging over UART). - nRF54H20 has 2 UARTs, but only one will print logs. It would be good to explain how to find which serial ports exposed by the DK (as far as I know
nrfutil device listprints ports).
In my opinion it could be better to convert this command to a generic one, for example:
nrfutil trace stm --database-config <domain_id>:build/<app_name>/zephyr/log_dictionary.json --input-serialport <port> --baudrate 115200 --stdout ascii
where:
- <domain_id> is an ID of the domain which the application is running on (for several domains use `,` comma to separate each domain),
- <app_name> is the application name,
- <port> is the serial port used for output.
It is up to you if you fixes all my comments or some of them as well as if you fix them now or later. IMHO I'd suggest to fix 5, 4 and 2 comments now as they are most confusing now and comments 2 and 3 at some point later.
FrancescoSer
force-pushed
the
54hloggingupd
branch
from
7b6ec8f
to
d5b7977
Compare
|
@PavelVPV @nordic-krch Please rereview. I realized the upstream doc was heavily based on the original orphaned page here, and it currently looks partly outdated. Best solution here would be to ensure that this document here is currently correct and updated for CS2, and then resolve the duplication of content upstream before 2.8. |
doc/nrf/app_dev/device_guides/working_with_nrf/nrf54h/ug_nrf54h20_logging.rst
Show resolved
Hide resolved
FrancescoSer
force-pushed
the
54hloggingupd
branch
from
d5b7977
to
95132a8
Compare
FrancescoSer
force-pushed
the
54hloggingupd
branch
from
95132a8
to
045a995
Compare
|
|
|
||
| This command contains the following parameters: | ||
|
|
||
| * ``<domain_id>`` is the ID of the domain which the application is running on. |
There was a problem hiding this comment.
As domain_id should be in decimals, then to be consistent, at csv-table:: nRF54H20 log prefixes there should be also decimals - currently there are hexadecimals.
There was a problem hiding this comment.
@PavelVPV Could you check this?
There was a problem hiding this comment.
That was point 5.2 from my comment: #17331 (review). It would be more friendlier if the table had ids in decimals, though I'm fine with how it is right now. Maybe there are other places where heximals are used. Up to you if you want to fix it or not.
| * It reduces the amount of data that needs to be sent to and processed by the application core, as the string formatting is offloaded to the host side. | ||
| To show the logs for a given domain in the UART output, you must enable the STM for both the application core (as the ETR buffer processing proxy) and that domain's CPU. | ||
| To do so, use the :ref:`zephyr:nordic-log-stm-dict` snippet when building the application for the related cores. | ||
| For example, the following command uses the snippet when building for the application core:: |
There was a problem hiding this comment.
There is sample for STM at https://github.com/zephyrproject-rtos/zephyr/tree/main/samples/boards/nordic/coresight_stm
maybe it can be linked here?
Maybe this sample require some doc/readme too?
There was a problem hiding this comment.
Moreover, this doc provides some example stm output but not stating from which application/code. How user can achieve such logs.
Thus would be great it IMHO better to show logs from sample.
There was a problem hiding this comment.
There is sample for STM at https://github.com/zephyrproject-rtos/zephyr/tree/main/samples/boards/nordic/coresight_stm maybe it can be linked here? Maybe this sample require some doc/readme too?
That's merged in Zephyr. Linking could be done here if considered relevant by @nordic-krch, however, documentation cannot be done upstream before CS2. This would be a task for a future release, if we actually support that sample. @nordic-krch Any input here?
There was a problem hiding this comment.
Moreover, this doc provides some example stm output but not stating from which application/code. How user can achieve such logs. Thus would be great it IMHO better to show logs from sample.
@nordic-krch Any input here?
| To read the dictionary-based STM log output, do the following: | ||
|
|
||
| 1. Set up the log capture. | ||
|
|
||
| Use the ``nrfutil trace stm`` command to start capturing logs from the device, specifying the database configuration for each domain ID, as well as the serial port, the baud rate, and the output file name:: | ||
|
|
||
| nrfutil trace stm --database-config 33:build/secdom/src/secdombuild/zephyr/log_dictionary.json,34:build/zephyr/log_dictionary.json --input-serialport /dev/ttyACM1 --baudrate 115200 --output-ascii out.txt | ||
| nrfutil trace stm --database-config <domain_id>:build/<app_name>/zephyr/log_dictionary.json --input-serialport <port> --baudrate 115200 --stdout ascii |
There was a problem hiding this comment.
If example command is now --stdout ascii then sections regarding file with output content are obsolete - Open the output file to review the decoded log messages., Capture and decode the logs.
With --stdout ascii results will be shown at console.
with --output-ascii out.txt will be save into file.
|
Why there is reference to logging from System Controller (SysCtrl) and Secure Domain as no such thing will be available in the system? |
|
Regarding |
| @@ -193,11 +184,11 @@ The following are the prefixes used to indicate the cores: | |||
| Peripheral Processor (PPR), ``ppr``, 0x2e | |||
| , ``mod``, 0x24 | |||
There was a problem hiding this comment.
There is no core name for mod - cell in a table is empty.
There was a problem hiding this comment.
Internal docs did not define this. @hubertmis Should we remove that line? @nordic-krch What exactly is "mod" in the logs?
|
|
It was never technically clarified if these names can be synonims or not. From the doc point of view, when we have product names like Application Core and Secure Domain, saying just Domain might confuse the reader, I'll try to clarify that sentence. |
@nordic-krch Could you provide an answer here? |
Updated 54h logging documentation. Added small fix to 54h custom pcb documentation. Signed-off-by: Francesco Domenico Servidio <francesco.servidio@nordicsemi.no>
FrancescoSer
force-pushed
the
54hloggingupd
branch
from
045a995
to
d0cf715
Compare
|
@nordic-piks As the PR is part of the CS2 milestone and received approvals already, I'll ask for merge. I'll also open a separate PR to continue the discussion on your feedback. |
Updated 54h logging documentation.
Added small fix to 54h custom pcb documentation.