USB: error in “Frame Number Register Low/High“

[fferraro]

I am using CodeWarrior 6.3 C Compiler for Coldfire MCF52223 on the Freescale M52223EVB Development board. I need to use the processor USB interface in Host mode. My software is based on the CMX routines (Host Demo) downloaded from Freescale Web Site.

My problems is:

1)  In the reference manual of the MCF52223 about chapter “16.4.1.17 Frame Number Register Low/High (FRM_NUML, FRM_NUMH)” the descriptor is errata. (it is the same of 16.4.1.16).

2) In the demo code CMX I have discovered a bug in this loop in subroutine “usb_host_start_transaction(..)” in the file “usb_host.c”

/* wait till frame is due */
    do {
      elapsed=(hcc_u16)(MCF_USB_FRM_NUM-my_device.eps[ep].last_due);
      elapsed &= ((1<<11)-1);
    } while(elapsed < my_device.eps[ep].interval);
   
    my_device.eps[ep].last_due += my_device.eps[ep].interval;
    my_device.eps[ep].last_due &= ((1<<11)-1);   

It seems that the MCF_USB_FRM_NUM register, sometimes returns invalid values. So the elapsed time between two transactions may be less than desired , so the device stops working.

Any suggestion?

[RichTestardi]

I believe you've found a bug...

The #define from mcf5222x_reg.h:

Code:

#define MCF_USB_FRM_NUM (MCF_USB_INT_STAT=MCF_USB_INT_STAT_SOF_TOK ,MCF_USB_FRM_NUML | (((hcc_u16)MCF_USB_FRM_NUMH)<<8))

Is not atomic -- it can read the low and high bytes in either order, and one can be before a rollover and the other can be after.

To protect this, I believe you'd need to assemble the 16 bit result and "validate" it -- if it looks invalid (i.e., if the low and high bytes don't appear to have consistently rolled over), you'd have to retry the assembly -- if new low is less than old low (i.e., rollover occurred) and new high is not greater than old high (rollover did not occur consistently), then retry the assembly.

Or for your purposes (depending on the interval required), you might be able to get by just using the low 8 bits of the frame number.

-- Rich

[RichTestardi]

And just to be accurate, the validation would need to make sure that rollover either occurred in both the low and high byte *or* did not occur in both the low and high byte...  So my example algorithm was just half of what you would need...  But my guess is if your interval is not huge that you can just get by using the low byte of the frame register.

[fferraro]

Hi Rich, sorry for my late.

Your note seems to be correct, that could be the problem.

Now I have solved the problem checking the loop number in the “wait till frame is due”. If this is less than 3, I wait other my_device.eps[ep].interval times.

This is not correct but now my device does not crash.

This is my code:

 numcicli=0;
    do {
      numcicli++;
      elapsed=(hcc_u16)(MCF_USB_FRM_NUM-my_device.eps[ep].last_due);
      elapsed &= ((1<<11)-1);
      if (MCF_USB_INT_STAT & MCF_USB_INT_STAT_USB_RST)
      {
        evt_disconnect();
        tr_error=tre_disconnected;
        return((hcc_u16)-1u);
      }
    } while(elapsed < my_device.eps[ep].interval);

   
    my_device.eps[ep].last_due += my_device.eps[ep].interval;
    my_device.eps[ep].last_due &= ((1<<11)-1);   

// correzione eventuale errore registro FRM
if(numcicli<3)
{
  host_ms_delay(my_device.eps[ep].interval);
       elapsed=(hcc_u16)(MCF_USB_FRM_NUM);
      elapsed &= ((1<<11)-1);
     my_device.eps[ep].last_due=elapsed;
}

I am using the mouse and the keyboard devices and typically the my_device.eps[ep].interval is 10ms,  therefore I can use only the low byte of the frame register.

I have another problem with the hid_set_report(hcc_u8 report_id, hcc_u8 *buffer, hcc_u16 length) to turn on/off the keyboard leds. I think that this problem is due to the SOF threshold and frame registers misaligned. I'm investigating this, if I can not find the solution I will open another thread in this forum.

In this moment I have already found an error of the toggle DATA0/1 in usb_host_start_transaction(..) in a setup transaction:

old code:

       /* After the setup we shall send/receive DATA1 packets. */
        my_device.eps[ep].tgl_tx=1;
        my_device.eps[ep].tgl_rx=1;

new code:

        /* After the setup we shall send/receive DATA1 packets. */
        my_device.eps[ep].tgl_tx=BDT_CTL_DATA;
        my_device.eps[ep].tgl_rx=BDT_CTL_DATA;

 

Thank you very much for your answers .

Francesco

[fferraro]

Now, using 8-bit works better, but at the start and at the change of ep=endpoint there is still the problem, because the MCF_USB_FRM_NUML and my_device.eps[ep].last_due misaligned.

[RichTestardi]

Hi,

I didn't understand why there is a problem changing endpoints:

> ... and at the change of ep=endpoint

Is it just that any transaction to another endpoint already (synchronously) waited, so this endpoint is likely past due already by the time it gets a transaction?

> but at the start ...

At the start, you could initialize the last_due field to some invalid value (rather than 0), and detect that in usb_host_start_transaction() and either request a full wait by setting the last_due field to MCF_USB_FRM_NUM or no wait by setting it to MCF_USB_FRM_NUM-my_device.eps[ep].interval.

But I'm guessing changing endpoints is your bigger problem... :smileysad:

-- Rich

[fferraro]

---

Rich T wrote:

But I'm guessing changing endpoints is your bigger problem... :smileysad:

-- Rich

---

Maybe, in fact if I use the hid_set_report(0, &leds, 1) to turn on/off the keyboard leds (I need to use EP=0 for this command and change to EP=1 for others commands) often the keyboard hangs up; if not both mouse and keyboard work correctly.