diff mbox

[v2,4/6] ACPI / osi: Fix default _OSI(Darwin) support

Message ID e8ea6e8841adce9e10e5bd0b278b47a7c799c276.1461736123.git.lv.zheng@intel.com (mailing list archive)
State Superseded, archived
Headers show

Commit Message

Lv Zheng April 27, 2016, 8:54 a.m. UTC
From: Chen Yu <yu.c.chen@intel.com>

The following commit always reports positive value when Apple hardware
queries _OSI("Darwin"):
 Commit: 7bc5a2bad0b8d9d1ac9f7b8b33150e4ddf197334
 Subject: ACPI: Support _OSI("Darwin") correctly
However since this implementation places the judgement in runtime, it
breaks acpi_osi=!Darwin and cannot return unsupported for _OSI("WinXXX")
invoked before invoking _OSI("Darwin").

This patch fixes the issues by reverting the wrong support and implementing
the default behavior of _OSI("Darwin")/_OSI("WinXXX") on Apple hardware via
DMI matching.

Fixes: 7bc5a2bad0b8 ("ACPI: Support _OSI("Darwin") correctly")
Cc: <stable@vger.kernel.org> # 3.18+
Link: https://bugzilla.kernel.org/show_bug.cgi?id=92111
Reported-by: Lukas Wunner <lukas@wunner.de>
Signed-off-by: Chen Yu <yu.c.chen@intel.com>
Signed-off-by: Lv Zheng <lv.zheng@intel.com>
---
 drivers/acpi/blacklist.c |   23 ++++++++++++++++++
 drivers/acpi/osl.c       |   58 ++++++++++++++++++++++++++++++++++++++++------
 include/linux/acpi.h     |    1 +
 3 files changed, 75 insertions(+), 7 deletions(-)

Comments

Chen Yu April 27, 2016, 9:45 a.m. UTC | #1
Hi Lv,

> -----Original Message-----
> From: Zheng, Lv
> Sent: Wednesday, April 27, 2016 4:55 PM
> To: Wysocki, Rafael J; Rafael J. Wysocki; Brown, Len
> Cc: Zheng, Lv; Lv Zheng; linux-kernel@vger.kernel.org; linux-
> acpi@vger.kernel.org; Chen, Yu C
> Subject: [PATCH v2 4/6] ACPI / osi: Fix default _OSI(Darwin) support
> 
> From: Chen Yu <yu.c.chen@intel.com>
> 
> The following commit always reports positive value when Apple hardware
> queries _OSI("Darwin"):
>  Commit: 7bc5a2bad0b8d9d1ac9f7b8b33150e4ddf197334
>  Subject: ACPI: Support _OSI("Darwin") correctly However since this
> implementation places the judgement in runtime, it breaks acpi_osi=!Darwin
> and cannot return unsupported for _OSI("WinXXX") invoked before invoking
> _OSI("Darwin").
> 
> This patch fixes the issues by reverting the wrong support and implementing
> the default behavior of _OSI("Darwin")/_OSI("WinXXX") on Apple hardware via
> DMI matching.
> 
> Fixes: 7bc5a2bad0b8 ("ACPI: Support _OSI("Darwin") correctly")
> Cc: <stable@vger.kernel.org> # 3.18+
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=92111
> Reported-by: Lukas Wunner <lukas@wunner.de>
> Signed-off-by: Chen Yu <yu.c.chen@intel.com>
> Signed-off-by: Lv Zheng <lv.zheng@intel.com>
> ---
>  drivers/acpi/blacklist.c |   23 ++++++++++++++++++
>  drivers/acpi/osl.c       |   58 ++++++++++++++++++++++++++++++++++++++++---
> ---
>  include/linux/acpi.h     |    1 +
>  3 files changed, 75 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/acpi/blacklist.c b/drivers/acpi/blacklist.c index
> 96809cd..e947d3e 100644
> --- a/drivers/acpi/blacklist.c
> +++ b/drivers/acpi/blacklist.c
> @@ -133,6 +133,11 @@ int __init acpi_blacklisted(void)
>  	return blacklisted;
>  }
>  #ifdef CONFIG_DMI
> +static int __init dmi_enable_osi_darwin(const struct dmi_system_id *d)
> +{
> +	acpi_dmi_osi_darwin(1, d);	/* enable */
> +	return 0;
> +}
>  static int __init dmi_enable_osi_linux(const struct dmi_system_id *d)  {
>  	acpi_dmi_osi_linux(1, d);	/* enable */
> @@ -331,6 +336,24 @@ static struct dmi_system_id acpi_osi_dmi_table[]
> __initdata = {
>  		},
>  	},
> 
> +	/*
> +	 * Enable _OSI("Darwin") for all apple platforms.
> +	 */
> +	{
> +	.callback = dmi_enable_osi_darwin,
> +	.ident = "Apple hardware",
> +	.matches = {
> +		     DMI_MATCH(DMI_SYS_VENDOR, "Apple INC."),
> +		},
> +	},
> +	{
> +	.callback = dmi_enable_osi_darwin,
> +	.ident = "Apple hardware",
> +	.matches = {
> +		     DMI_MATCH(DMI_SYS_VENDOR, "Apple Computer, INC."),
> +		},
> +	},
> +
The  vendor id should be 'Apple Inc.' and 'Apple Computer, Inc.' instead.

