Re: [PATCH 5/7] ACPI / PM: Provide device PM functions operating on struct acpi_device

2012-11-02 Thread Rafael J. Wysocki 
On Friday, November 02, 2012 01:17:10 PM Aaron Lu wrote:
> On 10/30/2012 11:20 PM, Rafael J. Wysocki wrote:
> > On Tuesday, October 30, 2012 03:28:45 PM Aaron Lu wrote:
> >> On Mon, Oct 29, 2012 at 10:11:20AM +0100, Rafael J. Wysocki wrote:
> >>> From: Rafael J. Wysocki 
> >>>
> >>> If the caller of acpi_bus_set_power() already has a pointer to the
> >>> struct acpi_device object corresponding to the device in question, it
> >>> doesn't make sense for it to go through acpi_bus_get_device(), which
> >>> may be costly, because it involves acquiring the global ACPI
> >>> namespace mutex.
> >>>
> >>> For this reason, export the function operating on struct acpi_device
> >>> objects used internally by acpi_bus_set_power(), so that it may be
> >>> called instead of acpi_bus_set_power() in the above case, and change
> >>> its name to acpi_device_set_power().
> >>>
> >>> Additionally, introduce two inline wrappers for checking ACPI PM
> >>> capabilities of devices represented by struct acpi_device objects.
> >>
> >> What about adding yet another wrapper to check power off capability of
> >> the device? If device has _PS3 or _PRx, it means the device can be
> >> powered off from ACPI's perspective. This is useful for ZPODD when
> >> deciding if platform has the required ability to support it.
> > 
> > Sure, no problem with that.  Perhaps you can cut a patch for that
> > on top of this series?
> 
> Do you think it is reasonable to add a new field to acpi_state.flags to
> represent if we, as OSPM, have a way to put the device into a ACPI
> device state? This field can be set once in acpi_bus_get_power_flags and
> used afterwards.
> 
> The valid field of acpi_state.flags is what we have today, and it means
> whether this ACPI device state is valid for the device, but not that if
> OSPM can actually put the device into that power state.

Yes, I think that adding such a new flag would make sense.

Thanks,
Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 5/7] ACPI / PM: Provide device PM functions operating on struct acpi_device

2012-11-02 Thread Rafael J. Wysocki 
On Friday, November 02, 2012 01:17:10 PM Aaron Lu wrote:
 On 10/30/2012 11:20 PM, Rafael J. Wysocki wrote:
  On Tuesday, October 30, 2012 03:28:45 PM Aaron Lu wrote:
  On Mon, Oct 29, 2012 at 10:11:20AM +0100, Rafael J. Wysocki wrote:
  From: Rafael J. Wysocki rafael.j.wyso...@intel.com
 
  If the caller of acpi_bus_set_power() already has a pointer to the
  struct acpi_device object corresponding to the device in question, it
  doesn't make sense for it to go through acpi_bus_get_device(), which
  may be costly, because it involves acquiring the global ACPI
  namespace mutex.
 
  For this reason, export the function operating on struct acpi_device
  objects used internally by acpi_bus_set_power(), so that it may be
  called instead of acpi_bus_set_power() in the above case, and change
  its name to acpi_device_set_power().
 
  Additionally, introduce two inline wrappers for checking ACPI PM
  capabilities of devices represented by struct acpi_device objects.
 
  What about adding yet another wrapper to check power off capability of
  the device? If device has _PS3 or _PRx, it means the device can be
  powered off from ACPI's perspective. This is useful for ZPODD when
  deciding if platform has the required ability to support it.
  
  Sure, no problem with that.  Perhaps you can cut a patch for that
  on top of this series?
 
 Do you think it is reasonable to add a new field to acpi_state.flags to
 represent if we, as OSPM, have a way to put the device into a ACPI
 device state? This field can be set once in acpi_bus_get_power_flags and
 used afterwards.
 
 The valid field of acpi_state.flags is what we have today, and it means
 whether this ACPI device state is valid for the device, but not that if
 OSPM can actually put the device into that power state.

Yes, I think that adding such a new flag would make sense.

Thanks,
Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 5/7] ACPI / PM: Provide device PM functions operating on struct acpi_device

2012-11-01 Thread Aaron Lu 
On 10/30/2012 11:20 PM, Rafael J. Wysocki wrote:
> On Tuesday, October 30, 2012 03:28:45 PM Aaron Lu wrote:
>> On Mon, Oct 29, 2012 at 10:11:20AM +0100, Rafael J. Wysocki wrote:
>>> From: Rafael J. Wysocki 
>>>
>>> If the caller of acpi_bus_set_power() already has a pointer to the
>>> struct acpi_device object corresponding to the device in question, it
>>> doesn't make sense for it to go through acpi_bus_get_device(), which
>>> may be costly, because it involves acquiring the global ACPI
>>> namespace mutex.
>>>
>>> For this reason, export the function operating on struct acpi_device
>>> objects used internally by acpi_bus_set_power(), so that it may be
>>> called instead of acpi_bus_set_power() in the above case, and change
>>> its name to acpi_device_set_power().
>>>
>>> Additionally, introduce two inline wrappers for checking ACPI PM
>>> capabilities of devices represented by struct acpi_device objects.
>>
>> What about adding yet another wrapper to check power off capability of
>> the device? If device has _PS3 or _PRx, it means the device can be
>> powered off from ACPI's perspective. This is useful for ZPODD when
>> deciding if platform has the required ability to support it.
> 
> Sure, no problem with that.  Perhaps you can cut a patch for that
> on top of this series?

