[PATCH v3] runtime PM support for I2C adapter devices

[PATCH v3] runtime PM support for I2C adapter devices

[Mika Westerberg]

Hi,

This is third revision of the patch. The previous version can be found
here: 
	http://lwn.net/Articles/566234/

It was pointed out that the previous solution was not good for the existing
devices for several reasons:
 - The I2C adapter is powered on before client ->probe() is called. This
   causes problems with some devices.
 - The I2C adapter is kept powered on if any of its children are active.

However, for ACPI enumerated devices we need to have parent child
relationship so that the runtime PM core can power on the adapter device if
any of its children become active.

In this version we still enable runtime PM for the I2C adapter device but
only for devices that are enumerated from ACPI. The existing drivers should
continue to work as they do today.

I combined patches [1/9] and [2/9] from the previous version and added
checks for ACPI_HANDLE(). I hope Aaron and Lv are OK with this.

I didn't include changes to the SPI bus (the $subject is changed again to
reflect that) because we don't need to change the existing drivers anymore.
If this approach is accepted we can do the same for the SPI bus as well.

Mika Westerberg (1):
  i2c: enable runtime PM for I2C adapter devices enumerated from ACPI

 drivers/i2c/i2c-core.c | 47 ++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 46 insertions(+), 1 deletion(-)

-- 
1.8.4.rc3

[PATCH v3] i2c: enable runtime PM for I2C adapter devices enumerated from ACPI

[Mika Westerberg]

The ACPI specification requires the parent device to be powered on before
any of its children. It can be only powered off when all the children are
already off.

Currently whenever there is no I2C traffic going on, the I2C controller
driver can put the device into low power state transparently to its
children (the I2C client devices). This violates the ACPI specification
because now the parent device is in lower power state than its children.

In order to keep ACPI happy we enable runtime PM for the I2C adapter device
if we find out that the I2C controller was in fact an ACPI device. In
addition to that we attach the I2C client devices to the ACPI power domain
and make sure that they are powered on when the driver ->probe() is called.

Non-ACPI devices are not affected by this change.

This patch is based on the work by Aaron Lu and Lv Zheng.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 drivers/i2c/i2c-core.c | 47 ++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 46 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index 29d3f04..fa861ad 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -236,6 +236,27 @@ int i2c_recover_bus(struct i2c_adapter *adap)
 	return adap->bus_recovery_info->recover_bus(adap);
 }
 
+static void acpi_i2c_device_pm_get(struct i2c_client *client)
+{
+	struct i2c_adapter *adap = client->adapter;
+
+	/* Make sure the adapter is active */
+	if (ACPI_HANDLE(adap->dev.parent))
+		pm_runtime_get_sync(&adap->dev);
+	if (ACPI_HANDLE(&client->dev))
+		acpi_dev_pm_attach(&client->dev, true);
+}
+
+static void acpi_i2c_device_pm_put(struct i2c_client *client, bool detach)
+{
+	struct i2c_adapter *adap = client->adapter;
+
+	if (ACPI_HANDLE(&client->dev) && detach)
+		acpi_dev_pm_detach(&client->dev, true);
+	if (ACPI_HANDLE(adap->dev.parent))
+		pm_runtime_put(&adap->dev);
+}
+
 static int i2c_device_probe(struct device *dev)
 {
 	struct i2c_client	*client = i2c_verify_client(dev);
@@ -254,11 +275,15 @@ static int i2c_device_probe(struct device *dev)
 					client->flags & I2C_CLIENT_WAKE);
 	dev_dbg(dev, "probe\n");
 
+	acpi_i2c_device_pm_get(client);
+
 	status = driver->probe(client, i2c_match_id(driver->id_table, client));
 	if (status) {
 		client->driver = NULL;
 		i2c_set_clientdata(client, NULL);
 	}
+
+	acpi_i2c_device_pm_put(client, !!status);
 	return status;
 }
 
@@ -271,6 +296,8 @@ static int i2c_device_remove(struct device *dev)
 	if (!client || !dev->driver)
 		return 0;
 
