2020-06-24 17:31:53

by Vaibhav Gupta

[permalink] [raw]
Subject: [PATCH v1 0/4] drivers: ide: use generic power management

Linux Kernel Mentee: Remove Legacy Power Management.

The purpose of this patch series is to remove legacy power management callbacks
from ide drivers.

The suspend() and resume() callbacks operations are still invoking
pci_save/restore_state(), pci_set_power_state(), pci_enable/disable_state(),
etc. and handling the power management themselves, which is not recommended.

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

All patches are compile-tested only.

Vaibhav Gupta (4):
ide: use generic power management
ide: triflex: use generic power management
ide: sc1200: use generic power management
ide: delkin_cb: use generic power management

drivers/ide/aec62xx.c | 3 +--
drivers/ide/alim15x3.c | 3 +--
drivers/ide/amd74xx.c | 3 +--
drivers/ide/atiixp.c | 3 +--
drivers/ide/cmd64x.c | 3 +--
drivers/ide/cs5520.c | 3 +--
drivers/ide/cs5530.c | 3 +--
drivers/ide/cs5535.c | 3 +--
drivers/ide/cs5536.c | 3 +--
drivers/ide/cy82c693.c | 3 +--
drivers/ide/delkin_cb.c | 35 ++++++----------------------
drivers/ide/hpt366.c | 3 +--
drivers/ide/ide-pci-generic.c | 3 +--
drivers/ide/it8172.c | 3 +--
drivers/ide/it8213.c | 3 +--
drivers/ide/it821x.c | 3 +--
drivers/ide/jmicron.c | 3 +--
drivers/ide/ns87415.c | 3 +--
drivers/ide/opti621.c | 3 +--
drivers/ide/pdc202xx_new.c | 3 +--
drivers/ide/pdc202xx_old.c | 3 +--
drivers/ide/piix.c | 3 +--
drivers/ide/sc1200.c | 43 ++++++++++++-----------------------
drivers/ide/serverworks.c | 3 +--
drivers/ide/setup-pci.c | 28 ++++-------------------
drivers/ide/siimage.c | 3 +--
drivers/ide/sis5513.c | 3 +--
drivers/ide/sl82c105.c | 3 +--
drivers/ide/slc90e66.c | 3 +--
drivers/ide/triflex.c | 24 +++++++++----------
drivers/ide/via82cxxx.c | 3 +--
include/linux/ide.h | 8 +------
32 files changed, 65 insertions(+), 154 deletions(-)

--
2.27.0


2020-06-24 17:32:21

by Vaibhav Gupta

[permalink] [raw]
Subject: [PATCH v1 2/4] ide: triflex: use generic power management

While upgrading ide_pci_suspend() and ide_pci_resume(), all other source
files, using same callbacks, were also updated except
drivers/ide/triflex.c. This is because the driver does not want to power
off the device during suspend. A quirk was required for the same.

This patch provides the fix. Another driver,
drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c, makes use of
a quirk for similar goal. Hence a similar quirk has been applied for
triflex.

Compile-tested only.

Signed-off-by: Vaibhav Gupta <[email protected]>
---
drivers/ide/triflex.c | 45 +++++++++++--------------------------------
1 file changed, 11 insertions(+), 34 deletions(-)

diff --git a/drivers/ide/triflex.c b/drivers/ide/triflex.c
index 1886bbfb9e5d..f707f11c296d 100644
--- a/drivers/ide/triflex.c
+++ b/drivers/ide/triflex.c
@@ -100,48 +100,25 @@ static const struct pci_device_id triflex_pci_tbl[] = {
};
MODULE_DEVICE_TABLE(pci, triflex_pci_tbl);