thanks,
yu

--
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
Rafael J. Wysocki April 27, 2016, 9:24 p.m. UTC | #2
On Wednesday, April 27, 2016 09:45:21 AM Chen, Yu C wrote:
> Hi Lv,
> 
> > -----Original Message-----
> > From: Zheng, Lv
> > Sent: Wednesday, April 27, 2016 4:55 PM
> > To: Wysocki, Rafael J; Rafael J. Wysocki; Brown, Len
> > Cc: Zheng, Lv; Lv Zheng; linux-kernel@vger.kernel.org; linux-
> > acpi@vger.kernel.org; Chen, Yu C
> > Subject: [PATCH v2 4/6] ACPI / osi: Fix default _OSI(Darwin) support
> > 
> > From: Chen Yu <yu.c.chen@intel.com>
> > 
> > The following commit always reports positive value when Apple hardware
> > queries _OSI("Darwin"):
> >  Commit: 7bc5a2bad0b8d9d1ac9f7b8b33150e4ddf197334
> >  Subject: ACPI: Support _OSI("Darwin") correctly However since this
> > implementation places the judgement in runtime, it breaks acpi_osi=!Darwin
> > and cannot return unsupported for _OSI("WinXXX") invoked before invoking
> > _OSI("Darwin").
> > 
> > This patch fixes the issues by reverting the wrong support and implementing
> > the default behavior of _OSI("Darwin")/_OSI("WinXXX") on Apple hardware via
> > DMI matching.
> > 
> > Fixes: 7bc5a2bad0b8 ("ACPI: Support _OSI("Darwin") correctly")
> > Cc: <stable@vger.kernel.org> # 3.18+
> > Link: https://bugzilla.kernel.org/show_bug.cgi?id=92111
> > Reported-by: Lukas Wunner <lukas@wunner.de>
> > Signed-off-by: Chen Yu <yu.c.chen@intel.com>
> > Signed-off-by: Lv Zheng <lv.zheng@intel.com>
> > ---
> >  drivers/acpi/blacklist.c |   23 ++++++++++++++++++
> >  drivers/acpi/osl.c       |   58 ++++++++++++++++++++++++++++++++++++++++---
> > ---
> >  include/linux/acpi.h     |    1 +
> >  3 files changed, 75 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/acpi/blacklist.c b/drivers/acpi/blacklist.c index
> > 96809cd..e947d3e 100644
> > --- a/drivers/acpi/blacklist.c
> > +++ b/drivers/acpi/blacklist.c
> > @@ -133,6 +133,11 @@ int __init acpi_blacklisted(void)
> >  	return blacklisted;
> >  }
> >  #ifdef CONFIG_DMI
> > +static int __init dmi_enable_osi_darwin(const struct dmi_system_id *d)
> > +{
> > +	acpi_dmi_osi_darwin(1, d);	/* enable */
> > +	return 0;
> > +}
> >  static int __init dmi_enable_osi_linux(const struct dmi_system_id *d)  {
> >  	acpi_dmi_osi_linux(1, d);	/* enable */
> > @@ -331,6 +336,24 @@ static struct dmi_system_id acpi_osi_dmi_table[]
> > __initdata = {
> >  		},
> >  	},
> > 
> > +	/*
> > +	 * Enable _OSI("Darwin") for all apple platforms.
> > +	 */
> > +	{
> > +	.callback = dmi_enable_osi_darwin,
> > +	.ident = "Apple hardware",
> > +	.matches = {
> > +		     DMI_MATCH(DMI_SYS_VENDOR, "Apple INC."),
> > +		},
> > +	},
> > +	{
> > +	.callback = dmi_enable_osi_darwin,
> > +	.ident = "Apple hardware",
> > +	.matches = {
> > +		     DMI_MATCH(DMI_SYS_VENDOR, "Apple Computer, INC."),
> > +		},
> > +	},
> > +
> The  vendor id should be 'Apple Inc.' and 'Apple Computer, Inc.' instead.

