• AVR Freaks

Buffer toggle synchronization issue

Author
latchup
New Member
  • Total Posts : 15
  • Reward points : 0
  • Joined: 2010/05/29 21:43:26
  • Location: Germany
  • Status: offline
2010/05/30 09:04:33 (permalink)
0

Buffer toggle synchronization issue

Hello together,

here is an issue I found testing the USB Stack v2.7 on a PIC18F2550 device.
I have some experience with the PIC18 microcontrollers, but I have not used the C compiler nor the USB Stack so far, so maybe
the problems just arise from my limited understanding of the library. I would appreciate if someone more experienced with the
USB Stack verified the issue before I feed it into the bug-tracking system.

----------------------

Issue in buffer toggle synchronization after a CLEAR_FEATURE(ENDPOINT_HALT) request:

According to the USB specification, a CLEAR_FEATURE(ENDPOINT_HALT) request has to syncronize the buffer toggling sequence to
DATA0.

The following behaviour can be reconstructed by using any of the USB Device - HID example programs installed by the Microchip
Application Libraries (with the default full ping pong configuration):

* Startup the device. A reception buffer gets installed on the EP1_OUT_EVEN descriptor (0x410)
* Let the host send a CLEAR_FEATURE(ENDPOINT_HALT) request to endpoint 1.
* The request gets serviced by USBStdFeatureReqHandler() (usb_device.c, line 1650).
      Line 1759-1762 effectively set
           BD[EP1_OUT_EVEN] = _USIE|_DAT1|_DTSEN;
           BD[EP1_OUT_ODD]  = _USIE|_DAT0|_DTSEN;
* The next data out transaction from the host will have a DATA0 token, and will therefore fail.

The discribed problem always appears, when the pBDTEntryOut[SetupPkt.EPNum] points to an ODD buffer descriptor position at
the time the request is received.
[edit] Correction: It will always fail. [/edit]

The problem should also concern the IN endpoints, but I have not tested this.

The probem may be possibly related to the issue mentioned in the following thread:
http://www.microchip.com/forums/tm.aspx?m=502884

The problem can locally by fixed by swapping lines 1759 and 1762 in usb_device.c. This works as long as only one buffer is
loaded. I can however, not see the implications of this change in the case where both ping pong buffers are loaded, since I
do not completely understand how the USB-SIE decides which buffer desciptor (even or odd) to use for the next transaction in
case both desciptors are installed. I would be grateful for a hint concerning this.

--------------------------------------------------

Speculating about the origin of this bug, I would put it down to the small but dangerous design inconsistency, that the
pBDTEntryOut[] entries point to the next-to-use BDT position in case of ping pong switched on, and therefore have to be
toggled AFTER installing the buffer. However, in the case of ping pong switched off, the DTS bit is toggled BEFORE installing
the buffer. The introduction of the pBDTEntryOut/In[] pointers anyway seems of limited use to me, since it only protects you
from installing a buffer descriptor on an uninitialized enpoint, but does not allow to install queues of more than two
buffers. (Oh, and it seems that in case both ping pong buffer descriptor positions are occupied, further calls to
USBTransferOnePacket() will just silently overwrite the oldest buffer descriptor without testing the UOWN bit...)

In the end I would like to ask your general opinion about the USB library. For a release version of a code with open sources
it seems to be in a rather poor condition to me.

Best regards
post edited by latchup - 2010/06/02 14:48:47
#1

