Re: acer-wmi auto-loaded on a Lenovo Thinkpad X1 Carbon 4th gen

["Bjørn Mork" <bjorn@mork.no>]

joeyli <jlee@suse.com> writes:

> Hi Bjørn, 
>
> Sorry for my delay because I stuck on other issues.

No, problem. The problem is an old one, so there is no hurry :)

And it's always best to take the time necessary to work out a good long
term solution.

> I have a question about removed 67C3371D-95A3-4C37-BB61-DD47B491DAAB. It
> already in acer-wmi a long time since Carlos contributed this driver in
> 2008. 
>
> Does it possible cause regression on some laptops if we removed this GUID
> support in acer-wmi? 

Yes, I guess the risk of that is high.  It might be unacceptably high.
That's for you to decide...

I just wanted to point out that this WMI match seems to be based on
false assumptions initially, and that the current blacklisting strategy
is a game you *will* lose. Or have already lost... Only a small fraction
of the false positives will ever be reported.

The negative match on the "norfkill_ids" list of ACPI IDs from a number
of different vendors for example, will of course make the driver work
with any Acer IDs.  But why not list those instead, doing a positive
match on the systems you want to support? So much easier, and actually
possible to complete.  The problem, of course, is that you don't have a
record of the IDs you want to match...  But collecting those should be
possible if you start making the driver report them. It is most likely
just a single one?

IMHO, for a driver like this, "working" on the target system is not
enough.  You need to ensure that it doesn't negatively affect other
systems. That's impossible when you do negative matching. I don't think
ACPI or WMI drivers should ever have to run a probe on any system they
don't support. Either you have a matching ID, which can be coded into
the module alias table, or you don't.  Or am I missing something?

Anyway, fixing the acer_wmi_get_handle() matching logic in
acer_wmi_accel_setup() should not cause any regressions, since that one
seems to be based on positive matching of the "SENR" method of the
"BST0001" device".  There's just a minor flaw in the ACPI lookup logic.


Bjørn