Add MCTP over USB Binding support #95347

eric-ocasio-nxp · 2025-09-02T15:11:01Z

Add USB device binding for MCTP. Created new USB device class for MCTP, libMCTP binding, and sample application.

github-actions · 2025-09-02T15:11:47Z

Hello @eric-ocasio-nxp, and thank you very much for your first pull request to the Zephyr project!
Our Continuous Integration pipeline will execute a series of checks on your Pull Request commit messages and code, and you are expected to address any failures by updating the PR. Please take a look at our commit message guidelines to find out how to format your commit messages, and at our contribution workflow to understand how to update your Pull Request. If you haven't already, please make sure to review the project's Contributor Expectations and update (by amending and force-pushing the commits) your pull request if necessary.
If you are stuck or need help please join us on Discord and ask your question there. Additionally, you can escalate the review when applicable. 😊

sonarqubecloud · 2025-09-02T15:14:52Z

Quality Gate passed

Issues
3 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

henrikbrixandersen · 2025-09-17T06:59:44Z

doc/connectivity/usb/device_next/api/usbd_mctp.rst

@@ -0,0 +1,11 @@
+.. _usbd_mctp:
+
+MCTP USB device API


Please detail what "MCTP" actually is here.

teburd · 2025-09-17T13:41:08Z

samples/modules/pmci/mctp/usb_endpoint/CMakeLists.txt

+
+cmake_minimum_required(VERSION 3.20.0)
+find_package(Zephyr REQUIRED HINTS $ENV{ZEPHYR_BASE})
+project(mctp_endpoint)


project name could use something more descriptive, I get I'm guilty of probably starting this but perhaps mctp_usb_endpoint would be better

teburd · 2025-09-17T13:41:28Z

samples/modules/pmci/mctp/usb_endpoint/CMakeLists.txt

