[PATCH] ahci: platform support for suspend/resume

[PATCH] ahci: platform support for suspend/resume

[Brian Norris]

Add platform hooks for custom suspend() and resume() functions. The
generic suspend/resume code in drivers/ata/ahci_platform.c is adapted
from the PCI version in drivers/ata/ahci.c.

Note that in order to suspend, we require that both suspend() and
resume() functions be supplied.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
Based on:
    git://github.com/jgarzik/libata-dev.git ALL

 drivers/ata/ahci_platform.c   |   72 +++++++++++++++++++++++++++++++++++++++++
 include/linux/ahci_platform.h |    2 +
 2 files changed, 74 insertions(+), 0 deletions(-)

diff --git a/drivers/ata/ahci_platform.c b/drivers/ata/ahci_platform.c
index ec55595..98bf5e9 100644
--- a/drivers/ata/ahci_platform.c
+++ b/drivers/ata/ahci_platform.c
@@ -202,6 +202,75 @@ static int __devexit ahci_remove(struct platform_device *pdev)
 	return 0;
 }
 
+#ifdef CONFIG_PM
+static int ahci_suspend(struct device *dev)
+{
+	struct ahci_platform_data *pdata = dev_get_platdata(dev);
+	struct ata_host *host = dev_get_drvdata(dev);
+	struct ahci_host_priv *hpriv = host->private_data;
+	void __iomem *mmio = hpriv->mmio;
+	u32 ctl;
+	int rc;
+
+	/* Does platform support suspend/resume? */
+	if (!pdata->suspend || !pdata->resume)
+		return -EINVAL;
+
+	if (hpriv->flags & AHCI_HFLAG_NO_SUSPEND) {
+		dev_err(dev, "firmware update required for suspend/resume\n");
+		return -EIO;
+	}
+
+	/*
+	 * 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 */
+
+	rc = ata_host_suspend(host, PMSG_SUSPEND);
+	if (rc)
+		return rc;
+
+	return pdata->suspend(dev);
+}
+
+static int ahci_resume(struct device *dev)
+{
+	struct ahci_platform_data *pdata = dev->platform_data;
+	struct ata_host *host = dev_get_drvdata(dev);
+	int rc;
+
+	/* Does platform support suspend/resume? */
+	if (!pdata->resume)
+		return -EINVAL;
+
+	rc = pdata->resume(dev);
+	if (rc)
+		return rc;
+
+	if (dev->power.power_state.event == PM_EVENT_SUSPEND) {
+		rc = ahci_reset_controller(host);
+		if (rc)
+			return rc;
+
+		ahci_init_controller(host);
+	}
+
+	ata_host_resume(host);
+
+	return 0;
+}
+
+static struct dev_pm_ops ahci_pm_ops = {
+	.suspend		= &ahci_suspend,
+	.resume			= &ahci_resume,
+};
+#endif
+
 static const struct of_device_id ahci_of_match[] = {
 	{ .compatible = "calxeda,hb-ahci", },
 	{},
@@ -214,6 +283,9 @@ static struct platform_driver ahci_driver = {
 		.name = "ahci",
 		.owner = THIS_MODULE,
 		.of_match_table = ahci_of_match,
+#ifdef CONFIG_PM
+		.pm = &ahci_pm_ops,
+#endif
 	},
 	.id_table	= ahci_devtype,
 };
diff --git a/include/linux/ahci_platform.h b/include/linux/ahci_platform.h
index be3d9a7..73a2500 100644
--- a/include/linux/ahci_platform.h
+++ b/include/linux/ahci_platform.h
@@ -23,6 +23,8 @@ struct ata_port_info;
 struct ahci_platform_data {
 	int (*init)(struct device *dev, void __iomem *addr);
 	void (*exit)(struct device *dev);
+	int (*suspend)(struct device *dev);
+	int (*resume)(struct device *dev);
 	const struct ata_port_info *ata_port_info;
 	unsigned int force_port_map;
 	unsigned int mask_port_map;
-- 
1.7.5.4

[PATCH] ahci: platform support for suspend/resume

[Brian Norris]

Add platform hooks for custom suspend() and resume() functions. The
generic suspend/resume code in drivers/ata/ahci_platform.c is adapted
from the PCI version in drivers/ata/ahci.c.

Note that in order to suspend, we require that both suspend() and
resume() functions be supplied.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
Based on:
    git://github.com/jgarzik/libata-dev.git ALL

 drivers/ata/ahci_platform.c   |   72 +++++++++++++++++++++++++++++++++++++++++
 include/linux/ahci_platform.h |    2 +
 2 files changed, 74 insertions(+), 0 deletions(-)

diff --git a/drivers/ata/ahci_platform.c b/drivers/ata/ahci_platform.c
index ec55595..98bf5e9 100644
--- a/drivers/ata/ahci_platform.c
+++ b/drivers/ata/ahci_platform.c
@@ -202,6 +202,75 @@ static int __devexit ahci_remove(struct platform_device *pdev)
 	return 0;
 }
 
+#ifdef CONFIG_PM
+static int ahci_suspend(struct device *dev)
+{
+	struct ahci_platform_data *pdata = dev_get_platdata(dev);
+	struct ata_host *host = dev_get_drvdata(dev);
+	struct ahci_host_priv *hpriv = host->private_data;
+	void __iomem *mmio = hpriv->mmio;
+	u32 ctl;
+	int rc;
+
+	/* Does platform support suspend/resume? */
+	if (!pdata->suspend || !pdata->resume)
+		return -EINVAL;
+
+	if (hpriv->flags & AHCI_HFLAG_NO_SUSPEND) {
+		dev_err(dev, "firmware update required for suspend/resume\n");
+		return -EIO;
+	}
+
+	/*
+	 * 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 */
+
+	rc = ata_host_suspend(host, PMSG_SUSPEND);
+	if (rc)
+		return rc;
+
+	return pdata->suspend(dev);
+}
+
+static int ahci_resume(struct device *dev)
+{
+	struct ahci_platform_data *pdata = dev->platform_data;
+	struct ata_host *host = dev_get_drvdata(dev);
+	int rc;
+
+	/* Does platform support suspend/resume? */
+	if (!pdata->resume)
+		return -EINVAL;
+
+	rc = pdata->resume(dev);
+	if (rc)
+		return rc;
+
+	if (dev->power.power_state.event == PM_EVENT_SUSPEND) {
+		rc = ahci_reset_controller(host);
+		if (rc)
+			return rc;
+
+		ahci_init_controller(host);
+	}
+
+	ata_host_resume(host);
+
+	return 0;
+}
+
+static struct dev_pm_ops ahci_pm_ops = {
+	.suspend		= &ahci_suspend,
+	.resume			= &ahci_resume,
+};
+#endif
+
 static const struct of_device_id ahci_of_match[] = {
 	{ .compatible = "calxeda,hb-ahci", },
 	{},
@@ -214,6 +283,9 @@ static struct platform_driver ahci_driver = {
 		.name = "ahci",
 		.owner = THIS_MODULE,
 		.of_match_table = ahci_of_match,
+#ifdef CONFIG_PM
+		.pm = &ahci_pm_ops,
+#endif
 	},
 	.id_table	= ahci_devtype,
 };
diff --git a/include/linux/ahci_platform.h b/include/linux/ahci_platform.h
index be3d9a7..73a2500 100644
--- a/include/linux/ahci_platform.h
+++ b/include/linux/ahci_platform.h
@@ -23,6 +23,8 @@ struct ata_port_info;
 struct ahci_platform_data {
 	int (*init)(struct device *dev, void __iomem *addr);
 	void (*exit)(struct device *dev);
+	int (*suspend)(struct device *dev);
+	int (*resume)(struct device *dev);
 	const struct ata_port_info *ata_port_info;
 	unsigned int force_port_map;
 	unsigned int mask_port_map;
-- 
1.7.5.4

Re: [PATCH] ahci: platform support for suspend/resume

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 1732 bytes --]

On Wed, Nov 16, 2011 at 12:27:59PM -0800, Brian Norris wrote:
> Add platform hooks for custom suspend() and resume() functions. The
> generic suspend/resume code in drivers/ata/ahci_platform.c is adapted
> from the PCI version in drivers/ata/ahci.c.
> 
> Note that in order to suspend, we require that both suspend() and
> resume() functions be supplied.
> 
> Signed-off-by: Brian Norris <computersforpeace@gmail.com>
> ---
> Based on:
>     git://github.com/jgarzik/libata-dev.git ALL
> 
>  drivers/ata/ahci_platform.c   |   72 +++++++++++++++++++++++++++++++++++++++++
>  include/linux/ahci_platform.h |    2 +
>  2 files changed, 74 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/ata/ahci_platform.c b/drivers/ata/ahci_platform.c
> index ec55595..98bf5e9 100644
> --- a/drivers/ata/ahci_platform.c
> +++ b/drivers/ata/ahci_platform.c
> @@ -202,6 +202,75 @@ static int __devexit ahci_remove(struct platform_device *pdev)
>  	return 0;
>  }
>  
> +#ifdef CONFIG_PM
> +static int ahci_suspend(struct device *dev)
> +{
> +	struct ahci_platform_data *pdata = dev_get_platdata(dev);
> +	struct ata_host *host = dev_get_drvdata(dev);
> +	struct ahci_host_priv *hpriv = host->private_data;
> +	void __iomem *mmio = hpriv->mmio;
> +	u32 ctl;
> +	int rc;
> +
> +	/* Does platform support suspend/resume? */
> +	if (!pdata->suspend || !pdata->resume)
> +		return -EINVAL;

should you really prevent suspend is platform doesn't provide
->suspend() ?

I mean, we could some platform where all clocks are autogated and
there's no need for platform to do anything. Would it be better to only
call pdata->suspend if it exists but still let the AHCI-specific part go
through ?

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] ahci: platform support for suspend/resume

