On 09/03/18 11:26, Roger Quadros wrote:

On 09/03/18 11:23, Felipe Balbi wrote:

quoted

Hi,

Roger Quadros [off-list ref] writes:

<snip>

quoted

quoted

quoted

When we set up the DWC3_DEPCMD_ENDTRANSFER command in
dwc3_stop_active_transfer(), we can do not set DWC3_DEPCMD_CMDIOC,
then there will no endpoint command complete interrupts I think.

cmd |= DWC3_DEPCMD_CMDIOC;

I remember some part of the databook mandating CMDIOC to be set. We
could test it out without and see if anything blows up. I would,
however, require a lengthy comment explaining that we're deviating from
databook revision x.yya, section foobar because $reasons. :-)

This is what the v3.10 databook says

"When issuing an End Transfer command, software must set the CmdIOC
bit (field 8) so that an Endpoint Command Complete event is generated
after the transfer ends. This is necessary to synchronize the
conclusion of system bus traffic before the End Transfer command is
completed."

with a note

"If GUCTL2[Rst_actbitlater] is set, Software can poll the completion
of the End Transfer command by polling the command active bit to be
cleared to 0."

fyi.

Rst_actbitlater - "Enable clearing of the command active bit for the
ENDXFER command after the command execution is completed.  This bit is
valid in device mode only."

So I'd prefer not to clear CMDIOC for all cases.

Could we some how just tackle the dwc3_gadget_exit case like I did in
this patch?

if you can send a version that doesn't iterate over all endpoints twice,
sure. We still need a comment somewhere, and I fear we may get
interrupts later in some cases. How would we deal with that?

how about explicitly masking that interrupt? Is it possible?

Other easy option is to use wait_event_interruptible_lock_irq_timeout()
instead of wait_event_lock_irq() in dwc3_gadget_stop().

Is a 200ms timeout sufficient? And after the first timeout we assume all
will timeout so no point in waiting 200ms for each endpoint.