drivers: usb: USB device driver for Kinetis USBFSOTG Controller

[jfischer-no]

USB device driver for Kinetis USBFSOTG Controller

I had a driver for Kinetis USBFSOTG controller left over here, so I adapted it for zephyr, it is still WIP/RFC. Tested with FRDM-K64F board. The LEDs/gpio code will be removed, it is only for tracing.

TODO:

• it needs better configuration options for endpoints, dependent on usb class application , currently 64 byte buffers are used by default -> see 259e568.
• ~mpu configuration (as workaround it is simply turned off 👊 )~~
• ~clock configuration~~
• testing with KW22D512 (arch: arm: support for KW2xD SiP and USB-KW24D512 #845)
• better use of both (even/odd) endpoint buffers

[jfischer-no]

@fvincenzo @MaureenHelm When the MPU is switched on (default on), the USBFSOTG controller signals the DMA error because it, Bus Master 4, can not access the SRAM (BD table and the buffer are located there). As I see, I can insert a new region into nxp_mpu_regions.c and/or add BM4 Flags to MPU_REGION_xxxxx, is it correct so far? see 678c39b

[MaureenHelm]

Thanks @jfischer-phytec-iot. This looks pretty good. I wasn't able to get the cdc sample to work on frdm_k64f though; it got stuck waiting for DTR.

@fvincenzo @MaureenHelm When the MPU is switched on (default on), the USBFSOTG controller signals the DMA error because it, Bus Master 4, can not access the SRAM (BD table and the buffer are located there). As I see, I can insert a new region into nxp_mpu_regions.c and/or add BM4 Flags to MPU_REGION_xxxxx, is it correct so far? see 678c39b

Let's add the flags to MPU_REGION_xxxx (as you've already done) to be consistent with ethernet.

[MaureenHelm]

Yes, please. Should also move to soc.c since the SIM is soc-specific.

[jfischer-no]

Done, I wanted to make the clock source configurable, but the IRC48 did not work for me, so for the first the PLL is the only possible clock source.

[dbkinder]

Does the sample app read from and write to the minicom terminal?

[dbkinder]

Don't you need to open minicom BEFORE you run the sample app (where does "Wait for DTR" mentioned above appear?)

[jfischer-no]

no, the sample waits for DTR, it appears after the minicom opens the device provided by the Zephyr.

[dbkinder]

This sample app demonstrates use of a USB Communication Device Class (CDC) Abstract Control Model (ACM) driver provided by the Zephyr project. Received data is echoed back to the serial port provided by this driver.

[dbkinder]

You don't say how to build (and flash) the sample app.

[dbkinder]

You should mention this issue earlier in the steps -- before plugging the board to your host system I suspect.

[dbkinder]

Does this sample app only work on specific boards? If so, you should list them here.

[dbkinder]

Is this the list of supported boards then?

[jfischer-no]

yes, but, I've just seen that the default board for this sample is quark_se_c1000_devboard.

[dbkinder]

(as shown by the Linux dmesg command):

[dbkinder]

suggested edits and clarifications

[jfischer-no]

@dbkinder I moved 4a4c6a0 and d8c63e1 to #825

[jfischer-no]

rebased on master, tested with CDC ACM and DFU, usable but still WIP (leds and blocking while must be removed)

[galak]

@jfischer-phytec-iot since still wip, added that to subject, also used github markup for the todo list. (might need a bit of cleaning up)

[finikorg]

LGTM, tested CDC ACM and ECM functionality on FRDM-K64F

[jfischer-no]

@nashif ping

[nashif]

@nashif ping

@jfischer-phytec-iot pong.

@MaureenHelm have you looked at this? I assume it is ready to be re-reviewed/tested now.

[MaureenHelm]

LGTM, but I'd like to get someone from my USB team to also take a look.

I tested CDC on frdm_k64f and it works fine. Do the other samples work too?

[MaureenHelm]

Remove the 0x from the address. You'll also need to update the fixups accordingly

[MaureenHelm]

Remove the 0x from the address. You'll also need to update the fixups accordingly

[jfischer-no]

@MaureenHelm

I tested CDC on frdm_k64f and it works fine. Do the other samples work too?

Yes, this is my reference driver for usb development (netusb related samples need CONFIG_ETH_MCUX to be disabled).