If this is the only problem with this patch, I can fix it up.  No need to resend.

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 April 28, 2016, 2:30 a.m. UTC | #3
SGksDQoNCj4gRnJvbTogbGludXgtYWNwaS1vd25lckB2Z2VyLmtlcm5lbC5vcmcgW21haWx0bzps
aW51eC1hY3BpLQ0KPiBvd25lckB2Z2VyLmtlcm5lbC5vcmddIE9uIEJlaGFsZiBPZiBSYWZhZWwg
Si4gV3lzb2NraQ0KPiBTdWJqZWN0OiBSZTogW1BBVENIIHYyIDQvNl0gQUNQSSAvIG9zaTogRml4
IGRlZmF1bHQgX09TSShEYXJ3aW4pIHN1cHBvcnQNCj4gDQo+IE9uIFdlZG5lc2RheSwgQXByaWwg
MjcsIDIwMTYgMDk6NDU6MjEgQU0gQ2hlbiwgWXUgQyB3cm90ZToNCj4gPiBIaSBMdiwNCj4gPg0K
PiA+ID4gRnJvbTogWmhlbmcsIEx2DQo+ID4gPiBTdWJqZWN0OiBbUEFUQ0ggdjIgNC82XSBBQ1BJ
IC8gb3NpOiBGaXggZGVmYXVsdCBfT1NJKERhcndpbikgc3VwcG9ydA0KPiA+ID4NCj4gPiA+IEZy
b206IENoZW4gWXUgPHl1LmMuY2hlbkBpbnRlbC5jb20+DQo+ID4gPg0KPiA+ID4gVGhlIGZvbGxv
d2luZyBjb21taXQgYWx3YXlzIHJlcG9ydHMgcG9zaXRpdmUgdmFsdWUgd2hlbiBBcHBsZSBoYXJk
d2FyZQ0KPiA+ID4gcXVlcmllcyBfT1NJKCJEYXJ3aW4iKToNCj4gPiA+ICBDb21taXQ6IDdiYzVh
MmJhZDBiOGQ5ZDFhYzlmN2I4YjMzMTUwZTRkZGYxOTczMzQNCj4gPiA+ICBTdWJqZWN0OiBBQ1BJ
OiBTdXBwb3J0IF9PU0koIkRhcndpbiIpIGNvcnJlY3RseSBIb3dldmVyIHNpbmNlIHRoaXMNCj4g
PiA+IGltcGxlbWVudGF0aW9uIHBsYWNlcyB0aGUganVkZ2VtZW50IGluIHJ1bnRpbWUsIGl0IGJy
ZWFrcw0KPiBhY3BpX29zaT0hRGFyd2luDQo+ID4gPiBhbmQgY2Fubm90IHJldHVybiB1bnN1cHBv
cnRlZCBmb3IgX09TSSgiV2luWFhYIikgaW52b2tlZCBiZWZvcmUgaW52b2tpbmcNCj4gPiA+IF9P
U0koIkRhcndpbiIpLg0KPiA+ID4NCj4gPiA+IFRoaXMgcGF0Y2ggZml4ZXMgdGhlIGlzc3VlcyBi
eSByZXZlcnRpbmcgdGhlIHdyb25nIHN1cHBvcnQgYW5kIGltcGxlbWVudGluZw0KPiA+ID4gdGhl
IGRlZmF1bHQgYmVoYXZpb3Igb2YgX09TSSgiRGFyd2luIikvX09TSSgiV2luWFhYIikgb24gQXBw
bGUgaGFyZHdhcmUNCj4gdmlhDQo+ID4gPiBETUkgbWF0Y2hpbmcuDQo+ID4gPg0KPiA+ID4gRml4
ZXM6IDdiYzVhMmJhZDBiOCAoIkFDUEk6IFN1cHBvcnQgX09TSSgiRGFyd2luIikgY29ycmVjdGx5
IikNCj4gPiA+IENjOiA8c3RhYmxlQHZnZXIua2VybmVsLm9yZz4gIyAzLjE4Kw0KPiA+ID4gTGlu
azogaHR0cHM6Ly9idWd6aWxsYS5rZXJuZWwub3JnL3Nob3dfYnVnLmNnaT9pZD05MjExMQ0KPiA+
ID4gUmVwb3J0ZWQtYnk6IEx1a2FzIFd1bm5lciA8bHVrYXNAd3VubmVyLmRlPg0KPiA+ID4gU2ln
bmVkLW9mZi1ieTogQ2hlbiBZdSA8eXUuYy5jaGVuQGludGVsLmNvbT4NCj4gPiA+IFNpZ25lZC1v
ZmYtYnk6IEx2IFpoZW5nIDxsdi56aGVuZ0BpbnRlbC5jb20+DQo+ID4gPiAtLS0NCj4gPiA+ICBk
cml2ZXJzL2FjcGkvYmxhY2tsaXN0LmMgfCAgIDIzICsrKysrKysrKysrKysrKysrKw0KPiA+ID4g
IGRyaXZlcnMvYWNwaS9vc2wuYyAgICAgICB8ICAgNTgNCj4gKysrKysrKysrKysrKysrKysrKysr
KysrKysrKysrKysrKysrKysrKy0tLQ0KPiA+ID4gLS0tDQo+ID4gPiAgaW5jbHVkZS9saW51eC9h
Y3BpLmggICAgIHwgICAgMSArDQo+ID4gPiAgMyBmaWxlcyBjaGFuZ2VkLCA3NSBpbnNlcnRpb25z
KCspLCA3IGRlbGV0aW9ucygtKQ0KPiA+ID4NCj4gPiA+IGRpZmYgLS1naXQgYS9kcml2ZXJzL2Fj
cGkvYmxhY2tsaXN0LmMgYi9kcml2ZXJzL2FjcGkvYmxhY2tsaXN0LmMgaW5kZXgNCj4gPiA+IDk2
ODA5Y2QuLmU5NDdkM2UgMTAwNjQ0DQo+ID4gPiAtLS0gYS9kcml2ZXJzL2FjcGkvYmxhY2tsaXN0
LmMNCj4gPiA+ICsrKyBiL2RyaXZlcnMvYWNwaS9ibGFja2xpc3QuYw0KPiA+ID4gQEAgLTEzMyw2
ICsxMzMsMTEgQEAgaW50IF9faW5pdCBhY3BpX2JsYWNrbGlzdGVkKHZvaWQpDQo+ID4gPiAgCXJl
dHVybiBibGFja2xpc3RlZDsNCj4gPiA+ICB9DQo+ID4gPiAgI2lmZGVmIENPTkZJR19ETUkNCj4g
PiA+ICtzdGF0aWMgaW50IF9faW5pdCBkbWlfZW5hYmxlX29zaV9kYXJ3aW4oY29uc3Qgc3RydWN0
IGRtaV9zeXN0ZW1faWQgKmQpDQo+ID4gPiArew0KPiA+ID4gKwlhY3BpX2RtaV9vc2lfZGFyd2lu
KDEsIGQpOwkvKiBlbmFibGUgKi8NCj4gPiA+ICsJcmV0dXJuIDA7DQo+ID4gPiArfQ0KPiA+ID4g
IHN0YXRpYyBpbnQgX19pbml0IGRtaV9lbmFibGVfb3NpX2xpbnV4KGNvbnN0IHN0cnVjdCBkbWlf
c3lzdGVtX2lkICpkKSAgew0KPiA+ID4gIAlhY3BpX2RtaV9vc2lfbGludXgoMSwgZCk7CS8qIGVu
YWJsZSAqLw0KPiA+ID4gQEAgLTMzMSw2ICszMzYsMjQgQEAgc3RhdGljIHN0cnVjdCBkbWlfc3lz
dGVtX2lkIGFjcGlfb3NpX2RtaV90YWJsZVtdDQo+ID4gPiBfX2luaXRkYXRhID0gew0KPiA+ID4g
IAkJfSwNCj4gPiA+ICAJfSwNCj4gPiA+DQo+ID4gPiArCS8qDQo+ID4gPiArCSAqIEVuYWJsZSBf
T1NJKCJEYXJ3aW4iKSBmb3IgYWxsIGFwcGxlIHBsYXRmb3Jtcy4NCj4gPiA+ICsJICovDQo+ID4g
PiArCXsNCj4gPiA+ICsJLmNhbGxiYWNrID0gZG1pX2VuYWJsZV9vc2lfZGFyd2luLA0KPiA+ID4g
KwkuaWRlbnQgPSAiQXBwbGUgaGFyZHdhcmUiLA0KPiA+ID4gKwkubWF0Y2hlcyA9IHsNCj4gPiA+
ICsJCSAgICAgRE1JX01BVENIKERNSV9TWVNfVkVORE9SLCAiQXBwbGUgSU5DLiIpLA0KPiA+ID4g
KwkJfSwNCj4gPiA+ICsJfSwNCj4gPiA+ICsJew0KPiA+ID4gKwkuY2FsbGJhY2sgPSBkbWlfZW5h
YmxlX29zaV9kYXJ3aW4sDQo+ID4gPiArCS5pZGVudCA9ICJBcHBsZSBoYXJkd2FyZSIsDQo+ID4g
PiArCS5tYXRjaGVzID0gew0KPiA+ID4gKwkJICAgICBETUlfTUFUQ0goRE1JX1NZU19WRU5ET1Is
ICJBcHBsZSBDb21wdXRlciwgSU5DLiIpLA0KPiA+ID4gKwkJfSwNCj4gPiA+ICsJfSwNCj4gPiA+
ICsNCj4gPiBUaGUgIHZlbmRvciBpZCBzaG91bGQgYmUgJ0FwcGxlIEluYy4nIGFuZCAnQXBwbGUg
Q29tcHV0ZXIsIEluYy4nIGluc3RlYWQuDQo+IA0KPiBJZiB0aGlzIGlzIHRoZSBvbmx5IHByb2Js
ZW0gd2l0aCB0aGlzIHBhdGNoLCBJIGNhbiBmaXggaXQgdXAuICBObyBuZWVkIHRvIHJlc2VuZC4N
CltMdiBaaGVuZ10gDQpUaGlzIGlzIHRoZSBvbmx5IHByb2JsZW0uDQpUaGFua3MgZm9yIHRoZSBo
ZWxwLg0KDQpCZXN0IHJlZ2FyZHMNCi1Mdg0KDQo+IA0KPiBUaGFua3MsDQo+IFJhZmFlbA0KPiAN
Cj4gLS0NCj4gVG8gdW5zdWJzY3JpYmUgZnJvbSB0aGlzIGxpc3Q6IHNlbmQgdGhlIGxpbmUgInVu
c3Vic2NyaWJlIGxpbnV4LWFjcGkiIGluDQo+IHRoZSBib2R5IG9mIGEgbWVzc2FnZSB0byBtYWpv
cmRvbW9Admdlci5rZXJuZWwub3JnDQo+IE1vcmUgbWFqb3Jkb21vIGluZm8gYXQgIGh0dHA6Ly92
Z2VyLmtlcm5lbC5vcmcvbWFqb3Jkb21vLWluZm8uaHRtbA0K
--
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
Chen Yu April 28, 2016, 7:54 a.m. UTC | #4
Hi 


> -----Original Message-----

> From: Rafael J. Wysocki [mailto:rjw@rjwysocki.net]

> Sent: Thursday, April 28, 2016 5:25 AM

> To: Chen, Yu C; Zheng, Lv

> Cc: Lv Zheng; linux-kernel@vger.kernel.org; linux-acpi@vger.kernel.org;

> Wysocki, Rafael J; Brown, Len

> Subject: Re: [PATCH v2 4/6] ACPI / osi: Fix default _OSI(Darwin) support

> 

> On Wednesday, April 27, 2016 09:45:21 AM Chen, Yu C wrote:

> > Hi Lv,

> >

> > > -----Original Message-----

> > > From: Zheng, Lv

> > > Sent: Wednesday, April 27, 2016 4:55 PM

> > > To: Wysocki, Rafael J; Rafael J. Wysocki; Brown, Len

> > > Cc: Zheng, Lv; Lv Zheng; linux-kernel@vger.kernel.org; linux-

> > > acpi@vger.kernel.org; Chen, Yu C

> > > Subject: [PATCH v2 4/6] ACPI / osi: Fix default _OSI(Darwin) support

> > >

> > > From: Chen Yu <yu.c.chen@intel.com>

> > >

> > > The following commit always reports positive value when Apple

> > > hardware queries _OSI("Darwin"):

