linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v1 0/3] char: use generic power management
@ 2020-07-17  7:49 Vaibhav Gupta
  2020-07-17  7:49 ` [PATCH v1 1/3] amd64-agp: " Vaibhav Gupta
                   ` (4 more replies)
  0 siblings, 5 replies; 9+ messages in thread
From: Vaibhav Gupta @ 2020-07-17  7:49 UTC (permalink / raw)
  To: Bjorn Helgaas, Bjorn Helgaas, Bjorn Helgaas, Vaibhav Gupta,
	David Airlie, Arnd Bergmann, Greg Kroah-Hartman
  Cc: Vaibhav Gupta, linux-kernel, linux-kernel-mentees, Shuah Khan

Linux Kernel Mentee: Remove Legacy Power Management.

The purpose of this patch series is to upgrade power management in char
drivers. This has been done by upgrading .suspend() and .resume() callbacks.

The upgrade makes sure that the involvement of PCI Core does not change the
order of operations executed in a driver. Thus, does not change its behavior.

In general, drivers with legacy PM, .suspend() and .resume() make use of PCI
helper functions like pci_enable/disable_device_mem(), pci_set_power_state(),
pci_save/restore_state(), pci_enable/disable_device(), etc. to complete
their job.

The conversion requires the removal of those function calls, change the
callbacks' definition accordingly and make use of dev_pm_ops structure.

All patches are compile-tested only.

Test tools:
    - Compiler: gcc (GCC) 10.1.0
    - allmodconfig build: make -j$(nproc) W=1 all

Vaibhav Gupta (3):
  amd64-agp: use generic power management
  sis-agp: use generic power management
  via-agp: use generic power management

 drivers/char/agp/amd64-agp.c | 24 ++++++------------------
 drivers/char/agp/sis-agp.c   | 25 ++++++-------------------
 drivers/char/agp/via-agp.c   | 25 +++++--------------------
 3 files changed, 17 insertions(+), 57 deletions(-)

-- 
2.27.0


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

* [PATCH v1 1/3] amd64-agp: use generic power management
  2020-07-17  7:49 [PATCH v1 0/3] char: use generic power management Vaibhav Gupta
@ 2020-07-17  7:49 ` Vaibhav Gupta
  2020-07-17  7:49 ` [PATCH v1 2/3] sis-agp: " Vaibhav Gupta
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: Vaibhav Gupta @ 2020-07-17  7:49 UTC (permalink / raw)
  To: Bjorn Helgaas, Bjorn Helgaas, Bjorn Helgaas, Vaibhav Gupta,
	David Airlie, Arnd Bergmann, Greg Kroah-Hartman
  Cc: Vaibhav Gupta, linux-kernel, linux-kernel-mentees, Shuah Khan

Drivers using legacy PM have to manage PCI states and device's PM states
themselves. They also need to take care of configuration registers.

With improved and powerful support of generic PM, PCI Core takes care of
above mentioned, device-independent, jobs.

This driver makes use of PCI helper functions like
pci_save/restore_state() and pci_set_power_state() to do required
operations. In generic mode, they are no longer needed.

Change function parameter in .resume() to "struct device*" type. Use
to_pci_dev() to get "struct pci_dev*" variable.

After upgrading, .suspend() became an empty-body function, hence defined as
NULL.

Compile-tested only.

Signed-off-by: Vaibhav Gupta <vaibhavgupta40@gmail.com>
---
 drivers/char/agp/amd64-agp.c | 24 ++++++------------------
 1 file changed, 6 insertions(+), 18 deletions(-)

diff --git a/drivers/char/agp/amd64-agp.c b/drivers/char/agp/amd64-agp.c
index b40edae32817..57fdcc61f88a 100644
--- a/drivers/char/agp/amd64-agp.c
+++ b/drivers/char/agp/amd64-agp.c
@@ -588,20 +588,11 @@ static void agp_amd64_remove(struct pci_dev *pdev)
 	agp_bridges_found--;
 }
 