[Brian Norris]

+linux-pm mailing list

Hi Felipe,

On Wed, Nov 16, 2011 at 12:34 PM, Felipe Balbi <balbi@ti.com> wrote:
> On Wed, Nov 16, 2011 at 12:27:59PM -0800, Brian Norris wrote:
>> +     /* Does platform support suspend/resume? */
>> +     if (!pdata->suspend || !pdata->resume)
>> +             return -EINVAL;
>
> should you really prevent suspend is platform doesn't provide
> ->suspend() ?

No, we shouldn't prevent suspend entirely; that would just be my
accidental side effect.

> I mean, we could some platform where all clocks are autogated and
> there's no need for platform to do anything. Would it be better to only
> call pdata->suspend if it exists but still let the AHCI-specific part go
> through ?

Perhaps. But would this have any unintended effects on systems that
don't implement AHCI power management at all? My intention was just to
provide an 'opt-in' interface for platform developers. I probably got
this wrong.

Brian

Re: [PATCH] ahci: platform support for suspend/resume

[Brian Norris]

+linux-pm mailing list

Hi Felipe,

On Wed, Nov 16, 2011 at 12:34 PM, Felipe Balbi <balbi@ti.com> wrote:
> On Wed, Nov 16, 2011 at 12:27:59PM -0800, Brian Norris wrote:
>> +     /* Does platform support suspend/resume? */
>> +     if (!pdata->suspend || !pdata->resume)
>> +             return -EINVAL;
>
> should you really prevent suspend is platform doesn't provide
> ->suspend() ?

No, we shouldn't prevent suspend entirely; that would just be my
accidental side effect.

> I mean, we could some platform where all clocks are autogated and
> there's no need for platform to do anything. Would it be better to only
> call pdata->suspend if it exists but still let the AHCI-specific part go
> through ?

Perhaps. But would this have any unintended effects on systems that
don't implement AHCI power management at all? My intention was just to
provide an 'opt-in' interface for platform developers. I probably got
this wrong.

Brian

Re: [PATCH] ahci: platform support for suspend/resume

[Felipe Balbi]

[-- Attachment #1.1: Type: text/plain, Size: 1218 bytes --]

On Thu, Nov 17, 2011 at 04:31:32PM -0800, Brian Norris wrote:
> +linux-pm mailing list
> 
> Hi Felipe,
> 
> On Wed, Nov 16, 2011 at 12:34 PM, Felipe Balbi <balbi@ti.com> wrote:
> > On Wed, Nov 16, 2011 at 12:27:59PM -0800, Brian Norris wrote:
> >> +     /* Does platform support suspend/resume? */
> >> +     if (!pdata->suspend || !pdata->resume)
> >> +             return -EINVAL;
> >
> > should you really prevent suspend is platform doesn't provide
> > ->suspend() ?
> 
> No, we shouldn't prevent suspend entirely; that would just be my
> accidental side effect.
> 
> > I mean, we could some platform where all clocks are autogated and
> > there's no need for platform to do anything. Would it be better to only
> > call pdata->suspend if it exists but still let the AHCI-specific part go
> > through ?
> 
> Perhaps. But would this have any unintended effects on systems that
> don't implement AHCI power management at all? My intention was just to
> provide an 'opt-in' interface for platform developers. I probably got
> this wrong.

I guess you could add a flag for that, but until we know that the AHCI
powermanagement is broken somewhere should we really care ? :-)

-- 
balbi

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH] ahci: platform support for suspend/resume

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 1218 bytes --]

On Thu, Nov 17, 2011 at 04:31:32PM -0800, Brian Norris wrote:
> +linux-pm mailing list
> 
> Hi Felipe,
> 
> On Wed, Nov 16, 2011 at 12:34 PM, Felipe Balbi <balbi@ti.com> wrote:
> > On Wed, Nov 16, 2011 at 12:27:59PM -0800, Brian Norris wrote:
> >> +     /* Does platform support suspend/resume? */
> >> +     if (!pdata->suspend || !pdata->resume)
> >> +             return -EINVAL;
> >
> > should you really prevent suspend is platform doesn't provide
> > ->suspend() ?
> 
> No, we shouldn't prevent suspend entirely; that would just be my
> accidental side effect.
> 
> > I mean, we could some platform where all clocks are autogated and
> > there's no need for platform to do anything. Would it be better to only
> > call pdata->suspend if it exists but still let the AHCI-specific part go
> > through ?
> 
> Perhaps. But would this have any unintended effects on systems that
> don't implement AHCI power management at all? My intention was just to
> provide an 'opt-in' interface for platform developers. I probably got
> this wrong.

I guess you could add a flag for that, but until we know that the AHCI
powermanagement is broken somewhere should we really care ? :-)

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]