Do you think it is reasonable to add a new field to acpi_state.flags to
represent if we, as OSPM, have a way to put the device into a ACPI
device state? This field can be set once in acpi_bus_get_power_flags and
used afterwards.

The valid field of acpi_state.flags is what we have today, and it means
whether this ACPI device state is valid for the device, but not that if
OSPM can actually put the device into that power state.

Thanks,
Aaron

> 
> Rafael
> 
> 
>>> Signed-off-by: Rafael J. Wysocki 
>>> ---
>>>  drivers/acpi/bus.c  |   15 ---
>>>  include/acpi/acpi_bus.h |   11 +++
>>>  2 files changed, 23 insertions(+), 3 deletions(-)
>>>
>>> Index: linux/drivers/acpi/bus.c
>>> ===
>>> --- linux.orig/drivers/acpi/bus.c
>>> +++ linux/drivers/acpi/bus.c
>>> @@ -257,7 +257,15 @@ static int __acpi_bus_get_power(struct a
>>>  }
>>>  
>>>  
>>> -static int __acpi_bus_set_power(struct acpi_device *device, int state)
>>> +/**
>>> + * acpi_device_set_power - Set power state of an ACPI device.
>>> + * @device: Device to set the power state of.
>>> + * @state: New power state to set.
>>> + *
>>> + * Callers must ensure that the device is power manageable before using 
>>> this
>>> + * function.
>>> + */
>>> +int acpi_device_set_power(struct acpi_device *device, int state)
>>>  {
>>> int result = 0;
>>> acpi_status status = AE_OK;
>>> @@ -341,6 +349,7 @@ static int __acpi_bus_set_power(struct a
>>>  
>>> return result;
>>>  }
>>> +EXPORT_SYMBOL(acpi_device_set_power);
>>>  
>>>  
>>>  int acpi_bus_set_power(acpi_handle handle, int state)
>>> @@ -359,7 +368,7 @@ int acpi_bus_set_power(acpi_handle handl
>>> return -ENODEV;
>>> }
>>>  
>>> -   return __acpi_bus_set_power(device, state);
>>> +   return acpi_device_set_power(device, state);
>>>  }
>>>  EXPORT_SYMBOL(acpi_bus_set_power);
>>>  
>>> @@ -402,7 +411,7 @@ int acpi_bus_update_power(acpi_handle ha
>>> if (result)
>>> return result;
>>>  
>>> -   result = __acpi_bus_set_power(device, state);
>>> +   result = acpi_device_set_power(device, state);
>>> if (!result && state_p)
>>> *state_p = state;
>>>  
>>> Index: linux/include/acpi/acpi_bus.h
>>> ===
>>> --- linux.orig/include/acpi/acpi_bus.h
>>> +++ linux/include/acpi/acpi_bus.h
>>> @@ -338,6 +338,7 @@ acpi_status acpi_bus_get_status_handle(a
>>>unsigned long long *sta);
>>>  int acpi_bus_get_status(struct acpi_device *device);
>>>  int acpi_bus_set_power(acpi_handle handle, int state);
>>> +int acpi_device_set_power(struct acpi_device *device, int state);
>>>  int acpi_bus_update_power(acpi_handle handle, int *state_p);
>>>  bool acpi_bus_power_manageable(acpi_handle handle);
>>>  bool acpi_bus_can_wakeup(acpi_handle handle);
>>> @@ -482,6 +483,16 @@ static inline int acpi_pm_device_sleep_w
>>>  }
>>>  #endif
>>>  
>>> +static inline bool acpi_device_power_manageable(struct acpi_device *adev)
>>> +{
>>> +   return adev->flags.power_manageable;
>>> +}
>>> +
>>> +static inline bool acpi_device_can_wakeup(struct acpi_device *adev)
>>> +{
>>> +   return adev->wakeup.flags.valid;
>>> +}
>>> +
>>>  #else  /* CONFIG_ACPI */
>>>  
>>>  static inline int register_acpi_bus_type(void *bus) { return 0; }
>>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
>> the body of a message to majord...@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a

Re: [PATCH 5/7] ACPI / PM: Provide device PM functions operating on struct acpi_device

