Skip to content

Conversation

pamaury
Copy link

@pamaury pamaury commented Oct 14, 2025

This PR creates a new chardev for the USBDEV block over which the driver can communicate with a virtual USB host. The protocol (still under development) is documented and implemented. At the moment, only bus events (vbus handling, connection, reset, stub suspend/resume) are implemented.

In order to test this implementation, a virtual USB host speaking the protocol must be used. I am currently testing this using a custom python script but I think it is too hacky to upstream at the moment.

See lowRISC/opentitan#28487 for the OpenTitan side

Comment on lines 1102 to 1083
ot_usbdev_server_write_packet(s, OT_USBDEV_SERVER_CMD_HELLO,
server->recv_pkt.id, &resp_hello,
sizeof(resp_hello));

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you not just send back the same hello packet now that you have checked its integrity, or am I missing something?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could but that feels like premature optimization, if the hello packet changed, it could become asymmetrical (e.g. different server and client versions).

@pamaury pamaury force-pushed the usbdev2 branch 3 times, most recently from 5fcbdcf to bdc6ba3 Compare October 19, 2025 07:27
Copy link

@AlexJones0 AlexJones0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks @pamaury!

@pamaury
Copy link
Author

pamaury commented Oct 20, 2025

Once this passes CI, can this be merged or are there any big blockers left? The code will still be modified quite a lot to add support for data transfers but I am relunctant to base too much work on this PR until it's merged.

@rivos-eblot
Copy link

Once this passes CI, can this be merged or are there any big blockers left? The code will still be modified quite a lot to add support for data transfers but I am relunctant to base too much work on this PR until it's merged.

There are a couple of remaining comments not closed (in the GitHub "hidden" conversation sections), and a couple of remaining details, mostly:

  • signed vs. unsigned values (enumerated <-> uint32_t) assignment without explicit casts
  • missing unsigned suffixes (0 -> 0u)
  • cast to (const) void* where the variable or the argument is a (const) uint8_t*

@pamaury pamaury force-pushed the usbdev2 branch 2 times, most recently from bcdf022 to 57d9ec5 Compare October 20, 2025 12:25
@pamaury
Copy link
Author

pamaury commented Oct 20, 2025

I hope I caught everything this time. I may have missed some void * casts, those are really annoying to find manually.

Copy link

@jwnrt jwnrt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(I only reviewed the protocol)

This commit creates a new chardev for the USBDEV block over which the driver
can communicate with a virtual USB host. The protocol (still under development)
is documented and implemented. At the moment, only bus events (vbus handling,
connection, reset, stub suspend/resume) are implemented.

Signed-off-by: Amaury Pouly <amaury.pouly@lowrisc.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants