netdev - Re: [PATCH v3] can: Convert to runtime

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <5477ABF3.1020605@pengutronix.de>
Date:	Thu, 27 Nov 2014 23:55:47 +0100
From:	Marc Kleine-Budde <mkl@...gutronix.de>
To:	Sören Brinkmann <soren.brinkmann@...inx.com>
CC:	Kedareswara rao Appana <appana.durga.rao@...inx.com>,
	wg@...ndegger.com, michal.simek@...inx.com,
	grant.likely@...aro.org, robh+dt@...nel.org,
	linux-can@...r.kernel.org, netdev@...r.kernel.org,
	linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
	devicetree@...r.kernel.org,
	Kedareswara rao Appana <appanad@...inx.com>
Subject: Re: [PATCH v3] can: Convert to runtime_pm

On 11/27/2014 11:47 PM, Sören Brinkmann wrote:
> On Thu, 2014-11-27 at 10:17PM +0100, Marc Kleine-Budde wrote:
>> On 11/27/2014 02:08 PM, Kedareswara rao Appana wrote:
>>> Instead of enabling/disabling clocks at several locations in the driver,
>>> use the runtime_pm framework. This consolidates the actions for
>>> runtime PM in the appropriate callbacks and makes the driver more
>>> readable and mantainable.
>>>
>>> Signed-off-by: Soren Brinkmann <soren.brinkmann@...inx.com>
>>> Signed-off-by: Kedareswara rao Appana <appanad@...inx.com>
>>> ---
>>> Changes for v3:
>>>   - Converted the driver to use runtime_pm.
>>> Changes for v2:
>>>   - Removed the struct platform_device* from suspend/resume
>>>     as suggest by Lothar.
>>>
>>>  drivers/net/can/xilinx_can.c |  119 +++++++++++++++++++++++++----------------
>>>  1 files changed, 72 insertions(+), 47 deletions(-)
>>>
>>> diff --git a/drivers/net/can/xilinx_can.c b/drivers/net/can/xilinx_can.c
>>> index 8a998e3..1be28ed 100644
>>> --- a/drivers/net/can/xilinx_can.c
>>> +++ b/drivers/net/can/xilinx_can.c
> [...]
>>> @@ -1030,7 +1046,10 @@ static int __maybe_unused xcan_resume(struct device *dev)
>>>  	return 0;
>>>  }
>>>  
>>> -static SIMPLE_DEV_PM_OPS(xcan_dev_pm_ops, xcan_suspend, xcan_resume);
>>> +static const struct dev_pm_ops xcan_dev_pm_ops = {
>>> +	SET_SYSTEM_SLEEP_PM_OPS(xcan_suspend, xcan_resume)
>>> +	SET_PM_RUNTIME_PM_OPS(xcan_runtime_suspend, xcan_runtime_resume, NULL)
>>> +};
>>>  
>>>  /**
>>>   * xcan_probe - Platform registration call
>>> @@ -1071,7 +1090,7 @@ static int xcan_probe(struct platform_device *pdev)
>>>  		return -ENOMEM;
>>>  
>>>  	priv = netdev_priv(ndev);
>>> -	priv->dev = ndev;
>>> +	priv->dev = &pdev->dev;
>>>  	priv->can.bittiming_const = &xcan_bittiming_const;
>>>  	priv->can.do_set_mode = xcan_do_set_mode;
>>>  	priv->can.do_get_berr_counter = xcan_get_berr_counter;
>>> @@ -1137,6 +1156,11 @@ static int xcan_probe(struct platform_device *pdev)
>>>  
>>>  	netif_napi_add(ndev, &priv->napi, xcan_rx_poll, rx_max);
>>>  
>>> +	pm_runtime_set_active(&pdev->dev);
>>> +	pm_runtime_irq_safe(&pdev->dev);
>>
>> You use just clock_enable()/disable() in the runtime functions, thus you
>> can say they are irq_safe. On the other the the zync grpio driver uses
>> "full" prepare_enable/disable_unprepare calls. What's best practice here?
> 
> IIRC, the prepare/unprepare functions can sleep. xcan_get_berr_counter
> is called from atomic context. So, I think we have to use the
> disable/enable functions without the prepare/unprepare.
> In the GPIO driver the that problem does not exist.

IC, yes, correct.

This is why we introducted in other drivers a __get_berr_counter()
function, that doesn't touch the clocks, which is used from within the
driver (from the atomic contects), while get_berr_counter() will fiddle
with the clocks. This function is used for the
priv->can.do_get_berr_counter callback.

Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


Download attachment "signature.asc" of type "application/pgp-signature" (820 bytes)