2012-11-01 Thread Aaron Lu 
On 10/30/2012 11:20 PM, Rafael J. Wysocki wrote:
 On Tuesday, October 30, 2012 03:28:45 PM Aaron Lu wrote:
 On Mon, Oct 29, 2012 at 10:11:20AM +0100, Rafael J. Wysocki wrote:
 From: Rafael J. Wysocki rafael.j.wyso...@intel.com

 If the caller of acpi_bus_set_power() already has a pointer to the
 struct acpi_device object corresponding to the device in question, it
 doesn't make sense for it to go through acpi_bus_get_device(), which
 may be costly, because it involves acquiring the global ACPI
 namespace mutex.

 For this reason, export the function operating on struct acpi_device
 objects used internally by acpi_bus_set_power(), so that it may be
 called instead of acpi_bus_set_power() in the above case, and change
 its name to acpi_device_set_power().

 Additionally, introduce two inline wrappers for checking ACPI PM
 capabilities of devices represented by struct acpi_device objects.

 What about adding yet another wrapper to check power off capability of
 the device? If device has _PS3 or _PRx, it means the device can be
 powered off from ACPI's perspective. This is useful for ZPODD when
 deciding if platform has the required ability to support it.
 
 Sure, no problem with that.  Perhaps you can cut a patch for that
 on top of this series?

Do you think it is reasonable to add a new field to acpi_state.flags to
represent if we, as OSPM, have a way to put the device into a ACPI
device state? This field can be set once in acpi_bus_get_power_flags and
used afterwards.

The valid field of acpi_state.flags is what we have today, and it means
whether this ACPI device state is valid for the device, but not that if
OSPM can actually put the device into that power state.

Thanks,
Aaron

 
 Rafael
 
 
 Signed-off-by: Rafael J. Wysocki rafael.j.wyso...@intel.com
 ---
  drivers/acpi/bus.c  |   15 ---
  include/acpi/acpi_bus.h |   11 +++
  2 files changed, 23 insertions(+), 3 deletions(-)

 Index: linux/drivers/acpi/bus.c
 ===
 --- linux.orig/drivers/acpi/bus.c
 +++ linux/drivers/acpi/bus.c
 @@ -257,7 +257,15 @@ static int __acpi_bus_get_power(struct a
  }
  
  
 -static int __acpi_bus_set_power(struct acpi_device *device, int state)
 +/**
 + * acpi_device_set_power - Set power state of an ACPI device.
 + * @device: Device to set the power state of.
 + * @state: New power state to set.
 + *
 + * Callers must ensure that the device is power manageable before using 
 this
 + * function.
 + */
 +int acpi_device_set_power(struct acpi_device *device, int state)
  {
 int result = 0;
 acpi_status status = AE_OK;
 @@ -341,6 +349,7 @@ static int __acpi_bus_set_power(struct a
  
 return result;
  }
 +EXPORT_SYMBOL(acpi_device_set_power);
  
  
  int acpi_bus_set_power(acpi_handle handle, int state)
 @@ -359,7 +368,7 @@ int acpi_bus_set_power(acpi_handle handl
 return -ENODEV;
 }
  
 -   return __acpi_bus_set_power(device, state);
 +   return acpi_device_set_power(device, state);
  }
  EXPORT_SYMBOL(acpi_bus_set_power);
  
 @@ -402,7 +411,7 @@ int acpi_bus_update_power(acpi_handle ha
 if (result)
 return result;
  
 -   result = __acpi_bus_set_power(device, state);
 +   result = acpi_device_set_power(device, state);
 if (!result  state_p)
 *state_p = state;
  
 Index: linux/include/acpi/acpi_bus.h
 ===
 --- linux.orig/include/acpi/acpi_bus.h
 +++ linux/include/acpi/acpi_bus.h
 @@ -338,6 +338,7 @@ acpi_status acpi_bus_get_status_handle(a
unsigned long long *sta);
  int acpi_bus_get_status(struct acpi_device *device);
  int acpi_bus_set_power(acpi_handle handle, int state);
 +int acpi_device_set_power(struct acpi_device *device, int state);
  int acpi_bus_update_power(acpi_handle handle, int *state_p);
  bool acpi_bus_power_manageable(acpi_handle handle);
  bool acpi_bus_can_wakeup(acpi_handle handle);
 @@ -482,6 +483,16 @@ static inline int acpi_pm_device_sleep_w
  }
  #endif
  
 +static inline bool acpi_device_power_manageable(struct acpi_device *adev)
 +{
 +   return adev-flags.power_manageable;
 +}
 +
 +static inline bool acpi_device_can_wakeup(struct acpi_device *adev)
 +{
 +   return adev-wakeup.flags.valid;
 +}
 +
  #else  /* CONFIG_ACPI */
  
  static inline int register_acpi_bus_type(void *bus) { return 0; }

 --
 To unsubscribe from this list: send the line unsubscribe linux-acpi in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 5/7] ACPI / PM: Provide device PM functions operating on struct acpi_device

2012-10-30 Thread Rafael J. Wysocki 
On Tuesday, October 30, 2012 03:28:45 PM Aaron Lu wrote:
> On Mon, Oct 29, 2012 at 10:11:20AM +0100, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki 
> > 
> > If the caller of acpi_bus_set_power() already has a pointer to the
> > struct acpi_device object corresponding to the device in question, it
> > doesn't make sense for it to go through acpi_bus_get_device(), which
> > may be costly, because it involves acquiring the global ACPI
> > namespace mutex.
> > 
> > For this reason, export the function operating on struct acpi_device
> > objects used internally by acpi_bus_set_power(), so that it may be
> > called instead of acpi_bus_set_power() in the above case, and change
> > its name to acpi_device_set_power().
> > 
> > Additionally, introduce two inline wrappers for checking ACPI PM
> > capabilities of devices represented by struct acpi_device objects.
> 
> What about adding yet another wrapper to check power off capability of
> the device? If device has _PS3 or _PRx, it means the device can be
> powered off from ACPI's perspective. This is useful for ZPODD when
> deciding if platform has the required ability to support it.

Sure, no problem with that.  Perhaps you can cut a patch for that
on top of this series?

Rafael


> > Signed-off-by: Rafael J. Wysocki 
> > ---
> >  drivers/acpi/bus.c  |   15 ---
> >  include/acpi/acpi_bus.h |   11 +++
> >  2 files changed, 23 insertions(+), 3 deletions(-)
> > 
> > Index: linux/drivers/acpi/bus.c
> > ===
> > --- linux.orig/drivers/acpi/bus.c
> > +++ linux/drivers/acpi/bus.c
> > @@ -257,7 +257,15 @@ static int __acpi_bus_get_power(struct a
> >  }
> >  
> >  
> > -static int __acpi_bus_set_power(struct acpi_device *device, int state)
> > +/**
> > + * acpi_device_set_power - Set power state of an ACPI device.
> > + * @device: Device to set the power state of.
> > + * @state: New power state to set.
> > + *
> > + * Callers must ensure that the device is power manageable before using 
> > this
> > + * function.
> > + */
> > +int acpi_device_set_power(struct acpi_device *device, int state)
> >  {
> > int result = 0;
> > acpi_status status = AE_OK;
> > @@ -341,6 +349,7 @@ static int __acpi_bus_set_power(struct a
> >  
> > return result;
> >  }
> > +EXPORT_SYMBOL(acpi_device_set_power);
> >  
> >  
> >  int acpi_bus_set_power(acpi_handle handle, int state)
> > @@ -359,7 +368,7 @@ int acpi_bus_set_power(acpi_handle handl
> > return -ENODEV;
> > }
> >  
> > -   return __acpi_bus_set_power(device, state);
> > +   return acpi_device_set_power(device, state);
> >  }
> >  EXPORT_SYMBOL(acpi_bus_set_power);
> >  
> > @@ -402,7 +411,7 @@ int acpi_bus_update_power(acpi_handle ha
> > if (result)
> > return result;
> >  
> > -   result = __acpi_bus_set_power(device, state);
> > +   result = acpi_device_set_power(device, state);
> > if (!result && state_p)
> > *state_p = state;
> >  
> > Index: linux/include/acpi/acpi_bus.h
> > ===
> > --- linux.orig/include/acpi/acpi_bus.h
> > +++ linux/include/acpi/acpi_bus.h
> > @@ -338,6 +338,7 @@ acpi_status acpi_bus_get_status_handle(a
> >unsigned long long *sta);
> >  int acpi_bus_get_status(struct acpi_device *device);
> >  int acpi_bus_set_power(acpi_handle handle, int state);
> > +int acpi_device_set_power(struct acpi_device *device, int state);
> >  int acpi_bus_update_power(acpi_handle handle, int *state_p);
> >  bool acpi_bus_power_manageable(acpi_handle handle);
> >  bool acpi_bus_can_wakeup(acpi_handle handle);
> > @@ -482,6 +483,16 @@ static inline int acpi_pm_device_sleep_w
> >  }
> >  #endif
> >  
> > +static inline bool acpi_device_power_manageable(struct acpi_device *adev)
> > +{
> > +   return adev->flags.power_manageable;
> > +}
> > +
> > +static inline bool acpi_device_can_wakeup(struct acpi_device *adev)
> > +{
> > +   return adev->wakeup.flags.valid;
> > +}
> > +
> >  #else  /* CONFIG_ACPI */
> >  
> >  static inline int register_acpi_bus_type(void *bus) { return 0; }
> > 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 5/7] ACPI / PM: Provide device PM functions operating on struct acpi_device

2012-10-30 Thread Aaron Lu 
On Mon, Oct 29, 2012 at 10:11:20AM +0100, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki 
> 
> If the caller of acpi_bus_set_power() already has a pointer to the
> struct acpi_device object corresponding to the device in question, it
> doesn't make sense for it to go through acpi_bus_get_device(), which
> may be costly, because it involves acquiring the global ACPI
> namespace mutex.
> 
> For this reason, export the function operating on struct acpi_device
> objects used internally by acpi_bus_set_power(), so that it may be
> called instead of acpi_bus_set_power() in the above case, and change
> its name to acpi_device_set_power().
> 
> Additionally, introduce two inline wrappers for checking ACPI PM
> capabilities of devices represented by struct acpi_device objects.

