Skip to content
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.

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

Update to RTEMS 6 with Libbsd and Legacy net support #61

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

kiwichris
Copy link

@kiwichris kiwichris commented Aug 7, 2024

This pull request adds support for RTEMS 6 for the legacy network and libbsd.

It needs a current build of RTEMS 6 and tools.

@kiwichris
Copy link
Author

I have reviewed the CI failures and I do not think they are related to the changes I have made.

@anjohnson
Copy link
Member

I just pushed a change to the master branch here which cleaned up some compiler warnings, and the subsequent CI build jobs for RTEMS 4.9 and 4.10 both succeeded. I also re-triggered those CI builds for this PR which unfortunately both failed again, so some code change inside this PR must have caused the failures.

I don't care about the pre-commit check failure, just the ability to build this module against RTEMS 4.9 and 4.10. Note that the EPICS Base 7.0 version that this CI is using at the moment doesn't include your RTEMS-6 PRs, and we expect future releases of this module to continue to build against older EPICS versions anyway — the CI jobs that build it for Linux against both 3.14 and 3.15 should hint at that.

@kiwichris kiwichris force-pushed the rtems-6-legacy-libbsd branch from 29a2d9e to 1a65c02 Compare August 25, 2024 03:34
@kiwichris
Copy link
Author

kiwichris commented Aug 25, 2024

@anjohnson I have resolved the issue with 4.10 and I assume 4.9. It seems the define _RTEMS_VIOLATE_KERNEL_VISIBILITY__ effects what is included by rtems.h and in this case wkspace.h was not being included. I have moved the include order to clean this up.

@hjunkes
Copy link

hjunkes commented Aug 26, 2024

Unfortunately, when I try to compile (MVME6100 legacy RTEMS6) I get this:
...
/include -I/home/rtems/MVME6100_6_legacy_INST/EPICS/epics-support/StreamDevice/include -c ../devIocStatsWaveform.c
In file included from ../os/RTEMS/devIocStatsOSD.h:84,
from ../devIocStats.h:52,
from ../devIocStatsWaveform.c:76:
/home/rtems/MVME6100_6_legacy_INST/rtems/6/powerpc-rtems6/include/stdlib.h:260:21: error: macro "random" passed 1 arguments, but takes just 0
260 | long random (void);
| ^
In file included from /home/rtems/MVME6100_6_legacy_INST/rtems/6/powerpc-rtems6/beatnik/lib/include/sys/malloc.h:39,
from /home/rtems/MVME6100_6_legacy_INST/rtems/6/powerpc-rtems6/beatnik/lib/include/sys/mbuf.h:38,
from ../os/RTEMS/devIocStatsOSD.h:65:
/home/rtems/MVME6100_6_legacy_INST/rtems/6/powerpc-rtems6/beatnik/lib/include/rtems/rtems_bsdnet_internal.h:63: note: macro "random" defined here
63 | #define random() rtems_bsdnet_random()
|
make[2]: *** [/home/rtems/MVME6100_6_legacy_INST/EPICS/epics-base/configure/RULES_BUILD:259: devIocStatsWaveform.o] Error 1
make[2]: Leaving directory '/home/rtems/MVME6100_6_legacy_INST/EPICS/epics-support/iocStats/devIocStats/O.RTEMS-beatnik'
make[1]: *** [/home/rtems/MVME6100_6_legacy_INST/EPICS/epics-base/configure/RULES_ARCHS:58: install.RTEMS-beatnik] Error 2
make[1]: Leaving directory '/home/rtems/MVME6100_6_legacy_INST/EPICS/epics-support/iocStats/devIocStats'
make: *** [/home/rtems/MVME6100_6_legacy_INST/EPICS/epics-base/configure/RULES_DIRS:85: devIocStats.install] Error 2

@hjunkes
Copy link

hjunkes commented Aug 26, 2024

e.g. in devIocStats/os/RTEMS/devIocStatsOSD.h
RTEMS_MAJOR, RTEMS_MINOR, RTEMS_REVISION
are not defined. Is that why the version status queries do not work?
defined in 6/powerpc-rtems6/beatnik/lib/include/rtems/score/cpuopts.h
So rtems.h must be included before the __RTEMS_XXX macros can be used?

