Re: [PATCH] can: c_can: Move pm_runtime_enable/disable calls to common code
On 09/13/2012 09:28 AM, AnilKumar Ch wrote:
Move pm_runtime_enable/disable calls to c_can.c driver. Current
implementation is such that platform driver is doing pm_runtime
enable/disable and core driver is doing put_sync/get_sync.
PM runtime calls should be invoked if there is a valid device
pointer from platform driver so moving enable/disable calls
to core driver.
Signed-off-by: AnilKumar Ch anilku...@ti.com
---
Incorporated Kevin's comments on can: c_can: Add runtime PM
support to Bosch C_CAN/D_CAN controller patch.
This patch is tested on AM335x-EVM and cleanly applies on
linux-can master
I'll squash this into can: c_can: Add runtime PM support to Bosch
C_CAN/D_CAN controller.
drivers/net/can/c_can/c_can.c | 18 +-
drivers/net/can/c_can/c_can_platform.c |5 -
2 files changed, 17 insertions(+), 6 deletions(-)
diff --git a/drivers/net/can/c_can/c_can.c b/drivers/net/can/c_can/c_can.c
index aa6c5eb..e472975 100644
--- a/drivers/net/can/c_can/c_can.c
+++ b/drivers/net/can/c_can/c_can.c
@@ -1155,10 +1155,23 @@ static const struct net_device_ops c_can_netdev_ops =
{
int register_c_can_dev(struct net_device *dev)
{
+ int ret;
+ struct c_can_priv *priv = netdev_priv(dev);
+
+ if (priv-device)
+ pm_runtime_enable(priv-device);
I'll add a static inline for these two new function, too.
+
dev-flags |= IFF_ECHO; /* we support local echo */
dev-netdev_ops = c_can_netdev_ops;
- return register_candev(dev);
+ ret = register_candev(dev);
+ if (ret) {
+ if (priv-device)
+ pm_runtime_disable(priv-device);
+ return ret;
+ }
+
+ return 0;
}
EXPORT_SYMBOL_GPL(register_c_can_dev);
@@ -1170,6 +1183,9 @@ void unregister_c_can_dev(struct net_device *dev)
c_can_enable_all_interrupts(priv, DISABLE_ALL_INTERRUPTS);
unregister_candev(dev);
+
+ if (priv-device)
+ pm_runtime_disable(priv-device);
}
EXPORT_SYMBOL_GPL(unregister_c_can_dev);
diff --git a/drivers/net/can/c_can/c_can_platform.c
b/drivers/net/can/c_can/c_can_platform.c
index c351975..491101a 100644
--- a/drivers/net/can/c_can/c_can_platform.c
+++ b/drivers/net/can/c_can/c_can_platform.c
@@ -32,7 +32,6 @@
#include linux/clk.h
#include linux/of.h
#include linux/of_device.h
-#include linux/pm_runtime.h
#include linux/pinctrl/consumer.h
#include linux/can/dev.h
@@ -185,8 +184,6 @@ 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;
@@ -209,7 +206,6 @@ static int __devinit c_can_plat_probe(struct
platform_device *pdev)
exit_clear_drvdata:
platform_set_drvdata(pdev, NULL);
- pm_runtime_disable(pdev-dev);
When squaahsing both patches we see still some changes here, I'll fix
that, too.
exit_free_device:
free_c_can_dev(dev);
exit_iounmap:
@@ -239,7 +235,6 @@ 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;
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
signature.asc
Description: OpenPGP digital signature
RE: [PATCH] can: c_can: Move pm_runtime_enable/disable calls to common code
Marc,
On Thu, Sep 13, 2012 at 13:18:07, Marc Kleine-Budde wrote:
On 09/13/2012 09:28 AM, AnilKumar Ch wrote:
Move pm_runtime_enable/disable calls to c_can.c driver. Current
implementation is such that platform driver is doing pm_runtime
enable/disable and core driver is doing put_sync/get_sync.
PM runtime calls should be invoked if there is a valid device
pointer from platform driver so moving enable/disable calls
to core driver.
Signed-off-by: AnilKumar Ch anilku...@ti.com
---
Incorporated Kevin's comments on can: c_can: Add runtime PM
support to Bosch C_CAN/D_CAN controller patch.
This patch is tested on AM335x-EVM and cleanly applies on
linux-can master
I'll squash this into can: c_can: Add runtime PM support to Bosch
C_CAN/D_CAN controller.
drivers/net/can/c_can/c_can.c | 18 +-
drivers/net/can/c_can/c_can_platform.c |5 -
2 files changed, 17 insertions(+), 6 deletions(-)
diff --git a/drivers/net/can/c_can/c_can.c b/drivers/net/can/c_can/c_can.c
index aa6c5eb..e472975 100644
--- a/drivers/net/can/c_can/c_can.c
+++ b/drivers/net/can/c_can/c_can.c
@@ -1155,10 +1155,23 @@ static const struct net_device_ops c_can_netdev_ops
= {
int register_c_can_dev(struct net_device *dev)
{
+ int ret;
+ struct c_can_priv *priv = netdev_priv(dev);
+
+ if (priv-device)
+ pm_runtime_enable(priv-device);
I'll add a static inline for these two new function, too.
My bad, this should be my first change.
+
dev-flags |= IFF_ECHO; /* we support local echo */
dev-netdev_ops = c_can_netdev_ops;
- return register_candev(dev);
+ ret = register_candev(dev);
+ if (ret) {
+ if (priv-device)
+ pm_runtime_disable(priv-device);
+ return ret;
+ }
+
+ return 0;
}
EXPORT_SYMBOL_GPL(register_c_can_dev);
@@ -1170,6 +1183,9 @@ void unregister_c_can_dev(struct net_device *dev)
c_can_enable_all_interrupts(priv, DISABLE_ALL_INTERRUPTS);
unregister_candev(dev);
+
+ if (priv-device)
+ pm_runtime_disable(priv-device);
}
EXPORT_SYMBOL_GPL(unregister_c_can_dev);
diff --git a/drivers/net/can/c_can/c_can_platform.c
b/drivers/net/can/c_can/c_can_platform.c
index c351975..491101a 100644
--- a/drivers/net/can/c_can/c_can_platform.c
+++ b/drivers/net/can/c_can/c_can_platform.c
@@ -32,7 +32,6 @@
#include linux/clk.h
#include linux/of.h
#include linux/of_device.h
-#include linux/pm_runtime.h
#include linux/pinctrl/consumer.h
#include linux/can/dev.h
@@ -185,8 +184,6 @@ 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;
@@ -209,7 +206,6 @@ static int __devinit c_can_plat_probe(struct
platform_device *pdev)
exit_clear_drvdata:
platform_set_drvdata(pdev, NULL);
- pm_runtime_disable(pdev-dev);
When squaahsing both patches we see still some changes here, I'll fix
that, too.
Thanks Marc
Thanks
AnilKumar
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] can: c_can: Move pm_runtime_enable/disable calls to common code
AnilKumar Ch anilku...@ti.com writes: Move pm_runtime_enable/disable calls to c_can.c driver. Current implementation is such that platform driver is doing pm_runtime enable/disable and core driver is doing put_sync/get_sync. PM runtime calls should be invoked if there is a valid device pointer from platform driver so moving enable/disable calls to core driver. Signed-off-by: AnilKumar Ch anilku...@ti.com --- Incorporated Kevin's comments on can: c_can: Add runtime PM support to Bosch C_CAN/D_CAN controller patch. This looks better, but in addition, you can get rid of the runtime PM helper functions you added (the ones that check for priv-device) and call the pm_runtime_get/put APIs directly. Kevin -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] can: c_can: Move pm_runtime_enable/disable calls to common code
On 09/13/2012 04:14 PM, Kevin Hilman wrote: AnilKumar Ch anilku...@ti.com writes: Move pm_runtime_enable/disable calls to c_can.c driver. Current implementation is such that platform driver is doing pm_runtime enable/disable and core driver is doing put_sync/get_sync. PM runtime calls should be invoked if there is a valid device pointer from platform driver so moving enable/disable calls to core driver. Signed-off-by: AnilKumar Ch anilku...@ti.com --- Incorporated Kevin's comments on can: c_can: Add runtime PM support to Bosch C_CAN/D_CAN controller patch. This looks better, but in addition, you can get rid of the runtime PM helper functions you added (the ones that check for priv-device) and call the pm_runtime_get/put APIs directly. But priv-device might be NULL. AFAICS pm_runtime_get() is not safe to be called with a NULL pointer. Marc -- Pengutronix e.K. | Marc Kleine-Budde | Industrial Linux Solutions| Phone: +49-231-2826-924 | Vertretung West/Dortmund | Fax: +49-5121-206917- | Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de | signature.asc Description: OpenPGP digital signature
Re: [PATCH] can: c_can: Move pm_runtime_enable/disable calls to common code
Marc Kleine-Budde m...@pengutronix.de writes: On 09/13/2012 04:14 PM, Kevin Hilman wrote: AnilKumar Ch anilku...@ti.com writes: Move pm_runtime_enable/disable calls to c_can.c driver. Current implementation is such that platform driver is doing pm_runtime enable/disable and core driver is doing put_sync/get_sync. PM runtime calls should be invoked if there is a valid device pointer from platform driver so moving enable/disable calls to core driver. Signed-off-by: AnilKumar Ch anilku...@ti.com --- Incorporated Kevin's comments on can: c_can: Add runtime PM support to Bosch C_CAN/D_CAN controller patch. This looks better, but in addition, you can get rid of the runtime PM helper functions you added (the ones that check for priv-device) and call the pm_runtime_get/put APIs directly. But priv-device might be NULL. AFAICS pm_runtime_get() is not safe to be called with a NULL pointer. Yes, you're right. Guess there's not a clean way to get rid of those helpers. Sorry for the noise, Kevin -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