What about adding yet another wrapper to check power off capability of
the device? If device has _PS3 or _PRx, it means the device can be
powered off from ACPI's perspective. This is useful for ZPODD when
deciding if platform has the required ability to support it.

Thanks,
Aaron

> 
> Signed-off-by: Rafael J. Wysocki 
> ---
>  drivers/acpi/bus.c  |   15 ---
>  include/acpi/acpi_bus.h |   11 +++
>  2 files changed, 23 insertions(+), 3 deletions(-)
> 
> Index: linux/drivers/acpi/bus.c
> ===
> --- linux.orig/drivers/acpi/bus.c
> +++ linux/drivers/acpi/bus.c
> @@ -257,7 +257,15 @@ static int __acpi_bus_get_power(struct a
>  }
>  
>  
> -static int __acpi_bus_set_power(struct acpi_device *device, int state)
> +/**
> + * acpi_device_set_power - Set power state of an ACPI device.
> + * @device: Device to set the power state of.
> + * @state: New power state to set.
> + *
> + * Callers must ensure that the device is power manageable before using this
> + * function.
> + */
> +int acpi_device_set_power(struct acpi_device *device, int state)
>  {
>   int result = 0;
>   acpi_status status = AE_OK;
> @@ -341,6 +349,7 @@ static int __acpi_bus_set_power(struct a
>  
>   return result;
>  }
> +EXPORT_SYMBOL(acpi_device_set_power);
>  
>  
>  int acpi_bus_set_power(acpi_handle handle, int state)
> @@ -359,7 +368,7 @@ int acpi_bus_set_power(acpi_handle handl
>   return -ENODEV;
>   }
>  
> - return __acpi_bus_set_power(device, state);
> + return acpi_device_set_power(device, state);
>  }
>  EXPORT_SYMBOL(acpi_bus_set_power);
>  
> @@ -402,7 +411,7 @@ int acpi_bus_update_power(acpi_handle ha
>   if (result)
>   return result;
>  
> - result = __acpi_bus_set_power(device, state);
> + result = acpi_device_set_power(device, state);
>   if (!result && state_p)
>   *state_p = state;
>  
> Index: linux/include/acpi/acpi_bus.h
> ===
> --- linux.orig/include/acpi/acpi_bus.h
> +++ linux/include/acpi/acpi_bus.h
> @@ -338,6 +338,7 @@ acpi_status acpi_bus_get_status_handle(a
>  unsigned long long *sta);
>  int acpi_bus_get_status(struct acpi_device *device);
>  int acpi_bus_set_power(acpi_handle handle, int state);
> +int acpi_device_set_power(struct acpi_device *device, int state);
>  int acpi_bus_update_power(acpi_handle handle, int *state_p);
>  bool acpi_bus_power_manageable(acpi_handle handle);
>  bool acpi_bus_can_wakeup(acpi_handle handle);
> @@ -482,6 +483,16 @@ static inline int acpi_pm_device_sleep_w
>  }
>  #endif
>  
> +static inline bool acpi_device_power_manageable(struct acpi_device *adev)
> +{
> + return adev->flags.power_manageable;
> +}
> +
> +static inline bool acpi_device_can_wakeup(struct acpi_device *adev)
> +{
> + return adev->wakeup.flags.valid;
> +}
> +
>  #else/* CONFIG_ACPI */
>  
>  static inline int register_acpi_bus_type(void *bus) { return 0; }
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 5/7] ACPI / PM: Provide device PM functions operating on struct acpi_device

2012-10-30 Thread Aaron Lu 
On Mon, Oct 29, 2012 at 10:11:20AM +0100, Rafael J. Wysocki wrote:
 From: Rafael J. Wysocki rafael.j.wyso...@intel.com
 
 If the caller of acpi_bus_set_power() already has a pointer to the
 struct acpi_device object corresponding to the device in question, it
 doesn't make sense for it to go through acpi_bus_get_device(), which
 may be costly, because it involves acquiring the global ACPI
 namespace mutex.
 
 For this reason, export the function operating on struct acpi_device
 objects used internally by acpi_bus_set_power(), so that it may be
 called instead of acpi_bus_set_power() in the above case, and change
 its name to acpi_device_set_power().
 
 Additionally, introduce two inline wrappers for checking ACPI PM
 capabilities of devices represented by struct acpi_device objects.

What about adding yet another wrapper to check power off capability of
the device? If device has _PS3 or _PRx, it means the device can be
powered off from ACPI's perspective. This is useful for ZPODD when
deciding if platform has the required ability to support it.

