[ECOS] USB-CDC on AT91

[ECOS] USB-CDC on AT91

[John Eigelaar]

I have been using the recent at91 usb driver to implement a USB-CDC
(USBSerial emulation ) device on an at91sam7s256 board.

My CDC device enumarates properly and is seens as a serial port on both
Linux and Windows. If I however try to connect to the serial port with
either minicom or hyperterminal the serial application hangs.

I could get hyperterminal to connect once and that work fine while the
session was connect but when I hanged up and tried again hyperterminal
hung.

I previously ported the FraaRTOS CDC code to eCos, before there was any
at91 driver available, and that code works fine as a serial port.

Any ideas on how to go about debugging this ? 
Any experiences with USB-CDC would be appreciated.

I have SnoopyPro and the diag serial port here at my disposal ...

Thankx
John Eigelaar


-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

Re: [ECOS] USB-CDC on AT91

[Andrew Lunn]

On Fri, Jun 02, 2006 at 07:54:57AM +0000, John Eigelaar wrote:
> I have been using the recent at91 usb driver to implement a USB-CDC
> (USBSerial emulation ) device on an at91sam7s256 board.
> 
> My CDC device enumarates properly and is seens as a serial port on both
> Linux and Windows. If I however try to connect to the serial port with
> either minicom or hyperterminal the serial application hangs.
> 
> I could get hyperterminal to connect once and that work fine while the
> session was connect but when I hanged up and tried again hyperterminal
> hung.
> 
> I previously ported the FraaRTOS CDC code to eCos, before there was any
> at91 driver available, and that code works fine as a serial port.
> 
> Any ideas on how to go about debugging this ? 
> Any experiences with USB-CDC would be appreciated.
> 
> I have SnoopyPro and the diag serial port here at my disposal ...
> 
> Thankx
> John Eigelaar

Hi John

I did find what i think is one silicon bug in the AT91SAM7S. Maybe it
is related?

I found that the first Bulk OUT message on an endpoint did not always
trigger an interrupt. It was only when the second Bulk OUT message
arrived was the interrupt triggered and i could process both messages.
After that it seemed to work O.K.

This bug causes the eCos USB tester software a problem. It sends a
single Bulk OUT message and then uses the control pipe to poll to see
if the message has been received. After 10 polls it gives up and gives
a warning. It then sends the next Bulk OUT message and another warning
is then generated by the target because it received an unexpected
message....

When do you see the problem? Do you receive the first Bulk OUT
message?

        Andrew

-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

Re: [ECOS] USB-CDC on AT91

[oli]

I fixed last week a problem in the AT91 USB-Driver. If the Driver is 
running - a IO-routine is wating for input - and an state-change 
happens, then in some cases the IO-routine will never end, because the 
driver lose the flag.how says: "the complete-funcion should be called".

Or in other words. The driver uses the interrupt-mask-bits as signal 
that an transfer is currentli running. If the flag is first cleared and 
then the transfer completts, the IO-routine will never compettes. 
Unfortunatly the enumeration clears sometime the flags whitout calling 
the completion-function.

There is a fix from Andrew whitch i want to integrate in the driver, and 
after this i will make a DIFF. The fix should be done this week...

Greatings Oliver



John Eigelaar schrieb:
> I have been using the recent at91 usb driver to implement a USB-CDC
> (USBSerial emulation ) device on an at91sam7s256 board.
>
> My CDC device enumarates properly and is seens as a serial port on both
> Linux and Windows. If I however try to connect to the serial port with
> either minicom or hyperterminal the serial application hangs.
>
> I could get hyperterminal to connect once and that work fine while the
> session was connect but when I hanged up and tried again hyperterminal
> hung.
>
> I previously ported the FraaRTOS CDC code to eCos, before there was any
> at91 driver available, and that code works fine as a serial port.
>
> Any ideas on how to go about debugging this ? Any experiences with 
> USB-CDC would be appreciated.
>
> I have SnoopyPro and the diag serial port here at my disposal ...
>
> Thankx
> John Eigelaar
>
>
>   

>   

-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

RE: [ECOS] USB-CDC on AT91

[John Eigelaar]

> -----Original Message-----
> From: oli@snr.ch [mailto:oli@snr.ch]
> Sent: Monday, June 05, 2006 11:12 PM
> To: John Eigelaar
> Cc: ecos-discuss@sources.redhat.com
> Subject: Re: [ECOS] USB-CDC on AT91
> 
> 
> There is a fix from Andrew whitch i want to integrate in the driver, and 
> after this i will make a DIFF. The fix should be done this week...
> 
>
Thankx Oliver. If you could just let me know when this is available 
that would be fantatstic.

