[v9] can: c_can: Add runtime PM support to Bosch C_CAN/D_CAN controller
diff mbox

Message ID 1345461654-439-1-git-send-email-anilkumar@ti.com
State New, archived
Headers show

Commit Message

AnilKumar, Chimata Aug. 20, 2012, 11:20 a.m. UTC
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 <anilkumar@ti.com>
---
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 v8:
	- corrected the return path sequence in c_can_probe()

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

Changes from v6:
	- Incorporated Marc's comments on v6
	  * changed dev pointer to priv
	  * removed platform_device.h include from c_can.c

Changes from v5:
	- Incorporated Marc's comments on v5
	  * changed runtime pm calls in c_can driver to handle
	    the drivers which are not using platform drivers.
	  * added device pointer protection in c_can driver if
	    not passed from platform/pci driver.

Changes from v4:
	- Incorporated Vaibhav H review comments on v4.
	  * Moved pm_runtime put/get_sync calls to appropriate positions.
	- This patch is from "Add DT support to C_CAN/D_CAN controller"
	  patch series. Rest of the patches in this series were applied
	  so this v5 contains only this patch.

 drivers/net/can/c_can/c_can.c          |   24 +++++++++++++++++++++++-
 drivers/net/can/c_can/c_can.h          |    1 +
 drivers/net/can/c_can/c_can_platform.c |   11 +++++++++--
 3 files changed, 33 insertions(+), 3 deletions(-)

Comments

AnilKumar, Chimata Aug. 27, 2012, 5:06 a.m. UTC | #1
Hi Marc,

On Mon, Aug 20, 2012 at 16:50:54, AnilKumar, Chimata wrote:
> 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 <anilkumar@ti.com>
> ---
> 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 v8:
> 	- corrected the return path sequence in c_can_probe()
> 
> Changes from v7:
> 	- Incorporated Marc's comments on v7
> 	  * changed device pointer to c_can_priv pointer
> 
> Changes from v6:
> 	- Incorporated Marc's comments on v6
> 	  * changed dev pointer to priv
> 	  * removed platform_device.h include from c_can.c
> 
> Changes from v5:
> 	- Incorporated Marc's comments on v5
> 	  * changed runtime pm calls in c_can driver to handle
> 	    the drivers which are not using platform drivers.
> 	  * added device pointer protection in c_can driver if
> 	    not passed from platform/pci driver.
> 
> Changes from v4:
> 	- Incorporated Vaibhav H review comments on v4.
> 	  * Moved pm_runtime put/get_sync calls to appropriate positions.
> 	- This patch is from "Add DT support to C_CAN/D_CAN controller"
> 	  patch series. Rest of the patches in this series were applied
> 	  so this v5 contains only this patch.
> 
>  drivers/net/can/c_can/c_can.c          |   24 +++++++++++++++++++++++-
>  drivers/net/can/c_can/c_can.h          |    1 +
>  drivers/net/can/c_can/c_can_platform.c |   11 +++++++++--
>  3 files changed, 33 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/can/c_can/c_can.c b/drivers/net/can/c_can/c_can.c
> index 4c538e3..966d318 100644
> --- a/drivers/net/can/c_can/c_can.c
> +++ b/drivers/net/can/c_can/c_can.c
> @@ -34,6 +34,7 @@
>  #include <linux/if_ether.h>
>  #include <linux/list.h>
>  #include <linux/io.h>
> +#include <linux/pm_runtime.h>
>  
>  #include <linux/can.h>
>  #include <linux/can/dev.h>
> @@ -201,6 +202,18 @@ static const struct can_bittiming_const c_can_bittiming_const = {
>  	.brp_inc = 1,
>  };
>  
> +static inline void c_can_pm_runtime_get_sync(struct c_can_priv *priv)
> +{
> +	if (priv->device)
> +		pm_runtime_get_sync(priv->device);
> +}
> +
> +static inline void c_can_pm_runtime_put_sync(struct c_can_priv *priv)
> +{
> +	if (priv->device)
> +		pm_runtime_put_sync(priv->device);
> +}
> +
>  static inline int get_tx_next_msg_obj(const struct c_can_priv *priv)
>  {
>  	return (priv->tx_next & C_CAN_NEXT_MSG_OBJ_MASK) +
> @@ -673,11 +686,15 @@ static int c_can_get_berr_counter(const struct net_device *dev,
>  	unsigned int reg_err_counter;
>  	struct c_can_priv *priv = netdev_priv(dev);
>  
> +	c_can_pm_runtime_get_sync(priv);
> +
>  	reg_err_counter = priv->read_reg(priv, C_CAN_ERR_CNT_REG);
>  	bec->rxerr = (reg_err_counter & ERR_CNT_REC_MASK) >>
>  				ERR_CNT_REC_SHIFT;
>  	bec->txerr = reg_err_counter & ERR_CNT_TEC_MASK;
>  
> +	c_can_pm_runtime_put_sync(priv);
> +
>  	return 0;
>  }
>  
> @@ -1053,11 +1070,13 @@ static int c_can_open(struct net_device *dev)
>  	int err;
>  	struct c_can_priv *priv = netdev_priv(dev);
>  
> +	c_can_pm_runtime_get_sync(priv);
> +
>  	/* open the can device */
>  	err = open_candev(dev);
>  	if (err) {
>  		netdev_err(dev, "failed to open can device\n");
> -		return err;
> +		goto exit_open_fail;
>  	}
>  
>  	/* register interrupt handler */
> @@ -1079,6 +1098,8 @@ static int c_can_open(struct net_device *dev)
>  
>  exit_irq_fail:
>  	close_candev(dev);
> +exit_open_fail:
> +	c_can_pm_runtime_put_sync(priv);
>  	return err;
>  }
>  
> @@ -1091,6 +1112,7 @@ static int c_can_close(struct net_device *dev)
>  	c_can_stop(dev);
>  	free_irq(dev->irq, dev);
>  	close_candev(dev);
> +	c_can_pm_runtime_put_sync(priv);
>  
>  	return 0;
>  }
> diff --git a/drivers/net/can/c_can/c_can.h b/drivers/net/can/c_can/c_can.h
> index 4e56baa..1437a6d 100644
> --- a/drivers/net/can/c_can/c_can.h
> +++ b/drivers/net/can/c_can/c_can.h
> @@ -153,6 +153,7 @@ struct c_can_priv {
>  	struct can_priv can;	/* must be the first member */
>  	struct napi_struct napi;
>  	struct net_device *dev;
> +	struct device *device;
>  	int tx_object;
>  	int current_status;
>  	int last_status;
> diff --git a/drivers/net/can/c_can/c_can_platform.c b/drivers/net/can/c_can/c_can_platform.c
> index d0a66cf..90801c4 100644
> --- 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;
>  	}
>  
> +	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;
>  
> @@ -189,15 +193,17 @@ static int __devinit c_can_plat_probe(struct platform_device *pdev)
>  	if (ret) {
>  		dev_err(&pdev->dev, "registering %s failed (err=%d)\n",
>  			KBUILD_MODNAME, ret);
> -		goto exit_free_device;
> +		goto exit_clear_drvdata;
>  	}
>  
>  	dev_info(&pdev->dev, "%s device registered (regs=%p, irq=%d)\n",
>  		 KBUILD_MODNAME, priv->base, dev->irq);
>  	return 0;
>  
> -exit_free_device:
> +exit_clear_drvdata:
>  	platform_set_drvdata(pdev, NULL);
> +	pm_runtime_disable(&pdev->dev);
> +exit_free_device:
>  	free_c_can_dev(dev);
>  exit_iounmap:
>  	iounmap(addr);
> @@ -226,6 +232,7 @@ static int __devexit c_can_plat_remove(struct platform_device *pdev)
>  	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>  	release_mem_region(mem->start, resource_size(mem));
>  
> +	pm_runtime_disable(&pdev->dev);
>  	clk_put(priv->priv);
>  
>  	return 0;
> -- 

