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@xxxxxxxxxxxx>
>> > ---
>> > 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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
