Re: [PATCH] sh_eth: ensure pm_runtime cannot suspend the device during init

[Ben Dooks <ben.dooks@codethink.co.uk>]

On 15/03/14 11:19, Laurent Pinchart wrote:
> Hi Ben,
>
> Thank you for the patch.
>
> On Friday 14 March 2014 19:00:05 Ben Dooks wrote:
>> It seems the pm_rumtime work queue is causing the device to be suspended
>> during initialisation,
>
> Have you investigated through which call path(s) this happens ? If device can
> be runtime suspended during their probe function despite calling
> pm_runtime_resume() then I don't see the point of that function at all.
>
>> thus the initialisation may not be able to access registers properly. Use
>> pm_runtime_get_sync() and pm_runtime_put_sync() to ensure that the pm system
>> does not suspend it during the probe() call.
>>
>> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
>> ---
>>   drivers/net/ethernet/renesas/sh_eth.c | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/net/ethernet/renesas/sh_eth.c
>> b/drivers/net/ethernet/renesas/sh_eth.c index 4f76b5e..88aac2c 100644
>> --- a/drivers/net/ethernet/renesas/sh_eth.c
>> +++ b/drivers/net/ethernet/renesas/sh_eth.c
>> @@ -2870,6 +2870,7 @@ static int sh_eth_drv_probe(struct platform_device
>> *pdev) mdp->pdev = pdev;
>>   	pm_runtime_enable(&pdev->dev);
>>   	pm_runtime_resume(&pdev->dev);
>> +	pm_runtime_get_sync(&pdev->dev);
>
> I believe pm_runtime_resume() isn't needed anymore if we call
> pm_runtime_get_sync().
>
>>   	if (pdev->dev.of_node)
>>   		pd = sh_eth_parse_dt(&pdev->dev);
>> @@ -2961,6 +2962,7 @@ static int sh_eth_drv_probe(struct platform_device
>> *pdev) pr_info("Base address at 0x%x, %pM, IRQ %d.\n",
>>   		(u32)ndev->base_addr, ndev->dev_addr, ndev->irq);
>>
>> +	pm_runtime_put_sync(&pdev->dev);
>>   	platform_set_drvdata(pdev, ndev);
>>
>>   	return ret;

I will look at removing the pm_runtime_resume call as well in this
patch.

-- 
Ben Dooks				http://www.codethink.co.uk/
Senior Engineer				Codethink - Providing Genius