If there are no comments can you please push this patch

Thanks
AnilKumar
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Marc Kleine-Budde Sept. 3, 2012, 8:52 a.m. UTC | #2
On 08/20/2012 01:20 PM, AnilKumar Ch wrote:
> 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 <anilkumar@ti.com>
> ---
> 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.

Looks good. I've marked the c_can_priv *priv pointer as const in
c_can_pm_runtime_{get,put}_sync.

thanks, Marc

> 
> This patch is based on "can-next/master" 
> 
> Changes from v8:
> 	- corrected the return path sequence in c_can_probe()
> 
> Changes from v7:
> 	- Incorporated Marc's comments on v7
> 	  * changed device pointer to c_can_priv pointer
> 
> Changes from v6:
> 	- Incorporated Marc's comments on v6
> 	  * changed dev pointer to priv
> 	  * removed platform_device.h include from c_can.c
> 
> Changes from v5:
> 	- Incorporated Marc's comments on v5
> 	  * changed runtime pm calls in c_can driver to handle
> 	    the drivers which are not using platform drivers.
> 	  * added device pointer protection in c_can driver if
> 	    not passed from platform/pci driver.
> 
> Changes from v4:
> 	- Incorporated Vaibhav H review comments on v4.
> 	  * Moved pm_runtime put/get_sync calls to appropriate positions.
> 	- This patch is from "Add DT support to C_CAN/D_CAN controller"
> 	  patch series. Rest of the patches in this series were applied
> 	  so this v5 contains only this patch.
> 
>  drivers/net/can/c_can/c_can.c          |   24 +++++++++++++++++++++++-
>  drivers/net/can/c_can/c_can.h          |    1 +
>  drivers/net/can/c_can/c_can_platform.c |   11 +++++++++--
>  3 files changed, 33 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/can/c_can/c_can.c b/drivers/net/can/c_can/c_can.c
> index 4c538e3..966d318 100644
> --- a/drivers/net/can/c_can/c_can.c
> +++ b/drivers/net/can/c_can/c_can.c
> @@ -34,6 +34,7 @@
>  #include <linux/if_ether.h>
>  #include <linux/list.h>
>  #include <linux/io.h>
> +#include <linux/pm_runtime.h>
>  
>  #include <linux/can.h>
>  #include <linux/can/dev.h>
> @@ -201,6 +202,18 @@ static const struct can_bittiming_const c_can_bittiming_const = {
>  	.brp_inc = 1,
>  };
>  
> +static inline void c_can_pm_runtime_get_sync(struct c_can_priv *priv)

I've marked the priv pointer as const.

> +{
> +	if (priv->device)
> +		pm_runtime_get_sync(priv->device);
> +}
> +
> +static inline void c_can_pm_runtime_put_sync(struct c_can_priv *priv)

dito.

> +{
> +	if (priv->device)
> +		pm_runtime_put_sync(priv->device);
> +}
> +
>  static inline int get_tx_next_msg_obj(const struct c_can_priv *priv)
>  {
>  	return (priv->tx_next & C_CAN_NEXT_MSG_OBJ_MASK) +
> @@ -673,11 +686,15 @@ static int c_can_get_berr_counter(const struct net_device *dev,
>  	unsigned int reg_err_counter;
>  	struct c_can_priv *priv = netdev_priv(dev);
>  
> +	c_can_pm_runtime_get_sync(priv);
> +
>  	reg_err_counter = priv->read_reg(priv, C_CAN_ERR_CNT_REG);
>  	bec->rxerr = (reg_err_counter & ERR_CNT_REC_MASK) >>
>  				ERR_CNT_REC_SHIFT;
>  	bec->txerr = reg_err_counter & ERR_CNT_TEC_MASK;
>  
> +	c_can_pm_runtime_put_sync(priv);
> +
>  	return 0;
>  }
>  
> @@ -1053,11 +1070,13 @@ static int c_can_open(struct net_device *dev)
>  	int err;
>  	struct c_can_priv *priv = netdev_priv(dev);
>  
> +	c_can_pm_runtime_get_sync(priv);
> +
>  	/* open the can device */
>  	err = open_candev(dev);
>  	if (err) {
>  		netdev_err(dev, "failed to open can device\n");
> -		return err;
> +		goto exit_open_fail;
>  	}
>  
>  	/* register interrupt handler */
> @@ -1079,6 +1098,8 @@ static int c_can_open(struct net_device *dev)
>  
>  exit_irq_fail:
>  	close_candev(dev);
> +exit_open_fail:
> +	c_can_pm_runtime_put_sync(priv);
>  	return err;
>  }
>  
> @@ -1091,6 +1112,7 @@ static int c_can_close(struct net_device *dev)
>  	c_can_stop(dev);
>  	free_irq(dev->irq, dev);
>  	close_candev(dev);
> +	c_can_pm_runtime_put_sync(priv);
>  
>  	return 0;
>  }
> diff --git a/drivers/net/can/c_can/c_can.h b/drivers/net/can/c_can/c_can.h
> index 4e56baa..1437a6d 100644
> --- a/drivers/net/can/c_can/c_can.h
> +++ b/drivers/net/can/c_can/c_can.h
> @@ -153,6 +153,7 @@ struct c_can_priv {
>  	struct can_priv can;	/* must be the first member */
>  	struct napi_struct napi;
>  	struct net_device *dev;
> +	struct device *device;
>  	int tx_object;
>  	int current_status;
>  	int last_status;
> diff --git a/drivers/net/can/c_can/c_can_platform.c b/drivers/net/can/c_can/c_can_platform.c
> index d0a66cf..90801c4 100644
> --- 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;
>  	}
>  
> +	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;
>  
> @@ -189,15 +193,17 @@ static int __devinit c_can_plat_probe(struct platform_device *pdev)
>  	if (ret) {
>  		dev_err(&pdev->dev, "registering %s failed (err=%d)\n",
>  			KBUILD_MODNAME, ret);
> -		goto exit_free_device;
> +		goto exit_clear_drvdata;
>  	}
>  
>  	dev_info(&pdev->dev, "%s device registered (regs=%p, irq=%d)\n",
>  		 KBUILD_MODNAME, priv->base, dev->irq);
>  	return 0;
>  
> -exit_free_device:
> +exit_clear_drvdata:
>  	platform_set_drvdata(pdev, NULL);
> +	pm_runtime_disable(&pdev->dev);
> +exit_free_device:
>  	free_c_can_dev(dev);
>  exit_iounmap:
>  	iounmap(addr);
> @@ -226,6 +232,7 @@ static int __devexit c_can_plat_remove(struct platform_device *pdev)
>  	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>  	release_mem_region(mem->start, resource_size(mem));
>  
> +	pm_runtime_disable(&pdev->dev);
>  	clk_put(priv->priv);
>  
>  	return 0;
>
AnilKumar, Chimata Sept. 3, 2012, 10:52 a.m. UTC | #3
Marc,

On Mon, Sep 03, 2012 at 14:22:22, Marc Kleine-Budde wrote:
> On 08/20/2012 01:20 PM, AnilKumar Ch wrote:
> > 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 <anilkumar@ti.com>
> > ---
> > 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.
> 
> Looks good. I've marked the c_can_priv *priv pointer as const in
> c_can_pm_runtime_{get,put}_sync.
> 

Yes, that is better.
Thanks marc.

Regards
AnilKumar
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Kevin Hilman Sept. 6, 2012, 11:37 p.m. UTC | #4
AnilKumar Ch <anilkumar@ti.com> writes:

> 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 <anilkumar@ti.com>

I'm not familar with the CAN specifics here, but some comments on the
runtime PM implementation.

> ---
> 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 v8:
> 	- corrected the return path sequence in c_can_probe()
>
> Changes from v7:
> 	- Incorporated Marc's comments on v7
> 	  * changed device pointer to c_can_priv pointer
>
> Changes from v6:
> 	- Incorporated Marc's comments on v6
> 	  * changed dev pointer to priv
> 	  * removed platform_device.h include from c_can.c
>
> Changes from v5:
> 	- Incorporated Marc's comments on v5
> 	  * changed runtime pm calls in c_can driver to handle
> 	    the drivers which are not using platform drivers.
> 	  * added device pointer protection in c_can driver if
> 	    not passed from platform/pci driver.
>
> Changes from v4:
> 	- Incorporated Vaibhav H review comments on v4.
> 	  * Moved pm_runtime put/get_sync calls to appropriate positions.
> 	- This patch is from "Add DT support to C_CAN/D_CAN controller"
> 	  patch series. Rest of the patches in this series were applied
> 	  so this v5 contains only this patch.
>
>  drivers/net/can/c_can/c_can.c          |   24 +++++++++++++++++++++++-
>  drivers/net/can/c_can/c_can.h          |    1 +
>  drivers/net/can/c_can/c_can_platform.c |   11 +++++++++--
>  3 files changed, 33 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/can/c_can/c_can.c b/drivers/net/can/c_can/c_can.c
> index 4c538e3..966d318 100644
> --- a/drivers/net/can/c_can/c_can.c
> +++ b/drivers/net/can/c_can/c_can.c
> @@ -34,6 +34,7 @@
>  #include <linux/if_ether.h>
>  #include <linux/list.h>
>  #include <linux/io.h>
> +#include <linux/pm_runtime.h>
>  
>  #include <linux/can.h>
>  #include <linux/can/dev.h>
> @@ -201,6 +202,18 @@ static const struct can_bittiming_const c_can_bittiming_const = {
>  	.brp_inc = 1,
>  };
>  
> +static inline void c_can_pm_runtime_get_sync(struct c_can_priv *priv)
> +{
> +	if (priv->device)
> +		pm_runtime_get_sync(priv->device);
> +}
> +
> +static inline void c_can_pm_runtime_put_sync(struct c_can_priv *priv)
> +{
> +	if (priv->device)
> +		pm_runtime_put_sync(priv->device);
> +}

IMO, these extra helpers are rather unsightly, and should not be needed.
The driver should just be directly doing get_sync/put_sync.  If
priv->device isn't presnt, then runtime PM should just never be enabled.

Kevin

>  static inline int get_tx_next_msg_obj(const struct c_can_priv *priv)
>  {
>  	return (priv->tx_next & C_CAN_NEXT_MSG_OBJ_MASK) +
> @@ -673,11 +686,15 @@ static int c_can_get_berr_counter(const struct net_device *dev,
>  	unsigned int reg_err_counter;
>  	struct c_can_priv *priv = netdev_priv(dev);
>  
> +	c_can_pm_runtime_get_sync(priv);
> +
>  	reg_err_counter = priv->read_reg(priv, C_CAN_ERR_CNT_REG);
>  	bec->rxerr = (reg_err_counter & ERR_CNT_REC_MASK) >>
>  				ERR_CNT_REC_SHIFT;
>  	bec->txerr = reg_err_counter & ERR_CNT_TEC_MASK;
>  
> +	c_can_pm_runtime_put_sync(priv);
> +
>  	return 0;
>  }
>  
> @@ -1053,11 +1070,13 @@ static int c_can_open(struct net_device *dev)
>  	int err;
>  	struct c_can_priv *priv = netdev_priv(dev);
>  
> +	c_can_pm_runtime_get_sync(priv);
> +
>  	/* open the can device */
>  	err = open_candev(dev);
>  	if (err) {
>  		netdev_err(dev, "failed to open can device\n");
> -		return err;
> +		goto exit_open_fail;
>  	}
>  
>  	/* register interrupt handler */
> @@ -1079,6 +1098,8 @@ static int c_can_open(struct net_device *dev)
>  
>  exit_irq_fail:
>  	close_candev(dev);
> +exit_open_fail:
> +	c_can_pm_runtime_put_sync(priv);
>  	return err;
>  }
>  
> @@ -1091,6 +1112,7 @@ static int c_can_close(struct net_device *dev)
>  	c_can_stop(dev);
>  	free_irq(dev->irq, dev);
>  	close_candev(dev);
> +	c_can_pm_runtime_put_sync(priv);
>  
>  	return 0;
>  }
> diff --git a/drivers/net/can/c_can/c_can.h b/drivers/net/can/c_can/c_can.h
> index 4e56baa..1437a6d 100644
> --- a/drivers/net/can/c_can/c_can.h
> +++ b/drivers/net/can/c_can/c_can.h
> @@ -153,6 +153,7 @@ struct c_can_priv {
>  	struct can_priv can;	/* must be the first member */
>  	struct napi_struct napi;
>  	struct net_device *dev;
> +	struct device *device;
>  	int tx_object;
>  	int current_status;
>  	int last_status;
> diff --git a/drivers/net/can/c_can/c_can_platform.c b/drivers/net/can/c_can/c_can_platform.c
> index d0a66cf..90801c4 100644
> --- 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;
>  	}
>  
> +	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;
>  
> @@ -189,15 +193,17 @@ static int __devinit c_can_plat_probe(struct platform_device *pdev)
>  	if (ret) {
>  		dev_err(&pdev->dev, "registering %s failed (err=%d)\n",
>  			KBUILD_MODNAME, ret);
> -		goto exit_free_device;
> +		goto exit_clear_drvdata;
>  	}
>  
>  	dev_info(&pdev->dev, "%s device registered (regs=%p, irq=%d)\n",
>  		 KBUILD_MODNAME, priv->base, dev->irq);
>  	return 0;
>  
> -exit_free_device:
> +exit_clear_drvdata:
>  	platform_set_drvdata(pdev, NULL);
> +	pm_runtime_disable(&pdev->dev);
> +exit_free_device:
>  	free_c_can_dev(dev);
>  exit_iounmap:
>  	iounmap(addr);
> @@ -226,6 +232,7 @@ static int __devexit c_can_plat_remove(struct platform_device *pdev)
>  	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>  	release_mem_region(mem->start, resource_size(mem));
>  
> +	pm_runtime_disable(&pdev->dev);
>  	clk_put(priv->priv);
>  
>  	return 0;
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
AnilKumar, Chimata Sept. 7, 2012, 3:28 a.m. UTC | #5
Hi Kevin,

On Fri, Sep 07, 2012 at 05:07:56, Kevin Hilman wrote:
> AnilKumar Ch <anilkumar@ti.com> writes:
> 
> > 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 <anilkumar@ti.com>
> 
> I'm not familar with the CAN specifics here, but some comments on the
> runtime PM implementation.

Thanks for the comments.

> 
> > ---
> > 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 v8:
> > 	- corrected the return path sequence in c_can_probe()
> >
> > Changes from v7:
> > 	- Incorporated Marc's comments on v7
> > 	  * changed device pointer to c_can_priv pointer
> >
> > Changes from v6:
> > 	- Incorporated Marc's comments on v6
> > 	  * changed dev pointer to priv
> > 	  * removed platform_device.h include from c_can.c
> >
> > Changes from v5:
> > 	- Incorporated Marc's comments on v5
> > 	  * changed runtime pm calls in c_can driver to handle
> > 	    the drivers which are not using platform drivers.
> > 	  * added device pointer protection in c_can driver if
> > 	    not passed from platform/pci driver.
> >
> > Changes from v4:
> > 	- Incorporated Vaibhav H review comments on v4.
> > 	  * Moved pm_runtime put/get_sync calls to appropriate positions.
> > 	- This patch is from "Add DT support to C_CAN/D_CAN controller"
> > 	  patch series. Rest of the patches in this series were applied
> > 	  so this v5 contains only this patch.
> >
> >  drivers/net/can/c_can/c_can.c          |   24 +++++++++++++++++++++++-
> >  drivers/net/can/c_can/c_can.h          |    1 +
> >  drivers/net/can/c_can/c_can_platform.c |   11 +++++++++--
> >  3 files changed, 33 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/net/can/c_can/c_can.c b/drivers/net/can/c_can/c_can.c
> > index 4c538e3..966d318 100644
> > --- a/drivers/net/can/c_can/c_can.c
> > +++ b/drivers/net/can/c_can/c_can.c
> > @@ -34,6 +34,7 @@
> >  #include <linux/if_ether.h>
> >  #include <linux/list.h>
> >  #include <linux/io.h>
> > +#include <linux/pm_runtime.h>
> >  
> >  #include <linux/can.h>
> >  #include <linux/can/dev.h>
> > @@ -201,6 +202,18 @@ static const struct can_bittiming_const c_can_bittiming_const = {
> >  	.brp_inc = 1,
> >  };
> >  
> > +static inline void c_can_pm_runtime_get_sync(struct c_can_priv *priv)
> > +{
> > +	if (priv->device)
> > +		pm_runtime_get_sync(priv->device);
> > +}
> > +
> > +static inline void c_can_pm_runtime_put_sync(struct c_can_priv *priv)
> > +{
> > +	if (priv->device)
> > +		pm_runtime_put_sync(priv->device);
> > +}
> 
> IMO, these extra helpers are rather unsightly, and should not be needed.
> The driver should just be directly doing get_sync/put_sync.  If
> priv->device isn't presnt, then runtime PM should just never be enabled.
> 

In case of c_can driver we have two drivers one is generic c_can.c driver,
provides the basic functionality of CAN. Another two drivers c_can_platform.c
and c_can_pci.c, which uses core c_can.c driver by exporting some platform
specific ops like read/write.

priv->device pointer is passed from c_can_platform.c by this means
"priv->device = &pdev->dev;" (see below) but not for c_can_pci.c

The purpose of check here is for *_pci.c driver which do not have runtime pm
implemented yet so we should do and get_sync/put_sync. In case of *_pci.c
driver there is no pm_runtime_enable/disable once that is implemented then
this check will be removed.

Thanks
AnilKumar

