Add USB CDC-ECM Host support#18815
Conversation
dlizewski
requested review from
jerpelea,
johannes-nivus and
yamt
as code owners
github-actions
Bot
added
Size: XL
|
Wow, very cool, thank you @dlizewski, and congratulations on your first contribution to the NuttX RTOS :-) Could you please provide also a documentation for the new functionality? :-) Pozdrawiam :-) |
|
Builds fine for me, but I have no board to test, tried on NUCLEO-H755ZI but I had problem flashing :-P |
cederom
left a comment
cederom
left a comment
There was a problem hiding this comment.
Thank you @dlizewski :-)
Please add documentation in a free moment :-)
| uerr("ERROR: Transfer failed: %d\n", nbytes); | ||
| } | ||
|
|
||
| delay = MSEC2TICK(30); |
There was a problem hiding this comment.
This is following a pattern was taken from other drivers like the ACM and MBIM.
The idea is that normally we should not trigger a delay if we get data, but if for example the device we are talking to gets into a bad state or there are uncontrolled bus errors, then without a delay this might turn into a tight poll loop of error -> callback -> work -> error
| * this case. | ||
| */ | ||
|
|
||
| delay = USBHOST_CDCECM_NTDELAY; |
|
|
||
| bool registered; /* true if the device was registered */ | ||
| bool bifup; /* true:ifup false:ifdown */ | ||
| struct net_driver_s netdev; /* Interface understood by the network */ |
There was a problem hiding this comment.
why not use netdev upperhalf to simplify the code
There was a problem hiding this comment.
I based this off of nuttx_latest/nuttx/drivers/usbhost/usbhost_cdcmbim.c so maybe I inherited some bad patterns.
Can you please elaborate a little more on what you mean by that?
There was a problem hiding this comment.
here is an simple introduction doc: https://nuttx.apache.org/docs/latest/components/net/netdriver.html.
and code: https://github.com/apache/nuttx/blob/master/drivers/net/netdev_upperhalf.c
the idea is simple keeping the common code in upperhalf driver, so the lower half can focus on the hardware.
https://github.com/apache/nuttx/blob/master/drivers/net/e1000.c is an example.
| FAR const struct usb_devdesc_s *devdesc, | ||
| FAR const struct usbhost_id_s *id) | ||
| { | ||
| if (id->vid == 0x0bda && id->pid == 0x8153) |
There was a problem hiding this comment.
should we add a callback to usbhost_registry_s or usbhost_class_s to select
There was a problem hiding this comment.
The problem is that the configuration needs to be selected before the host drivers are called.
Right now the code is:
set config X
for each host driver {
if(host matches){
break
}
}
The other approach I tried is to cycle the configs until we find a host driver that matches.
Something like:
for each config c {
set config c
for each host driver {
if(host matches){
goto exit
}
}
}
But I ran into some problems with certain host class drivers failing the 2nd time it was iniitalized for a given device. Likely a bug that could be solved if we think that is a better approach.
The other thing to note is that I dont really need this configuration selection. I added CDC-ECM since certain modems I need support ECM. On the modems they are configuration 0.
I added this configuration selection because the CDC-ECM USB-Ethernet adapters that I have present ECM as the 2nd interface and I wanted to provide testing logs using a standard off the shelf product.
But if we can accept logs from a custom board with modems (or a build where I temporarily hard-code it to 1 instead of 0) then I could revert the configuration selection portion until it is actually needed.
Added support for selecting a different USB configuration. Certain USB devices offer different classes using different configurations. This allows a board file to provide a callback to select the proper configuration for a given USB device. Signed-off-by: daniellizewski <daniellizewski@geotab.com>
Added support for USB host to use an USB CDC-ECM device. This class is used for usb-ethernet adapters as well as many modems. Signed-off-by: daniellizewski <daniellizewski@geotab.com>
dlizewski
force-pushed
the
feature/dl.usbecm
branch
from
38866d8 to
40ea04c
Compare
github-actions
Bot
added
Area: Documentation
Added docs for the new config and for CDC-ECM host driver |
5 participants
Summary
nucleo-h563zi:nshusbnet.Impact
Testing
nucleo-h563zi:nshusbnetconfiguration and connected the USB-Ethernet adaptor nucleo-h563zi:nshusbnet (A Realtek RTL8153 Gigabit Ethernet Adaptor with integrated hub). I then enabled the interface, set DHCP, verified it got an IP, then pinged 8.8.8.8 with no packet loss.Commands run:
ecm_log.txt