> > >  Commit: 7bc5a2bad0b8d9d1ac9f7b8b33150e4ddf197334

> > >  Subject: ACPI: Support _OSI("Darwin") correctly However since this

> > > implementation places the judgement in runtime, it breaks

> > > acpi_osi=!Darwin and cannot return unsupported for _OSI("WinXXX")

> > > invoked before invoking _OSI("Darwin").

> > >

> > > This patch fixes the issues by reverting the wrong support and

> > > implementing the default behavior of _OSI("Darwin")/_OSI("WinXXX")

> > > on Apple hardware via DMI matching.

> > >

> > > Fixes: 7bc5a2bad0b8 ("ACPI: Support _OSI("Darwin") correctly")

> > > Cc: <stable@vger.kernel.org> # 3.18+

> > > Link: https://bugzilla.kernel.org/show_bug.cgi?id=92111

> > > Reported-by: Lukas Wunner <lukas@wunner.de>

> > > Signed-off-by: Chen Yu <yu.c.chen@intel.com>

> > > Signed-off-by: Lv Zheng <lv.zheng@intel.com>

> > The  vendor id should be 'Apple Inc.' and 'Apple Computer, Inc.' instead.

> 

> If this is the only problem with this patch, I can fix it up.  No need to resend.

Yes, this is the only problem, please help fix it, thanks!
> 

> Thanks,