+	acpi_i2c_device_pm_get(client);
+
 	driver = to_i2c_driver(dev->driver);
 	if (driver->remove) {
 		dev_dbg(dev, "remove\n");
@@ -283,6 +310,8 @@ static int i2c_device_remove(struct device *dev)
 		client->driver = NULL;
 		i2c_set_clientdata(client, NULL);
 	}
+
+	acpi_i2c_device_pm_put(client, true);
 	return status;
 }
 
@@ -294,8 +323,11 @@ static void i2c_device_shutdown(struct device *dev)
 	if (!client || !dev->driver)
 		return;
 	driver = to_i2c_driver(dev->driver);
-	if (driver->shutdown)
+	if (driver->shutdown) {
+		acpi_i2c_device_pm_get(client);
 		driver->shutdown(client);
+		acpi_i2c_device_pm_put(client, false);
+	}
 }
 
 #ifdef CONFIG_PM_SLEEP
@@ -1263,6 +1295,16 @@ exit_recovery:
 	bus_for_each_drv(&i2c_bus_type, NULL, adap, __process_new_adapter);
 	mutex_unlock(&core_lock);
 
+	/*
+	 * For ACPI enumerated controllers we must make sure that the
+	 * controller is powered on before its children. Runtime PM handles
+	 * this for us once we have enabled it for the adapter device.
+	 */
+	if (ACPI_HANDLE(adap->dev.parent)) {
+		pm_runtime_no_callbacks(&adap->dev);
+		pm_runtime_enable(&adap->dev);
+	}
+
 	return 0;
 
 out_list:
@@ -1427,6 +1469,9 @@ void i2c_del_adapter(struct i2c_adapter *adap)
 		return;
 	}
 
+	if (ACPI_HANDLE(adap->dev.parent))
+		pm_runtime_disable(&adap->dev);
+
 	/* Tell drivers about this removal */
 	mutex_lock(&core_lock);
 	bus_for_each_drv(&i2c_bus_type, NULL, adap,
-- 
1.8.4.rc3

[PATCH v3] i2c: enable runtime PM for I2C adapter devices enumerated from ACPI

[Rafael J. Wysocki]

On Monday, September 30, 2013 05:43:48 PM Mika Westerberg wrote:
> The ACPI specification requires the parent device to be powered on before
> any of its children. It can be only powered off when all the children are
> already off.
> 
> Currently whenever there is no I2C traffic going on, the I2C controller
> driver can put the device into low power state transparently to its
> children (the I2C client devices). This violates the ACPI specification
> because now the parent device is in lower power state than its children.
> 
> In order to keep ACPI happy we enable runtime PM for the I2C adapter device
> if we find out that the I2C controller was in fact an ACPI device. In
> addition to that we attach the I2C client devices to the ACPI power domain
> and make sure that they are powered on when the driver ->probe() is called.
> 
> Non-ACPI devices are not affected by this change.
> 
> This patch is based on the work by Aaron Lu and Lv Zheng.
> 
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> ---
>  drivers/i2c/i2c-core.c | 47 ++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 46 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
> index 29d3f04..fa861ad 100644
> --- a/drivers/i2c/i2c-core.c
> +++ b/drivers/i2c/i2c-core.c
> @@ -236,6 +236,27 @@ int i2c_recover_bus(struct i2c_adapter *adap)
>  	return adap->bus_recovery_info->recover_bus(adap);
>  }
>  
> +static void acpi_i2c_device_pm_get(struct i2c_client *client)
> +{
> +	struct i2c_adapter *adap = client->adapter;
> +
> +	/* Make sure the adapter is active */
> +	if (ACPI_HANDLE(adap->dev.parent))
> +		pm_runtime_get_sync(&adap->dev);
> +	if (ACPI_HANDLE(&client->dev))
> +		acpi_dev_pm_attach(&client->dev, true);

It would be sufficient to do

	if (ACPI_HANDLE(&client->dev)) {
		pm_runtime_get_sync(&adap->dev);
		acpi_dev_pm_attach(&client->dev, true);
	}

here (and below), because I don't think the client with an ACPI handle and the
parent without one is extremely unlikely (to the point of non-existence
actually ;-)).  And even if something like that happens, then we only enable
runtime PM for the adapter if the parent has an ACPI handle, so it still should
be OK.