-#ifdef CONFIG_PM
-static int triflex_ide_pci_suspend(struct pci_dev *dev, pm_message_t state)
-{
- /*
- * We must not disable or powerdown the device.
- * APM bios refuses to suspend if IDE is not accessible.
- */
- pci_save_state(dev);
- return 0;
-}
-
-static int triflex_ide_pci_resume(struct pci_dev *dev)
+/*
+ * We must not disable or powerdown the device.
+ * APM bios refuses to suspend if IDE is not accessible.
+ */
+static void triflex_pci_pm_cap_fixup(struct pci_dev *pdev)
{
- struct ide_host *host = pci_get_drvdata(dev);
- int rc;
-
- pci_set_power_state(dev, PCI_D0);
-
- rc = pci_enable_device(dev);
- if (rc)
- return rc;
-
- pci_restore_state(dev);
- pci_set_master(dev);
-
- if (host->init_chipset)
- host->init_chipset(dev);
-
- return 0;
+ dev_info(&pdev->dev, "Disable triflex to be turned off by PCI CORE\n");
+ pdev->pm_cap = 0;
}
-#else
-#define triflex_ide_pci_suspend NULL
-#define triflex_ide_pci_resume NULL
-#endif
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_COMPAQ,
+ PCI_DEVICE_ID_COMPAQ_TRIFLEX_IDE,
+ triflex_pci_pm_cap_fixup);

static struct pci_driver triflex_pci_driver = {
.name = "TRIFLEX_IDE",
.id_table = triflex_pci_tbl,
.probe = triflex_init_one,
.remove = ide_pci_remove,
- .suspend = triflex_ide_pci_suspend,
- .resume = triflex_ide_pci_resume,
+ .driver.pm = &ide_pci_pm_ops,
};

static int __init triflex_ide_init(void)
--
2.27.0

2020-06-24 17:32:54

by Vaibhav Gupta

[permalink] [raw]
Subject: [PATCH v1 4/4] ide: delkin_cb: use generic power management

With the support of generic PM callbacks, drivers no longer need to use
legacy .suspend() and .resume() in which they had to maintain PCI states
changes and device's power state themselves. All required operations are
done by PCI core.

After converting it into generic model, suspend() became an empty function.
Hence, it is defined as NULL.

Compile-tested only.

Signed-off-by: Vaibhav Gupta <[email protected]>
---
drivers/ide/delkin_cb.c | 35 +++++++----------------------------
1 file changed, 7 insertions(+), 28 deletions(-)

diff --git a/drivers/ide/delkin_cb.c b/drivers/ide/delkin_cb.c
index 300daabaa575..8a3dc4dd6e66 100644
--- a/drivers/ide/delkin_cb.c
+++ b/drivers/ide/delkin_cb.c
@@ -123,39 +123,17 @@ delkin_cb_remove (struct pci_dev *dev)
pci_disable_device(dev);
}

-#ifdef CONFIG_PM
-static int delkin_cb_suspend(struct pci_dev *dev, pm_message_t state)
-{
- pci_save_state(dev);
- pci_disable_device(dev);
- pci_set_power_state(dev, pci_choose_state(dev, state));
-
- return 0;
-}
+#define delkin_cb_suspend NULL

-static int delkin_cb_resume(struct pci_dev *dev)
+static int __maybe_unused delkin_cb_resume(struct device *dev_d)
{
- struct ide_host *host = pci_get_drvdata(dev);
- int rc;
-
- pci_set_power_state(dev, PCI_D0);
-
- rc = pci_enable_device(dev);
- if (rc)
- return rc;
-
- pci_restore_state(dev);
- pci_set_master(dev);
+ struct ide_host *host = dev_get_drvdata(dev_d);

if (host->init_chipset)
- host->init_chipset(dev);
+ host->init_chipset(to_pci_dev(dev_d));

return 0;
}
-#else
-#define delkin_cb_suspend NULL
-#define delkin_cb_resume NULL
-#endif