> Rafael
Lv Zheng April 29, 2016, 2:07 a.m. UTC | #5
SGksIFJhZmFlbA0KDQo+IEZyb206IGxpbnV4LWFjcGktb3duZXJAdmdlci5rZXJuZWwub3JnIFtt
YWlsdG86bGludXgtYWNwaS0NCj4gb3duZXJAdmdlci5rZXJuZWwub3JnXSBPbiBCZWhhbGYgT2Yg
WmhlbmcsIEx2DQo+IFN1YmplY3Q6IFJFOiBbUEFUQ0ggdjIgNC82XSBBQ1BJIC8gb3NpOiBGaXgg
ZGVmYXVsdCBfT1NJKERhcndpbikgc3VwcG9ydA0KPiANCg0KW0x2IFpoZW5nXSANClNraXAuDQoN
Cj4gPiA+ID4gQEAgLTMzMSw2ICszMzYsMjQgQEAgc3RhdGljIHN0cnVjdCBkbWlfc3lzdGVtX2lk
IGFjcGlfb3NpX2RtaV90YWJsZVtdDQo+ID4gPiA+IF9faW5pdGRhdGEgPSB7DQo+ID4gPiA+ICAJ
CX0sDQo+ID4gPiA+ICAJfSwNCj4gPiA+ID4NCj4gPiA+ID4gKwkvKg0KPiA+ID4gPiArCSAqIEVu
YWJsZSBfT1NJKCJEYXJ3aW4iKSBmb3IgYWxsIGFwcGxlIHBsYXRmb3Jtcy4NCj4gPiA+ID4gKwkg
Ki8NCj4gPiA+ID4gKwl7DQo+ID4gPiA+ICsJLmNhbGxiYWNrID0gZG1pX2VuYWJsZV9vc2lfZGFy
d2luLA0KPiA+ID4gPiArCS5pZGVudCA9ICJBcHBsZSBoYXJkd2FyZSIsDQo+ID4gPiA+ICsJLm1h
dGNoZXMgPSB7DQo+ID4gPiA+ICsJCSAgICAgRE1JX01BVENIKERNSV9TWVNfVkVORE9SLCAiQXBw
bGUgSU5DLiIpLA0KPiA+ID4gPiArCQl9LA0KPiA+ID4gPiArCX0sDQo+ID4gPiA+ICsJew0KPiA+
ID4gPiArCS5jYWxsYmFjayA9IGRtaV9lbmFibGVfb3NpX2RhcndpbiwNCj4gPiA+ID4gKwkuaWRl
bnQgPSAiQXBwbGUgaGFyZHdhcmUiLA0KPiA+ID4gPiArCS5tYXRjaGVzID0gew0KPiA+ID4gPiAr
CQkgICAgIERNSV9NQVRDSChETUlfU1lTX1ZFTkRPUiwgIkFwcGxlIENvbXB1dGVyLCBJTkMuIiks
DQo+ID4gPiA+ICsJCX0sDQo+ID4gPiA+ICsJfSwNCj4gPiA+ID4gKw0KPiA+ID4gVGhlICB2ZW5k
b3IgaWQgc2hvdWxkIGJlICdBcHBsZSBJbmMuJyBhbmQgJ0FwcGxlIENvbXB1dGVyLCBJbmMuJyBp
bnN0ZWFkLg0KPiA+DQo+ID4gSWYgdGhpcyBpcyB0aGUgb25seSBwcm9ibGVtIHdpdGggdGhpcyBw
YXRjaCwgSSBjYW4gZml4IGl0IHVwLiAgTm8gbmVlZCB0byByZXNlbmQuDQo+IFtMdiBaaGVuZ10N
Cj4gVGhpcyBpcyB0aGUgb25seSBwcm9ibGVtLg0KPiBUaGFua3MgZm9yIHRoZSBoZWxwLg0KPiAN
Cg0KW0x2IFpoZW5nXSANCkkganVzdCBzZW50IFVQREFURSBvZiBQQVRDSCA0LzYgYW5kIFBBVENI
IDYvNiB0byB0aGUgbWFpbGluZyBsaXN0IHdpdGggdGhpcyBjb3JyZWN0ZWQuDQpJIHdhcyBob3Bp
bmcgdGhleSBjb3VsZCB1cGRhdGUgcGF0Y2h3b3JrIGNvbnRlbnQgc28gdGhhdCB0aGUgQnVnemls
bGEgcmVwb3J0ZXJzIG1pZ2h0IHVzZSB0aGUgdXBkYXRlZCBwYXRjaGVzIGZvciBjb25maXJtYXRp
b24uDQpUaGUgTWVzc2FnZS1JZChzKSBvZiB0aGUgMiBwYXRjaGVzIHdlcmUga2VwdCBhcyBzYW1l
IGFzIHRoZSBvbGQgb25lcy4NCg0KVGhhbmtzIGFuZCBiZXN0IHJlZ2FyZHMNCi1Mdg0K
--
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
Lukas Wunner April 29, 2016, 1:01 p.m. UTC | #6
Hi Lv Zheng,

On Fri, Apr 29, 2016 at 02:07:53AM +0000, Zheng, Lv wrote:
> I just sent UPDATE of PATCH 4/6 and PATCH 6/6 to the mailing list with this
> corrected.
> I was hoping they could update patchwork content so that the Bugzilla
> reporters might use the updated patches for confirmation.
> The Message-Id(s) of the 2 patches were kept as same as the old ones.

As promised on Bugzilla I've tested v2 of this series (with the "INC"
manually fixed up) on a MacBookPro9,1.

I only reviewed the patches in a superficial fashion, but one issue I've
noticed is that

#define pr_fmt(fmt) "ACPI: " fmt

is missing in osi.c (patch [6/6]).

Without command line arguments, Linux responds yay to _OSI("Darwin")
and nothing else and the Thunderbolt controller is powered up.

With "acpi_osi=!Darwin", Linux responds nay to everything and the
Thunderbolt controller is powered down.

So it seems to work as intended. If you want me to test anything else
please let me know.

Thanks,

