Skip to content

Conversation

@akloniex
Copy link
Member

In order to verify that running FW version is compatible with .ldc file parsed by sof-logger we need to extract fw_version from fw_ready message. This is done by moving fw_ready to separate elf section attached to sof_data memory region and pasting it to both .ri and .ldc files during rimage processing.

Copy link
Member

@lgirdwood lgirdwood left a comment

Choose a reason for hiding this comment

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

looks like you have not squashed changes ?

This is done in order to place fw_version, part of fw_ready message,
in both .ri and .ldc files, for later verification of compatibility
of trace logs reported by dsp with .ldc file parsed together by logger.

Signed-off-by: ArturX Kloniecki <arturx.kloniecki@linux.intel.com>

static const struct sof_ipc_fw_ready ready = {
static const struct sof_ipc_fw_ready ready
__attribute__((section(".fw_ready"))) = {
Copy link
Member

Choose a reason for hiding this comment

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

Just move to end of definition (like Linux) and we are good to merge.

Copy link
Member Author

@akloniex akloniex Oct 29, 2018

Choose a reason for hiding this comment

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

This is exactly in line with kernel examples:
e.g.: in macro expansion from include/linux/kernel.h:
#define ftrace_vprintk(fmt, vargs) \ do { \ if (__builtin_constant_p(fmt)) { \ static const char *trace_printk_fmt __used \ __attribute__((section("__trace_printk_fmt"))) = \ __builtin_constant_p(fmt) ? fmt : NULL; \ \ __ftrace_vbprintk(_THIS_IP_, trace_printk_fmt, vargs); \ } else \ __ftrace_vprintk(_THIS_IP_, fmt, vargs); \ } while (0)

It is a variable attribute, and cannot be placed after assignment operator, otherwise it will not compile.

Copy link
Member

Choose a reason for hiding this comment

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

ok, it seems we can do this where we define but not declare...

Copy link
Member Author

Choose a reason for hiding this comment

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

The problem is that it is defined and declared in the same statement.
I could split it into 2 separate statements if you'd like, but it seems redundant to me.


static const struct sof_ipc_fw_ready ready = {
static const struct sof_ipc_fw_ready ready
__attribute__((section(".fw_ready"))) = {
Copy link
Member

Choose a reason for hiding this comment

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

ok, it seems we can do this where we define but not declare...

@lgirdwood
Copy link
Member

@akloniex oh just a few checkpatch errors for alignment.

Extract fw_version from fw_ready section of elf and include it in
header of .ldc file to allow verification by sof-logger.

Signed-off-by: ArturX Kloniecki <arturx.kloniecki@linux.intel.com>
@akloniex
Copy link
Member Author

@lgirdwood I've corrected alignment.
Last error from checkpatch relates to use of CamelCase in Elf32_Shdr.

@lgirdwood lgirdwood merged commit 6b45d5c into thesofproject:master Oct 30, 2018
@akloniex akloniex deleted the extract-fw_version branch January 28, 2019 06:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants