* [RESEND PATCH v6 0/4] usb: musb: da8xx: Add DMA support @ 2017-03-24 14:35 Alexandre Bailon [not found] ` <20170324143600.4704-1-abailon-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> 0 siblings, 1 reply; 12+ messages in thread From: Alexandre Bailon @ 2017-03-24 14:35 UTC (permalink / raw) To: b-liu-l0cyMroinI0 Cc: linux-usb-u79uwXL29TY76Z2rM5mHXA, ptitiano-rdvid1DuHRBWk0Htik3J/w, khilman-rdvid1DuHRBWk0Htik3J/w, nsekhar-l0cyMroinI0, tony-4v6yS6AI5VpBDgjK7y7TUQ, sergei.shtylyov-M4DtvfQ/ZS1MRgGoP+s0PdBPR1lH4CV8, linux-omap-u79uwXL29TY76Z2rM5mHXA, Alexandre Bailon This series update MUSB driver to add DMA support to DA8xx. It should be applied on top of: "[PATCH v4 0/3] dmaengine: cppi41: Add dma support to da8xx" and "[PATCH v3] ARM: davinci: Make the usb20 clock available to PM runtime" Changes in v6: - Rebased on linux-next. Some patches were conflicting with a series adding suspend and resume support. Changes in v5: - Fix a typo in commit message of patch 4 Changes in v4: - Update and clarify the commit message of patch 5 - Fix the typo in patch 3 Changes in v3: - Remove PM runtime callbacks. I have update arch/arm/mach-davinci/pm_domain.c to let PM runtime control the usb20 clock. - Only use PM runtime sync operation. Changes in v2: - Clock and IRQ management has been moved to MUSB DA8xx glue (was in CPPI 4.1 driver) - I have added a partial support PM runtime. The goal was to use PM runtime to manage clock of MUSB and CPPI 4.1 (they use the same clock). - CPPI 4.1 is now achild of MUSB DA8xx glue. Alexandre Bailon (4): usb: musb: Use shared irq usb: musb: Add support of CPPI 4.1 DMA controller to DA8xx usb: musb: musb_cppi41: Workaround DMA stall issue during teardown usb: musb: da8xx: Add a primary support of PM runtime drivers/usb/musb/Kconfig | 4 +-- drivers/usb/musb/da8xx.c | 63 ++++++++++++++++++++++++++++-------------- drivers/usb/musb/musb_core.c | 2 +- drivers/usb/musb/musb_core.h | 1 + drivers/usb/musb/musb_cppi41.c | 4 +++ 5 files changed, 51 insertions(+), 23 deletions(-) -- 2.10.2 -- 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
[parent not found: <20170324143600.4704-1-abailon-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>]
* [RESEND PATCH v6 1/4] usb: musb: Use shared irq [not found] ` <20170324143600.4704-1-abailon-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> @ 2017-03-24 14:35 ` Alexandre Bailon 2017-03-24 14:35 ` [RESEND PATCH v6 2/4] usb: musb: Add support of CPPI 4.1 DMA controller to DA8xx Alexandre Bailon ` (2 subsequent siblings) 3 siblings, 0 replies; 12+ messages in thread From: Alexandre Bailon @ 2017-03-24 14:35 UTC (permalink / raw) To: b-liu-l0cyMroinI0 Cc: linux-usb-u79uwXL29TY76Z2rM5mHXA, ptitiano-rdvid1DuHRBWk0Htik3J/w, khilman-rdvid1DuHRBWk0Htik3J/w, nsekhar-l0cyMroinI0, tony-4v6yS6AI5VpBDgjK7y7TUQ, sergei.shtylyov-M4DtvfQ/ZS1MRgGoP+s0PdBPR1lH4CV8, linux-omap-u79uwXL29TY76Z2rM5mHXA, Alexandre Bailon In the DA8xx, USB and CPPI 4.1 are sharing the same interrupt line. Update the driver to request a shared irq. Signed-off-by: Alexandre Bailon <abailon-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> --- drivers/usb/musb/musb_core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c index 0c3664a..870da18 100644 --- a/drivers/usb/musb/musb_core.c +++ b/drivers/usb/musb/musb_core.c @@ -2332,7 +2332,7 @@ musb_init_controller(struct device *dev, int nIrq, void __iomem *ctrl) setup_timer(&musb->otg_timer, musb_otg_timer_func, (unsigned long) musb); /* attach to the IRQ */ - if (request_irq(nIrq, musb->isr, 0, dev_name(dev), musb)) { + if (request_irq(nIrq, musb->isr, IRQF_SHARED, dev_name(dev), musb)) { dev_err(dev, "request_irq %d failed!\n", nIrq); status = -ENODEV; goto fail3; -- 2.10.2 -- 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 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [RESEND PATCH v6 2/4] usb: musb: Add support of CPPI 4.1 DMA controller to DA8xx [not found] ` <20170324143600.4704-1-abailon-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> 2017-03-24 14:35 ` [RESEND PATCH v6 1/4] usb: musb: Use shared irq Alexandre Bailon @ 2017-03-24 14:35 ` Alexandre Bailon 2017-03-24 14:35 ` [RESEND PATCH v6 3/4] usb: musb: musb_cppi41: Workaround DMA stall issue during teardown Alexandre Bailon 2017-03-24 14:36 ` [RESEND PATCH v6 4/4] usb: musb: da8xx: Add a primary support of PM runtime Alexandre Bailon 3 siblings, 0 replies; 12+ messages in thread From: Alexandre Bailon @ 2017-03-24 14:35 UTC (permalink / raw) To: b-liu-l0cyMroinI0 Cc: linux-usb-u79uwXL29TY76Z2rM5mHXA, ptitiano-rdvid1DuHRBWk0Htik3J/w, khilman-rdvid1DuHRBWk0Htik3J/w, nsekhar-l0cyMroinI0, tony-4v6yS6AI5VpBDgjK7y7TUQ, sergei.shtylyov-M4DtvfQ/ZS1MRgGoP+s0PdBPR1lH4CV8, linux-omap-u79uwXL29TY76Z2rM5mHXA, Alexandre Bailon Currently, only the PIO mode is supported. This add support of CPPI 4.1 to DA8xx. As the In DA8xx the CPPI 4.1 DMA is a part of the USB. Create the CPPI 4.1 device as a child of USB. Signed-off-by: Alexandre Bailon <abailon-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> --- drivers/usb/musb/Kconfig | 4 ++-- drivers/usb/musb/da8xx.c | 36 +++++++++++++++++++++++++++++++++++- 2 files changed, 37 insertions(+), 3 deletions(-) diff --git a/drivers/usb/musb/Kconfig b/drivers/usb/musb/Kconfig index 72a2a50..5506a9c 100644 --- a/drivers/usb/musb/Kconfig +++ b/drivers/usb/musb/Kconfig @@ -160,8 +160,8 @@ config USB_TI_CPPI_DMA Enable DMA transfers when TI CPPI DMA is available. config USB_TI_CPPI41_DMA - bool 'TI CPPI 4.1 (AM335x)' - depends on ARCH_OMAP && DMADEVICES + bool 'TI CPPI 4.1' + depends on (ARCH_OMAP || ARCH_DAVINCI_DA8XX) && DMADEVICES select TI_CPPI41 config USB_TUSB_OMAP_DMA diff --git a/drivers/usb/musb/da8xx.c b/drivers/usb/musb/da8xx.c index d79c288..458a499 100644 --- a/drivers/usb/musb/da8xx.c +++ b/drivers/usb/musb/da8xx.c @@ -33,6 +33,7 @@ #include <linux/clk.h> #include <linux/err.h> #include <linux/io.h> +#include <linux/of_platform.h> #include <linux/phy/phy.h> #include <linux/platform_device.h> #include <linux/dma-mapping.h> @@ -456,12 +457,41 @@ static inline u8 get_vbus_power(struct device *dev) return current_uA / 1000 / 2; } +#ifdef CONFIG_USB_TI_CPPI41_DMA +static void da8xx_dma_controller_callback(struct dma_controller *c) +{ + struct musb *musb = c->musb; + void __iomem *reg_base = musb->ctrl_base; + + musb_writel(reg_base, DA8XX_USB_END_OF_INTR_REG, 0); +} + +static struct dma_controller * +da8xx_dma_controller_create(struct musb *musb, void __iomem *base) +{ + struct dma_controller *controller; + + controller = cppi41_dma_controller_create(musb, base); + if (IS_ERR_OR_NULL(controller)) + return controller; + + controller->dma_callback = da8xx_dma_controller_callback; + + return controller; +} +#endif + static const struct musb_platform_ops da8xx_ops = { - .quirks = MUSB_INDEXED_EP | MUSB_PRESERVE_SESSION, + .quirks = MUSB_INDEXED_EP | MUSB_PRESERVE_SESSION | + MUSB_DMA_CPPI41, .init = da8xx_musb_init, .exit = da8xx_musb_exit, .fifo_mode = 2, +#ifdef CONFIG_USB_TI_CPPI41_DMA + .dma_init = da8xx_dma_controller_create, + .dma_exit = cppi41_dma_controller_destroy, +#endif .enable = da8xx_musb_enable, .disable = da8xx_musb_disable, @@ -533,6 +563,10 @@ static int da8xx_probe(struct platform_device *pdev) } platform_set_drvdata(pdev, glue); + ret = of_platform_populate(pdev->dev.of_node, NULL, NULL, &pdev->dev); + if (ret) + return ret; + memset(musb_resources, 0x00, sizeof(*musb_resources) * ARRAY_SIZE(musb_resources)); -- 2.10.2 -- 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 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [RESEND PATCH v6 3/4] usb: musb: musb_cppi41: Workaround DMA stall issue during teardown [not found] ` <20170324143600.4704-1-abailon-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> 2017-03-24 14:35 ` [RESEND PATCH v6 1/4] usb: musb: Use shared irq Alexandre Bailon 2017-03-24 14:35 ` [RESEND PATCH v6 2/4] usb: musb: Add support of CPPI 4.1 DMA controller to DA8xx Alexandre Bailon @ 2017-03-24 14:35 ` Alexandre Bailon 2017-03-24 14:36 ` [RESEND PATCH v6 4/4] usb: musb: da8xx: Add a primary support of PM runtime Alexandre Bailon 3 siblings, 0 replies; 12+ messages in thread From: Alexandre Bailon @ 2017-03-24 14:35 UTC (permalink / raw) To: b-liu-l0cyMroinI0 Cc: linux-usb-u79uwXL29TY76Z2rM5mHXA, ptitiano-rdvid1DuHRBWk0Htik3J/w, khilman-rdvid1DuHRBWk0Htik3J/w, nsekhar-l0cyMroinI0, tony-4v6yS6AI5VpBDgjK7y7TUQ, sergei.shtylyov-M4DtvfQ/ZS1MRgGoP+s0PdBPR1lH4CV8, linux-omap-u79uwXL29TY76Z2rM5mHXA, Alexandre Bailon The DMA may hang up if a teardown is initiated while an endpoint is still active (Advisory 2.3.27 of DA8xx errata). To workaround this issue, add a delay before to initiate the teardown. Signed-off-by: Alexandre Bailon <abailon-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> --- drivers/usb/musb/da8xx.c | 2 +- drivers/usb/musb/musb_core.h | 1 + drivers/usb/musb/musb_cppi41.c | 4 ++++ 3 files changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/usb/musb/da8xx.c b/drivers/usb/musb/da8xx.c index 458a499..ed28afd 100644 --- a/drivers/usb/musb/da8xx.c +++ b/drivers/usb/musb/da8xx.c @@ -483,7 +483,7 @@ da8xx_dma_controller_create(struct musb *musb, void __iomem *base) static const struct musb_platform_ops da8xx_ops = { .quirks = MUSB_INDEXED_EP | MUSB_PRESERVE_SESSION | - MUSB_DMA_CPPI41, + MUSB_DMA_CPPI41 | MUSB_DA8XX, .init = da8xx_musb_init, .exit = da8xx_musb_exit, diff --git a/drivers/usb/musb/musb_core.h b/drivers/usb/musb/musb_core.h index 5b708be..3e98d42 100644 --- a/drivers/usb/musb/musb_core.h +++ b/drivers/usb/musb/musb_core.h @@ -172,6 +172,7 @@ struct musb_io; */ struct musb_platform_ops { +#define MUSB_DA8XX BIT(8) #define MUSB_PRESERVE_SESSION BIT(7) #define MUSB_DMA_UX500 BIT(6) #define MUSB_DMA_CPPI41 BIT(5) diff --git a/drivers/usb/musb/musb_cppi41.c b/drivers/usb/musb/musb_cppi41.c index 355655f..e7c8b1b 100644 --- a/drivers/usb/musb/musb_cppi41.c +++ b/drivers/usb/musb/musb_cppi41.c @@ -571,6 +571,10 @@ static int cppi41_dma_channel_abort(struct dma_channel *channel) } } + /* DA8xx Advisory 2.3.27: wait 250 ms before to start the teardown */ + if (musb->io.quirks & MUSB_DA8XX) + mdelay(250); + tdbit = 1 << cppi41_channel->port_num; if (is_tx) tdbit <<= 16; -- 2.10.2 -- 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 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [RESEND PATCH v6 4/4] usb: musb: da8xx: Add a primary support of PM runtime [not found] ` <20170324143600.4704-1-abailon-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> ` (2 preceding siblings ...) 2017-03-24 14:35 ` [RESEND PATCH v6 3/4] usb: musb: musb_cppi41: Workaround DMA stall issue during teardown Alexandre Bailon @ 2017-03-24 14:36 ` Alexandre Bailon [not found] ` <20170324143600.4704-5-abailon-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> 3 siblings, 1 reply; 12+ messages in thread From: Alexandre Bailon @ 2017-03-24 14:36 UTC (permalink / raw) To: b-liu-l0cyMroinI0 Cc: linux-usb-u79uwXL29TY76Z2rM5mHXA, ptitiano-rdvid1DuHRBWk0Htik3J/w, khilman-rdvid1DuHRBWk0Htik3J/w, nsekhar-l0cyMroinI0, tony-4v6yS6AI5VpBDgjK7y7TUQ, sergei.shtylyov-M4DtvfQ/ZS1MRgGoP+s0PdBPR1lH4CV8, linux-omap-u79uwXL29TY76Z2rM5mHXA, Alexandre Bailon Currently, MUSB DA8xx glue driver doesn't have PM runtime support. Because the CPPI 4.1 is using the same clock as MUSB DA8xx and CPPI 4.1 is a child of MUSB DA8xx glue, add support of PM runtime to the DA8xx glue driver in order to let the CPPI 4.1 driver manage the clock by using PM runtime. Signed-off-by: Alexandre Bailon <abailon-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> --- drivers/usb/musb/da8xx.c | 27 ++++++++------------------- 1 file changed, 8 insertions(+), 19 deletions(-) diff --git a/drivers/usb/musb/da8xx.c b/drivers/usb/musb/da8xx.c index ed28afd..89e12f6 100644 --- a/drivers/usb/musb/da8xx.c +++ b/drivers/usb/musb/da8xx.c @@ -30,7 +30,6 @@ */ #include <linux/module.h> -#include <linux/clk.h> #include <linux/err.h> #include <linux/io.h> #include <linux/of_platform.h> @@ -86,7 +85,6 @@ struct da8xx_glue { struct device *dev; struct platform_device *musb; struct platform_device *usb_phy; - struct clk *clk; struct phy *phy; }; @@ -376,11 +374,7 @@ static int da8xx_musb_init(struct musb *musb) musb->mregs += DA8XX_MENTOR_CORE_OFFSET; - ret = clk_prepare_enable(glue->clk); - if (ret) { - dev_err(glue->dev, "failed to enable clock\n"); - return ret; - } + pm_runtime_get_sync(musb->controller->parent); /* Returns zero if e.g. not clocked */ rev = musb_readl(reg_base, DA8XX_USB_REVISION_REG); @@ -423,7 +417,7 @@ static int da8xx_musb_init(struct musb *musb) err_phy_power_on: phy_exit(glue->phy); fail: - clk_disable_unprepare(glue->clk); + pm_runtime_put(musb->controller->parent); return ret; } @@ -435,7 +429,7 @@ static int da8xx_musb_exit(struct musb *musb) phy_power_off(glue->phy); phy_exit(glue->phy); - clk_disable_unprepare(glue->clk); + pm_runtime_put(musb->controller->parent); usb_put_phy(musb->xceiv); @@ -519,7 +513,6 @@ static int da8xx_probe(struct platform_device *pdev) struct musb_hdrc_platform_data *pdata = dev_get_platdata(&pdev->dev); struct da8xx_glue *glue; struct platform_device_info pinfo; - struct clk *clk; struct device_node *np = pdev->dev.of_node; int ret; @@ -527,12 +520,6 @@ static int da8xx_probe(struct platform_device *pdev) if (!glue) return -ENOMEM; - clk = devm_clk_get(&pdev->dev, "usb20"); - if (IS_ERR(clk)) { - dev_err(&pdev->dev, "failed to get clock\n"); - return PTR_ERR(clk); - } - glue->phy = devm_phy_get(&pdev->dev, "usb-phy"); if (IS_ERR(glue->phy)) { if (PTR_ERR(glue->phy) != -EPROBE_DEFER) @@ -541,7 +528,6 @@ static int da8xx_probe(struct platform_device *pdev) } glue->dev = &pdev->dev; - glue->clk = clk; if (IS_ENABLED(CONFIG_OF) && np) { pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL); @@ -587,6 +573,8 @@ static int da8xx_probe(struct platform_device *pdev) pinfo.data = pdata; pinfo.size_data = sizeof(*pdata); + pm_runtime_enable(&pdev->dev); + glue->musb = platform_device_register_full(&pinfo); ret = PTR_ERR_OR_ZERO(glue->musb); if (ret) { @@ -603,6 +591,7 @@ static int da8xx_remove(struct platform_device *pdev) platform_device_unregister(glue->musb); usb_phy_generic_unregister(glue->usb_phy); + pm_runtime_disable(&pdev->dev); return 0; } @@ -616,7 +605,7 @@ static int da8xx_suspend(struct device *dev) ret = phy_power_off(glue->phy); if (ret) return ret; - clk_disable_unprepare(glue->clk); + pm_runtime_put_sync(dev); return 0; } @@ -626,7 +615,7 @@ static int da8xx_resume(struct device *dev) int ret; struct da8xx_glue *glue = dev_get_drvdata(dev); - ret = clk_prepare_enable(glue->clk); + ret = pm_runtime_get_sync(dev); if (ret) return ret; return phy_power_on(glue->phy); -- 2.10.2 -- 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 ^ permalink raw reply related [flat|nested] 12+ messages in thread
[parent not found: <20170324143600.4704-5-abailon-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>]
* Re: [RESEND PATCH v6 4/4] usb: musb: da8xx: Add a primary support of PM runtime [not found] ` <20170324143600.4704-5-abailon-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> @ 2017-03-24 17:26 ` Grygorii Strashko [not found] ` <703e5e95-8bde-2b1c-0d63-4ed3e03f53b8-l0cyMroinI0@public.gmane.org> 2017-03-28 0:39 ` Kevin Hilman 1 sibling, 1 reply; 12+ messages in thread From: Grygorii Strashko @ 2017-03-24 17:26 UTC (permalink / raw) To: Alexandre Bailon, b-liu-l0cyMroinI0 Cc: linux-usb-u79uwXL29TY76Z2rM5mHXA, ptitiano-rdvid1DuHRBWk0Htik3J/w, khilman-rdvid1DuHRBWk0Htik3J/w, nsekhar-l0cyMroinI0, tony-4v6yS6AI5VpBDgjK7y7TUQ, sergei.shtylyov-M4DtvfQ/ZS1MRgGoP+s0PdBPR1lH4CV8, linux-omap-u79uwXL29TY76Z2rM5mHXA On 03/24/2017 09:36 AM, Alexandre Bailon wrote: > Currently, MUSB DA8xx glue driver doesn't have PM runtime support. > Because the CPPI 4.1 is using the same clock as MUSB DA8xx and > CPPI 4.1 is a child of MUSB DA8xx glue, add support of PM runtime > to the DA8xx glue driver in order to let the CPPI 4.1 driver manage > the clock by using PM runtime. > > Signed-off-by: Alexandre Bailon <abailon-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> > --- > drivers/usb/musb/da8xx.c | 27 ++++++++------------------- > 1 file changed, 8 insertions(+), 19 deletions(-) > > diff --git a/drivers/usb/musb/da8xx.c b/drivers/usb/musb/da8xx.c > index ed28afd..89e12f6 100644 > --- a/drivers/usb/musb/da8xx.c > +++ b/drivers/usb/musb/da8xx.c > @@ -30,7 +30,6 @@ > */ > [...] > > @@ -527,12 +520,6 @@ static int da8xx_probe(struct platform_device *pdev) > if (!glue) > return -ENOMEM; > > - clk = devm_clk_get(&pdev->dev, "usb20"); > - if (IS_ERR(clk)) { > - dev_err(&pdev->dev, "failed to get clock\n"); > - return PTR_ERR(clk); > - } > - > glue->phy = devm_phy_get(&pdev->dev, "usb-phy"); > if (IS_ERR(glue->phy)) { > if (PTR_ERR(glue->phy) != -EPROBE_DEFER) > @@ -541,7 +528,6 @@ static int da8xx_probe(struct platform_device *pdev) > } > > glue->dev = &pdev->dev; > - glue->clk = clk; > > if (IS_ENABLED(CONFIG_OF) && np) { > pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL); > @@ -587,6 +573,8 @@ static int da8xx_probe(struct platform_device *pdev) > pinfo.data = pdata; > pinfo.size_data = sizeof(*pdata); > > + pm_runtime_enable(&pdev->dev); > + > glue->musb = platform_device_register_full(&pinfo); > ret = PTR_ERR_OR_ZERO(glue->musb); > if (ret) { > @@ -603,6 +591,7 @@ static int da8xx_remove(struct platform_device *pdev) > > platform_device_unregister(glue->musb); > usb_phy_generic_unregister(glue->usb_phy); > + pm_runtime_disable(&pdev->dev); > > return 0; > } > @@ -616,7 +605,7 @@ static int da8xx_suspend(struct device *dev) > ret = phy_power_off(glue->phy); > if (ret) > return ret; > - clk_disable_unprepare(glue->clk); > + pm_runtime_put_sync(dev); This, most probably will do nothing as Suspend framework will increase ref counter. Better way might be to use PM runtime force API. pm_runtime_force_suspend() > > return 0; > } > @@ -626,7 +615,7 @@ static int da8xx_resume(struct device *dev) > int ret; > struct da8xx_glue *glue = dev_get_drvdata(dev); > > - ret = clk_prepare_enable(glue->clk); > + ret = pm_runtime_get_sync(dev); > if (ret) > return ret; Better way might be to use PM runtime force API. > return phy_power_on(glue->phy); > -- regards, -grygorii -- 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
[parent not found: <703e5e95-8bde-2b1c-0d63-4ed3e03f53b8-l0cyMroinI0@public.gmane.org>]
* Re: [RESEND PATCH v6 4/4] usb: musb: da8xx: Add a primary support of PM runtime [not found] ` <703e5e95-8bde-2b1c-0d63-4ed3e03f53b8-l0cyMroinI0@public.gmane.org> @ 2017-03-27 16:39 ` Alexandre Bailon [not found] ` <03cc95cd-5967-c0c7-06a7-89cf08dbde47-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> 0 siblings, 1 reply; 12+ messages in thread From: Alexandre Bailon @ 2017-03-27 16:39 UTC (permalink / raw) To: Grygorii Strashko Cc: linux-usb-u79uwXL29TY76Z2rM5mHXA, ptitiano-rdvid1DuHRBWk0Htik3J/w, khilman-rdvid1DuHRBWk0Htik3J/w, nsekhar-l0cyMroinI0, tony-4v6yS6AI5VpBDgjK7y7TUQ, sergei.shtylyov-M4DtvfQ/ZS1MRgGoP+s0PdBPR1lH4CV8, linux-omap-u79uwXL29TY76Z2rM5mHXA, b-liu-l0cyMroinI0 Hello Grygorii, On 03/24/2017 06:26 PM, Grygorii Strashko wrote: > > > On 03/24/2017 09:36 AM, Alexandre Bailon wrote: > > Currently, MUSB DA8xx glue driver doesn't have PM runtime support. > > Because the CPPI 4.1 is using the same clock as MUSB DA8xx and > > CPPI 4.1 is a child of MUSB DA8xx glue, add support of PM runtime > > to the DA8xx glue driver in order to let the CPPI 4.1 driver manage > > the clock by using PM runtime. > > > > Signed-off-by: Alexandre Bailon <abailon-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> > > --- > > drivers/usb/musb/da8xx.c | 27 ++++++++------------------- > > 1 file changed, 8 insertions(+), 19 deletions(-) > > > > diff --git a/drivers/usb/musb/da8xx.c b/drivers/usb/musb/da8xx.c > > index ed28afd..89e12f6 100644 > > --- a/drivers/usb/musb/da8xx.c > > +++ b/drivers/usb/musb/da8xx.c > > @@ -30,7 +30,6 @@ > > */ > > > > [...] > > > > > @@ -527,12 +520,6 @@ static int da8xx_probe(struct platform_device *pdev) > > if (!glue) > > return -ENOMEM; > > > > - clk = devm_clk_get(&pdev->dev, "usb20"); > > - if (IS_ERR(clk)) { > > - dev_err(&pdev->dev, "failed to get clock\n"); > > - return PTR_ERR(clk); > > - } > > - > > glue->phy = devm_phy_get(&pdev->dev, "usb-phy"); > > if (IS_ERR(glue->phy)) { > > if (PTR_ERR(glue->phy) != -EPROBE_DEFER) > > @@ -541,7 +528,6 @@ static int da8xx_probe(struct platform_device *pdev) > > } > > > > glue->dev = &pdev->dev; > > - glue->clk = clk; > > > > if (IS_ENABLED(CONFIG_OF) && np) { > > pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL); > > @@ -587,6 +573,8 @@ static int da8xx_probe(struct platform_device *pdev) > > pinfo.data = pdata; > > pinfo.size_data = sizeof(*pdata); > > > > + pm_runtime_enable(&pdev->dev); > > + > > glue->musb = platform_device_register_full(&pinfo); > > ret = PTR_ERR_OR_ZERO(glue->musb); > > if (ret) { > > @@ -603,6 +591,7 @@ static int da8xx_remove(struct platform_device *pdev) > > > > platform_device_unregister(glue->musb); > > usb_phy_generic_unregister(glue->usb_phy); > > + pm_runtime_disable(&pdev->dev); > > > > return 0; > > } > > @@ -616,7 +605,7 @@ static int da8xx_suspend(struct device *dev) > > ret = phy_power_off(glue->phy); > > if (ret) > > return ret; > > - clk_disable_unprepare(glue->clk); > > + pm_runtime_put_sync(dev); > > This, most probably will do nothing as Suspend framework will increase ref counter. > Better way might be to use PM runtime force API. pm_runtime_force_suspend() Good catch. Effectively, the device remain active. But we can't use pm_runtime_force_suspend() because it expect that all child have been runtime suspended which is usually not the case. > > > > > return 0; > > } > > @@ -626,7 +615,7 @@ static int da8xx_resume(struct device *dev) > > int ret; > > struct da8xx_glue *glue = dev_get_drvdata(dev); > > > > - ret = clk_prepare_enable(glue->clk); > > + ret = pm_runtime_get_sync(dev); > > if (ret) > > return ret; > > Better way might be to use PM runtime force API. Again, it will not work. Because the forced runtime suspend will not complete because child are not runtime suspended then the resume will not happen. > > > return phy_power_on(glue->phy); > > > Do you have any other suggestions to fix it? My original intent was to give a way to CPPI 4.1 DMA driver to enable or disable the usb20 clock owned DA8xx USB glue driver. Thanks, Alexandre -- 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
[parent not found: <03cc95cd-5967-c0c7-06a7-89cf08dbde47-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>]
* Re: [RESEND PATCH v6 4/4] usb: musb: da8xx: Add a primary support of PM runtime [not found] ` <03cc95cd-5967-c0c7-06a7-89cf08dbde47-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> @ 2017-03-27 17:38 ` Grygorii Strashko [not found] ` <1552a8e0-9efe-3658-e511-76e056ed5588-l0cyMroinI0@public.gmane.org> 0 siblings, 1 reply; 12+ messages in thread From: Grygorii Strashko @ 2017-03-27 17:38 UTC (permalink / raw) To: Alexandre Bailon Cc: linux-usb-u79uwXL29TY76Z2rM5mHXA, ptitiano-rdvid1DuHRBWk0Htik3J/w, khilman-rdvid1DuHRBWk0Htik3J/w, nsekhar-l0cyMroinI0, tony-4v6yS6AI5VpBDgjK7y7TUQ, sergei.shtylyov-M4DtvfQ/ZS1MRgGoP+s0PdBPR1lH4CV8, linux-omap-u79uwXL29TY76Z2rM5mHXA, b-liu-l0cyMroinI0 On 03/27/2017 11:39 AM, Alexandre Bailon wrote: > Hello Grygorii, > On 03/24/2017 06:26 PM, Grygorii Strashko wrote: >> >> >> On 03/24/2017 09:36 AM, Alexandre Bailon wrote: >> > Currently, MUSB DA8xx glue driver doesn't have PM runtime support. >> > Because the CPPI 4.1 is using the same clock as MUSB DA8xx and >> > CPPI 4.1 is a child of MUSB DA8xx glue, add support of PM runtime >> > to the DA8xx glue driver in order to let the CPPI 4.1 driver manage >> > the clock by using PM runtime. >> > >> > Signed-off-by: Alexandre Bailon <abailon-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> >> > --- >> > drivers/usb/musb/da8xx.c | 27 ++++++++------------------- >> > 1 file changed, 8 insertions(+), 19 deletions(-) >> > >> > diff --git a/drivers/usb/musb/da8xx.c b/drivers/usb/musb/da8xx.c >> > index ed28afd..89e12f6 100644 >> > --- a/drivers/usb/musb/da8xx.c >> > +++ b/drivers/usb/musb/da8xx.c >> > @@ -30,7 +30,6 @@ >> > */ >> > >> >> [...] >> >> > >> > @@ -527,12 +520,6 @@ static int da8xx_probe(struct platform_device >> *pdev) >> > if (!glue) >> > return -ENOMEM; >> > >> > - clk = devm_clk_get(&pdev->dev, "usb20"); >> > - if (IS_ERR(clk)) { >> > - dev_err(&pdev->dev, "failed to get clock\n"); >> > - return PTR_ERR(clk); >> > - } >> > - >> > glue->phy = devm_phy_get(&pdev->dev, "usb-phy"); >> > if (IS_ERR(glue->phy)) { >> > if (PTR_ERR(glue->phy) != -EPROBE_DEFER) >> > @@ -541,7 +528,6 @@ static int da8xx_probe(struct platform_device >> *pdev) >> > } >> > >> > glue->dev = &pdev->dev; >> > - glue->clk = clk; >> > >> > if (IS_ENABLED(CONFIG_OF) && np) { >> > pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL); >> > @@ -587,6 +573,8 @@ static int da8xx_probe(struct platform_device >> *pdev) >> > pinfo.data = pdata; >> > pinfo.size_data = sizeof(*pdata); >> > >> > + pm_runtime_enable(&pdev->dev); >> > + >> > glue->musb = platform_device_register_full(&pinfo); >> > ret = PTR_ERR_OR_ZERO(glue->musb); >> > if (ret) { >> > @@ -603,6 +591,7 @@ static int da8xx_remove(struct platform_device >> *pdev) >> > >> > platform_device_unregister(glue->musb); >> > usb_phy_generic_unregister(glue->usb_phy); >> > + pm_runtime_disable(&pdev->dev); >> > >> > return 0; >> > } >> > @@ -616,7 +605,7 @@ static int da8xx_suspend(struct device *dev) >> > ret = phy_power_off(glue->phy); >> > if (ret) >> > return ret; >> > - clk_disable_unprepare(glue->clk); >> > + pm_runtime_put_sync(dev); >> >> This, most probably will do nothing as Suspend framework will increase >> ref counter. >> Better way might be to use PM runtime force API. >> pm_runtime_force_suspend() > Good catch. Effectively, the device remain active. > But we can't use pm_runtime_force_suspend() because it expect that all > child have been > runtime suspended which is usually not the case. If this is the parent - it should be suspended the last and any children are not expected to be accessible after that. Also, if there are will be force_suspend() here and force_resume() in da8xx_resume() then parent should always be active before any child. So, I seems didn't get your point :( >> >> > >> > return 0; >> > } >> > @@ -626,7 +615,7 @@ static int da8xx_resume(struct device *dev) >> > int ret; >> > struct da8xx_glue *glue = dev_get_drvdata(dev); >> > >> > - ret = clk_prepare_enable(glue->clk); >> > + ret = pm_runtime_get_sync(dev); >> > if (ret) >> > return ret; >> >> Better way might be to use PM runtime force API. > Again, it will not work. Because the forced runtime suspend will not > complete > because child are not runtime suspended then the resume will not happen. >> >> > return phy_power_on(glue->phy); >> > >> > Do you have any other suggestions to fix it? > My original intent was to give a way to CPPI 4.1 DMA driver to enable or > disable the usb20 clock owned DA8xx USB glue driver. > > Thanks, > Alexandre -- regards, -grygorii -- 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
[parent not found: <1552a8e0-9efe-3658-e511-76e056ed5588-l0cyMroinI0@public.gmane.org>]
* Re: [RESEND PATCH v6 4/4] usb: musb: da8xx: Add a primary support of PM runtime [not found] ` <1552a8e0-9efe-3658-e511-76e056ed5588-l0cyMroinI0@public.gmane.org> @ 2017-03-27 19:54 ` Alexandre Bailon [not found] ` <322bdb6b-72c4-01e1-8ae7-bf04ea89a1ee-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> 0 siblings, 1 reply; 12+ messages in thread From: Alexandre Bailon @ 2017-03-27 19:54 UTC (permalink / raw) To: Grygorii Strashko Cc: linux-usb-u79uwXL29TY76Z2rM5mHXA, ptitiano-rdvid1DuHRBWk0Htik3J/w, khilman-rdvid1DuHRBWk0Htik3J/w, nsekhar-l0cyMroinI0, tony-4v6yS6AI5VpBDgjK7y7TUQ, sergei.shtylyov-M4DtvfQ/ZS1MRgGoP+s0PdBPR1lH4CV8, linux-omap-u79uwXL29TY76Z2rM5mHXA, b-liu-l0cyMroinI0 On 03/27/2017 07:38 PM, Grygorii Strashko wrote: > > > On 03/27/2017 11:39 AM, Alexandre Bailon wrote: >> Hello Grygorii, >> On 03/24/2017 06:26 PM, Grygorii Strashko wrote: >>> >>> >>> On 03/24/2017 09:36 AM, Alexandre Bailon wrote: >>>> Currently, MUSB DA8xx glue driver doesn't have PM runtime support. >>>> Because the CPPI 4.1 is using the same clock as MUSB DA8xx and >>>> CPPI 4.1 is a child of MUSB DA8xx glue, add support of PM runtime >>>> to the DA8xx glue driver in order to let the CPPI 4.1 driver manage >>>> the clock by using PM runtime. >>>> >>>> Signed-off-by: Alexandre Bailon <abailon-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> >>>> --- >>>> drivers/usb/musb/da8xx.c | 27 ++++++++------------------- >>>> 1 file changed, 8 insertions(+), 19 deletions(-) >>>> >>>> diff --git a/drivers/usb/musb/da8xx.c b/drivers/usb/musb/da8xx.c >>>> index ed28afd..89e12f6 100644 >>>> --- a/drivers/usb/musb/da8xx.c >>>> +++ b/drivers/usb/musb/da8xx.c >>>> @@ -30,7 +30,6 @@ >>>> */ >>>> >>> >>> [...] >>> >>>> >>>> @@ -527,12 +520,6 @@ static int da8xx_probe(struct platform_device >>> *pdev) >>>> if (!glue) >>>> return -ENOMEM; >>>> >>>> - clk = devm_clk_get(&pdev->dev, "usb20"); >>>> - if (IS_ERR(clk)) { >>>> - dev_err(&pdev->dev, "failed to get clock\n"); >>>> - return PTR_ERR(clk); >>>> - } >>>> - >>>> glue->phy = devm_phy_get(&pdev->dev, "usb-phy"); >>>> if (IS_ERR(glue->phy)) { >>>> if (PTR_ERR(glue->phy) != -EPROBE_DEFER) >>>> @@ -541,7 +528,6 @@ static int da8xx_probe(struct platform_device >>> *pdev) >>>> } >>>> >>>> glue->dev = &pdev->dev; >>>> - glue->clk = clk; >>>> >>>> if (IS_ENABLED(CONFIG_OF) && np) { >>>> pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL); >>>> @@ -587,6 +573,8 @@ static int da8xx_probe(struct platform_device >>> *pdev) >>>> pinfo.data = pdata; >>>> pinfo.size_data = sizeof(*pdata); >>>> >>>> + pm_runtime_enable(&pdev->dev); >>>> + >>>> glue->musb = platform_device_register_full(&pinfo); >>>> ret = PTR_ERR_OR_ZERO(glue->musb); >>>> if (ret) { >>>> @@ -603,6 +591,7 @@ static int da8xx_remove(struct platform_device >>> *pdev) >>>> >>>> platform_device_unregister(glue->musb); >>>> usb_phy_generic_unregister(glue->usb_phy); >>>> + pm_runtime_disable(&pdev->dev); >>>> >>>> return 0; >>>> } >>>> @@ -616,7 +605,7 @@ static int da8xx_suspend(struct device *dev) >>>> ret = phy_power_off(glue->phy); >>>> if (ret) >>>> return ret; >>>> - clk_disable_unprepare(glue->clk); >>>> + pm_runtime_put_sync(dev); >>> >>> This, most probably will do nothing as Suspend framework will increase >>> ref counter. >>> Better way might be to use PM runtime force API. >>> pm_runtime_force_suspend() >> Good catch. Effectively, the device remain active. >> But we can't use pm_runtime_force_suspend() because it expect that all >> child have been >> runtime suspended which is usually not the case. > > If this is the parent - it should be suspended the last and any children are > not expected to be accessible after that. Yes but suspended doesn't mean runtime suspended. In the case of system suspend, the MUSB core will be suspended but its runtime_status will remain active and so pm_runtime_force_suspend() will refuse to work because it will not consider the MUSB core as suspend. > > Also, if there are will be force_suspend() here and force_resume() in da8xx_resume() > then parent should always be active before any child. > > So, I seems didn't get your point :( I think with an example and some logs it should be more clear: rtcwake -d /dev/rtc0 -m mem -s 1 rtcwake: assuming RTC uses UTC ... rtcwake: wakeup from "mem" using /dev/rtc0 at Wed Mar 22 00:43:07 2017 PM: Syncing filesystems ... done. Freezing user space processes ... (elapsed 0.002 seconds) done. Freezing remaining freezable tasks ... (elapsed 0.001 seconds) done. Suspending console(s) (use no_console_suspend to debug) davinci_mdio davinci_mdio.0: resetting idled controller musb-da8xx musb-da8xx: runtime PM trying to suspend device but active child PM: suspend of devices complete after 167.287 msecs PM: late suspend of devices complete after 8.752 msecs PM: noirq suspend of devices complete after 8.389 msecs PM: noirq resume of devices complete after 4.385 msecs PM: early resume of devices complete after 5.880 msecs davinci_mdio davinci_mdio.0: resetting idled controller SMSC LAN8710/LAN8720 davinci_mdio.0:07: attached PHY driver [SMSC LAN8710/LAN8720] (mii_bus:phy_addr=davinci_mdio.0:07, irq=-1) tilcdc da8xx_lcdc.0: tilcdc_crtc_irq(0x00000161): FIFO underflow Suspended for 1.454 seconds davinci_emac davinci_emac.1 eth0: Link is Up - 100Mbps/Full - flow control off PM: resume of devices complete after 4178.211 msecs Restarting tasks ... usb 2-1: USB disconnect, device number 3 done. I'm using rtcwake to test suspend / resume. As you can see in the log, musb-da8xx doesn't complete the suspend because it child is active (though it doesn't prevent the suspend to happen). On resume, the USB device disconnects and from here the USB controller is dead. It will not detect any connect / disconnect anymore. This happens because pm_runtime_force_resume() fails and the resume callback exit before to turn on the OTG phy. > > > >>> >>>> >>>> return 0; >>>> } >>>> @@ -626,7 +615,7 @@ static int da8xx_resume(struct device *dev) >>>> int ret; >>>> struct da8xx_glue *glue = dev_get_drvdata(dev); >>>> >>>> - ret = clk_prepare_enable(glue->clk); >>>> + ret = pm_runtime_get_sync(dev); >>>> if (ret) >>>> return ret; >>> >>> Better way might be to use PM runtime force API. >> Again, it will not work. Because the forced runtime suspend will not >> complete >> because child are not runtime suspended then the resume will not happen. >>> >>>> return phy_power_on(glue->phy); >>>> >>> >> Do you have any other suggestions to fix it? >> My original intent was to give a way to CPPI 4.1 DMA driver to enable or >> disable the usb20 clock owned DA8xx USB glue driver. >> >> Thanks, >> Alexandre > Regards, Alexandre PS: My apologize to those who received this reply twice. I have attempted to reply to thread from my webmail which use html. As the reply has been rejected by the list, I'm re posting the reply. -- 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
[parent not found: <322bdb6b-72c4-01e1-8ae7-bf04ea89a1ee-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>]
* Re: [RESEND PATCH v6 4/4] usb: musb: da8xx: Add a primary support of PM runtime [not found] ` <322bdb6b-72c4-01e1-8ae7-bf04ea89a1ee-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> @ 2017-04-03 20:14 ` Grygorii Strashko 0 siblings, 0 replies; 12+ messages in thread From: Grygorii Strashko @ 2017-04-03 20:14 UTC (permalink / raw) To: Alexandre Bailon Cc: linux-usb-u79uwXL29TY76Z2rM5mHXA, ptitiano-rdvid1DuHRBWk0Htik3J/w, khilman-rdvid1DuHRBWk0Htik3J/w, nsekhar-l0cyMroinI0, tony-4v6yS6AI5VpBDgjK7y7TUQ, sergei.shtylyov-M4DtvfQ/ZS1MRgGoP+s0PdBPR1lH4CV8, linux-omap-u79uwXL29TY76Z2rM5mHXA, b-liu-l0cyMroinI0 On 03/27/2017 02:54 PM, Alexandre Bailon wrote: > On 03/27/2017 07:38 PM, Grygorii Strashko wrote: >> >> >> On 03/27/2017 11:39 AM, Alexandre Bailon wrote: >>> Hello Grygorii, >>> On 03/24/2017 06:26 PM, Grygorii Strashko wrote: >>>> >>>> >>>> On 03/24/2017 09:36 AM, Alexandre Bailon wrote: >>>>> Currently, MUSB DA8xx glue driver doesn't have PM runtime support. >>>>> Because the CPPI 4.1 is using the same clock as MUSB DA8xx and >>>>> CPPI 4.1 is a child of MUSB DA8xx glue, add support of PM runtime >>>>> to the DA8xx glue driver in order to let the CPPI 4.1 driver manage >>>>> the clock by using PM runtime. >>>>> >>>>> Signed-off-by: Alexandre Bailon <abailon-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> >>>>> --- >>>>> drivers/usb/musb/da8xx.c | 27 ++++++++------------------- >>>>> 1 file changed, 8 insertions(+), 19 deletions(-) >>>>> >>>>> diff --git a/drivers/usb/musb/da8xx.c b/drivers/usb/musb/da8xx.c >>>>> index ed28afd..89e12f6 100644 >>>>> --- a/drivers/usb/musb/da8xx.c >>>>> +++ b/drivers/usb/musb/da8xx.c >>>>> @@ -30,7 +30,6 @@ >>>>> */ >>>>> >>>> >>>> [...] >>>> >>>>> >>>>> @@ -527,12 +520,6 @@ static int da8xx_probe(struct platform_device >>>> *pdev) >>>>> if (!glue) >>>>> return -ENOMEM; >>>>> >>>>> - clk = devm_clk_get(&pdev->dev, "usb20"); >>>>> - if (IS_ERR(clk)) { >>>>> - dev_err(&pdev->dev, "failed to get clock\n"); >>>>> - return PTR_ERR(clk); >>>>> - } >>>>> - >>>>> glue->phy = devm_phy_get(&pdev->dev, "usb-phy"); >>>>> if (IS_ERR(glue->phy)) { >>>>> if (PTR_ERR(glue->phy) != -EPROBE_DEFER) >>>>> @@ -541,7 +528,6 @@ static int da8xx_probe(struct platform_device >>>> *pdev) >>>>> } >>>>> >>>>> glue->dev = &pdev->dev; >>>>> - glue->clk = clk; >>>>> >>>>> if (IS_ENABLED(CONFIG_OF) && np) { >>>>> pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL); >>>>> @@ -587,6 +573,8 @@ static int da8xx_probe(struct platform_device >>>> *pdev) >>>>> pinfo.data = pdata; >>>>> pinfo.size_data = sizeof(*pdata); >>>>> >>>>> + pm_runtime_enable(&pdev->dev); >>>>> + >>>>> glue->musb = platform_device_register_full(&pinfo); >>>>> ret = PTR_ERR_OR_ZERO(glue->musb); >>>>> if (ret) { >>>>> @@ -603,6 +591,7 @@ static int da8xx_remove(struct platform_device >>>> *pdev) >>>>> >>>>> platform_device_unregister(glue->musb); >>>>> usb_phy_generic_unregister(glue->usb_phy); >>>>> + pm_runtime_disable(&pdev->dev); >>>>> >>>>> return 0; >>>>> } >>>>> @@ -616,7 +605,7 @@ static int da8xx_suspend(struct device *dev) >>>>> ret = phy_power_off(glue->phy); >>>>> if (ret) >>>>> return ret; >>>>> - clk_disable_unprepare(glue->clk); >>>>> + pm_runtime_put_sync(dev); >>>> >>>> This, most probably will do nothing as Suspend framework will increase >>>> ref counter. >>>> Better way might be to use PM runtime force API. >>>> pm_runtime_force_suspend() >>> Good catch. Effectively, the device remain active. >>> But we can't use pm_runtime_force_suspend() because it expect that all >>> child have been >>> runtime suspended which is usually not the case. >> >> If this is the parent - it should be suspended the last and any >> children are >> not expected to be accessible after that. > Yes but suspended doesn't mean runtime suspended. > In the case of system suspend, the MUSB core will be suspended but its > runtime_status > will remain active and so pm_runtime_force_suspend() will refuse to work > because it will > not consider the MUSB core as suspend. >> >> Also, if there are will be force_suspend() here and force_resume() in >> da8xx_resume() >> then parent should always be active before any child. >> >> So, I seems didn't get your point :( > I think with an example and some logs it should be more clear: > rtcwake -d /dev/rtc0 -m mem -s 1 > rtcwake: assuming RTC uses UTC ... > rtcwake: wakeup from "mem" using /dev/rtc0 at Wed Mar 22 00:43:07 2017 > PM: Syncing filesystems ... done. > Freezing user space processes ... (elapsed 0.002 seconds) done. > Freezing remaining freezable tasks ... (elapsed 0.001 seconds) done. > Suspending console(s) (use no_console_suspend to debug) > davinci_mdio davinci_mdio.0: resetting idled controller > musb-da8xx musb-da8xx: runtime PM trying to suspend device but active child > PM: suspend of devices complete after 167.287 msecs > PM: late suspend of devices complete after 8.752 msecs > PM: noirq suspend of devices complete after 8.389 msecs > PM: noirq resume of devices complete after 4.385 msecs > PM: early resume of devices complete after 5.880 msecs > davinci_mdio davinci_mdio.0: resetting idled controller > SMSC LAN8710/LAN8720 davinci_mdio.0:07: attached PHY driver [SMSC > LAN8710/LAN8720] (mii_bus:phy_addr=davinci_mdio.0:07, irq=-1) > tilcdc da8xx_lcdc.0: tilcdc_crtc_irq(0x00000161): FIFO underflow > Suspended for 1.454 seconds > davinci_emac davinci_emac.1 eth0: Link is Up - 100Mbps/Full - flow > control off > PM: resume of devices complete after 4178.211 msecs > Restarting tasks ... > usb 2-1: USB disconnect, device number 3 > done. > > I'm using rtcwake to test suspend / resume. > As you can see in the log, musb-da8xx doesn't complete the suspend > because it child is active > (though it doesn't prevent the suspend to happen). > On resume, the USB device disconnects and from here the USB controller > is dead. > It will not detect any connect / disconnect anymore. This happens > because pm_runtime_force_resume() > fails and the resume callback exit before to turn on the OTG phy. >> More simple way to test it (at least on am335x-evm): 33 echo platform > /sys/power/pm_test 34 echo 1 > /sys/power/pm_print_times 35 echo mem > /sys/power/state I was able to reproduce and play with it on am335x-evm, unfortunately you are right and pm_runtime_force_x() APIs will not work out of the box, because USB framework keeps a lot of devices in its hierarchy in PM runtime Active state (and this hierarchy is not static - depends on what is plugged in port during suspend). So, I see two option here: 1) use pm_clk_suspend/pm_clk_resume() directly in .suspend()/.resume() 2) do some hacks as in diff below you might not need get/put in suspend as you going do get in .probe() without put. >From ac0178455f8dfda635d8d45e8235d73b936a19a9 Mon Sep 17 00:00:00 2001 From: Grygorii Strashko <grygorii.strashko-l0cyMroinI0@public.gmane.org> Date: Mon, 3 Apr 2017 14:53:57 -0500 Subject: [PATCH] [draft] drivers/usb/musb/musb_dsps: do suspend Signed-off-by: Grygorii Strashko <grygorii.strashko-l0cyMroinI0@public.gmane.org> --- drivers/usb/musb/musb_dsps.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c index 9c7ee26..43306a7 100644 --- a/drivers/usb/musb/musb_dsps.c +++ b/drivers/usb/musb/musb_dsps.c @@ -1011,6 +1011,8 @@ static int dsps_suspend(struct device *dev) struct musb *musb = platform_get_drvdata(glue->musb); void __iomem *mbase; + pm_runtime_get_sync(dev); + del_timer_sync(&glue->timer); if (!musb) @@ -1027,8 +1029,9 @@ static int dsps_suspend(struct device *dev) glue->context.rx_mode = musb_readl(mbase, wrp->rx_mode); dsps_dma_controller_suspend(glue); + pm_suspend_ignore_children(dev, true); - return 0; + return pm_runtime_force_suspend(dev); } static int dsps_resume(struct device *dev) @@ -1040,6 +1043,8 @@ static int dsps_resume(struct device *dev) if (!musb) return 0; + pm_suspend_ignore_children(dev, false); + pm_runtime_force_resume(dev); dsps_dma_controller_resume(glue); @@ -1055,6 +1060,8 @@ static int dsps_resume(struct device *dev) musb->port_mode == MUSB_PORT_MODE_DUAL_ROLE) dsps_mod_timer(glue, -1); + pm_runtime_put(dev); + return 0; } #endif -- 2.10.1 -- regards, -grygorii -- 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 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [RESEND PATCH v6 4/4] usb: musb: da8xx: Add a primary support of PM runtime [not found] ` <20170324143600.4704-5-abailon-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> 2017-03-24 17:26 ` Grygorii Strashko @ 2017-03-28 0:39 ` Kevin Hilman [not found] ` <m24lyeck07.fsf-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> 1 sibling, 1 reply; 12+ messages in thread From: Kevin Hilman @ 2017-03-28 0:39 UTC (permalink / raw) To: Alexandre Bailon Cc: b-liu-l0cyMroinI0, linux-usb-u79uwXL29TY76Z2rM5mHXA, ptitiano-rdvid1DuHRBWk0Htik3J/w, nsekhar-l0cyMroinI0, tony-4v6yS6AI5VpBDgjK7y7TUQ, sergei.shtylyov-M4DtvfQ/ZS1MRgGoP+s0PdBPR1lH4CV8, linux-omap-u79uwXL29TY76Z2rM5mHXA Alexandre Bailon <abailon-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> writes: > Currently, MUSB DA8xx glue driver doesn't have PM runtime support. > Because the CPPI 4.1 is using the same clock as MUSB DA8xx and > CPPI 4.1 is a child of MUSB DA8xx glue, add support of PM runtime > to the DA8xx glue driver in order to let the CPPI 4.1 driver manage > the clock by using PM runtime. > > Signed-off-by: Alexandre Bailon <abailon-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> Dumb question: even if the clock is shared with cppi4, doesn't the use-couting in the clock API handle it so that things should function fine even without this patch? Some other comments/questions below... > --- > drivers/usb/musb/da8xx.c | 27 ++++++++------------------- > 1 file changed, 8 insertions(+), 19 deletions(-) > > diff --git a/drivers/usb/musb/da8xx.c b/drivers/usb/musb/da8xx.c > index ed28afd..89e12f6 100644 > --- a/drivers/usb/musb/da8xx.c > +++ b/drivers/usb/musb/da8xx.c > @@ -30,7 +30,6 @@ > */ > > #include <linux/module.h> > -#include <linux/clk.h> > #include <linux/err.h> > #include <linux/io.h> > #include <linux/of_platform.h> > @@ -86,7 +85,6 @@ struct da8xx_glue { > struct device *dev; > struct platform_device *musb; > struct platform_device *usb_phy; > - struct clk *clk; > struct phy *phy; > }; > > @@ -376,11 +374,7 @@ static int da8xx_musb_init(struct musb *musb) > > musb->mregs += DA8XX_MENTOR_CORE_OFFSET; > > - ret = clk_prepare_enable(glue->clk); > - if (ret) { > - dev_err(glue->dev, "failed to enable clock\n"); > - return ret; > - } > + pm_runtime_get_sync(musb->controller->parent); > > /* Returns zero if e.g. not clocked */ > rev = musb_readl(reg_base, DA8XX_USB_REVISION_REG); > @@ -423,7 +417,7 @@ static int da8xx_musb_init(struct musb *musb) > err_phy_power_on: > phy_exit(glue->phy); > fail: > - clk_disable_unprepare(glue->clk); > + pm_runtime_put(musb->controller->parent); > return ret; > } > > @@ -435,7 +429,7 @@ static int da8xx_musb_exit(struct musb *musb) > > phy_power_off(glue->phy); > phy_exit(glue->phy); > - clk_disable_unprepare(glue->clk); > + pm_runtime_put(musb->controller->parent); > > usb_put_phy(musb->xceiv); > > @@ -519,7 +513,6 @@ static int da8xx_probe(struct platform_device *pdev) > struct musb_hdrc_platform_data *pdata = dev_get_platdata(&pdev->dev); > struct da8xx_glue *glue; > struct platform_device_info pinfo; > - struct clk *clk; > struct device_node *np = pdev->dev.of_node; > int ret; > > @@ -527,12 +520,6 @@ static int da8xx_probe(struct platform_device *pdev) > if (!glue) > return -ENOMEM; > > - clk = devm_clk_get(&pdev->dev, "usb20"); > - if (IS_ERR(clk)) { > - dev_err(&pdev->dev, "failed to get clock\n"); > - return PTR_ERR(clk); > - } Something isn't quite right here. This clk_get uses a con_id "usb20", but when converting to runtime PM, we rely on the pm_clk layer (used on davinci for runtime PM) to do clock lookups by the default con_ids. I guess this still probably works because we fallback to the NULL con_id. > glue->phy = devm_phy_get(&pdev->dev, "usb-phy"); > if (IS_ERR(glue->phy)) { > if (PTR_ERR(glue->phy) != -EPROBE_DEFER) > @@ -541,7 +528,6 @@ static int da8xx_probe(struct platform_device *pdev) > } > > glue->dev = &pdev->dev; > - glue->clk = clk; > > if (IS_ENABLED(CONFIG_OF) && np) { > pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL); > @@ -587,6 +573,8 @@ static int da8xx_probe(struct platform_device *pdev) > pinfo.data = pdata; > pinfo.size_data = sizeof(*pdata); > > + pm_runtime_enable(&pdev->dev); > + > glue->musb = platform_device_register_full(&pinfo); > ret = PTR_ERR_OR_ZERO(glue->musb); > if (ret) { > @@ -603,6 +591,7 @@ static int da8xx_remove(struct platform_device *pdev) > > platform_device_unregister(glue->musb); > usb_phy_generic_unregister(glue->usb_phy); > + pm_runtime_disable(&pdev->dev); > > return 0; > } > @@ -616,7 +605,7 @@ static int da8xx_suspend(struct device *dev) > ret = phy_power_off(glue->phy); > if (ret) > return ret; > - clk_disable_unprepare(glue->clk); > + pm_runtime_put_sync(dev); I'm a bit lots in the MUSB layering here, but does this 'dev' correspond to musb->controller->parent? Kevin > return 0; > } > @@ -626,7 +615,7 @@ static int da8xx_resume(struct device *dev) > int ret; > struct da8xx_glue *glue = dev_get_drvdata(dev); > > - ret = clk_prepare_enable(glue->clk); > + ret = pm_runtime_get_sync(dev); > if (ret) > return ret; > return phy_power_on(glue->phy); -- 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
[parent not found: <m24lyeck07.fsf-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>]
* Re: [RESEND PATCH v6 4/4] usb: musb: da8xx: Add a primary support of PM runtime [not found] ` <m24lyeck07.fsf-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> @ 2017-03-29 6:15 ` Sekhar Nori 0 siblings, 0 replies; 12+ messages in thread From: Sekhar Nori @ 2017-03-29 6:15 UTC (permalink / raw) To: Kevin Hilman, Alexandre Bailon Cc: b-liu-l0cyMroinI0, linux-usb-u79uwXL29TY76Z2rM5mHXA, ptitiano-rdvid1DuHRBWk0Htik3J/w, tony-4v6yS6AI5VpBDgjK7y7TUQ, sergei.shtylyov-M4DtvfQ/ZS1MRgGoP+s0PdBPR1lH4CV8, linux-omap-u79uwXL29TY76Z2rM5mHXA On Tuesday 28 March 2017 06:09 AM, Kevin Hilman wrote: > Alexandre Bailon <abailon-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> writes: > >> Currently, MUSB DA8xx glue driver doesn't have PM runtime support. >> Because the CPPI 4.1 is using the same clock as MUSB DA8xx and >> CPPI 4.1 is a child of MUSB DA8xx glue, add support of PM runtime >> to the DA8xx glue driver in order to let the CPPI 4.1 driver manage >> the clock by using PM runtime. >> >> Signed-off-by: Alexandre Bailon <abailon-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> > > Dumb question: even if the clock is shared with cppi4, doesn't the > use-couting in the clock API handle it so that things should function > fine even without this patch? > > Some other comments/questions below... > >> --- >> drivers/usb/musb/da8xx.c | 27 ++++++++------------------- >> 1 file changed, 8 insertions(+), 19 deletions(-) >> >> diff --git a/drivers/usb/musb/da8xx.c b/drivers/usb/musb/da8xx.c >> index ed28afd..89e12f6 100644 >> --- a/drivers/usb/musb/da8xx.c >> +++ b/drivers/usb/musb/da8xx.c >> @@ -30,7 +30,6 @@ >> */ >> >> #include <linux/module.h> >> -#include <linux/clk.h> >> #include <linux/err.h> >> #include <linux/io.h> >> #include <linux/of_platform.h> >> @@ -86,7 +85,6 @@ struct da8xx_glue { >> struct device *dev; >> struct platform_device *musb; >> struct platform_device *usb_phy; >> - struct clk *clk; >> struct phy *phy; >> }; >> >> @@ -376,11 +374,7 @@ static int da8xx_musb_init(struct musb *musb) >> >> musb->mregs += DA8XX_MENTOR_CORE_OFFSET; >> >> - ret = clk_prepare_enable(glue->clk); >> - if (ret) { >> - dev_err(glue->dev, "failed to enable clock\n"); >> - return ret; >> - } >> + pm_runtime_get_sync(musb->controller->parent); >> >> /* Returns zero if e.g. not clocked */ >> rev = musb_readl(reg_base, DA8XX_USB_REVISION_REG); >> @@ -423,7 +417,7 @@ static int da8xx_musb_init(struct musb *musb) >> err_phy_power_on: >> phy_exit(glue->phy); >> fail: >> - clk_disable_unprepare(glue->clk); >> + pm_runtime_put(musb->controller->parent); >> return ret; >> } >> >> @@ -435,7 +429,7 @@ static int da8xx_musb_exit(struct musb *musb) >> >> phy_power_off(glue->phy); >> phy_exit(glue->phy); >> - clk_disable_unprepare(glue->clk); >> + pm_runtime_put(musb->controller->parent); >> >> usb_put_phy(musb->xceiv); >> >> @@ -519,7 +513,6 @@ static int da8xx_probe(struct platform_device *pdev) >> struct musb_hdrc_platform_data *pdata = dev_get_platdata(&pdev->dev); >> struct da8xx_glue *glue; >> struct platform_device_info pinfo; >> - struct clk *clk; >> struct device_node *np = pdev->dev.of_node; >> int ret; >> >> @@ -527,12 +520,6 @@ static int da8xx_probe(struct platform_device *pdev) >> if (!glue) >> return -ENOMEM; >> >> - clk = devm_clk_get(&pdev->dev, "usb20"); >> - if (IS_ERR(clk)) { >> - dev_err(&pdev->dev, "failed to get clock\n"); >> - return PTR_ERR(clk); >> - } > > Something isn't quite right here. > > This clk_get uses a con_id "usb20", but when converting to runtime PM, > we rely on the pm_clk layer (used on davinci for runtime PM) to do clock > lookups by the default con_ids. I guess this still probably works > because we fallback to the NULL con_id. Right, since DaVinci uses pm_clk layer for runtime PM, we support runtime PM on only two types of clocks. Those with con_id in the list of con_ids recognized by pm_clk layer or those with NULL con_id. For devices which use a single clock, NULL con_id is preferred. Thanks, Sekhar -- 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2017-04-03 20:14 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-03-24 14:35 [RESEND PATCH v6 0/4] usb: musb: da8xx: Add DMA support Alexandre Bailon [not found] ` <20170324143600.4704-1-abailon-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> 2017-03-24 14:35 ` [RESEND PATCH v6 1/4] usb: musb: Use shared irq Alexandre Bailon 2017-03-24 14:35 ` [RESEND PATCH v6 2/4] usb: musb: Add support of CPPI 4.1 DMA controller to DA8xx Alexandre Bailon 2017-03-24 14:35 ` [RESEND PATCH v6 3/4] usb: musb: musb_cppi41: Workaround DMA stall issue during teardown Alexandre Bailon 2017-03-24 14:36 ` [RESEND PATCH v6 4/4] usb: musb: da8xx: Add a primary support of PM runtime Alexandre Bailon [not found] ` <20170324143600.4704-5-abailon-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> 2017-03-24 17:26 ` Grygorii Strashko [not found] ` <703e5e95-8bde-2b1c-0d63-4ed3e03f53b8-l0cyMroinI0@public.gmane.org> 2017-03-27 16:39 ` Alexandre Bailon [not found] ` <03cc95cd-5967-c0c7-06a7-89cf08dbde47-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> 2017-03-27 17:38 ` Grygorii Strashko [not found] ` <1552a8e0-9efe-3658-e511-76e056ed5588-l0cyMroinI0@public.gmane.org> 2017-03-27 19:54 ` Alexandre Bailon [not found] ` <322bdb6b-72c4-01e1-8ae7-bf04ea89a1ee-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> 2017-04-03 20:14 ` Grygorii Strashko 2017-03-28 0:39 ` Kevin Hilman [not found] ` <m24lyeck07.fsf-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> 2017-03-29 6:15 ` Sekhar Nori
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.