Lukas
--
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 May 3, 2016, 1:56 a.m. UTC | #7
Hi, Lukas

First, thanks for the test. :)

> From: Lukas Wunner [mailto:lukas@wunner.de]
> Subject: Re: [PATCH v2 4/6] ACPI / osi: Fix default _OSI(Darwin) support
> 
> Hi Lv Zheng,
> 
> On Fri, Apr 29, 2016 at 02:07:53AM +0000, Zheng, Lv wrote:
> > I just sent UPDATE of PATCH 4/6 and PATCH 6/6 to the mailing list with this
> > corrected.
> > I was hoping they could update patchwork content so that the Bugzilla
> > reporters might use the updated patches for confirmation.
> > The Message-Id(s) of the 2 patches were kept as same as the old ones.
> 
> As promised on Bugzilla I've tested v2 of this series (with the "INC"
> manually fixed up) on a MacBookPro9,1.
> 
> I only reviewed the patches in a superficial fashion, but one issue I've
> noticed is that
> 
> #define pr_fmt(fmt) "ACPI: " fmt
> 
> is missing in osi.c (patch [6/6]).
[Lv Zheng] 

This is a bug of internal.h.
I've been suffering from this in several my patches.
And people working for ACPI subsystem have been suffering from this for decades...
Sometimes we have to use printk(KERN_xxx PREFIX) here while checkpatch.pl complains coding style issues.
You comment makes it a good chance to escalate this issue to the maintainers.

This is a bug of internal.h because:
1. checkpatch.pl enforces pr_xxx() instead of printk(KERN_xxx...)
2. pr_xxx() macro is meant to eliminate KERN_xxx prefixes.
3. kernel provides pr_fmt(fmt) to allow drivers to add their own prefixes when pr_xxx() macros are used.
4. internal.h defines PREFIX for all ACPI drivers but doesn't provide the mean for the drivers to hide this prefix in pr_xxx() macros.
So this becomes a trap where developers can easily make mistakes.

Concluded from the point 1, 2, 3, the following code is wrong:
In drivers/acpi/osl.c:
		printk(KERN_ERR PREFIX "Cannot map memory that high\n");
printk(KERN_ERR PREFIX should be replaced by pr_fmt(

Concluded from the point 2, 3, the following code is wrong:
In drivers/acpi/osl.c:
	pr_info("ACPI: static SSDT installation disabled\n");
		pr_debug(PREFIX "%s: map reset_reg status %d\n", __func__, rv);
The PREFIX should be handled by pr_fmt().

Concluded from the point 3, 4, the following code is wrong:
In drivers/acpi/tables.c:
#define pr_fmt(fmt) "ACPI: " fmt
This should be handled by internal.h by default.

So in fact, my patch is written correctly on top of the assumption that the PREFIX have already been handled by internal.h not osi.c...

IMO, there are several ways to fix this long term issue:
1. Fix it with post-override:
This might be done in internal.h
We may
#ifndef PREFIX
#define PREFIX	"ACPI: "
#endif
#undef pr_fmt
#define pr_fmt(fmt)	PREFIX fmt
This enforces ec.c to be changed to define PREFIX instead of pr_fmt() which is not developers friendly as developers are trained to use pre-override for pr_fmt().

2. Fix it using pre-override:
The above fix looks a little bit confusing because the pr_fmt() is meant to be defined before including <linux/printk.h>
In internal.h:
#ifndef pr_fmt
#define pr_fmt(fmt) "ACPI: " fmt
#endif

#include <linux/xxxx.h>
...

In drivers/acpi/xxx.c, making internal.h the first one included by these files, so that they can override pr_fmt.
For example, in ec.c:

#define pr_fmt(fmt) "ACPI: EC: " fmt
#include "internal.h"
#include <linux/xxx.h>

It's hard for me to make a decision.
So I probably still have to work with this code quality issue and refresh this patchset.
Possibly I will still make same mistakes in the future.
Anyway, this is a chance for me to add your "Reported-and-tested-by:". :-)

> 
> Without command line arguments, Linux responds yay to _OSI("Darwin")
> and nothing else and the Thunderbolt controller is powered up.
> 
> With "acpi_osi=!Darwin", Linux responds nay to everything and the
> Thunderbolt controller is powered down.
> 
> So it seems to work as intended. If you want me to test anything else
> please let me know.
[Lv Zheng] 
Sounds good.
Thanks for the test.

Thanks and best regards
-Lv
--
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/blacklist.c b/drivers/acpi/blacklist.c
index 96809cd..e947d3e 100644
--- a/drivers/acpi/blacklist.c
+++ b/drivers/acpi/blacklist.c
@@ -133,6 +133,11 @@  int __init acpi_blacklisted(void)
 	return blacklisted;
 }
 #ifdef CONFIG_DMI
