Re: [Resend][PATCH 0/3] ACPI / PM: Patches missing from linux-acpi-2.6/test

[ykzhao <yakui.zhao@intel.com>]

On Tue, 2010-12-14 at 05:25 +0800, Rafael J. Wysocki wrote:
> On Monday, December 13, 2010, ykzhao wrote:
> > On Sun, 2010-12-12 at 06:39 +0800, Rafael J. Wysocki wrote:
> > > Hi Len,
> > > 
> > > The following three patches seem to have been dropped from your 'test' branch.
> > > 
> > > If that happened by accident, please reapply.  Otherwise, please let me know
> > > what's wrong with the patches so that I can fix them.
> > > 
> > > [1/3] - Make fujitsu_laptop use acpi_bus_update_power() instead of
> > >         acpi_bus_get_power() which is unsafe.
> > 
> > It seems that the function of acpi_bus_update_power not only obtains
> > the current power state, but also set the corresponding power state.
> > Right?
> 
> Yes, it does.
> 
> > If the device reports the bogus power state, maybe we will set the
> > incorrect power state for the corresponding device when using the
> > function of acpi_bus_update_power instead of acpi_bus_get_power.
> 
> Please actually look at acpi_bus_get_power() (being removed by [2/3]) and note
> that it _also_ modifies device->power.state (it doesn't return the state, actually),
> so if the returned state is really bogus, we'll have a mismatch between
> device->power.state and the real state of the device.  This cannot be good.

The device->power.state is also updated in the function of
acpi_bus_get_power. But it won't try to call the _PSC or _ON/OFF method
to really change the corresponding power state. It only reports the
corresponding power state. 

> In the case of acpi_bus_update_power() we at least _try_ to keep the two things
> in sync.

Yes. I agree that the acpi_bus_update_power can always assure that the
two things are in sync state. But some systems will report the bogus
power state although we already set another power state(For example:
Maybe it reports that it is in D3 state because of bogus BIOS code). In
such case if we try to turn off the corresponding power resource by
using _PSC/_ON/_OFF method, maybe we will cut off the power supply for
these devices.


> Note, this is _essentially_ important for power resources (if
> acpi_bus_get_power() is used, the refcounts are _guaranteed_ not to be in sync
> with device->power.state in some situations).
> 
> > In such case maybe the device can't work well. 
> > 
> > The bogus power state is reported for some devices on some laptops. For
> > example: 
> >     http://bugzilla.kernel.org/show_bug.cgi?id=8049
> >     http://bugzilla.kernel.org/show_bug.cgi?id=11000
> 
> These bugs are about acpi_bus_set_power() doing the acpi_bus_get_power()
> before setting the state, which is wrong and is being removed by my previous
> patches (now in the Len's tree).

It seems that another point is missed in previous patch. 
   Before the reworking patch of power resource, the force_power_state
flag is used when setting the corresponding power state for some ACPI
devices(For example: Fan). This flag will still force to call the
_PSC/_ON/_OFF method even when it is already the same as the target
state. Maybe this is to workaround the BIOS issue that the power state
is not reported correctly first time.

   Not sure whether my above understanding is reasonable.

Thanks.
    Yakui
   
> 
> Thanks,
> Rafael

--
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