static struct pci_device_id delkin_cb_pci_tbl[] = {
{ 0x1145, 0xf021, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0},
@@ -164,13 +142,14 @@ static struct pci_device_id delkin_cb_pci_tbl[] = {
};
MODULE_DEVICE_TABLE(pci, delkin_cb_pci_tbl);

+static SIMPLE_DEV_PM_OPS(delkin_cb_pm_ops, delkin_cb_suspend, delkin_cb_resume);
+
static struct pci_driver delkin_cb_pci_driver = {
.name = "Delkin-ASKA-Workbit Cardbus IDE",
.id_table = delkin_cb_pci_tbl,
.probe = delkin_cb_probe,
.remove = delkin_cb_remove,
- .suspend = delkin_cb_suspend,
- .resume = delkin_cb_resume,
+ .driver.pm = &delkin_cb_pm_ops,
};

module_pci_driver(delkin_cb_pci_driver);
--
2.27.0

2020-06-24 17:33:35

by Vaibhav Gupta

[permalink] [raw]
Subject: [PATCH v1 3/4] ide: sc1200: use generic power management

With the support of generic PM callbacks, drivers no longer need to use
legacy .suspend() and .resume() in which they had to maintain PCI states
changes and device's power state themselves. The required operations are
done by PCI core.

Compile-tested only.

Signed-off-by: Vaibhav Gupta <[email protected]>
---
drivers/ide/sc1200.c | 43 ++++++++++++++-----------------------------
1 file changed, 14 insertions(+), 29 deletions(-)

diff --git a/drivers/ide/sc1200.c b/drivers/ide/sc1200.c
index a5b701818405..91a197832d1f 100644
--- a/drivers/ide/sc1200.c
+++ b/drivers/ide/sc1200.c
@@ -222,46 +222,33 @@ static void sc1200_set_pio_mode(ide_hwif_t *hwif, ide_drive_t *drive)
sc1200_tunepio(drive, pio);
}

-#ifdef CONFIG_PM
struct sc1200_saved_state {
u32 regs[8];
};

-static int sc1200_suspend (struct pci_dev *dev, pm_message_t state)
+static int __maybe_unused sc1200_suspend(struct device *dev_d)
{
- printk("SC1200: suspend(%u)\n", state.event);
+ struct pci_dev *dev = to_pci_dev(dev_d);
+ struct ide_host *host = pci_get_drvdata(dev);
+ struct sc1200_saved_state *ss = host->host_priv;
+ unsigned int r;

/*
- * we only save state when going from full power to less
+ * save timing registers
+ * (this may be unnecessary if BIOS also does it)
*/
- if (state.event == PM_EVENT_ON) {
- struct ide_host *host = pci_get_drvdata(dev);
- struct sc1200_saved_state *ss = host->host_priv;
- unsigned int r;
-
- /*
- * save timing registers
- * (this may be unnecessary if BIOS also does it)
- */
- for (r = 0; r < 8; r++)
- pci_read_config_dword(dev, 0x40 + r * 4, &ss->regs[r]);
- }
+ for (r = 0; r < 8; r++)
+ pci_read_config_dword(dev, 0x40 + r * 4, &ss->regs[r]);

- pci_disable_device(dev);
- pci_set_power_state(dev, pci_choose_state(dev, state));
return 0;
}

-static int sc1200_resume (struct pci_dev *dev)
+static int __maybe_unused sc1200_resume(struct device *dev_d)
{
+ struct pci_dev *dev = to_pci_dev(dev_d);
struct ide_host *host = pci_get_drvdata(dev);
struct sc1200_saved_state *ss = host->host_priv;
unsigned int r;
- int i;
-
- i = pci_enable_device(dev);
- if (i)
- return i;

/*
* restore timing registers
@@ -272,7 +259,6 @@ static int sc1200_resume (struct pci_dev *dev)

return 0;
}
-#endif

static const struct ide_port_ops sc1200_port_ops = {
.set_pio_mode = sc1200_set_pio_mode,
@@ -326,15 +312,14 @@ static const struct pci_device_id sc1200_pci_tbl[] = {
};
MODULE_DEVICE_TABLE(pci, sc1200_pci_tbl);

+static SIMPLE_DEV_PM_OPS(sc1200_pm_ops, sc1200_suspend, sc1200_resume);
+
static struct pci_driver sc1200_pci_driver = {
.name = "SC1200_IDE",
.id_table = sc1200_pci_tbl,
.probe = sc1200_init_one,
.remove = ide_pci_remove,
-#ifdef CONFIG_PM
- .suspend = sc1200_suspend,
- .resume = sc1200_resume,
-#endif
+ .driver.pm = &sc1200_pm_ops,
};

static int __init sc1200_ide_init(void)
--
2.27.0