[MaureenHelm]

@jfischer-phytec-iot I'm sorry for the delay, but I got some feedback from our USB team. Here's what they said:

1. Some SOC configure code is in kinetis_usb_init(void) like:
   /* enable USB voltage regulator */
   SIM->SOPT1 |= SIM_SOPT1_USBREGEN_MASK;
   For different SoCs, the register to enable USB regulator might be different. This code only works for a part of Kinetis devices.

2. In the Kinetis USB controller, there is limitation that if the device is not ready to receive the setup token sent by the host, the device controller would hang. To avoid such issue, the driver needs to prime a transfer to receive the next setup token as soon as possible once the previous setup transfer is done. From the code we could see, the next setup prime operation would be called by the upper layer of the USB Kinetis driver, it might be not quick enough in some extreme cases.

3. The driver is using memcpy operation in the data transfer, which will cause huge throughput performance decrease.

4. dev_data.attached is set in function usb_dc_attach(), but it is not clear in usb_dc_detach

5. Can’t support multiple KHCI controllers case. Currently there is no multiple KHCI controllers cases in any existed NXP SoC, but from design viewpoint, it is necessary to take it into accounted.

6. Any STALL interrupt will cause the STALL status of EP0 be cleared. In case that both EP0 and EP1 should be stalled (sending out STALL handshake), if EP1’s STALL token is sent out and EP1’s STALL interrupt issued, the EP0’s STALL would be cleared before the STALL handshake for EP0 is sent out, then NO STALL handshake for EP0 would be sent out.

7. If one EP is in stall status, the transfers on this ep need to be forbidden

8. When the Clear_Feature(HALT) is received from the host via EP0, the data toggle needs to be reset. And any unfinished transfer needs to be cancelled. Please take the following case into accounted that the Host sends out a Clear_Feature(HALT) even the corresponding ep is NOT in stall status.

[jfischer-no]

@MaureenHelm thanks for review

Some SOC configure code is in kinetis_usb_init(void) like:
/* enable USB voltage regulator */
SIM->SOPT1 |= SIM_SOPT1_USBREGEN_MASK;
For different SoCs, the register to enable USB regulator might be different. This code only works for a part of Kinetis devices.

yes, it's an issue, but I do not have an elegant solution right now. It probably belongs in something like a power domain driver but I do not want to touch it now.

In the Kinetis USB controller, there is limitation that if the device is not ready to receive the setup token sent by the host, the device controller would hang. To avoid such issue, the driver needs to prime a transfer to receive the next setup token as soon as possible once the previous setup transfer is done. From the code we could see, the next setup prime operation would be called by the upper layer of the USB Kinetis driver, it might be not quick enough in some extreme cases.

If it is not possible to receive a setup token then a token was received and the TXSUSPENDTOKENBUSY is set until the stack(cooperative thread) calls the function usb_dc_ep_read. Currently, there is no way to signal the state of setup Stage to the driver. I will keep it in mind.

The driver is using memcpy operation in the data transfer, which will cause huge throughput performance decrease.

Yes, It can be improved, the buffer have not be copied again, but it not only affect the kinetis driver, I see it as a future enhancement to the USB stack.

dev_data.attached is set in function usb_dc_attach(), but it is not clear in usb_dc_detach

It will be set to false at the end of usb_dc_detach.

Can’t support multiple KHCI controllers case. Currently there is no multiple KHCI controllers cases in any existed NXP SoC, but from design viewpoint, it is necessary to take it into accounted.

Yes, it's not so nice, but the usb stack does not support multiple controller case either.

Any STALL interrupt will cause the STALL status of EP0 be cleared. In case that both EP0 and EP1 should be stalled (sending out STALL handshake), if EP1’s STALL token is sent out and EP1’s STALL interrupt issued, the EP0’s STALL would be cleared before the STALL handshake for EP0 is sent out, then NO STALL handshake for EP0 would be sent out.

I will check it.

If one EP is in stall status, the transfers on this ep need to be forbidden

I will look at it.

When the Clear_Feature(HALT) is received from the host via EP0, the data toggle needs to be reset. And any unfinished transfer needs to be cancelled. Please take the following case into accounted that the Host sends out a Clear_Feature(HALT) even the corresponding ep is NOT in stall status.

