More people have reported they need this for their machines to work
correctly.
https://bugzilla.kernel.org/show_bug.cgi?id=60682
Signed-off-by: Felipe Contreras <[email protected]>
---
drivers/acpi/blacklist.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 48 insertions(+)
diff --git a/drivers/acpi/blacklist.c b/drivers/acpi/blacklist.c
index 9515f18..f37dec5 100644
--- a/drivers/acpi/blacklist.c
+++ b/drivers/acpi/blacklist.c
@@ -297,6 +297,54 @@ static struct dmi_system_id acpi_osi_dmi_table[] __initdata = {
DMI_MATCH(DMI_PRODUCT_VERSION, "3259A2G"),
},
},
+ {
+ .callback = dmi_disable_osi_win8,
+ .ident = "ThinkPad Edge E530",
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
+ DMI_MATCH(DMI_PRODUCT_VERSION, "3259CTO"),
+ },
+ },
+ {
+ .callback = dmi_disable_osi_win8,
+ .ident = "ThinkPad Edge E530",
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
+ DMI_MATCH(DMI_PRODUCT_VERSION, "3259HJG"),
+ },
+ },
+ {
+ .callback = dmi_disable_osi_win8,
+ .ident = "Acer Aspire V5-573G",
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "Acer Aspire"),
+ DMI_MATCH(DMI_PRODUCT_VERSION, "V5-573G/Dazzle_HW"),
+ },
+ },
+ {
+ .callback = dmi_disable_osi_win8,
+ .ident = "Acer Aspire V5-572G",
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "Acer Aspire"),
+ DMI_MATCH(DMI_PRODUCT_VERSION, "V5-572G/Dazzle_CX"),
+ },
+ },
+ {
+ .callback = dmi_disable_osi_win8,
+ .ident = "ThinkPad T431s",
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
+ DMI_MATCH(DMI_PRODUCT_VERSION, "20AACTO1WW"),
+ },
+ },
+ {
+ .callback = dmi_disable_osi_win8,
+ .ident = "ThinkPad T430",
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
+ DMI_MATCH(DMI_PRODUCT_VERSION, "2349D15"),
+ },
+ },
/*
* BIOS invocation of _OSI(Linux) is almost always a BIOS bug.
--
1.8.4-fc
On Thu, Oct 3, 2013 at 12:13 PM, Felipe Contreras
<[email protected]> wrote:
> More people have reported they need this for their machines to work
> correctly.
>
> https://bugzilla.kernel.org/show_bug.cgi?id=60682
Reported-by: Stefan Hellermann <[email protected]>
Reported-by: Benedikt Sauer <[email protected]>
Reported-by: Erno Kuusela <[email protected]>
Reported-by: Jonathan Doman <[email protected]>
Reported-by: Christoph Klaffl <[email protected]>
Reported-by: Jan Hendrik Nielsen <[email protected]>
> Signed-off-by: Felipe Contreras <[email protected]>
--
Felipe Contreras
On Thu, Oct 03, 2013 at 12:13:03PM -0500, Felipe Contreras wrote:
> More people have reported they need this for their machines to work
> correctly.
Can you add a comment to each indicating why they're being added so we
can easily remove them again once the problem is fixed?
--
Matthew Garrett | [email protected]
On Fri, Oct 4, 2013 at 11:12 AM, Matthew Garrett <[email protected]> wrote:
> On Thu, Oct 03, 2013 at 12:13:03PM -0500, Felipe Contreras wrote:
>> More people have reported they need this for their machines to work
>> correctly.
>
> Can you add a comment to each indicating why they're being added so we
> can easily remove them again once the problem is fixed?
Because that's what users report:
https://bugzilla.kernel.org/show_bug.cgi?id=60682
--
Felipe Contreras
On Sun, Oct 06, 2013 at 03:29:10PM -0500, Felipe Contreras wrote:
> On Fri, Oct 4, 2013 at 11:12 AM, Matthew Garrett <[email protected]> wrote:
> > On Thu, Oct 03, 2013 at 12:13:03PM -0500, Felipe Contreras wrote:
> >> More people have reported they need this for their machines to work
> >> correctly.
> >
> > Can you add a comment to each indicating why they're being added so we
> > can easily remove them again once the problem is fixed?
>
> Because that's what users report:
> https://bugzilla.kernel.org/show_bug.cgi?id=60682
Yes. For each entry, add a comment describing the specific bug seen.
--
Matthew Garrett | [email protected]
On Sun, Oct 6, 2013 at 3:33 PM, Matthew Garrett <[email protected]> wrote:
> On Sun, Oct 06, 2013 at 03:29:10PM -0500, Felipe Contreras wrote:
>> On Fri, Oct 4, 2013 at 11:12 AM, Matthew Garrett <[email protected]> wrote:
>> > On Thu, Oct 03, 2013 at 12:13:03PM -0500, Felipe Contreras wrote:
>> >> More people have reported they need this for their machines to work
>> >> correctly.
>> >
>> > Can you add a comment to each indicating why they're being added so we
>> > can easily remove them again once the problem is fixed?
>>
>> Because that's what users report:
>> https://bugzilla.kernel.org/show_bug.cgi?id=60682
>
> Yes. For each entry, add a comment describing the specific bug seen.
In case you didn't hear, the plan was to add an entry if a user
reports the backlight not working correctly, and acpi_osi="!Windows
2012" fixing it with no negative effects. and that's what users did in
bug #60682.
--
Felipe Contreras
On Sun, Oct 06, 2013 at 03:40:42PM -0500, Felipe Contreras wrote:
> In case you didn't hear, the plan was to add an entry if a user
> reports the backlight not working correctly, and acpi_osi="!Windows
> 2012" fixing it with no negative effects. and that's what users did in
> bug #60682.
So add that to each entry you're adding to the list.
--
Matthew Garrett | [email protected]
On Sun, Oct 6, 2013 at 3:45 PM, Matthew Garrett <[email protected]> wrote:
> On Sun, Oct 06, 2013 at 03:40:42PM -0500, Felipe Contreras wrote:
>
>> In case you didn't hear, the plan was to add an entry if a user
>> reports the backlight not working correctly, and acpi_osi="!Windows
>> 2012" fixing it with no negative effects. and that's what users did in
>> bug #60682.
>
> So add that to each entry you're adding to the list.
No. These entries are already there, I'm merely updating it. I added a
comment before the list, but the comment was removed.
--
Felipe Contreras
On Sun, Oct 06, 2013 at 03:51:05PM -0500, Felipe Contreras wrote:
> On Sun, Oct 6, 2013 at 3:45 PM, Matthew Garrett <[email protected]> wrote:
> > On Sun, Oct 06, 2013 at 03:40:42PM -0500, Felipe Contreras wrote:
> >
> >> In case you didn't hear, the plan was to add an entry if a user
> >> reports the backlight not working correctly, and acpi_osi="!Windows
> >> 2012" fixing it with no negative effects. and that's what users did in
> >> bug #60682.
> >
> > So add that to each entry you're adding to the list.
>
> No. These entries are already there, I'm merely updating it. I added a
> comment before the list, but the comment was removed.
There's no point in having a comment that describes a list, because
someone will inevitably reorder them or insert things in between at some
point. Add a comment per entry.
--
Matthew Garrett | [email protected]
On Sun, Oct 6, 2013 at 3:59 PM, Matthew Garrett <[email protected]> wrote:
> On Sun, Oct 06, 2013 at 03:51:05PM -0500, Felipe Contreras wrote:
>> On Sun, Oct 6, 2013 at 3:45 PM, Matthew Garrett <[email protected]> wrote:
>> > On Sun, Oct 06, 2013 at 03:40:42PM -0500, Felipe Contreras wrote:
>> >
>> >> In case you didn't hear, the plan was to add an entry if a user
>> >> reports the backlight not working correctly, and acpi_osi="!Windows
>> >> 2012" fixing it with no negative effects. and that's what users did in
>> >> bug #60682.
>> >
>> > So add that to each entry you're adding to the list.
>>
>> No. These entries are already there, I'm merely updating it. I added a
>> comment before the list, but the comment was removed.
>
> There's no point in having a comment that describes a list, because
> someone will inevitably reorder them or insert things in between at some
> point. Add a comment per entry.
>From acpi_osi_dmi_table:
/*
* BIOS invocation of _OSI(Linux) is almost always a BIOS bug.
* Linux ignores it, except for the machines enumerated below.
*/
--
Felipe Contreras
On Sun, Oct 06, 2013 at 06:27:28PM -0500, Felipe Contreras wrote:
> From acpi_osi_dmi_table:
>
> /*
> * BIOS invocation of _OSI(Linux) is almost always a BIOS bug.
> * Linux ignores it, except for the machines enumerated below.
> */
Which was a mistake. We learn from mistakes rather than repeating them.
--
Matthew Garrett | [email protected]
On Sun, Oct 6, 2013 at 6:31 PM, Matthew Garrett <[email protected]> wrote:
> On Sun, Oct 06, 2013 at 06:27:28PM -0500, Felipe Contreras wrote:
>> From acpi_osi_dmi_table:
>>
>> /*
>> * BIOS invocation of _OSI(Linux) is almost always a BIOS bug.
>> * Linux ignores it, except for the machines enumerated below.
>> */
>
> Which was a mistake. We learn from mistakes rather than repeating them.
According to you.
Sane human beings have no problem with divisions.
/* section 1 */
a();
b();
c();
/* section 2 */
d();
e();
/* section 3 */
f();
--
Felipe Contreras
On Sun, Oct 06, 2013 at 06:36:57PM -0500, Felipe Contreras wrote:
> On Sun, Oct 6, 2013 at 6:31 PM, Matthew Garrett <[email protected]> wrote:
> > On Sun, Oct 06, 2013 at 06:27:28PM -0500, Felipe Contreras wrote:
> >> From acpi_osi_dmi_table:
> >>
> >> /*
> >> * BIOS invocation of _OSI(Linux) is almost always a BIOS bug.
> >> * Linux ignores it, except for the machines enumerated below.
> >> */
> >
> > Which was a mistake. We learn from mistakes rather than repeating them.
>
> According to you.
Cool. Look at that file and, without resorting to git blame, tell me
why each of those entries is there. If your answer is "Just use git
blame", then that's fine up until the point where someone reformats the
list or decides to change the order and now it's still *possible* it's
just really annoying, so why not just add the comments? They're cheap
and you could have done it trivially in the time it's taken you to reply
to this thread.
--
Matthew Garrett | [email protected]
On Sun, Oct 6, 2013 at 6:57 PM, Matthew Garrett <[email protected]> wrote:
> On Sun, Oct 06, 2013 at 06:36:57PM -0500, Felipe Contreras wrote:
>> On Sun, Oct 6, 2013 at 6:31 PM, Matthew Garrett <[email protected]> wrote:
>> > On Sun, Oct 06, 2013 at 06:27:28PM -0500, Felipe Contreras wrote:
>> >> From acpi_osi_dmi_table:
>> >>
>> >> /*
>> >> * BIOS invocation of _OSI(Linux) is almost always a BIOS bug.
>> >> * Linux ignores it, except for the machines enumerated below.
>> >> */
>> >
>> > Which was a mistake. We learn from mistakes rather than repeating them.
>>
>> According to you.
>
> Cool. Look at that file and, without resorting to git blame, tell me
> why each of those entries is there.
If my original comment was kept, I could tell you why the three
entries I added were there.
---
/*
* The following machines have broken backlight support when reporting
* the Windows 2012 OSI, so disable it until their support is fixed.
*/
{
.callback = dmi_disable_osi_win8,
.ident = "ASUS Zenbook Prime UX31A",
.matches = {
DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
DMI_MATCH(DMI_PRODUCT_NAME, "UX31A"),
},
},
{
.callback = dmi_disable_osi_win8,
.ident = "Dell Inspiron 15R SE",
.matches = {
DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
DMI_MATCH(DMI_PRODUCT_NAME, "Inspiron 7520"),
},
},
{
.callback = dmi_disable_osi_win8,
.ident = "Lenovo ThinkPad Edge E530",
.matches = {
DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
DMI_MATCH(DMI_PRODUCT_VERSION, "3259A2G"),
},
},
/*
* BIOS invocation of _OSI(Linux) is almost always a BIOS bug.
* Linux ignores it, except for the machines enumerated below.
*/
---
It is clear as day.
> If your answer is "Just use git
> blame", then that's fine up until the point where someone reformats the
> list or decides to change the order and now it's still *possible* it's
> just really annoying
That's not my answer.
> so why not just add the comments? They're cheap
> and you could have done it trivially in the time it's taken you to reply
> to this thread.
Because it's the wrong thing to do. Adding four lines of comments for
each one of the nine entries is a waste of code, it's completely
unnecessary, and doesn't bring any advantage that a single comment, on
top of the list doesn't.
This patch brings real benefits to real users, and does so without
introducing any problem to the format of this list that wasn't already
there. There is no reason not to apply it.
If _you_ want to add comments for each entry in the list you can do so
after this patch is applied.
--
Felipe Contreras
On Sun, Oct 06, 2013 at 07:27:48PM -0500, Felipe Contreras wrote:
> If _you_ want to add comments for each entry in the list you can do so
> after this patch is applied.
If you want to participate in a collaborative development effort you
should pay attention to other people's concerns. I don't get the final
say in whether or not this patch gets merged, but there's a decent
chance that I'm going to be the one who has to remove the entries again
once the backlight mess is fixed up. My life would be significantly
easier if the entries are unambiguously identified in such a way that I
can remove them without having to dig through git history to figure out
where each came from. Is that really an unreasonable request?
--
Matthew Garrett | [email protected]
On Sun, Oct 6, 2013 at 7:32 PM, Matthew Garrett <[email protected]> wrote:
> On Sun, Oct 06, 2013 at 07:27:48PM -0500, Felipe Contreras wrote:
>
>> If _you_ want to add comments for each entry in the list you can do so
>> after this patch is applied.
>
> If you want to participate in a collaborative development effort you
> should pay attention to other people's concerns.
I did that when I listened to your comment, and I argued against it.
Disagreeing is not the same as not paying attention.
> I don't get the final
> say in whether or not this patch gets merged, but there's a decent
> chance that I'm going to be the one who has to remove the entries again
> once the backlight mess is fixed up. My life would be significantly
> easier if the entries are unambiguously identified in such a way that I
> can remove them without having to dig through git history to figure out
> where each came from.
And a *single* comment on top of this group entries achieves that just
fine. You haven't provided a single argument as to why that wouldn't
be the case.
In fact, you are the one that is not paying attention.
> Is that really an unreasonable request?
That wasn't a request, that was an explanation of what would make your
life easier.
And if uncommented entries is a problem for you, you already have that
problem, because the entries to remove are already there, uncommented.
The original patch I sent had a comment, so that's not my fault.
This patch would not make your life any harder, so that is a red
herring. The problem is already there.
--
Felipe Contreras
On Sun, Oct 06, 2013 at 07:50:18PM -0500, Felipe Contreras wrote:
> On Sun, Oct 6, 2013 at 7:32 PM, Matthew Garrett <[email protected]> wrote:
> > I don't get the final
> > say in whether or not this patch gets merged, but there's a decent
> > chance that I'm going to be the one who has to remove the entries again
> > once the backlight mess is fixed up. My life would be significantly
> > easier if the entries are unambiguously identified in such a way that I
> > can remove them without having to dig through git history to figure out
> > where each came from.
>
> And a *single* comment on top of this group entries achieves that just
> fine. You haven't provided a single argument as to why that wouldn't
> be the case.
No, it demonstrably doesn't. The comments that do exist refer to only a
subset of the entries underneath them. Having a per-entry comment is
significantly clearer. Given that I have to delete things from this file
and you don't, I have absolutely no idea why you refuse to believe me on
this.
--
Matthew Garrett | [email protected]
On Sun, Oct 6, 2013 at 7:53 PM, Matthew Garrett <[email protected]> wrote:
> On Sun, Oct 06, 2013 at 07:50:18PM -0500, Felipe Contreras wrote:
>> On Sun, Oct 6, 2013 at 7:32 PM, Matthew Garrett <[email protected]> wrote:
>> > I don't get the final
>> > say in whether or not this patch gets merged, but there's a decent
>> > chance that I'm going to be the one who has to remove the entries again
>> > once the backlight mess is fixed up. My life would be significantly
>> > easier if the entries are unambiguously identified in such a way that I
>> > can remove them without having to dig through git history to figure out
>> > where each came from.
>>
>> And a *single* comment on top of this group entries achieves that just
>> fine. You haven't provided a single argument as to why that wouldn't
>> be the case.
>
> No, it demonstrably doesn't. The comments that do exist refer to only a
> subset of the entries underneath them.
That's not true.
/*
* BIOS invocation of _OSI(Linux) is almost always a BIOS bug.
* Linux ignores it, except for the machines enumerated below.
*/
> Having a per-entry comment is significantly clearer.
That is your opinion, it's not a demonstrable fact.
And just to be clear, you are saying that in the following code, you
have no idea which statements correspond to which sections. Am I
correct?
/* section 1 */
a();
b();
c();
/* section 2 */
d();
e();
/* section 3 */
f();
And once again, the problem with the **current** format of the list is
orthogonal to this patch.
--
Felipe Contreras
On Sun, Oct 06, 2013 at 08:01:34PM -0500, Felipe Contreras wrote:
> On Sun, Oct 6, 2013 at 7:53 PM, Matthew Garrett <[email protected]> wrote:
> > No, it demonstrably doesn't. The comments that do exist refer to only a
> > subset of the entries underneath them.
>
> That's not true.
>
> /*
> * BIOS invocation of _OSI(Linux) is almost always a BIOS bug.
> * Linux ignores it, except for the machines enumerated below.
> */
You appear to have missed the continuation of that comment directly
underneath which lists a subset of the devices covered by the quirks.
> > Having a per-entry comment is significantly clearer.
>
> That is your opinion, it's not a demonstrable fact.
Say one of the machines turns out to need the quirk for two different
reasons. How do we document that? Look, how about you add the comments
and I'll do a patch that adds documentation to the existing entries? I'm
not asking you to make up for other people's past mistakes, I'm asking
you not to perpetuate them.
> And just to be clear, you are saying that in the following code, you
> have no idea which statements correspond to which sections. Am I
> correct?
No, that's not what I'm saying. But I'm now going to a bar and drink
instead of having to justify why *clearly documenting this code* is a
worthwhile thing to do.
--
Matthew Garrett | [email protected]
On Mon, Oct 07, 2013 at 02:27:04AM +0100, Matthew Garrett wrote:
> > > Having a per-entry comment is significantly clearer.
> >
> > That is your opinion, it's not a demonstrable fact.
>
> Say one of the machines turns out to need the quirk for two different
> reasons. How do we document that? Look, how about you add the comments
> and I'll do a patch that adds documentation to the existing entries? I'm
> not asking you to make up for other people's past mistakes, I'm asking
> you not to perpetuate them.
Felipe,
I have to agree with Matthew here. Lists have a way of getting messed
up. If not in the upstream kernel, can we be sure that none of the
distribution maintainers might not respect the ordering?
How about doing something like this:
/*
* [1] Busted brightness controls
* [2] Attempted compatibility with ancient enterprise Linux kernel causes
* 20% performance regression on upstream kernels
* [3] Disables video card functionaity to be bug-for-bug compatible with
* Windows after attempted hobbling in the propietary driver
* was wored around, etc.
* etc.
*/
Then individual entries can be annotated with comments indicating
[1][2], etc.
That way, if someone clever decides that they want to alphabetize the
entries, or we have so many exceptions due to incompetent BIOS
programmers, and some future developers decides that he or she needs
to implement a binary search to speedup lookups, or some such, we
won't need to worry about ordering-specific semantics getting smashed.
Cheers,
- Ted
On Sun, Oct 6, 2013 at 8:27 PM, Matthew Garrett <[email protected]> wrote:
> On Sun, Oct 06, 2013 at 08:01:34PM -0500, Felipe Contreras wrote:
>> On Sun, Oct 6, 2013 at 7:53 PM, Matthew Garrett <[email protected]> wrote:
>> > No, it demonstrably doesn't. The comments that do exist refer to only a
>> > subset of the entries underneath them.
>>
>> That's not true.
>>
>> /*
>> * BIOS invocation of _OSI(Linux) is almost always a BIOS bug.
>> * Linux ignores it, except for the machines enumerated below.
>> */
>
> You appear to have missed the continuation of that comment directly
> underneath which lists a subset of the devices covered by the quirks.
What of it? The comment I'm referring to applies to *ALL* the entries
below, not a subset of them. All the entries below use
dmi_enable_osi_linux().
>> > Having a per-entry comment is significantly clearer.
>>
>> That is your opinion, it's not a demonstrable fact.
>
> Say one of the machines turns out to need the quirk for two different
> reasons. How do we document that?
/* 0) The following... disable Windows 2012 OSI */
a
b
/* 1) This particular... whatever */
c
d
/* 2) The following... enable OSI Linux */
Is it not clear that the comment 1) applies only to c? If it's not
clear for you we can reorder:
/* 0) The following... disable Windows 2012 OSI */
a
b
d
/* 1) This particular... whatever */
c
/* 2) The following... enable OSI Linux */
> Look, how about you add the comments
> and I'll do a patch that adds documentation to the existing entries? I'm
> not asking you to make up for other people's past mistakes, I'm asking
> you not to perpetuate them.
I will consider that *after* your patch lands. In the meantime I still
maintain that a single comment is better, and I think my patch should
land instead:
http://article.gmane.org/gmane.linux.acpi.devel/64243
>> And just to be clear, you are saying that in the following code, you
>> have no idea which statements correspond to which sections. Am I
>> correct?
>
> No, that's not what I'm saying. But I'm now going to a bar and drink
> instead of having to justify why *clearly documenting this code* is a
> worthwhile thing to do.
This is a rhetorical trick, by "clearly documenting this code" you
actually mean "format it in exactly the way I want". My way of
documenting this code[1] is also clear.
Ultimately it doesn't matter, because the fixes for the Intel driver
are supposed to come soon, and this blacklist should be short-lived,
thus this list is not going to be reordered, moved, or will have the
need for secondary comments.
Look, how about you set aside your objection to this patch so it can
go forward and fix real issues for real users, and deal with the
comments that are already missing anyway later?
[1] http://article.gmane.org/gmane.linux.acpi.devel/64243
--
Felipe Contreras
On Sun, Oct 6, 2013 at 8:54 PM, Theodore Ts'o <[email protected]> wrote:
> On Mon, Oct 07, 2013 at 02:27:04AM +0100, Matthew Garrett wrote:
>> > > Having a per-entry comment is significantly clearer.
>> >
>> > That is your opinion, it's not a demonstrable fact.
>>
>> Say one of the machines turns out to need the quirk for two different
>> reasons. How do we document that? Look, how about you add the comments
>> and I'll do a patch that adds documentation to the existing entries? I'm
>> not asking you to make up for other people's past mistakes, I'm asking
>> you not to perpetuate them.
>
> Felipe,
>
> I have to agree with Matthew here. Lists have a way of getting messed
> up. If not in the upstream kernel, can we be sure that none of the
> distribution maintainers might not respect the ordering?
That would be a problem for the distribution maintainers, wouldn't it?
And regardless of how we document the list, they can still mess it up.
> How about doing something like this:
>
> /*
> * [1] Busted brightness controls
> * [2] Attempted compatibility with ancient enterprise Linux kernel causes
> * 20% performance regression on upstream kernels
> * [3] Disables video card functionaity to be bug-for-bug compatible with
> * Windows after attempted hobbling in the propietary driver
> * was wored around, etc.
> * etc.
> */
>
> Then individual entries can be annotated with comments indicating
> [1][2], etc.
That would be better than Matthew's proposal, but it would make the
code less readable, for the same reason spaghetti code is not readable
(you have to jump back and forth to understand what's going on).
> That way, if someone clever decides that they want to alphabetize the
> entries, or we have so many exceptions due to incompetent BIOS
> programmers, and some future developers decides that he or she needs
> to implement a binary search to speedup lookups, or some such, we
> won't need to worry about ordering-specific semantics getting smashed.
How about we worry about hypothetical issues when they arise? (which
is probably going to be never).
Personally I think this is more than enough:
http://article.gmane.org/gmane.linux.acpi.devel/64243
--
Felipe Contreras
On Thu, Oct 3, 2013 at 12:13 PM, Felipe Contreras
<[email protected]> wrote:
> More people have reported they need this for their machines to work
> correctly.
>
> https://bugzilla.kernel.org/show_bug.cgi?id=60682
I see this was merged to the linux-next branch. Is there any reason
why it's not proposed for v3.12?
--
Felipe Contreras
On Sunday, October 13, 2013 10:30:50 PM Felipe Contreras wrote:
> On Thu, Oct 3, 2013 at 12:13 PM, Felipe Contreras
> <[email protected]> wrote:
> > More people have reported they need this for their machines to work
> > correctly.
> >
> > https://bugzilla.kernel.org/show_bug.cgi?id=60682
>
> I see this was merged to the linux-next branch. Is there any reason
> why it's not proposed for v3.12?
It is not a fix for a recent breakage, it doesn't fix an outright crash or
security issue and there is a chance, albeit arguably very small, that
something will not function on one of the affected systems for someone
as a result of the blacklisting.
Thanks!
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
On Mon, Oct 14, 2013 at 6:58 AM, Rafael J. Wysocki <[email protected]> wrote:
> On Sunday, October 13, 2013 10:30:50 PM Felipe Contreras wrote:
>> On Thu, Oct 3, 2013 at 12:13 PM, Felipe Contreras
>> <[email protected]> wrote:
>> > More people have reported they need this for their machines to work
>> > correctly.
>> >
>> > https://bugzilla.kernel.org/show_bug.cgi?id=60682
>>
>> I see this was merged to the linux-next branch. Is there any reason
>> why it's not proposed for v3.12?
>
> It is not a fix for a recent breakage, it doesn't fix an outright crash or
> security issue and there is a chance, albeit arguably very small, that
> something will not function on one of the affected systems for someone
> as a result of the blacklisting.
So? That chance is already there because the blacklist is already
there, but we cannot know until v3.12 is released, because that's when
most people would be testing that code.
It seems to me your worry is that somebody would report a regression
that happened v3.12-rc6 (if it has this patch applied), while it
worked correctly in v3.12-rc5, in which case you would get a complaint
from Linus.
But we all know that's not going to happen. The complaints (if any)
would arrive in v3.12, and that would happen regardless of this patch.
Besides, our goal is to make v3.12 as stable as possible, and it seems
to me you are blindly following the letter of a rule, instead if
interpreting what is the intent of rule. If I had sent the patch the
29th of June, it would have been aplied by now and be part of the
current blacklist, and the result would be the same; we cannot really
know until v3.12 is released.
--
Felipe Contreras
On Monday, October 14, 2013 08:14:28 AM Felipe Contreras wrote:
> On Mon, Oct 14, 2013 at 6:58 AM, Rafael J. Wysocki <[email protected]> wrote:
> > On Sunday, October 13, 2013 10:30:50 PM Felipe Contreras wrote:
> >> On Thu, Oct 3, 2013 at 12:13 PM, Felipe Contreras
> >> <[email protected]> wrote:
> >> > More people have reported they need this for their machines to work
> >> > correctly.
> >> >
> >> > https://bugzilla.kernel.org/show_bug.cgi?id=60682
> >>
> >> I see this was merged to the linux-next branch. Is there any reason
> >> why it's not proposed for v3.12?
> >
> > It is not a fix for a recent breakage, it doesn't fix an outright crash or
> > security issue and there is a chance, albeit arguably very small, that
> > something will not function on one of the affected systems for someone
> > as a result of the blacklisting.
>
> So? That chance is already there because the blacklist is already
> there, but we cannot know until v3.12 is released, because that's when
> most people would be testing that code.
>
> It seems to me your worry is that somebody would report a regression
> that happened v3.12-rc6 (if it has this patch applied), while it
> worked correctly in v3.12-rc5, in which case you would get a complaint
> from Linus.
>
> But we all know that's not going to happen. The complaints (if any)
> would arrive in v3.12, and that would happen regardless of this patch.
>
> Besides, our goal is to make v3.12 as stable as possible, and it seems
> to me you are blindly following the letter of a rule, instead if
> interpreting what is the intent of rule. If I had sent the patch the
> 29th of June, it would have been aplied by now and be part of the
> current blacklist, and the result would be the same; we cannot really
> know until v3.12 is released.
Oh, please give me a break.
I've applied your patch even though I really should have waited for an ACK from
Matthew before doing that, but that only made you *more* demanding.
I generally tend to be nice to people, but you evidently take that as a sign of
weakness, so trying to be nice to you doesn't really make sense to me. On the
other hand, I'm not too good at throwing flames at people and it distracts me
so much that I can't really afford it. Please find yourself someone else to
fight with.
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
On Mon, Oct 14, 2013 at 9:16 AM, Rafael J. Wysocki <[email protected]> wrote:
> On Monday, October 14, 2013 08:14:28 AM Felipe Contreras wrote:
>> On Mon, Oct 14, 2013 at 6:58 AM, Rafael J. Wysocki <[email protected]> wrote:
>> > On Sunday, October 13, 2013 10:30:50 PM Felipe Contreras wrote:
>> >> On Thu, Oct 3, 2013 at 12:13 PM, Felipe Contreras
>> >> <[email protected]> wrote:
>> >> > More people have reported they need this for their machines to work
>> >> > correctly.
>> >> >
>> >> > https://bugzilla.kernel.org/show_bug.cgi?id=60682
>> >>
>> >> I see this was merged to the linux-next branch. Is there any reason
>> >> why it's not proposed for v3.12?
>> >
>> > It is not a fix for a recent breakage, it doesn't fix an outright crash or
>> > security issue and there is a chance, albeit arguably very small, that
>> > something will not function on one of the affected systems for someone
>> > as a result of the blacklisting.
>>
>> So? That chance is already there because the blacklist is already
>> there, but we cannot know until v3.12 is released, because that's when
>> most people would be testing that code.
>>
>> It seems to me your worry is that somebody would report a regression
>> that happened v3.12-rc6 (if it has this patch applied), while it
>> worked correctly in v3.12-rc5, in which case you would get a complaint
>> from Linus.
>>
>> But we all know that's not going to happen. The complaints (if any)
>> would arrive in v3.12, and that would happen regardless of this patch.
>>
>> Besides, our goal is to make v3.12 as stable as possible, and it seems
>> to me you are blindly following the letter of a rule, instead if
>> interpreting what is the intent of rule. If I had sent the patch the
>> 29th of June, it would have been aplied by now and be part of the
>> current blacklist, and the result would be the same; we cannot really
>> know until v3.12 is released.
>
> Oh, please give me a break.
>
> I've applied your patch even though I really should have waited for an ACK from
> Matthew before doing that, but that only made you *more* demanding.
I'm not demanding anything, I'm making a case.
> I generally tend to be nice to people, but you evidently take that as a sign of
> weakness, so trying to be nice to you doesn't really make sense to me. On the
> other hand, I'm not too good at throwing flames at people and it distracts me
> so much that I can't really afford it. Please find yourself someone else to
> fight with.
I don't care if you are nice to me, be nice, be aggressive, I do not care.
All I care is that we do what is best here, and since many people's
back-light has been broken for so many releases and we have a very
simple fix that we know it works, because v3.6 worked on these laptops
(and all the previous releases), we *know* this patch cannot make
things worst than v3.6, and we *know* we broke things since v3.7.
If you don't want to fix the backlight in these laptops for v3.12,
fine, but don't blame it on my attitude, your users don't care about
my attitude, your users only care that Linux works correctly, and as
things stand v3.12 will *not* work correctly for these laptops, even
though we know how to fix it, and the fix is simple, and cannot make
things worst than v3.6.
We are going to fix this regression only on certain laptops, but not
others, even though we already know the fix works for those too. I
don't see how that makes sense to you, but it's your call.
--
Felipe Contreras
On Fri, Oct 18, 2013 at 5:43 AM, Stefan Hellermann
<[email protected]> wrote:
> 2013/10/3 Felipe Contreras <[email protected]>
>>
>> On Thu, Oct 3, 2013 at 12:13 PM, Felipe Contreras
>> <[email protected]> wrote:
>> > More people have reported they need this for their machines to work
>> > correctly.
>> >
>> > https://bugzilla.kernel.org/show_bug.cgi?id=60682
>>
>> Reported-by: Stefan Hellermann <[email protected]>
>
>
> That's not entirely correct! I reported bios version 2.52 is affected, but
> 2.54 is fixed. I think we should match all Lenovo 3259*, but only bios
> version < 2.54
As far as I know there's no way to do a match like that. So we have to
choose, either we apply the blacklist for all, or for none. Since
removing the win8 OSI doesn't cause any negative effects in 2.54, I
say we would do the former.
--
Felipe Contreras