-
Notifications
You must be signed in to change notification settings - Fork 8.1k
add CP210x USB class implementation #97648
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
Open
matsievskiysv
wants to merge
2
commits into
zephyrproject-rtos:main
Choose a base branch
from
matsievskiysv:cp210x-class
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
# Copyright (c) 2025 Sergey Matsievskiy | ||
# SPDX-License-Identifier: Apache-2.0 | ||
|
||
description: USB CP210X UART | ||
|
||
compatible: "zephyr,cp210x-uart" | ||
|
||
include: uart-controller.yaml | ||
|
||
on-bus: usb | ||
|
||
properties: | ||
tx-fifo-size: | ||
type: int | ||
default: 1024 | ||
description: | | ||
Size of the virtual CP210X UART TX FIFO | ||
|
||
rx-fifo-size: | ||
type: int | ||
default: 1024 | ||
description: | | ||
Size of the virtual CP210X UART RX FIFO | ||
|
||
label: | ||
description: | | ||
The string defined by the label property is also used for the USB | ||
interface string descriptor. |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
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.
If the goal is to not require interrupt endpoint, then why you don't make the CDC ACM interrupt endpoint optional?Universal Serial Bus Class Definitions for Communications Devices Revision 1.2 (Errata 1)
3.4.1 Communications Class Interface
3.5.1 Communications Class Endpoint Requirements
Therefore it is not mandatory for CDC ACM device to use interrupt endpoint.Working on making CDC ACM interrupt endpoint optional, and checking interoperability issues with different hosts would IMHO be more fruitful than abusing cp210x drivers.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.
Linux's cdc-acm driver is a bit hard for me to understand, but it looks like the interrupt endpoint is mandatory. However, in some cases the driver does look for the endpoints in the alternate configurations, which is worth checking out.
I wouldn't call it abuse. More of an emulation=)
Uh oh!
There was an error while loading. Please reload this page.
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.
Linux CDC ACM driver is just an implementation, not the specification. The specification states that notification element is optional. If what you want to achieve is to have Zephyr application having something that shows as tty under Linux, and consumes two endpoints per instance, then I would suggest:make the notification endpoint optional in Zephyr CDC ACM implementationsend PATCH to linux kernel to not require interrupt notification endpointI see no reason why either of the above would be rejected. I am fairly confident that the interrupt endpoint is optional. If it is not, then please point me to relevant section in https://www.usb.org/document-library/class-definitions-communication-devices-12There 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.
Turns out I was wrong, 3.2.2 Abstract Control Model
So while CDC itself does not require notification element, ACM does require it. :(
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.
However you name it, doing so will lead to a device that cannot pass USB certification.
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.
Looking closely, this line only handles cases where three endpoints belong to the same interface. It does not reduce the number of the required endpoints.
The company's PID/VID binding to the appropriate driver may be configured via udev. Are there any other certification blockers?
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.
You've literally left the name of a vendor's device in here of whom you are entirely cloning, a company I would add that is a zephyr member, that is clearly not acceptable
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.
@jhedberg for comment
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.
What do you mean "cloning"? I didn't use their IP or patent or anything. I've made a driver compatible with linux's cp210x driver and named it after that. There're 203 devices from different vendors in linux's PID-VID compatibility list for cp210x. Are they all cloning the original device? Quick search did not yield any restrictions on USB protocol emulation.
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.
I believe the 203 devices just use the actual chip in them with their own VID/PID (so these can be certified).