Hi,
On Tuesday 02 January 2018 09:37 PM, Marc Kleine-Budde wrote:
> On 12/22/2017 02:31 PM, Faiz Abbas wrote:
>> From: Franklin S Cooper Jr <fcooper@xxxxxx>
>>
>> Add support for PM Runtime which is the new way to handle managing clocks.
>> However, to avoid breaking SoCs not using PM_RUNTIME leave the old clk
>> management approach in place.
>
> There is no PM_RUNTIME anymore since 464ed18ebdb6 ("PM: Eliminate
> CONFIG_PM_RUNTIME")
Ok. Will change the commit message.
>
> Have a look at the discussion: https://patchwork.kernel.org/patch/9436507/ :
>
>>> Well, I admit it would be nicer if drivers didn't have to worry about
>>> whether or not CONFIG_PM was enabled. A slightly cleaner approach
>>> from the one outlined above would have the probe routine do this:
>>>
>>> my_power_up(dev);
>>> pm_runtime_set_active(dev);
>>> pm_runtime_get_noresume(dev);
>>> pm_runtime_enable(dev);
This discussion seems to be about cases in which CONFIG_PM is not
enabled. CONFIG_PM is always selected in the case of omap devices.
>
>> PM_RUNTIME is required by OMAP based devices to handle clock management.
>> Therefore, this allows future Texas Instruments SoCs that have the MCAN IP
>> to work with this driver.
>
> Who will set the SET_RUNTIME_PM_OPS in this case?
It is set with a common SET_RUNTIME_PM_OPS in the case of omap at
arch/arm/mach-omap2/omap_device.c:632
struct dev_pm_domain omap_device_pm_domain = {
.ops = {
SET_RUNTIME_PM_OPS(_od_runtime_suspend, _od_runtime_resume,
NULL)
USE_PLATFORM_PM_SLEEP_OPS
SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(_od_suspend_noirq,
_od_resume_noirq)
}
};
>
>> Signed-off-by: Franklin S Cooper Jr <fcooper@xxxxxx>
>> [nsekhar@xxxxxx: handle pm_runtime_get_sync() failure, fix some bugs]
>> Signed-off-by: Sekhar Nori <nsekhar@xxxxxx>
>> Signed-off-by: Faiz Abbas <faiz_abbas@xxxxxx>
>> ---
>> drivers/net/can/m_can/m_can.c | 38 ++++++++++++++++++++++++++++++++++----
>> 1 file changed, 34 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
>> index f72116e..53e764f 100644
>> --- a/drivers/net/can/m_can/m_can.c
>> +++ b/drivers/net/can/m_can/m_can.c
>> @@ -23,6 +23,7 @@
>> #include <linux/of.h>
>> #include <linux/of_device.h>
>> #include <linux/platform_device.h>
>> +#include <linux/pm_runtime.h>
>> #include <linux/iopoll.h>
>> #include <linux/can/dev.h>
>>
>> @@ -625,19 +626,33 @@ static int m_can_clk_start(struct m_can_priv *priv)
>> {
>> int err;
>>
>> + err = pm_runtime_get_sync(priv->device);
>> + if (err) {
>> + pm_runtime_put_noidle(priv->device);
>
> Why do you call this in case of an error?
pm_runtime_get_sync() increments the usage count of the device before
any error is returned. This needs to be decremented using
pm_runtime_put_noidle().
>
>> + return err;
>> + }
>> +
>> err = clk_prepare_enable(priv->hclk);
>> if (err)
>> - return err;
>> + goto pm_runtime_put;
>>
>> err = clk_prepare_enable(priv->cclk);
>> if (err)
>> - clk_disable_unprepare(priv->hclk);
>> + goto disable_hclk;
>>
>> return err;
>> +
>> +disable_hclk:
>> + clk_disable_unprepare(priv->hclk);
>> +pm_runtime_put:
>> + pm_runtime_put_sync(priv->device);
>> + return err;
>> }
>>
>> static void m_can_clk_stop(struct m_can_priv *priv)
>> {
>> + pm_runtime_put_sync(priv->device);
>> +
>> clk_disable_unprepare(priv->cclk);
>> clk_disable_unprepare(priv->hclk);
>> }
>> @@ -1577,13 +1592,20 @@ static int m_can_plat_probe(struct platform_device *pdev)
>> /* Enable clocks. Necessary to read Core Release in order to determine
>> * M_CAN version
>> */
>> + pm_runtime_enable(&pdev->dev);
>> + ret = pm_runtime_get_sync(&pdev->dev);
>> + if (ret) {
>> + pm_runtime_put_noidle(&pdev->dev);
>
> Why do you call this in case of error?
Same here.
Thanks,
Faiz
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
