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.

Sign up for GitHub

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

Jump to bottom

subsys: Add MCTP as a subsystem, built on libmctp with bindings #75743

Merged
merged 4 commits into from
Jan 14, 2025
Merged

subsys: Add MCTP as a subsystem, built on libmctp with bindings #75743

merged 4 commits into from
Jan 14, 2025

Conversation

teburd
Copy link
Collaborator

@teburd teburd commented Jul 11, 2024 •
edited
Loading

Adds libmctp as a zephyr module from my fork with a few small changes I intend on trying to upstream along with a binding to Zephyr's polling async UART API. Comes with a pair of sample applications that can be used on two boards connected via UART.

@teburd teburd changed the title Libmctp module modules: libmctp Jul 11, 2024
@teburd teburd requested review from nashif and KeHIntel July 11, 2024 00:48
@zephyrbot
Copy link
Collaborator

zephyrbot commented Jul 11, 2024 •
edited
Loading

The following west manifest projects have changed revision in this Pull Request:

Name Old Revision New Revision Diff
libmctp 🆕 N/A (Added) zephyrproject-rtos/libmctp@fa15266 (zephyr) N/A

⛔ DNM label due to: 1 added project

Note: This message is automatically posted and updated by the Manifest GitHub Action.

@zephyrbot zephyrbot added manifest manifest-libmctp DNM This PR should not be merged (Do Not Merge) labels Jul 11, 2024
@teburd teburd added this to the v4.0.0 milestone Jul 11, 2024
@ubieda ubieda self-requested a review July 31, 2024 16:24
@teburd teburd force-pushed the libmctp_module branch 6 times, most recently from e32c677 to 44752b4 Compare August 8, 2024 15:47
@teburd teburd force-pushed the libmctp_module branch 2 times, most recently from bd7c893 to 153b02d Compare August 14, 2024 20:17
@teburd
Copy link
Collaborator Author

teburd commented Aug 14, 2024

Need to work out some upstream stuff, but can then have the fork setup in the zephyrproject-rtos github org

@teburd teburd added the TSC Topics that need TSC discussion label Aug 21, 2024
@teburd teburd mentioned this pull request Aug 21, 2024
Closed
@teburd teburd force-pushed the libmctp_module branch 4 times, most recently from 91652d2 to f884e0d Compare August 22, 2024 15:46
@teburd
Copy link
Collaborator Author

teburd commented Aug 22, 2024

In my original samples I had hacked up the serial binding to work with Zephyr natively, to better integrate with Zephyr however its useful to provide native bindings in a subsys so that's what I've begun to do.

henrikbrixandersen
henrikbrixandersen reviewed Sep 4, 2024
View reviewed changes
doc/Makefile Outdated Show resolved Hide resolved
samples/modules/mctp/mctp_endpoint/CMakeLists.txt Outdated Show resolved Hide resolved
@teburd teburd force-pushed the libmctp_module branch 2 times, most recently from 29717fb to e1f9cbd Compare September 6, 2024 16:36
@teburd
modules: Add libmctp as a module
0661854 
Adds libmctp as a west module dependency

Signed-off-by: Tom Burdick <thomas.burdick@intel.com>
@zephyrbot zephyrbot added the DNM This PR should not be merged (Do Not Merge) label Jan 8, 2025
@teburd teburd removed the DNM This PR should not be merged (Do Not Merge) label Jan 8, 2025
@teburd teburd requested a review from dkalowsk January 9, 2025 16:50
@teburd
Copy link
Collaborator Author

teburd commented Jan 9, 2025

Now passing CI

nashif
nashif previously approved these changes Jan 9, 2025
View reviewed changes
KeHIntel
KeHIntel previously approved these changes Jan 10, 2025
View reviewed changes
kartben
kartben requested changes Jan 14, 2025
View reviewed changes
samples/modules/mctp/mctp_endpoint/README.rst Outdated Show resolved Hide resolved
samples/modules/mctp/mctp_host/README.rst Outdated Show resolved Hide resolved
samples/modules/mctp/mctp_endpoint/README.rst Outdated Show resolved Hide resolved
samples/modules/mctp/mctp_endpoint/README.rst Outdated Show resolved Hide resolved
samples/modules/mctp/mctp_host/README.rst Outdated Show resolved Hide resolved
subsys/mctp/mctp_uart.c Outdated Show resolved Hide resolved
samples/modules/mctp/mctp_host/README.rst Outdated Show resolved Hide resolved
teburd added 3 commits January 14, 2025 11:09
@teburd
mctp: Add mctp subsystem with uart binding
e81c690 
libmctp provides interfaces for wiring up a MCTP bus it calls bus
bindings. The bindings provided in libmctp however are not directly
useful to Zephyr without some work. Provide an initial uart binding that
directly uses Zephyr's async uart interface.

