linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] can: c_can: move runtime PM enable/disable to c_can_platform
@ 2021-03-01  4:15 Tong Zhang
  2021-03-01 15:08 ` Marc Kleine-Budde
  0 siblings, 1 reply; 6+ messages in thread
From: Tong Zhang @ 2021-03-01  4:15 UTC (permalink / raw)
  To: Wolfgang Grandegger, Marc Kleine-Budde, David S. Miller,
	Jakub Kicinski, Vincent Mailhol, Oliver Hartkopp, Tong Zhang,
	YueHaibing, Zhang Qilong, linux-can, netdev, linux-kernel

Currently doing modprobe c_can_pci will make kernel complain
"Unbalanced pm_runtime_enable!", this is caused by pm_runtime_enable()
called before pm is initialized in register_candev() and doing so will
also cause it to enable twice.
This fix is similar to 227619c3ff7c, move those pm_enable/disable code to
c_can_platform.

Signed-off-by: Tong Zhang <ztong0001@gmail.com>
---
 drivers/net/can/c_can/c_can.c          | 26 ++------------------------
 drivers/net/can/c_can/c_can_platform.c |  6 +++++-
 2 files changed, 7 insertions(+), 25 deletions(-)

diff --git a/drivers/net/can/c_can/c_can.c b/drivers/net/can/c_can/c_can.c
index ef474bae47a1..04f783b3d9d3 100644
--- a/drivers/net/can/c_can/c_can.c
+++ b/drivers/net/can/c_can/c_can.c
@@ -212,18 +212,6 @@ static const struct can_bittiming_const c_can_bittiming_const = {
 	.brp_inc = 1,
 };
 
-static inline void c_can_pm_runtime_enable(const struct c_can_priv *priv)
-{
-	if (priv->device)
-		pm_runtime_enable(priv->device);
-}
-
-static inline void c_can_pm_runtime_disable(const struct c_can_priv *priv)
-{
-	if (priv->device)
-		pm_runtime_disable(priv->device);
-}
-
 static inline void c_can_pm_runtime_get_sync(const struct c_can_priv *priv)
 {
 	if (priv->device)
@@ -1335,7 +1323,6 @@ static const struct net_device_ops c_can_netdev_ops = {
 
 int register_c_can_dev(struct net_device *dev)
 {
-	struct c_can_priv *priv = netdev_priv(dev);
 	int err;
 
 	/* Deactivate pins to prevent DRA7 DCAN IP from being
@@ -1345,28 +1332,19 @@ int register_c_can_dev(struct net_device *dev)
 	 */
 	pinctrl_pm_select_sleep_state(dev->dev.parent);
 
-	c_can_pm_runtime_enable(priv);
-
 	dev->flags |= IFF_ECHO;	/* we support local echo */
 	dev->netdev_ops = &c_can_netdev_ops;
 
 	err = register_candev(dev);
-	if (err)
-		c_can_pm_runtime_disable(priv);
-	else
-		devm_can_led_init(dev);
-
+	if (!err)
+	  devm_can_led_init(dev);
 	return err;
 }
 EXPORT_SYMBOL_GPL(register_c_can_dev);
 
 void unregister_c_can_dev(struct net_device *dev)
 {
-	struct c_can_priv *priv = netdev_priv(dev);
-
 	unregister_candev(dev);
-
-	c_can_pm_runtime_disable(priv);
 }
 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 05f425ceb53a..47b251b1607c 100644
--- a/drivers/net/can/c_can/c_can_platform.c
+++ b/drivers/net/can/c_can/c_can_platform.c
@@ -29,6 +29,7 @@
 #include <linux/list.h>
 #include <linux/io.h>
 #include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
 #include <linux/clk.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
@@ -386,6 +387,7 @@ static int c_can_plat_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, dev);
 	SET_NETDEV_DEV(dev, &pdev->dev);
 
+	pm_runtime_enable(priv->device);
 	ret = register_c_can_dev(dev);
 	if (ret) {
 		dev_err(&pdev->dev, "registering %s failed (err=%d)\n",
@@ -398,6 +400,7 @@ static int c_can_plat_probe(struct platform_device *pdev)
 	return 0;
 
 exit_free_device:
+	pm_runtime_disable(priv->device);
 	free_c_can_dev(dev);
 exit:
 	dev_err(&pdev->dev, "probe failed\n");
@@ -408,9 +411,10 @@ static int c_can_plat_probe(struct platform_device *pdev)
 static int c_can_plat_remove(struct platform_device *pdev)
 {
 	struct net_device *dev = platform_get_drvdata(pdev);
+	struct c_can_priv *priv = netdev_priv(dev);
 
 	unregister_c_can_dev(dev);
-
+	pm_runtime_disable(priv->device);
 	free_c_can_dev(dev);
 
 	return 0;
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] can: c_can: move runtime PM enable/disable to c_can_platform
  2021-03-01  4:15 [PATCH] can: c_can: move runtime PM enable/disable to c_can_platform Tong Zhang
@ 2021-03-01 15:08 ` Marc Kleine-Budde
  2021-03-02  2:55   ` [PATCH v2] " Tong Zhang
  2021-03-02  2:55   ` [PATCH] " Tong Zhang
  0 siblings, 2 replies; 6+ messages in thread
