On Mon, Aug 20, 2012 at 16:33:43, AnilKumar, Chimata wrote:

On Mon, Aug 20, 2012 at 16:27:53, Marc Kleine-Budde wrote:

quoted

On 08/20/2012 12:41 PM, Marc Kleine-Budde wrote:

quoted

On 08/20/2012 10:45 AM, AnilKumar Ch wrote:

quoted

Add Runtime PM support to C_CAN/D_CAN controller. The runtime PM
APIs control clocks for C_CAN/D_CAN IP and prevent access to the
register of C_CAN/D_CAN IP when clock is turned off.

Signed-off-by: AnilKumar Ch <redacted>
---
This patch has been tested on AM335X EVM. Due to lack of hardware
I am not able to test c_can functionality. I appreciate if anyone
can test c_can functionality with this patch.

This patch is based on "can-next/master" 

Changes from v7:
	- Incorporated Marc's commets on v7
	  * changed device pointer to c_can_priv pointer

looks good.

[...]

quoted

--- a/drivers/net/can/c_can/c_can_platform.c
+++ b/drivers/net/can/c_can/c_can_platform.c

@@ -32,6 +32,7 @@
 #include <linux/clk.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
+#include <linux/pm_runtime.h>
 
 #include <linux/can/dev.h>
 

@@ -177,8 +178,11 @@ static int __devinit c_can_plat_probe(struct platform_device *pdev)
 		goto exit_free_device;

                     ^^^^^^^^^^^^^^^^

quoted

 	}
 
+	pm_runtime_enable(&pdev->dev);
+
 	dev->irq = irq;
 	priv->base = addr;
+	priv->device = &pdev->dev;
 	priv->can.clock.freq = clk_get_rate(clk);
 	priv->priv = clk;
 

@@ -198,6 +202,7 @@ static int __devinit c_can_plat_probe(struct platform_device *pdev)
 
 exit_free_device:
 	platform_set_drvdata(pdev, NULL);
+	pm_runtime_disable(&pdev->dev);

I think this will end up in a NULL pointer deferf.

Should be no problem, as you're using pdev->dev not priv->device.

Sorry for the noise :)

One more catch here, return path is not proper.

exit_free_device: should contain only free_c_can_dev(dev);

And second goto exit_free_device; call should modify to
goto exit_clear_drvdata;

return path looks like this

exit_clear_drvdata:
	platform_set_drvdata(pdev, NULL);
	pm_runtime_disable(&pdev->dev);
exit_free_device:
	free_c_can_dev(dev);
...

Regards
AnilKumar