max_packet_size_ep0 < 32 bytes does not work

[mrh1997]

When simulating a device with a max_packet_size_ep0 of 8 or 16 Bytes (32/64 Bytes work) replying to the 4th request will fail. In fact I do not know if it is definitiely the 4th request, but I did not find any other commonality (like number of transactions or timing).

I tested on Windows where the 3rd request is "GetDescriptor CONFIGURATION" which is processed correctly. The 4th request ("GetDescriptor STRING iSerialNumber") is received correctly and FaceDancer transmits the correct answer via GreatDancerApp.send_on_endpoint().

But the firmware does not transfer the answer (any NAKs are transferred) while GreatDancerApp.send_on_endpoint() returns without error.

important: To make FaceDancer work with max_packet_size_ep0 < 64 this buggy line has to be fixed to self.backend.connect(self, self.max_packet_size_ep0)

[mrh1997]

The following FaceDancer code can be used to reproduce the bug.

from facedancer         import main
from facedancer.future  import *

@use_inner_classes_automatically
class BugDemoDevice(USBDevice):
    max_packet_size_ep0      : int  = 8
    class Configuration1(USBConfiguration):
        configuration_number   : int = 1

if __name__ == '__main__':
    main(BugDemoDevice)

Please note that this FaceDancer pullrequest is required to reproduce the bug

The outcome you can see in the analyser is like:

[mrh1997]

I forgotten to mention, than when setting max_packet_size_ep0 : int = 64 everything works as expected (aka GetDescriptor STRING and following commands are working)

[ktemkin]

Hey! Thanks for the report!

@Qyriad would you be up for taking a look at this?

[mrh1997]

Did some further investigations by now:

• the value of max_packet_size_ep0 is forwarded correctly to the LPC's USB controller (This can also be seen in the log above, as the transfers are split correctly into 8 byte transactions)
• Incidentially the serialnumber was too big to fit into a single 16 byte transfer but it fitted into a single 32 byte transfer. Thus I testwise added the line serial_number_string: str = "12" to my testscript. Now the answer to "GetDescriptor (String 3)" fitted into a single (8 byte) transaction. But even then the problem occured...

[mrh1997]

The Problem is, that the USB_TD_DTD_TOKEN_STATUS_ACTIVE will not be cleared if the max_packet_size of endpoint 0 is 8 or 16. This causes usb_queue_clean_up_transfers() not to free the dTD buffers after a IN transfer on endpoint 0 is completed. As there are only 4 buffers for PIPE 0 IN it is not possible to allocate memory for further transfers after the 4th GetDescriptor command.

I assume this is a bug in the LPC hardware because:

• As mentioned above a max_packet_size of 32 and 64 works fine.
• I doublechecked the LPC datasheet. Looks like the code is OK.

@Qyriad: any idea for a workaround?

[ktemkin]

This might be a case for expiring packets before they're marked non-active. They're currently at least expired and cleaned up when the next SETUP comes through; but perhaps there's another condition we could use for expiry?

Do we know if the interrupt-on-complete still fires?

[mrh1997]

Actually the usb.c Module flushes/frees already all transfers on endpoint 0 when receiving a SETUP packet.

But I saw that greatdancer.c does only rely partially on usb.c and implements a lot of the lowlevel logic on its own. Especially interrupt handling (including the flushing of endpoint 0 on SETUP packet) is not reused by greatdancer.c. Thus I would add flushing to the greatdancer_verb_read_setup().

Hope you are OK with that...

[Qyriad]

Fixed with #353.