• AVR Freaks

HID Bootloader reset bug (and fix)

Author
pburgess
Starting Member
  • Total Posts : 37
  • Reward points : 0
  • Joined: 2007/04/17 22:06:44
  • Location: Fremont, CA
  • Status: offline
2011/03/22 17:53:02 (permalink)
0

HID Bootloader reset bug (and fix)

A bit esoteric, but I've actually encountered this condition "in the wild," so it may warrant addressing:

Symptom: host-side (e.g. PC) bootloader application encounters a USB write error upon issuing a reset command to the HID bootloader. This was when working with a PIC32, but I've noticed the HID bootloader code for other chips is similarly implemented. This also varies with the host-side hardware (possibly a function of chipset) -- some systems handle the situation gracefully (no write error), others do not (despite same OS and version).

Hypothetical host-side code (here using libhid on Linux) might resemble:

  
    usbBuf[0] = RESET_DEVICE;
    status = hid_interrupt_write(hid,0x01,usbBuf,1,0); 


'status' should return HID_RET_SUCCESS on successful write, but on these problem systems instead returns an error. The reset does in fact occur, but the PC application might throw up an error if it's being thorough about return codes.

Digging through the device-side code, within folder "HID Bootloader - Firmware for PIC32MX Family Devices", file "main.c", is the following block:

  
    case RESET_DEVICE: 
         U1CON = 0x0000; //Disable USB module
         /* And wait awhile for the USB cable capacitance to 
            discharge down to disconnected (SE0) state.
            Otherwise host might not realize we
            disconnected/reconnected when we do the reset.
            A basic for() loop decrementing a 16 bit number
            would be simpler, but seems to take more code
            space for a given delay.  So do this instead: */
         for(j=0;j<0xFFFF;j++) Nop();
         Reset();
         break;


Now, I don't precisely know the deeper underbelly of USB, but my impression is that the host is expecting some form of acknowledgement for the transaction, but (on those problem systems) the abrupt disabling of the USB module essentially pulls the plug before this can happen; hence, write error.

Indeed, simply duplicating the delay to also occur before disabling the module seems to correct the issue:

  
    case RESET_DEVICE: 
         for(j=0;j<0xFFFF;j++) Nop();
         U1CON = 0x0000; //Disable USB module
         /* lengthy comment */ 
         for(j=0;j<0xFFFF;j++) Nop();
         Reset();
         break;


That appears to solve the issue (no more write error on reset), but the delay isn't based on anything scientific. A more proper approach might be to check for and flush any pending output before disabling the module.
post edited by pburgess - 2011/03/22 17:56:31
#1

1 Reply Related Threads

    chinzei
    Super Member
    • Total Posts : 2250
    • Reward points : 0
    • Joined: 2003/11/07 12:39:02
    • Location: Tokyo, Japan
    • Status: offline
    Re:HID Bootloader reset bug (and fix) 2011/03/22 19:26:20 (permalink)
    0

    Now, I don't precisely know the deeper underbelly of USB, but my impression is that the host is expecting some form of acknowledgement for the transaction, but (on those problem systems) the abrupt disabling of the USB module essentially pulls the plug before this can happen; hence, write error.

    My interpretation is as follows,
    The reset command is passed to the device using HID interrupt OUT endpoint. The interrupt OUT transaction on the bus has already finished, when the firmware is notified of the command. But on the PC side, host controller doesn't report the completion of the transfer to the device driver, until the start of next USB frame. Because of this delay on the host controller, disconnection of the device by U1CON is simultaneously reported with the transaction completion. The disconnection has higher priority than the transaction completion on the device driver, the hid_interrupt_write() call fails.

    That is, more than 1ms delay (one USB frame) is recommended before disabling U1CON, as you implemented.

    Tsuneo
    #2
    Jump to:
    © 2019 APG vNext Commercial Version 4.5