Happy Hacking
John Eigelaar 


--
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

Re: [ECOS] USB-CDC on AT91

[Oliver Munz @ SNR]

[-- Attachment #1: Type: text/plain, Size: 566 bytes --]

The DiFF...

Oliver

John Eigelaar schrieb:
>   
>> -----Original Message-----
>> From: oli@snr.ch [mailto:oli@snr.ch]
>> Sent: Monday, June 05, 2006 11:12 PM
>> To: John Eigelaar
>> Cc: ecos-discuss@sources.redhat.com
>> Subject: Re: [ECOS] USB-CDC on AT91
>>
>>
>> There is a fix from Andrew whitch i want to integrate in the driver, and 
>> after this i will make a DIFF. The fix should be done this week...
>>
>>
>>     
> Thankx Oliver. If you could just let me know when this is available 
> that would be fantatstic.
>
> Happy Hacking
> John Eigelaar 
>
>   

[-- Attachment #2: at91_usbs_enumeration_while_transfer_is_running_fix.diff --]
[-- Type: text/plain, Size: 11597 bytes --]

? packages/devs/usb/at91/current/src/usbs_at91.c_andrews_version
Index: packages/devs/usb/at91/current/ChangeLog
===================================================================
RCS file: /cvs/ecos/ecos/packages/devs/usb/at91/current/ChangeLog,v
retrieving revision 1.3
diff -u -r1.3 ChangeLog
--- packages/devs/usb/at91/current/ChangeLog	6 Jun 2006 13:30:11 -0000	1.3
+++ packages/devs/usb/at91/current/ChangeLog	9 Jun 2006 17:43:24 -0000
@@ -1,3 +1,8 @@
+2006-06-09  Oliver Munz  <oli@snr.ch>
+
+	* src/usbs_at91.c: Fixing wrong IMR-handling and fix the losing
+    of IMR-flag at enumeration problem via new booleans.
+
 2006-06-06  Andrew Lunn  <andrew.lunn@ascom.ch>
 
 	* cdl/usbs_at91.cdl: Allow EP0 to be enabled when there are no
Index: packages/devs/usb/at91/current/src/usbs_at91.c
===================================================================
RCS file: /cvs/ecos/ecos/packages/devs/usb/at91/current/src/usbs_at91.c,v
retrieving revision 1.2
diff -u -r1.2 usbs_at91.c
--- packages/devs/usb/at91/current/src/usbs_at91.c	2 Jun 2006 18:46:13 -0000	1.2
+++ packages/devs/usb/at91/current/src/usbs_at91.c	9 Jun 2006 17:43:25 -0000
@@ -87,14 +87,20 @@
      AT91_UDP_RXRSM  | AT91_UDP_RXSUSP    | AT91_UDP_EPINT0 | \
      AT91_UDP_EPINT1 | AT91_UDP_EPINT2    | AT91_UDP_EPINT3)
 
+#define AT91_UDP_STANDARD_IRQs \
+    (AT91_UDP_WAKEUP | AT91_UDP_ENDBUSRES | AT91_UDP_EXTRSM | \
+     AT91_UDP_RXRSM  | AT91_UDP_RXSUSP    | AT91_UDP_EPINT0)
+
 #define THERE_IS_A_NEW_PACKET_IN_THE_UDP 0xffff
+#define DONT_USE_IMR_AS_TRANSFER_FLAG
+
 
 // Fifo size for each end point.
 static const cyg_uint16 usbs_at91_endpoint_fifo_size[AT91_USB_ENDPOINTS] = {
   8,
   64,
   64,
-  64,
+  8
 };
 
 // Does an endpoint support ping pong buffering?
@@ -117,6 +123,11 @@
   { THERE_IS_A_NEW_PACKET_IN_THE_UDP, THERE_IS_A_NEW_PACKET_IN_THE_UDP,
     THERE_IS_A_NEW_PACKET_IN_THE_UDP, THERE_IS_A_NEW_PACKET_IN_THE_UDP};
 
+#ifdef DONT_USE_IMR_AS_TRANSFER_FLAG
+static bool usbs_at91_endpoint_transfer_aktive[AT91_USB_ENDPOINTS] = 
+  { false, false, false, false };
+#endif
+
 static cyg_interrupt usbs_at91_intr_data;
 static cyg_handle_t usbs_at91_intr_handle;
 
@@ -231,17 +242,40 @@
   EP0_LL_SET_ADDRESS,
 } ep0_low_level_status_t;
 
