-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Add MCTP over USB Binding support #95347
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,3 +13,4 @@ USB device support APIs | |
usbd_msc_device.rst | ||
usbd_midi2.rst | ||
usbd_dfu.rst | ||
usbd_mctp.rst |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
.. _usbd_mctp: | ||
|
||
MCTP USB device API | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please detail what "MCTP" actually is here. |
||
######################## | ||
|
||
MCTP USB device specific API defined in :zephyr_file:`include/zephyr/usb/class/usbd_mctp.h`. | ||
|
||
API Reference | ||
************* | ||
|
||
.. doxygengroup:: usbd_mctp |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -41,6 +41,8 @@ Samples | |
|
||
* :zephyr:code-sample:`usb-hid-mouse` | ||
|
||
* :zephyr:code-sample:`mctp-usb-endpoint` | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you suggest that I just update this file in a commit with the sample application, or do one 'super commit' with all of these feature/MCTP changes? I did it this way for human readability, but didn't fully understand the need for every commit to build. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. "Commits in a pull request should represent clear, logical units of change that are easy to review and maintain bisectability." https://docs.zephyrproject.org/latest/contribute/contributor_expectations.html#pr-requirements |
||
|
||
* :zephyr:code-sample:`zperf` To build the sample for the device support, | ||
set the configuration overlay file | ||
``-DDEXTRA_CONF_FILE=overlay-usbd_next_ecm.conf`` and devicetree overlay file | ||
|
@@ -217,6 +219,8 @@ instance (``n``) and is used as an argument to the :c:func:`usbd_register_class` | |
+-----------------------------------+-------------------------+-------------------------+ | ||
| USB Video Class (UVC) | Video device | :samp:`uvc_{n}` | | ||
+-----------------------------------+-------------------------+-------------------------+ | ||
| USB MCTP over USB Endpoint | :ref:`usbd_mctp` | :samp:`mctp_{n}` | | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. s/USB MCTP over USB Endpoint/MCTP over USB Binding |
||
+-----------------------------------+-------------------------+-------------------------+ | ||
|
||
CDC ACM UART | ||
============ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
# Copyright 2025 NXP | ||
# SPDX-License-Identifier: Apache-2.0 | ||
|
||
description: | | ||
A configuration for MCTP binding to a USB target. | ||
|
||
compatible: "zephyr,mctp-usb" | ||
|
||
properties: | ||
endpoint-id: | ||
type: int | ||
description: | | ||
MCTP Endpoint ID |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,68 @@ | ||
/* | ||
* Copyright (c) 2024 Intel Corporation | ||
* Copyright 2025 NXP | ||
* | ||
* SPDX-License-Identifier: Apache-2.0 | ||
* | ||
*/ | ||
|
||
#ifndef ZEPHYR_MCTP_USB_H_ | ||
#define ZEPHYR_MCTP_USB_H_ | ||
|
||
#include <stdint.h> | ||
#include <zephyr/kernel.h> | ||
#include <zephyr/device.h> | ||
#include <libmctp.h> | ||
|
||
#define MCTP_USB_HEADER_SIZE 4 | ||
#define MCTP_USB_MAX_PACKET_LENGTH 255 | ||
|
||
/** | ||
* @brief An MCTP binding for Zephyr's USB device stack | ||
*/ | ||
struct mctp_binding_usb { | ||
/** @cond INTERNAL_HIDDEN */ | ||
struct mctp_binding binding; | ||
const struct device *usb_dev; | ||
uint8_t endpoint_id; | ||
uint8_t tx_buf[MCTP_USB_HEADER_SIZE + MCTP_USB_MAX_PACKET_LENGTH]; | ||
struct k_sem tx_sem; | ||
struct mctp_pktbuf *rx_pkt; | ||
uint8_t rx_data_idx; | ||
enum { | ||
STATE_WAIT_HDR_DMTF0, | ||
STATE_WAIT_HDR_DMTF1, | ||
STATE_WAIT_HDR_RSVD0, | ||
STATE_WAIT_HDR_LEN, | ||
STATE_DATA | ||
} rx_state; | ||
/** @endcond INTERNAL_HIDDEN */ | ||
}; | ||
|
||
/** @cond INTERNAL_HIDDEN */ | ||
int mctp_usb_start(struct mctp_binding *binding); | ||
int mctp_usb_tx(struct mctp_binding *binding, struct mctp_pktbuf *pkt); | ||
/** @endcond INTERNAL_HIDDEN */ | ||
|
||
/** | ||
* @brief Define a MCTP bus binding for USB | ||
* | ||
* @param _name Symbolic name of the bus binding variable | ||
* @param _dev DeviceTree Node containing the configuration of this MCTP binding | ||
*/ | ||
#define MCTP_USB_DT_DEFINE(_name, _dev) \ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I do not get the purpose of this macro and the mctp_usb.c, you already have implemented instantiation and support for this function in the previous commit, what it that and why does it have to be divided? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not following. This macro was created based on the other libmctp binding implementations. What do you mean by it being divided? |
||
struct mctp_binding_usb _name = { \ | ||
.binding = { \ | ||
.name = STRINGIFY(_name), .version = 1, \ | ||
.pkt_size = \ | ||
MCTP_PACKET_SIZE(MCTP_USB_MAX_PACKET_LENGTH), \ | ||
.pkt_header = 0, .pkt_trailer = 0, \ | ||
.start = mctp_usb_start, .tx = mctp_usb_tx, \ | ||
}, \ | ||
Comment on lines
+56
to
+61
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Broken style. |
||
.usb_dev = DEVICE_DT_GET(_dev), \ | ||
.endpoint_id = DT_PROP_OR(_dev, endpoint_id, 0), \ | ||
.rx_pkt = NULL, \ | ||
.rx_data_idx = 0, \ | ||
.rx_state = STATE_WAIT_HDR_DMTF0}; | ||
|
||
#endif /* ZEPHYR_MCTP_USB_H_ */ |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,88 @@ | ||
/* | ||
* Copyright (c) 2025 Nordic Semiconductor ASA | ||
* Copyright 2025 NXP | ||
* | ||
* SPDX-License-Identifier: Apache-2.0 | ||
*/ | ||
|
||
/** | ||
* @file | ||
* @brief USB MCTP over USB Protocol Endpoint Device Class (MCTP) public header | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
* | ||
* This header describes MCTP class interactions desgined to be used by libMCTP. | ||
* The MCTP device itself is modelled with devicetree zephyr,mctp-usb compatible. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure what that means, but usually devicetree is used to describe and instantiate a device. Whether it makes sense here or not is disputable. |
||
* | ||
* This API is currently considered experimental. | ||
*/ | ||
|
||
#ifndef ZEPHYR_INCLUDE_USB_CLASS_USBD_MCTP_H_ | ||
#define ZEPHYR_INCLUDE_USB_CLASS_USBD_MCTP_H_ | ||
|
||
/** | ||
* @brief USB MCTP device API | ||
* @defgroup usbd_mctp USB MCTP device API | ||
* @ingroup usb | ||
* @since 4.2 | ||
* @version 0.1.0 | ||
* @{ | ||
*/ | ||
|
||
/** | ||
* @brief USB MCTP application event handlers | ||
*/ | ||
struct mctp_ops { | ||
/** | ||
* @brief Data received | ||
* | ||
* This function is called after the USB has received a packet of data, up to max packet | ||
* size. The MCTP USB binding for libMCTP is responsible for copying the data from the USB | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What |
||
* buffer and packetizing the data for the MCTP core. This callback is mandatory. | ||
* | ||
* @param dev MCTP USB device instance | ||
* @param buf Buffer containing USB packet data | ||
* @param size Number of bytes in the buffer | ||
* @param user_data Opaque user data pointer | ||
*/ | ||
void (*data_recv_cb)(const struct device *dev, void *buf, uint16_t size, void *user_data); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. void (*data_recv_cb)(const struct device *dev,
const void *const buf,
const uint16_t size,
const void *const user_data); Can it receive something else as |
||
/** | ||
* @brief TX complete | ||
* | ||
* This function is called after the USB has completed sending data to the MCTP bus owner. | ||
* This function is optional in general, but required if using the libMCTP USB binding. | ||
* | ||
* @param dev MCTP USB device instance | ||
* @param buf Buffer containing USB packet data | ||
* @param size Number of bytes in the buffer | ||
* @param user_data Opaque user data pointer | ||
*/ | ||
void (*tx_complete_cb)(const struct device *dev, void *user_data); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. void (*tx_complete_cb)(const struct device *dev,
const void *const user_data); |
||
}; | ||
|
||
/** | ||
* @brief Register MCTP USB application callbacks. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You modeled a I device, what I think you should not dp, but if, the description should be:
|
||
* | ||
* @param dev MCTP USB device instance | ||
* @param ops MCTP callback structure | ||
* @param user_data Opaque user data to pass to ops callbacks | ||
*/ | ||
void usbd_mctp_set_ops(const struct device *dev, const struct mctp_ops *ops, void *user_data); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. void usbd_mctp_register(const struct device *dev,
const struct mctp_ops *const ops,
const void *const user_data); |
||
|
||
/** | ||
* @brief Send data to the MCTP bus owner. | ||
* | ||
* @note Buffer ownership is transferred to the stack in case of success, in | ||
* case of an error the caller retains the ownership of the buffer. | ||
* | ||
* @param dev MCTP USB device instance | ||
* @param buf Buffer containing outgoing data | ||
* @param size Number of bytes to send | ||
* | ||
* @return 0 on success, negative value on error | ||
*/ | ||
int usbd_mctp_send(const struct device *dev, void *buf, uint16_t size); | ||
|
||
/** | ||
* @} | ||
*/ | ||
|
||
#endif /* ZEPHYR_INCLUDE_USB_CLASS_USBD_MCTP_H_ */ |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
# SPDX-License-Identifier: Apache-2.0 | ||
|
||
cmake_minimum_required(VERSION 3.20.0) | ||
find_package(Zephyr REQUIRED HINTS $ENV{ZEPHYR_BASE}) | ||
project(mctp_endpoint) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. project name could use something more descriptive, I get I'm guilty of probably starting this but perhaps mctp_usb_endpoint would be better |
||
|
||
include(${ZEPHYR_BASE}/samples/subsys/usb/common/common.cmake) | ||
FILE(GLOB app_sources src/*.c) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. please don't glob here |
||
target_sources(app PRIVATE ${app_sources}) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
# Copyright (c) 2023 Nordic Semiconductor ASA | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am confused, is PMCI/MCTP not part of the Zephyr but an external module? If so then I do not think you can add any dependency on it to the USB subsystem. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The way I looked at it is that MCTP is called out as a base class on usb.org, so the device class implementation can exists in Zephyr, even if the higher-level MCTP protocol management comes from an external module. Right now it comes from libmctp (https://github.com/openbmc/libmctp). I understand this concern. Is there something you think would create such a dependency? Again, I'm new to Zephyr, so maybe I don't fully understand something. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
mctp is not an external module, it is part of the main manifest and there are dependencies on it already. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Anyway, the implementation cannot be part of subsys/usb, and has to go to the modules. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The fact that MCTP is listed on usb.org doesn't matter then? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @jfischer-no I too am confused by the comment that this should go to modules, mctp is a subsystem in zephyr built on libmctp. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
yeah, this is confusing. The fact that libmctp is a module is irrelevant, this is implementing usb bindings, using both mctp and usb in zephyr, this has nothing to do with libmctp, the usb binding is a zephyr thing. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
It does not mean anything. The specification is from DMTF association, and I already mentioned that, see #95347 (comment). You should know that, as you had to read it to write this code, or where does the code come from?
What is external dependency, and like a sample in this PR is placed under
"usb binding" are useless without libmctp, "usb binding" is some trivial USB code that I will not accept in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @jfischer-no please, this is like saying I should not accept any code that depends on a hal, we've passed that rubicon long ago To say it should go in subsys/pmci/mctp that's fine, its where the rest of the binding code is anyways. I get if you don't want to take maintainership of this code that's very understandable and reasonable. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
right, but we have libmctp and it is not optional. Anyways, what is it exactly that you want?
|
||
# SPDX-License-Identifier: Apache-2.0 | ||
|
||
# Source common USB sample options used to initialize new experimental USB | ||
# device stack. The scope of these options is limited to USB samples in project | ||
# tree, you cannot use them in your own application. | ||
source "samples/subsys/usb/common/Kconfig.sample_usbd" | ||
|
||
source "Kconfig.zephyr" |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,113 @@ | ||
.. zephyr:code-sample:: mctp-usb-endpoint | ||
:name: MCTP USB Endpoint Node Sample | ||
:relevant-api: usbd_api | ||
|
||
Create an MCTP endpoint node using the USB device interface. | ||
|
||
Overview | ||
******** | ||
Sets up an MCTP endpoint node that listens for messages from the MCTP bus owner with EID 20. | ||
Responds with "Hello, bus owner" message. | ||
|
||
Requirements | ||
************ | ||
A board and SoC that provide support for USB device capability (either FS or HS). Testing requires | ||
a USB host that has the ability to enumerate the MCTP interface and interact with the board using | ||
the MCTP protocol. An easy way to do this is a Python script. | ||
|
||
An example script to test this interface is provided below. | ||
|
||
.. code-block:: python | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Honestly probably better to put this into scripts or into the sample directory itself as a python script with requirements.txt and all rather than pushing users to copy/paste, and figure out how to get the python deps installed. Can always include whole code files into the docs like this with literalinclude There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sounds good. I moved it to a script within the sample folder with a requirements.txt and instructions in the README. |
||
import usb.core | ||
import usb.util | ||
class USBDevice: | ||
def __init__(self, vid, pid): | ||
self.vid = vid | ||
self.pid = pid | ||
self.device = None | ||
self.endpoint_in = None | ||
self.endpoint_out = None | ||
self.interface = None | ||
def connect(self): | ||
# Find the device | ||
self.device = usb.core.find(idVendor=self.vid, idProduct=self.pid) | ||
if self.device is None: | ||
raise ValueError("Device not found") | ||
# Set the active configuration | ||
self.device.set_configuration() | ||
cfg = self.device.get_active_configuration() | ||
self.interface = cfg[(0, 0)] | ||
# Claim the interface | ||
usb.util.claim_interface(self.device, self.interface.bInterfaceNumber) | ||
# Find bulk IN and OUT endpoints | ||
for ep in self.interface: | ||
if usb.util.endpoint_direction(ep.bEndpointAddress) == usb.util.ENDPOINT_IN: | ||
self.endpoint_in = ep | ||
elif usb.util.endpoint_direction(ep.bEndpointAddress) == usb.util.ENDPOINT_OUT: | ||
self.endpoint_out = ep | ||
if not self.endpoint_in or not self.endpoint_out: | ||
raise ValueError("Bulk IN/OUT endpoints not found") | ||
def send_data(self, data): | ||
if not self.endpoint_out: | ||
raise RuntimeError("OUT endpoint not initialized") | ||
self.endpoint_out.write(data) | ||
def receive_data(self, size=64, timeout=1000): | ||
if not self.endpoint_in: | ||
raise RuntimeError("IN endpoint not initialized") | ||
return self.endpoint_in.read(size, timeout) | ||
def disconnect(self): | ||
usb.util.release_interface(self.device, self.interface.bInterfaceNumber) | ||
usb.util.dispose_resources(self.device) | ||
if __name__ == "__main__": | ||
usb_dev = USBDevice(0x2fe3, 0x0001) | ||
try: | ||
usb_dev.connect() | ||
print("Sending message with size smaller than USB FS MPS (<64b)") | ||
usb_dev.send_data(b'\x1a\xb4\x00\x0f\x01\x0a\x14\xc0\x48\x65\x6c\x6c\x6f\x2c\x20\x4d\x43\x58\x00') | ||
response = usb_dev.receive_data() | ||
print("Received:", bytes(response)) | ||
print("Sending message spanning two USB FS packets (>64b)") | ||
usb_dev.send_data(b'\x1a\xb4\x00\x4c\x01\x0a\x14\xc0\x48\x65\x6c\x6c\x6f\x2c\x20\x4d\x43\x58\x2e\x20\x4c\x65\x74\x27\x73\x20\x74\x65\x73\x74\x20\x61\x20\x6d\x75\x6c\x74\x69\x2d\x70\x61\x63\x6b\x65\x74\x20\x6d\x65\x73\x73\x61\x67\x65\x20\x74\x6f\x20\x73\x65\x65\x20\x68\x6f\x77\x20\x79\x6f\x75\x20\x68\x61\x6e\x64\x6c\x65\x20\x69\x74\x2e\x00') | ||
response = usb_dev.receive_data() | ||
print("Received:", bytes(response)) | ||
print("Sending message with two MCTP messages in a single USB FS packet") | ||
usb_dev.send_data(b'\x1a\xb4\x00\x12\x01\x0a\x14\xc0\x48\x65\x6c\x6c\x6f\x2c\x20\x4d\x43\x58\x2c\x20\x31\x00\x1a\xb4\x00\x12\x01\x0a\x14\xc0\x48\x65\x6c\x6c\x6f\x2c\x20\x4d\x43\x58\x2c\x20\x32\x00') | ||
response = usb_dev.receive_data() | ||
print("Received:", bytes(response)) | ||
print("Received:", bytes(response)) | ||
finally: | ||
usb_dev.disconnect() | ||
Wiring | ||
****** | ||
Connect a USB cable between the host (likely a PC) and the board. The type of cable (mini, micro, | ||
type C) depends on the host and board connectors. | ||
|
||
Building and Running | ||
******************** | ||
|
||
.. zephyr-app-commands:: | ||
:zephyr-app: samples/modules/pmci/mctp/usb_endpoint | ||
:host-os: unix | ||
:board: frdm_mcxn947_mcxn947_cpu0 | ||
:goals: run | ||
:compact: | ||
|
||
References | ||
********** | ||
|
||
`MCTP Base Specification 2019 <https://www.dmtf.org/sites/default/files/standards/documents/DSP0236_1.3.1.pdf>`_ |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
/* | ||
* Copyright 2025 NXP | ||
* | ||
* SPDX-License-Identifier: Apache-2.0 | ||
*/ | ||
|
||
&zephyr_udc0 { | ||
mctp_usb: mctp_usb { | ||
compatible = "zephyr,mctp-usb"; | ||
endpoint-id = <10>; | ||
}; | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
CONFIG_USB_DEVICE_STACK_NEXT=y | ||
CONFIG_USBD_MCTP_CLASS=y | ||
|
||
CONFIG_MCTP=y | ||
CONFIG_MCTP_USB=y | ||
|
||
CONFIG_LOG=y | ||
CONFIG_MCTP_LOG_LEVEL_ERR=y | ||
CONFIG_USBD_MCTP_LOG_LEVEL_ERR=y | ||
CONFIG_USBD_LOG_LEVEL_ERR=y | ||
CONFIG_UDC_DRIVER_LOG_LEVEL_ERR=y |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well no, it is specified by the "DMTF association", https://www.dmtf.org/sites/default/files/standards/documents/DSP0283_1.0.0.pdf
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's base class 0x14 in the USB spec, so I'm not sure what you mean here.
https://www.usb.org/defined-class-codes