18 Replies Related Threads

    chinzei
    Super Member
    • Total Posts : 2250
    • Reward points : 0
    • Joined: 2003/11/07 12:39:02
    • Location: Tokyo, Japan
    • Status: offline
    RE: Buffer toggle synchronization issue 2010/05/30 17:39:42 (permalink)
    0
    * Startup the device. A reception buffer gets installed on the EP1_OUT_EVEN descriptor (0x410)
    * Let the host send a CLEAR_FEATURE(ENDPOINT_HALT) request to endpoint 1.
    * The request gets serviced by USBStdFeatureReqHandler() (usb_device.c, line 1650).
    Line 1759-1762 effectively set
    BD[EP1_OUT_EVEN] = _USIE|_DAT1|_DTSEN;
    BD[EP1_OUT_ODD] = _USIE|_DAT0|_DTSEN;
    * The next data out transaction from the host will have a DATA0 token, and will therefore fail.

    Agree. It's a bug on the stack.

    I made a test application to prove this data toggle bug, modifying the PC app of "USB Device - WinUSB - Generic Driver Demo". The modification is simple. Just add a button which calls WinUsb_ResetPipe() for both of bulk IN/OUT pipe (attached file)

    1) Load the firmware example of "USB Device - WinUSB - Generic Driver Demo" to your dev board without any modification. Connect it to your PC.
    2) Run the attached PC application (.exe is contained in Release folder). And push "Connect" button.
    3) Push "Toggle LED(s)" button several times, and confirm it work as expected.
    4) Push "Pipe Reset" button once. Confirm that the device doesn't respond to next "Toggle LED(s)" button. Push "Toggle LED(s)" once again, the toggle operation recovers.

    Just after pipe reset, the bulk OUT transfer is ignored by the device.
    And next transfer is accepted.

    Attached bus analyzer trace, too.
    On the trace, it is confirmed that WinUsb_ResetPipe() issues Clear_Feature( ENDPOINT ).
    Also, no error is found on bulk OUT transfers after Clear_Feature() - correctly ACKed.
    But on the board, the transfer just after Clear_Feature() drops.

    This is the typical data toggle error symptom.

    Tsuneo

    Attached Image(s)

    #2
    chinzei
    Super Member
    • Total Posts : 2250
    • Reward points : 0
    • Joined: 2003/11/07 12:39:02
    • Location: Tokyo, Japan
    • Status: offline
    RE: Buffer toggle synchronization issue 2010/05/30 17:57:07 (permalink)
    0
    The probem may be possibly related to the issue mentioned in the following thread:
    http://www.microchip.com/forums/tm.aspx?m=502884

    Disagree on this issue.
    The error is found just in OP's case.
    In double check on my side, v2.7 stack passes USB20CV: Halt Endpoint Test without any error.

    It is obviously a problem on Get_Status( ENDPOINT ) of the OP's firmware, as the bus analyzer trace shows.
    Get_Status() should return 0x0000 or 0x0001, and the stack handler of Get_Status() is coded in this way. But it always returns 0xC361. This observation shows that the return value is overwritten somewhere after Get_Status() handler.

    This test puts the requests, Get_Status(ENDPOINT), Set_Feater(ENDPOINT_HALT), Clear_Feater(ENDPOINT_HALT)
    But it doesn't put any bulk IN/OUT transfer.
    Therefore, data toggle error on the stack is not detected with this test.

    Tsuneo
    #3
    xiaofan
    Super Member
    • Total Posts : 6247
    • Reward points : 0
    • Joined: 2005/04/14 07:05:25
    • Location: Singapore
    • Status: offline
    RE: Buffer toggle synchronization issue 2010/05/30 18:15:11 (permalink)
    0
    ORIGINAL: chinzei

    * Startup the device. A reception buffer gets installed on the EP1_OUT_EVEN descriptor (0x410)
    * Let the host send a CLEAR_FEATURE(ENDPOINT_HALT) request to endpoint 1.
    * The request gets serviced by USBStdFeatureReqHandler() (usb_device.c, line 1650).
    Line 1759-1762 effectively set
    BD[EP1_OUT_EVEN] = _USIE|_DAT1|_DTSEN;
    BD[EP1_OUT_ODD] = _USIE|_DAT0|_DTSEN;
    * The next data out transaction from the host will have a DATA0 token, and will therefore fail.

    Agree. It's a bug on the stack.



    Ah, this may well be the reason what I need a reset call for my libusb-1.0 test application for Jan Axelson's generic WinUSB example.
    http://www.microchip.com/forums/fb.aspx?m=475777
    I need to call libusb_reset_device(). This is quite similar to problems I met last time. If there is a data toggle problem, I often need to call libusb_reset_device() to get the device back on track.

    More discussion here. Alan Stern (one of the leading Linux USB developer thought that there is a firmware bug somewhere.

    http://old.nabble.com/Testing-the-Lakeview-Research-Generic-WinUSB-Firmware-with-libusb-1.0-td27447902.html

    That was for the Stack V2.5. But I will check if the same thing happens for V2.6/V2.7.

      USB_Links and libusb
    #4
    chinzei
    Super Member
    • Total Posts : 2250
    • Reward points : 0
    • Joined: 2003/11/07 12:39:02
    • Location: Tokyo, Japan
    • Status: offline
    RE: Buffer toggle synchronization issue 2010/05/30 18:36:05 (permalink)
    0
    The problem can locally by fixed by swapping lines 1759 and 1762 in usb_device.c. This works as long as only one buffer is loaded.

    Yes. It's right.

    I can however, not see the implications of this change in the case where both ping pong buffers are loaded, since I do not completely understand how the USB-SIE decides which buffer desciptor (even or odd) to use for the next transaction in case both desciptors are installed. I would be grateful for a hint concerning this.

    I'm glad to hear another one comes to the same conclusion on PIC ping-pong.

    The stack doesn't use ping-pong effectively, even when this macro is enabled in usb_config.h.
    #define USB_PING_PONG_MODE USB_PING_PONG__FULL_PING_PONG

    The stack always feeds single buffer to the ping-pong, alternately to EVEN or ODD. It waits for the transaction completion of one side. And then, it passes the buffer to the other side of the ping-pong. This operation kills primary ping-pong functionality (both BDs are available at the same time). There is no difference from NO_PING_PONG. Rather, enabling ping-pong slightly decreases the performance because of extra procedure for ping-pong handling. Also, waste of BDT memory.

    This example is the only one which uses ping-pong in the primary way.
    "USB Device - WinUSB - High Bandwidth Demo"

    However, because of the design defect on SIE (USB engine), we should not use ping-pong in the primary way, especially on Mass Storage class, in which HALT of bulk endpoints and Clear_Feature() are often applied.

    As of the fatal fault on the ping-pong implementation of the PIC SIE, I've discussed on it once.
    http://www.microchip.com/forums/fb.aspx?m=459119

    Simply speaking, the problem lies in the fact that there is no easy way for firmware to know the current BD (EVEN/ODD) for the SIE. Unfortunately, PIC SIE doesn't have any bit on the USB registers to show if the current BD is EVEN or ODD. PIC is the only one which lacks this flag. Other USB MCU manufacturers implement this flag for exposed ping-pong (double buffers).

    While single buffer is fed to ping-pong alternately, like Microchip stack does, the current BD for firmware matches to the current for SIE. Firmware can track it easily. But when the ping-pong is used in the primary way, firmware looses this tracking. The current for firmware doesn't always match to the current for SIE. Tracking of USTAT report is a possible workaround.

    Tsuneo
    #5
    chinzei
    Super Member
    • Total Posts : 2250
    • Reward points : 0
    • Joined: 2003/11/07 12:39:02
    • Location: Tokyo, Japan
    • Status: offline
    RE: Buffer toggle synchronization issue 2010/06/01 01:06:05 (permalink)
    0
    xiaofan:

    Ah, this may well be the reason what I need a reset call for my libusb-1.0 test application for Jan Axelson's generic WinUSB example.

    With quick look, I suppose your problem relates to Set_Interface() handling on the stack. The stack has data toggle bug on Set_Interface() handling, too.

    When the target interface doesn't have any alternate, the device has two options for handling of Set_Interface() request.

    1) STALL the request
    OR
    2) Accept the request, and initialize the endpoints on the interface, including data toggle.

    Microchip stack accepts Set_Interface(), but it does nothing to the endpoints. I've seen this bug from v2.4 (at least) to the latest.

    usb_device.c

    void USBCheckStdRequest(void)
    {
    if(SetupPkt.RequestType != USB_SETUP_TYPE_STANDARD_BITFIELD) return;

    switch(SetupPkt.bRequest)
    {
    ...
    case USB_REQUEST_SET_INTERFACE: // <----------- Set_Interface() handler
    // it does nothing other than
    // holding the alternate interface number.
    inPipes[0].info.bits.busy = 1;
    USBAlternateInterface[SetupPkt.bIntfID] = SetupPkt.bAltID;
    break;
    ...



    libusb_claim_interface() puts Set_Interface() request to the device, even when the device has no alternate interface. Of course, it is allowed to host, but I don't recommend it. Implementation of Set_Interface() handler often has error on many USB device stacks, not just Microchip's.

    Here is KEIL's one.
    http://www.keil.com/forum/docs/thread16287.asp

    Tsuneo
    post edited by chinzei - 2010/06/01 01:16:36
    #6
    latchup
    New Member
    • Total Posts : 15
    • Reward points : 0
    • Joined: 2010/05/29 21:43:26
    • Location: Germany
    • Status: offline
    RE: Buffer toggle synchronization issue 2010/06/01 09:28:55 (permalink)
    0
    Thanks for the review.

    I have simulated the case of two installed buffers (as demonstrated in the High-Bandwidth Demo). Here, buffer toggle sync works as long as there are always two buffers installed. This is NOT guaranteed in the High-Bandwidth Demo:
    Note, that the USB-Task is handled in the interrupt here (by default), while buffer processing is done in the context of the  main loop.
    Consider the following (rare) case, when a CLEAR_FEATURE(ENDPOINT_HALT) request is received before a prevously filled buffer gets reinstalled:

    * A buffer gets filled by the SIE and marked UOWN=0. Without loss of generality we may assume that it is the EP1_OUT_EVEN buffer. This seems
    to toggle the SIE's internal write pointer to the other (ODD) buffer descriptor. The SIE indicates the event by triggering the TRNIF interrupt. The USB-Task will call the USER_USB_CALLBACK_EVENT_HANDLER(EVENT_TRANSFER) fucntion, which ignores the event.

    * Before the main thread can process and re-install the buffer, a CLEAR_FEATURE(ENDPOINT_HALT) is received and gets serviced by the
    USBStdFeatureReqHandler(). The pBDTEntryOut[] entry is pointing to the buffer descriptor that is to be installed next, which in our case is the EVEN one which has just been filled but not yet been processed and reeinstalled. The USBStdFeatureReqHandler() will set
    BD[EP1_OUT_EVEN] = _USIE|_DAT0|_DTSEN;
    BD[EP1_OUT_ODD] =  _USIE|_DAT1|_DTSEN;

    * An OUT transaction arrives with DATA0 token. The SIE will direct it to the ODD buffer descriptor wich is set to _DAT1, so the transaction fails silently.

    As far as I can see, the problem can not be solved with a local modification only.
    A workaround for projects using double buffers would be to move buffer processing completely to the USER_USB_CALLBACK_EVENT_HANDLER(EVENT_TRANSFER) handler, which is called by the Stack directly after finishing a transaction, before any other transaction is handled. (This will also spare the funny EP1OUTEVEN/OddNeedsServicingNext-flag.)

    I will report the issue to the bug-tracking system. Until then I recommend the following workaraounds:
    * If you do not use double buffering, disable the ping-pong by #define USB_PING_PONG_MODE USB_PING_PONG__NO_PING_PONG
    * If you want to use double buffering, do all your buffer handling in the USER_USB_CALLBACK_EVENT_HANDLER(EVENT_TRANSFER). Make shure the last  filled buffer is reinstalled before returning from the callback.

    I wonder why the issue could survive more than 3 month, since it affects all of the USB Firmware examples.

    Best regards.
    post edited by latchup - 2010/06/02 14:27:23
    #7
    chinzei
    Super Member
    • Total Posts : 2250
    • Reward points : 0
    • Joined: 2003/11/07 12:39:02
    • Location: Tokyo, Japan
    • Status: offline
    RE: Buffer toggle synchronization issue 2010/06/03 14:03:26 (permalink)
    0
    I wonder why the issue could survive more than 3 month, since it affects all of the USB Firmware examples.

    Either CLEAR_FEATURE(ENDPOINT_HALT) or Set_Interface(), these requests don't appear so often in daily USB device usage on Windows. Just for error recovery. And when error occurs, programmers and users don't care so much, even if the device couldn't recover from the error, because it's on Windows wink

    Tsuneo
    #8
    latchup
    New Member
    • Total Posts : 15
    • Reward points : 0
    • Joined: 2010/05/29 21:43:26
    • Location: Germany
    • Status: offline
    RE: Buffer toggle synchronization issue 2010/06/04 16:00:14 (permalink)
    0

    Until then I recommend the following workaraounds:
    * If you do not use double buffering, disable the ping-pong by #define USB_PING_PONG_MODE USB_PING_PONG__NO_PING_PONG
    * If you want to use double buffering, do all your buffer handling in the USER_USB_CALLBACK_EVENT_HANDLER(EVENT_TRANSFER). Make shure the last  filled buffer is reinstalled before returning from the callback.


    I was wrong here. Disabling ping pong mode does not help.
    In case of USB_PING_PONG__NO_PING_PONG, the clear endpoit halt will be serviced in file usb_device.c, line 1764:
    p->STAT.Val = _USIE|_DAT1|_DTSEN;
    If a buffer is currently installed, this will set its toggle sync state to DATA1. The next OUT transaction will have a DATA0 token and will therefore be ignored. On the other hand, if no buffer is installed this seems like a fatal race condition to me, since the last filled buffer (which the user thinks he owns at the moment) is just re-installed.

    At the moment, I work around the problem by replacing the code at usb_device.c, line 1764 with the following:

    // ## :SM: Changed ##
    // Original code was:     p->STAT.Val = _USIE|_DAT1|_DTSEN;
    //
    // After a CLEAR_FEATURE(ENDPOINT_HALT), the next transaction will use a DATA0 token.
    if (p->STAT.Val & _USIE)     // Buffer is installed.
         p->STAT.Val = _USIE|_DAT0|_DTSEN;    // Sync to DATA0
    else                          // Buffer is not installed.
         p->STAT.Val = _UCPU|_DAT1|_DTSEN;    // Set to DATA1 since it will be toggled by the stack the next time a buffer is installed.
    // ## SM end ##




    When the target interface doesn't have any alternate, the device has two options for handling of Set_Interface() request.

    1) STALL the request
    OR
    2) Accept the request, and initialize the endpoints on the interface, including data toggle.


    Where exactly is this behaviour specified in the USB 2.0 specification? I do not understand this from section 9.4.10.
    Besides, the stack should at least notify the user about the interface change, so he can setup the appropriate endpoints.


    Either CLEAR_FEATURE(ENDPOINT_HALT) or Set_Interface(), these requests don't appear so often in daily USB device usage on Windows. Just for error recovery.


    I regard it as good practise to always reset my measurement devices to default states before I start to talk to them.
    The NI-VISA framework I am currently using to access the device also seems to send a SET_INTERFACE request after opening the device.
    And it is actually used by a lot of people, since it is the software layer LABVIEW uses for instrumentation control.
    At least this tracked down a bug in this case.
    #9
    chinzei
    Super Member
    • Total Posts : 2250
    • Reward points : 0
    • Joined: 2003/11/07 12:39:02
    • Location: Tokyo, Japan
    • Status: offline
    RE: Buffer toggle synchronization issue 2010/06/05 00:37:47 (permalink)
    0
    f a buffer is currently installed, this will set its toggle sync state to DATA1. The next OUT transaction will have a DATA0 token and will therefore be ignored. On the other hand, if no buffer is installed this seems like a fatal race condition to me, since the last filled buffer (which the user thinks he owns at the moment) is just re-installed.

    When the endpoint is armed on Clear_Feature( ENDPOINT_HALT ), the stack should "dis-arm" the endpoint. The packet on the endpoint is discarded. Also the stack has to notify to the "application" firmware that last transfer is canceled. Usually, stack defines a callback for this purpose.

    When the target interface doesn't have any alternate, the device has two options for handling of Set_Interface() request.

    1) STALL the request
    OR
    2) Accept the request, and initialize the endpoints on the interface, including data toggle.


    Where exactly is this behaviour specified in the USB 2.0 specification?

    Here it is.
    9.4.10 Set Interface (usb_20_052510.zip usb_20.pdf p259)
    Some USB devices have configurations with interfaces that have mutually exclusive settings. This request allows the host to select the desired alternate setting. If a device only supports a default setting for the specified interface, then a STALL may be returned in the Status stage of the request.

    9.1.1.5 Configured (usb_20_052510.zip usb_20.pdf p243)
    Configuring a device or changing an alternate setting causes all of the status and configuration values associated with endpoints in the affected interfaces to be set to their default values.
    This includes setting the data toggle of any endpoint using data toggles to the value DATA0.



    I regard it as good practise to always reset my measurement devices to default states before I start to talk to them.

    Microsoft-ism wink
    Reset is handy, but you may be better to reduce excess resets.

    Tsuneo
    #10
    xiaofan
    Super Member
    • Total Posts : 6247
    • Reward points : 0
    • Joined: 2005/04/14 07:05:25
    • Location: Singapore
    • Status: offline
    RE: Buffer toggle synchronization issue 2010/06/06 15:56:27 (permalink)
    0

    ORIGINAL: chinzei

    When the target interface doesn't have any alternate, the device has two options for handling of Set_Interface() request.

    1) STALL the request
    OR
    2) Accept the request, and initialize the endpoints on the interface, including data toggle.

    Microchip stack accepts Set_Interface(), but it does nothing to the endpoints. I've seen this bug from v2.4 (at least) to the latest.



    Now it is clear to me this is a bug. But I am not so sure how to fix it.
    One possibility is to call USBConfigureEndpoint() for every endpoint which the interface has. What is your opinion?

      USB_Links and libusb
    #11
    latchup
    New Member
    • Total Posts : 15
    • Reward points : 0
    • Joined: 2010/05/29 21:43:26
    • Location: Germany
    • Status: offline
    RE: Buffer toggle synchronization issue 2010/06/07 13:56:30 (permalink)
    0
    I see two problems here:

    (1)
    Only the user knows about the affiliation of endpoints to the different interfaces, so HE has to call USBConfigureEndpoint() for the appropriate endpoints.

    So we need an additional callback event: EVENT_SET_INTERFACE (similar to EVENT_CONFIGURED). The event parameter could be the interface number. Additionaly, usb_device.c might export the
    BYTE USBAlternateInterface[USB_MAX_NUM_INT]
    array, so the user can get the new alternate setting for the interface.

    (2)
    USBConfigureEndpoint() only resets the DTS bits for the buffer descriptors, it does however NOT reset the ping-pong buffer state of the SIE to the even buffers, so we will again provoke buffer toggle sync errors.
    The ping-pong buffer state of the SIE can only be reset for the entire module, so a single interface can not be reset so easily. As Tsuneo commented, this problem could be solved by tracking the USTAT.PPBI bit for the different endpoints, but I do not really feel like implementing this.

    So, as I see it, we can choose between two possibilities:

    (a) Either DISABLE PING-PONG

    (b) or enable ping-pong, use only one interface and reset ping-pong by touching UCON.PPBRST before reconfiguring the interface.

    If we take the maximum possible restriction:
    Disable ping-pong, use only one interface with only one alternate setting (this should apply to most devices), then a reconfiguration of the interface may be regarded as identical as an EVENT_CONFIGURED, so the implementation of an extra event can be dropped.

    If we want to make the set interface request behave according to the specification, we should also make shure that a Request Error is sent upon reception of an invalid alternate setting number. This seems to be simply done by setting
    outPipes[0].info.bits.busy = 0
    but I have not really figured out how exactely these in/outPipes[0].info.bits.busy flags are handled by the Stack.
    #12
    newfound
    Super Member
    • Total Posts : 1822
    • Reward points : 0
    • Joined: 2003/11/07 12:35:49
    • Status: offline
    RE: Buffer toggle synchronization issue 2010/06/07 21:27:03 (permalink)
    0
    Greetings

    I have been following this thread with interest and researching quietly and independently. Now I feel quite confident to post and confirm that indeed the microchip framework has a fundamental flaw in the ping-pong support as outlined by latchup and chinzei. I am not adding anything new here, it has all been covered by latchup and chinzei and all credit to them. I am just isolating the ping-pong issue from the non-ping-pong issue discussed in this thread and representing it in the hope that it helps "gell" all the bits and pieces. I found it difficult to understand at first and then I spent a lot of time trying to disprove this fault but in the end I can only confirm it.

    This thread is also required reading: http://www.microchip.com/forums/fb.aspx?m=459119

    The ping-pong issue goes further than just coding bugs. It is systemic. There are situations where there simply is no sure way to set the DATA0 and DATA1 toggles correctly (or to know if we are setting or clearing a HALT on an odd or even buffer) within the framework as it is currently written. Take for one example the "bug" in lines 1759 and 1762 of framework 2.7: (Remember this is just one example to show the systemic flaw, there will be other affected areas of code too...)



    else
    {
    //If the endpoint was an OUT endpoint then we
    // need to give control of the endpoint back to
    // the SIE so that the function driver can
    // receive the data as they expected. Also need
    // to set the DTS bit so the next packet will be
    // correct
    #if (USB_PING_PONG_MODE == USB_PING_PONG__ALL_BUT_EP0) || (USB_PING_PONG_MODE == USB_PING_PONG__FULL_PING_PONG)
    p->STAT.Val = _USIE|_DAT0|_DTSEN; <- LINE 1759
    //toggle over the to the next buffer
    ((BYTE_VAL*)&p)->Val ^= USB_NEXT_PING_PONG;
    p->STAT.Val = _USIE|_DAT1|_DTSEN; <- LINE 1762
    #else
    p->STAT.Val = _USIE|_DAT1|_DTSEN;
    #endif

    }


    Whether or not these lines need to be switched really depends on the STATE of the USB state machine. If you have ARMED both odd and even endpoints and there is a need to clear an end point HALT - then the questions is which buffer needs to be cleared and which buffer gets set to DATA0? You cannot tell by simply looking at the current state of your ODD/EVEN buffer pointer: "p", because the exception could be for either the odd or even buffer. Yet this is what the microchip framework does and why the ping-pong system is flawed.

    (Note: We cannot RESET the ping-pong of an individual end point. If we could then there would be be a problem as we would not need to know where the state machine was up to. We could simply reset it. But that is a feature the PIC SIE lacks.)

    Under some circumstances the above code will be correct and under other circumstances it is not. If you armed only one buffer then it is incorrect and the lines need to be switched. (I.E. a simple coding bug in that case, but the idea would work when coded correctly). However, if you armed both odd and even buffers then it is 50/50 as to being correct**. It depends on which odd/even buffer the SIE is dealing with. If you want true, full ping-pong then you cannot place the above lines correctly at design time. It is a run time decision.

    ** Actually if you are using interrupt driven USB this may not be the case. It becomes a timing issue, a critical race. The interrupt may fire before the firmware buffer pointer "p" is advanced the second time therefore it matches the SIE pointer. The system could then appear to work but that is good fortune, not good design.

    As chinzei said:


    As of the fatal fault on the ping-pong implementation of the PIC SIE, I've discussed on it once.
    http://www.microchip.com/forums/fb.aspx?m=459119

    Simply speaking, the problem lies in the fact that there is no easy way for firmware to know the current BD (EVEN/ODD) for the SIE. Unfortunately, PIC SIE doesn't have any bit on the USB registers to show if the current BD is EVEN or ODD. PIC is the only one which lacks this flag. Other USB MCU manufacturers implement this flag for exposed ping-pong (double buffers).

    While single buffer is fed to ping-pong alternately, like Microchip stack does, the current BD for firmware matches to the current for SIE. Firmware can track it easily. But when the ping-pong is used in the primary way, firmware looses this tracking. The current for firmware doesn't always match to the current for SIE. Tracking of USTAT report is a possible workaround.

    Tsuneo


    At run time the buffer pointer "p", needs to be updated by looking at the USTAT.PPBI bit to know for certain which ping-pong BD it has to address to clear the halt and to reset the DATAx toggles to match the USB spec. (The next transfer has to be a DATA0 transfer).

    This flaw certainly shows up in the case of a CLEAR FEATURE (end point) and will do with other CHAPTER 9 REQUESTS. (Those requests requiring the DATAx toggle to be set according to the next buffer to be used by the SIE for transfer). I.E. SET CONFIGURATION, SET INTERFACE.

    I'm sure there may be some work arounds for some CHAPTER 9 REQUESTS and ways to tinker with the code so that it covers the most likely anomalies and doing things like not putting ping-pong buffering on EP0 could help (as sometimes you can do a complete ping-pong reset) but really, a more robust and complete way would be to tie the firmware side buffer pointer "p", to the value of USTAT.PPBI for any CHAPTER 9 REQUEST requiring an end point DATAx toggle to be set in a certain way (and also for clear/set halt end point) so it covers all possibilities from the ground up and in the same systemic way so the code is easy to maintain and follow.






    #13
    xiaofan
    Super Member
    • Total Posts : 6247
    • Reward points : 0
    • Joined: 2005/04/14 07:05:25
    • Location: Singapore
    • Status: offline
    RE: Buffer toggle synchronization issue 2010/06/10 07:53:23 (permalink)
    0
    ORIGINAL: chinzei
    libusb_claim_interface() puts Set_Interface() request to the device, even when the device has no alternate interface. Of course, it is allowed to host, but I don't recommend it. Implementation of Set_Interface() handler often has error on many USB device stacks, not just Microchip's.

    Here is KEIL's one.
    http://www.keil.com/forum/docs/thread16287.asp

    Tsuneo


    Alan Stern explains to me that libusb_claim_interface() does not put Set_interface request.

    http://libusb.6.n5.nabble.com/libusb-claim-interface-and-SET-INTERFACE-td100471.html


    >> > I have some doubts of what libusb_claim_interface
    >> >
    >> > From here:
    >> > http://git.libusb.org/?p=libusb.git;a=blob;f=libusb/os/linux_usbfs.c
    >> >
    >> > libusb_claim_interface seems to use IOCTL_USBFS_CLAIMINTF,
    >> > what does it do? Does it send Set_Interface request to the device?
    >
    > No.  It updates some in-kernel data structures but it does not generate
    > any I/O.
    >
    >> > Similar question: what does IOCTL_USBFS_RELEASEINTF do?
    >
    > That ioctl will send a Set-Interface request to switch the interface
    > back to altsetting 0 if it isn't in altsetting 0 already.  Otherwise it
    > is similar to CLAIMINTF.


      USB_Links and libusb
    #14
    latchup
    New Member
    • Total Posts : 15
    • Reward points : 0
    • Joined: 2010/05/29 21:43:26
    • Location: Germany
    • Status: offline
    RE: Buffer toggle synchronization issue 2010/06/22 14:54:53 (permalink)
    0
    Newfound, thanks for the clear summary.

    Chinzei, again I have to ask, due to my insufficient experience with the USB
    specification:

    When the endpoint is armed on Clear_Feature( ENDPOINT_HALT ), the stack
    should "dis-arm" the endpoint. The packet on the endpoint is discarded. Also
    the stack has to notify to the "application" firmware that last transfer is
    canceled. Usually, stack defines a callback for this purpose.


    Is this behaviour specified by USB? From section 9.4.5 it seems to me, that
    a CLEAR_FEATURE(ENDPOINT_HALT) request does not necessarily indicate an
    error, so automatically canceling the pending transfer would be not be appropriate.


    According to your practical experience, what is the speed increase when using the
    ping-pong feature? Let me try an estimation. If I manually switch buffers in the
    TRANSFER_COMPLETE callback, then I have to accept a latency of approximately
    10us (100 cycles) from the TRN-interrupt. At maximum full speed bitrate, a 64Byte
    bulk buffer gets filled in approximately 40us. Therefore, an instantaneous ping-pong
    buffer switch over would result in a maximum speed increase of 25%.
    #15
    latchup
    New Member
    • Total Posts : 15
    • Reward points : 0
    • Joined: 2010/05/29 21:43:26
    • Location: Germany
    • Status: offline
    Re:Buffer toggle synchronization issue 2010/08/11 08:55:56 (permalink)
    0
    Sine the issue has no been fixed until now, here is the patched version of usb_device.c I am using at the moment. My edits are tagged with ":SM:".

    The following limitations apply:
    - Only one interface with only one alternative allowed.
    - Ping pong mode is not supported.

    Fixed:

    Buffer toggle sync has been corrected. The fix sets the correct DTS bit and clears the BSTALL bit. In contrast to the original code, the UOWN bit is preserved (to avoid the race condition discussed in a previous post). Note that there may be some additional user action required upon a CLEAR_FEATURE(ENDPOINT_HALT) request. The user can test for this request in the callback handler connected to the EVENT_EP0_REQUEST.

    A SET_INTERFACE request will reset all used endpoints and send an EVENT_CONFIGURED to the user callback handler. For only one Interface a SET_INTERFACE and a SET_CONFIGURATION can be treated equally, I suppose. (For more than one interface, a SET_INTERFACE request should only reset the endpoints affiliated to the addressed interface, which can only be done by the user. The EVENT_EP0_REQUEST could be utilized for this purpose again.)

    Open:

    A request with invalid Interface/Altinterface parameter should be STALLED. From my understanding of the stack code I guess it is sufficient to set outPipes[0].info.bits.busy = 0. Is this correct?

    Buffer overflow in USBStdFeatureReqHandler():
    The pBDTEntryIn/Out array will overflow if a SET/CLEAR_FEATURE(ENDPOINT_HALT) request is sent with endpoint number > USB_MAX_EP_NUMBER. Again, the request should be stalled in that case.  (The USBStdGetStatusHandler() seems to have the same vulnerability.)

    For convenience to the user, the request parsing should be done by the stack and a specific events should be connected to the SET_INTERFACE and CLEAR_FEATURE requests.
    #16
    newfound
    Super Member
    • Total Posts : 1822
    • Reward points : 0
    • Joined: 2003/11/07 12:35:49
    • Status: offline
    Re:Buffer toggle synchronization issue 2011/03/23 22:39:25 (permalink)
    0
    Follow-up. These issues have been addressed in version 2.8 of the microchip stack. 
    #17
    SugarCreek
    Senior Member
    • Total Posts : 180
    • Reward points : 0
    • Joined: 2004/09/10 16:40:56
    • Location: Indianapolis USA
    • Status: offline
    Re:Buffer toggle synchronization issue 2011/06/13 03:43:05 (permalink)
    0
    newfound

    Follow-up. These issues have been addressed in version 2.8 of the microchip stack. 
     
    Which of the issues mentioned have been fixed in v2.8?
     
    I thought that at least some of the issues mentioned in this thread were inherent in the PIC's SIE, due to the inability to reset or to know the PP status of individual EPs.
     
    As I poke through v2.8, it still strives to "know" (actually, to infer) each EP's PP status by manually toggling a shadow representation upon every TRNIF.  I'm still not sure that there aren't error cases where this is insufficient.
     
    BTW, v2.8 and predecessors make a point of putting extra instruction cycles between the assertion (PPBRST=1) and the de-assertion (PPBRST=0) of the request to reset PP to even buffers, as if there needs to be some delay similar to the 5-cycle delay before re-checking TRNIF after clearing it (as the USTAT FIFO advances).  Is there any reason why this is really necessary?
    #18
    uzslm
    Super Member
    • Total Posts : 352
    • Reward points : 0
    • Joined: 2016/06/28 18:46:27
    • Location: 0
    • Status: offline
    Re:Buffer toggle synchronization issue 2016/11/29 20:20:44 (permalink)
    0
    in the most new version of MLA,hid_custom,during routine of CLEAR_FEATURE,UOWN was handed over to MCU,
    but how could we re_arm the INTERRUPT-OUT endpoint so that we can receive any further packet from host?
    #19
    Jump to:
    © 2019 APG vNext Commercial Version 4.5