-// Enable/Disable interrupts for a specific endpoint.
 static void
-usbs_at91_endpoint_interrupt_enable (cyg_uint8 epn, bool enable)
+usbs_at91_endpoint_transfer_start (cyg_uint8 epn)
 {
-  CYG_ASSERT (epn < AT91_USB_ENDPOINTS, "Invalid endpoint");
-  
-  if (enable) {
+    CYG_ASSERT (epn < AT91_USB_ENDPOINTS, "Invalid endpoint");
+    
     HAL_WRITE_UINT32 (pIER, 1 << epn);
-  } else {
-    HAL_WRITE_UINT32 (pIDR, 1 << epn);
-  }
+#ifdef DONT_USE_IMR_AS_TRANSFER_FLAG
+    usbs_at91_endpoint_transfer_aktive[epn] = true;
+#endif
+}
+
+static void
+usbs_at91_endpoint_transfer_end (cyg_uint8 epn, usbs_control_return returncode)
+{
+    usbs_rx_endpoint *pep;
+
+    CYG_ASSERT (epn < AT91_USB_ENDPOINTS, "Invalid endpoint");
+    
+#ifdef DONT_USE_IMR_AS_TRANSFER_FLAG
+    if (usbs_at91_endpoint_transfer_aktive[epn]){
+        usbs_at91_endpoint_transfer_aktive[epn] = false;
+#else
+    if (BITS_ARE_SET (pIMR, 1 << epn)) {
+#endif
+        // If the end point is transmitting, call the complete function
+        // to terminate to transfer
+        pep = (usbs_rx_endpoint *) usbs_at91_endpoints[epn];
+        
+        // Disable interrupts from the endpoint
+        HAL_WRITE_UINT32 (pIDR, 1 << epn);
+        if (pep->complete_fn) {
+            (*pep->complete_fn) (pep->complete_data, returncode);
+        }
+    }
 }
 
 static cyg_uint8 *
@@ -273,7 +307,7 @@
 }
 
 /* Tell the host that the device is ready to start communication */