> 
> >  static inline int get_tx_next_msg_obj(const struct c_can_priv *priv)
> >  {
> >  	return (priv->tx_next & C_CAN_NEXT_MSG_OBJ_MASK) +
> > @@ -673,11 +686,15 @@ static int c_can_get_berr_counter(const struct net_device *dev,
> >  	unsigned int reg_err_counter;
> >  	struct c_can_priv *priv = netdev_priv(dev);
> >  
> > +	c_can_pm_runtime_get_sync(priv);
> > +
> >  	reg_err_counter = priv->read_reg(priv, C_CAN_ERR_CNT_REG);
> >  	bec->rxerr = (reg_err_counter & ERR_CNT_REC_MASK) >>
> >  				ERR_CNT_REC_SHIFT;
> >  	bec->txerr = reg_err_counter & ERR_CNT_TEC_MASK;
> >  
> > +	c_can_pm_runtime_put_sync(priv);
> > +
> >  	return 0;
> >  }
> >  
> > @@ -1053,11 +1070,13 @@ static int c_can_open(struct net_device *dev)
> >  	int err;
> >  	struct c_can_priv *priv = netdev_priv(dev);
> >  
> > +	c_can_pm_runtime_get_sync(priv);
> > +
> >  	/* open the can device */
> >  	err = open_candev(dev);
> >  	if (err) {
> >  		netdev_err(dev, "failed to open can device\n");
> > -		return err;
> > +		goto exit_open_fail;
> >  	}
> >  
> >  	/* register interrupt handler */
> > @@ -1079,6 +1098,8 @@ static int c_can_open(struct net_device *dev)
> >  
> >  exit_irq_fail:
> >  	close_candev(dev);
> > +exit_open_fail:
> > +	c_can_pm_runtime_put_sync(priv);
> >  	return err;
> >  }
> >  
> > @@ -1091,6 +1112,7 @@ static int c_can_close(struct net_device *dev)
> >  	c_can_stop(dev);
> >  	free_irq(dev->irq, dev);
> >  	close_candev(dev);
> > +	c_can_pm_runtime_put_sync(priv);
> >  
> >  	return 0;
> >  }
> > diff --git a/drivers/net/can/c_can/c_can.h b/drivers/net/can/c_can/c_can.h
> > index 4e56baa..1437a6d 100644
> > --- a/drivers/net/can/c_can/c_can.h
> > +++ b/drivers/net/can/c_can/c_can.h
> > @@ -153,6 +153,7 @@ struct c_can_priv {
> >  	struct can_priv can;	/* must be the first member */
> >  	struct napi_struct napi;
> >  	struct net_device *dev;
> > +	struct device *device;
> >  	int tx_object;
> >  	int current_status;
> >  	int last_status;
> > diff --git a/drivers/net/can/c_can/c_can_platform.c b/drivers/net/can/c_can/c_can_platform.c
> > index d0a66cf..90801c4 100644
> > --- 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;
> >  	}
> >  
> > +	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;
> >  
> > @@ -189,15 +193,17 @@ static int __devinit c_can_plat_probe(struct platform_device *pdev)
> >  	if (ret) {
> >  		dev_err(&pdev->dev, "registering %s failed (err=%d)\n",
> >  			KBUILD_MODNAME, ret);
> > -		goto exit_free_device;
> > +		goto exit_clear_drvdata;
> >  	}
> >  
> >  	dev_info(&pdev->dev, "%s device registered (regs=%p, irq=%d)\n",
> >  		 KBUILD_MODNAME, priv->base, dev->irq);
> >  	return 0;
> >  
> > -exit_free_device:
> > +exit_clear_drvdata:
> >  	platform_set_drvdata(pdev, NULL);
> > +	pm_runtime_disable(&pdev->dev);
> > +exit_free_device:
> >  	free_c_can_dev(dev);
> >  exit_iounmap:
> >  	iounmap(addr);
> > @@ -226,6 +232,7 @@ static int __devexit c_can_plat_remove(struct platform_device *pdev)
> >  	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> >  	release_mem_region(mem->start, resource_size(mem));
> >  
> > +	pm_runtime_disable(&pdev->dev);
> >  	clk_put(priv->priv);
> >  
> >  	return 0;
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Kevin Hilman Sept. 7, 2012, 9:01 p.m. UTC | #6
"AnilKumar, Chimata" <anilkumar@ti.com> writes:

> Hi Kevin,
>
> On Fri, Sep 07, 2012 at 05:07:56, Kevin Hilman wrote:
>> AnilKumar Ch <anilkumar@ti.com> writes:
>> 
>> > 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 <anilkumar@ti.com>
>> 
>> I'm not familar with the CAN specifics here, but some comments on the
>> runtime PM implementation.
>
> Thanks for the comments.
>
>> 
>> > ---
>> > 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 v8:
>> > 	- corrected the return path sequence in c_can_probe()
>> >
>> > Changes from v7:
>> > 	- Incorporated Marc's comments on v7
>> > 	  * changed device pointer to c_can_priv pointer
>> >
>> > Changes from v6:
>> > 	- Incorporated Marc's comments on v6
>> > 	  * changed dev pointer to priv
>> > 	  * removed platform_device.h include from c_can.c
>> >
>> > Changes from v5:
>> > 	- Incorporated Marc's comments on v5
>> > 	  * changed runtime pm calls in c_can driver to handle
>> > 	    the drivers which are not using platform drivers.
>> > 	  * added device pointer protection in c_can driver if
>> > 	    not passed from platform/pci driver.
>> >
>> > Changes from v4:
>> > 	- Incorporated Vaibhav H review comments on v4.
>> > 	  * Moved pm_runtime put/get_sync calls to appropriate positions.
>> > 	- This patch is from "Add DT support to C_CAN/D_CAN controller"
>> > 	  patch series. Rest of the patches in this series were applied
>> > 	  so this v5 contains only this patch.
>> >
>> >  drivers/net/can/c_can/c_can.c          |   24 +++++++++++++++++++++++-
>> >  drivers/net/can/c_can/c_can.h          |    1 +
>> >  drivers/net/can/c_can/c_can_platform.c |   11 +++++++++--
>> >  3 files changed, 33 insertions(+), 3 deletions(-)
>> >
>> > diff --git a/drivers/net/can/c_can/c_can.c b/drivers/net/can/c_can/c_can.c
>> > index 4c538e3..966d318 100644
>> > --- a/drivers/net/can/c_can/c_can.c
>> > +++ b/drivers/net/can/c_can/c_can.c
>> > @@ -34,6 +34,7 @@
>> >  #include <linux/if_ether.h>
>> >  #include <linux/list.h>
>> >  #include <linux/io.h>
>> > +#include <linux/pm_runtime.h>
>> >  
>> >  #include <linux/can.h>
>> >  #include <linux/can/dev.h>
>> > @@ -201,6 +202,18 @@ static const struct can_bittiming_const c_can_bittiming_const = {
>> >  	.brp_inc = 1,
>> >  };
>> >  
>> > +static inline void c_can_pm_runtime_get_sync(struct c_can_priv *priv)
>> > +{
>> > +	if (priv->device)
>> > +		pm_runtime_get_sync(priv->device);
>> > +}
>> > +
>> > +static inline void c_can_pm_runtime_put_sync(struct c_can_priv *priv)
>> > +{
>> > +	if (priv->device)
>> > +		pm_runtime_put_sync(priv->device);
>> > +}
>> 
>> IMO, these extra helpers are rather unsightly, and should not be needed.
>> The driver should just be directly doing get_sync/put_sync.  If
>> priv->device isn't presnt, then runtime PM should just never be enabled.
>> 
>
> In case of c_can driver we have two drivers one is generic c_can.c driver,
> provides the basic functionality of CAN. Another two drivers c_can_platform.c
> and c_can_pci.c, which uses core c_can.c driver by exporting some platform
> specific ops like read/write.
>
> priv->device pointer is passed from c_can_platform.c by this means
> "priv->device = &pdev->dev;" (see below) but not for c_can_pci.c
>
> The purpose of check here is for *_pci.c driver which do not have runtime pm
> implemented yet so we should do and get_sync/put_sync. In case of *_pci.c
> driver there is no pm_runtime_enable/disable once that is implemented then
> this check will be removed.