-#ifdef CONFIG_PM
+#define agp_amd64_suspend NULL
 
-static int agp_amd64_suspend(struct pci_dev *pdev, pm_message_t state)
+static int __maybe_unused agp_amd64_resume(struct device *dev)
 {
-	pci_save_state(pdev);
-	pci_set_power_state(pdev, pci_choose_state(pdev, state));
-
-	return 0;
-}
-
-static int agp_amd64_resume(struct pci_dev *pdev)
-{
-	pci_set_power_state(pdev, PCI_D0);
-	pci_restore_state(pdev);
+	struct pci_dev *pdev = to_pci_dev(dev);
 
 	if (pdev->vendor == PCI_VENDOR_ID_NVIDIA)
 		nforce3_agp_init(pdev);
@@ -609,8 +600,6 @@ static int agp_amd64_resume(struct pci_dev *pdev)
 	return amd_8151_configure();
 }
 
-#endif /* CONFIG_PM */
-
 static const struct pci_device_id agp_amd64_pci_table[] = {
 	{
 	.class		= (PCI_CLASS_BRIDGE_HOST << 8),
@@ -738,15 +727,14 @@ static const struct pci_device_id agp_amd64_pci_promisc_table[] = {
 	{ }
 };
 
+static SIMPLE_DEV_PM_OPS(agp_amd64_pm_ops, agp_amd64_suspend, agp_amd64_resume);
+
 static struct pci_driver agp_amd64_pci_driver = {
 	.name		= "agpgart-amd64",
 	.id_table	= agp_amd64_pci_table,
 	.probe		= agp_amd64_probe,
 	.remove		= agp_amd64_remove,
-#ifdef CONFIG_PM
-	.suspend	= agp_amd64_suspend,
-	.resume		= agp_amd64_resume,
-#endif
+	.driver.pm	= &agp_amd64_pm_ops,
 };
 
 
-- 
2.27.0


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

* [PATCH v1 2/3] sis-agp: use generic power management
  2020-07-17  7:49 [PATCH v1 0/3] char: use generic power management Vaibhav Gupta
  2020-07-17  7:49 ` [PATCH v1 1/3] amd64-agp: " Vaibhav Gupta
@ 2020-07-17  7:49 ` Vaibhav Gupta
  2020-07-17  7:49 ` [PATCH v1 3/3] via-agp: " Vaibhav Gupta
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: Vaibhav Gupta @ 2020-07-17  7:49 UTC (permalink / raw)
  To: Bjorn Helgaas, Bjorn Helgaas, Bjorn Helgaas, Vaibhav Gupta,
	David Airlie, Arnd Bergmann, Greg Kroah-Hartman
  Cc: Vaibhav Gupta, linux-kernel, linux-kernel-mentees, Shuah Khan

Drivers using legacy PM have to manage PCI states and device's PM states
themselves. They also need to take care of configuration registers.

With improved and powerful support of generic PM, PCI Core takes care of
above mentioned, device-independent, jobs.

This driver makes use of PCI helper functions like
pci_save/restore_state() and pci_set_power_state() to do required
operations. In generic mode, they are no longer needed.

Change function parameter in .resume() to "struct device*" type. Use
to_pci_dev() to get "struct pci_dev*" variable.

After upgrading, .suspend() became an empty-body function, hence defined as
NULL.

Compile-tested only.

Signed-off-by: Vaibhav Gupta <vaibhavgupta40@gmail.com>
---
 drivers/char/agp/sis-agp.c | 25 ++++++-------------------
 1 file changed, 6 insertions(+), 19 deletions(-)

diff --git a/drivers/char/agp/sis-agp.c b/drivers/char/agp/sis-agp.c
index 14909fc5d767..e2e62d5a8914 100644
--- a/drivers/char/agp/sis-agp.c
+++ b/drivers/char/agp/sis-agp.c
@@ -217,26 +217,14 @@ static void agp_sis_remove(struct pci_dev *pdev)
 	agp_put_bridge(bridge);
 }
 
-#ifdef CONFIG_PM
+#define agp_sis_suspend NULL
 
-static int agp_sis_suspend(struct pci_dev *pdev, pm_message_t state)
+static int __maybe_unused agp_sis_resume(
+	__attribute__((unused)) struct device *dev)
 {
-	pci_save_state(pdev);
-	pci_set_power_state(pdev, pci_choose_state(pdev, state));
-
-	return 0;
-}
-
-static int agp_sis_resume(struct pci_dev *pdev)
-{
-	pci_set_power_state(pdev, PCI_D0);
-	pci_restore_state(pdev);
-
 	return sis_driver.configure();
 }
 
-#endif /* CONFIG_PM */
-
 static const struct pci_device_id agp_sis_pci_table[] = {
 	{
 		.class		= (PCI_CLASS_BRIDGE_HOST << 8),
@@ -419,15 +407,14 @@ static const struct pci_device_id agp_sis_pci_table[] = {
 
 MODULE_DEVICE_TABLE(pci, agp_sis_pci_table);
 
+static SIMPLE_DEV_PM_OPS(agp_sis_pm_ops, agp_sis_suspend, agp_sis_resume);
+
 static struct pci_driver agp_sis_pci_driver = {
 	.name		= "agpgart-sis",
 	.id_table	= agp_sis_pci_table,
 	.probe		= agp_sis_probe,
 	.remove		= agp_sis_remove,
-#ifdef CONFIG_PM
-	.suspend	= agp_sis_suspend,
-	.resume		= agp_sis_resume,
-#endif
+	.driver.pm	= &agp_sis_pm_ops,
 };
 
 static int __init agp_sis_init(void)
-- 
2.27.0


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

* [PATCH v1 3/3] via-agp: use generic power management
  2020-07-17  7:49 [PATCH v1 0/3] char: use generic power management Vaibhav Gupta
  2020-07-17  7:49 ` [PATCH v1 1/3] amd64-agp: " Vaibhav Gupta
  2020-07-17  7:49 ` [PATCH v1 2/3] sis-agp: " Vaibhav Gupta
@ 2020-07-17  7:49 ` Vaibhav Gupta
  2020-08-17  7:49 ` [PATCH v1 0/3] char: " Vaibhav Gupta
  2021-01-12  8:09 ` [PATCH v2 " Vaibhav Gupta
  4 siblings, 0 replies; 9+ messages in thread
From: Vaibhav Gupta @ 2020-07-17  7:49 UTC (permalink / raw)
  To: Bjorn Helgaas, Bjorn Helgaas, Bjorn Helgaas, Vaibhav Gupta,
	David Airlie, Arnd Bergmann, Greg Kroah-Hartman
  Cc: Vaibhav Gupta, linux-kernel, linux-kernel-mentees, Shuah Khan

Drivers using legacy PM have to manage PCI states and device's PM states
themselves. They also need to take care of configuration registers.

With improved and powerful support of generic PM, PCI Core takes care of
above mentioned, device-independent, jobs.

This driver makes use of PCI helper functions like
pci_save/restore_state() and pci_set_power_state() to do required
operations. In generic mode, they are no longer needed.

Change function parameter in .resume() to "struct device*" type. Use
to_pci_dev() to get "struct pci_dev*" variable.

After upgrading, .suspend() became an empty-body function, hence defined as
NULL.

Compile-tested only.

Signed-off-by: Vaibhav Gupta <vaibhavgupta40@gmail.com>
---
 drivers/char/agp/via-agp.c | 25 +++++--------------------
 1 file changed, 5 insertions(+), 20 deletions(-)

diff --git a/drivers/char/agp/via-agp.c b/drivers/char/agp/via-agp.c
index 87a92a044570..afa941df7d44 100644
--- a/drivers/char/agp/via-agp.c
+++ b/drivers/char/agp/via-agp.c
@@ -492,22 +492,11 @@ static void agp_via_remove(struct pci_dev *pdev)
 	agp_put_bridge(bridge);
 }
 
-#ifdef CONFIG_PM
+#define agp_via_suspend NULL
 
-static int agp_via_suspend(struct pci_dev *pdev, pm_message_t state)
+static int __maybe_unused agp_via_resume(struct device *dev)
 {
-	pci_save_state (pdev);
-	pci_set_power_state (pdev, PCI_D3hot);
-
-	return 0;
-}
-
-static int agp_via_resume(struct pci_dev *pdev)
-{
-	struct agp_bridge_data *bridge = pci_get_drvdata(pdev);
-
-	pci_set_power_state (pdev, PCI_D0);
-	pci_restore_state(pdev);
+	struct agp_bridge_data *bridge = dev_get_drvdata(dev);
 
 	if (bridge->driver == &via_agp3_driver)
 		return via_configure_agp3();
@@ -517,8 +506,6 @@ static int agp_via_resume(struct pci_dev *pdev)
 	return 0;
 }
 
-#endif /* CONFIG_PM */
-
 /* must be the same order as name table above */
 static const struct pci_device_id agp_via_pci_table[] = {
 #define ID(x) \
@@ -567,16 +554,14 @@ static const struct pci_device_id agp_via_pci_table[] = {
 
 MODULE_DEVICE_TABLE(pci, agp_via_pci_table);
 
+static SIMPLE_DEV_PM_OPS(agp_via_pm_ops, agp_via_suspend, agp_via_resume);
 
 static struct pci_driver agp_via_pci_driver = {
 	.name		= "agpgart-via",
 	.id_table	= agp_via_pci_table,
 	.probe		= agp_via_probe,
 	.remove		= agp_via_remove,
-#ifdef CONFIG_PM
-	.suspend	= agp_via_suspend,
-	.resume		= agp_via_resume,
-#endif
+	.driver.pm	= &agp_via_pm_ops,
 };
 
 
-- 
2.27.0


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

* Re: [PATCH v1 0/3] char: use generic power management
  2020-07-17  7:49 [PATCH v1 0/3] char: use generic power management Vaibhav Gupta
                   ` (2 preceding siblings ...)
  2020-07-17  7:49 ` [PATCH v1 3/3] via-agp: " Vaibhav Gupta
@ 2020-08-17  7:49 ` Vaibhav Gupta
  2021-01-12  8:09 ` [PATCH v2 " Vaibhav Gupta
  4 siblings, 0 replies; 9+ messages in thread
From: Vaibhav Gupta @ 2020-08-17  7:49 UTC (permalink / raw)
  To: Bjorn Helgaas, Bjorn Helgaas, Bjorn Helgaas, Vaibhav Gupta,
	David Airlie, Arnd Bergmann, Greg Kroah-Hartman
  Cc: linux-kernel, linux-kernel-mentees, Shuah Khan

On Fri, Jul 17, 2020 at 01:19:34PM +0530, Vaibhav Gupta wrote:
> Linux Kernel Mentee: Remove Legacy Power Management.
> 
> The purpose of this patch series is to upgrade power management in char
> drivers. This has been done by upgrading .suspend() and .resume() callbacks.
> 
> The upgrade makes sure that the involvement of PCI Core does not change the
> order of operations executed in a driver. Thus, does not change its behavior.
> 
> In general, drivers with legacy PM, .suspend() and .resume() make use of PCI
> helper functions like pci_enable/disable_device_mem(), pci_set_power_state(),
> pci_save/restore_state(), pci_enable/disable_device(), etc. to complete
> their job.
> 
> The conversion requires the removal of those function calls, change the
> callbacks' definition accordingly and make use of dev_pm_ops structure.
> 
> All patches are compile-tested only.
> 
> Test tools:
>     - Compiler: gcc (GCC) 10.1.0
>     - allmodconfig build: make -j$(nproc) W=1 all
> 
> Vaibhav Gupta (3):
>   amd64-agp: use generic power management
>   sis-agp: use generic power management
>   via-agp: use generic power management
> 
>  drivers/char/agp/amd64-agp.c | 24 ++++++------------------
>  drivers/char/agp/sis-agp.c   | 25 ++++++-------------------
>  drivers/char/agp/via-agp.c   | 25 +++++--------------------
>  3 files changed, 17 insertions(+), 57 deletions(-)
> 
> -- 
> 2.27.0
> 

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

* [PATCH v2 0/3] char: use generic power management
  2020-07-17  7:49 [PATCH v1 0/3] char: use generic power management Vaibhav Gupta
                   ` (3 preceding siblings ...)
  2020-08-17  7:49 ` [PATCH v1 0/3] char: " Vaibhav Gupta
@ 2021-01-12  8:09 ` Vaibhav Gupta
  2021-01-12  8:09   ` [PATCH v2 1/3] amd64-agp: " Vaibhav Gupta
                     ` (2 more replies)
  4 siblings, 3 replies; 9+ messages in thread
From: Vaibhav Gupta @ 2021-01-12  8:09 UTC (permalink / raw)
  To: Bjorn Helgaas, Bjorn Helgaas, Bjorn Helgaas, Vaibhav Gupta, David Airlie
  Cc: Vaibhav Gupta, linux-kernel, linux-kernel-mentees, Shuah Khan

Linux Kernel Mentee: Remove Legacy Power Management.

The purpose of this patch series is to upgrade power management in char
drivers. This has been done by upgrading .suspend() and .resume() callbacks.

The upgrade makes sure that the involvement of PCI Core does not change the
order of operations executed in a driver. Thus, does not change its behavior.

In general, drivers with legacy PM, .suspend() and .resume() make use of PCI
helper functions like pci_enable/disable_device_mem(), pci_set_power_state(),
pci_save/restore_state(), pci_enable/disable_device(), etc. to complete
their job.

The conversion requires the removal of those function calls, change the
callbacks' definition accordingly and make use of dev_pm_ops structure.

All patches are compile-tested only.

Test tools:
    - Compiler: gcc (GCC) 10.2.0
    - allmodconfig build: make -j$(nproc) W=1 all

v2: patch-set rebased.

Vaibhav Gupta (3):
  amd64-agp: use generic power management
  sis-agp: use generic power management
  via-agp: use generic power management

 drivers/char/agp/amd64-agp.c | 24 ++++++------------------
 drivers/char/agp/sis-agp.c   | 25 ++++++-------------------
 drivers/char/agp/via-agp.c   | 25 +++++--------------------
 3 files changed, 17 insertions(+), 57 deletions(-)

-- 
2.30.0


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

* [PATCH v2 1/3] amd64-agp: use generic power management
  2021-01-12  8:09 ` [PATCH v2 " Vaibhav Gupta
@ 2021-01-12  8:09   ` Vaibhav Gupta
  2021-01-12  8:09   ` [PATCH v2 2/3] sis-agp: " Vaibhav Gupta
  2021-01-12  8:09   ` [PATCH v2 3/3] via-agp: " Vaibhav Gupta
  2 siblings, 0 replies; 9+ messages in thread
From: Vaibhav Gupta @ 2021-01-12  8:09 UTC (permalink / raw)
  To: Bjorn Helgaas, Bjorn Helgaas, Bjorn Helgaas, Vaibhav Gupta, David Airlie
  Cc: Vaibhav Gupta, linux-kernel, linux-kernel-mentees, Shuah Khan

Drivers should do only device-specific jobs. But in general, drivers using
legacy PCI PM framework for .suspend()/.resume() have to manage many PCI
PM-related tasks themselves which can be done by PCI Core itself. This
brings extra load on the driver and it directly calls PCI helper functions
to handle them.

Switch to the new generic framework by updating function signatures and
define a "struct dev_pm_ops" variable to bind PM callbacks. Also, remove
unnecessary calls to the PCI Helper functions along with the legacy
.suspend & .resume bindings.

After upgrading, .suspend() became an empty-body function, hence defined as
NULL.

Signed-off-by: Vaibhav Gupta <vaibhavgupta40@gmail.com>
---
 drivers/char/agp/amd64-agp.c | 24 ++++++------------------
 1 file changed, 6 insertions(+), 18 deletions(-)

diff --git a/drivers/char/agp/amd64-agp.c b/drivers/char/agp/amd64-agp.c
index b40edae32817..dc78a4fb879e 100644
--- a/drivers/char/agp/amd64-agp.c
+++ b/drivers/char/agp/amd64-agp.c
@@ -588,20 +588,11 @@ static void agp_amd64_remove(struct pci_dev *pdev)
 	agp_bridges_found--;
 }
 
-#ifdef CONFIG_PM
+#define agp_amd64_suspend NULL
 
-static int agp_amd64_suspend(struct pci_dev *pdev, pm_message_t state)
+static int __maybe_unused agp_amd64_resume(struct device *dev)
 {
-	pci_save_state(pdev);
-	pci_set_power_state(pdev, pci_choose_state(pdev, state));
-
-	return 0;
-}
-
-static int agp_amd64_resume(struct pci_dev *pdev)
-{
-	pci_set_power_state(pdev, PCI_D0);
-	pci_restore_state(pdev);
+	struct pci_dev *pdev = to_pci_dev(dev);
 
 	if (pdev->vendor == PCI_VENDOR_ID_NVIDIA)
 		nforce3_agp_init(pdev);
@@ -609,8 +600,6 @@ static int agp_amd64_resume(struct pci_dev *pdev)
 	return amd_8151_configure();
 }
 
-#endif /* CONFIG_PM */
-
 static const struct pci_device_id agp_amd64_pci_table[] = {
 	{
 	.class		= (PCI_CLASS_BRIDGE_HOST << 8),
@@ -738,15 +727,14 @@ static const struct pci_device_id agp_amd64_pci_promisc_table[] = {
 	{ }
 };
 
+static SIMPLE_DEV_PM_OPS(agp_amd64_pm_ops, agp_amd64_suspend, agp_amd64_resume);
+
 static struct pci_driver agp_amd64_pci_driver = {
 	.name		= "agpgart-amd64",
 	.id_table	= agp_amd64_pci_table,
 	.probe		= agp_amd64_probe,
 	.remove		= agp_amd64_remove,
-#ifdef CONFIG_PM
-	.suspend	= agp_amd64_suspend,
-	.resume		= agp_amd64_resume,
-#endif
+	.driver.pm  = &agp_amd64_pm_ops,
 };
 
 
-- 
2.30.0


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

* [PATCH v2 2/3] sis-agp: use generic power management
  2021-01-12  8:09 ` [PATCH v2 " Vaibhav Gupta
  2021-01-12  8:09   ` [PATCH v2 1/3] amd64-agp: " Vaibhav Gupta
@ 2021-01-12  8:09   ` Vaibhav Gupta
  2021-01-12  8:09   ` [PATCH v2 3/3] via-agp: " Vaibhav Gupta
  2 siblings, 0 replies; 9+ messages in thread
From: Vaibhav Gupta @ 2021-01-12  8:09 UTC (permalink / raw)
  To: Bjorn Helgaas, Bjorn Helgaas, Bjorn Helgaas, Vaibhav Gupta, David Airlie
  Cc: Vaibhav Gupta, linux-kernel, linux-kernel-mentees, Shuah Khan

Drivers should do only device-specific jobs. But in general, drivers using
legacy PCI PM framework for .suspend()/.resume() have to manage many PCI
PM-related tasks themselves which can be done by PCI Core itself. This
brings extra load on the driver and it directly calls PCI helper functions
to handle them.

Switch to the new generic framework by updating function signatures and
define a "struct dev_pm_ops" variable to bind PM callbacks. Also, remove
unnecessary calls to the PCI Helper functions along with the legacy
.suspend & .resume bindings.

After upgrading, .suspend() became an empty-body function, hence defined as
NULL.

Signed-off-by: Vaibhav Gupta <vaibhavgupta40@gmail.com>
---
 drivers/char/agp/sis-agp.c | 25 ++++++-------------------
 1 file changed, 6 insertions(+), 19 deletions(-)

diff --git a/drivers/char/agp/sis-agp.c b/drivers/char/agp/sis-agp.c
index 14909fc5d767..f8a02f4bef1b 100644
--- a/drivers/char/agp/sis-agp.c
+++ b/drivers/char/agp/sis-agp.c
@@ -217,26 +217,14 @@ static void agp_sis_remove(struct pci_dev *pdev)
 	agp_put_bridge(bridge);
 }
 
-#ifdef CONFIG_PM
+#define agp_sis_suspend NULL
 
-static int agp_sis_suspend(struct pci_dev *pdev, pm_message_t state)
+static int __maybe_unused agp_sis_resume(
+	__attribute__((unused)) struct device *dev)
 {
-	pci_save_state(pdev);
-	pci_set_power_state(pdev, pci_choose_state(pdev, state));
-
-	return 0;
-}
-
-static int agp_sis_resume(struct pci_dev *pdev)
-{
-	pci_set_power_state(pdev, PCI_D0);
-	pci_restore_state(pdev);
-
 	return sis_driver.configure();
 }
 
-#endif /* CONFIG_PM */
-
 static const struct pci_device_id agp_sis_pci_table[] = {
 	{
 		.class		= (PCI_CLASS_BRIDGE_HOST << 8),
@@ -419,15 +407,14 @@ static const struct pci_device_id agp_sis_pci_table[] = {
 
 MODULE_DEVICE_TABLE(pci, agp_sis_pci_table);
 
+static SIMPLE_DEV_PM_OPS(agp_sis_pm_ops, agp_sis_suspend, agp_sis_resume);
+
 static struct pci_driver agp_sis_pci_driver = {
 	.name		= "agpgart-sis",
 	.id_table	= agp_sis_pci_table,
 	.probe		= agp_sis_probe,
 	.remove		= agp_sis_remove,
-#ifdef CONFIG_PM
-	.suspend	= agp_sis_suspend,
-	.resume		= agp_sis_resume,
-#endif
+	.driver.pm      = &agp_sis_pm_ops,
 };
 
 static int __init agp_sis_init(void)
-- 
2.30.0


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

* [PATCH v2 3/3] via-agp: use generic power management
  2021-01-12  8:09 ` [PATCH v2 " Vaibhav Gupta
  2021-01-12  8:09   ` [PATCH v2 1/3] amd64-agp: " Vaibhav Gupta
  2021-01-12  8:09   ` [PATCH v2 2/3] sis-agp: " Vaibhav Gupta
@ 2021-01-12  8:09   ` Vaibhav Gupta
  2 siblings, 0 replies; 9+ messages in thread
From: Vaibhav Gupta @ 2021-01-12  8:09 UTC (permalink / raw)
  To: Bjorn Helgaas, Bjorn Helgaas, Bjorn Helgaas, Vaibhav Gupta, David Airlie
  Cc: Vaibhav Gupta, linux-kernel, linux-kernel-mentees, Shuah Khan

Drivers should do only device-specific jobs. But in general, drivers using
legacy PCI PM framework for .suspend()/.resume() have to manage many PCI
PM-related tasks themselves which can be done by PCI Core itself. This
brings extra load on the driver and it directly calls PCI helper functions
to handle them.

Switch to the new generic framework by updating function signatures and
define a "struct dev_pm_ops" variable to bind PM callbacks. Also, remove
unnecessary calls to the PCI Helper functions along with the legacy
.suspend & .resume bindings.

After upgrading, .suspend() became an empty-body function, hence defined as
NULL.

Signed-off-by: Vaibhav Gupta <vaibhavgupta40@gmail.com>
---
 drivers/char/agp/via-agp.c | 25 +++++--------------------
 1 file changed, 5 insertions(+), 20 deletions(-)

diff --git a/drivers/char/agp/via-agp.c b/drivers/char/agp/via-agp.c
index 87a92a044570..a460ae352772 100644
--- a/drivers/char/agp/via-agp.c
+++ b/drivers/char/agp/via-agp.c
@@ -492,22 +492,11 @@ static void agp_via_remove(struct pci_dev *pdev)
 	agp_put_bridge(bridge);
 }
 
-#ifdef CONFIG_PM
+#define agp_via_suspend NULL
 
-static int agp_via_suspend(struct pci_dev *pdev, pm_message_t state)
+static int __maybe_unused agp_via_resume(struct device *dev)
 {
-	pci_save_state (pdev);
-	pci_set_power_state (pdev, PCI_D3hot);
-
-	return 0;
-}
-
-static int agp_via_resume(struct pci_dev *pdev)
-{
-	struct agp_bridge_data *bridge = pci_get_drvdata(pdev);
-
-	pci_set_power_state (pdev, PCI_D0);
-	pci_restore_state(pdev);
+	struct agp_bridge_data *bridge = dev_get_drvdata(dev);
 
 	if (bridge->driver == &via_agp3_driver)
 		return via_configure_agp3();
@@ -517,8 +506,6 @@ static int agp_via_resume(struct pci_dev *pdev)
 	return 0;
 }
 
-#endif /* CONFIG_PM */
-
 /* must be the same order as name table above */
 static const struct pci_device_id agp_via_pci_table[] = {
 #define ID(x) \
@@ -567,16 +554,14 @@ static const struct pci_device_id agp_via_pci_table[] = {
 
 MODULE_DEVICE_TABLE(pci, agp_via_pci_table);
 
+static SIMPLE_DEV_PM_OPS(agp_via_pm_ops, agp_via_suspend, agp_via_resume);
 
 static struct pci_driver agp_via_pci_driver = {
 	.name		= "agpgart-via",
 	.id_table	= agp_via_pci_table,
 	.probe		= agp_via_probe,
 	.remove		= agp_via_remove,
-#ifdef CONFIG_PM
-	.suspend	= agp_via_suspend,
-	.resume		= agp_via_resume,
-#endif
+	.driver.pm      = &agp_via_pm_ops,
 };
 
 
-- 
2.30.0


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

end of thread, other threads:[~2021-01-12  8:13 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-17  7:49 [PATCH v1 0/3] char: use generic power management Vaibhav Gupta
2020-07-17  7:49 ` [PATCH v1 1/3] amd64-agp: " Vaibhav Gupta
2020-07-17  7:49 ` [PATCH v1 2/3] sis-agp: " Vaibhav Gupta
2020-07-17  7:49 ` [PATCH v1 3/3] via-agp: " Vaibhav Gupta
2020-08-17  7:49 ` [PATCH v1 0/3] char: " Vaibhav Gupta
2021-01-12  8:09 ` [PATCH v2 " Vaibhav Gupta
2021-01-12  8:09   ` [PATCH v2 1/3] amd64-agp: " Vaibhav Gupta
2021-01-12  8:09   ` [PATCH v2 2/3] sis-agp: " Vaibhav Gupta
2021-01-12  8:09   ` [PATCH v2 3/3] via-agp: " Vaibhav Gupta

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).