All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v5 0/4] block: convert to generic power management
@ 2021-12-08 19:24 ` Bjorn Helgaas
  0 siblings, 0 replies; 16+ 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>

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/

Vaibhav has converted around 180 drivers to generic power management, and
over 100 of those conversions have made it upstream.  If we can finish off
the remaining ones, we'll be able to remove quite a bit of ugly legacy code
from the PCI core.

I updated the commit logs to try to make it easier to verify these.

I also added a couple trivial mtip32xx cleanup patches in the same area and
changed the rsxx patch to completely drop the PM ops since the driver
doesn't support PM at all.

Bjorn Helgaas (3):
  mtip32xx: remove pointless drvdata checking
  mtip32xx: remove pointless drvdata lookups
  rsxx: Drop PCI legacy power management

Vaibhav Gupta (1):
  mtip32xx: convert to generic power management

 drivers/block/mtip32xx/mtip32xx.c | 86 ++++++++-----------------------
 drivers/block/rsxx/core.c         |  7 ---
 2 files changed, 22 insertions(+), 71 deletions(-)

-- 
2.25.1


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

* [PATCH v5 0/4] block: convert to generic power management
@ 2021-12-08 19:24 ` Bjorn Helgaas
  0 siblings, 0 replies; 16+ messages in thread
From: Bjorn Helgaas @ 2021-12-08 19:24 UTC (permalink / raw)
  To: Jens Axboe, Joshua Morris, Philip Kelleher
  Cc: Vaibhav Gupta, Rafael J . Wysocki, linux-kernel, linux-block,
	linux-pci, Bjorn Helgaas, linux-kernel-mentees

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/

Vaibhav has converted around 180 drivers to generic power management, and
over 100 of those conversions have made it upstream.  If we can finish off
the remaining ones, we'll be able to remove quite a bit of ugly legacy code
from the PCI core.

I updated the commit logs to try to make it easier to verify these.

I also added a couple trivial mtip32xx cleanup patches in the same area and
changed the rsxx patch to completely drop the PM ops since the driver
doesn't support PM at all.

Bjorn Helgaas (3):
  mtip32xx: remove pointless drvdata checking
  mtip32xx: remove pointless drvdata lookups
  rsxx: Drop PCI legacy power management

Vaibhav Gupta (1):
  mtip32xx: convert to generic power management

 drivers/block/mtip32xx/mtip32xx.c | 86 ++++++++-----------------------
 drivers/block/rsxx/core.c         |  7 ---
 2 files changed, 22 insertions(+), 71 deletions(-)

-- 
2.25.1

_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

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

* [PATCH v5 1/4] mtip32xx: remove pointless drvdata checking
  2021-12-08 19:24 ` Bjorn Helgaas
@ 2021-12-08 19:24   ` Bjorn Helgaas
  -1 siblings, 0 replies; 16+ 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] 16+ messages in thread

* [PATCH v5 1/4] mtip32xx: remove pointless drvdata checking
@ 2021-12-08 19:24   ` Bjorn Helgaas
  0 siblings, 0 replies; 16+ messages in thread
From: Bjorn Helgaas @ 2021-12-08 19:24 UTC (permalink / raw)
  To: Jens Axboe, Joshua Morris, Philip Kelleher
  Cc: Vaibhav Gupta, Rafael J . Wysocki, linux-kernel, linux-block,
	linux-pci, Bjorn Helgaas, linux-kernel-mentees

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

_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

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

* [PATCH v5 2/4] mtip32xx: remove pointless drvdata lookups
  2021-12-08 19:24 ` Bjorn Helgaas
@ 2021-12-08 19:24   ` Bjorn Helgaas
  -1 siblings, 0 replies; 16+ 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] 16+ messages in thread

* [PATCH v5 2/4] mtip32xx: remove pointless drvdata lookups
@ 2021-12-08 19:24   ` Bjorn Helgaas
  0 siblings, 0 replies; 16+ messages in thread
From: Bjorn Helgaas @ 2021-12-08 19:24 UTC (permalink / raw)
  To: Jens Axboe, Joshua Morris, Philip Kelleher
  Cc: Vaibhav Gupta, Rafael J . Wysocki, linux-kernel, linux-block,
	linux-pci, Bjorn Helgaas, linux-kernel-mentees

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

_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

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

* [PATCH v5 3/4] mtip32xx: convert to generic power management
  2021-12-08 19:24 ` Bjorn Helgaas
@ 2021-12-08 19:24   ` Bjorn Helgaas
  -1 siblings, 0 replies; 16+ 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] 16+ messages in thread

