From 592dc3b13388dd8a257776c0312a87f33628e4b0 Mon Sep 17 00:00:00 2001 From: Sriharsha Allenki Date: Tue, 23 Jun 2020 15:27:10 +0530 Subject: [PATCH] usb: dwc3: Handle race between composition switch and cable disconnect The current implementation of handling run/~stop of DCTL based on state of VBUS or the pullup call from the udc core can result in a below race condition where the run/~stop is cleared twice leading to an invalid state of controller. Thread -1 Thread-2 vbus_session (1) spin_lock no run_stop(1) call pullup(1) spin_unlock softconnect = 1 vbus_active checked vbus_session (0) spin_lock run_stop(0) called spin_unlock Prevent this by ensuring a proper state transitions and clear run/~stop only when when there is a transition from ACTIVE and there is an pullup(0) event or vbus_session(0) call. Change-Id: I03c4acfc2c6697e02c510d079c7d48d9a82bb322 Signed-off-by: Sriharsha Allenki Signed-off-by: Pratham Pratap --- drivers/usb/dwc3/core.h | 8 ++++ drivers/usb/dwc3/gadget.c | 86 +++++++++++++++++++++++++++++++-------- 2 files changed, 78 insertions(+), 16 deletions(-) diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h index 361b86c90a75..f7a437ed2ac7 100644 --- a/drivers/usb/dwc3/core.h +++ b/drivers/usb/dwc3/core.h @@ -850,6 +850,13 @@ enum dwc3_link_state { DWC3_LINK_STATE_MASK = 0x0f, }; +enum gadget_state { + DWC3_GADGET_INACTIVE, + DWC3_GADGET_SOFT_CONN, + DWC3_GADGET_CABLE_CONN, + DWC3_GADGET_ACTIVE, +}; + /* TRB Length, PCM and Status */ #define DWC3_TRB_SIZE_MASK (0x00ffffff) #define DWC3_TRB_SIZE_LENGTH(n) ((n) & DWC3_TRB_SIZE_MASK) @@ -1382,6 +1389,7 @@ struct dwc3 { unsigned int irq_event_count[MAX_INTR_STATS]; unsigned int irq_dbg_index; + enum gadget_state gadget_state; /* Indicate if the gadget was powered by the otg driver */ unsigned int vbus_active:1; /* Indicate if software connect was issued by the usb_gadget_driver */ diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index 834f286cf79d..37a261218e8c 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -2309,6 +2309,71 @@ static int dwc3_gadget_run_stop(struct dwc3 *dwc, int is_on, int suspend) return 0; } +static int dwc3_gadget_run_stop_util(struct dwc3 *dwc) +{ + int ret = 0; + + dev_dbg(dwc->dev, "%s: enter: %d\n", __func__, dwc->gadget_state); + switch (dwc->gadget_state) { + case DWC3_GADGET_INACTIVE: + if (dwc->vbus_active && dwc->softconnect) { + ret = dwc3_gadget_run_stop(dwc, true, false); + dwc->gadget_state = DWC3_GADGET_ACTIVE; + break; + } + + if (dwc->vbus_active) { + dwc->gadget_state = DWC3_GADGET_CABLE_CONN; + break; + } + + if (dwc->softconnect) { + dwc->gadget_state = DWC3_GADGET_SOFT_CONN; + break; + } + case DWC3_GADGET_SOFT_CONN: + if (!dwc->softconnect) { + dwc->gadget_state = DWC3_GADGET_INACTIVE; + break; + } + + if (dwc->vbus_active) { + ret = dwc3_gadget_run_stop(dwc, true, false); + dwc->gadget_state = DWC3_GADGET_ACTIVE; + } + break; + case DWC3_GADGET_CABLE_CONN: + if (!dwc->vbus_active) { + dwc->gadget_state = DWC3_GADGET_INACTIVE; + break; + } + + if (dwc->softconnect) { + ret = dwc3_gadget_run_stop(dwc, true, false); + dwc->gadget_state = DWC3_GADGET_ACTIVE; + } + break; + case DWC3_GADGET_ACTIVE: + if (!dwc->vbus_active) { + dwc->gadget_state = DWC3_GADGET_SOFT_CONN; + ret = dwc3_gadget_run_stop(dwc, false, false); + break; + } + + if (!dwc->softconnect) { + dwc->gadget_state = DWC3_GADGET_CABLE_CONN; + ret = dwc3_gadget_run_stop(dwc, false, false); + break; + } + break; + default: + dev_err(dwc->dev, "Invalid state\n"); + } + + dev_dbg(dwc->dev, "%s: exit: %d\n", __func__, dwc->gadget_state); + return ret; +} + static int dwc3_gadget_vbus_draw(struct usb_gadget *g, unsigned int mA) { struct dwc3 *dwc = gadget_to_dwc(g); @@ -2328,6 +2393,7 @@ static int dwc3_gadget_pullup(struct usb_gadget *g, int is_on) ktime_t diff; is_on = !!is_on; + spin_lock_irqsave(&dwc->lock, flags); dwc->softconnect = is_on; if (((dwc->dr_mode > USB_DR_MODE_HOST) && !dwc->vbus_active) @@ -2337,9 +2403,11 @@ static int dwc3_gadget_pullup(struct usb_gadget *g, int is_on) * Need to wait for vbus_session(on) from otg driver or to * the udc_start. */ + spin_unlock_irqrestore(&dwc->lock, flags); dbg_event(0xFF, "WaitPullup", 0); return 0; } + spin_unlock_irqrestore(&dwc->lock, flags); pm_runtime_get_sync(dwc->dev); dbg_event(0xFF, "Pullup gsync", @@ -2393,7 +2461,7 @@ static int dwc3_gadget_pullup(struct usb_gadget *g, int is_on) dwc->b_suspend = false; dwc3_notify_event(dwc, DWC3_CONTROLLER_NOTIFY_OTG_EVENT, 0); - ret = dwc3_gadget_run_stop(dwc, is_on, false); + ret = dwc3_gadget_run_stop_util(dwc); spin_unlock_irqrestore(&dwc->lock, flags); if (!is_on && ret == -ETIMEDOUT) { /* @@ -2514,21 +2582,7 @@ static int dwc3_gadget_vbus_session(struct usb_gadget *_gadget, int is_active) /* Mark that the vbus was powered */ dwc->vbus_active = is_active; - /* - * Check if upper level usb_gadget_driver was already registered with - * this udc controller driver (if dwc3_gadget_start was called) - */ - if (dwc->gadget_driver && dwc->softconnect) { - if (dwc->vbus_active) { - /* - * Both vbus was activated by otg and pullup was - * signaled by the gadget driver. - */ - ret = dwc3_gadget_run_stop(dwc, 1, false); - } else { - ret = dwc3_gadget_run_stop(dwc, 0, false); - } - } + ret = dwc3_gadget_run_stop_util(dwc); /* * Clearing run/stop bit might occur before disconnect event is seen.