Re: [PATCH] acpi: update win8 OSI blacklist

[Felipe Contreras]

On Sun, Oct 6, 2013 at 6:57 PM, Matthew Garrett <mjg59@xxxxxxxxxxxxx> 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 <mjg59@xxxxxxxxxxxxx> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/