-static void
+void
 usbs_at91_set_pullup (bool set)
 {                
 
@@ -314,20 +348,9 @@
 usbs_end_all_transfers (usbs_control_return returncode)
 {
   cyg_uint32 epn;
-  usbs_rx_endpoint *pep;
   
   for (epn = 1; epn < AT91_USB_ENDPOINTS; epn++) {
-    if (BITS_ARE_SET (pIMR, 1 << epn)) {
-      // If the end point is transmitting, call the complete function
-      // to terminate to transfer
-      pep = (usbs_rx_endpoint *) usbs_at91_endpoints[epn];
-
-      if (pep->complete_fn) {
-        (*pep->complete_fn) (pep->complete_data, returncode);
-      }
-      // Disable interrupts from the endpoint
-      usbs_at91_endpoint_interrupt_enable (epn, false);
-    }
+    usbs_at91_endpoint_transfer_end(epn, returncode);
   }
 }
 
@@ -336,11 +359,11 @@
 usbs_state_notify (usbs_control_endpoint * pcep)
 {
   static int old_state = USBS_STATE_CHANGE_POWERED;
-  int state = pcep->state & USBS_STATE_MASK;
+  int state = pcep->state;
   
   if (pcep->state != old_state) {
     usbs_end_all_transfers (-EPIPE);
-    switch (state) {
+    switch (state & USBS_STATE_MASK) {
       case USBS_STATE_DETACHED:
       case USBS_STATE_ATTACHED:
       case USBS_STATE_POWERED:
@@ -423,11 +446,7 @@
     pep->halted = new_value;
     
     if (new_value && BITS_ARE_SET (pIMR, 1 << epn)) {   
-      /* Ready to transmit */
-      if (pep->complete_fn) {
-        (*pep->complete_fn) (pep->complete_data, -EAGAIN);
-      }
-      usbs_at91_endpoint_interrupt_enable (epn, false);
+      usbs_at91_endpoint_transfer_end(epn, -EAGAIN);
       SET_BITS (pCSR, AT91_UDP_CSR_FORCESTALL);
     } else {
       CLEAR_BITS (pCSR, AT91_UDP_CSR_FORCESTALL);
@@ -445,7 +464,6 @@
   
   CYG_ASSERT (AT91_USB_ENDPOINTS > epn, "Invalid end point");
   
-  usbs_at91_endpoint_interrupt_enable (epn, false);
   /* Reset endpoint */
   HAL_WRITE_UINT32 (AT91_UDP + AT91_UDP_RST_EP, 1 << epn);      
   HAL_WRITE_UINT32 (AT91_UDP + AT91_UDP_RST_EP, 0);
@@ -469,10 +487,6 @@
 usbs_at91_handle_reset (void)
 {
   int epn;
-  const usb_endpoint_descriptor *usb_endpoints;
-  cyg_uint8 endpoint_type;
-  
-  usbs_end_all_transfers (-EPIPE);
   
   HAL_WRITE_UINT32 (AT91_UDP + AT91_UDP_IDR, 0xffffffff);
   HAL_WRITE_UINT32 (AT91_UDP + AT91_UDP_ICR, 0xffffffff);
@@ -482,35 +496,18 @@
   HAL_WRITE_UINT32 (AT91_UDP + AT91_UDP_FADDR, AT91_UDP_FADDR_FEN);
   HAL_WRITE_UINT32 (AT91_UDP + AT91_UDP_CSR0,
                     AT91_UDP_CSR_EPEDS | AT91_UDP_CSR_EPTYPE_CTRL);
-  HAL_WRITE_UINT32 (AT91_UDP + AT91_UDP_IER, AT91_UDP_ALLOWED_IRQs);
+  HAL_WRITE_UINT32 (AT91_UDP + AT91_UDP_IER, AT91_UDP_STANDARD_IRQs);
   
   for (epn=1; epn < AT91_USB_ENDPOINTS; epn++) {
     usbs_at91_endpoint_init ((usbs_rx_endpoint *)usbs_at91_endpoints[epn], 
                              0, false);
   }
-
-  // Now walk the endpoints configuring them correctly. This only
-  // works if there is one interface.
-  usb_endpoints = usbs_at91_ep0.enumeration_data->endpoints;
-  
-  for (epn = 1; 
-       epn <= usbs_at91_ep0.enumeration_data->total_number_endpoints; 
-       epn++) {
-    
-    endpoint_type = (usb_endpoints[epn-1].attributes |
-                     (usb_endpoints[epn-1].endpoint & 
-                      USB_ENDPOINT_DESCRIPTOR_ENDPOINT_IN ?
-                      USB_ENDPOINT_DESCRIPTOR_ENDPOINT_IN : 
-                      USB_ENDPOINT_DESCRIPTOR_ENDPOINT_OUT));
-    usbs_at91_endpoint_init((usbs_rx_endpoint *)usbs_at91_endpoints[epn],
-                            endpoint_type,
-                            true);
-  }
 }
 
 static void
 usbs_at91_ep0_start (usbs_control_endpoint * endpoint)
 {
+  
   usbs_at91_handle_reset ();
 
   // If there is additional platform-specific initialization to
@@ -537,6 +534,7 @@
   CYG_ASSERT (pep->complete_fn, "No complete_fn()");
 
   cyg_drv_dsr_lock ();
+
   if (usbs_at91_ep0.state != USBS_STATE_CONFIGURED) {   
     /* If not configured it means there is nothing to do */
     cyg_drv_dsr_unlock ();
@@ -567,7 +565,18 @@
     return;
   }
   
-  CYG_ASSERT (BITS_ARE_SET (pCSR, 1 << 9), "Wrong endpoint type");
   
   *ppbegin = pep->buffer;       /* Set the working pointers */
   *ppend = (cyg_uint8 *) ((cyg_uint32) pep->buffer + pep->buffer_size);
@@ -589,8 +598,8 @@
     }
   }
   
-  usbs_at91_endpoint_interrupt_enable (epn, true);
-  
+  usbs_at91_endpoint_transfer_start (epn);
+
   cyg_drv_dsr_unlock ();
 }
 
@@ -738,17 +747,13 @@
   CYG_ASSERT (AT91_USB_ENDPOINTS > epn && epn, "Invalid end point");
   CYG_ASSERT (pep->complete_fn, "No complete_fn()");
 
-  if (*ppend == *ppbegin) {     /* Transmitted/Received ? */
-    
+  if (*ppend == *ppbegin) {     /* Transmitted ? */
     pep->buffer_size = (cyg_uint32) * ppbegin - (cyg_uint32) pep->buffer;
-    if (pep->buffer_size && pep->complete_fn) {
-      if (!pep->halted) {
-        (*pep->complete_fn) (pep->complete_data, pep->buffer_size);
-      } else {
-        (*pep->complete_fn) (pep->complete_data, -EAGAIN);
-      }
+    if (!pep->halted) {
+      usbs_at91_endpoint_transfer_end(epn, pep->buffer_size);
+    } else {
+      usbs_at91_endpoint_transfer_end(epn, -EAGAIN);
     }
-    usbs_at91_endpoint_interrupt_enable (epn, false);
   }
 }
 
@@ -879,7 +884,7 @@
   
   usbs_at91_control_setup_send_ack();
   status = EP0_LL_SEND_READY;
-
+  
   switch (recipient) {
     case USB_DEVREQ_RECIPIENT_DEVICE:
       status = EP0_LL_STALL;
@@ -913,12 +918,10 @@
   cyg_uint8 **ppbegin = &usbs_at91_endpoint_pbegin[0];
   cyg_uint8 **ppend = &usbs_at91_endpoint_pend[0];
   usb_devreq *req = (usb_devreq *) usbs_at91_ep0.control_buffer;
-  cyg_uint8   protocol;
   cyg_uint16 length;
   bool dev_to_host;
   usbs_control_return usbcode;
-  bool handled = false;
-  
+
   usbs_at91_ep0.buffer_size = 0;
   usbs_at91_ep0.fill_buffer_fn = 0;
   usbs_at91_ep0.complete_fn = 0;
@@ -931,8 +934,6 @@
   CLEAR_BITS (pCSR0, AT91_UDP_CSR_DTGLE);
 
   status = EP0_LL_REQUEST;
-
-  protocol = req->type & (USB_DEVREQ_TYPE_MASK);
   
   // Set the next transfer direction
   if (dev_to_host) {        
@@ -940,33 +941,26 @@
   } else {
     CLEAR_BITS (pCSR0, AT91_UDP_CSR_DIR);   /* Set OUT direction */
   }
-
-  if (protocol == USB_DEVREQ_TYPE_STANDARD) {
-    handled = true;
-    switch (req->request) {
-      case USB_DEVREQ_GET_STATUS:
-        status = usbs_at91_control_setup_get_status();
-        break;
-      case USB_DEVREQ_SET_ADDRESS:
-        // Most of the hard work is done by the hardware. We just need
-        // to send an ACK.
-        usbs_at91_control_setup_send_ack();
-        status = EP0_LL_SEND_READY;
-        break;
-      case USB_DEVREQ_SET_FEATURE:     
-        status = usbs_at91_control_setup_set_feature();
-        break;
-      case USB_DEVREQ_CLEAR_FEATURE:
-        status = usbs_at91_control_setup_clear_feature();
-        break;
-      default:
-        handled = false;
-    }
-  }
-  if ((protocol != USB_DEVREQ_TYPE_STANDARD) || !handled) {
-    // Ask the layer above to process the message
-    usbcode = usbs_parse_host_get_command (&usbs_at91_ep0);
-    usbs_at91_ep0.buffer_size = MIN (usbs_at91_ep0.buffer_size, length);
+  
+  switch (req->request) {
+    case USB_DEVREQ_GET_STATUS:
+      status = usbs_at91_control_setup_get_status();
+      break;
+    case USB_DEVREQ_SET_ADDRESS:
+      // Most of the hard work is done by the hardware. We just need
+      // to send an ACK.
+      usbs_at91_control_setup_send_ack();
+      status = EP0_LL_SEND_READY;
+      break;
+    case USB_DEVREQ_SET_FEATURE:     
+      status = usbs_at91_control_setup_set_feature();
+      break;
+    case USB_DEVREQ_CLEAR_FEATURE:
+      status = usbs_at91_control_setup_clear_feature();
+      break;
+    default:
+      // Ask the layer above to process the message
+      usbcode = usbs_parse_host_get_command (&usbs_at91_ep0);
       
     *ppbegin = usbs_at91_ep0.buffer;
     *ppend = *ppbegin + usbs_at91_ep0.buffer_size; /* Ready to send... */

[-- Attachment #3: Type: text/plain, Size: 148 bytes --]

-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss