* [PATCH v5 1/4] mtip32xx: remove pointless drvdata checking
2021-12-08 19:24 [PATCH v5 0/4] block: convert to generic power management Bjorn Helgaas
@ 2021-12-08 19:24 ` Bjorn Helgaas
2021-12-08 19:24 ` [PATCH v5 2/4] mtip32xx: remove pointless drvdata lookups Bjorn Helgaas
` (3 subsequent siblings)
4 siblings, 0 replies; 8+ messages in thread
From: Bjorn Helgaas @ 2021-12-08 19:24 UTC (permalink / raw)
To: Jens Axboe, Joshua Morris, Philip Kelleher
Cc: Rafael J . Wysocki, Vaibhav Gupta, Vaibhav Gupta, linux-block,
linux-pci, linux-kernel, linux-kernel-mentees, Shuah Khan,
Bjorn Helgaas
From: Bjorn Helgaas <bhelgaas@google.com>
The .suspend() and .resume() methods are only called after the .probe()
method (mtip_pci_probe()) has set the drvdata and returned success.
Therefore, if we get to mtip_pci_suspend() or mtip_pci_resume(), the
drvdata must be valid. Drop the unnecessary checking.
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
drivers/block/mtip32xx/mtip32xx.c | 15 +--------------
1 file changed, 1 insertion(+), 14 deletions(-)
diff --git a/drivers/block/mtip32xx/mtip32xx.c b/drivers/block/mtip32xx/mtip32xx.c
index c91b9010c1a6..8677ac4c3431 100644
--- a/drivers/block/mtip32xx/mtip32xx.c
+++ b/drivers/block/mtip32xx/mtip32xx.c
@@ -4150,12 +4150,6 @@ static int mtip_pci_suspend(struct pci_dev *pdev, pm_message_t mesg)
int rv = 0;
struct driver_data *dd = pci_get_drvdata(pdev);
- if (!dd) {
- dev_err(&pdev->dev,
- "Driver private datastructure is NULL\n");
- return -EFAULT;
- }
-
set_bit(MTIP_DDF_RESUME_BIT, &dd->dd_flag);
/* Disable ports & interrupts then send standby immediate */
@@ -4189,14 +4183,7 @@ static int mtip_pci_suspend(struct pci_dev *pdev, pm_message_t mesg)
static int mtip_pci_resume(struct pci_dev *pdev)
{
int rv = 0;
- struct driver_data *dd;
-
- dd = pci_get_drvdata(pdev);
- if (!dd) {
- dev_err(&pdev->dev,
- "Driver private datastructure is NULL\n");
- return -EFAULT;
- }
+ struct driver_data *dd = pci_get_drvdata(pdev);
/* Move the device to active State */
pci_set_power_state(pdev, PCI_D0);
--
2.25.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v5 2/4] mtip32xx: remove pointless drvdata lookups
2021-12-08 19:24 [PATCH v5 0/4] block: convert to generic power management Bjorn Helgaas
2021-12-08 19:24 ` [PATCH v5 1/4] mtip32xx: remove pointless drvdata checking Bjorn Helgaas
@ 2021-12-08 19:24 ` Bjorn Helgaas
2021-12-08 19:24 ` [PATCH v5 3/4] mtip32xx: convert to generic power management Bjorn Helgaas
` (2 subsequent siblings)
4 siblings, 0 replies; 8+ messages in thread
From: Bjorn Helgaas @ 2021-12-08 19:24 UTC (permalink / raw)
To: Jens Axboe, Joshua Morris, Philip Kelleher
Cc: Rafael J . Wysocki, Vaibhav Gupta, Vaibhav Gupta, linux-block,
linux-pci, linux-kernel, linux-kernel-mentees, Shuah Khan,
Bjorn Helgaas
From: Bjorn Helgaas <bhelgaas@google.com>
Previously we passed a struct pci_dev * to mtip_check_surprise_removal(),
which immediately looked up the driver_data. But all callers already have
the driver_data pointer, so just pass it directly and skip the extra
lookup. No functional change intended.
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
drivers/block/mtip32xx/mtip32xx.c | 25 ++++++++++++-------------
1 file changed, 12 insertions(+), 13 deletions(-)
diff --git a/drivers/block/mtip32xx/mtip32xx.c b/drivers/block/mtip32xx/mtip32xx.c
index 8677ac4c3431..894020aaaaeb 100644
--- a/drivers/block/mtip32xx/mtip32xx.c
+++ b/drivers/block/mtip32xx/mtip32xx.c
@@ -136,16 +136,15 @@ struct mtip_compat_ide_task_request_s {
* return value
* true if device removed, else false
*/
-static bool mtip_check_surprise_removal(struct pci_dev *pdev)
+static bool mtip_check_surprise_removal(struct driver_data *dd)
{
u16 vendor_id = 0;
- struct driver_data *dd = pci_get_drvdata(pdev);
if (dd->sr)
return true;
/* Read the vendorID from the configuration space */
- pci_read_config_word(pdev, 0x00, &vendor_id);
+ pci_read_config_word(dd->pdev, 0x00, &vendor_id);
if (vendor_id == 0xFFFF) {
dd->sr = true;
if (dd->queue)
@@ -447,7 +446,7 @@ static int mtip_device_reset(struct driver_data *dd)
{
int rv = 0;
- if (mtip_check_surprise_removal(dd->pdev))
+ if (mtip_check_surprise_removal(dd))
return 0;
if (mtip_hba_reset(dd) < 0)
@@ -727,7 +726,7 @@ static inline void mtip_process_errors(struct driver_data *dd, u32 port_stat)
dev_warn(&dd->pdev->dev,
"Port stat errors %x unhandled\n",
(port_stat & ~PORT_IRQ_HANDLED));
- if (mtip_check_surprise_removal(dd->pdev))
+ if (mtip_check_surprise_removal(dd))
return;
}
if (likely(port_stat & (PORT_IRQ_TF_ERR | PORT_IRQ_IF_ERR))) {
@@ -752,7 +751,7 @@ static inline irqreturn_t mtip_handle_irq(struct driver_data *data)
/* Acknowledge the interrupt status on the port.*/
port_stat = readl(port->mmio + PORT_IRQ_STAT);
if (unlikely(port_stat == 0xFFFFFFFF)) {
- mtip_check_surprise_removal(dd->pdev);
+ mtip_check_surprise_removal(dd);
return IRQ_HANDLED;
}
writel(port_stat, port->mmio + PORT_IRQ_STAT);
@@ -796,7 +795,7 @@ static inline irqreturn_t mtip_handle_irq(struct driver_data *data)
}
if (unlikely(port_stat & PORT_IRQ_ERR)) {
- if (unlikely(mtip_check_surprise_removal(dd->pdev))) {
+ if (unlikely(mtip_check_surprise_removal(dd))) {
/* don't proceed further */
return IRQ_HANDLED;
}
@@ -915,7 +914,7 @@ static int mtip_quiesce_io(struct mtip_port *port, unsigned long timeout)
msleep(100);
- if (mtip_check_surprise_removal(port->dd->pdev))
+ if (mtip_check_surprise_removal(port->dd))
goto err_fault;
active = mtip_commands_active(port);
@@ -980,7 +979,7 @@ static int mtip_exec_internal_command(struct mtip_port *port,
return -EFAULT;
}
- if (mtip_check_surprise_removal(dd->pdev))
+ if (mtip_check_surprise_removal(dd))
return -EFAULT;
rq = blk_mq_alloc_request(dd->queue, REQ_OP_DRV_IN, BLK_MQ_REQ_RESERVED);
@@ -1022,7 +1021,7 @@ static int mtip_exec_internal_command(struct mtip_port *port,
fis->command, int_cmd->status);
rv = -EIO;
- if (mtip_check_surprise_removal(dd->pdev) ||
+ if (mtip_check_surprise_removal(dd) ||
test_bit(MTIP_DDF_REMOVE_PENDING_BIT,
&dd->dd_flag)) {
dev_err(&dd->pdev->dev,
@@ -2513,7 +2512,7 @@ static int mtip_ftl_rebuild_poll(struct driver_data *dd)
if (unlikely(test_bit(MTIP_DDF_REMOVE_PENDING_BIT,
&dd->dd_flag)))
return -EFAULT;
- if (mtip_check_surprise_removal(dd->pdev))
+ if (mtip_check_surprise_removal(dd))
return -EFAULT;
if (mtip_get_identify(dd->port, NULL) < 0)
@@ -2891,7 +2890,7 @@ static int mtip_hw_init(struct driver_data *dd)
time_before(jiffies, timeout)) {
mdelay(100);
}
- if (unlikely(mtip_check_surprise_removal(dd->pdev))) {
+ if (unlikely(mtip_check_surprise_removal(dd))) {
timetaken = jiffies - timetaken;
dev_warn(&dd->pdev->dev,
"Surprise removal detected at %u ms\n",
@@ -4098,7 +4097,7 @@ static void mtip_pci_remove(struct pci_dev *pdev)
list_add(&dd->remove_list, &removing_list);
spin_unlock_irqrestore(&dev_lock, flags);
- mtip_check_surprise_removal(pdev);
+ mtip_check_surprise_removal(dd);
synchronize_irq(dd->pdev->irq);
/* Spin until workers are done */
--
2.25.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v5 3/4] mtip32xx: convert to generic power management
2021-12-08 19:24 [PATCH v5 0/4] block: convert to generic power management Bjorn Helgaas
2021-12-08 19:24 ` [PATCH v5 1/4] mtip32xx: remove pointless drvdata checking Bjorn Helgaas
2021-12-08 19:24 ` [PATCH v5 2/4] mtip32xx: remove pointless drvdata lookups Bjorn Helgaas
@ 2021-12-08 19:24 ` Bjorn Helgaas
2021-12-08 19:24 ` [PATCH v5 4/4] rsxx: Drop PCI legacy " Bjorn Helgaas
2021-12-14 13:59 ` [PATCH v5 0/4] block: convert to generic " Jens Axboe
4 siblings, 0 replies; 8+ messages in thread
From: Bjorn Helgaas @ 2021-12-08 19:24 UTC (permalink / raw)
To: Jens Axboe, Joshua Morris, Philip Kelleher
Cc: Rafael J . Wysocki, Vaibhav Gupta, Vaibhav Gupta, linux-block,
linux-pci, linux-kernel, linux-kernel-mentees, Shuah Khan,
Bjorn Helgaas
From: Vaibhav Gupta <vaibhavgupta40@gmail.com>
Convert mtip32xx from legacy PCI power management to the generic power
management framework.
Previously, mtip32xx used legacy PCI power management, where
mtip_pci_suspend() and mtip_pci_resume() were responsible for both
device-specific things and generic PCI things:
mtip_pci_suspend
mtip_block_suspend(dd) <-- device-specific
pci_save_state(pdev) <-- generic PCI
pci_set_power_state(pdev, pci_choose_state(pdev, state))
mtip_pci_resume
pci_set_power_state(PCI_D0) <-- generic PCI
pci_restore_state(pdev) <-- generic PCI
pcim_enable_device(pdev) <-- generic PCI
pci_set_master(pdev) <-- generic PCI
mtip_block_resume(dd) <-- device-specific
With generic power management, the PCI bus PM methods do the generic PCI
things, and the driver needs only the device-specific part, i.e.,
suspend_devices_and_enter
dpm_suspend_start(PMSG_SUSPEND)
pci_pm_suspend # PCI bus .suspend() method
mtip_pci_suspend # dev->driver->pm->suspend
mtip_block_suspend <-- device-specific
suspend_enter
dpm_suspend_noirq(PMSG_SUSPEND)
pci_pm_suspend_noirq # PCI bus .suspend_noirq() method
pci_save_state <-- generic PCI
pci_prepare_to_sleep <-- generic PCI
pci_set_power_state
...
dpm_resume_end(PMSG_RESUME)
pci_pm_resume # PCI bus .resume() method
pci_restore_standard_config
pci_set_power_state(PCI_D0) <-- generic PCI
pci_restore_state <-- generic PCI
mtip_pci_resume # dev->driver->pm->resume
mtip_block_resume <-- device-specific
[bhelgaas: commit log]
Link: https://lore.kernel.org/r/20210114115423.52414-2-vaibhavgupta40@gmail.com
Signed-off-by: Vaibhav Gupta <vaibhavgupta40@gmail.com>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
drivers/block/mtip32xx/mtip32xx.c | 48 +++++++------------------------
1 file changed, 10 insertions(+), 38 deletions(-)
diff --git a/drivers/block/mtip32xx/mtip32xx.c b/drivers/block/mtip32xx/mtip32xx.c
index 894020aaaaeb..368b3c4e0744 100644
--- a/drivers/block/mtip32xx/mtip32xx.c
+++ b/drivers/block/mtip32xx/mtip32xx.c
@@ -4144,30 +4144,17 @@ static void mtip_pci_remove(struct pci_dev *pdev)
* 0 Success
* <0 Error
*/
-static int mtip_pci_suspend(struct pci_dev *pdev, pm_message_t mesg)
+static int __maybe_unused mtip_pci_suspend(struct device *dev)
{
int rv = 0;
- struct driver_data *dd = pci_get_drvdata(pdev);
+ struct driver_data *dd = dev_get_drvdata(dev);
set_bit(MTIP_DDF_RESUME_BIT, &dd->dd_flag);
/* Disable ports & interrupts then send standby immediate */
rv = mtip_block_suspend(dd);
- if (rv < 0) {
- dev_err(&pdev->dev,
- "Failed to suspend controller\n");
- return rv;
- }
-
- /*
- * Save the pci config space to pdev structure &
- * disable the device
- */
- pci_save_state(pdev);
- pci_disable_device(pdev);
-
- /* Move to Low power state*/
- pci_set_power_state(pdev, PCI_D3hot);
+ if (rv < 0)
+ dev_err(dev, "Failed to suspend controller\n");
return rv;
}
@@ -4179,25 +4166,10 @@ static int mtip_pci_suspend(struct pci_dev *pdev, pm_message_t mesg)
* 0 Success
* <0 Error
*/
-static int mtip_pci_resume(struct pci_dev *pdev)
+static int __maybe_unused mtip_pci_resume(struct device *dev)
{
int rv = 0;
- struct driver_data *dd = pci_get_drvdata(pdev);
-
- /* Move the device to active State */
- pci_set_power_state(pdev, PCI_D0);
-
- /* Restore PCI configuration space */
- pci_restore_state(pdev);
-
- /* Enable the PCI device*/
- rv = pcim_enable_device(pdev);
- if (rv < 0) {
- dev_err(&pdev->dev,
- "Failed to enable card during resume\n");
- goto err;
- }
- pci_set_master(pdev);
+ struct driver_data *dd = dev_get_drvdata(dev);
/*
* Calls hbaReset, initPort, & startPort function
@@ -4205,9 +4177,8 @@ static int mtip_pci_resume(struct pci_dev *pdev)
*/
rv = mtip_block_resume(dd);
if (rv < 0)
- dev_err(&pdev->dev, "Unable to resume\n");
+ dev_err(dev, "Unable to resume\n");
-err:
clear_bit(MTIP_DDF_RESUME_BIT, &dd->dd_flag);
return rv;
@@ -4238,14 +4209,15 @@ static const struct pci_device_id mtip_pci_tbl[] = {
{ 0 }
};
+static SIMPLE_DEV_PM_OPS(mtip_pci_pm_ops, mtip_pci_suspend, mtip_pci_resume);
+
/* Structure that describes the PCI driver functions. */
static struct pci_driver mtip_pci_driver = {
.name = MTIP_DRV_NAME,
.id_table = mtip_pci_tbl,
.probe = mtip_pci_probe,
.remove = mtip_pci_remove,
- .suspend = mtip_pci_suspend,
- .resume = mtip_pci_resume,
+ .driver.pm = &mtip_pci_pm_ops,
.shutdown = mtip_pci_shutdown,
};
--
2.25.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v5 4/4] rsxx: Drop PCI legacy power management
2021-12-08 19:24 [PATCH v5 0/4] block: convert to generic power management Bjorn Helgaas
` (2 preceding siblings ...)
2021-12-08 19:24 ` [PATCH v5 3/4] mtip32xx: convert to generic power management Bjorn Helgaas
@ 2021-12-08 19:24 ` Bjorn Helgaas
2021-12-14 7:55 ` Christoph Hellwig
2021-12-14 13:59 ` [PATCH v5 0/4] block: convert to generic " Jens Axboe
4 siblings, 1 reply; 8+ messages in thread
From: Bjorn Helgaas @ 2021-12-08 19:24 UTC (permalink / raw)
To: Jens Axboe, Joshua Morris, Philip Kelleher
Cc: Rafael J . Wysocki, Vaibhav Gupta, Vaibhav Gupta, linux-block,
linux-pci, linux-kernel, linux-kernel-mentees, Shuah Khan,
Bjorn Helgaas
From: Bjorn Helgaas <bhelgaas@google.com>
The rsxx driver doesn't support device suspend, so remove
rsxx_pci_suspend(), the legacy PCI .suspend() method, completely.
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
drivers/block/rsxx/core.c | 7 -------
1 file changed, 7 deletions(-)
diff --git a/drivers/block/rsxx/core.c b/drivers/block/rsxx/core.c
index 8d9d69f5dfbc..19b85d16d711 100644
--- a/drivers/block/rsxx/core.c
+++ b/drivers/block/rsxx/core.c
@@ -1037,12 +1037,6 @@ static void rsxx_pci_remove(struct pci_dev *dev)
kfree(card);
}
-static int rsxx_pci_suspend(struct pci_dev *dev, pm_message_t state)
-{
- /* We don't support suspend at this time. */
- return -ENOSYS;
-}
-
static void rsxx_pci_shutdown(struct pci_dev *dev)
{
struct rsxx_cardinfo *card = pci_get_drvdata(dev);
@@ -1083,7 +1077,6 @@ static struct pci_driver rsxx_pci_driver = {
.id_table = rsxx_pci_ids,
.probe = rsxx_pci_probe,
.remove = rsxx_pci_remove,
- .suspend = rsxx_pci_suspend,
.shutdown = rsxx_pci_shutdown,
.err_handler = &rsxx_err_handler,
};
--
2.25.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v5 4/4] rsxx: Drop PCI legacy power management
2021-12-08 19:24 ` [PATCH v5 4/4] rsxx: Drop PCI legacy " Bjorn Helgaas
@ 2021-12-14 7:55 ` Christoph Hellwig
2021-12-14 13:59 ` Jens Axboe
0 siblings, 1 reply; 8+ messages in thread
From: Christoph Hellwig @ 2021-12-14 7:55 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Jens Axboe, Joshua Morris, Philip Kelleher, Rafael J . Wysocki,
Vaibhav Gupta, Vaibhav Gupta, linux-block, linux-pci,
linux-kernel, linux-kernel-mentees, Shuah Khan, Bjorn Helgaas
Maybe it is time to just drop this driver? It was never widly used,
seems unmaintained and uses a cumbersome own queueing layer instead
of blk-mq.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v5 4/4] rsxx: Drop PCI legacy power management
2021-12-14 7:55 ` Christoph Hellwig
@ 2021-12-14 13:59 ` Jens Axboe
0 siblings, 0 replies; 8+ messages in thread
From: Jens Axboe @ 2021-12-14 13:59 UTC (permalink / raw)
To: Christoph Hellwig, Bjorn Helgaas
Cc: Joshua Morris, Philip Kelleher, Rafael J . Wysocki,
Vaibhav Gupta, Vaibhav Gupta, linux-block, linux-pci,
linux-kernel, linux-kernel-mentees, Shuah Khan, Bjorn Helgaas
On 12/14/21 12:55 AM, Christoph Hellwig wrote:
> Maybe it is time to just drop this driver? It was never widly used,
> seems unmaintained and uses a cumbersome own queueing layer instead
> of blk-mq.
I think so, but let's do that independently of this series.
--
Jens Axboe
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v5 0/4] block: convert to generic power management
2021-12-08 19:24 [PATCH v5 0/4] block: convert to generic power management Bjorn Helgaas
` (3 preceding siblings ...)
2021-12-08 19:24 ` [PATCH v5 4/4] rsxx: Drop PCI legacy " Bjorn Helgaas
@ 2021-12-14 13:59 ` Jens Axboe
4 siblings, 0 replies; 8+ messages in thread
From: Jens Axboe @ 2021-12-14 13:59 UTC (permalink / raw)
To: Philip Kelleher, Bjorn Helgaas, Joshua Morris
Cc: linux-pci, Shuah Khan, linux-block, Rafael J . Wysocki,
Vaibhav Gupta, Vaibhav Gupta, linux-kernel, linux-kernel-mentees,
Bjorn Helgaas
On Wed, 8 Dec 2021 13:24:45 -0600, Bjorn Helgaas wrote:
> From: Bjorn Helgaas <bhelgaas@google.com>
>
> This is a repost of patches from Vaibhav to convert from legacy PCI power
> management to generic power management. The most recent posting is here:
>
> https://lore.kernel.org/all/20210114115423.52414-1-vaibhavgupta40@gmail.com/
>
> [...]
Applied, thanks!
[1/4] mtip32xx: remove pointless drvdata checking
commit: 2920417c98dbe4b58200c12fc9dc152834b76e42
[2/4] mtip32xx: remove pointless drvdata lookups
commit: 9e541f142dab67264075baaf8fd2eb4423742c16
[3/4] mtip32xx: convert to generic power management
commit: cd97b7e0d78009b45e08b92441d9562f9f37968c
[4/4] rsxx: Drop PCI legacy power management
commit: ac6f6548fcb3c6da8ff1653a16c66fc1719a2a3e
Best regards,
--
Jens Axboe
^ permalink raw reply [flat|nested] 8+ messages in thread