Thanks,
Aaron

 
 Signed-off-by: Rafael J. Wysocki rafael.j.wyso...@intel.com
 ---
  drivers/acpi/bus.c  |   15 ---
  include/acpi/acpi_bus.h |   11 +++
  2 files changed, 23 insertions(+), 3 deletions(-)
 
 Index: linux/drivers/acpi/bus.c
 ===
 --- linux.orig/drivers/acpi/bus.c
 +++ linux/drivers/acpi/bus.c
 @@ -257,7 +257,15 @@ static int __acpi_bus_get_power(struct a
  }
  
  
 -static int __acpi_bus_set_power(struct acpi_device *device, int state)
 +/**
 + * acpi_device_set_power - Set power state of an ACPI device.
 + * @device: Device to set the power state of.
 + * @state: New power state to set.
 + *
 + * Callers must ensure that the device is power manageable before using this
 + * function.
 + */
 +int acpi_device_set_power(struct acpi_device *device, int state)
  {
   int result = 0;
   acpi_status status = AE_OK;
 @@ -341,6 +349,7 @@ static int __acpi_bus_set_power(struct a
  
   return result;
  }
 +EXPORT_SYMBOL(acpi_device_set_power);
  
  
  int acpi_bus_set_power(acpi_handle handle, int state)
 @@ -359,7 +368,7 @@ int acpi_bus_set_power(acpi_handle handl
   return -ENODEV;
   }
  
 - return __acpi_bus_set_power(device, state);
 + return acpi_device_set_power(device, state);
  }
  EXPORT_SYMBOL(acpi_bus_set_power);
  
 @@ -402,7 +411,7 @@ int acpi_bus_update_power(acpi_handle ha
   if (result)
   return result;
  
 - result = __acpi_bus_set_power(device, state);
 + result = acpi_device_set_power(device, state);
   if (!result  state_p)
   *state_p = state;
  
 Index: linux/include/acpi/acpi_bus.h
 ===
 --- linux.orig/include/acpi/acpi_bus.h
 +++ linux/include/acpi/acpi_bus.h
 @@ -338,6 +338,7 @@ acpi_status acpi_bus_get_status_handle(a
  unsigned long long *sta);
  int acpi_bus_get_status(struct acpi_device *device);
  int acpi_bus_set_power(acpi_handle handle, int state);
 +int acpi_device_set_power(struct acpi_device *device, int state);
  int acpi_bus_update_power(acpi_handle handle, int *state_p);
  bool acpi_bus_power_manageable(acpi_handle handle);
  bool acpi_bus_can_wakeup(acpi_handle handle);
 @@ -482,6 +483,16 @@ static inline int acpi_pm_device_sleep_w
  }
  #endif
  
 +static inline bool acpi_device_power_manageable(struct acpi_device *adev)
 +{
 + return adev-flags.power_manageable;
 +}
 +
 +static inline bool acpi_device_can_wakeup(struct acpi_device *adev)
 +{
 + return adev-wakeup.flags.valid;
 +}
 +
  #else/* CONFIG_ACPI */
  
  static inline int register_acpi_bus_type(void *bus) { return 0; }
 
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 5/7] ACPI / PM: Provide device PM functions operating on struct acpi_device

2012-10-30 Thread Rafael J. Wysocki 
On Tuesday, October 30, 2012 03:28:45 PM Aaron Lu wrote:
 On Mon, Oct 29, 2012 at 10:11:20AM +0100, Rafael J. Wysocki wrote:
  From: Rafael J. Wysocki rafael.j.wyso...@intel.com
  
  If the caller of acpi_bus_set_power() already has a pointer to the
  struct acpi_device object corresponding to the device in question, it
  doesn't make sense for it to go through acpi_bus_get_device(), which
  may be costly, because it involves acquiring the global ACPI
  namespace mutex.
  
  For this reason, export the function operating on struct acpi_device
  objects used internally by acpi_bus_set_power(), so that it may be
  called instead of acpi_bus_set_power() in the above case, and change
  its name to acpi_device_set_power().
  
  Additionally, introduce two inline wrappers for checking ACPI PM
  capabilities of devices represented by struct acpi_device objects.
 
 What about adding yet another wrapper to check power off capability of
 the device? If device has _PS3 or _PRx, it means the device can be
 powered off from ACPI's perspective. This is useful for ZPODD when
 deciding if platform has the required ability to support it.

Sure, no problem with that.  Perhaps you can cut a patch for that
on top of this series?

