USB: Replace OHCI USB host library #438

Ryzee119 · 2021-03-24T10:21:01Z

This PR replaces the legacy 'hacked' together linux usb stack currently in nxdk with a new library.

~~Note: CI will fail on the sdl samples because I havent update the hal/input.c code yet~~ It should work now, but I have submoduled in own SDL2 fork. XboxDev/nxdk-sdl#38 will need to be merged and SDL2 submodule update prior to merge.

This library was developed by OpenNuvoton (https://github.com/OpenNuvoton) who are a large manufacturer of ARM based embedded microcontrollers. This library was intended for use on their range of chips with OHCI hardware. I specifically imported it from https://github.com/OpenNuvoton/N9H30_emWin_NonOS/tree/master/Library/UsbHostLib with some modifications to make it xbox friendly. The largest change is the separation between virtual and physical memory which is not really a concept on microcontrollers.

The library has some advantages over the current implementation:

A more permissive license (Apache 2.0)
A full range of inbuilt USB class drivers. I have tested a range of these on my personal repo https://github.com/Ryzee119/nxdk_ohci_test. These samples are just quick hacks for testing. Not considered 'production ready'.
Proper OHCI interrupt management so that USB hot-plugging should work properly.
The XID driver can properly distinguish between various Xinput devices (Duke, Steel Battalion, Xremote)
Proper interrupt out writes, so rumble should work with no problem.
Some very useful features for developers is support for CDC class USB-Serial adaptors for improved 'printf debugging'.

**I have thoroughly tested this on my own repo projects. Some outstanding aspects: **

~~Untested on 1.0 xboxes with daughterboard. I would expect this to have no issues though.~~ Has been tested ok!
~~PR does not include changes to hal/input.c, consequently sdl2 joystick/gamecontroller will break. I am happy to do this work but would like some feedback on the preferred APIs going forward.~~ sdl_joystick driver rewritten. All old features plus more. (Rumble, hotplugging etc)
~~Currently the ohci library is included as a submodule to my personal repo. This should either be migrated here or commited directly into nxdk (or it can stay linked to my repo, I dont mind)~~ Its here https://github.com/XboxDev/libusbohci.
The mass storage class driver has been tested, but disabled in this PR due to the fact it is hardcoded to use FATFS. I think this requirement should be removed to allow other filesystem types (FATX etc). I can do this in a follow up PR. FATFS code removed. This class driver just supports standard SCSI needed for MSC. User filesystem drivers need to be linked in.

dracc · 2021-03-24T10:36:45Z

Amazing! 🎉
I'll test this tonight.

samples/usb/main.c

Teufelchen1 · 2021-03-24T11:33:01Z

Misses adjustment of the README.md

samples/usb/main.c

lib/usb/Makefile

Ryzee119 · 2021-03-27T08:59:04Z

Ive made a sdl_joystick driver using this new stack. It does not use hal/input.c at all and instead directly calls the usb HID APIs, so hal/input.c can be completely removed.

I've tested this on SDLPopX, OpenTyrian, Commander keen, Xenium-Tools, sdl_gamecontroller sample and NevolutionX
https://github.com/Ryzee119/nxdk-sdl/tree/usbh

It also supports native Xbox360, Xbone controllers too!

To test replace your lib/sdl/SDL2 with this fork.

cd lib/sdl/SDL2
git remote add ryzee https://github.com/Ryzee119/nxdk-sdl.git
git checkout ryzee usbh

Teufelchen1 · 2021-04-15T11:45:38Z

samples/sdl_gamecontroller/main.c

@@ -97,4 +121,4 @@ int main(void)
  }

  return 0;
-}
+}


Misses Newline 😏

JayFoxRox · 2021-04-15T18:03:07Z

samples/usb/main.c