Apart from this the patch looks good to me.

> +}
> +
> +static void acpi_i2c_device_pm_put(struct i2c_client *client, bool detach)
> +{
> +	struct i2c_adapter *adap = client->adapter;
> +
> +	if (ACPI_HANDLE(&client->dev) && detach)
> +		acpi_dev_pm_detach(&client->dev, true);
> +	if (ACPI_HANDLE(adap->dev.parent))
> +		pm_runtime_put(&adap->dev);
> +}
> +
>  static int i2c_device_probe(struct device *dev)
>  {
>  	struct i2c_client	*client = i2c_verify_client(dev);
> @@ -254,11 +275,15 @@ static int i2c_device_probe(struct device *dev)
>  					client->flags & I2C_CLIENT_WAKE);
>  	dev_dbg(dev, "probe\n");
>  
> +	acpi_i2c_device_pm_get(client);
> +
>  	status = driver->probe(client, i2c_match_id(driver->id_table, client));
>  	if (status) {
>  		client->driver = NULL;
>  		i2c_set_clientdata(client, NULL);
>  	}
> +
> +	acpi_i2c_device_pm_put(client, !!status);
>  	return status;
>  }
>  
> @@ -271,6 +296,8 @@ static int i2c_device_remove(struct device *dev)
>  	if (!client || !dev->driver)
>  		return 0;
>  
> +	acpi_i2c_device_pm_get(client);
> +
>  	driver = to_i2c_driver(dev->driver);
>  	if (driver->remove) {
>  		dev_dbg(dev, "remove\n");
> @@ -283,6 +310,8 @@ static int i2c_device_remove(struct device *dev)
>  		client->driver = NULL;
>  		i2c_set_clientdata(client, NULL);
>  	}
> +
> +	acpi_i2c_device_pm_put(client, true);
>  	return status;
>  }
>  
> @@ -294,8 +323,11 @@ static void i2c_device_shutdown(struct device *dev)
>  	if (!client || !dev->driver)
>  		return;
>  	driver = to_i2c_driver(dev->driver);
> -	if (driver->shutdown)
> +	if (driver->shutdown) {
> +		acpi_i2c_device_pm_get(client);
>  		driver->shutdown(client);
> +		acpi_i2c_device_pm_put(client, false);
> +	}
>  }
>  
>  #ifdef CONFIG_PM_SLEEP
> @@ -1263,6 +1295,16 @@ exit_recovery:
>  	bus_for_each_drv(&i2c_bus_type, NULL, adap, __process_new_adapter);
>  	mutex_unlock(&core_lock);
>  
> +	/*
> +	 * For ACPI enumerated controllers we must make sure that the
> +	 * controller is powered on before its children. Runtime PM handles
> +	 * this for us once we have enabled it for the adapter device.
> +	 */
> +	if (ACPI_HANDLE(adap->dev.parent)) {
> +		pm_runtime_no_callbacks(&adap->dev);
> +		pm_runtime_enable(&adap->dev);
> +	}
> +
>  	return 0;
>  
>  out_list:
> @@ -1427,6 +1469,9 @@ void i2c_del_adapter(struct i2c_adapter *adap)
>  		return;
>  	}
>  
> +	if (ACPI_HANDLE(adap->dev.parent))
> +		pm_runtime_disable(&adap->dev);
> +
>  	/* Tell drivers about this removal */
>  	mutex_lock(&core_lock);
>  	bus_for_each_drv(&i2c_bus_type, NULL, adap,
> 

Thanks!

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

[PATCH v3] i2c: enable runtime PM for I2C adapter devices enumerated from ACPI

[Rafael J. Wysocki]

On Monday, September 30, 2013 07:20:59 PM Rafael J. Wysocki wrote:
> On Monday, September 30, 2013 05:43:48 PM Mika Westerberg wrote:
> > The ACPI specification requires the parent device to be powered on before
> > any of its children. It can be only powered off when all the children are
> > already off.
> > 
> > Currently whenever there is no I2C traffic going on, the I2C controller
> > driver can put the device into low power state transparently to its
> > children (the I2C client devices). This violates the ACPI specification
> > because now the parent device is in lower power state than its children.
> > 
> > In order to keep ACPI happy we enable runtime PM for the I2C adapter device
> > if we find out that the I2C controller was in fact an ACPI device. In
> > addition to that we attach the I2C client devices to the ACPI power domain
> > and make sure that they are powered on when the driver ->probe() is called.
> > 
> > Non-ACPI devices are not affected by this change.
> > 
> > This patch is based on the work by Aaron Lu and Lv Zheng.
> > 
> > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> > ---
> >  drivers/i2c/i2c-core.c | 47 ++++++++++++++++++++++++++++++++++++++++++++++-
> >  1 file changed, 46 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
> > index 29d3f04..fa861ad 100644
> > --- a/drivers/i2c/i2c-core.c
> > +++ b/drivers/i2c/i2c-core.c
> > @@ -236,6 +236,27 @@ int i2c_recover_bus(struct i2c_adapter *adap)
> >  	return adap->bus_recovery_info->recover_bus(adap);
> >  }
> >  
> > +static void acpi_i2c_device_pm_get(struct i2c_client *client)
> > +{
> > +	struct i2c_adapter *adap = client->adapter;
> > +
> > +	/* Make sure the adapter is active */
> > +	if (ACPI_HANDLE(adap->dev.parent))
> > +		pm_runtime_get_sync(&adap->dev);
> > +	if (ACPI_HANDLE(&client->dev))
> > +		acpi_dev_pm_attach(&client->dev, true);
> 
> It would be sufficient to do
> 
> 	if (ACPI_HANDLE(&client->dev)) {
> 		pm_runtime_get_sync(&adap->dev);
> 		acpi_dev_pm_attach(&client->dev, true);
> 	}
> 
> here (and below), because I don't think the client with an ACPI handle and the

s/I don't think/I think/

> parent without one is extremely unlikely (to the point of non-existence
> actually ;-)).  And even if something like that happens, then we only enable
> runtime PM for the adapter if the parent has an ACPI handle, so it still should
> be OK.
> 
> Apart from this the patch looks good to me.
> 
> > +}
> > +
> > +static void acpi_i2c_device_pm_put(struct i2c_client *client, bool detach)
> > +{
> > +	struct i2c_adapter *adap = client->adapter;
> > +
> > +	if (ACPI_HANDLE(&client->dev) && detach)
> > +		acpi_dev_pm_detach(&client->dev, true);
> > +	if (ACPI_HANDLE(adap->dev.parent))
> > +		pm_runtime_put(&adap->dev);
> > +}
> > +
> >  static int i2c_device_probe(struct device *dev)
> >  {
> >  	struct i2c_client	*client = i2c_verify_client(dev);
> > @@ -254,11 +275,15 @@ static int i2c_device_probe(struct device *dev)
> >  					client->flags & I2C_CLIENT_WAKE);
> >  	dev_dbg(dev, "probe\n");
> >  
> > +	acpi_i2c_device_pm_get(client);
> > +
> >  	status = driver->probe(client, i2c_match_id(driver->id_table, client));
> >  	if (status) {
> >  		client->driver = NULL;
> >  		i2c_set_clientdata(client, NULL);
> >  	}
> > +
> > +	acpi_i2c_device_pm_put(client, !!status);
> >  	return status;
> >  }
> >  
> > @@ -271,6 +296,8 @@ static int i2c_device_remove(struct device *dev)
> >  	if (!client || !dev->driver)
> >  		return 0;
> >  
> > +	acpi_i2c_device_pm_get(client);
> > +
> >  	driver = to_i2c_driver(dev->driver);
> >  	if (driver->remove) {
> >  		dev_dbg(dev, "remove\n");
> > @@ -283,6 +310,8 @@ static int i2c_device_remove(struct device *dev)
> >  		client->driver = NULL;
> >  		i2c_set_clientdata(client, NULL);
> >  	}
> > +
> > +	acpi_i2c_device_pm_put(client, true);
> >  	return status;
> >  }
> >  
> > @@ -294,8 +323,11 @@ static void i2c_device_shutdown(struct device *dev)
> >  	if (!client || !dev->driver)
> >  		return;
> >  	driver = to_i2c_driver(dev->driver);
> > -	if (driver->shutdown)
> > +	if (driver->shutdown) {
> > +		acpi_i2c_device_pm_get(client);
> >  		driver->shutdown(client);
> > +		acpi_i2c_device_pm_put(client, false);
> > +	}
> >  }
> >  
> >  #ifdef CONFIG_PM_SLEEP
> > @@ -1263,6 +1295,16 @@ exit_recovery:
> >  	bus_for_each_drv(&i2c_bus_type, NULL, adap, __process_new_adapter);
> >  	mutex_unlock(&core_lock);
> >  
> > +	/*
> > +	 * For ACPI enumerated controllers we must make sure that the
> > +	 * controller is powered on before its children. Runtime PM handles
> > +	 * this for us once we have enabled it for the adapter device.
> > +	 */
> > +	if (ACPI_HANDLE(adap->dev.parent)) {
> > +		pm_runtime_no_callbacks(&adap->dev);
> > +		pm_runtime_enable(&adap->dev);
> > +	}
> > +
> >  	return 0;
> >  
> >  out_list:
> > @@ -1427,6 +1469,9 @@ void i2c_del_adapter(struct i2c_adapter *adap)
> >  		return;
> >  	}
> >  
> > +	if (ACPI_HANDLE(adap->dev.parent))
> > +		pm_runtime_disable(&adap->dev);
> > +
> >  	/* Tell drivers about this removal */
> >  	mutex_lock(&core_lock);
> >  	bus_for_each_drv(&i2c_bus_type, NULL, adap,
> > 
> 
> Thanks!
> 
> 
-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

[PATCH v4] i2c: enable runtime PM for I2C adapter devices enumerated from ACPI

[Mika Westerberg]

The ACPI specification requires the parent device to be powered on before
any of its children. It can be only powered off when all the children are
already off.

Currently whenever there is no I2C traffic going on, the I2C controller
driver can put the device into low power state transparently to its
children (the I2C client devices). This violates the ACPI specification
because now the parent device is in lower power state than its children.

In order to keep ACPI happy we enable runtime PM for the I2C adapter device
if we find out that the I2C controller was in fact an ACPI device. In
addition to that we attach the I2C client devices to the ACPI power domain
and make sure that they are powered on when the driver ->probe() is called.

Non-ACPI devices are not affected by this change.

This patch is based on the work by Aaron Lu and Lv Zheng.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
Changes to v3:
 * Check only client ACPI_HANDLE().
 * Fixed bug where we attach the device to ACPI power domain multiple
   times.
 * Functions are renamed to _get_attach/_put_attach to make it clear that
   we do attach/detach as well.

 drivers/i2c/i2c-core.c | 47 ++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 46 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index 29d3f04..e289b1d 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -236,6 +236,27 @@ int i2c_recover_bus(struct i2c_adapter *adap)
 	return adap->bus_recovery_info->recover_bus(adap);
 }
 
