On Wed, Feb 15, 2012 at 06:57:01PM -0800, Paul Zimmerman wrote:
> Some existing routines need to be refactored for hibernation. This
> includes event buffer setup, restoring saved state when an EP is
> configured, stopping a transfer without the FORCERM bit set, and
> stopping a transfer on EP0-OUT.
>
> In addition, some routines need to be made non-static, so that the
> hibernation code, which will live in a separate source file, can
> call them. In particular, this requires splitting dwc3_gadget_start
> into two functions, a driver-private one that just starts the
> hardware, and full-function one that is called through the gadget
> API.
>
> Signed-off-by: Paul Zimmerman <paulz@xxxxxxxxxxxx>
> ---
> drivers/usb/dwc3/core.c | 5 ++-
> drivers/usb/dwc3/core.h | 4 ++
> drivers/usb/dwc3/gadget.c | 116 ++++++++++++++++++++++++++------------------
> drivers/usb/dwc3/gadget.h | 8 +++
> 4 files changed, 84 insertions(+), 49 deletions(-)
>
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index 7124ec0..829c0d2 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -256,7 +256,7 @@ static int __devinit dwc3_alloc_event_buffers(struct dwc3 *dwc, unsigned length)
> *
> * Returns 0 on success otherwise negative errno.
> */
> -static int __devinit dwc3_event_buffers_setup(struct dwc3 *dwc)
> +int dwc3_event_buffers_setup(struct dwc3 *dwc)
> {
> struct dwc3_event_buffer *evt;
> int n;
> @@ -266,6 +266,7 @@ static int __devinit dwc3_event_buffers_setup(struct dwc3 *dwc)
> dev_dbg(dwc->dev, "Event buf %p dma %08llx length %d\n",
> evt->buf, (unsigned long long) evt->dma,
> evt->length);
> + evt->lpos = 0;
Why ? the events are zero-initialized anyway.
> dwc3_writel(dwc->regs, DWC3_GEVNTADRLO(n),
> lower_32_bits(evt->dma));
> @@ -286,6 +287,8 @@ static void dwc3_event_buffers_cleanup(struct dwc3 *dwc)
>
> for (n = 0; n < dwc->num_event_buffers; n++) {
> evt = dwc->ev_buffs[n];
> + evt->lpos = 0;
why ? this will only be called right before freeing the event buffer
memory.
> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
> index 87dab09..b300c30 100644
> --- a/drivers/usb/dwc3/core.h
> +++ b/drivers/usb/dwc3/core.h
> @@ -395,6 +395,7 @@ struct dwc3_event_buffer {
> * @type: set to bmAttributes & USB_ENDPOINT_XFERTYPE_MASK
> * @res_trans_idx: Resource transfer index
> * @interval: the intervall on which the ISOC transfer is started
> + * @saved_state: ep state saved during hibernation
> * @name: a human readable name e.g. ep1out-bulk
> * @direction: true for TX, false for RX
> * @stream_capable: true when streams are enabled
> @@ -428,6 +429,7 @@ struct dwc3_ep {
> u8 type;
> u8 res_trans_idx;
> u32 interval;
> + u32 saved_state;
>
> char name[20];
>
> @@ -838,6 +840,8 @@ void dwc3_host_exit(struct dwc3 *dwc);
> int dwc3_gadget_init(struct dwc3 *dwc);
> void dwc3_gadget_exit(struct dwc3 *dwc);
>
> +int dwc3_event_buffers_setup(struct dwc3 *dwc);
> +
> extern int dwc3_get_device_id(void);
> extern void dwc3_put_device_id(int id);
>
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index fdaef9a..93eb673 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -495,7 +495,8 @@ static int dwc3_gadget_start_config(struct dwc3 *dwc, struct dwc3_ep *dep)
>
> static int dwc3_gadget_set_ep_config(struct dwc3 *dwc, struct dwc3_ep *dep,
> const struct usb_endpoint_descriptor *desc,
> - const struct usb_ss_ep_comp_descriptor *comp_desc)
> + const struct usb_ss_ep_comp_descriptor *comp_desc,
> + bool restore)
> {
> struct dwc3_gadget_ep_cmd_params params;
>
> @@ -537,6 +538,11 @@ static int dwc3_gadget_set_ep_config(struct dwc3 *dwc, struct dwc3_ep *dep,
> dep->interval = 1 << (desc->bInterval - 1);
> }
>
> + if (restore) {
> + params.param0 |= DWC3_DEPCFG_ACTION_RESTORE;
> + params.param2 = dep->saved_state;
> + }
> +
> return dwc3_send_gadget_ep_cmd(dwc, dep->number,
> DWC3_DEPCMD_SETEPCONFIG, ¶ms);
> }
> @@ -560,9 +566,10 @@ static int dwc3_gadget_set_xfer_resource(struct dwc3 *dwc, struct dwc3_ep *dep)
> *
> * Caller should take care of locking
> */
> -static int __dwc3_gadget_ep_enable(struct dwc3_ep *dep,
> +int __dwc3_gadget_ep_enable(struct dwc3_ep *dep,
> const struct usb_endpoint_descriptor *desc,
> - const struct usb_ss_ep_comp_descriptor *comp_desc)
> + const struct usb_ss_ep_comp_descriptor *comp_desc,
> + bool restore)
> {
> struct dwc3 *dwc = dep->dwc;
> u32 reg;
> @@ -574,7 +581,7 @@ static int __dwc3_gadget_ep_enable(struct dwc3_ep *dep,
> return ret;
> }
>
> - ret = dwc3_gadget_set_ep_config(dwc, dep, desc, comp_desc);
> + ret = dwc3_gadget_set_ep_config(dwc, dep, desc, comp_desc, restore);
> if (ret)
> return ret;
>
> @@ -614,13 +621,12 @@ static int __dwc3_gadget_ep_enable(struct dwc3_ep *dep,
> return 0;
> }
>
> -static void dwc3_stop_active_transfer(struct dwc3 *dwc, u32 epnum);
> static void dwc3_remove_requests(struct dwc3 *dwc, struct dwc3_ep *dep)
> {
> struct dwc3_request *req;
>
> if (!list_empty(&dep->req_queued))
> - dwc3_stop_active_transfer(dwc, dep->number);
> + dwc3_stop_active_transfer(dwc, dep->number, true);
>
> while (!list_empty(&dep->request_list)) {
> req = next_request(&dep->request_list);
> @@ -720,7 +726,7 @@ static int dwc3_gadget_ep_enable(struct usb_ep *ep,
> dev_vdbg(dwc->dev, "Enabling %s\n", dep->name);
>
> spin_lock_irqsave(&dwc->lock, flags);
> - ret = __dwc3_gadget_ep_enable(dep, desc, ep->comp_desc);
> + ret = __dwc3_gadget_ep_enable(dep, desc, ep->comp_desc, false);
> spin_unlock_irqrestore(&dwc->lock, flags);
>
> return ret;
> @@ -1161,7 +1167,7 @@ static int dwc3_gadget_ep_dequeue(struct usb_ep *ep,
> }
> if (r == req) {
> /* wait until it is processed */
> - dwc3_stop_active_transfer(dwc, dep->number);
> + dwc3_stop_active_transfer(dwc, dep->number, true);
> goto out0;
> }
> dev_err(dwc->dev, "request %p was not queued to %s\n",
> @@ -1396,7 +1402,7 @@ static int dwc3_gadget_set_selfpowered(struct usb_gadget *g,
> return 0;
> }
>
> -static void dwc3_gadget_run_stop(struct dwc3 *dwc, int is_on)
> +void dwc3_gadget_run_stop(struct dwc3 *dwc, int is_on)
> {
> u32 reg;
> u32 timeout = 500;
> @@ -1451,47 +1457,24 @@ static int dwc3_gadget_pullup(struct usb_gadget *g, int is_on)
> return 0;
> }
>
> -static int dwc3_gadget_start(struct usb_gadget *g,
> - struct usb_gadget_driver *driver)
> +int __dwc3_gadget_start(struct dwc3 *dwc, bool restore)
I still don't see the need to expose this to another source file. Just
call this from your runtime_resume method, maybe that's what you're
doing but runtime_resume was defined on core.c... I will soo figure it
out, but even in that case, I believe exposing gadget_stat is the wrong
way to handle things, maybe you need to expose a new API:
dwc3_gadget_runtime_suspend()
dwc3_gadget_runtime_resume()
and call those from core.c's runtime_suspend/runtime_resume
respectively if core is configured as DRD or Device modes. Similarly, we
will need a set of hooks for host, I guess.
> {
> - struct dwc3 *dwc = gadget_to_dwc(g);
> - struct dwc3_ep *dep;
> - unsigned long flags;
> - int ret = 0;
> - u32 reg;
> -
> - spin_lock_irqsave(&dwc->lock, flags);
> -
> - if (dwc->gadget_driver) {
> - dev_err(dwc->dev, "%s is already bound to %s\n",
> - dwc->gadget.name,
> - dwc->gadget_driver->driver.name);
> - ret = -EBUSY;
> - goto err0;
> - }
> -
> - dwc->gadget_driver = driver;
> - dwc->gadget.dev.driver = &driver->driver;
> -
> - reg = dwc3_readl(dwc->regs, DWC3_DCFG);
> - reg &= ~(DWC3_DCFG_SPEED_MASK);
> - reg |= dwc->maximum_speed;
> - dwc3_writel(dwc->regs, DWC3_DCFG, reg);
> + struct dwc3_ep *dep;
> + int ret;
>
> dwc->start_config_issued = false;
>
> - /* Start with SuperSpeed Default */
> - dwc3_gadget_ep0_desc.wMaxPacketSize = cpu_to_le16(512);
> -
> dep = dwc->eps[0];
> - ret = __dwc3_gadget_ep_enable(dep, &dwc3_gadget_ep0_desc, NULL);
> + ret = __dwc3_gadget_ep_enable(dep, &dwc3_gadget_ep0_desc, NULL,
> + restore);
> if (ret) {
> dev_err(dwc->dev, "failed to enable %s\n", dep->name);
> goto err0;
> }
>
> dep = dwc->eps[1];
> - ret = __dwc3_gadget_ep_enable(dep, &dwc3_gadget_ep0_desc, NULL);
> + ret = __dwc3_gadget_ep_enable(dep, &dwc3_gadget_ep0_desc, NULL,
> + restore);
> if (ret) {
> dev_err(dwc->dev, "failed to enable %s\n", dep->name);
> goto err1;
> @@ -1501,14 +1484,47 @@ static int dwc3_gadget_start(struct usb_gadget *g,
> dwc->ep0state = EP0_SETUP_PHASE;
> dwc3_ep0_out_start(dwc);
>
> - spin_unlock_irqrestore(&dwc->lock, flags);
> -
> return 0;
>
> err1:
> __dwc3_gadget_ep_disable(dwc->eps[0]);
>
> err0:
> + return ret;
> +}
> +
> +static int dwc3_gadget_start(struct usb_gadget *g,
> + struct usb_gadget_driver *driver)
> +{
> + struct dwc3 *dwc = gadget_to_dwc(g);
> + unsigned long flags;
> + u32 reg;
> + int ret;
> +
> + spin_lock_irqsave(&dwc->lock, flags);
> +
> + if (dwc->gadget_driver) {
> + dev_err(dwc->dev, "%s is already bound to %s\n",
> + dwc->gadget.name,
> + dwc->gadget_driver->driver.name);
> + ret = -EBUSY;
> + goto out;
> + }
> +
> + dwc->gadget_driver = driver;
> + dwc->gadget.dev.driver = &driver->driver;
> +
> + /* Start with SuperSpeed Default */
> + dwc3_gadget_ep0_desc.wMaxPacketSize = cpu_to_le16(512);
> +
> + reg = dwc3_readl(dwc->regs, DWC3_DCFG);
> + reg &= ~DWC3_DCFG_SPEED_MASK;
> + reg |= dwc->maximum_speed;
> + dwc3_writel(dwc->regs, DWC3_DCFG, reg);
> +
> + ret = __dwc3_gadget_start(dwc, false);
> +
> +out:
> spin_unlock_irqrestore(&dwc->lock, flags);
>
> return ret;
> @@ -1882,7 +1898,7 @@ static void dwc3_disconnect_gadget(struct dwc3 *dwc)
> }
> }
>
> -static void dwc3_stop_active_transfer(struct dwc3 *dwc, u32 epnum)
> +void dwc3_stop_active_transfer(struct dwc3 *dwc, u32 epnum, bool force)
> {
> struct dwc3_ep *dep;
> struct dwc3_gadget_ep_cmd_params params;
> @@ -1891,10 +1907,14 @@ static void dwc3_stop_active_transfer(struct dwc3 *dwc, u32 epnum)
>
> dep = dwc->eps[epnum];
>
> - WARN_ON(!dep->res_trans_idx);
> - if (dep->res_trans_idx) {
> + if (epnum != 0)
> + WARN_ON(!dep->res_trans_idx);
> +
> + if (dep->res_trans_idx || epnum == 0) {
> cmd = DWC3_DEPCMD_ENDTRANSFER;
> - cmd |= DWC3_DEPCMD_HIPRI_FORCERM | DWC3_DEPCMD_CMDIOC;
> + if (force)
> + cmd |= DWC3_DEPCMD_HIPRI_FORCERM;
> + cmd |= DWC3_DEPCMD_CMDIOC;
> cmd |= DWC3_DEPCMD_PARAM(dep->res_trans_idx);
> memset(¶ms, 0, sizeof(params));
> ret = dwc3_send_gadget_ep_cmd(dwc, dep->number, cmd, ¶ms);
> @@ -2097,7 +2117,7 @@ static void dwc3_gadget_suspend_phy(struct dwc3 *dwc, u8 speed)
> }
> }
>
> -static void dwc3_gadget_conndone_interrupt(struct dwc3 *dwc)
> +void dwc3_gadget_conndone_interrupt(struct dwc3 *dwc)
> {
> struct dwc3_gadget_ep_cmd_params params;
> struct dwc3_ep *dep;
> @@ -2162,14 +2182,14 @@ static void dwc3_gadget_conndone_interrupt(struct dwc3 *dwc)
> }
>
> dep = dwc->eps[0];
> - ret = __dwc3_gadget_ep_enable(dep, &dwc3_gadget_ep0_desc, NULL);
> + ret = __dwc3_gadget_ep_enable(dep, &dwc3_gadget_ep0_desc, NULL, false);
> if (ret) {
> dev_err(dwc->dev, "failed to enable %s\n", dep->name);
> return;
> }
>
> dep = dwc->eps[1];
> - ret = __dwc3_gadget_ep_enable(dep, &dwc3_gadget_ep0_desc, NULL);
> + ret = __dwc3_gadget_ep_enable(dep, &dwc3_gadget_ep0_desc, NULL, false);
> if (ret) {
> dev_err(dwc->dev, "failed to enable %s\n", dep->name);
> return;
> diff --git a/drivers/usb/dwc3/gadget.h b/drivers/usb/dwc3/gadget.h
> index d668cb1..a962d53 100644
> --- a/drivers/usb/dwc3/gadget.h
> +++ b/drivers/usb/dwc3/gadget.h
> @@ -114,10 +114,18 @@ void dwc3_ep0_interrupt(struct dwc3 *dwc,
> void dwc3_ep0_out_start(struct dwc3 *dwc);
> int dwc3_gadget_ep0_queue(struct usb_ep *ep, struct usb_request *request,
> gfp_t gfp_flags);
> +int __dwc3_gadget_ep_enable(struct dwc3_ep *dep,
> + const struct usb_endpoint_descriptor *desc,
> + const struct usb_ss_ep_comp_descriptor *comp_desc,
> + bool restore);
> int __dwc3_gadget_ep_set_halt(struct dwc3_ep *dep, int value);
> int dwc3_send_gadget_ep_cmd(struct dwc3 *dwc, unsigned ep,
> unsigned cmd, struct dwc3_gadget_ep_cmd_params *params);
> int dwc3_send_gadget_dev_cmd(struct dwc3 *dwc, unsigned cmd, u32 param);
> +void dwc3_gadget_run_stop(struct dwc3 *dwc, int is_on);
This is also something I think doesn't need to be exposed. You should
save what's on DCTL instead, no ? That way, if we hibernated without
being connected to anything, we don't try to set run_stop bit when we
resume.
--
balbi
Attachment:
signature.asc
Description: Digital signature
