diff mbox

[1/9] ACPI / PM: Let acpi_dev_pm_detach() return an error code

Message ID 1409054837-5667-2-git-send-email-ulf.hansson@linaro.org
State New
Headers show

Commit Message

Ulf Hansson Aug. 26, 2014, 12:07 p.m. UTC 
To give callers the option of acting on a errors while removing the
pm_domain ops for the device in the ACPI power domain, let
acpi_dev_pm_detach() return an int to provide the error code.

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/acpi/device_pm.c | 4 ++++
 include/linux/acpi.h     | 7 +++++--
 2 files changed, 9 insertions(+), 2 deletions(-)

Comments

Rafael J. Wysocki Aug. 27, 2014, 12:16 a.m. UTC | #1 
On Tuesday, August 26, 2014 02:07:09 PM Ulf Hansson wrote:
> To give callers the option of acting on a errors while removing the
> pm_domain ops for the device in the ACPI power domain, let
> acpi_dev_pm_detach() return an int to provide the error code.
> 
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---
>  drivers/acpi/device_pm.c | 4 ++++
>  include/linux/acpi.h     | 7 +++++--
>  2 files changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/acpi/device_pm.c b/drivers/acpi/device_pm.c
> index 67075f8..fa78abb 100644
> --- a/drivers/acpi/device_pm.c
> +++ b/drivers/acpi/device_pm.c
> @@ -1087,6 +1087,8 @@ EXPORT_SYMBOL_GPL(acpi_dev_pm_attach);
>   *
>   * Callers must ensure proper synchronization of this function with power
>   * management callbacks.
> + *
> + * Returns 0 on successfully detached power domain or negative error code.

"PM domain" here, please, not "power domain".

>   */
>  void acpi_dev_pm_detach(struct device *dev, bool power_off)

It looks like you've never compiled this, have you?

>  {
> @@ -1107,7 +1109,9 @@ void acpi_dev_pm_detach(struct device *dev, bool power_off)
>  			acpi_device_wakeup(adev, ACPI_STATE_S0, false);
>  			acpi_dev_pm_low_power(dev, adev, ACPI_STATE_S0);
>  		}
> +		return 0;
>  	}
> +	return -ENODEV;

-EINVAL perhaps?