Rafael


  Signed-off-by: Rafael J. Wysocki rafael.j.wyso...@intel.com
  ---
   drivers/acpi/bus.c  |   15 ---
   include/acpi/acpi_bus.h |   11 +++
   2 files changed, 23 insertions(+), 3 deletions(-)
  
  Index: linux/drivers/acpi/bus.c
  ===
  --- linux.orig/drivers/acpi/bus.c
  +++ linux/drivers/acpi/bus.c
  @@ -257,7 +257,15 @@ static int __acpi_bus_get_power(struct a
   }
   
   
  -static int __acpi_bus_set_power(struct acpi_device *device, int state)
  +/**
  + * acpi_device_set_power - Set power state of an ACPI device.
  + * @device: Device to set the power state of.
  + * @state: New power state to set.
  + *
  + * Callers must ensure that the device is power manageable before using 
  this
  + * function.
  + */
  +int acpi_device_set_power(struct acpi_device *device, int state)
   {
  int result = 0;
  acpi_status status = AE_OK;
  @@ -341,6 +349,7 @@ static int __acpi_bus_set_power(struct a
   
  return result;
   }
  +EXPORT_SYMBOL(acpi_device_set_power);
   
   
   int acpi_bus_set_power(acpi_handle handle, int state)
  @@ -359,7 +368,7 @@ int acpi_bus_set_power(acpi_handle handl
  return -ENODEV;
  }
   
  -   return __acpi_bus_set_power(device, state);
  +   return acpi_device_set_power(device, state);
   }
   EXPORT_SYMBOL(acpi_bus_set_power);
   
  @@ -402,7 +411,7 @@ int acpi_bus_update_power(acpi_handle ha
  if (result)
  return result;
   
  -   result = __acpi_bus_set_power(device, state);
  +   result = acpi_device_set_power(device, state);
  if (!result  state_p)
  *state_p = state;
   
  Index: linux/include/acpi/acpi_bus.h
  ===
  --- linux.orig/include/acpi/acpi_bus.h
  +++ linux/include/acpi/acpi_bus.h
  @@ -338,6 +338,7 @@ acpi_status acpi_bus_get_status_handle(a
 unsigned long long *sta);
   int acpi_bus_get_status(struct acpi_device *device);
   int acpi_bus_set_power(acpi_handle handle, int state);
  +int acpi_device_set_power(struct acpi_device *device, int state);
   int acpi_bus_update_power(acpi_handle handle, int *state_p);
   bool acpi_bus_power_manageable(acpi_handle handle);
   bool acpi_bus_can_wakeup(acpi_handle handle);
  @@ -482,6 +483,16 @@ static inline int acpi_pm_device_sleep_w
   }
   #endif
   
  +static inline bool acpi_device_power_manageable(struct acpi_device *adev)
  +{
  +   return adev-flags.power_manageable;
  +}
  +
  +static inline bool acpi_device_can_wakeup(struct acpi_device *adev)
  +{
  +   return adev-wakeup.flags.valid;
  +}
  +
   #else  /* CONFIG_ACPI */
   
   static inline int register_acpi_bus_type(void *bus) { return 0; }
  
 --
 To unsubscribe from this list: send the line unsubscribe linux-acpi in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
 
-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 5/7] ACPI / PM: Provide device PM functions operating on struct acpi_device

2012-10-29 Thread Rafael J. Wysocki 
From: Rafael J. Wysocki 

If the caller of acpi_bus_set_power() already has a pointer to the
struct acpi_device object corresponding to the device in question, it
doesn't make sense for it to go through acpi_bus_get_device(), which
may be costly, because it involves acquiring the global ACPI
namespace mutex.

For this reason, export the function operating on struct acpi_device
objects used internally by acpi_bus_set_power(), so that it may be
called instead of acpi_bus_set_power() in the above case, and change
its name to acpi_device_set_power().

Additionally, introduce two inline wrappers for checking ACPI PM
capabilities of devices represented by struct acpi_device objects.

Signed-off-by: Rafael J. Wysocki 
---
 drivers/acpi/bus.c  |   15 ---
 include/acpi/acpi_bus.h |   11 +++
 2 files changed, 23 insertions(+), 3 deletions(-)

Index: linux/drivers/acpi/bus.c
===
--- linux.orig/drivers/acpi/bus.c
+++ linux/drivers/acpi/bus.c
@@ -257,7 +257,15 @@ static int __acpi_bus_get_power(struct a
 }
 
 
-static int __acpi_bus_set_power(struct acpi_device *device, int state)
+/**
+ * acpi_device_set_power - Set power state of an ACPI device.
+ * @device: Device to set the power state of.
+ * @state: New power state to set.
+ *
+ * Callers must ensure that the device is power manageable before using this
+ * function.
+ */
+int acpi_device_set_power(struct acpi_device *device, int state)
 {
int result = 0;
acpi_status status = AE_OK;
@@ -341,6 +349,7 @@ static int __acpi_bus_set_power(struct a
 
return result;
 }
+EXPORT_SYMBOL(acpi_device_set_power);
 
 
 int acpi_bus_set_power(acpi_handle handle, int state)
@@ -359,7 +368,7 @@ int acpi_bus_set_power(acpi_handle handl
return -ENODEV;
}
 
-   return __acpi_bus_set_power(device, state);
+   return acpi_device_set_power(device, state);
 }
 EXPORT_SYMBOL(acpi_bus_set_power);
 
