Skip to content

Conversation

@mausys
Copy link
Contributor

@mausys mausys commented Aug 20, 2024

in commit 1cd37f2 the global ram console buffer was replaced with a pointer. This didn't work with the OpenAMP resource table anymore (#75656). This commit removes also the unnecessary console buffer, if a ram console device tree node is configured.
Fixes #75656

@carlocaione
Copy link
Contributor

Hi @mausys can you make sure that the CI is all green?

@mausys
Copy link
Contributor Author

mausys commented Oct 22, 2024

This change is also included in #79605

@mmahadevan108
Copy link
Contributor

cc @alxlastur @Zhiqiang-Hou
Can you also please help review this PR

Copy link
Contributor

Choose a reason for hiding this comment

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

Is'nt it sufficient to just use the extern to get the already defined ram_console_buf. This has the RAM_CONOLE_BUF_ATTR defined for it which should place it in the appropriate RAM section.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The ram_console_buf in ram_console.c is only used when CONFIG_RAM_CONSOLE_BUFFER_SECTION is disabled. That's why I changed it in ram_console.c to not wasting memory.

Copy link
Contributor

Choose a reason for hiding this comment

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

For the condition CONFIG_RAM_CONSOLE_BUFFER_SECTION, the RAM_CONOLE_BUF_ATTR is for putting it in the RAM section, so no memory wasting. keep the ram_console_buf for both conditions to avoid to determine the RAM section condition in the caller.

@mmahadevan108 mmahadevan108 added the bug The issue is a bug, or the PR is fixing a bug label Oct 28, 2024
Copy link
Contributor

Choose a reason for hiding this comment

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

revert this and directly use extern char ram_console_buf[]; in lib/open-amp/resource_table.c.

@mausys mausys force-pushed the fix_ram_console branch 2 times, most recently from 5a47c85 to 09ecb39 Compare October 29, 2024 14:30
in commit 1cd37f2 the global ram console buffer was replaced
with a pointer. This didn't work with the OpenAMP resource table
anymore (zephyrproject-rtos#75656).

Signed-off-by: Simon Maurer <[email protected]>
@mmahadevan108
Copy link
Contributor

@carlocaione, can you help review this again.

@mmahadevan108 mmahadevan108 added this to the v4.0.0 milestone Oct 29, 2024
@mmahadevan108
Copy link
Contributor

@carlocaione , can you help take a look. This bug fix can be included in 4.0 release

@mmahadevan108 mmahadevan108 merged commit 18818c1 into zephyrproject-rtos:main Nov 8, 2024
23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: Console area: Open AMP bug The issue is a bug, or the PR is fixing a bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ram_console buffer pointer is incompatible with openamp trace log

7 participants