Should ENABLE and STALL flags be set at the same time in dcd_eptri?

[mateusz-holenko]

Question Description

When analyzing the code of dcd_eptri I encountered a line that sets both STALL and ENABLE flags when writing the OUT control register: https://github.com/hathach/tinyusb/blob/master/src/portable/valentyusb/eptri/dcd_eptri.c#L440.

According to the FOMU documentation though:

Note that OUT_CTRL.ENABLE should not be set at the same time 
as OUT_CTRL.STALL, as this will cause a conflict.

(The last sentence of the https://rm.fomu.im/usb.html#stalling-an-endpoint paragraph).

Is this a bug in the code or just an outdated documentation?

[xobs]

It's an outdated comment. Originally that would cause an error, but because the STALL packet needs to be sent only once, I saw reliability errors. Now the ENABLE bit tells the endpoint whether to send any data at all, and the STALL bit indicates whether that data is a STALL.

The lines in question are at https://github.com/im-tomu/valentyusb/blob/master/valentyusb/usbcore/cpu/eptri.py#L832-L837:

            If(ctrl.fields.reset | self.usb_reset,
                stall_status.eq(0),
            ).Elif(usb_core.setup | (ctrl.re & ~ctrl.fields.stall),
                # If a SETUP packet comes in, clear the STALL bit.
                stall_status.eq(stall_status & ~ep_mask),
            ).Elif(ctrl.re,
                stall_status.eq(stall_status | ep_mask),
            ),

Stall is cleared if a SETUP packet comes in OR the field is written to with the STALL bit cleared.

If the STALL bit is set, then it gets set in the ep_mask.

This is separate from the ENABLE logic at https://github.com/im-tomu/valentyusb/blob/master/valentyusb/usbcore/cpu/eptri.py#L888-L895:

            If(ctrl.fields.reset | self.usb_reset,
                stall_status.eq(0),
            ).Elif(usb_core.setup | (ctrl.re & ~ctrl.fields.stall),
                # If a SETUP packet comes in, clear the STALL bit.
                stall_status.eq(stall_status & ~ep_mask),
            ).Elif(ctrl.re,
                stall_status.eq(stall_status | ep_mask),
            ),

I'll update the hardware docs.

[xobs]

Updated in https://github.com/im-tomu/valentyusb/commit/f6fda568a2cd2589820e7269edfa86ba6a3af82c#diff-5683d6cf8f26eca7118a5e12d59d6abf

[mateusz-holenko]

@xobs Thanks for the explanaition! I'll make sure it works the same in Renode model.