PM: Renesas: Remove dummy runtime PM callbacks

PATCH Applied Jarkko Nikula 4 participants 12 messages Oct 24, 2018 – Oct 28, 2018

Subsystems (7)

Networking Network drivers USB I2C subsystem host drivers +3 more subsystems

lore

Share & tools

lore

Jarkko Nikula

author

2018-10-24 13:51 UTC #0 lore raw

cover PM: Renesas: Remove dummy runtime PM callbacks

I noticed these independent Renesas drivers have needless dummy runtime PM callbacks. I don't have the HW so only build tested.

Patches can be applied independently to their own subsystems. I wanted to send them together if some of them gets Tested-by or sees a regression.

Jarkko Nikula (3):
i2c: sh_mobile: Remove dummy runtime PM callbacks
net: ethernet: Remove dummy runtime PM callbacks from Renesas drivers
usb: renesas_usbhs: Remove dummy runtime PM callbacks

 drivers/i2c/busses/i2c-sh_mobile.c       | 18 ------------------
 drivers/net/ethernet/renesas/ravb_main.c | 13 -------------
 drivers/net/ethernet/renesas/sh_eth.c    | 13 -------------
 drivers/usb/renesas_usbhs/common.c       | 14 --------------
 4 files changed, 58 deletions(-)

Signature · 1 line

2.19.1

Jarkko Nikula

author

2018-10-24 13:51 UTC #1 lore raw

PATCH 1/3 i2c: sh_mobile: Remove dummy runtime PM callbacks

Platform drivers don't need dummy runtime PM callbacks that just return success and non-NULL pm pointer in their struct device_driver in order to have runtime PM happening. This has changed since following commits:

05aa55dddb9e ("PM / Runtime: Lenient generic runtime pm callbacks") 543f2503a956 ("PM / platform_bus: Allow runtime PM by default") 8b313a38ecff ("PM / Platform: Use generic runtime PM callbacks directly")

Signed-off-by Jarkko Nikula <[email protected]>

---
Only build tested.

---
 drivers/i2c/busses/i2c-sh_mobile.c | 18 ------------------
 1 file changed, 18 deletions(-)

drivers/i2c/busses/i2c-sh_mobile.c +0 ~~-18~~

index 818cab14e87c..a7a7a9c3bc7c 100644
@@ -947,27 +947,9 @@ static int sh_mobile_i2c_remove(struct platform_device *dev)
 	return 0;
 }
 
-static int sh_mobile_i2c_runtime_nop(struct device *dev)
-{
-	/* Runtime PM callback shared between ->runtime_suspend()
-	 * and ->runtime_resume(). Simply returns success.
-	 *
-	 * This driver re-initializes all registers after
-	 * pm_runtime_get_sync() anyway so there is no need
-	 * to save and restore registers here.
-	 */
-	return 0;
-}
-
-static const struct dev_pm_ops sh_mobile_i2c_dev_pm_ops = {
-	.runtime_suspend = sh_mobile_i2c_runtime_nop,
-	.runtime_resume = sh_mobile_i2c_runtime_nop,
-};
-
 static struct platform_driver sh_mobile_i2c_driver = {
 	.driver		= {
 		.name		= "i2c-sh_mobile",
-		.pm		= &sh_mobile_i2c_dev_pm_ops,
 		.of_match_table = sh_mobile_i2c_dt_ids,
 	},
 	.probe		= sh_mobile_i2c_probe,

Signature · 1 line

2.19.1

Wolfram Sang

Applied

2018-10-28 22:22 UTC #2 lore raw

Re: [PATCH 1/3] i2c: sh_mobile: Remove dummy runtime PM callbacks

Quoted 9 lines · Jarkko Nikula

Platform drivers don't need dummy runtime PM callbacks that just return success and non-NULL pm pointer in their struct device_driver in order to have runtime PM happening. This has changed since following commits:
05aa55dddb9e ("PM / Runtime: Lenient generic runtime pm callbacks") 543f2503a956 ("PM / platform_bus: Allow runtime PM by default") 8b313a38ecff ("PM / Platform: Use generic runtime PM callbacks directly")
Signed-off-by Jarkko Nikula <[email protected]>

Applied to for-next, thanks!

Jarkko Nikula

author

2018-10-24 13:51 UTC #3 lore raw

PATCH 2/3 net: ethernet: Remove dummy runtime PM callbacks from Renesas drivers

In reply to Jarkko Nikula

Platform drivers don't need dummy runtime PM callbacks that just return success in order to have runtime PM happening. This has changed since following commits:

Signed-off-by Jarkko Nikula <[email protected]>

---
Only build tested.

---
 drivers/net/ethernet/renesas/ravb_main.c | 13 -------------
 drivers/net/ethernet/renesas/sh_eth.c    | 13 -------------
 2 files changed, 26 deletions(-)

drivers/net/ethernet/renesas/ravb_main.c +0 ~~-13~~

index defed0d0c51d..9498f9d67501 100644
@@ -2335,21 +2335,8 @@ static int __maybe_unused ravb_resume(struct device *dev)
 	return ret;
 }
 
-static int __maybe_unused ravb_runtime_nop(struct device *dev)
-{
-	/* Runtime PM callback shared between ->runtime_suspend()
-	 * and ->runtime_resume(). Simply returns success.
-	 *
-	 * This driver re-initializes all registers after
-	 * pm_runtime_get_sync() anyway so there is no need
-	 * to save and restore registers here.
-	 */
-	return 0;
-}
-
 static const struct dev_pm_ops ravb_dev_pm_ops = {
 	SET_SYSTEM_SLEEP_PM_OPS(ravb_suspend, ravb_resume)
-	SET_RUNTIME_PM_OPS(ravb_runtime_nop, ravb_runtime_nop, NULL)
 };
 
 static struct platform_driver ravb_driver = {

drivers/net/ethernet/renesas/sh_eth.c +0 ~~-13~~

index f27a0dc8c563..980310686d45 100644
@@ -3447,21 +3447,8 @@ static int sh_eth_resume(struct device *dev)
 }
 #endif
 
-static int sh_eth_runtime_nop(struct device *dev)
-{
-	/* Runtime PM callback shared between ->runtime_suspend()
-	 * and ->runtime_resume(). Simply returns success.
-	 *
-	 * This driver re-initializes all registers after
-	 * pm_runtime_get_sync() anyway so there is no need
-	 * to save and restore registers here.
-	 */
-	return 0;
-}
-
 static const struct dev_pm_ops sh_eth_dev_pm_ops = {
 	SET_SYSTEM_SLEEP_PM_OPS(sh_eth_suspend, sh_eth_resume)
-	SET_RUNTIME_PM_OPS(sh_eth_runtime_nop, sh_eth_runtime_nop, NULL)
 };
 #define SH_ETH_PM_OPS (&sh_eth_dev_pm_ops)
 #else

Signature · 1 line

2.19.1

Sergei Shtylyov

Reviewed-by on v1 2/3

2018-10-24 17:44 UTC #4 lore raw

Re: [PATCH 2/3] net: ethernet: Remove dummy runtime PM callbacks from Renesas drivers

Hello!

Quoted 7 lines · Jarkko Nikula

Platform drivers don't need dummy runtime PM callbacks that just return success in order to have runtime PM happening. This has changed since following commits:
05aa55dddb9e ("PM / Runtime: Lenient generic runtime pm callbacks") 543f2503a956 ("PM / platform_bus: Allow runtime PM by default") 8b313a38ecff ("PM / Platform: Use generic runtime PM callbacks directly")

Wow, these are old! :-)

Quoted 3 lines

Signed-off-by Jarkko Nikula <[email protected]>
---
Only build tested.

Seems long overdue...

[...]

Reviewed-by Sergei Shtylyov <[email protected]>

MBR, Sergei

Wolfram Sang

Acked-by on v1 2/3

2018-10-25 22:54 UTC #5 lore raw

Re: [PATCH 2/3] net: ethernet: Remove dummy runtime PM callbacks from Renesas drivers

In reply to Jarkko Nikula

Quoted 9 lines · Jarkko Nikula

Platform drivers don't need dummy runtime PM callbacks that just return success in order to have runtime PM happening. This has changed since following commits:
05aa55dddb9e ("PM / Runtime: Lenient generic runtime pm callbacks") 543f2503a956 ("PM / platform_bus: Allow runtime PM by default") 8b313a38ecff ("PM / Platform: Use generic runtime PM callbacks directly")
Signed-off-by Jarkko Nikula <[email protected]>

Acked-by Wolfram Sang <[email protected]>

Jarkko Nikula

author

2018-10-24 13:51 UTC #6 lore raw

PATCH 3/3 usb: renesas_usbhs: Remove dummy runtime PM callbacks

In reply to Jarkko Nikula

Platform drivers don't need dummy runtime PM callbacks that just return success in order to have runtime PM happening. This has changed since following commits:

Signed-off-by Jarkko Nikula <[email protected]>

---
Only build tested.

---
 drivers/usb/renesas_usbhs/common.c | 14 --------------
 1 file changed, 14 deletions(-)

drivers/usb/renesas_usbhs/common.c +0 ~~-14~~

index a3e1290d682d..0e760f228dd8 100644
@@ -874,23 +874,9 @@ static int usbhsc_resume(struct device *dev)
 	return 0;
 }
 
-static int usbhsc_runtime_nop(struct device *dev)
-{
-	/* Runtime PM callback shared between ->runtime_suspend()
-	 * and ->runtime_resume(). Simply returns success.
-	 *
-	 * This driver re-initializes all registers after
-	 * pm_runtime_get_sync() anyway so there is no need
-	 * to save and restore registers here.
-	 */
-	return 0;
-}
-
 static const struct dev_pm_ops usbhsc_pm_ops = {
 	.suspend		= usbhsc_suspend,
 	.resume			= usbhsc_resume,
-	.runtime_suspend	= usbhsc_runtime_nop,
-	.runtime_resume		= usbhsc_runtime_nop,
 };
 
 static struct platform_driver renesas_usbhs_driver = {

Signature · 1 line

2.19.1

Yoshihiro Shimoda

Reviewed-by on v1 3/3

2018-10-25 02:45 UTC #7 lore raw

RE: [PATCH 3/3] usb: renesas_usbhs: Remove dummy runtime PM callbacks

Hi Jarkko,

Quoted 6 lines

From: Jarkko Nikula, Sent: Wednesday, October 24, 2018 10:52 PM
Platform drivers don't need dummy runtime PM callbacks that just return success in order to have runtime PM happening. This has changed since following commits:

Quoted 7 lines

05aa55dddb9e ("PM / Runtime: Lenient generic runtime pm callbacks") 543f2503a956 ("PM / platform_bus: Allow runtime PM by default") 8b313a38ecff ("PM / Platform: Use generic runtime PM callbacks directly")
Signed-off-by Jarkko Nikula <[email protected]>
---
Only build tested.

Thank you for the patch!

Reviewed-by Yoshihiro Shimoda <[email protected]>

Quoted 11 lines

---
 drivers/usb/renesas_usbhs/common.c | 14 --------------
 1 file changed, 14 deletions(-)

drivers/usb/renesas_usbhs/common.c +0 -0

index a3e1290d682d..0e760f228dd8 100644
@@ -874,23 +874,9 @@ static int usbhsc_resume(struct device *dev)
 	return 0;
 }

drivers/usb/renesas_usbhs/common.c static int usbhsc_resume(struct device *dev) · line 885

Quoted 10 lines

drivers/usb/renesas_usbhs/common.c +0 -9

 
-static int usbhsc_runtime_nop(struct device *dev)
-{
-	/* Runtime PM callback shared between ->runtime_suspend()
-	 * and ->runtime_resume(). Simply returns success.
-	 *
-	 * This driver re-initializes all registers after
-	 * pm_runtime_get_sync() anyway so there is no need
-	 * to save and restore registers here.
-	 */

I guess this code came from i2c-sh_mobile.c or sh_eth.c
and we didn't realize this code didn't need at that time (Oct 10 2011).

Best regards,
Yoshihiro Shimoda

Quoted 13 lines

drivers/usb/renesas_usbhs/common.c +0 -6

-	return 0;
-}
-
 static const struct dev_pm_ops usbhsc_pm_ops = {
 	.suspend		= usbhsc_suspend,
 	.resume			= usbhsc_resume,
-	.runtime_suspend	= usbhsc_runtime_nop,
-	.runtime_resume		= usbhsc_runtime_nop,
 };
 
 static struct platform_driver renesas_usbhs_driver = {
--

2.19.1

Wolfram Sang

Acked-by on v1 3/3

2018-10-25 22:57 UTC #8 lore raw

Re: [PATCH 3/3] usb: renesas_usbhs: Remove dummy runtime PM callbacks

In reply to Jarkko Nikula

Quoted 9 lines · Jarkko Nikula

Platform drivers don't need dummy runtime PM callbacks that just return success in order to have runtime PM happening. This has changed since following commits:
05aa55dddb9e ("PM / Runtime: Lenient generic runtime pm callbacks") 543f2503a956 ("PM / platform_bus: Allow runtime PM by default") 8b313a38ecff ("PM / Platform: Use generic runtime PM callbacks directly")
Signed-off-by Jarkko Nikula <[email protected]>

Acked-by Wolfram Sang <[email protected]>

drivers/usb/renesas_usbhs/common.c static int usbhsc_resume(struct device *dev) · line 879

Quoted 3 lines

static const struct dev_pm_ops usbhsc_pm_ops = {
drivers/usb/renesas_usbhs/common.c +0 -0
 	.suspend		= usbhsc_suspend,
 	.resume			= usbhsc_resume,

Unrelated to this patch, but I wonder right now: is there a reason not to use SET_SYSTEM_SLEEP_PM_OPS here? Shimoda-san?

Yoshihiro Shimoda

2018-10-26 02:09 UTC #9 lore raw

RE: [PATCH 3/3] usb: renesas_usbhs: Remove dummy runtime PM callbacks

Hi Wolfram-san,

Quoted 1 line

From: Wolfram Sang, Sent: Friday, October 26, 2018 7:58 AM

<snip>

Quoted 6 lines

drivers/usb/renesas_usbhs/common.c static int usbhsc_resume(struct device *dev) · line 879
Quoted 3 lines
static const struct dev_pm_ops usbhsc_pm_ops = {
drivers/usb/renesas_usbhs/common.c +0 -0
 	.suspend		= usbhsc_suspend,
 	.resume			= usbhsc_resume,
Unrelated to this patch, but I wonder right now: is there a reason not to use SET_SYSTEM_SLEEP_PM_OPS here? Shimoda-san?

I don't know why because this code is contributed from Morimoto-san. I'm guessing this code seems to come from sh_eth.c or i2c-sh_mobile.c and we don't have the SET_SYSTEM_SLEEP_PM_OPS macro at 2009. Morimoto-san contributed this code at 2010, but it seems not to realize we have such macro.

Anyway, I'll try to use SET_SYSTEM_SLEEP_PM_OPS on the renesas_usbhs driver.

Best regards,
Yoshihiro Shimoda

Wolfram Sang

2018-10-24 14:22 UTC #10 lore raw

In reply to Jarkko Nikula

Quoted 5 lines · Jarkko Nikula

I noticed these independent Renesas drivers have needless dummy runtime PM callbacks. I don't have the HW so only build tested.
Patches can be applied independently to their own subsystems. I wanted to send them together if some of them gets Tested-by or sees a regression.

At least the I2C driver part of this was on my todo list as well (just a bit lower :/). I wanted to find out why they have been there in the first place. Do you know if such callbacks were needed "back in the days"?

Adding Magnus to recipients...

Quoted 15 lines

Jarkko Nikula (3):
i2c: sh_mobile: Remove dummy runtime PM callbacks
net: ethernet: Remove dummy runtime PM callbacks from Renesas drivers
usb: renesas_usbhs: Remove dummy runtime PM callbacks
 drivers/i2c/busses/i2c-sh_mobile.c       | 18 ------------------
 drivers/net/ethernet/renesas/ravb_main.c | 13 -------------
 drivers/net/ethernet/renesas/sh_eth.c    | 13 -------------
 drivers/usb/renesas_usbhs/common.c       | 14 --------------
 4 files changed, 58 deletions(-)
Signature · 1 line
2.19.1

Wolfram Sang

2018-10-25 00:20 UTC #11 lore raw

Quoted 5 lines

At least the I2C driver part of this was on my todo list as well (just a bit lower :/). I wanted to find out why they have been there in the first place. Do you know if such callbacks were needed "back in the days"?

I see now that you referenced the relevant commits in the patch descriptions. Thanks!