Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1337164 > unrolled thread

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

Started byMika Westerberg <mika.westerberg@linux.intel.com>
First post2016-02-18 10:00 +0100
Last post2016-02-18 15:50 +0100
Articles 4 — 4 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  [PATCH 5/7] ahci: Convert driver to use modern PM hooks Mika Westerberg <mika.westerberg@linux.intel.com> - 2016-02-18 10:00 +0100
    Re: [PATCH 5/7] ahci: Convert driver to use modern PM hooks Andy Shevchenko <andy.shevchenko@gmail.com> - 2016-02-18 11:50 +0100
      Re: [PATCH 5/7] ahci: Convert driver to use modern PM hooks Tejun Heo <tj@kernel.org> - 2016-02-18 14:20 +0100
        Re: [PATCH 5/7] ahci: Convert driver to use modern PM hooks Christoph Hellwig <hch@infradead.org> - 2016-02-18 15:50 +0100

#1337164 — [PATCH 5/7] ahci: Convert driver to use modern PM hooks

FromMika Westerberg <mika.westerberg@linux.intel.com>
Date2016-02-18 10:00 +0100
Subject[PATCH 5/7] ahci: Convert driver to use modern PM hooks
Message-ID<r3sBk-311-5@gated-at.bofh.it>
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(-)

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);
-- 
2.7.0

[toc] | [next] | [standalone]


#1337259

FromAndy Shevchenko <andy.shevchenko@gmail.com>
Date2016-02-18 11:50 +0100
Message-ID<r3ujL-4dh-3@gated-at.bofh.it>
In reply to#1337164
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



-- 
With Best Regards,
Andy Shevchenko

[toc] | [prev] | [next] | [standalone]


#1337349

FromTejun Heo <tj@kernel.org>
Date2016-02-18 14:20 +0100
Message-ID<r3wEX-626-19@gated-at.bofh.it>
In reply to#1337259
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.

-- 
tejun

[toc] | [prev] | [next] | [standalone]


#1337429

FromChristoph Hellwig <hch@infradead.org>
Date2016-02-18 15:50 +0100
Message-ID<r3y42-74z-15@gated-at.bofh.it>
In reply to#1337349
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,
	},

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web