nxp-mcuxpresso / mcux-sdk

MCUXpresso SDK
BSD 3-Clause "New" or "Revised" License
301 stars 136 forks source link

Include USB RNDIS Ethernet Driver code so we can contribute enhancements #89

Closed tgibson-work closed 1 year ago

tgibson-work commented 1 year ago

Is your enhancement proposal related to a problem? Please describe. At the moment the USB RNDIS Ethernet driver code has some bugs in it and is missing support for e.g. multiple packets per transfer which some devices presume will be supported.

Describe the solution you'd like We have internally fixed some of these issues, but it would be better if we could contribute this code back to the SDK and not have to apply our patch to each release. Please incorporate the USB RNDIS Ethernet Driver code into the GitHub SDK repository so this is possible.

Describe alternatives you've considered N/A

Additional context N/A

mcuxsusan commented 1 year ago

Hi @tgibson-work, may you please check the repository https://github.com/NXPmicro/mcux-sdk-middleware-usb? For NXP GitHub SDK we adopted a multi-repo framework and each middleware is located in their repository. Using zephyr west tool to get https://github.com/NXPmicro/mcux-sdk/ you could get all SDK deliveries.

tgibson-work commented 1 year ago

Hi @mcuxsusan - thanks for the heads-up, I will check out zephyr west.

However, I have just had a look at the middleware-usb repository and it doesn't include the usb_ethernetif_freertos.c and usb_ethernetif.h files we have modified which are under the ./lwip/port subfolder in ZIP SDKs (I presume there's a usb_ethernetif_bm.c or similar file somewhere as well but we don't use BM at the moment).

I will get all the repositories with zephyr west and see if the files are in another one, but perhaps you might know another repository I should check also? Thanks!

mcuxsusan commented 1 year ago

Hi @tgibson-work, thanks for the detailed information. Sorry that not all available middleware in MCUX SDK is ready in GitHub now, for the lwip middleware repository which may include the file you could like to contribute is currently not available, the plan is in next month. I will update you once the lwip repository is available.

By the way, for all available middleware list please check the west manifest file: https://github.com/NXPmicro/mcux-sdk/blob/main/west.yml

tgibson-work commented 1 year ago

@mcuxsusan no problem, great to know the lwip repository is planned for release soon!

mcuxsusan commented 1 year ago

Hello @tgibson-work, the NXP lwip repository has been launched! And the porting files you are expecting is located under https://github.com/NXPmicro/lwip/tree/mcux_release_2.12.0/port. You could use west tool to do update on our MCUX_2.12.0_UPDATE tag in this repository to checkout the lwip repository.

But for contributions to this lwip repository, please wait notification from @stanislav-poboril who is the repo maintainer. Thanks in advance.

tgibson-work commented 1 year ago

@mcuxsusan that's great news, thanks for the update!

stanislav-poboril commented 1 year ago

Hello @tgibson-work, the NXP lwip repository has been recreated as a fork of the official project before a moment. You may direct your pull request into mcux_release_2.12.0 branch.

tgibson-work commented 1 year ago

You could use west tool to do update on our MCUX_2.12.0_UPDATE tag in this repository to checkout the lwip repository.

What is the correct procedure for doing this? The mcux-sdk repository documentation covers checking out a new repository to a specified tag, but not how to change the tag and update. How do I do this? I have read through the west documentation and it does not seem to explain how to get a new version of the manifest either.

stanislav-poboril commented 1 year ago

@tgibson-work, you can navigate to the manifest repository (core subfolder of the west workspace) and checkout the desired tag, sha or branch using regular git commands. This will update the manifest file core/west.yml which is referenced from the west workspace (configured in the file .west/config). The command west update will update all references according to the manifest file then. @mcuxsusan, maybe there is a better way using the west tool only?

tgibson-work commented 1 year ago

Thanks, I'll give that a go.