Hi,
On Fri, Oct 12, 2012 at 02:29:39PM +0530, Vikas Sajjan wrote:
> > > @@ -339,49 +390,9 @@ static int __devinit dwc3_core_init(struct dwc3
> > > *dwc)
> > > }
> > > dwc->revision = reg;
> > >
> > > - /* issue device SoftReset too */
> > > - timeout = jiffies + msecs_to_jiffies(500);
> > > - dwc3_writel(dwc->regs, DWC3_DCTL, DWC3_DCTL_CSFTRST);
> > > - do {
> > > - reg = dwc3_readl(dwc->regs, DWC3_DCTL);
> > > - if (!(reg & DWC3_DCTL_CSFTRST))
> > > - break;
> > > -
> > > - if (time_after(jiffies, timeout)) {
> > > - dev_err(dwc->dev, "Reset Timed Out\n");
> > > - ret = -ETIMEDOUT;
> > > - goto err0;
> > > - }
> > > -
> > > - cpu_relax();
> > > - } while (true);
> > > -
> > > - dwc3_core_soft_reset(dwc);
> > > -
> > > - dwc3_cache_hwparams(dwc);
> > > -
> > > - reg = dwc3_readl(dwc->regs, DWC3_GCTL);
> > > - reg &= ~DWC3_GCTL_SCALEDOWN_MASK;
> > > - reg &= ~DWC3_GCTL_DISSCRAMBLE;
> > > -
> > > - switch (DWC3_GHWPARAMS1_EN_PWROPT(dwc->hwparams.hwparams1)) {
> > > - case DWC3_GHWPARAMS1_EN_PWROPT_CLK:
> > > - reg &= ~DWC3_GCTL_DSBLCLKGTNG;
> > > - break;
> > > - default:
> > > - dev_dbg(dwc->dev, "No power optimization available\n");
> > > - }
> > > -
> > > - /*
> > > - * WORKAROUND: DWC3 revisions <1.90a have a bug
> > > - * where the device can fail to connect at SuperSpeed
> > > - * and falls back to high-speed mode which causes
> > > - * the device to enter a Connect/Disconnect loop
> > > - */
> > > - if (dwc->revision < DWC3_REVISION_190A)
> > > - reg |= DWC3_GCTL_U2RSTECN;
> > > -
> > > - dwc3_writel(dwc->regs, DWC3_GCTL, reg);
> > > + ret = dwc3_core_reset(dwc);
> > > + if (ret < 0)
> > > + goto err0;
> > >
> > > ret = dwc3_alloc_event_buffers(dwc, DWC3_EVENT_BUFFERS_SIZE);
> >
> > I would rather move dwc3_alloc_event_buffers() outside of
> > dwc3_core_init() and use that directly on ->resume(). dwc3_core_init()
> > shouldn't be doing any memory allocations, so we can re-use it, and it
> > doesn't matter when we allocate the event buffers anyway.
> >
> dwc3_alloc_event_buffers() was already inside the dwc3_core_init() in
> the existing code, the only change I made is, a common function called
> dwc3_core_reset() is added which will be used both in probe and
> resume.
> so basically resume only does dwc3_core_reset() and
> dwc3_event_buffers_setup() as dwc3_alloc_event_buffers() is done in
> probe.we already have too many functions call *reset*, adding another one will just make code more difficult to follow and it will be error-prone. This is what I suggested: commit 9aba985696690c1e8c2cf4f34e35b3f5b296175d Author: Felipe Balbi <[email protected]> Date: Thu Oct 11 13:54:36 2012 +0300 usb: dwc3: core: move event buffer allocation out of dwc3_core_init() This patch is in preparation for adding PM support dwc3 driver. We want to re-use dwc3_core_init and dwc3_core_exit() functions on resume() and suspend() callbacks respectively. Moving even buffer allocation away from dwc3_core_init() will allow us to reuse the event buffer which was allocated long ago on our probe() routine. Signed-off-by: Felipe Balbi <[email protected]> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c index 8d543ea..b923183 100644 --- a/drivers/usb/dwc3/core.c +++ b/drivers/usb/dwc3/core.c @@ -381,24 +381,14 @@ static int __devinit dwc3_core_init(struct dwc3 *dwc) dwc3_writel(dwc->regs, DWC3_GCTL, reg); - ret = dwc3_alloc_event_buffers(dwc, DWC3_EVENT_BUFFERS_SIZE); - if (ret) { - dev_err(dwc->dev, "failed to allocate event buffers\n"); - ret = -ENOMEM; - goto err1; - } - ret = dwc3_event_buffers_setup(dwc); if (ret) { dev_err(dwc->dev, "failed to setup event buffers\n"); - goto err1; + goto err0; } return 0; -err1: - dwc3_free_event_buffers(dwc); - err0: return ret; } @@ -406,7 +396,6 @@ err0: static void dwc3_core_exit(struct dwc3 *dwc) { dwc3_event_buffers_cleanup(dwc); - dwc3_free_event_buffers(dwc); } #define DWC3_ALIGN_MASK (16 - 1) @@ -509,10 +498,17 @@ static int __devinit dwc3_probe(struct platform_device *pdev) pm_runtime_get_sync(dev); pm_runtime_forbid(dev); + ret = dwc3_alloc_event_buffers(dwc, DWC3_EVENT_BUFFERS_SIZE); + if (ret) { + dev_err(dwc->dev, "failed to allocate event buffers\n"); + ret = -ENOMEM; + goto err0; + } + ret = dwc3_core_init(dwc); if (ret) { dev_err(dev, "failed to initialize core\n"); - return ret; + goto err0; } mode = DWC3_MODE(dwc->hwparams.hwparams0); @@ -584,6 +580,9 @@ err2: err1: dwc3_core_exit(dwc); +err0: + dwc3_free_event_buffers(dwc); + return ret; } please write your patches on top of this change. I will push it to my kernel.org tree, branch dwc3. Note that it will still rebase that branch on top of v3.7-rc1 once it's out, I'm pushing that patch now so you can use it on your PM changes. Note that it will still rebase that branch on top of v3.7-rc1 once it's out, I'm pushing that patch now so you can use it on your PM changes. Note that it will still rebase that branch on top of v3.7-rc1 once it's out, I'm pushing that patch now so you can use it on your PM changes. -- balbi
signature.asc
Description: Digital signature