>  }
>  EXPORT_SYMBOL_GPL(acpi_dev_pm_detach);
>  #endif /* CONFIG_PM */
> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
> index 5320153..a7bfdf6 100644
> --- a/include/linux/acpi.h
> +++ b/include/linux/acpi.h
> @@ -576,7 +576,7 @@ static inline int acpi_subsys_freeze(struct device *dev) { return 0; }
>  #if defined(CONFIG_ACPI) && defined(CONFIG_PM)
>  struct acpi_device *acpi_dev_pm_get_node(struct device *dev);
>  int acpi_dev_pm_attach(struct device *dev, bool power_on);
> -void acpi_dev_pm_detach(struct device *dev, bool power_off);
> +int acpi_dev_pm_detach(struct device *dev, bool power_off);
>  #else
>  static inline struct acpi_device *acpi_dev_pm_get_node(struct device *dev)
>  {
> @@ -586,7 +586,10 @@ static inline int acpi_dev_pm_attach(struct device *dev, bool power_on)
>  {
>  	return -ENODEV;
>  }
> -static inline void acpi_dev_pm_detach(struct device *dev, bool power_off) {}
> +static inline int acpi_dev_pm_detach(struct device *dev, bool power_off)
> +{
> +	return -ENODEV;
> +}
>  #endif
>  
>  #ifdef CONFIG_ACPI
>
Zheng, Lv Aug. 27, 2014, 5:43 a.m. UTC | #2 
Hi,

> From: linux-acpi-owner@vger.kernel.org [mailto:linux-acpi-owner@vger.kernel.org] On Behalf Of Ulf Hansson
> Sent: Tuesday, August 26, 2014 8:07 PM
> To: Rafael J. Wysocki; Brown, Len; Pavel Machek; Greg Kroah-Hartman; linux-pm@vger.kernel.org
> 
> To give callers the option of acting on a errors while removing the
> pm_domain ops for the device in the ACPI power domain, let
> acpi_dev_pm_detach() return an int to provide the error code.
> 
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---
>  drivers/acpi/device_pm.c | 4 ++++
>  include/linux/acpi.h     | 7 +++++--
>  2 files changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/acpi/device_pm.c b/drivers/acpi/device_pm.c
> index 67075f8..fa78abb 100644
> --- a/drivers/acpi/device_pm.c
> +++ b/drivers/acpi/device_pm.c
> @@ -1087,6 +1087,8 @@ EXPORT_SYMBOL_GPL(acpi_dev_pm_attach);
>   *
>   * Callers must ensure proper synchronization of this function with power
>   * management callbacks.
> + *
> + * Returns 0 on successfully detached power domain or negative error code.
>   */
>  void acpi_dev_pm_detach(struct device *dev, bool power_off)

Should the return type be "int" here?

Thanks and best regards
-Lv

>  {
> @@ -1107,7 +1109,9 @@ void acpi_dev_pm_detach(struct device *dev, bool power_off)
>  			acpi_device_wakeup(adev, ACPI_STATE_S0, false);
>  			acpi_dev_pm_low_power(dev, adev, ACPI_STATE_S0);
>  		}
> +		return 0;
>  	}
> +	return -ENODEV;
>  }
>  EXPORT_SYMBOL_GPL(acpi_dev_pm_detach);
>  #endif /* CONFIG_PM */
> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
> index 5320153..a7bfdf6 100644
> --- a/include/linux/acpi.h
> +++ b/include/linux/acpi.h
> @@ -576,7 +576,7 @@ static inline int acpi_subsys_freeze(struct device *dev) { return 0; }
>  #if defined(CONFIG_ACPI) && defined(CONFIG_PM)
>  struct acpi_device *acpi_dev_pm_get_node(struct device *dev);
>  int acpi_dev_pm_attach(struct device *dev, bool power_on);
> -void acpi_dev_pm_detach(struct device *dev, bool power_off);
> +int acpi_dev_pm_detach(struct device *dev, bool power_off);
>  #else
>  static inline struct acpi_device *acpi_dev_pm_get_node(struct device *dev)
>  {
> @@ -586,7 +586,10 @@ static inline int acpi_dev_pm_attach(struct device *dev, bool power_on)
>  {
>  	return -ENODEV;
>  }
> -static inline void acpi_dev_pm_detach(struct device *dev, bool power_off) {}
> +static inline int acpi_dev_pm_detach(struct device *dev, bool power_off)
> +{
> +	return -ENODEV;
> +}
>  #endif
> 
>  #ifdef CONFIG_ACPI
> --
> 1.9.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ulf Hansson Aug. 27, 2014, 7:25 a.m. UTC | #3 
On 27 August 2014 02:16, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> On Tuesday, August 26, 2014 02:07:09 PM Ulf Hansson wrote:
>> To give callers the option of acting on a errors while removing the
>> pm_domain ops for the device in the ACPI power domain, let
>> acpi_dev_pm_detach() return an int to provide the error code.
>>
>> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
>> ---
>>  drivers/acpi/device_pm.c | 4 ++++
>>  include/linux/acpi.h     | 7 +++++--
>>  2 files changed, 9 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/acpi/device_pm.c b/drivers/acpi/device_pm.c
>> index 67075f8..fa78abb 100644
>> --- a/drivers/acpi/device_pm.c
>> +++ b/drivers/acpi/device_pm.c
>> @@ -1087,6 +1087,8 @@ EXPORT_SYMBOL_GPL(acpi_dev_pm_attach);
>>   *
>>   * Callers must ensure proper synchronization of this function with power
>>   * management callbacks.
>> + *
>> + * Returns 0 on successfully detached power domain or negative error code.
>
> "PM domain" here, please, not "power domain".
>

Sure.

If you prefer that term, I can also change on the other patches in
this patchset. There are commit-msg etc , where I think I have used
"power domain".

>>   */
>>  void acpi_dev_pm_detach(struct device *dev, bool power_off)
>
> It looks like you've never compiled this, have you?

You are absolutely right. There were quite some combinations of
CONFIG_PM_* that I tried out, but I totally forgot ACPI, sorry! I will
make sure to do it in v2.

>
>>  {
>> @@ -1107,7 +1109,9 @@ void acpi_dev_pm_detach(struct device *dev, bool power_off)
>>                       acpi_device_wakeup(adev, ACPI_STATE_S0, false);
>>                       acpi_dev_pm_low_power(dev, adev, ACPI_STATE_S0);
>>               }
>> +             return 0;
>>       }
>> +     return -ENODEV;
>
> -EINVAL perhaps?

