diff mbox

[5/7] ahci: Convert driver to use modern PM hooks

Message ID 1455785657-22924-6-git-send-email-mika.westerberg@linux.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Mika Westerberg Feb. 18, 2016, 8:54 a.m. UTC
In order to add support for runtime PM to the ahci driver we first need to
convert the driver to use modern non-legacy system suspend hooks. There
should be no functional changes.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 drivers/ata/ahci.c | 49 ++++++++++++++++++++++---------------------------
 1 file changed, 22 insertions(+), 27 deletions(-)

Comments

Andy Shevchenko Feb. 18, 2016, 10:45 a.m. UTC | #1
On Thu, Feb 18, 2016 at 10:54 AM, Mika Westerberg
<mika.westerberg@linux.intel.com> wrote:
> In order to add support for runtime PM to the ahci driver we first need to
> convert the driver to use modern non-legacy system suspend hooks. There
> should be no functional changes.
>

One comment below, otherwise:
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> ---
>  drivers/ata/ahci.c | 49 ++++++++++++++++++++++---------------------------
>  1 file changed, 22 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
> index 546a3692774f..44f9d1c09bbe 100644
> --- a/drivers/ata/ahci.c
> +++ b/drivers/ata/ahci.c
> @@ -93,9 +93,9 @@ static void ahci_mcp89_apple_enable(struct pci_dev *pdev);
>  static bool is_mcp89_apple(struct pci_dev *pdev);
>  static int ahci_p5wdh_hardreset(struct ata_link *link, unsigned int *class,
>                                 unsigned long deadline);
> -#ifdef CONFIG_PM
> -static int ahci_pci_device_suspend(struct pci_dev *pdev, pm_message_t mesg);
> -static int ahci_pci_device_resume(struct pci_dev *pdev);
> +#ifdef CONFIG_PM_SLEEP
> +static int ahci_pci_device_suspend(struct device *dev);
> +static int ahci_pci_device_resume(struct device *dev);
>  #endif
>
>  static struct scsi_host_template ahci_sht = {
> @@ -557,16 +557,16 @@ static const struct pci_device_id ahci_pci_tbl[] = {
>         { }     /* terminate list */
>  };
>
> +static const struct dev_pm_ops ahci_pci_pm_ops = {
> +       SET_SYSTEM_SLEEP_PM_OPS(ahci_pci_device_suspend, ahci_pci_device_resume)
> +};
>
>  static struct pci_driver ahci_pci_driver = {
>         .name                   = DRV_NAME,
>         .id_table               = ahci_pci_tbl,
>         .probe                  = ahci_init_one,
>         .remove                 = ata_pci_remove_one,
> -#ifdef CONFIG_PM
> -       .suspend                = ahci_pci_device_suspend,
> -       .resume                 = ahci_pci_device_resume,
> -#endif
> +       .driver.pm              = &ahci_pci_pm_ops,

Please, do this in several assignments, since to support older compilers.

>  };
>
>  #if defined(CONFIG_PATA_MARVELL) || defined(CONFIG_PATA_MARVELL_MODULE)
> @@ -794,44 +794,39 @@ static int ahci_avn_hardreset(struct ata_link *link, unsigned int *class,
>  }
>
>
> -#ifdef CONFIG_PM
> -static int ahci_pci_device_suspend(struct pci_dev *pdev, pm_message_t mesg)
> +#ifdef CONFIG_PM_SLEEP
> +static int ahci_pci_device_suspend(struct device *dev)
>  {
> +       struct pci_dev *pdev = to_pci_dev(dev);
>         struct ata_host *host = pci_get_drvdata(pdev);
>         struct ahci_host_priv *hpriv = host->private_data;
>         void __iomem *mmio = hpriv->mmio;
>         u32 ctl;
>
> -       if (mesg.event & PM_EVENT_SUSPEND &&
> -           hpriv->flags & AHCI_HFLAG_NO_SUSPEND) {
> +       if (hpriv->flags & AHCI_HFLAG_NO_SUSPEND) {
>                 dev_err(&pdev->dev,
>                         "BIOS update required for suspend/resume\n");
>                 return -EIO;
>         }
>
> -       if (mesg.event & PM_EVENT_SLEEP) {
> -               /* AHCI spec rev1.1 section 8.3.3:
> -                * Software must disable interrupts prior to requesting a
> -                * transition of the HBA to D3 state.
> -                */
> -               ctl = readl(mmio + HOST_CTL);
> -               ctl &= ~HOST_IRQ_EN;
> -               writel(ctl, mmio + HOST_CTL);
> -               readl(mmio + HOST_CTL); /* flush */
> -       }
> +       /* AHCI spec rev1.1 section 8.3.3:
> +        * Software must disable interrupts prior to requesting a
> +        * transition of the HBA to D3 state.
> +        */
> +       ctl = readl(mmio + HOST_CTL);
> +       ctl &= ~HOST_IRQ_EN;
> +       writel(ctl, mmio + HOST_CTL);
> +       readl(mmio + HOST_CTL); /* flush */
>
> -       return ata_pci_device_suspend(pdev, mesg);
> +       return ata_host_suspend(host, PMSG_SUSPEND);
>  }
>
> -static int ahci_pci_device_resume(struct pci_dev *pdev)
> +static int ahci_pci_device_resume(struct device *dev)
>  {
> +       struct pci_dev *pdev = to_pci_dev(dev);
>         struct ata_host *host = pci_get_drvdata(pdev);
>         int rc;
>
> -       rc = ata_pci_device_do_resume(pdev);
> -       if (rc)
> -               return rc;
> -
>         /* Apple BIOS helpfully mangles the registers on resume */
>         if (is_mcp89_apple(pdev))
>                 ahci_mcp89_apple_enable(pdev);
> --
> 2.7.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tejun Heo Feb. 18, 2016, 1:12 p.m. UTC | #2
On Thu, Feb 18, 2016 at 12:45:05PM +0200, Andy Shevchenko wrote:
> > +       .driver.pm              = &ahci_pci_pm_ops,
> 
> Please, do this in several assignments, since to support older compilers.

Can you please elaborate?  That's a single assignment.  Are there
compilers which can't do that?

Thanks.
Christoph Hellwig Feb. 18, 2016, 2:41 p.m. UTC | #3
On Thu, Feb 18, 2016 at 08:12:46AM -0500, Tejun Heo wrote:
> On Thu, Feb 18, 2016 at 12:45:05PM +0200, Andy Shevchenko wrote:
> > > +       .driver.pm              = &ahci_pci_pm_ops,
> > 
> > Please, do this in several assignments, since to support older compilers.
> 
> Can you please elaborate?  That's a single assignment.  Are there
> compilers which can't do that?

Old gcc (I think 4.4 is the last one) needs:

	.driver = {
		.pm		= &ahci_pci_pm_ops,
	},
--
To unsubscribe from this list: send the line "unsubscribe linux-block" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
index 546a3692774f..44f9d1c09bbe 100644
--- a/drivers/ata/ahci.c
+++ b/drivers/ata/ahci.c
@@ -93,9 +93,9 @@  static void ahci_mcp89_apple_enable(struct pci_dev *pdev);
 static bool is_mcp89_apple(struct pci_dev *pdev);
 static int ahci_p5wdh_hardreset(struct ata_link *link, unsigned int *class,
 				unsigned long deadline);
-#ifdef CONFIG_PM
-static int ahci_pci_device_suspend(struct pci_dev *pdev, pm_message_t mesg);
-static int ahci_pci_device_resume(struct pci_dev *pdev);
+#ifdef CONFIG_PM_SLEEP
+static int ahci_pci_device_suspend(struct device *dev);
+static int ahci_pci_device_resume(struct device *dev);
 #endif
 
 static struct scsi_host_template ahci_sht = {
@@ -557,16 +557,16 @@  static const struct pci_device_id ahci_pci_tbl[] = {
 	{ }	/* terminate list */
 };
 
+static const struct dev_pm_ops ahci_pci_pm_ops = {
+	SET_SYSTEM_SLEEP_PM_OPS(ahci_pci_device_suspend, ahci_pci_device_resume)
+};
 
 static struct pci_driver ahci_pci_driver = {
 	.name			= DRV_NAME,
 	.id_table		= ahci_pci_tbl,
 	.probe			= ahci_init_one,
 	.remove			= ata_pci_remove_one,
-#ifdef CONFIG_PM
-	.suspend		= ahci_pci_device_suspend,
-	.resume			= ahci_pci_device_resume,
-#endif
+	.driver.pm		= &ahci_pci_pm_ops,
 };
 
 #if defined(CONFIG_PATA_MARVELL) || defined(CONFIG_PATA_MARVELL_MODULE)
@@ -794,44 +794,39 @@  static int ahci_avn_hardreset(struct ata_link *link, unsigned int *class,
 }
 
 
-#ifdef CONFIG_PM
-static int ahci_pci_device_suspend(struct pci_dev *pdev, pm_message_t mesg)
+#ifdef CONFIG_PM_SLEEP
+static int ahci_pci_device_suspend(struct device *dev)
 {
+	struct pci_dev *pdev = to_pci_dev(dev);
 	struct ata_host *host = pci_get_drvdata(pdev);
 	struct ahci_host_priv *hpriv = host->private_data;
 	void __iomem *mmio = hpriv->mmio;
 	u32 ctl;
 
-	if (mesg.event & PM_EVENT_SUSPEND &&
-	    hpriv->flags & AHCI_HFLAG_NO_SUSPEND) {
+	if (hpriv->flags & AHCI_HFLAG_NO_SUSPEND) {
 		dev_err(&pdev->dev,
 			"BIOS update required for suspend/resume\n");
 		return -EIO;
 	}
 
-	if (mesg.event & PM_EVENT_SLEEP) {
-		/* AHCI spec rev1.1 section 8.3.3:
-		 * Software must disable interrupts prior to requesting a
-		 * transition of the HBA to D3 state.
-		 */
-		ctl = readl(mmio + HOST_CTL);
-		ctl &= ~HOST_IRQ_EN;
-		writel(ctl, mmio + HOST_CTL);
-		readl(mmio + HOST_CTL); /* flush */
-	}
+	/* AHCI spec rev1.1 section 8.3.3:
+	 * Software must disable interrupts prior to requesting a
+	 * transition of the HBA to D3 state.
+	 */
+	ctl = readl(mmio + HOST_CTL);
+	ctl &= ~HOST_IRQ_EN;
+	writel(ctl, mmio + HOST_CTL);
+	readl(mmio + HOST_CTL); /* flush */
 
-	return ata_pci_device_suspend(pdev, mesg);
+	return ata_host_suspend(host, PMSG_SUSPEND);
 }
 
-static int ahci_pci_device_resume(struct pci_dev *pdev)
+static int ahci_pci_device_resume(struct device *dev)
 {
+	struct pci_dev *pdev = to_pci_dev(dev);
 	struct ata_host *host = pci_get_drvdata(pdev);
 	int rc;
 
-	rc = ata_pci_device_do_resume(pdev);
-	if (rc)
-		return rc;
-
 	/* Apple BIOS helpfully mangles the registers on resume */
 	if (is_mcp89_apple(pdev))
 		ahci_mcp89_apple_enable(pdev);