Add GCC example for rpmsg echo#113
Conversation
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
PR Code Suggestions ✨Explore these optional code suggestions:
|
||||||||||||||||
PR Compliance Guide 🔍Below is a summary of compliance checks for this PR:
Compliance status legend🟢 - Fully Compliant🟡 - Partial Compliant 🔴 - Not Compliant ⚪ - Requires Further Human Verification 🏷️ - Compliance label |
||||||||||||||||||
|
Hey @dinuxbg, thank you for submitting a pull request! I have not spent any time yet playing with a GCC version of the PRU compiler at this point in time, so please forgive me if my questions are a bit "beginner level".
|
I can add a CI pipeline to verify that the GCC example builds. Looking at this CI pipeline from a GCC user, it should be easy.
I admit that commercial users would probably pick the toolchain provided and supported by the SoC vendor (i.e. PRU-CGT). So if that it is the target for As author and maintainer of the PRU port for GCC, I'm biased. Anyway, here are my reasons for using it:
I know someone tried to port LLVM, but I think it never got finished or mainlined. Since the PRU port of GCC is in mainline, there is only one PRU GCC source code, and it is in the main GCC GIT repository. I provide prebuilt toolchains as a convenience in my gnupru Github page. I'm not aware of anyone else providing prebuilt toolchains, but I've shared my build scripts so anyone is free to do so.
This sounds reasonable. Regards, |
|
Hello @dinuxbg , Ok. Let's keep this PR for updates to the RPMsg libraries, but drop the GCC updates to the rpmsg_echo example. Our team has not done a great job of pointing customers to all the great work you and others are doing in the broader community. I can't promise that we will turn a full 180 degrees (e.g., I joined the beagleboard discord last year, but never have time to read it unless someone points me to a specific post), but I DO want to be more intentional about helping customers to find useful resources, regardless of whether those resources come from TI or from the community. In this PR or a separate PR, it would be great if I could get you to create a separate example, just for showing off GCC.
|
|
Additional notes:
|
Yes, GCC supports arbitrary number of inputs, outputs and clobbers for inline assembly.
The v4 instructions are not yet supported. I plan to add them. |
Good to know, I'll make a mental note about this if any customers ask
I have not yet added the v4 instructions to https://www.ti.com/lit/spruij2. We'll see if I get around to updating that doc in 1H2026, if you need guidance on the v4 instructions in the future feel free to create an e2e thread |
dinuxbg
force-pushed
the
gcc-for-rpmsg-example
branch
from
b040fe7 to
58da413
Compare
Review Summary by QodoAdd GCC toolchain support for PRU RPMSG echo example with compiler fixes
WalkthroughsDescription• Add GCC toolchain support for PRU RPMSG echo example • Fix compiler warnings and compatibility issues for GCC • Include missing headers and correct pointer-to-integer casts • Add CI workflow for GNU toolchain validation Diagramflowchart LR
A["GCC Example<br/>gcc_rpmsg_echo_linux"] --> B["Config & Headers"]
A --> C["Main Firmware<br/>with Inline ASM"]
D["Shared RPMSG<br/>Library"] --> E["Compiler Fixes"]
E --> F["String.h Include"]
E --> G["Pointer Cast Fix"]
E --> H["Sign Compare Fix"]
B --> I["Resource Table<br/>GCC Support"]
C --> J["Reversed Echo<br/>using ASM"]
K["CI Workflow"] --> L["GNU Toolchain<br/>Build & Test"]
File Changes1. examples/gcc_rpmsg_echo_linux/config.h
|
Code Review by Qodo
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
dinuxbg
force-pushed
the
gcc-for-rpmsg-example
branch
from
58da413 to
63757ef
Compare
Review Summary by QodoAdd GCC toolchain support for PRU RPMSG echo example
WalkthroughsDescription• Add GCC toolchain support for PRU RPMSG echo example • Fix compiler warnings and compatibility issues for GCC • Include missing header and fix pointer-to-integer casts • Add CI workflow for GNU toolchain validation Diagramflowchart LR
A["Shared RPMSG Library"] -->|"Fix warnings"| B["pru_virtqueue.c"]
A -->|"Add missing header"| C["pru_rpmsg.c"]
A -->|"Fix pointer cast"| D["pru_virtio_ring.h"]
E["Resource Table"] -->|"Add GCC pragmas"| F["resource_table.h"]
G["GCC Example"] -->|"New files"| H["gcc_rpmsg_echo_linux/"]
H -->|"Contains"| I["main.c, config.h, intc_map.h"]
J["CI Pipeline"] -->|"Validate build"| K["gnu-toolchain.yml"]
File Changes1. source/rpmsg/pru_rpmsg.c
|
|
Persistent review updated to latest commit 63757ef |
dinuxbg
force-pushed
the
gcc-for-rpmsg-example
branch
from
63757ef to
cba7115
Compare
Review Summary by QodoAdd GCC toolchain support for RPMSG echo example with CI
WalkthroughsDescription• Add GCC toolchain support for PRU RPMSG echo example • Fix compiler warnings and portability issues in shared code • Include GNU toolchain CI workflow for automated builds • Create new example with inline assembly demonstration Diagramflowchart LR
A["Shared RPMSG Library"] -->|Fix warnings| B["pru_virtqueue.c<br/>pru_rpmsg.c"]
A -->|Fix pointer cast| C["pru_virtio_ring.h"]
A -->|Add GCC pragmas| D["resource_table.h"]
E["GCC Example"] -->|New files| F["gcc_rpmsg_echo_linux/"]
F -->|Config & Build| G["Makefile<br/>config.h"]
F -->|RPMSG Setup| H["main.c<br/>intc_map.h"]
I["CI Pipeline"] -->|Build GCC| J["gnu-toolchain.yml"]
File Changes1. source/rpmsg/pru_rpmsg.c
|
|
Persistent review updated to latest commit cba7115 |
|
I am going to be out of office until mid May. Please rebase on top of main so we can see if this passes the new CI make infrastructure. As soon as the automated build stuff is working, please treat this as my approval of this PR. All the actual content of the PRU looks good. cc @pratheesh-ti @dhavaljk |
Using memcpy requires string.h header, so include it. Signed-off-by: Dimitar Dimitrov <dimitar@dinux.eu>
GCC issues the following warning:
../../../../../../source/rpmsg/pru_virtqueue.c:116:18: warning: comparison of integer expressions of different signedness: ‘int16_t’ {aka ‘short int’} and ‘uint32_t’ {aka ‘long unsigned int’} [-Wsign-compare]
116 | if (head > num)
Fix by explicitly using unsigned integers for the comparison.
Negative value should never be passed to pru_virtqueue_add_used_buf
because it is an invalid head. If it is passed, though, making it
unsigned would make it too large, and trigger the early exit with an
error.
Signed-off-by: Dimitar Dimitrov <dimitar@dinux.eu>
GCC issues this warning:
../../../../../../source/include/c_code/linux/pru_virtio_ring.h:138:41: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
138 | vr->used = (void *)(uintptr_t)(((uint64_t)&vr->avail->ring[num]
Fix by using uintptr_t consistently. On the 32-bit PRU the
upper bits of the 64-it intermediate result would be discarded
anyway by the second cast to uintptr_t.
Signed-off-by: Dimitar Dimitrov <dimitar@dinux.eu>
GCC and TI CGT have different methods to implement some non-standard C language features, which are used in the rpmsg library. Add such implementation for GCC, and use the `__GNUC__` macro to pick either the TI CGT or the GCC snippet. * The R30 and R31 global register declarations are different, and conveniently provided by GNU toolchain in the `pru/io.h` file. * Aggregate member initializers inside an aggregate type must be enclosed in curly braces (i.e. the initializer must follow the type hierarchy). * Placing a variable into a specific section is accomplished by an attribute in GCC. Signed-off-by: Dimitar Dimitrov <dimitar@dinux.eu>
dinuxbg
force-pushed
the
gcc-for-rpmsg-example
branch
from
cba7115 to
5602037
Compare
Review Summary by QodoAdd GCC toolchain support for PRU RPMSG echo example with compiler fixes
WalkthroughsDescription• Add GCC toolchain support for PRU RPMSG echo example • Fix compiler warnings and pointer casting issues • Include missing string.h header for memcpy usage • Add GitHub Actions CI workflow for GNU toolchain builds Diagramflowchart LR
A["GCC Example<br/>gcc_rpmsg_echo_linux"] --> B["Config & Headers"]
A --> C["Main Firmware"]
D["Shared RPMSG Library"] --> E["Compiler Fixes"]
E --> F["String.h Include"]
E --> G["Pointer Casting"]
E --> H["Sign Comparison"]
B --> I["MCU-specific Setup"]
C --> J["Inline Assembly"]
K["CI/CD"] --> L["GNU Toolchain Workflow"]
File Changes1. examples/gcc_rpmsg_echo_linux/config.h
|
|
Persistent review updated to latest commit 5602037 |
manojKoppolu
left a comment
manojKoppolu
left a comment
There was a problem hiding this comment.
Hi dinuxbg,
Please rebase your PR to trigger new CI make build.
Validated on BeaglePlay with kernel 6.12.43-ti-arm64-r54. Signed-off-by: Dimitar Dimitrov <dimitar@dinux.eu>
dinuxbg
force-pushed
the
gcc-for-rpmsg-example
branch
from
5602037 to
ca22512
Compare
Review Summary by QodoAdd GCC toolchain support for RPMSG echo example with compiler fixes
WalkthroughsDescription• Add GCC toolchain support for PRU RPMSG echo example • Fix compiler warnings and pointer casting issues • Include missing string.h header for memcpy usage • Add CI workflow for GNU toolchain validation Diagramflowchart LR
A["GCC Example<br/>gcc_rpmsg_echo_linux"] --> B["Config & Build<br/>config.h, Makefile"]
A --> C["Firmware Code<br/>main.c, intc_map.h"]
D["Shared RPMSG Library<br/>pru_rpmsg.c, pru_virtqueue.c"] --> E["Compiler Fixes<br/>string.h, type casts"]
F["Resource Table<br/>resource_table.h"] --> G["GCC Pragmas<br/>__attribute__ section"]
H["Shared Headers<br/>pru_virtio_ring.h"] --> I["Pointer Cast Fix<br/>uintptr_t consistency"]
J["CI Workflow<br/>gnu-toolchain.yml"] --> K["Automated Build<br/>Testing"]
File Changes1. examples/gcc_rpmsg_echo_linux/config.h
|
|
Persistent review updated to latest commit ca22512 |
4 participants
User description
I figured it would be nice to have one example with GCC.
The GCC support is relatively non-intrusive. A few places in shared code required
#ifdefstatements due to different syntax in GCC and CGT.I tested the firmware on BeaglePlay with kernel 6.12.43-ti-arm64-r54.
PR Type
Enhancement, Bug fix
Description
Add GCC compiler support for rpmsg echo example
Fix compiler warnings and compatibility issues
Update resource table initialization for GCC compatibility
Diagram Walkthrough
File Walkthrough
Makefile
GCC build system for PRU rpmsg echoexamples/rpmsg_echo_linux/firmware/am62x-sk/pruss0_pru0_fw/gnu-pru-gcc/Makefile
intc_map.h
INTC mapping header for GCC toolchainexamples/rpmsg_echo_linux/firmware/am62x-sk/pruss0_pru0_fw/gnu-pru-gcc/intc_map.h
main.c
GCC-compatible register access in mainexamples/rpmsg_echo_linux/firmware/main.c
__R31register accesspru/io.hheader instead of direct register declarationpru_intc.h
GCC-compatible INTC register declarationsource/include/c_code/am62x/pru_intc.h
CT_INTCdeclarationresource_table.h
GCC-compatible resource table definitionsource/include/c_code/linux/resource_table.h
__attribute__((section()))instead of pragmaspru_virtio_ring.h
Fix pointer cast warning in vring initsource/include/c_code/linux/pru_virtio_ring.h
vring_initfunctionuint64_ttouintptr_tpru_rpmsg.c
Add missing string.h headersource/rpmsg/pru_rpmsg.c
#include <string.h>headerpru_virtqueue.c
GCC compatibility and signedness warning fixsource/rpmsg/pru_virtqueue.c
__R31register accesspru/io.hheader instead of direct register declarationheadtouint32_t