Skip to contentSkip to conversation

i2c: nvidia-gpu: drop empty stub for runtime pm

PATCH Applied Vaibhav Gupta 3 participants 6 messages Nov 7, 2020 – Dec 2, 2020

Subsystems (2)
I2C subsystem host drivers I2C controller driver for NVIDIA GPU
lore
Share & tools
lore

Vaibhav Gupta

author
#0 lore raw

PATCH 1/1 i2c: nvidia-gpu: drop empty stub for runtime pm

After the commit c5eb1190074c ("PCI / PM: Allow runtime PM without callback functions") we no more need empty stubs for runtime-pm to work.

The driver has no device specific task(s) for .suspend() . The stub was placed just for runtime-pm, which can be dropped now.

Reported-by Bjorn Helgaas <[email protected]>
Signed-off-by Vaibhav Gupta <[email protected]>
---
 drivers/i2c/busses/i2c-nvidia-gpu.c | 10 +---------
 1 file changed, 1 insertion(+), 9 deletions(-)
drivers/i2c/busses/i2c-nvidia-gpu.c +1 -9
index f9a69b109e5c..6b20601ffb13 100644
@@ -353,15 +353,7 @@ static void gpu_i2c_remove(struct pci_dev *pdev)
 	pci_free_irq_vectors(pdev);
 }
 
-/*
- * We need gpu_i2c_suspend() even if it is stub, for runtime pm to work
- * correctly. Without it, lspci shows runtime pm status as "D0" for the card.
- * Documentation/power/pci.rst also insists for driver to provide this.
- */
-static __maybe_unused int gpu_i2c_suspend(struct device *dev)
-{
-	return 0;
-}
+#define gpu_i2c_suspend NULL
 
 static __maybe_unused int gpu_i2c_resume(struct device *dev)
 {
Signature · 1 line
2.28.0
author
#1 lore raw
Quoted 36 lines · Vaibhav Gupta

After the commit c5eb1190074c ("PCI / PM: Allow runtime PM without callback functions") we no more need empty stubs for runtime-pm to work.

The driver has no device specific task(s) for .suspend() . The stub was placed just for runtime-pm, which can be dropped now.

Reported-by Bjorn Helgaas <[email protected]>
Signed-off-by Vaibhav Gupta <[email protected]>
---
 drivers/i2c/busses/i2c-nvidia-gpu.c | 10 +---------
 1 file changed, 1 insertion(+), 9 deletions(-)
drivers/i2c/busses/i2c-nvidia-gpu.c +1 -9
index f9a69b109e5c..6b20601ffb13 100644
@@ -353,15 +353,7 @@ static void gpu_i2c_remove(struct pci_dev *pdev)
 	pci_free_irq_vectors(pdev);
 }
 
-/*
- * We need gpu_i2c_suspend() even if it is stub, for runtime pm to work
- * correctly. Without it, lspci shows runtime pm status as "D0" for the card.
- * Documentation/power/pci.rst also insists for driver to provide this.
- */
-static __maybe_unused int gpu_i2c_suspend(struct device *dev)
-{
-	return 0;
-}
+#define gpu_i2c_suspend NULL
 
 static __maybe_unused int gpu_i2c_resume(struct device *dev)
 {
Signature · 1 line
2.28.0

The patch is only compile-tested.

--Vaibhav

Reviewed-by
#2 lore raw
Quoted 21 lines · Vaibhav Gupta
Quoted 20 lines · Vaibhav Gupta

After the commit c5eb1190074c ("PCI / PM: Allow runtime PM without callback functions") we no more need empty stubs for runtime-pm to work.

The driver has no device specific task(s) for .suspend() . The stub was placed just for runtime-pm, which can be dropped now.

Reported-by Bjorn Helgaas <[email protected]>
Signed-off-by Vaibhav Gupta <[email protected]>
---
  drivers/i2c/busses/i2c-nvidia-gpu.c | 10 +---------
  1 file changed, 1 insertion(+), 9 deletions(-)
drivers/i2c/busses/i2c-nvidia-gpu.c +0 -0
index f9a69b109e5c..6b20601ffb13 100644
@@ -353,15 +353,7 @@ static void gpu_i2c_remove(struct pci_dev *pdev)
  	pci_free_irq_vectors(pdev);
  }
Quoted 11 lines

drivers/i2c/busses/i2c-nvidia-gpu.c static void gpu_i2c_remove(struct pci_dev *pdev) · line 357

Quoted 11 lines
drivers/i2c/busses/i2c-nvidia-gpu.c +1 -9
-/*
- * We need gpu_i2c_suspend() even if it is stub, for runtime pm to work
- * correctly. Without it, lspci shows runtime pm status as "D0" for the card.
- * Documentation/power/pci.rst also insists for driver to provide this.
- */
-static __maybe_unused int gpu_i2c_suspend(struct device *dev)
-{
-	return 0;
-}
+#define gpu_i2c_suspend NULL

Perhaps we can put NULL directly into UNIVERSAL_DEV_PM_OPS() for the suspend callback?

Quoted 7 lines
Quoted 5 lines
drivers/i2c/busses/i2c-nvidia-gpu.c +0 -0
  static __maybe_unused int gpu_i2c_resume(struct device *dev)
  {
Signature · 1 line
2.28.0

The patch is only compile-tested.

It should work also system suspend point of view. This patch affects also it since callbacks are set with the UNIVERSAL_DEV_PM_OPS(). Means the same callback is used for runtime PM and system suspend.

I quickly debugged this with an another driver and PCI stack does put the device into D3 state in system suspend from pci_prepare_to_sleep() if the suspend callback is not set and device was on prior it.

Reviewed-by Jarkko Nikula <[email protected]>
author
#3 lore raw
Quoted 25 lines · Jarkko Nikula
Quoted 24 lines · Vaibhav Gupta
Quoted 23 lines · Vaibhav Gupta

After the commit c5eb1190074c ("PCI / PM: Allow runtime PM without callback functions") we no more need empty stubs for runtime-pm to work.

The driver has no device specific task(s) for .suspend() . The stub was placed just for runtime-pm, which can be dropped now.

Reported-by Bjorn Helgaas <[email protected]>
Signed-off-by Vaibhav Gupta <[email protected]>
---
  drivers/i2c/busses/i2c-nvidia-gpu.c | 10 +---------
  1 file changed, 1 insertion(+), 9 deletions(-)
drivers/i2c/busses/i2c-nvidia-gpu.c +0 -4
index f9a69b109e5c..6b20601ffb13 100644
@@ -353,15 +353,7 @@ static void gpu_i2c_remove(struct pci_dev *pdev)
  	pci_free_irq_vectors(pdev);
  }
-/*
- * We need gpu_i2c_suspend() even if it is stub, for runtime pm to work
- * correctly. Without it, lspci shows runtime pm status as "D0" for the card.
- * Documentation/power/pci.rst also insists for driver to provide this.
Quoted 9 lines
Quoted 6 lines

drivers/i2c/busses/i2c-nvidia-gpu.c static void gpu_i2c_remove(struct pci_dev *pdev) · line 356

Quoted 6 lines
drivers/i2c/busses/i2c-nvidia-gpu.c +1 -5
- */
-static __maybe_unused int gpu_i2c_suspend(struct device *dev)
-{
-	return 0;
-}
+#define gpu_i2c_suspend NULL

Perhaps we can put NULL directly into UNIVERSAL_DEV_PM_OPS() for the suspend callback?

Yes. I have noticed that the approach for this is random. Many drivers pass NULL directly to the dev_pm_ops type variable, and rest of them use a macro.

I used it for symmetry. I mean there is 'gpu_i2c_resume', so although a macro, I have put a 'gpu_i2c_suspend'.

Although it won't make any significant change, but if required, I can send another patch where NULL is passed into UNIVERSAL_DEV_PM_OPS() instead.

Quoted 11 lines
Quoted 7 lines
Quoted 5 lines
drivers/i2c/busses/i2c-nvidia-gpu.c +0 -0
  static __maybe_unused int gpu_i2c_resume(struct device *dev)
  {
Signature · 1 line
2.28.0

The patch is only compile-tested.

It should work also system suspend point of view. This patch affects also it since callbacks are set with the UNIVERSAL_DEV_PM_OPS(). Means the same callback is used for runtime PM and system suspend.

Quoted 5 lines

I quickly debugged this with an another driver and PCI stack does put the device into D3 state in system suspend from pci_prepare_to_sleep() if the suspend callback is not set and device was on prior it.

Reviewed-by Jarkko Nikula <[email protected]>

Thanks for debugging. Theoretically too, according to generic framework, drivers are supposed to do only device specific jobs. If there is none, or callback is set as NULL, remaining work to put the device in lower power state is PCI stack's task.

--Vaibhav

#4 lore raw
Quoted 5 lines · Vaibhav Gupta
Quoted 4 lines · Jarkko Nikula
Quoted 1 line
Quoted 1 line

+#define gpu_i2c_suspend NULL

Perhaps we can put NULL directly into UNIVERSAL_DEV_PM_OPS() for the suspend callback?

Quoted 8 lines

Yes. I have noticed that the approach for this is random. Many drivers pass NULL directly to the dev_pm_ops type variable, and rest of them use a macro.

I used it for symmetry. I mean there is 'gpu_i2c_resume', so although a macro, I have put a 'gpu_i2c_suspend'.

Although it won't make any significant change, but if required, I can send another patch where NULL is passed into UNIVERSAL_DEV_PM_OPS() instead.

No need to resend from my side, it was just a remark and I gave already the reviewed-by tag.

Jarkko