From: Marc Kleine-Budde @ 2021-03-01 15:08 UTC (permalink / raw)
  To: Tong Zhang
  Cc: Wolfgang Grandegger, David S. Miller, Jakub Kicinski,
	Vincent Mailhol, Oliver Hartkopp, YueHaibing, Zhang Qilong,
	linux-can, netdev, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 2776 bytes --]

On 28.02.2021 23:15:48, Tong Zhang wrote:
> Currently doing modprobe c_can_pci will make kernel complain
> "Unbalanced pm_runtime_enable!", this is caused by pm_runtime_enable()
> called before pm is initialized in register_candev() and doing so will

I don't see where register_candev() is doing any pm related
initialization.

> also cause it to enable twice.

> This fix is similar to 227619c3ff7c, move those pm_enable/disable code to
> c_can_platform.

As I understand 227619c3ff7c ("can: m_can: move runtime PM
enable/disable to m_can_platform"), PCI devices automatically enable PM,
when the "PCI device is added".

Please clarify the above point, otherwise the code looks OK, small
nitpick inline:

> Signed-off-by: Tong Zhang <ztong0001@gmail.com>
> ---
>  drivers/net/can/c_can/c_can.c          | 26 ++------------------------
>  drivers/net/can/c_can/c_can_platform.c |  6 +++++-
>  2 files changed, 7 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/net/can/c_can/c_can.c b/drivers/net/can/c_can/c_can.c
> index ef474bae47a1..04f783b3d9d3 100644
> --- a/drivers/net/can/c_can/c_can.c
> +++ b/drivers/net/can/c_can/c_can.c
> @@ -212,18 +212,6 @@ static const struct can_bittiming_const c_can_bittiming_const = {
>  	.brp_inc = 1,
>  };
>  
> -static inline void c_can_pm_runtime_enable(const struct c_can_priv *priv)
> -{
> -	if (priv->device)
> -		pm_runtime_enable(priv->device);
> -}
> -
> -static inline void c_can_pm_runtime_disable(const struct c_can_priv *priv)
> -{
> -	if (priv->device)
> -		pm_runtime_disable(priv->device);
> -}
> -
>  static inline void c_can_pm_runtime_get_sync(const struct c_can_priv *priv)
>  {
>  	if (priv->device)
> @@ -1335,7 +1323,6 @@ static const struct net_device_ops c_can_netdev_ops = {
>  
>  int register_c_can_dev(struct net_device *dev)
>  {
> -	struct c_can_priv *priv = netdev_priv(dev);
>  	int err;
>  
>  	/* Deactivate pins to prevent DRA7 DCAN IP from being
> @@ -1345,28 +1332,19 @@ int register_c_can_dev(struct net_device *dev)
>  	 */
>  	pinctrl_pm_select_sleep_state(dev->dev.parent);
>  
> -	c_can_pm_runtime_enable(priv);
> -
>  	dev->flags |= IFF_ECHO;	/* we support local echo */
>  	dev->netdev_ops = &c_can_netdev_ops;
>  
>  	err = register_candev(dev);
> -	if (err)
> -		c_can_pm_runtime_disable(priv);
> -	else
> -		devm_can_led_init(dev);
> -
> +	if (!err)
> +	  devm_can_led_init(dev);

Please indent with two tabs here.

regards,
Marc

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v2] can: c_can: move runtime PM enable/disable to c_can_platform
  2021-03-01 15:08 ` Marc Kleine-Budde
@ 2021-03-02  2:55   ` Tong Zhang
  2021-03-02  7:26     ` Marc Kleine-Budde
  2021-03-05 11:32     ` Uwe Kleine-König
  2021-03-02  2:55   ` [PATCH] " Tong Zhang
  1 sibling, 2 replies; 6+ messages in thread
From: Tong Zhang @ 2021-03-02  2:55 UTC (permalink / raw)
  To: Wolfgang Grandegger, Marc Kleine-Budde, David S. Miller,
	Jakub Kicinski, Vincent Mailhol, Oliver Hartkopp, Tong Zhang,
	YueHaibing, Zhang Qilong, linux-can, netdev, linux-kernel

Currently doing modprobe c_can_pci will make kernel complain
"Unbalanced pm_runtime_enable!", this is caused by pm_runtime_enable()
called before pm is initialized.
This fix is similar to 227619c3ff7c, move those pm_enable/disable code to
c_can_platform.

Signed-off-by: Tong Zhang <ztong0001@gmail.com>
---
 drivers/net/can/c_can/c_can.c          | 24 +-----------------------
 drivers/net/can/c_can/c_can_platform.c |  6 +++++-
 2 files changed, 6 insertions(+), 24 deletions(-)

diff --git a/drivers/net/can/c_can/c_can.c b/drivers/net/can/c_can/c_can.c
index ef474bae47a1..6958830cb983 100644
--- a/drivers/net/can/c_can/c_can.c
+++ b/drivers/net/can/c_can/c_can.c
@@ -212,18 +212,6 @@ static const struct can_bittiming_const c_can_bittiming_const = {
 	.brp_inc = 1,
 };
 
-static inline void c_can_pm_runtime_enable(const struct c_can_priv *priv)
-{
-	if (priv->device)
-		pm_runtime_enable(priv->device);
-}
-
-static inline void c_can_pm_runtime_disable(const struct c_can_priv *priv)
-{
-	if (priv->device)
-		pm_runtime_disable(priv->device);
-}
-
 static inline void c_can_pm_runtime_get_sync(const struct c_can_priv *priv)
 {
 	if (priv->device)
@@ -1335,7 +1323,6 @@ static const struct net_device_ops c_can_netdev_ops = {
 
 int register_c_can_dev(struct net_device *dev)
 {
-	struct c_can_priv *priv = netdev_priv(dev);
 	int err;
 
 	/* Deactivate pins to prevent DRA7 DCAN IP from being
@@ -1345,28 +1332,19 @@ int register_c_can_dev(struct net_device *dev)
 	 */
 	pinctrl_pm_select_sleep_state(dev->dev.parent);
 
-	c_can_pm_runtime_enable(priv);
-
 	dev->flags |= IFF_ECHO;	/* we support local echo */
 	dev->netdev_ops = &c_can_netdev_ops;
 
 	err = register_candev(dev);
-	if (err)
-		c_can_pm_runtime_disable(priv);
-	else
+	if (!err)
 		devm_can_led_init(dev);
-
 	return err;
 }
 EXPORT_SYMBOL_GPL(register_c_can_dev);
 
 void unregister_c_can_dev(struct net_device *dev)
 {
-	struct c_can_priv *priv = netdev_priv(dev);
-
 	unregister_candev(dev);
-
-	c_can_pm_runtime_disable(priv);
 }
 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 05f425ceb53a..47b251b1607c 100644
--- a/drivers/net/can/c_can/c_can_platform.c
+++ b/drivers/net/can/c_can/c_can_platform.c
@@ -29,6 +29,7 @@
 #include <linux/list.h>
 #include <linux/io.h>
 #include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
 #include <linux/clk.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
@@ -386,6 +387,7 @@ static int c_can_plat_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, dev);
 	SET_NETDEV_DEV(dev, &pdev->dev);
 
+	pm_runtime_enable(priv->device);
 	ret = register_c_can_dev(dev);
 	if (ret) {
 		dev_err(&pdev->dev, "registering %s failed (err=%d)\n",
@@ -398,6 +400,7 @@ static int c_can_plat_probe(struct platform_device *pdev)
 	return 0;
 
 exit_free_device:
+	pm_runtime_disable(priv->device);
 	free_c_can_dev(dev);
 exit:
 	dev_err(&pdev->dev, "probe failed\n");
@@ -408,9 +411,10 @@ static int c_can_plat_probe(struct platform_device *pdev)
 static int c_can_plat_remove(struct platform_device *pdev)
 {
 	struct net_device *dev = platform_get_drvdata(pdev);
+	struct c_can_priv *priv = netdev_priv(dev);
 
 	unregister_c_can_dev(dev);
-
+	pm_runtime_disable(priv->device);
 	free_c_can_dev(dev);
 
 	return 0;
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] can: c_can: move runtime PM enable/disable to c_can_platform
  2021-03-01 15:08 ` Marc Kleine-Budde
  2021-03-02  2:55   ` [PATCH v2] " Tong Zhang
@ 2021-03-02  2:55   ` Tong Zhang
  1 sibling, 0 replies; 6+ messages in thread
From: Tong Zhang @ 2021-03-02  2:55 UTC (permalink / raw)
  To: Marc Kleine-Budde
  Cc: Wolfgang Grandegger, David S. Miller, Jakub Kicinski,
	Vincent Mailhol, Oliver Hartkopp, YueHaibing, Zhang Qilong,
	linux-can, netdev, open list

On Mon, Mar 1, 2021 at 2:49 PM Marc Kleine-Budde <mkl@pengutronix.de> wrote:
>
> On 28.02.2021 23:15:48, Tong Zhang wrote:
> > Currently doing modprobe c_can_pci will make kernel complain
> > "Unbalanced pm_runtime_enable!", this is caused by pm_runtime_enable()
> > called before pm is initialized in register_candev() and doing so will
>
> I don't see where register_candev() is doing any pm related
> initialization.
>
> > also cause it to enable twice.
>
> > This fix is similar to 227619c3ff7c, move those pm_enable/disable code to
> > c_can_platform.
>
> As I understand 227619c3ff7c ("can: m_can: move runtime PM
> enable/disable to m_can_platform"), PCI devices automatically enable PM,
> when the "PCI device is added".

Hi Marc,
Thanks for the comments. I thinks you are right -- I was mislead by the trace --
I have corrected the commit log along with the indent fix in v2 patch.
Thanks again for your help,
- Tong

>
> Please clarify the above point, otherwise the code looks OK, small
> nitpick inline:

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] can: c_can: move runtime PM enable/disable to c_can_platform
  2021-03-02  2:55   ` [PATCH v2] " Tong Zhang
@ 2021-03-02  7:26     ` Marc Kleine-Budde
  2021-03-05 11:32     ` Uwe Kleine-König
  1 sibling, 0 replies; 6+ messages in thread
From: Marc Kleine-Budde @ 2021-03-02  7:26 UTC (permalink / raw)
  To: Tong Zhang, Wolfgang Grandegger, David S. Miller, Jakub Kicinski,
	Vincent Mailhol, Oliver Hartkopp, YueHaibing, Zhang Qilong,
	linux-can, netdev, linux-kernel


[-- Attachment #1.1: Type: text/plain, Size: 686 bytes --]

On 3/2/21 3:55 AM, Tong Zhang wrote:
> Currently doing modprobe c_can_pci will make kernel complain
> "Unbalanced pm_runtime_enable!", this is caused by pm_runtime_enable()
> called before pm is initialized.
> This fix is similar to 227619c3ff7c, move those pm_enable/disable code to
> c_can_platform.
> 
> Signed-off-by: Tong Zhang <ztong0001@gmail.com>

Applied to linux-can/testing.

Thanks,
Marc

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] can: c_can: move runtime PM enable/disable to c_can_platform
  2021-03-02  2:55   ` [PATCH v2] " Tong Zhang
  2021-03-02  7:26     ` Marc Kleine-Budde
@ 2021-03-05 11:32     ` Uwe Kleine-König
  1 sibling, 0 replies; 6+ messages in thread
From: Uwe Kleine-König @ 2021-03-05 11:32 UTC (permalink / raw)
  To: Tong Zhang
  Cc: Wolfgang Grandegger, Marc Kleine-Budde, David S. Miller,
	Jakub Kicinski, Vincent Mailhol, Oliver Hartkopp, YueHaibing,
	Zhang Qilong, linux-can, netdev, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 683 bytes --]

On Mon, Mar 01, 2021 at 09:55:40PM -0500, Tong Zhang wrote:
> Currently doing modprobe c_can_pci will make kernel complain
> "Unbalanced pm_runtime_enable!", this is caused by pm_runtime_enable()
> called before pm is initialized.
> This fix is similar to 227619c3ff7c, move those pm_enable/disable code to
> c_can_platform.

I can confirm this makes the warning go away on a Congatec Atom board. I
didn't do any further runtime tests.

Tested-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Thanks
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-03-05 11:33 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-01  4:15 [PATCH] can: c_can: move runtime PM enable/disable to c_can_platform Tong Zhang
2021-03-01 15:08 ` Marc Kleine-Budde
2021-03-02  2:55   ` [PATCH v2] " Tong Zhang
2021-03-02  7:26     ` Marc Kleine-Budde
2021-03-05 11:32     ` Uwe Kleine-König
2021-03-02  2:55   ` [PATCH] " Tong Zhang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).