Sure!

>
>>  }
>>  EXPORT_SYMBOL_GPL(acpi_dev_pm_detach);
>>  #endif /* CONFIG_PM */
>> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
>> index 5320153..a7bfdf6 100644
>> --- a/include/linux/acpi.h
>> +++ b/include/linux/acpi.h
>> @@ -576,7 +576,7 @@ static inline int acpi_subsys_freeze(struct device *dev) { return 0; }
>>  #if defined(CONFIG_ACPI) && defined(CONFIG_PM)
>>  struct acpi_device *acpi_dev_pm_get_node(struct device *dev);
>>  int acpi_dev_pm_attach(struct device *dev, bool power_on);
>> -void acpi_dev_pm_detach(struct device *dev, bool power_off);
>> +int acpi_dev_pm_detach(struct device *dev, bool power_off);
>>  #else
>>  static inline struct acpi_device *acpi_dev_pm_get_node(struct device *dev)
>>  {
>> @@ -586,7 +586,10 @@ static inline int acpi_dev_pm_attach(struct device *dev, bool power_on)
>>  {
>>       return -ENODEV;
>>  }
>> -static inline void acpi_dev_pm_detach(struct device *dev, bool power_off) {}
>> +static inline int acpi_dev_pm_detach(struct device *dev, bool power_off)
>> +{
>> +     return -ENODEV;
>> +}
>>  #endif
>>
>>  #ifdef CONFIG_ACPI
>>
>
> --
> I speak only for myself.
> Rafael J. Wysocki, Intel Open Source Technology Center.

Thanks for reviewing!

Kind regards
Uffe
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ulf Hansson Aug. 27, 2014, 7:26 a.m. UTC | #4 
On 27 August 2014 07:43, Zheng, Lv <lv.zheng@intel.com> wrote:
> Hi,
>
>> From: linux-acpi-owner@vger.kernel.org [mailto:linux-acpi-owner@vger.kernel.org] On Behalf Of Ulf Hansson
>> Sent: Tuesday, August 26, 2014 8:07 PM
>> To: Rafael J. Wysocki; Brown, Len; Pavel Machek; Greg Kroah-Hartman; linux-pm@vger.kernel.org
>>
>> To give callers the option of acting on a errors while removing the
>> pm_domain ops for the device in the ACPI power domain, let
>> acpi_dev_pm_detach() return an int to provide the error code.
>>
>> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
>> ---
>>  drivers/acpi/device_pm.c | 4 ++++
>>  include/linux/acpi.h     | 7 +++++--
>>  2 files changed, 9 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/acpi/device_pm.c b/drivers/acpi/device_pm.c
>> index 67075f8..fa78abb 100644
>> --- a/drivers/acpi/device_pm.c
>> +++ b/drivers/acpi/device_pm.c
>> @@ -1087,6 +1087,8 @@ EXPORT_SYMBOL_GPL(acpi_dev_pm_attach);
>>   *
>>   * Callers must ensure proper synchronization of this function with power
>>   * management callbacks.
>> + *
>> + * Returns 0 on successfully detached power domain or negative error code.
>>   */
>>  void acpi_dev_pm_detach(struct device *dev, bool power_off)
>
> Should the return type be "int" here?

You are right, thanks!

Kind regards
Uffe

>
> Thanks and best regards
> -Lv
>
>>  {
>> @@ -1107,7 +1109,9 @@ void acpi_dev_pm_detach(struct device *dev, bool power_off)
>>                       acpi_device_wakeup(adev, ACPI_STATE_S0, false);
>>                       acpi_dev_pm_low_power(dev, adev, ACPI_STATE_S0);
>>               }
>> +             return 0;
>>       }
>> +     return -ENODEV;
>>  }
>>  EXPORT_SYMBOL_GPL(acpi_dev_pm_detach);
>>  #endif /* CONFIG_PM */
>> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
>> index 5320153..a7bfdf6 100644
>> --- a/include/linux/acpi.h
>> +++ b/include/linux/acpi.h
>> @@ -576,7 +576,7 @@ static inline int acpi_subsys_freeze(struct device *dev) { return 0; }
>>  #if defined(CONFIG_ACPI) && defined(CONFIG_PM)
>>  struct acpi_device *acpi_dev_pm_get_node(struct device *dev);
>>  int acpi_dev_pm_attach(struct device *dev, bool power_on);
>> -void acpi_dev_pm_detach(struct device *dev, bool power_off);
>> +int acpi_dev_pm_detach(struct device *dev, bool power_off);
>>  #else
>>  static inline struct acpi_device *acpi_dev_pm_get_node(struct device *dev)
>>  {
>> @@ -586,7 +586,10 @@ static inline int acpi_dev_pm_attach(struct device *dev, bool power_on)
>>  {
>>       return -ENODEV;
>>  }
>> -static inline void acpi_dev_pm_detach(struct device *dev, bool power_off) {}
>> +static inline int acpi_dev_pm_detach(struct device *dev, bool power_off)
>> +{
>> +     return -ENODEV;
>> +}
>>  #endif
>>
>>  #ifdef CONFIG_ACPI
>> --
>> 1.9.1
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/acpi/device_pm.c b/drivers/acpi/device_pm.c
index 67075f8..fa78abb 100644
--- a/drivers/acpi/device_pm.c
+++ b/drivers/acpi/device_pm.c
@@ -1087,6 +1087,8 @@  EXPORT_SYMBOL_GPL(acpi_dev_pm_attach);
  *
  * Callers must ensure proper synchronization of this function with power
  * management callbacks.
+ *
+ * Returns 0 on successfully detached power domain or negative error code.
  */
 void acpi_dev_pm_detach(struct device *dev, bool power_off)
 {
@@ -1107,7 +1109,9 @@  void acpi_dev_pm_detach(struct device *dev, bool power_off)
 			acpi_device_wakeup(adev, ACPI_STATE_S0, false);
 			acpi_dev_pm_low_power(dev, adev, ACPI_STATE_S0);
 		}
+		return 0;
 	}
+	return -ENODEV;
 }
 EXPORT_SYMBOL_GPL(acpi_dev_pm_detach);
 #endif /* CONFIG_PM */
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 5320153..a7bfdf6 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -576,7 +576,7 @@  static inline int acpi_subsys_freeze(struct device *dev) { return 0; }
 #if defined(CONFIG_ACPI) && defined(CONFIG_PM)
 struct acpi_device *acpi_dev_pm_get_node(struct device *dev);
 int acpi_dev_pm_attach(struct device *dev, bool power_on);
-void acpi_dev_pm_detach(struct device *dev, bool power_off);
+int acpi_dev_pm_detach(struct device *dev, bool power_off);
 #else
 static inline struct acpi_device *acpi_dev_pm_get_node(struct device *dev)
 {
@@ -586,7 +586,10 @@  static inline int acpi_dev_pm_attach(struct device *dev, bool power_on)
 {
 	return -ENODEV;
 }
-static inline void acpi_dev_pm_detach(struct device *dev, bool power_off) {}
+static inline int acpi_dev_pm_detach(struct device *dev, bool power_off)
+{
+	return -ENODEV;
+}
 #endif
 
 #ifdef CONFIG_ACPI