From: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
To: Felipe Balbi <balbi-l0cyMroinI0@public.gmane.org>
Cc: Grygorii Strashko
<grygorii.strashko-l0cyMroinI0@public.gmane.org>,
gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org,
linux-omap-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
"Rafael J. Wysocki"
<rafael.j.wysocki-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>,
Bin Liu <b-liu-l0cyMroinI0@public.gmane.org>,
Heikki Krogerus
<heikki.krogerus-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>,
Andy Shevchenko
<andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>,
Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>,
linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
Subject: Re: MUSB peripheral DMA regression caused by driver core runtime PM change
Date: Wed, 28 Oct 2015 10:14:41 -0700 [thread overview]
Message-ID: <20151028171441.GE3078@atomide.com> (raw)
In-Reply-To: <877fmdcyzi.fsf-HgARHv6XitJaoMGHk7MhZQC/G2K4zDHf@public.gmane.org>
* Felipe Balbi <balbi-l0cyMroinI0@public.gmane.org> [151023 09:48]:
>
> Hi,
>
> Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> writes:
> > From: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
> > Date: Fri, 23 Oct 2015 09:03:22 -0700
> > Subject: [PATCH] usb: musb: omap2430: Fix regression caused by driver core
> > change
> >
> > Commit ddef08dd00f5 ("Driver core: wakeup the parent device before trying
> > probe") started automatically ensuring the parent device is enabled when
> > the child gets probed.
> >
> > This however caused a regression for MUSB omap2430 interface as the
> > runtime PM for the parent device needs the child initialized to access
> > the MUSB hardware registers.
> >
> > Let's delay the enabling of PM runtime for the parent until the child
> > has been properly initialized as suggested in an earlier patch by
> > Grygorii Strashko <grygorii.strashko-l0cyMroinI0@public.gmane.org>.
> >
> > In addition to delaying pm_runtime_enable, we now also need to make sure
> > the parent is enabled during omap2430_musb_init. We also want to propagate
> > an error from omap2430_runtime_resume if struct musb is not initialized.
> >
> > Note that we use pm_runtime_put_noidle here for both the child and parent
> > to prevent an extra runtime_suspend/resume cycle.
> >
> > Let's also add some comments to avoid confusion between the
> > two different devices.
> >
> > Fixes: ddef08dd00f5 ("Driver core: wakeup the parent device before
> > trying probe")
> > Suggested-by: Grygorii Strashko <grygorii.strashko-l0cyMroinI0@public.gmane.org>
> > Signed-off-by: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
>
> I'm fine with this patch to fix this v4.3 regression. Greg, do you want
> a pull request or can you take this in as a patch ? In any case:
>
> Acked-by: Felipe Balbi <balbi-l0cyMroinI0@public.gmane.org>
OK no response for a few days, so I'll pick this up into omap-for-v4.3/fixes
as I have another fix there too.
Regards,
Tony
> > --- a/drivers/usb/musb/omap2430.c
> > +++ b/drivers/usb/musb/omap2430.c
> > @@ -391,9 +391,20 @@ static int omap2430_musb_init(struct musb *musb)
> > }
> > musb->isr = omap2430_musb_interrupt;
> >
> > + /*
> > + * Enable runtime PM for musb parent (this driver). We can't
> > + * do it earlier as struct musb is not yet allocated and we
> > + * need to touch the musb registers for runtime PM.
> > + */
> > + pm_runtime_enable(glue->dev);
> > + status = pm_runtime_get_sync(glue->dev);
> > + if (status < 0)
> > + goto err1;
> > +
> > status = pm_runtime_get_sync(dev);
> > if (status < 0) {
> > dev_err(dev, "pm_runtime_get_sync FAILED %d\n", status);
> > + pm_runtime_put_sync(glue->dev);
> > goto err1;
> > }
> >
> > @@ -426,6 +437,7 @@ static int omap2430_musb_init(struct musb *musb)
> > phy_power_on(musb->phy);
> >
> > pm_runtime_put_noidle(musb->controller);
> > + pm_runtime_put_noidle(glue->dev);
> > return 0;
> >
> > err1:
> > @@ -626,7 +638,11 @@ static int omap2430_probe(struct platform_device *pdev)
> > goto err2;
> > }
> >
> > - pm_runtime_enable(&pdev->dev);
> > + /*
> > + * Note that we cannot enable PM runtime yet for this
> > + * driver as we need struct musb initialized first.
> > + * See omap2430_musb_init above.
> > + */
> >
> > ret = platform_device_add(musb);
> > if (ret) {
> > @@ -675,11 +691,12 @@ static int omap2430_runtime_resume(struct device *dev)
> > struct omap2430_glue *glue = dev_get_drvdata(dev);
> > struct musb *musb = glue_to_musb(glue);
> >
> > - if (musb) {
> > - omap2430_low_level_init(musb);
> > - musb_writel(musb->mregs, OTG_INTERFSEL,
> > - musb->context.otg_interfsel);
> > - }
> > + if (!musb)
> > + return -EPROBE_DEFER;
> > +
> > + omap2430_low_level_init(musb);
> > + musb_writel(musb->mregs, OTG_INTERFSEL,
> > + musb->context.otg_interfsel);
> >
> > return 0;
> > }
>
> --
> balbi
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
WARNING: multiple messages have this Message-ID (diff)
From: tony@atomide.com (Tony Lindgren)
To: linux-arm-kernel@lists.infradead.org
Subject: MUSB peripheral DMA regression caused by driver core runtime PM change
Date: Wed, 28 Oct 2015 10:14:41 -0700 [thread overview]
Message-ID: <20151028171441.GE3078@atomide.com> (raw)
In-Reply-To: <877fmdcyzi.fsf@saruman.tx.rr.com>
* Felipe Balbi <balbi@ti.com> [151023 09:48]:
>
> Hi,
>
> Tony Lindgren <tony@atomide.com> writes:
> > From: Tony Lindgren <tony@atomide.com>
> > Date: Fri, 23 Oct 2015 09:03:22 -0700
> > Subject: [PATCH] usb: musb: omap2430: Fix regression caused by driver core
> > change
> >
> > Commit ddef08dd00f5 ("Driver core: wakeup the parent device before trying
> > probe") started automatically ensuring the parent device is enabled when
> > the child gets probed.
> >
> > This however caused a regression for MUSB omap2430 interface as the
> > runtime PM for the parent device needs the child initialized to access
> > the MUSB hardware registers.
> >
> > Let's delay the enabling of PM runtime for the parent until the child
> > has been properly initialized as suggested in an earlier patch by
> > Grygorii Strashko <grygorii.strashko@ti.com>.
> >
> > In addition to delaying pm_runtime_enable, we now also need to make sure
> > the parent is enabled during omap2430_musb_init. We also want to propagate
> > an error from omap2430_runtime_resume if struct musb is not initialized.
> >
> > Note that we use pm_runtime_put_noidle here for both the child and parent
> > to prevent an extra runtime_suspend/resume cycle.
> >
> > Let's also add some comments to avoid confusion between the
> > two different devices.
> >
> > Fixes: ddef08dd00f5 ("Driver core: wakeup the parent device before
> > trying probe")
> > Suggested-by: Grygorii Strashko <grygorii.strashko@ti.com>
> > Signed-off-by: Tony Lindgren <tony@atomide.com>
>
> I'm fine with this patch to fix this v4.3 regression. Greg, do you want
> a pull request or can you take this in as a patch ? In any case:
>
> Acked-by: Felipe Balbi <balbi@ti.com>
OK no response for a few days, so I'll pick this up into omap-for-v4.3/fixes
as I have another fix there too.
Regards,
Tony
> > --- a/drivers/usb/musb/omap2430.c
> > +++ b/drivers/usb/musb/omap2430.c
> > @@ -391,9 +391,20 @@ static int omap2430_musb_init(struct musb *musb)
> > }
> > musb->isr = omap2430_musb_interrupt;
> >
> > + /*
> > + * Enable runtime PM for musb parent (this driver). We can't
> > + * do it earlier as struct musb is not yet allocated and we
> > + * need to touch the musb registers for runtime PM.
> > + */
> > + pm_runtime_enable(glue->dev);
> > + status = pm_runtime_get_sync(glue->dev);
> > + if (status < 0)
> > + goto err1;
> > +
> > status = pm_runtime_get_sync(dev);
> > if (status < 0) {
> > dev_err(dev, "pm_runtime_get_sync FAILED %d\n", status);
> > + pm_runtime_put_sync(glue->dev);
> > goto err1;
> > }
> >
> > @@ -426,6 +437,7 @@ static int omap2430_musb_init(struct musb *musb)
> > phy_power_on(musb->phy);
> >
> > pm_runtime_put_noidle(musb->controller);
> > + pm_runtime_put_noidle(glue->dev);
> > return 0;
> >
> > err1:
> > @@ -626,7 +638,11 @@ static int omap2430_probe(struct platform_device *pdev)
> > goto err2;
> > }
> >
> > - pm_runtime_enable(&pdev->dev);
> > + /*
> > + * Note that we cannot enable PM runtime yet for this
> > + * driver as we need struct musb initialized first.
> > + * See omap2430_musb_init above.
> > + */
> >
> > ret = platform_device_add(musb);
> > if (ret) {
> > @@ -675,11 +691,12 @@ static int omap2430_runtime_resume(struct device *dev)
> > struct omap2430_glue *glue = dev_get_drvdata(dev);
> > struct musb *musb = glue_to_musb(glue);
> >
> > - if (musb) {
> > - omap2430_low_level_init(musb);
> > - musb_writel(musb->mregs, OTG_INTERFSEL,
> > - musb->context.otg_interfsel);
> > - }
> > + if (!musb)
> > + return -EPROBE_DEFER;
> > +
> > + omap2430_low_level_init(musb);
> > + musb_writel(musb->mregs, OTG_INTERFSEL,
> > + musb->context.otg_interfsel);
> >
> > return 0;
> > }
>
> --
> balbi
next prev parent reply other threads:[~2015-10-28 17:14 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-10-21 23:41 MUSB peripheral DMA regression caused by driver core runtime PM change Tony Lindgren
2015-10-21 23:41 ` Tony Lindgren
[not found] ` <20151021234134.GQ3078-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
2015-10-22 18:02 ` Tony Lindgren
2015-10-22 18:02 ` Tony Lindgren
[not found] ` <20151022180216.GT3078-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
2015-10-22 23:01 ` Tony Lindgren
2015-10-22 23:01 ` Tony Lindgren
[not found] ` <20151022230133.GV3078-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
2015-10-23 12:50 ` Grygorii Strashko
2015-10-23 12:50 ` Grygorii Strashko
[not found] ` <562A2D0D.3060104-l0cyMroinI0@public.gmane.org>
2015-10-23 16:43 ` Tony Lindgren
2015-10-23 16:43 ` Tony Lindgren
[not found] ` <20151023163957.GW3078-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
2015-10-23 16:48 ` Felipe Balbi
2015-10-23 16:48 ` Felipe Balbi
[not found] ` <877fmdcyzi.fsf-HgARHv6XitJaoMGHk7MhZQC/G2K4zDHf@public.gmane.org>
2015-10-23 17:58 ` Grygorii Strashko
2015-10-23 17:58 ` Grygorii Strashko
[not found] ` <562A7549.3070400-l0cyMroinI0@public.gmane.org>
2015-10-23 18:27 ` Alan Stern
2015-10-23 18:27 ` Alan Stern
[not found] ` <Pine.LNX.4.44L0.1510231406200.1644-100000-IYeN2dnnYyZXsRXLowluHWD2FQJk+8+b@public.gmane.org>
2015-10-23 19:20 ` Tony Lindgren
2015-10-23 19:20 ` Tony Lindgren
2015-10-23 20:33 ` Alan Stern
2015-10-23 20:33 ` Alan Stern
2015-10-23 20:36 ` Tony Lindgren
2015-10-23 20:36 ` Tony Lindgren
2015-10-28 17:14 ` Tony Lindgren [this message]
2015-10-28 17:14 ` Tony Lindgren
[not found] ` <20151028171441.GE3078-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
2015-10-28 17:16 ` Felipe Balbi
2015-10-28 17:16 ` Felipe Balbi
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20151028171441.GE3078@atomide.com \
--to=tony-4v6ys6ai5vpbdgjk7y7tuq@public.gmane.org \
--cc=andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA@public.gmane.org \
--cc=b-liu-l0cyMroinI0@public.gmane.org \
--cc=balbi-l0cyMroinI0@public.gmane.org \
--cc=gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org \
--cc=grygorii.strashko-l0cyMroinI0@public.gmane.org \
--cc=heikki.krogerus-VuQAYsv1563Yd54FQh9/CA@public.gmane.org \
--cc=lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org \
--cc=linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org \
--cc=linux-omap-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
--cc=linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
--cc=rafael.j.wysocki-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.