+project(mctp_endpoint)
+
+include(${ZEPHYR_BASE}/samples/subsys/usb/common/common.cmake)
+FILE(GLOB app_sources src/*.c)


please don't glob here

teburd · 2025-09-17T13:46:55Z

samples/modules/pmci/mctp/usb_endpoint/README.rst

+
+An example script to test this interface is provided below.
+
+.. code-block:: python


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

Sounds good. I moved it to a script within the sample folder with a requirements.txt and instructions in the README.

teburd · 2025-09-17T16:19:50Z

Non-blocking comments obviously, but would be good to see improvements.

jfischer-no · 2025-09-17T17:12:30Z

doc/connectivity/usb/device_next/api/index.rst

In order to support the USB binding for libMCTP, we must first add a device
class for MCTP. MCTP is one of the base classes in the USB specification.

Well no, it is specified by the "DMTF association", https://www.dmtf.org/sites/default/files/standards/documents/DSP0283_1.0.0.pdf

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

Please refer latest specification https://www.dmtf.org/sites/default/files/standards/documents/DSP0283_1.0.1.pdf

jfischer-no · 2025-09-17T17:14:18Z

doc/connectivity/usb/device_next/usb_device.rst

 +-----------------------------------+-------------------------+-------------------------+
 | USB Video Class (UVC)             | Video device            | :samp:`uvc_{n}`         |
 +-----------------------------------+-------------------------+-------------------------+
+| USB MCTP over USB Endpoint        | :ref:`usbd_mctp`        | :samp:`mctp_{n}`        |


s/USB MCTP over USB Endpoint/MCTP over USB Binding

jfischer-no · 2025-09-17T17:16:27Z

doc/connectivity/usb/device_next/usb_device.rst


 * :zephyr:code-sample:`usb-hid-mouse`

+* :zephyr:code-sample:`mctp-usb-endpoint`


* :zephyr:code-sample:mctp-usb-binding`` Anyway, you cannot add it in this commit, this sample does not exist yet.

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.

"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

jfischer-no · 2025-09-17T17:17:53Z

include/zephyr/usb/class/usbd_mctp.h

+
+/**
+ * @file
+ * @brief USB MCTP over USB Protocol Endpoint Device Class (MCTP) public header


@brief MCTP over USB Binding implementation

jfischer-no · 2025-09-17T17:19:39Z

include/zephyr/usb/class/usbd_mctp.h

+ * @brief USB MCTP over USB Protocol Endpoint Device Class (MCTP) public header
+ *
+ * This header describes MCTP class interactions desgined to be used by libMCTP.
+ * The MCTP device itself is modelled with devicetree zephyr,mctp-usb compatible.


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.

jfischer-no · 2025-09-17T18:08:47Z

subsys/usb/device_next/class/usbd_mctp.c

+#include <zephyr/logging/log.h>
+LOG_MODULE_REGISTER(usbd_mctp, CONFIG_USBD_MCTP_LOG_LEVEL);
+
+#define DT_DRV_COMPAT zephyr_mctp_usb


Why do you thing it should model a device and use DT for instantiation. You do not have to follow usbd_hid approach. This looks more like a backend implementation, and for that you could provide hooks and instantiation macro, please take a look at new USB DFU device implementation.

I did it this way because there exists the possibility of having multiple MCTP buses/bindings. If the application requires more than one MCTP endpoint, a USB composite device could be used with more than one instance of USBD MCTP. Is there a way to accomplish this without DT?

jfischer-no · 2025-09-17T18:11:14Z

subsys/pmci/mctp/Kconfig


+config MCTP_USB
+	bool "MCTP USB Binding"
+	depends on USB_DEVICE_STACK_NEXT


redundant, USBD_MCTP_CLASS already depends on USB_DEVICE_STACK_NEXT.

jfischer-no · 2025-09-17T18:11:46Z

include/zephyr/pmci/mctp/mctp_usb.h

+			.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,              \
+			},                                                                         \


Broken style.

jfischer-no · 2025-09-17T18:15:22Z

include/zephyr/pmci/mctp/mctp_usb.h

+ * @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)                                                            \


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?

I'm not following. This macro was created based on the other libmctp binding implementations. What do you mean by it being divided?

jfischer-no · 2025-09-17T18:18:40Z

samples/modules/pmci/mctp/usb_endpoint/Kconfig

@@ -0,0 +1,9 @@
+# Copyright (c) 2023 Nordic Semiconductor ASA


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.

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.

I am confused, is PMCI/MCTP not part of the Zephyr but an external module?

mctp is not an external module, it is part of the main manifest and there are dependencies on it already.

I am confused, is PMCI/MCTP not part of the Zephyr but an external module?

mctp is not an external module, it is part of the main manifest and there are dependencies on it already.

Anyway, the implementation cannot be part of subsys/usb, and has to go to the modules.

The fact that MCTP is listed on usb.org doesn't matter then?

@teburd ok, I'll explore this.

Regarding dependency in general, I want to clarify something since it may not be clear from this initial PR. The re-worked PR I have locally has cleaner lines of separation...

Besides the reference to the MCTP sample in usb_device.rst, the USB device class has no dependency on libmctp. Yes, libmctp uses it, but any other MCTP implementation can as well. You can remove libmctp and the device class stands on its own.

Can you post the re-worked PR? Would that come as separate PR or a force-push here?

Sure, I'll try to find some time to get it done today. The plan was for force push.

@jfischer-no

What in Zephyr depends on libmctp?

git grep libmctp.h subsys/ include/ include/zephyr/pmci/mctp/mctp_i2c_gpio_controller.h:#include <libmctp.h> include/zephyr/pmci/mctp/mctp_i2c_gpio_target.h:#include <libmctp.h> include/zephyr/pmci/mctp/mctp_uart.h:#include <libmctp.h> subsys/pmci/mctp/mctp_memory.c:#include <libmctp.h>

Even MCTP samples are only located in samples/modules.

is this your issue? really? #96995

…nding Add support for MCTP over USB by providing a new USBD class, device tree binding and sample application. The USB binding for libmctp follows existing design from other transports. The USBD class is a basic implementation, as defined in the DMTF spec, and is solely providing transmit and receive data pipes for a higher-level MCTP transport and protocol manager. The USB-specific wrapping of MCTP data is added and parsed to/from the raw USB data in the MCTP USB binding (libmctp in this case), not in the USBD class. The goal of this design is to provide flexibility for alternative MCTP implementations. Signed-off-by: Eric Ocasio <[email protected]>

Add license header to usb_host_tester.py, fix typo in usbd_mctp.h and rogue space in usbd_mctp.h. Signed-off-by: Eric Ocasio <[email protected]>

sonarqubecloud · 2025-10-04T00:48:24Z

Quality Gate passed

Issues
7 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

dleach02 · 2025-10-08T14:00:40Z

@jfischer-no, @eric-ocasio-nxp, @teburd, @nashif

Would it be helpful to move this to the arch WG call (@carlescufi) so we can collectively discuss what is needed to get this PR to move? It is unclear to me what the specific request is that will satisfy @jfischer-no change request and the back-and-forth in this PR doesn't seem to be productive?

carlescufi · 2025-10-08T15:32:15Z

@jfischer-no, @eric-ocasio-nxp, @teburd, @nashif

Would it be helpful to move this to the arch WG call (@carlescufi) so we can collectively discuss what is needed to get this PR to move? It is unclear to me what the specific request is that will satisfy @jfischer-no change request and the back-and-forth in this PR doesn't seem to be productive?

Yes, I will schedule it for next Tuesday (CC @henrikbrixandersen)

zephyrbot added area: Devicetree area: USB Universal Serial Bus area: PMCI Platform Management Communications Infrastructure area: Samples Samples labels Sep 2, 2025

zephyrbot requested review from JarmouniA, KeHIntel, decsny, galak, jfischer-no, kartben, mbolivar, nashif, rruuaanng, teburd and tmon-nordic September 2, 2025 15:12

zephyrbot assigned teburd Sep 2, 2025

henrikbrixandersen reviewed Sep 17, 2025

View reviewed changes

teburd previously approved these changes Sep 17, 2025

View reviewed changes

jfischer-no changed the title ~~Feature/add mctp usb binding~~ Add MCTP over USB Binding support Sep 17, 2025

jfischer-no requested changes Sep 17, 2025

View reviewed changes

eric-ocasio-nxp dismissed teburd’s stale review via 303df7d October 3, 2025 21:02

eric-ocasio-nxp force-pushed the feature/add_mctp_usb_binding branch from 8c07b95 to 303df7d Compare October 3, 2025 21:02

zephyrbot added area: Boards/SoCs area: Devicetree Bindings labels Oct 3, 2025

usb: pmci: mctp: samples: Fix issues from CI checks

c4093f5

Add license header to usb_host_tester.py, fix typo in usbd_mctp.h and rogue space in usbd_mctp.h. Signed-off-by: Eric Ocasio <[email protected]>

dleach02 added the Architecture Review Discussion in the Architecture WG required label Oct 8, 2025

github-project-automation bot added this to Architecture Review Oct 8, 2025

github-project-automation bot moved this to Todo in Architecture Review Oct 8, 2025


		An example script to test this interface is provided below.

		.. code-block:: python


		* :zephyr:code-sample:`usb-hid-mouse`

		* :zephyr:code-sample:`mctp-usb-endpoint`

		@@ -0,0 +1,9 @@
		# Copyright (c) 2023 Nordic Semiconductor ASA

Add MCTP over USB Binding support #95347

Are you sure you want to change the base?

Add MCTP over USB Binding support #95347

Conversation

eric-ocasio-nxp commented Sep 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Sep 2, 2025

Uh oh!

sonarqubecloud bot commented Sep 2, 2025

Quality Gate passed

Uh oh!

Choose a reason for hiding this comment

Uh oh!

teburd Sep 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

teburd commented Sep 17, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nashif Oct 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented Oct 4, 2025

Quality Gate passed

eric-ocasio-nxp commented Sep 2, 2025 •

edited

Loading

teburd Sep 17, 2025 •

edited

Loading

nashif Oct 3, 2025 •

edited

Loading