diff mbox

[v3,1/3] ACPI / button: Remove initial lid state notification

Message ID 009785fc9fed68dc2f0496e6d014586d8eaa7cf2.1464775699.git.lv.zheng@intel.com (mailing list archive)
State Accepted, archived
Delegated to: Rafael Wysocki
Headers show

Commit Message

Lv Zheng June 1, 2016, 10:10 a.m. UTC
The _LID control method's initial returning value is not reliable.

The _LID control method is described to return the "current" lid state.
However the word of "current" has ambiguity, many BIOSen return the lid
state upon the last lid notification instead of returning the lid state
upon the last _LID evaluation. There won't be difference when the _LID
control method is evaluated during the runtime, the problem is its initial
returning value. When the BIOSen implement this control method with cached
value, the initial returning value is likely not reliable. There are simply
so many examples retuning "close" as initial lid state (Link 1), sending
this state to the userspace causes suspending right after booting/resuming.

Since the lid state is implemented by the BIOSen, the kernel lid driver has
no idea how it can be correct, this patch stops sending the initial lid
state to the userspace to try to avoid sending the wrong lid state to the
userspace to trigger such kind of wrong suspending. This actually reverts
the following commit introduced for fixing a novell bug (Link 2):
  Commit: 23de5d9ef2a4bbc4f733f58311bcb7cf6239c813
  Subject: ACPI: button: send initial lid state after add and resume

Link 1: https://bugzilla.kernel.org/show_bug.cgi?id=89211
        https://bugzilla.kernel.org/show_bug.cgi?id=106151
        https://bugzilla.kernel.org/show_bug.cgi?id=106941
Link 2: https://bugzilla.novell.com/show_bug.cgi?id=326814
Signed-off-by: Lv Zheng <lv.zheng@intel.com>
---
 drivers/acpi/button.c |    3 ---
 1 file changed, 3 deletions(-)

Comments

Rafael J. Wysocki June 23, 2016, 12:36 a.m. UTC | #1
On Wednesday, June 01, 2016 06:10:34 PM Lv Zheng wrote:
> The _LID control method's initial returning value is not reliable.
> 
> The _LID control method is described to return the "current" lid state.
> However the word of "current" has ambiguity, many BIOSen return the lid
> state upon the last lid notification instead of returning the lid state
> upon the last _LID evaluation. There won't be difference when the _LID
> control method is evaluated during the runtime, the problem is its initial
> returning value. When the BIOSen implement this control method with cached
> value, the initial returning value is likely not reliable. There are simply
> so many examples retuning "close" as initial lid state (Link 1), sending
> this state to the userspace causes suspending right after booting/resuming.
> 
> Since the lid state is implemented by the BIOSen, the kernel lid driver has
> no idea how it can be correct, this patch stops sending the initial lid
> state to the userspace to try to avoid sending the wrong lid state to the
> userspace to trigger such kind of wrong suspending. This actually reverts
> the following commit introduced for fixing a novell bug (Link 2):
>   Commit: 23de5d9ef2a4bbc4f733f58311bcb7cf6239c813
>   Subject: ACPI: button: send initial lid state after add and resume
> 
> Link 1: https://bugzilla.kernel.org/show_bug.cgi?id=89211
>         https://bugzilla.kernel.org/show_bug.cgi?id=106151
>         https://bugzilla.kernel.org/show_bug.cgi?id=106941
> Link 2: https://bugzilla.novell.com/show_bug.cgi?id=326814
> Signed-off-by: Lv Zheng <lv.zheng@intel.com>

I've applied [1-3/3] from this series, but I'm not sure if the last patch
is the latest version.  Can you please verify in linux-next?

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
Lv Zheng June 23, 2016, 12:57 a.m. UTC | #2
SGksIFJhZmFlbA0KDQo+IEZyb206IFJhZmFlbCBKLiBXeXNvY2tpIFttYWlsdG86cmp3QHJqd3lz
b2NraS5uZXRdDQo+IFN1YmplY3Q6IFJlOiBbUEFUQ0ggdjMgMS8zXSBBQ1BJIC8gYnV0dG9uOiBS
ZW1vdmUgaW5pdGlhbCBsaWQgc3RhdGUNCj4gbm90aWZpY2F0aW9uDQo+IA0KPiBPbiBXZWRuZXNk
YXksIEp1bmUgMDEsIDIwMTYgMDY6MTA6MzQgUE0gTHYgWmhlbmcgd3JvdGU6DQo+ID4gVGhlIF9M
SUQgY29udHJvbCBtZXRob2QncyBpbml0aWFsIHJldHVybmluZyB2YWx1ZSBpcyBub3QgcmVsaWFi
bGUuDQo+ID4NCj4gPiBUaGUgX0xJRCBjb250cm9sIG1ldGhvZCBpcyBkZXNjcmliZWQgdG8gcmV0
dXJuIHRoZSAiY3VycmVudCIgbGlkIHN0YXRlLg0KPiA+IEhvd2V2ZXIgdGhlIHdvcmQgb2YgImN1
cnJlbnQiIGhhcyBhbWJpZ3VpdHksIG1hbnkgQklPU2VuIHJldHVybiB0aGUNCj4gbGlkDQo+ID4g
c3RhdGUgdXBvbiB0aGUgbGFzdCBsaWQgbm90aWZpY2F0aW9uIGluc3RlYWQgb2YgcmV0dXJuaW5n
IHRoZSBsaWQgc3RhdGUNCj4gPiB1cG9uIHRoZSBsYXN0IF9MSUQgZXZhbHVhdGlvbi4gVGhlcmUg
d29uJ3QgYmUgZGlmZmVyZW5jZSB3aGVuIHRoZSBfTElEDQo+ID4gY29udHJvbCBtZXRob2QgaXMg
ZXZhbHVhdGVkIGR1cmluZyB0aGUgcnVudGltZSwgdGhlIHByb2JsZW0gaXMgaXRzIGluaXRpYWwN
Cj4gPiByZXR1cm5pbmcgdmFsdWUuIFdoZW4gdGhlIEJJT1NlbiBpbXBsZW1lbnQgdGhpcyBjb250
cm9sIG1ldGhvZCB3aXRoDQo+IGNhY2hlZA0KPiA+IHZhbHVlLCB0aGUgaW5pdGlhbCByZXR1cm5p
bmcgdmFsdWUgaXMgbGlrZWx5IG5vdCByZWxpYWJsZS4gVGhlcmUgYXJlIHNpbXBseQ0KPiA+IHNv
IG1hbnkgZXhhbXBsZXMgcmV0dW5pbmcgImNsb3NlIiBhcyBpbml0aWFsIGxpZCBzdGF0ZSAoTGlu
ayAxKSwgc2VuZGluZw0KPiA+IHRoaXMgc3RhdGUgdG8gdGhlIHVzZXJzcGFjZSBjYXVzZXMgc3Vz
cGVuZGluZyByaWdodCBhZnRlcg0KPiBib290aW5nL3Jlc3VtaW5nLg0KPiA+DQo+ID4gU2luY2Ug
dGhlIGxpZCBzdGF0ZSBpcyBpbXBsZW1lbnRlZCBieSB0aGUgQklPU2VuLCB0aGUga2VybmVsIGxp
ZCBkcml2ZXIgaGFzDQo+ID4gbm8gaWRlYSBob3cgaXQgY2FuIGJlIGNvcnJlY3QsIHRoaXMgcGF0
Y2ggc3RvcHMgc2VuZGluZyB0aGUgaW5pdGlhbCBsaWQNCj4gPiBzdGF0ZSB0byB0aGUgdXNlcnNw
YWNlIHRvIHRyeSB0byBhdm9pZCBzZW5kaW5nIHRoZSB3cm9uZyBsaWQgc3RhdGUgdG8gdGhlDQo+
ID4gdXNlcnNwYWNlIHRvIHRyaWdnZXIgc3VjaCBraW5kIG9mIHdyb25nIHN1c3BlbmRpbmcuIFRo
aXMgYWN0dWFsbHkgcmV2ZXJ0cw0KPiA+IHRoZSBmb2xsb3dpbmcgY29tbWl0IGludHJvZHVjZWQg
Zm9yIGZpeGluZyBhIG5vdmVsbCBidWcgKExpbmsgMik6DQo+ID4gICBDb21taXQ6IDIzZGU1ZDll
ZjJhNGJiYzRmNzMzZjU4MzExYmNiN2NmNjIzOWM4MTMNCj4gPiAgIFN1YmplY3Q6IEFDUEk6IGJ1
dHRvbjogc2VuZCBpbml0aWFsIGxpZCBzdGF0ZSBhZnRlciBhZGQgYW5kIHJlc3VtZQ0KPiA+DQo+
ID4gTGluayAxOiBodHRwczovL2J1Z3ppbGxhLmtlcm5lbC5vcmcvc2hvd19idWcuY2dpP2lkPTg5
MjExDQo+ID4gICAgICAgICBodHRwczovL2J1Z3ppbGxhLmtlcm5lbC5vcmcvc2hvd19idWcuY2dp
P2lkPTEwNjE1MQ0KPiA+ICAgICAgICAgaHR0cHM6Ly9idWd6aWxsYS5rZXJuZWwub3JnL3Nob3df
YnVnLmNnaT9pZD0xMDY5NDENCj4gPiBMaW5rIDI6IGh0dHBzOi8vYnVnemlsbGEubm92ZWxsLmNv
bS9zaG93X2J1Zy5jZ2k/aWQ9MzI2ODE0DQo+ID4gU2lnbmVkLW9mZi1ieTogTHYgWmhlbmcgPGx2
LnpoZW5nQGludGVsLmNvbT4NCj4gDQo+IEkndmUgYXBwbGllZCBbMS0zLzNdIGZyb20gdGhpcyBz
ZXJpZXMsIGJ1dCBJJ20gbm90IHN1cmUgaWYgdGhlIGxhc3QgcGF0Y2gNCj4gaXMgdGhlIGxhdGVz
dCB2ZXJzaW9uLiAgQ2FuIHlvdSBwbGVhc2UgdmVyaWZ5IGluIGxpbnV4LW5leHQ/DQoNCltMdiBa
aGVuZ10gDQpJIGNvbmZpcm1lZCwgdGhlIHBhdGNoIGluIGxpbnV4LW5leHQgaXMgdGhlIGxhdGVz
dCB2ZXJzaW9uLg0KDQpBbmQganVzdCBmb3IgeW91ciBpbmZvcm1hdGlvbi4NCkkgc3RpbGwgaGF2
ZSB0aGluZ3MgdG8gZG8gd2l0aCB0aGUgbGlkIGRyaXZlciB0byBhZGRyZXNzIEJhc3RpZW4gYW5k
IEJlbmphbWluJ3MgcmVxdWVzdDoNCjEuIEFCSSBkb2N1bWVudGF0aW9uDQoyLiBuZXcga2V5IGV2
ZW50IEFCSSBmb3IgYWNwaSBsaWQgY2xvc2UgZXZlbnQgKGZvciBleGFtcGxlLCBQTV9MSUQpDQpU
aGF0J3Mgbm90IHJlbGF0ZWQgdG8gdGhpcyBxdWlyay4NCkFuZCBJJ2xsIGdvIGJhY2sgdG8gdGhh
dCBhZnRlciBJIGZpbmlzaCBteSBjdXJyZW50IHBlbmRpbmcgd29yay4NCg0KVGhhbmtzIGFuZCBi
ZXN0IHJlZ2FyZHMNCi1Mdg0K
--
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
diff mbox

Patch

diff --git a/drivers/acpi/button.c b/drivers/acpi/button.c
index 5c3b091..9863278 100644
--- a/drivers/acpi/button.c
+++ b/drivers/acpi/button.c
@@ -334,8 +334,6 @@  static int acpi_button_resume(struct device *dev)
 	struct acpi_button *button = acpi_driver_data(device);
 
 	button->suspended = false;
-	if (button->type == ACPI_BUTTON_TYPE_LID)
-		return acpi_lid_send_state(device);
 	return 0;
 }
 #endif
@@ -416,7 +414,6 @@  static int acpi_button_add(struct acpi_device *device)
 	if (error)
 		goto err_remove_fs;
 	if (button->type == ACPI_BUTTON_TYPE_LID) {
-		acpi_lid_send_state(device);
 		/*
 		 * This assumes there's only one lid device, or if there are
 		 * more we only care about the last one...