Then you should probably move the pm_runtime_enable/disable into the
common code (where the get_sync/put_sync) are.  Then you could simply 
avoid the pm_runtime_enable() if there is no priv->device.

Kevin
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
AnilKumar, Chimata Sept. 10, 2012, 10:44 a.m. UTC | #7
Hi Kevin,

On Sat, Sep 08, 2012 at 02:31:22, Kevin Hilman wrote:
> "AnilKumar, Chimata" <anilkumar@ti.com> writes:
> 
> > Hi Kevin,
> >
> > On Fri, Sep 07, 2012 at 05:07:56, Kevin Hilman wrote:
> >> AnilKumar Ch <anilkumar@ti.com> writes:
> >> 
> >> > 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 <anilkumar@ti.com>
> >> 
> >> I'm not familar with the CAN specifics here, but some comments on the
> >> runtime PM implementation.
> >
> > Thanks for the comments.
> >
> >> 
> >> > ---
> >> > 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 v8:
> >> > 	- corrected the return path sequence in c_can_probe()
> >> >
> >> > Changes from v7:
> >> > 	- Incorporated Marc's comments on v7
> >> > 	  * changed device pointer to c_can_priv pointer
> >> >
> >> > Changes from v6:
> >> > 	- Incorporated Marc's comments on v6
> >> > 	  * changed dev pointer to priv
> >> > 	  * removed platform_device.h include from c_can.c
> >> >
> >> > Changes from v5:
> >> > 	- Incorporated Marc's comments on v5
> >> > 	  * changed runtime pm calls in c_can driver to handle
> >> > 	    the drivers which are not using platform drivers.
> >> > 	  * added device pointer protection in c_can driver if
> >> > 	    not passed from platform/pci driver.
> >> >
> >> > Changes from v4:
> >> > 	- Incorporated Vaibhav H review comments on v4.
> >> > 	  * Moved pm_runtime put/get_sync calls to appropriate positions.
> >> > 	- This patch is from "Add DT support to C_CAN/D_CAN controller"
> >> > 	  patch series. Rest of the patches in this series were applied
> >> > 	  so this v5 contains only this patch.
> >> >
> >> >  drivers/net/can/c_can/c_can.c          |   24 +++++++++++++++++++++++-
> >> >  drivers/net/can/c_can/c_can.h          |    1 +
> >> >  drivers/net/can/c_can/c_can_platform.c |   11 +++++++++--
> >> >  3 files changed, 33 insertions(+), 3 deletions(-)
> >> >
> >> > diff --git a/drivers/net/can/c_can/c_can.c b/drivers/net/can/c_can/c_can.c
> >> > index 4c538e3..966d318 100644
> >> > --- a/drivers/net/can/c_can/c_can.c
> >> > +++ b/drivers/net/can/c_can/c_can.c
> >> > @@ -34,6 +34,7 @@
> >> >  #include <linux/if_ether.h>
> >> >  #include <linux/list.h>
> >> >  #include <linux/io.h>
> >> > +#include <linux/pm_runtime.h>
> >> >  
> >> >  #include <linux/can.h>
> >> >  #include <linux/can/dev.h>
> >> > @@ -201,6 +202,18 @@ static const struct can_bittiming_const c_can_bittiming_const = {
> >> >  	.brp_inc = 1,
> >> >  };
> >> >  
> >> > +static inline void c_can_pm_runtime_get_sync(struct c_can_priv *priv)
> >> > +{
> >> > +	if (priv->device)
> >> > +		pm_runtime_get_sync(priv->device);
> >> > +}
> >> > +
> >> > +static inline void c_can_pm_runtime_put_sync(struct c_can_priv *priv)
> >> > +{
> >> > +	if (priv->device)
> >> > +		pm_runtime_put_sync(priv->device);
> >> > +}
> >> 
> >> IMO, these extra helpers are rather unsightly, and should not be needed.
> >> The driver should just be directly doing get_sync/put_sync.  If
> >> priv->device isn't presnt, then runtime PM should just never be enabled.
> >> 
> >
> > In case of c_can driver we have two drivers one is generic c_can.c driver,
> > provides the basic functionality of CAN. Another two drivers c_can_platform.c
> > and c_can_pci.c, which uses core c_can.c driver by exporting some platform
> > specific ops like read/write.
> >
> > priv->device pointer is passed from c_can_platform.c by this means
> > "priv->device = &pdev->dev;" (see below) but not for c_can_pci.c
> >
> > The purpose of check here is for *_pci.c driver which do not have runtime pm
> > implemented yet so we should do and get_sync/put_sync. In case of *_pci.c
> > driver there is no pm_runtime_enable/disable once that is implemented then
> > this check will be removed.
> 
> Then you should probably move the pm_runtime_enable/disable into the
> common code (where the get_sync/put_sync) are.  Then you could simply 
> avoid the pm_runtime_enable() if there is no priv->device.
> 

