diff mbox

[3/3] spi: attach/detach SPI device to the ACPI power domain

Message ID 1381327461-10562-4-git-send-email-mika.westerberg@linux.intel.com
State Not Applicable
Headers show

Commit Message

Mika Westerberg Oct. 9, 2013, 2:04 p.m. UTC
If the SPI device is enumerated from ACPI namespace (it has an ACPI handle)
it might have ACPI methods that needs to be called in order to transition
the device to different power states (such as _PSx).

We follow what has been done for platform and I2C buses here and attach the
SPI device to the ACPI power domain if the device has an ACPI handle. This
makes sure that the device is powered on when its ->probe() is called.

For non-ACPI devices this patch is a no-op.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 drivers/spi/spi.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

Comments

Mark Brown Oct. 9, 2013, 5:55 p.m. UTC | #1
On Wed, Oct 09, 2013 at 05:04:21PM +0300, Mika Westerberg wrote:
> If the SPI device is enumerated from ACPI namespace (it has an ACPI handle)
> it might have ACPI methods that needs to be called in order to transition
> the device to different power states (such as _PSx).

Acked-by: Mark Brown <broonie@linaro.org>

> +	if (ACPI_HANDLE(&spi->dev))
> +		acpi_dev_pm_attach(&spi->dev, true);

Though I do wonder if it wouldn't be sensible to push the if () here
inside acpi_dev_pm_attach() and similarly for _detach().  Terribly
trivial either way.
Mika Westerberg Oct. 10, 2013, 6:12 a.m. UTC | #2
On Wed, Oct 09, 2013 at 06:55:28PM +0100, Mark Brown wrote:
> On Wed, Oct 09, 2013 at 05:04:21PM +0300, Mika Westerberg wrote:
> > If the SPI device is enumerated from ACPI namespace (it has an ACPI handle)
> > it might have ACPI methods that needs to be called in order to transition
> > the device to different power states (such as _PSx).
> 
> Acked-by: Mark Brown <broonie@linaro.org>

Thanks!

> > +	if (ACPI_HANDLE(&spi->dev))
> > +		acpi_dev_pm_attach(&spi->dev, true);
> 
> Though I do wonder if it wouldn't be sensible to push the if () here
> inside acpi_dev_pm_attach() and similarly for _detach().  Terribly
> trivial either way.

Actually, the check is already there in acpi_dev_pm_attach()/detach(). The
above code follows what Rafael did for platform bus previously. I think the
idea is to have visual hint that this is only for ACPI enumerated devices.

If preferred, I can drop the if() checks, though.
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mark Brown Oct. 10, 2013, 9:38 a.m. UTC | #3
On Thu, Oct 10, 2013 at 09:12:56AM +0300, Mika Westerberg wrote:
> On Wed, Oct 09, 2013 at 06:55:28PM +0100, Mark Brown wrote:
> > On Wed, Oct 09, 2013 at 05:04:21PM +0300, Mika Westerberg wrote:

> > > +	if (ACPI_HANDLE(&spi->dev))
> > > +		acpi_dev_pm_attach(&spi->dev, true);

> > Though I do wonder if it wouldn't be sensible to push the if () here
> > inside acpi_dev_pm_attach() and similarly for _detach().  Terribly
> > trivial either way.

> Actually, the check is already there in acpi_dev_pm_attach()/detach(). The
> above code follows what Rafael did for platform bus previously. I think the
> idea is to have visual hint that this is only for ACPI enumerated devices.

> If preferred, I can drop the if() checks, though.

It'd seem neater - the fact that the function is acpi_ ought to be
enough of a hint.
diff mbox

Patch

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 9e039c6..c522aa1 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -240,15 +240,29 @@  EXPORT_SYMBOL_GPL(spi_bus_type);
 static int spi_drv_probe(struct device *dev)
 {
 	const struct spi_driver		*sdrv = to_spi_driver(dev->driver);
+	struct spi_device		*spi = to_spi_device(dev);
+	int ret;
+
+	if (ACPI_HANDLE(&spi->dev))
+		acpi_dev_pm_attach(&spi->dev, true);
+	ret = sdrv->probe(spi);
+	if (ret && ACPI_HANDLE(&spi->dev))
+		acpi_dev_pm_detach(&spi->dev, true);
 
-	return sdrv->probe(to_spi_device(dev));
+	return ret;
 }
 
 static int spi_drv_remove(struct device *dev)
 {
 	const struct spi_driver		*sdrv = to_spi_driver(dev->driver);
+	struct spi_device		*spi = to_spi_device(dev);
+	int ret;
+
+	ret = sdrv->remove(spi);
+	if (ACPI_HANDLE(&spi->dev))
+		acpi_dev_pm_detach(&spi->dev, true);
 
-	return sdrv->remove(to_spi_device(dev));
+	return ret;
 }
 
 static void spi_drv_shutdown(struct device *dev)
@@ -1025,8 +1039,10 @@  static acpi_status acpi_spi_add_device(acpi_handle handle, u32 level,
 		return AE_OK;
 	}
 
+	adev->power.flags.ignore_parent = true;
 	strlcpy(spi->modalias, dev_name(&adev->dev), sizeof(spi->modalias));
 	if (spi_add_device(spi)) {
+		adev->power.flags.ignore_parent = false;
 		dev_err(&master->dev, "failed to add SPI device %s from ACPI\n",
 			dev_name(&adev->dev));
 		spi_dev_put(spi);