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 <balbi@xxxxxx>
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 <balbi@xxxxxx>
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
Attachment:
signature.asc
Description: Digital signature