Thanks for the comments

I got your point, will move pm_runtime_enable/disable to common code.

Thanks
AnilKumar
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch
diff mbox

diff --git a/drivers/net/can/c_can/c_can.c b/drivers/net/can/c_can/c_can.c
index 4c538e3..966d318 100644
--- a/drivers/net/can/c_can/c_can.c
+++ b/drivers/net/can/c_can/c_can.c
@@ -34,6 +34,7 @@ 
 #include <linux/if_ether.h>
 #include <linux/list.h>
 #include <linux/io.h>
+#include <linux/pm_runtime.h>
 
 #include <linux/can.h>
 #include <linux/can/dev.h>
@@ -201,6 +202,18 @@  static const struct can_bittiming_const c_can_bittiming_const = {
 	.brp_inc = 1,
 };
 
+static inline void c_can_pm_runtime_get_sync(struct c_can_priv *priv)
+{
+	if (priv->device)
+		pm_runtime_get_sync(priv->device);
+}
+
+static inline void c_can_pm_runtime_put_sync(struct c_can_priv *priv)
+{
+	if (priv->device)
+		pm_runtime_put_sync(priv->device);
+}
+
 static inline int get_tx_next_msg_obj(const struct c_can_priv *priv)
 {
 	return (priv->tx_next & C_CAN_NEXT_MSG_OBJ_MASK) +
@@ -673,11 +686,15 @@  static int c_can_get_berr_counter(const struct net_device *dev,
 	unsigned int reg_err_counter;
 	struct c_can_priv *priv = netdev_priv(dev);
 
+	c_can_pm_runtime_get_sync(priv);
+
 	reg_err_counter = priv->read_reg(priv, C_CAN_ERR_CNT_REG);
 	bec->rxerr = (reg_err_counter & ERR_CNT_REC_MASK) >>
 				ERR_CNT_REC_SHIFT;
 	bec->txerr = reg_err_counter & ERR_CNT_TEC_MASK;
 
+	c_can_pm_runtime_put_sync(priv);
+
 	return 0;
 }
 
@@ -1053,11 +1070,13 @@  static int c_can_open(struct net_device *dev)
 	int err;
 	struct c_can_priv *priv = netdev_priv(dev);
 
+	c_can_pm_runtime_get_sync(priv);
+
 	/* open the can device */
 	err = open_candev(dev);
 	if (err) {
 		netdev_err(dev, "failed to open can device\n");
-		return err;
+		goto exit_open_fail;
 	}
 
 	/* register interrupt handler */
@@ -1079,6 +1098,8 @@  static int c_can_open(struct net_device *dev)
 
 exit_irq_fail:
 	close_candev(dev);
+exit_open_fail:
+	c_can_pm_runtime_put_sync(priv);
 	return err;
 }
 
@@ -1091,6 +1112,7 @@  static int c_can_close(struct net_device *dev)
 	c_can_stop(dev);
 	free_irq(dev->irq, dev);
 	close_candev(dev);
+	c_can_pm_runtime_put_sync(priv);
 
 	return 0;
 }
diff --git a/drivers/net/can/c_can/c_can.h b/drivers/net/can/c_can/c_can.h
index 4e56baa..1437a6d 100644
--- a/drivers/net/can/c_can/c_can.h
+++ b/drivers/net/can/c_can/c_can.h
@@ -153,6 +153,7 @@  struct c_can_priv {
 	struct can_priv can;	/* must be the first member */
 	struct napi_struct napi;
 	struct net_device *dev;
+	struct device *device;
 	int tx_object;
 	int current_status;
 	int last_status;
diff --git a/drivers/net/can/c_can/c_can_platform.c b/drivers/net/can/c_can/c_can_platform.c
index d0a66cf..90801c4 100644
--- 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;
 	}
 
+	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;
 
@@ -189,15 +193,17 @@  static int __devinit c_can_plat_probe(struct platform_device *pdev)
 	if (ret) {
 		dev_err(&pdev->dev, "registering %s failed (err=%d)\n",
 			KBUILD_MODNAME, ret);
-		goto exit_free_device;
+		goto exit_clear_drvdata;
 	}
 
 	dev_info(&pdev->dev, "%s device registered (regs=%p, irq=%d)\n",
 		 KBUILD_MODNAME, priv->base, dev->irq);
 	return 0;
 
-exit_free_device:
+exit_clear_drvdata:
 	platform_set_drvdata(pdev, NULL);
+	pm_runtime_disable(&pdev->dev);
+exit_free_device:
 	free_c_can_dev(dev);
 exit_iounmap:
 	iounmap(addr);
@@ -226,6 +232,7 @@  static int __devexit c_can_plat_remove(struct platform_device *pdev)
 	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	release_mem_region(mem->start, resource_size(mem));
 
+	pm_runtime_disable(&pdev->dev);
 	clk_put(priv->priv);
 
 	return 0;