On 16/03/16 15:14, Felipe Balbi wrote:
> Roger Quadros <rogerq@xxxxxx> writes:
>
>> [ text/plain ]
>> The existing workaround of forcing DEVSPD to SUPER_SPEED
>> for HIGH_SPEED ports is causing another side effect
>> which causes erratic interrupts and delayed gadget
>> enumeration of upto 2 seconds.
>>
>> Work around the run/stop issue by detecting if
>> it happened using debug LTSSM state and issuing
>> soft reset to the device controller when changing RUN_STOP
>> from 0 to 1.
>>
>> We apply the workaround only if PRTCAP is DEVICE mode
>> as we don't yet support this workaround in OTG mode.
>>
>> Use USB RESET event as exit condition for workaround.
>>
>> Signed-off-by: Roger Quadros <rogerq@xxxxxx>
>> ---
>> drivers/usb/dwc3/core.h | 1 +
>> drivers/usb/dwc3/gadget.c | 175 +++++++++++++++++++++++++++++++++++++---------
>> 2 files changed, 144 insertions(+), 32 deletions(-)
>>
>> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
>> index 2bea1ac..a724c0d 100644
>> --- a/drivers/usb/dwc3/core.h
>> +++ b/drivers/usb/dwc3/core.h
>> @@ -762,6 +762,7 @@ struct dwc3 {
>>
>> struct usb_gadget gadget;
>> struct usb_gadget_driver *gadget_driver;
>> + struct completion reset_event; /* used for run/stop workaround */
>
> the fact that you needed a struct completion here already points to the
> fact this is really, really bad :-)
>
>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
>> index 3ac170f..03418b8 100644
>> --- a/drivers/usb/dwc3/gadget.c
>> +++ b/drivers/usb/dwc3/gadget.c
>> @@ -35,6 +35,9 @@
>> #include "gadget.h"
>> #include "io.h"
>>
>> +static void dwc3_gadget_disable_irq(struct dwc3 *dwc);
>> +static int dwc3_gadget_restart(struct dwc3 *dwc);
>> +
>> /**
>> * dwc3_gadget_set_test_mode - Enables USB2 Test Modes
>> * @dwc: pointer to our context structure
>> @@ -1570,13 +1573,100 @@ static int dwc3_gadget_pullup(struct usb_gadget *g, int is_on)
>> struct dwc3 *dwc = gadget_to_dwc(g);
>> unsigned long flags;
>> int ret;
>> + int trys = 0;
>>
>> is_on = !!is_on;
>>
>> + if (is_on)
>> + reinit_completion(&dwc->reset_event);
>> +
>> spin_lock_irqsave(&dwc->lock, flags);
>> ret = dwc3_gadget_run_stop(dwc, is_on, false);
>> spin_unlock_irqrestore(&dwc->lock, flags);
>>
>> +try:
>> + /**
>> + * WORKAROUND: DWC3 revision < 2.20a have an issue
>> + * which would cause metastability state on Run/Stop
>> + * bit if we try to force the IP to USB2-only mode.
>> + *
>> + * Because of that, we check if we hit that issue and
>> + * reset core and retry if we did.
>> + *
>> + * We only attempt this workaround if we are in
>> + * DEVICE mode (i.e. not OTG).
>> + *
>> + * Refers to:
>> + *
>> + * STAR#9000525659: Clock Domain Crossing on DCTL in
>> + * USB 2.0 Mode
>> + */
>> + if (is_on && dwc->revision < DWC3_REVISION_220A &&
>> + dwc->prtcap_mode == DWC3_GCTL_PRTCAP_DEVICE) {
>> + u32 devspd, ltssm;
>> + unsigned long t;
>> +
>> + /* only applicable if devspd != SUPERSPEED */
>> + devspd = dwc3_readl(dwc->regs, DWC3_DCFG) & DWC3_DCFG_SPEED_MASK;
>> + if (devspd == DWC3_DCFG_SUPERSPEED)
>> + goto done;
>> +
>> + /* get link state */
>> + ltssm = dwc3_readl(dwc->regs, DWC3_GDBGLTSSM);
>> + ltssm = (ltssm >> 22) & 0xf;
>> +
>> + /**
>> + * Need to wait for 100ms and check if ltssm != 4 to detect
>> + * metastability issue. If we got a reset event then we are
>> + * safe and can continue.
>> + */
>> + t = wait_for_completion_timeout(&dwc->reset_event,
>> + msecs_to_jiffies(100));
>> + if (t)
>> + goto done;
>> +
>> + /**
>> + * If link state != 4 we've hit the metastability issue, soft reset.
>> + */
>> + if (ltssm == 4)
>> + goto done;
>> +
>> + dwc3_trace(trace_dwc3_gadget,
>> + "applying metastability workaround\n");
>> + trys++;
>> + if (trys == 2) {
>> + dev_WARN_ONCE(dwc->dev, true,
>> + "metastability workaround failed!\n");
>> + return -ETIMEDOUT;
>> + }
>> +
>> + spin_lock_irqsave(&dwc->lock, flags);
>> + /* stop gadget */
>> + dwc3_gadget_disable_irq(dwc);
>> + __dwc3_gadget_ep_disable(dwc->eps[0]);
>> + __dwc3_gadget_ep_disable(dwc->eps[1]);
>> +
>> + /* soft reset device and restart */
>> + ret = dwc3_device_reinit(dwc);
>> + if (ret) {
>> + dev_err(dwc->dev, "device reinit failed\n");
>> + return ret;
>> + }
>> +
>> + reinit_completion(&dwc->reset_event);
>> + /* restart gadget */
>> + ret = dwc3_gadget_restart(dwc);
>> + if (ret) {
>> + dev_err(dwc->dev, "failed to re-init gadget\n");
>> + return ret;
>> + }
>> +
>> + spin_unlock_irqrestore(&dwc->lock, flags);
>> + goto try;
>> + }
>
> holy crap dude ?!? Now ->pullup() has to reinitialize dwc3 completely ?
>
Only if it hits the error state. We're only re-initializing the device controller
not complete dwc3.
cheers,
-roger
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
