Support libmctp as a zephyr module #15
Conversation
|
I had a quick read through the zephyr module documentation and it seems we have a couple of approaches available. One is to do as you've done in this PR and integrate the zephyr metadata upstream. The other approach is to host a separate zephyr module repo for libmctp, where the zephyr metadata resides, merging in the changes from this upstream repo as required. I understand the former might be convenient, but do you have concerns regarding the latter? The zephyr metadata isn't directly relevant for non-zephyr consumers of libmctp, and keeping it where its relevant (closer to the zephyr project) feels preferable. |
I don't have a strong feeling on this, regardless I believe to have this be a direct dependency in Zephyr we'll need a fork in our github org, but it would be nice to have it be effectively upstream libmctp rather than a stack of patches on libmctp. If this doesn't seem useful to libmctp users, then that's the route I'll go. |
serial.c
Outdated
| @@ -5,6 +5,7 @@ | |||
| #include <stdbool.h> | |||
| #include <stdlib.h> | |||
| #include <string.h> | |||
| #include <sys/types.h> | |||
There was a problem hiding this comment.
For size_t? Did you hit a compilation failure?
serial.c
Outdated
| @@ -25,6 +26,10 @@ static const size_t write(int fd, void *buf, size_t len) | |||
|
|
|||
| #define pr_fmt(x) "serial: " x | |||
|
|
|||
| #ifdef CONFIG_MCTP | |||
| #define typeof __typeof__ | |||
There was a problem hiding this comment.
In range.h we have
#ifndef typeof
#define typeof __typeof__
#endif
That should probably be put elsewhere, but regardless, perhaps we could use the same test here as well rather than depend on CONFIG_MCTP, which is zephyr-specific.
There was a problem hiding this comment.
I've actually ended up porting most of the serial.c binding into a Zephyr native uart binding as there was enough of a difference to warrant it, this removes the conflicting issues in serial.c as well
serial.c
Outdated
| @@ -49,6 +54,7 @@ static const size_t write(int fd, void *buf, size_t len) | |||
| len ? wrote : 0; \ | |||
| }) | |||
|
|
|||
|
|
|||
There was a problem hiding this comment.
serial.c changes have all been removed in favor of creating a native zephyr uart binding in the zephyr tree, see zephyrproject-rtos/zephyr#75743
Signed-off-by: Tom Burdick <thomas.burdick@intel.com>
|
@amboar thank you for your review earlier, sorry its taken me awhile to get back to this, was working out all the details on how best to integrate with Zephyr. I've gone for more native binding approach which works well in my sample applications I have written. Would appreciate another look over. |
|
I can't say I'm too familiar with the zephyr conventions wrt packaging and modules, but it seems a bit odd to put the packaging metadata into the upstream tree. That said, I have no objection to the actual implementation if this is normal, especially given everything is contained to a A few queries/requests though:
|
Adds the needed changes to support directly using libmctp from Zephyr