Signed-off-by: Tom Burdick <thomas.burdick@intel.com>
@teburd
samples: Add mctp host and client samples
d1a7a41 
Samples work by sending MCTP encoded messages over a uart between two
boards.

Signed-off-by: Tom Burdick <thomas.burdick@intel.com>
@teburd
MAINTAINERS.yml: Add libmctp module maintainership
55876a9 
Adds myself, nashif, and inteljiangwe1 to the maintainers file covering
the libmctp module.

Signed-off-by: Tom Burdick <thomas.burdick@intel.com>
@teburd teburd dismissed stale reviews from KeHIntel and nashif via 55876a9 January 14, 2025 17:09
@teburd teburd requested a review from kartben January 14, 2025 17:09
@zephyrbot zephyrbot added the DNM This PR should not be merged (Do Not Merge) label Jan 14, 2025
@teburd teburd requested review from KeHIntel and nashif January 14, 2025 17:53
@teburd teburd removed the DNM This PR should not be merged (Do Not Merge) label Jan 14, 2025
@henrikbrixandersen henrikbrixandersen merged commit 00ab7a9 into zephyrproject-rtos:main Jan 14, 2025
28 of 29 checks passed
@teburd teburd deleted the libmctp_module branch January 14, 2025 22:18
@ofirshe
Copy link
Contributor

ofirshe commented Jan 15, 2025

Hi,
Upon rebasing to the latest Zephyr after long time, I’m seeing this warning:

CMake Warning at zephyr/CMakeLists.txt:1002 (message):  
  No SOURCES given to Zephyr library: modules_mctp  
  Excluding target from build.

I also noticed that CONFIG_MCTP isn’t set in output .config file. If it’s disabled, why does this warning appear?
Any guidance would be appreciated!

@ubieda
Copy link
Member

ubieda commented Jan 15, 2025

Hi, Upon rebasing to the latest Zephyr after long time, I’m seeing this warning:

CMake Warning at zephyr/CMakeLists.txt:1002 (message):  
  No SOURCES given to Zephyr library: modules_mctp  
  Excluding target from build.

I also noticed that CONFIG_MCTP isn’t set in output .config file. If it’s disabled, why does this warning appear? Any guidance would be appreciated!

Could you try west update and see if it's gone?

@ofirshe
Copy link
Contributor

ofirshe commented Jan 15, 2025

@ubieda I ran west update immediately after rebasing.

@kartben
Copy link
Collaborator

kartben commented Jan 15, 2025 •
edited
Loading

just opened #84031

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@henrikbrixandersen henrikbrixandersen henrikbrixandersen left review comments

@ubieda ubieda ubieda approved these changes

@nashif nashif nashif approved these changes

@kartben kartben kartben approved these changes

@MaureenHelm MaureenHelm Awaiting requested review from MaureenHelm

@stephanosio stephanosio Awaiting requested review from stephanosio

@lyakh lyakh Awaiting requested review from lyakh

@pillo79 pillo79 Awaiting requested review from pillo79

@dkalowsk dkalowsk Awaiting requested review from dkalowsk

@KeHIntel KeHIntel Awaiting requested review from KeHIntel

Assignees

@kartben kartben

Labels
area: Process area: Samples Samples manifest manifest-libmctp
Projects
TSC Attention Needed
Status: Done
Milestone
v4.1.0
Development

Successfully merging this pull request may close these issues.

Module for libmctp
10 participants
@teburd @zephyrbot @KeHIntel @dkalowsk @ofirshe @ubieda @kartben @nashif @henrikbrixandersen @mmahadevan108