@@ -402,7 +411,7 @@ int acpi_bus_update_power(acpi_handle ha
if (result)
return result;
 
-   result = __acpi_bus_set_power(device, state);
+   result = acpi_device_set_power(device, state);
if (!result && state_p)
*state_p = state;
 
Index: linux/include/acpi/acpi_bus.h
===
--- linux.orig/include/acpi/acpi_bus.h
+++ linux/include/acpi/acpi_bus.h
@@ -338,6 +338,7 @@ acpi_status acpi_bus_get_status_handle(a
   unsigned long long *sta);
 int acpi_bus_get_status(struct acpi_device *device);
 int acpi_bus_set_power(acpi_handle handle, int state);
+int acpi_device_set_power(struct acpi_device *device, int state);
 int acpi_bus_update_power(acpi_handle handle, int *state_p);
 bool acpi_bus_power_manageable(acpi_handle handle);
 bool acpi_bus_can_wakeup(acpi_handle handle);
@@ -482,6 +483,16 @@ static inline int acpi_pm_device_sleep_w
 }
 #endif
 
+static inline bool acpi_device_power_manageable(struct acpi_device *adev)
+{
+   return adev->flags.power_manageable;
+}
+
+static inline bool acpi_device_can_wakeup(struct acpi_device *adev)
+{
+   return adev->wakeup.flags.valid;
+}
+
 #else  /* CONFIG_ACPI */
 
 static inline int register_acpi_bus_type(void *bus) { return 0; }

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 5/7] ACPI / PM: Provide device PM functions operating on struct acpi_device

2012-10-29 Thread Rafael J. Wysocki 
From: Rafael J. Wysocki rafael.j.wyso...@intel.com

If the caller of acpi_bus_set_power() already has a pointer to the
struct acpi_device object corresponding to the device in question, it
doesn't make sense for it to go through acpi_bus_get_device(), which
may be costly, because it involves acquiring the global ACPI
namespace mutex.

For this reason, export the function operating on struct acpi_device
objects used internally by acpi_bus_set_power(), so that it may be
called instead of acpi_bus_set_power() in the above case, and change
its name to acpi_device_set_power().

Additionally, introduce two inline wrappers for checking ACPI PM
capabilities of devices represented by struct acpi_device objects.

Signed-off-by: Rafael J. Wysocki rafael.j.wyso...@intel.com
---
 drivers/acpi/bus.c  |   15 ---
 include/acpi/acpi_bus.h |   11 +++
 2 files changed, 23 insertions(+), 3 deletions(-)

Index: linux/drivers/acpi/bus.c
===
--- linux.orig/drivers/acpi/bus.c
+++ linux/drivers/acpi/bus.c
@@ -257,7 +257,15 @@ static int __acpi_bus_get_power(struct a
 }
 
 
-static int __acpi_bus_set_power(struct acpi_device *device, int state)
+/**
+ * acpi_device_set_power - Set power state of an ACPI device.
+ * @device: Device to set the power state of.
+ * @state: New power state to set.
+ *
+ * Callers must ensure that the device is power manageable before using this
+ * function.
+ */
+int acpi_device_set_power(struct acpi_device *device, int state)
 {
int result = 0;
acpi_status status = AE_OK;
@@ -341,6 +349,7 @@ static int __acpi_bus_set_power(struct a
 
return result;
 }
+EXPORT_SYMBOL(acpi_device_set_power);
 
 
 int acpi_bus_set_power(acpi_handle handle, int state)
@@ -359,7 +368,7 @@ int acpi_bus_set_power(acpi_handle handl
return -ENODEV;
}
 
-   return __acpi_bus_set_power(device, state);
+   return acpi_device_set_power(device, state);
 }
 EXPORT_SYMBOL(acpi_bus_set_power);
 
@@ -402,7 +411,7 @@ int acpi_bus_update_power(acpi_handle ha
if (result)
return result;
 
-   result = __acpi_bus_set_power(device, state);
+   result = acpi_device_set_power(device, state);
if (!result  state_p)
*state_p = state;
 
Index: linux/include/acpi/acpi_bus.h
===
--- linux.orig/include/acpi/acpi_bus.h
+++ linux/include/acpi/acpi_bus.h
@@ -338,6 +338,7 @@ acpi_status acpi_bus_get_status_handle(a
   unsigned long long *sta);
 int acpi_bus_get_status(struct acpi_device *device);
 int acpi_bus_set_power(acpi_handle handle, int state);
+int acpi_device_set_power(struct acpi_device *device, int state);
 int acpi_bus_update_power(acpi_handle handle, int *state_p);
 bool acpi_bus_power_manageable(acpi_handle handle);
 bool acpi_bus_can_wakeup(acpi_handle handle);
@@ -482,6 +483,16 @@ static inline int acpi_pm_device_sleep_w
 }
 #endif
 
+static inline bool acpi_device_power_manageable(struct acpi_device *adev)
+{
+   return adev-flags.power_manageable;
+}
+
+static inline bool acpi_device_can_wakeup(struct acpi_device *adev)
+{
+   return adev-wakeup.flags.valid;
+}
+
 #else  /* CONFIG_ACPI */
 
 static inline int register_acpi_bus_type(void *bus) { return 0; }

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/