+*/
+void device_connection_callback(UDEV_T *udev, int status)
+{
+    debugPrint("Device connected on port %u (PID: %04x VID: %04x)\n", udev->port_num,


What do these ports look like? If they are USB ports and not gamepad ports, then that's relevant for the message, in my opinion.

They are the number of the USB hub port. I have updated the wording here.

JayFoxRox · 2021-04-15T18:04:14Z

samples/usb/main.c

+
+    usbh_core_init();
+    usbh_install_conn_callback(&device_connection_callback, &device_disconnect_callback);
+    debugPrint("Plug and unplug USB devices to test\n");


This should be a bit more explicit.
People might assume that this will test their gamepad like that XDK sample that everybody loves (with the 3D gamepad).
This, however, merely seems to show plug and unplug events?

debugPrint("Plug and unplug USB devices to print USB device VID and PID\n");

updated the debug print here to make it clearer what it is doing

JayFoxRox · 2021-04-15T18:06:59Z

samples/usb/main.c

+
+void device_disconnect_callback(UDEV_T *udev, int status)
+{
+     debugPrint("Device disconnected on port %u (PID: %04x VID: %04x)\n", udev->port_num,


nit: I think I might prefer order 1. VID, 2. PID; but I'm not sure what other common USB tools use. (Idea is to visually group them by manufacturer first.. so that should come first)

Makes sense to me. Updated this

JayFoxRox · 2021-04-15T18:09:01Z

samples/usb/Makefile

@@ -0,0 +1,5 @@
+XBE_TITLE = nxdk\ sample\ -\ usb


nit: I think just calling this sample "usb" is not ideal; but I have trouble coming up with my own name.

Just to clarify, I mean both: XBE title, but more importantly, pathname

(I'd assume there'll be different USB samples eventually, at least some of which will likely deserve a "usb-" prefix)

Perhaps call it hotplugging?

Agree that it needs a better name. This sample really just shows the absolute basic core init so it atleast builds during ci tests. People might get drawn into it for gamecontroller stuff when they should be using SDL_gamecontroller.
I am working on an 'all-in-one` usb sample. https://github.com/Ryzee119/nxdk_ohci_test/blob/xbox/main.c, advantage being it includes all source files in ci runs to catch issues better.

JayFoxRox · 2021-04-15T18:12:57Z

lib/usb/Makefile

+
+.PHONY: clean-usb
+clean-usb:
+	$(VE)rm -f $(USBH_OBJS) $(NXDK_DIR)/lib/libnxdk_usb.lib


I think this Makefile should be part of the USB submodule, unless that submodule remains platform unspecific (which, to be honest, sounds like a good idea to me; but only if we can attract other stakeholders / convince upstream to submodule it)

Other maintainers might have different thoughts on this matter though.

JayFoxRox · 2021-04-15T18:16:27Z

.gitmodules

@@ -31,3 +31,6 @@
 [submodule "lib/sdl/SDL2_image"]
 	path = lib/sdl/SDL2_image
 	url = https://github.com/libsdl-org/SDL_image.git
+[submodule "lib/usb/ohci_usbhostlib"]
+	path = lib/usb/ohci_usbhostlib
+	url = https://github.com/Ryzee119/ohci_usbhostlib.git


Where to discuss Ryzee119/libusbohci@master...xbox?

What's Ryzee119/libusbohci@xbox...timing?

What are your plans for https://github.com/Ryzee119/nxdk_ohci_test/tree/master?

I created a self PR, happy to discuss stack specifics there. Xbox port: Ryzee119/libusbohci#2

Timing branch was a initial attempt to minimise/remove some of the blocking delays in the stack. Have not spent any time on this yet. afair the whole stack is non-block except during device connection/removal/suspend stuff

Master branch is/was intended to be a clean copy of upsteam to track diff.

I created a self PR, happy to discuss stack specifics there. Ryzee119/libusbohci#1

Please add this information to the PR description - I assume it could be more important than the discussion on this repository.

What are your plans for https://github.com/Ryzee119/nxdk_ohci_test/tree/master?

Master branch is/was intended to be a clean copy of upsteam to track diff.

I should probably have made a separate comment, but I was asking about "nxdk_ohci_test" and not "ohci_usbhostlib"; trying to get an idea of the existing ecosystem.

What are your plans for https://github.com/Ryzee119/nxdk_ohci_test/tree/master?

I think it could make a good nxdk sample if/when i clean it up to it suitable quality. Probably something I will do in the near future. See the 'xbox' branch for something a bit better quality.

espes · 2021-05-11T14:01:32Z

This is fantastic!

Currently the ohci library is included as a submodule to my personal repo. This should either be migrated here or commited directly into nxdk (or it can stay linked to my repo, I dont mind)

I've invited you to the xboxdev org if you want to move it

JayFoxRox · 2021-05-26T17:45:23Z

This PR was merged in a bad state - XboxDev/nxdk-sdl@60ff410 is not part of the XboxDev git repo. GitHub only finds it because it's part of the same fork tree.

Teufelchen1 suggested changes Mar 24, 2021

View reviewed changes

samples/usb/main.c Outdated Show resolved Hide resolved

Teufelchen1 reviewed Mar 24, 2021

View reviewed changes

samples/usb/main.c Outdated Show resolved Hide resolved

Ryzee119 force-pushed the usbh branch from 3f420c0 to 2fa0114 Compare March 27, 2021 07:56

Teufelchen1 suggested changes Mar 27, 2021

View reviewed changes

lib/usb/Makefile Outdated Show resolved Hide resolved

Ryzee119 force-pushed the usbh branch from 2fa0114 to 9177984 Compare March 27, 2021 08:14

Ryzee119 force-pushed the usbh branch 3 times, most recently from fdaf570 to 6720601 Compare March 31, 2021 09:28

Ryzee119 force-pushed the usbh branch 4 times, most recently from f7d93e5 to 72fd41d Compare April 10, 2021 00:44

Ryzee119 mentioned this pull request Apr 10, 2021

joystick: Rewrite driver for replacement USB stack XboxDev/nxdk-sdl#38

Merged

Ryzee119 marked this pull request as ready for review April 10, 2021 01:08

Ryzee119 force-pushed the usbh branch 2 times, most recently from 6edbfaa to 54d25d8 Compare April 10, 2021 02:23

dracc mentioned this pull request Apr 10, 2021

fails to launch with no controller in port 1 MakeMHz/xbox-hdmi-app#9

Open

Ryzee119 force-pushed the usbh branch 3 times, most recently from ecd3610 to ab3254b Compare April 15, 2021 10:36

Teufelchen1 reviewed Apr 15, 2021

View reviewed changes

JayFoxRox reviewed Apr 15, 2021

View reviewed changes

Ryzee119 force-pushed the usbh branch from e7386f1 to adf2b71 Compare April 19, 2021 09:16

Ryzee119 force-pushed the usbh branch 8 times, most recently from e5da652 to 87a4962 Compare May 3, 2021 09:45

Ryzee119 force-pushed the usbh branch from 87a4962 to 631015c Compare May 4, 2021 07:14

Ryzee119 force-pushed the usbh branch 2 times, most recently from eaabadd to f835fe6 Compare May 19, 2021 04:22

usb: Remove legacy library

0e974f0

Ryzee119 force-pushed the usbh branch from f835fe6 to 985bbef Compare May 25, 2021 09:11

Ryzee119 added 9 commits May 25, 2021 19:17

usb: Add new OHCI USB lib submodule

c953eb3

usb: Add Makefile

e2d7898

usb: Add Xbox specific setup

1888394

usb: Add initial xid driver

42f6ab2

usb: Add basic USB sample

92c8794

input: Remove legacy input code

95749c3

samples: Add rumble and hotplugging to SDL gamecontroller sample

72c68e1

samples: Remove unneeded input header

cff9d44

sdl2: Update submodule to 60ff410

8f1b170

Ryzee119 force-pushed the usbh branch from 985bbef to 8f1b170 Compare May 25, 2021 09:48

espes merged commit dd0337c into XboxDev:master May 26, 2021

Ryzee119 deleted the usbh branch May 27, 2021 05:54

thrimbor mentioned this pull request Dec 16, 2021

Kernel assertion: USB code allocates memory region with size of 0 bytes #121

Closed

thrimbor mentioned this pull request Mar 14, 2022

hal: Remove leftover pad.h #561

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

USB: Replace OHCI USB host library #438

USB: Replace OHCI USB host library #438

Ryzee119 commented Mar 24, 2021 •

edited

Loading

dracc commented Mar 24, 2021

Teufelchen1 commented Mar 24, 2021

Ryzee119 commented Mar 27, 2021 •

edited

Loading

Teufelchen1 Apr 15, 2021

JayFoxRox Apr 15, 2021

Ryzee119 Apr 19, 2021

JayFoxRox Apr 15, 2021 •

edited

Loading

Ryzee119 Apr 19, 2021

JayFoxRox Apr 15, 2021

Ryzee119 Apr 19, 2021

JayFoxRox Apr 15, 2021 •

edited

Loading

kosmas12 Apr 15, 2021

Ryzee119 Apr 16, 2021 •

edited

Loading

JayFoxRox Apr 15, 2021

JayFoxRox Apr 15, 2021

Ryzee119 Apr 16, 2021 •

edited

Loading

JayFoxRox Apr 16, 2021

Ryzee119 Apr 19, 2021 •

edited

Loading

espes commented May 11, 2021

JayFoxRox commented May 26, 2021

USB: Replace OHCI USB host library #438

USB: Replace OHCI USB host library #438

Conversation

Ryzee119 commented Mar 24, 2021 • edited Loading

dracc commented Mar 24, 2021

Teufelchen1 commented Mar 24, 2021

Ryzee119 commented Mar 27, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JayFoxRox Apr 15, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JayFoxRox Apr 15, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Ryzee119 Apr 16, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Ryzee119 Apr 16, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Ryzee119 Apr 19, 2021 • edited Loading

Choose a reason for hiding this comment

espes commented May 11, 2021

JayFoxRox commented May 26, 2021

Ryzee119 commented Mar 24, 2021 •

edited

Loading

Ryzee119 commented Mar 27, 2021 •

edited

Loading

JayFoxRox Apr 15, 2021 •

edited

Loading

JayFoxRox Apr 15, 2021 •

edited

Loading

Ryzee119 Apr 16, 2021 •

edited

Loading

Ryzee119 Apr 16, 2021 •

edited

Loading

Ryzee119 Apr 19, 2021 •

edited

Loading