* [PATCH v5 3/4] mtip32xx: convert to generic power management
@ 2021-12-08 19:24   ` Bjorn Helgaas
  0 siblings, 0 replies; 16+ messages in thread
From: Bjorn Helgaas @ 2021-12-08 19:24 UTC (permalink / raw)
  To: Jens Axboe, Joshua Morris, Philip Kelleher
  Cc: Vaibhav Gupta, Rafael J . Wysocki, linux-kernel, linux-block,
	linux-pci, Bjorn Helgaas, linux-kernel-mentees

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

_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

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

* [PATCH v5 4/4] rsxx: Drop PCI legacy power management
  2021-12-08 19:24 ` Bjorn Helgaas
@ 2021-12-08 19:24   ` Bjorn Helgaas
  -1 siblings, 0 replies; 16+ 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] 16+ messages in thread

* [PATCH v5 4/4] rsxx: Drop PCI legacy power management
@ 2021-12-08 19:24   ` Bjorn Helgaas
  0 siblings, 0 replies; 16+ messages in thread
From: Bjorn Helgaas @ 2021-12-08 19:24 UTC (permalink / raw)
  To: Jens Axboe, Joshua Morris, Philip Kelleher
  Cc: Vaibhav Gupta, Rafael J . Wysocki, linux-kernel, linux-block,
	linux-pci, Bjorn Helgaas, linux-kernel-mentees

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

_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

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

* Re: [PATCH v5 4/4] rsxx: Drop PCI legacy power management
  2021-12-08 19:24   ` Bjorn Helgaas
@ 2021-12-14  7:55     ` Christoph Hellwig
  -1 siblings, 0 replies; 16+ 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] 16+ messages in thread

* Re: [PATCH v5 4/4] rsxx: Drop PCI legacy power management
@ 2021-12-14  7:55     ` Christoph Hellwig
  0 siblings, 0 replies; 16+ messages in thread
From: Christoph Hellwig @ 2021-12-14  7:55 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Jens Axboe, Vaibhav Gupta, Rafael J . Wysocki, linux-kernel,
	linux-block, linux-pci, Bjorn Helgaas, Philip Kelleher,
	linux-kernel-mentees, Joshua Morris

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.
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

^ permalink raw reply	[flat|nested] 16+ 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
  -1 siblings, 0 replies; 16+ 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] 16+ messages in thread

* Re: [PATCH v5 4/4] rsxx: Drop PCI legacy power management
@ 2021-12-14 13:59       ` Jens Axboe
  0 siblings, 0 replies; 16+ messages in thread
From: Jens Axboe @ 2021-12-14 13:59 UTC (permalink / raw)
  To: Christoph Hellwig, Bjorn Helgaas
  Cc: Vaibhav Gupta, Rafael J . Wysocki, linux-kernel, linux-block,
	linux-pci, Bjorn Helgaas, Philip Kelleher, linux-kernel-mentees,
	Joshua Morris

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

_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

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

* Re: [PATCH v5 0/4] block: convert to generic power management
  2021-12-08 19:24 ` Bjorn Helgaas
@ 2021-12-14 13:59   ` Jens Axboe
  -1 siblings, 0 replies; 16+ 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] 16+ messages in thread

* Re: [PATCH v5 0/4] block: convert to generic power management
@ 2021-12-14 13:59   ` Jens Axboe
  0 siblings, 0 replies; 16+ messages in thread
From: Jens Axboe @ 2021-12-14 13:59 UTC (permalink / raw)
  To: Philip Kelleher, Bjorn Helgaas, Joshua Morris
  Cc: Rafael J . Wysocki, linux-pci, linux-kernel, linux-block,
	Vaibhav Gupta, Bjorn Helgaas, linux-kernel-mentees

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


_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

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

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

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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 1/4] mtip32xx: remove pointless drvdata checking 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
2021-12-08 19:24   ` Bjorn Helgaas
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-08 19:24 ` [PATCH v5 4/4] rsxx: Drop PCI legacy " Bjorn Helgaas
2021-12-08 19:24   ` Bjorn Helgaas
2021-12-14  7:55   ` Christoph Hellwig
2021-12-14  7:55     ` Christoph Hellwig
2021-12-14 13:59     ` Jens Axboe
2021-12-14 13:59       ` Jens Axboe
2021-12-14 13:59 ` [PATCH v5 0/4] block: convert to generic " Jens Axboe
2021-12-14 13:59   ` Jens Axboe

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.