+static void acpi_i2c_device_pm_get_attach(struct i2c_client *client,
+					  bool attach)
+{
+	if (ACPI_HANDLE(&client->dev)) {
+		/* Make sure the adapter is active */
+		pm_runtime_get_sync(&client->adapter->dev);
+		if (attach)
+			acpi_dev_pm_attach(&client->dev, true);
+	}
+}
+
+static void acpi_i2c_device_pm_put_detach(struct i2c_client *client,
+					  bool detach)
+{
+	if (ACPI_HANDLE(&client->dev)) {
+		if (detach)
+			acpi_dev_pm_detach(&client->dev, true);
+		pm_runtime_put(&client->adapter->dev);
+	}
+}
+
 static int i2c_device_probe(struct device *dev)
 {
 	struct i2c_client	*client = i2c_verify_client(dev);
@@ -254,11 +275,15 @@ static int i2c_device_probe(struct device *dev)
 					client->flags & I2C_CLIENT_WAKE);
 	dev_dbg(dev, "probe\n");
 
+	acpi_i2c_device_pm_get_attach(client, true);
+
 	status = driver->probe(client, i2c_match_id(driver->id_table, client));
 	if (status) {
 		client->driver = NULL;
 		i2c_set_clientdata(client, NULL);
 	}
+
+	acpi_i2c_device_pm_put_detach(client, !!status);
 	return status;
 }
 