But then I get this error:
support/StreamDevice/include -c ../os/RTEMS/osdFdUsage.c
../os/RTEMS/osdFdUsage.c: In function 'devIocStatsGetFDUsage':
../os/RTEMS/osdFdUsage.c:49:16: error: implicit declaration of function 'rtems_libio_count_open_iops' [-Werror=implicit-function-declaration]
49 | pval->used = rtems_libio_count_open_iops();
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1: some warnings being treated as errors
make[2]: *** [/home/rtems/MVME6100_6_legacy_INST/EPICS/epics-base/configure/RULES_BUILD:259: osdFdUsage.o] Error 1
make[2]: Leaving directory '/home/rtems/MVME6100_6_legacy_INST/EPICS/epics-support/iocStats/devIocStats/O.RTEMS-beatnik'
make[1]: *** [/home/rtems/MVME6100_6_legacy_INST/EPICS/epics-base/configure/RULES_ARCHS:58: install.RTEMS-beatnik] Error 2
make[1]: Leaving directory '/home/rtems/MVME6100_6_legacy_INST/EPICS/epics-support/iocStats/devIocStats'
make: *** [/home/rtems/MVME6100_6_legacy_INST/EPICS/epics-base/configure/RULES_DIRS:85: devIocStats.install] Error 2

@hjunkes
Copy link

hjunkes commented Aug 26, 2024

ok, found this:
https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/117#note_109499
Will update my RTEMS ...

@kiwichris
Copy link
Author

The rtems.h needs to be after #define __RTEMS_VIOLATE_KERNEL_VISIBILITY__ for the build to complete for the pre RTEMS 6 versions. I could have broken it for 6 making the change?

@hjunkes
Copy link

hjunkes commented Aug 27, 2024

Works with the current RTEMS6 master(main). rtems_libio_count_open_iops is defined there.

@kiwichris
Copy link
Author

Works with the current RTEMS6 master(main). rtems_libio_count_open_iops is defined there.

Thanks for confirming this. I do not know what the approval for workflows means.

@anjohnson
Copy link
Member

@simon-ess will have to speak up if he cares about the clang-format failure (those checks seem rather obnoxious to me), but this looks good for the RTEMS builds, thanks!

@simon-ess
Copy link
Contributor

I would prefer that the formatting check pass; otherwise it should be disabled, since we would then be just ignoring a failing test.

We could discuss exactly what the best formatting is, but I don't like the idea of letting it fail.

@kiwichris
Copy link
Author

I would prefer that the formatting check pass; otherwise it should be disabled, since we would then be just ignoring a failing test.

I had no idea there was a coding standard. I am happy to take a look.

We could discuss exactly what the best formatting is, but I don't like the idea of letting it fail.

This makes sense. I can fix it.

@kiwichris kiwichris force-pushed the rtems-6-legacy-libbsd branch from 1a65c02 to a5ccfbe Compare September 5, 2024 05:37
@kiwichris
Copy link
Author

I have pushed changes to fix the formatting. I did the changes manually from the diff in the CI log.

Is this OK?

@hjunkes
Copy link

hjunkes commented Oct 5, 2024

I found a problem when I wanted to use iocStats for beaglebone black (arm).
I had to change the following:
diff --git a/devIocStats/os/RTEMS/osdClustInfo.c b/devIocStats/os/RTEMS/osdClustInfo.c
index 1e07474..e1c74b7 100644
--- a/devIocStats/os/RTEMS/osdClustInfo.c
+++ b/devIocStats/os/RTEMS/osdClustInfo.c
@@ -110,6 +110,8 @@ int devIocStatsGetClusterInfo(int pool, clustInfo *pval) {
return 0;
}

+int devIocStatsGetClusterUsage(int pool, int pval) { return -1; }
+
#else /
RTEMS_LIBBSD_STACK */

/* This would otherwise need _KERNEL to be defined... */

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.

4 participants