I will check it.

[jfischer-no]

@nashif @MaureenHelm @finikorg
Updated, see last commit. I have tested it together with #7435 and the driver passes all tests.

./testusb -v 512 -D /dev/bus/usb/005/026                           
unknown speed   /dev/bus/usb/005/026    0
/dev/bus/usb/005/026 test 0,    0.000006 secs
/dev/bus/usb/005/026 test 1,    2.000006 secs
/dev/bus/usb/005/026 test 2,    1.999993 secs
/dev/bus/usb/005/026 test 3,    1.001027 secs
/dev/bus/usb/005/026 test 4,    1.000963 secs
/dev/bus/usb/005/026 test 5,   57.978878 secs
/dev/bus/usb/005/026 test 6,   34.993027 secs
/dev/bus/usb/005/026 test 7,   29.998934 secs
/dev/bus/usb/005/026 test 8,   18.001992 secs
/dev/bus/usb/005/026 test 9,    4.979957 secs
/dev/bus/usb/005/026 test 10,   11.952035 secs
/dev/bus/usb/005/026 test 11,   16.084910 secs
/dev/bus/usb/005/026 test 12,   16.086950 secs
/dev/bus/usb/005/026 test 13,   17.955939 secs
/dev/bus/usb/005/026 test 14,    2.464990 secs
/dev/bus/usb/005/026 test 17,    1.999973 secs
/dev/bus/usb/005/026 test 18,    2.000033 secs
/dev/bus/usb/005/026 test 19,    1.999984 secs
/dev/bus/usb/005/026 test 20,    2.000021 secs
/dev/bus/usb/005/026 test 21,    2.443951 secs
/dev/bus/usb/005/026 test 24,   55.060829 secs
/dev/bus/usb/005/026 test 27,   56.895873 secs
/dev/bus/usb/005/026 test 28,   34.153936 secs

Linux kernel output:

usbtest 5-1:1.0: TEST 0:  NOP
usbtest 5-1:1.0: TEST 1:  write 1024 bytes 1000 times                                   
usbtest 5-1:1.0: TEST 2:  read 1024 bytes 1000 times                                    
usbtest 5-1:1.0: TEST 3:  write/512 0..1024 bytes 1000 times                            
usbtest 5-1:1.0: TEST 4:  read/512 0..1024 bytes 1000 times                             
usbtest 5-1:1.0: TEST 5:  write 1000 sglists 32 entries of 1024 bytes                   
usbtest 5-1:1.0: TEST 6:  read 1000 sglists 32 entries of 1024 bytes                    
usbtest 5-1:1.0: TEST 7:  write/512 1000 sglists 32 entries 0..1024 bytes               
usbtest 5-1:1.0: TEST 8:  read/512 1000 sglists 32 entries 0..1024 bytes                
usbtest 5-1:1.0: TEST 9:  ch9 (subset) control tests, 1000 times                        
usbtest 5-1:1.0: TEST 10:  queue 32 control calls, 1000 times                           
usbtest 5-1:1.0: TEST 11:  unlink 1000 reads of 1024                                    
usbtest 5-1:1.0: TEST 12:  unlink 1000 writes of 1024                                   
usbtest 5-1:1.0: TEST 13:  set/clear 1000 halts                                         
usbtest 5-1:1.0: TEST 14:  1000 ep0out, 1..1024 vary 512                                
usbtest 5-1:1.0: TEST 17:  write odd addr 1024 bytes 1000 times core map                
usbtest 5-1:1.0: TEST 18:  read odd addr 1024 bytes 1000 times core map                 
usbtest 5-1:1.0: TEST 19:  write odd addr 1024 bytes 1000 times premapped               
usbtest 5-1:1.0: TEST 20:  read odd addr 1024 bytes 1000 times premapped                
usbtest 5-1:1.0: TEST 21:  1000 ep0out odd addr, 1..1024 vary 512                       
usbtest 5-1:1.0: TEST 24:  unlink from 1000 queues of 32 1024-byte writes               
usbtest 5-1:1.0: TEST 27: bulk write 31Mbytes                                           
usbtest 5-1:1.0: TEST 28: bulk read 31Mbytes     

Doc changes were moved to #825