+static int __init dmi_enable_osi_darwin(const struct dmi_system_id *d)
+{
+	acpi_dmi_osi_darwin(1, d);	/* enable */
+	return 0;
+}
 static int __init dmi_enable_osi_linux(const struct dmi_system_id *d)
 {
 	acpi_dmi_osi_linux(1, d);	/* enable */
@@ -331,6 +336,24 @@  static struct dmi_system_id acpi_osi_dmi_table[] __initdata = {
 		},
 	},
 
+	/*
+	 * Enable _OSI("Darwin") for all apple platforms.
+	 */
+	{
+	.callback = dmi_enable_osi_darwin,
+	.ident = "Apple hardware",
+	.matches = {
+		     DMI_MATCH(DMI_SYS_VENDOR, "Apple INC."),
+		},
+	},
+	{
+	.callback = dmi_enable_osi_darwin,
+	.ident = "Apple hardware",
+	.matches = {
+		     DMI_MATCH(DMI_SYS_VENDOR, "Apple Computer, INC."),
+		},
+	},
+
 #ifdef CONFIG_ACPI_REV_OVERRIDE_POSSIBLE
 	/*
 	 * DELL XPS 13 (2015) switches sound between HDA and I2S
diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
index fbedea7..5dd2a79 100644
--- a/drivers/acpi/osl.c
+++ b/drivers/acpi/osl.c
@@ -135,6 +135,9 @@  static struct acpi_osi_config {
 	unsigned int	linux_enable:1;
 	unsigned int	linux_dmi:1;
 	unsigned int	linux_cmdline:1;
+	unsigned int	darwin_enable:1;
+	unsigned int	darwin_dmi:1;
+	unsigned int	darwin_cmdline:1;
 	u8		default_disabling;
 } osi_config = {0, 0, 0, 0};
 
@@ -150,13 +153,12 @@  static u32 acpi_osi_handler(acpi_string interface, u32 supported)
 	}
 
 	if (!strcmp("Darwin", interface)) {
-		/*
-		 * Apple firmware will behave poorly if it receives positive
-		 * answers to "Darwin" and any other OS. Respond positively
-		 * to Darwin and then disable all other vendor strings.
-		 */
-		acpi_update_interfaces(ACPI_DISABLE_ALL_VENDOR_STRINGS);
-		supported = ACPI_UINT32_MAX;
+
+		printk_once(KERN_NOTICE PREFIX
+			"BIOS _OSI(Darwin) query %s%s\n",
+			osi_config.darwin_enable ? "honored" : "ignored",
+			osi_config.darwin_cmdline ? " via cmdline" :
+			osi_config.darwin_dmi ? " via DMI" : "");
 	}
 
 	return supported;
@@ -1509,6 +1511,44 @@  void __init acpi_osi_setup(char *str)
 	}
 }
 
+static void __init set_osi_darwin(unsigned int enable)
+{
+	if (osi_config.darwin_enable != enable)
+		osi_config.darwin_enable = enable;
+
+	if (enable) {
+		acpi_osi_setup("!");
+		acpi_osi_setup("Darwin");
+	} else {
+		acpi_osi_setup("!!");
+		acpi_osi_setup("!Darwin");
+	}
+}
+
+static void __init acpi_cmdline_osi_darwin(unsigned int enable)
+{
+	/* cmdline set the default and override DMI */
+	osi_config.darwin_cmdline = 1;
+	osi_config.darwin_dmi = 0;
+	set_osi_darwin(enable);
+
+	return;
+}
+
+void __init acpi_dmi_osi_darwin(int enable, const struct dmi_system_id *d)
+{
+	printk(KERN_NOTICE PREFIX "DMI detected: %s\n", d->ident);
+
+	if (enable == -1)
+		return;
+
+	/* DMI knows that this box asks OSI(Darwin) */
+	osi_config.darwin_dmi = 1;
+	set_osi_darwin(enable);
+
+	return;
+}
+
 static void __init set_osi_linux(unsigned int enable)
 {
 	if (osi_config.linux_enable != enable)
@@ -1596,6 +1636,10 @@  static int __init osi_setup(char *str)
 		acpi_cmdline_osi_linux(1);
 	else if (str && !strcmp("!Linux", str))
 		acpi_cmdline_osi_linux(0);
+	else if (str && !strcmp("Darwin", str))
+		acpi_cmdline_osi_darwin(1);
+	else if (str && !strcmp("!Darwin", str))
+		acpi_cmdline_osi_darwin(0);
 	else
 		acpi_osi_setup(str);
 
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index be5d206..8fd3a82 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -354,6 +354,7 @@  extern char acpi_video_backlight_string[];
 extern long acpi_is_video_device(acpi_handle handle);
 extern int acpi_blacklisted(void);
 extern void acpi_dmi_osi_linux(int enable, const struct dmi_system_id *d);
+extern void acpi_dmi_osi_darwin(int enable, const struct dmi_system_id *d);
 extern void acpi_osi_setup(char *str);
 extern bool acpi_osi_is_win8(void);