@@ -271,6 +296,8 @@ static int i2c_device_remove(struct device *dev)
 	if (!client || !dev->driver)
 		return 0;
 
+	acpi_i2c_device_pm_get_attach(client, false);
+
 	driver = to_i2c_driver(dev->driver);
 	if (driver->remove) {
 		dev_dbg(dev, "remove\n");
@@ -283,6 +310,8 @@ static int i2c_device_remove(struct device *dev)
 		client->driver = NULL;
 		i2c_set_clientdata(client, NULL);
 	}
+
+	acpi_i2c_device_pm_put_detach(client, true);
 	return status;
 }
 
@@ -294,8 +323,11 @@ static void i2c_device_shutdown(struct device *dev)
 	if (!client || !dev->driver)
 		return;
 	driver = to_i2c_driver(dev->driver);
-	if (driver->shutdown)
+	if (driver->shutdown) {
+		acpi_i2c_device_pm_get_attach(client, false);
 		driver->shutdown(client);
+		acpi_i2c_device_pm_put_detach(client, false);
+	}
 }
 
 #ifdef CONFIG_PM_SLEEP
@@ -1263,6 +1295,16 @@ exit_recovery:
 	bus_for_each_drv(&i2c_bus_type, NULL, adap, __process_new_adapter);
 	mutex_unlock(&core_lock);
 
