-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Add support for Allwinner F1C100s family #1220
Conversation
ARMCC also sets __GNU__ macro, but doesn't support GCC diagnostic pragmas.
Allwinner F1Cx00s family is a series of SoCs with Mentor MUSB controller and HS phy. It comes with a slightly different register space layout, and some quirks, so it's not multiplexed with the existing musb support library. This library currently requires to be compiled with https://github.com/hongxuyao/F1C100s_with_Keil_RTX4_emWin5
Superb !! Thank you very muhc, this is great PR. This would be great option to run TinyUSB on another popular SOC besides rpi broadcom. As required for maintaining the driver, could you please:
PS: I am ok with Allwinner F1C100s with is own musb driver for now. The existing musb is also newly added. We could later on merge them together in the future (much like synopsys dwc2 driver). |
Hi hathach, Thanks very much for your response! I'm indeed testing with a Lichee Pi Nano, but it's more convenient to use a Widora Tiny200 v2 (also called MangoPi-R3c). MangoPi-R3c have two USB ports for OTG and UART and comes with dedicated RESET and BOOT button, which brings a lot convenience to development. Now a basic example is provided here: https://github.com/t123yh/F1C100s_RTX4_USB, which requires Keil to be compiled. I'll try to port it to the BSPs in TinyMCU later. I forgot to mention that there's currently a bug in the current code: TX and RX cannot share an Endpoint Number. I don't know whether it's a hardware limitation or software bug, though from existing examples the hardware likely supports sharing Endpoint Numbers. This needs to be digged deeper. |
@t123yh thank you for the tip, I just placed an order for the MangoPi R3 from aliexpress instead of Lichee Pi nano. It probably takes a couple of weeks for it to deliver. Meanwhile we could try to get familiar with the chip and the PR finalized 👍 👍 |
BTW, currently I have only tried Keil RTX on F1C100s, but I believe RT-Thread will also work. I'll try later. |
BSP support is now added. No RTOS is required. Please see README.md for details. Currently tested with |
The datasheet says 2KB FIFO, but accroding to many code examples, the F1C100s has at least 4KB of FIFO memory. This is working with cdc_msc example, but I'm not sure, this should be checked.
After fixing a stupid mistake, the cdc_msc example is now working. |
Just played with the msc. It seems that there're a few STALLs before the disk is properly detected. Seems that the cause is the code below. I'm not familiar with msc, so I don't know what's happening. if ( p_msc->stage == MSC_STAGE_STATUS )
{
// skip status if epin is currently stalled, will do it when received Clear Stall request
if ( !usbd_edpt_stalled(rhport, p_msc->ep_in) )
{
if ( (p_cbw->total_bytes > p_msc->xferred_len) && is_data_in(p_cbw->dir) )
{
// 6.7 The 13 Cases: case 5 (Hi > Di): STALL before status
// TU_LOG(MSC_DEBUG, " SCSI case 5 (Hi > Di): %lu > %lu\r\n", p_cbw->total_bytes, p_msc->xferred_len);
usbd_edpt_stall(rhport, p_msc->ep_in);
}else
{
TU_ASSERT( send_csw(rhport, p_msc) );
}
} Also I tried to do a cdc benchmark with dd (loopback code commented out). The result is not very impressive (as a High Speed device). Also, after repeating a few times of benchmark, the device just dies. No further benchmark can be made.
|
Superb !! That is great, too bad, my tiny200 haven't shipped to me just yet for trying this out. This will make things super easy to support and maintain F1C in the future.
MSC device may need to stall unsupported command and request from host. It is fairly typical, and required to pass USB compliance test for MSC device #1059 . We may have issue if we haven't fully implement stall/clear stall properly for this port, which is very typical for new chip.
It is new port and may need some tweak, I have tested fullspeed device with cdc that could reach 80-90% of the bandwidth. I am pretty sure it is not the stack issue, we just need to tune thing up E.g you could try to increase the It is a great start, I am looking forward to trying this out. |
Now there's one thing that's to be solved: if we want to build a flashable bin file to run on SPI Flash, we must use mksunxi tool to process the output bin file. I'm not sure where can I insert this into our build flow, could you please help me? |
@t123yh in general, tool should be installed separately by user and added to PATH similarly to gcc toolchain or openocd. You could also make it as makefile variable to pass through command line e.g we can put it in the board/family readme doc.
if user need to, it can be passed to makefile as |
I've written a Python version to avoid the need of installing mksunxi. Now it will build a flashable image that you can write to SPI. README.md is updated to reflect the changes. |
great works, do you have a need to merge it asap ? my board from aliexpress could take 2 more weeks to arrive. I would like to have hand-on testing and could make some cosmetic changes if needed. But we could merge it earlier if this blocks your work. |
No need, it's better to merge after you have tested the whole thing. I don't think it's currently stable enough to be merged. I can just work on my own repo before merging. BTW, after fixing a bug (commit 527036b) that causes MMU and cache not initialized when executing from FEL, serial performance test reaches 15.7MB/s, which is a lot better than previous tests. |
If this adjustments are applied, after the step 24 of @hathach If this is not acceptable, I would consider whether this condition can be avoided by adding a new flag. |
@kkitayam the diagram is super useful, though I haven't tried to look closely at musb so far. Please give me a bit of time to catch up with this. |
Hi hathach, Just a friendly reminder on this issue. Could you please take some time to figure this out? This has been a blocking bug for me for a while. BTW, have you received your Tiny200 board? Thanks! |
@t123yh thanks for the reminder, I did indeed receive tiny200 board a few days ago. I have been busy with other works. Please give me a bit more time to get it setup and trying this out. |
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.
PR looks great, thank you very much for your effort. I only have one question regarding the keil warning suppression of "statement is unreachable"
src/device/usbd.c
Outdated
@@ -44,6 +44,11 @@ | |||
#define CFG_TUD_TASK_QUEUE_SZ 16 | |||
#endif | |||
|
|||
#ifdef __ARMCC_VERSION | |||
// Supress "statement is unreachable" warning | |||
#pragma diag_suppress 111 |
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 don't have keil to try this out. Maybe it catches an actual valid warnings that worth fixing. Would you tell more on this, which specific line that this
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.
The warnings are for the switch(desc_type)
inside process_get_descriptor
in which there're several break;
that is unreachable because are return
statements in both if
and else
block. Line 1016, 1033, 1059, 1072, 1086.
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.
thank you I see, we could just remove the break;
they are mostly are there for case/break pair.
PS: just push an update to fix the warning. Let me know if that would fix your compiling with keil
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 have tried it on my tiny200, and it works great. Thank you very much for the excellent PR. I did a bit of cosmetic change e.g adding license term to dcd file, move hw/mcu/allwinner to its own submodule etc.. Once the ci is passed, we could merge this
Hi @hathach , Thank you for testing. The bug mentioned above still exists, and I believe it needs to be resolved before merging. I have an example to reproduce this issue. I'll upload it to a branch soon. |
@t123yh no problem at all, I am downloading keil trial version on my VM windows, will try it out if you have any ready-to-build example |
No you don't have to download keil. You can run my example totally within the tinyusb example framework. The code to reproduce the problem is based on gud-pico which is part of the gud project. You need to have RPi with the latest RPi OS (which backported gud to 5.10 kernel) or PC with Arch Linux to run my example. |
@hathach The example is ready. You can downloaded it from gud-test branch. This is a dirty quick version based on earlier version of my port, but this shouldn't matter. It has my workaround commit for the problem discussed above with kkitayam applied. You have to prepare a Linux host running kernel with gud kernel module. Latest RPi OS have this module backported. To test the example, you can connect the USB device port to the host Linux device, and the USB-UART port to your serial monitor on PC (note that the serial port of F1C100s is changed to UART1 to adapt to Tiny200v2), then compile and flash the gud code just like other examples. If you run my code directly, it should be recognized immediately on RPi as a USB screen without out any errors (although it's sorry that you probably doesn't have a screen on your Tiny200v2, but it shouldn't matter), with the following logs on your serial monitor:
Note the Please tell me if you can reproduce this problem. |
Thank you for the detail info, I will grab an Pi4 and try to reproduce and look into this issue. To be honest, I haven't read the musb specs/code yet, thankfully, kkitayam already does an great job illustrated it. |
musb doc can be found by Google |
yeah, I mean I haven't got time to really look at musb yet. So far all the great work is done by @kkitayam :) |
@t123yh sorry I am lacking behind PRs/issues. I still haven't managed time to look at musb and test the above problem just yet. This PR is already pending for a while and is relatively large. I think we should merge this as it is for now, since it has good running example already. And then do another PR to fix the problem, we could create an github ticket/issue for discussion. That would also minimize effort for other contributors to fix the issue as well. What do you say ? |
Yes, I agree with you. Thanks for your time and effort. |
Thanks, I wish I could be more involved with this port, but the Lunar New Year is coming and there is lots of other works/project. I will look at this later on when I have time. Thank you @t123yh for your contribution. I will make an new issue as reminder for us after the merge |
I totally understand this as a Chinese. Happy New Year! |
thanks, Happy New Year to you as well !!! 🎉 |
Allwinner F1Cx00s family is a series of SoCs with Mentor MUSB controller and HS phy. It comes with a slightly different register space layout, and some quirks, so it's not multiplexed with the existing musb support library.
This porting currently requires to be compiled with https://github.com/hongxuyao/F1C100s_with_Keil_RTX4_emWin5 . Support for Keil RTX4 is also added in this pull request.
There is a working CDC loopback demo project at https://github.com/t123yh/F1C100s_RTX4_USB .