Message ID | 1321475279-29930-1-git-send-email-computersforpeace@gmail.com |
---|---|
State | Not Applicable |
Delegated to: | David Miller |
Headers | show |
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 ?
+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 -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 ? :-)
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;
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(-)