+	/*
+	 * For ACPI enumerated controllers we must make sure that the
+	 * controller is powered on before its children. Runtime PM handles
+	 * this for us once we have enabled it for the adapter device.
+	 */
+	if (ACPI_HANDLE(adap->dev.parent)) {
+		pm_runtime_no_callbacks(&adap->dev);
+		pm_runtime_enable(&adap->dev);
+	}
+
 	return 0;
 
 out_list:
@@ -1427,6 +1469,9 @@ void i2c_del_adapter(struct i2c_adapter *adap)
 		return;
 	}
 
+	if (ACPI_HANDLE(adap->dev.parent))
+		pm_runtime_disable(&adap->dev);
+
 	/* Tell drivers about this removal */
 	mutex_lock(&core_lock);
 	bus_for_each_drv(&i2c_bus_type, NULL, adap,
-- 
1.8.4.rc3

[PATCH v4] i2c: enable runtime PM for I2C adapter devices enumerated from ACPI

[Rafael J. Wysocki]

On Tuesday, October 01, 2013 04:09:42 PM Mika Westerberg wrote:
> The ACPI specification requires the parent device to be powered on before
> any of its children. It can be only powered off when all the children are
> already off.
> 
> Currently whenever there is no I2C traffic going on, the I2C controller
> driver can put the device into low power state transparently to its
> children (the I2C client devices). This violates the ACPI specification
> because now the parent device is in lower power state than its children.
> 
> In order to keep ACPI happy we enable runtime PM for the I2C adapter device
> if we find out that the I2C controller was in fact an ACPI device. In
> addition to that we attach the I2C client devices to the ACPI power domain
> and make sure that they are powered on when the driver ->probe() is called.
> 
> Non-ACPI devices are not affected by this change.
> 
> This patch is based on the work by Aaron Lu and Lv Zheng.
> 
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>

Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

> ---
> Changes to v3:
>  * Check only client ACPI_HANDLE().
>  * Fixed bug where we attach the device to ACPI power domain multiple
>    times.
>  * Functions are renamed to _get_attach/_put_attach to make it clear that
>    we do attach/detach as well.
> 
>  drivers/i2c/i2c-core.c | 47 ++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 46 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
> index 29d3f04..e289b1d 100644
> --- a/drivers/i2c/i2c-core.c
> +++ b/drivers/i2c/i2c-core.c
> @@ -236,6 +236,27 @@ int i2c_recover_bus(struct i2c_adapter *adap)
>  	return adap->bus_recovery_info->recover_bus(adap);
>  }
>  
> +static void acpi_i2c_device_pm_get_attach(struct i2c_client *client,
> +					  bool attach)
> +{
> +	if (ACPI_HANDLE(&client->dev)) {
> +		/* Make sure the adapter is active */
> +		pm_runtime_get_sync(&client->adapter->dev);
> +		if (attach)
> +			acpi_dev_pm_attach(&client->dev, true);
> +	}
> +}
> +
> +static void acpi_i2c_device_pm_put_detach(struct i2c_client *client,
> +					  bool detach)
> +{
> +	if (ACPI_HANDLE(&client->dev)) {
> +		if (detach)
> +			acpi_dev_pm_detach(&client->dev, true);
> +		pm_runtime_put(&client->adapter->dev);
> +	}
> +}
> +
>  static int i2c_device_probe(struct device *dev)
>  {
>  	struct i2c_client	*client = i2c_verify_client(dev);
> @@ -254,11 +275,15 @@ static int i2c_device_probe(struct device *dev)
>  					client->flags & I2C_CLIENT_WAKE);
>  	dev_dbg(dev, "probe\n");
>  
> +	acpi_i2c_device_pm_get_attach(client, true);
> +
>  	status = driver->probe(client, i2c_match_id(driver->id_table, client));
>  	if (status) {
>  		client->driver = NULL;
>  		i2c_set_clientdata(client, NULL);
>  	}
> +
> +	acpi_i2c_device_pm_put_detach(client, !!status);
>  	return status;
>  }
>  
> @@ -271,6 +296,8 @@ static int i2c_device_remove(struct device *dev)
>  	if (!client || !dev->driver)
>  		return 0;
>  
> +	acpi_i2c_device_pm_get_attach(client, false);
> +
>  	driver = to_i2c_driver(dev->driver);
>  	if (driver->remove) {
>  		dev_dbg(dev, "remove\n");
> @@ -283,6 +310,8 @@ static int i2c_device_remove(struct device *dev)
>  		client->driver = NULL;
>  		i2c_set_clientdata(client, NULL);
>  	}
> +
> +	acpi_i2c_device_pm_put_detach(client, true);
>  	return status;
>  }
>  
> @@ -294,8 +323,11 @@ static void i2c_device_shutdown(struct device *dev)
>  	if (!client || !dev->driver)
>  		return;
>  	driver = to_i2c_driver(dev->driver);
> -	if (driver->shutdown)
> +	if (driver->shutdown) {
> +		acpi_i2c_device_pm_get_attach(client, false);
>  		driver->shutdown(client);
> +		acpi_i2c_device_pm_put_detach(client, false);
> +	}
>  }
>  
>  #ifdef CONFIG_PM_SLEEP
> @@ -1263,6 +1295,16 @@ exit_recovery:
>  	bus_for_each_drv(&i2c_bus_type, NULL, adap, __process_new_adapter);
>  	mutex_unlock(&core_lock);
>  
> +	/*
> +	 * For ACPI enumerated controllers we must make sure that the
> +	 * controller is powered on before its children. Runtime PM handles
> +	 * this for us once we have enabled it for the adapter device.
> +	 */
> +	if (ACPI_HANDLE(adap->dev.parent)) {
> +		pm_runtime_no_callbacks(&adap->dev);
> +		pm_runtime_enable(&adap->dev);
> +	}
> +
>  	return 0;
>  
>  out_list:
> @@ -1427,6 +1469,9 @@ void i2c_del_adapter(struct i2c_adapter *adap)
>  		return;
>  	}
>  
> +	if (ACPI_HANDLE(adap->dev.parent))
> +		pm_runtime_disable(&adap->dev);
> +
>  	/* Tell drivers about this removal */
>  	mutex_lock(&core_lock);
>  	bus_for_each_drv(&i2c_bus_type, NULL, adap,
> 
-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

[PATCH v4] i2c: enable runtime PM for I2C adapter devices enumerated from ACPI

[Mark Brown]

On Sat, Oct 05, 2013 at 11:09:01AM +0300, Mika Westerberg wrote:

> It looks like Windows actually powers the I2C controller off independently
> of the I2C client power state. We should probably do the same in Linux even
> though it is not following what the ACPI spec says (but makes sense with
> serial buses like I2C and SPI).

Sounds like it's probably worth going to the effort of getting the spec
amended...
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20131005/d387696f/attachment.sig>